REFACTORING: adjust commands to the common style in the MediaGallery and change exception handle logic

Author: coderimus

app/code/Magento/MediaGallery/Model/Asset/Command/DeleteByPath.php

@@ -62,11 +62,11 @@ public function execute(string $mediaAssetPath): void
             $tableName = $this->resourceConnection->getTableName(self::TABLE_MEDIA_GALLERY_ASSET);
             $connection->delete($tableName, [self::MEDIA_GALLERY_ASSET_PATH . ' = ?' => $mediaAssetPath]);
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __(
                 'Could not delete media asset with path %path: %error',
                 ['path' => $mediaAssetPath, 'error' => $exception->getMessage()]
             );
-            $this->logger->critical($message);
             throw new CouldNotDeleteException($message, $exception);
         }
     }

app/code/Magento/MediaGallery/Model/Asset/Command/GetById.php

@@ -79,11 +79,11 @@ public function execute(int $mediaAssetId): AssetInterface
 
             return $this->assetFactory->create(['data' => $data]);
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __(
                 'En error occurred during get media asset with id %id by id: %error',
                 ['id' => $mediaAssetId, 'error' => $exception->getMessage()]
             );
-            $this->logger->critical($message);
             throw new IntegrationException($message, $exception);
         }
     }

app/code/Magento/MediaGallery/Model/Asset/Command/GetByPath.php

@@ -82,8 +82,8 @@ public function execute(string $mediaFilePath): AssetInterface
 
             return $mediaAssets;
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __('An error occurred during get media asset list: %1', $exception->getMessage());
-            $this->logger->critical($message);
             throw new IntegrationException($message, $exception);
         }
     }

app/code/Magento/MediaGallery/Model/Asset/Command/Save.php

@@ -71,8 +71,8 @@ public function execute(AssetInterface $mediaAsset): int
             $connection->insertOnDuplicate($tableName, $this->extractor->extract($mediaAsset, AssetInterface::class));
             return (int) $connection->lastInsertId($tableName);
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __('An error occurred during media asset save: %1', $exception->getMessage());
-            $this->logger->critical($message);
             throw new CouldNotSaveException($message, $exception);
         }
     }

app/code/Magento/MediaGallery/Model/DataExtractor.php

@@ -15,7 +15,13 @@
 class DataExtractor implements DataExtractorInterface
 {
     /**
-     * @inheritdoc
+     * Extract data from an object using available getters (does not process extension attributes)
+     *
+     * @param object $object
+     * @param string|null $interface
+     *
+     * @return array
+     * @throws \ReflectionException
      */
     public function extract($object, string $interface = null): array
     {

app/code/Magento/MediaGallery/Model/Keyword/Command/GetAssetKeywords.php

@@ -7,11 +7,12 @@
 
 namespace Magento\MediaGallery\Model\Keyword\Command;
 
+use Magento\Framework\Exception\IntegrationException;
 use Magento\MediaGalleryApi\Api\Data\KeywordInterface;
 use Magento\MediaGalleryApi\Api\Data\KeywordInterfaceFactory;
 use Magento\MediaGalleryApi\Model\Keyword\Command\GetAssetKeywordsInterface;
 use Magento\Framework\App\ResourceConnection;
-use Magento\Framework\Exception\NotFoundException;
+use Psr\Log\LoggerInterface;
 
 /**
  * ClassGetAssetKeywords
@@ -31,27 +32,35 @@ class GetAssetKeywords implements GetAssetKeywordsInterface
      */
     private $assetKeywordFactory;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * GetAssetKeywords constructor.
      *
      * @param ResourceConnection $resourceConnection
      * @param KeywordInterfaceFactory $assetKeywordFactory
+     * @param LoggerInterface $logger
      */
     public function __construct(
         ResourceConnection $resourceConnection,
-        KeywordInterfaceFactory $assetKeywordFactory
+        KeywordInterfaceFactory $assetKeywordFactory,
+        LoggerInterface $logger
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->assetKeywordFactory = $assetKeywordFactory;
+        $this->logger = $logger;
     }
 
     /**
      * Get asset related keywords.
      *
      * @param int $assetId
      *
-     * @return KeywordInterface[]
-     * @throws NotFoundException
+     * @return KeywordInterface[]|[]
+     * @throws IntegrationException
      */
     public function execute(int $assetId): array
     {
@@ -71,8 +80,9 @@ public function execute(int $assetId): array
 
             return $keywords;
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __('An error occurred during get asset keywords: %1', $exception->getMessage());
-            throw new NotFoundException($message, $exception);
+            throw new IntegrationException($message, $exception);
         }
     }
 }

app/code/Magento/MediaGallery/Model/Keyword/Command/SaveAssetKeywords.php

@@ -13,6 +13,7 @@
 use Magento\Framework\DB\Adapter\AdapterInterface;
 use Magento\Framework\DB\Adapter\Pdo\Mysql;
 use Magento\Framework\Exception\CouldNotSaveException;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class SaveAssetKeywords
@@ -33,22 +34,34 @@ class SaveAssetKeywords implements SaveAssetKeywordsInterface
      */
     private $saveAssetLinks;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * SaveAssetKeywords constructor.
      *
      * @param ResourceConnection $resourceConnection
      * @param SaveAssetLinks $saveAssetLinks
+     * @param LoggerInterface $logger
      */
     public function __construct(
         ResourceConnection $resourceConnection,
-        SaveAssetLinks $saveAssetLinks
+        SaveAssetLinks $saveAssetLinks,
+        LoggerInterface $logger
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->saveAssetLinks = $saveAssetLinks;
+        $this->logger = $logger;
     }
 
     /**
-     * @inheritdoc
+     * Save asset keywords.
+     *
+     * @param KeywordInterface[] $keywords
+     * @param int $assetId
+     * @throws CouldNotSaveException
      */
     public function execute(array $keywords, int $assetId): void
     {
@@ -72,6 +85,7 @@ public function execute(array $keywords, int $assetId): void
                 $this->saveAssetLinks->execute($assetId, $this->getKeywordIds($data));
             }
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __('An error occurred during save asset keyword: %1', $exception->getMessage());
             throw new CouldNotSaveException($message, $exception);
         }

app/code/Magento/MediaGallery/Model/Keyword/Command/SaveAssetLinks.php

@@ -13,6 +13,7 @@
 use Magento\Framework\DB\Adapter\AdapterInterface;
 use Magento\Framework\DB\Adapter\Pdo\Mysql;
 use Magento\Framework\Exception\CouldNotSaveException;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class SaveAssetLinks
@@ -29,14 +30,22 @@ class SaveAssetLinks
     private $resourceConnection;
 
     /**
-     * SaveAssetKeywords constructor.
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    /**
+     * SaveAssetLinks constructor.
      *
      * @param ResourceConnection $resourceConnection
+     * @param LoggerInterface $logger
      */
     public function __construct(
-        ResourceConnection $resourceConnection
+        ResourceConnection $resourceConnection,
+        LoggerInterface $logger
     ) {
         $this->resourceConnection = $resourceConnection;
+        $this->logger = $logger;
     }
 
     /**
@@ -66,6 +75,7 @@ public function execute(int $assetId, array $keywordIds): void
                 );
             }
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __('An error occurred during save asset keyword links: %1', $exception->getMessage());
             throw new CouldNotSaveException($message, $exception);
         }

app/code/Magento/MediaGallery/Plugin/Product/Gallery/Processor.php

@@ -67,8 +67,7 @@ public function afterRemoveImage(
         try {
             $this->deleteMediaAssetByPath->execute($file);
         } catch (\Exception $exception) {
-            $message = __('An error occurred during media asset delete at media processor: %1', $exception->getMessage());
-            $this->logger->critical($message->render());
+            $this->logger->critical($$exception);
         }
 
         return $result;

app/code/Magento/MediaGallery/Test/Unit/Model/Keyword/Command/GetAssetKeywordsTest.php

@@ -16,7 +16,11 @@
 use Magento\Framework\Exception\NotFoundException;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
+/**
+ * GetAssetKeywordsTest
+ */
 class GetAssetKeywordsTest extends TestCase
 {
     /**
@@ -34,14 +38,21 @@ class GetAssetKeywordsTest extends TestCase
      */
     private $assetKeywordFactoryStub;
 
+    /**
+     * @var LoggerInterface|MockObject
+     */
+    private $logger;
+
     protected function setUp(): void
     {
         $this->resourceConnectionStub = $this->createMock(ResourceConnection::class);
         $this->assetKeywordFactoryStub = $this->createMock(KeywordInterfaceFactory::class);
+        $this->logger = $this->createMock(LoggerInterface::class);
 
         $this->sut = new GetAssetKeywords(
             $this->resourceConnectionStub,
-            $this->assetKeywordFactoryStub
+            $this->assetKeywordFactoryStub,
+            $this->logger
         );
     }
 
@@ -80,7 +91,7 @@ public function casesProvider(): array
     }
 
     /**
-     * Negative test
+     * Test case when an error occured during get data request.
      *
      * @throws NotFoundException
      */
@@ -93,6 +104,9 @@ public function testNotFoundBecauseOfError(): void
             ->willThrowException((new \Exception()));
 
         $this->expectException(NotFoundException::class);
+        $this->logger->expects($this->once())
+            ->method('critical')
+            ->willReturnSelf();
 
         $this->sut->execute($randomAssetId);
     }