Skip to content

Commit 4c8d30a

Browse files
bubasumabubasuma
committed
MC-34522: Two near-simultaneous product imports interfere with each other
- Add user id to "import data" table in order to allow concurrent import
1 parent 57c527e commit 4c8d30a

File tree

5 files changed

+199
-18
lines changed

5 files changed

+199
-18
lines changed

‎app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/CustomerComposite/DataTest.php

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111
namespace Magento\CustomerImportExport\Test\Unit\Model\ResourceModel\Import\CustomerComposite;
1212

13+
use Magento\Backend\Model\Auth\Session;
1314
use Magento\CustomerImportExport\Model\Import\Address;
1415
use Magento\CustomerImportExport\Model\Import\CustomerComposite;
1516
use Magento\Framework\App\ResourceConnection;
@@ -20,13 +21,44 @@
2021
use Magento\Framework\Json\Helper\Data;
2122
use Magento\Framework\Model\ResourceModel\Db\Context;
2223
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
24+
use PHPUnit\Framework\MockObject\MockBuilder;
2325
use PHPUnit\Framework\TestCase;
2426

2527
/**
2628
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2729
*/
2830
class DataTest extends TestCase
2931
{
32+
/**
33+
* @var string[]
34+
*/
35+
private $mockBuilderCallbacks = [
36+
Session::class => 'getSessionMock'
37+
];
38+
39+
/**
40+
* @inheritDoc
41+
*/
42+
public function getMockBuilder(string $className): MockBuilder
43+
{
44+
$mockBuilder = parent::getMockBuilder($className);
45+
if (isset($this->mockBuilderCallbacks[$className])) {
46+
$mockBuilder = call_user_func_array([$this, $this->mockBuilderCallbacks[$className]], [$mockBuilder]);
47+
}
48+
49+
return $mockBuilder;
50+
}
51+
52+
/**
53+
* @param MockBuilder $mockBuilder
54+
* @return MockBuilder
55+
* @SuppressWarnings(PHPMD.UnusedPrivateMethod) Used in callback.
56+
*/
57+
private function getSessionMock(MockBuilder $mockBuilder): MockBuilder
58+
{
59+
return $mockBuilder->onlyMethods(['isLoggedIn']);
60+
}
61+
3062
/**
3163
* Array of customer attributes
3264
*
@@ -57,9 +89,10 @@ protected function _getDependencies($entityType, $bunchData)
5789
);
5890

5991
/** @var $selectMock \Magento\Framework\DB\Select */
60-
$selectMock = $this->createPartialMock(Select::class, ['from', 'order']);
92+
$selectMock = $this->createPartialMock(Select::class, ['from', 'order', 'where']);
6193
$selectMock->expects($this->any())->method('from')->willReturnSelf();
6294
$selectMock->expects($this->any())->method('order')->willReturnSelf();
95+
$selectMock->expects($this->any())->method('where')->willReturnSelf();
6396

6497
/** @var AdapterInterface $connectionMock */
6598
$connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class)

‎app/code/Magento/ImportExport/Model/ResourceModel/Import/Data.php

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,23 @@
55
*/
66
namespace Magento\ImportExport\Model\ResourceModel\Import;
77

8+
use Magento\Backend\Model\Auth\Session;
9+
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\DB\Select;
11+
812
/**
913
* ImportExport import data resource model
1014
*
1115
* @api
1216
* @since 100.0.2
17+
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse) Necessary to get current logged in user without modifying methods
1318
*/
1419
class Data extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb implements \IteratorAggregate
1520
{
21+
/**
22+
* Offline import user ID
23+
*/
24+
private const DEFAULT_USER_ID = 0;
1625
/**
1726
* @var \Iterator
1827
*/
@@ -24,21 +33,28 @@ class Data extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb implemen
2433
* @var \Magento\Framework\Json\Helper\Data
2534
*/
2635
protected $jsonHelper;
36+
/**
37+
* @var Session
38+
*/
39+
private $authSession;
2740

2841
/**
2942
* Class constructor
3043
*
3144
* @param \Magento\Framework\Model\ResourceModel\Db\Context $context
3245
* @param \Magento\Framework\Json\Helper\Data $jsonHelper
33-
* @param string $connectionName
46+
* @param string|null $connectionName
47+
* @param Session|null $authSession
3448
*/
3549
public function __construct(
3650
\Magento\Framework\Model\ResourceModel\Db\Context $context,
3751
\Magento\Framework\Json\Helper\Data $jsonHelper,
38-
$connectionName = null
52+
$connectionName = null,
53+
?Session $authSession = null
3954
) {
4055
parent::__construct($context, $connectionName);
4156
$this->jsonHelper = $jsonHelper;
57+
$this->authSession = $authSession ?? ObjectManager::getInstance()->get(Session::class);
4258
}
4359

4460
/**
@@ -60,6 +76,7 @@ public function getIterator()
6076
{
6177
$connection = $this->getConnection();
6278
$select = $connection->select()->from($this->getMainTable(), ['data'])->order('id ASC');
79+
$select = $this->prepareSelect($select);
6380
$stmt = $connection->query($select);
6481

6582
$stmt->setFetchMode(\Zend_Db::FETCH_NUM);
@@ -81,7 +98,7 @@ public function getIterator()
8198
*/
8299
public function cleanBunches()
83100
{
84-
return $this->getConnection()->delete($this->getMainTable());
101+
return $this->getConnection()->delete($this->getMainTable(), $this->prepareDelete([]));
85102
}
86103

87104
/**
@@ -114,7 +131,9 @@ public function getEntityTypeCode()
114131
public function getUniqueColumnData($code)
115132
{
116133
$connection = $this->getConnection();
117-
$values = array_unique($connection->fetchCol($connection->select()->from($this->getMainTable(), [$code])));
134+
$select = $connection->select()->from($this->getMainTable(), [$code]);
135+
$select = $this->prepareSelect($select);
136+
$values = array_unique($connection->fetchCol($select));
118137

119138
if (count($values) != 1) {
120139
throw new \Magento\Framework\Exception\LocalizedException(
@@ -169,7 +188,56 @@ public function saveBunch($entity, $behavior, array $data)
169188

170189
return $this->getConnection()->insert(
171190
$this->getMainTable(),
172-
['behavior' => $behavior, 'entity' => $entity, 'data' => $encodedData]
191+
$this->prepareInsert(['behavior' => $behavior, 'entity' => $entity, 'data' => $encodedData])
173192
);
174193
}
194+
195+
/**
196+
* Prepare select for query
197+
*
198+
* @param Select $select
199+
* @return Select
200+
*/
201+
private function prepareSelect(Select $select): Select
202+
{
203+
$select->where('user_id=?', $this->getCurrentUserId() ?? self::DEFAULT_USER_ID);
204+
205+
return $select;
206+
}
207+
208+
/**
209+
* Prepare data for insert
210+
*
211+
* @param array $data
212+
* @return array
213+
*/
214+
private function prepareInsert(array $data): array
215+
{
216+
$data['user_id'] = $this->getCurrentUserId() ?? self::DEFAULT_USER_ID;
217+
218+
return $data;
219+
}
220+
221+
/**
222+
* Prepare delete constraints
223+
*
224+
* @param array $where
225+
* @return array
226+
*/
227+
private function prepareDelete(array $where): array
228+
{
229+
$where['user_id=?'] = $this->getCurrentUserId() ?? self::DEFAULT_USER_ID;
230+
231+
return $where;
232+
}
233+
234+
/**
235+
* Get current user ID
236+
*
237+
* @return int|null
238+
*/
239+
private function getCurrentUserId(): ?int
240+
{
241+
return $this->authSession->isLoggedIn() ? $this->authSession->getUser()->getId() : null;
242+
}
175243
}

‎app/code/Magento/ImportExport/etc/db_schema.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
<column xsi:type="varchar" name="entity" nullable="false" length="50" comment="Entity"/>
1313
<column xsi:type="varchar" name="behavior" nullable="false" length="10" default="append" comment="Behavior"/>
1414
<column xsi:type="longtext" name="data" nullable="true" comment="Data"/>
15+
<column xsi:type="int" name="user_id" unsigned="true" nullable="false" identity="false" default="0"
16+
comment="User ID"/>
1517
<constraint xsi:type="primary" referenceId="PRIMARY">
1618
<column name="id"/>
1719
</constraint>

‎app/code/Magento/ImportExport/etc/db_schema_whitelist.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"id": true,
55
"entity": true,
66
"behavior": true,
7-
"data": true
7+
"data": true,
8+
"user_id": true
89
},
910
"constraint": {
1011
"PRIMARY": true
@@ -24,4 +25,4 @@
2425
"PRIMARY": true
2526
}
2627
}
27-
}
28+
}

‎dev/tests/integration/testsuite/Magento/ImportExport/Model/ResourceModel/Import/DataTest.php

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,23 @@
55
*/
66
namespace Magento\ImportExport\Model\ResourceModel\Import;
77

8+
use Magento\Backend\Model\Auth;
9+
use Magento\Framework\Exception\LocalizedException;
10+
use Magento\Framework\Registry;
11+
use Magento\TestFramework\Bootstrap;
12+
use Magento\TestFramework\ObjectManager;
13+
use PHPUnit\Framework\TestCase;
14+
use Zend_Db_Expr;
15+
816
/**
917
* Test Import Data resource model
1018
*
1119
* @magentoDataFixture Magento/ImportExport/_files/import_data.php
1220
*/
13-
class DataTest extends \PHPUnit\Framework\TestCase
21+
class DataTest extends TestCase
1422
{
1523
/**
16-
* @var \Magento\ImportExport\Model\ResourceModel\Import\Data
24+
* @var Data
1725
*/
1826
protected $_model;
1927

@@ -22,7 +30,7 @@ protected function setUp(): void
2230
parent::setUp();
2331

2432
$this->_model = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
25-
\Magento\ImportExport\Model\ResourceModel\Import\Data::class
33+
Data::class
2634
);
2735
}
2836

@@ -31,11 +39,11 @@ protected function setUp(): void
3139
*/
3240
public function testGetUniqueColumnData()
3341
{
34-
/** @var $objectManager \Magento\TestFramework\ObjectManager */
42+
/** @var $objectManager ObjectManager */
3543
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
3644

3745
$expectedBunches = $objectManager->get(
38-
\Magento\Framework\Registry::class
46+
Registry::class
3947
)->registry(
4048
'_fixture/Magento_ImportExport_Import_Data'
4149
);
@@ -49,7 +57,7 @@ public function testGetUniqueColumnData()
4957
*/
5058
public function testGetUniqueColumnDataException()
5159
{
52-
$this->expectException(\Magento\Framework\Exception\LocalizedException::class);
60+
$this->expectException(LocalizedException::class);
5361

5462
$this->_model->getUniqueColumnData('data');
5563
}
@@ -59,11 +67,11 @@ public function testGetUniqueColumnDataException()
5967
*/
6068
public function testGetBehavior()
6169
{
62-
/** @var $objectManager \Magento\TestFramework\ObjectManager */
70+
/** @var $objectManager ObjectManager */
6371
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
6472

6573
$expectedBunches = $objectManager->get(
66-
\Magento\Framework\Registry::class
74+
Registry::class
6775
)->registry(
6876
'_fixture/Magento_ImportExport_Import_Data'
6977
);
@@ -76,15 +84,84 @@ public function testGetBehavior()
7684
*/
7785
public function testGetEntityTypeCode()
7886
{
79-
/** @var $objectManager \Magento\TestFramework\ObjectManager */
87+
/** @var $objectManager ObjectManager */
8088
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
8189

8290
$expectedBunches = $objectManager->get(
83-
\Magento\Framework\Registry::class
91+
Registry::class
8492
)->registry(
8593
'_fixture/Magento_ImportExport_Import_Data'
8694
);
8795

8896
$this->assertEquals($expectedBunches[0]['entity'], $this->_model->getEntityTypeCode());
8997
}
98+
99+
/**
100+
* Test that users import data are isolated from each other
101+
*/
102+
public function testUsersImportDataShouldBeIsolated()
103+
{
104+
$count = $this->_model->getConnection()->fetchOne(
105+
$this->_model->getConnection()->select()->from($this->_model->getMainTable(), new Zend_Db_Expr('count(*)'))
106+
);
107+
$auth = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(Auth::class);
108+
$auth->login(Bootstrap::ADMIN_NAME, Bootstrap::ADMIN_PASSWORD);
109+
$bunches = [
110+
0 => [
111+
'entity' => 'customer',
112+
'behavior' => 'delete',
113+
'data' => [
114+
[
115+
'email' => 'mike.miller.101@magento.com',
116+
'_website' => 'base',
117+
],
118+
[
119+
'email' => 'john.doe.102@magento.com',
120+
'_website' => 'base',
121+
]
122+
]
123+
],
124+
1 => [
125+
'entity' => 'customer',
126+
'behavior' => 'delete',
127+
'data' => [
128+
[
129+
'email' => 'jack.simon.103@magento.com',
130+
'_website' => 'base',
131+
],
132+
],
133+
],
134+
];
135+
$expectedData = [];
136+
foreach ($bunches as $bunch) {
137+
$this->_model->saveBunch($bunch['entity'], $bunch['behavior'], $bunch['data']);
138+
$expectedData[] = $bunch['data'];
139+
}
140+
$expectedData = array_merge(...$expectedData);
141+
$actualData = [];
142+
while ($data = $this->_model->getNextBunch()) {
143+
$actualData[] = $data;
144+
}
145+
$actualData = array_merge(...$actualData);
146+
$this->assertEquals($expectedData, $actualData);
147+
$this->_model->cleanBunches();
148+
$actualData = [];
149+
while ($data = $this->_model->getNextBunch()) {
150+
$actualData[] = $data;
151+
}
152+
$this->assertEmpty($actualData);
153+
$newCount = $this->_model->getConnection()->fetchOne(
154+
$this->_model->getConnection()->select()->from($this->_model->getMainTable(), new Zend_Db_Expr('count(*)'))
155+
);
156+
$this->assertEquals($count, $newCount);
157+
}
158+
159+
/**
160+
* @inheritDoc
161+
*/
162+
protected function tearDown(): void
163+
{
164+
$this->_model->getConnection()->delete($this->_model->getMainTable());
165+
parent::tearDown();
166+
}
90167
}

0 commit comments

Comments
 (0)