BurningFlipside /
CommonCode
Complex classes like SQLAuthenticator often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use SQLAuthenticator, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 71 | class SQLAuthenticator extends Authenticator |
||
| 72 | { |
||
| 73 | public $dataSet = null; |
||
| 74 | public $pendingDataSet = null; |
||
| 75 | private $dataTables = array(); |
||
| 76 | private $params; |
||
| 77 | |||
| 78 | public function __construct($params) |
||
| 79 | { |
||
| 80 | parent::__construct($params); |
||
| 81 | $this->params = $params; |
||
| 82 | if($this->current) |
||
| 83 | { |
||
| 84 | $this->dataSet = $this->getCurrentDataSet(); |
||
| 85 | } |
||
| 86 | if($this->pending) |
||
| 87 | { |
||
| 88 | $this->pendingDataSet = $this->getPendingDataSet(); |
||
| 89 | } |
||
| 90 | } |
||
| 91 | |||
| 92 | /** |
||
| 93 | * @SuppressWarnings("StaticAccess") |
||
| 94 | */ |
||
| 95 | private function getCurrentDataSet() |
||
| 96 | { |
||
| 97 | if(isset($this->params['current_data_set'])) |
||
| 98 | { |
||
| 99 | return \DataSetFactory::getDataSetByName($this->params['current_data_set']); |
||
| 100 | } |
||
| 101 | return \DataSetFactory::getDataSetByName('authentication'); |
||
| 102 | } |
||
| 103 | |||
| 104 | /** |
||
| 105 | * @SuppressWarnings("StaticAccess") |
||
| 106 | */ |
||
| 107 | private function getPendingDataSet() |
||
| 108 | { |
||
| 109 | if(isset($this->params['pending_data_set'])) |
||
| 110 | { |
||
| 111 | return \DataSetFactory::getDataSetByName($this->params['pending_data_set']); |
||
| 112 | } |
||
| 113 | return \DataSetFactory::getDataSetByName('pending_authentication'); |
||
| 114 | } |
||
| 115 | |||
| 116 | private function getDataTable($name, $pending = false) |
||
| 117 | { |
||
| 118 | if(isset($this->dataTables[$name]) && isset($this->dataTables[$name][$pending])) |
||
| 119 | { |
||
| 120 | return $this->dataTables[$name][$pending]; |
||
| 121 | } |
||
| 122 | $dataSet = $this->dataSet; |
||
| 123 | if($pending) |
||
| 124 | { |
||
| 125 | $dataSet = $this->pendingDataSet; |
||
| 126 | } |
||
| 127 | if($dataSet === null) |
||
| 128 | { |
||
| 129 | throw new \Exception('Unable to obtain dataset for SQL Authentication!'); |
||
| 130 | } |
||
| 131 | $dataTable = $dataSet[$name]; |
||
| 132 | if(!isset($this->dataTables[$name])) |
||
| 133 | { |
||
| 134 | $this->dataTables[$name] = array(); |
||
| 135 | } |
||
| 136 | $this->dataTables[$name][$pending] = $dataTable; |
||
| 137 | return $dataTable; |
||
| 138 | } |
||
| 139 | |||
| 140 | /** |
||
| 141 | * Get the data table for Pending Users |
||
| 142 | * |
||
| 143 | * @return boolean|\Data\DataTable The Pending User Data Table |
||
| 144 | */ |
||
| 145 | private function getPendingUserDataTable() |
||
| 153 | |||
| 154 | public function login($username, $password) |
||
| 155 | { |
||
| 156 | if($this->current === false) |
||
| 157 | { |
||
| 158 | return false; |
||
| 159 | } |
||
| 160 | $userDataTable = $this->getDataTable('user'); |
||
| 161 | $filter = new \Data\Filter("uid eq '$username'"); |
||
| 162 | $users = $userDataTable->read($filter); |
||
| 163 | if($users === false || !isset($users[0])) |
||
| 164 | { |
||
| 165 | return false; |
||
| 166 | } |
||
| 167 | if(password_verify($password, $users[0]['pass'])) |
||
| 168 | { |
||
| 169 | return array('res'=>true, 'extended'=>$users[0]); |
||
| 170 | } |
||
| 171 | return false; |
||
| 172 | } |
||
| 173 | |||
| 174 | public function isLoggedIn($data) |
||
| 182 | |||
| 183 | public function getUser($data) |
||
| 184 | { |
||
| 185 | if(isset($this->params['current_data_set'])) |
||
| 186 | { |
||
| 187 | $data['current_data_set'] = $this->params['current_data_set']; |
||
| 188 | } |
||
| 189 | return new SQLUser($data, $this); |
||
|
|
|||
| 190 | } |
||
| 191 | |||
| 192 | private function getEntityByFilter($tableName, $filterStr, $className) |
||
| 203 | |||
| 204 | public function getGroupByName($name) |
||
| 205 | { |
||
| 206 | return $this->getEntityByFilter('group', "gid eq '$name'", '\Auth\SQLGroup'); |
||
| 207 | } |
||
| 208 | |||
| 209 | public function getUserByName($name) |
||
| 210 | { |
||
| 211 | return $this->getEntityByFilter('user', "uid eq '$name'", '\Auth\SQLUser'); |
||
| 212 | } |
||
| 213 | |||
| 214 | private function getDataByFilter($dataTableName, $filter, $select, $top, $skip, $orderby) |
||
| 219 | |||
| 220 | /** |
||
| 221 | * @param string $dataTableName The Data Table to serach |
||
| 222 | * @param string $className The class to obtain data in |
||
| 223 | * @param boolean|array $select The fields to read |
||
| 224 | * @param boolean|integer $top The number of entities to read |
||
| 225 | * @param boolean|integer $skip The number of entities to skip |
||
| 226 | * @param boolean|array $orderby The fields to sort by |
||
| 227 | */ |
||
| 228 | private function convertDataToClass($dataTableName, $className, $filter, $select, $top, $skip, $orderby) |
||
| 229 | { |
||
| 230 | $data = $this->getDataByFilter($dataTableName, $filter, $select, $top, $skip, $orderby); |
||
| 231 | if($data === false) |
||
| 232 | { |
||
| 233 | return false; |
||
| 234 | } |
||
| 235 | $count = count($data); |
||
| 236 | for($i = 0; $i < $count; $i++) |
||
| 237 | { |
||
| 238 | $data[$i] = new $className($groups[$i], $this); |
||
| 239 | } |
||
| 240 | return $data; |
||
| 241 | } |
||
| 242 | |||
| 243 | /** |
||
| 244 | * @param boolean|array $select The fields to read |
||
| 245 | * @param boolean|integer $top The number of entities to read |
||
| 246 | * @param boolean|integer $skip The number of entities to skip |
||
| 247 | * @param boolean|array $orderby The fields to sort by |
||
| 248 | */ |
||
| 249 | public function getGroupsByFilter($filter, $select = false, $top = false, $skip = false, $orderby = false) |
||
| 250 | { |
||
| 251 | return $this->convertDataToClass('group', 'SQLGroup', $filter, $select, $top, $skip, $orderby); |
||
| 252 | } |
||
| 253 | |||
| 254 | /** |
||
| 255 | * @param boolean|array $select The fields to read |
||
| 256 | * @param boolean|integer $top The number of entities to read |
||
| 257 | * @param boolean|integer $skip The number of entities to skip |
||
| 258 | * @param boolean|array $orderby The fields to sort by |
||
| 259 | */ |
||
| 260 | public function getUsersByFilter($filter, $select = false, $top = false, $skip = false, $orderby = false) |
||
| 261 | { |
||
| 262 | return $this->convertDataToClass('group', 'SQLUser', $filter, $select, $top, $skip, $orderby); |
||
| 263 | } |
||
| 264 | |||
| 265 | public function getPendingUserCount() |
||
| 266 | { |
||
| 267 | if($this->pending === false) |
||
| 268 | { |
||
| 269 | return 0; |
||
| 270 | } |
||
| 271 | $dataTable = $this->getPendingUserDataTable(); |
||
| 272 | if($dataTable === null) |
||
| 273 | { |
||
| 274 | return 0; |
||
| 275 | } |
||
| 276 | return $dataTable->count(); |
||
| 277 | } |
||
| 278 | |||
| 279 | private function searchPendingUsers($filter, $select, $top, $skip, $orderby) |
||
| 280 | { |
||
| 281 | $userDataTable = $this->getPendingUserDataTable(); |
||
| 282 | $fieldData = $filter->to_mongo_filter(); |
||
| 283 | $firstFilter = new \Data\Filter('substringof(data,"'.implode($fieldData, ' ').'")'); |
||
| 284 | $users = $userDataTable->read($firstFilter, $select, $top, $skip, $orderby); |
||
| 285 | if($users === false) |
||
| 286 | { |
||
| 287 | return false; |
||
| 288 | } |
||
| 289 | $ret = array(); |
||
| 290 | $count = count($users); |
||
| 291 | for($i = 0; $i < $count; $i++) |
||
| 292 | { |
||
| 293 | $user = new SQLPendingUser($users[$i], $userDataTable); |
||
| 294 | $err = false; |
||
| 295 | foreach($fieldData as $field=>$data) |
||
| 296 | { |
||
| 297 | if(strcasecmp($user[$field], $data) !== 0) |
||
| 298 | { |
||
| 299 | $err = true; break; |
||
| 300 | } |
||
| 301 | } |
||
| 302 | if(!$err) |
||
| 303 | { |
||
| 304 | array_push($ret, $user); |
||
| 305 | } |
||
| 306 | } |
||
| 307 | return $ret; |
||
| 308 | } |
||
| 309 | |||
| 310 | /** |
||
| 311 | * @param \Data\Filter $filter The filter to read with |
||
| 312 | * @param boolean|array $select The fields to read |
||
| 313 | * @param boolean|integer $top The number of entities to read |
||
| 314 | * @param boolean|integer $skip The number of entities to skip |
||
| 315 | * @param boolean|array $orderby The fields to sort by |
||
| 316 | */ |
||
| 317 | public function getPendingUsersByFilter($filter, $select = false, $top = false, $skip = false, $orderby = false) |
||
| 318 | { |
||
| 319 | if($this->pending === false) |
||
| 320 | { |
||
| 321 | return false; |
||
| 322 | } |
||
| 323 | if($filter !== false && !$filter->contains('hash')) |
||
| 324 | { |
||
| 325 | return $this->searchPendingUsers($filter, $select, $top, $skip, $orderby); |
||
| 326 | } |
||
| 327 | $userDataTable = $this->getPendingUserDataTable(); |
||
| 328 | $users = $userDataTable->read($filter, $select, $top, $skip, $orderby); |
||
| 329 | if($users === false) |
||
| 330 | { |
||
| 331 | return false; |
||
| 332 | } |
||
| 333 | $count = count($users); |
||
| 334 | for($i = 0; $i < $count; $i++) |
||
| 335 | { |
||
| 336 | $users[$i] = new SQLPendingUser($users[$i], $userDataTable); |
||
| 337 | } |
||
| 338 | return $users; |
||
| 339 | } |
||
| 340 | |||
| 341 | public function activatePendingUser($user) |
||
| 342 | { |
||
| 343 | if($this->pending === false) |
||
| 344 | { |
||
| 345 | return false; |
||
| 346 | } |
||
| 347 | $userDataTable = $this->getPendingUserDataTable(); |
||
| 348 | if(isset($user->password2)) |
||
| 349 | { |
||
| 350 | unset($user->password2); |
||
| 351 | } |
||
| 352 | $json = json_encode($user); |
||
| 353 | $hash = hash('sha512', $json); |
||
| 354 | $array = array('hash'=>$hash, 'data'=>$json); |
||
| 355 | $ret = $userDataTable->create($array); |
||
| 356 | if($ret !== false) |
||
| 357 | { |
||
| 358 | $users = $this->getPendingUsersByFilter(new \Data\Filter("hash eq '$hash'")); |
||
| 359 | if($users === false || !isset($users[0])) |
||
| 360 | { |
||
| 361 | throw new \Exception('Error retreiving user object after successful create!'); |
||
| 362 | } |
||
| 363 | $users[0]->sendEmail(); |
||
| 364 | } |
||
| 365 | return $ret; |
||
| 366 | } |
||
| 367 | |||
| 368 | public function getTempUserByHash($hash) |
||
| 377 | } |
||
| 378 | /* vim: set tabstop=4 shiftwidth=4 expandtab: */ |
||
| 379 |
Loading history...
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: