diff --git a/UPGRADE.md b/UPGRADE.md index 53b8feb8d39..c71f0edc5c4 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -145,6 +145,18 @@ This applies to the following methods: - `Doctrine\ORM\Query\Expr\OrderBy::__construct()` - `Doctrine\ORM\Query\Expr\OrderBy::add()` +This deprecation does NOT apply to `EntityRepository::findBy()` and +`findOneBy()`: since `doctrine/persistence` 3.x and 4.x document strings as +the only order values in `ObjectRepository`, these methods keep accepting +strings (`'ASC'` / `'DESC'`, case-insensitive) without deprecation. Both +strings and `\SortDirection` are accepted; static analysis only allows +`\SortDirection` when the repository is typed as `EntityRepository`, not as +`Doctrine\Persistence\ObjectRepository`. `doctrine/persistence` 5.0 will +also accept `\SortDirection` in `ObjectRepository`, at which point the enum +form will be statically valid regardless of how the repository is typed. +Sort directions given to `matching()` / `Criteria` are also not deprecated +(handled by `doctrine/collections`). + ```diff -$qb->orderBy('u.name', 'ASC') - ->addOrderBy('u.createdAt', 'DESC'); @@ -246,6 +258,13 @@ for extending classes. If you extend the querybuilder and override any of the above methods, you will need to update the method signature to add support for `\SortDirection` as well. Same goes for `Expr\OrderBy::add()`. +`EntityRepository::findBy()` and `findOneBy()` keep their native signature +(`array|null $orderBy`), but their documented `$orderBy` type is widened to +also accept `\SortDirection`. If you extend `EntityRepository` and override +`findBy()` or `findOneBy()`, you will need to update the method docblock to +add support for `\SortDirection` as well. The `ObjectRepository` interface +is unchanged. + ## Conditional breaking changes 3.7 adds support for `doctrine/collections` 3. If you upgrade to that version diff --git a/docs/en/reference/query-builder.rst b/docs/en/reference/query-builder.rst index f85c19742e8..b64c44c8873 100644 --- a/docs/en/reference/query-builder.rst +++ b/docs/en/reference/query-builder.rst @@ -214,8 +214,10 @@ Here is a complete list of helper methods available in ``QueryBuilder``: // Example - $qb->orderBy('u.surname', \SortDirection::Descending) public function orderBy($sort, $order = null); - // Example - $qb->addOrderBy('u.firstName') - public function addOrderBy($sort, $order = null); // Default $order = 'ASC' + // Example - $qb->addOrderBy('u.firstName', \SortDirection::Ascending) + // NOTE: the default is \SortDirection::Ascending when $order is omitted; + // passing strings or null as $order is deprecated + public function addOrderBy($sort, $order = null); } Binding parameters to your query diff --git a/docs/en/reference/working-with-objects.rst b/docs/en/reference/working-with-objects.rst index d780b553941..9cfc78ca694 100644 --- a/docs/en/reference/working-with-objects.rst +++ b/docs/en/reference/working-with-objects.rst @@ -666,6 +666,21 @@ The ``EntityRepository#findBy()`` method additionally accepts orderings, limit a getRepository('MyProject\Domain\User')->findBy(array('age' => 20), array('name' => 'ASC'), 10, 0); +The ``findBy()`` and ``findOneBy()`` methods also accept ``\SortDirection`` +values as sort directions, which can be mixed with ``'ASC'`` / ``'DESC'`` +strings: + +.. code-block:: php + + getRepository('MyProject\Domain\User') + ->findBy(array('age' => 20), array('name' => \SortDirection::Descending), 10, 0); + +Unlike ``QueryBuilder#orderBy()`` and the mapped ``OrderBy`` attribute, +repository methods do not deprecate string sort directions: they must keep +accepting strings while ``doctrine/persistence`` 3.x and 4.x document them as +the only statically valid values. Both forms are therefore supported. + If you pass an array of values Doctrine will convert the query into a WHERE field IN (..) query automatically: .. code-block:: php diff --git a/src/EntityRepository.php b/src/EntityRepository.php index 6d7f86e4073..d5f6582467a 100644 --- a/src/EntityRepository.php +++ b/src/EntityRepository.php @@ -16,6 +16,7 @@ use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\Repository\Exception\InvalidMagicMethodCall; use Doctrine\Persistence\ObjectRepository; +use SortDirection; use function array_slice; use function lcfirst; @@ -102,6 +103,8 @@ public function findAll(): array * * {@inheritDoc} * + * @phpstan-param array|null $orderBy + * * @phpstan-return list */ public function findBy(array $criteria, array|null $orderBy = null, int|null $limit = null, int|null $offset = null): array @@ -115,7 +118,7 @@ public function findBy(array $criteria, array|null $orderBy = null, int|null $li * Finds a single entity by a set of criteria. * * @phpstan-param array $criteria - * @phpstan-param array|null $orderBy + * @phpstan-param array|null $orderBy * * @phpstan-return T|null */ diff --git a/src/Persisters/Entity/EntityPersister.php b/src/Persisters/Entity/EntityPersister.php index c74c75a314e..c0cad0104db 100644 --- a/src/Persisters/Entity/EntityPersister.php +++ b/src/Persisters/Entity/EntityPersister.php @@ -143,21 +143,21 @@ public function getOwningTable(string $fieldName): string; /** * Loads an entity by a list of field criteria. * - * @param mixed[] $criteria The criteria by which to load the entity. - * @param object|null $entity The entity to load the data into. If not specified, - * a new entity is created. - * @param AssociationMapping|null $assoc The association that connects the entity - * to load to another entity, if any. - * @param mixed[] $hints Hints for entity creation. - * @param LockMode|int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants - * or NULL if no specific lock mode should be used - * for loading the entity. - * @param int|null $limit Limit number of results. - * @param string[]|null $orderBy Criteria to order by. - * @phpstan-param array $criteria - * @phpstan-param array $hints - * @phpstan-param LockMode::*|null $lockMode - * @phpstan-param array|null $orderBy + * @param mixed[] $criteria The criteria by which to load the entity. + * @param object|null $entity The entity to load the data into. If not specified, + * a new entity is created. + * @param AssociationMapping|null $assoc The association that connects the entity + * to load to another entity, if any. + * @param mixed[] $hints Hints for entity creation. + * @param LockMode|int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants + * or NULL if no specific lock mode should be used + * for loading the entity. + * @param int|null $limit Limit number of results. + * @param array|null $orderBy Criteria to order by. + * @phpstan-param array $criteria + * @phpstan-param array $hints + * @phpstan-param LockMode::*|null $lockMode + * @phpstan-param array|null $orderBy * * @return object|null The loaded and managed entity instance or NULL if the entity can not be found. * diff --git a/tests/StaticAnalysis/Repository/find-by-sort-direction.php b/tests/StaticAnalysis/Repository/find-by-sort-direction.php new file mode 100644 index 00000000000..a77ea5c02ba --- /dev/null +++ b/tests/StaticAnalysis/Repository/find-by-sort-direction.php @@ -0,0 +1,37 @@ + $repository + * + * @return list + */ + public function findBy(EntityRepository $repository): array + { + return $repository->findBy([], [ + 'name' => SortDirection::Ascending, + 'id' => SortDirection::Descending, + ]); + } + + /** + * @param EntityRepository $repository + * + * @return T|null + */ + public function findOneBy(EntityRepository $repository): object|null + { + return $repository->findOneBy([], [ + 'name' => SortDirection::Descending, + ]); + } +} diff --git a/tests/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Tests/ORM/Functional/EntityRepositoryTest.php index 300bf526031..5dadda38b67 100644 --- a/tests/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Tests/ORM/Functional/EntityRepositoryTest.php @@ -8,6 +8,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Criteria; use Doctrine\DBAL\LockMode; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Exception\ORMException; use Doctrine\ORM\Exception\UnrecognizedIdentifierFields; @@ -27,6 +28,7 @@ use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\Group; +use SortDirection; use function array_values; use function defined; @@ -34,6 +36,8 @@ class EntityRepositoryTest extends OrmFunctionalTestCase { + use VerifyDeprecations; + protected function setUp(): void { $this->useModelSet('cms'); @@ -405,6 +409,7 @@ public function testFindOneByAssociationKey(): void #[Group('DDC-1241')] public function testFindOneByOrderBy(): void { + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/11313'); $this->loadFixture(); $repos = $this->_em->getRepository(CmsUser::class); @@ -414,6 +419,17 @@ public function testFindOneByOrderBy(): void self::assertNotSame($userAsc, $userDesc); } + public function testFindOneByOrderBySortDirection(): void + { + $this->loadFixture(); + + $repos = $this->_em->getRepository(CmsUser::class); + $userAsc = $repos->findOneBy([], ['username' => SortDirection::Ascending]); + $userDesc = $repos->findOneBy([], ['username' => SortDirection::Descending]); + + self::assertNotSame($userAsc, $userDesc); + } + #[Group('DDC-817')] public function testFindByAssociationKey(): void { @@ -489,6 +505,7 @@ public function testFindByLimitOffset(): void #[Group('DDC-1094')] public function testFindByOrderBy(): void { + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/11313'); $this->loadFixture(); $repos = $this->_em->getRepository(CmsUser::class); @@ -501,6 +518,20 @@ public function testFindByOrderBy(): void self::assertSame($usersAsc[3], $usersDesc[0]); } + public function testFindByOrderBySortDirection(): void + { + $this->loadFixture(); + + $repos = $this->_em->getRepository(CmsUser::class); + $usersAsc = $repos->findBy([], ['username' => SortDirection::Ascending]); + $usersDesc = $repos->findBy([], ['username' => SortDirection::Descending]); + + self::assertCount(4, $usersAsc, 'Pre-condition: only four users in fixture'); + self::assertCount(4, $usersDesc, 'Pre-condition: only four users in fixture'); + self::assertSame($usersAsc[0], $usersDesc[3]); + self::assertSame($usersAsc[3], $usersDesc[0]); + } + #[Group('DDC-1376')] public function testFindByOrderByAssociation(): void {