TrashManager - Code Metrics - Inspection of "more fixes and tests for trash handling" - kaliop-uk/ezmigrationbundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( ae10c9...492c7b )

by Gaetano

created 2017-10-29 00:11 UTC

TrashManager B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	200
Duplicated Lines	53.5 %

Coupling/Cohesion

Components	1
Dependencies	11

Importance

Changes

Metric	Value
wmc	41
lcom	1
cbo	11
dl	107
loc	200
c	0
b	0
f	0
rs	8.2769

6 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	5	1
A	purge()	0	8	1
A	recover()	0	18	4
A	delete()	0	19	4
A	matchItems()	11	11	2
D	setReferences()	96	96	29

How to fix Duplicated Code Complexity

Duplicated Code

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like TrashManager 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 TrashManager, and based on these observations, apply Extract Interface, too.

<?php

namespace Kaliop\eZMigrationBundle\Core\Executor;

use Kaliop\eZMigrationBundle\API\Collection\LocationCollection;
use Kaliop\eZMigrationBundle\API\Collection\TrashedItemCollection;
use Kaliop\eZMigrationBundle\Core\Matcher\TrashMatcher;
use Kaliop\eZMigrationBundle\Core\Helper\SortConverter;

/**
 * Handles trash migrations.
 */
class TrashManager extends RepositoryExecutor
{
    protected $supportedActions = array('purge', 'recover', 'delete');
    protected $supportedStepTypes = array('trash');

    /** @var TrashMatcher $trashMatcher */
    protected $trashMatcher;

    protected $sortConverter;

    /**
     * @param TrashMatcher $trashMatcher
     */
    public function __construct(TrashMatcher $trashMatcher, SortConverter $sortConverter)
    {
        $this->trashMatcher = $trashMatcher;
        $this->sortConverter = $sortConverter;
    }

    /**
     * Handles emptying the trash
     */
    protected function purge($step)

    {
        $trashService = $this->repository->getTrashService();

        $trashService->emptyTrash();

        return true;
    }

    /**
     * Handles the trash-restore migration action
     *
     * @todo support handling of restoration to custom locations
     */
    protected function recover($step)
    {
        $itemsCollection = $this->matchItems('restore', $step);

        if (count($itemsCollection) > 1 && array_key_exists('references', $step->dsl)) {
            throw new \Exception("Can not execute Trash restore because multiple types match, and a references section is specified in the dsl. References can be set when only 1 section matches");
        }

        $locations = array();
        $trashService = $this->repository->getTrashService();
        foreach ($itemsCollection as $key => $item) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
            $locations[] = $trashService->recover($item);
        }

        $this->setReferences(new LocationCollection($locations), $step);

        return $itemsCollection;
    }

    /**
     * Handles the trash-delete migration action
     */
    protected function delete($step)
    {
        $itemsCollection = $this->matchItems('delete', $step);

        if (count($itemsCollection) > 1 && array_key_exists('references', $step->dsl)) {
            throw new \Exception("Can not execute Trash restore because multiple types match, and a references section is specified in the dsl. References can be set when only 1 section matches");
        }

        $this->setReferences($itemsCollection, $step);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        $trashService = $this->repository->getTrashService();
        foreach ($itemsCollection as $key => $item) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
            $trashService->deleteTrashItem($item);
        }

        $this->setReferences($itemsCollection, $step);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        return $itemsCollection;
    }

    /**
     * @param string $action
     * @return TrashedItemCollection
     * @throws \Exception
     */
    protected function matchItems($action, $step)

    {
        if (!isset($step->dsl['match'])) {
            throw new \Exception("A match condition is required to $action trash items");
        }

        // convert the references passed in the match
        $match = $this->resolveReferencesRecursively($step->dsl['match']);


        return $this->trashMatcher->match($match);
    }

    /**
     * Sets references to certain trashed-item attributes.
     *
     * @param \eZ\Publish\API\Repository\Values\Content\TrashItem|TrashedItemCollection|\eZ\Publish\API\Repository\Values\Content\Location|LocationCollection $item
     * @param $step
     * @throws \InvalidArgumentException When trying to set a reference to an unsupported attribute
     * @return boolean
     */
    protected function setReferences($item, $step)

    {
        if (!array_key_exists('references', $step->dsl)) {
            return false;
        }

        $references = $this->setReferencesCommon($item, $step->dsl['references']);
        $item = $this->insureSingleEntity($item, $references);

        foreach ($references as $reference) {
            switch ($reference['attribute']) {
                // a trashed item extends a location, so in theory everything 'location' here should work
                case 'location_id':
                case 'id':
                    $value = $item->id;
                    break;
                case 'remote_id':
                case 'location_remote_id':
                    $value = $item->remoteId;
                    break;
                case 'always_available':
                    $value = $item->contentInfo->alwaysAvailable;
                    break;
                case 'content_id':
                    $value = $item->contentId;
                    break;
                case 'content_type_id':
                    $value = $item->contentInfo->contentTypeId;
                    break;
                case 'content_type_identifier':
                    $contentTypeService = $this->repository->getContentTypeService();
                    $value = $contentTypeService->loadContentType($item->contentInfo->contentTypeId)->identifier;
                    break;
                case 'current_version':
                case 'current_version_no':
                    $value = $item->contentInfo->currentVersionNo;
                    break;
                case 'depth':
                    $value = $item->depth;
                    break;
                case 'is_hidden':
                    $value = $item->hidden;
                    break;
                case 'main_location_id':
                    $value = $item->contentInfo->mainLocationId;
                    break;
                case 'main_language_code':
                    $value = $item->contentInfo->mainLanguageCode;
                    break;
                case 'modification_date':
                    $value = $item->contentInfo->modificationDate->getTimestamp();
                    break;
                case 'name':
                    $value = $item->contentInfo->name;
                    break;
                case 'owner_id':
                    $value = $item->contentInfo->ownerId;
                    break;
                case 'parent_location_id':
                    $value = $item->parentLocationId;
                    break;
                case 'path':
                    $value = $item->pathString;
                    break;
                case 'priority':
                    $value = $item->priority;
                    break;
                case 'publication_date':
                    $value = $item->contentInfo->publishedDate->getTimestamp();
                    break;
                case 'section_id':
                    $value = $item->contentInfo->sectionId;
                    break;
                case 'section_identifier':
                    $sectionService = $this->repository->getSectionService();
                    $value = $sectionService->loadSection($item->contentInfo->sectionId)->identifier;
                    break;
                case 'sort_field':
                    $value = $this->sortConverter->sortField2Hash($item->sortField);
                    break;
                case 'sort_order':
                    $value = $this->sortConverter->sortOrder2Hash($item->sortOrder);
                    break;
                default:
                    throw new \InvalidArgumentException('Trash Manager does not support setting references for attribute ' . $reference['attribute']);
            }

            $overwrite = false;
            if (isset($reference['overwrite'])) {
                $overwrite = $reference['overwrite'];
            }
            $this->referenceResolver->addReference($reference['identifier'], $value, $overwrite);
        }

        return true;
    }
}


1		<?php
2
3		namespace Kaliop\eZMigrationBundle\Core\Executor;
4
5		use Kaliop\eZMigrationBundle\API\Collection\LocationCollection;
6		use Kaliop\eZMigrationBundle\API\Collection\TrashedItemCollection;
7		use Kaliop\eZMigrationBundle\Core\Matcher\TrashMatcher;
8		use Kaliop\eZMigrationBundle\Core\Helper\SortConverter;
9
10		/**
11		* Handles trash migrations.
12		*/
13		class TrashManager extends RepositoryExecutor
14		{
15		protected $supportedActions = array('purge', 'recover', 'delete');
16		protected $supportedStepTypes = array('trash');
17
18		/** @var TrashMatcher $trashMatcher */
19		protected $trashMatcher;
20
21		protected $sortConverter;
22
23		/**
24		* @param TrashMatcher $trashMatcher
25		*/
26		public function __construct(TrashMatcher $trashMatcher, SortConverter $sortConverter)
27		{
28		$this->trashMatcher = $trashMatcher;
29		$this->sortConverter = $sortConverter;
30		}
31
32		/**
33		* Handles emptying the trash
34		*/
35		protected function purge($step)
		0 ignored issues – show Unused Code introduced 2017-10-28 23:27 UTC by Report Bug Copy Issue Report The parameter `$step` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
36		{
37		$trashService = $this->repository->getTrashService();
38
39		$trashService->emptyTrash();
40
41		return true;
42		}
43
44		/**
45		* Handles the trash-restore migration action
46		*
47		* @todo support handling of restoration to custom locations
48		*/
49		protected function recover($step)
50		{
51		$itemsCollection = $this->matchItems('restore', $step);
52
53		if (count($itemsCollection) > 1 && array_key_exists('references', $step->dsl)) {
54		throw new \Exception("Can not execute Trash restore because multiple types match, and a references section is specified in the dsl. References can be set when only 1 section matches");
55		}
56
57		$locations = array();
58		$trashService = $this->repository->getTrashService();
59		foreach ($itemsCollection as $key => $item) {
		0 ignored issues – show Bug introduced 2017-10-28 23:27 UTC by Report Bug Copy Issue Report The expression `$itemsCollection` of type `object<Kaliop\eZMigratio...hedItemCollection>\|null` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
60		$locations[] = $trashService->recover($item);
61		}
62
63		$this->setReferences(new LocationCollection($locations), $step);
64
65		return $itemsCollection;
66		}
67
68		/**
69		* Handles the trash-delete migration action
70		*/
71		protected function delete($step)
72		{
73		$itemsCollection = $this->matchItems('delete', $step);
74
75		if (count($itemsCollection) > 1 && array_key_exists('references', $step->dsl)) {
76		throw new \Exception("Can not execute Trash restore because multiple types match, and a references section is specified in the dsl. References can be set when only 1 section matches");
77		}
78
79		$this->setReferences($itemsCollection, $step);
		0 ignored issues – show Bug introduced 2017-10-28 23:27 UTC by Report Bug Copy Issue Report It seems like `$itemsCollection` defined by `$this->matchItems('delete', $step)` on line `73` can be `null`; however, `Kaliop\eZMigrationBundle...anager::setReferences()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
80
81		$trashService = $this->repository->getTrashService();
82		foreach ($itemsCollection as $key => $item) {
		0 ignored issues – show Bug introduced 2017-10-28 23:27 UTC by Report Bug Copy Issue Report The expression `$itemsCollection` of type `object<Kaliop\eZMigratio...hedItemCollection>\|null` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
83		$trashService->deleteTrashItem($item);
84		}
85
86		$this->setReferences($itemsCollection, $step);
		0 ignored issues – show Bug introduced 2017-10-28 23:27 UTC by Report Bug Copy Issue Report It seems like `$itemsCollection` defined by `$this->matchItems('delete', $step)` on line `73` can be `null`; however, `Kaliop\eZMigrationBundle...anager::setReferences()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
87
88		return $itemsCollection;
89		}
90
91		/**
92		* @param string $action
93		* @return TrashedItemCollection
94		* @throws \Exception
95		*/
96	View Code Duplication	protected function matchItems($action, $step)
		0 ignored issues – show Duplication introduced 2017-08-29 17:16 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. 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. Loading history...
97		{
98		if (!isset($step->dsl['match'])) {
99		throw new \Exception("A match condition is required to $action trash items");
100		}
101
102		// convert the references passed in the match
103		$match = $this->resolveReferencesRecursively($step->dsl['match']);
		0 ignored issues – show Deprecated Code introduced 2017-02-25 20:35 UTC by Report Bug Copy Issue Report The method `Kaliop\eZMigrationBundle...ReferencesRecursively()` has been deprecated with message: will be moved into the reference resolver classes This method has been deprecated. The supplier of the class has supplied an explanatory message. The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead. Loading history...
104
105		return $this->trashMatcher->match($match);
106		}
107
108		/**
109		* Sets references to certain trashed-item attributes.
110		*
111		* @param \eZ\Publish\API\Repository\Values\Content\TrashItem\|TrashedItemCollection\|\eZ\Publish\API\Repository\Values\Content\Location\|LocationCollection $item
112		* @param $step
113		* @throws \InvalidArgumentException When trying to set a reference to an unsupported attribute
114		* @return boolean
115		*/
116	View Code Duplication	protected function setReferences($item, $step)
		0 ignored issues – show Duplication introduced 2017-10-29 01:00 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. 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. Loading history...
117		{
118		if (!array_key_exists('references', $step->dsl)) {
119		return false;
120		}
121
122		$references = $this->setReferencesCommon($item, $step->dsl['references']);
123		$item = $this->insureSingleEntity($item, $references);
124
125		foreach ($references as $reference) {
126		switch ($reference['attribute']) {
127		// a trashed item extends a location, so in theory everything 'location' here should work
128		case 'location_id':
129		case 'id':
130		$value = $item->id;
131		break;
132		case 'remote_id':
133		case 'location_remote_id':
134		$value = $item->remoteId;
135		break;
136		case 'always_available':
137		$value = $item->contentInfo->alwaysAvailable;
138		break;
139		case 'content_id':
140		$value = $item->contentId;
141		break;
142		case 'content_type_id':
143		$value = $item->contentInfo->contentTypeId;
144		break;
145		case 'content_type_identifier':
146		$contentTypeService = $this->repository->getContentTypeService();
147		$value = $contentTypeService->loadContentType($item->contentInfo->contentTypeId)->identifier;
148		break;
149		case 'current_version':
150		case 'current_version_no':
151		$value = $item->contentInfo->currentVersionNo;
152		break;
153		case 'depth':
154		$value = $item->depth;
155		break;
156		case 'is_hidden':
157		$value = $item->hidden;
158		break;
159		case 'main_location_id':
160		$value = $item->contentInfo->mainLocationId;
161		break;
162		case 'main_language_code':
163		$value = $item->contentInfo->mainLanguageCode;
164		break;
165		case 'modification_date':
166		$value = $item->contentInfo->modificationDate->getTimestamp();
167		break;
168		case 'name':
169		$value = $item->contentInfo->name;
170		break;
171		case 'owner_id':
172		$value = $item->contentInfo->ownerId;
173		break;
174		case 'parent_location_id':
175		$value = $item->parentLocationId;
176		break;
177		case 'path':
178		$value = $item->pathString;
179		break;
180		case 'priority':
181		$value = $item->priority;
182		break;
183		case 'publication_date':
184		$value = $item->contentInfo->publishedDate->getTimestamp();
185		break;
186		case 'section_id':
187		$value = $item->contentInfo->sectionId;
188		break;
189		case 'section_identifier':
190		$sectionService = $this->repository->getSectionService();
191		$value = $sectionService->loadSection($item->contentInfo->sectionId)->identifier;
192		break;
193		case 'sort_field':
194		$value = $this->sortConverter->sortField2Hash($item->sortField);
195		break;
196		case 'sort_order':
197		$value = $this->sortConverter->sortOrder2Hash($item->sortOrder);
198		break;
199		default:
200		throw new \InvalidArgumentException('Trash Manager does not support setting references for attribute ' . $reference['attribute']);
201		}
202
203		$overwrite = false;
204		if (isset($reference['overwrite'])) {
205		$overwrite = $reference['overwrite'];
206		}
207		$this->referenceResolver->addReference($reference['identifier'], $value, $overwrite);
208		}
209
210		return true;
211		}
212		}
213

kaliop-uk / ezmigrationbundle