Project

General

Profile

Actions

Bug #90215

open

Getters of class LazyLoadingProxy can't be called by Fluid

Added by Chris no-lastname-given over 4 years ago. Updated over 3 years ago.

Status:
Accepted
Priority:
Must have
Assignee:
-
Category:
Extbase
Target version:
-
Start date:
2020-01-27
Due date:
% Done:

0%

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

Description

I would like to point to an closed issue with the same headline, since the problem still exists: Bug #87651.

In TYPO3 version 9.5.13 (non composer installation), the n:1 relation can't be resolved in fluid templates.


Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Bug #87651: Lazy loading (sometimes) not working in Fluid (only n:1 relations)Closed2019-02-05

Actions
Related to TYPO3 Core - Bug #87899: Magic getters stop to be evaluated in Fluid starting from TYPO3 9Closed2019-03-13

Actions
Actions #1

Updated by Chris no-lastname-given over 4 years ago

  • Related to Bug #87651: Lazy loading (sometimes) not working in Fluid (only n:1 relations) added
Actions #2

Updated by Wolfgang Klinger over 4 years ago

  • Priority changed from Should have to Must have

I can confirm this in composer mode too … 9.5.13, typo3fluid/fluid 2.6.8

If I use a view helper like

                                    <f:groupedFor groupBy="department" groupKey="department" each="{person.roles}" as="relations">
                                        <f:for each="{relations -> v:iterator.sort(sortBy: 'role.priority', order: 'DESC')}" as="relation">
                                            {relation.role.name}, {relation.department.name}<br>
                                        </f:for>
                                    </f:groupedFor>

{relation.role.name} is output (loaded because of the v:iterator.sort view helper), but {relation.department.name} is not

Actions #3

Updated by Guillaume Germain about 4 years ago

I also see the problem with TYPO3 9.5.13 and typo3fluid/fluid 2.6.9 in Composer mode.

This bug has an impact on performances.

Actions #4

Updated by Alexander Vogt about 4 years ago

We have the same issue (9.5.13, composer mode). Currently we are using a viewhelper as workaround. For example:

<site:lazyLoad model="{relation.department}" />
...
public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext)
{
  if($arguments['model'] instanceof LazyLoadingProxy)
  {
    $arguments['model']->_loadRealInstance();
  }
}
...
Actions #5

Updated by Johannes Rebhan about 4 years ago

We have the same problem. A silly workaround seems to be

<f:if condition="{element.lazy} == null">
... do something with {element.lazy.something}
</f:if>

This seems to trigger the Proxy and initialize the subobject.

Actions #6

Updated by B. Kausch about 4 years ago

That is pretty nasty... can please somebody fix this?

Actions #7

Updated by B. Kausch about 4 years ago

Guillaume Germain wrote:

I also see the problem with TYPO3 9.5.13 and typo3fluid/fluid 2.6.9 in Composer mode.

This bug has an impact on performances.

Same problem here. Have found the bug: https://github.com/TYPO3/Fluid/issues/513

It only occurs in composer mode when the LazyLoadingProxy is a standard/root variable.

Actions #8

Updated by B. Kausch about 4 years ago

  • Subject changed from Lazy loading (sometimes) not working in Fluid (only n:1 relations) to Getters of class LazyLoadingProxy can't be called by Fluid

Maintainer of the Fluid library says, the problem should be fixed inside of the Typo3 core:

As such, the problem is already placed on the implementer instead of the library. The library imposes the restriction that overloaded methods don't work - the implementer then has to provide a formalised way to avoid depending on overloaded methods to do the work. One such method in TYPO3 CMS context is to provide a virtual, secondary getter for methods/properties that would return incompatible overloaded method implementers. The virtual getter would then either forcibly read the data in a way that triggers the overloaded method, or call formalised methods that perform the necessary action (in this case, thaws the lazy storage).

Reasoning behind this: https://github.com/TYPO3/Fluid/pull/486#issuecomment-545661652

Actions #9

Updated by Georg Ringer about 4 years ago

  • Status changed from New to Accepted
Actions #11

Updated by Claus Due about 4 years ago

  • Category changed from Fluid to Extbase

Moving issue to Extbase category to be resolved there.

Suggested compatibility approach: use ArrayAccess on LazyLoadingProxy to trigger the loading and proxy to the thawed object when a property is attempted accessed on the proxy.

See for reference:

https://github.com/TYPO3/Fluid/issues/513#issuecomment-594823546

Actions #12

Updated by Rémy DANIEL about 4 years ago

Maybe I am to naive, but does adding thoses lines to LazyLoadingProxy will do the job?


class LazyLoadingProxy implements \Iterator, LoadingStrategyInterface, \ArrayAccess

// ...

    public function offsetExists($offset)
    {
        return $this->__isset($offset);
    }

    public function offsetGet($offset)
    {
        return $this->__get($offset);
    }

    public function offsetSet($offset, $value)
    {
        return $this->__set($offset, $value);
    }

    public function offsetUnset($offset)
    {
        return $this->__unset($offset);
    }
}

I've added those line on master, and ran successfully Extbase's unit and functional tests.
I've also tested it on a real project, and my "fluid" issue was solved.

In the meantime, would it be okay-ish to use the $this->_loadRealInstance() inside the getters of @lazy properties?

Actions #13

Updated by Markus Gerdes almost 4 years ago

Hi Daniel,
for me your workaround does not work completely.

My LazyLoaded object inherits some properties from its parent class. Those could not be printed whereas the normal properties show up after your workaround.

DANIEL Rémy wrote:

Maybe I am to naive, but does adding thoses lines to LazyLoadingProxy will do the job?

[...]

I've added those line on master, and ran successfully Extbase's unit and functional tests.
I've also tested it on a real project, and my "fluid" issue was solved.

In the meantime, would it be okay-ish to use the $this->_loadRealInstance() inside the getters of @lazy properties?

Actions #14

Updated by Stephan Großberndt over 3 years ago

  • Related to Bug #87899: Magic getters stop to be evaluated in Fluid starting from TYPO3 9 added
Actions #15

Updated by Gerrit Code Review over 3 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/65803

Actions #16

Updated by Stephan Großberndt over 3 years ago

The proposed patch makes AbstractDomainObject implement \ArrayAccess and load the real extbase domain model hidden behind a LazyLoadingProxy on calling offsetGet(). This makes all magic getters of extbase domain models with @lazy annotation available for rendering in Fluid again if they haven't been resolved yet, which was broken since the removal of CmsVariableProvider in TYPO3 v9.0 in https://review.typo3.org/c/Packages/TYPO3.CMS/+/53227

Fluid will only resolve the properties that are actually used in the template, and only once - once resolved, the property value is changed (because LazyLoadingProxy violates visibility and sets it on the parent instance through forced reflection).

This is obviously also broken in v9.5 but will not be fixed there anymore as the version only receives security relevant bugfixes.

This solution also fixes the case of lazy loaded objects inheriting properties from their parent class from https://forge.typo3.org/issues/90215#note-13

In order to reproduce the issue you need:


class \MyVendor\MyExtension\Domain\Model\MyChild extends \MyVendor\MyExtension\Domain\Model\MyParent {
}

class \MyVendor\MyExtension\Domain\Model\MyParent extends \TYPO3\CMS\Extbase\DomainObject\AbstractEntity {

 /**
  * @var \MyVendor\MyExtension\Domain\Model\MyReference
  * @lazy
  */
  protected $myReference;
}

If you put a MyChild instance containing a valid MyReference into a view $this->view->assign('myChild', $myChild); and accessing {myChild.myReference.uid} in a Fluid template this will return an empty uid in Fluid without the patch. With the patch the UID is there.

Actions #17

Updated by Gerrit Code Review over 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/65803

Actions #18

Updated by Gerrit Code Review over 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/65803

Actions #19

Updated by Stephan Großberndt over 3 years ago

  • Status changed from Under Review to Accepted

Patch was abandoned as implementing \ArrayAccess on AbstractDomainObject lead to problems with ObjectAccess and JsonView of Extbase.

Actions #20

Updated by Stephan Großberndt over 3 years ago

Reviving the CmsVariableProvider class from https://review.typo3.org/c/Packages/TYPO3.CMS/+/53227/4/typo3/sysext/fluid/Classes/Core/Variables/CmsVariableProvider.php solves the issue for me in TYPO3 v9 (method resolveSubVariableReferences() is not needed as it is the same in StandardVariableProvider).

But this is a poor workaround only for a TYPO3 v9, which is a version that only receives security updates nowadays, so this is only valuable knowledge for people stuck to v9 at the moment. If you want to upgrade to v10 anyway (which you should), this will not help, as there were changes in the reflection area of Extbase and according to Claus Due this will probably break it - I did not test it though.

The better way is to explicitly resolve the LazyLoadingProxy in your domain model:

Change your model from

class \MyVendor\MyExtension\Domain\Model\MyModel extends \TYPO3\CMS\Extbase\DomainObject\AbstractEntity {

 /**
  * @var \MyVendor\MyExtension\Domain\Model\MyReference
  * @lazy
  */
  protected $myReference;

 /**
  * @return TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy|\MyVendor\MyExtension\Domain\Model\MyReference
  */
  public function getMyReference() {
    return $this->myReference;
  }
}

to

class \MyVendor\MyExtension\Domain\Model\MyModel extends \TYPO3\CMS\Extbase\DomainObject\AbstractEntity {

 /**
  * @var \MyVendor\MyExtension\Domain\Model\MyReference
  * @TYPO3\CMS\Extbase\Annotation\ORM\Lazy
  */
  protected $myReference;

 /**
  * @return \MyVendor\MyExtension\Domain\Model\MyReference
  */
  public function getMyReference() {
    if ($this->myReference instanceof LazyLoadingProxy) {
      $this->myReference->_loadRealInstance();
    }

    return $this->myReference;
  }
}

Now you are able to use strict types on the reference, do not have to care about LazyLoadingProxy anywhere else in your controllers or services and the getters work in Fluid too as if the getter is accessed always the real instance is returned.

This of course means you no longer work with a LazyLoadingProxy at all and do not profit from changes like Allow fetching uid of a LazyLoadingProxy without loading the object first

By the way: This is only an issue with LazyLoadingProxy, NOT with LazyObjectStorage, there no such explicit loading is necessary.

Actions

Also available in: Atom PDF