Skip to content

Commit 1287421

Browse files
cparticacpartica
committed
MAGETWO-58924: SQL error wait timeout error when saving categories
- fix issues with code review, add phpdocs for renaming and fixing unit tests
1 parent fec7208 commit 1287421

File tree

9 files changed

+155
-74
lines changed

9 files changed

+155
-74
lines changed

‎app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryHashMap.php

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,30 +42,13 @@ public function __construct(
4242
$this->categoryResource = $categoryResource;
4343
}
4444

45-
/**
46-
* {@inheritdoc}
47-
*/
48-
public function getAllData($categoryId)
49-
{
50-
return $this->generateData($categoryId);
51-
}
52-
53-
/**
54-
* {@inheritdoc}
55-
*/
56-
public function getData($categoryId, $key)
57-
{
58-
$categorySpecificData = $this->generateData($categoryId);
59-
return $categorySpecificData[$key];
60-
}
61-
6245
/**
6346
* Returns an array of categories ids that includes category identified by $categoryId and all its subcategories
6447
*
6548
* @param int $categoryId
6649
* @return array
6750
*/
68-
private function generateData($categoryId)
51+
public function getAllData($categoryId)
6952
{
7053
if (!isset($this->hashMap[$categoryId])) {
7154
$category = $this->categoryRepository->get($categoryId);
@@ -75,6 +58,18 @@ private function generateData($categoryId)
7558
return $this->hashMap[$categoryId];
7659
}
7760

61+
/**
62+
* {@inheritdoc}
63+
*/
64+
public function getData($categoryId, $key)
65+
{
66+
$categorySpecificData = $this->getAllData($categoryId);
67+
if (isset($categorySpecificData[$key])) {
68+
return $categorySpecificData[$key];
69+
}
70+
return [];
71+
}
72+
7873
/**
7974
* Queries the database for sub-categories ids from a category
8075
*

‎app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUsedInProductsHashMap.php

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,14 @@ public function __construct(
3333
$this->hashMapPool = $hashMapPool;
3434
}
3535

36-
/**
37-
* {@inheritdoc}
38-
*/
39-
public function getAllData($categoryId)
40-
{
41-
return $this->generateData($categoryId);
42-
}
43-
44-
/**
45-
* {@inheritdoc}
46-
*/
47-
public function getData($categoryId, $key)
48-
{
49-
$categorySpecificData = $this->generateData($categoryId);
50-
return $categorySpecificData[$key];
51-
}
52-
5336
/**
5437
* Returns an array of product ids for all DataProductHashMap list,
5538
* that occur in other categories not part of DataCategoryHashMap list
5639
*
5740
* @param int $categoryId
5841
* @return array
5942
*/
60-
private function generateData($categoryId)
43+
public function getAllData($categoryId)
6144
{
6245
if (!isset($this->hashMap[$categoryId])) {
6346
$productsLinkConnection = $this->connection->getConnection();
@@ -91,6 +74,18 @@ private function generateData($categoryId)
9174
return $this->hashMap[$categoryId];
9275
}
9376

77+
/**
78+
* {@inheritdoc}
79+
*/
80+
public function getData($categoryId, $key)
81+
{
82+
$categorySpecificData = $this->getAllData($categoryId);
83+
if (isset($categorySpecificData[$key])) {
84+
return $categorySpecificData[$key];
85+
}
86+
return [];
87+
}
88+
9489
/**
9590
* {@inheritdoc}
9691
*/

‎app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductHashMap.php

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,13 @@ public function __construct(
4040
$this->connection = $connection;
4141
}
4242

43-
/**
44-
* {@inheritdoc}
45-
*/
46-
public function getAllData($categoryId)
47-
{
48-
return $this->generateData($categoryId);
49-
}
50-
51-
/**
52-
* {@inheritdoc}
53-
*/
54-
public function getData($categoryId, $key)
55-
{
56-
$categorySpecificData = $this->generateData($categoryId);
57-
return $categorySpecificData[$key];
58-
}
59-
6043
/**
6144
* Returns an array of ids of all visible products and assigned to a category and all its subcategories
6245
*
6346
* @param int $categoryId
6447
* @return array
6548
*/
66-
private function generateData($categoryId)
49+
public function getAllData($categoryId)
6750
{
6851
if (!isset($this->hashMap[$categoryId])) {
6952
$productsCollection = $this->collectionFactory->create();
@@ -89,6 +72,18 @@ private function generateData($categoryId)
8972
return $this->hashMap[$categoryId];
9073
}
9174

75+
/**
76+
* {@inheritdoc}
77+
*/
78+
public function getData($categoryId, $key)
79+
{
80+
$categorySpecificData = $this->getAllData($categoryId);
81+
if (isset($categorySpecificData[$key])) {
82+
return $categorySpecificData[$key];
83+
}
84+
return [];
85+
}
86+
9287
/**
9388
* {@inheritdoc}
9489
*/

‎app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapPool.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,18 @@ public function getDataMap($instanceName, $categoryId)
4444
{
4545
$key = $instanceName . '-' . $categoryId;
4646
if (!isset($this->dataArray[$key])) {
47-
$this->dataArray[$key] = $this->objectManager->create(
47+
$instance = $this->objectManager->create(
4848
$instanceName,
4949
[
5050
'category' => $categoryId
5151
]
5252
);
53-
if (!$this->dataArray[$key] instanceof DatabaseMapInterface) {
53+
if (!$instance instanceof DatabaseMapInterface) {
5454
throw new \InvalidArgumentException(
5555
$instanceName . ' does not implement interface ' . DatabaseMapInterface::class
5656
);
5757
}
58+
$this->dataArray[$key] = $instance;
5859
}
5960
return $this->dataArray[$key];
6061
}

‎app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapPool.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,18 @@ public function getDataMap($instanceName, $categoryId)
4545
{
4646
$key = $instanceName . '-' . $categoryId;
4747
if (!isset($this->dataArray[$key])) {
48-
$this->dataArray[$key] = $this->objectManager->create(
48+
$instance = $this->objectManager->create(
4949
$instanceName,
5050
[
5151
'category' => $categoryId
5252
]
5353
);
54-
if (!$this->dataArray[$key] instanceof HashMapInterface) {
54+
if (!$instance instanceof HashMapInterface) {
5555
throw new \InvalidArgumentException(
5656
$instanceName . ' does not implement interface ' . HashMapInterface::class
5757
);
5858
}
59+
$this->dataArray[$key] = $instance;
5960
}
6061
return $this->dataArray[$key];
6162
}

‎app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/HashMapPoolTest.php

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\CatalogUrlRewrite\Model\Map\HashMapPool;
1010
use Magento\CatalogUrlRewrite\Model\Map\DataCategoryHashMap;
1111
use Magento\CatalogUrlRewrite\Model\Map\DataProductHashMap;
12+
use Magento\CatalogUrlRewrite\Model\Map\DataCategoryUsedInProductsHashMap;
1213
use Magento\Framework\ObjectManagerInterface;
1314

1415
/**
@@ -41,15 +42,35 @@ public function testGetDataMap()
4142
{
4243
$dataCategoryMapMock = $this->getMock(DataCategoryHashMap::class, [], [], '', false);
4344
$dataProductMapMock = $this->getMock(DataProductHashMap::class, [], [], '', false);
44-
$dataProductMapMockOtherCategory = $this->getMock(DataProductHashMap::class, [], [], '', false);
45+
$dataProductMapMockOtherCategory = $this->getMock(DataCategoryUsedInProductsHashMap::class, [], [], '', false);
4546

4647
$this->objectManagerMock->expects($this->any())
4748
->method('create')
48-
->willReturnOnConsecutiveCalls($dataCategoryMapMock, $dataProductMapMock, $dataProductMapMockOtherCategory);
49-
$this->assertEquals($dataCategoryMapMock, $this->model->getDataMap(DataCategoryHashMap::class, 1));
49+
->willReturnMap(
50+
[
51+
[
52+
DataCategoryHashMap::class,
53+
['category' => 1],
54+
$dataCategoryMapMock
55+
],
56+
[
57+
DataProductHashMap::class,
58+
['category' => 1],
59+
$dataProductMapMock
60+
],
61+
[
62+
DataCategoryUsedInProductsHashMap::class,
63+
['category' => 2],
64+
$dataProductMapMockOtherCategory
65+
]
66+
]
67+
);
5068
$this->assertSame($dataCategoryMapMock, $this->model->getDataMap(DataCategoryHashMap::class, 1));
51-
$this->assertEquals($dataProductMapMock, $this->model->getDataMap(DataProductHashMap::class, 1));
52-
$this->assertEquals($dataProductMapMockOtherCategory, $this->model->getDataMap(DataCategoryHashMap::class, 2));
69+
$this->assertSame($dataProductMapMock, $this->model->getDataMap(DataProductHashMap::class, 1));
70+
$this->assertSame(
71+
$dataProductMapMockOtherCategory,
72+
$this->model->getDataMap(DataCategoryUsedInProductsHashMap::class, 2)
73+
);
5374
}
5475

5576
/**

‎app/etc/di.xml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,4 +1221,17 @@
12211221
</argument>
12221222
</arguments>
12231223
</type>
1224+
<type name="Magento\Framework\DB\TemporaryTableService">
1225+
<arguments>
1226+
<argument name="allowedIndexMethods" xsi:type="array">
1227+
<item name="HASH" xsi:type="string">HASH</item>
1228+
<item name="BTREE" xsi:type="string">BTREE</item>
1229+
</argument>
1230+
<argument name="allowedEngines" xsi:type="array">
1231+
<item name="INNODB" xsi:type="string">INNODB</item>
1232+
<item name="MEMORY" xsi:type="string">MEMORY</item>
1233+
<item name="MYISAM" xsi:type="string">MYISAM</item>
1234+
</argument>
1235+
</arguments>
1236+
</type>
12241237
</config>

‎lib/internal/Magento/Framework/DB/TemporaryTableService.php

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,21 @@
1616
*/
1717
class TemporaryTableService
1818
{
19+
CONST HASH = 'HASH';
20+
CONST INNODB = 'INNODB';
21+
22+
/**
23+
* @var string[]
24+
*/
25+
private $allowedIndexMethods;
26+
1927
/**
20-
* @var $random
28+
* @var string[]
29+
*/
30+
private $allowedEngines;
31+
32+
/**
33+
* @var \Magento\Framework\Math\Random
2134
*/
2235
private $random;
2336

@@ -28,10 +41,17 @@ class TemporaryTableService
2841

2942
/**
3043
* @param \Magento\Framework\Math\Random $random
44+
* @param string[] $allowedIndexMethods
45+
* @param string[] $allowedEngines
3146
*/
32-
public function __construct(\Magento\Framework\Math\Random $random)
33-
{
47+
public function __construct(
48+
\Magento\Framework\Math\Random $random,
49+
$allowedIndexMethods = [self::HASH],
50+
$allowedEngines = [self::INNODB]
51+
) {
3452
$this->random = $random;
53+
$this->allowedIndexMethods = $allowedIndexMethods;
54+
$this->allowedEngines = $allowedEngines;
3555
}
3656

3757
/**
@@ -55,16 +75,29 @@ public function __construct(\Magento\Framework\Math\Random $random)
5575
* @param AdapterInterface $adapter
5676
* @param array $indexes
5777
* @param string $indexMethod
58-
* @param string $engine
78+
* @param string $dbEngine
5979
* @return string
80+
* @throws \InvalidArgumentException
6081
*/
6182
public function createFromSelect(
6283
Select $select,
6384
AdapterInterface $adapter,
6485
array $indexes = [],
65-
$indexMethod = 'HASH',
66-
$engine = 'INNODB'
86+
$indexMethod = self::HASH,
87+
$dbEngine = self::INNODB
6788
) {
89+
if (!in_array($indexMethod, $this->allowedIndexMethods)) {
90+
throw new \InvalidArgumentException(
91+
sprintf('indexMethod must be of type %s', implode(',', $this->allowedIndexMethods))
92+
);
93+
}
94+
95+
if (!in_array($dbEngine, $this->allowedEngines)) {
96+
throw new \InvalidArgumentException(
97+
sprintf('dbEngine must be of type %s', implode(',', $this->allowedEngines))
98+
);
99+
}
100+
68101
$name = $this->random->getUniqueHash('tmp_select_');
69102

70103
$indexStatements = [];
@@ -90,7 +123,7 @@ public function createFromSelect(
90123
'CREATE TEMPORARY TABLE %s %s ENGINE=%s IGNORE (%s)',
91124
$adapter->quoteIdentifier($name),
92125
$indexStatements ? '(' . implode(',', $indexStatements) . ')' : '',
93-
$adapter->quoteIdentifier($engine),
126+
$adapter->quoteIdentifier($dbEngine),
94127
"{$select}"
95128
);
96129

0 commit comments

Comments
 (0)