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

Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject #163

Open
Tracked by #16
KidoVin01 opened this issue Sep 28, 2021 · 7 comments
Open
Tracked by #16
Labels
diagnostic Diagnostic item project-info-needed Cannot be implemented until there is logic to get more information from the Java project

Comments

@KidoVin01
Copy link
Contributor

KidoVin01 commented Sep 28, 2021

What is ambiguous dependency?

An ambiguous dependency exists at an injection point when multiple beans are eligible for injection to the injection point.

Dependency injection and lookup

When resolving a bean at an injection point, the container considers bean type, qualifiers and selected alternatives

Which Java classes are managed beans?

A Java class is a managed bean if it meets all of the following conditions:

  • is not an inner class.
  • It is a non-abstract class, or is annotated @decorator.
  • It does not implement jakarta.enterprise.inject.spi.Extension.
  • It is not annotated @Vetoed or in a package annotated @Vetoed.
  • It has an appropriate constructor - either:
  • the class has a constructor with no parameters, or
  • the class declares a constructor annotated @Inject.
    All Java classes that meet these conditions are managed beans and thus no special declaration is required to define a managed bean.

Beans and Attributes

A bean comprises the following attributes:

  • A (nonempty) set of bean types
  • A (nonempty) set of qualifiers
  • A scope
  • Optionally, a bean name
  • A set of interceptor bindings
  • A bean implementation

Furthermore, a bean may or may not be an alternative.

Error Identification:

  1. Consider a field, method or constructor annotated with @Inject and a set of default or user-specified bean attributes
  2. Check if the dependency class consists of multiple sub-classes that are of valid managed beans.
  3. Check if the dependency class (or its sub-classes) has EXACTLY ONE implementation that corresponds the same set of bean attributes.

Note: if no classes match the set of bean attributes, it is an unsatisfied error, refer to #164

Potential Diagnostics:

  • if none of dependency classes has unique set of bean attributes, suggest creating, applying or removing one of the dependency's class bean attributes
  • if there exist ONE dependency classes with unique set of bean attributes, suggest applying bean attributes to the @Inject field/method/constructor

Example error for @Alternative:

[INFO] [ERROR   ] CWWKZ0004E: An exception occurred while starting the application demo-servlet. The exception message was: com.ibm.ws.container.service.state.StateChangeException: org.jboss.weld.exceptions.DeploymentException: WELD-001409: Ambiguous dependencies for type GreetingItf with qualifiers @Default
[INFO]   at injection point [BackedAnnotatedField] @Inject private io.openliberty.sample.jakarta.di.ambiguous.alt.GreetingServlet.greeting
[INFO]   at io.openliberty.sample.jakarta.di.ambiguous.alt.GreetingServlet.greeting(GreetingServlet.java:0)
[INFO]   Possible dependencies: 
[INFO]   - Managed Bean [class io.openliberty.sample.jakarta.di.ambiguous.alt.GreetingA] with qualifiers [@Any @Default],
[INFO]   - Managed Bean [class io.openliberty.sample.jakarta.di.ambiguous.alt.GreetingB] with qualifiers [@Any @Default]

Note: The diagnostics and error identification depends on #159 for reading class information across files.

Related to #153

@kathrynkodama
Copy link
Contributor

Can you link/quote where in the spec this is mentioned and provide an example of how this might look?

@kathrynkodama kathrynkodama added the diagnostic Diagnostic item label Sep 29, 2021
@KidoVin01 KidoVin01 changed the title Add support for Ambigious Injection Depedency Injection: Diagnostic for Ambigious Injection Oct 1, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious Injection Depedency Injection: Diagnostic for Ambigious @Inject and @Qualifiers Oct 1, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious @Inject and @Qualifiers Depedency Injection: Diagnostic for Ambigious @Inject and @Qualifiers Oct 1, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious @Inject and @Qualifiers Depedency Injection: Diagnostic for Ambigious @Inject, @Qualifiers and @Default Oct 1, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious @Inject, @Qualifiers and @Default Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers and @Default Oct 1, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers and @Default Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers, @Default and @Named Oct 1, 2021
@kathrynkodama
Copy link
Contributor

@AlvinTan2000 Does this section of the CDI spec cover the ambiguous injection you are referring to?

https://jakarta.ee/specifications/cdi/3.0/jakarta-cdi-spec-3.0.html#unsatisfied_and_ambig_dependencies

If so, we need to come up with some concrete ways of checking whether it is an ambiguous injection

@kathrynkodama kathrynkodama added the project-info-needed Cannot be implemented until there is logic to get more information from the Java project label Oct 15, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers, @Default and @Named Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers, @Alternative and @Named Oct 21, 2021
@KidoVin01 KidoVin01 changed the title Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject, @Qualifiers, @Alternative and @Named Depedency Injection: Diagnostic for Ambigious Dependencies with @Inject Oct 21, 2021
@KidoVin01
Copy link
Contributor Author

KidoVin01 commented Oct 21, 2021

Provided some examples code demonstrating the errors.

@kathrynkodama
Copy link
Contributor

Provided some examples code demonstrating the errors.

What conditions would we check for when delivering the diagnostic?

@KidoVin01
Copy link
Contributor Author

We would check for the whether all the applicable sub-classes share the conflicting annotations, which would depend on #159

@kathrynkodama
Copy link
Contributor

kathrynkodama commented Oct 29, 2021

How to check for ambiguous injection (expanding on the error identification section above):

  1. @Inject on a field, method or constructor
@Inject 
private GreetingItf greeting;
  1. Check if the injected field is an interface, for methods and constructors check that the corresponding parameter is an interface
public interface GreetingItf {
	public String greet(String greeting);
}
  1. Search the java project for classes that implement the corresponding interface. If there is more than one class that implements the interface then we need to if every implementation has the same bean attributes. If they do, then deliver a diagnostic warning that the injection is ambiguous.

Detecting if every implementation has the same bean attributes gets a bit tricky, the logic is if every implementation has:

  • the same scope defined, (possible scopes are: @RequestScoped, @SessionScoped, @ApplicationScoped, @Dependent, @ConversationScoped)
    AND IF @Named is present: the same name defined (value set by: @Named )
    AND IF a custom @Qualifier annotation is present: the same qualifier defined (would involve checking for custom qualifiers)
    AND IF@Alternative is present: all are marked @Alternative or ![all but one] are marked @Alternative
    AND IF [add priority logic here]
    AND IF [add type logic here] @AlvinTan2000 I am leaving these logic sections empty as I am not sure the rules associated. Please fill in with an additional comment.

To start, we can cover the base case and iteratively build up support.
The base case would be:

  • Search the java project for classes that implement the corresponding interface. If there is more than one class that implements the interface with the: same scope defined, (possible scopes are: @RequestScoped, @SessionScoped, @ApplicationScoped, @Dependent, @ConversationScoped) and NO other annotations present on those classes then deliver a warning diagnostic.
    ie. both the following classes implement GreetingItf, have @ApplicationScoped, and DO NOT have any other annotations.
@ApplicationScoped
public class GreetingA implements GreetingItf{

    public String greet(String name) {
        return "GreetingA, " + name;
    }

}
@ApplicationScoped
public class GreetingB implements GreetingItf{

    public String greet(String name) {
        return "GreetingB, " + name;
    }

}

We can break this issue down into subsequent tasks and iteratively build:

  • diagnostic for ambiguous injection when multiple implementations have the same scope
  • above + @Named support
  • above + @Qualifier support
  • above + @Alternative support
  • above + @Priority support
  • above + @Type support

@KidoVin01
Copy link
Contributor Author

KidoVin01 commented Nov 6, 2021

Detecting if every implementation has the same bean attributes gets a bit tricky, the logic is if every implementation has:

  • the same scope defined, (possible scopes are: @RequestScoped, @SessionScoped, @ApplicationScoped, @dependent, @ConversationScoped)
    AND IF @nAmed is present: the same name defined (value set by: @nAmed )
    AND IF a custom @qualifier annotation is present: the same qualifier defined (would involve checking for custom qualifiers)
    AND IF@Alternative is present: all are marked @Alternative or ![all but one] are marked @Alternative
    AND IF [add priority logic here]
    AND IF [add type logic here] @AlvinTan2000 I am leaving these logic sections empty as I am not sure the rules associated. Please fill in with an additional comment.

AND IF @Priority(value) is present: at least two shares the same highest value

Don't think it is possible to purposely create an ambiguous error with just a single @Type annotation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostic Diagnostic item project-info-needed Cannot be implemented until there is logic to get more information from the Java project
Projects
None yet
Development

No branches or pull requests

2 participants