BaseModel::fieldShouldNotBeSaved() - Code Metrics - Inspection of "Get rid of deprecated stuff" - arrilot/bitrix-models - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 6ab1e5...2d6522 )

by Nekrasov

created 2017-01-31 23:58 UTC

BaseModel::fieldShouldNotBeSaved() B

↳ Parent: BaseModel

Complexity

Conditions	5
Paths	5

Size

Total Lines	15
Code Lines	11

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	15
rs	8.8571
c	0
b	0
f	0
cc	5
eloc	11
nc	5
nop	3

<?php

namespace Arrilot\BitrixModels\Models;

use Arrilot\BitrixModels\ModelEventsTrait;
use Arrilot\BitrixModels\Queries\BaseQuery;
use Arrilot\BitrixModels\Relations\BaseRelation;
use Exception;
use InvalidArgumentException;
use LogicException;

abstract class BaseModel extends ArrayableModel
{
    use ModelEventsTrait;

    /**
     * Bitrix entity object.
     *
     * @var object
     */
    public static $bxObject;

    /**
     * Corresponding object class name.
     *
     * @var string
     */
    protected static $objectClass = '';

    /**
     * Have fields been already fetched from DB?
     *
     * @var bool
     */
    protected $fieldsAreFetched = false;

    /**
     * Refresh model from database and place data to $this->fields.
     *
     * @return array
     */
    abstract public function refresh();

    /**
     * Refresh model fields from database and place them to $this->fields.
     *
     * @return array
     */
    abstract public function refreshFields();

    /**
     * Constructor.
     *
     * @param $id
     * @param $fields
     */
    public function __construct($id = null, $fields = null)
    {
        static::instantiateObject();

        $this->id = $id;

        $this->fill($fields);
    }

    /**
     * Get all model attributes from cache or database.
     *
     * @return array
     */
    public function get()
    {
        if (!$this->fieldsAreFetched) {
            $this->refresh();
        }

        return $this->fields;
    }

    /**
     * Get user groups from cache or database.
     *
     * @return array
     */
    public function getFields()
    {
        if ($this->fieldsAreFetched) {
            return $this->fields;
        }

        return $this->refreshFields();
    }

    /**
     * Fill model fields if they are already known.
     * Saves DB queries.
     *
     * @param array $fields
     *
     * @return null
     */
    public function fill($fields)
    {
        if (!is_array($fields)) {
            return;
        }

        if (isset($fields['ID'])) {
            $this->id = $fields['ID'];
        }

        $this->fields = $fields;

        $this->fieldsAreFetched = true;

        if (method_exists($this, 'afterFill')) {
            $this->afterFill();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
        }
    }

    /**
     * Activate model.
     *
     * @return bool
     */
    public function activate()
    {
        $this->fields['ACTIVE'] = 'Y';

        return $this->save(['ACTIVE']);
    }

    /**
     * Deactivate model.
     *
     * @return bool
     */
    public function deactivate()
    {
        $this->fields['ACTIVE'] = 'N';

        return $this->save(['ACTIVE']);
    }

    /**
     * Create new item in database.
     *
     * @param $fields
     *
     * @throws Exception
     *
     * @return static
     */
    public static function create($fields)
    {
        $model = new static(null, $fields);

        if ($model->onBeforeSave() === false || $model->onBeforeCreate() === false) {
            return false;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
        }

        $bxObject = static::instantiateObject();
        $id = $bxObject->add($fields);
        $model->setId($id);

        $result = $id ? true : false;

        $model->onAfterCreate($result);
        $model->onAfterSave($result);

        if (!$result) {
            throw new Exception($bxObject->LAST_ERROR);
        }

        return $model;
    }

    /**
     * @deprecated in favour of `static::query()->count()`
     *
     * Get count of items that match $filter.
     *
     * @param array $filter
     *
     * @return int
     */
    public static function count(array $filter = [])
    {
        return static::query()->filter($filter)->count();
    }

    /**
     * Get item by its id.
     *
     * @param int $id
     *
     * @return static
     */
    public static function find($id)
    {
        return static::query()->getById($id);
    }

    /**
     *
     * Get item by its id.
     *
     * @param int $id
     *
     * @return static
     */
    public static function getById($id)
    {
        return static::query()->getById($id);
    }

    /**
     * Delete model.
     *
     * @return bool
     */
    public function delete()
    {
        if ($this->onBeforeDelete() === false) {
            return false;
        }

        $result = static::$bxObject->delete($this->id);

        $this->onAfterDelete($result);

        return $result;
    }

    /**
     * Update model.
     *
     * @param array $fields
     *
     * @return bool
     */
    public function update(array $fields = [])
    {
        $keys = [];
        foreach ($fields as $key => $value) {
            array_set($this->fields, $key, $value);
            $keys[] = $key;
        }

        return $this->save($keys);
    }

    /**
     * Save model to database.
     *
     * @param array $selectedFields save only these fields instead of all.
     *
     * @return bool
     */
    public function save($selectedFields = [])
    {
        $selectedFields = is_array($selectedFields) ? $selectedFields : func_get_args();

        if ($this->onBeforeSave() === false || $this->onBeforeUpdate() === false) {
            return false;
        }

        $fields = $this->normalizeFieldsForSave($selectedFields);
        $result = !empty($fields) ? static::$bxObject->update($this->id, $fields) : false;
        if ($this instanceof ElementModel) {
            $savePropsResult = $this->saveProps($selectedFields);
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
            $result = $result || $savePropsResult;
        }

        $this->onAfterUpdate($result);
        $this->onAfterSave($result);

        return $result;
    }

    /**
     * Scope to get only active items.
     *
     * @param BaseQuery $query
     *
     * @return BaseQuery
     */
    public function scopeActive($query)
    {
        $query->filter['ACTIVE'] = 'Y';

        return $query;
    }

    /**
     * Create an array of fields that will be saved to database.
     *
     * @param $selectedFields
     *
     * @return array
     */
    protected function normalizeFieldsForSave($selectedFields)
    {
        $fields = [];
        if ($this->fields === null) {
            return [];
        }

        foreach ($this->fields as $field => $value) {
            if (!$this->fieldShouldNotBeSaved($field, $value, $selectedFields)) {
                $fields[$field] = $value;
            }
        }

        return $fields;
    }

    /**
     * Determine whether the field should be stopped from passing to "update".
     *
     * @param string $field
     * @param mixed  $value
     * @param array  $selectedFields
     *
     * @return bool
     */
    protected function fieldShouldNotBeSaved($field, $value, $selectedFields)

    {
        $blacklistedFields = [
            'ID',
            'IBLOCK_ID',
            'PROPERTIES',
            'GROUPS',
            'PROPERTY_VALUES',
        ];

        return (!empty($selectedFields) && !in_array($field, $selectedFields))
            || in_array($field, $blacklistedFields)
            || (substr($field, 0, 1) === '~')
            || (substr($field, 0, 9) === 'PROPERTY_');
    }

    /**
     * Instantiate bitrix entity object.
     *
     * @throws Exception
     *
     * @return object
     */
    public static function instantiateObject()
    {
        if (static::$bxObject) {
            return static::$bxObject;
        }

        if (class_exists(static::$objectClass)) {
            return static::$bxObject = new static::$objectClass();
        }

        throw new Exception('Object initialization failed');
    }

    /**
     * Destroy bitrix entity object.
     *
     * @return void
     */
    public static function destroyObject()
    {
        static::$bxObject = null;
    }

    /**
     * Instantiate a query object for the model.
     *
     * @throws Exception
     *
     * @return BaseQuery
     */
    public static function query()
    {
        throw new Exception('public static function query() is not implemented');
    }

    /**
     * Set current model id.
     *
     * @param $id
     */
    protected function setId($id)
    {
        $this->id = $id;
        $this->fields['ID'] = $id;
    }
}


Push — master ( 6ab1e5...2d6522 )

BaseModel::fieldShouldNotBeSaved() B

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

1			<?php
2
3			namespace Arrilot\BitrixModels\Models;
4
5			use Arrilot\BitrixModels\ModelEventsTrait;
6			use Arrilot\BitrixModels\Queries\BaseQuery;
7			use Arrilot\BitrixModels\Relations\BaseRelation;
8			use Exception;
9			use InvalidArgumentException;
10			use LogicException;
11
12			abstract class BaseModel extends ArrayableModel
13			{
14			use ModelEventsTrait;
15
16			/**
17			* Bitrix entity object.
18			*
19			* @var object
20			*/
21			public static $bxObject;
22
23			/**
24			* Corresponding object class name.
25			*
26			* @var string
27			*/
28			protected static $objectClass = '';
29
30			/**
31			* Have fields been already fetched from DB?
32			*
33			* @var bool
34			*/
35			protected $fieldsAreFetched = false;
36
37			/**
38			* Refresh model from database and place data to $this->fields.
39			*
40			* @return array
41			*/
42			abstract public function refresh();
43
44			/**
45			* Refresh model fields from database and place them to $this->fields.
46			*
47			* @return array
48			*/
49			abstract public function refreshFields();
50
51			/**
52			* Constructor.
53			*
54			* @param $id
55			* @param $fields
56			*/
57			public function __construct($id = null, $fields = null)
58			{
59			static::instantiateObject();
60
61			$this->id = $id;
62
63			$this->fill($fields);
64			}
65
66			/**
67			* Get all model attributes from cache or database.
68			*
69			* @return array
70			*/
71			public function get()
72			{
73			if (!$this->fieldsAreFetched) {
74			$this->refresh();
75			}
76
77			return $this->fields;
78			}
79
80			/**
81			* Get user groups from cache or database.
82			*
83			* @return array
84			*/
85			public function getFields()
86			{
87			if ($this->fieldsAreFetched) {
88			return $this->fields;
89			}
90
91			return $this->refreshFields();
92			}
93
94			/**
95			* Fill model fields if they are already known.
96			* Saves DB queries.
97			*
98			* @param array $fields
99			*
100			* @return null
101			*/
102			public function fill($fields)
103			{
104			if (!is_array($fields)) {
105			return;
106			}
107
108			if (isset($fields['ID'])) {
109			$this->id = $fields['ID'];
110			}
111
112			$this->fields = $fields;
113
114			$this->fieldsAreFetched = true;
115
116			if (method_exists($this, 'afterFill')) {
117			$this->afterFill();
			0 ignored issues – show Bug introduced 2015-12-01 20:27 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Arrilot\BitrixModels\Models\BaseModel` as the method `afterFill()` does only exist in the following sub-classes of `Arrilot\BitrixModels\Models\BaseModel`: `Arrilot\BitrixModels\Models\ElementModel`, `Arrilot\BitrixModels\Models\UserModel`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
118			}
119			}
120
121			/**
122			* Activate model.
123			*
124			* @return bool
125			*/
126			public function activate()
127			{
128			$this->fields['ACTIVE'] = 'Y';
129
130			return $this->save(['ACTIVE']);
131			}
132
133			/**
134			* Deactivate model.
135			*
136			* @return bool
137			*/
138			public function deactivate()
139			{
140			$this->fields['ACTIVE'] = 'N';
141
142			return $this->save(['ACTIVE']);
143			}
144
145			/**
146			* Create new item in database.
147			*
148			* @param $fields
149			*
150			* @throws Exception
151			*
152			* @return static
153			*/
154			public static function create($fields)
155			{
156			$model = new static(null, $fields);
157
158			if ($model->onBeforeSave() === false \|\| $model->onBeforeCreate() === false) {
159			return false;
			0 ignored issues – show Bug Best Practice introduced 2015-12-01 20:27 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `Arrilot\BitrixModels\Models\BaseModel::create` of type `Arrilot\BitrixModels\Models\BaseModel`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author { private $name; public function __construct($name) { $this->name = $name; } public function getName() { return $this->name; } } abstract class Post { public function getAuthor() { return 'Johannes'; } } class BlogPost extends Post { public function getAuthor() { return new Author('Johannes'); } } class ForumPost extends Post { /* ... */ } function my_function(Post $post) { echo strtoupper($post->getAuthor()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
160			}
161
162			$bxObject = static::instantiateObject();
163			$id = $bxObject->add($fields);
164			$model->setId($id);
165
166			$result = $id ? true : false;
167
168			$model->onAfterCreate($result);
169			$model->onAfterSave($result);
170
171			if (!$result) {
172			throw new Exception($bxObject->LAST_ERROR);
173			}
174
175			return $model;
176			}
177
178			/**
179			* @deprecated in favour of `static::query()->count()`
180			*
181			* Get count of items that match $filter.
182			*
183			* @param array $filter
184			*
185			* @return int
186			*/
187			public static function count(array $filter = [])
188			{
189			return static::query()->filter($filter)->count();
190			}
191
192			/**
193			* Get item by its id.
194			*
195			* @param int $id
196			*
197			* @return static
198			*/
199			public static function find($id)
200			{
201			return static::query()->getById($id);
202			}
203
204			/**
205			*
206			* Get item by its id.
207			*
208			* @param int $id
209			*
210			* @return static
211			*/
212			public static function getById($id)
213			{
214			return static::query()->getById($id);
215			}
216
217			/**
218			* Delete model.
219			*
220			* @return bool
221			*/
222			public function delete()
223			{
224			if ($this->onBeforeDelete() === false) {
225			return false;
226			}
227
228			$result = static::$bxObject->delete($this->id);
229
230			$this->onAfterDelete($result);
231
232			return $result;
233			}
234
235			/**
236			* Update model.
237			*
238			* @param array $fields
239			*
240			* @return bool
241			*/
242			public function update(array $fields = [])
243			{
244			$keys = [];
245			foreach ($fields as $key => $value) {
246			array_set($this->fields, $key, $value);
247			$keys[] = $key;
248			}
249
250			return $this->save($keys);
251			}
252
253			/**
254			* Save model to database.
255			*
256			* @param array $selectedFields save only these fields instead of all.
257			*
258			* @return bool
259			*/
260			public function save($selectedFields = [])
261			{
262			$selectedFields = is_array($selectedFields) ? $selectedFields : func_get_args();
263
264			if ($this->onBeforeSave() === false \|\| $this->onBeforeUpdate() === false) {
265			return false;
266			}
267
268			$fields = $this->normalizeFieldsForSave($selectedFields);
269			$result = !empty($fields) ? static::$bxObject->update($this->id, $fields) : false;
270			if ($this instanceof ElementModel) {
271			$savePropsResult = $this->saveProps($selectedFields);
			0 ignored issues – show Bug introduced 2015-12-01 20:27 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Arrilot\BitrixModels\Models\BaseModel` as the method `saveProps()` does only exist in the following sub-classes of `Arrilot\BitrixModels\Models\BaseModel`: `Arrilot\BitrixModels\Models\ElementModel`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
272			$result = $result \|\| $savePropsResult;
273			}
274
275			$this->onAfterUpdate($result);
276			$this->onAfterSave($result);
277
278			return $result;
279			}
280
281			/**
282			* Scope to get only active items.
283			*
284			* @param BaseQuery $query
285			*
286			* @return BaseQuery
287			*/
288			public function scopeActive($query)
289			{
290			$query->filter['ACTIVE'] = 'Y';
291
292			return $query;
293			}
294
295			/**
296			* Create an array of fields that will be saved to database.
297			*
298			* @param $selectedFields
299			*
300			* @return array
301			*/
302			protected function normalizeFieldsForSave($selectedFields)
303			{
304			$fields = [];
305			if ($this->fields === null) {
306			return [];
307			}
308
309			foreach ($this->fields as $field => $value) {
310			if (!$this->fieldShouldNotBeSaved($field, $value, $selectedFields)) {
311			$fields[$field] = $value;
312			}
313			}
314
315			return $fields;
316			}
317
318			/**
319			* Determine whether the field should be stopped from passing to "update".
320			*
321			* @param string $field
322			* @param mixed $value
323			* @param array $selectedFields
324			*
325			* @return bool
326			*/
327			protected function fieldShouldNotBeSaved($field, $value, $selectedFields)
			0 ignored issues – show Unused Code introduced 2015-12-01 20:27 UTC by Report Bug Copy Issue Report The parameter `$value` 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...
328			{
329			$blacklistedFields = [
330			'ID',
331			'IBLOCK_ID',
332			'PROPERTIES',
333			'GROUPS',
334			'PROPERTY_VALUES',
335			];
336
337			return (!empty($selectedFields) && !in_array($field, $selectedFields))
338			\|\| in_array($field, $blacklistedFields)
339			\|\| (substr($field, 0, 1) === '~')
340			\|\| (substr($field, 0, 9) === 'PROPERTY_');
341			}
342
343			/**
344			* Instantiate bitrix entity object.
345			*
346			* @throws Exception
347			*
348			* @return object
349			*/
350			public static function instantiateObject()
351			{
352			if (static::$bxObject) {
353			return static::$bxObject;
354			}
355
356			if (class_exists(static::$objectClass)) {
357			return static::$bxObject = new static::$objectClass();
358			}
359
360			throw new Exception('Object initialization failed');
361			}
362
363			/**
364			* Destroy bitrix entity object.
365			*
366			* @return void
367			*/
368			public static function destroyObject()
369			{
370			static::$bxObject = null;
371			}
372
373			/**
374			* Instantiate a query object for the model.
375			*
376			* @throws Exception
377			*
378			* @return BaseQuery
379			*/
380			public static function query()
381			{
382			throw new Exception('public static function query() is not implemented');
383			}
384
385			/**
386			* Set current model id.
387			*
388			* @param $id
389			*/
390			protected function setId($id)
391			{
392			$this->id = $id;
393			$this->fields['ID'] = $id;
394			}
395			}
396

arrilot / bitrix-models

Push — master ( 6ab1e5...2d6522 )

BaseModel::fieldShouldNotBeSaved() B

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like