FEATURE: review implementation

* it is called configuration not configs! * renamed ImageVariantService::getAllPresetsByConfigs to getAllPresetsOfIdentifier * refactored ImageVariantService::getAllPresetsOfIdentifier for usability * specified configuresPresets type in ImageVariantRepository::findAllWithOutdatedPresets * better naming for query condition + example ISSUE: neos#3086
queoGmbH · Oct 20, 2022 · 18e63a0 · 18e63a0
1 parent 6a25df1
commit 18e63a0
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 47 deletions.
diff --git a/Neos.Media/Classes/Command/MediaCommandController.php b/Neos.Media/Classes/Command/MediaCommandController.php
@@ -420,18 +420,21 @@ public function renderThumbnailsCommand(int $limit = null, bool $quiet = false)
      */
     public function removeOutdatedVariantsCommand(bool $quiet = false, bool $assumeYes = false, bool $filterCroppedVariants = false, int $limit = null, string $assetSource = '', string $presetIdentifier = '')
     {
-        // will read all presets defined in 'Settings.Neos.Media.yaml'
-        $variantPresetConfigs = $this->assetVariantGenerator->getVariantPresets();
-
-        $filterByAssetSourceIdentifier = $assetSource;
+        if (empty($presetIdentifier)) {
+            // will read all presets defined in 'Settings.Neos.Media.yaml'
+            $currentPresets = $this->imageVariantService->getAllPresetsByConfiguration();
+        } else {
+            // is --preset-identifier used
+            $currentPresets = $this->imageVariantService->getAllPresetsOfIdentifier($presetIdentifier);
+        }
 
-        // is --preset-identifier used
-        $currentPresets = $this->imageVariantService->getAllPresetsByConfigs($variantPresetConfigs, $presetIdentifier);
         if (empty($currentPresets)) {
             !$quiet && $this->output->outputLine(PHP_EOL . PHP_EOL . '<em>No presets found.</em>');
             exit;
         }
 
+        $filterByAssetSourceIdentifier = $assetSource;
+
         // is --asset-source-filter used
         if (empty($filterByAssetSourceIdentifier)) {
             !$quiet && $this->outputLine(PHP_EOL . '<b>Searching for assets in all sources:</b>');

diff --git a/Neos.Media/Classes/Domain/Repository/ImageVariantRepository.php b/Neos.Media/Classes/Domain/Repository/ImageVariantRepository.php
@@ -15,6 +15,7 @@
 
 use Neos\Flow\Annotations as Flow;
 use Neos\Media\Domain\Model\ImageVariant;
+use Neos\Media\Domain\ValueObject\Configuration\VariantPreset;
 
 /**
  * A repository for ImageVariants
@@ -26,25 +27,40 @@ class ImageVariantRepository extends AssetRepository
     /**
      * Returns array of ImageVariants with outdated presets
      *
-     * @param String[] $configuredPresets
+     * @param VariantPreset[] $configuredPresets
+     * @param bool $deleteOnlyFromGivenPresets
+     * @param bool $filterCroppedVariants
+     * @param int|null $limit
      * @return ImageVariant[]
      */
-    public function findAllWithOutdatedPresets(array $configuredPresets, bool $isCliPresetFilter, bool $filterCroppedVariants, int $limit = null): array
+    public function findAllWithOutdatedPresets(array $configuredPresets, bool $deleteOnlyFromGivenPresets, bool $filterCroppedVariants, int $limit = null): array
     {
         $configuredIdentifiers = array_keys($configuredPresets);
         $queryBuilder = $this->entityManager->createQueryBuilder()
             ->select('iv')
             ->from(ImageVariant::class, 'iv')
             ->setMaxResults($limit);
 
-        if (!$isCliPresetFilter) {
+        if (!$deleteOnlyFromGivenPresets) {
+            /**
+             * for completely outdated preset configurations
+             *
+             * EXAMPLE:
+             *  - you have the identifiers Neos.Cool, Neos.Yeah (and there was a Neos.Awesome previously)
+             * case 1:
+             *  - the user want to delete variants from Neos.Yeah
+             *  - condition will not be executed - deleteFromGivenPresets is true
+             * case 2:
+             *  - no preset to delete from configured
+             *  - condition will be executed - whole Neos.Awesome will be added to query
+             */
             $queryBuilder
                 ->where('iv.presetIdentifier NOT IN (:configuredIdentifiers)')
                 ->setParameter('configuredIdentifiers', $configuredIdentifiers);
         }
 
         if ($filterCroppedVariants) {
-            // custom crops will be saved with null value as presetIdentifier and presetVariantName
+            // custom cropped variants will be saved with null value as presetIdentifier and presetVariantName
             $queryBuilder
                 ->orWhere('iv.presetIdentifier IS NULL AND iv.presetVariantName IS NULL');
         }

diff --git a/Neos.Media/Classes/Domain/Service/ImageVariantService.php b/Neos.Media/Classes/Domain/Service/ImageVariantService.php
@@ -22,36 +22,39 @@
  */
 class ImageVariantService
 {
+    /**
+     * @Flow\Inject
+     * @var AssetVariantGenerator
+     */
+    protected $assetVariantGenerator;
+
     /**
      * Return all presets defined in 'Settings.Neos.Media.yaml' with presetName as key
      *
-     * @param array $variantPresetConfigs
      * @param string $presetIdentifier
-     * @return array
+     * @return VariantPreset[]
      */
-    public function getAllPresetsByConfigs(array $variantPresetConfigs, string $presetIdentifier = ''): array
+    public function getAllPresetsOfIdentifier(string $presetIdentifier): array
     {
-        $presets = [];
+        $variantPresetConfigurations = $this->getAllPresetsByConfiguration();
 
-        if (!empty($presetIdentifier) && !key_exists($presetIdentifier, $variantPresetConfigs)) {
-            return $presets;
-        }
+        $variantPresetName = array_key_exists($presetIdentifier, $variantPresetConfigurations);
 
-        /** @var VariantPreset[] $variantPresetConfigs */
-        if (!empty($presetIdentifier)) {
-            foreach ($variantPresetConfigs[$presetIdentifier]->variants() as $presetVariant) {
-                $presets[$presetIdentifier][] = $presetVariant->identifier();
-            }
-        } else {
-            foreach ($variantPresetConfigs as $presetsConfig) {
-                $variantPresetName = array_search($presetsConfig, $variantPresetConfigs);
-                $presetKeys = array_keys($presetsConfig->variants());
-                foreach ($presetKeys as $preset) {
-                    $presets[(string)$variantPresetName][] = $preset;
-                }
-            }
+        if (!$variantPresetName) {
+            // the given presetIdentifier ist not included in variantPresetConfigurations
+            return [];
         }
 
-        return $presets;
+        return [$variantPresetConfigurations[$presetIdentifier]];
+    }
+
+    /**
+     * Return presets from 'Settings.Neos.Media.yaml'
+     *
+     * @return VariantPreset[]
+     */
+    public function getAllPresetsByConfiguration(): array
+    {
+        return $this->assetVariantGenerator->getVariantPresets();
     }
 }
diff --git a/Neos.Media/Tests/Unit/Domain/Service/ImageVariantServiceTest.php b/Neos.Media/Tests/Unit/Domain/Service/ImageVariantServiceTest.php
@@ -1,4 +1,5 @@
 <?php
+
 namespace Neos\Media\Tests\Unit\Domain\Service;
 
 /*
@@ -12,16 +13,19 @@
  */
 
 use Neos\Flow\Tests\UnitTestCase;
+use Neos\Media\Domain\Service\AssetVariantGenerator;
 use Neos\Media\Domain\Service\ImageVariantService;
 use Neos\Media\Domain\ValueObject\Configuration\Label;
 use Neos\Media\Domain\ValueObject\Configuration\Variant;
 use Neos\Media\Domain\ValueObject\Configuration\VariantPreset;
+use PHPUnit\Framework\TestCase;
 use ReflectionClass;
+use ReflectionException;
 
 /**
  * Test case for the ImageVariant Service
  */
-class ImageVariantServiceTest extends UnitTestCase
+class ImageVariantServiceTest extends TestCase
 {
     /**
      * @var ImageVariantService
@@ -36,7 +40,7 @@ protected function setUp(): void
     /**
      * @return array
      */
-    public function getAllPresetsByConfigsProvider(): array
+    public function getAllPresetsByConfigurationsProvider(): array
     {
         $neosPreset = new VariantPreset(new Label('neosTestImageVariants'));
         $flowPreset = new VariantPreset(new Label('flowTestImageVariants'));
@@ -58,30 +62,42 @@ public function getAllPresetsByConfigsProvider(): array
         $variantPresetReflectionVariants->setValue($flowPreset, $flowPresetConfiguration);
 
         return [
-            [['neos' => $neosPreset, 'flow' => $flowPreset], null],
-            [['neos' => $neosPreset, 'flow' => $flowPreset], ''],
-            [['neos' => $neosPreset, 'flow' => $flowPreset], 'neos']
+            'empty' => [['neos' => $neosPreset, 'flow' => $flowPreset], '', true],
+            'known preset' => [['neos' => $neosPreset, 'flow' => $flowPreset], 'neos', false],
+            'unknown preset' => [['neos' => $neosPreset, 'flow' => $flowPreset], 'imageVariant', true],
         ];
     }
 
     /**
      * @test
-     * @param array $variantPresetConfigs
+     * @dataProvider getAllPresetsByConfigurationsProvider
+     *
+     * @param VariantPreset[] $variantPresets
      * @param string|null $presetIdentifier
-     * @dataProvider getAllPresetsByConfigsProvider
+     * @param bool $emptyResult
+     *
+     * @throws ReflectionException
      */
-    public function getAllPresetsByConfigs(array $variantPresetConfigs, ?string $presetIdentifier): void
+    public function getAllPresetsByConfigs(array $variantPresets, ?string $presetIdentifier, bool $emptyResult): void
     {
-        if (is_null($presetIdentifier)) {
-            $presetsConfig = $this->imageVariantService->getAllPresetsByConfigs($variantPresetConfigs);
-        } else {
-            $presetsConfig = $this->imageVariantService->getAllPresetsByConfigs($variantPresetConfigs, $presetIdentifier);
-        }
+        $assetVariantGeneratorMock = $this->getMockBuilder(AssetVariantGenerator::class)->getMock();
+        $assetVariantGeneratorMock->expects($this->once())->method('getVariantPresets')->willReturn($variantPresets);
+
+        $imageVariantService = new ImageVariantService();
+
+        $imageVariantServiceReflection = new ReflectionClass(ImageVariantService::class);
+        $reflectionProperty = $imageVariantServiceReflection->getProperty('assetVariantGenerator');
+        $reflectionProperty->setAccessible(true);
+        $reflectionProperty->setValue($imageVariantService, $assetVariantGeneratorMock);
+
+        $reflectionMethod = $imageVariantServiceReflection->getMethod('getAllPresetsOfIdentifier');
+        $reflectionMethod->setAccessible(true);
+        $presetsConfig = $reflectionMethod->invokeArgs($imageVariantService, [$presetIdentifier]);
 
-        if (!$presetIdentifier) {
-            self::assertEquals(['neos' => ['square', 'portrait'], 'flow' => ['panorama']], $presetsConfig);
+        if ($emptyResult) {
+            self::assertEquals([], $presetsConfig);
         } else {
-            self::assertEquals(['neos' => ['square', 'portrait']], $presetsConfig);
+            self::assertEquals([$variantPresets['neos']], $presetsConfig);
         }
     }
 }