Skip to content

Commit 487fdb0

Browse files
bubasumabubasuma
committed
ACP2E-2652: [On-Premise] Re-index process is inefficient when creating Catalog Price Rules
1 parent 4b1ac5f commit 487fdb0

File tree

4 files changed

+165
-71
lines changed

4 files changed

+165
-71
lines changed

‎app/code/Magento/CatalogRule/Model/Indexer/Rule/GetAffectedProductIds.php

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@
1818

1919
namespace Magento\CatalogRule\Model\Indexer\Rule;
2020

21+
use Magento\CatalogRule\Model\ResourceModel\Rule as RuleResourceModel;
2122
use Magento\CatalogRule\Model\ResourceModel\Rule\CollectionFactory;
2223
use Magento\CatalogRule\Model\Rule;
23-
use Magento\Framework\App\ResourceConnection;
2424

2525
class GetAffectedProductIds
2626
{
2727
/**
2828
* @param CollectionFactory $ruleCollectionFactory
29-
* @param ResourceConnection $resource
29+
* @param RuleResourceModel $ruleResourceModel
3030
*/
3131
public function __construct(
3232
private readonly CollectionFactory $ruleCollectionFactory,
33-
private readonly ResourceConnection $resource
33+
private readonly RuleResourceModel $ruleResourceModel
3434
) {
3535
}
3636

@@ -42,20 +42,7 @@ public function __construct(
4242
*/
4343
public function execute(array $ids): array
4444
{
45-
$connection = $this->resource->getConnection();
46-
$select = $connection->select()
47-
->from(
48-
['t' => $this->resource->getTableName('catalogrule_product')],
49-
['t.product_id']
50-
)
51-
->where(
52-
't.rule_id IN (?)',
53-
array_map('intval', $ids)
54-
)
55-
->distinct(
56-
true
57-
);
58-
$productIds = array_map('intval', $connection->fetchCol($select));
45+
$productIds = $this->ruleResourceModel->getProductIdsByRuleIds($ids);
5946
$rules = $this->ruleCollectionFactory->create()
6047
->addFieldToFilter('rule_id', ['in' => array_map('intval', $ids)]);
6148
foreach ($rules as $rule) {

‎app/code/Magento/CatalogRule/Model/ResourceModel/Rule.php

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Magento\CatalogRule\Model\ResourceModel;
1313

1414
use Magento\Catalog\Model\Product;
15+
use Magento\Framework\App\ObjectManager;
16+
use Magento\Framework\EntityManager\EntityManager;
1517
use Magento\Framework\Model\AbstractModel;
1618
use Magento\Framework\Pricing\PriceCurrencyInterface;
1719

@@ -73,7 +75,7 @@ class Rule extends \Magento\Rule\Model\ResourceModel\AbstractResource
7375
protected $priceCurrency;
7476

7577
/**
76-
* @var \Magento\Framework\EntityManager\EntityManager
78+
* @var EntityManager
7779
*/
7880
protected $entityManager;
7981

@@ -90,6 +92,7 @@ class Rule extends \Magento\Rule\Model\ResourceModel\AbstractResource
9092
* @param \Magento\Framework\Stdlib\DateTime $dateTime
9193
* @param PriceCurrencyInterface $priceCurrency
9294
* @param string|null $connectionName
95+
* @param EntityManager|null $entityManager
9396
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
9497
*/
9598
public function __construct(
@@ -103,7 +106,8 @@ public function __construct(
103106
\Psr\Log\LoggerInterface $logger,
104107
\Magento\Framework\Stdlib\DateTime $dateTime,
105108
PriceCurrencyInterface $priceCurrency,
106-
$connectionName = null
109+
$connectionName = null,
110+
?EntityManager $entityManager = null
107111
) {
108112
$this->_storeManager = $storeManager;
109113
$this->_conditionFactory = $conditionFactory;
@@ -114,7 +118,11 @@ public function __construct(
114118
$this->_logger = $logger;
115119
$this->dateTime = $dateTime;
116120
$this->priceCurrency = $priceCurrency;
117-
$this->_associatedEntitiesMap = $this->getAssociatedEntitiesMap();
121+
$this->entityManager = $entityManager ?? ObjectManager::getInstance()->get(EntityManager::class);
122+
$this->_associatedEntitiesMap = ObjectManager::getInstance()
123+
// phpstan:ignore this is a virtual class
124+
->get(\Magento\CatalogRule\Model\ResourceModel\Rule\AssociatedEntityMap::class)
125+
->getData();
118126
parent::__construct($context, $connectionName);
119127
}
120128

@@ -209,7 +217,7 @@ public function getRulesFromProduct($date, $websiteId, $customerGroupId, $produc
209217
*/
210218
public function load(\Magento\Framework\Model\AbstractModel $object, $value, $field = null)
211219
{
212-
$this->getEntityManager()->load($object, $value);
220+
$this->entityManager->load($object, $value);
213221
return $this;
214222
}
215223

@@ -218,7 +226,7 @@ public function load(\Magento\Framework\Model\AbstractModel $object, $value, $fi
218226
*/
219227
public function save(\Magento\Framework\Model\AbstractModel $object)
220228
{
221-
$this->getEntityManager()->save($object);
229+
$this->entityManager->save($object);
222230
return $this;
223231
}
224232

@@ -231,41 +239,31 @@ public function save(\Magento\Framework\Model\AbstractModel $object)
231239
*/
232240
public function delete(AbstractModel $object)
233241
{
234-
$this->getEntityManager()->delete($object);
242+
$this->entityManager->delete($object);
235243
return $this;
236244
}
237245

238246
/**
239-
* Returns instance of associated entity map
247+
* Get product ids matching specified rules
240248
*
249+
* @param array $ruleIds
241250
* @return array
242-
* @deprecated 100.1.0
243-
* @see __construct()
244251
*/
245-
private function getAssociatedEntitiesMap()
252+
public function getProductIdsByRuleIds(array $ruleIds): array
246253
{
247-
if (!$this->_associatedEntitiesMap) {
248-
$this->_associatedEntitiesMap = \Magento\Framework\App\ObjectManager::getInstance()
249-
// phpstan:ignore this is a virtual class
250-
->get(\Magento\CatalogRule\Model\ResourceModel\Rule\AssociatedEntityMap::class)
251-
->getData();
252-
}
253-
return $this->_associatedEntitiesMap;
254-
}
255-
256-
/**
257-
* Returns instance of EntityManager
258-
*
259-
* @return \Magento\Framework\EntityManager\EntityManager
260-
* @deprecated 100.1.0
261-
* @see __construct()
262-
*/
263-
private function getEntityManager()
264-
{
265-
if (null === $this->entityManager) {
266-
$this->entityManager = \Magento\Framework\App\ObjectManager::getInstance()
267-
->get(\Magento\Framework\EntityManager\EntityManager::class);
268-
}
269-
return $this->entityManager;
254+
$connection = $this->getConnection();
255+
$select = $connection->select()
256+
->from(
257+
$this->getTable('catalogrule_product'),
258+
['product_id']
259+
)
260+
->where(
261+
'rule_id IN (?)',
262+
array_map('intval', $ruleIds)
263+
)
264+
->distinct(
265+
true
266+
);
267+
return array_map('intval', $connection->fetchCol($select));
270268
}
271269
}

‎app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/Rule/GetAffectedProductIdsTest.php

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,10 @@
2020

2121
use ArrayIterator;
2222
use Magento\CatalogRule\Model\Indexer\Rule\GetAffectedProductIds;
23+
use Magento\CatalogRule\Model\ResourceModel\Rule as RuleResourceModel;
2324
use Magento\CatalogRule\Model\ResourceModel\Rule\Collection;
2425
use Magento\CatalogRule\Model\ResourceModel\Rule\CollectionFactory;
2526
use Magento\CatalogRule\Model\Rule;
26-
use Magento\Framework\App\ResourceConnection;
27-
use Magento\Framework\DB\Adapter\AdapterInterface;
28-
use Magento\Framework\DB\Select;
2927
use PHPUnit\Framework\MockObject\MockObject;
3028
use PHPUnit\Framework\TestCase;
3129

@@ -40,9 +38,9 @@ class GetAffectedProductIdsTest extends TestCase
4038
private $ruleCollectionFactory;
4139

4240
/**
43-
* @var ResourceConnection|MockObject
41+
* @var RuleResourceModel|MockObject
4442
*/
45-
private $resource;
43+
private $ruleResourceModel;
4644

4745
/**
4846
* @var GetAffectedProductIds
@@ -55,11 +53,11 @@ class GetAffectedProductIdsTest extends TestCase
5553
protected function setUp(): void
5654
{
5755
$this->ruleCollectionFactory = $this->createMock(CollectionFactory::class);
58-
$this->resource = $this->createMock(ResourceConnection::class);
56+
$this->ruleResourceModel = $this->createMock(RuleResourceModel::class);
5957

6058
$this->getAffectedProductIds = new GetAffectedProductIds(
6159
$this->ruleCollectionFactory,
62-
$this->resource
60+
$this->ruleResourceModel
6361
);
6462
}
6563

@@ -71,20 +69,9 @@ public function testExecute(): void
7169
$ruleIds = [1, 2, 5];
7270
$oldMatch = [3, 7, 9];
7371
$newMatch = [6];
74-
$connection = $this->createMock(AdapterInterface::class);
75-
$select = $this->createMock(Select::class);
76-
$connection->expects($this->once())->method('fetchCol')->willReturn($oldMatch);
77-
$connection->expects($this->once())->method('select')->willReturn($select);
78-
$select->expects($this->once())->method('from')->willReturnSelf();
79-
$select->expects($this->once())
80-
->method('where')
81-
->with('t.rule_id IN (?)', $ruleIds)
82-
->willReturnSelf();
83-
$select->expects($this->once())
84-
->method('distinct')
85-
->with(true)
86-
->willReturnSelf();
87-
$this->resource->expects($this->once())->method('getConnection')->willReturn($connection);
72+
$this->ruleResourceModel->expects($this->once())
73+
->method('getProductIdsByRuleIds')
74+
->willReturn($oldMatch);
8875

8976
$collection = $this->createMock(Collection::class);
9077
$rule = $this->createMock(Rule::class);

‎app/code/Magento/CatalogRule/Test/Unit/Model/ResourceModel/RuleTest.php

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
<?php
2+
/************************************************************************
3+
*
4+
* Copyright 2024 Adobe
5+
* All Rights Reserved.
6+
*
7+
* NOTICE: All information contained herein is, and remains
8+
* the property of Adobe and its suppliers, if any. The intellectual
9+
* and technical concepts contained herein are proprietary to Adobe
10+
* and its suppliers and are protected by all applicable intellectual
11+
* property laws, including trade secret and copyright laws.
12+
* Dissemination of this information or reproduction of this material
13+
* is strictly forbidden unless prior written permission is obtained
14+
* from Adobe.
15+
* ************************************************************************
16+
*/
17+
declare(strict_types=1);
18+
19+
namespace Magento\CatalogRule\Test\Unit\Model\ResourceModel;
20+
21+
use Magento\Catalog\Model\Product\ConditionFactory;
22+
use Magento\CatalogRule\Helper\Data;
23+
use Magento\CatalogRule\Model\ResourceModel\Rule;
24+
use Magento\CatalogRule\Model\ResourceModel\Rule\AssociatedEntityMap;
25+
use Magento\Eav\Model\Config;
26+
use Magento\Framework\App\ResourceConnection;
27+
use Magento\Framework\DataObject;
28+
use Magento\Framework\DB\Adapter\AdapterInterface;
29+
use Magento\Framework\DB\Select;
30+
use Magento\Framework\EntityManager\EntityManager;
31+
use Magento\Framework\Event\ManagerInterface;
32+
use Magento\Framework\Model\ResourceModel\Db\Context;
33+
use Magento\Framework\Pricing\PriceCurrencyInterface;
34+
use Magento\Framework\Stdlib\DateTime;
35+
use Magento\Framework\TestFramework\Unit\Listener\ReplaceObjectManager\TestProvidesServiceInterface;
36+
use Magento\Store\Model\StoreManagerInterface;
37+
use PHPUnit\Framework\MockObject\MockObject;
38+
use PHPUnit\Framework\TestCase;
39+
use Psr\Log\LoggerInterface;
40+
41+
/**
42+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
43+
*/
44+
class RuleTest extends TestCase implements TestProvidesServiceInterface
45+
{
46+
/**
47+
* @var ResourceConnection|MockObject
48+
*/
49+
private $resource;
50+
51+
/**
52+
* @var Rule
53+
*/
54+
private $model;
55+
56+
/**
57+
* @inheritDoc
58+
*/
59+
protected function setUp(): void
60+
{
61+
parent::setUp();
62+
63+
$this->resource = $this->createMock(ResourceConnection::class);
64+
$context = $this->createMock(Context::class);
65+
$context->expects($this->once())->method('getResources')->willReturn($this->resource);
66+
67+
$this->model = new Rule(
68+
$context,
69+
$this->createMock(StoreManagerInterface::class),
70+
$this->createMock(ConditionFactory::class),
71+
$this->createMock(DateTime\DateTime::class),
72+
$this->createMock(Config::class),
73+
$this->createMock(ManagerInterface::class),
74+
$this->createMock(Data::class),
75+
$this->createMock(LoggerInterface::class),
76+
$this->createMock(DateTime::class),
77+
$this->createMock(PriceCurrencyInterface::class),
78+
null,
79+
$this->createMock(EntityManager::class),
80+
);
81+
}
82+
83+
/**
84+
* @return void
85+
*/
86+
public function testExecute(): void
87+
{
88+
$ruleIds = [1, 2, 5];
89+
$productIds = [3, 7, 9];
90+
$connection = $this->createMock(AdapterInterface::class);
91+
$select = $this->createMock(Select::class);
92+
$this->resource->expects($this->once())->method('getConnection')->willReturn($connection);
93+
$this->resource->expects($this->once())->method('getTableName')->willReturnArgument(0);
94+
$connection->expects($this->once())->method('select')->willReturn($select);
95+
$connection->expects($this->once())->method('fetchCol')->willReturn($productIds);
96+
$select->expects($this->once())
97+
->method('from')
98+
->with('catalogrule_product', ['product_id'])
99+
->willReturnSelf();
100+
$select->expects($this->once())
101+
->method('where')
102+
->with('rule_id IN (?)', $ruleIds)
103+
->willReturnSelf();
104+
$select->expects($this->once())
105+
->method('distinct')
106+
->with(true)
107+
->willReturnSelf();
108+
109+
$this->assertEquals($productIds, $this->model->getProductIdsByRuleIds($ruleIds));
110+
}
111+
112+
/**
113+
* @inheritDoc
114+
*/
115+
public function getServiceForObjectManager(string $type): ?object
116+
{
117+
// phpstan:ignore this is a virtual class
118+
return $type === AssociatedEntityMap::class
119+
? new DataObject()
120+
: null;
121+
}
122+
}

0 commit comments

Comments
 (0)