Skip to content

Commit

Permalink
Improve exception handling in bean override support
Browse files Browse the repository at this point in the history
- Do not silently abort bean override processing if the ApplicationContext
  is not a BeanDefinitionRegistry.

- Include conflicting bean names in error messages instead of just the
  number of conflicting beans.

- Consistently throw IllegalStateException.

- etc.
  • Loading branch information
sbrannen committed Jun 3, 2024
1 parent 7a11899 commit acdd8a0
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,16 @@ private void registerReplaceDefinition(ConfigurableListableBeanFactory beanFacto
String beanName = overrideMetadata.getBeanName();
BeanDefinition existingBeanDefinition = null;
if (beanName == null) {
Set<String> candidates = getExistingBeanNamesByType(beanFactory, overrideMetadata, true);
if (candidates.size() != 1) {
Field f = overrideMetadata.getField();
throw new IllegalStateException("Unable to select a bean definition to override: " +
candidates.size() + " bean definitions found of type " + overrideMetadata.getBeanType() +
" (as required by annotated field '" + f.getDeclaringClass().getSimpleName() +
"." + f.getName() + "')");
Set<String> candidateNames = getExistingBeanNamesByType(beanFactory, overrideMetadata, true);
int candidateCount = candidateNames.size();
if (candidateCount != 1) {
Field field = overrideMetadata.getField();
throw new IllegalStateException("Unable to select a bean definition to override: found " +
candidateCount + " bean definitions of type " + overrideMetadata.getBeanType() +
" (as required by annotated field '" + field.getDeclaringClass().getSimpleName() +
"." + field.getName() + "')" + (candidateCount > 0 ? ": " + candidateNames : ""));
}
beanName = candidates.iterator().next();
beanName = candidateNames.iterator().next();
existingBeanDefinition = beanFactory.getBeanDefinition(beanName);
}
else {
Expand All @@ -147,8 +148,8 @@ private void registerReplaceDefinition(ConfigurableListableBeanFactory beanFacto
existingBeanDefinition = beanFactory.getBeanDefinition(beanName);
}
else if (enforceExistingDefinition) {
throw new IllegalStateException("Unable to override bean '" + beanName + "': there is no" +
" bean definition to replace with that name of type " + overrideMetadata.getBeanType());
throw new IllegalStateException("Unable to override bean '" + beanName + "': there is no " +
"bean definition to replace with that name of type " + overrideMetadata.getBeanType());
}
}

Expand Down Expand Up @@ -181,20 +182,21 @@ private void registerWrapBean(ConfigurableListableBeanFactory beanFactory, Overr
String beanName = metadata.getBeanName();
if (beanName == null) {
Set<String> candidateNames = getExistingBeanNamesByType(beanFactory, metadata, true);
if (candidateNames.size() != 1) {
Field f = metadata.getField();
throw new IllegalStateException("Unable to select a bean to override by wrapping: " +
candidateNames.size() + " bean instances found of type " + metadata.getBeanType() +
" (as required by annotated field '" + f.getDeclaringClass().getSimpleName() +
"." + f.getName() + "')");
int candidateCount = candidateNames.size();
if (candidateCount != 1) {
Field field = metadata.getField();
throw new IllegalStateException("Unable to select a bean to override by wrapping: found " +
candidateCount + " bean instances of type " + metadata.getBeanType() +
" (as required by annotated field '" + field.getDeclaringClass().getSimpleName() +
"." + field.getName() + "')" + (candidateCount > 0 ? ": " + candidateNames : ""));
}
beanName = candidateNames.iterator().next();
}
else {
Set<String> candidates = getExistingBeanNamesByType(beanFactory, metadata, false);
if (!candidates.contains(beanName)) {
throw new IllegalStateException("Unable to override bean '" + beanName + "' by wrapping: there is no" +
" existing bean instance with that name of type " + metadata.getBeanType());
throw new IllegalStateException("Unable to override bean '" + beanName + "' by wrapping: there is no " +
"existing bean instance with that name of type " + metadata.getBeanType());
}
}
this.overrideRegistrar.markWrapEarly(metadata, beanName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ private static void addInfrastructureBeanDefinition(BeanDefinitionRegistry regis

@Override
public void customizeContext(ConfigurableApplicationContext context, MergedContextConfiguration mergedConfig) {
if (context instanceof BeanDefinitionRegistry registry) {
registerInfrastructure(registry, this.detectedClasses);
if (!(context instanceof BeanDefinitionRegistry registry)) {
throw new IllegalStateException("Cannot process bean overrides with an ApplicationContext " +
"that doesn't implement BeanDefinitionRegistry: " + context.getClass());
}
registerInfrastructure(registry, this.detectedClasses);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ static Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnTyp
@Override
public TestBeanOverrideMetadata createMetadata(Annotation overrideAnnotation, Class<?> testClass, Field field) {
if (!(overrideAnnotation instanceof TestBean testBeanAnnotation)) {
throw new IllegalStateException(String.format("Invalid annotation passed to %s: expected @TestBean on field %s.%s",
TestBeanOverrideProcessor.class.getSimpleName(), field.getDeclaringClass().getName(),
field.getName()));
throw new IllegalStateException("Invalid annotation passed to %s: expected @TestBean on field %s.%s"
.formatted(getClass().getSimpleName(), field.getDeclaringClass().getName(), field.getName()));
}
Method overrideMethod;
String methodName = testBeanAnnotation.methodName();
Expand Down Expand Up @@ -172,7 +171,7 @@ protected Object createOverride(String beanName, @Nullable BeanDefinition existi
return this.overrideMethod.invoke(null);
}
catch (IllegalAccessException | InvocationTargetException ex) {
throw new IllegalArgumentException("Could not invoke bean overriding method " + this.overrideMethod.getName() +
throw new IllegalStateException("Failed to invoke bean overriding method " + this.overrideMethod.getName() +
"; a static method with no formal parameters is expected", ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.test.context.bean.override.convention;

import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -49,8 +51,8 @@ void zeroCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean definition to override: 0 bean definitions \
found of type %s (as required by annotated field '%s.example')"""
Unable to select a bean definition to override: found 0 bean definitions \
of type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
}

Expand All @@ -62,9 +64,9 @@ void tooManyCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean definition to override: 2 bean definitions \
found of type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
Unable to select a bean definition to override: found 2 bean definitions \
of type %s (as required by annotated field '%s.example'): %s"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName(), List.of("bean1", "bean2"))))));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.test.context.bean.override.mockito;

import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -49,8 +51,8 @@ void zeroCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean definition to override: 0 bean definitions \
found of type %s (as required by annotated field '%s.example')"""
Unable to select a bean definition to override: found 0 bean definitions \
of type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
}

Expand All @@ -62,9 +64,9 @@ void tooManyCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean definition to override: 2 bean definitions \
found of type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
Unable to select a bean definition to override: found 2 bean definitions \
of type %s (as required by annotated field '%s.example'): %s"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName(), List.of("bean1", "bean2"))))));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.test.context.bean.override.mockito;

import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -48,7 +50,7 @@ void zeroCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean to override by wrapping: 0 bean instances found of \
Unable to select a bean to override by wrapping: found 0 bean instances of \
type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
}
Expand All @@ -61,9 +63,9 @@ void tooManyCandidates() {
cause(
instanceOf(IllegalStateException.class),
message("""
Unable to select a bean to override by wrapping: 2 bean instances found of \
type %s (as required by annotated field '%s.example')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
Unable to select a bean to override by wrapping: found 2 bean instances of \
type %s (as required by annotated field '%s.example'): %s"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName(), List.of("bean1", "bean2"))))));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void failWhenBeanNotPresentByType() {
finishedWithFailure(
cause(instanceOf(IllegalStateException.class),
message("""
Unable to select a bean to override by wrapping: 0 bean instances found \
Unable to select a bean to override by wrapping: found 0 bean instances \
of type %s (as required by annotated field '%s.notPresent')"""
.formatted(ExampleService.class.getName(), testClass.getSimpleName())))));
}
Expand Down

0 comments on commit acdd8a0

Please sign in to comment.