Prohibit circular references by default

Closes gh-27652

spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java

@@ -26,6 +26,7 @@ import java.util.function.Supplier;
 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;
@@ -199,6 +200,17 @@ public abstract class AbstractApplicationContextRunner<SELF extends AbstractAppl
 		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.
@@ -427,9 +439,13 @@ public abstract class AbstractApplicationContextRunner<SELF extends AbstractAppl
 	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);
@@ -504,6 +520,8 @@ public abstract class AbstractApplicationContextRunner<SELF extends AbstractAppl
 
 		private boolean allowBeanDefinitionOverriding = false;
 
+		private boolean allowCircularReferences = false;
+
 		private List<ApplicationContextInitializer<? super C>> initializers = Collections.emptyList();
 
 		private TestPropertyValues environmentProperties = TestPropertyValues.empty();
@@ -525,6 +543,7 @@ public abstract class AbstractApplicationContextRunner<SELF extends AbstractAppl
 		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;
@@ -540,6 +559,12 @@ public abstract class AbstractApplicationContextRunner<SELF extends AbstractAppl
 			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);

spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java

@@ -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.
@@ -23,7 +23,10 @@ import java.util.concurrent.atomic.AtomicBoolean;
 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;
@@ -181,6 +184,22 @@ abstract class AbstractApplicationContextRunnerTests<T extends AbstractApplicati
 		});
 	}
 
+	@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")
@@ -250,4 +269,41 @@ abstract class AbstractApplicationContextRunnerTests<T extends AbstractApplicati
 
 	}
 
+	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) -> {
+			};
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java

@@ -36,6 +36,7 @@ import org.springframework.beans.CachedIntrospectionResults;
 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;
@@ -212,6 +213,8 @@ public class SpringApplication {
 
 	private boolean allowBeanDefinitionOverriding;
 
+	private boolean allowCircularReferences;
+
 	private boolean isCustomEnvironment = false;
 
 	private boolean lazyInitialization = false;
@@ -374,9 +377,12 @@ public class SpringApplication {
 		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());
@@ -918,6 +924,17 @@ public class SpringApplication {
 		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

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/builder/SpringApplicationBuilder.java

@@ -27,6 +27,7 @@ import java.util.Properties;
 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;
@@ -600,4 +601,17 @@ public class SpringApplicationBuilder {
 		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;
+	}
+
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java

@@ -41,7 +41,12 @@ class BeanCurrentlyInCreationFailureAnalyzer extends AbstractFailureAnalyzer<Bea
 		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) {

spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -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",

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java

@@ -42,6 +42,10 @@ import reactor.core.publisher.Mono;
 
 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;
@@ -115,6 +119,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 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;
@@ -1099,6 +1104,21 @@ class SpringApplicationTests {
 				.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);
@@ -1698,4 +1718,41 @@ class SpringApplicationTests {
 
 	}
 
+	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) -> {
+			};
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java

@@ -64,6 +64,7 @@ class BeanCurrentlyInCreationFailureAnalyzerTests {
 		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
@@ -84,6 +85,7 @@ class BeanCurrentlyInCreationFailureAnalyzerTests {
 		assertThat(lines.get(7)).startsWith(
 				"|  " + BeanTwoConfiguration.class.getName() + " (field private " + BeanThree.class.getName());
 		assertThat(lines.get(8)).isEqualTo("└─────┘");
+		assertThat(analysis.getAction()).isNotNull();
 	}
 
 	@Test
@@ -107,6 +109,7 @@ class BeanCurrentlyInCreationFailureAnalyzerTests {
 		assertThat(lines.get(10))
 				.startsWith("|  three defined in " + CycleReferencedViaOtherBeansConfiguration.class.getName());
 		assertThat(lines.get(11)).isEqualTo("└─────┘");
+		assertThat(analysis.getAction()).isNotNull();
 	}
 
 	@Test
@@ -120,6 +123,7 @@ class BeanCurrentlyInCreationFailureAnalyzerTests {
 		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