From aeabca88902a8a2c1984e0bbb76f895b42a3d5cd Mon Sep 17 00:00:00 2001 From: Benjamin Morel Date: Mon, 27 Jan 2020 11:18:58 +0100 Subject: [PATCH] Add CompositeExpression::with(), deprecate add*() --- UPGRADE.md | 7 +++++ .../Query/Expression/CompositeExpression.php | 26 +++++++++++++++++++ lib/Doctrine/DBAL/Query/QueryBuilder.php | 8 +++--- .../Expression/CompositeExpressionTest.php | 15 +++++++---- .../Expression/ExpressionBuilderTest.php | 12 ++------- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index e6f03cfc34f..8f8a8445a8d 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -4,6 +4,13 @@ The usage of the `andX()` and `orX()` methods of the `ExpressionBuilder` class has been deprecated. Use `and()` and `or()` instead. +# Upgrade to 2.11 + +## Deprecated `CompositeExpression` methods + +The usage of the `add()` and `addMultiple()` methods of the `CompositeExpression` class has been deprecated. Use `with()` instead, which returns a new instance. +In the future, the `add*()` methods will be removed and the class will be effectively immutable. + # Upgrade to 2.10 ## Deprecated `Doctrine\DBAL\Event\ConnectionEventArgs` methods diff --git a/lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php b/lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php index 443d71bc37e..72bc052a990 100644 --- a/lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php +++ b/lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php @@ -49,6 +49,8 @@ public function __construct($type, array $parts = []) /** * Adds multiple parts to composite expression. * + * @deprecated This class will be made immutable. Use with() instead. + * * @param self[]|string[] $parts * * @return \Doctrine\DBAL\Query\Expression\CompositeExpression @@ -65,6 +67,8 @@ public function addMultiple(array $parts = []) /** * Adds an expression to composite expression. * + * @deprecated This class will be made immutable. Use with() instead. + * * @param mixed $part * * @return \Doctrine\DBAL\Query\Expression\CompositeExpression @@ -84,6 +88,28 @@ public function add($part) return $this; } + /** + * Returns a CompositeExpression with the given part added. + * + * This methods returns a new instance. The current instance is unaffected by this method call. + * + * @param self|string ...$parts + */ + public function with(...$parts) : self + { + $that = clone $this; + + foreach ($parts as $part) { + if ($part instanceof self && count($part) === 0) { + continue; + } + + $that->parts[] = $part; + } + + return $that; + } + /** * Retrieves the amount of expressions on composite expression. * diff --git a/lib/Doctrine/DBAL/Query/QueryBuilder.php b/lib/Doctrine/DBAL/Query/QueryBuilder.php index 587e26656ab..58d7062f0a7 100644 --- a/lib/Doctrine/DBAL/Query/QueryBuilder.php +++ b/lib/Doctrine/DBAL/Query/QueryBuilder.php @@ -823,7 +823,7 @@ public function andWhere($where) $where = $this->getQueryPart('where'); if ($where instanceof CompositeExpression && $where->getType() === CompositeExpression::TYPE_AND) { - $where->addMultiple($args); + $where = $where->with(...$args); } else { array_unshift($args, $where); $where = new CompositeExpression(CompositeExpression::TYPE_AND, $args); @@ -856,7 +856,7 @@ public function orWhere($where) $where = $this->getQueryPart('where'); if ($where instanceof CompositeExpression && $where->getType() === CompositeExpression::TYPE_OR) { - $where->addMultiple($args); + $where = $where->with(...$args); } else { array_unshift($args, $where); $where = new CompositeExpression(CompositeExpression::TYPE_OR, $args); @@ -998,7 +998,7 @@ public function andHaving($having) $having = $this->getQueryPart('having'); if ($having instanceof CompositeExpression && $having->getType() === CompositeExpression::TYPE_AND) { - $having->addMultiple($args); + $having = $having->with(...$args); } else { array_unshift($args, $having); $having = new CompositeExpression(CompositeExpression::TYPE_AND, $args); @@ -1021,7 +1021,7 @@ public function orHaving($having) $having = $this->getQueryPart('having'); if ($having instanceof CompositeExpression && $having->getType() === CompositeExpression::TYPE_OR) { - $having->addMultiple($args); + $having = $having->with(...$args); } else { array_unshift($args, $having); $having = new CompositeExpression(CompositeExpression::TYPE_OR, $args); diff --git a/tests/Doctrine/Tests/DBAL/Query/Expression/CompositeExpressionTest.php b/tests/Doctrine/Tests/DBAL/Query/Expression/CompositeExpressionTest.php index 0a8492f795f..222e7b822e8 100644 --- a/tests/Doctrine/Tests/DBAL/Query/Expression/CompositeExpressionTest.php +++ b/tests/Doctrine/Tests/DBAL/Query/Expression/CompositeExpressionTest.php @@ -16,22 +16,22 @@ public function testCount() : void self::assertCount(1, $expr); - $expr->add('u.group_id = 2'); + $expr = $expr->with('u.group_id = 2'); self::assertCount(2, $expr); } - public function testAdd() : void + public function testAddWith() : void { $expr = new CompositeExpression(CompositeExpression::TYPE_OR, ['u.group_id = 1']); self::assertCount(1, $expr); - $expr->add(new CompositeExpression(CompositeExpression::TYPE_AND, [])); + $expr = $expr->with(new CompositeExpression(CompositeExpression::TYPE_AND, [])); self::assertCount(1, $expr); - $expr->add(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1'])); + $expr = $expr->with(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1'])); self::assertCount(2, $expr); @@ -39,7 +39,12 @@ public function testAdd() : void self::assertCount(2, $expr); - $expr->add('u.user_id = 1'); + // test immutability + $expr->with('u.user_id = 1'); + + self::assertCount(2, $expr); + + $expr = $expr->with('u.user_id = 1'); self::assertCount(3, $expr); } diff --git a/tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php b/tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php index 190ff99e4e4..4aca440e03b 100644 --- a/tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php +++ b/tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php @@ -45,11 +45,7 @@ public function testAnd(array $parts, string $expected) : void */ public function testAndX(array $parts, string $expected) : void { - $composite = $this->expr->andX(); - - foreach ($parts as $part) { - $composite->add($part); - } + $composite = $this->expr->andX()->with(...$parts); self::assertEquals($expected, (string) $composite); } @@ -118,11 +114,7 @@ public function testOr(array $parts, string $expected) : void */ public function testOrX(array $parts, string $expected) : void { - $composite = $this->expr->orX(); - - foreach ($parts as $part) { - $composite->add($part); - } + $composite = $this->expr->orX()->with(...$parts); self::assertEquals($expected, (string) $composite); }