Completed
Pull Request — 20.x (#73)
by Tim
03:03 queued 01:34
created

UrlKeyAndPathObserver::process()   C

Complexity

Conditions 10
Paths 62

Size

Total Lines 69

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 69
rs 6.8096
c 0
b 0
f 0
cc 10
nc 62
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\Utils\StoreViewCodes;
25
use TechDivision\Import\Utils\UrlKeyUtilInterface;
26
use TechDivision\Import\Utils\Filter\UrlKeyFilterTrait;
27
use TechDivision\Import\Category\Utils\ColumnKeys;
28
use TechDivision\Import\Category\Utils\MemberNames;
29
use TechDivision\Import\Category\Utils\ConfigurationKeys;
30
use TechDivision\Import\Subjects\UrlKeyAwareSubjectInterface;
31
use TechDivision\Import\Category\Services\CategoryBunchProcessorInterface;
32
33
/**
34
 * Observer that extracts the URL key/path from the category path
35
 * and adds them as two new columns with the their values.
36
 *
37
 * @author    Tim Wagner <[email protected]>
38
 * @copyright 2019 TechDivision GmbH <[email protected]>
39
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
40
 * @link      https://github.com/techdivision/import-category
41
 * @link      http://www.techdivision.com
42
 */
43
class UrlKeyAndPathObserver extends AbstractCategoryImportObserver
44
{
45
46
    /**
47
     * The trait that provides string => URL key conversion functionality.
48
     *
49
     * @var \TechDivision\Import\Utils\Filter\UrlKeyFilterTrait
50
     */
51
    use UrlKeyFilterTrait;
52
53
    /**
54
     * The URL key utility instance.
55
     *
56
     * @var \TechDivision\Import\Utils\UrlKeyUtilInterface
57
     */
58
    protected $urlKeyUtil;
59
60
    /**
61
     * The category bunch processor instance.
62
     *
63
     * @var \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface
64
     */
65
    protected $categoryBunchProcessor;
66
67
    /**
68
     * Initialize the observer with the passed product bunch processor instance.
69
     *
70
     * @param \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface $categoryBunchProcessor  The category bunch processor instance
71
     * @param \Zend\Filter\FilterInterface                                           $convertLiteralUrlFilter The URL filter instance
72
     * @param \TechDivision\Import\Utils\UrlKeyUtilInterface                         $urlKeyUtil              The URL key utility instance
73
     */
74
    public function __construct(
75
        CategoryBunchProcessorInterface $categoryBunchProcessor,
76
        FilterInterface $convertLiteralUrlFilter,
77
        UrlKeyUtilInterface $urlKeyUtil
78
    ) {
79
80
        // set the processor and the URL filter instance
81
        $this->categoryBunchProcessor = $categoryBunchProcessor;
82
        $this->convertLiteralUrlFilter = $convertLiteralUrlFilter;
83
        $this->urlKeyUtil = $urlKeyUtil;
84
    }
85
86
    /**
87
     * Process the observer's business logic.
88
     *
89
     * @return void
90
     */
91
    protected function process()
92
    {
93
94
        // initialize the URL key and array for the categories
95
        $urlKey = null;
0 ignored issues
show
Unused Code introduced by
$urlKey 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...
96
        $category = array();
97
98
        // set the entity ID for the category with the passed path
99
        try {
100
            $this->setIds($category = $this->getCategoryByPath($this->getValue(ColumnKeys::PATH)));
101
        } catch (\Exception $e) {
102
            $this->setIds(array());
103
        }
104
105
        // query whether or not the URL key column has a value
106
        if ($this->hasValue(ColumnKeys::URL_KEY)) {
107
            $urlKey = $this->makeUnique($this->getSubject(), $this->getValue(ColumnKeys::URL_KEY));
0 ignored issues
show
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...
108
        } else {
109
            // query whether or not the column `url_key` has a value
110
            if ($category &&
0 ignored issues
show
Bug Best Practice introduced by
The expression $category of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
111
                !$this->getSubject()->getConfiguration()->getParam(ConfigurationKeys::UPDATE_URL_KEY_FROM_NAME, true)
112
            ) {
113
                // product already exists and NO new URL key
114
                // has been specified in column `url_key`, so
115
                // we stop processing here
116
                return;
117
            }
118
            $this->setValue(
119
                ColumnKeys::URL_KEY,
120
                $urlKey = $this->makeUnique($this->getSubject(), $this->convertNameToUrlKey($this->getValue(ColumnKeys::NAME)))
0 ignored issues
show
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...
121
            );
122
        }
123
124
        // explode the path into the category names
125
        if ($categories = $this->explode($this->getValue(ColumnKeys::PATH), '/')) {
126
            // initialize the category with the actual category's URL key
127
            $categoryPaths = array($urlKey);
128
            // prepare the store view code
129
            $this->prepareStoreViewCode();
130
            // load ID of the actual store view
131
            $storeId = $this->getRowStoreId(StoreViewCodes::ADMIN);
132
133
            // iterate over the category names and try to load the category therefore
134
            for ($i = sizeof($categories) - 1; $i > 1; $i--) {
135
                try {
136
                    // prepare the expected category name
137
                    $categoryPath = implode('/', array_slice($categories, 0, $i));
138
                    // load the existing category and prepend the URL key the array with the category URL keys
139
                    $existingCategory = $this->getCategoryByPkAndStoreId($this->mapPath($categoryPath), $storeId);
140
                    // query whether or not an URL key is available or not
141
                    if (isset($existingCategory[MemberNames::URL_KEY])) {
142
                        array_unshift($categoryPaths, $existingCategory[MemberNames::URL_KEY]);
143
                    } else {
144
                        $this->getSystemLogger()->debug(sprintf('Can\'t find URL key for category %s', $categoryPath));
145
                    }
146
                } catch (\Exception $e) {
147
                    $this->getSystemLogger()->debug(sprintf('Can\'t load parent category %s', $categoryPath));
148
                }
149
            }
150
151
            // create the header for the URL path
152
            if (!$this->hasHeader(ColumnKeys::URL_PATH)) {
153
                $this->addHeader(ColumnKeys::URL_PATH);
154
            }
155
156
            // set the URL path
157
            $this->setValue(ColumnKeys::URL_PATH, implode('/', $categoryPaths));
158
        }
159
    }
160
161
    /**
162
     * Return the primary key member name.
163
     *
164
     * @return string The primary key member name
165
     */
166
    protected function getPkMemberName()
167
    {
168
        return MemberNames::ENTITY_ID;
169
    }
170
171
    /**
172
     * Returns the category bunch processor instance.
173
     *
174
     * @return \TechDivision\Import\Category\Services\CategoryBunchProcessorInterface The category bunch processor instance
175
     */
176
    protected function getCategoryBunchProcessor()
177
    {
178
        return $this->categoryBunchProcessor;
179
    }
180
181
    /**
182
     * Returns the URL key utility instance.
183
     *
184
     * @return \TechDivision\Import\Utils\UrlKeyUtilInterface The URL key utility instance
185
     */
186
    protected function getUrlKeyUtil()
187
    {
188
        return $this->urlKeyUtil;
189
    }
190
191
    /**
192
     * Return's the category with the passed path.
193
     *
194
     * @param string $path The path of the category to return
195
     *
196
     * @return array The category
197
     * @throws \Exception Is thrown, if the requested category is not available
198
     */
199
    protected function getCategoryByPath($path)
200
    {
201
        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...
202
    }
203
204
    /**
205
     * Returns the category with the passed primary key and the attribute values for the passed store ID.
206
     *
207
     * @param string  $pk      The primary key of the category to return
208
     * @param integer $storeId The store ID of the category values
209
     *
210
     * @return array|null The category data
211
     */
212
    protected function getCategoryByPkAndStoreId($pk, $storeId)
213
    {
214
        return $this->getCategoryBunchProcessor()->getCategoryByPkAndStoreId($pk, $storeId);
215
    }
216
217
    /**
218
     * Temporarily persist's the IDs of the passed category.
219
     *
220
     * @param array $category The category to temporarily persist the IDs for
221
     *
222
     * @return void
223
     */
224
    protected function setIds(array $category)
225
    {
226
        $this->setLastEntityId(isset($category[$this->getPkMemberName()]) ? $category[$this->getPkMemberName()] : null);
227
    }
228
229
    /**
230
     * Set's the ID of the category that has been created recently.
231
     *
232
     * @param string $lastEntityId The entity ID
233
     *
234
     * @return void
235
     */
236
    protected function setLastEntityId($lastEntityId)
237
    {
238
        $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...
239
    }
240
241
    /**
242
     * Make's the passed URL key unique by adding the next number to the end.
243
     *
244
     * @param \TechDivision\Import\Subjects\UrlKeyAwareSubjectInterface $subject The subject to make the URL key unique for
245
     * @param string                                                    $urlKey  The URL key to make unique
246
     *
247
     * @return string The unique URL key
248
     */
249
    protected function makeUnique(UrlKeyAwareSubjectInterface $subject, $urlKey)
250
    {
251
        return $this->getUrlKeyUtil()->makeUnique($subject, $urlKey);
252
    }
253
}
254