Review TCF cache support for bean override

This commit reviews how bean override support can influence the key of
an application context cached by the TCF. OverrideMetadata and its
subclasses now implement a proper equals/hashCode pair that is tested
in various scenarios.

Due to how the TCF operates, OverrideMetadata has to be computed in
two locations:

1. In a ContextCustomizerFactory, using the metadata in the enclosing
class if any. This determines whether a customizer is needed in the
first place. The computed set of unique metadata identifies the
customizer and participates in the application context cache's key.
2. In the TestExecutionListener so that it knows the override points
it has to process.

Parsing of the metadata based on a test class has been greatly
simplified and moved to OverrideMetadata proper as we don't need several
flavors. 1 and 2 are using the same algorithm with the former wrapping
that in a Set to compute a proper key.

BeanOverrideContextCustomizerEqualityTests provides a framework for
testing edge cases as we only care about whether the created
ContextCustomizer behaves correctly against the identity of another.

Closes gh-32884
This commit is contained in:
Stéphane Nicoll
2024-06-07 16:23:07 +02:00
parent 02517e5011
commit 7916f74942
23 changed files with 1239 additions and 331 deletions

View File

@@ -41,38 +41,41 @@ import org.springframework.core.ResolvableType;
import org.springframework.util.StringUtils;
/**
* A {@link BeanFactoryPostProcessor} implementation that processes test classes
* and adapts the {@link BeanFactory} for any {@link BeanOverride} it may define.
* A {@link BeanFactoryPostProcessor} implementation that processes identified
* use of {@link BeanOverride} and adapts the {@link BeanFactory} accordingly.
*
* <p>A set of classes from which to parse {@link OverrideMetadata} must be
* provided to this processor. Each test class is expected to use any
* annotation meta-annotated with {@link BeanOverride @BeanOverride} to mark
* beans to override. The {@link BeanOverrideParsingUtils#hasBeanOverride(Class)}
* method can be used to check if a class matches the above criteria.
* <p>For each override, the bean factory is prepared according to the chosen
* {@link BeanOverrideStrategy overriding strategy}. The override value is created,
* if necessary, and the necessary infrastructure is updated to allow the value
* to be injected in the corresponding {@linkplain OverrideMetadata#getField() field}
* of the test class.
*
* <p>The provided classes are fully parsed at creation to build a metadata set.
* This processor implements several {@link BeanOverrideStrategy overriding
* strategies} and chooses the correct one according to each override metadata's
* {@link OverrideMetadata#getStrategy()} method. Additionally, it provides
* support for injecting the overridden bean instances into their corresponding
* annotated {@link Field fields}.
* <p>This processor does not work against a particular test class, it only prepares
* the bean factory for the identified, unique, set of bean overrides.
*
* @author Simon Baslé
* @author Stephane Nicoll
* @since 6.2
*/
class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, Ordered {
private final Set<OverrideMetadata> metadata;
private final BeanOverrideRegistrar overrideRegistrar;
/**
* Create a new {@code BeanOverrideBeanFactoryPostProcessor} instance with
* the given {@link BeanOverrideRegistrar}, which contains a set of parsed
* {@link OverrideMetadata}.
* the set of {@link OverrideMetadata} to process, using the given
* {@link BeanOverrideRegistrar}.
* @param metadata the {@link OverrideMetadata} instances to process
* @param overrideRegistrar the {@link BeanOverrideRegistrar} used to track
* metadata
*/
public BeanOverrideBeanFactoryPostProcessor(BeanOverrideRegistrar overrideRegistrar) {
public BeanOverrideBeanFactoryPostProcessor(Set<OverrideMetadata> metadata,
BeanOverrideRegistrar overrideRegistrar) {
this.metadata = metadata;
this.overrideRegistrar = overrideRegistrar;
}
@@ -92,7 +95,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
}
private void postProcessWithRegistry(ConfigurableListableBeanFactory beanFactory, BeanDefinitionRegistry registry) {
for (OverrideMetadata metadata : this.overrideRegistrar.getOverrideMetadata()) {
for (OverrideMetadata metadata : this.metadata) {
registerBeanOverride(beanFactory, registry, metadata);
}
}

View File

@@ -49,10 +49,10 @@ class BeanOverrideContextCustomizer implements ContextCustomizer {
"org.springframework.test.context.bean.override.internalWrapEarlyBeanPostProcessor";
private final Set<Class<?>> detectedClasses;
private final Set<OverrideMetadata> metadata;
BeanOverrideContextCustomizer(Set<Class<?>> detectedClasses) {
this.detectedClasses = detectedClasses;
BeanOverrideContextCustomizer(Set<OverrideMetadata> metadata) {
this.metadata = metadata;
}
@Override
@@ -61,19 +61,25 @@ class BeanOverrideContextCustomizer implements ContextCustomizer {
throw new IllegalStateException("Cannot process bean overrides with an ApplicationContext " +
"that doesn't implement BeanDefinitionRegistry: " + context.getClass());
}
registerInfrastructure(registry, this.detectedClasses);
registerInfrastructure(registry);
}
private void registerInfrastructure(BeanDefinitionRegistry registry, Set<Class<?>> detectedClasses) {
Set<OverrideMetadata> getMetadata() {
return this.metadata;
}
private void registerInfrastructure(BeanDefinitionRegistry registry) {
addInfrastructureBeanDefinition(registry, BeanOverrideRegistrar.class, REGISTRAR_BEAN_NAME,
constructorArgs -> constructorArgs.addIndexedArgumentValue(0, detectedClasses));
constructorArgs -> {});
RuntimeBeanReference registrarReference = new RuntimeBeanReference(REGISTRAR_BEAN_NAME);
addInfrastructureBeanDefinition(
registry, WrapEarlyBeanPostProcessor.class, EARLY_INFRASTRUCTURE_BEAN_NAME,
constructorArgs -> constructorArgs.addIndexedArgumentValue(0, registrarReference));
addInfrastructureBeanDefinition(
registry, BeanOverrideBeanFactoryPostProcessor.class, INFRASTRUCTURE_BEAN_NAME,
addInfrastructureBeanDefinition(registry, WrapEarlyBeanPostProcessor.class, EARLY_INFRASTRUCTURE_BEAN_NAME,
constructorArgs -> constructorArgs.addIndexedArgumentValue(0, registrarReference));
addInfrastructureBeanDefinition(registry, BeanOverrideBeanFactoryPostProcessor.class, INFRASTRUCTURE_BEAN_NAME,
constructorArgs -> {
constructorArgs.addIndexedArgumentValue(0, this.metadata);
constructorArgs.addIndexedArgumentValue(1, registrarReference);
});
}
private void addInfrastructureBeanDefinition(BeanDefinitionRegistry registry,
@@ -97,12 +103,12 @@ class BeanOverrideContextCustomizer implements ContextCustomizer {
return false;
}
BeanOverrideContextCustomizer that = (BeanOverrideContextCustomizer) other;
return this.detectedClasses.equals(that.detectedClasses);
return this.metadata.equals(that.metadata);
}
@Override
public int hashCode() {
return this.detectedClasses.hashCode();
return this.metadata.hashCode();
}
}

View File

@@ -16,13 +16,12 @@
package org.springframework.test.context.bean.override;
import java.util.LinkedHashSet;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.springframework.lang.Nullable;
import org.springframework.test.context.ContextConfigurationAttributes;
import org.springframework.test.context.ContextCustomizer;
import org.springframework.test.context.ContextCustomizerFactory;
import org.springframework.test.context.TestContextAnnotationUtils;
@@ -31,6 +30,7 @@ import org.springframework.test.context.TestContextAnnotationUtils;
* Bean Overriding.
*
* @author Simon Baslé
* @author Stephane Nicoll
* @since 6.2
* @see BeanOverride
*/
@@ -38,24 +38,22 @@ class BeanOverrideContextCustomizerFactory implements ContextCustomizerFactory {
@Override
@Nullable
public ContextCustomizer createContextCustomizer(Class<?> testClass,
public BeanOverrideContextCustomizer createContextCustomizer(Class<?> testClass,
List<ContextConfigurationAttributes> configAttributes) {
Set<Class<?>> detectedClasses = new LinkedHashSet<>();
findClassesWithBeanOverride(testClass, detectedClasses);
if (detectedClasses.isEmpty()) {
Set<OverrideMetadata> metadata = new HashSet<>();
findOverrideMetadata(testClass, metadata);
if (metadata.isEmpty()) {
return null;
}
return new BeanOverrideContextCustomizer(detectedClasses);
return new BeanOverrideContextCustomizer(metadata);
}
private void findClassesWithBeanOverride(Class<?> testClass, Set<Class<?>> detectedClasses) {
if (BeanOverrideParsingUtils.hasBeanOverride(testClass)) {
detectedClasses.add(testClass);
}
private void findOverrideMetadata(Class<?> testClass, Set<OverrideMetadata> metadata) {
List<OverrideMetadata> overrideMetadata = OverrideMetadata.forTestClass(testClass);
metadata.addAll(overrideMetadata);
if (TestContextAnnotationUtils.searchEnclosingClass(testClass)) {
findClassesWithBeanOverride(testClass.getEnclosingClass(), detectedClasses);
findOverrideMetadata(testClass.getEnclosingClass(), metadata);
}
}

View File

@@ -1,101 +0,0 @@
/*
* Copyright 2002-2024 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.test.context.bean.override;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import org.springframework.beans.BeanUtils;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.util.Assert;
import org.springframework.util.ReflectionUtils;
import static org.springframework.core.annotation.MergedAnnotations.SearchStrategy.DIRECT;
/**
* Internal parsing utilities to discover the presence of
* {@link BeanOverride @BeanOverride} and create the relevant
* {@link OverrideMetadata} if necessary.
*
* @author Simon Baslé
* @author Sam Brannen
* @since 6.2
*/
abstract class BeanOverrideParsingUtils {
/**
* Check if at least one field of the given {@code clazz} is meta-annotated
* with {@link BeanOverride @BeanOverride}.
* @param clazz the class which fields to inspect
* @return {@code true} if there is a bean override annotation present,
* {@code false} otherwise
*/
static boolean hasBeanOverride(Class<?> clazz) {
AtomicBoolean hasBeanOverride = new AtomicBoolean();
ReflectionUtils.doWithFields(clazz, field -> {
if (hasBeanOverride.get()) {
return;
}
boolean present = MergedAnnotations.from(field, DIRECT).isPresent(BeanOverride.class);
hasBeanOverride.compareAndSet(false, present);
});
return hasBeanOverride.get();
}
/**
* Parse the specified classes for the presence of fields annotated with
* {@link BeanOverride @BeanOverride}, and create an {@link OverrideMetadata}
* for each identified field.
* @param classes the classes to parse
*/
static Set<OverrideMetadata> parse(Iterable<Class<?>> classes) {
Set<OverrideMetadata> result = new LinkedHashSet<>();
classes.forEach(c -> ReflectionUtils.doWithFields(c, field -> parseField(field, c, result)));
return result;
}
/**
* Convenience method to parse a single test class.
* @see #parse(Iterable)
*/
static Set<OverrideMetadata> parse(Class<?> clazz) {
return parse(List.of(clazz));
}
private static void parseField(Field field, Class<?> testClass, Set<OverrideMetadata> metadataSet) {
AtomicBoolean overrideAnnotationFound = new AtomicBoolean();
MergedAnnotations.from(field, DIRECT).stream(BeanOverride.class).forEach(mergedAnnotation -> {
MergedAnnotation<?> metaSource = mergedAnnotation.getMetaSource();
Assert.state(metaSource != null, "@BeanOverride annotation must be meta-present");
BeanOverride beanOverride = mergedAnnotation.synthesize();
BeanOverrideProcessor processor = BeanUtils.instantiateClass(beanOverride.value());
Annotation composedAnnotation = metaSource.synthesize();
Assert.state(overrideAnnotationFound.compareAndSet(false, true),
() -> "Multiple @BeanOverride annotations found on field: " + field);
OverrideMetadata metadata = processor.createMetadata(composedAnnotation, testClass, field);
metadataSet.add(metadata);
});
}
}

View File

@@ -19,7 +19,6 @@ package org.springframework.test.context.bean.override;
import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanCreationException;
@@ -45,21 +44,10 @@ class BeanOverrideRegistrar implements BeanFactoryAware {
private final Map<String, OverrideMetadata> earlyOverrideMetadata = new HashMap<>();
private final Set<OverrideMetadata> overrideMetadata;
@Nullable
private ConfigurableBeanFactory beanFactory;
/**
* Create a new registrar and immediately parse the provided classes.
* @param classesToParse the initial set of classes that have been
* detected to contain bean overriding annotations
*/
BeanOverrideRegistrar(Set<Class<?>> classesToParse) {
this.overrideMetadata = BeanOverrideParsingUtils.parse(classesToParse);
}
@Override
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
if (!(beanFactory instanceof ConfigurableBeanFactory cbf)) {
@@ -69,13 +57,6 @@ class BeanOverrideRegistrar implements BeanFactoryAware {
this.beanFactory = cbf;
}
/**
* Get the detected {@link OverrideMetadata} instances.
*/
Set<OverrideMetadata> getOverrideMetadata() {
return this.overrideMetadata;
}
/**
* Check {@link #markWrapEarly(OverrideMetadata, String) early override}
* records and use the {@link OverrideMetadata} to create an override

View File

@@ -17,6 +17,7 @@
package org.springframework.test.context.bean.override;
import java.lang.reflect.Field;
import java.util.List;
import java.util.function.BiConsumer;
import org.springframework.test.context.TestContext;
@@ -74,12 +75,12 @@ public class BeanOverrideTestExecutionListener extends AbstractTestExecutionList
if (Boolean.TRUE.equals(
testContext.getAttribute(DependencyInjectionTestExecutionListener.REINJECT_DEPENDENCIES_ATTRIBUTE))) {
postProcessFields(testContext, (testMetadata, postProcessor) -> {
postProcessFields(testContext, (testMetadata, registrar) -> {
Object testInstance = testMetadata.testInstance;
Field field = testMetadata.overrideMetadata.getField();
ReflectionUtils.makeAccessible(field);
ReflectionUtils.setField(field, testInstance, null);
postProcessor.inject(testInstance, testMetadata.overrideMetadata);
registrar.inject(testInstance, testMetadata.overrideMetadata);
});
}
}
@@ -90,13 +91,11 @@ public class BeanOverrideTestExecutionListener extends AbstractTestExecutionList
Class<?> testClass = testContext.getTestClass();
Object testInstance = testContext.getTestInstance();
if (BeanOverrideParsingUtils.hasBeanOverride(testClass)) {
List<OverrideMetadata> metadataForFields = OverrideMetadata.forTestClass(testClass);
if (!metadataForFields.isEmpty()) {
BeanOverrideRegistrar registrar =
testContext.getApplicationContext().getBean(BeanOverrideRegistrar.class);
for (OverrideMetadata metadata : registrar.getOverrideMetadata()) {
if (!metadata.getField().getDeclaringClass().isAssignableFrom(testClass)) {
continue;
}
for (OverrideMetadata metadata : metadataForFields) {
consumer.accept(new TestContextOverrideMetadata(testInstance, metadata), registrar);
}
}

View File

@@ -16,14 +16,26 @@
package org.springframework.test.context.bean.override;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.SingletonBeanRegistry;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.style.ToStringCreator;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ReflectionUtils;
import static org.springframework.core.annotation.MergedAnnotations.SearchStrategy.DIRECT;
/**
* Metadata for Bean Override injection points, also responsible for creation of
@@ -61,6 +73,34 @@ public abstract class OverrideMetadata {
this.strategy = strategy;
}
/**
* Parse the given {@code testClass} and provide the use of bean override.
* @param testClass the class to parse
* @return a list of bean overrides metadata
*/
public static List<OverrideMetadata> forTestClass(Class<?> testClass) {
List<OverrideMetadata> all = new LinkedList<>();
ReflectionUtils.doWithFields(testClass, field -> parseField(field, testClass, all));
return all;
}
private static void parseField(Field field, Class<?> testClass, List<OverrideMetadata> metadataList) {
AtomicBoolean overrideAnnotationFound = new AtomicBoolean();
MergedAnnotations.from(field, DIRECT).stream(BeanOverride.class).forEach(mergedAnnotation -> {
MergedAnnotation<?> metaSource = mergedAnnotation.getMetaSource();
Assert.state(metaSource != null, "@BeanOverride annotation must be meta-present");
BeanOverride beanOverride = mergedAnnotation.synthesize();
BeanOverrideProcessor processor = BeanUtils.instantiateClass(beanOverride.value());
Annotation composedAnnotation = metaSource.synthesize();
Assert.state(overrideAnnotationFound.compareAndSet(false, true),
() -> "Multiple @BeanOverride annotations found on field: " + field);
OverrideMetadata metadata = processor.createMetadata(composedAnnotation, testClass, field);
metadataList.add(metadata);
});
}
/**
* Get the annotated {@link Field}.
@@ -127,15 +167,16 @@ public abstract class OverrideMetadata {
return false;
}
OverrideMetadata that = (OverrideMetadata) obj;
return Objects.equals(this.beanType, that.beanType) &&
return Objects.equals(this.beanType.getType(), that.beanType.getType()) &&
Objects.equals(this.beanName, that.beanName) &&
Objects.equals(this.strategy, that.strategy) &&
Objects.equals(this.field, that.field);
Arrays.equals(this.field.getAnnotations(), that.field.getAnnotations());
}
@Override
public int hashCode() {
return Objects.hash(this.beanType, this.beanName, this.strategy, this.field);
return Objects.hash(this.beanType.getType(), this.beanName, this.strategy,
Arrays.hashCode(this.field.getAnnotations()));
}
@Override

View File

@@ -79,6 +79,6 @@ final class TestBeanOverrideMetadata extends OverrideMetadata {
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), this.overrideMethod);
return this.overrideMethod.hashCode() * 29 + super.hashCode();
}
}

View File

@@ -43,6 +43,7 @@ import static org.mockito.Mockito.mock;
* {@link OverrideMetadata} implementation for Mockito {@code mock} support.
*
* @author Phillip Webb
* @author Stephane Nicoll
* @since 6.2
*/
class MockitoBeanOverrideMetadata extends MockitoOverrideMetadata {
@@ -142,7 +143,7 @@ class MockitoBeanOverrideMetadata extends MockitoOverrideMetadata {
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), this.extraInterfaces, this.answer, this.serializable);
return Objects.hash(this.extraInterfaces, this.answer, this.serializable) + super.hashCode();
}
@Override

View File

@@ -31,6 +31,7 @@ import org.springframework.util.ObjectUtils;
* Base {@link OverrideMetadata} implementation for Mockito.
*
* @author Phillip Webb
* @author Stephane Nicoll
* @since 6.2
*/
abstract class MockitoOverrideMetadata extends OverrideMetadata {
@@ -98,7 +99,7 @@ abstract class MockitoOverrideMetadata extends OverrideMetadata {
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), this.reset, this.proxyTargetAware);
return Objects.hash(this.reset, this.proxyTargetAware) + super.hashCode();
}
}

View File

@@ -18,7 +18,6 @@ package org.springframework.test.context.bean.override.mockito;
import java.lang.reflect.Field;
import java.lang.reflect.Proxy;
import java.util.Objects;
import org.mockito.AdditionalAnswers;
import org.mockito.MockSettings;
@@ -34,7 +33,6 @@ import org.springframework.test.context.bean.override.BeanOverrideStrategy;
import org.springframework.test.context.bean.override.OverrideMetadata;
import org.springframework.test.util.AopTestUtils;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import static org.mockito.Mockito.mock;
@@ -44,6 +42,7 @@ import static org.mockito.Mockito.mock;
*
* @author Phillip Webb
* @author Simon Baslé
* @author Stephane Nicoll
* @since 6.2
*/
class MockitoSpyBeanOverrideMetadata extends MockitoOverrideMetadata {
@@ -108,13 +107,12 @@ class MockitoSpyBeanOverrideMetadata extends MockitoOverrideMetadata {
if (other == null || other.getClass() != getClass()) {
return false;
}
MockitoSpyBeanOverrideMetadata that = (MockitoSpyBeanOverrideMetadata) other;
return (super.equals(that) && ObjectUtils.nullSafeEquals(getBeanType(), that.getBeanType()));
return super.equals(other);
}
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), getBeanType());
return getClass().hashCode() * 29 + super.hashCode();
}
@Override