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
This commit is contained in:
Phillip Webb 2018-10-10 21:15:32 -07:00
parent b1d4cf4ea8
commit 85f86243c9
5 changed files with 83 additions and 37 deletions

View File

@ -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<String> 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<String> candidates) {
private ConditionOutcome getOutcome(String candidates) {
try {
List<String> 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;
}
}
}

View File

@ -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;
}

View File

@ -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<String, ValueExtractor> 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<Object> getValues(AnnotationMirror annotation);
static ValueExtractor allFrom(String... attributes) {
Set<String> names = new HashSet<>(Arrays.asList(attributes));
return new AbstractValueExtractor() {
@Override
public List<Object> getValues(AnnotationMirror annotation) {
List<Object> 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<String> names;
NamedValuesExtractor(String... names) {
this.names = new HashSet<>(Arrays.asList(names));
};
@Override
public List<Object> getValues(AnnotationMirror annotation) {
List<Object> 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<Object> getValues(AnnotationMirror annotation) {
List<Object> 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");
}
}
}

View File

@ -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");

View File

@ -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)