Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like IndexController 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 IndexController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 31 | class IndexController extends AbstractActionController |
||
| 32 | { |
||
| 33 | |||
| 34 | const LOGIN='login'; |
||
| 35 | const REGISTER='register'; |
||
| 36 | |||
| 37 | /** |
||
| 38 | * @var AuthenticationService |
||
| 39 | */ |
||
| 40 | protected $auth; |
||
| 41 | |||
| 42 | /** |
||
| 43 | * @var array |
||
| 44 | */ |
||
| 45 | protected $forms; |
||
| 46 | |||
| 47 | /** |
||
| 48 | * @var LoggerInterface |
||
| 49 | */ |
||
| 50 | protected $logger; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * @var ModuleOptions |
||
| 54 | */ |
||
| 55 | protected $options; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @param $auth AuthenticationService |
||
| 59 | * @param $logger LoggerInterface |
||
| 60 | * @param $forms |
||
| 61 | * @param $options ModuleOptions |
||
| 62 | */ |
||
| 63 | public function __construct(AuthenticationService $auth, LoggerInterface $logger, array $forms, $options) |
||
| 64 | { |
||
| 65 | $this->auth = $auth; |
||
| 66 | $this->forms = $forms; |
||
| 67 | $this->logger = $logger; |
||
| 68 | $this->options = $options; |
||
| 69 | } |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Login with username and password |
||
| 73 | * |
||
| 74 | * @return \Zend\Http\Response|ViewModel |
||
| 75 | */ |
||
| 76 | public function indexAction() |
||
| 77 | { |
||
| 78 | if ($this->auth->hasIdentity()) { |
||
| 79 | return $this->redirect()->toRoute('lang'); |
||
| 80 | } |
||
| 81 | |||
| 82 | $viewModel = new ViewModel(); |
||
| 83 | $services = $this->getServiceLocator(); |
||
| 84 | |||
| 85 | /* @var $loginForm Login */ |
||
| 86 | $loginForm = $this->forms[self::LOGIN]; |
||
| 87 | /* @var $registerForm Register */ |
||
| 88 | $registerForm = $this->forms[self::REGISTER]; |
||
| 89 | |||
| 90 | /* @var $request \Zend\Http\Request */ |
||
| 91 | $request = $this->getRequest(); |
||
| 92 | |||
| 93 | if ($request->isPost()) { |
||
| 94 | $data = $this->params()->fromPost(); |
||
| 95 | $adapter = $services->get('Auth/Adapter/UserLogin'); |
||
| 96 | // inject suffixes via shared Events |
||
| 97 | $loginSuffix = ''; |
||
| 98 | // @TODO: replace this by the Plugin LoginFilter |
||
| 99 | $e = $this->getEvent(); |
||
| 100 | $loginSuffixResponseCollection = $this->getEventManager()->trigger('login.getSuffix', $e); |
||
| 101 | if (!$loginSuffixResponseCollection->isEmpty()) { |
||
| 102 | $loginSuffix = $loginSuffixResponseCollection->last(); |
||
| 103 | } |
||
| 104 | |||
| 105 | $loginForm->setData($data); |
||
| 106 | if (array_key_exists('credentials', $data) && |
||
| 107 | array_key_exists('login', $data['credentials']) && |
||
| 108 | array_key_exists('credential', $data['credentials'])) { |
||
| 109 | $adapter->setIdentity($data['credentials']['login'] . $loginSuffix) |
||
| 110 | ->setCredential($data['credentials']['credential']); |
||
| 111 | } |
||
| 112 | |||
| 113 | $auth = $this->auth; |
||
| 114 | $result = $auth->authenticate($adapter); |
||
|
|
|||
| 115 | |||
| 116 | |||
| 117 | if ($result->isValid()) { |
||
| 118 | $user = $auth->getUser(); |
||
| 119 | $settings = $user->getSettings('Core'); |
||
| 120 | $language = $settings->localization->language; |
||
| 121 | View Code Duplication | if (!$language) { |
|
| 122 | $headers = $request->getHeaders(); |
||
| 123 | if ($headers->has('Accept-Language')) { |
||
| 124 | $locales = $headers->get('Accept-Language')->getPrioritized(); |
||
| 125 | $language = $locales[0]->type; |
||
| 126 | } else { |
||
| 127 | $language = 'en'; |
||
| 128 | } |
||
| 129 | } |
||
| 130 | $this->logger->info('User ' . $user->login . ' logged in'); |
||
| 131 | |||
| 132 | $ref = $this->params()->fromQuery('ref', false); |
||
| 133 | |||
| 134 | if ($ref) { |
||
| 135 | $ref = urldecode($ref); |
||
| 136 | $url = preg_replace('~/[a-z]{2}(/|$)~', '/' . $language . '$1', $ref); |
||
| 137 | $url = $request->getBasePath() . $url; |
||
| 138 | } else { |
||
| 139 | $urlHelper = $services->get('ViewHelperManager')->get('url'); |
||
| 140 | $url = $urlHelper('lang', array('lang' => $language)); |
||
| 141 | } |
||
| 142 | $this->notification()->success(/*@translate*/ 'You are now logged in.'); |
||
| 143 | return $this->redirect()->toUrl($url); |
||
| 144 | } else { |
||
| 145 | $loginName = $data['credentials']['login']; |
||
| 146 | if (!empty($loginSuffix)) { |
||
| 147 | $loginName = $loginName . ' (' . $loginName . $loginSuffix . ')'; |
||
| 148 | } |
||
| 149 | $this->logger->info('Failed to authenticate User ' . $loginName); |
||
| 150 | $this->notification()->danger(/*@translate*/ 'Authentication failed.'); |
||
| 151 | } |
||
| 152 | } |
||
| 153 | |||
| 154 | $ref = $this->params()->fromQuery('ref', false); |
||
| 155 | |||
| 156 | if ($ref) { |
||
| 157 | $req = $this->params()->fromQuery('req', false); |
||
| 158 | if ($req) { |
||
| 159 | $this->getResponse()->setStatusCode(403); |
||
| 160 | $viewModel->setVariable('required', true); |
||
| 161 | } |
||
| 162 | $viewModel->setVariable('ref', $ref); |
||
| 163 | } |
||
| 164 | |||
| 165 | $allowRegister = $this->options->getEnableRegistration(); |
||
| 166 | $allowResetPassword = $this->options->getEnableResetPassword(); |
||
| 167 | if (isset($allowRegister)) { |
||
| 168 | $viewModel->setVariables( |
||
| 169 | [ |
||
| 170 | 'allowRegister' => $allowRegister, |
||
| 171 | 'allowResetPassword' => $allowResetPassword |
||
| 172 | ] |
||
| 173 | ); |
||
| 174 | } |
||
| 175 | |||
| 176 | $viewModel->setVariable('loginForm', $loginForm); |
||
| 177 | $viewModel->setVariable('registerForm', $registerForm); |
||
| 178 | |||
| 179 | /* @deprecated use loginForm instead of form in your view scripts */ |
||
| 180 | $viewModel->setVariable('form', $loginForm); |
||
| 181 | |||
| 182 | |||
| 183 | return $viewModel; |
||
| 184 | } |
||
| 185 | |||
| 186 | /** |
||
| 187 | * Login with HybridAuth |
||
| 188 | * |
||
| 189 | * Passed in Params: |
||
| 190 | * - provider: HybridAuth provider identifier. |
||
| 191 | * |
||
| 192 | * Redirects To: Route 'home' |
||
| 193 | */ |
||
| 194 | public function loginAction() |
||
| 195 | { |
||
| 196 | $ref = urldecode($this->getRequest()->getBasePath().$this->params()->fromQuery('ref')); |
||
| 197 | $provider = $this->params('provider', '--keiner--'); |
||
| 198 | $hauth = $this->getServiceLocator()->get('HybridAuthAdapter'); |
||
| 199 | $hauth->setProvider($provider); |
||
| 200 | $auth = $this->auth; |
||
| 201 | $result = $auth->authenticate($hauth); |
||
| 202 | $resultMessage = $result->getMessages(); |
||
| 203 | |||
| 204 | if (array_key_exists('firstLogin', $resultMessage) && $resultMessage['firstLogin'] === true) { |
||
| 205 | try { |
||
| 206 | $user = $auth->getUser(); |
||
| 207 | $password = substr(md5(uniqid()), 0, 6); |
||
| 208 | $login = uniqid() . ($this->options->auth_suffix != "" ? '@' . $this->options->auth_suffix : ''); |
||
| 209 | $externalLogin = isset($user->login)?$user->login:'-- not communicated --'; |
||
| 210 | $this->logger->debug('first login via ' . $provider . ' as: ' . $externalLogin); |
||
| 211 | |||
| 212 | $user->login=$login; |
||
| 213 | $user->setPassword($password); |
||
| 214 | $user->setRole($this->options->getRole()); |
||
| 215 | |||
| 216 | $mail = $this->mailer('htmltemplate'); |
||
| 217 | $mail->setTemplate('mail/first-socialmedia-login'); |
||
| 218 | $mail->setSubject($this->options->getMailSubjectRegistration()); |
||
| 219 | $mail->setVariables( |
||
| 220 | array( |
||
| 221 | 'displayName'=> $user->getInfo()->getDisplayName(), |
||
| 222 | 'provider' => $provider, |
||
| 223 | 'login' => $login, |
||
| 224 | 'password' => $password, |
||
| 225 | ) |
||
| 226 | ); |
||
| 227 | $mail->addTo($user->getInfo()->getEmail()); |
||
| 228 | |||
| 229 | $loggerId = $login . ' (' . $provider . ': ' . $externalLogin . ')'; |
||
| 230 | if (isset($mail) && $this->mailer($mail)) { |
||
| 231 | $this->logger->info('Mail first-login for ' . $loggerId . ' sent to ' . $user->getInfo()->getEmail()); |
||
| 232 | } else { |
||
| 233 | $this->logger->warn('No Mail was sent for ' . $loggerId); |
||
| 234 | } |
||
| 235 | } catch (\Exception $e) { |
||
| 236 | $this->logger->crit($e); |
||
| 237 | $this->notification()->danger( |
||
| 238 | /*@translate*/ 'An unexpected error has occurred, please contact your system administrator' |
||
| 239 | ); |
||
| 240 | } |
||
| 241 | } |
||
| 242 | |||
| 243 | $user = $auth->getUser(); |
||
| 244 | $this->logger->info('User ' . $auth->getUser()->getInfo()->getDisplayName() . ' logged in via ' . $provider); |
||
| 245 | $settings = $user->getSettings('Core'); |
||
| 246 | if (null !== $settings->localization->language) { |
||
| 247 | $basePath = $this->getRequest()->getBasePath(); |
||
| 248 | $ref = preg_replace('~^'.$basePath . '/[a-z]{2}(/)?~', $basePath . '/' . $settings->localization->language . '$1', $ref); |
||
| 249 | } |
||
| 250 | return $this->redirect()->toUrl($ref); |
||
| 251 | } |
||
| 252 | |||
| 253 | /** |
||
| 254 | * Login via an external Application. This will get obsolet as soon we'll have a full featured Rest API. |
||
| 255 | * |
||
| 256 | * Passed in params: |
||
| 257 | * - appKey: Application identifier key |
||
| 258 | * - user: Name of the user to log in |
||
| 259 | * - pass: Password of the user to log in |
||
| 260 | * |
||
| 261 | * Returns an json response with the session-id. |
||
| 262 | * Non existent users will be created! |
||
| 263 | * |
||
| 264 | */ |
||
| 265 | public function loginExternAction() |
||
| 367 | |||
| 368 | public function groupAction() |
||
| 369 | { |
||
| 370 | //$adapter = $this->getServiceLocator()->get('ExternalApplicationAdapter'); |
||
| 437 | |||
| 438 | /** |
||
| 439 | * Logout |
||
| 440 | * |
||
| 441 | * Redirects To: Route 'home' |
||
| 442 | */ |
||
| 443 | public function logoutAction() |
||
| 456 | } |
||
| 457 |
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: