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 LocationService 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 LocationService, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
43 | class LocationService implements LocationServiceInterface |
||
44 | { |
||
45 | /** |
||
46 | * @var \eZ\Publish\Core\Repository\Repository |
||
47 | */ |
||
48 | protected $repository; |
||
49 | |||
50 | /** |
||
51 | * @var \eZ\Publish\SPI\Persistence\Handler |
||
52 | */ |
||
53 | protected $persistenceHandler; |
||
54 | |||
55 | /** |
||
56 | * @var array |
||
57 | */ |
||
58 | protected $settings; |
||
59 | |||
60 | /** |
||
61 | * @var \eZ\Publish\Core\Repository\Helper\DomainMapper |
||
62 | */ |
||
63 | protected $domainMapper; |
||
64 | |||
65 | /** |
||
66 | * @var \eZ\Publish\Core\Repository\Helper\NameSchemaService |
||
67 | */ |
||
68 | protected $nameSchemaService; |
||
69 | |||
70 | /** |
||
71 | * @var \eZ\Publish\API\Repository\PermissionCriterionResolver |
||
72 | */ |
||
73 | protected $permissionCriterionResolver; |
||
74 | |||
75 | /** |
||
76 | * @var \Psr\Log\LoggerInterface |
||
77 | */ |
||
78 | private $logger; |
||
79 | |||
80 | /** |
||
81 | * Setups service with reference to repository object that created it & corresponding handler. |
||
82 | * |
||
83 | * @param \eZ\Publish\API\Repository\Repository $repository |
||
84 | * @param \eZ\Publish\SPI\Persistence\Handler $handler |
||
85 | * @param \eZ\Publish\Core\Repository\Helper\DomainMapper $domainMapper |
||
86 | * @param \eZ\Publish\Core\Repository\Helper\NameSchemaService $nameSchemaService |
||
87 | * @param \eZ\Publish\API\Repository\PermissionCriterionResolver $permissionCriterionResolver |
||
88 | * @param array $settings |
||
89 | * @param \Psr\Log\LoggerInterface|null $logger |
||
90 | */ |
||
91 | public function __construct( |
||
111 | |||
112 | /** |
||
113 | * Copies the subtree starting from $subtree as a new subtree of $targetLocation. |
||
114 | * |
||
115 | * Only the items on which the user has read access are copied. |
||
116 | * |
||
117 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed copy the subtree to the given parent location |
||
118 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user does not have read access to the whole source subtree |
||
119 | * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if the target location is a sub location of the given location |
||
120 | * |
||
121 | * @param \eZ\Publish\API\Repository\Values\Content\Location $subtree - the subtree denoted by the location to copy |
||
122 | * @param \eZ\Publish\API\Repository\Values\Content\Location $targetParentLocation - the target parent location for the copy operation |
||
123 | * |
||
124 | * @return \eZ\Publish\API\Repository\Values\Content\Location The newly created location of the copied subtree |
||
125 | */ |
||
126 | public function copySubtree(APILocation $subtree, APILocation $targetParentLocation) |
||
127 | { |
||
128 | $loadedSubtree = $this->loadLocation($subtree->id); |
||
129 | $loadedTargetLocation = $this->loadLocation($targetParentLocation->id); |
||
130 | |||
131 | if (stripos($loadedTargetLocation->pathString, $loadedSubtree->pathString) !== false) { |
||
132 | throw new InvalidArgumentException('targetParentLocation', 'target parent location is a sub location of the given subtree'); |
||
133 | } |
||
134 | |||
135 | // check create permission on target |
||
136 | if (!$this->repository->canUser('content', 'create', $loadedSubtree->getContentInfo(), $loadedTargetLocation)) { |
||
137 | throw new UnauthorizedException('content', 'create', ['locationId' => $loadedTargetLocation->id]); |
||
138 | } |
||
139 | |||
140 | /** Check read access to whole source subtree |
||
141 | * @var bool|\eZ\Publish\API\Repository\Values\Content\Query\Criterion |
||
142 | */ |
||
143 | $contentReadCriterion = $this->permissionCriterionResolver->getPermissionsCriterion(); |
||
144 | View Code Duplication | if ($contentReadCriterion === false) { |
|
145 | throw new UnauthorizedException('content', 'read'); |
||
146 | } elseif ($contentReadCriterion !== true) { |
||
147 | // Query if there are any content in subtree current user don't have access to |
||
148 | $query = new Query( |
||
149 | array( |
||
150 | 'limit' => 0, |
||
151 | 'filter' => new CriterionLogicalAnd( |
||
152 | array( |
||
153 | new CriterionSubtree($loadedSubtree->pathString), |
||
154 | new CriterionLogicalNot($contentReadCriterion), |
||
155 | ) |
||
156 | ), |
||
157 | ) |
||
158 | ); |
||
159 | $result = $this->repository->getSearchService()->findContent($query, array(), false); |
||
160 | if ($result->totalCount > 0) { |
||
161 | throw new UnauthorizedException('content', 'read'); |
||
162 | } |
||
163 | } |
||
164 | |||
165 | $this->repository->beginTransaction(); |
||
166 | try { |
||
167 | $newLocation = $this->persistenceHandler->locationHandler()->copySubtree( |
||
168 | $loadedSubtree->id, |
||
169 | $loadedTargetLocation->id, |
||
170 | $this->repository->getPermissionResolver()->getCurrentUserReference()->getUserId() |
||
171 | ); |
||
172 | |||
173 | $content = $this->repository->getContentService()->loadContent($newLocation->contentId); |
||
174 | $urlAliasNames = $this->nameSchemaService->resolveUrlAliasSchema($content); |
||
175 | View Code Duplication | foreach ($urlAliasNames as $languageCode => $name) { |
|
176 | $this->persistenceHandler->urlAliasHandler()->publishUrlAliasForLocation( |
||
177 | $newLocation->id, |
||
178 | $loadedTargetLocation->id, |
||
179 | $name, |
||
180 | $languageCode, |
||
181 | $content->contentInfo->alwaysAvailable |
||
182 | ); |
||
183 | } |
||
184 | |||
185 | $this->persistenceHandler->urlAliasHandler()->locationCopied( |
||
186 | $loadedSubtree->id, |
||
187 | $newLocation->id, |
||
188 | $loadedTargetLocation->id |
||
189 | ); |
||
190 | |||
191 | $this->repository->commit(); |
||
192 | } catch (Exception $e) { |
||
193 | $this->repository->rollback(); |
||
194 | throw $e; |
||
195 | } |
||
196 | |||
197 | return $this->domainMapper->buildLocationDomainObject($newLocation); |
||
198 | } |
||
199 | |||
200 | /** |
||
201 | * Loads a location object from its $locationId. |
||
202 | * |
||
203 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to read this location |
||
204 | * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException If the specified location is not found |
||
205 | * |
||
206 | * @param mixed $locationId |
||
207 | * |
||
208 | * @return \eZ\Publish\API\Repository\Values\Content\Location |
||
209 | */ |
||
210 | public function loadLocation($locationId) |
||
220 | |||
221 | /** |
||
222 | * Loads a location object from its $remoteId. |
||
223 | * |
||
224 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to read this location |
||
225 | * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException If more than one location with same remote ID was found |
||
226 | * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException If the specified location is not found |
||
227 | * |
||
228 | * @param string $remoteId |
||
229 | * |
||
230 | * @return \eZ\Publish\API\Repository\Values\Content\Location |
||
231 | */ |
||
232 | public function loadLocationByRemoteId($remoteId) |
||
246 | |||
247 | /** |
||
248 | * {@inheritdoc} |
||
249 | */ |
||
250 | public function loadLocations(ContentInfo $contentInfo, APILocation $rootLocation = null) |
||
251 | { |
||
252 | if (!$contentInfo->published) { |
||
253 | throw new BadStateException('$contentInfo', 'ContentInfo has no published versions'); |
||
254 | } |
||
255 | |||
256 | $spiLocations = $this->persistenceHandler->locationHandler()->loadLocationsByContent( |
||
257 | $contentInfo->id, |
||
258 | $rootLocation !== null ? $rootLocation->id : null |
||
259 | ); |
||
260 | |||
261 | $locations = []; |
||
262 | foreach ($spiLocations as $spiLocation) { |
||
263 | $location = $this->domainMapper->buildLocationDomainObject($spiLocation); |
||
264 | if ($this->repository->canUser('content', 'read', $location->getContentInfo(), $location)) { |
||
265 | $locations[] = $location; |
||
266 | } |
||
267 | } |
||
268 | |||
269 | return $locations; |
||
270 | } |
||
271 | |||
272 | /** |
||
273 | * Loads children which are readable by the current user of a location object sorted by sortField and sortOrder. |
||
274 | * |
||
275 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
276 | * @param int $offset the start offset for paging |
||
277 | * @param int $limit the number of locations returned |
||
278 | * |
||
279 | * @return \eZ\Publish\API\Repository\Values\Content\LocationList |
||
280 | */ |
||
281 | public function loadLocationChildren(APILocation $location, $offset = 0, $limit = 25) |
||
312 | |||
313 | /** |
||
314 | * Returns the number of children which are readable by the current user of a location object. |
||
315 | * |
||
316 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
317 | * |
||
318 | * @return int |
||
319 | */ |
||
320 | public function getLocationChildCount(APILocation $location) |
||
326 | |||
327 | /** |
||
328 | * Searches children locations of the provided parent location id. |
||
329 | * |
||
330 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
331 | * @param int $offset |
||
332 | * @param int $limit |
||
333 | * |
||
334 | * @return \eZ\Publish\API\Repository\Values\Content\Search\SearchResult |
||
335 | */ |
||
336 | protected function searchChildrenLocations(APILocation $location, $offset = 0, $limit = -1) |
||
347 | |||
348 | /** |
||
349 | * Creates the new $location in the content repository for the given content. |
||
350 | * |
||
351 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to create this location |
||
352 | * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if the content is already below the specified parent |
||
353 | * or the parent is a sub location of the location of the content |
||
354 | * or if set the remoteId exists already |
||
355 | * |
||
356 | * @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo |
||
357 | * @param \eZ\Publish\API\Repository\Values\Content\LocationCreateStruct $locationCreateStruct |
||
358 | * |
||
359 | * @return \eZ\Publish\API\Repository\Values\Content\Location the newly created Location |
||
360 | */ |
||
361 | public function createLocation(ContentInfo $contentInfo, LocationCreateStruct $locationCreateStruct) |
||
428 | |||
429 | /** |
||
430 | * Updates $location in the content repository. |
||
431 | * |
||
432 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to update this location |
||
433 | * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if if set the remoteId exists already |
||
434 | * |
||
435 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
436 | * @param \eZ\Publish\API\Repository\Values\Content\LocationUpdateStruct $locationUpdateStruct |
||
437 | * |
||
438 | * @return \eZ\Publish\API\Repository\Values\Content\Location the updated Location |
||
439 | */ |
||
440 | public function updateLocation(APILocation $location, LocationUpdateStruct $locationUpdateStruct) |
||
441 | { |
||
442 | if ($locationUpdateStruct->priority !== null && !is_int($locationUpdateStruct->priority)) { |
||
443 | throw new InvalidArgumentValue('priority', $locationUpdateStruct->priority, 'LocationUpdateStruct'); |
||
444 | } |
||
445 | |||
446 | View Code Duplication | if ($locationUpdateStruct->remoteId !== null && (!is_string($locationUpdateStruct->remoteId) || empty($locationUpdateStruct->remoteId))) { |
|
447 | throw new InvalidArgumentValue('remoteId', $locationUpdateStruct->remoteId, 'LocationUpdateStruct'); |
||
448 | } |
||
449 | |||
450 | if ($locationUpdateStruct->sortField !== null && !$this->domainMapper->isValidLocationSortField($locationUpdateStruct->sortField)) { |
||
451 | throw new InvalidArgumentValue('sortField', $locationUpdateStruct->sortField, 'LocationUpdateStruct'); |
||
452 | } |
||
453 | |||
454 | if ($locationUpdateStruct->sortOrder !== null && !$this->domainMapper->isValidLocationSortOrder($locationUpdateStruct->sortOrder)) { |
||
455 | throw new InvalidArgumentValue('sortOrder', $locationUpdateStruct->sortOrder, 'LocationUpdateStruct'); |
||
456 | } |
||
457 | |||
458 | $loadedLocation = $this->loadLocation($location->id); |
||
459 | |||
460 | if ($locationUpdateStruct->remoteId !== null) { |
||
461 | try { |
||
462 | $existingLocation = $this->loadLocationByRemoteId($locationUpdateStruct->remoteId); |
||
463 | if ($existingLocation !== null && $existingLocation->id !== $loadedLocation->id) { |
||
464 | throw new InvalidArgumentException('locationUpdateStruct', 'location with provided remote ID already exists'); |
||
465 | } |
||
466 | } catch (APINotFoundException $e) { |
||
467 | } |
||
468 | } |
||
469 | |||
470 | if (!$this->repository->canUser('content', 'edit', $loadedLocation->getContentInfo(), $loadedLocation)) { |
||
471 | throw new UnauthorizedException('content', 'edit', ['locationId' => $loadedLocation->id]); |
||
472 | } |
||
473 | |||
474 | $updateStruct = new UpdateStruct(); |
||
475 | $updateStruct->priority = $locationUpdateStruct->priority !== null ? $locationUpdateStruct->priority : $loadedLocation->priority; |
||
476 | $updateStruct->remoteId = $locationUpdateStruct->remoteId !== null ? trim($locationUpdateStruct->remoteId) : $loadedLocation->remoteId; |
||
477 | $updateStruct->sortField = $locationUpdateStruct->sortField !== null ? $locationUpdateStruct->sortField : $loadedLocation->sortField; |
||
478 | $updateStruct->sortOrder = $locationUpdateStruct->sortOrder !== null ? $locationUpdateStruct->sortOrder : $loadedLocation->sortOrder; |
||
479 | |||
480 | $this->repository->beginTransaction(); |
||
481 | try { |
||
482 | $this->persistenceHandler->locationHandler()->update($updateStruct, $loadedLocation->id); |
||
483 | $this->repository->commit(); |
||
484 | } catch (Exception $e) { |
||
485 | $this->repository->rollback(); |
||
486 | throw $e; |
||
487 | } |
||
488 | |||
489 | return $this->loadLocation($loadedLocation->id); |
||
490 | } |
||
491 | |||
492 | /** |
||
493 | * Swaps the contents held by $location1 and $location2. |
||
494 | * |
||
495 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to swap content |
||
496 | * |
||
497 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location1 |
||
498 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location2 |
||
499 | */ |
||
500 | public function swapLocation(APILocation $location1, APILocation $location2) |
||
527 | |||
528 | /** |
||
529 | * Hides the $location and marks invisible all descendants of $location. |
||
530 | * |
||
531 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to hide this location |
||
532 | * |
||
533 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
534 | * |
||
535 | * @return \eZ\Publish\API\Repository\Values\Content\Location $location, with updated hidden value |
||
536 | */ |
||
537 | View Code Duplication | public function hideLocation(APILocation $location) |
|
554 | |||
555 | /** |
||
556 | * Unhides the $location. |
||
557 | * |
||
558 | * This method and marks visible all descendants of $locations |
||
559 | * until a hidden location is found. |
||
560 | * |
||
561 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to unhide this location |
||
562 | * |
||
563 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
564 | * |
||
565 | * @return \eZ\Publish\API\Repository\Values\Content\Location $location, with updated hidden value |
||
566 | */ |
||
567 | View Code Duplication | public function unhideLocation(APILocation $location) |
|
584 | |||
585 | /** |
||
586 | * Moves the subtree to $newParentLocation. |
||
587 | * |
||
588 | * If a user has the permission to move the location to a target location |
||
589 | * he can do it regardless of an existing descendant on which the user has no permission. |
||
590 | * |
||
591 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user is not allowed to move this location to the target |
||
592 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user user does not have read access to the whole source subtree |
||
593 | * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException If the new parent is in a subtree of the location |
||
594 | * |
||
595 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
596 | * @param \eZ\Publish\API\Repository\Values\Content\Location $newParentLocation |
||
597 | */ |
||
598 | public function moveSubtree(APILocation $location, APILocation $newParentLocation) |
||
668 | |||
669 | /** |
||
670 | * Deletes $location and all its descendants. |
||
671 | * |
||
672 | * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException If the current user is not allowed to delete this location or a descendant |
||
673 | * |
||
674 | * @param \eZ\Publish\API\Repository\Values\Content\Location $location |
||
675 | */ |
||
676 | public function deleteLocation(APILocation $location) |
||
722 | |||
723 | /** |
||
724 | * Instantiates a new location create class. |
||
725 | * |
||
726 | * @param mixed $parentLocationId the parent under which the new location should be created |
||
727 | * @param eZ\Publish\API\Repository\Values\ContentType\ContentType|null $contentType |
||
728 | * |
||
729 | * @return \eZ\Publish\API\Repository\Values\Content\LocationCreateStruct |
||
730 | */ |
||
731 | View Code Duplication | public function newLocationCreateStruct($parentLocationId, ContentType $contentType = null) |
|
743 | |||
744 | /** |
||
745 | * Instantiates a new location update class. |
||
746 | * |
||
747 | * @return \eZ\Publish\API\Repository\Values\Content\LocationUpdateStruct |
||
748 | */ |
||
749 | public function newLocationUpdateStruct() |
||
753 | |||
754 | /** |
||
755 | * Get the total number of all existing Locations. Can be combined with loadAllLocations. |
||
756 | * |
||
757 | * @see loadAllLocations |
||
758 | * |
||
759 | * @return int Total number of Locations |
||
760 | */ |
||
761 | public function getAllLocationsCount() |
||
765 | |||
766 | /** |
||
767 | * Bulk-load all existing Locations, constrained by $limit and $offset to paginate results. |
||
768 | * |
||
769 | * @param int $offset |
||
770 | * @param int $limit |
||
771 | * |
||
772 | * @return \eZ\Publish\API\Repository\Values\Content\Location[] |
||
773 | * |
||
774 | * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException |
||
775 | * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException |
||
776 | */ |
||
777 | public function loadAllLocations($offset = 0, $limit = 25) |
||
823 | } |
||
824 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a given class or a super-class is assigned to a property that is type hinted more strictly.
Either this assignment is in error or an instanceof check should be added for that assignment.