Skip to content

Improve getCustomAttribute() performance #13308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Feb 22, 2018
Merged

Improve getCustomAttribute() performance #13308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
08ab509
Refactor: move interface attributes array to interface itself
schmengler Jan 22, 2018
74deee4
Move getCustomAttributeCodes from product model to resource model
schmengler Jan 22, 2018
53e03b2
Update unit tests
schmengler Jan 22, 2018
ddc1b03
Move getCustomAttributeCodes from category model to resource model
schmengler Jan 22, 2018
e69be44
Add missing method call for reflection class name
schmengler Jan 22, 2018
ab558bd
Fix error with uninstantiated resource model
schmengler Jan 22, 2018
7d4523c
Make new properties private
schmengler Feb 8, 2018
98a9c36
WIP: GetCustomAttributeCodes service
schmengler Feb 9, 2018
8378eff
Move retrieval of custom attributes to dedicated services
schmengler Feb 15, 2018
c587d3a
Merge branch '2.3-develop' into custom-attributes-performance
nmalevanec Feb 21, 2018
208b1ec
Test stabilization.
nmalevanec Feb 21, 2018
f286a98
Test stabilization.
nmalevanec Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions app/code/Magento/Catalog/Api/Data/CategoryInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,37 @@
*/
interface CategoryInterface extends \Magento\Framework\Api\CustomAttributesDataInterface
{
/**#@+
* Constants defined for keys of data array
*/
const KEY_PARENT_ID = 'parent_id';
const KEY_NAME = 'name';
const KEY_IS_ACTIVE = 'is_active';
const KEY_POSITION = 'position';
const KEY_LEVEL = 'level';
const KEY_UPDATED_AT = 'updated_at';
const KEY_CREATED_AT = 'created_at';
const KEY_PATH = 'path';
const KEY_AVAILABLE_SORT_BY = 'available_sort_by';
const KEY_INCLUDE_IN_MENU = 'include_in_menu';
const KEY_PRODUCT_COUNT = 'product_count';
const KEY_CHILDREN_DATA = 'children_data';

const ATTRIBUTES = [
'id',
self::KEY_PARENT_ID,
self::KEY_NAME,
self::KEY_IS_ACTIVE,
self::KEY_POSITION,
self::KEY_LEVEL,
self::KEY_UPDATED_AT,
self::KEY_CREATED_AT,
self::KEY_AVAILABLE_SORT_BY,
self::KEY_INCLUDE_IN_MENU,
self::KEY_CHILDREN_DATA,
];
/**#@-*/

/**
* @return int|null
*/
Expand Down
18 changes: 18 additions & 0 deletions app/code/Magento/Catalog/Api/Data/ProductInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,24 @@ interface ProductInterface extends \Magento\Framework\Api\CustomAttributesDataIn

const UPDATED_AT = 'updated_at';

const MEDIA_GALLERY = 'media_gallery';

const TIER_PRICE = 'tier_price';

const ATTRIBUTES = [
self::SKU,
self::NAME,
self::PRICE,
self::WEIGHT,
self::STATUS,
self::VISIBILITY,
self::ATTRIBUTE_SET_ID,
self::TYPE_ID,
self::CREATED_AT,
self::UPDATED_AT,
self::MEDIA_GALLERY,
self::TIER_PRICE,
];
/**#@-*/

/**
Expand Down
55 changes: 19 additions & 36 deletions app/code/Magento/Catalog/Model/Category.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
namespace Magento\Catalog\Model;

use Magento\Catalog\Api\CategoryRepositoryInterface;
use Magento\Catalog\Api\Data\CategoryInterface;
use Magento\Catalog\Model\Entity\GetCategoryCustomAttributeCodes;
use Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator;
use Magento\Eav\Model\Entity\GetCustomAttributeCodesInterface;
use Magento\Framework\Api\AttributeValueFactory;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\Convert\ConvertArray;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\Profiler;
Expand Down Expand Up @@ -69,23 +73,6 @@ class Category extends \Magento\Catalog\Model\AbstractModel implements

const CACHE_TAG = 'cat_c';

/**#@+
* Constants
*/
const KEY_PARENT_ID = 'parent_id';
const KEY_NAME = 'name';
const KEY_IS_ACTIVE = 'is_active';
const KEY_POSITION = 'position';
const KEY_LEVEL = 'level';
const KEY_UPDATED_AT = 'updated_at';
const KEY_CREATED_AT = 'created_at';
const KEY_PATH = 'path';
const KEY_AVAILABLE_SORT_BY = 'available_sort_by';
const KEY_INCLUDE_IN_MENU = 'include_in_menu';
const KEY_PRODUCT_COUNT = 'product_count';
const KEY_CHILDREN_DATA = 'children_data';
/**#@-*/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constants removal from class and moving them to interface is backward incompatible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? TheClass::THE_CONSTANT still works

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schmengler
Possible fatal error if there is another implementation of the interface with same constant name declaration.
But this is more like the theoretical issue.

Currently, I raised this question for voting on the Internal Architectural Council to allow moving constants preserving names and values from Implementation to Interface being implemented. And thus fix our Backward Compatibility Guideline http://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html saying

Removing or renaming constants
Do not remove or rename constants.

And to adapt internal tests which break on such changes.

I will get back to you shortly as soon as get results.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maghamed It wouldn't even be a problem: https://3v4l.org/H4mYd 😄

/**#@-*/
protected $_eventPrefix = 'catalog_category';

Expand Down Expand Up @@ -142,21 +129,11 @@ class Category extends \Magento\Catalog\Model\AbstractModel implements
/**
* Attributes are that part of interface
*
* @deprecated
* @see CategoryInterface::ATTRIBUTES
* @var array
*/
protected $interfaceAttributes = [
'id',
self::KEY_PARENT_ID,
self::KEY_NAME,
self::KEY_IS_ACTIVE,
self::KEY_POSITION,
self::KEY_LEVEL,
self::KEY_UPDATED_AT,
self::KEY_CREATED_AT,
self::KEY_AVAILABLE_SORT_BY,
self::KEY_INCLUDE_IN_MENU,
self::KEY_CHILDREN_DATA,
];
protected $interfaceAttributes = CategoryInterface::ATTRIBUTES;

/**
* Category tree model
Expand Down Expand Up @@ -230,6 +207,11 @@ class Category extends \Magento\Catalog\Model\AbstractModel implements
*/
protected $metadataService;

/**
* @var GetCustomAttributeCodesInterface
*/
private $getCustomAttributeCodes;

/**
* @param \Magento\Framework\Model\Context $context
* @param \Magento\Framework\Registry $registry
Expand All @@ -252,6 +234,7 @@ class Category extends \Magento\Catalog\Model\AbstractModel implements
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
* @param array $data
* @param GetCustomAttributeCodesInterface|null $getCustomAttributeCodes
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -275,7 +258,8 @@ public function __construct(
CategoryRepositoryInterface $categoryRepository,
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
array $data = []
array $data = [],
GetCustomAttributeCodesInterface $getCustomAttributeCodes = null
) {
$this->metadataService = $metadataService;
$this->_treeModel = $categoryTreeResource;
Expand All @@ -290,6 +274,9 @@ public function __construct(
$this->urlFinder = $urlFinder;
$this->indexerRegistry = $indexerRegistry;
$this->categoryRepository = $categoryRepository;
$this->getCustomAttributeCodes = $getCustomAttributeCodes ?? ObjectManager::getInstance()->get(
GetCategoryCustomAttributeCodes::class
);
parent::__construct(
$context,
$registry,
Expand Down Expand Up @@ -323,11 +310,7 @@ protected function _construct()
*/
protected function getCustomAttributesCodes()
{
if ($this->customAttributesCodes === null) {
$this->customAttributesCodes = $this->getEavAttributesCodes($this->metadataService);
$this->customAttributesCodes = array_diff($this->customAttributesCodes, $this->interfaceAttributes);
}
return $this->customAttributesCodes;
return $this->getCustomAttributeCodes->execute($this->metadataService);
}

/**
Expand Down
37 changes: 37 additions & 0 deletions app/code/Magento/Catalog/Model/Entity/GetCategoryCustomAttributeCodes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Catalog\Model\Entity;

use Magento\Catalog\Api\Data\CategoryInterface;
use Magento\Eav\Model\Entity\GetCustomAttributeCodesInterface;
use Magento\Framework\Api\MetadataServiceInterface;

class GetCategoryCustomAttributeCodes implements GetCustomAttributeCodesInterface
{
/**
* @var GetCustomAttributeCodesInterface
*/
private $baseCustomAttributeCodes;

/**
* @param GetCustomAttributeCodesInterface $baseCustomAttributeCodes
*/
public function __construct(
GetCustomAttributeCodesInterface $baseCustomAttributeCodes
) {
$this->baseCustomAttributeCodes = $baseCustomAttributeCodes;
}

/**
* @inheritdoc
*/
public function execute(MetadataServiceInterface $metadataService): array
{
$customAttributesCodes = $this->baseCustomAttributeCodes->execute($metadataService);
return array_diff($customAttributesCodes, CategoryInterface::ATTRIBUTES);
}
}
37 changes: 37 additions & 0 deletions app/code/Magento/Catalog/Model/Entity/GetProductCustomAttributeCodes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Catalog\Model\Entity;

use Magento\Catalog\Api\Data\ProductInterface;
use Magento\Eav\Model\Entity\GetCustomAttributeCodesInterface;
use Magento\Framework\Api\MetadataServiceInterface;

class GetProductCustomAttributeCodes implements GetCustomAttributeCodesInterface
{
/**
* @var GetCustomAttributeCodesInterface
*/
private $baseCustomAttributeCodes;

/**
* @param GetCustomAttributeCodesInterface $baseCustomAttributeCodes
*/
public function __construct(
GetCustomAttributeCodesInterface $baseCustomAttributeCodes
) {
$this->baseCustomAttributeCodes = $baseCustomAttributeCodes;
}

/**
* @inheritdoc
*/
public function execute(MetadataServiceInterface $metadataService): array
{
$customAttributesCodes = $this->baseCustomAttributeCodes->execute($metadataService);
return array_diff($customAttributesCodes, ProductInterface::ATTRIBUTES);
}
}
40 changes: 19 additions & 21 deletions app/code/Magento/Catalog/Model/Product.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
use Magento\Catalog\Api\Data\ProductAttributeMediaGalleryEntryInterface;
use Magento\Catalog\Api\Data\ProductInterface;
use Magento\Catalog\Api\ProductLinkRepositoryInterface;
use Magento\Catalog\Model\Entity\GetProductCustomAttributeCodes;
use Magento\Catalog\Model\Product\Attribute\Backend\Media\EntryConverterPool;
use Magento\Eav\Model\Entity\GetCustomAttributeCodesInterface;
use Magento\Framework\Api\AttributeValueFactory;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\DataObject\IdentityInterface;
use Magento\Framework\Pricing\SaleableInterface;

Expand Down Expand Up @@ -305,22 +308,12 @@ class Product extends \Magento\Catalog\Model\AbstractModel implements

/**
* List of attributes in ProductInterface
*
* @deprecated
* @see ProductInterface::ATTRIBUTES
* @var array
*/
protected $interfaceAttributes = [
ProductInterface::SKU,
ProductInterface::NAME,
ProductInterface::PRICE,
ProductInterface::WEIGHT,
ProductInterface::STATUS,
ProductInterface::VISIBILITY,
ProductInterface::ATTRIBUTE_SET_ID,
ProductInterface::TYPE_ID,
ProductInterface::CREATED_AT,
ProductInterface::UPDATED_AT,
'media_gallery',
'tier_price',
];
protected $interfaceAttributes = ProductInterface::ATTRIBUTES;

/**
* @var \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface
Expand All @@ -346,6 +339,11 @@ class Product extends \Magento\Catalog\Model\AbstractModel implements
*/
protected $linkTypeProvider;

/**
* @var GetCustomAttributeCodesInterface
*/
private $getCustomAttributeCodes;

/**
* Product constructor.
* @param \Magento\Framework\Model\Context $context
Expand Down Expand Up @@ -383,7 +381,7 @@ class Product extends \Magento\Catalog\Model\AbstractModel implements
* @param \Magento\Framework\Api\DataObjectHelper $dataObjectHelper
* @param \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $joinProcessor
* @param array $data
*
* @param GetCustomAttributeCodesInterface|null $getCustomAttributeCodes
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
Expand Down Expand Up @@ -422,7 +420,8 @@ public function __construct(
EntryConverterPool $mediaGalleryEntryConverterPool,
\Magento\Framework\Api\DataObjectHelper $dataObjectHelper,
\Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $joinProcessor,
array $data = []
array $data = [],
GetCustomAttributeCodesInterface $getCustomAttributeCodes = null
) {
$this->metadataService = $metadataService;
$this->_itemOptionFactory = $itemOptionFactory;
Expand Down Expand Up @@ -451,6 +450,9 @@ public function __construct(
$this->mediaGalleryEntryConverterPool = $mediaGalleryEntryConverterPool;
$this->dataObjectHelper = $dataObjectHelper;
$this->joinProcessor = $joinProcessor;
$this->getCustomAttributeCodes = $getCustomAttributeCodes ?? ObjectManager::getInstance()->get(
GetProductCustomAttributeCodes::class
);
parent::__construct(
$context,
$registry,
Expand Down Expand Up @@ -478,11 +480,7 @@ protected function _construct()
*/
protected function getCustomAttributesCodes()
{
if ($this->customAttributesCodes === null) {
$this->customAttributesCodes = $this->getEavAttributesCodes($this->metadataService);
$this->customAttributesCodes = array_diff($this->customAttributesCodes, $this->interfaceAttributes);
}
return $this->customAttributesCodes;
return $this->getCustomAttributeCodes->execute($this->metadataService);
}

/**
Expand Down
Loading