Skip to content

Commit

Permalink
Introduce Order enum
Browse files Browse the repository at this point in the history
The plan is to ultimately use it and only it when dealing with orderBy()
and getOrderings().
  • Loading branch information
greg0ire committed Feb 17, 2024
1 parent bdba62b commit 456b4ce
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 15 deletions.
2 changes: 1 addition & 1 deletion docs/en/expressions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::ASC]);
setFirstResult
--------------
Expand Down
2 changes: 1 addition & 1 deletion src/ArrayCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ public function matching(Criteria $criteria)
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::DESC->value ? -1 : 1, $next);
}

uasort($filtered, $next);
Expand Down
19 changes: 14 additions & 5 deletions src/Criteria.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Criteria

private static ExpressionBuilder|null $expressionBuilder = null;

/** @var array<string, string> */
/** @var array<string, Order> */
private array $orderings = [];

private int|null $firstResult = null;
Expand Down Expand Up @@ -155,16 +155,19 @@ public function getWhereExpression()
*/
public function getOrderings()
{
return $this->orderings;
return array_map(
static fn (Order $ordering): string => $ordering->value,
$this->orderings,
);
}

/**
* Sets the ordering of the result of this Criteria.
*
* Keys are field and values are the order, being either ASC or DESC.
*
* @see Criteria::ASC
* @see Criteria::DESC
* @see Order::ASC
* @see Order::DESC
*
* @param array<string, string> $orderings
*
Expand All @@ -173,7 +176,13 @@ public function getOrderings()
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) {

Check failure on line 180 in src/Criteria.php

View workflow job for this annotation

GitHub Actions / Static Analysis / Psalm (8.1)

DocblockTypeContradiction

src/Criteria.php:180:21: DocblockTypeContradiction: Cannot resolve types for $ordering - docblock-defined type string does not contain Doctrine\Common\Collections\Order (see https://psalm.dev/155)
return $ordering;
}

return strtoupper($ordering) === Order::ASC->value ? Order::ASC : Order::DESC;

Check warning on line 184 in src/Criteria.php

View check run for this annotation

Codecov / codecov/patch

src/Criteria.php#L184

Added line #L184 was not covered by tests
},
$orderings,
);

Expand Down
11 changes: 11 additions & 0 deletions src/Order.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Doctrine\Common\Collections;

enum Order: string
{
case ASC = 'ASC';
case DESC = 'DESC';
}
5 changes: 3 additions & 2 deletions tests/Common/Collections/BaseArrayCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Order;
use Doctrine\Common\Collections\Selectable;
use PHPUnit\Framework\TestCase;
use stdClass;
Expand Down Expand Up @@ -331,7 +332,7 @@ public function testMatchingWithSortingPreserveKeys(): void
'object1' => $object1,
],
$collection
->matching(new Criteria(null, ['sortField' => Criteria::ASC]))
->matching(new Criteria(null, ['sortField' => Order::ASC]))
->toArray(),
);
}
Expand Down Expand Up @@ -443,7 +444,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::DESC, 'bar' => Order::DESC]))
->toArray(),
);
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Common/Collections/CollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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::DESC]));

self::assertInstanceOf(Collection::class, $col);
self::assertNotSame($col, $this->collection);
Expand Down
11 changes: 6 additions & 5 deletions tests/Common/Collections/CriteriaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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::ASC], 10, 20);

self::assertSame($expr, $criteria->getWhereExpression());
self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
self::assertSame(['foo' => Order::ASC->value], $criteria->getOrderings());
self::assertSame(10, $criteria->getFirstResult());
self::assertSame(20, $criteria->getMaxResults());
}
Expand All @@ -38,7 +39,7 @@ 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::ASC], null, 20);

self::assertSame($expr, $criteria->getWhereExpression());
self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
Expand Down Expand Up @@ -122,9 +123,9 @@ public function testOrWhereWithoutWhere(): void
public function testOrderings(): void
{
$criteria = Criteria::create()
->orderBy(['foo' => 'ASC']);
->orderBy(['foo' => Order::ASC]);

self::assertEquals(['foo' => 'ASC'], $criteria->getOrderings());
self::assertEquals(['foo' => Order::ASC->value], $criteria->getOrderings());
}

public function testExpr(): void
Expand Down

0 comments on commit 456b4ce

Please sign in to comment.