Project

General

Profile

Actions

Bug #88464

open

LazyLoadingProxy does not resolve into \SplObjectStorage, resulting in infinite loop (same for `@var array`)

Added by Leonie Philine over 5 years ago. Updated over 5 years ago.

Status:
Accepted
Priority:
Should have
Assignee:
-
Category:
Extbase
Start date:
2019-05-30
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
9
PHP Version:
7.2
Tags:
Complexity:
easy
Is Regression:
Sprint Focus:

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)

Actions

Also available in: Atom PDF