Skip to content

Commit 0ace0be

Browse files
philwebbphilwebb
committed
Fix annotation value mapping regression
Refine `TypeMappedAnnotation.getValueFromMetaAnnotation` to correctly account for aliases when finding values from meta-annotations. Prior to this commit, only convention based mappings were considered when searching for values on meta-annotations. This meant that declared meta-annotations that used aliases could return the "default value" rather than the merged value. Closes gh-22654
1 parent 88a2729 commit 0ace0be

File tree

3 files changed

+151
-26
lines changed

3 files changed

+151
-26
lines changed

‎spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ final class AnnotationTypeMapping {
6767

6868
private final int[] conventionMappings;
6969

70+
private final int[] annotationValueMappings;
71+
72+
private final AnnotationTypeMapping[] annotationValueSource;
73+
7074
private final Map<Method, List<Method>> aliasedBy;
7175

7276
private final Set<Method> claimedAliases = new HashSet<>();
@@ -92,9 +96,12 @@ final class AnnotationTypeMapping {
9296
this.mirrorSets = new MirrorSets();
9397
this.aliasMappings = filledIntArray(this.attributes.size(), -1);
9498
this.conventionMappings = filledIntArray(this.attributes.size(), -1);
99+
this.annotationValueMappings = filledIntArray(this.attributes.size(), -1);
100+
this.annotationValueSource = new AnnotationTypeMapping[this.attributes.size()];
95101
this.aliasedBy = resolveAliasedForTargets();
96102
processAliases();
97103
addConventionMappings();
104+
addConventionAnnotationValues();
98105
}
99106

100107

@@ -200,7 +207,7 @@ private void processAliases() {
200207
aliases.add(this.attributes.get(i));
201208
collectAliases(aliases);
202209
if (aliases.size() > 1) {
203-
processAliases(aliases);
210+
processAliases(i, aliases);
204211
}
205212
}
206213
}
@@ -219,7 +226,7 @@ private void collectAliases(List<Method> aliases) {
219226
}
220227
}
221228

222-
private void processAliases(List<Method> aliases) {
229+
private void processAliases(int attributeIndex, List<Method> aliases) {
223230
int rootAttributeIndex = getFirstRootAttributeIndex(aliases);
224231
AnnotationTypeMapping mapping = this;
225232
while (mapping != null) {
@@ -232,6 +239,16 @@ private void processAliases(List<Method> aliases) {
232239
}
233240
mapping.mirrorSets.updateFrom(aliases);
234241
mapping.claimedAliases.addAll(aliases);
242+
if (mapping.annotation != null) {
243+
int[] resolvedMirrors = mapping.mirrorSets.resolve(null,
244+
mapping.annotation, ReflectionUtils::invokeMethod);
245+
for (int i = 0; i < mapping.attributes.size(); i++) {
246+
if (aliases.contains(mapping.attributes.get(i))) {
247+
this.annotationValueMappings[attributeIndex] = resolvedMirrors[i];
248+
this.annotationValueSource[attributeIndex] = mapping;
249+
}
250+
}
251+
}
235252
mapping = mapping.parent;
236253
}
237254
}
@@ -267,6 +284,22 @@ private void addConventionMappings() {
267284
}
268285
}
269286

287+
private void addConventionAnnotationValues() {
288+
for (int i = 0; i < this.attributes.size(); i++) {
289+
Method attribute = this.attributes.get(i);
290+
AnnotationTypeMapping mapping = this;
291+
while (mapping.depth > 0) {
292+
int mapped = mapping.getAttributes().indexOf(attribute.getName());
293+
if (mapped != -1 && (this.annotationValueMappings[i] == -1
294+
|| this.annotationValueSource[i].depth > mapping.depth)) {
295+
this.annotationValueMappings[i] = mapped;
296+
this.annotationValueSource[i] = mapping;
297+
}
298+
mapping = mapping.parent;
299+
}
300+
}
301+
}
302+
270303
/**
271304
* Method called after all mappings have been set. At this point no further
272305
* lookups from child mappings will occur.
@@ -389,6 +422,26 @@ int getConventionMapping(int attributeIndex) {
389422
return this.conventionMappings[attributeIndex];
390423
}
391424

425+
/**
426+
* Return a mapped attribute value from the most suitable
427+
* {@link #getAnnotation() meta-annotation}. The resulting value is obtained
428+
* from the closest meta-annotation, taking into consideration both
429+
* convention and alias based mapping rules. For root mappings, this method
430+
* will always return {@code null}.
431+
* @param attributeIndex the attribute index of the source attribute
432+
* @return the mapped annotation value, or {@code null}
433+
*/
434+
@Nullable
435+
Object getMappedAnnotationValue(int attributeIndex) {
436+
int mapped = this.annotationValueMappings[attributeIndex];
437+
if (mapped == -1) {
438+
return null;
439+
}
440+
AnnotationTypeMapping source = this.annotationValueSource[attributeIndex];
441+
return ReflectionUtils.invokeMethod(source.attributes.get(mapped),
442+
source.annotation);
443+
}
444+
392445
/**
393446
* Return if the specified value is equivalent to the default value of the
394447
* attribute at the given index.
@@ -615,5 +668,4 @@ int getAttributeIndex(int index) {
615668
}
616669

617670
}
618-
619671
}

‎spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ private TypeMappedAnnotation(AnnotationTypeMapping mapping,
140140
private Object getValueForMirrorResolution(Method attribute, Object annotation) {
141141
int attributeIndex = this.mapping.getAttributes().indexOf(attribute);
142142
boolean valueAttribute = VALUE.equals(attribute.getName());
143-
return getValue(attributeIndex, !valueAttribute, false);
143+
return getValue(attributeIndex, !valueAttribute, true);
144144
}
145145

146146
@Override
@@ -183,7 +183,7 @@ public MergedAnnotation<?> getParent() {
183183
@Override
184184
public boolean hasDefaultValue(String attributeName) {
185185
int attributeIndex = getAttributeIndex(attributeName, true);
186-
Object value = getValue(attributeIndex, true, true);
186+
Object value = getValue(attributeIndex, true, false);
187187
return value == null || this.mapping.isEquivalentToDefaultValue(attributeIndex, value,
188188
this.valueExtractor);
189189
}
@@ -371,7 +371,7 @@ protected final <T> T getRequiredValue(int attributeIndex, Class<T> type) {
371371
@Nullable
372372
private <T> T getValue(int attributeIndex, Class<T> type) {
373373
Method attribute = this.mapping.getAttributes().get(attributeIndex);
374-
Object value = getValue(attributeIndex, true, true);
374+
Object value = getValue(attributeIndex, true, false);
375375
if (value == null) {
376376
value = attribute.getDefaultValue();
377377
}
@@ -380,7 +380,8 @@ private <T> T getValue(int attributeIndex, Class<T> type) {
380380

381381
@Nullable
382382
private Object getValue(int attributeIndex, boolean useConventionMapping,
383-
boolean resolveMirrors) {
383+
boolean forMirrorResolution) {
384+
384385
AnnotationTypeMapping mapping = this.mapping;
385386
if (this.useMergedValues) {
386387
int mappedIndex = this.mapping.getAliasMapping(attributeIndex);
@@ -392,39 +393,30 @@ private Object getValue(int attributeIndex, boolean useConventionMapping,
392393
attributeIndex = mappedIndex;
393394
}
394395
}
395-
if (resolveMirrors) {
396+
if (!forMirrorResolution) {
396397
attributeIndex = (mapping.getDepth() != 0 ?
397398
this.resolvedMirrors :
398399
this.resolvedRootMirrors)[attributeIndex];
399400
}
400401
if (attributeIndex == -1) {
401402
return null;
402403
}
403-
Method attribute = mapping.getAttributes().get(attributeIndex);
404404
if (mapping.getDepth() == 0) {
405+
Method attribute = mapping.getAttributes().get(attributeIndex);
405406
return this.valueExtractor.apply(attribute, this.rootAttributes);
406407
}
407-
return getValueFromMetaAnnotation(attribute);
408+
return getValueFromMetaAnnotation(attributeIndex, forMirrorResolution);
408409
}
409410

410411
@Nullable
411-
private Object getValueFromMetaAnnotation(Method attribute) {
412-
AnnotationTypeMapping mapping = this.mapping;
413-
if (this.useMergedValues && !VALUE.equals(attribute.getName())) {
414-
AnnotationTypeMapping candidate = mapping;
415-
while (candidate != null && candidate.getDepth() > 0) {
416-
int attributeIndex = candidate.getAttributes().indexOf(attribute.getName());
417-
if (attributeIndex != -1) {
418-
Method candidateAttribute = candidate.getAttributes().get(attributeIndex);
419-
if (candidateAttribute.getReturnType().equals(attribute.getReturnType())) {
420-
mapping = candidate;
421-
attribute = candidateAttribute;
422-
}
423-
}
424-
candidate = candidate.getParent();
425-
}
412+
private Object getValueFromMetaAnnotation(int attributeIndex,
413+
boolean forMirrorResolution) {
414+
415+
if (this.useMergedValues && !forMirrorResolution) {
416+
return this.mapping.getMappedAnnotationValue(attributeIndex);
426417
}
427-
return ReflectionUtils.invokeMethod(attribute, mapping.getAnnotation());
418+
Method attribute = this.mapping.getAttributes().get(attributeIndex);
419+
return ReflectionUtils.invokeMethod(attribute, this.mapping.getAnnotation());
428420
}
429421

430422
@SuppressWarnings("unchecked")

‎spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,6 +2036,25 @@ public void getValueWhenHasDefaultOverride() {
20362036
assertThat(annotation.getString("text")).isEqualTo("metameta");
20372037
}
20382038

2039+
@Test // gh-22654
2040+
public void getValueWhenHasDefaultOverrideWithImplicitAlias() {
2041+
MergedAnnotation<?> annotation1 = MergedAnnotations.from(
2042+
DefaultOverrideImplicitAliasMetaClass1.class).get(DefaultOverrideRoot.class);
2043+
assertThat(annotation1.getString("text")).isEqualTo("alias-meta-1");
2044+
MergedAnnotation<?> annotation2 = MergedAnnotations.from(
2045+
DefaultOverrideImplicitAliasMetaClass2.class).get(DefaultOverrideRoot.class);
2046+
assertThat(annotation2.getString("text")).isEqualTo("alias-meta-2");
2047+
}
2048+
2049+
@Test // gh-22654
2050+
public void getValueWhenHasDefaultOverrideWithExplicitAlias() {
2051+
MergedAnnotation<?> annotation = MergedAnnotations.from(
2052+
DefaultOverrideExplicitAliasRootMetaMetaClass.class).get(
2053+
DefaultOverrideExplicitAliasRoot.class);
2054+
assertThat(annotation.getString("text")).isEqualTo("meta");
2055+
assertThat(annotation.getString("value")).isEqualTo("meta");
2056+
}
2057+
20392058
// @formatter:off
20402059

20412060
@Retention(RetentionPolicy.RUNTIME)
@@ -3359,6 +3378,68 @@ static class DefaultOverrideClass {
33593378

33603379
}
33613380

3381+
@Retention(RetentionPolicy.RUNTIME)
3382+
@DefaultOverrideRoot
3383+
@interface DefaultOverrideImplicitAlias {
3384+
3385+
@AliasFor(annotation=DefaultOverrideRoot.class, attribute="text")
3386+
String text1() default "alias";
3387+
3388+
@AliasFor(annotation=DefaultOverrideRoot.class, attribute="text")
3389+
String text2() default "alias";
3390+
3391+
}
3392+
3393+
@Retention(RetentionPolicy.RUNTIME)
3394+
@DefaultOverrideImplicitAlias(text1="alias-meta-1")
3395+
@interface DefaultOverrideAliasImplicitMeta1 {
3396+
3397+
}
3398+
3399+
@Retention(RetentionPolicy.RUNTIME)
3400+
@DefaultOverrideImplicitAlias(text2="alias-meta-2")
3401+
@interface DefaultOverrideImplicitAliasMeta2 {
3402+
3403+
}
3404+
3405+
@DefaultOverrideAliasImplicitMeta1
3406+
static class DefaultOverrideImplicitAliasMetaClass1 {
3407+
3408+
}
3409+
3410+
@DefaultOverrideImplicitAliasMeta2
3411+
static class DefaultOverrideImplicitAliasMetaClass2 {
3412+
3413+
}
3414+
3415+
@Retention(RetentionPolicy.RUNTIME)
3416+
@interface DefaultOverrideExplicitAliasRoot {
3417+
3418+
@AliasFor("value")
3419+
String text() default "";
3420+
3421+
@AliasFor("text")
3422+
String value() default "";
3423+
3424+
}
3425+
3426+
@Retention(RetentionPolicy.RUNTIME)
3427+
@DefaultOverrideExplicitAliasRoot("meta")
3428+
@interface DefaultOverrideExplicitAliasRootMeta {
3429+
3430+
}
3431+
3432+
@Retention(RetentionPolicy.RUNTIME)
3433+
@DefaultOverrideExplicitAliasRootMeta
3434+
@interface DefaultOverrideExplicitAliasRootMetaMeta {
3435+
3436+
}
3437+
3438+
@DefaultOverrideExplicitAliasRootMetaMeta
3439+
static class DefaultOverrideExplicitAliasRootMetaMetaClass {
3440+
3441+
}
3442+
33623443
// @formatter:on
33633444

33643445
}

0 commit comments

Comments
 (0)