Allow auto-configure sort with incomplete chain

Update `AutoConfigurationSorter` so that all `@AutoConfigureBefore` and
`@AutoConfigureAfter` classes are considered even if they are ultimately
not part of the requested set.

Prior to this commit, given classes ordered with annotations such that
A -> B -> C a call to sort only [A, B] could return the incorrect order.

Fixes gh-12660
This commit is contained in:
Phillip Webb 2018-03-29 14:36:29 -07:00
parent 4b4a8acb9d
commit 7649eb6230
2 changed files with 97 additions and 28 deletions

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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -52,7 +52,7 @@ class AutoConfigurationSorter {
} }
public List<String> getInPriorityOrder(Collection<String> classNames) { public List<String> getInPriorityOrder(Collection<String> classNames) {
final AutoConfigurationClasses classes = new AutoConfigurationClasses( AutoConfigurationClasses classes = new AutoConfigurationClasses(
this.metadataReaderFactory, this.autoConfigurationMetadata, classNames); this.metadataReaderFactory, this.autoConfigurationMetadata, classNames);
List<String> orderedClassNames = new ArrayList<>(classNames); List<String> orderedClassNames = new ArrayList<>(classNames);
// Initially sort alphabetically // Initially sort alphabetically
@ -71,11 +71,13 @@ class AutoConfigurationSorter {
private List<String> sortByAnnotation(AutoConfigurationClasses classes, private List<String> sortByAnnotation(AutoConfigurationClasses classes,
List<String> classNames) { List<String> classNames) {
List<String> toSort = new ArrayList<>(classNames); List<String> toSort = new ArrayList<>(classNames);
toSort.addAll(classes.getAllNames());
Set<String> sorted = new LinkedHashSet<>(); Set<String> sorted = new LinkedHashSet<>();
Set<String> processing = new LinkedHashSet<>(); Set<String> processing = new LinkedHashSet<>();
while (!toSort.isEmpty()) { while (!toSort.isEmpty()) {
doSortByAfterAnnotation(classes, toSort, sorted, processing, null); doSortByAfterAnnotation(classes, toSort, sorted, processing, null);
} }
sorted.retainAll(classNames);
return new ArrayList<>(sorted); return new ArrayList<>(sorted);
} }
@ -104,9 +106,32 @@ class AutoConfigurationSorter {
AutoConfigurationClasses(MetadataReaderFactory metadataReaderFactory, AutoConfigurationClasses(MetadataReaderFactory metadataReaderFactory,
AutoConfigurationMetadata autoConfigurationMetadata, AutoConfigurationMetadata autoConfigurationMetadata,
Collection<String> classNames) { Collection<String> classNames) {
addToClasses(metadataReaderFactory, autoConfigurationMetadata, classNames,
true);
}
public Set<String> getAllNames() {
return this.classes.keySet();
}
private void addToClasses(MetadataReaderFactory metadataReaderFactory,
AutoConfigurationMetadata autoConfigurationMetadata,
Collection<String> classNames, boolean required) {
for (String className : classNames) { for (String className : classNames) {
this.classes.put(className, new AutoConfigurationClass(className, if (!this.classes.containsKey(className)) {
metadataReaderFactory, autoConfigurationMetadata)); AutoConfigurationClass autoConfigurationClass = new AutoConfigurationClass(
className, metadataReaderFactory, autoConfigurationMetadata);
boolean available = autoConfigurationClass.isAvailable();
if (required || available) {
this.classes.put(className, autoConfigurationClass);
}
if (available) {
addToClasses(metadataReaderFactory, autoConfigurationMetadata,
autoConfigurationClass.getBefore(), false);
addToClasses(metadataReaderFactory, autoConfigurationMetadata,
autoConfigurationClass.getAfter(), false);
}
}
} }
} }
@ -136,11 +161,11 @@ class AutoConfigurationSorter {
private final AutoConfigurationMetadata autoConfigurationMetadata; private final AutoConfigurationMetadata autoConfigurationMetadata;
private AnnotationMetadata annotationMetadata; private volatile AnnotationMetadata annotationMetadata;
private final Set<String> before; private volatile Set<String> before;
private final Set<String> after; private volatile Set<String> after;
AutoConfigurationClass(String className, AutoConfigurationClass(String className,
MetadataReaderFactory metadataReaderFactory, MetadataReaderFactory metadataReaderFactory,
@ -148,15 +173,37 @@ class AutoConfigurationSorter {
this.className = className; this.className = className;
this.metadataReaderFactory = metadataReaderFactory; this.metadataReaderFactory = metadataReaderFactory;
this.autoConfigurationMetadata = autoConfigurationMetadata; this.autoConfigurationMetadata = autoConfigurationMetadata;
this.before = readBefore(); }
this.after = readAfter();
public boolean isAvailable() {
try {
if (!wasProcessed()) {
getAnnotationMetadata();
}
return true;
}
catch (Exception ex) {
return false;
}
} }
public Set<String> getBefore() { public Set<String> getBefore() {
if (this.before == null) {
this.before = (wasProcessed()
? this.autoConfigurationMetadata.getSet(this.className,
"AutoConfigureBefore", Collections.emptySet())
: getAnnotationValue(AutoConfigureBefore.class));
}
return this.before; return this.before;
} }
public Set<String> getAfter() { public Set<String> getAfter() {
if (this.after == null) {
this.after = (wasProcessed()
? this.autoConfigurationMetadata.getSet(this.className,
"AutoConfigureAfter", Collections.emptySet())
: getAnnotationValue(AutoConfigureAfter.class));
}
return this.after; return this.after;
} }
@ -171,22 +218,6 @@ class AutoConfigurationSorter {
: (Integer) attributes.get("value")); : (Integer) attributes.get("value"));
} }
private Set<String> readBefore() {
if (wasProcessed()) {
return this.autoConfigurationMetadata.getSet(this.className,
"AutoConfigureBefore", Collections.emptySet());
}
return getAnnotationValue(AutoConfigureBefore.class);
}
private Set<String> readAfter() {
if (wasProcessed()) {
return this.autoConfigurationMetadata.getSet(this.className,
"AutoConfigureAfter", Collections.emptySet());
}
return getAnnotationValue(AutoConfigureAfter.class);
}
private boolean wasProcessed() { private boolean wasProcessed() {
return (this.autoConfigurationMetadata != null return (this.autoConfigurationMetadata != null
&& this.autoConfigurationMetadata.wasProcessed(this.className)); && this.autoConfigurationMetadata.wasProcessed(this.className));

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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,6 +16,7 @@
package org.springframework.boot.autoconfigure; package org.springframework.boot.autoconfigure;
import java.io.IOException;
import java.util.Arrays; import java.util.Arrays;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
@ -29,6 +30,7 @@ import org.junit.rules.ExpectedException;
import org.springframework.core.Ordered; import org.springframework.core.Ordered;
import org.springframework.core.type.classreading.CachingMetadataReaderFactory; import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
import org.springframework.core.type.classreading.MetadataReader;
import org.springframework.core.type.classreading.MetadataReaderFactory; import org.springframework.core.type.classreading.MetadataReaderFactory;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
@ -82,7 +84,7 @@ public class AutoConfigurationSorterTests {
@Before @Before
public void setup() { public void setup() {
this.sorter = new AutoConfigurationSorter(new CachingMetadataReaderFactory(), this.sorter = new AutoConfigurationSorter(new SkipCycleMetadataReaderFactory(),
this.autoConfigurationMetadata); this.autoConfigurationMetadata);
} }
@ -140,6 +142,8 @@ public class AutoConfigurationSorterTests {
@Test @Test
public void byAutoConfigureAfterWithCycle() { public void byAutoConfigureAfterWithCycle() {
this.sorter = new AutoConfigurationSorter(new CachingMetadataReaderFactory(),
this.autoConfigurationMetadata);
this.thrown.expect(IllegalStateException.class); this.thrown.expect(IllegalStateException.class);
this.thrown.expectMessage("AutoConfigure cycle detected"); this.thrown.expectMessage("AutoConfigure cycle detected");
this.sorter.getInPriorityOrder(Arrays.asList(A, B, C, D)); this.sorter.getInPriorityOrder(Arrays.asList(A, B, C, D));
@ -147,7 +151,7 @@ public class AutoConfigurationSorterTests {
@Test @Test
public void usesAnnotationPropertiesWhenPossible() throws Exception { public void usesAnnotationPropertiesWhenPossible() throws Exception {
MetadataReaderFactory readerFactory = mock(MetadataReaderFactory.class); MetadataReaderFactory readerFactory = new SkipCycleMetadataReaderFactory();
this.autoConfigurationMetadata = getAutoConfigurationMetadata(A2, B, C, W2, X); this.autoConfigurationMetadata = getAutoConfigurationMetadata(A2, B, C, W2, X);
this.sorter = new AutoConfigurationSorter(readerFactory, this.sorter = new AutoConfigurationSorter(readerFactory,
this.autoConfigurationMetadata); this.autoConfigurationMetadata);
@ -156,6 +160,27 @@ public class AutoConfigurationSorterTests {
assertThat(actual).containsExactly(C, W2, B, A2, X); assertThat(actual).containsExactly(C, W2, B, A2, X);
} }
@Test
public void useAnnotationWithNoDirectLink() throws Exception {
MetadataReaderFactory readerFactory = new SkipCycleMetadataReaderFactory();
this.autoConfigurationMetadata = getAutoConfigurationMetadata(A, B, E);
this.sorter = new AutoConfigurationSorter(readerFactory,
this.autoConfigurationMetadata);
List<String> actual = this.sorter.getInPriorityOrder(Arrays.asList(A, E));
assertThat(actual).containsExactly(E, A);
}
@Test
public void useAnnotationWithNoDirectLinkAndCycle() throws Exception {
MetadataReaderFactory readerFactory = new CachingMetadataReaderFactory();
this.autoConfigurationMetadata = getAutoConfigurationMetadata(A, B, D);
this.sorter = new AutoConfigurationSorter(readerFactory,
this.autoConfigurationMetadata);
this.thrown.expect(IllegalStateException.class);
this.thrown.expectMessage("AutoConfigure cycle detected");
this.sorter.getInPriorityOrder(Arrays.asList(D, B));
}
private AutoConfigurationMetadata getAutoConfigurationMetadata(String... classNames) private AutoConfigurationMetadata getAutoConfigurationMetadata(String... classNames)
throws Exception { throws Exception {
Properties properties = new Properties(); Properties properties = new Properties();
@ -263,4 +288,17 @@ public class AutoConfigurationSorterTests {
} }
private static class SkipCycleMetadataReaderFactory
extends CachingMetadataReaderFactory {
@Override
public MetadataReader getMetadataReader(String className) throws IOException {
if (className.equals(D)) {
throw new IOException();
}
return super.getMetadataReader(className);
}
}
} }