Commit 9bb6b3cb authored by Markus Klein's avatar Markus Klein Committed by Georg Ringer

[BUGFIX] Apply enableFields in JOIN's ON condition

If an OUTER JOIN is requested, the "disabled" rows
of the joined table must be filtered before the join
is executed. Therefore the enableFields conditions
must be applied in the ON condition of the join.

Resolves: #88919
Releases: master, 10.4, 9.5
Change-Id: I996fe4929b2ca9a5dbaa8c5907a4ad6141bd8853
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64937Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: default avatarMartin Kutschker <mkutschker-typo3@yahoo.com>
Tested-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
Reviewed-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Richard Haeser's avatarRichard Haeser <richard@richardhaeser.com>
Reviewed-by: default avatarHenrik Elsner <helsner@dfau.de>
Reviewed-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
parent 373b25b8
......@@ -67,6 +67,9 @@ class Connection extends \Doctrine\DBAL\Connection implements LoggerAwareInterfa
*/
const PARAM_BOOL = \PDO::PARAM_BOOL; // 5
/** @var ExpressionBuilder */
protected $_expr;
/**
* @var array
*/
......@@ -458,4 +461,14 @@ class Connection extends \Doctrine\DBAL\Connection implements LoggerAwareInterfa
return (string)parent::lastInsertId($tableName);
}
/**
* Gets the ExpressionBuilder for the connection.
*
* @return ExpressionBuilder
*/
public function getExpressionBuilder()
{
return $this->_expr;
}
}
......@@ -17,6 +17,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Core\Database\Query;
use Doctrine\DBAL\Driver\Statement;
use Doctrine\DBAL\Platforms\MySqlPlatform;
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Platforms\PostgreSQL94Platform as PostgreSqlPlatform;
......@@ -28,6 +29,7 @@ use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
use TYPO3\CMS\Core\Database\Query\Restriction\DefaultRestrictionContainer;
use TYPO3\CMS\Core\Database\Query\Restriction\LimitToTablesRestrictionContainer;
use TYPO3\CMS\Core\Database\Query\Restriction\QueryRestrictionContainerInterface;
use TYPO3\CMS\Core\Database\Query\Restriction\QueryRestrictionInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
......@@ -70,13 +72,24 @@ class QueryBuilder
*/
protected $additionalRestrictions;
/**
* List of table aliases which are completely ignored
* when generating the table restrictions in the where-clause.
*
* Aliases added here are part of a LEFT/RIGHT JOIN, having
* their restrictions applied in the JOIN's ON condition already.
*
* @var string[]
*/
private $restrictionsAppliedInJoinCondition = [];
/**
* Initializes a new QueryBuilder.
*
* @param Connection $connection The DBAL Connection.
* @param QueryRestrictionContainerInterface $restrictionContainer
* @param \Doctrine\DBAL\Query\QueryBuilder $concreteQueryBuilder
* @param array $additionalRestrictions
* @param QueryRestrictionContainerInterface|null $restrictionContainer
* @param \Doctrine\DBAL\Query\QueryBuilder|null $concreteQueryBuilder
* @param array|null $additionalRestrictions
*/
public function __construct(
Connection $connection,
......@@ -105,6 +118,7 @@ class QueryBuilder
{
foreach ($this->additionalRestrictions as $restrictionClass => $options) {
if (empty($options['disabled'])) {
/** @var QueryRestrictionInterface $restriction */
$restriction = GeneralUtility::makeInstance($restrictionClass);
$restrictionContainer->add($restriction);
}
......@@ -190,7 +204,7 @@ class QueryBuilder
/**
* Executes this query using the bound parameters and their types.
*
* @return \Doctrine\DBAL\Driver\Statement|int
* @return Statement|int
*/
public function execute()
{
......@@ -461,7 +475,7 @@ class QueryBuilder
*
* @param string $delete The table whose rows are subject to the deletion.
* Will be quoted according to database platform automatically.
* @param string $alias The table alias used in the constructed query.
* @param string|null $alias The table alias used in the constructed query.
* Will be quoted according to database platform automatically.
*
* @return QueryBuilder This QueryBuilder instance.
......@@ -481,7 +495,7 @@ class QueryBuilder
* a certain table
*
* @param string $update The table whose rows are subject to the update.
* @param string $alias The table alias used in the constructed query.
* @param string|null $alias The table alias used in the constructed query.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -515,7 +529,7 @@ class QueryBuilder
* given alias, forming a cartesian product with any existing query roots.
*
* @param string $from The table. Will be quoted according to database platform automatically.
* @param string $alias The alias of the table. Will be quoted according to database platform automatically.
* @param string|null $alias The alias of the table. Will be quoted according to database platform automatically.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -535,7 +549,7 @@ class QueryBuilder
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string $alias The alias of the join table.
* @param string $condition The condition for the join.
* @param string|null $condition The condition for the join.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -557,7 +571,7 @@ class QueryBuilder
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string $alias The alias of the join table.
* @param string $condition The condition for the join.
* @param string|null $condition The condition for the join.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -579,12 +593,18 @@ class QueryBuilder
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string $alias The alias of the join table.
* @param string $condition The condition for the join.
* @param string|null $condition The condition for the join.
*
* @return QueryBuilder This QueryBuilder instance.
*/
public function leftJoin(string $fromAlias, string $join, string $alias, string $condition = null): QueryBuilder
{
$condition = $this->expr()->andX(
$condition,
$this->restrictionContainer->buildExpression([$alias ?? $join => $join], $this->expr())
);
$this->restrictionsAppliedInJoinCondition[] = $alias ?? $join;
$this->concreteQueryBuilder->leftJoin(
$this->quoteIdentifier($fromAlias),
$this->quoteIdentifier($join),
......@@ -601,12 +621,27 @@ class QueryBuilder
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string $alias The alias of the join table.
* @param string $condition The condition for the join.
* @param string|null $condition The condition for the join.
*
* @return QueryBuilder This QueryBuilder instance.
*/
public function rightJoin(string $fromAlias, string $join, string $alias, string $condition = null): QueryBuilder
{
$fromTable = $fromAlias;
// find the table belonging to the $fromAlias, if it's an alias at all
foreach ($this->getQueryPart('from') ?: [] as $from) {
if (isset($from['alias']) && $this->unquoteSingleIdentifier($from['alias']) === $fromAlias) {
$fromTable = $this->unquoteSingleIdentifier($from['table']);
break;
}
}
$condition = $this->expr()->andX(
$condition,
$this->restrictionContainer->buildExpression([$fromAlias => $fromTable], $this->expr())
);
$this->restrictionsAppliedInJoinCondition[] = $fromAlias;
$this->concreteQueryBuilder->rightJoin(
$this->quoteIdentifier($fromAlias),
$this->quoteIdentifier($join),
......@@ -804,7 +839,7 @@ class QueryBuilder
* Replaces any previously specified orderings, if any.
*
* @param string $fieldName The fieldName to order by. Will be quoted according to database platform automatically.
* @param string $order The ordering direction. No automatic quoting/escaping.
* @param string|null $order The ordering direction. No automatic quoting/escaping.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -819,7 +854,7 @@ class QueryBuilder
* Adds an ordering to the query results.
*
* @param string $fieldName The fieldName to order by. Will be quoted according to database platform automatically.
* @param string $order The ordering direction.
* @param string|null $order The ordering direction.
*
* @return QueryBuilder This QueryBuilder instance.
*/
......@@ -904,7 +939,7 @@ class QueryBuilder
*
* @param mixed $value
* @param int $type
* @param string $placeHolder The name to bind with. The string must start with a colon ':'.
* @param string|null $placeHolder The name to bind with. The string must start with a colon ':'.
*
* @return string the placeholder name used.
*/
......@@ -1129,7 +1164,9 @@ class QueryBuilder
foreach ($this->getQueryPart('from') as $from) {
$tableName = $this->unquoteSingleIdentifier($from['table']);
$tableAlias = isset($from['alias']) ? $this->unquoteSingleIdentifier($from['alias']) : $tableName;
$queriedTables[$tableAlias] = $tableName;
if (!in_array($tableAlias, $this->restrictionsAppliedInJoinCondition, true)) {
$queriedTables[$tableAlias] = $tableName;
}
}
// Loop through all JOIN tables
......@@ -1137,7 +1174,9 @@ class QueryBuilder
foreach ($joins as $join) {
$tableName = $this->unquoteSingleIdentifier($join['joinTable']);
$tableAlias = isset($join['joinAlias']) ? $this->unquoteSingleIdentifier($join['joinAlias']) : $tableName;
$queriedTables[$tableAlias] = $tableName;
if (!in_array($tableAlias, $this->restrictionsAppliedInJoinCondition, true)) {
$queriedTables[$tableAlias] = $tableName;
}
}
}
......
......@@ -105,10 +105,9 @@ class LimitToTablesRestrictionContainer implements QueryRestrictionContainerInte
$filteredTables = [];
foreach ($this->applicableTableAliases[$name] as $tableAlias) {
if (!isset($queriedTables[$tableAlias])) {
throw new \LogicException(sprintf('Applicable table alias "%s" is not in queried tables', $tableAlias), 1558354033);
if (isset($queriedTables[$tableAlias])) {
$filteredTables[$tableAlias] = $queriedTables[$tableAlias];
}
$filteredTables[$tableAlias] = $queriedTables[$tableAlias];
}
return $filteredTables;
......
......@@ -588,10 +588,13 @@ class QueryBuilderTest extends UnitTestCase
$this->connection->quoteIdentifier('alias')
->shouldBeCalled()
->willReturnArgument(0);
$this->concreteQueryBuilder->leftJoin('fromAlias', 'join', 'alias', null)
$this->concreteQueryBuilder->leftJoin('fromAlias', 'join', 'alias', Argument::cetera())
->shouldBeCalled()
->willReturn($this->subject);
$expressionBuilder = GeneralUtility::makeInstance(ExpressionBuilder::class, $this->connection->reveal());
$this->connection->getExpressionBuilder()->willReturn($expressionBuilder);
$this->subject->leftJoin('fromAlias', 'join', 'alias');
}
......@@ -609,10 +612,15 @@ class QueryBuilderTest extends UnitTestCase
$this->connection->quoteIdentifier('alias')
->shouldBeCalled()
->willReturnArgument(0);
$this->concreteQueryBuilder->rightJoin('fromAlias', 'join', 'alias', null)
$this->concreteQueryBuilder->rightJoin('fromAlias', 'join', 'alias', Argument::cetera())
->shouldBeCalled()
->willReturn($this->subject);
$this->concreteQueryBuilder->getQueryPart('from')->willReturn([]);
$expressionBuilder = GeneralUtility::makeInstance(ExpressionBuilder::class, $this->connection->reveal());
$this->connection->getExpressionBuilder()->willReturn($expressionBuilder);
$this->subject->rightJoin('fromAlias', 'join', 'alias');
}
......@@ -1547,4 +1555,102 @@ class QueryBuilderTest extends UnitTestCase
$subject->execute();
}
/**
* @test
*/
public function restrictionsAreAppliedInJoinConditionForLeftJoins(): void
{
$GLOBALS['TCA']['tt_content']['ctrl'] = $GLOBALS['TCA']['pages']['ctrl'] = [
'delete' => 'deleted',
'enablecolumns' => [
'disabled' => 'hidden',
],
];
$this->connection->quoteIdentifier(Argument::cetera())
->willReturnArgument(0);
$this->connection->quoteIdentifiers(Argument::cetera())
->willReturnArgument(0);
$connectionBuilder = GeneralUtility::makeInstance(
\Doctrine\DBAL\Query\QueryBuilder::class,
$this->connection->reveal()
);
$expressionBuilder = GeneralUtility::makeInstance(ExpressionBuilder::class, $this->connection->reveal());
$this->connection->getExpressionBuilder()->willReturn($expressionBuilder);
$subject = new QueryBuilder(
$this->connection->reveal(),
null,
$connectionBuilder
);
$subject->select('*')
->from('pages')
->leftJoin(
'pages',
'tt_content',
'content',
'pages.uid = content.pid'
)
->where($expressionBuilder->eq('uid', 1));
$this->connection->executeQuery(
'SELECT * FROM pages LEFT JOIN tt_content content ON (pages.uid = content.pid) AND ((content.deleted = 0) AND (content.hidden = 0)) WHERE (uid = 1) AND ((pages.deleted = 0) AND (pages.hidden = 0))',
Argument::cetera()
)->shouldBeCalled();
$subject->execute();
}
/**
* @test
*/
public function restrictionsAreAppliedInJoinConditionForRightJoins(): void
{
$GLOBALS['TCA']['tt_content']['ctrl'] = $GLOBALS['TCA']['pages']['ctrl'] = [
'delete' => 'deleted',
'enablecolumns' => [
'disabled' => 'hidden',
],
];
$this->connection->quoteIdentifier(Argument::cetera())
->willReturnArgument(0);
$this->connection->quoteIdentifiers(Argument::cetera())
->willReturnArgument(0);
$connectionBuilder = GeneralUtility::makeInstance(
\Doctrine\DBAL\Query\QueryBuilder::class,
$this->connection->reveal()
);
$expressionBuilder = GeneralUtility::makeInstance(ExpressionBuilder::class, $this->connection->reveal());
$this->connection->getExpressionBuilder()->willReturn($expressionBuilder);
$subject = new QueryBuilder(
$this->connection->reveal(),
null,
$connectionBuilder
);
$subject->select('*')
->from('tt_content')
->rightJoin(
'tt_content',
'pages',
'pages',
'pages.uid = tt_content.pid'
)
->where($expressionBuilder->eq('uid', 1));
$this->connection->executeQuery(
'SELECT * FROM tt_content RIGHT JOIN pages pages ON (pages.uid = tt_content.pid) AND ((tt_content.deleted = 0) AND (tt_content.hidden = 0)) WHERE (uid = 1) AND ((pages.deleted = 0) AND (pages.hidden = 0))',
Argument::cetera()
)->shouldBeCalled();
$subject->execute();
}
}
......@@ -82,15 +82,4 @@ class LimitToTablesRestrictionContainerTest extends AbstractRestrictionTestCase
$subject->removeByType(DeletedRestriction::class);
$subject->buildExpression(['aTable' => 'aTable', 'bTable' => 'bTable', 'bt' => 'bTable'], $this->expressionBuilder);
}
/**
* @test
*/
public function buildRestrictionsThrowsExceptionWhenGivenAliasIsNotInQueriedTables(): void
{
$this->expectException(\LogicException::class);
$subject = new LimitToTablesRestrictionContainer();
$subject->addForTables(new HiddenRestriction(), ['bt']);
$subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment