From 0165529d976a03712ff6cb5969826167201ec3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Fri, 7 Jun 2024 15:31:25 +0200 Subject: [PATCH] Align OverrideMetadata arguments and harmonize bean name Rather than having to override the getBeanName method when there is one, this commit moves it as a @Nullable argument. This makes sure that equals and hashCode consistently use the bean name. getBeanName cannot be final just yet as the test infrastructure overrides it. Also, arguments are now ordered consistently, which improves code readability and type signature. --- .../bean/override/OverrideMetadata.java | 38 +++++++++++-------- .../convention/TestBeanOverrideMetadata.java | 29 +++++--------- .../convention/TestBeanOverrideProcessor.java | 3 +- .../mockito/MockitoBeanOverrideMetadata.java | 32 ++++++++-------- .../mockito/MockitoBeanOverrideProcessor.java | 4 +- .../mockito/MockitoOverrideMetadata.java | 19 ++-------- .../MockitoSpyBeanOverrideMetadata.java | 22 ++++++----- .../bean/override/OverrideMetadataTests.java | 2 +- .../example/TestOverrideMetadata.java | 22 +++++------ 9 files changed, 78 insertions(+), 93 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/OverrideMetadata.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/OverrideMetadata.java index c139c18af4..08510af51b 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/OverrideMetadata.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/OverrideMetadata.java @@ -38,6 +38,7 @@ import org.springframework.lang.Nullable; * annotation or the annotated field. * * @author Simon Baslé + * @author Stephane Nicoll * @since 6.2 */ public abstract class OverrideMetadata { @@ -46,25 +47,26 @@ public abstract class OverrideMetadata { private final ResolvableType beanType; + @Nullable + private final String beanName; + private final BeanOverrideStrategy strategy; - protected OverrideMetadata(Field field, ResolvableType beanType, + protected OverrideMetadata(Field field, ResolvableType beanType, @Nullable String beanName, BeanOverrideStrategy strategy) { - this.field = field; this.beanType = beanType; + this.beanName = beanName; this.strategy = strategy; } + /** - * Get the bean name to override, or {@code null} to look for a single - * matching bean of type {@link #getBeanType()}. - *

Defaults to {@code null}. + * Get the annotated {@link Field}. */ - @Nullable - protected String getBeanName() { - return null; + public final Field getField() { + return this.field; } /** @@ -75,10 +77,12 @@ public abstract class OverrideMetadata { } /** - * Get the annotated {@link Field}. + * Get the bean name to override, or {@code null} to look for a single + * matching bean of type {@link #getBeanType()}. */ - public final Field getField() { - return this.field; + @Nullable + public String getBeanName() { + return this.beanName; } /** @@ -123,22 +127,24 @@ public abstract class OverrideMetadata { return false; } OverrideMetadata that = (OverrideMetadata) obj; - return Objects.equals(this.strategy, that.strategy) && - Objects.equals(this.field, that.field) && - Objects.equals(this.beanType, that.beanType); + return Objects.equals(this.beanType, that.beanType) && + Objects.equals(this.beanName, that.beanName) && + Objects.equals(this.strategy, that.strategy) && + Objects.equals(this.field, that.field); } @Override public int hashCode() { - return Objects.hash(this.strategy, this.field, this.beanType); + return Objects.hash(this.beanType, this.beanName, this.strategy, this.field); } @Override public String toString() { return new ToStringCreator(this) - .append("strategy", this.strategy) .append("field", this.field) .append("beanType", this.beanType) + .append("beanName", this.beanName) + .append("strategy", this.strategy) .toString(); } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideMetadata.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideMetadata.java index d32be05eee..b951082238 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideMetadata.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideMetadata.java @@ -27,7 +27,6 @@ import org.springframework.lang.Nullable; import org.springframework.test.context.bean.override.BeanOverrideStrategy; import org.springframework.test.context.bean.override.OverrideMetadata; import org.springframework.util.ReflectionUtils; -import org.springframework.util.StringUtils; /** * {@link OverrideMetadata} implementation for {@link TestBean}. @@ -40,21 +39,14 @@ final class TestBeanOverrideMetadata extends OverrideMetadata { private final Method overrideMethod; - private final String beanName; - TestBeanOverrideMetadata(Field field, Method overrideMethod, TestBean overrideAnnotation, - ResolvableType typeToOverride) { + TestBeanOverrideMetadata(Field field, ResolvableType beanType, @Nullable String beanName, + Method overrideMethod) { - super(field, typeToOverride, BeanOverrideStrategy.REPLACE_DEFINITION); - this.beanName = overrideAnnotation.name(); + super(field, beanType, beanName, BeanOverrideStrategy.REPLACE_DEFINITION); this.overrideMethod = overrideMethod; } - @Override - @Nullable - protected String getBeanName() { - return StringUtils.hasText(this.beanName) ? this.beanName : super.getBeanName(); - } @Override protected Object createOverride(String beanName, @Nullable BeanDefinition existingBeanDefinition, @@ -71,23 +63,22 @@ final class TestBeanOverrideMetadata extends OverrideMetadata { } @Override - public boolean equals(@Nullable Object o) { - if (this == o) { + public boolean equals(@Nullable Object other) { + if (this == other) { return true; } - if (o == null || getClass() != o.getClass()) { + if (other == null || getClass() != other.getClass()) { return false; } - if (!super.equals(o)) { + if (!super.equals(other)) { return false; } - TestBeanOverrideMetadata that = (TestBeanOverrideMetadata) o; - return Objects.equals(this.overrideMethod, that.overrideMethod) - && Objects.equals(this.beanName, that.beanName); + TestBeanOverrideMetadata that = (TestBeanOverrideMetadata) other; + return Objects.equals(this.overrideMethod, that.overrideMethod); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), this.overrideMethod, this.beanName); + return Objects.hash(super.hashCode(), this.overrideMethod); } } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java index e7a92c0022..b7667108bc 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java @@ -124,7 +124,8 @@ class TestBeanOverrideProcessor implements BeanOverrideProcessor { overrideMethod = findTestBeanFactoryMethod(testClass, field.getType(), candidateMethodNames); } - return new TestBeanOverrideMetadata(field, overrideMethod, testBeanAnnotation, ResolvableType.forField(field, testClass)); + String beanName = (StringUtils.hasText(testBeanAnnotation.name()) ? testBeanAnnotation.name() : null); + return new TestBeanOverrideMetadata(field, ResolvableType.forField(field, testClass), beanName, overrideMethod); } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideMetadata.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideMetadata.java index 69dae343b8..9abb42b669 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideMetadata.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverrideMetadata.java @@ -54,16 +54,16 @@ class MockitoBeanOverrideMetadata extends MockitoOverrideMetadata { private final boolean serializable; - MockitoBeanOverrideMetadata(MockitoBean annotation, Field field, ResolvableType typeToMock) { - this(annotation.name(), annotation.reset(), field, typeToMock, - annotation.extraInterfaces(), annotation.answers(), annotation.serializable()); + MockitoBeanOverrideMetadata(Field field, ResolvableType typeToMock, MockitoBean annotation) { + this(field, typeToMock, (StringUtils.hasText(annotation.name()) ? annotation.name() : null), + annotation.reset(), annotation.extraInterfaces(), annotation.answers(), annotation.serializable()); } - MockitoBeanOverrideMetadata(String name, MockReset reset, Field field, ResolvableType typeToMock, + MockitoBeanOverrideMetadata(Field field, ResolvableType typeToMock, @Nullable String beanName, MockReset reset, Class[] extraInterfaces, @Nullable Answers answer, boolean serializable) { - super(name, reset, false, field, typeToMock, BeanOverrideStrategy.REPLACE_OR_CREATE_DEFINITION); - Assert.notNull(typeToMock, "TypeToMock must not be null"); + super(field, typeToMock, beanName, BeanOverrideStrategy.REPLACE_OR_CREATE_DEFINITION, reset, false); + Assert.notNull(typeToMock, "'typeToMock' must not be null"); this.extraInterfaces = asClassSet(extraInterfaces); this.answer = (answer != null) ? answer : Answers.RETURNS_DEFAULTS; this.serializable = serializable; @@ -108,18 +108,18 @@ class MockitoBeanOverrideMetadata extends MockitoOverrideMetadata { } @Override - public boolean equals(@Nullable Object obj) { - if (obj == this) { + public boolean equals(@Nullable Object other) { + if (other == this) { return true; } - if (obj == null || obj.getClass() != getClass()) { + if (other == null || other.getClass() != getClass()) { return false; } - MockitoBeanOverrideMetadata other = (MockitoBeanOverrideMetadata) obj; - boolean result = super.equals(obj); - result = result && ObjectUtils.nullSafeEquals(this.extraInterfaces, other.extraInterfaces); - result = result && ObjectUtils.nullSafeEquals(this.answer, other.answer); - result = result && this.serializable == other.serializable; + MockitoBeanOverrideMetadata that = (MockitoBeanOverrideMetadata) other; + boolean result = super.equals(that); + result = result && ObjectUtils.nullSafeEquals(this.extraInterfaces, that.extraInterfaces); + result = result && ObjectUtils.nullSafeEquals(this.answer, that.answer); + result = result && this.serializable == that.serializable; return result; } @@ -131,12 +131,12 @@ class MockitoBeanOverrideMetadata extends MockitoOverrideMetadata { @Override public String toString() { return new ToStringCreator(this) + .append("beanType", getBeanType()) .append("beanName", getBeanName()) - .append("fieldType", getBeanType()) + .append("reset", getReset()) .append("extraInterfaces", getExtraInterfaces()) .append("answer", getAnswer()) .append("serializable", isSerializable()) - .append("reset", getReset()) .toString(); } 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 0344a921f1..febb7538b3 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 @@ -36,10 +36,10 @@ class MockitoBeanOverrideProcessor implements BeanOverrideProcessor { @Override public MockitoOverrideMetadata createMetadata(Annotation overrideAnnotation, Class testClass, Field field) { if (overrideAnnotation instanceof MockitoBean mockBean) { - return new MockitoBeanOverrideMetadata(mockBean, field, ResolvableType.forField(field, testClass)); + return new MockitoBeanOverrideMetadata(field, ResolvableType.forField(field, testClass), mockBean); } else if (overrideAnnotation instanceof MockitoSpyBean spyBean) { - return new MockitoSpyBeanOverrideMetadata(spyBean, field, ResolvableType.forField(field, testClass)); + return new MockitoSpyBeanOverrideMetadata(field, ResolvableType.forField(field, testClass), spyBean); } throw new IllegalStateException(String.format("Invalid annotation passed to MockitoBeanOverrideProcessor: " + "expected @MockitoBean/@MockitoSpyBean on field %s.%s", diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoOverrideMetadata.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoOverrideMetadata.java index c4296a214f..d2c06a4794 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoOverrideMetadata.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoOverrideMetadata.java @@ -26,7 +26,6 @@ import org.springframework.lang.Nullable; import org.springframework.test.context.bean.override.BeanOverrideStrategy; import org.springframework.test.context.bean.override.OverrideMetadata; import org.springframework.util.ObjectUtils; -import org.springframework.util.StringUtils; /** * Base {@link OverrideMetadata} implementation for Mockito. @@ -36,29 +35,20 @@ import org.springframework.util.StringUtils; */ abstract class MockitoOverrideMetadata extends OverrideMetadata { - protected final String name; - private final MockReset reset; private final boolean proxyTargetAware; - MockitoOverrideMetadata(String name, @Nullable MockReset reset, boolean proxyTargetAware, Field field, - ResolvableType typeToOverride, BeanOverrideStrategy strategy) { + protected MockitoOverrideMetadata(Field field, ResolvableType beanType, @Nullable String beanName, + BeanOverrideStrategy strategy, @Nullable MockReset reset, boolean proxyTargetAware) { - super(field, typeToOverride, strategy); - this.name = name; + super(field, beanType, beanName, strategy); this.reset = (reset != null) ? reset : MockReset.AFTER; this.proxyTargetAware = proxyTargetAware; } - @Override - @Nullable - protected String getBeanName() { - return StringUtils.hasText(this.name) ? this.name : super.getBeanName(); - } - /** * Return the mock reset mode. * @return the reset mode @@ -101,7 +91,6 @@ abstract class MockitoOverrideMetadata extends OverrideMetadata { } MockitoOverrideMetadata other = (MockitoOverrideMetadata) obj; boolean result = super.equals(obj); - result = result && ObjectUtils.nullSafeEquals(this.name, other.name); result = result && ObjectUtils.nullSafeEquals(this.reset, other.reset); result = result && ObjectUtils.nullSafeEquals(this.proxyTargetAware, other.proxyTargetAware); return result; @@ -109,7 +98,7 @@ abstract class MockitoOverrideMetadata extends OverrideMetadata { @Override public int hashCode() { - return Objects.hash(super.hashCode(), this.name, this.reset, this.proxyTargetAware); + return Objects.hash(super.hashCode(), this.reset, this.proxyTargetAware); } } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideMetadata.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideMetadata.java index 901fc21705..8910472e30 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideMetadata.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanOverrideMetadata.java @@ -48,13 +48,15 @@ import static org.mockito.Mockito.mock; */ class MockitoSpyBeanOverrideMetadata extends MockitoOverrideMetadata { - MockitoSpyBeanOverrideMetadata(MockitoSpyBean spyAnnotation, Field field, ResolvableType typeToSpy) { - this(spyAnnotation.name(), spyAnnotation.reset(), spyAnnotation.proxyTargetAware(), - field, typeToSpy); + MockitoSpyBeanOverrideMetadata(Field field, ResolvableType typeToSpy, MockitoSpyBean spyAnnotation) { + this(field, typeToSpy, (StringUtils.hasText(spyAnnotation.name()) ? spyAnnotation.name() : null), + spyAnnotation.reset(), spyAnnotation.proxyTargetAware()); } - MockitoSpyBeanOverrideMetadata(String name, MockReset reset, boolean proxyTargetAware, Field field, ResolvableType typeToSpy) { - super(name, reset, proxyTargetAware, field, typeToSpy, BeanOverrideStrategy.WRAP_BEAN); + MockitoSpyBeanOverrideMetadata(Field field, ResolvableType typeToSpy, @Nullable String beanName, + MockReset reset, boolean proxyTargetAware) { + + super(field, typeToSpy, beanName, BeanOverrideStrategy.WRAP_BEAN, reset, proxyTargetAware); Assert.notNull(typeToSpy, "typeToSpy must not be null"); } @@ -98,16 +100,16 @@ class MockitoSpyBeanOverrideMetadata extends MockitoOverrideMetadata { } @Override - public boolean equals(@Nullable Object obj) { - if (obj == this) { + public boolean equals(@Nullable Object other) { + if (other == this) { return true; } // For SpyBean we want the class to be exactly the same. - if (obj == null || obj.getClass() != getClass()) { + if (other == null || other.getClass() != getClass()) { return false; } - MockitoSpyBeanOverrideMetadata that = (MockitoSpyBeanOverrideMetadata) obj; - return (super.equals(obj) && ObjectUtils.nullSafeEquals(getBeanType(), that.getBeanType())); + MockitoSpyBeanOverrideMetadata that = (MockitoSpyBeanOverrideMetadata) other; + return (super.equals(that) && ObjectUtils.nullSafeEquals(getBeanType(), that.getBeanType())); } @Override diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/OverrideMetadataTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/OverrideMetadataTests.java index c2d3e51074..7217d9a902 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/OverrideMetadataTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/OverrideMetadataTests.java @@ -56,7 +56,7 @@ class OverrideMetadataTests { ConcreteOverrideMetadata(Field field, ResolvableType typeToOverride, BeanOverrideStrategy strategy) { - super(field, typeToOverride, strategy); + super(field, typeToOverride, null, strategy); } @Override diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/example/TestOverrideMetadata.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/example/TestOverrideMetadata.java index 0532561ff1..5395937a5d 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/example/TestOverrideMetadata.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/example/TestOverrideMetadata.java @@ -37,9 +37,6 @@ class TestOverrideMetadata extends OverrideMetadata { @Nullable private final Method method; - @Nullable - private final String beanName; - private final String methodName; @Nullable @@ -79,25 +76,24 @@ class TestOverrideMetadata extends OverrideMetadata { } public TestOverrideMetadata(Field field, ExampleBeanOverrideAnnotation overrideAnnotation, ResolvableType typeToOverride) { - super(field, typeToOverride, overrideAnnotation.createIfMissing() ? + super(field, typeToOverride, overrideAnnotation.beanName(), overrideAnnotation.createIfMissing() ? BeanOverrideStrategy.REPLACE_OR_CREATE_DEFINITION: BeanOverrideStrategy.REPLACE_DEFINITION); this.method = findMethod(field, overrideAnnotation.value()); this.methodName = overrideAnnotation.value(); - this.beanName = overrideAnnotation.beanName(); } //Used to trigger duplicate detection in parser test TestOverrideMetadata(String duplicateTrigger) { - super(null, null, null); + super(null, null, duplicateTrigger, null); this.method = null; this.methodName = duplicateTrigger; - this.beanName = duplicateTrigger; } @Override - protected String getBeanName() { - if (StringUtils.hasText(this.beanName)) { - return this.beanName; + public String getBeanName() { + String name = super.getBeanName(); + if (StringUtils.hasText(name)) { + return name; } return getField().getName(); } @@ -124,8 +120,8 @@ class TestOverrideMetadata extends OverrideMetadata { public boolean equals(Object obj) { if (this.method == null) { return obj instanceof TestOverrideMetadata tem && - tem.beanName != null && - tem.beanName.startsWith(ExampleBeanOverrideAnnotation.DUPLICATE_TRIGGER); + tem.getBeanName() != null && + tem.getBeanName().startsWith(ExampleBeanOverrideAnnotation.DUPLICATE_TRIGGER); } return super.equals(obj); } @@ -141,7 +137,7 @@ class TestOverrideMetadata extends OverrideMetadata { @Override public String toString() { if (this.method == null) { - return "{" + this.beanName + "}"; + return "{" + this.getBeanName() + "}"; } return this.methodName; }