From c59ca087b4066b99eabc76348fdfc1c2c500e68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Mon, 23 Dec 2024 11:29:32 +0100 Subject: [PATCH 1/3] Backport tests for exact match resolution See gh-34124 (cherry picked from commit 898d3ec86aaaaa76f69f1e65adf9b52f6a26e7ff) --- .../util/PropertyPlaceholderHelperTests.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java index 3b0e1650177..842ceb7a7f1 100644 --- a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java +++ b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java @@ -153,6 +153,23 @@ class PropertyPlaceholderHelperTests { ); } + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("exactMatchPlaceholders") + void placeholdersWithExactMatchAreConsidered(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("prefix://my-service", "example-service"); + properties.setProperty("px", "prefix"); + properties.setProperty("p1", "${prefix://my-service}"); + assertThat(this.helper.replacePlaceholders(text, properties)).isEqualTo(expected); + } + + static Stream exactMatchPlaceholders() { + return Stream.of( + Arguments.of("${prefix://my-service}", "example-service"), + Arguments.of("${p1}", "example-service") + ); + } + } PlaceholderResolver mockPlaceholderResolver(String... pairs) { From 2ba002270475ba7bdf9d9aba677654165f593a85 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 3 Jan 2025 17:23:07 +0200 Subject: [PATCH 2/3] Polishing --- .../springframework/util/ReflectionUtils.java | 3 +- .../util/PropertyPlaceholderHelperTests.java | 48 ++++++++----------- .../mockito/MockitoBeanOverrideHandler.java | 4 +- .../mockito/MockitoBeanOverrideProcessor.java | 4 +- .../MockitoSpyBeanOverrideHandler.java | 12 ++--- .../override/convention/TestBeanTests.java | 2 +- 6 files changed, 28 insertions(+), 45 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java index 4d1af9d2175..2748e5e2675 100644 --- a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java @@ -719,8 +719,7 @@ public abstract class ReflectionUtils { // Keep backing up the inheritance hierarchy. Class targetClass = clazz; do { - Field[] fields = getDeclaredFields(targetClass); - for (Field field : fields) { + for (Field field : getDeclaredFields(targetClass)) { if (ff != null && !ff.matches(field)) { continue; } diff --git a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java index 842ceb7a7f1..8f580df3c8c 100644 --- a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java +++ b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java @@ -17,13 +17,11 @@ package org.springframework.util; import java.util.Properties; -import java.util.stream.Stream; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver; @@ -44,6 +42,7 @@ class PropertyPlaceholderHelperTests { private final PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}"); + @Test void withProperties() { String text = "foo=${foo}"; @@ -116,8 +115,8 @@ class PropertyPlaceholderHelperTests { props.setProperty("foo", "bar"); PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", null, null, false); - assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> - helper.replacePlaceholders(text, props)); + assertThatExceptionOfType(PlaceholderResolutionException.class) + .isThrownBy(() -> helper.replacePlaceholders(text, props)); } @Nested @@ -126,7 +125,14 @@ class PropertyPlaceholderHelperTests { private final PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", ":", null, true); @ParameterizedTest(name = "{0} -> {1}") - @MethodSource("defaultValues") + @CsvSource(delimiterString = "->", textBlock = """ + ${invalid:test} -> test + ${invalid:${one}} -> 1 + ${invalid:${one}${two}} -> 12 + ${invalid:${one}:${two}} -> 1:2 + ${invalid:${also_invalid:test}} -> test + ${invalid:${also_invalid:${one}}} -> 1 + """) void defaultValueIsApplied(String text, String value) { Properties properties = new Properties(); properties.setProperty("one", "1"); @@ -142,19 +148,11 @@ class PropertyPlaceholderHelperTests { verify(resolver, never()).resolvePlaceholder("two"); } - static Stream defaultValues() { - return Stream.of( - Arguments.of("${invalid:test}", "test"), - Arguments.of("${invalid:${one}}", "1"), - Arguments.of("${invalid:${one}${two}}", "12"), - Arguments.of("${invalid:${one}:${two}}", "1:2"), - Arguments.of("${invalid:${also_invalid:test}}", "test"), - Arguments.of("${invalid:${also_invalid:${one}}}", "1") - ); - } - @ParameterizedTest(name = "{0} -> {1}") - @MethodSource("exactMatchPlaceholders") + @CsvSource(delimiterString = "->", textBlock = """ + ${prefix://my-service} -> example-service + ${p1} -> example-service + """) void placeholdersWithExactMatchAreConsidered(String text, String expected) { Properties properties = new Properties(); properties.setProperty("prefix://my-service", "example-service"); @@ -162,21 +160,14 @@ class PropertyPlaceholderHelperTests { properties.setProperty("p1", "${prefix://my-service}"); assertThat(this.helper.replacePlaceholders(text, properties)).isEqualTo(expected); } - - static Stream exactMatchPlaceholders() { - return Stream.of( - Arguments.of("${prefix://my-service}", "example-service"), - Arguments.of("${p1}", "example-service") - ); - } - } - PlaceholderResolver mockPlaceholderResolver(String... pairs) { + + private static PlaceholderResolver mockPlaceholderResolver(String... pairs) { if (pairs.length % 2 == 1) { throw new IllegalArgumentException("size must be even, it is a set of key=value pairs"); } - PlaceholderResolver resolver = mock(PlaceholderResolver.class); + PlaceholderResolver resolver = mock(); for (int i = 0; i < pairs.length; i += 2) { String key = pairs[i]; String value = pairs[i + 1]; @@ -185,5 +176,4 @@ class PropertyPlaceholderHelperTests { return resolver; } - } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideHandler.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideHandler.java index 64ddd2f1b02..2f871c40da4 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideHandler.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideHandler.java @@ -64,13 +64,13 @@ class MockitoBeanOverrideHandler extends AbstractMockitoBeanOverrideHandler { } private MockitoBeanOverrideHandler(Field field, ResolvableType typeToMock, @Nullable String beanName, - BeanOverrideStrategy strategy, MockReset reset, Class[] extraInterfaces, @Nullable Answers answers, + BeanOverrideStrategy strategy, MockReset reset, Class[] extraInterfaces, Answers answers, boolean serializable) { super(field, typeToMock, beanName, strategy, reset); Assert.notNull(typeToMock, "'typeToMock' must not be null"); this.extraInterfaces = asClassSet(extraInterfaces); - this.answers = (answers != null ? answers : Answers.RETURNS_DEFAULTS); + this.answers = answers; this.serializable = serializable; } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideProcessor.java index fd4e3d389b4..21e2bd42973 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideProcessor.java @@ -35,8 +35,8 @@ class MockitoBeanOverrideProcessor implements BeanOverrideProcessor { @Override public AbstractMockitoBeanOverrideHandler createHandler(Annotation overrideAnnotation, Class testClass, Field field) { - if (overrideAnnotation instanceof MockitoBean mockBean) { - return new MockitoBeanOverrideHandler(field, ResolvableType.forField(field, testClass), mockBean); + if (overrideAnnotation instanceof MockitoBean mockitoBean) { + return new MockitoBeanOverrideHandler(field, ResolvableType.forField(field, testClass), mockitoBean); } else if (overrideAnnotation instanceof MockitoSpyBean spyBean) { return new MockitoSpyBeanOverrideHandler(field, ResolvableType.forField(field, testClass), spyBean); diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideHandler.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideHandler.java index ce929171e97..e02d59b4552 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideHandler.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideHandler.java @@ -48,15 +48,9 @@ class MockitoSpyBeanOverrideHandler extends AbstractMockitoBeanOverrideHandler { new SpringAopBypassingVerificationStartedListener(); - MockitoSpyBeanOverrideHandler(Field field, ResolvableType typeToSpy, MockitoSpyBean spyAnnotation) { - this(field, typeToSpy, (StringUtils.hasText(spyAnnotation.name()) ? spyAnnotation.name() : null), - spyAnnotation.reset()); - } - - MockitoSpyBeanOverrideHandler(Field field, ResolvableType typeToSpy, @Nullable String beanName, - MockReset reset) { - - super(field, typeToSpy, beanName, BeanOverrideStrategy.WRAP, reset); + MockitoSpyBeanOverrideHandler(Field field, ResolvableType typeToSpy, MockitoSpyBean spyBean) { + super(field, typeToSpy, (StringUtils.hasText(spyBean.name()) ? spyBean.name() : null), + BeanOverrideStrategy.WRAP, spyBean.reset()); Assert.notNull(typeToSpy, "typeToSpy must not be null"); } 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 9685ed07179..8dfbbba78ee 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 @@ -113,7 +113,7 @@ public class TestBeanTests { } @Test - void contextCustomizerCannotBeCreatedWitCompetingOverrideMethods() { + void contextCustomizerCannotBeCreatedWithCompetingOverrideMethods() { GenericApplicationContext context = new GenericApplicationContext(); context.registerBean("bean", String.class, () -> "example"); assertThatIllegalStateException() From 0da4ae96b4c3bc61524e4287de28c4f6a22f99d9 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 2 Jan 2025 12:49:05 +0200 Subject: [PATCH 3/3] Rename internal MockitoBeans class to MockBeans This is a prerequisite for gh-33925 which will introduce a public @MockitoBeans container annotation in the same package. --- .../AbstractMockitoBeanOverrideHandler.java | 18 +++++++++--------- .../{MockitoBeans.java => MockBeans.java} | 2 +- .../MockitoResetTestExecutionListener.java | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) rename spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/{MockitoBeans.java => MockBeans.java} (98%) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanOverrideHandler.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanOverrideHandler.java index 3e7b39275fb..a90980fe66c 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanOverrideHandler.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanOverrideHandler.java @@ -56,20 +56,20 @@ abstract class AbstractMockitoBeanOverrideHandler extends BeanOverrideHandler { @Override protected void trackOverrideInstance(Object mock, SingletonBeanRegistry trackingBeanRegistry) { - getMockitoBeans(trackingBeanRegistry).add(mock); + getMockBeans(trackingBeanRegistry).add(mock); } - private static MockitoBeans getMockitoBeans(SingletonBeanRegistry trackingBeanRegistry) { - String beanName = MockitoBeans.class.getName(); - MockitoBeans mockitoBeans = null; + private static MockBeans getMockBeans(SingletonBeanRegistry trackingBeanRegistry) { + String beanName = MockBeans.class.getName(); + MockBeans mockBeans = null; if (trackingBeanRegistry.containsSingleton(beanName)) { - mockitoBeans = (MockitoBeans) trackingBeanRegistry.getSingleton(beanName); + mockBeans = (MockBeans) trackingBeanRegistry.getSingleton(beanName); } - if (mockitoBeans == null) { - mockitoBeans = new MockitoBeans(); - trackingBeanRegistry.registerSingleton(beanName, mockitoBeans); + if (mockBeans == null) { + mockBeans = new MockBeans(); + trackingBeanRegistry.registerSingleton(beanName, mockBeans); } - return mockitoBeans; + return mockBeans; } @Override diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeans.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockBeans.java similarity index 98% rename from spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeans.java rename to spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockBeans.java index 1c636f629d2..d57c3fe411a 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeans.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockBeans.java @@ -28,7 +28,7 @@ import org.mockito.Mockito; * @author Sam Brannen * @since 6.2 */ -class MockitoBeans { +class MockBeans { private final List beans = new ArrayList<>(); diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java index f4556a26192..be850476584 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java @@ -116,7 +116,7 @@ public class MockitoResetTestExecutionListener extends AbstractTestExecutionList } } try { - beanFactory.getBean(MockitoBeans.class).resetAll(reset); + beanFactory.getBean(MockBeans.class).resetAll(reset); } catch (NoSuchBeanDefinitionException ex) { // Continue