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

Support bindInterceptor on final classes #11

Closed
reubenfirmin opened this issue Apr 12, 2019 · 6 comments
Closed

Support bindInterceptor on final classes #11

reubenfirmin opened this issue Apr 12, 2019 · 6 comments

Comments

@reubenfirmin
Copy link

Since everything is final by default in kotlin, guice's interceptors don't immediately work, and in fact silently fail. This is not hard to google a solution to, assuming you have tests around the caching; however, the kotlin wrapper should either detect this and raise an error, or (ideally) provide a solution (perhaps an open delegating proxy) that allows aop to work.

@JLLeitschuh
Copy link

and in fact silently fail

Sounds like a bug in Guice?

@johnlcox
Copy link
Member

I'm having trouble understanding the issue here, and I'm apt to agree with @JLLeitschuh that the silent failing sounds like an issue in Guice rather than kotlin-guice.

Could you provide a Minimal Complete Verifiable Example showing what you are trying to do with a final class?

@reubenfirmin
Copy link
Author

Java is non-final by default. Kotlin is final by default.

bindInterceptor only works on non-final classes, and non-final methods (https://github.com/google/guice/wiki/AOP). When guice was written, this was a reasonable decision since you have to go out of your way to make classes/methods final. In Kotlin, the inverse is true; you have to go out of your way to make things non-final. Therefore I think it's reasonable that it'd be the job of kotlin-guice to make bindInterceptor (perhaps renamed - bindInterceptorProxy) work seamlessly while maintaining the final-by-default setup. Otherwise, the code ends up becoming marked with "open" throughout for no obvious good reason.

@johnlcox
Copy link
Member

I'm not sure it's possible to make interceptors work with final classes. The reason Guice doesn't support final classes is because the interception is implemented with an AOP library like cglib, which generates a subclass dynamically.

It looks like there is an issue logged to Guice to detect binding final (and other invalid) classes as interceptors: google/guice#294

Do you have some idea in mind for working around the limitations of the underlying AOP library implementation for interceptors?

@reubenfirmin
Copy link
Author

You could potentially use an InvocationHandler and proxy to the matched methods via the interceptor.

@johnlcox
Copy link
Member

We could proxy final classes, but so could Guice.

As long as Guice doesn't allow binding final classes I don't intend to support it in this extension project.

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

No branches or pull requests

3 participants