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

Resolve QueryBuilder deprecations #9953

Merged
merged 1 commit into from
Aug 4, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,39 @@
# Upgrade to 3.0

## BC BREAK: Removed `QueryBuilder` methods and constants.

The following `QueryBuilder` constants and methods have been removed:

1. `SELECT`,
2. `DELETE`,
3. `UPDATE`,
4. `STATE_DIRTY`,
5. `STATE_CLEAN`,
6. `getState()`,
7. `getType()`.

## BC BREAK: Omitting only the alias argument for `QueryBuilder::update` and `QueryBuilder::delete` is not supported anymore

When building an UPDATE or DELETE query and when passing a class/type to the function, the alias argument must not be omitted.

### Before

```php
$qb = $em->createQueryBuilder()
->delete('User u')
->where('u.id = :user_id')
->setParameter('user_id', 1);
```

### After

```php
$qb = $em->createQueryBuilder()
->delete('User', 'u')
->where('u.id = :user_id')
->setParameter('user_id', 1);
```

## BC BREAK: Split output walkers and tree walkers

`SqlWalker` and its child classes don't implement the `TreeWalker` interface
Expand Down
12 changes: 12 additions & 0 deletions lib/Doctrine/ORM/Internal/QueryType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Internal;

enum QueryType
{
case Select;
case Delete;
case Update;
}
118 changes: 22 additions & 96 deletions lib/Doctrine/ORM/QueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Internal\QueryType;
use Doctrine\ORM\Query\Expr;
use Doctrine\ORM\Query\Parameter;
use Doctrine\ORM\Query\QueryExpressionVisitor;
Expand Down Expand Up @@ -38,21 +38,6 @@
*/
class QueryBuilder
{
/** @deprecated */
public const SELECT = 0;

/** @deprecated */
public const DELETE = 1;

/** @deprecated */
public const UPDATE = 2;

/** @deprecated */
public const STATE_DIRTY = 0;

/** @deprecated */
public const STATE_CLEAN = 1;

/**
* The array of DQL parts collected.
*
Expand All @@ -70,19 +55,7 @@ class QueryBuilder
'orderBy' => [],
];

/**
* The type of query this is. Can be select, update or delete.
*
* @psalm-var self::SELECT|self::DELETE|self::UPDATE
*/
private int $type = self::SELECT;

/**
* The state of the query object. Can be dirty or clean.
*
* @psalm-var self::STATE_*
*/
private int $state = self::STATE_CLEAN;
private QueryType $type = QueryType::Select;

/**
* The complete DQL string for this query.
Expand Down Expand Up @@ -240,25 +213,6 @@ public function setCacheMode(int $cacheMode): static
return $this;
}

/**
* Gets the type of the currently built query.
*
* @deprecated If necessary, track the type of the query being built outside of the builder.
*
* @psalm-return self::SELECT|self::DELETE|self::UPDATE
*/
public function getType(): int
{
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/orm/pull/9945',
'Relying on the type of the query being built is deprecated.'
. ' If necessary, track the type of the query being built outside of the builder.'
);

return $this->type;
}

/**
* Gets the associated EntityManager for this query builder.
*/
Expand All @@ -267,25 +221,6 @@ public function getEntityManager(): EntityManagerInterface
return $this->em;
}

/**
* Gets the state of this query builder instance.
*
* @deprecated The builder state is an internal concern.
*
* @return int Either QueryBuilder::STATE_DIRTY or QueryBuilder::STATE_CLEAN.
* @psalm-return self::STATE_*
*/
public function getState(): int
{
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/orm/pull/9945',
'Relying on the query builder state is deprecated as it is an internal concern.'
);

return $this->state;
}

/**
* Gets the complete DQL string formed by the current specifications of this QueryBuilder.
*
Expand All @@ -298,20 +233,11 @@ public function getState(): int
*/
public function getDQL(): string
{
if ($this->dql !== null && $this->state === self::STATE_CLEAN) {
return $this->dql;
}

$dql = match ($this->type) {
self::DELETE => $this->getDQLForDelete(),
self::UPDATE => $this->getDQLForUpdate(),
self::SELECT => $this->getDQLForSelect(),
return $this->dql ??= match ($this->type) {
QueryType::Select => $this->getDQLForSelect(),
QueryType::Delete => $this->getDQLForDelete(),
QueryType::Update => $this->getDQLForUpdate(),
};

$this->state = self::STATE_CLEAN;
$this->dql = $dql;

return $dql;
}

/**
Expand Down Expand Up @@ -670,7 +596,7 @@ public function add(string $dqlPartName, string|object|array $dqlPart, bool $app
$this->dqlParts[$dqlPartName] = $isMultiple ? [$dqlPart] : $dqlPart;
}

$this->state = self::STATE_DIRTY;
$this->dql = null;

return $this;
}
Expand All @@ -690,7 +616,7 @@ public function add(string $dqlPartName, string|object|array $dqlPart, bool $app
*/
public function select(mixed ...$select): static
{
$this->type = self::SELECT;
$this->type = QueryType::Select;

if ($select === []) {
return $this;
Expand Down Expand Up @@ -733,7 +659,7 @@ public function distinct(bool $flag = true): static
*/
public function addSelect(mixed ...$select): static
{
$this->type = self::SELECT;
$this->type = QueryType::Select;

if ($select === []) {
return $this;
Expand All @@ -760,18 +686,18 @@ public function addSelect(mixed ...$select): static
*/
public function delete(?string $delete = null, ?string $alias = null): static
{
$this->type = self::DELETE;
$this->type = QueryType::Delete;

if (! $delete) {
return $this;
}

if (! $alias) {
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/9733',
'Omitting the alias is deprecated and will throw an exception in Doctrine 3.0.'
);
throw new InvalidArgumentException(sprintf(
'%s(): The alias for entity %s must not be omitted.',
__METHOD__,
$delete
));
}

return $this->add('from', new Expr\From($delete, $alias));
Expand All @@ -795,18 +721,18 @@ public function delete(?string $delete = null, ?string $alias = null): static
*/
public function update(?string $update = null, ?string $alias = null): static
{
$this->type = self::UPDATE;
$this->type = QueryType::Update;

if (! $update) {
return $this;
}

if (! $alias) {
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/9733',
'Omitting the alias is deprecated and will throw an exception in Doctrine 3.0.'
);
throw new InvalidArgumentException(sprintf(
'%s(): The alias for entity %s must not be omitted.',
__METHOD__,
$update
));
}

return $this->add('from', new Expr\From($update, $alias));
Expand Down Expand Up @@ -1386,7 +1312,7 @@ public function resetDQLParts(?array $parts = null): static
public function resetDQLPart(string $part): static
{
$this->dqlParts[$part] = is_array($this->dqlParts[$part]) ? [] : null;
$this->state = self::STATE_DIRTY;
$this->dql = null;

return $this;
}
Expand Down
4 changes: 0 additions & 4 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2062,10 +2062,6 @@
<PossiblyInvalidIterator occurrences="1">
<code>$dqlPart</code>
</PossiblyInvalidIterator>
<PossiblyNullArgument occurrences="2">
<code>$alias</code>
<code>$alias</code>
</PossiblyNullArgument>
</file>
<file src="lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php">
<InvalidReturnStatement occurrences="1">
Expand Down
1 change: 0 additions & 1 deletion psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
<DeprecatedConstant>
<errorLevel type="suppress">
<file name="lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php" />
<file name="lib/Doctrine/ORM/QueryBuilder.php"/>
</errorLevel>
</DeprecatedConstant>
<DeprecatedMethod>
Expand Down
56 changes: 13 additions & 43 deletions tests/Doctrine/Tests/ORM/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use Doctrine\ORM\Cache;
use Doctrine\ORM\Query;
use Doctrine\ORM\Query\Expr\Join;
Expand All @@ -29,8 +28,6 @@
*/
class QueryBuilderTest extends OrmTestCase
{
use VerifyDeprecations;

private EntityManagerMock $entityManager;

protected function setUp(): void
Expand All @@ -52,16 +49,7 @@ public function testSelectSetsType(): void
->delete(CmsUser::class, 'u')
->select('u.id', 'u.username');

self::assertEquals($qb->getType(), QueryBuilder::SELECT);
}

public function testEmptySelectSetsType(): void
{
$qb = $this->entityManager->createQueryBuilder()
->delete(CmsUser::class, 'u')
->select();

self::assertEquals($qb->getType(), QueryBuilder::SELECT);
$this->assertValidQueryBuilder($qb, 'SELECT u.id, u.username FROM Doctrine\Tests\Models\CMS\CmsUser u');
}

public function testDeleteSetsType(): void
Expand All @@ -70,7 +58,7 @@ public function testDeleteSetsType(): void
->from(CmsUser::class, 'u')
->delete();

self::assertEquals($qb->getType(), QueryBuilder::DELETE);
$this->assertValidQueryBuilder($qb, 'DELETE Doctrine\Tests\Models\CMS\CmsUser u');
}

public function testUpdateSetsType(): void
Expand All @@ -79,7 +67,7 @@ public function testUpdateSetsType(): void
->from(CmsUser::class, 'u')
->update();

self::assertEquals($qb->getType(), QueryBuilder::UPDATE);
$this->assertValidQueryBuilder($qb, 'UPDATE Doctrine\Tests\Models\CMS\CmsUser u');
}

public function testSimpleSelect(): void
Expand Down Expand Up @@ -828,21 +816,6 @@ public function testGetEntityManager(): void
self::assertEquals($this->entityManager, $qb->getEntityManager());
}

public function testInitialStateIsClean(): void
{
$qb = $this->entityManager->createQueryBuilder();
self::assertEquals(QueryBuilder::STATE_CLEAN, $qb->getState());
}

public function testAlteringQueryChangesStateToDirty(): void
{
$qb = $this->entityManager->createQueryBuilder()
->select('u')
->from(CmsUser::class, 'u');

self::assertEquals(QueryBuilder::STATE_DIRTY, $qb->getState());
}

public function testSelectWithFuncExpression(): void
{
$qb = $this->entityManager->createQueryBuilder();
Expand Down Expand Up @@ -1281,24 +1254,21 @@ public function testJoin(): void
self::assertSame('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u LEFT JOIN Doctrine\Tests\Models\CMS\CmsArticle a0 INNER JOIN Doctrine\Tests\Models\CMS\CmsArticle a1', $builder->getDQL());
}

public function testUpdateDeprecationMessage(): void
public function testUpdateWithoutAlias(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/9733');

$qb = $this->entityManager->createQueryBuilder()
->update(CmsUser::class . ' u')
->set('u.username', ':username');
$qb = $this->entityManager->createQueryBuilder();

$this->assertValidQueryBuilder($qb, 'UPDATE Doctrine\Tests\Models\CMS\CmsUser u SET u.username = :username');
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Doctrine\ORM\QueryBuilder::update(): The alias for entity Doctrine\Tests\Models\CMS\CmsUser u must not be omitted.');
$qb->update(CmsUser::class . ' u');
}

public function testDeleteDeprecationMessage(): void
public function testDeleteWithoutAlias(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/9733');

$qb = $this->entityManager->createQueryBuilder()
->delete(CmsUser::class . ' u');
$qb = $this->entityManager->createQueryBuilder();

$this->assertValidQueryBuilder($qb, 'DELETE Doctrine\Tests\Models\CMS\CmsUser u ');
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Doctrine\ORM\QueryBuilder::delete(): The alias for entity Doctrine\Tests\Models\CMS\CmsUser u must not be omitted.');
$qb->delete(CmsUser::class . ' u');
}
}