Skip to content

Commit

Permalink
Postpone deprecation of string representation of order
Browse files Browse the repository at this point in the history
In downstream packages that representation is recommended in places
where type hints do not allow using an enum yet, which means we would
have to modify the public API of downstream packages to allow it.

Let us postpone the deprecation until we figure what exactly needs to be
done.
  • Loading branch information
greg0ire committed May 3, 2024
1 parent 98aff5e commit f614f5d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 54 deletions.
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());
}
}

0 comments on commit f614f5d

Please sign in to comment.