Skip to content

Commit

Permalink
Prohibit circular references by default
Browse files Browse the repository at this point in the history
Closes gh-27652
  • Loading branch information
wilkinsona committed Aug 17, 2021
1 parent 228e4e3 commit 01e741d
Show file tree
Hide file tree
Showing 8 changed files with 193 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.BeanDefinitionCustomizer;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory;
import org.springframework.beans.factory.support.BeanNameGenerator;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.boot.context.annotation.Configurations;
Expand Down Expand Up @@ -199,6 +200,17 @@ public SELF withAllowBeanDefinitionOverriding(boolean allowBeanDefinitionOverrid
return newInstance(this.runnerConfiguration.withAllowBeanDefinitionOverriding(allowBeanDefinitionOverriding));
}

/**
* Specify if circular references between beans should be allowed.
* @param allowCircularReferences if circular references between beans are allowed
* @return a new instance with the updated circular references policy
* @since 2.6.0
* @see AbstractAutowireCapableBeanFactory#setAllowCircularReferences(boolean)
*/
public SELF withAllowCircularReferences(boolean allowCircularReferences) {
return newInstance(this.runnerConfiguration.withAllowCircularReferences(allowCircularReferences));
}

/**
* Add an {@link ApplicationContextInitializer} to be called when the context is
* created.
Expand Down Expand Up @@ -427,9 +439,13 @@ private A createAssertableContext() {
private C createAndLoadContext() {
C context = this.runnerConfiguration.contextFactory.get();
ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
if (beanFactory instanceof DefaultListableBeanFactory) {
((DefaultListableBeanFactory) beanFactory)
.setAllowBeanDefinitionOverriding(this.runnerConfiguration.allowBeanDefinitionOverriding);
if (beanFactory instanceof AbstractAutowireCapableBeanFactory) {
((AbstractAutowireCapableBeanFactory) beanFactory)
.setAllowCircularReferences(this.runnerConfiguration.allowCircularReferences);
if (beanFactory instanceof DefaultListableBeanFactory) {
((DefaultListableBeanFactory) beanFactory)
.setAllowBeanDefinitionOverriding(this.runnerConfiguration.allowBeanDefinitionOverriding);
}
}
try {
configureContext(context);
Expand Down Expand Up @@ -504,6 +520,8 @@ protected static final class RunnerConfiguration<C extends ConfigurableApplicati

private boolean allowBeanDefinitionOverriding = false;

private boolean allowCircularReferences = false;

private List<ApplicationContextInitializer<? super C>> initializers = Collections.emptyList();

private TestPropertyValues environmentProperties = TestPropertyValues.empty();
Expand All @@ -525,6 +543,7 @@ private RunnerConfiguration(Supplier<C> contextFactory) {
private RunnerConfiguration(RunnerConfiguration<C> source) {
this.contextFactory = source.contextFactory;
this.allowBeanDefinitionOverriding = source.allowBeanDefinitionOverriding;
this.allowCircularReferences = source.allowCircularReferences;
this.initializers = source.initializers;
this.environmentProperties = source.environmentProperties;
this.systemProperties = source.systemProperties;
Expand All @@ -540,6 +559,12 @@ private RunnerConfiguration<C> withAllowBeanDefinitionOverriding(boolean allowBe
return config;
}

private RunnerConfiguration<C> withAllowCircularReferences(boolean allowCircularReferences) {
RunnerConfiguration<C> config = new RunnerConfiguration<>(this);
config.allowCircularReferences = allowCircularReferences;
return config;
}

private RunnerConfiguration<C> withInitializer(ApplicationContextInitializer<? super C> initializer) {
Assert.notNull(initializer, "Initializer must not be null");
RunnerConfiguration<C> config = new RunnerConfiguration<>(this);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,7 +23,10 @@
import com.google.gson.Gson;
import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.BeanCurrentlyInCreationException;
import org.springframework.beans.factory.BeanDefinitionStoreException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.assertj.ApplicationContextAssertProvider;
Expand Down Expand Up @@ -181,6 +184,22 @@ void runDisablesBeanOverridingByDefault() {
});
}

@Test
void runDisablesCircularReferencesByDefault() {
get().withUserConfiguration(ExampleConsumerConfiguration.class, ExampleProducerConfiguration.class)
.run((context) -> {
assertThat(context).hasFailed();
assertThat(context).getFailure().hasRootCauseInstanceOf(BeanCurrentlyInCreationException.class);
});
}

@Test
void circularReferencesCanBeAllowed() {
get().withAllowCircularReferences(true)
.withUserConfiguration(ExampleConsumerConfiguration.class, ExampleProducerConfiguration.class)
.run((context) -> assertThat(context).hasNotFailed());
}

@Test
void runWithUserBeanShouldBeRegisteredInOrder() {
get().withAllowBeanDefinitionOverriding(true).withBean(String.class, () -> "one")
Expand Down Expand Up @@ -250,4 +269,41 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)

}

static class Example {

}

@FunctionalInterface
interface ExampleConfigurer {

void configure(Example example);

}

@Configuration(proxyBeanMethods = false)
static class ExampleProducerConfiguration {

@Bean
Example example(ObjectProvider<ExampleConfigurer> configurers) {
Example example = new Example();
configurers.orderedStream().forEach((configurer) -> configurer.configure(example));
return example;
}

}

@Configuration(proxyBeanMethods = false)
static class ExampleConsumerConfiguration {

@Autowired
Example example;

@Bean
ExampleConfigurer configurer() {
return (example) -> {
};
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader;
import org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.beans.factory.support.BeanNameGenerator;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
Expand Down Expand Up @@ -212,6 +213,8 @@ public class SpringApplication {

private boolean allowBeanDefinitionOverriding;

private boolean allowCircularReferences;

private boolean isCustomEnvironment = false;

private boolean lazyInitialization = false;
Expand Down Expand Up @@ -374,9 +377,12 @@ private void prepareContext(DefaultBootstrapContext bootstrapContext, Configurab
if (printedBanner != null) {
beanFactory.registerSingleton("springBootBanner", printedBanner);
}
if (beanFactory instanceof DefaultListableBeanFactory) {
((DefaultListableBeanFactory) beanFactory)
.setAllowBeanDefinitionOverriding(this.allowBeanDefinitionOverriding);
if (beanFactory instanceof AbstractAutowireCapableBeanFactory) {
((AbstractAutowireCapableBeanFactory) beanFactory).setAllowCircularReferences(this.allowCircularReferences);
if (beanFactory instanceof DefaultListableBeanFactory) {
((DefaultListableBeanFactory) beanFactory)
.setAllowBeanDefinitionOverriding(this.allowBeanDefinitionOverriding);
}
}
if (this.lazyInitialization) {
context.addBeanFactoryPostProcessor(new LazyInitializationBeanFactoryPostProcessor());
Expand Down Expand Up @@ -918,6 +924,17 @@ public void setAllowBeanDefinitionOverriding(boolean allowBeanDefinitionOverridi
this.allowBeanDefinitionOverriding = allowBeanDefinitionOverriding;
}

/**
* Sets whether to allow circular references between beans and automatically try to
* resolve them. Defaults to {@code false}.
* @param allowCircularReferences if circular references are allowed
* @since 2.6.0
* @see AbstractAutowireCapableBeanFactory#setAllowCircularReferences(boolean)
*/
public void setAllowCircularReferences(boolean allowCircularReferences) {
this.allowCircularReferences = allowCircularReferences;
}

/**
* Sets if beans should be initialized lazily. Defaults to {@code false}.
* @param lazyInitialization if initialization should be lazy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory;
import org.springframework.beans.factory.support.BeanNameGenerator;
import org.springframework.boot.ApplicationContextFactory;
import org.springframework.boot.Banner;
Expand Down Expand Up @@ -600,4 +601,17 @@ public SpringApplicationBuilder applicationStartup(ApplicationStartup applicatio
return this;
}

/**
* Whether to allow circular references between beans and automatically try to resolve
* them.
* @param allowCircularReferences whether circular references are allows
* @return the current builder
* @since 2.6.0
* @see AbstractAutowireCapableBeanFactory#setAllowCircularReferences(boolean)
*/
public SpringApplicationBuilder allowCircularReferences(boolean allowCircularReferences) {
this.application.setAllowCircularReferences(allowCircularReferences);
return this;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ protected FailureAnalysis analyze(Throwable rootFailure, BeanCurrentlyInCreation
if (dependencyCycle == null) {
return null;
}
return new FailureAnalysis(buildMessage(dependencyCycle), null, cause);
return new FailureAnalysis(buildMessage(dependencyCycle),
"Relying upon circular references is discouraged and they are prohibited by default. "
+ "Update your application to remove the dependency cycle between beans. "
+ "As a last resort, it may be possible to break the cycle automatically be setting "
+ "spring.main.allow-circular-references to true if you have not already done so.",
cause);
}

private DependencyCycle findCycle(Throwable rootFailure) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,13 @@
"description": "Whether bean definition overriding, by registering a definition with the same name as an existing definition, is allowed.",
"defaultValue": false
},
{
"name": "spring.main.allow-circular-references",
"type": "java.lang.Boolean",
"sourceType": "org.springframework.boot.SpringApplication",
"description": "Whether to allow circular references between beans and automatically try to resolve them.",
"defaultValue": false
},
{
"name": "spring.main.banner-mode",
"type": "org.springframework.boot.Banner$Mode",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@

import org.springframework.beans.CachedIntrospectionResults;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.BeanCurrentlyInCreationException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.UnsatisfiedDependencyException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.support.BeanDefinitionOverrideException;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
Expand Down Expand Up @@ -115,6 +119,7 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.argThat;
Expand Down Expand Up @@ -1099,6 +1104,21 @@ void beanDefinitionOverridingCanBeEnabled() {
.getBean("someBean")).isEqualTo("override");
}

@Test
void circularReferencesAreDisabledByDefault() {
assertThatExceptionOfType(UnsatisfiedDependencyException.class)
.isThrownBy(() -> new SpringApplication(ExampleProducerConfiguration.class,
ExampleConsumerConfiguration.class).run("--spring.main.web-application-type=none"))
.withRootCauseInstanceOf(BeanCurrentlyInCreationException.class);
}

@Test
void circularReferencesCanBeEnabled() {
assertThatNoException().isThrownBy(
() -> new SpringApplication(ExampleProducerConfiguration.class, ExampleConsumerConfiguration.class).run(
"--spring.main.web-application-type=none", "--spring.main.allow-circular-references=true"));
}

@Test
void relaxedBindingShouldWorkBeforeEnvironmentIsPrepared() {
SpringApplication application = new SpringApplication(ExampleConfig.class);
Expand Down Expand Up @@ -1698,4 +1718,41 @@ <E extends ApplicationEvent> E getEvent(Class<E> type) {

}

static class Example {

}

@FunctionalInterface
interface ExampleConfigurer {

void configure(Example example);

}

@Configuration(proxyBeanMethods = false)
static class ExampleProducerConfiguration {

@Bean
Example example(ObjectProvider<ExampleConfigurer> configurers) {
Example example = new Example();
configurers.orderedStream().forEach((configurer) -> configurer.configure(example));
return example;
}

}

@Configuration(proxyBeanMethods = false)
static class ExampleConsumerConfiguration {

@Autowired
Example example;

@Bean
ExampleConfigurer configurer() {
return (example) -> {
};
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ void cyclicBeanMethods() throws IOException {
assertThat(lines.get(6)).isEqualTo("↑ ↓");
assertThat(lines.get(7)).startsWith("| three defined in " + CyclicBeanMethodsConfiguration.class.getName());
assertThat(lines.get(8)).isEqualTo("└─────┘");
assertThat(analysis.getAction()).isNotNull();
}

@Test
Expand All @@ -84,6 +85,7 @@ void cycleWithAutowiredFields() throws IOException {
assertThat(lines.get(7)).startsWith(
"| " + BeanTwoConfiguration.class.getName() + " (field private " + BeanThree.class.getName());
assertThat(lines.get(8)).isEqualTo("└─────┘");
assertThat(analysis.getAction()).isNotNull();
}

@Test
Expand All @@ -107,6 +109,7 @@ void cycleReferencedViaOtherBeans() throws IOException {
assertThat(lines.get(10))
.startsWith("| three defined in " + CycleReferencedViaOtherBeansConfiguration.class.getName());
assertThat(lines.get(11)).isEqualTo("└─────┘");
assertThat(analysis.getAction()).isNotNull();
}

@Test
Expand All @@ -120,6 +123,7 @@ void testSelfReferenceCycle() throws IOException {
assertThat(lines.get(2)).isEqualTo("┌──->──┐");
assertThat(lines.get(3)).startsWith("| bean defined in " + SelfReferenceBeanConfiguration.class.getName());
assertThat(lines.get(4)).isEqualTo("└──<-──┘");
assertThat(analysis.getAction()).isNotNull();
}

@Test
Expand Down

0 comments on commit 01e741d

Please sign in to comment.