Skip to content

Commit 56ec141

Browse files
Merge pull request #1572 from magento-tango/2.1-DEVELOP-PR
Bugs - MAGETWO-70287 [Backport] - Visual Merchandiser category edit performance issue - for 2.1 - MAGETWO-71587 [Backport to 2.1-develop] Redundant indexers invalidation - RIATCS-340 - MAGETWO-81736 Stabilization Jenkins on 2.1-develop
2 parents 2e18329 + 3504cff commit 56ec141

File tree

20 files changed

+504
-52
lines changed

20 files changed

+504
-52
lines changed

app/code/Magento/Catalog/Model/Category.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
* @method Category setUrlPath(string $urlPath)
2828
* @method Category getSkipDeleteChildren()
2929
* @method Category setSkipDeleteChildren(boolean $value)
30+
* @method Category setChangedProductIds(array $categoryIds) Set products ids that inserted or deleted for category
31+
* @method array getChangedProductIds() Get products ids that inserted or deleted for category
3032
*
3133
* @SuppressWarnings(PHPMD.LongVariable)
3234
* @SuppressWarnings(PHPMD.ExcessivePublicCount)
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
/**
3+
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Catalog\Model\Category\Product;
7+
8+
/**
9+
* Resolver to get product positions by ids assigned to specific category
10+
*/
11+
class PositionResolver extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb
12+
{
13+
/**
14+
* @param \Magento\Framework\Model\ResourceModel\Db\Context $context
15+
* @param string $connectionName
16+
*/
17+
public function __construct(
18+
\Magento\Framework\Model\ResourceModel\Db\Context $context,
19+
$connectionName = null
20+
) {
21+
parent::__construct($context, $connectionName);
22+
}
23+
24+
/**
25+
* Initialize resource model
26+
*
27+
* @return void
28+
*/
29+
protected function _construct()
30+
{
31+
$this->_init('catalog_product_entity', 'entity_id');
32+
}
33+
34+
/**
35+
* Get category product positions
36+
*
37+
* @param int $categoryId
38+
* @return array
39+
*/
40+
public function getPositions($categoryId)
41+
{
42+
$connection = $this->getConnection();
43+
44+
$select = $connection->select()->from(
45+
['cpe' => $this->getTable('catalog_product_entity')],
46+
'entity_id'
47+
)->joinLeft(
48+
['ccp' => $this->getTable('catalog_category_product')],
49+
'ccp.product_id=cpe.entity_id'
50+
)->where(
51+
'ccp.category_id = ?',
52+
$categoryId
53+
)->order(
54+
'ccp.position ' . \Magento\Framework\DB\Select::SQL_ASC
55+
);
56+
57+
return array_flip($connection->fetchCol($select));
58+
}
59+
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,18 @@ protected function _saveCategoryProducts($category)
409409
* Update product positions in category
410410
*/
411411
if (!empty($update)) {
412+
$newPositions = [];
412413
foreach ($update as $productId => $position) {
413-
$where = ['category_id = ?' => (int)$id, 'product_id = ?' => (int)$productId];
414-
$bind = ['position' => (int)$position];
414+
$delta = $position - $oldProducts[$productId];
415+
if (!isset($newPositions[$delta])) {
416+
$newPositions[$delta] = [];
417+
}
418+
$newPositions[$delta][] = $productId;
419+
}
420+
421+
foreach ($newPositions as $delta => $productIds) {
422+
$bind = ['position' => new \Zend_Db_Expr("position + ({$delta})")];
423+
$where = ['category_id = ?' => (int)$id, 'product_id IN (?)' => $productIds];
415424
$connection->update($this->getCategoryProductTable(), $bind, $where);
416425
}
417426
}
@@ -422,6 +431,8 @@ protected function _saveCategoryProducts($category)
422431
'catalog_category_change_products',
423432
['category' => $category, 'product_ids' => $productIds]
424433
);
434+
435+
$category->setChangedProductIds($productIds);
425436
}
426437

427438
if (!empty($insert) || !empty($update) || !empty($delete)) {
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php
2+
/**
3+
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Catalog\Test\Unit\Model\Category\Product;
7+
8+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
9+
use Magento\Catalog\Model\Category\Product\PositionResolver;
10+
use Magento\Framework\Model\ResourceModel\Db\Context;
11+
use Magento\Framework\App\ResourceConnection;
12+
use Magento\Framework\DB\Adapter\AdapterInterface;
13+
use Magento\Framework\DB\Select;
14+
15+
class PositionResolverTest extends \PHPUnit_Framework_TestCase
16+
{
17+
/**
18+
* @var Context|\PHPUnit_Framework_MockObject_MockObject
19+
*/
20+
private $context;
21+
22+
/**
23+
* @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
private $resources;
26+
27+
/**
28+
* @var AdapterInterface|\PHPUnit_Framework_MockObject_MockObject
29+
*/
30+
private $connection;
31+
32+
/**
33+
* @var Select|\PHPUnit_Framework_MockObject_MockObject
34+
*/
35+
private $select;
36+
37+
/**
38+
* @var PositionResolver
39+
*/
40+
private $model;
41+
42+
/**
43+
* @var array
44+
*/
45+
private $positions = [
46+
'3' => 100,
47+
'2' => 101,
48+
'1' => 102
49+
];
50+
51+
private $flippedPositions = [
52+
'100' => 3,
53+
'101' => 2,
54+
'102' => 1
55+
];
56+
57+
protected function setUp()
58+
{
59+
$this->context = $this->getMockBuilder(Context::class)
60+
->disableOriginalConstructor()
61+
->getMock();
62+
63+
$this->resources = $this->getMockBuilder(ResourceConnection::class)
64+
->disableOriginalConstructor()
65+
->getMock();
66+
67+
$this->connection = $this->getMockBuilder(AdapterInterface::class)
68+
->disableOriginalConstructor()
69+
->getMockForAbstractClass();
70+
71+
$this->select = $this->getMockBuilder(Select::class)
72+
->disableOriginalConstructor()
73+
->getMock();
74+
75+
$this->model = (new ObjectManager($this))->getObject(
76+
PositionResolver::class,
77+
[
78+
'context' => $this->context,
79+
null,
80+
'_resources' => $this->resources
81+
]
82+
);
83+
}
84+
85+
public function testGetPositions()
86+
{
87+
$this->resources->expects($this->once())
88+
->method('getConnection')
89+
->willReturn($this->connection);
90+
91+
$this->connection->expects($this->once())
92+
->method('select')
93+
->willReturn($this->select);
94+
$this->select->expects($this->once())
95+
->method('from')
96+
->willReturnSelf();
97+
$this->select->expects($this->once())
98+
->method('where')
99+
->willReturnSelf();
100+
$this->select->expects($this->once())
101+
->method('order')
102+
->willReturnSelf();
103+
$this->select->expects($this->once())
104+
->method('joinLeft')
105+
->willReturnSelf();
106+
$this->connection->expects($this->once())
107+
->method('fetchCol')
108+
->willReturn($this->positions);
109+
110+
$this->assertEquals($this->flippedPositions, $this->model->getPositions(1));
111+
}
112+
}

app/code/Magento/CatalogImportExport/Model/Import/Product.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2791,7 +2791,7 @@ private function formatBunchToStockDataRows(
27912791
$this->_connection->insertOnDuplicate($entityTable, array_values($stockData));
27922792
}
27932793

2794-
if ($productIdsToReindex && !$indexer->isScheduled()) {
2794+
if (!empty($productIdsToReindex) && !$indexer->isScheduled()) {
27952795
$indexer->reindexList($productIdsToReindex);
27962796
}
27972797
}

app/code/Magento/CatalogImportExport/Model/Indexer/Product/Flat/Plugin/Import.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,30 @@
55
*/
66
namespace Magento\CatalogImportExport\Model\Indexer\Product\Flat\Plugin;
77

8+
use Magento\Catalog\Model\Indexer\Product\Flat\State as FlatState;
9+
810
class Import
911
{
12+
/**
13+
* @var \Magento\Catalog\Model\Indexer\Product\Flat\State
14+
*/
15+
private $flatState;
16+
1017
/**
1118
* @var \Magento\Catalog\Model\Indexer\Product\Flat\Processor
1219
*/
1320
protected $_productFlatIndexerProcessor;
1421

1522
/**
1623
* @param \Magento\Catalog\Model\Indexer\Product\Flat\Processor $productFlatIndexerProcessor
24+
* @param \Magento\Catalog\Model\Indexer\Product\Flat\State $flatState
1725
*/
18-
public function __construct(\Magento\Catalog\Model\Indexer\Product\Flat\Processor $productFlatIndexerProcessor)
19-
{
26+
public function __construct(
27+
\Magento\Catalog\Model\Indexer\Product\Flat\Processor $productFlatIndexerProcessor,
28+
FlatState $flatState
29+
) {
2030
$this->_productFlatIndexerProcessor = $productFlatIndexerProcessor;
31+
$this->flatState = $flatState;
2132
}
2233

2334
/**
@@ -31,7 +42,10 @@ public function __construct(\Magento\Catalog\Model\Indexer\Product\Flat\Processo
3142
*/
3243
public function afterImportSource(\Magento\ImportExport\Model\Import $subject, $import)
3344
{
34-
$this->_productFlatIndexerProcessor->markIndexerAsInvalid();
45+
if ($this->flatState->isFlatEnabled() && !$this->_productFlatIndexerProcessor->isIndexerScheduled()) {
46+
$this->_productFlatIndexerProcessor->markIndexerAsInvalid();
47+
}
48+
3549
return $import;
3650
}
3751
}

app/code/Magento/CatalogImportExport/Test/Unit/Model/Indexer/Product/Flat/Plugin/ImportTest.php

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,89 @@
55
*/
66
namespace Magento\CatalogImportExport\Test\Unit\Model\Indexer\Product\Flat\Plugin;
77

8+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
9+
810
class ImportTest extends \PHPUnit_Framework_TestCase
911
{
10-
public function testAfterImportSource()
12+
/**
13+
* @var \Magento\Catalog\Model\Indexer\Product\Flat\Processor|\PHPUnit_Framework_MockObject_MockObject
14+
*/
15+
private $processorMock;
16+
17+
/**
18+
* @var \Magento\CatalogImportExport\Model\Indexer\Product\Flat\Plugin\Import
19+
*/
20+
private $model;
21+
22+
/**
23+
* @var \Magento\Catalog\Model\Indexer\Product\Flat\State|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
private $flatStateMock;
26+
27+
/**
28+
* @var \Magento\ImportExport\Model\Import|\PHPUnit_Framework_MockObject_MockObject
29+
*/
30+
private $subjectMock;
31+
32+
protected function setUp()
1133
{
12-
/**
13-
* @var \Magento\Catalog\Model\Indexer\Product\Flat\Processor|
14-
* \PHPUnit_Framework_MockObject_MockObject $processorMock
15-
*/
16-
$processorMock = $this->getMock(
34+
$this->processorMock = $this->getMock(
1735
'Magento\Catalog\Model\Indexer\Product\Flat\Processor',
18-
['markIndexerAsInvalid'],
36+
['markIndexerAsInvalid', 'isIndexerScheduled'],
37+
[],
38+
'',
39+
false
40+
);
41+
42+
$this->flatStateMock = $this->getMock(
43+
'\Magento\Catalog\Model\Indexer\Product\Flat\State',
44+
['isFlatEnabled'],
45+
[],
46+
'',
47+
false
48+
);
49+
50+
$this->subjectMock = $this->getMock(
51+
'Magento\ImportExport\Model\Import',
52+
[],
1953
[],
2054
'',
2155
false
2256
);
2357

24-
$subjectMock = $this->getMock('Magento\ImportExport\Model\Import', [], [], '', false);
25-
$processorMock->expects($this->once())->method('markIndexerAsInvalid');
58+
$this->model = (new ObjectManager($this))->getObject(
59+
'Magento\CatalogImportExport\Model\Indexer\Product\Flat\Plugin\Import',
60+
[
61+
'productFlatIndexerProcessor' => $this->processorMock,
62+
'flatState' => $this->flatStateMock
63+
]
64+
);
65+
}
2666

67+
public function testAfterImportSourceWithFlatEnabledAndIndexerScheduledDisabled()
68+
{
69+
$this->flatStateMock->expects($this->once())->method('isFlatEnabled')->willReturn(true);
70+
$this->processorMock->expects($this->once())->method('isIndexerScheduled')->willReturn(false);
71+
$this->processorMock->expects($this->once())->method('markIndexerAsInvalid');
2772
$someData = [1, 2, 3];
73+
$this->assertEquals($someData, $this->model->afterImportSource($this->subjectMock, $someData));
74+
}
2875

29-
$model = new \Magento\CatalogImportExport\Model\Indexer\Product\Flat\Plugin\Import($processorMock);
30-
$this->assertEquals($someData, $model->afterImportSource($subjectMock, $someData));
76+
public function testAfterImportSourceWithFlatDisabledAndIndexerScheduledDisabled()
77+
{
78+
79+
$this->flatStateMock->expects($this->once())->method('isFlatEnabled')->willReturn(false);
80+
$this->processorMock->expects($this->never())->method('isIndexerScheduled')->willReturn(false);
81+
$this->processorMock->expects($this->never())->method('markIndexerAsInvalid');
82+
$someData = [1, 2, 3];
83+
$this->assertEquals($someData, $this->model->afterImportSource($this->subjectMock, $someData));
84+
}
85+
public function testAfterImportSourceWithFlatEnabledAndIndexerScheduledEnabled()
86+
{
87+
$this->flatStateMock->expects($this->once())->method('isFlatEnabled')->willReturn(true);
88+
$this->processorMock->expects($this->once())->method('isIndexerScheduled')->willReturn(true);
89+
$this->processorMock->expects($this->never())->method('markIndexerAsInvalid');
90+
$someData = [1, 2, 3];
91+
$this->assertEquals($someData, $this->model->afterImportSource($this->subjectMock, $someData));
3192
}
3293
}

0 commit comments

Comments
 (0)