PersistentCollection::matching() merges orderings incorrectly when association has default sorting

[paxuclus]

Bug Report

When PersistentCollection::matching() is called with orderings, they are not correctly merged together with the default sortings of an association.

Q	A
BC Break	yes/no
Version	2.6.4

Summary

When a relationship has a default sorting configured using @ORM\OrderBy({"lastName" = "ASC", "firstName" = "ASC"}), the orderings are not correctly merged together with the orderings of the $criteria in PersistentCollection::matching().

Current behavior

The orderings will be sorted like the array given to the @ORM\OrderBy annotation, leading to unexpected results when passing another orderBy to the $criteria of PersistentCollection::matching().

How to reproduce

• Define a relationship with @ORM\OrderBy({"lastName" = "ASC", "firstName" = "ASC"}) (sort by at least 2 properties)
• Call $relationship->matching(Criteria::create()->orderBy(['firstName' => 'DESC'])) on the PersistentCollection of that relationship

Expected behavior

The collection should be ordered by firstName DESC, lastName ASC, but instead it will be ordered by lastName ASC, firstName DESC.

Origin of the bug

Original Issue: #7767
The bug was introduced in this PR: https://github.com/doctrine/orm/pull/7766/files#diff-108586f774fc8acb02163ed709e05e86R675

Using $criteria->orderBy(array_merge($criteria->getOrderings(), $this->association['orderBy'] ?? [], $criteria->getOrderings())); there would fix the ordering.

[SenseException]

Thank you for reporting this issue. If you like, you can create a PR with a fix and test(s) to this issue.

As far as I understand you describe a regression between two versions/commits for the orderBy behavior. But I'm not sure if there's an intended or defined behavior for a use case like this.

When I read $relationship->matching(Criteria::create()->orderBy(['firstName' => 'DESC'])) shouldn't it override the OrderBy annotation instead of mixing it? I would expect that firstName DESC will be the only order in here.

If you want to create a PR, you should aim to restore the previous known behavior you described, but I wonder if we should change it for the next major release.

[paxuclus]

Hey @SenseException,

thinking about this again I agree with you that "orderBy()" should override the default sorting.

Previously (before #7766 was merged), the "->matching()" would completely ignore the OrderBy annotation, even if there were no orderings in the $criteria.

I think the PersistentCollection should only use the OrderBy annotation if the $criteria does not contain any orderings.

I will try to create a PR this week. 👍

[lcobucci]

Handled by #7850