Skip to content

Commit d8753ba

Browse files
bubasumabubasuma
committed
ACP2E-148: [Cloud] Backend : Cart Price Rules Save operation takes over 10 mins
1 parent 8de016a commit d8753ba

File tree

5 files changed

+254
-12
lines changed

5 files changed

+254
-12
lines changed

‎app/code/Magento/SalesRule/Model/Rule/RuleQuoteRecollectTotalsOnDemand.php

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace Magento\SalesRule\Model\Rule;
1010

11+
use Magento\Framework\DB\Select;
1112
use Magento\Quote\Model\ResourceModel\Quote;
1213
use Magento\SalesRule\Model\Spi\RuleQuoteRecollectTotalsInterface;
1314

@@ -16,6 +17,16 @@
1617
*/
1718
class RuleQuoteRecollectTotalsOnDemand implements RuleQuoteRecollectTotalsInterface
1819
{
20+
/**
21+
* Select queries batch size
22+
*/
23+
private const SELECT_BATCH_SIZE = 10000;
24+
25+
/**
26+
* Update queries batch size
27+
*/
28+
private const UPDATE_BATCH_SIZE = 1000;
29+
1930
/**
2031
* @var Quote
2132
*/
@@ -39,14 +50,32 @@ public function __construct(Quote $quoteResourceModel)
3950
*/
4051
public function execute(int $ruleId): void
4152
{
42-
$this->quoteResourceModel->getConnection()
43-
->update(
44-
$this->quoteResourceModel->getMainTable(),
45-
['trigger_recollect' => 1],
46-
[
47-
'is_active = ?' => 1,
48-
'FIND_IN_SET(?, applied_rule_ids)' => $ruleId
49-
]
50-
);
53+
$connection = $this->quoteResourceModel->getConnection();
54+
55+
$lastEntityId = 0;
56+
do {
57+
$select = $connection->select()
58+
->from($this->quoteResourceModel->getMainTable(), ['entity_id'])
59+
->where('is_active = ?', 1)
60+
->where('FIND_IN_SET(?, applied_rule_ids)', $ruleId)
61+
->where('entity_id > ?', (int)$lastEntityId)
62+
->order('entity_id ' . Select::SQL_ASC)
63+
->limit(self::SELECT_BATCH_SIZE);
64+
$entityIds = $connection->fetchCol($select);
65+
$lastEntityId = null;
66+
if ($entityIds) {
67+
$lastEntityId = $entityIds[self::SELECT_BATCH_SIZE - 1] ?? null;
68+
foreach (array_chunk($entityIds, self::UPDATE_BATCH_SIZE) as $batchEntityIds) {
69+
$connection->update(
70+
$this->quoteResourceModel->getMainTable(),
71+
['trigger_recollect' => 1],
72+
[
73+
'entity_id IN (?)' => array_map('intval', $batchEntityIds),
74+
]
75+
);
76+
}
77+
}
78+
79+
} while ($lastEntityId !== null);
5180
}
5281
}

‎app/code/Magento/SalesRule/Observer/RuleQuoteRecollectTotalsObserver.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ public function execute(Observer $observer): void
4343
{
4444
/** @var Rule $rule */
4545
$rule = $observer->getRule();
46-
if (!$rule->isObjectNew() && (!$rule->getIsActive() || $rule->isDeleted())) {
46+
if (!$rule->isObjectNew()
47+
&& ($rule->isDeleted() || ($rule->dataHasChangedFor('is_active') && !$rule->getIsActive()))
48+
) {
4749
$this->recollectTotals->execute((int) $rule->getId());
4850
}
4951
}

‎app/code/Magento/SalesRule/Test/Unit/Model/Rule/RuleQuoteRecollectTotalsOnDemandTest.php

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\SalesRule\Test\Unit\Model\Rule;
9+
10+
use Magento\Framework\DB\Adapter\Pdo\Mysql;
11+
use Magento\Framework\DB\Select;
12+
use Magento\Quote\Model\ResourceModel\Quote;
13+
use Magento\SalesRule\Model\Rule\RuleQuoteRecollectTotalsOnDemand;
14+
use PHPUnit\Framework\MockObject\MockObject;
15+
use PHPUnit\Framework\TestCase;
16+
17+
class RuleQuoteRecollectTotalsOnDemandTest extends TestCase
18+
{
19+
/**
20+
* @var Quote|MockObject
21+
*/
22+
private $resourceModel;
23+
24+
/**
25+
* @var RuleQuoteRecollectTotalsOnDemand
26+
*/
27+
private $model;
28+
29+
/**
30+
* @inheritdoc
31+
*/
32+
protected function setUp(): void
33+
{
34+
parent::setUp();
35+
$this->resourceModel = $this->getMockBuilder(Quote::class)
36+
->disableOriginalConstructor()
37+
->onlyMethods(['getConnection', 'getMainTable'])
38+
->getMockForAbstractClass();
39+
$this->model = new RuleQuoteRecollectTotalsOnDemand($this->resourceModel);
40+
}
41+
42+
/**
43+
* Test that multiple updates query are executed on large result
44+
*
45+
* @return void
46+
*/
47+
public function testExecute(): void
48+
{
49+
$ruleId = 1;
50+
$mainTableName = 'quote';
51+
$selectRange1 = $this->getMockBuilder(Select::class)
52+
->disableOriginalConstructor()
53+
->onlyMethods(['from', 'where', 'order', 'limit'])
54+
->getMockForAbstractClass();
55+
$selectRange2 = $this->getMockBuilder(Select::class)
56+
->disableOriginalConstructor()
57+
->onlyMethods(['from', 'where', 'order', 'limit'])
58+
->getMockForAbstractClass();
59+
$selectRange1->method('from')
60+
->willReturnSelf();
61+
$selectRange1->method('where')
62+
->withConsecutive(
63+
['is_active = ?', 1],
64+
['FIND_IN_SET(?, applied_rule_ids)', $ruleId],
65+
['entity_id > ?', 0],
66+
)
67+
->willReturnSelf();
68+
$selectRange1->method('order')
69+
->with('entity_id ' . Select::SQL_ASC)
70+
->willReturnSelf();
71+
$selectRange1->method('limit')
72+
->with(10000)
73+
->willReturnSelf();
74+
$selectRange2->method('from')
75+
->willReturnSelf();
76+
$selectRange2->method('where')
77+
->withConsecutive(
78+
['is_active = ?', 1],
79+
['FIND_IN_SET(?, applied_rule_ids)', $ruleId],
80+
['entity_id > ?', 10000],
81+
)
82+
->willReturnSelf();
83+
$selectRange2->method('order')
84+
->with('entity_id ' . Select::SQL_ASC)
85+
->willReturnSelf();
86+
$selectRange2->method('limit')
87+
->with(10000)
88+
->willReturnSelf();
89+
$connection = $this->getMockBuilder(Mysql::class)
90+
->disableOriginalConstructor()
91+
->onlyMethods(['select', 'fetchCol', 'update'])
92+
->getMockForAbstractClass();
93+
$connection->expects($this->exactly(2))
94+
->method('select')
95+
->willReturnOnConsecutiveCalls($selectRange1, $selectRange2);
96+
$connection->expects($this->exactly(2))
97+
->method('fetchCol')
98+
->willReturn(range(1, 10000), range(10001, 18999));
99+
$connection->expects($this->exactly(19))
100+
->method('update')
101+
->withConsecutive(
102+
...array_map(
103+
static function (int $iteration) use ($mainTableName) {
104+
return [
105+
$mainTableName,
106+
['trigger_recollect' => 1],
107+
[
108+
'entity_id IN (?)' => range(
109+
$iteration * 1000 + 1,
110+
min(18999, ($iteration * 1000 + 1000))
111+
),
112+
]
113+
];
114+
},
115+
range(0, 18)
116+
)
117+
);
118+
$this->resourceModel->method('getConnection')
119+
->willReturn($connection);
120+
$this->resourceModel->method('getMainTable')
121+
->willReturn($mainTableName);
122+
123+
$this->model->execute($ruleId);
124+
}
125+
}

‎app/code/Magento/SalesRule/Test/Unit/Observer/RuleQuoteRecollectTotalsObserverTest.php

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\SalesRule\Test\Unit\Observer;
9+
10+
use Magento\Framework\Event\Observer;
11+
use Magento\SalesRule\Model\Rule;
12+
use Magento\SalesRule\Model\Spi\RuleQuoteRecollectTotalsInterface;
13+
use Magento\SalesRule\Observer\RuleQuoteRecollectTotalsObserver;
14+
use PHPUnit\Framework\MockObject\MockObject;
15+
use PHPUnit\Framework\TestCase;
16+
17+
class RuleQuoteRecollectTotalsObserverTest extends TestCase
18+
{
19+
/**
20+
* @var RuleQuoteRecollectTotalsInterface|MockObject
21+
*/
22+
private $ruleQuoteRecollectTotals;
23+
24+
/**
25+
* @var RuleQuoteRecollectTotalsObserver
26+
*/
27+
private $model;
28+
29+
/**
30+
* @inheritdoc
31+
*/
32+
protected function setUp(): void
33+
{
34+
parent::setUp();
35+
$this->ruleQuoteRecollectTotals = $this->getMockForAbstractClass(RuleQuoteRecollectTotalsInterface::class);
36+
$this->model = new RuleQuoteRecollectTotalsObserver($this->ruleQuoteRecollectTotals);
37+
}
38+
39+
/**
40+
* @param array $origData
41+
* @param array $data
42+
* @param bool $isDeleted
43+
* @param bool $recollect
44+
* @return void
45+
* @dataProvider executeDataProvider
46+
*/
47+
public function testExecute(
48+
array $origData,
49+
array $data,
50+
bool $isDeleted,
51+
bool $recollect
52+
): void {
53+
$this->ruleQuoteRecollectTotals->expects($recollect ? $this->once() : $this->never())
54+
->method('execute');
55+
$rule = $this->getMockBuilder(Rule::class)
56+
->disableOriginalConstructor()
57+
->getMockForAbstractClass();
58+
$id = $data['id'] ?? 1;
59+
unset($data['id']);
60+
$rule->isDeleted($isDeleted);
61+
$rule->setData($origData);
62+
$rule->setOrigData();
63+
$rule->setData($data);
64+
$rule->setId($id);
65+
$observer = new Observer(['rule' => $rule]);
66+
$this->model->execute($observer);
67+
}
68+
69+
/**
70+
* @return array[]
71+
*/
72+
public function executeDataProvider(): array
73+
{
74+
return [
75+
[[], ['id' => null], false, false],
76+
[[], [], false, false],
77+
[[], [], true, true],
78+
[[], ['is_active' => false], false, false],
79+
[[], ['is_active' => true], false, false],
80+
[['is_active' => false], ['is_active' => false], false, false],
81+
[['is_active' => false], ['is_active' => true], false, false],
82+
[['is_active' => true], ['is_active' => false], false, true],
83+
[['is_active' => true], ['is_active' => true], false, false],
84+
];
85+
}
86+
}

‎app/code/Magento/SalesRule/etc/events.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@
3030
<event name="sales_quote_address_collect_totals_before">
3131
<observer name="coupon_code_validation" instance="Magento\SalesRule\Observer\CouponCodeValidation" />
3232
</event>
33-
<event name="salesrule_rule_save_after">
33+
<event name="salesrule_rule_save_commit_after">
3434
<observer name="salesrule_quote_recollect_totals_on_disabled" instance="\Magento\SalesRule\Observer\RuleQuoteRecollectTotalsObserver" />
3535
</event>
36-
<event name="salesrule_rule_delete_after">
36+
<event name="salesrule_rule_delete_commit_after">
3737
<observer name="salesrule_quote_recollect_totals_on_delete" instance="\Magento\SalesRule\Observer\RuleQuoteRecollectTotalsObserver" />
3838
</event>
3939
<event name="sales_quote_collect_totals_before">

0 commit comments

Comments
 (0)