Completed
Push — pac-264--pdo-exception ( ca7491 )
by Tim
01:49
created

UrlKeyAndPathObserver::process()   C

Complexity

Conditions 10
Paths 25

Size

Total Lines 75

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 75
rs 6.6787
c 0
b 0
f 0
cc 10
nc 25
nop 0

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\Observers\UrlKeyAndPathObserver
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 2019 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\Observers;
22
23
use Zend\Filter\FilterInterface;
24
use TechDivision\Import\Category\Utils\ColumnKeys;
25
use TechDivision\Import\Category\Utils\MemberNames;
26
use TechDivision\Import\Utils\StoreViewCodes;
27
use TechDivision\Import\Utils\UrlKeyUtilInterface;
28
use TechDivision\Import\Utils\Filter\UrlKeyFilterTrait;
29
use TechDivision\Import\Subjects\UrlKeyAwareSubjectInterface;
30
use TechDivision\Import\Category\Services\CategoryBunchProcessorInterface;
31
32
/**
33
 * Observer that extracts the URL key/path from the category path
34
 * and adds them as two new columns with the their values.
35
 *
36
 * @author    Tim Wagner <[email protected]>
37
 * @copyright 2019 TechDivision GmbH <[email protected]>
38
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
39
 * @link      https://github.com/techdivision/import-category
40
 * @link      http://www.techdivision.com
41
 */
42
class UrlKeyAndPathObserver extends AbstractCategoryImportObserver
43
{
44
45
    /**
46
     * The trait that provides string => URL key conversion functionality.
47
     *
48
     * @var \TechDivision\Import\Utils\Filter\UrlKeyFilterTrait
49
     */
50
    use UrlKeyFilterTrait;
51
52
    /**
53
     * The URL key utility instance.
54
     *
55
     * @var \TechDivision\Import\Utils\UrlKeyUtilInterface
56
     */
57
    protected $urlKeyUtil;
58
59
    /**
60
     * The category bunch processor instance.
61
     *
62
     * @var \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface
63
     */
64
    protected $categoryBunchProcessor;
65
66
    /**
67
     * Initialize the observer with the passed product bunch processor instance.
68
     *
69
     * @param \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface $categoryBunchProcessor  The category bunch processor instance
70
     * @param \Zend\Filter\FilterInterface                                           $convertLiteralUrlFilter The URL filter instance
71
     * @param \TechDivision\Import\Utils\UrlKeyUtilInterface                         $urlKeyUtil              The URL key utility instance
72
     */
73
    public function __construct(
74
        CategoryBunchProcessorInterface $categoryBunchProcessor,
75
        FilterInterface $convertLiteralUrlFilter,
76
        UrlKeyUtilInterface $urlKeyUtil
77
    ) {
78
79
        // set the processor and the URL filter instance
80
        $this->categoryBunchProcessor = $categoryBunchProcessor;
81
        $this->convertLiteralUrlFilter = $convertLiteralUrlFilter;
82
        $this->urlKeyUtil = $urlKeyUtil;
83
    }
84
85
    /**
86
     * Process the observer's business logic.
87
     *
88
     * @return void
89
     * @todo See PAC-307
90
     */
91
    protected function process()
92
    {
93
94
        try {
95
            // try to set the entity IDs for the category with the passed path
96
            if ($category = $this->getCategoryByPath($this->getValue(ColumnKeys::PATH)) && $this->hasValue(ColumnKeys::URL_KEY)) {
97
                $this->setIds($category);
0 ignored issues
show
Documentation introduced by
$category is of type boolean, but the function expects a array.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
98
            } else {
99
                // @todo See PAC-307
100
                // category already exists and NO new URL key
101
                // has been specified in column `url_key`, so
102
                // we stop processing here
103
104
                return;
105
            }
106
        } catch (\Exception $e) {
107
            $this->setIds(array());
108
        }
109
110
        // prepare the store view code
111
        $this->prepareStoreViewCode();
112
113
        // load ID of the actual store view
114
        $storeId = $this->getRowStoreId(StoreViewCodes::ADMIN);
115
116
        // explode the path into the category names
117
        if ($categories = $this->explode($this->getValue(ColumnKeys::PATH), '/')) {
118
            // initialize the array for the category paths
119
            $categoryPaths = array();
120
            // iterate over the parent category names and try
121
            // to load the categories to build the URL path
122
            for ($i = sizeof($categories) - 1; $i > 1; $i--) {
123
                try {
124
                    // prepare the expected category name
125
                    $categoryPath = implode('/', array_slice($categories, 0, $i));
126
                    // load the existing category and prepend the URL key the array with the category URL keys
127
                    $existingCategory = $this->getCategoryByPkAndStoreId($this->mapPath($categoryPath), $storeId);
128
                    // query whether or not an URL key is available or not
129
                    if (isset($existingCategory[MemberNames::URL_KEY])) {
130
                        array_unshift($categoryPaths, $existingCategory[MemberNames::URL_KEY]);
131
                    } else {
132
                        $this->getSystemLogger()->debug(sprintf('Can\'t find URL key for category "%s"', $categoryPath));
133
                    }
134
                } catch (\Exception $e) {
135
                    $this->getSystemLogger()->debug(sprintf('Can\'t load parent category "%s"', $categoryPath));
136
                }
137
            }
138
        }
139
140
        // query whether or not the URL key column has a
141
        // value, if yes, use the value from the column
142
        if ($this->hasValue(ColumnKeys::URL_KEY)) {
143
            $urlKey =  $this->getValue(ColumnKeys::URL_KEY);
144
        } else {
145
            // initialize the URL key with the converted name
146
            $urlKey = $this->convertNameToUrlKey($this->getValue(ColumnKeys::NAME));
147
        }
148
149
        // update the URL key with the unique value
150
        $this->setValue(
151
            ColumnKeys::URL_KEY,
152
            $urlKey = $this->makeUnique($this->getSubject(), $urlKey, implode('/', $categoryPaths))
0 ignored issues
show
Bug introduced by
The variable $categoryPaths does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
Documentation introduced by
$this->getSubject() is of type object<TechDivision\Impo...jects\SubjectInterface>, but the function expects a object<TechDivision\Impo...yAwareSubjectInterface>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
Unused Code introduced by
The call to UrlKeyAndPathObserver::makeUnique() has too many arguments starting with implode('/', $categoryPaths).

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
153
        );
154
155
        // finally, append the URL key as last element to the path
156
        array_push($categoryPaths, $urlKey);
157
158
        // create the virtual column for the URL path
159
        if ($this->hasHeader(ColumnKeys::URL_PATH) === false) {
160
            $this->addHeader(ColumnKeys::URL_PATH);
161
        }
162
163
        // set the URL path
164
        $this->setValue(ColumnKeys::URL_PATH, implode('/', $categoryPaths));
165
    }
166
167
    /**
168
     * Return the primary key member name.
169
     *
170
     * @return string The primary key member name
171
     */
172
    protected function getPkMemberName()
173
    {
174
        return MemberNames::ENTITY_ID;
175
    }
176
177
    /**
178
     * Returns the category bunch processor instance.
179
     *
180
     * @return \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface The category bunch processor instance
181
     */
182
    protected function getCategoryBunchProcessor()
183
    {
184
        return $this->categoryBunchProcessor;
185
    }
186
187
    /**
188
     * Returns the URL key utility instance.
189
     *
190
     * @return \TechDivision\Import\Utils\UrlKeyUtilInterface The URL key utility instance
191
     */
192
    protected function getUrlKeyUtil()
193
    {
194
        return $this->urlKeyUtil;
195
    }
196
197
    /**
198
     * Return's the category with the passed path.
199
     *
200
     * @param string $path The path of the category to return
201
     *
202
     * @return array The category
203
     * @throws \Exception Is thrown, if the requested category is not available
204
     */
205
    protected function getCategoryByPath($path)
206
    {
207
        return $this->getSubject()->getCategoryByPath($path);
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 getCategoryByPath() does only exist in the following implementations of said interface: TechDivision\Import\Cate...AbstractCategorySubject, TechDivision\Import\Category\Subjects\BunchSubject, 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...
208
    }
209
210
    /**
211
     * Returns the category with the passed primary key and the attribute values for the passed store ID.
212
     *
213
     * @param string  $pk      The primary key of the category to return
214
     * @param integer $storeId The store ID of the category values
215
     *
216
     * @return array|null The category data
217
     */
218
    protected function getCategoryByPkAndStoreId($pk, $storeId)
219
    {
220
        return $this->getCategoryBunchProcessor()->getCategoryByPkAndStoreId($pk, $storeId);
221
    }
222
223
    /**
224
     * Temporarily persist's the IDs of the passed category.
225
     *
226
     * @param array $category The category to temporarily persist the IDs for
227
     *
228
     * @return void
229
     */
230
    protected function setIds(array $category)
231
    {
232
        $this->setLastEntityId(isset($category[$this->getPkMemberName()]) ? $category[$this->getPkMemberName()] : null);
233
    }
234
235
    /**
236
     * Set's the ID of the category that has been created recently.
237
     *
238
     * @param string $lastEntityId The entity ID
239
     *
240
     * @return void
241
     */
242
    protected function setLastEntityId($lastEntityId)
243
    {
244
        $this->getSubject()->setLastEntityId($lastEntityId);
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 setLastEntityId() does only exist in the following implementations of said interface: TechDivision\Import\Cate...AbstractCategorySubject, 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...
245
    }
246
247
    /**
248
     * Make's the passed URL key unique by adding the next number to the end.
249
     *
250
     * @param \TechDivision\Import\Subjects\UrlKeyAwareSubjectInterface $subject The subject to make the URL key unique for
251
     * @param string                                                    $urlKey  The URL key to make unique
252
     *
253
     * @return string The unique URL key
254
     */
255
    protected function makeUnique(UrlKeyAwareSubjectInterface $subject, $urlKey)
256
    {
257
        return $this->getUrlKeyUtil()->makeUnique($subject, $urlKey);
258
    }
259
}
260