Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompositeExpression and()/or() factory methods #3864

Merged
merged 1 commit into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ The usage of the `andX()` and `orX()` methods of the `ExpressionBuilder` class h

## 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.
- 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.
- The usage of the `CompositeExpression` constructor has been deprecated. Use the `and()` / `or()` factory methods.

## Deprecated calling `QueryBuilder` methods with an array argument

Expand Down
13 changes: 13 additions & 0 deletions lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\DBAL\Query\Expression;

use Countable;
use function array_merge;
use function count;
use function implode;

Expand Down Expand Up @@ -36,6 +37,8 @@ class CompositeExpression implements Countable
private $parts = [];

/**
* @internal Use the and() / or() factory methods.
*
* @param string $type Instance type of composite expression.
* @param self[]|string[] $parts Composition of expressions to be joined on composite expression.
*/
Expand All @@ -46,6 +49,16 @@ public function __construct($type, array $parts = [])
$this->addMultiple($parts);
}

public static function and($part, ...$parts) : self
{
return new self(self::TYPE_AND, array_merge([$part], $parts));
}

public static function or($part, ...$parts) : self
{
return new self(self::TYPE_OR, array_merge([$part], $parts));
}

/**
* Adds multiple parts to composite expression.
*
Expand Down
5 changes: 2 additions & 3 deletions lib/Doctrine/DBAL/Query/Expression/ExpressionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Doctrine\DBAL\Query\Expression;

use Doctrine\DBAL\Connection;
use function array_merge;
use function func_get_arg;
use function func_get_args;
use function func_num_args;
Expand Down Expand Up @@ -47,7 +46,7 @@ public function __construct(Connection $connection)
*/
public function and($expression, ...$expressions) : CompositeExpression
{
return new CompositeExpression(CompositeExpression::TYPE_AND, array_merge([$expression], $expressions));
return CompositeExpression::and($expression, ...$expressions);
}

/**
Expand All @@ -58,7 +57,7 @@ public function and($expression, ...$expressions) : CompositeExpression
*/
public function or($expression, ...$expressions) : CompositeExpression
{
return new CompositeExpression(CompositeExpression::TYPE_OR, array_merge([$expression], $expressions));
return CompositeExpression::or($expression, ...$expressions);
BenMorel marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
12 changes: 6 additions & 6 deletions lib/Doctrine/DBAL/Query/QueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ public function set($key, $value)
public function where($predicates)
{
if (! (func_num_args() === 1 && $predicates instanceof CompositeExpression)) {
$predicates = new CompositeExpression(CompositeExpression::TYPE_AND, func_get_args());
$predicates = CompositeExpression::and(...func_get_args());
}

return $this->add('where', $predicates);
Expand Down Expand Up @@ -832,7 +832,7 @@ public function andWhere($where)
$where = $where->with(...$args);
} else {
array_unshift($args, $where);
$where = new CompositeExpression(CompositeExpression::TYPE_AND, $args);
$where = CompositeExpression::and(...$args);
}

return $this->add('where', $where, true);
Expand Down Expand Up @@ -865,7 +865,7 @@ public function orWhere($where)
$where = $where->with(...$args);
} else {
array_unshift($args, $where);
$where = new CompositeExpression(CompositeExpression::TYPE_OR, $args);
$where = CompositeExpression::or(...$args);
}

return $this->add('where', $where, true);
Expand Down Expand Up @@ -990,7 +990,7 @@ public function values(array $values)
public function having($having)
{
if (! (func_num_args() === 1 && $having instanceof CompositeExpression)) {
$having = new CompositeExpression(CompositeExpression::TYPE_AND, func_get_args());
$having = CompositeExpression::and(...func_get_args());
}

return $this->add('having', $having);
Expand All @@ -1013,7 +1013,7 @@ public function andHaving($having)
$having = $having->with(...$args);
} else {
array_unshift($args, $having);
$having = new CompositeExpression(CompositeExpression::TYPE_AND, $args);
$having = CompositeExpression::and(...$args);
}

return $this->add('having', $having);
Expand All @@ -1036,7 +1036,7 @@ public function orHaving($having)
$having = $having->with(...$args);
} else {
array_unshift($args, $having);
$having = new CompositeExpression(CompositeExpression::TYPE_OR, $args);
$having = CompositeExpression::or(...$args);
}

return $this->add('having', $having);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class CompositeExpressionTest extends DbalTestCase
{
public function testCount() : void
{
$expr = new CompositeExpression(CompositeExpression::TYPE_OR, ['u.group_id = 1']);
$expr = CompositeExpression::or('u.group_id = 1');

self::assertCount(1, $expr);

Expand All @@ -23,15 +23,15 @@ public function testCount() : void

public function testAdd() : void
{
$expr = new CompositeExpression(CompositeExpression::TYPE_OR, ['u.group_id = 1']);
$expr = CompositeExpression::or('u.group_id = 1');

self::assertCount(1, $expr);

$expr->add(new CompositeExpression(CompositeExpression::TYPE_AND, []));

self::assertCount(1, $expr);

$expr->add(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1']));
$expr->add(CompositeExpression::or('u.user_id = 1'));

self::assertCount(2, $expr);

Expand All @@ -46,16 +46,16 @@ public function testAdd() : void

public function testWith() : void
{
$expr = new CompositeExpression(CompositeExpression::TYPE_OR, ['u.group_id = 1']);
$expr = CompositeExpression::or('u.group_id = 1');

self::assertCount(1, $expr);

// test immutability
$expr->with(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1']));
$expr->with(CompositeExpression::or('u.user_id = 1'));

self::assertCount(1, $expr);

$expr = $expr->with(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1']));
$expr = $expr->with(CompositeExpression::or('u.user_id = 1'));

self::assertCount(2, $expr);

Expand Down Expand Up @@ -106,9 +106,9 @@ public static function provideDataForConvertToString() : iterable
CompositeExpression::TYPE_AND,
[
'u.user = 1',
new CompositeExpression(
CompositeExpression::TYPE_OR,
['u.group_id = 1', 'u.group_id = 2']
CompositeExpression::or(
'u.group_id = 1',
'u.group_id = 2'
),
],
'(u.user = 1) AND ((u.group_id = 1) OR (u.group_id = 2))',
Expand All @@ -117,9 +117,9 @@ public static function provideDataForConvertToString() : iterable
CompositeExpression::TYPE_OR,
[
'u.group_id = 1',
new CompositeExpression(
CompositeExpression::TYPE_AND,
['u.user = 1', 'u.group_id = 2']
CompositeExpression::and(
'u.user = 1',
'u.group_id = 2'
),
],
'(u.group_id = 1) OR ((u.user = 1) AND (u.group_id = 2))',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ public static function provideDataForAnd() : iterable
[
[
'u.user = 1',
new CompositeExpression(
CompositeExpression::TYPE_OR,
['u.group_id = 1', 'u.group_id = 2']
CompositeExpression::or(
'u.group_id = 1',
'u.group_id = 2'
),
],
'(u.user = 1) AND ((u.group_id = 1) OR (u.group_id = 2))',
],
[
[
'u.group_id = 1',
new CompositeExpression(
CompositeExpression::TYPE_AND,
['u.user = 1', 'u.group_id = 2']
CompositeExpression::and(
'u.user = 1',
'u.group_id = 2'
),
],
'(u.group_id = 1) AND ((u.user = 1) AND (u.group_id = 2))',
Expand Down Expand Up @@ -152,19 +152,19 @@ public static function provideDataForOr() : iterable
[
[
'u.user = 1',
new CompositeExpression(
CompositeExpression::TYPE_OR,
['u.group_id = 1', 'u.group_id = 2']
CompositeExpression::or(
'u.group_id = 1',
'u.group_id = 2'
),
],
'(u.user = 1) OR ((u.group_id = 1) OR (u.group_id = 2))',
],
[
[
'u.group_id = 1',
new CompositeExpression(
CompositeExpression::TYPE_AND,
['u.user = 1', 'u.group_id = 2']
CompositeExpression::and(
'u.user = 1',
'u.group_id = 2'
),
],
'(u.group_id = 1) OR ((u.user = 1) AND (u.group_id = 2))',
Expand Down