Skip to content

Commit 22b5d72

Browse files
committed
ACP2E-2631: Hidden directories in pub/media/catalog/product cause disk space issues
1 parent 296a311 commit 22b5d72

File tree

2 files changed

+37
-47
lines changed

2 files changed

+37
-47
lines changed

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class Image extends \Magento\Framework\Model\AbstractModel
4646
* Default quality value (for JPEG images only).
4747
*
4848
* @var int
49-
* @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
5051
*/
5152
protected $_quality = null;
5253

@@ -102,7 +103,8 @@ class Image extends \Magento\Framework\Model\AbstractModel
102103

103104
/**
104105
* @var int
105-
* @deprecated unused
106+
* @deprecated
107+
* @see Not used anymore
106108
*/
107109
protected $_angle;
108110

@@ -308,7 +310,8 @@ public function getHeight()
308310
*
309311
* @param int $quality
310312
* @return $this
311-
* @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
312315
*/
313316
public function setQuality($quality)
314317
{
@@ -458,6 +461,7 @@ public function getBaseFile()
458461
* Get new file
459462
*
460463
* @deprecated 102.0.0
464+
* @see Image::getBaseFile
461465
* @return bool|string
462466
*/
463467
public function getNewFile()
@@ -837,10 +841,15 @@ public function getWatermarkHeight()
837841
public function clearCache()
838842
{
839843
$directory = $this->_catalogProductMediaConfig->getBaseMediaPath() . '/cache';
844+
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.
848+
// To avoid errors on the storefront, we wrap the deletion in a try/catch block and silently handle any
849+
// exceptions, allowing the process to continue smoothly.
840850
try {
841851
$this->_mediaDirectory->delete($directory);
842-
}
843-
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch
852+
} // phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch
844853
catch (FileSystemException $e) {
845854
}
846855

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

Lines changed: 23 additions & 42 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. This is expected behavior and is not a cause for concern.
521+
* The test asserts that the cache cleaning process completes successfully even if the directory cannot be deleted.
522+
*
523+
* @return void
524+
* @throws FileSystemException
548525
*/
549-
public function clearCacheDataProvider(): array
550-
{
551-
return [
552-
[true, '/^catalog\/product\/\.[0-9A-ZA-z-_]{3}$/'],
553-
[false, '/^catalog\/product\/cache$/'],
554-
];
526+
public function testClearCacheWithUnableToDeleteDirectory(): void {
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)