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

Mixed use BeanNameAutoProxyCreator and AnnotationAwareAspectJAutoProxyCreator to proxy same bean [SPR-16677] #21218

Closed
spring-issuemaster opened this issue Apr 1, 2018 · 3 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 1, 2018

David opened SPR-16677 and commented

As described in this question. Looking forward to your reply ! Best wishes!


Affects: 4.3.10

Reference URL: https://stackoverflow.com/questions/49558599/mix-use-beannameautoproxycreator-and-annotationawareaspectjautoproxycreator-to-p

Issue Links:

  • #21216 Comprehensively cache annotated methods for interfaces and superclasses
  • #21264 AspectJ execution pointcut does not detect methods in superinterface anymore
  • #21298 AopUtils.getMostSpecificMethod should expose dynamic proxy class methods
  • #21343 AspectJ annotation pointcuts fail to evaluate against interface-based proxies
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 2, 2018

Juergen Hoeller commented

Do the #21216 changes make a difference for your scenario already, as long as you're using CGLIB proxies? You could try the latest 5.0.5.BUILD-SNAPSHOT to find out.

The user class enforcement in AopUtils was primarily motivated by runtime-generated configuration classes and not by proxies...but might help for proxy scenarios as well. And if it does help there, we should indeed consider interface-based double-proxying scenarios as well.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 3, 2018

Juergen Hoeller commented

I've resolved this through ignoring proxy classes in AopUtils.getMostSpecificMethod, always returning a user-level method handle there. This same change also goes nicely with #21216, allowing it to expose the actual target class to MethodMatcher implementations still... just iterating over the user-class methods right away in order to avoid the superclass traversal.

As for double-proxying with interface proxies, this is generally a much tougher issue since semantically the inner proxy needs to expose the corresponding metadata to the next level... and AspectJ annotation pointcuts do not really support interface-based matching to begin with. I recommend the use of target-class proxies for such scenarios for the time being.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 3, 2018

David commented

Thanks for your reply! I can't get the latest 5.0.5.BUILD-SNAPSHOT in maven repo to find out. So I have reviewed the commit . As for my scenario ( double-proxying the target class), I found the crucial code

public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
      Class<?> specificTargetClass = (targetClass != null && !Proxy.isProxyClass(targetClass) ?
       ClassUtils.getUserClass(targetClass) : null);
      Method resolvedMethod = ClassUtils.getMostSpecificMethod(method, specificTargetClass);
                                                                                               // If we are dealing with method with generic parameters, find the original method.	
             return BridgeMethodResolver.findBridgedMethod(resolvedMethod);
                  }		  

If the targetClass is a proxy class generated by CGLIB, then you will use the user class to find the bridged method. So if the annotation is present in the target class (implementation not the interface), the BeanNameAutoProxyCreator and AnnotationAwareAspectJAutoProxyCreator can work together (event thought we can not decide the execution order of the two aspect ).
Thanks for your excellent work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.