Skip to content

Commit 40c9d62

Browse files
dvoskoboinikovdvoskoboinikov
authored
Merge pull request magento#2073 from magento-qwerty/2.2.4-PR-20170111
Fixed issues: - MAGETWO-84397: Overlapping reindex processes of catalogrule_rule - MAGETWO-84992: Bundle Product is_salable check is wrong - MAGETWO-85205: Multiselect Attribute is not saved for a product in the admin panel when it has a related product using another attribute set
2 parents 2bbf7e9 + b9959cd commit 40c9d62

File tree

22 files changed

+460
-117
lines changed

22 files changed

+460
-117
lines changed

‎app/code/Magento/Bundle/Model/Product/Type.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Magento\Framework\Serialize\Serializer\Json;
1515
use Magento\Framework\EntityManager\MetadataPool;
1616
use Magento\Bundle\Model\ResourceModel\Selection\Collection\FilterApplier as SelectionCollectionFilterApplier;
17+
use Magento\Bundle\Model\ResourceModel\Selection\Collection as Selections;
1718

1819
/**
1920
* Bundle Type Model
@@ -486,7 +487,9 @@ public function getSelectionsCollection($optionIds, $product)
486487
\Magento\Catalog\Api\Data\ProductInterface::class
487488
);
488489

489-
$selectionsCollection = $this->_bundleCollection->create()
490+
/** @var Selections $selectionsCollection */
491+
$selectionsCollection = $this->_bundleCollection->create();
492+
$selectionsCollection
490493
->addAttributeToSelect($this->_config->getProductAttributes())
491494
->addAttributeToSelect('tax_class_id') //used for calculation item taxes in Bundle with Dynamic Price
492495
->setFlag('product_children', true)
@@ -587,6 +590,7 @@ public function isSalable($product)
587590
foreach ($this->getOptionsCollection($product) as $option) {
588591
$hasSalable = false;
589592

593+
/** @var Selections $selectionsCollection */
590594
$selectionsCollection = $this->_bundleCollection->create();
591595
$selectionsCollection->addAttributeToSelect('status');
592596
$selectionsCollection->addQuantityFilter();
@@ -855,8 +859,9 @@ public function getSelectionsByIds($selectionIds, $product)
855859

856860
if (!$usedSelections || $usedSelectionsIds !== $selectionIds) {
857861
$storeId = $product->getStoreId();
858-
$usedSelections = $this->_bundleCollection
859-
->create()
862+
/** @var Selections $usedSelections */
863+
$usedSelections = $this->_bundleCollection->create();
864+
$usedSelections
860865
->addAttributeToSelect('*')
861866
->setFlag('product_children', true)
862867
->addStoreFilter($this->getStoreFilter($product))

‎app/code/Magento/Bundle/Model/ResourceModel/Selection/Collection.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,21 +163,33 @@ public function setPositionOrder()
163163
}
164164

165165
/**
166-
* Add filtering of product then havent enoght stock
166+
* Add filtering of products that have 0 items left.
167167
*
168168
* @return $this
169169
* @since 100.2.0
170170
*/
171171
public function addQuantityFilter()
172172
{
173+
$stockItemTable = $this->getTable('cataloginventory_stock_item');
174+
$stockStatusTable = $this->getTable('cataloginventory_stock_status');
173175
$this->getSelect()
174176
->joinInner(
175-
['stock' => $this->getTable('cataloginventory_stock_status')],
177+
['stock' => $stockStatusTable],
176178
'selection.product_id = stock.product_id',
177179
[]
180+
)->joinInner(
181+
['stock_item' => $stockItemTable],
182+
'selection.product_id = stock_item.product_id',
183+
[]
178184
)
179185
->where(
180-
'(selection.selection_can_change_qty or selection.selection_qty <= stock.qty) and stock.stock_status'
186+
'('
187+
. 'selection.selection_can_change_qty'
188+
. ' or '
189+
. 'selection.selection_qty <= stock.qty'
190+
. ' or '
191+
.'stock_item.manage_stock = 0'
192+
. ') and stock.stock_status = 1'
181193
);
182194
return $this;
183195
}

‎app/code/Magento/Bundle/Test/Unit/Model/ResourceModel/Selection/CollectionTest.php‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,17 +111,38 @@ protected function setUp()
111111

112112
public function testAddQuantityFilter()
113113
{
114-
$tableName = 'cataloginventory_stock_status';
115-
$this->entity->expects($this->once())
114+
$statusTableName = 'cataloginventory_stock_status';
115+
$itemTableName = 'cataloginventory_stock_item';
116+
$this->entity->expects($this->exactly(2))
116117
->method('getTable')
117-
->willReturn($tableName);
118-
$this->select->expects($this->once())
118+
->willReturnOnConsecutiveCalls($itemTableName, $statusTableName);
119+
$this->select->expects($this->exactly(2))
119120
->method('joinInner')
120-
->with(
121-
['stock' => $tableName],
122-
'selection.product_id = stock.product_id',
123-
[]
121+
->withConsecutive(
122+
[
123+
['stock' => $statusTableName],
124+
'selection.product_id = stock.product_id',
125+
[]
126+
],
127+
[
128+
['stock_item' => $itemTableName],
129+
'selection.product_id = stock_item.product_id',
130+
[]
131+
]
124132
)->willReturnSelf();
133+
$this->select
134+
->expects($this->once())
135+
->method('where')
136+
->with(
137+
'('
138+
. 'selection.selection_can_change_qty'
139+
. ' or '
140+
. 'selection.selection_qty <= stock.qty'
141+
. ' or '
142+
.'stock_item.manage_stock = 0'
143+
. ') and stock.stock_status = 1'
144+
)
145+
->willReturnSelf();
125146
$this->assertEquals($this->model, $this->model->addQuantityFilter());
126147
}
127148
}

‎app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\Framework\App\ObjectManager;
1313
use Magento\Framework\Pricing\PriceCurrencyInterface;
1414
use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader;
15+
use Magento\CatalogRule\Model\Indexer\IndexerTableSwapperInterface as TableSwapper;
1516

1617
/**
1718
* @api
@@ -132,9 +133,9 @@ class IndexBuilder
132133
private $pricesPersistor;
133134

134135
/**
135-
* @var \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher
136+
* @var TableSwapper
136137
*/
137-
private $activeTableSwitcher;
138+
private $tableSwapper;
138139

139140
/**
140141
* @var ProductLoader
@@ -160,7 +161,9 @@ class IndexBuilder
160161
* @param RuleProductPricesPersistor|null $pricesPersistor
161162
* @param \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher|null $activeTableSwitcher
162163
* @param ProductLoader|null $productLoader
164+
* @param TableSwapper|null $tableSwapper
163165
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
166+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
164167
*/
165168
public function __construct(
166169
RuleCollectionFactory $ruleCollectionFactory,
@@ -180,7 +183,8 @@ public function __construct(
180183
ReindexRuleProductPrice $reindexRuleProductPrice = null,
181184
RuleProductPricesPersistor $pricesPersistor = null,
182185
\Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null,
183-
ProductLoader $productLoader = null
186+
ProductLoader $productLoader = null,
187+
TableSwapper $tableSwapper = null
184188
) {
185189
$this->resource = $resource;
186190
$this->connection = $resource->getConnection();
@@ -212,12 +216,11 @@ public function __construct(
212216
$this->pricesPersistor = $pricesPersistor ?? ObjectManager::getInstance()->get(
213217
RuleProductPricesPersistor::class
214218
);
215-
$this->activeTableSwitcher = $activeTableSwitcher ?? ObjectManager::getInstance()->get(
216-
\Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher::class
217-
);
218219
$this->productLoader = $productLoader ?? ObjectManager::getInstance()->get(
219220
ProductLoader::class
220221
);
222+
$this->tableSwapper = $tableSwapper ??
223+
ObjectManager::getInstance()->get(TableSwapper::class);
221224
}
222225

223226
/**
@@ -296,22 +299,14 @@ public function reindexFull()
296299
*/
297300
protected function doReindexFull()
298301
{
299-
$this->connection->truncateTable(
300-
$this->getTable($this->activeTableSwitcher->getAdditionalTableName('catalogrule_product'))
301-
);
302-
$this->connection->truncateTable(
303-
$this->getTable($this->activeTableSwitcher->getAdditionalTableName('catalogrule_product_price'))
304-
);
305-
306302
foreach ($this->getAllRules() as $rule) {
307303
$this->reindexRuleProduct->execute($rule, $this->batchCount, true);
308304
}
309305

310306
$this->reindexRuleProductPrice->execute($this->batchCount, null, true);
311307
$this->reindexRuleGroupWebsite->execute(true);
312308

313-
$this->activeTableSwitcher->switchTable(
314-
$this->connection,
309+
$this->tableSwapper->swapIndexTables(
315310
[
316311
$this->getTable('catalogrule_product'),
317312
$this->getTable('catalogrule_product_price'),

‎app/code/Magento/CatalogRule/Model/Indexer/IndexerTableSwapper.php‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\CatalogRule\Model\Indexer;
8+
9+
use Magento\Framework\App\ResourceConnection;
10+
11+
/**
12+
* @inheritDoc
13+
*/
14+
class IndexerTableSwapper implements IndexerTableSwapperInterface
15+
{
16+
/**
17+
* Keys are original tables' names, values - created temporary tables.
18+
*
19+
* @var string[]
20+
*/
21+
private $temporaryTables = [];
22+
23+
/**
24+
* @var ResourceConnection
25+
*/
26+
private $resourceConnection;
27+
28+
/**
29+
* @param ResourceConnection $resource
30+
*/
31+
public function __construct(ResourceConnection $resource)
32+
{
33+
$this->resourceConnection = $resource;
34+
}
35+
36+
/**
37+
* Create temporary table based on given table to use instead of original.
38+
*
39+
* @param string $originalTableName
40+
*
41+
* @return string Created table name.
42+
* @throws \Throwable
43+
*/
44+
private function createTemporaryTable(string $originalTableName): string
45+
{
46+
$temporaryTableName = $this->resourceConnection->getTableName(
47+
$originalTableName . '__temp' . $this->generateRandomSuffix()
48+
);
49+
50+
$this->resourceConnection->getConnection()->query(
51+
sprintf(
52+
'create table %s like %s',
53+
$temporaryTableName,
54+
$this->resourceConnection->getTableName($originalTableName)
55+
)
56+
);
57+
58+
return $temporaryTableName;
59+
}
60+
61+
/**
62+
* Random suffix for temporary tables not to conflict with each other.
63+
*
64+
* @return string
65+
*/
66+
private function generateRandomSuffix(): string
67+
{
68+
return bin2hex(random_bytes(4));
69+
}
70+
71+
/**
72+
* @inheritDoc
73+
*/
74+
public function getWorkingTableName(string $originalTable): string
75+
{
76+
$originalTable = $this->resourceConnection->getTableName($originalTable);
77+
if (!array_key_exists($originalTable, $this->temporaryTables)) {
78+
$this->temporaryTables[$originalTable]
79+
= $this->createTemporaryTable($originalTable);
80+
}
81+
82+
return $this->temporaryTables[$originalTable];
83+
}
84+
85+
/**
86+
* @inheritDoc
87+
*/
88+
public function swapIndexTables(array $originalTablesNames)
89+
{
90+
$toRename = [];
91+
/** @var string[] $toDrop */
92+
$toDrop = [];
93+
/** @var string[] $temporaryTablesRenamed */
94+
$temporaryTablesRenamed = [];
95+
//Renaming temporary tables to original tables' names, dropping old
96+
//tables.
97+
foreach ($originalTablesNames as $tableName) {
98+
$tableName = $this->resourceConnection->getTableName($tableName);
99+
$temporaryOriginalName = $this->resourceConnection->getTableName(
100+
$tableName . $this->generateRandomSuffix()
101+
);
102+
$temporaryTableName = $this->getWorkingTableName($tableName);
103+
$toRename[] = [
104+
'oldName' => $tableName,
105+
'newName' => $temporaryOriginalName
106+
];
107+
$toRename[] = [
108+
'oldName' => $temporaryTableName,
109+
'newName' => $tableName
110+
];
111+
$toDrop[] = $temporaryOriginalName;
112+
$temporaryTablesRenamed[] = $tableName;
113+
}
114+
115+
//Swapping tables.
116+
$this->resourceConnection->getConnection()->renameTablesBatch($toRename);
117+
//Cleaning up.
118+
foreach ($temporaryTablesRenamed as $tableName) {
119+
unset($this->temporaryTables[$tableName]);
120+
}
121+
//Removing old ones.
122+
foreach ($toDrop as $tableName) {
123+
$this->resourceConnection->getConnection()->dropTable($tableName);
124+
}
125+
}
126+
}

‎app/code/Magento/CatalogRule/Model/Indexer/IndexerTableSwapperInterface.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\CatalogRule\Model\Indexer;
8+
9+
/**
10+
* Manage additional tables used while building new index to preserve
11+
* index tables until the process finishes.
12+
*/
13+
interface IndexerTableSwapperInterface
14+
{
15+
/**
16+
* Get working table name used to build index.
17+
*
18+
* @param string $originalTable
19+
*
20+
* @return string
21+
*/
22+
public function getWorkingTableName(string $originalTable): string;
23+
24+
/**
25+
* Swap working tables with actual tables to save new indexes.
26+
*
27+
* @param string[] $originalTablesNames
28+
*
29+
* @return void
30+
*/
31+
public function swapIndexTables(array $originalTablesNames);
32+
}

0 commit comments

Comments
 (0)