Skip to content

Remove AopProxyUtils#completeJdkProxyInterfaces #28998

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

Closed
sdeleuze opened this issue Aug 23, 2022 · 4 comments
Closed

Remove AopProxyUtils#completeJdkProxyInterfaces #28998

sdeleuze opened this issue Aug 23, 2022 · 4 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: aot An issue related to Ahead-of-time processing type: task A general task

Comments

@sdeleuze
Copy link
Contributor

Follow-up of #28980 since it should not be useful anymore now that related proxies are inferred.

@sdeleuze sdeleuze added type: task A general task theme: aot An issue related to Ahead-of-time processing labels Aug 23, 2022
@sdeleuze sdeleuze added this to the 6.0.0-M6 milestone Aug 23, 2022
@sdeleuze sdeleuze self-assigned this Aug 23, 2022
@sdeleuze sdeleuze changed the title Remove AopProxyUtils#completeProxiedInterfaces Remove AopProxyUtils#completeProxiedInterfaces Aug 23, 2022
@sdeleuze
Copy link
Contributor Author

sdeleuze commented Sep 5, 2022

@snicoll Maybe we should keep it (potentially with an updated Javadoc) for the manual proxy creation that can't be inferred?

@snicoll
Copy link
Member

snicoll commented Sep 5, 2022

We could. I still don't like the current method arrangement. It's consistent with similar methods in this class but the consumers of the new method could use something a bit more explicit.

I'd like to see something like hints.proxies.registerJdkProxy(MyProxy.class, asSpringJdkProxy()) or something like that where asSpringJdkProxy is a consumer of the builder, adding the 3 interfaces.

@sdeleuze sdeleuze changed the title Remove AopProxyUtils#completeProxiedInterfaces Remove AopProxyUtils#completeJdkProxyInterfaces Sep 6, 2022
@snicoll
Copy link
Member

snicoll commented Sep 6, 2022

I think you misunderstood it, indeed. I am not talking about adding something to the core hint API (that would be a conceptual cycle). I am rather advocating that spring-aop provides a first-class, idiomatic method that allows to register a JDK proxy as an AOP proxy. I believe that the method above achieves this scenario, something like:

public static Consumer<Builder> asAopProxy() {
    return builder -> builder.proxiedInterfaces(SpringProxy.class, Advised.class, DecoratingProxy.class);
}

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Sep 6, 2022

Thanks for the additional details. I don't think AopProxyUtils should import hints APIs and creating a dedicated class in spring-aop for it seems too much for me as well. I would be for your proposal if there was a way to make it available directly in the hints API in spring-core. Since that's not the case, I think what we have currently is reasonable and I would be for closing this issue and keep what we have.

@sdeleuze sdeleuze closed this as not planned Won't fix, can't repro, duplicate, stale Sep 6, 2022
@sdeleuze sdeleuze removed this from the 6.0.0-M6 milestone Sep 6, 2022
@sdeleuze sdeleuze added the status: declined A suggestion or change that we don't feel we should currently apply label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: aot An issue related to Ahead-of-time processing type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants