Skip to content

Commit

Permalink
Merge branch '2.15.x' into 3.0.x
Browse files Browse the repository at this point in the history
* 2.15.x:
  Support of NOT expression from doctrine/collections ^2.1 (doctrine#10234)
  Fix Psalm errors with Collection 2.1.2 (doctrine#10343)
  Added warning about query cache in relation to parameters (doctrine#10276)
  • Loading branch information
derrabus committed Dec 28, 2022
2 parents d8a2e32 + ae9fb8c commit 8aa05b8
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 11 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"php": "^8.1",
"composer-runtime-api": "^2",
"ext-ctype": "*",
"doctrine/collections": "^2.0",
"doctrine/collections": "^2.1",
"doctrine/common": "^3.3",
"doctrine/dbal": "^3.5",
"doctrine/deprecations": "^0.5.3 || ^1",
Expand Down
9 changes: 5 additions & 4 deletions docs/en/reference/filters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ table alias of the SQL table of the entity.
In the case of joined or single table inheritance, you always get passed the ClassMetadata of the
inheritance root. This is necessary to avoid edge cases that would break the SQL when applying the filters.

Parameters for the query should be set on the filter object by
``SQLFilter#setParameter()``. Only parameters set via this function can be used
in filters. The ``SQLFilter#getParameter()`` function takes care of the
proper quoting of parameters.
For the filter to correctly function, the following rules must be followed. Failure to do so will lead to unexpected results from the query cache.
1. Parameters for the query should be set on the filter object by ``SQLFilter#setParameter()`` before the filter is used by the ORM ( i.e. do not set parameters inside ``SQLFilter#addFilterConstraint()`` function ).
2. The filter must be deterministic. Don't change the values base on external inputs.

The ``SQLFilter#getParameter()`` function takes care of the proper quoting of parameters.

.. code-block:: php
Expand Down
1 change: 1 addition & 0 deletions docs/en/reference/working-with-associations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ methods:

* ``andX($arg1, $arg2, ...)``
* ``orX($arg1, $arg2, ...)``
* ``not($expression)``
* ``eq($field, $value)``
* ``gt($field, $value)``
* ``lt($field, $value)``
Expand Down
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Persisters/SqlExpressionVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public function walkCompositeExpression(CompositeExpression $expr): string
return match ($expr->getType()) {
CompositeExpression::TYPE_AND => '(' . implode(' AND ', $expressionList) . ')',
CompositeExpression::TYPE_OR => '(' . implode(' OR ', $expressionList) . ')',
CompositeExpression::TYPE_NOT => 'NOT (' . $expressionList[0] . ')',
default => throw new RuntimeException('Unknown composite ' . $expr->getType()),
};
}
Expand Down
6 changes: 6 additions & 0 deletions lib/Doctrine/ORM/Query/QueryExpressionVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use RuntimeException;

use function count;
use function defined;
use function str_replace;
use function str_starts_with;

Expand Down Expand Up @@ -92,6 +93,11 @@ public function walkCompositeExpression(CompositeExpression $expr)
return new Expr\Orx($expressionList);

default:
// Multiversion support for `doctrine/collections` before and after v2.1.0
if (defined(CompositeExpression::class . '::TYPE_NOT') && $expr->getType() === CompositeExpression::TYPE_NOT) {
return $this->expr->not($expressionList[0]);
}

throw new RuntimeException('Unknown composite ' . $expr->getType());
}
}
Expand Down
7 changes: 1 addition & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@
<InvalidFalsableReturnType occurrences="1">
<code>Parameter|null</code>
</InvalidFalsableReturnType>
<LessSpecificReturnStatement occurrences="1">
<code>$queryCacheProfile-&gt;generateCacheKeys($sql, $parameters, $hints)</code>
</LessSpecificReturnStatement>
<MissingClosureParamType occurrences="3">
<code>$alias</code>
<code>$data</code>
<code>$data</code>
</MissingClosureParamType>
<MoreSpecificReturnType occurrences="1">
<code>array{string, string}</code>
</MoreSpecificReturnType>
<PossiblyInvalidArgument occurrences="1">
<code>$stmt</code>
<code>$stmt</code>
</PossiblyInvalidArgument>
<PossiblyNullReference occurrences="2">
<code>getCacheLogger</code>
Expand Down
24 changes: 24 additions & 0 deletions tests/Doctrine/Tests/ORM/Query/QueryExpressionVisitorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use Doctrine\ORM\Query\QueryExpressionVisitor;
use PHPUnit\Framework\TestCase;

use function method_exists;

/**
* Test for QueryExpressionVisitor
*/
Expand Down Expand Up @@ -104,6 +106,28 @@ public function testWalkOrCompositeExpression(): void
self::assertCount(2, $expr->getParts());
}

public function testWalkNotCompositeExpression(): void
{
if (! method_exists(CriteriaBuilder::class, 'not')) {
self::markTestSkipped('doctrine/collections in version ^2.1 is required for this test to run.');
}

$qb = new QueryBuilder();
$cb = new CriteriaBuilder();

$expr = $this->visitor->walkCompositeExpression(
$cb->not(
$cb->eq('foo', 1),
),
);

self::assertInstanceOf(QueryBuilder\Func::class, $expr);
self::assertEquals('NOT', $expr->getName());
self::assertCount(1, $expr->getArguments());
self::assertEquals($qb->eq('o.foo', ':foo'), $expr->getArguments()[0]);
self::assertEquals(new ArrayCollection([new Parameter('foo', 1)]), $this->visitor->getParameters());
}

public function testWalkValue(): void
{
self::assertEquals('value', $this->visitor->walkValue(new Value('value')));
Expand Down
54 changes: 54 additions & 0 deletions tests/Doctrine/Tests/ORM/Query/SqlExpressionVisitorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Query;

use Doctrine\Common\Collections\ExpressionBuilder as CriteriaBuilder;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Persisters\Entity\BasicEntityPersister;
use Doctrine\ORM\Persisters\SqlExpressionVisitor;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

use function method_exists;

class SqlExpressionVisitorTest extends TestCase
{
/** @var SqlExpressionVisitor */
private $visitor;

/** @var BasicEntityPersister&MockObject */
private $persister;
/** @var ClassMetadata */
private $classMetadata;

protected function setUp(): void
{
$this->persister = $this->createMock(BasicEntityPersister::class);
$this->classMetadata = new ClassMetadata('Dummy');
$this->visitor = new SqlExpressionVisitor($this->persister, $this->classMetadata);
}

public function testWalkNotCompositeExpression(): void
{
if (! method_exists(CriteriaBuilder::class, 'not')) {
self::markTestSkipped('doctrine/collections in version ^2.1 is required for this test to run.');
}

$cb = new CriteriaBuilder();

$this->persister
->expects(self::once())
->method('getSelectConditionStatementSQL')
->will(self::returnValue('dummy expression'));

$expr = $this->visitor->walkCompositeExpression(
$cb->not(
$cb->eq('foo', 1),
),
);

self::assertEquals('NOT (dummy expression)', $expr);
}
}

0 comments on commit 8aa05b8

Please sign in to comment.