From baf3cf0acf90ce25f1f5f8399c26c9acb8dfd9bc Mon Sep 17 00:00:00 2001 From: Teun Lassche Date: Fri, 1 Feb 2019 12:32:45 +0100 Subject: [PATCH 1/6] Improve bundle load speeds When a bundle has a lot of selection, the tax GetPriceConfigurationObserver becomes a real performance hog because it loads the selections over and over again. This fixes that by caching the selection in memory --- .../GetPriceConfigurationObserver.php | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php index 21828c35a81dc..78a60a1391866 100644 --- a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php +++ b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php @@ -23,6 +23,8 @@ class GetPriceConfigurationObserver implements ObserverInterface */ protected $registry; + private $selectionCache = []; + /** * @param \Magento\Framework\Registry $registry * @param \Magento\Tax\Helper\Data $taxData @@ -107,15 +109,19 @@ private function updatePriceForBundle($holder, $key) /** @var \Magento\Catalog\Model\Product $product */ $product = $this->registry->registry('current_product'); if ($product->getTypeId() == \Magento\Catalog\Model\Product\Type::TYPE_BUNDLE) { - $typeInstance = $product->getTypeInstance(); - $typeInstance->setStoreFilter($product->getStoreId(), $product); + if (!isset($this->selectionCache[$product->getId()])) { + $typeInstance = $product->getTypeInstance(); + $typeInstance->setStoreFilter($product->getStoreId(), $product); - $selectionCollection = $typeInstance->getSelectionsCollection( - $typeInstance->getOptionsIds($product), - $product - ); + $selectionCollection = $typeInstance->getSelectionsCollection( + $typeInstance->getOptionsIds($product), + $product + ); + $this->selectionCache[$product->getId()] = $selectionCollection->getItems(); + } + $arrSelections = $this->selectionCache[$product->getId()]; - foreach ($selectionCollection->getItems() as $selectionItem) { + foreach ($arrSelections as $selectionItem) { if ($holder['optionId'] == $selectionItem->getId()) { /** @var \Magento\Framework\Pricing\Amount\Base $baseAmount */ $baseAmount = $selectionItem->getPriceInfo()->getPrice(BasePrice::PRICE_CODE)->getAmount(); From 5b99562bb81684fb6c7d8e3a8efbceca12e19e89 Mon Sep 17 00:00:00 2001 From: Teun Lassche Date: Fri, 1 Feb 2019 20:54:51 +0100 Subject: [PATCH 2/6] Clear selection cache before each execute run --- app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php index 78a60a1391866..5f0e67356f73d 100644 --- a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php +++ b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php @@ -46,6 +46,7 @@ public function __construct( */ public function execute(\Magento\Framework\Event\Observer $observer) { + $this->selectionCache = []; if ($this->taxData->displayPriceIncludingTax()) { /** @var \Magento\Catalog\Model\Product $product */ $product = $this->registry->registry('current_product'); From f6d45759337d38d393768cb6f6025114d6d84480 Mon Sep 17 00:00:00 2001 From: Teun Lassche Date: Fri, 1 Feb 2019 21:16:19 +0100 Subject: [PATCH 3/6] Add comment on the selection cache variable --- .../Magento/Tax/Observer/GetPriceConfigurationObserver.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php index 5f0e67356f73d..d7314af8ae23e 100644 --- a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php +++ b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php @@ -23,6 +23,9 @@ class GetPriceConfigurationObserver implements ObserverInterface */ protected $registry; + /** + * @var array Cache of the current bundle selection items + */ private $selectionCache = []; /** From f9595df6708334306758edfc4a156407780b293f Mon Sep 17 00:00:00 2001 From: Teun Lassche Date: Mon, 4 Feb 2019 06:43:42 +0100 Subject: [PATCH 4/6] Fix GetPriceConfigurationObserverTest product object mock --- .../Test/Unit/Observer/GetPriceConfigurationObserverTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php index 1dd1efceb9dbd..f314b1b2fe6b6 100644 --- a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php +++ b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php @@ -118,7 +118,7 @@ public function testExecute($testArray, $expectedArray) $product = $this->createPartialMock( \Magento\Bundle\Model\Product\Type::class, - ['getTypeInstance', 'getTypeId', 'getStoreId', 'getSelectionsCollection'] + ['getTypeInstance', 'getTypeId', 'getStoreId', 'getSelectionsCollection','getId'] ); $product->expects($this->any()) ->method('getTypeInstance') From 75665aa16343b23f8cdfb6c83fb57c16621367e4 Mon Sep 17 00:00:00 2001 From: Teun Lassche Date: Thu, 7 Feb 2019 19:58:48 +0100 Subject: [PATCH 5/6] Add missing space --- .../Test/Unit/Observer/GetPriceConfigurationObserverTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php index f314b1b2fe6b6..ba242f7a00242 100644 --- a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php +++ b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php @@ -118,7 +118,7 @@ public function testExecute($testArray, $expectedArray) $product = $this->createPartialMock( \Magento\Bundle\Model\Product\Type::class, - ['getTypeInstance', 'getTypeId', 'getStoreId', 'getSelectionsCollection','getId'] + ['getTypeInstance', 'getTypeId', 'getStoreId', 'getSelectionsCollection', 'getId'] ); $product->expects($this->any()) ->method('getTypeInstance') From eb2c10041b5d85555b84ed4fe57f8a5f1b8e0feb Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Wed, 17 Jul 2019 15:49:25 +0300 Subject: [PATCH 6/6] magento/magento2#20877: Static test fix. --- .../GetPriceConfigurationObserver.php | 65 ++++++++++--------- .../GetPriceConfigurationObserverTest.php | 1 + 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php index d7314af8ae23e..bad9757dafd89 100644 --- a/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php +++ b/app/code/Magento/Tax/Observer/GetPriceConfigurationObserver.php @@ -5,10 +5,13 @@ */ namespace Magento\Tax\Observer; -use Magento\Framework\Event\ObserverInterface; use Magento\Catalog\Pricing\Price\BasePrice; use Magento\Catalog\Pricing\Price\RegularPrice; +use Magento\Framework\Event\ObserverInterface; +/** + * Modifies the bundle config for the front end to resemble the tax included price when tax included prices. + */ class GetPriceConfigurationObserver implements ObserverInterface { /** @@ -84,12 +87,11 @@ private function recurConfigAndUpdatePrice($input, $searchKey) if (is_array($el)) { $holder[$key] = $this->recurConfigAndUpdatePrice($el, $searchKey); - if ($key === $searchKey) { - if ((array_key_exists('basePrice', $holder[$key]))) { - if (array_key_exists('optionId', $input)) { - $holder = $this->updatePriceForBundle($holder, $key); - } - } + if ($key === $searchKey + && array_key_exists('optionId', $input) + && array_key_exists('basePrice', $holder[$key]) + ) { + $holder = $this->updatePriceForBundle($holder, $key); } } else { $holder[$key] = $el; @@ -108,36 +110,35 @@ private function recurConfigAndUpdatePrice($input, $searchKey) */ private function updatePriceForBundle($holder, $key) { - if (array_key_exists($key, $holder)) { - if (array_key_exists('basePrice', $holder[$key])) { - /** @var \Magento\Catalog\Model\Product $product */ - $product = $this->registry->registry('current_product'); - if ($product->getTypeId() == \Magento\Catalog\Model\Product\Type::TYPE_BUNDLE) { - if (!isset($this->selectionCache[$product->getId()])) { - $typeInstance = $product->getTypeInstance(); - $typeInstance->setStoreFilter($product->getStoreId(), $product); + if (array_key_exists($key, $holder) + && array_key_exists('basePrice', $holder[$key])) { + /** @var \Magento\Catalog\Model\Product $product */ + $product = $this->registry->registry('current_product'); + if ($product->getTypeId() == \Magento\Catalog\Model\Product\Type::TYPE_BUNDLE) { + if (!isset($this->selectionCache[$product->getId()])) { + $typeInstance = $product->getTypeInstance(); + $typeInstance->setStoreFilter($product->getStoreId(), $product); - $selectionCollection = $typeInstance->getSelectionsCollection( - $typeInstance->getOptionsIds($product), - $product - ); - $this->selectionCache[$product->getId()] = $selectionCollection->getItems(); - } - $arrSelections = $this->selectionCache[$product->getId()]; + $selectionCollection = $typeInstance->getSelectionsCollection( + $typeInstance->getOptionsIds($product), + $product + ); + $this->selectionCache[$product->getId()] = $selectionCollection->getItems(); + } + $arrSelections = $this->selectionCache[$product->getId()]; - foreach ($arrSelections as $selectionItem) { - if ($holder['optionId'] == $selectionItem->getId()) { - /** @var \Magento\Framework\Pricing\Amount\Base $baseAmount */ - $baseAmount = $selectionItem->getPriceInfo()->getPrice(BasePrice::PRICE_CODE)->getAmount(); - /** @var \Magento\Framework\Pricing\Amount\Base $oldAmount */ - $oldAmount = + foreach ($arrSelections as $selectionItem) { + if ($holder['optionId'] == $selectionItem->getId()) { + /** @var \Magento\Framework\Pricing\Amount\Base $baseAmount */ + $baseAmount = $selectionItem->getPriceInfo()->getPrice(BasePrice::PRICE_CODE)->getAmount(); + /** @var \Magento\Framework\Pricing\Amount\Base $oldAmount */ + $oldAmount = $selectionItem->getPriceInfo()->getPrice(RegularPrice::PRICE_CODE)->getAmount(); - if ($baseAmount->hasAdjustment('tax')) { - $holder[$key]['basePrice']['amount'] = + if ($baseAmount->hasAdjustment('tax')) { + $holder[$key]['basePrice']['amount'] = $baseAmount->getBaseAmount() + $baseAmount->getAdjustmentAmount('tax'); - $holder[$key]['oldPrice']['amount'] = + $holder[$key]['oldPrice']['amount'] = $oldAmount->getBaseAmount() + $oldAmount->getAdjustmentAmount('tax'); - } } } } diff --git a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php index ba242f7a00242..e8fcf03807e6e 100644 --- a/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php +++ b/app/code/Magento/Tax/Test/Unit/Observer/GetPriceConfigurationObserverTest.php @@ -3,6 +3,7 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Tax\Test\Unit\Observer; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;