Complex classes like ThreemaGateway_Handler_Action_TfaCallback_Abstract 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 ThreemaGateway_Handler_Action_TfaCallback_Abstract, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 11 | abstract class ThreemaGateway_Handler_Action_TfaCallback_Abstract extends ThreemaGateway_Handler_Action_Abstract |
||
| 12 | { |
||
| 13 | /** |
||
| 14 | * @var array Cache of models |
||
| 15 | */ |
||
| 16 | protected $modelCache = []; |
||
| 17 | |||
| 18 | /** |
||
| 19 | * @var XenForo_Session|null Imitiated session |
||
| 20 | */ |
||
| 21 | protected $session = null; |
||
| 22 | |||
| 23 | /** |
||
| 24 | * @var array user data cache |
||
| 25 | */ |
||
| 26 | protected $user = []; |
||
| 27 | |||
| 28 | /** |
||
| 29 | * @var string how the provider data has been fetched |
||
| 30 | */ |
||
| 31 | protected $dataFetchMode; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * @var ThreemaGateway_Handler_Action_Callback |
||
| 35 | */ |
||
| 36 | protected $callback; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @var Threema\MsgApi\Helpers\ReceiveMessageResult |
||
| 40 | */ |
||
| 41 | protected $receiveResult; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * @var Threema\MsgApi\Messages\ThreemaMessage |
||
| 45 | */ |
||
| 46 | protected $threemaMsg; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @var array|string |
||
| 50 | */ |
||
| 51 | protected $log; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * @var bool |
||
| 55 | */ |
||
| 56 | protected $saveMessage; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var bool |
||
| 60 | */ |
||
| 61 | protected $debugMode; |
||
| 62 | |||
| 63 | /** |
||
| 64 | * @var string name of the processed message |
||
| 65 | */ |
||
| 66 | protected $name = 'message'; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * @var string name of the secret contained in a message |
||
| 70 | */ |
||
| 71 | protected $nameSecret = 'data'; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * @var int the type of the request |
||
| 75 | */ |
||
| 76 | protected $pendingRequestType; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * @var array filters/conditions applied to the message |
||
| 80 | */ |
||
| 81 | protected $filters; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var array all pending request messages found for the current message |
||
| 85 | */ |
||
| 86 | protected $pendingRequests; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * Checks whether text messages contain code used for the receiver 2FA. |
||
| 90 | * |
||
| 91 | * You should set the "event hint" to "1" to only pass text messages to the |
||
| 92 | * listener. Otherwise errors may happen. |
||
| 93 | * |
||
| 94 | * @param ThreemaGateway_Handler_Action_Callback $handler |
||
| 95 | * @param Threema\MsgApi\Helpers\ReceiveMessageResult $receiveResult |
||
| 96 | * @param Threema\MsgApi\Messages\ThreemaMessage $threemaMsg |
||
| 97 | * @param array|string $output [$logType, $debugLog, $publicLog] |
||
| 98 | * @param bool $saveMessage |
||
| 99 | * @param bool $debugMode |
||
| 100 | * |
||
| 101 | * @throws XenForo_Exception |
||
| 102 | */ |
||
| 103 | public function __construct(ThreemaGateway_Handler_Action_Callback $handler, |
||
| 117 | |||
| 118 | /** |
||
| 119 | * Prepare the message handling. |
||
| 120 | * |
||
| 121 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 122 | * |
||
| 123 | * @throws XenForo_Exception |
||
| 124 | * @return bool |
||
| 125 | */ |
||
| 126 | abstract public function prepareProcessing(); |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Filters the passed data/message. |
||
| 130 | * |
||
| 131 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 132 | * The filters have had to be set by [@see addFilter()] before. |
||
| 133 | * |
||
| 134 | * @return bool |
||
| 135 | */ |
||
| 136 | public function applyFilters() |
||
| 151 | |||
| 152 | /** |
||
| 153 | * Processes the pending requests, i.e. iterates all confirm requests and handles |
||
| 154 | * them. |
||
| 155 | * |
||
| 156 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 157 | * |
||
| 158 | * @param array $processOptions options, which are passed to {@link processConfirmRequest()}. |
||
| 159 | * @throws XenForo_Exception |
||
| 160 | * |
||
| 161 | * @return bool |
||
| 162 | */ |
||
| 163 | public function processPending(array $processOptions = []) |
||
| 195 | |||
| 196 | /** |
||
| 197 | * Verifies & saves data for one confirm request. |
||
| 198 | * |
||
| 199 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 200 | * childs should call the parent here as the things done in this class are |
||
| 201 | * essential! |
||
| 202 | * |
||
| 203 | * @throws XenForo_Exception |
||
| 204 | * @return bool |
||
| 205 | */ |
||
| 206 | protected function processConfirmRequest($confirmRequest) |
||
| 217 | |||
| 218 | /** |
||
| 219 | * Sets the name of the processed message. |
||
| 220 | * |
||
| 221 | * @param string $name what message this is, e.g.: XY message |
||
| 222 | * @param string $nameSecret what secrets etc. conmtains the message: XY code |
||
| 223 | */ |
||
| 224 | final public function setMessageTypeName($name, $nameSecret) |
||
| 229 | |||
| 230 | /** |
||
| 231 | * Sets the type of the request. |
||
| 232 | * |
||
| 233 | * Use one of the constants in |
||
| 234 | * ThreemaGateway_Model_TfaPendingMessagesConfirmation::PENDING_* |
||
| 235 | * |
||
| 236 | * @param int $pendingRequestType |
||
| 237 | */ |
||
| 238 | final public function setPrendingRequestType($pendingRequestType) |
||
| 242 | |||
| 243 | /** |
||
| 244 | * Returns the log and save message data you passed to this class when initiating. |
||
| 245 | * |
||
| 246 | * This should be called at least once at the end as this is the only way |
||
| 247 | * to update the log and save message values. |
||
| 248 | * |
||
| 249 | * @param array|string $output [$logType, $debugLog, $publicLog] |
||
| 250 | * @param bool $saveMessage |
||
| 251 | */ |
||
| 252 | final public function getReferencedData(&$output, &$saveMessage) |
||
| 257 | |||
| 258 | /** |
||
| 259 | * Adds a filter, which is applied to the message (data) when |
||
| 260 | * {@see applyFilter()} is called. |
||
| 261 | * |
||
| 262 | * @param int $filterType please use the constants FILTER_* |
||
| 263 | * @param mixed $filterData any data the filter uses |
||
| 264 | * @param bool $failOnError whether the filter should fail on errors (true) |
||
| 265 | * or silently ignore them (false) |
||
| 266 | */ |
||
| 267 | public function addFilter($filterType, $filterData, $failOnError = true) |
||
| 275 | |||
| 276 | /** |
||
| 277 | * Filters the passed data/message. |
||
| 278 | * |
||
| 279 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 280 | * |
||
| 281 | * @param int $filterType please use the constants FILTER_* |
||
| 282 | * @param mixed $filterData any data the filter uses |
||
| 283 | * @param bool $failOnError whether the filter should fail on errors (true) |
||
| 284 | * or silently ignore them (false) |
||
| 285 | * |
||
| 286 | * @throws XenForo_Exception |
||
| 287 | * @return bool |
||
| 288 | */ |
||
| 289 | abstract protected function applyFilter($filterType, $filterData, $failOnError = true); |
||
| 290 | |||
| 291 | /** |
||
| 292 | * Analyses/filters/validates the existing old provider data e.g. to |
||
| 293 | * compare it with the new data to set. |
||
| 294 | * |
||
| 295 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 296 | * |
||
| 297 | * @param array $confirmRequest the confirm request |
||
| 298 | * @param array $oldProviderData old data read |
||
| 299 | * @param array $setData new data to set |
||
| 300 | * @param array $processOptions custom options (optional) |
||
| 301 | * |
||
| 302 | * @throws XenForo_Exception |
||
| 303 | * @return bool |
||
| 304 | */ |
||
| 305 | protected function preSaveData(array $confirmRequest, array &$oldProviderData, array &$setData, array $processOptions = []) |
||
| 309 | |||
| 310 | /** |
||
| 311 | * Handles the already merged provider data. |
||
| 312 | * |
||
| 313 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 314 | * |
||
| 315 | * @param array $confirmRequest the confirm request |
||
| 316 | * @param array $providerData merged provider data |
||
| 317 | * @param array $processOptions custom options (optional) |
||
| 318 | * |
||
| 319 | * @throws XenForo_Exception |
||
| 320 | * @return bool |
||
| 321 | */ |
||
| 322 | protected function preSaveDataMerged(array $confirmRequest, array &$providerData, array $processOptions = []) |
||
| 326 | |||
| 327 | /** |
||
| 328 | * Does some stuff with the data after it has been saved. |
||
| 329 | * |
||
| 330 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 331 | * |
||
| 332 | * @param array $confirmRequest the confirm request |
||
| 333 | * @param array $providerData old data read |
||
| 334 | * @param array $processOptions custom options (optional) |
||
| 335 | * |
||
| 336 | * @throws XenForo_Exception |
||
| 337 | * @return bool |
||
| 338 | */ |
||
| 339 | protected function postSaveData(array $confirmRequest, array &$providerData, array $processOptions = []) |
||
| 343 | |||
| 344 | /** |
||
| 345 | * Does stuff needed to be done before processing the actual requests. |
||
| 346 | * |
||
| 347 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 348 | * Childs should call the parent here as the things done in this class are |
||
| 349 | * essential! |
||
| 350 | * |
||
| 351 | * @throws XenForo_Exception |
||
| 352 | * @return bool |
||
| 353 | */ |
||
| 354 | protected function preProcessPending() |
||
| 364 | |||
| 365 | /** |
||
| 366 | * Does stuff, which needs to be done after processing the requests. |
||
| 367 | * |
||
| 368 | * Returns "false" if the process should be canceled. Otherwise "true". |
||
| 369 | * Childs should call the parent here as the things done in this class are |
||
| 370 | * essential! |
||
| 371 | * |
||
| 372 | * @param bool $success whether the data was processed successfully |
||
| 373 | * @throws XenForo_Exception |
||
| 374 | * @return bool |
||
| 375 | */ |
||
| 376 | protected function postProcessPending($success) |
||
| 385 | |||
| 386 | /** |
||
| 387 | * Returns the pending messages for a given. |
||
| 388 | * |
||
| 389 | * @return array|false |
||
| 390 | */ |
||
| 391 | protected function getPendingRequests() |
||
| 410 | |||
| 411 | /** |
||
| 412 | * Checks whether a message is expired. |
||
| 413 | * |
||
| 414 | * This uses the date given by the Threema Gateway server (this is the |
||
| 415 | * send date) to verify that the message is not expired. |
||
| 416 | * Thus the current date is not used for this comparison as this should |
||
| 417 | * be done in the 2FA provider directly when verifying the data |
||
| 418 | * (verifyFromInput). |
||
| 419 | * |
||
| 420 | * @param array $confirmRequest the confirmation message request |
||
| 421 | * @throws XenForo_Exception |
||
| 422 | * |
||
| 423 | * @return bool |
||
| 424 | */ |
||
| 425 | protected function messageIsExpired(array $confirmRequest) |
||
| 437 | |||
| 438 | /** |
||
| 439 | * Saves data for a confirm request (as the provider data of the 2FA method). |
||
| 440 | * |
||
| 441 | * @param array $confirmRequest the confirmation message request |
||
| 442 | * @param array $setData an array of the data to set |
||
| 443 | * @param array $processOptions custom options (optional) |
||
| 444 | * @throws XenForo_Exception |
||
| 445 | */ |
||
| 446 | protected function setDataForRequest( |
||
| 526 | |||
| 527 | /** |
||
| 528 | * Fetches and returns the provider data using the session. |
||
| 529 | * |
||
| 530 | * @param array $confirmRequest the confirmation message request |
||
| 531 | * @throws XenForo_Exception |
||
| 532 | * @return array |
||
| 533 | */ |
||
| 534 | protected function getProviderDataBySession(array $confirmRequest) |
||
| 552 | |||
| 553 | /** |
||
| 554 | * Fetches and returns the provider data. |
||
| 555 | * |
||
| 556 | * @param array $confirmRequest the confirmation message request |
||
| 557 | * @throws XenForo_Exception |
||
| 558 | * @return array |
||
| 559 | */ |
||
| 560 | protected function getProviderDataByModel(array $confirmRequest) |
||
| 585 | |||
| 586 | /** |
||
| 587 | * Gets model from cache or initializes a new model if needed. |
||
| 588 | * |
||
| 589 | * @param array $newProviderData provider data to save |
||
| 590 | * @param array $confirmRequest the confirmation message request |
||
| 591 | * |
||
| 592 | * @throws XenForo_Exception |
||
| 593 | */ |
||
| 594 | protected function saveProviderData(array $newProviderData, array $confirmRequest) |
||
| 619 | |||
| 620 | /** |
||
| 621 | * Gets model from cache or initializes a new model if needed. |
||
| 622 | * |
||
| 623 | * @param string $class Name of class to load |
||
| 624 | * |
||
| 625 | * @return XenForo_Model |
||
| 626 | */ |
||
| 627 | final protected function getModelFromCache($class) |
||
| 635 | |||
| 636 | /** |
||
| 637 | * Returns the XenForo session. |
||
| 638 | * |
||
| 639 | * @return XenForo_Session |
||
| 640 | */ |
||
| 641 | final protected function getSession() |
||
| 650 | |||
| 651 | /** |
||
| 652 | * Adds some data to the log. |
||
| 653 | * |
||
| 654 | * @param string $logDetailed |
||
| 655 | * @param string|null $logGeneral |
||
| 656 | * @return XenForo_Session |
||
| 657 | */ |
||
| 658 | final protected function log($logDetailed, $logGeneral = null) |
||
| 662 | |||
| 663 | /** |
||
| 664 | * Returns the user array. |
||
| 665 | * |
||
| 666 | * @param int $userId |
||
| 667 | * @return array |
||
| 668 | */ |
||
| 669 | final protected function getUserData($userId) |
||
| 685 | } |
||
| 686 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)or! empty(...)instead.