From 9c146c0a405f2088fec6b2f86e1991dd2512e316 Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Thu, 9 Jan 2025 12:24:03 +0200 Subject: [PATCH 1/2] Polish OnPropertyCondition Signed-off-by: Dmytro Nosan See gh-43754 --- .../condition/OnPropertyCondition.java | 38 ++++++----- .../ConditionalOnBooleanPropertyTests.java | 67 +++++++++++++++++++ .../condition/ConditionalOnPropertyTests.java | 25 ++++++- 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java index d3e1d663b6..3f86cc35ef 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java @@ -16,9 +16,9 @@ package org.springframework.boot.autoconfigure.condition; +import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.stream.Stream; import org.springframework.boot.autoconfigure.condition.ConditionMessage.Style; @@ -33,6 +33,7 @@ import org.springframework.core.annotation.Order; import org.springframework.core.env.PropertyResolver; import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; /** @@ -43,6 +44,7 @@ import org.springframework.util.StringUtils; * @author Stephane Nicoll * @author Andy Wilkinson * @see ConditionalOnProperty + * @see ConditionalOnBooleanProperty */ @Order(Ordered.HIGHEST_PRECEDENCE + 40) class OnPropertyCondition extends SpringBootCondition { @@ -50,16 +52,15 @@ class OnPropertyCondition extends SpringBootCondition { @Override public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) { MergedAnnotations annotations = metadata.getAnnotations(); - List allAnnotationAttributes = Stream + List> allAnnotations = Stream .concat(annotations.stream(ConditionalOnProperty.class.getName()), annotations.stream(ConditionalOnBooleanProperty.class.getName())) .filter(MergedAnnotationPredicates.unique(MergedAnnotation::getMetaTypes)) - .map(MergedAnnotation::asAnnotationAttributes) .toList(); List noMatch = new ArrayList<>(); List match = new ArrayList<>(); - for (AnnotationAttributes annotationAttributes : allAnnotationAttributes) { - ConditionOutcome outcome = determineOutcome(annotationAttributes, context.getEnvironment()); + for (MergedAnnotation annotation : allAnnotations) { + ConditionOutcome outcome = determineOutcome(annotation, context.getEnvironment()); (outcome.isMatch() ? match : noMatch).add(outcome.getConditionMessage()); } if (!noMatch.isEmpty()) { @@ -68,27 +69,29 @@ class OnPropertyCondition extends SpringBootCondition { return ConditionOutcome.match(ConditionMessage.of(match)); } - private ConditionOutcome determineOutcome(AnnotationAttributes annotationAttributes, PropertyResolver resolver) { - Spec spec = new Spec(annotationAttributes); + private ConditionOutcome determineOutcome(MergedAnnotation annotation, PropertyResolver resolver) { + Class annotationType = annotation.getType(); + Spec spec = new Spec(annotationType, annotation.asAnnotationAttributes()); List missingProperties = new ArrayList<>(); List nonMatchingProperties = new ArrayList<>(); spec.collectProperties(resolver, missingProperties, nonMatchingProperties); if (!missingProperties.isEmpty()) { - return ConditionOutcome.noMatch(ConditionMessage.forCondition(ConditionalOnProperty.class, spec) + return ConditionOutcome.noMatch(ConditionMessage.forCondition(annotationType, spec) .didNotFind("property", "properties") .items(Style.QUOTE, missingProperties)); } if (!nonMatchingProperties.isEmpty()) { - return ConditionOutcome.noMatch(ConditionMessage.forCondition(ConditionalOnProperty.class, spec) + return ConditionOutcome.noMatch(ConditionMessage.forCondition(annotationType, spec) .found("different value in property", "different value in properties") .items(Style.QUOTE, nonMatchingProperties)); } - return ConditionOutcome - .match(ConditionMessage.forCondition(ConditionalOnProperty.class, spec).because("matched")); + return ConditionOutcome.match(ConditionMessage.forCondition(annotationType, spec).because("matched")); } private static class Spec { + private final Class annotationType; + private final String prefix; private final String[] names; @@ -97,7 +100,8 @@ class OnPropertyCondition extends SpringBootCondition { private final boolean matchIfMissing; - Spec(AnnotationAttributes annotationAttributes) { + Spec(Class annotationType, AnnotationAttributes annotationAttributes) { + this.annotationType = annotationType; this.prefix = (!annotationAttributes.containsKey("prefix")) ? "" : getPrefix(annotationAttributes); this.names = getNames(annotationAttributes); this.havingValue = annotationAttributes.get("havingValue").toString(); @@ -112,13 +116,13 @@ class OnPropertyCondition extends SpringBootCondition { return prefix; } - private String[] getNames(Map annotationAttributes) { + private String[] getNames(AnnotationAttributes annotationAttributes) { String[] value = (String[]) annotationAttributes.get("value"); String[] name = (String[]) annotationAttributes.get("name"); - Assert.state(value.length > 0 || name.length > 0, - "The name or value attribute of @ConditionalOnProperty must be specified"); - Assert.state(value.length == 0 || name.length == 0, - "The name and value attributes of @ConditionalOnProperty are exclusive"); + Assert.state(value.length > 0 || name.length > 0, "The name or value attribute of @%s must be specified" + .formatted(ClassUtils.getShortName(this.annotationType))); + Assert.state(value.length == 0 || name.length == 0, "The name and value attributes of @%s are exclusive" + .formatted(ClassUtils.getShortName(this.annotationType))); return (value.length > 0) ? value : name; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java index 4b083b90cf..aff83eac9e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.autoconfigure.condition; +import java.util.function.Consumer; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -29,6 +31,7 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.StandardEnvironment; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link ConditionalOnBooleanProperty @ConditionalOnBooleanProperty}. @@ -144,6 +147,48 @@ class ConditionalOnBooleanPropertyTests { assertThat(this.context.containsBean("foo")).isTrue(); } + @Test + void nameOrValueMustBeSpecified() { + assertThatIllegalStateException().isThrownBy(() -> load(NoNameOrValueAttribute.class, "some.property")) + .satisfies(causeMessageContaining( + "The name or value attribute of @ConditionalOnBooleanProperty must be specified")); + } + + @Test + void nameAndValueMustNotBeSpecified() { + assertThatIllegalStateException().isThrownBy(() -> load(NameAndValueAttribute.class, "some.property")) + .satisfies(causeMessageContaining( + "The name and value attributes of @ConditionalOnBooleanProperty are exclusive")); + } + + @Test + void conditionReportWhenMatched() { + load(Defaults.class, "test=true"); + assertThat(this.context.containsBean("foo")).isTrue(); + assertThat(getConditionEvaluationReport()).contains("@ConditionalOnBooleanProperty (test=true) matched"); + } + + @Test + void conditionReportWhenDoesNotMatch() { + load(Defaults.class, "test=false"); + assertThat(this.context.containsBean("foo")).isFalse(); + assertThat(getConditionEvaluationReport()) + .contains("@ConditionalOnBooleanProperty (test=true) found different value in property 'test'"); + } + + private Consumer causeMessageContaining(String message) { + return (ex) -> assertThat(ex.getCause()).hasMessageContaining(message); + } + + private String getConditionEvaluationReport() { + ConditionEvaluationReport report = ConditionEvaluationReport.get(this.context.getBeanFactory()); + StringBuilder builder = new StringBuilder(); + report.getConditionAndOutcomesBySource() + .values() + .forEach((outcomes) -> outcomes.forEach((outcome) -> builder.append(outcome.toString()).append('\n'))); + return builder.toString(); + } + private void load(Class config, String... environment) { TestPropertyValues.of(environment).applyTo(this.environment); this.context = new SpringApplicationBuilder(config).environment(this.environment) @@ -196,4 +241,26 @@ class ConditionalOnBooleanPropertyTests { } + @Configuration(proxyBeanMethods = false) + @ConditionalOnBooleanProperty + static class NoNameOrValueAttribute { + + @Bean + String foo() { + return "foo"; + } + + } + + @Configuration(proxyBeanMethods = false) + @ConditionalOnBooleanProperty(value = "x", name = "y") + static class NameAndValueAttribute { + + @Bean + String foo() { + return "foo"; + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java index f663915ce9..73d8dafff1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2025 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. @@ -271,6 +271,20 @@ class ConditionalOnPropertyTests { assertThat(this.context.containsBean("foo")).isTrue(); } + @Test + void conditionReportWhenMatched() { + load(MultiplePropertiesRequiredConfiguration.class, "property1=value1", "property2=value2"); + assertThat(this.context.containsBean("foo")).isTrue(); + assertThat(getConditionEvaluationReport()).contains("@ConditionalOnProperty ([property1,property2]) matched"); + } + + @Test + void conditionReportWhenDoesNotMatch() { + load(MultiplePropertiesRequiredConfiguration.class, "property1=value1"); + assertThat(getConditionEvaluationReport()) + .contains("@ConditionalOnProperty ([property1,property2]) did not find property 'property2'"); + } + private void load(Class config, String... environment) { TestPropertyValues.of(environment).applyTo(this.environment); this.context = new SpringApplicationBuilder(config).environment(this.environment) @@ -278,6 +292,15 @@ class ConditionalOnPropertyTests { .run(); } + private String getConditionEvaluationReport() { + ConditionEvaluationReport report = ConditionEvaluationReport.get(this.context.getBeanFactory()); + StringBuilder builder = new StringBuilder(); + report.getConditionAndOutcomesBySource() + .values() + .forEach((outcomes) -> outcomes.forEach((outcome) -> builder.append(outcome.toString()).append('\n'))); + return builder.toString(); + } + @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(name = { "property1", "property2" }) static class MultiplePropertiesRequiredConfiguration { From 4812328be2945752ba72d985e05565b698148c5c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 13 Jan 2025 14:25:20 -0800 Subject: [PATCH 2/2] Polish OnPropertyCondition See gh-43754 --- .../condition/ConditionEvaluationReport.java | 11 +++++++++++ .../condition/OnPropertyCondition.java | 10 ++++------ .../ConditionalOnBooleanPropertyTests.java | 13 ++++++++----- .../condition/ConditionalOnPropertyTests.java | 13 ++++++++----- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java index 702a456d82..5b87e4553e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java @@ -27,6 +27,8 @@ import java.util.Map; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -237,6 +239,15 @@ public final class ConditionEvaluationReport { return true; } + /** + * Return a {@link Stream} of the {@link ConditionAndOutcome} items. + * @return a stream of the {@link ConditionAndOutcome} items. + * @since 3.5.0 + */ + public Stream stream() { + return StreamSupport.stream(spliterator(), false); + } + @Override public Iterator iterator() { return Collections.unmodifiableSet(this.outcomes).iterator(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java index 3f86cc35ef..45d65167d1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnPropertyCondition.java @@ -28,7 +28,6 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationPredicates; -import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.Order; import org.springframework.core.env.PropertyResolver; import org.springframework.core.type.AnnotatedTypeMetadata; @@ -51,15 +50,14 @@ class OnPropertyCondition extends SpringBootCondition { @Override public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) { - MergedAnnotations annotations = metadata.getAnnotations(); - List> allAnnotations = Stream - .concat(annotations.stream(ConditionalOnProperty.class.getName()), - annotations.stream(ConditionalOnBooleanProperty.class.getName())) + List> annotations = Stream + .concat(metadata.getAnnotations().stream(ConditionalOnProperty.class.getName()), + metadata.getAnnotations().stream(ConditionalOnBooleanProperty.class.getName())) .filter(MergedAnnotationPredicates.unique(MergedAnnotation::getMetaTypes)) .toList(); List noMatch = new ArrayList<>(); List match = new ArrayList<>(); - for (MergedAnnotation annotation : allAnnotations) { + for (MergedAnnotation annotation : annotations) { ConditionOutcome outcome = determineOutcome(annotation, context.getEnvironment()); (outcome.isMatch() ? match : noMatch).add(outcome.getConditionMessage()); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java index aff83eac9e..32bb1f2744 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBooleanPropertyTests.java @@ -17,11 +17,13 @@ package org.springframework.boot.autoconfigure.condition; import java.util.function.Consumer; +import java.util.stream.Collectors; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.boot.WebApplicationType; +import org.springframework.boot.autoconfigure.condition.ConditionEvaluationReport.ConditionAndOutcomes; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.context.ConfigurableApplicationContext; @@ -181,12 +183,13 @@ class ConditionalOnBooleanPropertyTests { } private String getConditionEvaluationReport() { - ConditionEvaluationReport report = ConditionEvaluationReport.get(this.context.getBeanFactory()); - StringBuilder builder = new StringBuilder(); - report.getConditionAndOutcomesBySource() + return ConditionEvaluationReport.get(this.context.getBeanFactory()) + .getConditionAndOutcomesBySource() .values() - .forEach((outcomes) -> outcomes.forEach((outcome) -> builder.append(outcome.toString()).append('\n'))); - return builder.toString(); + .stream() + .flatMap(ConditionAndOutcomes::stream) + .map(Object::toString) + .collect(Collectors.joining("\n")); } private void load(Class config, String... environment) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java index 73d8dafff1..9df542bae1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnPropertyTests.java @@ -21,11 +21,13 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.util.function.Consumer; +import java.util.stream.Collectors; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.boot.WebApplicationType; +import org.springframework.boot.autoconfigure.condition.ConditionEvaluationReport.ConditionAndOutcomes; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.context.ConfigurableApplicationContext; @@ -293,12 +295,13 @@ class ConditionalOnPropertyTests { } private String getConditionEvaluationReport() { - ConditionEvaluationReport report = ConditionEvaluationReport.get(this.context.getBeanFactory()); - StringBuilder builder = new StringBuilder(); - report.getConditionAndOutcomesBySource() + return ConditionEvaluationReport.get(this.context.getBeanFactory()) + .getConditionAndOutcomesBySource() .values() - .forEach((outcomes) -> outcomes.forEach((outcome) -> builder.append(outcome.toString()).append('\n'))); - return builder.toString(); + .stream() + .flatMap(ConditionAndOutcomes::stream) + .map(Object::toString) + .collect(Collectors.joining("\n")); } @Configuration(proxyBeanMethods = false)