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.

Sign up for GitHub

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
Merged

GH-2655: AOT support user declared binders #2689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/spring-cloud-stream/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-core-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-autoconfigure-processor</artifactId>
Expand Down
21 changes: 19 additions & 2 deletions .../src/main/java/org/springframework/cloud/stream/binder/BinderChildContextInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
import org.springframework.beans.factory.aot.BeanRegistrationAotProcessor;
import org.springframework.beans.factory.aot.BeanRegistrationCode;
import org.springframework.beans.factory.support.RegisteredBean;
import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.cloud.stream.config.BindingServiceConfiguration;
import org.springframework.cloud.stream.config.BindingServiceProperties;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationContextInitializer;
Expand Down Expand Up @@ -85,7 +89,13 @@ public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registe
if (registeredBean.getBeanClass().equals(getClass())) { //&& registeredBean.getBeanFactory().equals(this.context)) {
this.logger.debug(() -> "Beginning AOT processing for binder child contexts");
ensureBinderFactoryIsSet();
Map<String, BinderConfiguration> binderConfigurations = this.binderFactory.getBinderConfigurations();
// Load the binding service properties from the environment and update the binder factory with them
// in order to pick up any user-declared binders. Without this step only the default binder defined
// in 'META-INF/spring.binders' will be processed.
BindingServiceProperties declaredBinders = this.createBindingServiceProperties();
Map<String, BinderConfiguration> binderConfigurations = BindingServiceConfiguration.getBinderConfigurations(
this.binderFactory.getBinderTypeRegistry(), declaredBinders);
this.binderFactory.updateBinderConfigurations(binderConfigurations);
Map<String, ConfigurableApplicationContext> binderChildContexts = binderConfigurations.entrySet().stream()
.map(e -> Map.entry(e.getKey(), binderFactory.createBinderContextForAOT(e.getKey())))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
Expand All @@ -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.

BindingServiceProperties bindingServiceProperties = new BindingServiceProperties();
Binder.get(this.context.getEnvironment())
.bind("spring.cloud.stream", Bindable.ofInstance(bindingServiceProperties));
return bindingServiceProperties;
}

private void ensureBinderFactoryIsSet() {
if (this.binderFactory == null) {
Assert.notNull(this.context, () -> "Unable to lookup binder factory from context as this.context is null");
Expand All @@ -111,7 +128,7 @@ private void ensureBinderFactoryIsSet() {
@SuppressWarnings({"unused", "raw"})
public BinderChildContextInitializer withChildContextInitializers(
Map<String, ApplicationContextInitializer<? extends ConfigurableApplicationContext>> childContextInitializers) {
this.logger.debug(() -> "Replacing instance w/ one that uses; child context initializers");
this.logger.debug(() -> "Replacing instance w/ one that uses child context initializers");
Map<String, ApplicationContextInitializer<ConfigurableApplicationContext>> downcastedInitializers =
childContextInitializers.entrySet().stream()
.map(e -> Map.entry(e.getKey(), (ApplicationContextInitializer<ConfigurableApplicationContext>) e.getValue()))
Expand Down
75 changes: 50 additions & 25 deletions ...ud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,21 @@ public DefaultBinderFactory(Map<String, BinderConfiguration> binderConfiguration
this.binderCustomizer = binderCustomizer;
}

/**
* Replaces the existing binder configurations - useful in AOT processing where the binding service properties
* have to be manually loaded after the binder factory is constructed.
*
* @param binderConfigurations the updated configurations
*/
void updateBinderConfigurations(Map<String, BinderConfiguration> binderConfigurations) {
this.binderConfigurations.clear();
this.binderConfigurations.putAll(binderConfigurations);
}

BinderTypeRegistry getBinderTypeRegistry() {
return this.binderTypeRegistry;
}

@Override
public void setApplicationContext(ApplicationContext applicationContext) {
Assert.isInstanceOf(ConfigurableApplicationContext.class, applicationContext);
Expand All @@ -126,17 +141,14 @@ public void destroy() {

@SuppressWarnings({ "unchecked", "rawtypes" })
@Override
public synchronized <T> Binder<T, ?, ?> getBinder(String name,
Class<? extends T> bindingTargetType) {

public synchronized <T> Binder<T, ?, ?> getBinder(String name, Class<? extends T> bindingTargetType) {
String binderName = StringUtils.hasText(name) ? name : this.defaultBinder;

Map<String, Binder> binders = this.context == null ? Collections.emptyMap()
: this.context.getBeansOfType(Binder.class);
Map<String, Binder> binders = this.context == null ? Collections.emptyMap() : this.context.getBeansOfType(Binder.class);
Binder<T, ConsumerProperties, ProducerProperties> binder;
if (StringUtils.hasText(binderName) && binders.containsKey(binderName)) {
binder = (Binder<T, ConsumerProperties, ProducerProperties>) this.context
.getBean(binderName);
binder = (Binder<T, ConsumerProperties, ProducerProperties>) this.context.getBean(binderName);
}
else if (binders.size() == 1) {
binder = binders.values().iterator().next();
Expand All @@ -149,7 +161,7 @@ else if (binders.size() > 1) {
}
else {
/*
* This is the fall back to the old bootstrap that relies on spring.binders.
* This is the fallback to the old bootstrap that relies on spring.binders.
*/
binder = this.doGetBinder(binderName, bindingTargetType);
}
Expand All @@ -160,27 +172,37 @@ else if (binders.size() > 1) {
}

private <T> Binder<T, ConsumerProperties, ProducerProperties> doGetBinder(String name, Class<? extends T> bindingTargetType) {
if (CollectionUtils.isEmpty(this.binderChildContextInitializers)) {
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.

}
else {
if ((!StringUtils.hasText(name) || this.defaultBinder != null) && this.binderChildContextInitializers.size() == 1) {
return this.doGetBinderConventional(name, bindingTargetType);
}

private <T> Binder<T, ConsumerProperties, ProducerProperties> doGetBinderAOT(String name, Class<? extends T> bindingTargetType) {
// If neither name nor default given - return single or fail when > 1
if (!StringUtils.hasText(name) && !StringUtils.hasText(this.defaultBinder)) {
if (this.binderChildContextInitializers.size() == 1) {
String configurationName = this.binderChildContextInitializers.keySet().iterator().next();
this.logger.debug("No specific name or default given - using single available child initializer '" + configurationName + "'");
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

// Handling default binder when different binders are present on the classpath.
for (String binderName : this.binderChildContextInitializers.keySet()) {
if (binderName.equals(this.defaultBinder)) {
return this.getBinderInstance(binderName);
}
}
throw new IllegalStateException("Default binder provided, but can't determine which binder to initialize");
}
else {
throw new IllegalStateException("Can't determine which binder to use: " + name + "/" + this.binderChildContextInitializers.size());
}
throw new IllegalStateException("No specific name or default given - can't determine which binder to use");
}

// Prefer specific name over default
String configurationName = name;
if (!StringUtils.hasText(configurationName)) {
configurationName = this.defaultBinder;
}

// Check for matching child initializer
if (this.binderChildContextInitializers.containsKey(configurationName)) {
return this.getBinderInstance(configurationName);
}

throw new IllegalStateException("Requested binder '" + name + "' did not match available binders: " +
this.binderChildContextInitializers.keySet());
}

private <T> Binder<T, ConsumerProperties, ProducerProperties> doGetBinderConventional(String name,
Expand Down Expand Up @@ -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.

this.flatten(null, binderConfiguration.getProperties(), binderProperties);
}
binderProducingContext = this.createUnitializedContextForAOT(configurationName, binderProperties, binderConfiguration);
this.binderChildContextInitializers.get(configurationName).initialize(binderProducingContext);
binderProducingContext.refresh();
}
else {
this.logger.info("Constructing binder child context for " + configurationName);
Assert.state(binderConfiguration != null, "Unknown binder configuration: " + configurationName);
this.flatten(null, binderConfiguration.getProperties(), binderProperties);
BinderType binderType = this.binderTypeRegistry.get(binderConfiguration.getBinderType());
Assert.notNull(binderType, "Binder type " + binderConfiguration.getBinderType() + " is not defined");
this.flatten(null, binderConfiguration.getProperties(), binderProperties);
this.logger.info("Constructing binder child context for " + configurationName);
binderProducingContext = this.initializeBinderContextSimple(configurationName, binderProperties,
binderType, binderConfiguration, true);
}
Expand Down
2 changes: 1 addition & 1 deletion ...am/src/main/java/org/springframework/cloud/stream/config/BindingServiceConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public class BindingServiceConfiguration {
@Autowired(required = false)
private Collection<DefaultBinderFactory.Listener> binderFactoryListeners;

private static Map<String, BinderConfiguration> getBinderConfigurations(
public static Map<String, BinderConfiguration> getBinderConfigurations(
BinderTypeRegistry binderTypeRegistry,
BindingServiceProperties bindingServiceProperties) {

Expand Down
Loading