Skip to content

Commit

Permalink
Fix context caching for Bean Overrides w/ different annotation order
Browse files Browse the repository at this point in the history
Prior to this commit, ApplicationContext caching support was broken if
two Bean Override fields declared the same annotations but in a
different order.

This commit fixes that by switching to Set semantics for the
annotations declared on a Bean Override field.

Closes gh-33633
  • Loading branch information
sbrannen committed Oct 2, 2024
1 parent 8cd2c40 commit 32309e8
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import org.springframework.beans.BeanUtils;
Expand Down Expand Up @@ -61,6 +64,8 @@ public abstract class OverrideMetadata {

private final Field field;

private final Set<Annotation> fieldAnnotations;

private final ResolvableType beanType;

@Nullable
Expand All @@ -73,6 +78,7 @@ protected OverrideMetadata(Field field, ResolvableType beanType, @Nullable Strin
BeanOverrideStrategy strategy) {

this.field = field;
this.fieldAnnotations = annotationSet(field);
this.beanType = beanType;
this.beanName = beanName;
this.strategy = strategy;
Expand Down Expand Up @@ -183,16 +189,17 @@ public boolean equals(Object other) {
if (this.beanName != null) {
return true;
}
// by type lookup
return Objects.equals(this.field.getName(), that.field.getName()) &&
Arrays.equals(this.field.getAnnotations(), that.field.getAnnotations());

// by-type lookup
return (Objects.equals(this.field.getName(), that.field.getName()) &&
this.fieldAnnotations.equals(that.fieldAnnotations));
}

@Override
public int hashCode() {
int hash = Objects.hash(getClass(), this.beanType.getType(), this.beanName, this.strategy);
return (this.beanName != null ? hash : hash +
Objects.hash(this.field.getName(), Arrays.hashCode(this.field.getAnnotations())));
Objects.hash(this.field.getName(), this.fieldAnnotations));
}

@Override
Expand All @@ -205,4 +212,9 @@ public String toString() {
.toString();
}

private static Set<Annotation> annotationSet(Field field) {
Annotation[] annotations = field.getAnnotations();
return (annotations.length != 0 ? new HashSet<>(Arrays.asList(annotations)) : Collections.emptySet());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

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

import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.LinkedHashSet;
import java.util.Objects;
Expand All @@ -26,9 +25,9 @@
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.core.ResolvableType;
import org.springframework.lang.Nullable;
import org.springframework.util.ReflectionUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

/**
* Tests for {@link BeanOverrideContextCustomizer}.
Expand Down Expand Up @@ -72,7 +71,8 @@ private static class DummyOverrideMetadata extends OverrideMetadata {
private final String key;

public DummyOverrideMetadata(String key) {
super(mock(Field.class), ResolvableType.forClass(Object.class), null, BeanOverrideStrategy.REPLACE_DEFINITION);
super(ReflectionUtils.findField(DummyOverrideMetadata.class, "key"),
ResolvableType.forClass(Object.class), null, BeanOverrideStrategy.REPLACE_DEFINITION);
this.key = key;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.function.Consumer;
Expand All @@ -42,6 +43,7 @@
*
* @author Simon Baslé
* @author Stephane Nicoll
* @author Sam Brannen
* @since 6.2
*/
class OverrideMetadataTests {
Expand Down Expand Up @@ -143,6 +145,20 @@ void isEqualToWithSameMetadataAndSameQualifierValues() {
assertThat(metadata).hasSameHashCodeAs(metadata2);
}

@Test
void isEqualToWithSameMetadataAndSameQualifierValuesButWithAnnotationsDeclaredInDifferentOrder() {
Field field1 = field(ConfigA.class, "qualifiedDummyBean");
Field field2 = field(ConfigB.class, "qualifiedDummyBean");

// Prerequisite
assertThat(Arrays.equals(field1.getAnnotations(), field2.getAnnotations())).isFalse();

OverrideMetadata metadata1 = createMetadata(field1);
OverrideMetadata metadata2 = createMetadata(field2);
assertThat(metadata1).isEqualTo(metadata2);
assertThat(metadata1).hasSameHashCodeAs(metadata2);
}

@Test
void isNotEqualToWithSameMetadataAndDifferentQualifierValues() {
OverrideMetadata metadata = createMetadata(field(ConfigA.class, "directQualifier"));
Expand Down Expand Up @@ -241,6 +257,10 @@ static class ConfigA {

@CustomQualifier
ExampleService customQualifier;

@DummyBean
@Qualifier("test")
ExampleService qualifiedDummyBean;
}

static class ConfigB {
Expand All @@ -251,6 +271,10 @@ static class ConfigB {

@Qualifier("test")
ExampleService directQualifier;

@Qualifier("test")
@DummyBean
ExampleService qualifiedDummyBean;
}

@Target(ElementType.FIELD)
Expand Down

0 comments on commit 32309e8

Please sign in to comment.