Skip to content

Commit a7302ef

Browse files
committed
minor #42135 [DoctrineBridge] Backport workaround for closures (derrabus)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [DoctrineBridge] Backport workaround for closures | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | https://github.com/symfony/symfony/pull/42109/files#r670585475 | License | MIT | Doc PR | N/A Backported from #42109. `DoctrineChoiceLoader` inspects the callable `$value` to check if it's an array where it can extract an `IdReader` from. For 6.0, we want to wrap that callable in a closure. If we do so, that check fails silently, breaking the optimization code here which means that all entities are being loaded (instead of just the selected one). I "fixed" that by extracting the `IdReader` from the closure now, but I really believe `DoctrineChoiceLoader` should not make that kind of assumption. Commits ------- b474c39f78 [DoctrineBridge] Backport workaround for closures
2 parents 27df173 + 7d0f5c4 commit a7302ef

File tree

1 file changed

+12
-3
lines changed

1 file changed

+12
-3
lines changed

Form/ChoiceList/DoctrineChoiceLoader.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,27 @@ protected function doLoadChoicesForValues(array $values, ?callable $value): arra
9191
trigger_deprecation('symfony/doctrine-bridge', '5.1', 'Not defining explicitly the IdReader as value callback when query can be optimized is deprecated. Don\'t pass the IdReader to "%s" or define the "choice_value" option instead.', __CLASS__);
9292
}
9393

94+
$idReader = null;
95+
if (\is_array($value) && $value[0] instanceof IdReader) {
96+
$idReader = $value[0];
97+
} elseif ($value instanceof \Closure && ($rThis = (new \ReflectionFunction($value))->getClosureThis()) instanceof IdReader) {
98+
$idReader = $rThis;
99+
} elseif ($legacy) {
100+
$idReader = $this->idReader;
101+
}
102+
94103
// Optimize performance in case we have an object loader and
95104
// a single-field identifier
96-
if (($legacy || \is_array($value) && $this->idReader === $value[0]) && $this->objectLoader) {
105+
if ($idReader && $this->objectLoader) {
97106
$objects = [];
98107
$objectsById = [];
99108

100109
// Maintain order and indices from the given $values
101110
// An alternative approach to the following loop is to add the
102111
// "INDEX BY" clause to the Doctrine query in the loader,
103112
// but I'm not sure whether that's doable in a generic fashion.
104-
foreach ($this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values) as $object) {
105-
$objectsById[$this->idReader->getIdValue($object)] = $object;
113+
foreach ($this->objectLoader->getEntitiesByIds($idReader->getIdField(), $values) as $object) {
114+
$objectsById[$idReader->getIdValue($object)] = $object;
106115
}
107116

108117
foreach ($values as $i => $id) {

0 commit comments

Comments
 (0)