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 qualifiers with members #488

Merged
merged 22 commits into from
Feb 14, 2024
Merged

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Jan 24, 2024

Can now use a qualifier's members to help distinguish beans:

So given:

@Qualifier
@Retention(RetentionPolicy.RUNTIME)
public @interface TempQualifier {
  Scale[] value();

  String someOtherString();

  int defaultVal() default 0;

  NestedAnnotation[] inject() default {@NestedAnnotation()};

  enum Scale {
    CELSIUS,
    FAHRENHEIT,
  }

  @interface NestedAnnotation {

    Inject[] inject() default {};
  }
}

@Singleton
class Meters {

  Thermometer imperial;
  Thermometer metric;

  public Meters(
      @TempQualifier(value = Scale.FAHRENHEIT, someOtherString = "far") Thermometer imperial,
      @TempQualifier(value = Scale.CELSIUS, someOtherString = "celsi") Thermometer metric) {
    this.imperial = imperial;
    this.metric = metric;
  }
}

this will generate

public final class Meters$DI  {

  /**
   * Create and register Meters.
   */
  public static void build(Builder builder) {
    if (builder.isAddBeanFor(Meters.class)) {
      var bean = new Meters(builder.get(Thermometer.class,"@tempqualifier(defaultval=0, inject={@nestedannotation(inject={})}, someotherstring=\"far\", value={fahrenheit})"), builder.get(Thermometer.class,"@tempqualifier(defaultval=0, inject={@nestedannotation(inject={})}, someotherstring=\"celsi\", value={celsius})"));
      builder.register(bean);
    }
  }
}

@SentryMan
Copy link
Collaborator Author

SentryMan commented Jan 24, 2024

Curious, it seems only JDK 11 has some weird issue that prevents this from working.

@SentryMan
Copy link
Collaborator Author

It seems that in JDK 11, the qualifier code generates differently, it'll wire, but the ability to use test mocking is impaired.

@SentryMan SentryMan marked this pull request as draft January 24, 2024 17:15
@SentryMan
Copy link
Collaborator Author

SentryMan commented Jan 24, 2024

the problem here seems to be that there's a discrepancy with how the annotation are rendered at runtime with Annotation#toString(), might have to only support a single value member at this time.

@SentryMan SentryMan added the enhancement New feature or request label Jan 25, 2024
@SentryMan SentryMan self-assigned this Jan 25, 2024
@SentryMan SentryMan added this to the 9.11 milestone Jan 25, 2024
@SentryMan SentryMan marked this pull request as ready for review January 25, 2024 18:53
@SentryMan
Copy link
Collaborator Author

I managed to get it working with regex, with the only limitation being that qualifiers with annotation members are only supported if the nested annotation type has only a single member.

@rob-bygrave
Copy link
Contributor

rob-bygrave commented Feb 14, 2024

Cool. So we probably should add a new test for the case where someone uses @Named("@Foo") [to be explicit that the @ is not trimmed - anyone using that convention isn't broken by this change].

I'm also thinking there could be a test for the "simple case" where the qualifier only has 1 attribute? Like what was the motivation for this feature / I'm not expecting anyone to use nested attributes so TempQualifier to me looks like it would not match the common case (it's more complex than the common case which is good for testing but to me looks like we don't actually test the common/expected case).

@SentryMan
Copy link
Collaborator Author

ehh, I reverted it, we could always do it later

@SentryMan
Copy link
Collaborator Author

Like what was the motivation for this feature / I'm not expecting anyone to use nested attributes

Was looking at Weld and got inspired. In any case, it helps cut down on annotation overload so I thought it was a good idea.

@rbygrave rbygrave merged commit 1c46a3d into avaje:master Feb 14, 2024
4 checks passed
@SentryMan SentryMan deleted the qualified-members branch February 14, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants