From ee7bca6ed4f814dd7f1ffb3f27c81413160fd922 Mon Sep 17 00:00:00 2001 From: Mahesh Singh Date: Thu, 16 Jan 2020 18:32:58 +0530 Subject: [PATCH 1/5] FIXED: Discount fixed amount whole cart applied mutiple time when customer use Check Out with Multiple Addresses --- .../Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php index 2ae1c1c7ac63a..70e5b0bdc843e 100644 --- a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php +++ b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php @@ -19,7 +19,7 @@ class CartFixed extends AbstractDiscount * * @var int[] */ - protected $_cartFixedRuleUsedForAddress = []; + protected static $_cartFixedRuleUsedForAddress = []; /** * @var DeltaPriceRound From 1ff48cd996781b9e113e57be6e7e90a87c08e7f0 Mon Sep 17 00:00:00 2001 From: Mahesh Singh Date: Thu, 16 Jan 2020 20:11:34 +0530 Subject: [PATCH 2/5] static variable accessible changes --- .../SalesRule/Model/Rule/Action/Discount/CartFixed.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php index 70e5b0bdc843e..4fcebd4eeae0d 100644 --- a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php +++ b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php @@ -136,7 +136,7 @@ public function calculate($rule, $item, $qty) */ protected function setCartFixedRuleUsedForAddress($ruleId, $itemId) { - $this->_cartFixedRuleUsedForAddress[$ruleId] = $itemId; + self::$_cartFixedRuleUsedForAddress[$ruleId] = $itemId; } /** @@ -147,8 +147,8 @@ protected function setCartFixedRuleUsedForAddress($ruleId, $itemId) */ protected function getCartFixedRuleUsedForAddress($ruleId) { - if (isset($this->_cartFixedRuleUsedForAddress[$ruleId])) { - return $this->_cartFixedRuleUsedForAddress[$ruleId]; + if (isset(self::$_cartFixedRuleUsedForAddress[$ruleId])) { + return self::$_cartFixedRuleUsedForAddress[$ruleId]; } return null; } From b6f84336ac9f0059b76146206a8483d5e4e7c81e Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 24 Feb 2020 17:41:45 -0600 Subject: [PATCH 3/5] MC-29276: Discount fixed amount whole cart applied mutiple time when customer use Check Out with Multiple Addresses - Fix "discount fixed amount whole cart" for multi-shipping quote - Add integration for "Discount fixed amount whole cart" with multiple shipping --- .../Model/Rule/Action/Discount/CartFixed.php | 26 +- .../Model/Rule/QuoteResetAppliedRules.php | 24 ++ .../Spi/QuoteResetAppliedRulesInterface.php | 22 ++ .../QuoteResetAppliedRulesObserver.php | 39 +++ .../Rule/Action/Discount/CartFixedTest.php | 15 +- app/code/Magento/SalesRule/etc/di.xml | 2 + app/code/Magento/SalesRule/etc/events.xml | 3 + .../Rule/Action/Discount/CartFixedTest.php | 290 +++++++++++++++++- 8 files changed, 397 insertions(+), 24 deletions(-) create mode 100644 app/code/Magento/SalesRule/Model/Rule/QuoteResetAppliedRules.php create mode 100644 app/code/Magento/SalesRule/Model/Spi/QuoteResetAppliedRulesInterface.php create mode 100644 app/code/Magento/SalesRule/Observer/QuoteResetAppliedRulesObserver.php diff --git a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php index 4fcebd4eeae0d..e44200614fa00 100644 --- a/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php +++ b/app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php @@ -19,7 +19,7 @@ class CartFixed extends AbstractDiscount * * @var int[] */ - protected static $_cartFixedRuleUsedForAddress = []; + protected $_cartFixedRuleUsedForAddress = []; /** * @var DeltaPriceRound @@ -64,25 +64,13 @@ public function calculate($rule, $item, $qty) $ruleTotals = $this->validator->getRuleItemTotalsInfo($rule->getId()); $quote = $item->getQuote(); - $address = $item->getAddress(); $itemPrice = $this->validator->getItemPrice($item); $baseItemPrice = $this->validator->getItemBasePrice($item); $itemOriginalPrice = $this->validator->getItemOriginalPrice($item); $baseItemOriginalPrice = $this->validator->getItemBaseOriginalPrice($item); - /** - * prevent applying whole cart discount for every shipping order, but only for first order - */ - if ($quote->getIsMultiShipping()) { - $usedForAddressId = $this->getCartFixedRuleUsedForAddress($rule->getId()); - if ($usedForAddressId && $usedForAddressId != $address->getId()) { - return $discountData; - } else { - $this->setCartFixedRuleUsedForAddress($rule->getId(), $address->getId()); - } - } - $cartRules = $address->getCartFixedRules(); + $cartRules = $quote->getCartFixedRules(); if (!isset($cartRules[$rule->getId()])) { $cartRules[$rule->getId()] = $rule->getDiscountAmount(); } @@ -122,7 +110,7 @@ public function calculate($rule, $item, $qty) $discountData->setOriginalAmount(min($itemOriginalPrice * $qty, $quoteAmount)); $discountData->setBaseOriginalAmount($this->priceCurrency->round($baseItemOriginalPrice)); } - $address->setCartFixedRules($cartRules); + $quote->setCartFixedRules($cartRules); return $discountData; } @@ -130,25 +118,27 @@ public function calculate($rule, $item, $qty) /** * Set information about usage cart fixed rule by quote address * + * @deprecated should be removed as it is not longer used * @param int $ruleId * @param int $itemId * @return void */ protected function setCartFixedRuleUsedForAddress($ruleId, $itemId) { - self::$_cartFixedRuleUsedForAddress[$ruleId] = $itemId; + $this->_cartFixedRuleUsedForAddress[$ruleId] = $itemId; } /** * Retrieve information about usage cart fixed rule by quote address * + * @deprecated should be removed as it is not longer used * @param int $ruleId * @return int|null */ protected function getCartFixedRuleUsedForAddress($ruleId) { - if (isset(self::$_cartFixedRuleUsedForAddress[$ruleId])) { - return self::$_cartFixedRuleUsedForAddress[$ruleId]; + if (isset($this->_cartFixedRuleUsedForAddress[$ruleId])) { + return $this->_cartFixedRuleUsedForAddress[$ruleId]; } return null; } diff --git a/app/code/Magento/SalesRule/Model/Rule/QuoteResetAppliedRules.php b/app/code/Magento/SalesRule/Model/Rule/QuoteResetAppliedRules.php new file mode 100644 index 0000000000000..5c1d98b65a0e4 --- /dev/null +++ b/app/code/Magento/SalesRule/Model/Rule/QuoteResetAppliedRules.php @@ -0,0 +1,24 @@ +setCartFixedRules([]); + } +} diff --git a/app/code/Magento/SalesRule/Model/Spi/QuoteResetAppliedRulesInterface.php b/app/code/Magento/SalesRule/Model/Spi/QuoteResetAppliedRulesInterface.php new file mode 100644 index 0000000000000..ba73ab83608db --- /dev/null +++ b/app/code/Magento/SalesRule/Model/Spi/QuoteResetAppliedRulesInterface.php @@ -0,0 +1,22 @@ +resetAppliedRules = $resetAppliedRules; + } + + /** + * @inheritDoc + */ + public function execute(Observer $observer) + { + $this->resetAppliedRules->execute($observer->getQuote()); + } +} diff --git a/app/code/Magento/SalesRule/Test/Unit/Model/Rule/Action/Discount/CartFixedTest.php b/app/code/Magento/SalesRule/Test/Unit/Model/Rule/Action/Discount/CartFixedTest.php index 13f26124c464d..cafe194d05de0 100644 --- a/app/code/Magento/SalesRule/Test/Unit/Model/Rule/Action/Discount/CartFixedTest.php +++ b/app/code/Magento/SalesRule/Test/Unit/Model/Rule/Action/Discount/CartFixedTest.php @@ -65,10 +65,17 @@ protected function setUp() $this->item = $this->createMock(\Magento\Quote\Model\Quote\Item\AbstractItem::class); $this->data = $this->createPartialMock(\Magento\SalesRule\Model\Rule\Action\Discount\Data::class, []); - $this->quote = $this->createMock(\Magento\Quote\Model\Quote::class); + $this->quote = $this->createPartialMock( + \Magento\Quote\Model\Quote::class, + [ + 'getStore', + 'getCartFixedRules', + 'setCartFixedRules', + ] + ); $this->address = $this->createPartialMock( \Magento\Quote\Model\Quote\Address::class, - ['getCartFixedRules', 'setCartFixedRules', '__wakeup'] + ['__wakeup'] ); $this->item->expects($this->any())->method('getQuote')->will($this->returnValue($this->quote)); $this->item->expects($this->any())->method('getAddress')->will($this->returnValue($this->address)); @@ -101,7 +108,7 @@ public function testCalculate() { $this->rule->setData(['id' => 1, 'discount_amount' => 10.0]); - $this->address->expects($this->any())->method('getCartFixedRules')->will($this->returnValue([])); + $this->quote->expects($this->any())->method('getCartFixedRules')->will($this->returnValue([])); $store = $this->createMock(\Magento\Store\Model\Store::class); $this->priceCurrency->expects($this->atLeastOnce())->method('convert')->will($this->returnArgument(0)); $this->priceCurrency->expects($this->atLeastOnce())->method('round')->will($this->returnArgument(0)); @@ -145,7 +152,7 @@ public function testCalculate() $this->returnValue(100) ); - $this->address->expects($this->once())->method('setCartFixedRules')->with([1 => 0.0]); + $this->quote->expects($this->once())->method('setCartFixedRules')->with([1 => 0.0]); $this->model->calculate($this->rule, $this->item, 1); $this->assertEquals($this->data->getAmount(), 10); diff --git a/app/code/Magento/SalesRule/etc/di.xml b/app/code/Magento/SalesRule/etc/di.xml index 0e5fe6d29aed6..c4bc9c3a6decb 100644 --- a/app/code/Magento/SalesRule/etc/di.xml +++ b/app/code/Magento/SalesRule/etc/di.xml @@ -36,6 +36,8 @@ type="Magento\SalesRule\Model\Data\DiscountData" /> + diff --git a/app/code/Magento/SalesRule/etc/events.xml b/app/code/Magento/SalesRule/etc/events.xml index 5f899fb0cca5c..c55c37de71aac 100644 --- a/app/code/Magento/SalesRule/etc/events.xml +++ b/app/code/Magento/SalesRule/etc/events.xml @@ -36,4 +36,7 @@ + + + diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Action/Discount/CartFixedTest.php b/dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Action/Discount/CartFixedTest.php index f56fb0a2c6135..583316d97d68c 100644 --- a/dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Action/Discount/CartFixedTest.php +++ b/dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Action/Discount/CartFixedTest.php @@ -10,7 +10,9 @@ use Magento\Catalog\Api\Data\ProductInterface; use Magento\Catalog\Model\Product; use Magento\Catalog\Model\ProductRepository; +use Magento\Checkout\Model\Session as CheckoutSession; use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Multishipping\Model\Checkout\Type\Multishipping; use Magento\Quote\Api\CartRepositoryInterface; use Magento\Quote\Api\Data\CartItemInterface; use Magento\Quote\Api\GuestCartItemRepositoryInterface; @@ -21,6 +23,10 @@ use Magento\Quote\Model\QuoteIdMask; use Magento\Sales\Api\Data\OrderInterface; use Magento\Sales\Api\OrderRepositoryInterface; +use Magento\Sales\Model\Order; +use Magento\Sales\Model\Order\Email\Sender\OrderSender; +use Magento\SalesRule\Api\RuleRepositoryInterface; +use Magento\SalesRule\Model\Rule; use Magento\TestFramework\Helper\Bootstrap; /** @@ -187,11 +193,12 @@ public function testDiscountsOnQuoteWithFixedDiscount(): void /** * Load cart from fixture. * + * @param string $reservedOrderId * @return Quote */ - private function getQuote(): Quote + private function getQuote(string $reservedOrderId = 'test01'): Quote { - $searchCriteria = $this->criteriaBuilder->addFilter('reserved_order_id', 'test01')->create(); + $searchCriteria = $this->criteriaBuilder->addFilter('reserved_order_id', $reservedOrderId)->create(); $carts = $this->quoteRepository->getList($searchCriteria) ->getItems(); if (!$carts) { @@ -290,4 +297,283 @@ private function getOrder(string $incrementId): OrderInterface return array_pop($items); } + + /** + * Checks "fixed amount discount for whole cart" with multiple orders with different shipping addresses + * + * @magentoAppIsolation enabled + * @magentoDataFixture Magento/SalesRule/_files/coupon_cart_fixed_discount.php + * @magentoDataFixture Magento/Multishipping/Fixtures/quote_with_split_items.php + * @dataProvider multishippingDataProvider + * @param float $discount + * @param array $firstOrderTotals + * @param array $secondOrderTotals + * @param array $thirdOrderTotals + * @return void + */ + public function testMultishipping( + float $discount, + array $firstOrderTotals, + array $secondOrderTotals, + array $thirdOrderTotals + ): void { + $store = $this->objectManager->get(\Magento\Store\Model\StoreManagerInterface::class)->getStore(); + $salesRule = $this->getRule('15$ fixed discount on whole cart'); + $salesRule->setDiscountAmount($discount); + $this->saveRule($salesRule); + $quote = $this->getQuote('multishipping_quote_id'); + $quote->setStoreId($store->getId()); + $quote->setCouponCode('CART_FIXED_DISCOUNT_15'); + $quote->collectTotals(); + $this->quoteRepository->save($quote); + /** @var CheckoutSession $session */ + $session = $this->objectManager->get(CheckoutSession::class); + $session->replaceQuote($quote); + $orderSender = $this->getMockBuilder(OrderSender::class) + ->disableOriginalConstructor() + ->getMock(); + + $model = $this->objectManager->create( + Multishipping::class, + ['orderSender' => $orderSender] + ); + $model->createOrders(); + $orderList = $this->getOrderList((int)$quote->getId()); + $this->assertCount(3, $orderList); + /** + * The order with $10 simple product + * @var Order $firstOrder + */ + $firstOrder = array_shift($orderList); + + $this->assertEquals( + $firstOrderTotals['subtotal'], + $firstOrder->getSubtotal() + ); + $this->assertEquals( + $firstOrderTotals['discount_amount'], + $firstOrder->getDiscountAmount() + ); + $this->assertEquals( + $firstOrderTotals['shipping_amount'], + $firstOrder->getShippingAmount() + ); + $this->assertEquals( + $firstOrderTotals['grand_total'], + $firstOrder->getGrandTotal() + ); + /** + * The order with $20 simple product + * @var Order $secondOrder + */ + $secondOrder = array_shift($orderList); + $this->assertEquals( + $secondOrderTotals['subtotal'], + $secondOrder->getSubtotal() + ); + $this->assertEquals( + $secondOrderTotals['discount_amount'], + $secondOrder->getDiscountAmount() + ); + $this->assertEquals( + $secondOrderTotals['shipping_amount'], + $secondOrder->getShippingAmount() + ); + $this->assertEquals( + $secondOrderTotals['grand_total'], + $secondOrder->getGrandTotal() + ); + /** + * The order with $5 virtual product and billing address as shipping + * @var Order $thirdOrder + */ + $thirdOrder = array_shift($orderList); + $this->assertEquals( + $thirdOrderTotals['subtotal'], + $thirdOrder->getSubtotal() + ); + $this->assertEquals( + $thirdOrderTotals['discount_amount'], + $thirdOrder->getDiscountAmount() + ); + $this->assertEquals( + $thirdOrderTotals['shipping_amount'], + $thirdOrder->getShippingAmount() + ); + $this->assertEquals( + $thirdOrderTotals['grand_total'], + $thirdOrder->getGrandTotal() + ); + } + + /** + * @return array + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function multishippingDataProvider(): array + { + return [ + 'Discount < 1stOrderSubtotal: only 1st order gets discount' => [ + 5, + [ + 'subtotal' => 10.00, + 'discount_amount' => -5.00, + 'shipping_amount' => 5.00, + 'grand_total' => 10.00, + ], + [ + 'subtotal' => 20.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 5.00, + 'grand_total' => 25.00, + ], + [ + 'subtotal' => 5.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 0.00, + 'grand_total' => 5.00, + ] + ], + 'Discount = 1stOrderSubtotal: only 1st order gets discount' => [ + 10, + [ + 'subtotal' => 10.00, + 'discount_amount' => -10.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 20.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 5.00, + 'grand_total' => 25.00, + ], + [ + 'subtotal' => 5.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 0.00, + 'grand_total' => 5.00, + ] + ], + 'Discount > 1stOrderSubtotal: 1st order get 100% discount and 2nd order get the remaining discount' => [ + 15, + [ + 'subtotal' => 10.00, + 'discount_amount' => -10.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 20.00, + 'discount_amount' => -5.00, + 'shipping_amount' => 5.00, + 'grand_total' => 20.00, + ], + [ + 'subtotal' => 5.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 0.00, + 'grand_total' => 5.00, + ] + ], + 'Discount = 1stOrderSubtotal + 2ndOrderSubtotal: 1st order and 2nd order get 100% discount' => [ + 30, + [ + 'subtotal' => 10.00, + 'discount_amount' => -10.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 20.00, + 'discount_amount' => -20.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 5.00, + 'discount_amount' => -0.00, + 'shipping_amount' => 0.00, + 'grand_total' => 5.00, + ] + ], + 'Discount > 1stOrdSubtotal + 2ndOrdSubtotal: 1st order and 2nd order get 100% discount + and 3rd order get remaining discount' => [ + 31, + [ + 'subtotal' => 10.00, + 'discount_amount' => -10.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 20.00, + 'discount_amount' => -20.00, + 'shipping_amount' => 5.00, + 'grand_total' => 5.00, + ], + [ + 'subtotal' => 5.00, + 'discount_amount' => -1.00, + 'shipping_amount' => 0.00, + 'grand_total' => 4.00, + ] + ] + ]; + } + + /** + * Get list of orders by quote id. + * + * @param int $quoteId + * @return array + */ + private function getOrderList(int $quoteId): array + { + /** @var SearchCriteriaBuilder $searchCriteriaBuilder */ + $searchCriteriaBuilder = $this->objectManager->get(SearchCriteriaBuilder::class); + $searchCriteria = $searchCriteriaBuilder->addFilter('quote_id', $quoteId) + ->create(); + + /** @var OrderRepositoryInterface $orderRepository */ + $orderRepository = $this->objectManager->get(OrderRepositoryInterface::class); + return $orderRepository->getList($searchCriteria)->getItems(); + } + + /** + * Get rule by name + * + * @param string $name + * @return Rule + */ + private function getRule(string $name): Rule + { + /** @var SearchCriteriaBuilder $searchCriteriaBuilder */ + $searchCriteriaBuilder = $this->objectManager->get(SearchCriteriaBuilder::class); + $searchCriteria = $searchCriteriaBuilder->addFilter('name', $name) + ->create(); + /** @var RuleRepositoryInterface $ruleRepository */ + $ruleRepository = $this->objectManager->get(RuleRepositoryInterface::class); + $items = $ruleRepository->getList($searchCriteria) + ->getItems(); + /** @var Rule $salesRule */ + $dataModel = array_pop($items); + /** @var Rule $ruleModel */ + $ruleModel = $this->objectManager->get(\Magento\SalesRule\Model\RuleFactory::class)->create(); + $ruleModel->load($dataModel->getRuleId()); + return $ruleModel; + } + + /** + * Save rule into database + * + * @param Rule $rule + * @return void + */ + private function saveRule(Rule $rule): void + { + /** @var \Magento\SalesRule\Model\ResourceModel\Rule $resourceModel */ + $resourceModel = $this->objectManager->get(\Magento\SalesRule\Model\ResourceModel\Rule::class); + $resourceModel->save($rule); + } } From 0fd8a5146cdf4e524150e68f89085d90f0d42be3 Mon Sep 17 00:00:00 2001 From: Oleksandr Iegorov Date: Tue, 3 Mar 2020 10:58:59 -0600 Subject: [PATCH 4/5] MC-23890: Customer module Recurring setup script performance problems --- app/code/Magento/Customer/Model/Customer.php | 29 +++- .../Magento/Customer/Setup/RecurringData.php | 28 +++- .../Test/Unit/Setup/RecurringDataTest.php | 129 ++++++++++++++++++ 3 files changed, 174 insertions(+), 12 deletions(-) create mode 100644 app/code/Magento/Customer/Test/Unit/Setup/RecurringDataTest.php diff --git a/app/code/Magento/Customer/Model/Customer.php b/app/code/Magento/Customer/Model/Customer.php index 2692d1edf0143..1e4914b152de3 100644 --- a/app/code/Magento/Customer/Model/Customer.php +++ b/app/code/Magento/Customer/Model/Customer.php @@ -21,6 +21,7 @@ use Magento\Store\Model\ScopeInterface; use Magento\Framework\App\ObjectManager; use Magento\Framework\Math\Random; +use Magento\Framework\Indexer\IndexerInterface; /** * Customer model @@ -62,8 +63,7 @@ class Customer extends \Magento\Framework\Model\AbstractModel const XML_PATH_RESET_PASSWORD_TEMPLATE = 'customer/password/reset_password_template'; /** - * @deprecated - * @see AccountConfirmation::XML_PATH_IS_CONFIRM + * @deprecated @see \Magento\Customer\Model\AccountConfirmation::XML_PATH_IS_CONFIRM */ const XML_PATH_IS_CONFIRM = 'customer/create_account/confirm'; @@ -227,6 +227,11 @@ class Customer extends \Magento\Framework\Model\AbstractModel */ private $storedAddress; + /** + * @var IndexerInterface|null + */ + private $indexer; + /** * @param \Magento\Framework\Model\Context $context * @param \Magento\Framework\Registry $registry @@ -304,6 +309,19 @@ public function __construct( ); } + /** + * Micro-caching optimization + * + * @return IndexerInterface + */ + private function getIndexer() : IndexerInterface + { + if ($this->indexer === null) { + $this->indexer = $this->indexerRegistry->get(self::CUSTOMER_GRID_INDEXER_ID); + } + return $this->indexer; + } + /** * Initialize customer model * @@ -1075,8 +1093,7 @@ public function resetErrors() */ public function afterSave() { - $indexer = $this->indexerRegistry->get(self::CUSTOMER_GRID_INDEXER_ID); - if ($indexer->getState()->getStatus() == StateInterface::STATUS_VALID) { + if ($this->getIndexer()->getState()->getStatus() == StateInterface::STATUS_VALID) { $this->_getResource()->addCommitCallback([$this, 'reindex']); } return parent::afterSave(); @@ -1100,9 +1117,7 @@ public function afterDeleteCommit() */ public function reindex() { - /** @var \Magento\Framework\Indexer\IndexerInterface $indexer */ - $indexer = $this->indexerRegistry->get(self::CUSTOMER_GRID_INDEXER_ID); - $indexer->reindexRow($this->getId()); + $this->getIndexer()->reindexRow($this->getId()); } /** diff --git a/app/code/Magento/Customer/Setup/RecurringData.php b/app/code/Magento/Customer/Setup/RecurringData.php index fbef4c05d126d..76ff818ca7e5e 100644 --- a/app/code/Magento/Customer/Setup/RecurringData.php +++ b/app/code/Magento/Customer/Setup/RecurringData.php @@ -7,6 +7,7 @@ namespace Magento\Customer\Setup; use Magento\Framework\Indexer\IndexerRegistry; +use Magento\Framework\Indexer\StateInterface; use Magento\Framework\Setup\InstallDataInterface; use Magento\Framework\Setup\ModuleContextInterface; use Magento\Framework\Setup\ModuleDataSetupInterface; @@ -27,17 +28,34 @@ class RecurringData implements InstallDataInterface * * @param IndexerRegistry $indexerRegistry */ - public function __construct(IndexerRegistry $indexerRegistry) - { + public function __construct( + IndexerRegistry $indexerRegistry + ) { $this->indexerRegistry = $indexerRegistry; } /** - * {@inheritdoc} + * @inheritDoc */ public function install(ModuleDataSetupInterface $setup, ModuleContextInterface $context) { - $indexer = $this->indexerRegistry->get(Customer::CUSTOMER_GRID_INDEXER_ID); - $indexer->reindexAll(); + if ($this->isNeedToDoReindex($setup)) { + $indexer = $this->indexerRegistry->get(Customer::CUSTOMER_GRID_INDEXER_ID); + $indexer->reindexAll(); + } + } + + /** + * Check is re-index needed + * + * @param ModuleDataSetupInterface $setup + * @return bool + */ + private function isNeedToDoReindex(ModuleDataSetupInterface $setup) : bool + { + return !$setup->tableExists('customer_grid_flat') + || $this->indexerRegistry->get(Customer::CUSTOMER_GRID_INDEXER_ID) + ->getState() + ->getStatus() == StateInterface::STATUS_INVALID; } } diff --git a/app/code/Magento/Customer/Test/Unit/Setup/RecurringDataTest.php b/app/code/Magento/Customer/Test/Unit/Setup/RecurringDataTest.php new file mode 100644 index 0000000000000..ee1af91552f6d --- /dev/null +++ b/app/code/Magento/Customer/Test/Unit/Setup/RecurringDataTest.php @@ -0,0 +1,129 @@ +objectManagerHelper = new ObjectManagerHelper($this); + $this->state = $this->getMockBuilder(StateInterface::class) + ->setMethods(['getStatus']) + ->getMockForAbstractClass(); + $this->indexer = $this->getMockBuilder(IndexerInterface::class) + ->setMethods(['getState', 'reindexAll']) + ->getMockForAbstractClass(); + $this->indexer->expects($this->any()) + ->method('getState') + ->willReturn($this->state); + $this->indexerRegistry = $this->getMockBuilder(IndexerRegistry::class) + ->disableOriginalConstructor() + ->setMethods(['get']) + ->getMock(); + $this->indexerRegistry->expects($this->any()) + ->method('get') + ->with(Customer::CUSTOMER_GRID_INDEXER_ID) + ->willReturn($this->indexer); + $this->setup = $this->getMockBuilder(ModuleDataSetupInterface::class) + ->setMethods(['tableExists']) + ->getMockForAbstractClass(); + $this->context = $this->getMockBuilder(ModuleContextInterface::class) + ->getMockForAbstractClass(); + + $this->recurringData = $this->objectManagerHelper->getObject( + RecurringData::class, + [ + 'indexerRegistry' => $this->indexerRegistry + ] + ); + } + + /** + * @param bool $isTableExists + * @param string $indexerState + * @param int $countReindex + * @return void + * @dataProvider installDataProvider + */ + public function testInstall(bool $isTableExists, string $indexerState, int $countReindex) + { + $this->setup->expects($this->any()) + ->method('tableExists') + ->with('customer_grid_flat') + ->willReturn($isTableExists); + $this->state->expects($this->any()) + ->method('getStatus') + ->willReturn($indexerState); + $this->indexer->expects($this->exactly($countReindex)) + ->method('reindexAll'); + $this->recurringData->install($this->setup, $this->context); + } + + /** + * @return array + */ + public function installDataProvider() : array + { + return [ + [true, StateInterface::STATUS_INVALID, 1], + [false, StateInterface::STATUS_INVALID, 1], + [true, StateInterface::STATUS_VALID, 0], + [false, StateInterface::STATUS_VALID, 1], + ]; + } +} From 436d0ae410101e526ac9326483788153de507f26 Mon Sep 17 00:00:00 2001 From: Oleksandr Iegorov Date: Tue, 3 Mar 2020 11:19:07 -0600 Subject: [PATCH 5/5] MC-23890: Customer module Recurring setup script performance problems --- app/code/Magento/Customer/Model/Customer.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Customer/Model/Customer.php b/app/code/Magento/Customer/Model/Customer.php index 1e4914b152de3..ea52994735c63 100644 --- a/app/code/Magento/Customer/Model/Customer.php +++ b/app/code/Magento/Customer/Model/Customer.php @@ -1003,6 +1003,7 @@ public function getSharedWebsiteIds() */ public function getAttributeSetId() { + // phpstan:ignore "Call to an undefined static method*" return parent::getAttributeSetId() ?: CustomerMetadataInterface::ATTRIBUTE_SET_ID_CUSTOMER; }