From 3037bc3748f367dfa5daa9b9b77e76f78b5c6099 Mon Sep 17 00:00:00 2001 From: Harald Witt <harald.witt@dpfa.de> Date: Mon, 20 Feb 2023 10:35:12 +0100 Subject: [PATCH] [BUGFIX] Typo3DbQueryParser allows records with no child Typo3DbQueryParser ignored records with no child records in some cases. This is mitigated by adding a correct WHERE condition for the match-field/parent evaluation. This effects TCA relations with MM_match_fields, foreign_match_fields or foreign_table_field. Resolves: #93337 Releases: main, 12.4, 11.5 Change-Id: Id4804ccb93252be32666ba74aa7c03dab25e6a92 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79145 Tested-by: core-ci <typo3@b13.com> Tested-by: Georg Ringer <georg.ringer@gmail.com> Reviewed-by: Georg Ringer <georg.ringer@gmail.com> --- Build/phpstan/phpstan-baseline.neon | 2 +- .../Generic/Storage/Typo3DbQueryParser.php | 55 ++++++++++--------- .../Functional/Persistence/Fixtures/blogs.csv | 1 + .../Persistence/QueryParserTest.php | 17 ++++++ .../Functional/Persistence/WorkspaceTest.php | 7 ++- 5 files changed, 51 insertions(+), 31 deletions(-) diff --git a/Build/phpstan/phpstan-baseline.neon b/Build/phpstan/phpstan-baseline.neon index 05c8f3804e35..8cd7d4b4e263 100644 --- a/Build/phpstan/phpstan-baseline.neon +++ b/Build/phpstan/phpstan-baseline.neon @@ -2892,7 +2892,7 @@ parameters: - message: "#^Method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\QueryInterface\\:\\:logicalOr\\(\\) invoked with 2 parameters, 1 required\\.$#" - count: 2 + count: 3 path: ../../typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php - diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php index 2e9b037c1a9d..eb6bbe366bf1 100644 --- a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php +++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php @@ -341,14 +341,14 @@ class Typo3DbQueryParser $relationTableName = (string)$columnMap->getRelationTableName(); $queryBuilderForSubselect = $this->queryBuilder->getConnection()->createQueryBuilder(); $queryBuilderForSubselect - ->select($columnMap->getParentKeyFieldName()) - ->from($relationTableName) - ->where( - $queryBuilderForSubselect->expr()->eq( - $columnMap->getChildKeyFieldName(), - $this->queryBuilder->createNamedParameter($value) - ) - ); + ->select($columnMap->getParentKeyFieldName()) + ->from($relationTableName) + ->where( + $queryBuilderForSubselect->expr()->eq( + $columnMap->getChildKeyFieldName(), + $this->queryBuilder->createNamedParameter($value) + ) + ); $additionalWhereForMatchFields = $this->getAdditionalMatchFieldsStatement($queryBuilderForSubselect->expr(), $columnMap, $relationTableName, $relationTableName); if ($additionalWhereForMatchFields) { $queryBuilderForSubselect->andWhere($additionalWhereForMatchFields); @@ -368,14 +368,14 @@ class Typo3DbQueryParser // Build the SQL statement of the subselect $queryBuilderForSubselect = $this->queryBuilder->getConnection()->createQueryBuilder(); $queryBuilderForSubselect - ->select($parentKeyFieldName) - ->from($childTableName) - ->where( - $queryBuilderForSubselect->expr()->eq( - 'uid', - (int)$value - ) - ); + ->select($parentKeyFieldName) + ->from($childTableName) + ->where( + $queryBuilderForSubselect->expr()->eq( + 'uid', + (int)$value + ) + ); // Add it to the main query return $this->queryBuilder->expr()->eq( @@ -997,40 +997,41 @@ class Typo3DbQueryParser if ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_ONE) { if (isset($parentKeyFieldName)) { // @todo: no test for this part yet - $joinConditionExpression = $this->queryBuilder->expr()->eq( + $basicJoinCondition = $this->queryBuilder->expr()->eq( $tableName . '.uid', $this->queryBuilder->quoteIdentifier($childTableAlias . '.' . $parentKeyFieldName) ); } else { - $joinConditionExpression = $this->queryBuilder->expr()->eq( + $basicJoinCondition = $this->queryBuilder->expr()->eq( $tableName . '.' . $columnName, $this->queryBuilder->quoteIdentifier($childTableAlias . '.uid') ); } - $this->queryBuilder->leftJoin($tableName, $childTableName, $childTableAlias, $joinConditionExpression); - $this->unionTableAliasCache[] = $childTableAlias; - $this->queryBuilder->andWhere( + $joinConditionExpression = $this->queryBuilder->expr()->and( + $basicJoinCondition, $this->getAdditionalMatchFieldsStatement($this->queryBuilder->expr(), $columnMap, $childTableAlias, $realTableName) ); + $this->queryBuilder->leftJoin($tableName, $childTableName, $childTableAlias, (string)$joinConditionExpression); + $this->unionTableAliasCache[] = $childTableAlias; } elseif ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_MANY) { - // @todo: no tests for this part yet if (isset($parentKeyFieldName)) { - $joinConditionExpression = $this->queryBuilder->expr()->eq( + $basicJoinCondition = $this->queryBuilder->expr()->eq( $tableName . '.uid', $this->queryBuilder->quoteIdentifier($childTableAlias . '.' . $parentKeyFieldName) ); } else { - $joinConditionExpression = $this->queryBuilder->expr()->inSet( + $basicJoinCondition = $this->queryBuilder->expr()->inSet( $tableName . '.' . $columnName, $this->queryBuilder->quoteIdentifier($childTableAlias . '.uid'), true ); } - $this->queryBuilder->leftJoin($tableName, $childTableName, $childTableAlias, $joinConditionExpression); - $this->unionTableAliasCache[] = $childTableAlias; - $this->queryBuilder->andWhere( + $joinConditionExpression = $this->queryBuilder->expr()->and( + $basicJoinCondition, $this->getAdditionalMatchFieldsStatement($this->queryBuilder->expr(), $columnMap, $childTableAlias, $realTableName) ); + $this->queryBuilder->leftJoin($tableName, $childTableName, $childTableAlias, (string)$joinConditionExpression); + $this->unionTableAliasCache[] = $childTableAlias; $this->suggestDistinctQuery = true; } elseif ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_AND_BELONGS_TO_MANY) { $relationTableName = (string)$columnMap->getRelationTableName(); diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.csv b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.csv index 205149f0a821..59f9a676dbc8 100644 --- a/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.csv +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Fixtures/blogs.csv @@ -6,6 +6,7 @@ tx_blogexample_domain_model_blog,,,,,,,,,,,,,, ,4,0,Blog4Hidden,"Blog4 Description",,,0,1,0,0,0,0,0,1 ,5,0,Blog5Deleted,"Blog5 Description",,,1,1,3,0,0,0,0,0 ,6,0,Blog6Hidden,"Blog6 Description",,,0,1,0,0,0,0,0,1 +,7,0,BlogNoPosts,"Blog with w/o posts",,,0,0,0,0,0,0,0,0 ,101,0,"WorkspaceOverlay Blog1","WorkspaceOverlay Blog1 Description",,,0,10,0,2,1,0,1,0 ,102,0,"WorkspaceOverlay Blog6Enabled","WorkspaceOverlay Blog6 Description",,,0,1,0,1,6,0,1,0 ,103,0,"WorkspaceOverlay Blog2HiddenInWorkspace","WorkspaceOverlay Blog2HiddenInWorkspace Description",,,0,1,0,0,2,0,1,1 diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php index 038883cd35e7..e4b9cd9ea695 100644 --- a/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/QueryParserTest.php @@ -17,7 +17,9 @@ declare(strict_types=1); namespace TYPO3\CMS\Extbase\Tests\Functional\Persistence; +use ExtbaseTeam\BlogExample\Domain\Model\Blog; use ExtbaseTeam\BlogExample\Domain\Repository\AdministratorRepository; +use ExtbaseTeam\BlogExample\Domain\Repository\BlogRepository; use ExtbaseTeam\BlogExample\Domain\Repository\PostRepository; use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase; @@ -174,4 +176,19 @@ class QueryParserTest extends FunctionalTestCase $post = $query->matching($query->equals('uid', 1))->execute()->current(); self::assertCount(3, $post->getCategories()); } + + /** + * @test + */ + public function queryForBlogsAndPostsWithNoPostsShowsBlogRecord(): void + { + $blogRepository = $this->get(BlogRepository::class); + $query = $blogRepository->createQuery(); + /** @var Blog $blog */ + $blog = $query->matching($query->logicalOr( + $query->like('description', '%w/o%'), + $query->like('posts.title', '%w/o%'), + ))->execute()->current(); + self::assertSame(7, $blog->getUid()); + } } diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/WorkspaceTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/WorkspaceTest.php index 681c65dd96f9..300722fd8e1b 100644 --- a/typo3/sysext/extbase/Tests/Functional/Persistence/WorkspaceTest.php +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/WorkspaceTest.php @@ -88,7 +88,7 @@ class WorkspaceTest extends FunctionalTestCase // In workspace all records need to be fetched, thus enableFields is ignored // This means we select even hidden (but not deleted) records for count() - self::assertSame(5, $query->execute()->count()); + self::assertSame(6, $query->execute()->count()); } /** @@ -163,14 +163,15 @@ class WorkspaceTest extends FunctionalTestCase $blogs = $query->execute()->toArray(); - self::assertCount(3, $blogs); + self::assertCount(4, $blogs); // Check first blog was overlaid with workspace preview $firstBlog = array_shift($blogs); self::assertSame(1, $firstBlog->getUid()); self::assertSame('WorkspaceOverlay Blog1', $firstBlog->getTitle()); - // Check last blog was enabled in workspace preview + // Check second-last blog was enabled in workspace preview + array_pop($blogs); $lastBlog = array_pop($blogs); self::assertSame(6, $lastBlog->getUid()); self::assertSame('WorkspaceOverlay Blog6Enabled', $lastBlog->getTitle()); -- GitLab