doctrine / doctrine/collections
ClosureExpressionVisitor::sortByField() fails when comparing Objects/Dates
- Dominant language
- PHP
- Stars
- 6k
- Forks
- 187
- PR merge metrics
- No merged PRs in 30d
Description
In method `ClosureExpressionVisitor::sortByField()` we have the following sort function...
```
return function ($a, $b) use ($name, $next, $orientation) {
$aValue = ClosureExpressionVisitor::getObjectFieldValue($a, $name);
$bValue = ClosureExpressionVisitor::getObjectFieldValue($b, $name);
if ($aValue === $bValue) {
return $next($a, $b);
}
return (($aValue > $bValue) ? 1 : -1) * $orientation;
};
```
... which does not work with Objects, especially DateTime objects. I do not know if it makes sense to compare objects in general if they are greater or lesser, but it definitely makes sense for DateTime objects.
The problem is, that equal DateTime objects will not be recognised by `$aValue === $bValue`. Instead `$aValue > $bValue` will evaluate to false in these cases. Not a big deal you may think, as the a element is put below the b element, which is fine, as they are equal. That's right, but $next will neither be executed.
In my use case, sorting by date is the first order criteria, sorting by ID is the second. That code will never evaluate the second criteria, though.
Possible fix:
```
if ($aValue instanceof \DateTime && $aValue == $bValue || $aValue === $bValue) {
return $next($a, $b);
}
```
You have to judge whether it makes sense to compare other object types. For DateTimes however, this is necessary, I suppose. Sorting by date with a second order by criteria is not possible otherwise.
Please have a look into the matter.
See https://github.com/doctrine/doctrine2/issues/6712 for original issue and PR demonstrating it.
Contributor guide
Research direction
Start by locating ClosureExpressionVisitor::sortByField() and reproduce the comparator with equal DateTime values and a secondary ID criterion. Check the existing comparison behavior and verify that equal dates delegate to $next so the secondary ordering is applied.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100