Allow private property access

[jakzal]

It's a slight inconvenience that an AOP framework imposes an implementation on me by forcing my properties to be at least protected.

I'd like to initialise values of annotated properties:

class FindByAspect implements Aspect
{
    /**
     * @Before("@access(Zalas\Annotation\FindBy)")
     */
    public function beforeFieldAccess(FieldAccess $fieldAccess): void
    {
        if (null === $fieldAccess->getValue() && FieldAccess::READ === $fieldAccess->getAccessType()) {
            $field = $fieldAccess->getField()->name;
            $setter = \Closure::bind(function ($object, $value) use ($field) {
                $object->$field = $value;
            }, null, $fieldAccess->getThis());
            $setter($fieldAccess->getThis(), $this->getValue($fieldAccess));
        }
    }

    // ...
}

class Foo
{
    /**
     * @FindBy(xpath="//div")
     */
    private $bar;

    public function getBar(): string
    {
        return $this->bar;
    }
}

The above code works only if the property is protected or public.

[TheCelavi]

Hi, it is more complex than that, actually. GoAOP uses proxy classes to do its weaving in runtime, and proxying is done via inheritance.

If you are familiar with Java and Spring AOP - similar limitations apply (although, in Spring AOP, proxies are implemented as wrappers, not via inheritance - due to language support).

When weaving is done in such matter that no proxy is used (original class is modified/weaved) - then access/weaving of private members is possible (like AspectJ).

Alexander implemented proxy weaving because one of the important feature of any AOP framework must be usage of debugger, and that is possible only if proxies are used, or some kind of source map is used when original class is compiled/weaved (like in AspectJ, but, AFAIK - no debugging FW for PHP has support for source map).

I can provide you with more information and/or pointers.

However, I have to close this one, since, this is known limitation of GoAOP and proxy weaving approach in general.

[jakzal]

You closed it a bit prematurely perhaps. I created this ticket on @lisachenko's request: http://disq.us/p/1zkpwg8

It is possible to update the parent's private property in the proxy class:

<?php

class Foo
{
    private $bar;
    
    public function getBar()
    {
        return $this->bar;
    }
}

class FooProxy extends Foo
{
    public function getBar()
    {
        $this->init();
        return parent::getBar();
    }
    
    private function init()
    {
        \Closure::bind(function (Foo $foo, $value) {
            $foo->bar = $value;
        }, null, \get_parent_class($this))($this, 'bar42');
    }
}

$foo = new FooProxy();
var_dump($foo->getBar());

[TheCelavi]

Oh, sorry - yes, you are right! I totally forgot about \Closure::bind. Sorry.

Unfortunately - this only solves issue with private properties, weaving private methods via proxies is still impossible.

[jakzal]

weaving private methods via proxies is still impossible

I think this is a different (not as desired) use case.

[TheCelavi]

Oh, dude, I am so sorry - I was giving answers thinking that issue is in regards to weaving. OMFG.

[lisachenko]

@jakzal Hi there! ) So, your example https://3v4l.org/pOkj4 looking close to my expected version of private properties interception, but it isn't implemented yet.

Starting points are PropertyInterceptionTrait with InterceptedConstructorGenerator. Maybe there are some extra checks in AdviceMatcher

        if ($filterKind & Aop\PointFilter::KIND_PROPERTY) {
            $mask = ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED;
            //...
        }

[lisachenko]

You can also define a privileged advice via PointcutBuilder and pure Closure with scope rebinding.

To have an idea how to do this, look at this example:

    protected function configureAop(AspectContainer $container)
    {
        $pointcutBuilder = new PointcutBuilder($container);
        $pointcutBuilder->before(
            'within(Demo\Example\LoggingDemo) && execution(public **->*(*))',
            function (MethodInvocation $point) {
                $advice = function ($object) {
                    var_dump(get_object_vars($object));
                };
                ($advice->bindTo($point->getThis(), $point->getMethod()->getDeclaringClass()->name))($point->getThis());
            }
        );
    }

In this case $advice closure will be called in the parent context, thus will be able to unset/change the value of private properties as well.

[lisachenko]

PR for private properties interception is ready #412, please review/test it

[tonix-tuft]

This is an interesting ticket.

I did not fully understand how @lisachenko implemented it and I still have a few knowledge of the GO! AOP internals, but I would like to share a solution I thought about being very humble.

It requires to move the code which unsets the property from the proxy class to the parent __AopProxied one, e.g. taking one of the classes of the demo:

class CacheableDemo__AopProxied {
  
    /**
     * Private prop.
     */
    private $prop;

    public function __construct() { $this->__properties = array('prop' => &$this->prop); unset($this->prop); // <---- This is the code which would normally be in the CacheableDemo proxy class which extends this one.
        $this->example = $example;
    }

}
include_once AOP_CACHE_DIR . '/_proxies/Demo/Example/CacheableDemo.php/Demo/Example/CacheableDemo.php';
include_once AOP_CACHE_DIR . '/_functions/Demo/Example.php';

I know, it looks quite dirty because $this->__properties does not exist on CacheableDemo__AopProxied as the \Go\Proxy\PropertyInterceptionTrait would still be used within the child CacheableDemo proxy class, but it seems to do the job. I also know that this would imply to "pollute" the code of the *__AopProxied class which should be equal to the original ones in order to allow a proper debugging experience throught xdebug during development, that's why I put the code on the same line of the constructor.

I'd like to hear your opinion regarding this workaround just to know what you guys think.

Thank you for the attention!

[lisachenko]

@tonix-tuft Thank you for sharing your thoughts. I can confirm that your solution is possible, but I have a decision not to touch original files as much as possible, because all previous AOP attempts in PHP have failed due to the generated spaghetti instead of more complex but more reliable OOP sour code, based on inheritance and composition.
To sum up: framework should keep original classes untouched (at least, for now).

[lisachenko]

Implemented in #412

[jakzal]

🍻