Skip to content

Commit

Permalink
Merge pull request #5423 from magento-tango/MC-23890-1
Browse files Browse the repository at this point in the history
[tango] MC-23890: 2.3 Customer module Recurring setup script performance problems.
  • Loading branch information
dhorytskyi authored Mar 7, 2020
2 parents 90012f2 + 2143a27 commit cac512f
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 12 deletions.
30 changes: 23 additions & 7 deletions app/code/Magento/Customer/Model/Customer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -985,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;
}

Expand Down Expand Up @@ -1075,8 +1094,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();
Expand All @@ -1100,9 +1118,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());
}

/**
Expand Down
28 changes: 23 additions & 5 deletions app/code/Magento/Customer/Setup/RecurringData.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
129 changes: 129 additions & 0 deletions app/code/Magento/Customer/Test/Unit/Setup/RecurringDataTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Customer\Test\Unit\Setup;

use Magento\Framework\Indexer\IndexerInterface;
use Magento\Framework\Indexer\StateInterface;
use Magento\Framework\Indexer\IndexerRegistry;
use Magento\Framework\Setup\ModuleDataSetupInterface;
use Magento\Framework\Setup\ModuleContextInterface;
use Magento\Customer\Model\Customer;
use Magento\Customer\Setup\RecurringData;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;

/**
* Test for recurring data
*/
class RecurringDataTest extends \PHPUnit\Framework\TestCase
{
/**
* @var ObjectManagerHelper
*/
private $objectManagerHelper;

/**
* @var IndexerInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $indexer;

/**
* @var StateInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $state;

/**
* @var IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject
*/
private $indexerRegistry;

/**
* @var ModuleDataSetupInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $setup;

/**
* @var ModuleContextInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $context;

/**
* @var RecurringData
*/
private $recurringData;

/**
* @inheritdoc
*/
protected function setUp()
{
$this->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],
];
}
}

0 comments on commit cac512f

Please sign in to comment.