Skip to content

Commit bbf5800

Browse files
jhoellerjhoeller
committed
ConfigurationClassParser processes late-arriving DeferredImportSelectors as regular import selectors
Also contains refined exception handling, treating regular class loading and ASM-based loading consistently in terms of exception wrapping, and always mentioning the current configuration class in all exception messages. Issue: SPR-11997
1 parent 10a4c2c commit bbf5800

File tree

2 files changed

+65
-67
lines changed

2 files changed

+65
-67
lines changed

‎spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java‎

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,11 @@ public void parse(Set<BeanDefinitionHolder> configCandidates) {
158158
parse(bd.getBeanClassName(), holder.getBeanName());
159159
}
160160
}
161-
catch (IOException ex) {
162-
throw new BeanDefinitionStoreException("Failed to load bean class: " + bd.getBeanClassName(), ex);
161+
catch (BeanDefinitionStoreException ex) {
162+
throw ex;
163+
}
164+
catch (Exception ex) {
165+
throw new BeanDefinitionStoreException("Failed to parse configuration class [" + bd.getBeanClassName() + "]", ex);
163166
}
164167
}
165168
processDeferredImportSelectors();
@@ -228,7 +231,7 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
228231
* multiple times as relevant sources are discovered.
229232
* @param configClass the configuration class being build
230233
* @param sourceClass a source class
231-
* @return the superclass, {@code null} if none found or previously processed
234+
* @return the superclass, or {@code null} if none found or previously processed
232235
*/
233236
protected final SourceClass doProcessConfigurationClass(ConfigurationClass configClass, SourceClass sourceClass) throws IOException {
234237
// recursively process any member (nested) classes first
@@ -258,7 +261,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
258261
}
259262

260263
// process any @Import annotations
261-
processImports(configClass, sourceClass, getImports(sourceClass), true);
264+
processImports(configClass, sourceClass, getImports(sourceClass), true, false);
262265

263266
// process any @ImportResource annotations
264267
if (sourceClass.getMetadata().isAnnotated(ImportResource.class.getName())) {
@@ -283,12 +286,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
283286
if (!superclass.startsWith("java") && !this.knownSuperclasses.containsKey(superclass)) {
284287
this.knownSuperclasses.put(superclass, configClass);
285288
// superclass found, return its annotation metadata and recurse
286-
try {
287-
return sourceClass.getSuperClass();
288-
}
289-
catch (ClassNotFoundException ex) {
290-
throw new IllegalStateException(ex);
291-
}
289+
return sourceClass.getSuperClass();
292290
}
293291
}
294292

@@ -367,39 +365,38 @@ private Set<SourceClass> getImports(SourceClass sourceClass) throws IOException
367365
* @throws IOException if there is any problem reading metadata from the named class
368366
*/
369367
private void collectImports(SourceClass sourceClass, Set<SourceClass> imports, Set<SourceClass> visited) throws IOException {
370-
try {
371-
if (visited.add(sourceClass)) {
372-
for (SourceClass annotation : sourceClass.getAnnotations()) {
373-
String annName = annotation.getMetadata().getClassName();
374-
if (!annName.startsWith("java") && !annName.equals(Import.class.getName())) {
375-
collectImports(annotation, imports, visited);
376-
}
368+
if (visited.add(sourceClass)) {
369+
for (SourceClass annotation : sourceClass.getAnnotations()) {
370+
String annName = annotation.getMetadata().getClassName();
371+
if (!annName.startsWith("java") && !annName.equals(Import.class.getName())) {
372+
collectImports(annotation, imports, visited);
377373
}
378-
imports.addAll(sourceClass.getAnnotationAttributes(Import.class.getName(), "value"));
379374
}
380-
}
381-
catch (ClassNotFoundException ex) {
382-
throw new NestedIOException("Unable to collect imports", ex);
375+
imports.addAll(sourceClass.getAnnotationAttributes(Import.class.getName(), "value"));
383376
}
384377
}
385378

386379
private void processDeferredImportSelectors() {
387380
Collections.sort(this.deferredImportSelectors, DEFERRED_IMPORT_COMPARATOR);
388381
for (DeferredImportSelectorHolder deferredImport : this.deferredImportSelectors) {
382+
ConfigurationClass configClass = deferredImport.getConfigurationClass();
389383
try {
390-
ConfigurationClass configClass = deferredImport.getConfigurationClass();
391384
String[] imports = deferredImport.getImportSelector().selectImports(configClass.getMetadata());
392-
processImports(configClass, asSourceClass(configClass), asSourceClasses(imports), false);
385+
processImports(configClass, asSourceClass(configClass), asSourceClasses(imports), false, true);
386+
}
387+
catch (BeanDefinitionStoreException ex) {
388+
throw ex;
393389
}
394390
catch (Exception ex) {
395-
throw new BeanDefinitionStoreException("Failed to load bean class: ", ex);
391+
throw new BeanDefinitionStoreException("Failed to process import candidates for configuration class [" +
392+
configClass.getMetadata().getClassName() + "]", ex);
396393
}
397394
}
398395
this.deferredImportSelectors.clear();
399396
}
400397

401398
private void processImports(ConfigurationClass configClass, SourceClass currentSourceClass,
402-
Collection<SourceClass> importCandidates, boolean checkForCircularImports) throws IOException {
399+
Collection<SourceClass> importCandidates, boolean checkForCircularImports, boolean deferred) throws IOException {
403400

404401
if (importCandidates.isEmpty()) {
405402
return;
@@ -416,13 +413,13 @@ private void processImports(ConfigurationClass configClass, SourceClass currentS
416413
Class<?> candidateClass = candidate.loadClass();
417414
ImportSelector selector = BeanUtils.instantiateClass(candidateClass, ImportSelector.class);
418415
invokeAwareMethods(selector);
419-
if (selector instanceof DeferredImportSelector) {
416+
if (!deferred && selector instanceof DeferredImportSelector) {
420417
this.deferredImportSelectors.add(new DeferredImportSelectorHolder(configClass, (DeferredImportSelector) selector));
421418
}
422419
else {
423420
String[] importClassNames = selector.selectImports(currentSourceClass.getMetadata());
424421
Collection<SourceClass> importSourceClasses = asSourceClasses(importClassNames);
425-
processImports(configClass, currentSourceClass, importSourceClasses, false);
422+
processImports(configClass, currentSourceClass, importSourceClasses, false, false);
426423
}
427424
}
428425
else if (candidate.isAssignable(ImportBeanDefinitionRegistrar.class)) {
@@ -439,8 +436,12 @@ else if (candidate.isAssignable(ImportBeanDefinitionRegistrar.class)) {
439436
}
440437
}
441438
}
442-
catch (ClassNotFoundException ex) {
443-
throw new NestedIOException("Failed to load import candidate class", ex);
439+
catch (BeanDefinitionStoreException ex) {
440+
throw ex;
441+
}
442+
catch (Exception ex) {
443+
throw new BeanDefinitionStoreException("Failed to process import candidates for configuration class [" +
444+
configClass.getMetadata().getClassName() + "]", ex);
444445
}
445446
finally {
446447
this.importStack.pop();
@@ -515,22 +516,17 @@ ImportRegistry getImportRegistry() {
515516
* Factory method to obtain a {@link SourceClass} from a {@link ConfigurationClass}.
516517
*/
517518
public SourceClass asSourceClass(ConfigurationClass configurationClass) throws IOException {
518-
try {
519-
AnnotationMetadata metadata = configurationClass.getMetadata();
520-
if (metadata instanceof StandardAnnotationMetadata) {
521-
return asSourceClass(((StandardAnnotationMetadata) metadata).getIntrospectedClass());
522-
}
523-
return asSourceClass(configurationClass.getMetadata().getClassName());
524-
}
525-
catch (ClassNotFoundException ex) {
526-
throw new IllegalStateException(ex);
519+
AnnotationMetadata metadata = configurationClass.getMetadata();
520+
if (metadata instanceof StandardAnnotationMetadata) {
521+
return asSourceClass(((StandardAnnotationMetadata) metadata).getIntrospectedClass());
527522
}
523+
return asSourceClass(configurationClass.getMetadata().getClassName());
528524
}
529525

530526
/**
531527
* Factory method to obtain a {@link SourceClass} from a {@link Class}.
532528
*/
533-
public SourceClass asSourceClass(Class<?> classType) throws IOException, ClassNotFoundException {
529+
public SourceClass asSourceClass(Class<?> classType) throws IOException {
534530
try {
535531
// Sanity test that we can read annotations, if not fall back to ASM
536532
classType.getAnnotations();
@@ -545,7 +541,7 @@ public SourceClass asSourceClass(Class<?> classType) throws IOException, ClassNo
545541
/**
546542
* Factory method to obtain {@link SourceClass}s from class names.
547543
*/
548-
public Collection<SourceClass> asSourceClasses(String[] classNames) throws IOException, ClassNotFoundException {
544+
public Collection<SourceClass> asSourceClasses(String[] classNames) throws IOException {
549545
List<SourceClass> annotatedClasses = new ArrayList<SourceClass>();
550546
for (String className : classNames) {
551547
annotatedClasses.add(asSourceClass(className));
@@ -556,10 +552,15 @@ public Collection<SourceClass> asSourceClasses(String[] classNames) throws IOExc
556552
/**
557553
* Factory method to obtain a {@link SourceClass} from a class name.
558554
*/
559-
public SourceClass asSourceClass(String className) throws IOException, ClassNotFoundException {
555+
public SourceClass asSourceClass(String className) throws IOException {
560556
if (className.startsWith("java")) {
561557
// Never use ASM for core java types
562-
return new SourceClass(this.resourceLoader.getClassLoader().loadClass(className));
558+
try {
559+
return new SourceClass(this.resourceLoader.getClassLoader().loadClass(className));
560+
}
561+
catch (ClassNotFoundException ex) {
562+
throw new NestedIOException("Failed to load class [" + className + "]", ex);
563+
}
563564
}
564565
return new SourceClass(this.metadataReaderFactory.getMetadataReader(className));
565566
}
@@ -653,7 +654,7 @@ public DeferredImportSelector getImportSelector() {
653654
*/
654655
private class SourceClass {
655656

656-
private final Object source; // Class or MetaDataReader
657+
private final Object source; // Class or MetaDataReader
657658

658659
private final AnnotationMetadata metadata;
659660

@@ -675,7 +676,7 @@ public Class<?> loadClass() throws ClassNotFoundException {
675676
if (this.source instanceof Class<?>) {
676677
return (Class<?>) this.source;
677678
}
678-
String className = ((MetadataReader) source).getClassMetadata().getClassName();
679+
String className = ((MetadataReader) this.source).getClassMetadata().getClassName();
679680
return resourceLoader.getClassLoader().loadClass(className);
680681
}
681682

@@ -695,19 +696,13 @@ public ConfigurationClass asConfigClass(ConfigurationClass importedBy) throws IO
695696

696697
public Collection<SourceClass> getMemberClasses() throws IOException {
697698
Object sourceToProcess = this.source;
698-
699699
if (sourceToProcess instanceof Class<?>) {
700700
Class<?> sourceClass = (Class<?>) sourceToProcess;
701701
try {
702702
Class<?>[] declaredClasses = sourceClass.getDeclaredClasses();
703703
List<SourceClass> members = new ArrayList<SourceClass>(declaredClasses.length);
704704
for (Class<?> declaredClass : declaredClasses) {
705-
try {
706-
members.add(asSourceClass(declaredClass));
707-
}
708-
catch (ClassNotFoundException ex) {
709-
// ignore
710-
}
705+
members.add(asSourceClass(declaredClass));
711706
}
712707
return members;
713708
}
@@ -723,17 +718,12 @@ public Collection<SourceClass> getMemberClasses() throws IOException {
723718
String[] memberClassNames = sourceReader.getClassMetadata().getMemberClassNames();
724719
List<SourceClass> members = new ArrayList<SourceClass>(memberClassNames.length);
725720
for (String memberClassName : memberClassNames) {
726-
try {
727-
members.add(asSourceClass(memberClassName));
728-
}
729-
catch (ClassNotFoundException ex) {
730-
// ignore
731-
}
721+
members.add(asSourceClass(memberClassName));
732722
}
733723
return members;
734724
}
735725

736-
public SourceClass getSuperClass() throws IOException, ClassNotFoundException {
726+
public SourceClass getSuperClass() throws IOException {
737727
if (this.source instanceof Class<?>) {
738728
return asSourceClass(((Class<?>) this.source).getSuperclass());
739729
}
@@ -754,9 +744,7 @@ public Set<SourceClass> getAnnotations() throws IOException {
754744
return result;
755745
}
756746

757-
public Collection<SourceClass> getAnnotationAttributes(String annotationType, String attribute)
758-
throws IOException, ClassNotFoundException {
759-
747+
public Collection<SourceClass> getAnnotationAttributes(String annotationType, String attribute) throws IOException {
760748
Map<String, Object> annotationAttributes = this.metadata.getAnnotationAttributes(annotationType, true);
761749
if (annotationAttributes == null || !annotationAttributes.containsKey(attribute)) {
762750
return Collections.emptySet();
@@ -769,7 +757,7 @@ public Collection<SourceClass> getAnnotationAttributes(String annotationType, St
769757
return result;
770758
}
771759

772-
private SourceClass getRelated(String className) throws IOException, ClassNotFoundException {
760+
private SourceClass getRelated(String className) throws IOException {
773761
if (this.source instanceof Class<?>) {
774762
try {
775763
Class<?> clazz = resourceLoader.getClassLoader().loadClass(className);
@@ -778,7 +766,7 @@ private SourceClass getRelated(String className) throws IOException, ClassNotFou
778766
catch (ClassNotFoundException ex) {
779767
// Ignore -> fall back to ASM next, except for core java types.
780768
if (className.startsWith("java")) {
781-
throw ex;
769+
throw new NestedIOException("Failed to load class [" + className + "]", ex);
782770
}
783771
return new SourceClass(metadataReaderFactory.getMetadataReader(className));
784772
}

‎spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,16 @@ public void orderingIsLifo() {
9999
}
100100
}
101101

102-
@Test(expected=IllegalArgumentException.class)
102+
@Test
103103
public void withUnresolvablePlaceholder() {
104104
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
105105
ctx.register(ConfigWithUnresolvablePlaceholder.class);
106-
ctx.refresh();
106+
try {
107+
ctx.refresh();
108+
}
109+
catch (BeanDefinitionStoreException ex) {
110+
assertTrue(ex.getCause() instanceof IllegalArgumentException);
111+
}
107112
}
108113

109114
@Test
@@ -124,11 +129,16 @@ public void withResolvablePlaceholder() {
124129
System.clearProperty("path.to.properties");
125130
}
126131

127-
@Test(expected = IllegalArgumentException.class)
132+
@Test
128133
public void withEmptyResourceLocations() {
129134
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
130135
ctx.register(ConfigWithEmptyResourceLocations.class);
131-
ctx.refresh();
136+
try {
137+
ctx.refresh();
138+
}
139+
catch (BeanDefinitionStoreException ex) {
140+
assertTrue(ex.getCause() instanceof IllegalArgumentException);
141+
}
132142
}
133143

134144
// SPR-10820

0 commit comments

Comments
 (0)