Skip to content

Commit

Permalink
[BUGFIX] Typo3DbQueryParser allows records with no child
Browse files Browse the repository at this point in the history
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/+/79118
Tested-by: core-ci <typo3@b13.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
  • Loading branch information
haraldwitt authored and georgringer committed May 25, 2023
1 parent 9595c9d commit f9cea7c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 30 deletions.
55 changes: 28 additions & 27 deletions Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -341,14 +341,14 @@ protected function parseComparison(ComparisonInterface $comparison, SourceInterf
$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);
Expand All @@ -368,14 +368,14 @@ protected function parseComparison(ComparisonInterface $comparison, SourceInterf
// 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(
Expand Down Expand Up @@ -993,40 +993,41 @@ protected function addUnionStatement(&$className, &$tableName, &$propertyPath, &
if ($columnMap->getTypeOfRelation() === 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() === 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() === Relation::HAS_AND_BELONGS_TO_MANY) {
$relationTableName = (string)$columnMap->getRelationTableName();
Expand Down
1 change: 1 addition & 0 deletions Tests/Functional/Persistence/Fixtures/blogs.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions Tests/Functional/Persistence/QueryParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
use TYPO3\CMS\Core\Core\SystemEnvironmentBuilder;
use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
use TYPO3Tests\BlogExample\Domain\Model\Blog;
use TYPO3Tests\BlogExample\Domain\Repository\AdministratorRepository;
use TYPO3Tests\BlogExample\Domain\Repository\BlogRepository;
use TYPO3Tests\BlogExample\Domain\Repository\PostRepository;

final class QueryParserTest extends FunctionalTestCase
Expand Down Expand Up @@ -175,4 +177,19 @@ public function queryForPostWithCategoriesReturnsPostWithCategories(): void
$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());
}
}
7 changes: 4 additions & 3 deletions Tests/Functional/Persistence/WorkspaceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function countReturnsCorrectNumberOfBlogs(string $context): void

// 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());
}

/**
Expand Down Expand Up @@ -156,14 +156,15 @@ public function fetchingAllBlogsReturnsCorrectNumberOfBlogs(string $context): vo

$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());
Expand Down

0 comments on commit f9cea7c

Please sign in to comment.