Merge pull request #990 from magento-dragons/perf-pr-configurable

[Dragons][Performance] Configurable Product

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

@@ -317,11 +317,15 @@ public function getProductPrice(\Magento\Catalog\Model\Product $product)
     }
 
     /**
+     * Specifies that price rendering should be done for the list of products
+     * i.e. rendering happens in the scope of product list, but not single product
+     *
      * @return \Magento\Framework\Pricing\Render
      */
     protected function getPriceRender()
     {
-        return $this->getLayout()->getBlock('product.price.render.default');
+        return $this->getLayout()->getBlock('product.price.render.default')
+            ->setData('is_product_list', true);
     }
 
     /**

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

@@ -193,6 +193,19 @@ public function getCacheKeyInfo()
     {
         $cacheKeys = parent::getCacheKeyInfo();
         $cacheKeys['display_minimal_price'] = $this->getDisplayMinimalPrice();
+        $cacheKeys['is_product_list'] = $this->isProductList();
         return $cacheKeys;
     }
+
+    /**
+     * Get flag that price rendering should be done for the list of products
+     * By default (if flag is not set) is false
+     *
+     * @return bool
+     */
+    public function isProductList()
+    {
+        $isProductList = $this->getData('is_product_list');
+        return $isProductList === true;
+    }
 }

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

@@ -5,6 +5,12 @@
  */
 namespace Magento\Catalog\Test\Unit\Block\Product;
 
+use Magento\Catalog\Block\Product\Context;
+use Magento\Framework\Event\ManagerInterface;
+use Magento\Framework\Pricing\Render;
+use Magento\Framework\Url\Helper\Data;
+use Magento\Framework\View\LayoutInterface;
+
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -46,7 +52,7 @@ class ListProductTest extends \PHPUnit_Framework_TestCase
     protected $typeInstanceMock;
 
     /**
-     * @var \Magento\Framework\Url\Helper\Data | \PHPUnit_Framework_MockObject_MockObject
+     * @var Data | \PHPUnit_Framework_MockObject_MockObject
      */
     protected $urlHelperMock;
 
@@ -70,6 +76,16 @@ class ListProductTest extends \PHPUnit_Framework_TestCase
      */
     protected $toolbarMock;
 
+    /**
+     * @var Context|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $context;
+
+    /**
+     * @var Render|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $renderer;
+
     protected function setUp()
     {
         $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -141,20 +157,28 @@ protected function setUp()
             false
         );
 
-        $this->urlHelperMock = $this->getMockBuilder(\Magento\Framework\Url\Helper\Data::class)
-            ->disableOriginalConstructor()->getMock();
+        $this->urlHelperMock = $this->getMockBuilder(Data::class)->disableOriginalConstructor()->getMock();
+        $this->context = $this->getMockBuilder(Context::class)->disableOriginalConstructor()->getMock();
+        $this->renderer = $this->getMockBuilder(Render::class)->disableOriginalConstructor()->getMock();
+        $eventManager = $this->getMockForAbstractClass(ManagerInterface::class, [], '', false);
+
+        $this->context->expects($this->any())->method('getRegistry')->willReturn($this->registryMock);
+        $this->context->expects($this->any())->method('getCartHelper')->willReturn($this->cartHelperMock);
+        $this->context->expects($this->any())->method('getLayout')->willReturn($this->layoutMock);
+        $this->context->expects($this->any())->method('getEventManager')->willReturn($eventManager);
+
         $this->block = $objectManager->getObject(
             \Magento\Catalog\Block\Product\ListProduct::class,
             [
                 'registry' => $this->registryMock,
+                'context' => $this->context,
                 'layerResolver' => $layerResolver,
                 'cartHelper' => $this->cartHelperMock,
                 'postDataHelper' => $this->postDataHelperMock,
                 'urlHelper' => $this->urlHelperMock,
             ]
         );
         $this->block->setToolbarBlockName('mock');
-        $this->block->setLayout($this->layoutMock);
     }
 
     protected function tearDown()
@@ -257,4 +281,18 @@ public function testGetAddToCartPostParams()
         $result = $this->block->getAddToCartPostParams($this->productMock);
         $this->assertEquals($expectedPostData, $result);
     }
+
+    public function testSetIsProductListFlagOnGetProductPrice()
+    {
+        $this->renderer->expects($this->once())
+            ->method('setData')
+            ->with('is_product_list', true)
+            ->willReturnSelf();
+        $this->layoutMock->expects($this->once())
+            ->method('getBlock')
+            ->with('product.price.render.default')
+            ->willReturn($this->renderer);
+
+        $this->block->getProductPrice($this->productMock);
+    }
 }

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

@@ -401,8 +401,63 @@ public function testGetCacheKey()
         $this->assertStringEndsWith('list-category-page', $result);
     }
 
-    public function testGetCacheKeyInfo()
+    public function testGetCacheKeyInfoContainsDisplayMinimalPrice()
     {
         $this->assertArrayHasKey('display_minimal_price', $this->object->getCacheKeyInfo());
     }
+
+    /**
+     * Test when is_product_list flag is not specified
+     */
+    public function testGetCacheKeyInfoContainsIsProductListFlagByDefault()
+    {
+        $cacheInfo = $this->object->getCacheKeyInfo();
+        self::assertArrayHasKey('is_product_list', $cacheInfo);
+        self::assertFalse($cacheInfo['is_product_list']);
+    }
+
+    /**
+     * Test when is_product_list flag is specified
+     *
+     * @param bool $flag
+     * @dataProvider isProductListDataProvider
+     */
+    public function testGetCacheKeyInfoContainsIsProductListFlag($flag)
+    {
+        $this->object->setData('is_product_list', $flag);
+        $cacheInfo = $this->object->getCacheKeyInfo();
+        self::assertArrayHasKey('is_product_list', $cacheInfo);
+        self::assertEquals($flag, $cacheInfo['is_product_list']);
+    }
+
+    /**
+     * Test when is_product_list flag is not specified
+     */
+    public function testIsProductListByDefault()
+    {
+        self::assertFalse($this->object->isProductList());
+    }
+
+    /**
+     * Test when is_product_list flag is specified
+     *
+     * @param bool $flag
+     * @dataProvider isProductListDataProvider
+     */
+    public function testIsProductList($flag)
+    {
+        $this->object->setData('is_product_list', $flag);
+        self::assertEquals($flag, $this->object->isProductList());
+    }
+
+    /**
+     * @return array
+     */
+    public function isProductListDataProvider()
+    {
+        return [
+            'is_not_product_list' => [false],
+            'is_product_list' => [true],
+        ];
+    }
 }

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

@@ -7,6 +7,7 @@
 
 use Magento\Catalog\Api\Data\ProductAttributeInterface;
 use Magento\Catalog\Api\Data\ProductInterface;
+use Magento\Catalog\Api\Data\ProductInterfaceFactory;
 use Magento\Catalog\Api\ProductRepositoryInterface;
 use Magento\Catalog\Model\Config;
 use Magento\Framework\App\ObjectManager;
@@ -163,6 +164,11 @@ class Configurable extends \Magento\Catalog\Model\Product\Type\AbstractType
      */
     private $customerSession;
 
+    /**
+     * @var ProductInterfaceFactory
+     */
+    private $productFactory;
+
     /**
      * @codingStandardsIgnoreStart/End
      *
@@ -184,6 +190,7 @@ class Configurable extends \Magento\Catalog\Model\Product\Type\AbstractType
      * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
      * @param \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $extensionAttributesJoinProcessor
      * @param \Magento\Framework\Serialize\Serializer\Json $serializer
+     * @param ProductInterfaceFactory $productFactory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -206,7 +213,8 @@ public function __construct(
         \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $extensionAttributesJoinProcessor,
         \Magento\Framework\Cache\FrontendInterface $cache = null,
         \Magento\Customer\Model\Session $customerSession = null,
-        \Magento\Framework\Serialize\Serializer\Json $serializer = null
+        \Magento\Framework\Serialize\Serializer\Json $serializer = null,
+        ProductInterfaceFactory $productFactory = null
     ) {
         $this->typeConfigurableFactory = $typeConfigurableFactory;
         $this->_eavAttributeFactory = $eavAttributeFactory;
@@ -218,6 +226,8 @@ public function __construct(
         $this->extensionAttributesJoinProcessor = $extensionAttributesJoinProcessor;
         $this->cache = $cache;
         $this->customerSession = $customerSession;
+        $this->productFactory = $productFactory ?: ObjectManager::getInstance()
+            ->get(ProductInterfaceFactory::class);
         parent::__construct(
             $catalogProductOption,
             $eavConfig,
@@ -529,15 +539,16 @@ public function getUsedProducts($product, $requiredAttributeIds = null)
                     ]
                 )
             );
-            $collection = $this->getUsedProductCollection($product);
             $data = $this->serializer->unserialize($this->getCache()->load($key));
             if (!empty($data)) {
                 $usedProducts = [];
                 foreach ($data as $item) {
-                    $productItem = $collection->getNewEmptyItem()->setData($item);
+                    $productItem = $this->productFactory->create();
+                    $productItem->setData($item);
                     $usedProducts[] = $productItem;
                 }
             } else {
+                $collection = $this->getUsedProductCollection($product);
                 $collection
                     ->setFlag('has_stock_status_filter', true)
                     ->addAttributeToSelect($this->getCatalogConfig()->getProductAttributes())

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

@@ -30,6 +30,7 @@
  *
  * @SuppressWarnings(PHPMD.LongVariable)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ * @SuppressWarnings(PHPMD.TooManyFields)
  */
 class ConfigurableTest extends \PHPUnit_Framework_TestCase
 {
@@ -110,6 +111,11 @@ class ConfigurableTest extends \PHPUnit_Framework_TestCase
      */
     protected $catalogConfig;
 
+    /**
+     * @var \Magento\Catalog\Api\Data\ProductInterfaceFactory|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $productFactory;
+
     /**
      * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
@@ -177,6 +183,10 @@ protected function setUp()
             ->method('getMetadata')
             ->with(ProductInterface::class)
             ->willReturn($this->entityMetadata);
+        $this->productFactory = $this->getMockBuilder(\Magento\Catalog\Api\Data\ProductInterfaceFactory::class)
+            ->setMethods(['create'])
+            ->disableOriginalConstructor()
+            ->getMock();
 
         $this->_model = $this->_objectHelper->getObject(
             Configurable::class,
@@ -197,6 +207,7 @@ protected function setUp()
                 'cache' => $this->cache,
                 'catalogConfig' => $this->catalogConfig,
                 'serializer' => $this->serializer,
+                'productFactory' => $this->productFactory,
             ]
         );
         $refClass = new \ReflectionClass(Configurable::class);
@@ -374,6 +385,50 @@ public function testGetUsedProducts()
         $this->_model->getUsedProducts($product);
     }
 
+    public function testGetUsedProductsWithDataInCache()
+    {
+        $product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $childProduct = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $dataKey = '_cache_instance_products';
+        $usedProductsData = [['first']];
+        $usedProducts = [$childProduct];
+
+        $product->expects($this->once())
+            ->method('hasData')
+            ->with($dataKey)
+            ->willReturn(false);
+        $product->expects($this->once())
+            ->method('setData')
+            ->with($dataKey, $usedProducts);
+        $product->expects($this->any())
+            ->method('getData')
+            ->willReturnOnConsecutiveCalls(1, $usedProducts);
+
+        $childProduct->expects($this->once())
+            ->method('setData')
+            ->with($usedProductsData[0]);
+
+        $this->productFactory->expects($this->once())
+            ->method('create')
+            ->willReturn($childProduct);
+
+        $this->cache->expects($this->once())
+            ->method('load')
+            ->willReturn($usedProductsData);
+
+        $this->serializer->expects($this->once())
+            ->method('unserialize')
+            ->with($usedProductsData)
+            ->willReturn($usedProductsData);
+
+        self::assertEquals($usedProducts, $this->_model->getUsedProducts($product));
+    }
+
     /**
      * @param int $productStore
      *

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

@@ -19,7 +19,7 @@ $finalPriceModel = $block->getPriceType('final_price');
 $idSuffix = $block->getIdSuffix() ? $block->getIdSuffix() : '';
 $schema = ($block->getZone() == 'item_view') ? true : false;
 ?>
-<?php if ($block->hasSpecialPrice()): ?>
+<?php if (!$block->isProductList() && $block->hasSpecialPrice()): ?>
     <span class="special-price">
         <?php /* @escapeNotVerified */ echo $block->renderAmount($finalPriceModel->getAmount(), [
             'display_label'     => __('Special Price'),

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

@@ -3,7 +3,6 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
-
 namespace Magento\CatalogRule\Test\Constraint;
 
 use Magento\Cms\Test\Page\CmsIndex;

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

@@ -172,7 +172,6 @@
             <data name="shipping/shipping_method" xsi:type="string">Fixed</data>
             <data name="shippingAddress/dataset" xsi:type="string">UK_address</data>
             <data name="payment/method" xsi:type="string">checkmo</data>
-            <constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedCatalogPage" />
             <constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedProductPage" />
             <constraint name="Magento\CatalogRule\Test\Constraint\AssertCatalogPriceRuleAppliedShoppingCart" />
         </variation>