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

Simplify hint registration for Spring AOP proxies #28745

Closed
sbrannen opened this issue Jul 2, 2022 · 7 comments
Closed

Simplify hint registration for Spring AOP proxies #28745

sbrannen opened this issue Jul 2, 2022 · 7 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone

Comments

@sbrannen
Copy link
Member

sbrannen commented Jul 2, 2022

For JDK dynamic proxies created by Spring's AOP support, SpringProxy, Advised, and DecoratingProxy will often be included in the interfaces that the proxy implements.

Here's an example taken from Spring Integration.

proxyHints
    .registerJdkProxy(RequestReplyExchanger.class, SpringProxy.class, Advised.class, DecoratingProxy.class)
    .registerJdkProxy(AbstractReplyProducingMessageHandler.RequestHandler.class, SpringProxy.class, Advised.class, DecoratingProxy.class)
    .registerJdkProxy(IntegrationFlow.class, SmartLifecycle.class, SpringProxy.class, Advised.class, DecoratingProxy.class);

We should investigate options for simplifying the proxy hint registration for Spring AOP proxies so that users are not required to specify SpringProxy, Advised, and DecoratingProxy.

One option would be to introduce a new registerSpringJdkProxy(...) method (or similar) in ProxyHints that automatically registers the required Spring AOP interfaces. Though, it is not always the case that all 3 of those interfaces are implemented by the proxy. So we could document that this particular registerSpringJdkProxy(...) variant always registers those 3 particular interfaces to cover common use cases and allow users to continue to use registerJdkProxy(...) when the additional Spring AOP interfaces differ from that common set of 3.

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Jul 2, 2022
@sbrannen sbrannen added this to the 6.0.0-M5 milestone Jul 2, 2022
@snicoll
Copy link
Member

snicoll commented Jul 2, 2022

We can't really do that, can we? Spring AOP is higher in the dependency tree. Perhaps something along the lines of RuntimeHintsUtils, but for SpringAopProxy?

I am afk but perhaps AopUtils could be used?

@sbrannen
Copy link
Member Author

sbrannen commented Jul 2, 2022

Spring AOP is higher in the dependency tree.

That's a good point. We definitely cannot refer to the Class references, and it does unfortunately seem a bit out of place to be talking about Spring AOP in Spring Core.

We can't really do that, can we?

Well, by using the class names we can implement it like this:

public ProxyHints registerSpringAopJdkProxy(Class<?>... proxiedInterfaces) {
    return registerJdkProxy(jdkProxyHint -> {
        jdkProxyHint.proxiedInterfaces(proxiedInterfaces);
        jdkProxyHint.proxiedInterfaces(
            TypeReference.of("org.springframework.aop.SpringProxy"),
            TypeReference.of("org.springframework.aop.framework.Advised"),
            TypeReference.of("org.springframework.core.DecoratingProxy"));
    });
}

Perhaps something along the lines of RuntimeHintsUtils, but for SpringAopProxy?

Sure. If we don't want to use the class name based approach I pasted above, we could introduce something along the lines of RuntimeHintsUtils for Spring AOP proxies in spring-aop.

I am afk but perhaps AopUtils could be used?

Not that I'm aware of.

@snicoll
Copy link
Member

snicoll commented Jul 3, 2022

Well, by using the class names we can implement it like this:

That sounds like hiding a conceptual cycle to me.

Sure. If we don't want to use the class name based approach I pasted above,

Are we doing this elsewhere, except in Javadoc links?

Not that I'm aware of.

How do you mean? I believe AopUtils#registerAopProxyHints(Class<?>... proxiedInterfaces) could be a possibility.

@sbrannen sbrannen self-assigned this Jul 4, 2022
@sbrannen
Copy link
Member Author

sbrannen commented Jul 4, 2022

Team Decision:

  • Introduce static utility methods in AopProxyUtils that combine user provided interfaces with SpringProxy, Advised, and DecoratingProxy.
  • This method could be named something along the lines of completeProxiedInterfaces() to align with the existing methods in that class.
  • We will need to two variants, one accepting Class<?>... and one accepting TypeReference....
  • The returned arrays should be able to be supplied as input to both variants of proxyHints.registerJdkProxy().
  • Cross reference these new methods from the Javadoc in the existing registerJdkProxy() methods in spring-core to make the AOP specific support discoverable via the core user API.

@snicoll
Copy link
Member

snicoll commented Jul 4, 2022

Cross reference these new methods from the Javadoc in the existing registerJdkProxy() methods in spring-core to make the AOP specific support discoverable via the core user API.

If that is so important this was discussed, I'd like us to review RuntimeHintsUtils#registerAnnotation.

@sbrannen
Copy link
Member Author

sbrannen commented Jul 5, 2022

If that is so important this was discussed, I'd like us to review RuntimeHintsUtils#registerAnnotation.

Do you mean you'd like to cross reference RuntimeHintsUtils from ProxyHints?

If so, I agree that that's a good idea.

In any case, feel free to bring it up in the team or open an issue to discuss what you'd like to review.

@sbrannen
Copy link
Member Author

sbrannen commented Jul 11, 2022

Reopening to reduce the scope of this feature to Class references.

@sbrannen sbrannen reopened this Jul 11, 2022
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants