Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] initialization pointcut makes other advices not bound to target #425

Closed
l00k opened this issue Aug 7, 2019 · 8 comments
Closed

[Bug] initialization pointcut makes other advices not bound to target #425

l00k opened this issue Aug 7, 2019 · 8 comments
Milestone

Comments

@l00k
Copy link

l00k commented Aug 7, 2019

class MonitorAspect
    implements Aspect
{

    /**
     * @Before("execution(public App\Example->*(*))")
     */
    public function beforeMethodExecution(MethodInvocation $invocation)
    {
        $obj = $invocation->getThis();
        echo 'BEFORE: ',
            is_object($obj) ? get_class($obj) : $obj,
            $invocation->getMethod()->isStatic() ? '::' : '->',
            $invocation->getMethod()->getName(),
            '()',
            ' with arguments: ',
            json_encode($invocation->getArguments()),
            PHP_EOL;
    }

    (... other pointcuts)

    /**
     * @After("initialization(App\Example)")
     */
    public function initialization(ReflectionConstructorInvocation $invocation)
    {
        $obj = $invocation->getThis();
        echo 'INIT: ',
            get_class($invocation->getThis()),
            PHP_EOL;
    }

}

If I add initialization pointcut to aspect it makes only this one executed.
Also class is changed to App\Example__AopProxied (normally in methodInvocation I get App\Example).

For staticinitialization it works ok - nothing wrong happens. But If I add initialization targeting same class as other advices it makes all those advices stop working - only initialization is executed.

@lisachenko
Copy link
Member

Hello, thank you for reporting this.

Unfortunately, initialization joinpoint is one of the most tricky, thus not reliable part of framework and requires to process all files with new keywords, because without extension it's impossible to mock an object initialization properly.

@lisachenko
Copy link
Member

I'll try to have a look into this issue, maybe it isn't so complicated...

@l00k
Copy link
Author

l00k commented Aug 9, 2019

It could be fixed with some workaround - call to other method in constructor.
However I think it would be nice to have it built in.

BTW. Your framework is awesome, thanks for you time!

@l00k
Copy link
Author

l00k commented Aug 11, 2019

I have prepared bugfix for that issue. Pull request created.
#427
However I don't have required knowledge about project to know all related parts that may be influenced with that change.
Better just check my solution and wrote your own, maybe based on my one.

I have tested it in my sandbox project.

Also I see that master branch has lot of changes wiht comparsion to 2.x branch. When do you plan make next release?

@lisachenko
Copy link
Member

I've being checked the code right now. Do you use a construction $value = new self() in your source code?
This can be related to the #415, #420 and #397. I have a guess that two code visitors/transformers just override result of each other, thus can produce invalid source code...

@l00k
Copy link
Author

l00k commented Aug 12, 2019

No, it was used outside of class.
$obj = new Sample();
Just simples possibile - do you mean you can't reproduce?

@lisachenko
Copy link
Member

Just simples possibile - do you mean you can't reproduce?

I've reproduced this. But things are worse when using self keyword with initialization pointcut.

lisachenko added a commit that referenced this issue Aug 13, 2019
* origin/2.x:
  Fix constructor invocation to know about special parent class names, fix #425
@lisachenko lisachenko added this to the 2.3.3 milestone Aug 13, 2019
@lisachenko
Copy link
Member

Please, test 2.x or master branch to confirm that this issue is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants