Skip to content
Closed
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
28 changes: 28 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,34 @@ awareness about deprecated code.
- Use of our low-overhead runtime deprecation API, details:
https://github.com/doctrine/deprecations/

# Upgrade to 4.3

## `ObjectRepository::findBy()` `$orderBy` additionally accepts the `SortDirection` enum

The PHPDoc contract of `Doctrine\Persistence\ObjectRepository::findBy()` now also
accepts the PHP 8.6 `SortDirection` enum (`SortDirection::Ascending` /
`SortDirection::Descending`) for the values of the `$orderBy` parameter, in addition
to the `'asc'`, `'desc'`, `'ASC'` and `'DESC'` literal strings. The native signature
is unchanged and the strings remain accepted.

Callers may now pass `SortDirection` values. Implementations of `ObjectRepository`
that do not already do so must accept the enum and map it to their underlying sort
direction, otherwise a caller passing `SortDirection` will trigger a runtime error.

Support already shipped by downstream packages:

- `doctrine/orm` supports `SortDirection` in `findBy()` and `findOneBy()` starting
with `3.7.0`. Older versions (`3.6.x` and below, including the `2.20.x`/`2.21.x`
LTS lines) only accept strings and will not receive this change; upgrade to
`3.7.0` or later to use the enum.
- `doctrine/mongodb-odm` supports `SortDirection` in `findBy()` starting with
`2.18`. Older versions (`2.17.x` and below) only accept strings and will not
receive this change; upgrade to `2.18` or later. Its query builder `sort()`
already accepts the enum since `2.17.0`, but `findBy()` bypasses the query
builder.

Passing the enum requires PHP 8.6+ at runtime (or `symfony/polyfill-php86`).

# Upgrade to 4.2

## Add `getFieldValue` and `setFieldValue` to `ClassMetadata` implementation
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"doctrine/coding-standard": "^14",
"phpunit/phpunit": "^10.5.58 || ^12",
"symfony/cache": "^4.4 || ^5.4 || ^6.0 || ^7.0 || ^8.0",
"symfony/finder": "^4.4 || ^5.4 || ^6.0 || ^7.0 || ^8.0"
"symfony/finder": "^4.4 || ^5.4 || ^6.0 || ^7.0 || ^8.0",
"symfony/polyfill-php86": "^1.41"
},
"autoload": {
"psr-4": {
Expand Down
7 changes: 4 additions & 3 deletions src/ObjectRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\Persistence;

use SortDirection;
use UnexpectedValueException;

/**
Expand Down Expand Up @@ -38,9 +39,9 @@ public function findAll(): array;
* an UnexpectedValueException if certain values of the sorting or limiting details are
* not supported.
*
* @param array<string, mixed> $criteria
* @param array<string, string>|null $orderBy
* @phpstan-param array<string, 'asc'|'desc'|'ASC'|'DESC'>|null $orderBy
* @param array<string, mixed> $criteria
* @param array<string, string|SortDirection>|null $orderBy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Widening parameter type is a BC break for implementers (as implementers are not allowed to use a more restrictive type based on the Liskov Substitution Principle). So to me, this change must happen only in the 5.0.x branch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right. And even if it doesn't break the PHP code because the type isn't enforced by PHP, we're providing information about a supported type that isn't respected by all older versions of ORM/ODM. Adding a composer conflict rule with older ORM versions would be really dirty.

The solution, therefore, is to accept string|SortDirection in 5.0 and (possibly) remove support for string in 6.0.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a composer conflict rule with older ORM versions would be really dirty.

Indeed. An interface package that would be forced to add conflicts for older versions of implementation packages would be a clear signal that the interface package does not follow semver (assuming implementation packages don't use unbounded constraints of course but rely on semver constraints)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this maybe be achieved with @method?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@greg0ire no. We don't want to add a new method. We want to change the signature of an existing method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, too bad there isn't an annotation to suggest this.

* @phpstan-param array<string, 'asc'|'desc'|'ASC'|'DESC'|SortDirection>|null $orderBy
*
* @return array<int, object> The objects.
* @phpstan-return list<T>
Expand Down
25 changes: 25 additions & 0 deletions tests/ObjectRepositorySortDirectionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Persistence;

use Doctrine\Persistence\ObjectRepository;
use PHPUnit\Framework\TestCase;
use SortDirection;

final class ObjectRepositorySortDirectionTest extends TestCase
{
public function testFindByAcceptsSortDirectionEnumValue(): void
{
$repository = self::createMock(ObjectRepository::class);

$repository
->expects(self::once())
->method('findBy')
->with([], ['created_at' => SortDirection::Descending])
->willReturn([]);

$repository->findBy([], ['created_at' => SortDirection::Descending]);
}
}