Adjustements for dimesion content query enhancer

Content/Domain/Model/DimensionContentCollection.php

@@ -15,6 +15,7 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
+use Sulu\Component\Util\SortUtils;
 
 /**
  * @implements \IteratorAggregate<DimensionContentInterface>
@@ -26,16 +27,6 @@ class DimensionContentCollection implements \IteratorAggregate, DimensionContent
      */
     private $dimensionContents;
 
-    /**
-     * @var DimensionContentInterface|null
-     */
-    private $unlocalizedDimensionContent;
-
-    /**
-     * @var DimensionContentInterface|null
-     */
-    private $localizedDimensionContent;
-
     /**
      * @var mixed[]
      */
@@ -63,23 +54,14 @@ public function __construct(
         array $dimensionAttributes,
         string $dimensionContentClass
     ) {
-        $this->dimensionContents = new ArrayCollection($dimensionContents);
         $this->dimensionContentClass = $dimensionContentClass;
         $this->defaultDimensionAttributes = $dimensionContentClass::getDefaultDimensionAttributes();
-
-        $this->unlocalizedDimensionContent = $this->dimensionContents->filter(
-            function(DimensionContentInterface $dimensionContent) {
-                return null === $dimensionContent->getLocale();
-            }
-        )->first() ?: null;
-
-        $this->localizedDimensionContent = $this->dimensionContents->filter(
-            function(DimensionContentInterface $dimensionContent) {
-                return null !== $dimensionContent->getLocale();
-            }
-        )->first() ?: null;
-
         $this->dimensionAttributes = $dimensionContentClass::getEffectiveDimensionAttributes($dimensionAttributes);
+
+        $this->dimensionContents = new ArrayCollection(
+            // sort dimension content correctly by effective attributes for later merge
+            SortUtils::multisort($dimensionContents, \array_keys($this->dimensionAttributes), 'asc')
+        );
     }
 
     public function getDimensionContentClass(): string

Content/Infrastructure/Doctrine/DimensionContentQueryEnhancer.php

@@ -45,31 +45,31 @@ public function __construct(
      * Withs represents additional selects which can be load to join and select specific sub entities.
      * They are used by groups and fields.
      */
-    public const WITH_EXCERPT_TAGS = 'with-excerpt-tags';
-    public const WITH_EXCERPT_CATEGORIES = 'with-excerpt-categories';
-    public const WITH_EXCERPT_CATEGORIES_TRANSLATION = 'with-excerpt-categories-translation';
+    public const SELECT_EXCERPT_TAGS = 'excerpt-tags';
+    public const SELECT_EXCERPT_CATEGORIES = 'excerpt-categories';
+    public const SELECT_EXCERPT_CATEGORIES_TRANSLATION = 'excerpt-categories-translation';
 
     /**
      * Groups are used in controllers and represents serialization / resolver group,
      * this allows that no controller need to be overwritten when something additional should be
      * loaded at that endpoint.
      */
-    public const GROUP_CONTENT_ADMIN = 'content_admin';
-    public const GROUP_CONTENT_WEBSITE = 'content_website';
+    public const GROUP_SELECT_CONTENT_ADMIN = 'content_admin';
+    public const GROUP_SELECT_CONTENT_WEBSITE = 'content_website';
 
     /**
      * TODO it should be possible to extend fields and groups inside the SELECTS.
      */
     private const SELECTS = [
         // GROUPS
-        self::GROUP_CONTENT_ADMIN => [
-            self::WITH_EXCERPT_TAGS => true,
-            self::WITH_EXCERPT_CATEGORIES => true,
+        self::GROUP_SELECT_CONTENT_ADMIN => [
+            self::SELECT_EXCERPT_TAGS => true,
+            self::SELECT_EXCERPT_CATEGORIES => true,
         ],
-        self::GROUP_CONTENT_WEBSITE => [
-            self::WITH_EXCERPT_TAGS => true,
-            self::WITH_EXCERPT_CATEGORIES => true,
-            self::WITH_EXCERPT_CATEGORIES_TRANSLATION => true,
+        self::GROUP_SELECT_CONTENT_WEBSITE => [
+            self::SELECT_EXCERPT_TAGS => true,
+            self::SELECT_EXCERPT_CATEGORIES => true,
+            self::SELECT_EXCERPT_CATEGORIES_TRANSLATION => true,
         ],
     ];
 
@@ -105,6 +105,8 @@ public function addFilters(
             'filterDimensionContent.' . $contentRichEntityAlias . ' = ' . $contentRichEntityAlias . ''
         );
 
+        // TODO filter to shadow dimension
+
         foreach ($effectiveAttributes as $key => $value) {
             if (null === $value) {
                 $queryBuilder->andWhere('filterDimensionContent.' . $key . ' IS NULL');
@@ -262,25 +264,24 @@ public function addSelects(
         }
 
         $effectiveAttributes = $dimensionContentClassName::getEffectiveDimensionAttributes($dimensionAttributes);
-        $this->addSortBy($queryBuilder, $effectiveAttributes);
         $queryBuilder->addCriteria($this->getAttributesCriteria('dimensionContent', $effectiveAttributes));
         $queryBuilder->addSelect('dimensionContent');
 
         $locale = $dimensionAttributes['locale'] ?? null;
 
         if (\is_subclass_of($dimensionContentClassName, ExcerptInterface::class)) {
-            if ($selects[self::WITH_EXCERPT_TAGS] ?? false) {
+            if ($selects[self::SELECT_EXCERPT_TAGS] ?? false) {
                 $queryBuilder->leftJoin('dimensionContent.excerptTags', 'contentExcerptTag')
                     ->addSelect('contentExcerptTag');
             }
 
-            if ($selects[self::WITH_EXCERPT_CATEGORIES] ?? false) {
+            if ($selects[self::SELECT_EXCERPT_CATEGORIES] ?? false) {
                 $queryBuilder->leftJoin('dimensionContent.excerptCategories', 'contentExcerptCategory')
                     ->addSelect('contentExcerptCategory');
             }
 
-            if ($selects[self::WITH_EXCERPT_CATEGORIES_TRANSLATION] ?? false) {
-                Assert::notFalse($selects[self::WITH_EXCERPT_CATEGORIES] ?? false);
+            if ($selects[self::SELECT_EXCERPT_CATEGORIES_TRANSLATION] ?? false) {
+                Assert::notFalse($selects[self::SELECT_EXCERPT_CATEGORIES] ?? false);
                 Assert::notNull($locale);
                 $queryBuilder->leftJoin(
                     'contentExcerptCategory.translations',
@@ -297,18 +298,6 @@ public function addSelects(
         }
     }
 
-    /**
-     * Less specific should be returned first to merge correctly.
-     *
-     * @param mixed[] $attributes
-     */
-    private function addSortBy(QueryBuilder $queryBuilder, array $attributes): void
-    {
-        foreach ($attributes as $key => $value) {
-            $queryBuilder->addOrderBy('dimensionContent.' . $key);
-        }
-    }
-
     /**
      * @param mixed[] $attributes
      */

Content/Infrastructure/Doctrine/DimensionContentRepository.php

@@ -65,7 +65,7 @@ public function load(
             $queryBuilder,
             $dimensionContentClass,
             $dimensionAttributes,
-            [DimensionContentQueryEnhancer::GROUP_CONTENT_ADMIN => true]
+            [DimensionContentQueryEnhancer::GROUP_SELECT_CONTENT_ADMIN => true]
         );
 
         /** @var DimensionContentInterface[] $dimensionContents */

Tests/Application/ExampleTestBundle/Repository/ExampleRepository.php

@@ -50,13 +50,13 @@ class ExampleRepository
         self::GROUP_EXAMPLE_ADMIN => [
             self::WITH_EXAMPLE_TRANSLATION => true,
             self::WITH_EXAMPLE_CONTENT => [
-                DimensionContentQueryEnhancer::GROUP_CONTENT_ADMIN => true,
+                DimensionContentQueryEnhancer::GROUP_SELECT_CONTENT_ADMIN => true,
             ],
         ],
         self::GROUP_EXAMPLE_WEBSITE => [
             self::WITH_EXAMPLE_TRANSLATION => true,
             self::WITH_EXAMPLE_CONTENT => [
-                DimensionContentQueryEnhancer::GROUP_CONTENT_WEBSITE => true,
+                DimensionContentQueryEnhancer::GROUP_SELECT_CONTENT_WEBSITE => true,
             ],
         ],
     ];
@@ -303,47 +303,13 @@ private function createQueryBuilder(array $filters, array $sortBy = [], array $s
             $queryBuilder->setFirstResult($offset);
         }
 
-        $contentFilters = [];
-        foreach ([
-            'locale',
-            'stage',
-            'categoryIds',
-            'categoryKeys',
-            'categoryOperator',
-            'tagIds',
-            'tagNames',
-            'tagOperator',
-            'templateKeys',
-        ] as $key) {
-            if (\array_key_exists($key, $filters)) {
-                $contentFilters[$key] = $filters[$key];
-            }
-        }
-
-        /**
-         * @see https://github.com/phpstan/phpstan/issues/5223
-         *
-         * @var array{
-         *     locale?: string|null,
-         *     stage?: string|null,
-         *     categoryIds?: int[],
-         *     categoryKeys?: string[],
-         *     categoryOperator?: 'AND'|'OR',
-         *     tagIds?: int[],
-         *     tagNames?: string[],
-         *     tagOperator?: 'AND'|'OR',
-         *     templateKeys?: string[],
-         * } $contentFilters
-         */
-        if (!empty($contentFilters)) {
-            Assert::keyExists($contentFilters, 'locale');
-            Assert::keyExists($contentFilters, 'stage');
-
+        if (\array_key_exists('locale', $filters) // should also work with locale = null
+            && \array_key_exists('stage', $filters)) {
             $this->dimensionContentQueryEnhancer->addFilters(
                 $queryBuilder,
                 'example',
                 ExampleDimensionContent::class,
-                $contentFilters
+                $filters
             );
         }
 
@@ -359,12 +325,12 @@ private function createQueryBuilder(array $filters, array $sortBy = [], array $s
             $this->dimensionContentQueryEnhancer->addSelects(
                 $queryBuilder,
                 ExampleDimensionContent::class,
-                $contentFilters,
+                $filters,
                 $contentSelects
             );
         }
 
-        $locale = $dimensionAttributes['locale'] ?? null;
+        $locale = $filters['locale'] ?? null;
         if ($selects['with-example-translations'] ?? null) {
             Assert::notNull($locale);
 

Tests/Functional/Content/Infrastructure/Doctrine/DimensionContentQueryEnhancerTest.php

@@ -183,7 +183,7 @@ public function testGroupContentAdminDisabledSelect(): void
                 ExampleRepository::GROUP_EXAMPLE_ADMIN => true,
                 ExampleRepository::GROUP_EXAMPLE_WEBSITE => false,
                 ExampleRepository::WITH_EXAMPLE_CONTENT => [
-                    DimensionContentQueryEnhancer::WITH_EXCERPT_TAGS => false,
+                    DimensionContentQueryEnhancer::SELECT_EXCERPT_TAGS => false,
                 ],
             ]
         );

Tests/Unit/Content/Domain/Model/DimensionContentCollectionTest.php

@@ -53,6 +53,28 @@ public function testCount(): void
         $this->assertSame(2, $dimensionContentCollection->count()); // @phpstan-ignore-line
     }
 
+    public function testSortedByAttributes(): void
+    {
+        $dimensionContent1 = $this->prophesize(DimensionContentInterface::class);
+        $dimensionContent1->getLocale()->willReturn(null);
+        $dimensionContent1->getStage()->willReturn('draft');
+        $dimensionContent2 = $this->prophesize(DimensionContentInterface::class);
+        $dimensionContent2->getLocale()->willReturn('de');
+        $dimensionContent2->getStage()->willReturn('draft');
+
+        $attributes = ['locale' => 'de'];
+
+        $dimensionContentCollection = $this->createDimensionContentCollectionInstance([
+            $dimensionContent2->reveal(),
+            $dimensionContent1->reveal(),
+        ], $attributes);
+
+        $this->assertSame([
+            $dimensionContent1->reveal(),
+            $dimensionContent2->reveal(),
+        ], \iterator_to_array($dimensionContentCollection));
+    }
+
     public function testIterator(): void
     {
         $dimensionContent1 = $this->prophesize(DimensionContentInterface::class);