While analyzing #28590, I noticed that many of the examples involving setter injection and Kotlin incorrectly use field injection.
Searching for lateinit var movieFinder: MovieFinder will reveal several of those -- for example, those using @Resource, @Autowired, @Inject.
There may well be other cases in the reference manual.
Regarding the "fix", #28590 proposed the @Required set shorthand syntax for annotating a setter method in Kotlin. I'm not a Kotlin expert and therefore assume that works, but I'm wondering if it wouldn't be better to demonstrate annotated setter methods with a complete setXyz(...) method.
Comment From: sbrannen
I'm wondering if it wouldn't be better to demonstrate annotated setter methods with a complete
setXyz(...)method.
@sdeleuze, what are your thoughts on this?
Comment From: sbrannen
@SchroedingersGitHub, since you raised #28590, would you be interested in submitting a PR to address additional issues with annotated setter methods in Kotlin?
Comment From: sdeleuze
In Kotlin those are properties not fields. For most use cases, I think what is documented is fine (it is the idiomatic way to do injection in Kotlin with Spring) unless I am mistaken. When you need to specify that the annotation should be applied on a getter or setter, the recommended way to do this is to use annotation use site targets.
So I think #28590 should be refined to use @set:Required lateinit var movieFinder: MovieFinder (I will create a related issue), and this issue can probably be closed without further commit unless we find specific cases not working as expected (usually validation related).
cc @poutsma
Comment From: sbrannen
When you need to specify that the annotation should be applied on a getter or setter, the recommended way to do this is to use annotation use site targets.
Thanks for enlightening me. That syntax looks much clearer.
So I think #28590 should be refined to use
@set:Required lateinit var movieFinder: MovieFinder(I will create a related issue),
Sounds good. Thanks.
In Kotlin those are properties not fields. For most use cases, I think what is documented is fine (it is the idiomatic way to do injection in Kotlin with Spring) unless I am mistaken.
I can well imagine that what's demonstrated in the examples is the idiomatic way to do things in Kotlin. That would make sense to keep it simple.
But what's documented is a different story, and that is the main reason I created this issue.
For example, we show the reader the following for both setter and field injection, which is incorrect. I hope it is not in fact performing both setter and field injection. Rather, I hope it's resulting in only one form of DI.
@Autowired
lateinit var movieFinder: MovieFinder
My point is that we should either change the examples or change the documentation.
If you think nobody actually uses true setter injection in Kotlin (which I imagine is the case), then let's stick to the idiomatic way people use this but improve the documentation to point out that it's technically field injection (or "property injection") instead of setter injection.
Comment From: sdeleuze
Maybe a pragmatic outcome could be : in the places where we document specifically and explicitly setter injection, we can maybe specify @set:Autowired or similar for correctness. But what I want to avoid is to see those @set:Autowired and similar everywhere in the doc. For other places where setter inject is not explicitly documented, I think we should keep @Autowired to provide the idiomatic version.
Side note : the most idiomatic version in Kotlin is Constructor injection of val properties.