StandardVariableProvider does not allow calling of magical methods of a standard variable

[benwick]

See: #438

This fix allowed the calling of magical methods of an extracted variable. But when the variable is a standard variable, we still can't call magical methods. See: https://github.com/TYPO3/Fluid/blob/master/src/Core/Variables/StandardVariableProvider.php#L347

Related: https://forge.typo3.org/issues/90215

[benwick]

I have forgotten this: #479

The main problem is that Typo3 uses the LazyLoadingProxy for lazy loaded entity relations. If you save such an proxy in a standard variable you can't use it, because the magical methods are not called. Mh... What a conundrum!

[benwick]

That is the class: https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php

How about you transfer the solving of the problem to the people who uses the library? Maybe prepare an Interface which will say if a method exists once implemented...

if (method_exists($subject, $getter) || ($subject implements GetterExistenceInterface && $subject->hasGetter($getter)))

Just an idea. So the problem could be fixed outside of this lib ...

[NamelessCoder]

Hi @benwick,

The issues listed in #489 explain the full story. The short version is that Fluid will not receive support for overloaded methods since any way these are implemented, either has side effects or depends on (heavy) use of Reflection - and even then, the object state cannot be fully trusted until the method is actually called.

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).

TL;DR: overloaded methods are pretty bad and not adviseble in general, but especially cause severe issues when attempting to determine presence of properties/methods that are only callable through overloaded methods.

[benwick]

I fully understand that you can't solve the problem. But then again, this library is used mainly inside of Typo3 CMS. And it breaks with the pretty widley used LazyLoadingProxy. I can understand, that this isn't your concern. But just to say: "overloaded methods are pretty bad [we just do not support it]" - this can't be the solution for the entire Typo3 environment. My proposal would be to ease the thawing process of proxy objects for the implementer with an interface. It could look like this:

<?php
namespace TYPO3Fluid\Fluid\Core\Variables;

interface ThawableVariableInterface
{
    public function thawVariable();
}

And if anybody is adding a variable to the StandardVariableProvider:

<?php
namespace TYPO3Fluid\Fluid\Core\Variables;

/**
 * Class StandardVariableProvider
 */
class StandardVariableProvider implements VariableProviderInterface
{
    /**
     * Add a variable to the context
     *
     * @param string $identifier Identifier of the variable to add
     * @param mixed $value The variable's value
     * @return void
     * @api
     */
    public function add($identifier, $value)
    {
        if (is_object($value) && $value implements ThawableVariableInterface)
        {
            $value = $value->thawVariable();
        }
        $this->variables[$identifier] = $value;
    }

Or maybe the thawing could be done later, when a value is extracted. It is just a rough proposal. But there would be no severe performance penalty. And you would ease the implementation of the library. And the Typo3 core team could fix a pretty ugly bug ;)

[NamelessCoder]

I still don't think the way to approach this is to add a workaround in Fluid, which further complicates the way variables can be accessed. My suggestion above is meant as a way any developer who creates a model which depends on overloaded methods can implement a formalised method to avoid that dependency for any context (not just Fluid!) which doesn't support overloaded methods. In other words, something you can do today to solve the problem without requiring a change in either Fluid or the TYPO3 CMS core.

It should also be noted here that the problem with TYPO3 CMS's LazyLoadingProxy is not just about overloaded methods. It also violates visibility principles by using Reflection to force access to an otherwise protected property, forcibly setting the value on the parent object when the proxy is thawed. To be completely frank, it really isn't a nice way to solve such a lazy-loading use case and should be reconsidered. I've also personally urged TYPO3 CMS developers (which I am, too) to begin thinking about not depending on overloaded methods and definitely not depending on violations of visibility principles. The other problem, visibility violation, however does not affect Fluid and doesn't necessarily need to be discussed here. I mention it to illustrate that LazyLoadingProxy has not just one, but two major problems associated with it.

There are at least two possible long-term solutions to this that could be adopted by the TYPO3 CMS core, making the models more compatible with any third party context that like Fluid has decided that overloaded methods are not supported:

• The StandardVariableProvider is built in a way that allows it to be replaced by a different implementation. This different implementation can then extract variables in any way it deems sensible - TYPO3 CMS actually used to have such a custom VariableProvider, which has since been removed. The removal happened for two reasons primarily: one, to reduce the number of custom implementations TYPO3 CMS had to deliver in order to integrate with Fluid (for example, reflection-based support for arguments on the render() method were deprecated for the same reason, resulting in the later removal of the ViewHelper base class replacements). And two, the extraction of variables in Fluid were deemed identical to how the VariableProvider in TYPO3 CMS was doing things - however, this was before the problem of overloaded methods was known.

I don't think that restoring this custom implementation of VariableProvider in TYPO3 CMS is the right solution. Ideally the progress would be toward fewer custom implementations, not more. Although it isn't directly related to this current situation, Fluid 3.0 also aims to further reduce the need for such custom implementations by getting rid of caching and template pre-processors (and introducing alternative methods not based on custom implementations).

Then there is a second long-term solution:

• Fluid already supports and will keep supporting ArrayAccess. This means that any object implementing ArrayAccess will actually use array-style variable access as the primary means of extracting properties - if Fluid sees object is implementing ArrayAccess, Fluid will not attempt to resolve various getter/asserter method names and will not check for method presence. This completely avoids the problems described in the issue linked above (which links to another 5-6 similar issues all dealing with the headache surrounding overloaded methods).

This solution is the more viable of the two. It requires no replacement implementations for VariableProvider and also means the object is usable for any other third-party context - and it doesn't require a custom interface with new logic behind it. It also has the benefit of solving the problem with overloaded methods in all versions of Fluid, not just future ones but any past version as well.

The ArrayAccess solution can be readily added to LazyLoadingProxy and trigger precisely the same logic that the overloaded methods will trigger: whenever the proxy is read with $proxy['property'] it would also be thawed, and internally would call the getter method of the "real" object hidden behind the proxy. Functionally identical to $proxy->getProperty() which depends on the overloaded getter method, but without the drawbacks that the overloaded method brings to the table.

Using ArrayAccess is a compromise, though. The lesser of two evils. It still doesn't formalise the actual composition of the object (by being explicit about which properties exist on it) but at least it means that standard checks for whether or not the value exists can be predictable and trustworthy, which neither method_exists nor is_callable can when overloaded methods are used.

So this would be my recommendation - it has less of an impact on TYPO3 CMS (which would not need to expliclty require a version of Fluid in which the new interface you proposed exists), it builds entirely on existing logic and so requires no change in Fluid or any other library. It would result in not only Fluid being able to read those objects, but also any other third-party code which understands ArrayAccess but ignores or disregards overloaded methods (for the same reasons Fluid does).

[benwick]

Thanks for the input. I understand your reasoning behind this. And the idea with the ArrayAccess interface is a solution, which can be implemented fairly easy.