From 85f86243c90d7cb4d76639ccab18fcefe90b7c6d Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 10 Oct 2018 21:15:32 -0700 Subject: [PATCH] Order class filter checks and exit early Update the logic in `OnClassCondition` so that filtering exits on the first missing class. Also refactor the implementation to save unnecessary `Set` creation when there is just a single class to check. The `AutoConfigureAnnotationProcessor` has also been updated to order classes so that any starting `org.springframework` are considered last. The assumption being that other classes are more likely to be missing. Closes gh-12131 --- .../condition/OnClassCondition.java | 36 +++++++--- ...ionAutoConfigurationImportFilterTests.java | 12 ++-- .../AutoConfigureAnnotationProcessor.java | 67 ++++++++++++++----- ...AutoConfigureAnnotationProcessorTests.java | 2 +- .../TestClassConfiguration.java | 3 +- 5 files changed, 83 insertions(+), 37 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnClassCondition.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnClassCondition.java index 6ba4a8bef4..2892804069 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnClassCondition.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnClassCondition.java @@ -20,7 +20,6 @@ import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Set; import org.springframework.boot.autoconfigure.AutoConfigurationImportFilter; import org.springframework.boot.autoconfigure.AutoConfigurationMetadata; @@ -31,6 +30,7 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; /** * {@link Condition} and {@link AutoConfigurationImportFilter} that checks for the @@ -200,8 +200,8 @@ class OnClassCondition extends FilteringSpringBootCondition { for (int i = start; i < end; i++) { String autoConfigurationClass = autoConfigurationClasses[i]; if (autoConfigurationClass != null) { - Set candidates = autoConfigurationMetadata - .getSet(autoConfigurationClass, "ConditionalOnClass"); + String candidates = autoConfigurationMetadata + .get(autoConfigurationClass, "ConditionalOnClass"); if (candidates != null) { outcomes[i - start] = getOutcome(candidates); } @@ -210,15 +210,19 @@ class OnClassCondition extends FilteringSpringBootCondition { return outcomes; } - private ConditionOutcome getOutcome(Set candidates) { + private ConditionOutcome getOutcome(String candidates) { try { - List missing = filter(candidates, ClassNameFilter.MISSING, - this.beanClassLoader); - if (!missing.isEmpty()) { - return ConditionOutcome.noMatch( - ConditionMessage.forCondition(ConditionalOnClass.class) - .didNotFind("required class", "required classes") - .items(Style.QUOTE, missing)); + if (!candidates.contains(",")) { + return getOutcome(candidates, ClassNameFilter.MISSING, + this.beanClassLoader); + } + for (String candidate : StringUtils + .commaDelimitedListToStringArray(candidates)) { + ConditionOutcome outcome = getOutcome(candidate, + ClassNameFilter.MISSING, this.beanClassLoader); + if (outcome != null) { + return outcome; + } } } catch (Exception ex) { @@ -227,6 +231,16 @@ class OnClassCondition extends FilteringSpringBootCondition { return null; } + private ConditionOutcome getOutcome(String className, + ClassNameFilter classNameFilter, ClassLoader classLoader) { + if (classNameFilter.matches(className, classLoader)) { + return ConditionOutcome.noMatch(ConditionMessage + .forCondition(ConditionalOnClass.class) + .didNotFind("required class").items(Style.QUOTE, className)); + } + return null; + } + } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/OnClassConditionAutoConfigurationImportFilterTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/OnClassConditionAutoConfigurationImportFilterTests.java index 7dae28adb0..e7c7b73842 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/OnClassConditionAutoConfigurationImportFilterTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/OnClassConditionAutoConfigurationImportFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * 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. @@ -16,8 +16,6 @@ package org.springframework.boot.autoconfigure.condition; -import java.util.Collections; - import org.junit.Before; import org.junit.Test; @@ -75,11 +73,11 @@ public class OnClassConditionAutoConfigurationImportFilterTests { private AutoConfigurationMetadata getAutoConfigurationMetadata() { AutoConfigurationMetadata metadata = mock(AutoConfigurationMetadata.class); given(metadata.wasProcessed("test.match")).willReturn(true); - given(metadata.getSet("test.match", "ConditionalOnClass")) - .willReturn(Collections.singleton("java.io.InputStream")); + given(metadata.get("test.match", "ConditionalOnClass")) + .willReturn("java.io.InputStream"); given(metadata.wasProcessed("test.nomatch")).willReturn(true); - given(metadata.getSet("test.nomatch", "ConditionalOnClass")) - .willReturn(Collections.singleton("java.io.DoesNotExist")); + given(metadata.get("test.nomatch", "ConditionalOnClass")) + .willReturn("java.io.DoesNotExist"); return metadata; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/main/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessor.java b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/main/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessor.java index 2095eb8041..d04f3adc1e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/main/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessor.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/main/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessor.java @@ -21,6 +21,7 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -98,7 +99,7 @@ public class AutoConfigureAnnotationProcessor extends AbstractProcessor { private void addValueExtractors(Map attributes) { attributes.put("Configuration", ValueExtractor.allFrom("value")); - attributes.put("ConditionalOnClass", ValueExtractor.allFrom("value", "name")); + attributes.put("ConditionalOnClass", new OnClassConditionValueExtractor()); attributes.put("ConditionalOnBean", new OnBeanConditionValueExtractor()); attributes.put("ConditionalOnSingleCandidate", new OnBeanConditionValueExtractor()); @@ -206,22 +207,8 @@ public class AutoConfigureAnnotationProcessor extends AbstractProcessor { List getValues(AnnotationMirror annotation); - static ValueExtractor allFrom(String... attributes) { - Set names = new HashSet<>(Arrays.asList(attributes)); - return new AbstractValueExtractor() { - - @Override - public List getValues(AnnotationMirror annotation) { - List result = new ArrayList<>(); - annotation.getElementValues().forEach((key, value) -> { - if (names.contains(key.getSimpleName().toString())) { - extractValues(value).forEach(result::add); - } - }); - return result; - } - - }; + static ValueExtractor allFrom(String... names) { + return new NamedValuesExtractor(names); } } @@ -250,6 +237,27 @@ public class AutoConfigureAnnotationProcessor extends AbstractProcessor { } + private static class NamedValuesExtractor extends AbstractValueExtractor { + + private final Set names; + + NamedValuesExtractor(String... names) { + this.names = new HashSet<>(Arrays.asList(names)); + }; + + @Override + public List getValues(AnnotationMirror annotation) { + List result = new ArrayList<>(); + annotation.getElementValues().forEach((key, value) -> { + if (this.names.contains(key.getSimpleName().toString())) { + extractValues(value).forEach(result::add); + } + }); + return result; + } + + } + private static class OnBeanConditionValueExtractor extends AbstractValueExtractor { @Override @@ -268,4 +276,29 @@ public class AutoConfigureAnnotationProcessor extends AbstractProcessor { } + private static class OnClassConditionValueExtractor extends NamedValuesExtractor { + + OnClassConditionValueExtractor() { + super("value", "name"); + } + + @Override + public List getValues(AnnotationMirror annotation) { + List values = super.getValues(annotation); + Collections.sort(values, this::compare); + return values; + } + + private int compare(Object o1, Object o2) { + return Comparator.comparing(this::isSpringClass) + .thenComparing(String.CASE_INSENSITIVE_ORDER) + .compare(o1.toString(), o2.toString()); + } + + private boolean isSpringClass(String type) { + return type.startsWith("org.springframework"); + } + + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessorTests.java b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessorTests.java index e1ae7f4cf3..e65153ad35 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessorTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/AutoConfigureAnnotationProcessorTests.java @@ -55,7 +55,7 @@ public class AutoConfigureAnnotationProcessorTests { "org.springframework.boot.autoconfigureprocessor." + "TestClassConfiguration.ConditionalOnClass", "java.io.InputStream,org.springframework.boot.autoconfigureprocessor." - + "TestClassConfiguration$Nested"); + + "TestClassConfiguration$Nested,org.springframework.foo"); assertThat(properties) .containsKey("org.springframework.boot.autoconfigureprocessor." + "TestClassConfiguration"); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/TestClassConfiguration.java b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/TestClassConfiguration.java index 5bb32cb5b6..0bf7802895 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/TestClassConfiguration.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-autoconfigure-processor/src/test/java/org/springframework/boot/autoconfigureprocessor/TestClassConfiguration.java @@ -24,7 +24,8 @@ import org.springframework.boot.autoconfigureprocessor.TestConditionalOnWebAppli * @author Madhura Bhave */ @TestConfiguration -@TestConditionalOnClass(name = "java.io.InputStream", value = TestClassConfiguration.Nested.class) +@TestConditionalOnClass(name = { "org.springframework.foo", + "java.io.InputStream" }, value = TestClassConfiguration.Nested.class) @TestConditionalOnBean(type = "java.io.OutputStream") @TestConditionalOnSingleCandidate(type = "java.io.OutputStream") @TestConditionalOnWebApplication(type = Type.SERVLET)