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

Postpone deprecation of string representation of order #403

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

# Upgrade to 2.2

## Deprecated string representation of sort order

Criteria orderings direction is now represented by the
`Doctrine\Common\Collection\Order` enum.

As a consequence:

- `Criteria::ASC` and `Criteria::DESC` are deprecated in favor of
`Order::Ascending` and `Order::Descending`, respectively.
- `Criteria::getOrderings()` is deprecated in favor of `Criteria::orderings()`,
which returns `array<string, Order>`.
- `Criteria::orderBy()` accepts `array<string, string|Order>`, but passing
anything other than `array<string, Order>` is deprecated.

# Upgrade to 2.0

## BC breaking changes
Expand Down
30 changes: 2 additions & 28 deletions src/Criteria.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@
*/
class Criteria
{
/** @deprecated use Order::Ascending instead */
final public const ASC = 'ASC';

/** @deprecated use Order::Descending instead */
final public const ASC = 'ASC';
final public const DESC = 'DESC';

private static ExpressionBuilder|null $expressionBuilder = null;
Expand Down Expand Up @@ -154,20 +151,10 @@ public function getWhereExpression()
/**
* Gets the current orderings of this Criteria.
*
* @deprecated use orderings() instead
*
* @return array<string, string>
*/
public function getOrderings()
{
Deprecation::trigger(
'doctrine/collections',
'https://github.com/doctrine/collections/pull/389',
'Calling %s() is deprecated. Use %s::orderings() instead.',
__METHOD__,
self::class,
);

return array_map(
static fn (Order $ordering): string => $ordering->value,
$this->orderings,
Expand Down Expand Up @@ -200,24 +187,11 @@ public function orderBy(array $orderings)
{
$method = __METHOD__;
$this->orderings = array_map(
static function (string|Order $ordering) use ($method): Order {
static function (string|Order $ordering): Order {
if ($ordering instanceof Order) {
return $ordering;
}

static $triggered = false;

if (! $triggered) {
Deprecation::trigger(
'doctrine/collections',
'https://github.com/doctrine/collections/pull/389',
'Passing non-Order enum values to %s() is deprecated. Pass Order enum values instead.',
$method,
);
}

$triggered = true;

return strtoupper($ordering) === Order::Ascending->value ? Order::Ascending : Order::Descending;
},
$orderings,
Expand Down
22 changes: 12 additions & 10 deletions tests/CriteriaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,29 +134,31 @@ public function testExpr(): void
self::assertInstanceOf(ExpressionBuilder::class, Criteria::expr());
}

public function testPassingNonOrderEnumToOrderByIsDeprecated(): void
public function testPassingNonOrderEnumToOrderByStillWorks(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
$criteria = Criteria::create()->orderBy(['foo' => 'ASC']);

self::assertEquals(['foo' => Order::Ascending], $criteria->orderings());
}

public function testConstructingCriteriaWithNonOrderEnumIsDeprecated(): void
public function testConstructingCriteriaWithNonOrderEnumStillWorks(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
$criteria = new Criteria(null, ['foo' => 'ASC']);

self::assertEquals(['foo' => Order::Ascending], $criteria->orderings());
}

public function testUsingOrderEnumIsTheRightWay(): void
public function testUsingOrderEnumInConstructorWorks(): void
{
$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
Criteria::create()->orderBy(['foo' => Order::Ascending]);
new Criteria(null, ['foo' => Order::Ascending]);
$criteria = new Criteria(null, ['foo' => Order::Ascending]);

self::assertSame(['foo' => Order::Ascending], $criteria->orderings());
}

public function testCallingGetOrderingsIsDeprecated(): void
public function testCallingGetOrderingsStillReturnsAnArrayOfString(): void
{
$criteria = Criteria::create()->orderBy(['foo' => Order::Ascending]);
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
$criteria->getOrderings();
self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
}
}