From cf46f391d74977d4e0dca300c2046d2279bd1db9 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Tue, 26 Nov 2024 15:04:37 +0800 Subject: [PATCH 1/2] Improve diagnostics when a Bean Override cannot be selected by type See gh-34004 Closes gh-34006 Signed-off-by: Yanming Zhou --- .../BeanOverrideBeanFactoryPostProcessor.java | 14 +++++++++++--- .../BeanOverrideBeanFactoryPostProcessorTests.java | 6 +++--- .../bean/override/convention/TestBeanTests.java | 6 +++--- .../MockitoBeanConfigurationErrorTests.java | 6 +++--- .../MockitoSpyBeanConfigurationErrorTests.java | 2 +- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java index 1a01295a17e..fa8aefec263 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java @@ -59,6 +59,7 @@ import org.springframework.util.Assert; * @author Simon Baslé * @author Stephane Nicoll * @author Sam Brannen + * @author Yanming Zhou * @since 6.2 */ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, Ordered { @@ -67,6 +68,9 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, private static final BeanNameGenerator beanNameGenerator = DefaultBeanNameGenerator.INSTANCE; + private static final String unableToOverrideByTypeDiagnosticsMessage = " If the bean is defined from a @Bean method," + + " please make sure the return type is the most specific type (recommended) or type can be assigned to %s"; + private final Set beanOverrideHandlers; private final BeanOverrideRegistry beanOverrideRegistry; @@ -170,7 +174,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, Field field = handler.getField(); throw new IllegalStateException( "Unable to replace bean: there is no bean with name '%s' and type %s%s." - .formatted(beanName, handler.getBeanType(), requiredByField(field))); + .formatted(beanName, handler.getBeanType(), requiredByField(field, handler.getBeanType()))); } // 4) We are creating a bean by-name with the provided beanName. } @@ -257,7 +261,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, String message = "Unable to select a bean to wrap: "; int candidateCount = candidateNames.size(); if (candidateCount == 0) { - message += "there are no beans of type %s%s.".formatted(beanType, requiredByField(field)); + message += "there are no beans of type %s%s.".formatted(beanType, requiredByField(field, beanType)); } else { message += "found %d beans of type %s%s: %s" @@ -299,7 +303,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, if (requireExistingBean) { throw new IllegalStateException( "Unable to override bean: there are no beans of type %s%s." - .formatted(beanType, requiredByField(field))); + .formatted(beanType, requiredByField(field, beanType))); } return null; } @@ -483,4 +487,8 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, field.getDeclaringClass().getSimpleName(), field.getName()); } + private static String requiredByField(@Nullable Field field, ResolvableType requiredBeanType) { + return requiredByField(field) + '.' + unableToOverrideByTypeDiagnosticsMessage.formatted(requiredBeanType); + } + } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java index 8b3ce3c1a6b..affcd21db70 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java @@ -85,7 +85,7 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'descriptionBean' \ and type java.lang.String (as required by field 'ByNameTestCase.description')."""); } @@ -97,7 +97,7 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'descriptionBean' \ and type java.lang.String (as required by field 'ByNameTestCase.description')."""); } @@ -144,7 +144,7 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to override bean: there are no beans of type java.lang.Integer \ (as required by field 'ByTypeTestCase.counter')."""); } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java index d771af6333e..a036872775b 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java @@ -40,7 +40,7 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); } @@ -52,7 +52,7 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); } @@ -63,7 +63,7 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByTypeLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to override bean: there are no beans of \ type %s (as required by field '%s.example').""", String.class.getName(), FailureByTypeLookup.class.getSimpleName()); diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java index 1182a8afc59..07a06927b41 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java @@ -40,7 +40,7 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); } @@ -52,7 +52,7 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); } @@ -63,7 +63,7 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByTypeLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to override bean: there are no beans of \ type java.lang.String (as required by field 'FailureByTypeLookup.example')."""); } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java index efdbf887333..23b42908a7f 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java @@ -50,7 +50,7 @@ class MockitoSpyBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(ByTypeSingleLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessage(""" + .withMessageStartingWith(""" Unable to select a bean to wrap: there are no beans of type java.lang.String \ (as required by field 'ByTypeSingleLookup.example')."""); } From d59991fcc931b93a365da62565fe347095f3730c Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:42:24 +0100 Subject: [PATCH 2/2] Revise contribution See gh-34006 --- .../BeanOverrideBeanFactoryPostProcessor.java | 35 ++++++++++--------- ...OverrideBeanFactoryPostProcessorTests.java | 18 ++++++---- .../override/convention/TestBeanTests.java | 19 ++++++---- .../MockitoBeanConfigurationErrorTests.java | 20 +++++++---- ...MockitoSpyBeanConfigurationErrorTests.java | 12 ++++--- 5 files changed, 64 insertions(+), 40 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java index fa8aefec263..11d282f405d 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java @@ -68,9 +68,6 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, private static final BeanNameGenerator beanNameGenerator = DefaultBeanNameGenerator.INSTANCE; - private static final String unableToOverrideByTypeDiagnosticsMessage = " If the bean is defined from a @Bean method," - + " please make sure the return type is the most specific type (recommended) or type can be assigned to %s"; - private final Set beanOverrideHandlers; private final BeanOverrideRegistry beanOverrideRegistry; @@ -172,9 +169,11 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, } else if (requireExistingBean) { Field field = handler.getField(); - throw new IllegalStateException( - "Unable to replace bean: there is no bean with name '%s' and type %s%s." - .formatted(beanName, handler.getBeanType(), requiredByField(field, handler.getBeanType()))); + throw new IllegalStateException(""" + Unable to replace bean: there is no bean with name '%s' and type %s%s. \ + If the bean is defined in a @Bean method, make sure the return type is the \ + most specific type possible (for example, the concrete implementation type).""" + .formatted(beanName, handler.getBeanType(), requiredByField(field))); } // 4) We are creating a bean by-name with the provided beanName. } @@ -261,7 +260,11 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, String message = "Unable to select a bean to wrap: "; int candidateCount = candidateNames.size(); if (candidateCount == 0) { - message += "there are no beans of type %s%s.".formatted(beanType, requiredByField(field, beanType)); + message += """ + there are no beans of type %s%s. \ + If the bean is defined in a @Bean method, make sure the return type is the \ + most specific type possible (for example, the concrete implementation type).""" + .formatted(beanType, requiredByField(field)); } else { message += "found %d beans of type %s%s: %s" @@ -275,8 +278,10 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, // We are wrapping an existing bean by-name. Set candidates = getExistingBeanNamesByType(beanFactory, handler, false); if (!candidates.contains(beanName)) { - throw new IllegalStateException( - "Unable to wrap bean: there is no bean with name '%s' and type %s%s." + throw new IllegalStateException(""" + Unable to wrap bean: there is no bean with name '%s' and type %s%s. \ + If the bean is defined in a @Bean method, make sure the return type is the \ + most specific type possible (for example, the concrete implementation type).""" .formatted(beanName, beanType, requiredByField(field))); } } @@ -301,9 +306,11 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, int candidateCount = candidateNames.size(); if (candidateCount == 0) { if (requireExistingBean) { - throw new IllegalStateException( - "Unable to override bean: there are no beans of type %s%s." - .formatted(beanType, requiredByField(field, beanType))); + throw new IllegalStateException(""" + Unable to override bean: there are no beans of type %s%s. \ + If the bean is defined in a @Bean method, make sure the return type is the \ + most specific type possible (for example, the concrete implementation type).""" + .formatted(beanType, requiredByField(field))); } return null; } @@ -487,8 +494,4 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, field.getDeclaringClass().getSimpleName(), field.getName()); } - private static String requiredByField(@Nullable Field field, ResolvableType requiredBeanType) { - return requiredByField(field) + '.' + unableToOverrideByTypeDiagnosticsMessage.formatted(requiredBeanType); - } - } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java index affcd21db70..76cb72be8b1 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java @@ -85,9 +85,11 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'descriptionBean' \ - and type java.lang.String (as required by field 'ByNameTestCase.description')."""); + and type java.lang.String (as required by field 'ByNameTestCase.description'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -97,9 +99,11 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'descriptionBean' \ - and type java.lang.String (as required by field 'ByNameTestCase.description')."""); + and type java.lang.String (as required by field 'ByNameTestCase.description'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -144,9 +148,11 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to override bean: there are no beans of type java.lang.Integer \ - (as required by field 'ByTypeTestCase.counter')."""); + (as required by field 'ByTypeTestCase.counter'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java index a036872775b..08600f7c09c 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanTests.java @@ -40,9 +40,11 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ - and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); + and type java.lang.String (as required by field 'FailureByNameLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -52,9 +54,11 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ - and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); + and type java.lang.String (as required by field 'FailureByNameLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -63,9 +67,10 @@ public class TestBeanTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByTypeLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" - Unable to override bean: there are no beans of \ - type %s (as required by field '%s.example').""", + .withMessage(""" + Unable to override bean: there are no beans of type %s (as required by field '%s.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type).""", String.class.getName(), FailureByTypeLookup.class.getSimpleName()); } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java index 07a06927b41..3090ccd10de 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanConfigurationErrorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -40,9 +40,11 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ - and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); + and type java.lang.String (as required by field 'FailureByNameLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -52,9 +54,11 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByNameLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to replace bean: there is no bean with name 'beanToOverride' \ - and type java.lang.String (as required by field 'FailureByNameLookup.example')."""); + and type java.lang.String (as required by field 'FailureByNameLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -63,9 +67,11 @@ class MockitoBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(FailureByTypeLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to override bean: there are no beans of \ - type java.lang.String (as required by field 'FailureByTypeLookup.example')."""); + type java.lang.String (as required by field 'FailureByTypeLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java index 23b42908a7f..c1564c1dd3e 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanConfigurationErrorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -41,7 +41,9 @@ class MockitoSpyBeanConfigurationErrorTests { .isThrownBy(context::refresh) .withMessage(""" Unable to wrap bean: there is no bean with name 'beanToSpy' and \ - type java.lang.String (as required by field 'ByNameSingleLookup.example')."""); + type java.lang.String (as required by field 'ByNameSingleLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test @@ -50,9 +52,11 @@ class MockitoSpyBeanConfigurationErrorTests { BeanOverrideContextCustomizerTestUtils.customizeApplicationContext(ByTypeSingleLookup.class, context); assertThatIllegalStateException() .isThrownBy(context::refresh) - .withMessageStartingWith(""" + .withMessage(""" Unable to select a bean to wrap: there are no beans of type java.lang.String \ - (as required by field 'ByTypeSingleLookup.example')."""); + (as required by field 'ByTypeSingleLookup.example'). \ + If the bean is defined in a @Bean method, make sure the return type is the most \ + specific type possible (for example, the concrete implementation type)."""); } @Test