Completed
Push — master ( b367bc...778190 )
by Tim
01:37
created

SortCategoryListener::categoriesOnSameLevel()   C

Complexity

Conditions 10
Paths 16

Size

Total Lines 64

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 64
rs 6.9187
c 0
b 0
f 0
cc 10
nc 16
nop 1

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
/**
4
 * TechDivision\Import\Category\Listeners\SortCategoryListener
5
 *
6
 * NOTICE OF LICENSE
7
 *
8
 * This source file is subject to the Open Software License (OSL 3.0)
9
 * that is available through the world-wide-web at this URL:
10
 * http://opensource.org/licenses/osl-3.0.php
11
 *
12
 * PHP version 5
13
 *
14
 * @author    Tim Wagner <[email protected]>
15
 * @copyright 2020 TechDivision GmbH <[email protected]>
16
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
17
 * @link      https://github.com/techdivision/import-category
18
 * @link      http://www.techdivision.com
19
 */
20
21
namespace TechDivision\Import\Category\Listeners;
22
23
use League\Event\EventInterface;
24
use League\Event\AbstractListener;
25
use TechDivision\Import\Utils\StoreViewCodes;
26
use TechDivision\Import\Subjects\SubjectInterface;
27
use TechDivision\Import\Services\RegistryProcessorInterface;
28
use TechDivision\Import\Category\Utils\ColumnKeys;
29
use TechDivision\Import\Category\Utils\MemberNames;
30
use TechDivision\Import\Category\Utils\RegistryKeys;
31
use TechDivision\Import\Category\Observers\CopyCategoryObserver;
32
33
/**
34
 * A listener implementation that sorts categories by their path and the position that has
35
 * been specified in the `position` column.
36
 *
37
 * This process can be quite complicated as it is possible that the import file contains
38
 *
39
 *   - only a subset of the categories
40
 *   - existing categories has been moved
41
 *   - the positions changes
42
 *
43
 * Therefore the position calculation is a very expensive operation an needs to handled
44
 * and refactored very carefully.
45
 *
46
 * @author    Tim Wagner <[email protected]>
47
 * @copyright 2020 TechDivision GmbH <[email protected]>
48
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
49
 * @link      https://github.com/techdivision/import-category
50
 * @link      http://www.techdivision.com
51
 */
52
class SortCategoryListener extends AbstractListener
53
{
54
55
    /**
56
     * The registry processor instance.
57
     *
58
     * @var \TechDivision\Import\Services\RegistryProcessorInterface
59
     */
60
    protected $registryProcessor;
61
62
    /**
63
     * The array with the already existing categories.
64
     *
65
     * @var array
66
     */
67
    private $existingCategories = array();
68
69
    /**
70
     * The array that is used to temporary store the main rows (where we NEED to set the position for sorting).
71
     *
72
     * @var array
73
     */
74
    private $mainRows = array();
75
76
    /**
77
     * The array that is used to temporary store the store view specific rows.
78
     *
79
     * @var array
80
     */
81
    private $storeViewRows = array();
82
83
    /**
84
     * The array with the artefacts the we want to export with the appropriate position.
85
     *
86
     * @var array
87
     */
88
    private $artefacts = array();
89
90
    /**
91
     * The array with the existing attribute sets.
92
     *
93
     * @var array
94
     */
95
    private $attributeSets = array();
96
97
    /**
98
     * The actual subject instance.
99
     *
100
     * @var \TechDivision\Import\Subjects\SubjectInterface
101
     */
102
    private $subject;
103
104
    /**
105
     * The subject's serializer instance.
106
     *
107
     * @var \TechDivision\Import\Serializers\SerializerInterface
108
     */
109
    private $serializer;
110
111
    /**
112
     * Initializes the listener with the registry processor instance.
113
     *
114
     * @param \TechDivision\Import\Services\RegistryProcessorInterface $registryProcessor The registry processor instance
115
     */
116
    public function __construct(RegistryProcessorInterface $registryProcessor)
117
    {
118
        $this->registryProcessor = $registryProcessor;
119
    }
120
121
    /**
122
     * Handle an event.
123
     *
124
     * @param \League\Event\EventInterface                   $event   The event that triggered the event
125
     * @param \TechDivision\Import\Subjects\SubjectInterface $subject The subject instance
126
     *
127
     * @return void
128
     */
129
    public function handle(EventInterface $event, SubjectInterface $subject = null) : void
130
    {
131
132
        // initialize subject and serializer
133
        $this->subject = $subject;
134
        $this->serializer = $subject->getImportAdapter()->getSerializer();
0 ignored issues
show
Bug introduced by
It seems like $subject is not always an object, but can also be of type null. Maybe add an additional type check?

If a variable is not always an object, we recommend to add an additional type check to ensure your method call is safe:

function someFunction(A $objectMaybe = null)
{
    if ($objectMaybe instanceof A) {
        $objectMaybe->doSomething();
    }
}
Loading history...
135
136
        // load the status of the actual import
137
        $status = $this->registryProcessor->getAttribute(RegistryKeys::STATUS);
138
139
        // load the categories for the admin store view from the global data
140
        if (isset($status[RegistryKeys::GLOBAL_DATA][RegistryKeys::CATEGORIES])) {
141
            $this->existingCategories = $status[RegistryKeys::GLOBAL_DATA][RegistryKeys::CATEGORIES][StoreViewCodes::ADMIN];
142
            $this->attributeSets = $status[RegistryKeys::GLOBAL_DATA][RegistryKeys::ATTRIBUTE_SETS][$subject->getEntityTypeCode()];
143
        }
144
145
        // load the artefacts from the subject
146
        $artefacts = $subject->getArtefacts();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechDivision\Import\Subjects\SubjectInterface as the method getArtefacts() does only exist in the following implementations of said interface: TechDivision\Import\Category\Subjects\BunchSubject, TechDivision\Import\Cate...ts\SortableBunchSubject, TechDivision\Import\Plugins\ExportableSubjectImpl, TechDivision\Import\Subjects\ExportableTraitImpl.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
147
148
        // query whether or not the artefacts with the given type are available
149
        if (isset($artefacts[$type = CopyCategoryObserver::ARTEFACT_TYPE])) {
150
            // load the artefacts for the given type
151
            $newCategories = array_shift($artefacts[CopyCategoryObserver::ARTEFACT_TYPE]);
152
            // iterate over ALL rows found in the actual CSV file
153
            foreach ($newCategories as $newCategory) {
154
                // we only want to process main rows, so temporary persist store view specific rows
155
                if ($newCategory[ColumnKeys::STORE_VIEW_CODE]) {
156
                    $this->storeViewRows[] = $this->template($newCategory);
157
                } else {
158
                    // clean-up the path to avoid encoding/quotation specific differences
159
                    $path = implode('/', $this->serializer->unserialize($newCategory[ColumnKeys::PATH], '/'));
160
                    // add the main category to the new categories (we want to load/update the position for)
161
                    $this->mainRows[$path] = $newCategory;
162
                }
163
            }
164
165
            // sort the main rows by the path, store_view_code and position
166
            // ATTENTION: we use uasort, because we NEED to preserve the keys
167
            uasort($this->mainRows, function ($a, $b) {
168
                return
169
                    strcmp($a[ColumnKeys::PATH], $b[ColumnKeys::PATH]) ?:
170
                    strcmp($a[ColumnKeys::STORE_VIEW_CODE], $b[ColumnKeys::STORE_VIEW_CODE]) ?:
171
                    strcmp($a[ColumnKeys::POSITION], $b[ColumnKeys::POSITION]);
172
            });
173
174
            // update the position of the categories and the categories on the same level
175
            foreach ($this->mainRows as $path => $category) {
176
                $this->update($path, $category);
177
            }
178
179
            // merge the processed rows with the previously excluded store view
180
            // base rows, because in the new CSV file we want to have them both
181
            $this->artefacts = array_merge(array_values($this->artefacts), $this->storeViewRows);
182
183
            // sort the artefacts again, because we want to export them in the expected order
184
            usort($this->artefacts, function ($a, $b) {
185
                return
186
                    strcmp($a[ColumnKeys::PATH], $b[ColumnKeys::PATH]) ?:
187
                    strcmp($a[ColumnKeys::STORE_VIEW_CODE], $b[ColumnKeys::STORE_VIEW_CODE]);
188
            });
189
190
            // replace the artefacts to be exported later
191
            $subject->setArtefactsByType($type, array($this->artefacts));
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface TechDivision\Import\Subjects\SubjectInterface as the method setArtefactsByType() does only exist in the following implementations of said interface: TechDivision\Import\Cate...ts\SortableBunchSubject.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
192
        }
193
    }
194
195
    /**
196
     * Template method to create a new array based on the passed category data.
197
     *
198
     * @param array $category The category data to create the new array from
199
     *
200
     * @return array The array with the category data
201
     */
202
    private function template(array $category) : array
203
    {
204
205
        // initialize the array for the category data
206
        $row = array();
0 ignored issues
show
Unused Code introduced by
$row is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
207
208
        // initialize the array's keys from the actual headers AND add the position (which is the important column we are here for)
209
        $keys = array_keys($row = array_replace($this->subject->getHeaders(), array(ColumnKeys::POSITION => null)));
210
211
        // initialize the category with the data from the passed category
212
        foreach ($keys as $key) {
213
            $row[$key] = isset($category[$key]) ? $category[$key] : null;
214
        }
215
216
        // return the row
217
        return $row;
218
    }
219
220
    /**
221
     * Return's an array, sorted by position, with categories on the same level as the one with
222
     * the passed path is and have the same parent node.
223
     *
224
     * Therefore, the we cut off the last element of the passed path and compare it with the
225
     * already existing categories and the categories that has been processed within the
226
     * actual import file.
227
     *
228
     * @param string $path The path to return the categories on the same level for
229
     *
230
     * @return array The array, sorted by position, of the categories on the same level
231
     */
232
    private function categoriesOnSameLevel(string $path) : array
233
    {
234
235
        // initialize the array for the categories an the same level
236
        $categoriesOnSameLevel = array();
237
238
        // explode the path by the category separator
239
        $elements = $this->serializer->unserialize($path, '/');
240
241
        // iterate over the existing categories to load the one's on the same level
242
        foreach ($this->existingCategories as $p => $category) {
243
            // initialize the counter with the size of elements
244
            $sizeOfElements = sizeof($elements);
245
            // query whether or not the level (integer) is the same as the number of elements
246
            // AND the category is NOT a root category. This means we virtually cut off the
247
            // last element of the passed category. Then we know the category is at least on
248
            // the same level, but NOT if it has the same parent category!!!!!
249
            if ((int) $category[MemberNames::LEVEL] == $sizeOfElements && $sizeOfElements > 1) {
250
                // extract the category's path by the category separator
251
                $el = $this->serializer->unserialize($p, '/');
252
                // diff the path of the parent category to make sure they are children of the same parent node
253
                $diff = array_diff(array_slice($elements, 0, sizeof($elements) - 1), array_slice($el, 0, sizeof($elements)));
254
                // BINGO: We found a category on the same level that has the same parent
255
                if (sizeof($diff) === 0) {
256
                    $categoriesOnSameLevel[$p] = $this->template(
257
                        array_merge(
258
                            $category,
259
                            array(ColumnKeys::ATTRIBUTE_SET_CODE => $this->getAttributeSetNameById($category[MemberNames::ATTRIBUTE_SET_ID]))
260
                        )
261
                    );
262
                }
263
            }
264
        }
265
266
        // iterate over the already processed categories and try to find the one's on the same level
267
        // because it's possible we've a NEW category in the CSV file which is NOT in the array with
268
        // the existing ones.
269
        foreach ($this->artefacts as $p => $artefact) {
270
            // extract the category's path by the category separator
271
            $el = $this->serializer->unserialize($p, '/');
272
            // query whether or not the categories are at least on the same level
273
            if (sizeof($el) == sizeof($elements)) {
274
                // diff the path of the parent category to make sure they are children of the same parent node
275
                $diff = array_diff(array_slice($elements, 0, sizeof($elements) - 1), array_slice($el, 0, sizeof($elements)));
276
                // BINGO: We found a category on the same level that has the same parent
277
                if (sizeof($diff) === 0) {
278
                    $categoriesOnSameLevel[$p] = $artefact;
279
                }
280
            }
281
        }
282
283
        // sor the categories by their position and KEEP the keys
284
        uasort($categoriesOnSameLevel, function ($a, $b) {
285
            // return 0 when the position is equal (should never happen)
286
            if ($a[MemberNames::POSITION] == $b[MemberNames::POSITION]) {
287
                return 0;
288
            }
289
            // return 1 or -1 if the categories position differs
290
            return $a[MemberNames::POSITION] > $b[MemberNames::POSITION] ? 1 : -1;
291
        });
292
293
        // return the sorted array with categories on the same level and the same parent node
294
        return $categoriesOnSameLevel;
295
    }
296
297
    /**
298
     * Return's the next free position for the category with the passed path.
299
     *
300
     * The calculation happens on the already exising categories on the same
301
     * level and the same parent not as well as the categories that already
302
     * have been processed.
303
     *
304
     * @param string $path The path of the category to return the position for
305
     *
306
     * @return int The next free position
307
     */
308
    private function nextPosition(string $path) : int
309
    {
310
311
        // load the categories, sorted by position, that have the same level
312
        // as well as the same parent node
313
        $categoriesOnSameLevel = $this->categoriesOnSameLevel($path);
314
315
        // load the last one, because this must be the one with the highest position
316
        $lastCategory = end($categoriesOnSameLevel);
317
318
        // raise the position counter either by loading the position of the last category
319
        // OR the number of categories on the same level and with the same parent node
320
        if (isset($lastCategory[MemberNames::POSITION])) {
321
            $position = (int) $lastCategory[MemberNames::POSITION] + 1;
322
        } else {
323
            $position = sizeof($categoriesOnSameLevel) + 1;
324
        }
325
326
        // return the next position
327
        return $position;
328
    }
329
330
    /**
331
     * Updates the positions (if necessary) of ALL categories on the same level as the passed one is and
332
     * add them to the array of categories that has to be exported with the position column filled.
333
     *
334
     * @param string $path The unqiue path ot the passed category (ATTENTION: We do NOT use the column `path`, because this can have format specific encoding or quoting)
335
     * @param array  $cat  The category to raise the positions on the same level for
336
     *
337
     * @return void
338
     */
339
    private function update(string $path, array $cat) : void
340
    {
341
342
        // query whether or not the the category with the passed path already exists
343
        if (isset($this->existingCategories[$path])) {
344
            // if yes, load the exisisting category
345
            $existingCategory = $this->existingCategories[$path];
346
            // query whether or not the passed category has a position, eventually a new and
347
            // different one has been set, if NOT use the position of the existing one
348
            if (isset($cat[ColumnKeys::POSITION]) === false) {
349
                $cat[ColumnKeys::POSITION] = $existingCategory[MemberNames::POSITION];
350
            }
351
352
            // query whether or not the new position is different from the exising one, if not we do
353
            // NOT have to process the categories on the same level and raise their positions as well
354
            if ((int) $cat[ColumnKeys::POSITION] === (int) $existingCategory[MemberNames::POSITION]) {
355
                $this->artefacts[$path] = $this->template($cat);
356
                return;
357
            }
358
        }
359
360
        // category is NEW and has NO position > load the next available one and append the category
361
        // on the end of the existing categories on the same level and the same parent node
362
        if (isset($cat[ColumnKeys::POSITION]) === false) {
363
            $cat[ColumnKeys::POSITION] = $this->nextPosition($path);
364
        } else {
365
            // load the categories on the same level
366
            $categoriesOnSameLevel = $this->categoriesOnSameLevel($path);
367
            // iterate over the categories on the same level and the same parent node
368
            foreach ($categoriesOnSameLevel as $p => $category) {
369
                // if the position of the existing category is lower than the one of the
370
                // passed category we do not have to care about raising the position
371
                if ((int) $category[ColumnKeys::POSITION] < (int) $cat[ColumnKeys::POSITION]) {
372
                    continue;
373
                }
374
375
                // in case the position is equal or higher, we've to raise the position
376
                // make sure the new category will be rendered before the existing one
377
                $this->artefacts[$p] = $this->template(
378
                    array_merge(
379
                        $category,
380
                        array(
381
                            ColumnKeys::PATH               => $category[MemberNames::PATH],
382
                            ColumnKeys::POSITION           => (int) $category[MemberNames::POSITION] + 1,
383
                            ColumnKeys::ATTRIBUTE_SET_CODE => $category[ColumnKeys::ATTRIBUTE_SET_CODE]
384
                        )
385
                    )
386
                );
387
            }
388
        }
389
390
        // finally append the passed category to the array with the artefacts
391
        $this->artefacts[$path] = $this->template($cat);
392
    }
393
394
    /**
395
     * Return's the attribute set name of the attribute set with the given ID.
396
     *
397
     * @param int $attributeSetId The ID of the attribute set to return the name for
398
     *
399
     * @return string The attribute set name
400
     * @throws \InvalidArgumentException Is thrown, if the attribute set with the passed ID is NOT available
401
     */
402
    private function getAttributeSetNameById(int $attributeSetId) : string
403
    {
404
405
        // try to load the attribute set with the given ID
406
        foreach ($this->attributeSets as $attributeSet) {
407
            if ((int) $attributeSet[MemberNames::ATTRIBUTE_SET_ID] === $attributeSetId) {
408
                return $attributeSet[MemberNames::ATTRIBUTE_SET_NAME];
409
            }
410
        }
411
412
        // throw an exception if the attribute set is NOT available
413
        throw new \InvalidArgumentException(sprintf('Can\'t find attribute set with ID "%s"', $attributeSetId));
414
    }
415
}
416