This repository was archived by the owner on Apr 29, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 29
Memory error on customer import #97
Merged
magento-engcom-team
merged 8 commits into
magento-engcom:2.3-develop
from
dmanners:memory-error-on-customer-import
Feb 1, 2018
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f148e35
magento-engcom/import-export-improvements#43: make the customer impor…
dmanners 62f4722
magento-engcom/import-export-improvements#43: refactor the addCustome…
dmanners 3ee8961
magento-engcom/import-export-improvements#43: fix the AddressTests to…
dmanners 5cc36b7
magento-engcom/import-export-improvements#43: refactor the _createCus…
dmanners d45ade0
magento-engcom/import-export-improvements#43: refactor the StorageTests
dmanners e653591
magento-engcom/import-export-improvements#43: add a fallback for when…
dmanners c07b97b
magento-engcom/import-export-improvements#43: fix app/code/Magento/Cu…
dmanners 330634b
magento-engcom/import-export-improvements#43: update the AddressTest …
dmanners File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,34 +76,47 @@ public function __construct( | |
public function load() | ||
{ | ||
if ($this->_isCollectionLoaded == false) { | ||
$collection = clone $this->_customerCollection; | ||
$collection->removeAttributeToSelect(); | ||
$tableName = $collection->getResource()->getEntityTable(); | ||
$collection->getSelect()->from($tableName, ['entity_id', 'website_id', 'email']); | ||
|
||
$this->_byPagesIterator->iterate( | ||
$this->_customerCollection, | ||
$this->_pageSize, | ||
[[$this, 'addCustomer']] | ||
); | ||
$connection = $this->_customerCollection->getConnection(); | ||
$select = $connection->select(); | ||
$select->from($this->_customerCollection->getMainTable(), ['entity_id', 'website_id', 'email']); | ||
$results = $connection->fetchAll($select); | ||
foreach ($results as $customer) { | ||
$this->addCustomerByArray($customer); | ||
} | ||
|
||
$this->_isCollectionLoaded = true; | ||
} | ||
} | ||
|
||
/** | ||
* @param array $customer | ||
* @return $this | ||
*/ | ||
public function addCustomerByArray(array $customer) | ||
{ | ||
$email = strtolower(trim($customer['email'])); | ||
if (!isset($this->_customerIds[$email])) { | ||
$this->_customerIds[$email] = []; | ||
} | ||
$this->_customerIds[$email][$customer['website_id']] = $customer['entity_id']; | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* Add customer to array | ||
* | ||
* @deprecated @see addCustomerByArray | ||
* @param \Magento\Framework\DataObject|\Magento\Customer\Model\Customer $customer | ||
* @return $this | ||
*/ | ||
public function addCustomer(\Magento\Framework\DataObject $customer) | ||
{ | ||
$email = strtolower(trim($customer->getEmail())); | ||
if (!isset($this->_customerIds[$email])) { | ||
$this->_customerIds[$email] = []; | ||
$customerData = $customer->toArray(); | ||
if (!isset($customerData['entity_id']) && isset($customer['id'])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added this in here as there could be the case that we get an object that only has the |
||
$customerData['entity_id'] = $customerData['id']; | ||
} | ||
$this->_customerIds[$email][$customer->getWebsiteId()] = $customer->getId(); | ||
$this->addCustomerByArray($customerData); | ||
|
||
return $this; | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,8 +56,8 @@ class AddressTest extends \PHPUnit\Framework\TestCase | |
* @var array | ||
*/ | ||
protected $_customers = [ | ||
['id' => 1, 'email' => '[email protected]', 'website_id' => 1], | ||
['id' => 2, 'email' => '[email protected]', 'website_id' => 2], | ||
['entity_id' => 1, 'email' => '[email protected]', 'website_id' => 1], | ||
['entity_id' => 2, 'email' => '[email protected]', 'website_id' => 2], | ||
]; | ||
|
||
/** | ||
|
@@ -233,28 +233,54 @@ protected function _createAttrCollectionMock() | |
*/ | ||
protected function _createCustomerStorageMock() | ||
{ | ||
$customerStorage = $this->createPartialMock( | ||
\Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage::class, | ||
['load'] | ||
); | ||
$resourceMock = $this->createPartialMock( | ||
\Magento\Customer\Model\ResourceModel\Customer::class, | ||
['getIdFieldName'] | ||
); | ||
$resourceMock->expects($this->any())->method('getIdFieldName')->will($this->returnValue('id')); | ||
/** @var \Magento\Framework\DB\Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */ | ||
$selectMock = $this->getMockBuilder(\Magento\Framework\DB\Select::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['from']) | ||
->getMock(); | ||
$selectMock->expects($this->any())->method('from')->will($this->returnSelf()); | ||
|
||
/** @var $connectionMock \Magento\Framework\DB\Adapter\AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */ | ||
$connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['select', 'fetchAll']) | ||
->getMock(); | ||
$connectionMock->expects($this->any()) | ||
->method('select') | ||
->will($this->returnValue($selectMock)); | ||
|
||
/** @var \Magento\Customer\Model\ResourceModel\Customer\Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */ | ||
$customerCollection = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\Collection::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['getConnection']) | ||
->getMock(); | ||
$customerCollection->expects($this->any()) | ||
->method('getConnection') | ||
->will($this->returnValue($connectionMock)); | ||
|
||
/** @var \Magento\Customer\Model\ResourceModel\Customer\CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */ | ||
$collectionFactory = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\CollectionFactory::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['create']) | ||
->getMock(); | ||
$collectionFactory->expects($this->any()) | ||
->method('create') | ||
->willReturn($customerCollection); | ||
|
||
/** @var \Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */ | ||
$byPagesIteratorFactory = $this->getMockBuilder(\Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['create']) | ||
->getMock(); | ||
|
||
/** @var \Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage|\PHPUnit_Framework_MockObject_MockObject $customerStorage */ | ||
$customerStorage = $this->getMockBuilder(\Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage::class) | ||
->setMethods(['load']) | ||
->setConstructorArgs([$collectionFactory, $byPagesIteratorFactory]) | ||
->getMock(); | ||
|
||
foreach ($this->_customers as $customerData) { | ||
$data = [ | ||
'resource' => $resourceMock, | ||
'data' => $customerData, | ||
$this->createMock(\Magento\Customer\Model\Config\Share::class), | ||
$this->createMock(\Magento\Customer\Model\AddressFactory::class), | ||
$this->createMock(\Magento\Customer\Model\ResourceModel\Address\CollectionFactory::class), | ||
$this->createMock(\Magento\Customer\Model\GroupFactory::class), | ||
$this->createMock(\Magento\Customer\Model\AttributeFactory::class), | ||
]; | ||
/** @var $customer \Magento\Customer\Model\Customer */ | ||
$customer = $this->_objectManagerMock->getObject(\Magento\Customer\Model\Customer::class, $data); | ||
$customerStorage->addCustomer($customer); | ||
$customerStorage->addCustomerByArray($customerData); | ||
} | ||
return $customerStorage; | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,74 +26,63 @@ class StorageTest extends \PHPUnit\Framework\TestCase | |
|
||
protected function setUp() | ||
{ | ||
$this->_model = new \Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage( | ||
$this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\CollectionFactory::class) | ||
->disableOriginalConstructor() | ||
->getMock(), | ||
$this->getMockBuilder(\Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory::class) | ||
->disableOriginalConstructor() | ||
->getMock(), | ||
$this->_getModelDependencies() | ||
); | ||
$this->_model->load(); | ||
} | ||
|
||
protected function tearDown() | ||
{ | ||
unset($this->_model); | ||
} | ||
|
||
/** | ||
* Retrieve all necessary objects mocks which used inside customer storage | ||
* | ||
* @return array | ||
*/ | ||
protected function _getModelDependencies() | ||
{ | ||
$select = $this->getMockBuilder(\Magento\Framework\DB\Select::class) | ||
/** @var \Magento\Framework\DB\Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */ | ||
$selectMock = $this->getMockBuilder(\Magento\Framework\DB\Select::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['from']) | ||
->getMock(); | ||
$select->expects($this->any())->method('from')->will($this->returnCallback([$this, 'validateFrom'])); | ||
$selectMock->expects($this->any())->method('from')->will($this->returnSelf()); | ||
|
||
/** @var $connectionMock \Magento\Framework\DB\Adapter\AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */ | ||
$connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['select', 'fetchAll']) | ||
->getMock(); | ||
$connectionMock->expects($this->any()) | ||
->method('select') | ||
->will($this->returnValue($selectMock)); | ||
$connectionMock->expects($this->any()) | ||
->method('fetchAll') | ||
->will($this->returnValue([])); | ||
|
||
/** @var \Magento\Customer\Model\ResourceModel\Customer\Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */ | ||
$customerCollection = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\Collection::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['load', 'removeAttributeToSelect', 'getResource', 'getSelect']) | ||
->setMethods(['getConnection','getMainTable']) | ||
->getMock(); | ||
$customerCollection->expects($this->any()) | ||
->method('getConnection') | ||
->will($this->returnValue($connectionMock)); | ||
|
||
$resourceStub = new \Magento\Framework\DataObject(); | ||
$resourceStub->setEntityTable($this->_entityTable); | ||
$customerCollection->expects($this->once())->method('getResource')->will($this->returnValue($resourceStub)); | ||
$customerCollection->expects($this->any()) | ||
->method('getMainTable') | ||
->willReturn('customer_entity'); | ||
|
||
$customerCollection->expects($this->once())->method('getSelect')->will($this->returnValue($select)); | ||
/** @var \Magento\Customer\Model\ResourceModel\Customer\CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */ | ||
$collectionFactory = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\CollectionFactory::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['create']) | ||
->getMock(); | ||
$collectionFactory->expects($this->any()) | ||
->method('create') | ||
->willReturn($customerCollection); | ||
|
||
$byPagesIterator = $this->createPartialMock(\stdClass::class, ['iterate']); | ||
$byPagesIterator->expects($this->once()) | ||
->method('iterate') | ||
->will($this->returnCallback([$this, 'iterate'])); | ||
/** @var \Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */ | ||
$byPagesIteratorFactory = $this->getMockBuilder(\Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory::class) | ||
->disableOriginalConstructor() | ||
->setMethods(['create']) | ||
->getMock(); | ||
|
||
return [ | ||
'customer_collection' => $customerCollection, | ||
'collection_by_pages_iterator' => $byPagesIterator, | ||
'page_size' => 10 | ||
]; | ||
$this->_model = new \Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage( | ||
$collectionFactory, | ||
$byPagesIteratorFactory | ||
); | ||
$this->_model->load(); | ||
} | ||
|
||
/** | ||
* Iterate stub | ||
* | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
* | ||
* @param \Magento\Framework\Data\Collection $collection | ||
* @param int $pageSize | ||
* @param array $callbacks | ||
*/ | ||
public function iterate(\Magento\Framework\Data\Collection $collection, $pageSize, array $callbacks) | ||
protected function tearDown() | ||
{ | ||
foreach ($collection as $customer) { | ||
foreach ($callbacks as $callback) { | ||
call_user_func($callback, $customer); | ||
} | ||
} | ||
unset($this->_model); | ||
} | ||
|
||
/** | ||
|
@@ -112,13 +101,26 @@ public function testLoad() | |
} | ||
|
||
public function testAddCustomer() | ||
{ | ||
$customer = new \Magento\Framework\DataObject(['id' => 1, 'website_id' => 1, 'email' => '[email protected]']); | ||
$this->_model->addCustomer($customer); | ||
|
||
$propertyName = '_customerIds'; | ||
$this->assertAttributeCount(1, $propertyName, $this->_model); | ||
$this->assertAttributeContains([$customer->getWebsiteId() => $customer->getId()], $propertyName, $this->_model); | ||
$this->assertEquals( | ||
$customer->getId(), | ||
$this->_model->getCustomerId($customer->getEmail(), $customer->getWebsiteId()) | ||
); | ||
} | ||
|
||
public function testAddCustomerByArray() | ||
{ | ||
$propertyName = '_customerIds'; | ||
$customer = $this->_addCustomerToStorage(); | ||
|
||
$this->assertAttributeCount(1, $propertyName, $this->_model); | ||
|
||
$expectedCustomerData = [$customer->getWebsiteId() => $customer->getId()]; | ||
$expectedCustomerData = [$customer['website_id'] => $customer['entity_id']]; | ||
$this->assertAttributeContains($expectedCustomerData, $propertyName, $this->_model); | ||
} | ||
|
||
|
@@ -127,19 +129,19 @@ public function testGetCustomerId() | |
$customer = $this->_addCustomerToStorage(); | ||
|
||
$this->assertEquals( | ||
$customer->getId(), | ||
$this->_model->getCustomerId($customer->getEmail(), $customer->getWebsiteId()) | ||
$customer['entity_id'], | ||
$this->_model->getCustomerId($customer['email'], $customer['website_id']) | ||
); | ||
$this->assertFalse($this->_model->getCustomerId('[email protected]', $customer->getWebsiteId())); | ||
$this->assertFalse($this->_model->getCustomerId('[email protected]', $customer['website_id'])); | ||
} | ||
|
||
/** | ||
* @return \Magento\Framework\DataObject | ||
* @return array | ||
*/ | ||
protected function _addCustomerToStorage() | ||
{ | ||
$customer = new \Magento\Framework\DataObject(['id' => 1, 'website_id' => 1, 'email' => '[email protected]']); | ||
$this->_model->addCustomer($customer); | ||
$customer = ['entity_id' => 1, 'website_id' => 1, 'email' => '[email protected]']; | ||
$this->_model->addCustomerByArray($customer); | ||
|
||
return $customer; | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not such a big fan of doing things via array. Since we are only using id, email and website id would people suggest passing these in specifically?