Skip to content

Commit 847e5bc

Browse files
rogyarrogyar
committed
Errors handling refactoring and additional test coverage
1 parent 8679c9a commit 847e5bc

File tree

3 files changed

+182
-9
lines changed

3 files changed

+182
-9
lines changed

‎app/code/Magento/MediaGallery/Model/Asset/Command/DeleteByDirectoryPath.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use Magento\Framework\App\ResourceConnection;
1111
use Magento\Framework\DB\Adapter\AdapterInterface;
1212
use Magento\Framework\Exception\CouldNotDeleteException;
13-
use Magento\Framework\Exception\LocalizedException;
1413
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByDirectoryPathInterface;
1514
use Psr\Log\LoggerInterface;
1615

@@ -55,13 +54,13 @@ public function __construct(
5554
* @param string $directoryPath
5655
*
5756
* @return void
57+
*
5858
* @throws CouldNotDeleteException
5959
*/
6060
public function execute(string $directoryPath): void
6161
{
62+
$this->validateDirectoryPath($directoryPath);
6263
try {
63-
$this->validateDirectoryPath($directoryPath);
64-
6564
// Make sure that the path has a trailing slash
6665
$directoryPath = rtrim($directoryPath, '/') . '/';
6766

@@ -83,12 +82,13 @@ public function execute(string $directoryPath): void
8382
* Validate the directory path
8483
*
8584
* @param string $directoryPath
86-
* @throws LocalizedException
85+
*
86+
* @throws CouldNotDeleteException
8787
*/
8888
private function validateDirectoryPath(string $directoryPath): void
8989
{
9090
if (!$directoryPath || trim($directoryPath) === '') {
91-
throw new LocalizedException(__('Cannot remove assets, the directory path does not exist'));
91+
throw new CouldNotDeleteException(__('Cannot remove assets, the directory path does not exist'));
9292
}
9393
}
9494
}

‎app/code/Magento/MediaGallery/Plugin/Wysiwyg/Images/Storage.php

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
namespace Magento\MediaGallery\Plugin\Wysiwyg\Images;
1010

11+
use Magento\Framework\Exception\CouldNotDeleteException;
1112
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByDirectoryPathInterface;
1213
use Magento\MediaGalleryApi\Model\Asset\Command\GetByPathInterface;
1314
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
1415
use Magento\Cms\Model\Wysiwyg\Images\Storage as StorageSubject;
1516
use Magento\Framework\App\Filesystem\DirectoryList;
1617
use Magento\Framework\Filesystem;
1718
use Psr\Log\LoggerInterface;
19+
use Magento\Framework\Exception\ValidatorException;
1820

1921
/**
2022
* Ensures that metadata is removed from the database when a file is deleted and it is an image
@@ -34,7 +36,7 @@ class Storage
3436
/**
3537
* @var DeleteByDirectoryPathInterface
3638
*/
37-
private $deleteMediAssetByDirectoryPath;
39+
private $deleteMediaAssetByDirectoryPath;
3840

3941
/**
4042
* @var Filesystem
@@ -64,7 +66,7 @@ public function __construct(
6466
) {
6567
$this->getMediaAssetByPath = $getMediaAssetByPath;
6668
$this->deleteMediaAssetByPath = $deleteMediaAssetByPath;
67-
$this->deleteMediAssetByDirectoryPath = $deleteByDirectoryPath;
69+
$this->deleteMediaAssetByDirectoryPath = $deleteByDirectoryPath;
6870
$this->filesystem = $filesystem;
6971
$this->logger = $logger;
7072
}
@@ -109,6 +111,7 @@ public function afterDeleteFile(StorageSubject $subject, StorageSubject $result,
109111
*
110112
* @return null
111113
*
114+
* @throws CouldNotDeleteException
112115
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
113116
*/
114117
public function afterDeleteDirectory(StorageSubject $subject, $result, $path)
@@ -117,8 +120,16 @@ public function afterDeleteDirectory(StorageSubject $subject, $result, $path)
117120
return $result;
118121
}
119122

120-
$relativePath = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA)->getRelativePath($path);
121-
$this->deleteMediAssetByDirectoryPath->execute($relativePath);
123+
try {
124+
$mediaDirectoryRead = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA);
125+
$relativePath = $mediaDirectoryRead->getRelativePath($path);
126+
if ($mediaDirectoryRead->isExist($relativePath) === false) {
127+
throw new CouldNotDeleteException(__('Cannot remove assets, the provided path does not exist'));
128+
}
129+
$this->deleteMediaAssetByDirectoryPath->execute($relativePath);
130+
} catch (ValidatorException $exception) {
131+
$this->logger->critical($exception);
132+
}
122133

123134
return $result;
124135
}

‎app/code/Magento/MediaGallery/Test/Unit/Plugin/Images/StorageTest.php

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\MediaGallery\Test\Unit\Plugin\Images;
9+
10+
use Magento\Cms\Model\Wysiwyg\Images\Storage as StorageSubject;
11+
use Magento\Framework\Exception\ValidatorException;
12+
use Magento\Framework\Filesystem;
13+
use Magento\Framework\Filesystem\Directory\ReadInterface;
14+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
15+
use Magento\MediaGallery\Model\Asset\Command\DeleteByDirectoryPath;
16+
use Magento\MediaGallery\Plugin\Wysiwyg\Images\Storage as StoragePlugin;
17+
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByDirectoryPathInterface;
18+
use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
19+
use Magento\MediaGalleryApi\Model\Asset\Command\GetByPathInterface;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
use PHPUnit\Framework\TestCase;
22+
use Psr\Log\LoggerInterface;
23+
24+
/**
25+
* Test the DeleteByDirectoryPath command model
26+
*/
27+
class StorageTest extends TestCase
28+
{
29+
private const NON_STRING_PATH = 2020;
30+
private const NON_EXISTENT_PATH = 'non_existent';
31+
private const INVALID_PATH = '&&';
32+
private const VALID_PATH = 'test-directory-path/';
33+
34+
/**
35+
* @var GetByPathInterface|MockObject
36+
*/
37+
private $getMediaAssetByPath;
38+
39+
/**
40+
* @var DeleteByPathInterface|MockObject
41+
*/
42+
private $deleteMediaAssetByPath;
43+
44+
/**
45+
* @var DeleteByDirectoryPathInterface|MockObject
46+
*/
47+
private $deleteMediaAssetByDirectoryPath;
48+
49+
/**
50+
* @var Filesystem|MockObject
51+
*/
52+
private $filesystem;
53+
54+
/**
55+
* @var LoggerInterface|MockObject
56+
*/
57+
private $logger;
58+
59+
/**
60+
* @var StoragePlugin
61+
*/
62+
private $storagePlugin;
63+
64+
/**
65+
* Initialize basic test class mocks
66+
*/
67+
protected function setUp(): void
68+
{
69+
$this->logger = $this->createMock(LoggerInterface::class);
70+
$this->getMediaAssetByPath = $this->createMock(GetByPathInterface::class);
71+
$this->deleteMediaAssetByPath = $this->createMock(DeleteByPathInterface::class);
72+
$this->deleteMediaAssetByDirectoryPath = $this->createMock(DeleteByDirectoryPath::class);
73+
$this->filesystem = $this->createMock(Filesystem::class);
74+
75+
$this->storagePlugin = (new ObjectManager($this))->getObject(
76+
StoragePlugin::class,
77+
[
78+
'getMediaAssetByPath' => $this->getMediaAssetByPath,
79+
'deleteMediaAssetByPath' => $this->deleteMediaAssetByPath,
80+
'deleteByDirectoryPath' => $this->deleteMediaAssetByDirectoryPath,
81+
'filesystem' => $this->filesystem,
82+
'logger' => $this->logger
83+
]
84+
);
85+
}
86+
87+
/**
88+
* @param string $path
89+
*
90+
* @dataProvider pathPathDataProvider
91+
*/
92+
public function testAfterDeleteDirectory(string $path): void
93+
{
94+
/** @var StorageSubject|MockObject $storageSubject */
95+
$storageSubject = $this->getMockBuilder(StorageSubject::class)
96+
->disableOriginalConstructor()
97+
->getMock();
98+
$directoryRead = $this->createMock(ReadInterface::class);
99+
$this->filesystem->expects($this->any())
100+
->method('getDirectoryRead')
101+
->willReturn($directoryRead);
102+
103+
switch ($path) {
104+
case self::NON_STRING_PATH:
105+
$result = $this->storagePlugin->afterDeleteDirectory($storageSubject, null, (int)$path);
106+
self::assertNull($result);
107+
break;
108+
case self::NON_EXISTENT_PATH:
109+
$directoryRead->expects($this->once())
110+
->method('getRelativePath')
111+
->with($path)
112+
->willReturn($path);
113+
$directoryRead->expects($this->once())
114+
->method('isExist')
115+
->with($path)
116+
->willReturn(false);
117+
self::expectException('Magento\Framework\Exception\CouldNotDeleteException');
118+
$this->storagePlugin->afterDeleteDirectory($storageSubject, null, $path);
119+
break;
120+
case self::INVALID_PATH:
121+
$exception = new ValidatorException(__('Path cannot be used with directory'));
122+
$directoryRead->expects($this->once())
123+
->method('getRelativePath')
124+
->with($path)
125+
->willThrowException($exception);
126+
$this->logger->expects($this->once())
127+
->method('critical')
128+
->with($exception);
129+
$this->storagePlugin->afterDeleteDirectory($storageSubject, null, $path);
130+
break;
131+
case self::VALID_PATH:
132+
$directoryRead->expects($this->once())
133+
->method('getRelativePath')
134+
->with($path)
135+
->willReturn($path);
136+
$directoryRead->expects($this->once())
137+
->method('isExist')
138+
->with($path)
139+
->willReturn(true);
140+
$this->deleteMediaAssetByDirectoryPath->expects($this->once())
141+
->method('execute')
142+
->with($path);
143+
$this->storagePlugin->afterDeleteDirectory($storageSubject, null, $path);
144+
break;
145+
}
146+
}
147+
148+
/**
149+
* Data provider for path
150+
*
151+
* @return array
152+
*/
153+
public function pathPathDataProvider(): array
154+
{
155+
return [
156+
'Non string path' => [2020],
157+
'Non-existent path' => [self::NON_EXISTENT_PATH],
158+
'Invalid path' => [self::INVALID_PATH],
159+
'Existent path' => [self::VALID_PATH]
160+
];
161+
}
162+
}

0 commit comments

Comments
 (0)