cdi icon indicating copy to clipboard operation
cdi copied to clipboard

More convenient injection into Java records

Open hantsy opened this issue 1 year ago • 13 comments

CDI @Singleton bean and @Decorator do not generate proxy at runtime, it should allow it to be declared as a record type. And inject dependent beans via constructor injection.

hantsy avatar Dec 25 '24 12:12 hantsy

CDI @Singleton bean and @Decorator do not generate proxy at runtime

Decorated beans are implemented through subclassing, just like intercepted beans, so this would not really work.

@Singleton alone should work just fine or is there any issue with it?

manovotn avatar Dec 25 '24 18:12 manovotn

When using record type, I must add a cocanial constructor and an extra @Inject to inject the dependent beans.

  1. @Inject can not be applied on the record class level
  2. If possible make all record fields injectable by default without a @inject

hantsy avatar Dec 26 '24 07:12 hantsy

@Inject can not be applied on the record class level

This seems doable, just a few thoughts:

  • Records can also have multiple c-tors so we'd need to check that
  • We do not allow this kind of declaration for any other bean and allowing it just here means creating a special case
    • We could allow this for all beans - so long as they have a single c-tor so CDI can be deterministic; throw error otherwise
  • To specify a special behavior for records, the spec needs to target JDK 14+ (which it doesn't for 4.1 but surely will for 5,x)

If possible make all record fields injectable by default without a @inject

Not sure what you mean here. The injection into records would need to be through constructors only - all other parts are by definition final and meant to be immutable.

manovotn avatar Dec 27 '24 00:12 manovotn

Not sure what you mean here. The injection into records would need to be through constructors only - all other parts are by definition final and meant to be immutable.

What I said here is that when a record class is annotated with @Singletone, all fields in the default constructor are injected automatically without an explicit @Inject.

hantsy avatar Dec 27 '24 02:12 hantsy

I see, that's a little different from what I meant in my previous comment and would require us to special case records for sure.

manovotn avatar Dec 27 '24 15:12 manovotn

When using record type, I must add a cocanial constructor and an extra @Inject to inject the dependent beans.

  1. @Inject can not be applied on the record class level
  2. If possible make all record fields injectable by default without a @inject

This is Java the language decision. They simply don't provide a way to annotate the constructor without actually declaring it. I don't see why CDI / AtInject should try to fix that "problem".

Ladicek avatar Jan 02 '25 13:01 Ladicek

This is Java the language decision. They simply don't provide a way to annotate the constructor without actually declaring it. I don't see why CDI / AtInject should try to fix that "problem".

I think the only reason is just convenience.

Special casing records is IMO not worth it. However, I was thinking we could change the spec to state that for any [discovered] class based bean with a single declared constructor, such c-tor is automatically treated as if it was annotated with @Inject. This would solve the issue presented here with records but would also work for any other bean. In case said bean has multiple c-tors (or needs to have them for proxyability), we are back to current behavior and explicit @Inject annotation.

Now, I might have missed something but so far I can't think of anything preventing us from doing this. Thoughts?

manovotn avatar Jan 02 '25 15:01 manovotn

Historically with a discovery mode of "all" this could cause a lot of beans to exist. I (very dimly) seem to recall in the earliest days of the specification people deciding this was a bad idea. Maybe that's no longer a relevant situation/case; I don't know.

ljnelson avatar Jan 02 '25 16:01 ljnelson

Historically with a discovery mode of "all" this could cause a lot of beans to exist. I (very dimly) seem to recall in the earliest days of the specification people deciding this was a bad idea. Maybe that's no longer a relevant situation/case; I don't know.

Hm, that's a fair point, thanks for mentioning that. It would indeed pick up more classes as bean candidates 🤔

manovotn avatar Jan 02 '25 18:01 manovotn

But with the new default mode of needing a bean defining annotation, it would only pick up classes that someone was trying to use a bean. To date no class with a non-default ctor could in fact be a bean, so the impact of this should tend to zero.

starksm64 avatar Jan 03 '25 03:01 starksm64

But with the new default mode of needing a bean defining annotation, it would only pick up classes that someone was trying to use a bean. To date no class with a non-default ctor could in fact be a bean, so the impact of this should tend to zero.

The all discovery mode still exists and is perfectly valid in CDI Full. So this change would affect existing applications using that and would be detrimental to all discovery mode in general as it would lower its usability - you cannot really turn that off.

I guess we could further tweak the suggestion in my previous comment and state that this only works for class-based beans that have a bean defining annotation on them?

manovotn avatar Jan 03 '25 05:01 manovotn

Seriously, is

@Dependent
public record Foo(Bar bar) {
    @Inject
    Foo {
    }
}

so much worse than

@Dependent
public record Foo(Bar bar) {
}

?

Ladicek avatar Jan 03 '25 08:01 Ladicek

We discussed this today ( Jan 28, 2025) in the CDI call and the consensus was to remove this from the milestone. The functionality as such can already be achieved by adding the constructor so this would just create a shortcut (and not much shorter for that matter) and force us to special case records in the spec text. Note that records as such are not very good bean candidates because they are not proxyable. This limits them to being singletons or dependent beans and even then they cannot be intercepted/decorated; all in all, they can rarely be used as proper beans to justify this.

If there is more interest in this issue, we can revisit it in the future.

manovotn avatar Jan 28 '25 15:01 manovotn