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

only load downloader if needed #13

Closed
3 tasks done
CXwudi opened this issue Jun 17, 2021 · 3 comments
Closed
3 tasks done

only load downloader if needed #13

CXwudi opened this issue Jun 17, 2021 · 3 comments

Comments

@CXwudi
Copy link
Owner

CXwudi commented Jun 17, 2021

when we have more and more downloaders choice in the futures, we can't force users to fill in all downloaders config, so we need:

  • multiple downloaders for one PV service
  • validate that a downloader for one PV service can not be used in another PV service
  • load downloaders base on enablement and pv-preference
    • will probably use spring @ConditionOnBean to check if the corresponding downloader config existBIG NOTICE: base on Spring Doc, Spring Boot issue 1 and issue 2, any @ConditionalOn annotations that rely on knowledge about Spring beans will NOT work outside of Spring auto-configuration (PS: @ConditionalOnProperty would probably work)
    • downloader config bean is @ConditionOnExpression with the check if its name is in enablement config map Because of this, SpEL can not resolve properties (except @Value). So we need another solution to load only enabled downloaders
    • other solutions, see below comments
@CXwudi CXwudi added this to To do in After-completion Improvement via automation Jun 17, 2021
@CXwudi
Copy link
Owner Author

CXwudi commented Jun 19, 2021

So we have an enablement config with map of PV services to downloader names, but how to validate this map

Q: can we put custom ConstrainValidator into spring ApplicationContext and let it has depending bean of Preference?
A: Yes, a ConstrainValidator can be a spring component with other dependent components
Q2: can the custom validator contains a bean of Environment and call checkProperty() method to validate if a property root path exist?
A: Yes and No, we can only check if the full name property exists, but not the root

@CXwudi
Copy link
Owner Author

CXwudi commented Jun 27, 2021

About solution for loading only enabled downloaders:

  • use @Conditional(MyCondition.class).
    • Try either annotate MyCondition with @Component or using the BeanFactory in ConditionContext from the implemented method
    • An: since Condition initialized at the very first stage of the Spring life cycle, we can't use any beans. In fact, we will get a null BeanFactory from ConditionContext. The only thing we can do is use Environment from ConditionContext to check some properties
  • let all downloader configs have two common properties, enable and order. and let all downloaders @ConditionalOnProperty on their enable property this works if only on enable property, but how to disable the downloader if pv-preference doesn't include the target PV service, see features about downloading pvs #11
    • validation on downloader config will be fully working as the user may disable the downloader you can put @Validated on downloader class itself and put @field:Valid on the holding config bean
  • Spring Lazy Initialization. Since all downloaders are getten through BeanFactory only, we could try Lazy Initialization
    • can @ConfigurationProperties with @Validated been lazily validated and initialized?
    • An: Yes, but must enable global lazy initialization by spring.main.lazy-initialization=true. Otherwise @ConfigurationProperties always initialized at startup even if it is @Lazy. @Beans and @Components can be lazily loaded.
    • If don't want spring.main.lazy-initialization=true, we can use next option
  • call Validator.validate(). e.g. inside lazy {} or init {} block
    • is an easy understand solution, but Validator bean will be passed to various business codes. Make things look uglier

@CXwudi
Copy link
Owner Author

CXwudi commented Jul 9, 2021

FInal answer: use @Conditonal(MyCondition.class)
Reason:

  • eager initialization is still needed so that no config validation failure during the business logic part (remember that RecordListener listens to all exception)
  • good isolation of validation code and business code, although this method led to more code to write
  • very very very, fortunately, all our condition is based on configuration properties, so the limitation of using @Conditonal(MyCondition.class) doesn't affect our project.
    • but this might not be the case in future projects, consider using Micronauts framework or POC creating our own auto-configuration classes (this is for writing libraries)

@CXwudi CXwudi closed this as completed Jun 15, 2022
After-completion Improvement automation moved this from To do to Done Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

1 participant