Polish "Handle null value in OnExpressionCondition"

Closes gh-8128

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -52,7 +52,7 @@ class OnExpressionCondition extends SpringBootCondition {
 			resolver = new StandardBeanExpressionResolver();
 		}
 		Object result = resolver.evaluate(expression, expressionContext);
-		boolean match = result == null ? false : (boolean) result;
+		boolean match = result != null && (boolean) result;
 		return new ConditionOutcome(match, ConditionMessage
 				.forCondition(ConditionalOnExpression.class, "(" + rawExpression + ")")
 				.resultedIn(result));

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -28,13 +28,14 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link ConditionalOnExpression}.
  *
  * @author Dave Syer
+ * @author Stephane Nicoll
  */
 public class ConditionalOnExpressionTests {
 
 	private final AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
 
 	@Test
-	public void testResourceExists() {
+	public void expressionIsTrue() {
 		this.context.register(BasicConfiguration.class);
 		this.context.refresh();
 		assertThat(this.context.containsBean("foo")).isTrue();
@@ -42,12 +43,19 @@ public class ConditionalOnExpressionTests {
 	}
 
 	@Test
-	public void testResourceNotExists() {
+	public void expressionIsFalse() {
 		this.context.register(MissingConfiguration.class);
 		this.context.refresh();
 		assertThat(this.context.containsBean("foo")).isFalse();
 	}
 
+	@Test
+	public void expressionIsNull() {
+		this.context.register(NullConfiguration.class);
+		this.context.refresh();
+		assertThat(this.context.containsBean("foo")).isFalse();
+	}
+
 	@Configuration
 	@ConditionalOnExpression("false")
 	protected static class MissingConfiguration {
@@ -70,4 +78,15 @@ public class ConditionalOnExpressionTests {
 
 	}
 
+	@Configuration
+	@ConditionalOnExpression("true ? null : false")
+	protected static class NullConfiguration {
+
+		@Bean
+		public String foo() {
+			return "foo";
+		}
+
+	}
+
 }

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

@@ -1,82 +0,0 @@
-/*
- * Copyright 2012-2017 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 java.util.Collections;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-import org.springframework.beans.factory.config.BeanExpressionResolver;
-import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
-import org.springframework.context.annotation.ConditionContext;
-import org.springframework.core.env.Environment;
-import org.springframework.core.type.AnnotatedTypeMetadata;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.BDDMockito.when;
-
-/**
- * Tests for the {@link OnExpressionCondition}.
- *
- * @author Kristine Jetzke
- */
-@RunWith(MockitoJUnitRunner.class)
-public class OnExpressionConditionTest {
-
-	@Mock
-	private ConditionContext context;
-
-	@Mock
-	private AnnotatedTypeMetadata metadata;
-
-	@Mock
-	private Environment environment;
-
-	@Mock
-	private ConfigurableListableBeanFactory beanFactory;
-
-	@Mock
-	private BeanExpressionResolver resolver;
-
-	@Before
-	public void setup() {
-		when(this.context.getEnvironment()).thenReturn(this.environment);
-		when(this.environment.resolvePlaceholders(any())).thenReturn("foo");
-		when(this.context.getBeanFactory()).thenReturn(this.beanFactory);
-		when(this.beanFactory.getBeanExpressionResolver()).thenReturn(this.resolver);
-
-		when(this.metadata.getAnnotationAttributes(any()))
-				.thenReturn(Collections.singletonMap("value", "bar"));
-	}
-
-	@Test
-	public void nullShouldEvaluateToFalse() {
-		when(this.resolver.evaluate(any(), any())).thenReturn(null);
-
-		OnExpressionCondition condition = new OnExpressionCondition();
-		ConditionOutcome conditionOutcome = condition.getMatchOutcome(this.context,
-				this.metadata);
-
-		assertThat(conditionOutcome.isMatch()).isFalse();
-	}
-
-}