Skip to content

Commit 9dd923d

Browse files
committed
Merge branch 'ACP2E-2631' of https://github.com/magento-l3/magento2ce into PR-12-19-2023
2 parents 03f7ae1 + e60e910 commit 9dd923d

File tree

2 files changed

+36
-72
lines changed

2 files changed

+36
-72
lines changed

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

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Catalog\Model\View\Asset\PlaceholderFactory;
1212
use Magento\Framework\App\Filesystem\DirectoryList;
1313
use Magento\Framework\App\ObjectManager;
14+
use Magento\Framework\Exception\FileSystemException;
1415
use Magento\Framework\Image as MagentoImage;
1516
use Magento\Framework\Serialize\SerializerInterface;
1617

@@ -45,7 +46,8 @@ class Image extends \Magento\Framework\Model\AbstractModel
4546
* Default quality value (for JPEG images only).
4647
*
4748
* @var int
48-
* @deprecated 103.0.1 use config setting with path self::XML_PATH_JPEG_QUALITY
49+
* @deprecated 103.0.1
50+
* @see Use config setting with path self::XML_PATH_JPEG_QUALITY
4951
*/
5052
protected $_quality = null;
5153

@@ -101,7 +103,8 @@ class Image extends \Magento\Framework\Model\AbstractModel
101103

102104
/**
103105
* @var int
104-
* @deprecated unused
106+
* @deprecated
107+
* @see Not used anymore
105108
*/
106109
protected $_angle;
107110

@@ -307,7 +310,8 @@ public function getHeight()
307310
*
308311
* @param int $quality
309312
* @return $this
310-
* @deprecated 103.0.1 use config setting with path self::XML_PATH_JPEG_QUALITY
313+
* @deprecated 103.0.1
314+
* @see Use config setting with path self::XML_PATH_JPEG_QUALITY
311315
*/
312316
public function setQuality($quality)
313317
{
@@ -457,6 +461,7 @@ public function getBaseFile()
457461
* Get new file
458462
*
459463
* @deprecated 102.0.0
464+
* @see Image::getBaseFile
460465
* @return bool|string
461466
*/
462467
public function getNewFile()
@@ -836,38 +841,16 @@ public function getWatermarkHeight()
836841
public function clearCache()
837842
{
838843
$directory = $this->_catalogProductMediaConfig->getBaseMediaPath() . '/cache';
839-
$directoryToDelete = $directory;
840-
// Fixes issue when deleting cache directory at the same time that images are being
841-
// lazy-loaded on storefront leading to new directories and files generation in the cache directory
842-
// that would prevent deletion of the cache directory.
843-
// RCA: the method delete() recursively enumerates and delete all subdirectories and files before deleting
844-
// the target directory, which gives other processes time to create directories and files in the same directory.
845-
// Solution: Rename the directory to simulate deletion and delete the destination directory afterward
846844

845+
// If the directory cannot be deleted, it is likely because it is not empty anymore due to lazy loading from
846+
// the storefront triggering new cache file creation.
847+
// This is expected behavior and is not a cause for concern. Deletable files were deleted as expected.
847848
try {
848-
//generate name in format: \.[0-9A-ZA-z-_]{3} (e.g .QX3)
849-
$tmpDirBasename = strrev(strtr(base64_encode(random_bytes(2)), '+/=', '-_.'));
850-
$tmpDirectory = $this->_catalogProductMediaConfig->getBaseMediaPath() . '/' . $tmpDirBasename;
851-
//delete temporary directory if exists
852-
if ($this->_mediaDirectory->isDirectory($tmpDirectory)) {
853-
$this->_mediaDirectory->delete($tmpDirectory);
854-
}
855-
//rename the directory to simulate deletion and delete the destination directory
856-
if ($this->_mediaDirectory->isDirectory($directory) &&
857-
true === $this->_mediaDirectory->getDriver()->rename(
858-
$this->_mediaDirectory->getAbsolutePath($directory),
859-
$this->_mediaDirectory->getAbsolutePath($tmpDirectory)
860-
)
861-
) {
862-
$directoryToDelete = $tmpDirectory;
863-
}
864-
} catch (\Throwable $exception) {
865-
//ignore exceptions thrown during renaming
866-
$directoryToDelete = $directory;
849+
$this->_mediaDirectory->delete($directory);
850+
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch
851+
} catch (FileSystemException $e) {
867852
}
868853

869-
$this->_mediaDirectory->delete($directoryToDelete);
870-
871854
$this->_coreFileStorageDatabase->deleteFolder($this->_mediaDirectory->getAbsolutePath($directory));
872855
$this->clearImageInfoFromCache();
873856
}

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

Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
use Magento\Catalog\Model\View\Asset\PlaceholderFactory;
1515
use Magento\Framework\App\CacheInterface;
1616
use Magento\Framework\App\Filesystem\DirectoryList;
17+
use Magento\Framework\Exception\FileSystemException;
1718
use Magento\Framework\Filesystem;
1819
use Magento\Framework\Filesystem\Directory\Write;
1920
use Magento\Framework\Filesystem\Directory\WriteInterface;
20-
use Magento\Framework\Filesystem\DriverInterface;
2121
use Magento\Framework\Image\Factory;
2222
use Magento\Framework\Model\Context;
2323
use Magento\Framework\Serialize\SerializerInterface;
@@ -141,7 +141,7 @@ protected function setUp(): void
141141

142142
$this->mediaDirectory = $this->getMockBuilder(Write::class)
143143
->disableOriginalConstructor()
144-
->onlyMethods(['create', 'isFile', 'isExist', 'getAbsolutePath', 'isDirectory', 'getDriver', 'delete'])
144+
->onlyMethods(['create', 'isFile', 'isExist', 'getAbsolutePath', 'delete'])
145145
->getMock();
146146

147147
$this->filesystem = $this->createMock(Filesystem::class);
@@ -504,54 +504,35 @@ public function testIsCached(): void
504504
}
505505

506506
/**
507-
* @param bool $isRenameSuccessful
508-
* @param string $expectedDirectoryToDelete
509507
* @return void
510-
* @throws \Magento\Framework\Exception\FileSystemException
511-
* @dataProvider clearCacheDataProvider
512-
*/
513-
public function testClearCache(
514-
bool $isRenameSuccessful,
515-
string $expectedDirectoryToDelete
516-
): void {
517-
$driver = $this->createMock(DriverInterface::class);
518-
$this->mediaDirectory->method('getAbsolutePath')
519-
->willReturnCallback(
520-
function (string $path) {
521-
return 'path/to/media/' . $path;
522-
}
523-
);
524-
$this->mediaDirectory->expects($this->exactly(2))
525-
->method('isDirectory')
526-
->willReturnOnConsecutiveCalls(false, true);
527-
$this->mediaDirectory->expects($this->once())
528-
->method('getDriver')
529-
->willReturn($driver);
530-
$driver->expects($this->once())
531-
->method('rename')
532-
->with(
533-
'path/to/media/catalog/product/cache',
534-
$this->matchesRegularExpression('/^path\/to\/media\/catalog\/product\/\.[0-9A-ZA-z-_]{3}$/')
535-
)
536-
->willReturn($isRenameSuccessful);
537-
$this->mediaDirectory->expects($this->once())
538-
->method('delete')
539-
->with($this->matchesRegularExpression($expectedDirectoryToDelete));
540-
508+
* @throws FileSystemException
509+
*/
510+
public function testClearCache(): void
511+
{
541512
$this->coreFileHelper->expects($this->once())->method('deleteFolder')->willReturn(true);
542513
$this->cacheManager->expects($this->once())->method('clean');
543514
$this->image->clearCache();
544515
}
545516

546517
/**
547-
* @return array
518+
* This test verifies that if the cache directory cannot be deleted because it is no longer empty (due to newly
519+
* cached files being created after the old ones were deleted), the cache clean method should handle the exception
520+
* and complete the clean successfully even if the directory cannot be deleted.
521+
*
522+
* @return void
523+
* @throws FileSystemException
548524
*/
549-
public function clearCacheDataProvider(): array
525+
public function testClearCacheWithUnableToDeleteDirectory(): void
550526
{
551-
return [
552-
[true, '/^catalog\/product\/\.[0-9A-ZA-z-_]{3}$/'],
553-
[false, '/^catalog\/product\/cache$/'],
554-
];
527+
$this->mediaDirectory->expects($this->once())
528+
->method('delete')
529+
->willThrowException(new FileSystemException(__('Cannot delete non-empty dir.')));
530+
531+
// Image cache should complete successfully even if the directory cannot be deleted.
532+
$this->coreFileHelper->expects($this->once())->method('deleteFolder')->willReturn(true);
533+
$this->cacheManager->expects($this->once())->method('clean');
534+
535+
$this->image->clearCache();
555536
}
556537

557538
/**

0 commit comments

Comments
 (0)