Proposal: using ctor-level dependency injection

[Misagh Moayyed <mi...@tirasa.net>]

Hello all,

I want to discuss and propose a design change in the way Syncope components, specially *Logic classes are constructed. For a concrete example, this component [1] might be a good baseline.

Components such as [1] do two things that seem less than ideal:

1) The class is directly annotated with a @Component
2) It uses field-injections by annotating fields with @Autowire and such

I submit that this approach generally proves challenging, specially when it comes to constructing a context for integration tests and dealing with classpath scanning. There is lot of literature on why this (field injections) might not be an ideal approach; The "test context and component scanning" is one practical example that I myself ran into; Purists might also argue that business-level components and logic classes should not be tied to the upper framework per se (though I don't actually find myself in this camp all too often!). 

A better alternative perhaps would be:

- Avoid field-injections and use ctor-level injections (this is the general recommendation from Spring)
- Do not use autowire/component/etc directly in business-level classes
- ...which means do not rely as much (if ever) on classpath/context scanning
- ...and instead, create and instantiate the bean directory in @Configuration classes, conditionally and with direct control
- ...or use a middle-ground for now, with something like this [2]

The work feels largely cosmetic perhaps; I think it will pay off in the future specially if it's something that is advocated by Spring and family.

WDYT?

--Misagh

[1] https://github.com/apache/syncope/blob/master/core/am/logic/src/main/java/org/apache/syncope/core/logic/OIDCJWKSLogic.java
[2] https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/using-boot-spring-beans-and-dependency-injection.html

Re: Proposal: using ctor-level dependency injection

[Francesco Chicchiriccò <il...@apache.org>]

On 19/02/21 11:41, Misagh Moayyed wrote:
> Hello all,
>
> I want to discuss and propose a design change in the way Syncope components, specially *Logic classes are constructed. For a concrete example, this component [1] might be a good baseline.
>
> Components such as [1] do two things that seem less than ideal:
>
> 1) The class is directly annotated with a @Component
> 2) It uses field-injections by annotating fields with @Autowire and such
>
> I submit that this approach generally proves challenging, specially when it comes to constructing a context for integration tests and dealing with classpath scanning. There is lot of literature on why this (field injections) might not be an ideal approach; The "test context and component scanning" is one practical example that I myself ran into; Purists might also argue that business-level components and logic classes should not be tied to the upper framework per se (though I don't actually find myself in this camp all too often!). 
>
> A better alternative perhaps would be:
>
> - Avoid field-injections and use ctor-level injections (this is the general recommendation from Spring)
> - Do not use autowire/component/etc directly in business-level classes
> - ...which means do not rely as much (if ever) on classpath/context scanning
> - ...and instead, create and instantiate the bean directory in @Configuration classes, conditionally and with direct control
> - ...or use a middle-ground for now, with something like this [2]
>
> The work feels largely cosmetic perhaps; I think it will pay off in the future specially if it's something that is advocated by Spring and family.
>
> WDYT?

Hi Misagh,
thanks for bringing up this discussion.

Moving (especially, but not only) Logic classes from the current "@Component + @Autowired" setup to something more in line with [2] would be beneficial not only for cosmetics but also to simplify override or extensions for deployments.

At least for Logic, I suspect [3] would be the central place at least for IdRepo, then we'll need to setup similar classes form IdM and AM, respectively.

Globally, I'd say not a difficult but labor intensive task to accomplish.
A big +1 from my side.

Regards.

> [1] https://github.com/apache/syncope/blob/master/core/am/logic/src/main/java/org/apache/syncope/core/logic/OIDCJWKSLogic.java
> [2] https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/using-boot-spring-beans-and-dependency-injection.html
[3] https://github.com/apache/syncope/blob/master/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/LogicContext.java

-- 
Francesco Chicchiriccò

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/

Re: Proposal: using ctor-level dependency injection

[Misagh Moayyed <mi...@tirasa.net>]

Thank you all. Sounds like we have consensus. 

I'll proceed to create a JIRA to track the task.  

--Misagh

----- Original Message -----
> From: "Andrea Patricelli" <an...@apache.org>
> To: "dev" <de...@syncope.apache.org>
> Sent: Monday, February 22, 2021 7:44:13 PM
> Subject: Re: Proposal: using ctor-level dependency injection

> Hi Misagh,
> 
> definitely +1 also from me!
> 
> Best regards,
> Andrea
> 
> On 19/02/21 11:41, Misagh Moayyed wrote:
>> Hello all,
>>
>> I want to discuss and propose a design change in the way Syncope components,
>> specially *Logic classes are constructed. For a concrete example, this
>> component [1] might be a good baseline.
>>
>> Components such as [1] do two things that seem less than ideal:
>>
>> 1) The class is directly annotated with a @Component
>> 2) It uses field-injections by annotating fields with @Autowire and such
>>
>> I submit that this approach generally proves challenging, specially when it
>> comes to constructing a context for integration tests and dealing with
>> classpath scanning. There is lot of literature on why this (field injections)
>> might not be an ideal approach; The "test context and component scanning" is
>> one practical example that I myself ran into; Purists might also argue that
>> business-level components and logic classes should not be tied to the upper
>> framework per se (though I don't actually find myself in this camp all too
>> often!).
>>
>> A better alternative perhaps would be:
>>
>> - Avoid field-injections and use ctor-level injections (this is the general
>> recommendation from Spring)
>> - Do not use autowire/component/etc directly in business-level classes
>> - ...which means do not rely as much (if ever) on classpath/context scanning
>> - ...and instead, create and instantiate the bean directory in @Configuration
>> classes, conditionally and with direct control
>> - ...or use a middle-ground for now, with something like this [2]
>>
>> The work feels largely cosmetic perhaps; I think it will pay off in the future
>> specially if it's something that is advocated by Spring and family.
>>
>> WDYT?
>>
>> --Misagh
>>
>> [1]
>> https://github.com/apache/syncope/blob/master/core/am/logic/src/main/java/org/apache/syncope/core/logic/OIDCJWKSLogic.java
>> [2]
>> https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/using-boot-spring-beans-and-dependency-injection.html
>>
> --
> Andrea Patricelli
> 
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
> 
> Member at The Apache Software Foundation
> Syncope

Re: Proposal: using ctor-level dependency injection

[Andrea Patricelli <an...@apache.org>]

Hi Misagh,

definitely +1 also from me!

Best regards,
Andrea

On 19/02/21 11:41, Misagh Moayyed wrote:
> Hello all,
>
> I want to discuss and propose a design change in the way Syncope components, specially *Logic classes are constructed. For a concrete example, this component [1] might be a good baseline.
>
> Components such as [1] do two things that seem less than ideal:
>
> 1) The class is directly annotated with a @Component
> 2) It uses field-injections by annotating fields with @Autowire and such
>
> I submit that this approach generally proves challenging, specially when it comes to constructing a context for integration tests and dealing with classpath scanning. There is lot of literature on why this (field injections) might not be an ideal approach; The "test context and component scanning" is one practical example that I myself ran into; Purists might also argue that business-level components and logic classes should not be tied to the upper framework per se (though I don't actually find myself in this camp all too often!).
>
> A better alternative perhaps would be:
>
> - Avoid field-injections and use ctor-level injections (this is the general recommendation from Spring)
> - Do not use autowire/component/etc directly in business-level classes
> - ...which means do not rely as much (if ever) on classpath/context scanning
> - ...and instead, create and instantiate the bean directory in @Configuration classes, conditionally and with direct control
> - ...or use a middle-ground for now, with something like this [2]
>
> The work feels largely cosmetic perhaps; I think it will pay off in the future specially if it's something that is advocated by Spring and family.
>
> WDYT?
>
> --Misagh
>
> [1] https://github.com/apache/syncope/blob/master/core/am/logic/src/main/java/org/apache/syncope/core/logic/OIDCJWKSLogic.java
> [2] https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/using-boot-spring-beans-and-dependency-injection.html
>
-- 
Andrea Patricelli

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope

Re: Proposal: using ctor-level dependency injection

[Matteo Alessandroni <sk...@apache.org>]

Hello Misagh,

I've recently come across similar discussions while working on a 
Java-based application and I'm getting curious about the topic you've 
just raised.
Although I'm still attached to the field injections (@Autowired) way, 
I'd be happy to discover new paths about that! From my side the best 
advantages might show up on the "test context and component scanning" 
side, since sometimes it's kind of a pain to deal with that!

So, thank you for proposing (highly possible) better alternatives and 
I'm gonna carefully follow the discussion!

Regards,
Matteo


On 19/02/21 11:41, Misagh Moayyed wrote:
> Hello all,
>
> I want to discuss and propose a design change in the way Syncope components, specially *Logic classes are constructed. For a concrete example, this component [1] might be a good baseline.
>
> Components such as [1] do two things that seem less than ideal:
>
> 1) The class is directly annotated with a @Component
> 2) It uses field-injections by annotating fields with @Autowire and such
>
> I submit that this approach generally proves challenging, specially when it comes to constructing a context for integration tests and dealing with classpath scanning. There is lot of literature on why this (field injections) might not be an ideal approach; The "test context and component scanning" is one practical example that I myself ran into; Purists might also argue that business-level components and logic classes should not be tied to the upper framework per se (though I don't actually find myself in this camp all too often!).
>
> A better alternative perhaps would be:
>
> - Avoid field-injections and use ctor-level injections (this is the general recommendation from Spring)
> - Do not use autowire/component/etc directly in business-level classes
> - ...which means do not rely as much (if ever) on classpath/context scanning
> - ...and instead, create and instantiate the bean directory in @Configuration classes, conditionally and with direct control
> - ...or use a middle-ground for now, with something like this [2]
>
> The work feels largely cosmetic perhaps; I think it will pay off in the future specially if it's something that is advocated by Spring and family.
>
> WDYT?
>
> --Misagh
>
> [1] https://github.com/apache/syncope/blob/master/core/am/logic/src/main/java/org/apache/syncope/core/logic/OIDCJWKSLogic.java
> [2] https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/using-boot-spring-beans-and-dependency-injection.html
>