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 UsersController 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 UsersController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 58 | class UsersController extends Controller { |
||
| 59 | /** @var IL10N */ |
||
| 60 | private $l10n; |
||
| 61 | /** @var IUserSession */ |
||
| 62 | private $userSession; |
||
| 63 | /** @var bool */ |
||
| 64 | private $isAdmin; |
||
| 65 | /** @var IUserManager */ |
||
| 66 | private $userManager; |
||
| 67 | /** @var IGroupManager */ |
||
| 68 | private $groupManager; |
||
| 69 | /** @var IConfig */ |
||
| 70 | private $config; |
||
| 71 | /** @var ILogger */ |
||
| 72 | private $log; |
||
| 73 | /** @var \OC_Defaults */ |
||
| 74 | private $defaults; |
||
| 75 | /** @var IMailer */ |
||
| 76 | private $mailer; |
||
| 77 | /** @var string */ |
||
| 78 | private $fromMailAddress; |
||
| 79 | /** @var IURLGenerator */ |
||
| 80 | private $urlGenerator; |
||
| 81 | /** @var bool contains the state of the encryption app */ |
||
| 82 | private $isEncryptionAppEnabled; |
||
| 83 | /** @var bool contains the state of the admin recovery setting */ |
||
| 84 | private $isRestoreEnabled = false; |
||
| 85 | /** @var IAvatarManager */ |
||
| 86 | private $avatarManager; |
||
| 87 | /** @var ISecureRandom */ |
||
| 88 | protected $secureRandom; |
||
| 89 | /** @var ITimeFactory */ |
||
| 90 | protected $timeFactory; |
||
| 91 | |||
| 92 | /** |
||
| 93 | * @param string $appName |
||
| 94 | * @param IRequest $request |
||
| 95 | * @param IUserManager $userManager |
||
| 96 | * @param IGroupManager $groupManager |
||
| 97 | * @param IUserSession $userSession |
||
| 98 | * @param IConfig $config |
||
| 99 | * @param ISecureRandom $secureRandom |
||
| 100 | * @param IL10N $l10n |
||
| 101 | * @param ILogger $log |
||
| 102 | * @param \OC_Defaults $defaults |
||
| 103 | * @param IMailer $mailer |
||
| 104 | * @param ITimeFactory $timeFactory |
||
| 105 | * @param $fromMailAddress |
||
| 106 | * @param IURLGenerator $urlGenerator |
||
| 107 | * @param IAppManager $appManager |
||
| 108 | * @param IAvatarManager $avatarManager |
||
| 109 | */ |
||
| 110 | public function __construct($appName, |
||
| 111 | IRequest $request, |
||
| 112 | IUserManager $userManager, |
||
| 113 | IGroupManager $groupManager, |
||
| 114 | IUserSession $userSession, |
||
| 115 | IConfig $config, |
||
| 116 | ISecureRandom $secureRandom, |
||
| 117 | IL10N $l10n, |
||
| 118 | ILogger $log, |
||
| 119 | \OC_Defaults $defaults, |
||
| 120 | IMailer $mailer, |
||
| 121 | ITimeFactory $timeFactory, |
||
| 122 | $fromMailAddress, |
||
| 123 | IURLGenerator $urlGenerator, |
||
| 124 | IAppManager $appManager, |
||
| 125 | IAvatarManager $avatarManager) { |
||
| 126 | parent::__construct($appName, $request); |
||
| 127 | $this->userManager = $userManager; |
||
| 128 | $this->groupManager = $groupManager; |
||
| 129 | $this->userSession = $userSession; |
||
| 130 | $this->config = $config; |
||
| 131 | $this->l10n = $l10n; |
||
| 132 | $this->secureRandom = $secureRandom; |
||
| 133 | $this->log = $log; |
||
| 134 | $this->defaults = $defaults; |
||
| 135 | $this->mailer = $mailer; |
||
| 136 | $this->timeFactory = $timeFactory; |
||
| 137 | $this->fromMailAddress = $fromMailAddress; |
||
| 138 | $this->urlGenerator = $urlGenerator; |
||
| 139 | $this->avatarManager = $avatarManager; |
||
| 140 | $this->isAdmin = $this->isAdmin(); |
||
| 141 | |||
| 142 | // check for encryption state - TODO see formatUserForIndex |
||
| 143 | $this->isEncryptionAppEnabled = $appManager->isEnabledForUser('encryption'); |
||
| 144 | if($this->isEncryptionAppEnabled) { |
||
| 145 | // putting this directly in empty is possible in PHP 5.5+ |
||
| 146 | $result = $config->getAppValue('encryption', 'recoveryAdminEnabled', 0); |
||
| 147 | $this->isRestoreEnabled = !empty($result); |
||
| 148 | } |
||
| 149 | } |
||
| 150 | |||
| 151 | /** |
||
| 152 | * @param IUser $user |
||
| 153 | * @param array $userGroups |
||
| 154 | * @return array |
||
| 155 | */ |
||
| 156 | private function formatUserForIndex(IUser $user, array $userGroups = null) { |
||
| 157 | |||
| 158 | // TODO: eliminate this encryption specific code below and somehow |
||
| 159 | // hook in additional user info from other apps |
||
| 160 | |||
| 161 | // recovery isn't possible if admin or user has it disabled and encryption |
||
| 162 | // is enabled - so we eliminate the else paths in the conditional tree |
||
| 163 | // below |
||
| 164 | $restorePossible = false; |
||
| 165 | |||
| 166 | if ($this->isEncryptionAppEnabled) { |
||
| 167 | if ($this->isRestoreEnabled) { |
||
| 168 | // check for the users recovery setting |
||
| 169 | $recoveryMode = $this->config->getUserValue($user->getUID(), 'encryption', 'recoveryEnabled', '0'); |
||
| 170 | // method call inside empty is possible with PHP 5.5+ |
||
| 171 | $recoveryModeEnabled = !empty($recoveryMode); |
||
| 172 | if ($recoveryModeEnabled) { |
||
| 173 | // user also has recovery mode enabled |
||
| 174 | $restorePossible = true; |
||
| 175 | } |
||
| 176 | } |
||
| 177 | } else { |
||
| 178 | // recovery is possible if encryption is disabled (plain files are |
||
| 179 | // available) |
||
| 180 | $restorePossible = true; |
||
| 181 | } |
||
| 182 | |||
| 183 | $subAdminGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($user); |
||
| 184 | foreach($subAdminGroups as $key => $subAdminGroup) { |
||
| 185 | $subAdminGroups[$key] = $subAdminGroup->getGID(); |
||
| 186 | } |
||
| 187 | |||
| 188 | $displayName = $user->getEMailAddress(); |
||
| 189 | if (is_null($displayName)) { |
||
| 190 | $displayName = ''; |
||
| 191 | } |
||
| 192 | |||
| 193 | $avatarAvailable = false; |
||
| 194 | if ($this->config->getSystemValue('enable_avatars', true) === true) { |
||
| 195 | try { |
||
| 196 | $avatarAvailable = $this->avatarManager->getAvatar($user->getUID())->exists(); |
||
| 197 | } catch (\Exception $e) { |
||
| 198 | //No avatar yet |
||
| 199 | } |
||
| 200 | } |
||
| 201 | |||
| 202 | return [ |
||
| 203 | 'name' => $user->getUID(), |
||
| 204 | 'displayname' => $user->getDisplayName(), |
||
| 205 | 'groups' => (empty($userGroups)) ? $this->groupManager->getUserGroupIds($user, 'management') : $userGroups, |
||
| 206 | 'subadmin' => $subAdminGroups, |
||
| 207 | 'isEnabled' => $user->isEnabled(), |
||
| 208 | 'quota' => $user->getQuota(), |
||
| 209 | 'storageLocation' => $user->getHome(), |
||
| 210 | 'lastLogin' => $user->getLastLogin() * 1000, |
||
| 211 | 'backend' => $user->getBackendClassName(), |
||
| 212 | 'email' => $displayName, |
||
| 213 | 'isRestoreDisabled' => !$restorePossible, |
||
| 214 | 'isAvatarAvailable' => $avatarAvailable, |
||
| 215 | ]; |
||
| 216 | } |
||
| 217 | |||
| 218 | /** |
||
| 219 | * @param array $userIDs Array with schema [$uid => $displayName] |
||
| 220 | * @return IUser[] |
||
| 221 | */ |
||
| 222 | private function getUsersForUID(array $userIDs) { |
||
| 223 | $users = []; |
||
| 224 | foreach ($userIDs as $uid => $displayName) { |
||
| 225 | $users[$uid] = $this->userManager->get($uid); |
||
| 226 | } |
||
| 227 | return $users; |
||
| 228 | } |
||
| 229 | |||
| 230 | /** |
||
| 231 | * @param string $token |
||
| 232 | * @param string $userId |
||
| 233 | * @throws \Exception |
||
| 234 | */ |
||
| 235 | private function checkEmailChangeToken($token, $userId) { |
||
| 236 | $user = $this->userManager->get($userId); |
||
| 237 | |||
| 238 | if ($user === null) { |
||
| 239 | throw new \Exception($this->l10n->t('Couldn\'t change the email address because the user does not exist')); |
||
| 240 | } |
||
| 241 | |||
| 242 | $splittedToken = explode(':', $this->config->getUserValue($userId, 'owncloud', 'changeMail', null)); |
||
| 243 | View Code Duplication | if(count($splittedToken) !== 3) { |
|
|
|
|||
| 244 | $this->config->deleteUserValue($userId, 'owncloud', 'changeMail'); |
||
| 245 | throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid')); |
||
| 246 | } |
||
| 247 | |||
| 248 | View Code Duplication | if ($splittedToken[0] < ($this->timeFactory->getTime() - 60*60*12)) { |
|
| 249 | $this->config->deleteUserValue($userId, 'owncloud', 'changeMail'); |
||
| 250 | throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid')); |
||
| 251 | } |
||
| 252 | |||
| 253 | View Code Duplication | if (!hash_equals($splittedToken[1], $token)) { |
|
| 254 | $this->config->deleteUserValue($userId, 'owncloud', 'changeMail'); |
||
| 255 | throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid')); |
||
| 256 | } |
||
| 257 | } |
||
| 258 | |||
| 259 | /** |
||
| 260 | * @NoAdminRequired |
||
| 261 | * |
||
| 262 | * @param int $offset |
||
| 263 | * @param int $limit |
||
| 264 | * @param string $gid GID to filter for |
||
| 265 | * @param string $pattern Pattern to find in the account table (userid, displayname, email, additional search terms) |
||
| 266 | * @param string $backend Backend to filter for (class-name) |
||
| 267 | * @return DataResponse |
||
| 268 | * |
||
| 269 | * TODO: Tidy up and write unit tests - code is mainly static method calls |
||
| 270 | */ |
||
| 271 | public function index($offset = 0, $limit = 10, $gid = '', $pattern = '', $backend = '') { |
||
| 343 | |||
| 344 | /** |
||
| 345 | * @NoAdminRequired |
||
| 346 | * |
||
| 347 | * @param string $username |
||
| 348 | * @param string $password |
||
| 349 | * @param array $groups |
||
| 350 | * @param string $email |
||
| 351 | * @return DataResponse |
||
| 352 | */ |
||
| 353 | public function create($username, $password, array $groups= [], $email='') { |
||
| 475 | |||
| 476 | /** |
||
| 477 | * @NoAdminRequired |
||
| 478 | * |
||
| 479 | * @param string $id |
||
| 480 | * @return DataResponse |
||
| 481 | */ |
||
| 482 | public function destroy($id) { |
||
| 534 | |||
| 535 | /** |
||
| 536 | * Set the mail address of a user |
||
| 537 | * |
||
| 538 | * @NoAdminRequired |
||
| 539 | * @NoSubadminRequired |
||
| 540 | * |
||
| 541 | * @param string $id |
||
| 542 | * @param string $mailAddress |
||
| 543 | * @return DataResponse |
||
| 544 | */ |
||
| 545 | public function setMailAddress($id, $mailAddress) { |
||
| 546 | $userId = $this->userSession->getUser()->getUID(); |
||
| 547 | $user = $this->userManager->get($id); |
||
| 548 | |||
| 549 | View Code Duplication | if($userId !== $id |
|
| 550 | && !$this->isAdmin |
||
| 551 | && !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) { |
||
| 552 | return new DataResponse( |
||
| 553 | [ |
||
| 554 | 'status' => 'error', |
||
| 555 | 'data' => [ |
||
| 556 | 'message' => (string)$this->l10n->t('Forbidden') |
||
| 557 | ] |
||
| 558 | ], |
||
| 559 | Http::STATUS_FORBIDDEN |
||
| 560 | ); |
||
| 561 | } |
||
| 562 | |||
| 563 | if($mailAddress !== '' && !$this->mailer->validateMailAddress($mailAddress)) { |
||
| 564 | return new DataResponse( |
||
| 565 | [ |
||
| 566 | 'status' => 'error', |
||
| 567 | 'data' => [ |
||
| 568 | 'message' => (string)$this->l10n->t('Invalid mail address') |
||
| 569 | ] |
||
| 570 | ], |
||
| 571 | Http::STATUS_UNPROCESSABLE_ENTITY |
||
| 572 | ); |
||
| 573 | } |
||
| 574 | |||
| 575 | View Code Duplication | if(!$user){ |
|
| 576 | return new DataResponse( |
||
| 577 | [ |
||
| 578 | 'status' => 'error', |
||
| 579 | 'data' => [ |
||
| 580 | 'message' => (string)$this->l10n->t('Invalid user') |
||
| 581 | ] |
||
| 582 | ], |
||
| 583 | Http::STATUS_UNPROCESSABLE_ENTITY |
||
| 584 | ); |
||
| 585 | } |
||
| 586 | |||
| 587 | // this is the only permission a backend provides and is also used |
||
| 588 | // for the permission of setting a email address |
||
| 589 | View Code Duplication | if(!$user->canChangeDisplayName()){ |
|
| 590 | return new DataResponse( |
||
| 591 | [ |
||
| 592 | 'status' => 'error', |
||
| 593 | 'data' => [ |
||
| 594 | 'message' => (string)$this->l10n->t('Unable to change mail address') |
||
| 595 | ] |
||
| 596 | ], |
||
| 597 | Http::STATUS_FORBIDDEN |
||
| 598 | ); |
||
| 599 | } |
||
| 600 | |||
| 601 | // admins can set email without verification |
||
| 602 | View Code Duplication | if ($mailAddress === '' || $this->isAdmin) { |
|
| 603 | $this->setEmailAddress($userId, $mailAddress); |
||
| 604 | return new DataResponse( |
||
| 605 | [ |
||
| 606 | 'status' => 'success', |
||
| 607 | 'data' => [ |
||
| 608 | 'message' => (string)$this->l10n->t('Email has been changed successfully.') |
||
| 609 | ] |
||
| 610 | ], |
||
| 611 | Http::STATUS_OK |
||
| 612 | ); |
||
| 613 | } |
||
| 614 | |||
| 615 | try { |
||
| 616 | if ($this->sendEmail($userId, $mailAddress)) { |
||
| 617 | return new DataResponse( |
||
| 618 | [ |
||
| 619 | 'status' => 'success', |
||
| 620 | 'data' => [ |
||
| 621 | 'username' => $id, |
||
| 622 | 'mailAddress' => $mailAddress, |
||
| 623 | 'message' => (string) $this->l10n->t('An email has been sent to this address for confirmation') |
||
| 624 | ] |
||
| 625 | ], |
||
| 626 | Http::STATUS_OK |
||
| 627 | ); |
||
| 628 | View Code Duplication | } else { |
|
| 629 | return new DataResponse( |
||
| 630 | [ |
||
| 631 | 'status' => 'error', |
||
| 632 | 'data' => [ |
||
| 633 | 'username' => $id, |
||
| 634 | 'mailAddress' => $mailAddress, |
||
| 635 | 'message' => (string) $this->l10n->t('No email was sent because you already sent one recently. Please try again later.') |
||
| 636 | ] |
||
| 637 | ], |
||
| 638 | Http::STATUS_OK |
||
| 639 | ); |
||
| 640 | } |
||
| 641 | |||
| 642 | } catch (\Exception $e){ |
||
| 643 | return new DataResponse( |
||
| 644 | [ |
||
| 645 | 'status' => 'error', |
||
| 646 | 'data' => [ |
||
| 647 | 'message' => (string)$e->getMessage() |
||
| 648 | ] |
||
| 649 | ] |
||
| 650 | ); |
||
| 651 | } |
||
| 652 | |||
| 653 | } |
||
| 654 | |||
| 655 | /** |
||
| 656 | * Count all unique users visible for the current admin/subadmin. |
||
| 657 | * |
||
| 658 | * @NoAdminRequired |
||
| 659 | * |
||
| 660 | * @return DataResponse |
||
| 661 | */ |
||
| 662 | public function stats() { |
||
| 691 | |||
| 692 | |||
| 693 | /** |
||
| 694 | * Set the displayName of a user |
||
| 695 | * |
||
| 696 | * @NoAdminRequired |
||
| 697 | * @NoSubadminRequired |
||
| 698 | * |
||
| 699 | * @param string $username |
||
| 700 | * @param string $displayName |
||
| 701 | * @return DataResponse |
||
| 702 | */ |
||
| 703 | public function setDisplayName($username, $displayName) { |
||
| 746 | |||
| 747 | /** |
||
| 748 | * @param string $userId |
||
| 749 | * @param string $mailAddress |
||
| 750 | * @throws \Exception |
||
| 751 | * @return boolean |
||
| 752 | */ |
||
| 753 | public function sendEmail($userId, $mailAddress) { |
||
| 789 | |||
| 790 | /** |
||
| 791 | * @NoAdminRequired |
||
| 792 | * |
||
| 793 | * @param string $id |
||
| 794 | * @param string $mailAddress |
||
| 795 | */ |
||
| 796 | public function setEmailAddress($id, $mailAddress) { |
||
| 797 | $user = $this->userManager->get($id); |
||
| 798 | |||
| 799 | // Only Admin and SubAdmins are allowed to set email |
||
| 800 | if($this->isAdmin || |
||
| 801 | ($this->groupManager->getSubAdmin()->isSubAdmin($this->userSession->getUser()) && |
||
| 802 | $this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user))) { |
||
| 803 | $user->setEMailAddress($mailAddress); |
||
| 804 | if ($this->config->getUserValue($id, 'owncloud', 'changeMail') !== '') { |
||
| 805 | $this->config->deleteUserValue($id, 'owncloud', 'changeMail'); |
||
| 806 | } |
||
| 807 | } else { |
||
| 808 | return new JSONResponse([ |
||
| 809 | 'error' => 'cannotSetEmailAddress', |
||
| 810 | 'message' => 'Cannot set email address for user' |
||
| 811 | ], HTTP::STATUS_NOT_FOUND); |
||
| 812 | } |
||
| 813 | } |
||
| 814 | |||
| 815 | /** |
||
| 816 | * @NoCSRFRequired |
||
| 817 | * @NoAdminRequired |
||
| 818 | * @NoSubadminRequired |
||
| 819 | * |
||
| 820 | * @param $token |
||
| 821 | * @param $userId |
||
| 822 | * @return RedirectResponse |
||
| 823 | * @throws \Exception |
||
| 824 | */ |
||
| 825 | public function changeMail($token, $userId) { |
||
| 868 | |||
| 869 | /* |
||
| 870 | * @NoAdminRequired |
||
| 871 | * |
||
| 872 | * @param string $id |
||
| 873 | * @return DataResponse |
||
| 874 | */ |
||
| 875 | public function setEnabled($id, $enabled) { |
||
| 934 | |||
| 935 | View Code Duplication | private function isAdmin() { |
|
| 936 | // Check if current user (active and not in incognito mode) |
||
| 937 | // is an admin |
||
| 938 | $activeUser = $this->userSession->getUser(); |
||
| 939 | if($activeUser !== null) { |
||
| 944 | } |
||
| 945 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.