Skip to content

Commit bd439cd

Browse files
sidolovsidolov
authored
ENGCOM-4004: [Backport] catalog:images:resize total images count calculates incorrectly #18387 #18809 
2 parents 818d1a1 + bf25c7e commit bd439cd

File tree

4 files changed

+251
-5
lines changed

4 files changed

+251
-5
lines changed

‎app/code/Magento/Catalog/Model/ResourceModel/Product/Image.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use Magento\Framework\App\ResourceConnection;
1414

1515
/**
16-
* Class for fast retrieval of all product images
16+
* Class for retrieval of all product images
1717
*/
1818
class Image
1919
{
@@ -76,15 +76,24 @@ public function getAllProductImages(): \Generator
7676

7777
/**
7878
* Get the number of unique pictures of products
79+
*
7980
* @return int
8081
*/
8182
public function getCountAllProductImages(): int
8283
{
83-
$select = $this->getVisibleImagesSelect()->reset('columns')->columns('count(*)');
84+
$select = $this->getVisibleImagesSelect()
85+
->reset('columns')
86+
->reset('distinct')
87+
->columns(
88+
new \Zend_Db_Expr('count(distinct value)')
89+
);
90+
8491
return (int) $this->connection->fetchOne($select);
8592
}
8693

8794
/**
95+
* Return Select to fetch all products images
96+
*
8897
* @return Select
8998
*/
9099
private function getVisibleImagesSelect(): Select

‎app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/ImageTest.php

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Catalog\Test\Unit\Model\ResourceModel\Product;
9+
10+
use Magento\Catalog\Model\ResourceModel\Product\Image;
11+
use Magento\Framework\DB\Adapter\AdapterInterface;
12+
use Magento\Framework\DB\Query\Generator;
13+
use Magento\Framework\DB\Select;
14+
use Magento\Framework\App\ResourceConnection;
15+
use Magento\Catalog\Model\ResourceModel\Product\Gallery;
16+
use PHPUnit_Framework_MockObject_MockObject as MockObject;
17+
use Magento\Framework\DB\Query\BatchIteratorInterface;
18+
19+
class ImageTest extends \PHPUnit\Framework\TestCase
20+
{
21+
/**
22+
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
23+
*/
24+
protected $objectManager;
25+
26+
/**
27+
* @var AdapterInterface | MockObject
28+
*/
29+
protected $connectionMock;
30+
31+
/**
32+
* @var Generator | MockObject
33+
*/
34+
protected $generatorMock;
35+
36+
/**
37+
* @var ResourceConnection | MockObject
38+
*/
39+
protected $resourceMock;
40+
41+
protected function setUp()
42+
{
43+
$this->objectManager =
44+
new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
45+
$this->connectionMock = $this->createMock(AdapterInterface::class);
46+
$this->resourceMock = $this->createMock(ResourceConnection::class);
47+
$this->resourceMock->method('getConnection')
48+
->willReturn($this->connectionMock);
49+
$this->resourceMock->method('getTableName')
50+
->willReturnArgument(0);
51+
$this->generatorMock = $this->createMock(Generator::class);
52+
}
53+
54+
/**
55+
* @return MockObject
56+
*/
57+
protected function getVisibleImagesSelectMock(): MockObject
58+
{
59+
$selectMock = $this->getMockBuilder(Select::class)
60+
->disableOriginalConstructor()
61+
->getMock();
62+
$selectMock->expects($this->once())
63+
->method('distinct')
64+
->willReturnSelf();
65+
$selectMock->expects($this->once())
66+
->method('from')
67+
->with(
68+
['images' => Gallery::GALLERY_TABLE],
69+
'value as filepath'
70+
)->willReturnSelf();
71+
$selectMock->expects($this->once())
72+
->method('where')
73+
->with('disabled = 0')
74+
->willReturnSelf();
75+
76+
return $selectMock;
77+
}
78+
79+
/**
80+
* @param int $imagesCount
81+
* @dataProvider dataProvider
82+
*/
83+
public function testGetCountAllProductImages(int $imagesCount)
84+
{
85+
$selectMock = $this->getVisibleImagesSelectMock();
86+
$selectMock->expects($this->exactly(2))
87+
->method('reset')
88+
->withConsecutive(
89+
['columns'],
90+
['distinct']
91+
)->willReturnSelf();
92+
$selectMock->expects($this->once())
93+
->method('columns')
94+
->with(new \Zend_Db_Expr('count(distinct value)'))
95+
->willReturnSelf();
96+
97+
$this->connectionMock->expects($this->once())
98+
->method('select')
99+
->willReturn($selectMock);
100+
$this->connectionMock->expects($this->once())
101+
->method('fetchOne')
102+
->with($selectMock)
103+
->willReturn($imagesCount);
104+
105+
$imageModel = $this->objectManager->getObject(
106+
Image::class,
107+
[
108+
'generator' => $this->generatorMock,
109+
'resourceConnection' => $this->resourceMock
110+
]
111+
);
112+
113+
$this->assertSame(
114+
$imagesCount,
115+
$imageModel->getCountAllProductImages()
116+
);
117+
}
118+
119+
/**
120+
* @param int $imagesCount
121+
* @param int $batchSize
122+
* @dataProvider dataProvider
123+
*/
124+
public function testGetAllProductImages(
125+
int $imagesCount,
126+
int $batchSize
127+
) {
128+
$this->connectionMock->expects($this->once())
129+
->method('select')
130+
->willReturn($this->getVisibleImagesSelectMock());
131+
132+
$batchCount = (int)ceil($imagesCount / $batchSize);
133+
$fetchResultsCallback = $this->getFetchResultCallbackForBatches($imagesCount, $batchSize);
134+
$this->connectionMock->expects($this->exactly($batchCount))
135+
->method('fetchAll')
136+
->will($this->returnCallback($fetchResultsCallback));
137+
138+
/** @var Select | MockObject $selectMock */
139+
$selectMock = $this->getMockBuilder(Select::class)
140+
->disableOriginalConstructor()
141+
->getMock();
142+
143+
$this->generatorMock->expects($this->once())
144+
->method('generate')
145+
->with(
146+
'value_id',
147+
$selectMock,
148+
$batchSize,
149+
BatchIteratorInterface::NON_UNIQUE_FIELD_ITERATOR
150+
)->will(
151+
$this->returnCallback(
152+
$this->getBatchIteratorCallback($selectMock, $batchCount)
153+
)
154+
);
155+
156+
$imageModel = $this->objectManager->getObject(
157+
Image::class,
158+
[
159+
'generator' => $this->generatorMock,
160+
'resourceConnection' => $this->resourceMock,
161+
'batchSize' => $batchSize
162+
]
163+
);
164+
165+
$this->assertCount($imagesCount, $imageModel->getAllProductImages());
166+
}
167+
168+
/**
169+
* @param int $imagesCount
170+
* @param int $batchSize
171+
* @return \Closure
172+
*/
173+
protected function getFetchResultCallbackForBatches(
174+
int $imagesCount,
175+
int $batchSize
176+
): \Closure {
177+
$fetchResultsCallback = function () use (&$imagesCount, $batchSize) {
178+
$batchSize =
179+
($imagesCount >= $batchSize) ? $batchSize : $imagesCount;
180+
$imagesCount -= $batchSize;
181+
182+
$getFetchResults = function ($batchSize): array {
183+
$result = [];
184+
$count = $batchSize;
185+
while ($count) {
186+
$count--;
187+
$result[$count] = $count;
188+
}
189+
190+
return $result;
191+
};
192+
193+
return $getFetchResults($batchSize);
194+
};
195+
196+
return $fetchResultsCallback;
197+
}
198+
199+
/**
200+
* @param Select | MockObject $selectMock
201+
* @param int $batchCount
202+
* @return \Closure
203+
*/
204+
protected function getBatchIteratorCallback(
205+
MockObject $selectMock,
206+
int $batchCount
207+
): \Closure {
208+
$iteratorCallback = function () use ($batchCount, $selectMock): array {
209+
$result = [];
210+
$count = $batchCount;
211+
while ($count) {
212+
$count--;
213+
$result[$count] = $selectMock;
214+
}
215+
216+
return $result;
217+
};
218+
219+
return $iteratorCallback;
220+
}
221+
222+
/**
223+
* Data Provider
224+
* @return array
225+
*/
226+
public function dataProvider(): array
227+
{
228+
return [
229+
[300, 300],
230+
[300, 100],
231+
[139, 100],
232+
[67, 10],
233+
[154, 47],
234+
[0, 100]
235+
];
236+
}
237+
}

‎lib/internal/Magento/Framework/DB/Query/BatchRangeIterator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function __construct(
107107
public function current()
108108
{
109109
if (null === $this->currentSelect) {
110-
$this->isValid = ($this->currentOffset + $this->batchSize) <= $this->totalItemCount;
110+
$this->isValid = $this->currentOffset < $this->totalItemCount;
111111
$this->currentSelect = $this->initSelectObject();
112112
}
113113
return $this->currentSelect;
@@ -138,7 +138,7 @@ public function next()
138138
if (null === $this->currentSelect) {
139139
$this->current();
140140
}
141-
$this->isValid = ($this->batchSize + $this->currentOffset) <= $this->totalItemCount;
141+
$this->isValid = $this->currentOffset < $this->totalItemCount;
142142
$select = $this->initSelectObject();
143143
if ($this->isValid) {
144144
$this->iteration++;

‎lib/internal/Magento/Framework/Test/Unit/DB/Query/BatchRangeIteratorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,6 @@ public function testIterations()
116116
$iterations++;
117117
}
118118

119-
$this->assertEquals(10, $iterations);
119+
$this->assertEquals(11, $iterations);
120120
}
121121
}

0 commit comments

Comments
 (0)