ENGCOM-6293: REFACTORING: adjust commands to the common style in the MediaGallery and change exception handle logic #25614

 - Merge Pull Request #25614 from coderimus/magento2:refactoring/exception-handling
 - Merged commits:
   1. 3a82b4a
   2. e694954
   3. 8f7085f
   4. 20a6a37
   5. d282c10

Author: magento-engcom-team

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

@@ -7,10 +7,10 @@
 
 namespace Magento\MediaGallery\Model\Asset\Command;
 
-use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
 use Magento\Framework\App\ResourceConnection;
-use Magento\Framework\Exception\CouldNotDeleteException;
 use Magento\Framework\DB\Adapter\AdapterInterface;
+use Magento\Framework\Exception\CouldNotDeleteException;
+use Magento\MediaGalleryApi\Model\Asset\Command\DeleteByPathInterface;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -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

@@ -73,23 +73,23 @@ public function execute(int $mediaAssetId): AssetInterface
                 ->where('amg.id = ?', $mediaAssetId);
             $mediaAssetData = $connection->query($select)->fetch();
         } catch (\Exception $exception) {
+            $this->logger->critical($exception);
             $message = __(
                 'En error occurred during get media asset data by id %id: %error',
                 ['id' => $mediaAssetId, 'error' => $exception->getMessage()]
             );
-            $this->logger->critical($message);
             throw new IntegrationException($message, $exception);
         }
 
         if (empty($mediaAssetData)) {
-            $message = __('There is no such media asset with id "%1"', $mediaAssetId);
+            $message = __('There is no such media asset with id %id', ['id' => $mediaAssetId]);
             throw new NoSuchEntityException($message);
         }
 
         try {
             return $this->assetFactory->create(['data' => $mediaAssetData]);
         } catch (\Exception $exception) {
-            $this->logger->critical($exception->getMessage());
+            $this->logger->critical($exception);
             $message = __(
                 'En error occurred during initialize media asset with id %id: %error',
                 ['id' => $mediaAssetId, 'error' => $exception->getMessage()]

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

@@ -7,16 +7,20 @@
 
 namespace Magento\MediaGallery\Test\Unit\Model\Keyword\Command;
 
+use Magento\Framework\Exception\IntegrationException;
 use Magento\MediaGallery\Model\Keyword\Command\GetAssetKeywords;
 use Magento\MediaGalleryApi\Api\Data\KeywordInterface;
 use Magento\MediaGalleryApi\Api\Data\KeywordInterfaceFactory;
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DB\Adapter\AdapterInterface;
 use Magento\Framework\DB\Select;
-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 $loggerMock;
+
     protected function setUp(): void
     {
         $this->resourceConnectionStub = $this->createMock(ResourceConnection::class);
         $this->assetKeywordFactoryStub = $this->createMock(KeywordInterfaceFactory::class);
+        $this->loggerMock = $this->createMock(LoggerInterface::class);
 
         $this->sut = new GetAssetKeywords(
             $this->resourceConnectionStub,
-            $this->assetKeywordFactoryStub
+            $this->assetKeywordFactoryStub,
+            $this->loggerMock
         );
     }
 
@@ -51,7 +62,6 @@ protected function setUp(): void
      * @dataProvider casesProvider()
      * @param array $databaseQueryResult
      * @param int $expectedNumberOfFoundKeywords
-     * @throws NotFoundException
      */
     public function testFind(array $databaseQueryResult, int $expectedNumberOfFoundKeywords): void
     {
@@ -80,9 +90,9 @@ public function casesProvider(): array
     }
 
     /**
-     * Negative test
+     * Test case when an error occured during get data request.
      *
-     * @throws NotFoundException
+     * @throws IntegrationException
      */
     public function testNotFoundBecauseOfError(): void
     {
@@ -92,7 +102,10 @@ public function testNotFoundBecauseOfError(): void
             ->method('getConnection')
             ->willThrowException((new \Exception()));
 
-        $this->expectException(NotFoundException::class);
+        $this->expectException(IntegrationException::class);
+        $this->loggerMock->expects($this->once())
+            ->method('critical')
+            ->willReturnSelf();
 
         $this->sut->execute($randomAssetId);
     }

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

@@ -7,15 +7,16 @@
 
 namespace Magento\MediaGallery\Test\Unit\Model\Keyword\Command;
 
-use Magento\MediaGallery\Model\Keyword\Command\SaveAssetKeywords;
-use Magento\MediaGallery\Model\Keyword\Command\SaveAssetLinks;
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DataObject;
 use Magento\Framework\DB\Adapter\Pdo\Mysql;
 use Magento\Framework\DB\Select;
 use Magento\Framework\Exception\CouldNotSaveException;
+use Magento\MediaGallery\Model\Keyword\Command\SaveAssetKeywords;
+use Magento\MediaGallery\Model\Keyword\Command\SaveAssetLinks;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
 /**
  * SaveAssetKeywordsTest.
@@ -47,6 +48,11 @@ class SaveAssetKeywordsTest extends TestCase
      */
     private $selectMock;
 
+    /**
+     * @var LoggerInterface|MockObject
+     */
+    private $loggerMock;
+
     /**
      * SetUp
      */
@@ -58,10 +64,12 @@ public function setUp(): void
             ->disableOriginalConstructor()
             ->getMock();
         $this->selectMock = $this->createMock(Select::class);
+        $this->loggerMock = $this->createMock(LoggerInterface::class);
 
         $this->sut = new SaveAssetKeywords(
             $this->resourceConnectionMock,
-            $this->saveAssetLinksMock
+            $this->saveAssetLinksMock,
+            $this->loggerMock
         );
     }
 
@@ -106,6 +114,9 @@ public function testAssetNotSavingCausedByError(): void
             ->method('getConnection')
             ->willThrowException((new \Exception()));
         $this->expectException(CouldNotSaveException::class);
+        $this->loggerMock->expects($this->once())
+            ->method('critical')
+            ->willReturnSelf();
 
         $this->sut->execute([$keyword], 1);
     }