Ignore 'value' attribute in AnnotationTypeMapping

Update `addConventionAnnotationValues` so that `value` attributes do not override existing annotation values. The aligns the new `MergedAnnotations` API with the previous `AnnotatedElementUtils` logic. Closes gh-22703
spring-projects · Mar 28, 2019 · 800cbf2 · 800cbf2
1 parent 45e4221
commit 800cbf2
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 2 deletions.
diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java
@@ -272,11 +272,11 @@ private void addConventionMappings() {
 	private void addConventionAnnotationValues() {
 		for (int i = 0; i < this.attributes.size(); i++) {
 			Method attribute = this.attributes.get(i);
+			boolean isValueAttribute = MergedAnnotation.VALUE.equals(attribute.getName());
 			AnnotationTypeMapping mapping = this;
 			while (mapping != null && mapping.depth > 0) {
 				int mapped = mapping.getAttributes().indexOf(attribute.getName());
-				if (mapped != -1 && (this.annotationValueMappings[i] == -1 ||
-						this.annotationValueSource[i].depth > mapping.depth)) {
+				if (mapped != -1  && isBetterConventionAnnotationValue(i, isValueAttribute, mapping)) {
 					this.annotationValueMappings[i] = mapped;
 					this.annotationValueSource[i] = mapping;
 				}
@@ -285,6 +285,15 @@ private void addConventionAnnotationValues() {
 		}
 	}
 
+	private boolean isBetterConventionAnnotationValue(int index, boolean isValueAttribute,
+			AnnotationTypeMapping mapping) {
+		if (this.annotationValueMappings[index] == -1) {
+			return true;
+		}
+		int existingDepth = this.annotationValueSource[index].depth;
+		return !isValueAttribute && existingDepth > mapping.depth;
+	}
+
 	/**
 	 * Method called after all mappings have been set. At this point no further
 	 * lookups from child mappings will occur.

diff --git a/...ng-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/...ng-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java
@@ -822,6 +822,14 @@ public void forAnnotationsCreatesCopyOfArrayOnEachCall() {
 		assertThat(element.getDeclaredAnnotations()).isNotSameAs(element.getDeclaredAnnotations());
 	}
 
+	@Test // gh-22703
+	public void getMergedAnnotationOnThreeDeepMetaWithValue() {
+		ValueAttribute annotation = AnnotatedElementUtils.getMergedAnnotation(
+				ValueAttributeMetaMetaClass.class, ValueAttribute.class);
+		assertThat(annotation.value()).containsExactly("FromValueAttributeMeta");
+	}
+
+
 	// -------------------------------------------------------------------------
 
 	@MetaCycle3
@@ -1394,4 +1402,32 @@ public Object doIt() {
 	class ForAnnotationsClass {
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	static @interface ValueAttribute {
+
+		String[] value();
+
+	}
+
+	@Retention(RetentionPolicy.RUNTIME)
+	@ValueAttribute("FromValueAttributeMeta")
+	static @interface ValueAttributeMeta {
+
+		@AliasFor("alias")
+		String[] value() default {};
+
+		@AliasFor("value")
+		String[] alias() default {};
+
+	}
+
+	@Retention(RetentionPolicy.RUNTIME)
+	@ValueAttributeMeta("FromValueAttributeMetaMeta")
+	static @interface ValueAttributeMetaMeta {
+	}
+
+	@ValueAttributeMetaMeta
+	static class ValueAttributeMetaMetaClass {
+	}
+
 }
diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java
@@ -2045,6 +2045,14 @@ public void getValueWhenHasDefaultOverrideWithExplicitAlias() {
 		assertThat(annotation.getString("value")).isEqualTo("meta");
 	}
 
+	@Test // gh-22703
+	public void getValueWhenThreeDeepMetaWithValue() {
+		MergedAnnotation<?> annotation = MergedAnnotations.from(
+				ValueAttributeMetaMetaClass.class).get(ValueAttribute.class);
+		assertThat(annotation.getStringArray(MergedAnnotation.VALUE)).containsExactly(
+				"FromValueAttributeMeta");
+	}
+
 	// @formatter:off
 
 	@Retention(RetentionPolicy.RUNTIME)
@@ -3430,6 +3438,31 @@ static class DefaultOverrideExplicitAliasRootMetaMetaClass {
 
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	static @interface ValueAttribute {
+
+		String[] value();
+
+	}
+
+	@Retention(RetentionPolicy.RUNTIME)
+	@ValueAttribute("FromValueAttributeMeta")
+	static @interface ValueAttributeMeta {
+
+		String[] value() default {};
+
+	}
+
+	@Retention(RetentionPolicy.RUNTIME)
+	@ValueAttributeMeta("FromValueAttributeMetaMeta")
+	static @interface ValueAttributeMetaMeta {
+
+	}
+
+	@ValueAttributeMetaMeta
+	static class ValueAttributeMetaMetaClass {
+
+	}
 	// @formatter:on
 
 }