diff --git a/UPGRADE.md b/UPGRADE.md index ea3ea406..aca9d251 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,22 @@ 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`. +- `Criteria::orderBy()` accepts `array`, but passing + anything other than `array` is deprecated. + # Upgrade to 2.0 ## BC breaking changes diff --git a/docs/en/expressions.rst b/docs/en/expressions.rst index ef431bd3..0b218d99 100644 --- a/docs/en/expressions.rst +++ b/docs/en/expressions.rst @@ -76,7 +76,7 @@ orderBy Sets the ordering of the result of this Criteria. .. code-block:: php - $criteria->orderBy(['name' => Criteria::ASC]); + $criteria->orderBy(['name' => Order::Ascending]); setFirstResult -------------- diff --git a/src/ArrayCollection.php b/src/ArrayCollection.php index c5041310..c8a36754 100644 --- a/src/ArrayCollection.php +++ b/src/ArrayCollection.php @@ -466,12 +466,12 @@ public function matching(Criteria $criteria) $filtered = array_filter($filtered, $filter); } - $orderings = $criteria->getOrderings(); + $orderings = $criteria->orderings(); if ($orderings) { $next = null; foreach (array_reverse($orderings) as $field => $ordering) { - $next = ClosureExpressionVisitor::sortByField($field, $ordering === Criteria::DESC ? -1 : 1, $next); + $next = ClosureExpressionVisitor::sortByField($field, $ordering === Order::Descending ? -1 : 1, $next); } uasort($filtered, $next); diff --git a/src/Criteria.php b/src/Criteria.php index eb49cbc6..6959aa9e 100644 --- a/src/Criteria.php +++ b/src/Criteria.php @@ -19,12 +19,15 @@ */ class Criteria { - final public const ASC = 'ASC'; + /** @deprecated use Order::Ascending instead */ + final public const ASC = 'ASC'; + + /** @deprecated use Order::Descending instead */ final public const DESC = 'DESC'; private static ExpressionBuilder|null $expressionBuilder = null; - /** @var array */ + /** @var array */ private array $orderings = []; private int|null $firstResult = null; @@ -57,7 +60,7 @@ public static function expr() /** * Construct a new Criteria. * - * @param array|null $orderings + * @param array|null $orderings */ public function __construct( private Expression|null $expression = null, @@ -151,9 +154,32 @@ public function getWhereExpression() /** * Gets the current orderings of this Criteria. * + * @deprecated use orderings() instead + * * @return array */ 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, + ); + } + + /** + * Gets the current orderings of this Criteria. + * + * @return array + */ + public function orderings(): array { return $this->orderings; } @@ -161,19 +187,38 @@ public function getOrderings() /** * Sets the ordering of the result of this Criteria. * - * Keys are field and values are the order, being either ASC or DESC. + * Keys are field and values are the order, being a valid Order enum case. * - * @see Criteria::ASC - * @see Criteria::DESC + * @see Order::Ascending + * @see Order::Descending * - * @param array $orderings + * @param array $orderings * * @return $this */ public function orderBy(array $orderings) { $this->orderings = array_map( - static fn (string $ordering): string => strtoupper($ordering) === self::ASC ? self::ASC : self::DESC, + 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, ); diff --git a/src/Order.php b/src/Order.php new file mode 100644 index 00000000..4741bbaa --- /dev/null +++ b/src/Order.php @@ -0,0 +1,11 @@ +matching(new Criteria(null, ['sortField' => Criteria::ASC])) ->toArray(), ); + self::assertSame( + [ + 'object2' => $object2, + 'object1' => $object1, + ], + $collection + ->matching(new Criteria(null, ['sortField' => Order::Ascending])) + ->toArray(), + ); } /** @@ -443,7 +453,7 @@ public function testMultiColumnSortAppliesAllSorts(): void self::assertSame( $expected, $collection - ->matching(new Criteria(null, ['foo' => Criteria::DESC, 'bar' => Criteria::DESC])) + ->matching(new Criteria(null, ['foo' => Order::Descending, 'bar' => Order::Descending])) ->toArray(), ); } diff --git a/tests/CollectionTest.php b/tests/CollectionTest.php index a3115cc6..2717d225 100644 --- a/tests/CollectionTest.php +++ b/tests/CollectionTest.php @@ -9,6 +9,7 @@ use Doctrine\Common\Collections\Criteria; use Doctrine\Common\Collections\Expr\Expression; use Doctrine\Common\Collections\Expr\Value; +use Doctrine\Common\Collections\Order; use RuntimeException; use stdClass; @@ -72,7 +73,7 @@ public function testMatchingOrdering(): void { $this->fillMatchingFixture(); - $col = $this->collection->matching(new Criteria(null, ['foo' => 'DESC'])); + $col = $this->collection->matching(new Criteria(null, ['foo' => Order::Descending])); self::assertInstanceOf(Collection::class, $col); self::assertNotSame($col, $this->collection); diff --git a/tests/CriteriaTest.php b/tests/CriteriaTest.php index be855cc6..8360d640 100644 --- a/tests/CriteriaTest.php +++ b/tests/CriteriaTest.php @@ -8,6 +8,7 @@ use Doctrine\Common\Collections\Expr\Comparison; use Doctrine\Common\Collections\Expr\CompositeExpression; use Doctrine\Common\Collections\ExpressionBuilder; +use Doctrine\Common\Collections\Order; use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use PHPUnit\Framework\TestCase; @@ -25,10 +26,10 @@ public function testCreate(): void public function testConstructor(): void { $expr = new Comparison('field', '=', 'value'); - $criteria = new Criteria($expr, ['foo' => 'ASC'], 10, 20); + $criteria = new Criteria($expr, ['foo' => Order::Ascending], 10, 20); self::assertSame($expr, $criteria->getWhereExpression()); - self::assertSame(['foo' => 'ASC'], $criteria->getOrderings()); + self::assertSame(['foo' => Order::Ascending->value], $criteria->getOrderings()); self::assertSame(10, $criteria->getFirstResult()); self::assertSame(20, $criteria->getMaxResults()); } @@ -38,10 +39,11 @@ public function testDeprecatedNullOffset(): void $expr = new Comparison('field', '=', 'value'); $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/311'); - $criteria = new Criteria($expr, ['foo' => 'ASC'], null, 20); + $criteria = new Criteria($expr, ['foo' => Order::Ascending], null, 20); self::assertSame($expr, $criteria->getWhereExpression()); self::assertSame(['foo' => 'ASC'], $criteria->getOrderings()); + self::assertSame(['foo' => Order::Ascending], $criteria->orderings()); self::assertNull($criteria->getFirstResult()); self::assertSame(20, $criteria->getMaxResults()); } @@ -122,13 +124,39 @@ public function testOrWhereWithoutWhere(): void public function testOrderings(): void { $criteria = Criteria::create() - ->orderBy(['foo' => 'ASC']); + ->orderBy(['foo' => Order::Ascending]); - self::assertEquals(['foo' => 'ASC'], $criteria->getOrderings()); + self::assertEquals(['foo' => Order::Ascending], $criteria->orderings()); } public function testExpr(): void { self::assertInstanceOf(ExpressionBuilder::class, Criteria::expr()); } + + public function testPassingNonOrderEnumToOrderByIsDeprecated(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389'); + $criteria = Criteria::create()->orderBy(['foo' => 'ASC']); + } + + public function testConstructingCriteriaWithNonOrderEnumIsDeprecated(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389'); + $criteria = new Criteria(null, ['foo' => 'ASC']); + } + + public function testUsingOrderEnumIsTheRightWay(): void + { + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389'); + Criteria::create()->orderBy(['foo' => Order::Ascending]); + new Criteria(null, ['foo' => Order::Ascending]); + } + + public function testCallingGetOrderingsIsDeprecated(): void + { + $criteria = Criteria::create()->orderBy(['foo' => Order::Ascending]); + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389'); + $criteria->getOrderings(); + } }