Flowpack/jobqueue-doctrine

Private method breaks proxy building

fcool opened this issue · 11 comments

fcool commented

With the new release 3.2.0 proxy building broke in our projects.

Message is obvious:

The Neos\Flow\Aop\Builder\AdvicedMethodInterceptorBuilder cannot build interceptor code for private method Flowpack\JobQueue\Doctrine\Queue\DoctrineQueue::reconnectDatabaseConnection().
Please change the scope to at least protected or adjust the pointcut
expression in the corresponding aspect

We checked all Advices and in theory no advice match the DoctrineQueue-Class. But probably this is related to neos/flow-development-collection#2553.

Never the less: The class should probably be proxy-buildable or get annotated with Proxy(false)

@fcool Thanks for reporting. Do you know, which feature broke the behavior? And can you share the advice that leads to the exception?

fcool commented

Hey Bastian!

It's the private method in reconnectDatabaseConnection - there were no private method in 3.1.x.
There is the problem:
https://github.com/Flowpack/jobqueue-doctrine/compare/3.1.1..3.2.0#diff-b6e7785332c512714b7a2aa138c87369cec5e9c27465f51904db355bd5e57e10R358

Regarding the pointcut - I'm not 100% sure. Removing these four pointcuts cured the compile run again - but is of course no possibility for the project.

* @Flow\Around("within(Vendor\Package\Adapter\AdapterInterface) && method(.*->createPayment())")
* @Flow\Around("within(Vendor\Package\Adapter\AdapterInterface) && method(.*->createResponse())")
* @Flow\Before("setting(Vendor.Package.positionProtectionWithRedirect) && classAnnotatedWith(Vendor\Package\Annotations\PositionProtected) && method(.*->initializeView())")
* @Flow\Around("setting(Vendor.Package.performanceLogging.active) && within(Vendor\Package\Api\RequestInterface) && method(.*->submit())")

As they all start with .* I assume the reason is the same as in neos/flow-development-collection#2553

Thanks for the detailed explanation.
The private method was introduced with #22 – we could of course make that protected to cure the symptoms. But I would much rather like to fix the root cause.. I'll have a look into the .* issue

@fcool I investigated the mentioned issue (see neos/flow-development-collection#2553 (comment)) but I'm not sure if it's really related.

I can't reproduce your issue unfortunately. Here is what I tried:

I expected some exception, but it compiles just fine

fcool commented

I would have expected some exception, too. I'll try again.

But: Unregarding fixing the "Pointcut matches too much" problem, a class which is in the scope of Flows object management (as flow packages are ;) ) they should
a) follow the conventions, so proxies can be build
b) disable the generation of proxies completely, so every author knows, that the author of this class decided to break Flows unique selling point, and does not care for AOP compatibility.

If you violate the Flow compatibility (e.g. by having an Interface with a constructor declaration), you get an exception and can exclude the class explicitly from object management.
Excluding it automagically might be an option, but it would be equally confusing if your pointcut expression didn't work suddenly IMO.

The case you describe above would be that the exception occurs even though you didn't introduce an incompatible class / pointcut

fcool commented

Agreed.
In the "best" world the class would be proxyable, but simple build no proxy method for privates - which, IIRC works usually, so nothing to do there.

In the "best" world the class would be proxyable, but simple build no proxy method for privates

That's exactly what happens. But if a pointcut expression matches a private method, it leads to this exception. Which brings up the question which expression is that?
Could you reproduce the issue with my minimal example?

Can this be closed as "seems to be fixed"?

I'm not 100% sure whether it's fixed, but I would also rather close this for now due to lack of feedback. If we stumble upon it again, we can always re-open

Hey, just in case someone wants to reproduce, I found the annotation that caused the error:
@Flow\AfterThrowing("within(Flowpack\JobQueue\Common\Queue\QueueInterface)")

Changing it so private methods are excluded works:
@Flow\AfterThrowing("within(Flowpack\JobQueue\Common\Queue\QueueInterface) && method((public|protected) .*->.*())")