From 763c315545a12cc583c9614ad3404650dbc73749 Mon Sep 17 00:00:00 2001
From: Alexander Schnitzler <git@alexanderschnitzler.de>
Date: Sun, 5 Mar 2023 13:06:40 +0100
Subject: [PATCH] [BUGFIX] Respect TCA field foreign_default_sortby by extbase
Inside the extbase context child relations are always ordered
by the foreign_sortby field if set or by order of creation.
Even if the field foreign_default_sortby is set, it only
impacts the order of the backend view.
This patch brings the order of extbase results inline with
the order of the backend view.
Resolves: #64197
Releases: main, 11.5
Change-Id: I582576b5ac3741e2bbc4107664140fd9a2f63a16
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77985
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
---
.../Persistence/Generic/Mapper/ColumnMap.php | 17 ++
.../Generic/Mapper/DataMapFactory.php | 3 +-
.../Persistence/Generic/Mapper/DataMapper.php | 44 +++-
.../TCA/tx_blogexample_domain_model_post.php | 1 +
.../Persistence/Fixtures/comments.csv | 7 +
.../Generic/Mapper/DataMapperTest.php | 25 ++
.../Generic/Mapper/DataMapperTest.php | 249 ++++++++++++++++++
7 files changed, 343 insertions(+), 3 deletions(-)
create mode 100644 typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/comments.csv
create mode 100644 typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/ColumnMap.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/ColumnMap.php
index 37a7991422db..d21509fc4adc 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/ColumnMap.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/ColumnMap.php
@@ -81,6 +81,13 @@ class ColumnMap
*/
private $childTableName;
+ /**
+ * The name of the fields with direction the results from the child's table are sorted by default
+ *
+ * @see https://docs.typo3.org/m/typo3/reference-tca/main/en-us/ColumnsConfig/Type/Inline/Properties/ForeignDefaultSortby.html
+ */
+ private ?string $childTableDefaultSortings = null;
+
/**
* todo: Check if this property should support null. If not, set default value.
* The name of the field the results from the child's table are sorted by
@@ -224,6 +231,16 @@ class ColumnMap
return $this->childTableName;
}
+ public function setChildTableDefaultSortings(?string $childTableDefaultSortings): void
+ {
+ $this->childTableDefaultSortings = $childTableDefaultSortings;
+ }
+
+ public function getChildTableDefaultSortings(): ?string
+ {
+ return $this->childTableDefaultSortings;
+ }
+
public function setChildSortByFieldName(?string $childSortByFieldName): void
{
$this->childSortByFieldName = $childSortByFieldName;
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapFactory.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapFactory.php
index f9dbaf01f157..84c031326f78 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapFactory.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapFactory.php
@@ -369,7 +369,7 @@ class DataMapFactory implements SingletonInterface
* @param ColumnMap $columnMap The column map
* @param array|null $columnConfiguration The column configuration from $TCA
*/
- protected function setOneToManyRelation(ColumnMap $columnMap, array $columnConfiguration = null): ColumnMap
+ public function setOneToManyRelation(ColumnMap $columnMap, array $columnConfiguration = null): ColumnMap
{
// todo: this method should only be called with proper arguments which means that the TCA integrity check should
// todo: take place outside this method.
@@ -381,6 +381,7 @@ class DataMapFactory implements SingletonInterface
}
// todo: don't update column map if value(s) isn't/aren't set.
$columnMap->setChildSortByFieldName($columnConfiguration['foreign_sortby'] ?? null);
+ $columnMap->setChildTableDefaultSortings($columnConfiguration['foreign_default_sortby'] ?? null);
$columnMap->setParentKeyFieldName($columnConfiguration['foreign_field'] ?? null);
$columnMap->setParentTableFieldName($columnConfiguration['foreign_table_field'] ?? null);
if (isset($columnConfiguration['foreign_match_fields']) && is_array($columnConfiguration['foreign_match_fields'])) {
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
index 41d58ab7bbda..8e75b1aa8df5 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
@@ -421,8 +421,8 @@ class DataMapper
$query->getQuerySettings()->setLanguageAspect($languageAspect);
if ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_MANY) {
- if ($columnMap->getChildSortByFieldName() !== null) {
- $query->setOrderings([$columnMap->getChildSortByFieldName() => QueryInterface::ORDER_ASCENDING]);
+ if (null !== $orderings = $this->getOrderingsForColumnMap($columnMap)) {
+ $query->setOrderings($orderings);
}
} elseif ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_AND_BELONGS_TO_MANY) {
$query->setSource($this->getSource($parentObject, $propertyName));
@@ -434,6 +434,46 @@ class DataMapper
return $query;
}
+ /**
+ * Get orderings array for extbase query by columnMap
+ *
+ * @phpstan-return array<non-empty-string, QueryInterface::ORDER_*>|null
+ * @return array<string, string>|null
+ */
+ public function getOrderingsForColumnMap(ColumnMap $columnMap): array|null
+ {
+ if ($columnMap->getChildSortByFieldName() !== null) {
+ return [$columnMap->getChildSortByFieldName() => QueryInterface::ORDER_ASCENDING];
+ }
+
+ if ($columnMap->getChildTableDefaultSortings() === null) {
+ return null;
+ }
+
+ $orderings = [];
+ $fields = QueryHelper::parseOrderBy($columnMap->getChildTableDefaultSortings());
+ foreach ($fields as $field) {
+ $fieldName = $field[0] ?? null;
+ if ($fieldName === null) {
+ continue;
+ }
+
+ if (($fieldOrdering = $field[1] ?? null) === null) {
+ $orderings[$fieldName] = QueryInterface::ORDER_ASCENDING;
+ continue;
+ }
+
+ $fieldOrdering = strtoupper($fieldOrdering);
+ if (!in_array($fieldOrdering, [QueryInterface::ORDER_ASCENDING, QueryInterface::ORDER_DESCENDING], true)) {
+ $orderings[$fieldName] = QueryInterface::ORDER_ASCENDING;
+ continue;
+ }
+
+ $orderings[$fieldName] = $fieldOrdering;
+ }
+ return $orderings !== [] ? $orderings : null;
+ }
+
/**
* Builds and returns the constraint for multi value properties.
*
diff --git a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_post.php b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_post.php
index 297f613064ae..f11a8a63b346 100644
--- a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_post.php
+++ b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Configuration/TCA/tx_blogexample_domain_model_post.php
@@ -196,6 +196,7 @@ return [
'type' => 'inline',
'foreign_table' => 'tx_blogexample_domain_model_comment',
'foreign_field' => 'post',
+ 'foreign_default_sortby' => 'uid desc',
'size' => 10,
'autoSizeMax' => 30,
'multiple' => 0,
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/comments.csv b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/comments.csv
new file mode 100644
index 000000000000..45ca83f8d01f
--- /dev/null
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/comments.csv
@@ -0,0 +1,7 @@
+tx_blogexample_domain_model_comment,,,,,
+,uid,pid,post,author,content,date
+,1,0,1,1,Comment1,"2023-01-05 00:00:00"
+,2,0,1,2,Comment2,"2023-01-06 00:00:00"
+,3,0,1,1,Comment3,"2023-02-23 00:00:00"
+,4,0,1,3,Comment4,"2023-02-25 00:00:00"
+,5,0,1,2,Comment5,"2023-03-08 00:00:00"
diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php
index bbb752a96fec..a68a7003e61d 100644
--- a/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php
+++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php
@@ -18,6 +18,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Extbase\Tests\Functional\Persistence\Generic\Mapper;
use ExtbaseTeam\BlogExample\Domain\Model\Blog;
+use ExtbaseTeam\BlogExample\Domain\Model\Comment;
use ExtbaseTeam\BlogExample\Domain\Model\DateExample;
use ExtbaseTeam\BlogExample\Domain\Model\DateTimeImmutableExample;
use ExtbaseTeam\BlogExample\Domain\Model\Post;
@@ -485,4 +486,28 @@ class DataMapperTest extends FunctionalTestCase
self::assertSame(1, $plainValue);
}
+
+ /**
+ * @test
+ */
+ public function fetchRelatedRespectsForeignDefaultSortByTCAConfiguration(): void
+ {
+ // Arrange
+ $this->importCSVDataSet('typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/posts.csv');
+ $this->importCSVDataSet('typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/comments.csv');
+
+ $dataMapper = $this->get(DataMapper::class);
+
+ $post = new Post();
+ $post->_setProperty('uid', 1);
+
+ // Act
+ $comments = $dataMapper->fetchRelated($post, 'comments', '5', false)->toArray();
+
+ // Assert
+ self::assertSame(
+ [5, 4, 3, 2, 1], // foreign_default_sortby is set to uid desc, see
+ array_map(fn (Comment $comment) => $comment->getUid(), $comments)
+ );
+ }
}
diff --git a/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php
new file mode 100644
index 000000000000..b086f2b24e7d
--- /dev/null
+++ b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php
@@ -0,0 +1,249 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Extbase\Tests\Unit\Persistence\Generic\Mapper;
+
+use Psr\EventDispatcher\EventDispatcherInterface;
+use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Extbase\Configuration\ConfigurationManager;
+use TYPO3\CMS\Extbase\Persistence\ClassesConfiguration;
+use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap;
+use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapFactory;
+use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
+use TYPO3\CMS\Extbase\Persistence\Generic\Qom\QueryObjectModelFactory;
+use TYPO3\CMS\Extbase\Persistence\Generic\QueryFactory;
+use TYPO3\CMS\Extbase\Persistence\Generic\Session;
+use TYPO3\CMS\Extbase\Persistence\QueryInterface;
+use TYPO3\CMS\Extbase\Reflection\ReflectionService;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+class DataMapperTest extends UnitTestCase
+{
+ protected ColumnMap $columnMap;
+ protected DataMapFactory $dataMapFactory;
+ protected DataMapper $dataMapper;
+
+ protected function setUp(): void
+ {
+ parent::setUp();
+
+ $this->columnMap = new ColumnMap('foo', 'foo');
+
+ $this->dataMapFactory = new DataMapFactory(
+ $this->createMock(ReflectionService::class),
+ $this->createMock(ConfigurationManager::class),
+ $this->createMock(CacheManager::class),
+ $this->createMock(ClassesConfiguration::class),
+ 'foo'
+ );
+
+ $this->dataMapper = new DataMapper(
+ $this->createMock(ReflectionService::class),
+ $this->createMock(QueryObjectModelFactory::class),
+ $this->createMock(Session::class),
+ $this->dataMapFactory,
+ $this->createMock(QueryFactory::class),
+ $this->createMock(EventDispatcherInterface::class),
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function getOrderingsForColumnMapReturnsNullIfNeitherForeignSortByNorForeignDefaultSortByAreSet(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertNull($orderings);
+ }
+
+ /**
+ * @test
+ */
+ public function getOrderingsForColumnMapReturnsNullIfForeignDefaultSortByIsEmpty(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_default_sortby' => '',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertNull($orderings);
+ }
+
+ /**
+ * @test
+ */
+ public function getOrderingsForColumnMapFallBackToAscendingOrdering(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_default_sortby' => 'pid invalid',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['pid' => QueryInterface::ORDER_ASCENDING],
+ $orderings
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function setOneToManyRelationDetectsForeignSortBy(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_sortby' => 'uid',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['uid' => QueryInterface::ORDER_ASCENDING],
+ $orderings
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function setOneToManyRelationDetectsForeignSortByWithForeignDefaultSortBy(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_sortby' => 'uid',
+ 'foreign_default_sortby' => 'pid',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['uid' => QueryInterface::ORDER_ASCENDING],
+ $orderings
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function setOneToManyRelationDetectsForeignDefaultSortByWithoutDirection(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_default_sortby' => 'pid',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['pid' => QueryInterface::ORDER_ASCENDING],
+ $orderings
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function setOneToManyRelationDetectsForeignDefaultSortByWithDirection(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_default_sortby' => 'pid desc',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['pid' => QueryInterface::ORDER_DESCENDING],
+ $orderings
+ );
+ }
+
+ /**
+ * @test
+ */
+ public function setOneToManyRelationDetectsMultipleForeignDefaultSortByWithAndWithoutDirection(): void
+ {
+ // Arrange
+ $this->dataMapFactory->setOneToManyRelation(
+ $this->columnMap,
+ [
+ 'foreign_table' => 'tx_myextension_bar',
+ 'foreign_default_sortby' => 'pid desc, title, uid asc',
+ ]
+ );
+
+ // Act
+ $orderings = $this->dataMapper->getOrderingsForColumnMap($this->columnMap);
+
+ // Assert
+ self::assertSame(
+ ['pid' => QueryInterface::ORDER_DESCENDING, 'title' => QueryInterface::ORDER_ASCENDING, 'uid' => QueryInterface::ORDER_ASCENDING],
+ $orderings
+ );
+ }
+}
--
GitLab