Merge remote-tracking branch 'magento-l3/ACP2E-315' into L3_PR_21-12-13

Author: veloraven

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

@@ -29,7 +29,7 @@ class Image extends \Magento\Framework\Model\AbstractModel
     /**
      * Config path for the jpeg image quality value
      */
-    const XML_PATH_JPEG_QUALITY = 'system/upload_configuration/jpeg_quality';
+    public const XML_PATH_JPEG_QUALITY = 'system/upload_configuration/jpeg_quality';
 
     /**
      * @var int
@@ -836,7 +836,37 @@ public function getWatermarkHeight()
     public function clearCache()
     {
         $directory = $this->_catalogProductMediaConfig->getBaseMediaPath() . '/cache';
-        $this->_mediaDirectory->delete($directory);
+        $directoryToDelete = $directory;
+        // Fixes issue when deleting cache directory at the same time that images are being
+        // lazy-loaded on storefront leading to new directories and files generation in the cache directory
+        // that would prevent deletion of the cache directory.
+        // RCA: the method delete() recursively enumerates and delete all subdirectories and files before deleting
+        // the target directory, which gives other processes time to create directories and files in the same directory.
+        // Solution: Rename the directory to simulate deletion and delete the destination directory afterward
+
+        try {
+            //generate name in format: \.[0-9A-ZA-z-_]{3} (e.g .QX3)
+            $tmpDirBasename = strrev(strtr(base64_encode(random_bytes(2)), '+/=', '-_.'));
+            $tmpDirectory = $this->_catalogProductMediaConfig->getBaseMediaPath() . '/' . $tmpDirBasename;
+            //delete temporary directory if exists
+            if ($this->_mediaDirectory->isDirectory($tmpDirectory)) {
+                $this->_mediaDirectory->delete($tmpDirectory);
+            }
+            //rename the directory to simulate deletion and delete the destination directory
+            if ($this->_mediaDirectory->isDirectory($directory) &&
+                true === $this->_mediaDirectory->getDriver()->rename(
+                    $this->_mediaDirectory->getAbsolutePath($directory),
+                    $this->_mediaDirectory->getAbsolutePath($tmpDirectory)
+                )
+            ) {
+                $directoryToDelete = $tmpDirectory;
+            }
+        } catch (\Throwable $exception) {
+            //ignore exceptions thrown during renaming
+            $directoryToDelete = $directory;
+        }
+
+        $this->_mediaDirectory->delete($directoryToDelete);
 
         $this->_coreFileStorageDatabase->deleteFolder($this->_mediaDirectory->getAbsolutePath($directory));
         $this->clearImageInfoFromCache();
@@ -870,6 +900,7 @@ protected function _fileExists($filename)
     public function getResizedImageInfo()
     {
         try {
+            $image = null;
             if ($this->isBaseFilePlaceholder() == true) {
                 $image = $this->imageAsset->getSourceFile();
             } else {
@@ -920,6 +951,7 @@ private function getImageSize($imagePath)
     {
         $imageInfo = $this->loadImageInfoFromCache($imagePath);
         if (!isset($imageInfo['size'])) {
+            // phpcs:ignore Magento2.Functions.DiscouragedFunction
             $imageSize = getimagesize($imagePath);
             $this->saveImageInfoToCache(['size' => $imageSize], $imagePath);
             return $imageSize;

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

@@ -17,6 +17,7 @@
 use Magento\Framework\Filesystem;
 use Magento\Framework\Filesystem\Directory\Write;
 use Magento\Framework\Filesystem\Directory\WriteInterface;
+use Magento\Framework\Filesystem\DriverInterface;
 use Magento\Framework\Image\Factory;
 use Magento\Framework\Model\Context;
 use Magento\Framework\Serialize\SerializerInterface;
@@ -140,7 +141,7 @@ protected function setUp(): void
 
         $this->mediaDirectory = $this->getMockBuilder(Write::class)
             ->disableOriginalConstructor()
-            ->onlyMethods(['create', 'isFile', 'isExist', 'getAbsolutePath'])
+            ->onlyMethods(['create', 'isFile', 'isExist', 'getAbsolutePath', 'isDirectory', 'getDriver', 'delete'])
             ->getMock();
 
         $this->filesystem = $this->createMock(Filesystem::class);
@@ -503,15 +504,56 @@ public function testIsCached(): void
     }
 
     /**
+     * @param bool $isRenameSuccessful
+     * @param string $expectedDirectoryToDelete
      * @return void
-     */
-    public function testClearCache(): void
-    {
+     * @throws \Magento\Framework\Exception\FileSystemException
+     * @dataProvider clearCacheDataProvider
+     */
+    public function testClearCache(
+        bool $isRenameSuccessful,
+        string $expectedDirectoryToDelete
+    ): void {
+        $driver = $this->createMock(DriverInterface::class);
+        $this->mediaDirectory->method('getAbsolutePath')
+            ->willReturnCallback(
+                function (string $path) {
+                    return 'path/to/media/' . $path;
+                }
+            );
+        $this->mediaDirectory->expects($this->exactly(2))
+            ->method('isDirectory')
+            ->willReturnOnConsecutiveCalls(false, true);
+        $this->mediaDirectory->expects($this->once())
+            ->method('getDriver')
+            ->willReturn($driver);
+        $driver->expects($this->once())
+            ->method('rename')
+            ->with(
+                'path/to/media/catalog/product/cache',
+                $this->matchesRegularExpression('/^path\/to\/media\/catalog\/product\/\.[0-9A-ZA-z-_]{3}$/')
+            )
+            ->willReturn($isRenameSuccessful);
+        $this->mediaDirectory->expects($this->once())
+            ->method('delete')
+            ->with($this->matchesRegularExpression($expectedDirectoryToDelete));
+
         $this->coreFileHelper->expects($this->once())->method('deleteFolder')->willReturn(true);
         $this->cacheManager->expects($this->once())->method('clean');
         $this->image->clearCache();
     }
 
+    /**
+     * @return array
+     */
+    public function clearCacheDataProvider(): array
+    {
+        return [
+            [true, '/^catalog\/product\/\.[0-9A-ZA-z-_]{3}$/'],
+            [false, '/^catalog\/product\/cache$/'],
+        ];
+    }
+
     /**
      * @return void
      */