Skip to content

Commit

Permalink
Address deprecations from Collection 2.2
Browse files Browse the repository at this point in the history
  • Loading branch information
derrabus committed Feb 27, 2024
1 parent 719d007 commit 013726b
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 30 deletions.
5 changes: 5 additions & 0 deletions phpcs.xml.dist
Expand Up @@ -279,4 +279,9 @@
<!-- https://github.com/doctrine/orm/issues/8537 -->
<exclude-pattern>src/QueryBuilder.php</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.PHP.UselessParentheses">
<!-- We need those parentheses to make enum access seem like valid syntax on PHP 7 -->
<exclude-pattern>src/Mapping/Driver/XmlDriver.php</exclude-pattern>
</rule>
</ruleset>
13 changes: 13 additions & 0 deletions phpstan-persistence2.neon
Expand Up @@ -31,6 +31,19 @@ parameters:
message: '/^Instanceof between Doctrine\\DBAL\\Platforms\\AbstractPlatform and Doctrine\\DBAL\\Platforms\\MySQLPlatform will always evaluate to false\.$/'
path: src/Utility/LockSqlHelper.php

# Forward compatibility with Collections 3
-
message: '#^Parameter \$order of anonymous function has invalid type Doctrine\\Common\\Collections\\Order\.$#'
path: src/Internal/CriteriaOrderings.php

-
message: '#^Access to property \$value on an unknown class Doctrine\\Common\\Collections\\Order\.$#'
path: src/Internal/CriteriaOrderings.php

-
message: '#^Call to an undefined method Doctrine\\Common\\Collections\\Criteria\:\:orderings\(\)\.$#'
path: src/Internal/CriteriaOrderings.php

# False positive
-
message: '/^Call to an undefined method Doctrine\\Common\\Cache\\Cache::deleteAll\(\)\.$/'
Expand Down
5 changes: 4 additions & 1 deletion src/Cache/Persister/Entity/AbstractEntityPersister.php
Expand Up @@ -16,6 +16,7 @@
use Doctrine\ORM\Cache\TimestampCacheKey;
use Doctrine\ORM\Cache\TimestampRegion;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Internal\CriteriaOrderings;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\PersistentCollection;
Expand All @@ -30,6 +31,8 @@

abstract class AbstractEntityPersister implements CachedEntityPersister
{
use CriteriaOrderings;

/** @var UnitOfWork */
protected $uow;

Expand Down Expand Up @@ -475,7 +478,7 @@ public function count($criteria = [])
*/
public function loadCriteria(Criteria $criteria)
{
$orderBy = $criteria->getOrderings();
$orderBy = self::getCriteriaOrderings($criteria);
$limit = $criteria->getMaxResults();
$offset = $criteria->getFirstResult();
$query = $this->persister->getSelectSQL($criteria);
Expand Down
54 changes: 54 additions & 0 deletions src/Internal/CriteriaOrderings.php
@@ -0,0 +1,54 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Internal;

use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Order;

use function array_map;
use function class_exists;
use function method_exists;
use function strtoupper;

trait CriteriaOrderings
{
/**
* @return array<string, string>
*
* @psalm-suppress DeprecatedMethod We need to call the deprecated API if the new one does not exist yet.
*/
private static function getCriteriaOrderings(Criteria $criteria): array
{
if (! method_exists(Criteria::class, 'orderings')) {
return $criteria->getOrderings();
}

return array_map(
static function (Order $order): string {
return $order->value;
},
$criteria->orderings()
);
}

/**
* @param array<string, string> $orderings
*
* @return array<string, string>|array<string, Order>
*/
private static function mapToOrderEnumIfAvailable(array $orderings): array
{
if (! class_exists(Order::class)) {
return $orderings;
}

return array_map(
static function (string $order): Order {
return Order::from(strtoupper($order));
},
$orderings
);
}
}
8 changes: 6 additions & 2 deletions src/Mapping/Driver/XmlDriver.php
Expand Up @@ -5,6 +5,7 @@
namespace Doctrine\ORM\Mapping\Driver;

use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Order;
use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\MappingException;
Expand All @@ -16,6 +17,7 @@
use SimpleXMLElement;

use function assert;
use function class_exists;
use function constant;
use function count;
use function defined;
Expand Down Expand Up @@ -481,9 +483,10 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
if (isset($oneToManyElement->{'order-by'})) {
$orderBy = [];
foreach ($oneToManyElement->{'order-by'}->{'order-by-field'} ?? [] as $orderByField) {
/** @psalm-suppress DeprecatedConstant */
$orderBy[(string) $orderByField['name']] = isset($orderByField['direction'])
? (string) $orderByField['direction']
: Criteria::ASC;
: (class_exists(Order::class) ? (Order::Ascending)->value : Criteria::ASC);
}

$mapping['orderBy'] = $orderBy;
Expand Down Expand Up @@ -609,9 +612,10 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
if (isset($manyToManyElement->{'order-by'})) {
$orderBy = [];
foreach ($manyToManyElement->{'order-by'}->{'order-by-field'} ?? [] as $orderByField) {
/** @psalm-suppress DeprecatedConstant */
$orderBy[(string) $orderByField['name']] = isset($orderByField['direction'])
? (string) $orderByField['direction']
: Criteria::ASC;
: (class_exists(Order::class) ? (Order::Ascending)->value : Criteria::ASC);
}

$mapping['orderBy'] = $orderBy;
Expand Down
7 changes: 6 additions & 1 deletion src/PersistentCollection.php
Expand Up @@ -9,6 +9,7 @@
use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Selectable;
use Doctrine\ORM\Internal\CriteriaOrderings;
use Doctrine\ORM\Mapping\ClassMetadata;
use ReturnTypeWillChange;
use RuntimeException;
Expand Down Expand Up @@ -41,6 +42,8 @@
*/
final class PersistentCollection extends AbstractLazyCollection implements Selectable
{
use CriteriaOrderings;

/**
* A snapshot of the collection at the moment it was fetched from the database.
* This is used to create a diff of the collection at commit time.
Expand Down Expand Up @@ -671,7 +674,9 @@ public function matching(Criteria $criteria): Collection

$criteria = clone $criteria;
$criteria->where($expression);
$criteria->orderBy($criteria->getOrderings() ?: $association['orderBy'] ?? []);
$criteria->orderBy(self::mapToOrderEnumIfAvailable(
self::getCriteriaOrderings($criteria) ?: $association['orderBy'] ?? []
));

$persister = $this->getUnitOfWork()->getEntityPersister($association['targetEntity']);

Expand Down
5 changes: 4 additions & 1 deletion src/Persisters/Collection/ManyToManyPersister.php
Expand Up @@ -8,6 +8,7 @@
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Expr\Comparison;
use Doctrine\DBAL\Exception as DBALException;
use Doctrine\ORM\Internal\CriteriaOrderings;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\SqlValueVisitor;
Expand All @@ -30,6 +31,8 @@
*/
class ManyToManyPersister extends AbstractCollectionPersister
{
use CriteriaOrderings;

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -745,7 +748,7 @@ private function expandCriteriaParameters(Criteria $criteria): array

private function getOrderingSql(Criteria $criteria, ClassMetadata $targetClass): string
{
$orderings = $criteria->getOrderings();
$orderings = self::getCriteriaOrderings($criteria);
if ($orderings) {
$orderBy = [];
foreach ($orderings as $name => $direction) {
Expand Down
4 changes: 3 additions & 1 deletion src/Persisters/Entity/BasicEntityPersister.php
Expand Up @@ -15,6 +15,7 @@
use Doctrine\DBAL\Types\Types;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Internal\CriteriaOrderings;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\ORM\Mapping\QuoteStrategy;
Expand Down Expand Up @@ -93,6 +94,7 @@
*/
class BasicEntityPersister implements EntityPersister
{
use CriteriaOrderings;
use LockSqlHelper;

/** @var array<string,string> */
Expand Down Expand Up @@ -884,7 +886,7 @@ public function count($criteria = [])
*/
public function loadCriteria(Criteria $criteria)
{
$orderBy = $criteria->getOrderings();
$orderBy = self::getCriteriaOrderings($criteria);
$limit = $criteria->getMaxResults();
$offset = $criteria->getFirstResult();
$query = $this->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);
Expand Down
27 changes: 14 additions & 13 deletions src/QueryBuilder.php
Expand Up @@ -7,6 +7,7 @@
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Internal\CriteriaOrderings;
use Doctrine\ORM\Query\Expr;
use Doctrine\ORM\Query\Parameter;
use Doctrine\ORM\Query\QueryExpressionVisitor;
Expand Down Expand Up @@ -39,6 +40,8 @@
*/
class QueryBuilder
{
use CriteriaOrderings;

/** @deprecated */
public const SELECT = 0;

Expand Down Expand Up @@ -1375,22 +1378,20 @@ public function addCriteria(Criteria $criteria)
}
}

if ($criteria->getOrderings()) {
foreach ($criteria->getOrderings() as $sort => $order) {
$hasValidAlias = false;
foreach ($allAliases as $alias) {
if (str_starts_with($sort . '.', $alias . '.')) {
$hasValidAlias = true;
break;
}
}

if (! $hasValidAlias) {
$sort = $allAliases[0] . '.' . $sort;
foreach (self::getCriteriaOrderings($criteria) as $sort => $order) {
$hasValidAlias = false;
foreach ($allAliases as $alias) {
if (str_starts_with($sort . '.', $alias . '.')) {
$hasValidAlias = true;
break;
}
}

$this->addOrderBy($sort, $order);
if (! $hasValidAlias) {
$sort = $allAliases[0] . '.' . $sort;
}

$this->addOrderBy($sort, $order);
}

// Overwrite limits only if they was set in criteria
Expand Down
6 changes: 4 additions & 2 deletions tests/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
Expand Up @@ -6,6 +6,7 @@

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Order;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\UnitOfWork;
use Doctrine\Tests\Models\CMS\CmsGroup;
Expand All @@ -14,6 +15,7 @@
use Doctrine\Tests\OrmFunctionalTestCase;

use function assert;
use function class_exists;
use function get_class;

/**
Expand Down Expand Up @@ -436,7 +438,7 @@ public function testManyToManyOrderByIsNotIgnored(): void
$user = $this->_em->find(get_class($user), $user->id);

$criteria = Criteria::create()
->orderBy(['name' => Criteria::ASC]);
->orderBy(['name' => class_exists(Order::class) ? Order::Ascending : Criteria::ASC]);

self::assertEquals(
['A', 'B', 'C', 'Developers_0'],
Expand Down Expand Up @@ -478,7 +480,7 @@ public function testManyToManyOrderByHonorsFieldNameColumnNameAliases(): void
$user = $this->_em->find(get_class($user), $user->id);

$criteria = Criteria::create()
->orderBy(['name' => Criteria::ASC]);
->orderBy(['name' => class_exists(Order::class) ? Order::Ascending : Criteria::ASC]);

self::assertEquals(
['A', 'B', 'C'],
Expand Down
11 changes: 8 additions & 3 deletions tests/Tests/ORM/Functional/Ticket/GH7767Test.php
Expand Up @@ -6,6 +6,8 @@

use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\Order;
use Doctrine\Common\Collections\Selectable;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
Expand All @@ -16,6 +18,7 @@
use Doctrine\Tests\OrmFunctionalTestCase;

use function assert;
use function class_exists;

/** @group GH7767 */
class GH7767Test extends OrmFunctionalTestCase
Expand Down Expand Up @@ -53,7 +56,9 @@ public function testMatchingOverrulesCollectionOrdering(): void
$parent = $this->_em->find(GH7767ParentEntity::class, 1);
assert($parent instanceof GH7767ParentEntity);

$children = $parent->getChildren()->matching(Criteria::create()->orderBy(['position' => 'DESC']));
$children = $parent->getChildren()->matching(
Criteria::create()->orderBy(['position' => class_exists(Order::class) ? Order::Descending : 'DESC'])
);

self::assertEquals(300, $children[0]->position);
self::assertEquals(200, $children[1]->position);
Expand All @@ -73,7 +78,7 @@ class GH7767ParentEntity
private $id;

/**
* @psalm-var Collection<int, GH7767ChildEntity>
* @psalm-var Collection<int, GH7767ChildEntity>&Selectable<int, GH7767ChildEntity>
* @OneToMany(targetEntity=GH7767ChildEntity::class, mappedBy="parent", fetch="EXTRA_LAZY", cascade={"persist"})
* @OrderBy({"position" = "ASC"})
*/
Expand All @@ -84,7 +89,7 @@ public function addChild(int $position): void
$this->children[] = new GH7767ChildEntity($this, $position);
}

/** @psalm-return Collection<int, GH7767ChildEntity> */
/** @psalm-return Collection<int, GH7767ChildEntity>&Selectable<int, GH7767ChildEntity> */
public function getChildren(): Collection
{
return $this->children;
Expand Down

0 comments on commit 013726b

Please sign in to comment.