Skip to content

GH-2655: AOT support user declared binders #2689

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

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

onobc
Copy link
Contributor

@onobc onobc commented Apr 3, 2023

  • Adds support for AOT child context generation for user-declared binder configurations.
  • Add AOT processing test to verify single and multi-binder cases'

Fixes #2655

@onobc onobc requested review from olegz and sobychacko April 3, 2023 18:25
@@ -94,6 +104,13 @@ public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registe
return null;
}

private BindingServiceProperties createBindingServiceProperties() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this stage of AOT processing, the env/yml is not available by default and as such the BindingServiceProperties passed into the DefaultBinderFactory have defaults and not the user-declared binders.

return this.doGetBinderConventional(name, bindingTargetType);
// If child initializers - use AOT lookup
if (!CollectionUtils.isEmpty(this.binderChildContextInitializers)) {
return this.doGetBinderAOT(name, bindingTargetType);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the AOT flavor or "doGetBinder" into its own method.

return this.getBinderInstance(configurationName);
}
else if (this.defaultBinder != null && this.binderChildContextInitializers.size() > 1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This improves the existing impl and now behaves as follows (inline comments hopefully describe this as well):

  • if no name or no default specified then fail if > 1 binders or return single binder if 1 binder available
  • if name specified be sure it exists in child initializers (else fail)
  • if default specified be sure it exists in child initializers (else fail)
  • else fail

@@ -296,16 +318,19 @@ private <T> Binder<T, ConsumerProperties, ProducerProperties> getBinderInstance(
ConfigurableApplicationContext binderProducingContext;
if (this.binderChildContextInitializers.containsKey(configurationName)) {
this.logger.info("Using AOT pre-prepared initializer to construct binder child context for " + configurationName);
if (binderConfiguration != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This carries any binder configuration properties from the yaml into the child context. AOT processing does not cover the values in the yaml file - only the context. Just like native/AOT apps still need/use application.yml files - this does to.

- Adds support for AOT child context generation
  for user-declared binder configurations.

Fixes spring-cloud#2655
@onobc onobc force-pushed the GH-2655-aot-multi-binder-fix branch from 54ccf5d to 1731493 Compare April 3, 2023 18:41
@sobychacko sobychacko merged commit a01bf30 into spring-cloud:main Apr 5, 2023
@sobychacko
Copy link
Contributor

Looks good. Merged upstream.

@onobc onobc deleted the GH-2655-aot-multi-binder-fix branch April 5, 2023 03:09
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

Successfully merging this pull request may close these issues.

AOT support for multiple binders
2 participants