Fail fast if nested condition uses a phase inappropriate for its members

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/AbstractNestedCondition.java

@@ -61,7 +61,8 @@ public ConfigurationPhase getConfigurationPhase() {
 	public ConditionOutcome getMatchOutcome(ConditionContext context,
 			AnnotatedTypeMetadata metadata) {
 		String className = getClass().getName();
-		MemberConditions memberConditions = new MemberConditions(context, className);
+		MemberConditions memberConditions = new MemberConditions(context,
+				this.configurationPhase, className);
 		MemberMatchOutcomes memberOutcomes = new MemberMatchOutcomes(memberConditions);
 		return getFinalMatchOutcome(memberOutcomes);
 	}
@@ -108,12 +109,19 @@ private static class MemberConditions {
 
 		private final MetadataReaderFactory readerFactory;
 
+		private final ConfigurationPhase nestedPhase;
+
+		private final String nestedClassName;
+
 		private final Map<AnnotationMetadata, List<Condition>> memberConditions;
 
-		MemberConditions(ConditionContext context, String className) {
+		MemberConditions(ConditionContext context, ConfigurationPhase nestedPhase,
+				String className) {
 			this.context = context;
 			this.readerFactory = new SimpleMetadataReaderFactory(
 					context.getResourceLoader());
+			this.nestedPhase = nestedPhase;
+			this.nestedClassName = className;
 			String[] members = getMetadata(className).getMemberClassNames();
 			this.memberConditions = getMemberConditions(members);
 		}
@@ -126,13 +134,27 @@ private Map<AnnotationMetadata, List<Condition>> getMemberConditions(
 				for (String[] conditionClasses : getConditionClasses(metadata)) {
 					for (String conditionClass : conditionClasses) {
 						Condition condition = getCondition(conditionClass);
+						validateMemberCondition(condition);
 						memberConditions.add(metadata, condition);
 					}
 				}
 			}
 			return Collections.unmodifiableMap(memberConditions);
 		}
 
+		private void validateMemberCondition(Condition condition) {
+			if (this.nestedPhase == ConfigurationPhase.PARSE_CONFIGURATION
+					&& condition instanceof ConfigurationCondition) {
+				ConfigurationPhase memberPhase = ((ConfigurationCondition) condition)
+						.getConfigurationPhase();
+				if (memberPhase == ConfigurationPhase.REGISTER_BEAN) {
+					throw new IllegalStateException("Nested condition "
+							+ this.nestedClassName + " uses a configuration "
+							+ "phase that is inappropriate for " + condition.getClass());
+				}
+			}
+		}
+
 		private AnnotationMetadata getMetadata(String className) {
 			try {
 				return this.readerFactory.getMetadataReader(className)

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/AbstractNestedConditionTest.java

@@ -0,0 +1,163 @@
+/*
+ * Copyright 2012-2018 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.autoconfigure.condition;
+
+import org.junit.Test;
+
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Conditional;
+import org.springframework.context.annotation.Configuration;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link AbstractNestedCondition}.
+ *
+ * @author Razib Shahriar
+ */
+public class AbstractNestedConditionTest {
+
+	private final ApplicationContextRunner contextRunner = new ApplicationContextRunner();
+
+	@Test
+	public void validMemberPhaseEvaluatesCorrectly() {
+		this.contextRunner.withUserConfiguration(ValidConfig.class)
+				.run((context) -> assertThat(context).hasBean("myBean"));
+	}
+
+	@Test
+	public void invalidMemberPhaseThrowsIllegalState() {
+		this.contextRunner.withUserConfiguration(InvalidConfig.class).run((context) -> {
+			assertThat(context).hasFailed();
+			assertThat(context.getStartupFailure().getCause())
+					.isInstanceOf(IllegalStateException.class)
+					.hasMessageContaining("Nested condition "
+							+ InvalidNestedCondition.class.getName()
+							+ " uses a configuration phase that is inappropriate for class "
+							+ OnBeanCondition.class.getName());
+		});
+	}
+
+	@Test
+	public void invalidNestedMemberPhaseThrowsIllegalState() {
+		this.contextRunner.withUserConfiguration(DoubleNestedConfig.class)
+				.run((context) -> {
+					assertThat(context).hasFailed();
+					assertThat(context.getStartupFailure().getCause())
+							.isInstanceOf(IllegalStateException.class)
+							.hasMessageContaining("Nested condition "
+									+ DoubleNestedCondition.class.getName()
+									+ " uses a configuration phase that is inappropriate for class "
+									+ ValidNestedCondition.class.getName());
+				});
+	}
+
+	private AnnotationConfigApplicationContext load(Class<?> config) {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
+		context.register(config);
+		context.refresh();
+		return context;
+	}
+
+	@Configuration
+	@Conditional(ValidNestedCondition.class)
+	public static class ValidConfig {
+
+		@Bean
+		public String myBean() {
+			return "myBean";
+		}
+
+	}
+
+	static class ValidNestedCondition extends AbstractNestedCondition {
+
+		ValidNestedCondition() {
+			super(ConfigurationPhase.REGISTER_BEAN);
+		}
+
+		@Override
+		protected ConditionOutcome getFinalMatchOutcome(
+				MemberMatchOutcomes memberOutcomes) {
+			return ConditionOutcome.match();
+		}
+
+		@ConditionalOnMissingBean(name = "myBean")
+		static class MissingMyBean {
+
+		}
+
+	}
+
+	@Configuration
+	@Conditional(InvalidNestedCondition.class)
+	public static class InvalidConfig {
+
+		@Bean
+		public String myBean() {
+			return "myBean";
+		}
+
+	}
+
+	static class InvalidNestedCondition extends AbstractNestedCondition {
+
+		InvalidNestedCondition() {
+			super(ConfigurationPhase.PARSE_CONFIGURATION);
+		}
+
+		@Override
+		protected ConditionOutcome getFinalMatchOutcome(
+				MemberMatchOutcomes memberOutcomes) {
+			return ConditionOutcome.match();
+		}
+
+		@ConditionalOnMissingBean(name = "myBean")
+		static class MissingMyBean {
+
+		}
+
+	}
+
+	@Configuration
+	@Conditional(DoubleNestedCondition.class)
+	public static class DoubleNestedConfig {
+
+	}
+
+	static class DoubleNestedCondition extends AbstractNestedCondition {
+
+		DoubleNestedCondition() {
+			super(ConfigurationPhase.PARSE_CONFIGURATION);
+		}
+
+		@Override
+		protected ConditionOutcome getFinalMatchOutcome(
+				MemberMatchOutcomes memberOutcomes) {
+			return ConditionOutcome.match();
+		}
+
+		@Conditional(ValidNestedCondition.class)
+		static class NestedConditionThatIsValid {
+
+		}
+
+	}
+
+}