Skip to content

Commit 3881f04

Browse files
🔃 [EngCom] Public Pull Requests - 2.3-develop
Accepted Public Pull Requests: - magento-engcom/php-7.2-support#31: Removed `each()` function usage (by @joni-jones) - magento-engcom/import-export-improvements#97: Memory error on customer import (by @dmanners) - #12624: Ranged selects always miss the last range (by @ajpevers)
2 parents 872638b + 77c4b73 commit 3881f04

File tree

14 files changed

+244
-115
lines changed

14 files changed

+244
-115
lines changed

app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -571,8 +571,7 @@ public function getAttributeRawValue($entityId, $attribute, $store)
571571
}
572572

573573
if (is_array($attributesData) && sizeof($attributesData) == 1) {
574-
$_data = each($attributesData);
575-
$attributesData = $_data[1];
574+
$attributesData = array_shift($attributesData);
576575
}
577576

578577
return $attributesData === false ? false : $attributesData;

app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,14 +308,13 @@ public function validateRow(array $rowData, $rowNumber)
308308
// Add new customer data into customer storage for address entity instance
309309
$websiteId = $this->_customerEntity->getWebsiteId($this->_currentWebsiteCode);
310310
if (!$this->_addressEntity->getCustomerStorage()->getCustomerId($this->_currentEmail, $websiteId)) {
311-
$customerData = new \Magento\Framework\DataObject(
311+
$this->_addressEntity->getCustomerStorage()->addCustomerByArray(
312312
[
313-
'id' => $this->_nextCustomerId,
313+
'entity_id' => $this->_nextCustomerId,
314314
'email' => $this->_currentEmail,
315315
'website_id' => $websiteId,
316316
]
317317
);
318-
$this->_addressEntity->getCustomerStorage()->addCustomer($customerData);
319318
$this->_nextCustomerId++;
320319
}
321320

app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,34 +76,47 @@ public function __construct(
7676
public function load()
7777
{
7878
if ($this->_isCollectionLoaded == false) {
79-
$collection = clone $this->_customerCollection;
80-
$collection->removeAttributeToSelect();
81-
$tableName = $collection->getResource()->getEntityTable();
82-
$collection->getSelect()->from($tableName, ['entity_id', 'website_id', 'email']);
83-
84-
$this->_byPagesIterator->iterate(
85-
$this->_customerCollection,
86-
$this->_pageSize,
87-
[[$this, 'addCustomer']]
88-
);
79+
$connection = $this->_customerCollection->getConnection();
80+
$select = $connection->select();
81+
$select->from($this->_customerCollection->getMainTable(), ['entity_id', 'website_id', 'email']);
82+
$results = $connection->fetchAll($select);
83+
foreach ($results as $customer) {
84+
$this->addCustomerByArray($customer);
85+
}
8986

9087
$this->_isCollectionLoaded = true;
9188
}
9289
}
9390

91+
/**
92+
* @param array $customer
93+
* @return $this
94+
*/
95+
public function addCustomerByArray(array $customer)
96+
{
97+
$email = strtolower(trim($customer['email']));
98+
if (!isset($this->_customerIds[$email])) {
99+
$this->_customerIds[$email] = [];
100+
}
101+
$this->_customerIds[$email][$customer['website_id']] = $customer['entity_id'];
102+
103+
return $this;
104+
}
105+
94106
/**
95107
* Add customer to array
96108
*
109+
* @deprecated @see addCustomerByArray
97110
* @param \Magento\Framework\DataObject|\Magento\Customer\Model\Customer $customer
98111
* @return $this
99112
*/
100113
public function addCustomer(\Magento\Framework\DataObject $customer)
101114
{
102-
$email = strtolower(trim($customer->getEmail()));
103-
if (!isset($this->_customerIds[$email])) {
104-
$this->_customerIds[$email] = [];
115+
$customerData = $customer->toArray();
116+
if (!isset($customerData['entity_id']) && isset($customer['id'])) {
117+
$customerData['entity_id'] = $customerData['id'];
105118
}
106-
$this->_customerIds[$email][$customer->getWebsiteId()] = $customer->getId();
119+
$this->addCustomerByArray($customerData);
107120

108121
return $this;
109122
}

app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88

99
use Magento\CustomerImportExport\Model\Import\Address;
1010
use Magento\ImportExport\Model\Import\AbstractEntity;
11+
use Magento\Framework\DB\Select;
12+
use Magento\Framework\DB\Adapter\AdapterInterface;
13+
use Magento\Customer\Model\ResourceModel\Customer\Collection;
14+
use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory;
15+
use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory;
16+
use Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage;
1117

1218
/**
1319
* Class AddressTest
@@ -56,8 +62,8 @@ class AddressTest extends \PHPUnit\Framework\TestCase
5662
* @var array
5763
*/
5864
protected $_customers = [
59-
['id' => 1, 'email' => '[email protected]', 'website_id' => 1],
60-
['id' => 2, 'email' => '[email protected]', 'website_id' => 2],
65+
['entity_id' => 1, 'email' => '[email protected]', 'website_id' => 1],
66+
['entity_id' => 2, 'email' => '[email protected]', 'website_id' => 2],
6167
];
6268

6369
/**
@@ -233,28 +239,54 @@ protected function _createAttrCollectionMock()
233239
*/
234240
protected function _createCustomerStorageMock()
235241
{
236-
$customerStorage = $this->createPartialMock(
237-
\Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage::class,
238-
['load']
239-
);
240-
$resourceMock = $this->createPartialMock(
241-
\Magento\Customer\Model\ResourceModel\Customer::class,
242-
['getIdFieldName']
243-
);
244-
$resourceMock->expects($this->any())->method('getIdFieldName')->will($this->returnValue('id'));
242+
/** @var Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */
243+
$selectMock = $this->getMockBuilder(Select::class)
244+
->disableOriginalConstructor()
245+
->setMethods(['from'])
246+
->getMock();
247+
$selectMock->expects($this->any())->method('from')->will($this->returnSelf());
248+
249+
/** @var $connectionMock AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */
250+
$connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class)
251+
->disableOriginalConstructor()
252+
->setMethods(['select', 'fetchAll'])
253+
->getMock();
254+
$connectionMock->expects($this->any())
255+
->method('select')
256+
->will($this->returnValue($selectMock));
257+
258+
/** @var Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */
259+
$customerCollection = $this->getMockBuilder(Collection::class)
260+
->disableOriginalConstructor()
261+
->setMethods(['getConnection'])
262+
->getMock();
263+
$customerCollection->expects($this->any())
264+
->method('getConnection')
265+
->will($this->returnValue($connectionMock));
266+
267+
/** @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */
268+
$collectionFactory = $this->getMockBuilder(CollectionFactory::class)
269+
->disableOriginalConstructor()
270+
->setMethods(['create'])
271+
->getMock();
272+
$collectionFactory->expects($this->any())
273+
->method('create')
274+
->willReturn($customerCollection);
275+
276+
/** @var CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */
277+
$byPagesIteratorFactory = $this->getMockBuilder(CollectionByPagesIteratorFactory::class)
278+
->disableOriginalConstructor()
279+
->setMethods(['create'])
280+
->getMock();
281+
282+
/** @var Storage|\PHPUnit_Framework_MockObject_MockObject $customerStorage */
283+
$customerStorage = $this->getMockBuilder(Storage::class)
284+
->setMethods(['load'])
285+
->setConstructorArgs([$collectionFactory, $byPagesIteratorFactory])
286+
->getMock();
287+
245288
foreach ($this->_customers as $customerData) {
246-
$data = [
247-
'resource' => $resourceMock,
248-
'data' => $customerData,
249-
$this->createMock(\Magento\Customer\Model\Config\Share::class),
250-
$this->createMock(\Magento\Customer\Model\AddressFactory::class),
251-
$this->createMock(\Magento\Customer\Model\ResourceModel\Address\CollectionFactory::class),
252-
$this->createMock(\Magento\Customer\Model\GroupFactory::class),
253-
$this->createMock(\Magento\Customer\Model\AttributeFactory::class),
254-
];
255-
/** @var $customer \Magento\Customer\Model\Customer */
256-
$customer = $this->_objectManagerMock->getObject(\Magento\Customer\Model\Customer::class, $data);
257-
$customerStorage->addCustomer($customer);
289+
$customerStorage->addCustomerByArray($customerData);
258290
}
259291
return $customerStorage;
260292
}

app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php

Lines changed: 70 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
namespace Magento\CustomerImportExport\Test\Unit\Model\ResourceModel\Import\Customer;
77

88
use Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage;
9+
use Magento\Framework\DB\Adapter\AdapterInterface;
10+
use Magento\Customer\Model\ResourceModel\Customer\Collection;
11+
use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory;
12+
use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory;
913

1014
class StorageTest extends \PHPUnit\Framework\TestCase
1115
{
@@ -26,74 +30,63 @@ class StorageTest extends \PHPUnit\Framework\TestCase
2630

2731
protected function setUp()
2832
{
29-
$this->_model = new \Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage(
30-
$this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\CollectionFactory::class)
31-
->disableOriginalConstructor()
32-
->getMock(),
33-
$this->getMockBuilder(\Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory::class)
34-
->disableOriginalConstructor()
35-
->getMock(),
36-
$this->_getModelDependencies()
37-
);
38-
$this->_model->load();
39-
}
40-
41-
protected function tearDown()
42-
{
43-
unset($this->_model);
44-
}
45-
46-
/**
47-
* Retrieve all necessary objects mocks which used inside customer storage
48-
*
49-
* @return array
50-
*/
51-
protected function _getModelDependencies()
52-
{
53-
$select = $this->getMockBuilder(\Magento\Framework\DB\Select::class)
33+
/** @var \Magento\Framework\DB\Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */
34+
$selectMock = $this->getMockBuilder(\Magento\Framework\DB\Select::class)
5435
->disableOriginalConstructor()
5536
->setMethods(['from'])
5637
->getMock();
57-
$select->expects($this->any())->method('from')->will($this->returnCallback([$this, 'validateFrom']));
58-
$customerCollection = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Customer\Collection::class)
38+
$selectMock->expects($this->any())->method('from')->will($this->returnSelf());
39+
40+
/** @var $connectionMock AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */
41+
$connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class)
42+
->disableOriginalConstructor()
43+
->setMethods(['select', 'fetchAll'])
44+
->getMock();
45+
$connectionMock->expects($this->any())
46+
->method('select')
47+
->will($this->returnValue($selectMock));
48+
$connectionMock->expects($this->any())
49+
->method('fetchAll')
50+
->will($this->returnValue([]));
51+
52+
/** @var Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */
53+
$customerCollection = $this->getMockBuilder(Collection::class)
5954
->disableOriginalConstructor()
60-
->setMethods(['load', 'removeAttributeToSelect', 'getResource', 'getSelect'])
55+
->setMethods(['getConnection','getMainTable'])
6156
->getMock();
57+
$customerCollection->expects($this->any())
58+
->method('getConnection')
59+
->will($this->returnValue($connectionMock));
6260

63-
$resourceStub = new \Magento\Framework\DataObject();
64-
$resourceStub->setEntityTable($this->_entityTable);
65-
$customerCollection->expects($this->once())->method('getResource')->will($this->returnValue($resourceStub));
61+
$customerCollection->expects($this->any())
62+
->method('getMainTable')
63+
->willReturn('customer_entity');
6664

67-
$customerCollection->expects($this->once())->method('getSelect')->will($this->returnValue($select));
65+
/** @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */
66+
$collectionFactory = $this->getMockBuilder(CollectionFactory::class)
67+
->disableOriginalConstructor()
68+
->setMethods(['create'])
69+
->getMock();
70+
$collectionFactory->expects($this->any())
71+
->method('create')
72+
->willReturn($customerCollection);
6873

69-
$byPagesIterator = $this->createPartialMock(\stdClass::class, ['iterate']);
70-
$byPagesIterator->expects($this->once())
71-
->method('iterate')
72-
->will($this->returnCallback([$this, 'iterate']));
74+
/** @var CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */
75+
$byPagesIteratorFactory = $this->getMockBuilder(CollectionByPagesIteratorFactory::class)
76+
->disableOriginalConstructor()
77+
->setMethods(['create'])
78+
->getMock();
7379

74-
return [
75-
'customer_collection' => $customerCollection,
76-
'collection_by_pages_iterator' => $byPagesIterator,
77-
'page_size' => 10
78-
];
80+
$this->_model = new Storage(
81+
$collectionFactory,
82+
$byPagesIteratorFactory
83+
);
84+
$this->_model->load();
7985
}
8086

81-
/**
82-
* Iterate stub
83-
*
84-
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
85-
*
86-
* @param \Magento\Framework\Data\Collection $collection
87-
* @param int $pageSize
88-
* @param array $callbacks
89-
*/
90-
public function iterate(\Magento\Framework\Data\Collection $collection, $pageSize, array $callbacks)
87+
protected function tearDown()
9188
{
92-
foreach ($collection as $customer) {
93-
foreach ($callbacks as $callback) {
94-
call_user_func($callback, $customer);
95-
}
96-
}
89+
unset($this->_model);
9790
}
9891

9992
/**
@@ -112,13 +105,26 @@ public function testLoad()
112105
}
113106

114107
public function testAddCustomer()
108+
{
109+
$customer = new \Magento\Framework\DataObject(['id' => 1, 'website_id' => 1, 'email' => '[email protected]']);
110+
$this->_model->addCustomer($customer);
111+
112+
$propertyName = '_customerIds';
113+
$this->assertAttributeCount(1, $propertyName, $this->_model);
114+
$this->assertAttributeContains([$customer->getWebsiteId() => $customer->getId()], $propertyName, $this->_model);
115+
$this->assertEquals(
116+
$customer->getId(),
117+
$this->_model->getCustomerId($customer->getEmail(), $customer->getWebsiteId())
118+
);
119+
}
120+
121+
public function testAddCustomerByArray()
115122
{
116123
$propertyName = '_customerIds';
117124
$customer = $this->_addCustomerToStorage();
118125

119126
$this->assertAttributeCount(1, $propertyName, $this->_model);
120-
121-
$expectedCustomerData = [$customer->getWebsiteId() => $customer->getId()];
127+
$expectedCustomerData = [$customer['website_id'] => $customer['entity_id']];
122128
$this->assertAttributeContains($expectedCustomerData, $propertyName, $this->_model);
123129
}
124130

@@ -127,19 +133,19 @@ public function testGetCustomerId()
127133
$customer = $this->_addCustomerToStorage();
128134

129135
$this->assertEquals(
130-
$customer->getId(),
131-
$this->_model->getCustomerId($customer->getEmail(), $customer->getWebsiteId())
136+
$customer['entity_id'],
137+
$this->_model->getCustomerId($customer['email'], $customer['website_id'])
132138
);
133-
$this->assertFalse($this->_model->getCustomerId('[email protected]', $customer->getWebsiteId()));
139+
$this->assertFalse($this->_model->getCustomerId('[email protected]', $customer['website_id']));
134140
}
135141

136142
/**
137-
* @return \Magento\Framework\DataObject
143+
* @return array
138144
*/
139145
protected function _addCustomerToStorage()
140146
{
141-
$customer = new \Magento\Framework\DataObject(['id' => 1, 'website_id' => 1, 'email' => '[email protected]']);
142-
$this->_model->addCustomer($customer);
147+
$customer = ['entity_id' => 1, 'website_id' => 1, 'email' => '[email protected]'];
148+
$this->_model->addCustomerByArray($customer);
143149

144150
return $customer;
145151
}

app/code/Magento/Eav/Model/Entity/Collection/AbstractCollection.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ public function joinTable($table, $bind, $fields = null, $cond = null, $joinType
803803
{
804804
$tableAlias = null;
805805
if (is_array($table)) {
806-
list($tableAlias, $tableName) = each($table);
806+
list($tableAlias, $tableName) = [key($table), current($table)];
807807
} else {
808808
$tableName = $table;
809809
}

0 commit comments

Comments
 (0)