ENGCOM-5173: Improve command catalog:images:resize #23005

VladimirZaets · web-flow · commit ee095ec6d969 · 2019-06-10T20:50:30.000-05:00
diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Product/Image.php b/app/code/Magento/Catalog/Model/ResourceModel/Product/Image.php
@@ -54,7 +54,7 @@ public function __construct(
     }
 
     /**
-     * Returns product images
+     * Get all product images.
      *
      * @return \Generator
      */
@@ -75,7 +75,28 @@ public function getAllProductImages(): \Generator
     }
 
     /**
-     * Get the number of unique pictures of products
+     * Get used product images.
+     *
+     * @return \Generator
+     */
+    public function getUsedProductImages(): \Generator
+    {
+        $batchSelectIterator = $this->batchQueryGenerator->generate(
+            'value_id',
+            $this->getUsedImagesSelect(),
+            $this->batchSize,
+            \Magento\Framework\DB\Query\BatchIteratorInterface::NON_UNIQUE_FIELD_ITERATOR
+        );
+
+        foreach ($batchSelectIterator as $select) {
+            foreach ($this->connection->fetchAll($select) as $key => $value) {
+                yield $key => $value;
+            }
+        }
+    }
+
+    /**
+     * Get the number of unique images of products.
      *
      * @return int
      */
@@ -92,7 +113,24 @@ public function getCountAllProductImages(): int
     }
 
     /**
-     * Return Select to fetch all products images
+     * Get the number of unique and used images of products.
+     *
+     * @return int
+     */
+    public function getCountUsedProductImages(): int
+    {
+        $select = $this->getUsedImagesSelect()
+            ->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
      */
@@ -106,4 +144,23 @@ private function getVisibleImagesSelect(): Select
                 'disabled = 0'
             );
     }
+
+    /**
+     * Return select to fetch all used product images.
+     *
+     * @return Select
+     */
+    private function getUsedImagesSelect(): Select
+    {
+        return $this->connection->select()->distinct()
+            ->from(
+                ['images' => $this->resourceConnection->getTableName(Gallery::GALLERY_TABLE)],
+                'value as filepath'
+            )->joinInner(
+                ['image_value' => $this->resourceConnection->getTableName(Gallery::GALLERY_VALUE_TABLE)],
+                'images.value_id = image_value.value_id'
+            )->where(
+                'images.disabled = 0 AND image_value.disabled = 0'
+            );
+    }
 }
diff --git a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/ImageTest.php
@@ -16,6 +16,10 @@
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 use Magento\Framework\DB\Query\BatchIteratorInterface;
 
+/**
+ * Class ImageTest
+ * @package Magento\Catalog\Test\Unit\Model\ResourceModel\Product
+ */
 class ImageTest extends \PHPUnit\Framework\TestCase
 {
     /**
@@ -76,6 +80,37 @@ protected function getVisibleImagesSelectMock(): MockObject
         return $selectMock;
     }
 
+    /**
+     * @return MockObject
+     */
+    protected function getUsedImagesSelectMock(): 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('joinInner')
+            ->with(
+                ['image_value' => Gallery::GALLERY_VALUE_TABLE],
+                'images.value_id = image_value.value_id'
+            )->willReturnSelf();
+        $selectMock->expects($this->once())
+            ->method('where')
+            ->with('images.disabled = 0 AND image_value.disabled = 0')
+            ->willReturnSelf();
+
+        return $selectMock;
+    }
+
     /**
      * @param int $imagesCount
      * @dataProvider dataProvider
@@ -116,15 +151,53 @@ public function testGetCountAllProductImages(int $imagesCount): void
         );
     }
 
+    /**
+     * @param int $imagesCount
+     * @dataProvider dataProvider
+     */
+    public function testGetCountUsedProductImages(int $imagesCount): void
+    {
+        $selectMock = $this->getUsedImagesSelectMock();
+        $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->getCountUsedProductImages()
+        );
+    }
+
     /**
      * @param int $imagesCount
      * @param int $batchSize
      * @dataProvider dataProvider
      */
-    public function testGetAllProductImages(
-        int $imagesCount,
-        int $batchSize
-    ): void {
+    public function testGetAllProductImages(int $imagesCount, int $batchSize): void
+    {
         $this->connectionMock->expects($this->once())
             ->method('select')
             ->willReturn($this->getVisibleImagesSelectMock());
@@ -165,6 +238,54 @@ public function testGetAllProductImages(
         $this->assertCount($imagesCount, $imageModel->getAllProductImages());
     }
 
+    /**
+     * @param int $imagesCount
+     * @param int $batchSize
+     * @dataProvider dataProvider
+     */
+    public function testGetUsedProductImages(int $imagesCount, int $batchSize): void
+    {
+        $this->connectionMock->expects($this->once())
+            ->method('select')
+            ->willReturn($this->getUsedImagesSelectMock());
+
+        $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)
+                )
+            );
+
+        /** @var Image $imageModel */
+        $imageModel = $this->objectManager->getObject(
+            Image::class,
+            [
+                'generator' => $this->generatorMock,
+                'resourceConnection' => $this->resourceMock,
+                'batchSize' => $batchSize
+            ]
+        );
+
+        $this->assertCount($imagesCount, $imageModel->getUsedProductImages());
+    }
+
     /**
      * @param int $imagesCount
      * @param int $batchSize
diff --git a/app/code/Magento/MediaStorage/Console/Command/ImagesResizeCommand.php b/app/code/Magento/MediaStorage/Console/Command/ImagesResizeCommand.php
@@ -8,13 +8,20 @@
 namespace Magento\MediaStorage\Console\Command;
 
 use Magento\Framework\App\Area;
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\State;
+use Magento\Framework\ObjectManagerInterface;
 use Magento\MediaStorage\Service\ImageResize;
+use Symfony\Component\Console\Helper\ProgressBar;
+use Symfony\Component\Console\Helper\ProgressBarFactory;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\Console\Helper\ProgressBar;
-use Magento\Framework\ObjectManagerInterface;
 
+/**
+ * Resizes product images according to theme view definitions.
+ *
+ * @package Magento\MediaStorage\Console\Command
+ */
 class ImagesResizeCommand extends \Symfony\Component\Console\Command\Command
 {
     /**
@@ -28,28 +35,32 @@ class ImagesResizeCommand extends \Symfony\Component\Console\Command\Command
     private $appState;
 
     /**
-     * @var ObjectManagerInterface
+     * @var ProgressBarFactory
      */
-    private $objectManager;
+    private $progressBarFactory;
 
     /**
      * @param State $appState
      * @param ImageResize $resize
      * @param ObjectManagerInterface $objectManager
+     * @param ProgressBarFactory $progressBarFactory
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
     public function __construct(
         State $appState,
         ImageResize $resize,
-        ObjectManagerInterface $objectManager
+        ObjectManagerInterface $objectManager,
+        ProgressBarFactory $progressBarFactory = null
     ) {
         parent::__construct();
         $this->resize = $resize;
         $this->appState = $appState;
-        $this->objectManager = $objectManager;
+        $this->progressBarFactory = $progressBarFactory
+            ?: ObjectManager::getInstance()->get(ProgressBarFactory::class);
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      */
     protected function configure()
     {
@@ -58,7 +69,9 @@ protected function configure()
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
+     * @param InputInterface $input
+     * @param OutputInterface $output
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
@@ -67,10 +80,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $generator = $this->resize->resizeFromThemes();
 
             /** @var ProgressBar $progress */
-            $progress = $this->objectManager->create(ProgressBar::class, [
-                'output' => $output,
-                'max' => $generator->current()
-            ]);
+            $progress = $this->progressBarFactory->create(
+                [
+                    'output' => $output,
+                    'max' => $generator->current()
+                ]
+            );
             $progress->setFormat(
                 "%current%/%max% [%bar%] %percent:3s%% %elapsed% %memory:6s% \t| <info>%message%</info>"
             );
@@ -79,9 +94,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
                 $progress->setOverwrite(false);
             }
 
-            for (; $generator->valid(); $generator->next()) {
+            while ($generator->valid()) {
                 $progress->setMessage($generator->key());
                 $progress->advance();
+                $generator->next();
             }
         } catch (\Exception $e) {
             $output->writeln("<error>{$e->getMessage()}</error>");
@@ -91,5 +107,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $output->write(PHP_EOL);
         $output->writeln("<info>Product images resized successfully</info>");
+
+        return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
     }
 }
diff --git a/app/code/Magento/MediaStorage/Service/ImageResize.php b/app/code/Magento/MediaStorage/Service/ImageResize.php
@@ -152,12 +152,12 @@ public function resizeFromImageName(string $originalImageName)
      */
     public function resizeFromThemes(array $themes = null): \Generator
     {
-        $count = $this->productImage->getCountAllProductImages();
+        $count = $this->productImage->getCountUsedProductImages();
         if (!$count) {
             throw new NotFoundException(__('Cannot resize images - product images not found'));
         }
 
-        $productImages = $this->productImage->getAllProductImages();
+        $productImages = $this->productImage->getUsedProductImages();
         $viewImages = $this->getViewImages($themes ?? $this->getThemesInUse());
 
         foreach ($productImages as $image) {
@@ -202,10 +202,12 @@ private function getViewImages(array $themes): array
         $viewImages = [];
         /** @var \Magento\Theme\Model\Theme $theme */
         foreach ($themes as $theme) {
-            $config = $this->viewConfig->getViewConfig([
-                'area' => Area::AREA_FRONTEND,
-                'themeModel' => $theme,
-            ]);
+            $config = $this->viewConfig->getViewConfig(
+                [
+                    'area' => Area::AREA_FRONTEND,
+                    'themeModel' => $theme,
+                ]
+            );
             $images = $config->getMediaEntities('Magento_Catalog', ImageHelper::MEDIA_TYPE_CONFIG_NODE);
             foreach ($images as $imageId => $imageData) {
                 $uniqIndex = $this->getUniqueImageIndex($imageData);
@@ -226,6 +228,7 @@ private function getUniqueImageIndex(array $imageData): string
     {
         ksort($imageData);
         unset($imageData['type']);
+        // phpcs:disable Magento2.Security.InsecureFunction
         return md5(json_encode($imageData));
     }
 
