#27499: Code review fixes

Author: Stanislav Idolov

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

@@ -28,11 +28,6 @@ class Save implements SaveInterface
      */
     private $resourceConnection;
 
-    /**
-     * @var DataObjectProcessor
-     */
-    private $objectProcessor;
-
     /**
      * @var LoggerInterface
      */
@@ -42,16 +37,13 @@ class Save implements SaveInterface
      * Save constructor.
      *
      * @param ResourceConnection $resourceConnection
-     * @param DataObjectProcessor $objectProcessor
      * @param LoggerInterface $logger
      */
     public function __construct(
         ResourceConnection $resourceConnection,
-        DataObjectProcessor $objectProcessor,
         LoggerInterface $logger
     ) {
         $this->resourceConnection = $resourceConnection;
-        $this->objectProcessor = $objectProcessor;
         $this->logger = $logger;
     }
 
@@ -72,37 +64,24 @@ public function execute(AssetInterface $mediaAsset): int
 
             $connection->insertOnDuplicate(
                 $tableName,
-                $this->filterData($this->objectProcessor->buildOutputDataArray($mediaAsset, AssetInterface::class))
+                [
+                    'id' => $mediaAsset->getId(),
+                    'path' => $mediaAsset->getPath(),
+                    'title' => $mediaAsset->getTitle(),
+                    'source' => $mediaAsset->getSource(),
+                    'content_type' => $mediaAsset->getContentType(),
+                    'width' => $mediaAsset->getWidth(),
+                    'height' => $mediaAsset->getHeight(),
+                    'size' => $mediaAsset->getSize(),
+                    'created_at' => $mediaAsset->getCreatedAt(),
+                    'updated_at' => $mediaAsset->getUpdatedAt(),
+                ]
             );
-            return (int) $connection->lastInsertId($tableName);
+            return (int)$connection->lastInsertId($tableName);
         } catch (\Exception $exception) {
             $this->logger->critical($exception);
             $message = __('An error occurred during media asset save: %1', $exception->getMessage());
             throw new CouldNotSaveException($message, $exception);
         }
     }
-
-    /**
-     * Filter data to get flat array without null values
-     *
-     * @param array $data
-     * @return array
-     */
-    private function filterData(array $data): array
-    {
-        $filteredData = [];
-        foreach ($data as $key => $value) {
-            if ($value === null) {
-                continue;
-            }
-            if (is_array($value)) {
-                continue;
-            }
-            if (is_object($value)) {
-                continue;
-            }
-            $filteredData[$key] = $value;
-        }
-        return $filteredData;
-    }
 }

app/code/Magento/MediaGallery/Model/Directory/Config.php

@@ -8,11 +8,12 @@
 namespace Magento\MediaGallery\Model\Directory;
 
 use Magento\Framework\Config\DataInterface;
+use Magento\MediaGalleryApi\Model\BlacklistPatternsConfigInterface;
 
 /**
  * Media gallery directory config
  */
-class Config
+class Config implements BlacklistPatternsConfigInterface
 {
     private const XML_PATH_BLACKLIST_PATTERNS = 'blacklist/patterns';
 
@@ -29,25 +30,13 @@ public function __construct(DataInterface $data)
         $this->data = $data;
     }
 
-    /**
-     * Get config value by key.
-     *
-     * @param string|null $key
-     * @param string|null $default
-     * @return array
-     */
-    public function get($key = null, $default = null)
-    {
-        return $this->data->get($key, $default);
-    }
-
     /**
      * Returns list of blacklist regexp patterns
      *
      * @return array
      */
-    public function getBlacklistPatterns() : array
+    public function get() : array
     {
-        return $this->get(self::XML_PATH_BLACKLIST_PATTERNS);
+        return $this->data->get(self::XML_PATH_BLACKLIST_PATTERNS);
     }
 }

app/code/Magento/MediaGallery/Model/Directory/IsBlacklisted.php

@@ -8,21 +8,22 @@
 namespace Magento\MediaGallery\Model\Directory;
 
 use Magento\MediaGalleryApi\Api\IsPathBlacklistedInterface;
+use Magento\MediaGalleryApi\Model\BlacklistPatternsConfigInterface;
 
 /**
  * Check if the path is blacklisted for media gallery. Directory path may be blacklisted if it's reserved by the system
  */
 class IsBlacklisted implements IsPathBlacklistedInterface
 {
     /**
-     * @var Config
+     * @var BlacklistPatternsConfigInterface
      */
     private $config;
 
-    /**
-     * @param Config $config
+    /*
+     * @param BlacklistPatternsConfigInterface $config
      */
-    public function __construct(Config $config)
+    public function __construct(BlacklistPatternsConfigInterface $config)
     {
         $this->config = $config;
     }
@@ -35,7 +36,7 @@ public function __construct(Config $config)
      */
     public function execute(string $path): bool
     {
-        foreach ($this->config->getBlacklistPatterns() as $pattern) {
+        foreach ($this->config->get() as $pattern) {
             if (empty($pattern)) {
                 continue;
             }

app/code/Magento/MediaGallery/Model/ResourceModel/SaveAssets.php

@@ -26,11 +26,6 @@ class SaveAssets implements SaveAssetsInterface
      */
     private $resourceConnection;
 
-    /**
-     * @var DataObjectProcessor
-     */
-    private $objectProcessor;
-
     /**
      * @var LoggerInterface
      */
@@ -40,16 +35,13 @@ class SaveAssets implements SaveAssetsInterface
      * Save constructor.
      *
      * @param ResourceConnection $resourceConnection
-     * @param DataObjectProcessor $objectProcessor
      * @param LoggerInterface $logger
      */
     public function __construct(
         ResourceConnection $resourceConnection,
-        DataObjectProcessor $objectProcessor,
         LoggerInterface $logger
     ) {
         $this->resourceConnection = $resourceConnection;
-        $this->objectProcessor = $objectProcessor;
         $this->logger = $logger;
     }
 
@@ -66,7 +58,18 @@ public function execute(array $assets): void
             try {
                 $connection->insertOnDuplicate(
                     $tableName,
-                    $this->filterData($this->objectProcessor->buildOutputDataArray($asset, AssetInterface::class))
+                    [
+                        'id' => $asset->getId(),
+                        'path' => $asset->getPath(),
+                        'title' => $asset->getTitle(),
+                        'source' => $asset->getSource(),
+                        'content_type' => $asset->getContentType(),
+                        'width' => $asset->getWidth(),
+                        'height' => $asset->getHeight(),
+                        'size' => $asset->getSize(),
+                        'created_at' => $asset->getCreatedAt(),
+                        'updated_at' => $asset->getUpdatedAt(),
+                    ]
                 );
             } catch (\Exception $exception) {
                 $this->logger->critical($exception);
@@ -85,28 +88,4 @@ public function execute(array $assets): void
             );
         }
     }
-
-    /**
-     * Filter data to get flat array without null values
-     *
-     * @param array $data
-     * @return array
-     */
-    private function filterData(array $data): array
-    {
-        $filteredData = [];
-        foreach ($data as $key => $value) {
-            if ($value === null) {
-                continue;
-            }
-            if (is_array($value)) {
-                continue;
-            }
-            if (is_object($value)) {
-                continue;
-            }
-            $filteredData[$key] = $value;
-        }
-        return $filteredData;
-    }
 }

app/code/Magento/MediaGallery/Test/Unit/Model/Directory/IsBlacklistedTest.php

@@ -10,6 +10,7 @@
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\MediaGallery\Model\Directory\IsBlacklisted;
 use Magento\MediaGallery\Model\Directory\Config;
+use Magento\MediaGalleryApi\Model\BlacklistPatternsConfigInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -24,7 +25,7 @@ class IsBlacklistedTest extends TestCase
     private $object;
 
     /**
-     * @var Config|MockObject
+     * @var BlacklistPatternsConfigInterface|MockObject
      */
     private $config;
 
@@ -33,8 +34,8 @@ class IsBlacklistedTest extends TestCase
      */
     protected function setUp(): void
     {
-        $this->config = $this->getMockBuilder(Config::class)->disableOriginalConstructor()->getMock();
-        $this->config->expects($this->at(0))->method('getBlacklistPatterns')->willReturn([
+        $this->config = $this->getMockBuilder(BlacklistPatternsConfigInterface::class)->disableOriginalConstructor()->getMock();
+        $this->config->expects($this->at(0))->method('get')->willReturn([
             'tmp' => '/pub\/media\/tmp/',
             'captcha' => '/pub\/media\/captcha/'
         ]);

app/code/Magento/MediaGallery/etc/di.xml

@@ -56,4 +56,6 @@
             <argument name="data" xsi:type="object">Magento\MediaGallery\Model\Directory\Config\Data</argument>
         </arguments>
     </type>
+
+    <preference for="Magento\MediaGalleryApi\Model\BlacklistPatternsConfigInterface" type="Magento\MediaGallery\Model\Directory\Config"/>
 </config>

app/code/Magento/MediaGalleryApi/Api/CreateDirectoriesByPathsInterface.php

@@ -17,6 +17,7 @@ interface CreateDirectoriesByPathsInterface
      * Create new directories by provided paths
      *
      * @param string[] $paths
+     * @return void
      * @throws \Magento\Framework\Exception\CouldNotSaveException
      */
     public function execute(array $paths): void;

app/code/Magento/MediaGalleryApi/Api/DeleteDirectoriesByPathsInterface.php

@@ -17,6 +17,7 @@ interface DeleteDirectoriesByPathsInterface
      * Deletes the existing folders
      *
      * @param string[] $paths
+     * @return void
      * @throws \Magento\Framework\Exception\CouldNotDeleteException
      */
     public function execute(array $paths): void;

app/code/Magento/MediaGalleryApi/Api/IsPathBlacklistedInterface.php

@@ -8,7 +8,9 @@
 namespace Magento\MediaGalleryApi\Api;
 
 /**
- * Directory paths that are reserved by system and not be included in the media gallery
+ * Check if the path is blacklisted for media gallery.
+ *
+ * Directory path may be blacklisted if it's reserved by the system.
  * @api
  */
 interface IsPathBlacklistedInterface

app/code/Magento/MediaGalleryApi/Api/SaveAssetsInterface.php

@@ -18,6 +18,7 @@ interface SaveAssetsInterface
      * Save media asset. The saved asset can later be retrieved by path
      *
      * @param \Magento\MediaGalleryApi\Api\Data\AssetInterface[] $assets
+     * @return void
      * @throws \Magento\Framework\Exception\CouldNotSaveException
      */
     public function execute(array $assets): void;