nextcloud /
circles
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 MembersService 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 MembersService, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 60 | class MembersService { |
||
| 61 | |||
| 62 | /** @var string */ |
||
| 63 | private $userId; |
||
| 64 | |||
| 65 | /** @var IL10N */ |
||
| 66 | private $l10n; |
||
| 67 | |||
| 68 | /** @var IUserManager */ |
||
| 69 | private $userManager; |
||
| 70 | |||
| 71 | /** @var ConfigService */ |
||
| 72 | private $configService; |
||
| 73 | |||
| 74 | /** @var CirclesRequest */ |
||
| 75 | private $circlesRequest; |
||
| 76 | |||
| 77 | /** @var MembersRequest */ |
||
| 78 | private $membersRequest; |
||
| 79 | |||
| 80 | /** @var SharesRequest */ |
||
| 81 | private $sharesRequest; |
||
| 82 | |||
| 83 | /** @var TokensRequest */ |
||
| 84 | private $tokensRequest; |
||
| 85 | |||
| 86 | /** @var CirclesService */ |
||
| 87 | private $circlesService; |
||
| 88 | |||
| 89 | /** @var EventsService */ |
||
| 90 | private $eventsService; |
||
| 91 | |||
| 92 | /** @var GSUpstreamService */ |
||
| 93 | private $gsUpstreamService; |
||
| 94 | |||
| 95 | /** @var FileSharingBroadcaster */ |
||
| 96 | private $fileSharingBroadcaster; |
||
| 97 | |||
| 98 | /** @var MiscService */ |
||
| 99 | private $miscService; |
||
| 100 | |||
| 101 | /** |
||
| 102 | * MembersService constructor. |
||
| 103 | * |
||
| 104 | * @param string $userId |
||
| 105 | * @param IL10N $l10n |
||
| 106 | * @param IUserManager $userManager |
||
| 107 | * @param ConfigService $configService |
||
| 108 | * @param CirclesRequest $circlesRequest |
||
| 109 | * @param MembersRequest $membersRequest |
||
| 110 | * @param SharesRequest $sharesRequest |
||
| 111 | * @param TokensRequest $tokensRequest |
||
| 112 | * @param CirclesService $circlesService |
||
| 113 | * @param EventsService $eventsService |
||
| 114 | * @param FileSharingBroadcaster $fileSharingBroadcaster |
||
| 115 | * @param MiscService $miscService |
||
| 116 | */ |
||
| 117 | View Code Duplication | public function __construct( |
|
|
|
|||
| 118 | $userId, IL10N $l10n, IUserManager $userManager, ConfigService $configService, |
||
| 119 | CirclesRequest $circlesRequest, MembersRequest $membersRequest, SharesRequest $sharesRequest, |
||
| 120 | TokensRequest $tokensRequest, CirclesService $circlesService, EventsService $eventsService, |
||
| 121 | GSUpstreamService $gsUpstreamService, FileSharingBroadcaster $fileSharingBroadcaster, |
||
| 122 | MiscService $miscService |
||
| 123 | ) { |
||
| 124 | $this->userId = $userId; |
||
| 125 | $this->l10n = $l10n; |
||
| 126 | $this->userManager = $userManager; |
||
| 127 | $this->configService = $configService; |
||
| 128 | $this->circlesRequest = $circlesRequest; |
||
| 129 | $this->membersRequest = $membersRequest; |
||
| 130 | $this->sharesRequest = $sharesRequest; |
||
| 131 | $this->tokensRequest = $tokensRequest; |
||
| 132 | $this->circlesService = $circlesService; |
||
| 133 | $this->eventsService = $eventsService; |
||
| 134 | $this->gsUpstreamService = $gsUpstreamService; |
||
| 135 | $this->fileSharingBroadcaster = $fileSharingBroadcaster; |
||
| 136 | $this->miscService = $miscService; |
||
| 137 | } |
||
| 138 | |||
| 139 | |||
| 140 | /** |
||
| 141 | * addMember(); |
||
| 142 | * |
||
| 143 | * add a new member to a circle. |
||
| 144 | * |
||
| 145 | * @param string $circleUniqueId |
||
| 146 | * @param $ident |
||
| 147 | * @param int $type |
||
| 148 | * @param string $instance |
||
| 149 | * |
||
| 150 | * @param bool $force |
||
| 151 | * |
||
| 152 | * @return array |
||
| 153 | * @throws Exception |
||
| 154 | */ |
||
| 155 | public function addMember($circleUniqueId, $ident, $type, string $instance, bool $force = false) { |
||
| 156 | View Code Duplication | if ($force === true) { |
|
| 157 | $circle = $this->circlesRequest->forceGetCircle($circleUniqueId); |
||
| 158 | } else { |
||
| 159 | $circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId); |
||
| 160 | $circle->getHigherViewer() |
||
| 161 | ->hasToBeModerator(); |
||
| 162 | } |
||
| 163 | |||
| 164 | if (!$this->addMassiveMembers($circle, $ident, $type)) { |
||
| 165 | $this->addSingleMember($circle, $ident, $type, $instance); |
||
| 166 | } |
||
| 167 | |||
| 168 | return $this->membersRequest->getMembers($circle->getUniqueId(), $circle->getHigherViewer(), $force); |
||
| 169 | } |
||
| 170 | |||
| 171 | |||
| 172 | /** |
||
| 173 | * add a single member to a circle. |
||
| 174 | * |
||
| 175 | * @param Circle $circle |
||
| 176 | * @param string $ident |
||
| 177 | * @param int $type |
||
| 178 | * |
||
| 179 | * @param string $instance |
||
| 180 | * |
||
| 181 | * @throws Exception |
||
| 182 | */ |
||
| 183 | private function addSingleMember(Circle $circle, $ident, $type, $instance = '') { |
||
| 184 | |||
| 185 | $this->verifyIdentBasedOnItsType($ident, $type, $instance); |
||
| 186 | |||
| 187 | $member = $this->membersRequest->getFreshNewMember($circle->getUniqueId(), $ident, $type, $instance); |
||
| 188 | // $this->membersRequest->createMember($member); |
||
| 189 | // $member->hasToBeInviteAble(); |
||
| 190 | |||
| 191 | // $this->circlesService->checkThatCircleIsNotFull($circle); |
||
| 192 | |||
| 193 | // $this->addMemberBasedOnItsType($circle, $member); |
||
| 194 | |||
| 195 | $event = new GSevent(GSEvent::MEMBER_ADD); |
||
| 196 | $event->setSeverity(GSEvent::SEVERITY_HIGH); |
||
| 197 | $event->setCircle($circle); |
||
| 198 | $event->setMember($member); |
||
| 199 | $this->gsUpstreamService->newEvent($event); |
||
| 200 | |||
| 201 | // $this->membersRequest->updateMember($member); |
||
| 202 | // $this->fileSharingBroadcaster->sendMailAboutExistingShares($circle, $member); |
||
| 203 | // $this->eventsService->onMemberNew($circle, $member); |
||
| 204 | |||
| 205 | } |
||
| 206 | |||
| 207 | |||
| 208 | /** |
||
| 209 | * add a bunch of users to a circle based on the type of the 'bunch' |
||
| 210 | * |
||
| 211 | * @param Circle $circle |
||
| 212 | * @param string $ident |
||
| 213 | * @param int $type |
||
| 214 | * |
||
| 215 | * @return bool |
||
| 216 | * @throws Exception |
||
| 217 | */ |
||
| 218 | private function addMassiveMembers(Circle $circle, $ident, $type) { |
||
| 219 | if ($type === Member::TYPE_GROUP) { |
||
| 220 | return $this->addGroupMembers($circle, $ident); |
||
| 221 | } |
||
| 222 | |||
| 223 | if ($type === Member::TYPE_USER) { |
||
| 224 | return $this->addMassiveMails($circle, $ident); |
||
| 225 | } |
||
| 226 | |||
| 227 | return false; |
||
| 228 | } |
||
| 229 | |||
| 230 | |||
| 231 | /** |
||
| 232 | * add a new member based on its type. |
||
| 233 | * |
||
| 234 | * @param Circle $circle |
||
| 235 | * @param Member $member |
||
| 236 | * |
||
| 237 | * @throws CircleTypeNotValidException |
||
| 238 | * @throws MemberCantJoinCircleException |
||
| 239 | */ |
||
| 240 | public function addMemberBasedOnItsType(Circle $circle, Member &$member) { |
||
| 245 | |||
| 246 | |||
| 247 | /** |
||
| 248 | * @param Circle $circle |
||
| 249 | * @param Member $member |
||
| 250 | * |
||
| 251 | * @throws CircleTypeNotValidException |
||
| 252 | * @throws MemberCantJoinCircleException |
||
| 253 | */ |
||
| 254 | private function addLocalMember(Circle $circle, Member $member) { |
||
| 266 | |||
| 267 | |||
| 268 | /** |
||
| 269 | * add mail address as contact. |
||
| 270 | * |
||
| 271 | * @param Member $member |
||
| 272 | */ |
||
| 273 | private function addEmailAddress(Member $member) { |
||
| 281 | |||
| 282 | |||
| 283 | /** |
||
| 284 | * Add contact as member. |
||
| 285 | * |
||
| 286 | * @param Member $member |
||
| 287 | */ |
||
| 288 | private function addContact(Member $member) { |
||
| 296 | |||
| 297 | |||
| 298 | /** |
||
| 299 | * Verify the availability of an ident, based on its type. |
||
| 300 | * |
||
| 301 | * @param string $ident |
||
| 302 | * @param int $type |
||
| 303 | * @param string $instance |
||
| 304 | * |
||
| 305 | * @throws EmailAccountInvalidFormatException |
||
| 306 | * @throws NoUserException |
||
| 307 | */ |
||
| 308 | public function verifyIdentBasedOnItsType(&$ident, $type, string $instance = '') { |
||
| 313 | |||
| 314 | |||
| 315 | /** |
||
| 316 | * Verify if a local account is valid. |
||
| 317 | * |
||
| 318 | * @param $ident |
||
| 319 | * @param $type |
||
| 320 | * |
||
| 321 | * @param string $instance |
||
| 322 | * |
||
| 323 | * @throws NoUserException |
||
| 324 | */ |
||
| 325 | private function verifyIdentLocalMember(&$ident, $type, string $instance = '') { |
||
| 338 | |||
| 339 | |||
| 340 | /** |
||
| 341 | * Verify if a mail have a valid format. |
||
| 342 | * |
||
| 343 | * @param $ident |
||
| 344 | * @param $type |
||
| 345 | * |
||
| 346 | * @throws EmailAccountInvalidFormatException |
||
| 347 | */ |
||
| 348 | private function verifyIdentEmailAddress(&$ident, $type) { |
||
| 366 | |||
| 367 | |||
| 368 | /** |
||
| 369 | * Verify if a contact exist in current user address books. |
||
| 370 | * |
||
| 371 | * @param $ident |
||
| 372 | * @param $type |
||
| 373 | * |
||
| 374 | * @throws NoUserException |
||
| 375 | * @throws EmailAccountInvalidFormatException |
||
| 376 | */ |
||
| 377 | private function verifyIdentContact(&$ident, $type) { |
||
| 396 | |||
| 397 | |||
| 398 | /** |
||
| 399 | * @param Circle $circle |
||
| 400 | * @param string $groupId |
||
| 401 | * |
||
| 402 | * @return bool |
||
| 403 | * @throws Exception |
||
| 404 | */ |
||
| 405 | private function addGroupMembers(Circle $circle, $groupId) { |
||
| 424 | |||
| 425 | |||
| 426 | /** |
||
| 427 | * @param Circle $circle |
||
| 428 | * @param string $mails |
||
| 429 | * |
||
| 430 | * @return bool |
||
| 431 | */ |
||
| 432 | private function addMassiveMails(Circle $circle, $mails) { |
||
| 453 | |||
| 454 | |||
| 455 | /** |
||
| 456 | * getMember(); |
||
| 457 | * |
||
| 458 | * Will return any data of a user related to a circle (as a Member). User can be a 'non-member' |
||
| 459 | * Viewer needs to be at least Member of the Circle |
||
| 460 | * |
||
| 461 | * @param $circleId |
||
| 462 | * @param $userId |
||
| 463 | * @param $type |
||
| 464 | * @param bool $forceAll |
||
| 465 | * |
||
| 466 | * @return Member |
||
| 467 | * @throws CircleDoesNotExistException |
||
| 468 | * @throws ConfigNoCircleAvailableException |
||
| 469 | * @throws MemberDoesNotExistException |
||
| 470 | */ |
||
| 471 | public function getMember($circleId, $userId, $type, $forceAll = false) { |
||
| 483 | |||
| 484 | |||
| 485 | /** |
||
| 486 | * @param string $memberId |
||
| 487 | * |
||
| 488 | * @return Member |
||
| 489 | * @throws MemberDoesNotExistException |
||
| 490 | */ |
||
| 491 | public function getMemberById(string $memberId): Member { |
||
| 494 | |||
| 495 | |||
| 496 | /** |
||
| 497 | * @param string $circleUniqueId |
||
| 498 | * @param string $name |
||
| 499 | * @param int $type |
||
| 500 | * @param int $level |
||
| 501 | * @param bool $force |
||
| 502 | * |
||
| 503 | * @return array |
||
| 504 | * @throws CircleDoesNotExistException |
||
| 505 | * @throws CircleTypeNotValidException |
||
| 506 | * @throws ConfigNoCircleAvailableException |
||
| 507 | * @throws MemberDoesNotExistException |
||
| 508 | * @throws MemberTypeCantEditLevelException |
||
| 509 | * @throws Exception |
||
| 510 | */ |
||
| 511 | public function levelMember($circleUniqueId, $name, $type, $instance, $level, bool $force = false) { |
||
| 551 | |||
| 552 | |||
| 553 | /** |
||
| 554 | * @param string $circleUniqueId |
||
| 555 | * @param string $name |
||
| 556 | * @param int $type |
||
| 557 | * @param string $instance |
||
| 558 | * @param bool $force |
||
| 559 | * |
||
| 560 | * @return Member[] |
||
| 561 | * @throws CircleDoesNotExistException |
||
| 562 | * @throws ConfigNoCircleAvailableException |
||
| 563 | * @throws MemberDoesNotExistException |
||
| 564 | * @throws MemberIsNotModeratorException |
||
| 565 | * @throws Exception |
||
| 566 | */ |
||
| 567 | public function removeMember(string $circleUniqueId, string $name, int $type, string $instance, bool $force = false): array { |
||
| 604 | |||
| 605 | |||
| 606 | /** |
||
| 607 | * When a user is removed, remove him from all Circles |
||
| 608 | * |
||
| 609 | * @param $userId |
||
| 610 | */ |
||
| 611 | public function onUserRemoved($userId) { |
||
| 615 | |||
| 616 | |||
| 617 | } |
||
| 618 | |||
| 619 |
Loading history...
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.