Skip to content

Commit 58edd7f

Browse files
author
Yaroslav Onischenko
authored
Merge pull request #990 from magento-dragons/perf-pr-configurable
[Dragons][Performance] Configurable Product
2 parents 436d046 + e9d62bc commit 58edd7f

File tree

13 files changed

+511
-12
lines changed

13 files changed

+511
-12
lines changed

app/code/Magento/Catalog/Block/Product/ListProduct.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,11 +317,15 @@ public function getProductPrice(\Magento\Catalog\Model\Product $product)
317317
}
318318

319319
/**
320+
* Specifies that price rendering should be done for the list of products
321+
* i.e. rendering happens in the scope of product list, but not single product
322+
*
320323
* @return \Magento\Framework\Pricing\Render
321324
*/
322325
protected function getPriceRender()
323326
{
324-
return $this->getLayout()->getBlock('product.price.render.default');
327+
return $this->getLayout()->getBlock('product.price.render.default')
328+
->setData('is_product_list', true);
325329
}
326330

327331
/**

app/code/Magento/Catalog/Pricing/Render/FinalPriceBox.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,19 @@ public function getCacheKeyInfo()
193193
{
194194
$cacheKeys = parent::getCacheKeyInfo();
195195
$cacheKeys['display_minimal_price'] = $this->getDisplayMinimalPrice();
196+
$cacheKeys['is_product_list'] = $this->isProductList();
196197
return $cacheKeys;
197198
}
199+
200+
/**
201+
* Get flag that price rendering should be done for the list of products
202+
* By default (if flag is not set) is false
203+
*
204+
* @return bool
205+
*/
206+
public function isProductList()
207+
{
208+
$isProductList = $this->getData('is_product_list');
209+
return $isProductList === true;
210+
}
198211
}

app/code/Magento/Catalog/Test/Unit/Block/Product/ListProductTest.php

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
*/
66
namespace Magento\Catalog\Test\Unit\Block\Product;
77

8+
use Magento\Catalog\Block\Product\Context;
9+
use Magento\Framework\Event\ManagerInterface;
10+
use Magento\Framework\Pricing\Render;
11+
use Magento\Framework\Url\Helper\Data;
12+
use Magento\Framework\View\LayoutInterface;
13+
814
/**
915
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1016
*/
@@ -46,7 +52,7 @@ class ListProductTest extends \PHPUnit_Framework_TestCase
4652
protected $typeInstanceMock;
4753

4854
/**
49-
* @var \Magento\Framework\Url\Helper\Data | \PHPUnit_Framework_MockObject_MockObject
55+
* @var Data | \PHPUnit_Framework_MockObject_MockObject
5056
*/
5157
protected $urlHelperMock;
5258

@@ -70,6 +76,16 @@ class ListProductTest extends \PHPUnit_Framework_TestCase
7076
*/
7177
protected $toolbarMock;
7278

79+
/**
80+
* @var Context|\PHPUnit_Framework_MockObject_MockObject
81+
*/
82+
private $context;
83+
84+
/**
85+
* @var Render|\PHPUnit_Framework_MockObject_MockObject
86+
*/
87+
private $renderer;
88+
7389
protected function setUp()
7490
{
7591
$objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -141,20 +157,28 @@ protected function setUp()
141157
false
142158
);
143159

144-
$this->urlHelperMock = $this->getMockBuilder(\Magento\Framework\Url\Helper\Data::class)
145-
->disableOriginalConstructor()->getMock();
160+
$this->urlHelperMock = $this->getMockBuilder(Data::class)->disableOriginalConstructor()->getMock();
161+
$this->context = $this->getMockBuilder(Context::class)->disableOriginalConstructor()->getMock();
162+
$this->renderer = $this->getMockBuilder(Render::class)->disableOriginalConstructor()->getMock();
163+
$eventManager = $this->getMockForAbstractClass(ManagerInterface::class, [], '', false);
164+
165+
$this->context->expects($this->any())->method('getRegistry')->willReturn($this->registryMock);
166+
$this->context->expects($this->any())->method('getCartHelper')->willReturn($this->cartHelperMock);
167+
$this->context->expects($this->any())->method('getLayout')->willReturn($this->layoutMock);
168+
$this->context->expects($this->any())->method('getEventManager')->willReturn($eventManager);
169+
146170
$this->block = $objectManager->getObject(
147171
\Magento\Catalog\Block\Product\ListProduct::class,
148172
[
149173
'registry' => $this->registryMock,
174+
'context' => $this->context,
150175
'layerResolver' => $layerResolver,
151176
'cartHelper' => $this->cartHelperMock,
152177
'postDataHelper' => $this->postDataHelperMock,
153178
'urlHelper' => $this->urlHelperMock,
154179
]
155180
);
156181
$this->block->setToolbarBlockName('mock');
157-
$this->block->setLayout($this->layoutMock);
158182
}
159183

160184
protected function tearDown()
@@ -257,4 +281,18 @@ public function testGetAddToCartPostParams()
257281
$result = $this->block->getAddToCartPostParams($this->productMock);
258282
$this->assertEquals($expectedPostData, $result);
259283
}
284+
285+
public function testSetIsProductListFlagOnGetProductPrice()
286+
{
287+
$this->renderer->expects($this->once())
288+
->method('setData')
289+
->with('is_product_list', true)
290+
->willReturnSelf();
291+
$this->layoutMock->expects($this->once())
292+
->method('getBlock')
293+
->with('product.price.render.default')
294+
->willReturn($this->renderer);
295+
296+
$this->block->getProductPrice($this->productMock);
297+
}
260298
}

app/code/Magento/Catalog/Test/Unit/Pricing/Render/FinalPriceBoxTest.php

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,63 @@ public function testGetCacheKey()
401401
$this->assertStringEndsWith('list-category-page', $result);
402402
}
403403

404-
public function testGetCacheKeyInfo()
404+
public function testGetCacheKeyInfoContainsDisplayMinimalPrice()
405405
{
406406
$this->assertArrayHasKey('display_minimal_price', $this->object->getCacheKeyInfo());
407407
}
408+
409+
/**
410+
* Test when is_product_list flag is not specified
411+
*/
412+
public function testGetCacheKeyInfoContainsIsProductListFlagByDefault()
413+
{
414+
$cacheInfo = $this->object->getCacheKeyInfo();
415+
self::assertArrayHasKey('is_product_list', $cacheInfo);
416+
self::assertFalse($cacheInfo['is_product_list']);
417+
}
418+
419+
/**
420+
* Test when is_product_list flag is specified
421+
*
422+
* @param bool $flag
423+
* @dataProvider isProductListDataProvider
424+
*/
425+
public function testGetCacheKeyInfoContainsIsProductListFlag($flag)
426+
{
427+
$this->object->setData('is_product_list', $flag);
428+
$cacheInfo = $this->object->getCacheKeyInfo();
429+
self::assertArrayHasKey('is_product_list', $cacheInfo);
430+
self::assertEquals($flag, $cacheInfo['is_product_list']);
431+
}
432+
433+
/**
434+
* Test when is_product_list flag is not specified
435+
*/
436+
public function testIsProductListByDefault()
437+
{
438+
self::assertFalse($this->object->isProductList());
439+
}
440+
441+
/**
442+
* Test when is_product_list flag is specified
443+
*
444+
* @param bool $flag
445+
* @dataProvider isProductListDataProvider
446+
*/
447+
public function testIsProductList($flag)
448+
{
449+
$this->object->setData('is_product_list', $flag);
450+
self::assertEquals($flag, $this->object->isProductList());
451+
}
452+
453+
/**
454+
* @return array
455+
*/
456+
public function isProductListDataProvider()
457+
{
458+
return [
459+
'is_not_product_list' => [false],
460+
'is_product_list' => [true],
461+
];
462+
}
408463
}

app/code/Magento/ConfigurableProduct/Model/Product/Type/Configurable.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use Magento\Catalog\Api\Data\ProductAttributeInterface;
99
use Magento\Catalog\Api\Data\ProductInterface;
10+
use Magento\Catalog\Api\Data\ProductInterfaceFactory;
1011
use Magento\Catalog\Api\ProductRepositoryInterface;
1112
use Magento\Catalog\Model\Config;
1213
use Magento\Framework\App\ObjectManager;
@@ -163,6 +164,11 @@ class Configurable extends \Magento\Catalog\Model\Product\Type\AbstractType
163164
*/
164165
private $customerSession;
165166

167+
/**
168+
* @var ProductInterfaceFactory
169+
*/
170+
private $productFactory;
171+
166172
/**
167173
* @codingStandardsIgnoreStart/End
168174
*
@@ -184,6 +190,7 @@ class Configurable extends \Magento\Catalog\Model\Product\Type\AbstractType
184190
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
185191
* @param \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $extensionAttributesJoinProcessor
186192
* @param \Magento\Framework\Serialize\Serializer\Json $serializer
193+
* @param ProductInterfaceFactory $productFactory
187194
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
188195
*/
189196
public function __construct(
@@ -206,7 +213,8 @@ public function __construct(
206213
\Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $extensionAttributesJoinProcessor,
207214
\Magento\Framework\Cache\FrontendInterface $cache = null,
208215
\Magento\Customer\Model\Session $customerSession = null,
209-
\Magento\Framework\Serialize\Serializer\Json $serializer = null
216+
\Magento\Framework\Serialize\Serializer\Json $serializer = null,
217+
ProductInterfaceFactory $productFactory = null
210218
) {
211219
$this->typeConfigurableFactory = $typeConfigurableFactory;
212220
$this->_eavAttributeFactory = $eavAttributeFactory;
@@ -218,6 +226,8 @@ public function __construct(
218226
$this->extensionAttributesJoinProcessor = $extensionAttributesJoinProcessor;
219227
$this->cache = $cache;
220228
$this->customerSession = $customerSession;
229+
$this->productFactory = $productFactory ?: ObjectManager::getInstance()
230+
->get(ProductInterfaceFactory::class);
221231
parent::__construct(
222232
$catalogProductOption,
223233
$eavConfig,
@@ -529,15 +539,16 @@ public function getUsedProducts($product, $requiredAttributeIds = null)
529539
]
530540
)
531541
);
532-
$collection = $this->getUsedProductCollection($product);
533542
$data = $this->serializer->unserialize($this->getCache()->load($key));
534543
if (!empty($data)) {
535544
$usedProducts = [];
536545
foreach ($data as $item) {
537-
$productItem = $collection->getNewEmptyItem()->setData($item);
546+
$productItem = $this->productFactory->create();
547+
$productItem->setData($item);
538548
$usedProducts[] = $productItem;
539549
}
540550
} else {
551+
$collection = $this->getUsedProductCollection($product);
541552
$collection
542553
->setFlag('has_stock_status_filter', true)
543554
->addAttributeToSelect($this->getCatalogConfig()->getProductAttributes())

app/code/Magento/ConfigurableProduct/Test/Unit/Model/Product/Type/ConfigurableTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
*
3131
* @SuppressWarnings(PHPMD.LongVariable)
3232
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
33+
* @SuppressWarnings(PHPMD.TooManyFields)
3334
*/
3435
class ConfigurableTest extends \PHPUnit_Framework_TestCase
3536
{
@@ -110,6 +111,11 @@ class ConfigurableTest extends \PHPUnit_Framework_TestCase
110111
*/
111112
protected $catalogConfig;
112113

114+
/**
115+
* @var \Magento\Catalog\Api\Data\ProductInterfaceFactory|\PHPUnit_Framework_MockObject_MockObject
116+
*/
117+
private $productFactory;
118+
113119
/**
114120
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
115121
*/
@@ -177,6 +183,10 @@ protected function setUp()
177183
->method('getMetadata')
178184
->with(ProductInterface::class)
179185
->willReturn($this->entityMetadata);
186+
$this->productFactory = $this->getMockBuilder(\Magento\Catalog\Api\Data\ProductInterfaceFactory::class)
187+
->setMethods(['create'])
188+
->disableOriginalConstructor()
189+
->getMock();
180190

181191
$this->_model = $this->_objectHelper->getObject(
182192
Configurable::class,
@@ -197,6 +207,7 @@ protected function setUp()
197207
'cache' => $this->cache,
198208
'catalogConfig' => $this->catalogConfig,
199209
'serializer' => $this->serializer,
210+
'productFactory' => $this->productFactory,
200211
]
201212
);
202213
$refClass = new \ReflectionClass(Configurable::class);
@@ -374,6 +385,50 @@ public function testGetUsedProducts()
374385
$this->_model->getUsedProducts($product);
375386
}
376387

388+
public function testGetUsedProductsWithDataInCache()
389+
{
390+
$product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
391+
->disableOriginalConstructor()
392+
->getMock();
393+
$childProduct = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
394+
->disableOriginalConstructor()
395+
->getMock();
396+
397+
$dataKey = '_cache_instance_products';
398+
$usedProductsData = [['first']];
399+
$usedProducts = [$childProduct];
400+
401+
$product->expects($this->once())
402+
->method('hasData')
403+
->with($dataKey)
404+
->willReturn(false);
405+
$product->expects($this->once())
406+
->method('setData')
407+
->with($dataKey, $usedProducts);
408+
$product->expects($this->any())
409+
->method('getData')
410+
->willReturnOnConsecutiveCalls(1, $usedProducts);
411+
412+
$childProduct->expects($this->once())
413+
->method('setData')
414+
->with($usedProductsData[0]);
415+
416+
$this->productFactory->expects($this->once())
417+
->method('create')
418+
->willReturn($childProduct);
419+
420+
$this->cache->expects($this->once())
421+
->method('load')
422+
->willReturn($usedProductsData);
423+
424+
$this->serializer->expects($this->once())
425+
->method('unserialize')
426+
->with($usedProductsData)
427+
->willReturn($usedProductsData);
428+
429+
self::assertEquals($usedProducts, $this->_model->getUsedProducts($product));
430+
}
431+
377432
/**
378433
* @param int $productStore
379434
*

app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ $finalPriceModel = $block->getPriceType('final_price');
1919
$idSuffix = $block->getIdSuffix() ? $block->getIdSuffix() : '';
2020
$schema = ($block->getZone() == 'item_view') ? true : false;
2121
?>
22-
<?php if ($block->hasSpecialPrice()): ?>
22+
<?php if (!$block->isProductList() && $block->hasSpecialPrice()): ?>
2323
<span class="special-price">
2424
<?php /* @escapeNotVerified */ echo $block->renderAmount($finalPriceModel->getAmount(), [
2525
'display_label' => __('Special Price'),

dev/tests/functional/tests/app/Magento/CatalogRule/Test/Constraint/AssertCatalogPriceRuleAppliedCatalogPage.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
76
namespace Magento\CatalogRule\Test\Constraint;
87

98
use Magento\Cms\Test\Page\CmsIndex;

dev/tests/functional/tests/app/Magento/CatalogRule/Test/TestCase/ApplyCatalogPriceRulesTest.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@
172172
<data name="shipping/shipping_method" xsi:type="string">Fixed</data>
173173
<data name="shippingAddress/dataset" xsi:type="string">UK_address</data>
174174
<data name="payment/method" xsi:type="string">checkmo</data>
175-
<constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedCatalogPage" />
176175
<constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedProductPage" />
177176
<constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedShoppingCart" />
178177
</variation>

0 commit comments

Comments
 (0)