Skip to content

Commit 53f9407

Browse files
bubasumabubasuma
committed
ACP2E-1016: [Performance] Updating Tier price through API generates unnecessary SELECT queries
1 parent cd826aa commit 53f9407

File tree

5 files changed

+301
-45
lines changed

5 files changed

+301
-45
lines changed

‎app/code/Magento/Catalog/Model/Product/Price/TierPriceFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ public function create(array $rawPrice, $sku)
9595
$price->setCustomerGroup(
9696
$rawPrice['all_groups'] == $this->allGroupsId
9797
? $this->allGroupsValue
98-
: $this->customerGroupRepository->getById($rawPrice['customer_group_id'])->getCode()
98+
: ($rawPrice['customer_group_code']
99+
?? $this->customerGroupRepository->getById($rawPrice['customer_group_id'])->getCode())
99100
);
100101
$price->setQuantity($rawPrice['qty']);
101102

‎app/code/Magento/Catalog/Model/Product/Price/TierPriceStorage.php

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Catalog\Model\Indexer\Product\Price\Processor as PriceIndexerProcessor;
1212
use Magento\Catalog\Model\Product\Price\Validation\TierPriceValidator;
1313
use Magento\Catalog\Model\ProductIdLocatorInterface;
14+
use Magento\Customer\Model\ResourceModel\Group\GetCustomerGroupCodesByIds;
1415

1516
class TierPriceStorage implements TierPriceStorageInterface
1617
{
@@ -22,8 +23,6 @@ class TierPriceStorage implements TierPriceStorageInterface
2223
private $tierPricePersistence;
2324

2425
/**
25-
* Tier price validator.
26-
*
2726
* @var TierPriceValidator
2827
*/
2928
private $tierPriceValidator;
@@ -36,38 +35,42 @@ class TierPriceStorage implements TierPriceStorageInterface
3635
private $tierPriceFactory;
3736

3837
/**
39-
* Price index processor.
40-
*
4138
* @var PriceIndexerProcessor
4239
*/
4340
private $priceIndexProcessor;
4441

4542
/**
46-
* Product ID locator.
47-
*
4843
* @var ProductIdLocatorInterface
4944
*/
5045
private $productIdLocator;
5146

47+
/**
48+
* @var GetCustomerGroupCodesByIds
49+
*/
50+
private $getCustomerGroupCodesByIds;
51+
5252
/**
5353
* @param TierPricePersistence $tierPricePersistence
5454
* @param TierPriceValidator $tierPriceValidator
5555
* @param TierPriceFactory $tierPriceFactory
5656
* @param PriceIndexerProcessor $priceIndexProcessor
5757
* @param ProductIdLocatorInterface $productIdLocator
58+
* @param GetCustomerGroupCodesByIds $getCustomerGroupCodesByIds
5859
*/
5960
public function __construct(
6061
TierPricePersistence $tierPricePersistence,
6162
TierPriceValidator $tierPriceValidator,
6263
TierPriceFactory $tierPriceFactory,
6364
PriceIndexerProcessor $priceIndexProcessor,
64-
ProductIdLocatorInterface $productIdLocator
65+
ProductIdLocatorInterface $productIdLocator,
66+
GetCustomerGroupCodesByIds $getCustomerGroupCodesByIds
6567
) {
6668
$this->tierPricePersistence = $tierPricePersistence;
6769
$this->tierPriceValidator = $tierPriceValidator;
6870
$this->tierPriceFactory = $tierPriceFactory;
6971
$this->priceIndexProcessor = $priceIndexProcessor;
7072
$this->productIdLocator = $productIdLocator;
73+
$this->getCustomerGroupCodesByIds = $getCustomerGroupCodesByIds;
7174
}
7275

7376
/**
@@ -148,8 +151,22 @@ private function getExistingPrices(array $skus, bool $groupBySku = false): array
148151
if ($rawPrices) {
149152
$linkField = $this->tierPricePersistence->getEntityLinkField();
150153
$skuByIdLookup = $this->buildSkuByIdLookup($skus);
154+
$customerGroupCodesByIds = $this->getCustomerGroupCodesByIds->execute(
155+
array_column(
156+
array_filter(
157+
$rawPrices,
158+
static function (array $row) {
159+
return (int) $row['all_groups'] !== 1;
160+
}
161+
),
162+
'customer_group_id'
163+
),
164+
);
151165
foreach ($rawPrices as $rawPrice) {
152166
$sku = $skuByIdLookup[$rawPrice[$linkField]];
167+
if (isset($customerGroupCodesByIds[$rawPrice['customer_group_id']])) {
168+
$rawPrice['customer_group_code'] = $customerGroupCodesByIds[$rawPrice['customer_group_id']];
169+
}
153170
$price = $this->tierPriceFactory->create($rawPrice, $sku);
154171
if ($groupBySku) {
155172
$prices[$sku][] = $price;

‎app/code/Magento/Catalog/Test/Unit/Model/Product/Price/TierPriceFactoryTest.php

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
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\Catalog\Test\Unit\Model\Product\Price;
9+
10+
use Magento\Catalog\Api\Data\TierPriceInterfaceFactory;
11+
use Magento\Catalog\Model\Product\Price\TierPrice;
12+
use Magento\Catalog\Model\Product\Price\TierPriceFactory;
13+
use Magento\Catalog\Model\Product\Price\TierPricePersistence;
14+
use Magento\Customer\Api\Data\GroupInterface;
15+
use Magento\Customer\Api\GroupRepositoryInterface;
16+
use Magento\Framework\Api\FilterBuilder;
17+
use Magento\Framework\Api\SearchCriteriaBuilder;
18+
use PHPUnit\Framework\MockObject\MockObject;
19+
use PHPUnit\Framework\TestCase;
20+
21+
class TierPriceFactoryTest extends TestCase
22+
{
23+
/**
24+
* @var TierPriceInterfaceFactory|MockObject
25+
*/
26+
private $tierPriceFactory;
27+
28+
/**
29+
* @var TierPricePersistence|MockObject
30+
*/
31+
private $tierPricePersistence;
32+
33+
/**
34+
* @var GroupRepositoryInterface|MockObject
35+
*/
36+
private $customerGroupRepository;
37+
38+
/**
39+
* @var SearchCriteriaBuilder|MockObject
40+
*/
41+
private $searchCriteriaBuilder;
42+
43+
/**
44+
* @var FilterBuilder|MockObject
45+
*/
46+
private $filterBuilder;
47+
48+
/**
49+
* @var TierPriceFactory
50+
*/
51+
private $model;
52+
53+
/**
54+
* @inheritdoc
55+
*/
56+
protected function setUp(): void
57+
{
58+
parent::setUp();
59+
60+
$this->tierPriceFactory = $this->createMock(TierPriceInterfaceFactory::class);
61+
$this->tierPricePersistence = $this->createMock(TierPricePersistence::class);
62+
$this->customerGroupRepository = $this->createMock(GroupRepositoryInterface::class);
63+
$this->searchCriteriaBuilder = $this->createMock(SearchCriteriaBuilder::class);
64+
$this->filterBuilder = $this->createMock(FilterBuilder::class);
65+
66+
$this->model = new TierPriceFactory(
67+
$this->tierPriceFactory,
68+
$this->tierPricePersistence,
69+
$this->customerGroupRepository,
70+
$this->searchCriteriaBuilder,
71+
$this->filterBuilder
72+
);
73+
}
74+
75+
/**
76+
* @dataProvider createDataProvider
77+
* @param array $rawData
78+
* @param array $expected
79+
* @return void
80+
*/
81+
public function testCreate(array $rawData, array $expected): void
82+
{
83+
$rawData = array_merge(
84+
[
85+
'value_id' => 1,
86+
'entity_id' => 1,
87+
'all_groups' => 1,
88+
'customer_group_id' => 0,
89+
'qty' => 2.0000,
90+
'value' => 2.0000,
91+
'percentage_value' => null,
92+
'website_id' => 0
93+
],
94+
$rawData
95+
);
96+
$expected = array_merge(
97+
[
98+
'sku' => 'simple',
99+
'price' => 2.0,
100+
'price_type' => TierPrice::PRICE_TYPE_FIXED,
101+
'website_id' => 0,
102+
'quantity' => 2.000,
103+
'customer_group' => 'all groups'
104+
],
105+
$expected
106+
);
107+
$customerGroupMock = $this->getMockForAbstractClass(GroupInterface::class);
108+
$customerGroupMock->method('getCode')
109+
->willReturn('NOT LOGGED IN');
110+
111+
$isCustomerGroupResolved = isset($rawData['customer_group_code']) || $rawData['all_groups'];
112+
$this->customerGroupRepository->expects($isCustomerGroupResolved ? $this->never() : $this->once())
113+
->method('getById')
114+
->willReturn($customerGroupMock);
115+
$expectedTierPrice = $this->getMockBuilder(TierPrice::class)
116+
->disableOriginalConstructor()
117+
->getMockForAbstractClass();
118+
$expectedTierPrice->setData($expected);
119+
$tierPriceMock = $this->getMockBuilder(TierPrice::class)
120+
->disableOriginalConstructor()
121+
->getMockForAbstractClass();
122+
$this->tierPriceFactory->method('create')
123+
->willReturn($tierPriceMock);
124+
$tierPrice = $this->model->create($rawData, 'simple');
125+
$this->assertEquals($expectedTierPrice->getSku(), $tierPrice->getSku());
126+
$this->assertEquals($expectedTierPrice->getPrice(), $tierPrice->getPrice());
127+
$this->assertEquals($expectedTierPrice->getPriceType(), $tierPrice->getPriceType());
128+
$this->assertEquals($expectedTierPrice->getWebsiteId(), $tierPrice->getWebsiteId());
129+
$this->assertEquals($expectedTierPrice->getCustomerGroup(), $tierPrice->getCustomerGroup());
130+
$this->assertEquals($expectedTierPrice->getQuantity(), $tierPrice->getQuantity());
131+
}
132+
133+
/**
134+
* @return array
135+
*/
136+
public function createDataProvider(): array
137+
{
138+
return [
139+
[
140+
[],
141+
[]
142+
],
143+
[
144+
[
145+
'all_groups' => 0,
146+
'customer_group_id' => 1,
147+
],
148+
[
149+
'customer_group' => 'NOT LOGGED IN'
150+
]
151+
],
152+
[
153+
[
154+
'all_groups' => 0,
155+
'customer_group_id' => 2,
156+
'customer_group_code' => 'custom',
157+
],
158+
[
159+
'customer_group' => 'custom'
160+
]
161+
]
162+
];
163+
}
164+
}

0 commit comments

Comments
 (0)