Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport] catalog:images:resize total images count calculates incorrectly #18387 #18809

13 changes: 11 additions & 2 deletions app/code/Magento/Catalog/Model/ResourceModel/Product/Image.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use Magento\Framework\App\ResourceConnection;

/**
* Class for fast retrieval of all product images
* Class for retrieval of all product images
*/
class Image
{
Expand Down Expand Up @@ -76,15 +76,24 @@ public function getAllProductImages(): \Generator

/**
* Get the number of unique pictures of products
*
* @return int
*/
public function getCountAllProductImages(): int
{
$select = $this->getVisibleImagesSelect()->reset('columns')->columns('count(*)');
$select = $this->getVisibleImagesSelect()
->reset('columns')
->reset('distinct')
->columns(
new \Zend_Db_Expr('count(distinct value)')
);

return (int) $this->connection->fetchOne($select);
}

/**
* Return Select to fetch all products images
*
* @return Select
*/
private function getVisibleImagesSelect(): Select
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Catalog\Test\Unit\Model\ResourceModel\Product;

use Magento\Catalog\Model\ResourceModel\Product\Image;
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\DB\Query\Generator;
use Magento\Framework\DB\Select;
use Magento\Framework\App\ResourceConnection;
use Magento\Catalog\Model\ResourceModel\Product\Gallery;
use PHPUnit_Framework_MockObject_MockObject as MockObject;
use Magento\Framework\DB\Query\BatchIteratorInterface;

class ImageTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
*/
protected $objectManager;

/**
* @var AdapterInterface | MockObject
*/
protected $connectionMock;

/**
* @var Generator | MockObject
*/
protected $generatorMock;

/**
* @var ResourceConnection | MockObject
*/
protected $resourceMock;

protected function setUp()
{
$this->objectManager =
new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$this->connectionMock = $this->createMock(AdapterInterface::class);
$this->resourceMock = $this->createMock(ResourceConnection::class);
$this->resourceMock->method('getConnection')
->willReturn($this->connectionMock);
$this->resourceMock->method('getTableName')
->willReturnArgument(0);
$this->generatorMock = $this->createMock(Generator::class);
}

/**
* @return MockObject
*/
protected function getVisibleImagesSelectMock(): MockObject
{
$selectMock = $this->getMockBuilder(Select::class)
->disableOriginalConstructor()
->getMock();
$selectMock->expects($this->once())
->method('distinct')
->willReturnSelf();
$selectMock->expects($this->once())
->method('from')
->with(
['images' => Gallery::GALLERY_TABLE],
'value as filepath'
)->willReturnSelf();
$selectMock->expects($this->once())
->method('where')
->with('disabled = 0')
->willReturnSelf();

return $selectMock;
}

/**
* @param int $imagesCount
* @dataProvider dataProvider
*/
public function testGetCountAllProductImages(int $imagesCount)
{
$selectMock = $this->getVisibleImagesSelectMock();
$selectMock->expects($this->exactly(2))
->method('reset')
->withConsecutive(
['columns'],
['distinct']
)->willReturnSelf();
$selectMock->expects($this->once())
->method('columns')
->with(new \Zend_Db_Expr('count(distinct value)'))
->willReturnSelf();

$this->connectionMock->expects($this->once())
->method('select')
->willReturn($selectMock);
$this->connectionMock->expects($this->once())
->method('fetchOne')
->with($selectMock)
->willReturn($imagesCount);

$imageModel = $this->objectManager->getObject(
Image::class,
[
'generator' => $this->generatorMock,
'resourceConnection' => $this->resourceMock
]
);

$this->assertSame(
$imagesCount,
$imageModel->getCountAllProductImages()
);
}

/**
* @param int $imagesCount
* @param int $batchSize
* @dataProvider dataProvider
*/
public function testGetAllProductImages(
int $imagesCount,
int $batchSize
) {
$this->connectionMock->expects($this->once())
->method('select')
->willReturn($this->getVisibleImagesSelectMock());

$batchCount = (int)ceil($imagesCount / $batchSize);
$fetchResultsCallback = $this->getFetchResultCallbackForBatches($imagesCount, $batchSize);
$this->connectionMock->expects($this->exactly($batchCount))
->method('fetchAll')
->will($this->returnCallback($fetchResultsCallback));

/** @var Select | MockObject $selectMock */
$selectMock = $this->getMockBuilder(Select::class)
->disableOriginalConstructor()
->getMock();

$this->generatorMock->expects($this->once())
->method('generate')
->with(
'value_id',
$selectMock,
$batchSize,
BatchIteratorInterface::NON_UNIQUE_FIELD_ITERATOR
)->will(
$this->returnCallback(
$this->getBatchIteratorCallback($selectMock, $batchCount)
)
);

$imageModel = $this->objectManager->getObject(
Image::class,
[
'generator' => $this->generatorMock,
'resourceConnection' => $this->resourceMock,
'batchSize' => $batchSize
]
);

$this->assertCount($imagesCount, $imageModel->getAllProductImages());
}

/**
* @param int $imagesCount
* @param int $batchSize
* @return \Closure
*/
protected function getFetchResultCallbackForBatches(
int $imagesCount,
int $batchSize
): \Closure {
$fetchResultsCallback = function () use (&$imagesCount, $batchSize) {
$batchSize =
($imagesCount >= $batchSize) ? $batchSize : $imagesCount;
$imagesCount -= $batchSize;

$getFetchResults = function ($batchSize): array {
$result = [];
$count = $batchSize;
while ($count) {
$count--;
$result[$count] = $count;
}

return $result;
};

return $getFetchResults($batchSize);
};

return $fetchResultsCallback;
}

/**
* @param Select | MockObject $selectMock
* @param int $batchCount
* @return \Closure
*/
protected function getBatchIteratorCallback(
MockObject $selectMock,
int $batchCount
): \Closure {
$iteratorCallback = function () use ($batchCount, $selectMock): array {
$result = [];
$count = $batchCount;
while ($count) {
$count--;
$result[$count] = $selectMock;
}

return $result;
};

return $iteratorCallback;
}

/**
* Data Provider
* @return array
*/
public function dataProvider(): array
{
return [
[300, 300],
[300, 100],
[139, 100],
[67, 10],
[154, 47],
[0, 100]
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function __construct(
public function current()
{
if (null === $this->currentSelect) {
$this->isValid = ($this->currentOffset + $this->batchSize) <= $this->totalItemCount;
$this->isValid = $this->currentOffset < $this->totalItemCount;
$this->currentSelect = $this->initSelectObject();
}
return $this->currentSelect;
Expand Down Expand Up @@ -138,7 +138,7 @@ public function next()
if (null === $this->currentSelect) {
$this->current();
}
$this->isValid = ($this->batchSize + $this->currentOffset) <= $this->totalItemCount;
$this->isValid = $this->currentOffset < $this->totalItemCount;
$select = $this->initSelectObject();
if ($this->isValid) {
$this->iteration++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,6 @@ public function testIterations()
$iterations++;
}

$this->assertEquals(10, $iterations);
$this->assertEquals(11, $iterations);
}
}