Bug #88464
openLazyLoadingProxy does not resolve into \SplObjectStorage, resulting in infinite loop (same for `@var array`)
0%
Description
This report is about \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper::mapResultToPropertyValue()
and the magic (__call()
etc) and \Iterator
interface methods of \TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy
:
Preface¶
\SplObjectStorage
used to be buggy, which is why the Extbase ObjectStorage
was created. In currently supported PHP versions \SplObjectStorage
is no longer buggy and it would be time to shed the non-standard Extbase ObjectStorage
.
The Problem¶
Code indicates that \SplObjectStorage
is supported. However, LazyLoadingProxy
fails to map fetched related objects onto an \SplObjectStorage
:
it checks the following:
if (in_array($propertyMetaData['type'], ['array', 'ArrayObject', 'SplObjectStorage', Persistence\ObjectStorage::class], true)) {
...
}
(Note that array
is supported as well, which will lead to the same infinite loop when iterating through the results!)
The method fetches the objects into an array $objects[]
. For a target @var
type ArrayObject
or Extbase Persistence ObjectStorage
, the objects array is boxed into the respective container.
There is no handling for \SplObjectStorage
though, which causes a plain array to be set as the $parentObject
's resolved formerly-lazy property.
When iterating through the \Iterator
, e.g. with a for(... as ...)
loop, the LazyLoadingProxy
calls \TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy::_loadRealInstance()
, to then call current()
, next()
or reset()
on the result.
This works well for object containers, but not for arrays, because PHP returns arrays by value, not by reference .
On each call to $this->_loadRealInstance()
, a new copy of the fetched related objects is made, stored in $realInstance
and current()
, next()
or reset()
called on the copy. These calls do not at all influence the actual real instance.
Solution steps:¶
1) Reference assignments¶
In the LazyLoadingProxy
magic methods (__call()
etc) and \Iterator
interface methods, use reference ("&
") assignments:
$realInstance = &$this->_loadRealInstance();
This step will fix the redirect loop for \@var array
, but we still need to fulfill the @var \SplObjectStorage<Vendor\Ext\Domain\Model\Something>
annotation
2) Handling \SplObjectStorage
property mapping targets¶
In \TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper::mapResultToPropertyValue()
, implement a section for \SplObjectStorage
:
} elseif ($propertyMetaData['type'] === \SplObjectStorage::class) {
$propertyValue = new \SplObjectStorage();
foreach ($objects as $object) {
$propertyValue->attach($object);
}
$propertyValue->_memorizeCleanState();
}
This section should probably share code - except for the constructor - with the Extbase Persistence
ObjectStorage
section.
Note:
Long ago it was tried to base ObjectStorage
on SplObjectStorage
(https://forge.typo3.org/issues/63989), but that was abandoned: https://review.typo3.org/c/Packages/TYPO3.CMS/+/35585
I believe it is time to decide:¶
1)
Drop ObjectStorage
and switch Extbase to the native SplObjectStorage
2)
Or: Drop support for SplObjectStorage
and only support ObjectStorage
(then what about ArrayObject
and array
?)
3)
Or: Properly support SplObjectStorage
and array
like ObjectStorage
and ArrayObject
are supported. (solution indicated above)