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

Inconsistent behavior when finding unproxyable bean #673

Closed
manovotn opened this issue May 23, 2023 · 1 comment
Closed

Inconsistent behavior when finding unproxyable bean #673

manovotn opened this issue May 23, 2023 · 1 comment

Comments

@manovotn
Copy link
Contributor

manovotn commented May 23, 2023

Recently, I was looking into how CDI spec defines what happens if you find an unproxyable bean and it seems to me that there cases in which it behaves rather weirdly and I thought we could improve that.

There are two key parts to this. Firstly, there is a definition of unproxyable bean which states the following:

A bean type must be proxyable if an injection point resolves to a bean:

  • that requires a client proxy, or
  • that has a bound interceptor.

In which case it leads to:

Otherwise, the container automatically detects the problem, and treats it as a deployment problem.

However, the above only applies if an injection point resolves to a bean.
There is another section in the spec that states:

If the bean has a normal scope and the given bean type cannot be proxied by the container, as defined in Unproxyable bean types, the container throws an UnproxyableResolutionException.

Why doesn't the spec just throw deployment exception for all unproxyable beans? You have the whole set of beans known at deployment time (or, in case of Lite, possibly in build time).

Few notes:

  • Current wording allows to have a normal scoped bean that is not proxyable in your deployment so long as it doesn't have an injection point
    • Yet if you ever try to resolve it, it will blow up all the same
  • Current wording throws UnproxyableResolutionException only for client proxies - proxyability due to interception/decoration is unaffected and will still throw a deployment exception even if no injection point resolves to this bean
  • UnproxyableResolutionException is also used in InterceptionFactory, where it is thrown in case the bean is unproxyable for whatever reason - interception subclass included
    • However, this is IMO understandable, because you cannot really predict usages of InterceptionFactory during deployment
  • There is a TCK test asserting this - UnproxyableManagedBeanTest
    • This forces all impl to deffer some of the already known error to runtime for no real use; it is especially weird for build time approaches which generate all proxies and subclasses before you get to runtime

Could we change this behavior to early detection and deployment error for all beans?
Is there a use case I am missing that this explicit spec statement covers?

@manovotn
Copy link
Contributor Author

manovotn commented Nov 6, 2023

Is there a use case I am missing that this explicit spec statement covers?

I wanted to take a closer look into this issue and investigation in Weld code as I had the suspicion that I am missing some use case here.

What I realized is that CDI specification determines proxyability based on bean types and not beans as such.
This means a single bean can have one proxyable and one unproxyable type and that can still work perfectly well.

As an example, consider the following bean:

public interface Foointerface{}

@RequestScopes //will trigger proxyability requirement but the class itself is final making in unproxyable bean type
public final class FooImpl implements FooInterface{}

Now, let's have the following injection point:

@Inject
FooInterface foo1;

The specification in its current wording allows this to work even if the actual bean implementation is not proxyable.
If we changed that as I originally suggested, we'd lose this flexibility and OFC introduce a breaking change.

Here is an old Weld JIRA which also mentioned this - https://issues.redhat.com/browse/WELD-1052
And here is a set of tests that showcase the scenarios such as the one I tried to describe above - https://github.com/weld/core/tree/master/tests-arquillian/src/test/java/org/jboss/weld/tests/proxy/weld1052

Mind that the above is true for client proxies and if the bean (FooImpl) had any interceptor bindings, it would lead to unproxyable exception as in such case you actually need to create a subclass of the impl class, not just the interface.

One other note from my original comment was that the container is required to sometimes treat this as a deployment error and at other times throw UnproxyableResolutionException which happens at runtime. However, under current wording and functionality I depicted above, it seems needed as you can technically encounter this issue as late as in runtime given that you can do it with Instance.select(FooImpl.class),get().

Given all of the above, I will close this issue and remove it from the milestone.

@manovotn manovotn closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2023
@manovotn manovotn removed this from the CDI 4.1 milestone Nov 6, 2023
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

1 participant