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

Javadoc for @Named is misleading to users of CDI #33

Open
Azquelt opened this issue Jun 14, 2023 · 6 comments
Open

Javadoc for @Named is misleading to users of CDI #33

Azquelt opened this issue Jun 14, 2023 · 6 comments

Comments

@Azquelt
Copy link

Azquelt commented Jun 14, 2023

Is your feature request related to a problem? Please describe.
The CDI specification recommends that @Named is not used as a qualifier at injection points. However, the Javadoc for @Named merely says that it's a string based qualifier and gives an example of it's use in this context.

CDI uses @Named to assign a bean name for lookup from technologies like JSF. Bean names mostly have to be unique among the application.

Describe the solution you'd like
Can we add a note to the @Named annotation indicating that CDI recommends against its use as a qualifier for injection points?

Describe alternatives you've considered
We could just remove the example showing @Named used in the way that CDI recommends against.

We could do nothing and acknowledge that the choice to have shared injection annotations means that the Javadoc can't be as helpful to users as we'd like.

Additional context
I realise that there are other consumers of @Named other than CDI (I'm aware of Spring and Guice). Any guidance for use in CDI would have to be written in a way that doesn't mislead users of these technologies.

@Azquelt Azquelt changed the title Javadoc for @Named Javadoc for @Named is misleading to users of CDI Jun 14, 2023
@rbygrave
Copy link
Contributor

rbygrave commented Jun 21, 2023

These jakarata.inject annotations originate from JSR-330: Dependency Injection for Java (rather than CDI 1.0 but I think both specs came out around the same time). The Javadoc of @Named is part of the JSR-330 Specification as I understand it (and there is a test suite somewhere).

I don't yet see an inconsistency between JSR-330 @Named and CDI? That is, I don't see anything in the linked CDI Section 2.2.9 that is inconsistent with JSR-330 @Named? What am I missing?

CDI uses @Named to assign a bean name

I believe that CDI is using @Named as a qualifier and using it consistently with JSR-330 and consistently with the Javadoc in @Named.

As I see it, we need to careful using the term "bean name" (as opposed to "qualifier"). As I see it, "bean name" really can be different to a "qualifier" and used very differently by various DI implementations.

Perhaps you can explain where you see the inconsistency? Is this something specific with use with JSF or something else?

We could just remove the example showing @nAmed used in the way that CDI recommends against.

That does not sound very acceptable from a JSR-330 perspective. CDI might not recommend use of @Named along the lines of "string qualifiers are not as good as typed qualifiers" but that is very different from the Javadoc not being correct.

The example in the Javadoc exactly matches the expected use for JSR-330 and there are a number of very popular DI libraries that indeed comply to the JSR-330 used of @Named. We can add Dagger2 which might be the most popular DI library in the Android space?

As I see it, there is now a really large amount of application code out there which is using @Named exactly as it is intended as per JSR-330 and exactly matching the Javadoc example.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 21, 2023

My personal opinion is that CDI pretty much screwed up @Named and it's up to CDI to make up for that. I don't think we should change AtInject just for the sake of CDI.

@rbygrave It's a little complicated. In CDI, an injection point @Inject MyBean bean is actually treated as @Inject @Default MyBean bean, where @Default is qualifier that beans obtain automatically when they don't have any other qualifier but @Any (a qualifier that all beans have, unconditionally) or @Named. This means that if a bean is only given the @Named qualifier, it also automatically receives the @Default qualifier and hence can be injected unqualified.

I believe CDI should have used a new qualifier called @ExternalIdentifier or so, instead of repurposing @Named, but that ship has sailed long before I even learned about CDI.

@manovotn
Copy link

I believe CDI should have used a new qualifier called @ExternalIdentifier or so, instead of repurposing @nAmed, but that ship has sailed long before I even learned about CDI.

While I agree with this, that doesn't help us solve the issue CDI has.
I think what @Azquelt proposes is a nice middle ground between silently pretending the problem doesn't exist and breaking many CDI apps by re-defining how it works.
I am not exactly a fan of adding these these special case docs but having this defined just in the CDI spec (as it is now) apparently is insufficient as we keep seeing this issue. I assume the reason for that is that most people will not read the whole spec and reading just javadocs, they can see that @Named == just-another-qualifier 🤷

@rob-bygrave
Copy link

rob-bygrave commented Jun 21, 2023

For myself, I don't mind a note for CDI being added. I'd be reasonably confident that could be done in such a way that does not confuse JSR-330 users. [Removing the example from the Javadoc imo is not a viable option]

I have a silly question:

In CDI, if we have:

@Inject @Named("red") MyBean redBean

... and CDI transparently adds a @Default qualifier to then become:

@Inject @Named("red") @Default MyBean redBean

What is the purpose / expected use case for this? How is the combination of @Named("red") @Default intended to be useful?

That is, why does CDI just NOT automatically add the @Default qualifier when there is already a @Named qualifier present (and instead require the users to explicitly add @Default if that is what they truely wanted)? That is, why is not treated as a CDI bug?

Said differently, given the CDI docs recommending to not use @Named that suggests there was no "cool use case" this was intended for. So why is CDI automatically adding the @Defaultqualifier when there is already a @Named? At first glance it looks "fixable".

Edit: Agh, note I'm also @rbygrave - apologies if that is confusing.

@manovotn
Copy link

That is, why does CDI just NOT automatically add the @default qualifier when there is already a @nAmed qualifier present (and instead require the users to explicitly add @default if that is what they truely wanted)? That is, why is not treated as a CDI bug?

Historical reasons and decisions I suppose. I wasn't there when this was done so take it with a pinch of salt but my guess is it was probably a perceived way of making it easier to inject those beans.

@Named seems like a natural candidate for giving beans unique string based names, but unfortunately, it also means that with other CDI rules in place, a bean that is, say, @Dependent @Named("foo") public class Foo{} would not be injectable without the qualifier and it's binding string value (because it would lack the @Default). You'd need to explicitly state @Inject @Named("foo") Foo to get the bean (or use @Any of course).

So why is CDI automatically adding the @Default qualifier when there is already a @Named?

Adding the exception to the CDI spec could have been (again, just a speculation) a way to allow users to have a simple bean declaration that:
a) allows you to give the name unique string name to be able to use the bean in non typesafe environment (such as unified expression lang)
b) at the same time allows you to still inject it via simple @Inject Foo with no extra hassle in typesafe environments

Then again, according to my googling, the same or very similar text existed even in CDI 1.0 so it's really just guesswork :)

Said differently, given the CDI docs recommending to not use @Named that suggests there was no "cool use case" this was intended for.

Note that CDI suggests that @Named should not be used as standard qualifiers but it still go-to for naming your beans.

At first glance it looks "fixable".

What kind of fix do you have in mind?
IMO we'd need to introduce a new way of naming beans with a separate annotation that isn't even a qualifier (which is theoretically fine).
But the tricky bit is backwards compatibility and what do we do with @Named usage overall - changing how it works and whether @Default is or isn't added breaks basically any app that uses @Named bean with no other qualifier and performs resolution for such a bean. And that is a lot of apps as you mostly declare beans without any qualifiers (unless there is multiple matching types) but possibly with a name.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 22, 2023

I have a silly question:

In CDI, if we have:

@Inject @Named("red") MyBean redBean

... and CDI transparently adds a @Default qualifier to then become:

@Inject @Named("red") @Default MyBean redBean

What is the purpose / expected use case for this?

Not a silly question -- I explained it too briefly to be understandable. That's not what is happening. What's happening is the following.

If I declare

@Dependent // scope annotation, not using `@Singleton` for idiosyncratic reasons
public class MyBean1 {
}

CDI automatically adds the @Default qualifier (and @Any, which all beans always have), because no other qualifier is used.

Now, if I declare

@Dependent
@MyQualifier
public class MyBean2 {
}

CDI does not add @Default, because there is another qualifier. (Again, CDI does add @Any, because all beans always have it.)

Nothing terribly surprising here. There might be legitimate questions as to what is the purpose of the @Default and @Any qualifiers, but those are irrelevant to the present issue.

Now, if I declare

@Dependent
@Named("myBean3")
public class MyBean3 {
}

CDI does add @Default (and @Any).

Now, there are 2 important things:

  1. If an injection point doesn't declare any qualifiers, @Default is implied. So @Inject MyBeanX effectively becomes @Inject @Default MyBeanX.
  2. A bean must declare all the qualifiers of an injection point to be considered for injection into it. It may also declare more qualifiers. So if I have a bean of type MyBeanX with qualifiers @Foo and @Bar (and @Any, but clearly not @Default), that bean will be injected into @Inject @Foo MyBeanX, into @Inject @Bar MyBeanX or into @Inject @Foo @Bar MyBeanX, but not into @Inject MyBeanX.

With that said, it should be clear that:

  1. MyBean1 above will be injected into @Inject MyBean1
  2. MyBean2 above will not be injected into @Inject MyBean2, but it will be injected into @Inject @MyQualifier MyBean2
  3. MyBean3 will be injected into @Inject @Named("myBean3") MyBean3, as everyone would expect, but it will also be injected into @Inject MyBean3.

The 3rd point is how @Named doesn't behave like all other qualifiers.

I'm not exactly sure we can explain this succinctly in the Named javadoc, frankly. We could add something like

In CDI, the usage of @Named on injection points is not recommended, because CDI treats the @Named qualifier specially. See the CDI specification for more details.

But that isn't terribly helpful 🤷

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

5 participants