nextcloud /
server
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 User_LDAP 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 User_LDAP, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 47 | class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP { |
||
| 48 | /** @var string[] $homesToKill */ |
||
| 49 | protected $homesToKill = array(); |
||
| 50 | |||
| 51 | /** @var \OCP\IConfig */ |
||
| 52 | protected $ocConfig; |
||
| 53 | |||
| 54 | /** @var INotificationManager */ |
||
| 55 | protected $notificationManager; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @param Access $access |
||
| 59 | * @param \OCP\IConfig $ocConfig |
||
| 60 | * @param \OCP\Notification\IManager $notificationManager |
||
| 61 | */ |
||
| 62 | public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager) { |
||
| 67 | |||
| 68 | /** |
||
| 69 | * checks whether the user is allowed to change his avatar in Nextcloud |
||
| 70 | * @param string $uid the Nextcloud user name |
||
| 71 | * @return boolean either the user can or cannot |
||
| 72 | */ |
||
| 73 | public function canChangeAvatar($uid) { |
||
| 84 | |||
| 85 | /** |
||
| 86 | * returns the username for the given login name, if available |
||
| 87 | * |
||
| 88 | * @param string $loginName |
||
| 89 | * @return string|false |
||
| 90 | */ |
||
| 91 | public function loginName2UserName($loginName) { |
||
| 115 | |||
| 116 | /** |
||
| 117 | * returns the username for the given LDAP DN, if available |
||
| 118 | * |
||
| 119 | * @param string $dn |
||
| 120 | * @return string|false with the username |
||
| 121 | */ |
||
| 122 | public function dn2UserName($dn) { |
||
| 125 | |||
| 126 | /** |
||
| 127 | * returns an LDAP record based on a given login name |
||
| 128 | * |
||
| 129 | * @param string $loginName |
||
| 130 | * @return array |
||
| 131 | * @throws NotOnLDAP |
||
| 132 | */ |
||
| 133 | public function getLDAPUserByLoginName($loginName) { |
||
| 143 | |||
| 144 | /** |
||
| 145 | * Check if the password is correct without logging in the user |
||
| 146 | * |
||
| 147 | * @param string $uid The username |
||
| 148 | * @param string $password The password |
||
| 149 | * @return false|string |
||
| 150 | */ |
||
| 151 | public function checkPassword($uid, $password) { |
||
| 185 | |||
| 186 | /** |
||
| 187 | * Set password |
||
| 188 | * @param string $uid The username |
||
| 189 | * @param string $password The new password |
||
| 190 | * @return bool |
||
| 191 | */ |
||
| 192 | public function setPassword($uid, $password) { |
||
| 193 | $user = $this->access->userManager->get($uid); |
||
| 194 | |||
| 195 | if(!$user instanceof User) { |
||
| 196 | throw new \Exception('LDAP setPassword: Could not get user object for uid ' . $uid . |
||
| 197 | '. Maybe the LDAP entry has no set display name attribute?'); |
||
| 198 | } |
||
| 199 | if($user->getUsername() !== false && $this->access->setPassword($user->getDN(), $password)) { |
||
| 200 | $ldapDefaultPPolicyDN = $this->access->connection->ldapDefaultPPolicyDN; |
||
| 201 | $turnOnPasswordChange = $this->access->connection->turnOnPasswordChange; |
||
| 202 | if (!empty($ldapDefaultPPolicyDN) && (intval($turnOnPasswordChange) === 1)) { |
||
| 203 | //remove last password expiry warning if any |
||
| 204 | $notification = $this->notificationManager->createNotification(); |
||
| 205 | $notification->setApp('user_ldap') |
||
| 206 | ->setUser($uid) |
||
| 207 | ->setObject('pwd_exp_warn', $uid) |
||
| 208 | ; |
||
| 209 | $this->notificationManager->markProcessed($notification); |
||
| 210 | } |
||
| 211 | return true; |
||
| 212 | } |
||
| 213 | |||
| 214 | return false; |
||
| 215 | } |
||
| 216 | |||
| 217 | /** |
||
| 218 | * Get a list of all users |
||
| 219 | * |
||
| 220 | * @param string $search |
||
| 221 | * @param integer $limit |
||
| 222 | * @param integer $offset |
||
| 223 | * @return string[] an array of all uids |
||
| 224 | */ |
||
| 225 | public function getUsers($search = '', $limit = 10, $offset = 0) { |
||
| 260 | |||
| 261 | /** |
||
| 262 | * checks whether a user is still available on LDAP |
||
| 263 | * |
||
| 264 | * @param string|\OCA\User_LDAP\User\User $user either the Nextcloud user |
||
| 265 | * name or an instance of that user |
||
| 266 | * @return bool |
||
| 267 | * @throws \Exception |
||
| 268 | * @throws \OC\ServerNotAvailableException |
||
| 269 | */ |
||
| 270 | public function userExistsOnLDAP($user) { |
||
| 309 | |||
| 310 | /** |
||
| 311 | * check if a user exists |
||
| 312 | * @param string $uid the username |
||
| 313 | * @return boolean |
||
| 314 | * @throws \Exception when connection could not be established |
||
| 315 | */ |
||
| 316 | public function userExists($uid) { |
||
| 342 | |||
| 343 | /** |
||
| 344 | * returns whether a user was deleted in LDAP |
||
| 345 | * |
||
| 346 | * @param string $uid The username of the user to delete |
||
| 347 | * @return bool |
||
| 348 | */ |
||
| 349 | public function deleteUser($uid) { |
||
| 368 | |||
| 369 | /** |
||
| 370 | * get the user's home directory |
||
| 371 | * |
||
| 372 | * @param string $uid the username |
||
| 373 | * @return bool|string |
||
| 374 | * @throws NoUserException |
||
| 375 | * @throws \Exception |
||
| 376 | */ |
||
| 377 | public function getHome($uid) { |
||
| 409 | |||
| 410 | /** |
||
| 411 | * get display name of the user |
||
| 412 | * @param string $uid user ID of the user |
||
| 413 | * @return string|false display name |
||
| 414 | */ |
||
| 415 | public function getDisplayName($uid) { |
||
| 459 | |||
| 460 | /** |
||
| 461 | * Get a list of all display names |
||
| 462 | * |
||
| 463 | * @param string $search |
||
| 464 | * @param string|null $limit |
||
| 465 | * @param string|null $offset |
||
| 466 | * @return array an array of all displayNames (value) and the corresponding uids (key) |
||
| 467 | */ |
||
| 468 | public function getDisplayNames($search = '', $limit = null, $offset = null) { |
||
| 482 | |||
| 483 | /** |
||
| 484 | * Check if backend implements actions |
||
| 485 | * @param int $actions bitwise-or'ed actions |
||
| 486 | * @return boolean |
||
| 487 | * |
||
| 488 | * Returns the supported actions as int to be |
||
| 489 | * compared with OC_USER_BACKEND_CREATE_USER etc. |
||
| 490 | */ |
||
| 491 | public function implementsActions($actions) { |
||
| 500 | |||
| 501 | /** |
||
| 502 | * @return bool |
||
| 503 | */ |
||
| 504 | public function hasUserListings() { |
||
| 507 | |||
| 508 | /** |
||
| 509 | * counts the users in LDAP |
||
| 510 | * |
||
| 511 | * @return int|bool |
||
| 512 | */ |
||
| 513 | public function countUsers() { |
||
| 523 | |||
| 524 | /** |
||
| 525 | * Backend name to be shown in user management |
||
| 526 | * @return string the name of the backend to be shown |
||
| 527 | */ |
||
| 528 | public function getBackendName(){ |
||
| 531 | |||
| 532 | /** |
||
| 533 | * Return access for LDAP interaction. |
||
| 534 | * @param string $uid |
||
| 535 | * @return Access instance of Access for LDAP interaction |
||
| 536 | */ |
||
| 537 | public function getLDAPAccess($uid) { |
||
| 540 | |||
| 541 | /** |
||
| 542 | * Return LDAP connection resource from a cloned connection. |
||
| 543 | * The cloned connection needs to be closed manually. |
||
| 544 | * of the current access. |
||
| 545 | * @param string $uid |
||
| 546 | * @return resource of the LDAP connection |
||
| 547 | */ |
||
| 548 | public function getNewLDAPConnection($uid) { |
||
| 552 | } |
||
| 553 |
Loading history...
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.