Skip to content

Commit 31fa156

Browse files
committed
Avoid synthesizing annotations unnecessarily
Prior to Spring Framework 5.2, some of our annotation utilities would not synthesize an annotation if it was already synthesized or not synthesizable (i.e., did not declare local aliases via @AliasFor and did not declare attributes that could override attributes in the meta-annotation hierarchy above the given annotation); however, we lost most of this functionality with the introduction of the new MergedAnnotations API. This commit revises the implementation of createSynthesized() in TypeMappedAnnotation so that, for invocations of MergedAnnotation.synthesize() and indirectly for invocations of AnnotatedElementUtils.findMergedAnnotation(), etc.: 1. An annotation that was previously synthesized will not be synthesized again. 2. An annotation that is not "synthesizable" will not be synthesized. For both of the above use cases, the original annotation is now returned from createSynthesized(). Closes gh-24861
1 parent 8d31dca commit 31fa156

File tree

2 files changed

+68
-0
lines changed

2 files changed

+68
-0
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,18 @@ private <T extends Map<String, Object>> Object adaptValueForMapOptions(Method at
329329
}
330330

331331
@Override
332+
@SuppressWarnings("unchecked")
332333
protected A createSynthesized() {
334+
if (this.rootAttributes instanceof Annotation) {
335+
// Nothing to synthesize?
336+
if (this.resolvedRootMirrors.length == 0 && this.resolvedMirrors.length == 0) {
337+
return (A) this.rootAttributes;
338+
}
339+
// Already synthesized?
340+
if (this.rootAttributes instanceof SynthesizedAnnotation) {
341+
return (A) this.rootAttributes;
342+
}
343+
}
333344
return SynthesizedMergedAnnotationInvocationHandler.createProxy(this, getType());
334345
}
335346

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.springframework.util.MultiValueMap;
5151
import org.springframework.util.ReflectionUtils;
5252

53+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
5354
import static org.assertj.core.api.Assertions.assertThat;
5455
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
5556
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
@@ -1376,6 +1377,50 @@ void synthesizeAlreadySynthesized() throws Exception {
13761377
assertThat(synthesizedWebMapping.value()).containsExactly("/test");
13771378
}
13781379

1380+
@Test
1381+
void synthesizeShouldNotSynthesizeNonsynthesizableAnnotations() throws Exception {
1382+
Method method = getClass().getDeclaredMethod("getId");
1383+
Id id = method.getAnnotation(Id.class);
1384+
assertThat(id).isNotNull();
1385+
1386+
Id synthesizedId = MergedAnnotation.from(id).synthesize();
1387+
assertThat(id).isEqualTo(synthesizedId);
1388+
// It doesn't make sense to synthesize {@code @Id} since it declares zero attributes.
1389+
assertThat(synthesizedId).isNotInstanceOf(SynthesizedAnnotation.class);
1390+
assertThat(id).isSameAs(synthesizedId);
1391+
}
1392+
1393+
/**
1394+
* If an attempt is made to synthesize an annotation from an annotation instance
1395+
* that has already been synthesized, the original synthesized annotation should
1396+
* ideally be returned as-is without creating a new proxy instance with the same
1397+
* values.
1398+
*/
1399+
@Test
1400+
void synthesizeShouldNotResynthesizeAlreadySynthesizedAnnotations() throws Exception {
1401+
Method method = WebController.class.getMethod("handleMappedWithValueAttribute");
1402+
RequestMapping webMapping = method.getAnnotation(RequestMapping.class);
1403+
assertThat(webMapping).isNotNull();
1404+
1405+
MergedAnnotation<RequestMapping> mergedAnnotation1 = MergedAnnotation.from(webMapping);
1406+
RequestMapping synthesizedWebMapping1 = mergedAnnotation1.synthesize();
1407+
RequestMapping synthesizedWebMapping2 = MergedAnnotation.from(webMapping).synthesize();
1408+
1409+
assertThat(synthesizedWebMapping1).isInstanceOf(SynthesizedAnnotation.class);
1410+
assertThat(synthesizedWebMapping2).isInstanceOf(SynthesizedAnnotation.class);
1411+
assertThat(synthesizedWebMapping1).isEqualTo(synthesizedWebMapping2);
1412+
1413+
// Synthesizing an annotation from a different MergedAnnotation results in a different synthesized annotation instance.
1414+
assertThat(synthesizedWebMapping1).isNotSameAs(synthesizedWebMapping2);
1415+
// Synthesizing an annotation from the same MergedAnnotation results in the same synthesized annotation instance.
1416+
assertThat(synthesizedWebMapping1).isSameAs(mergedAnnotation1.synthesize());
1417+
1418+
RequestMapping synthesizedAgainWebMapping = MergedAnnotation.from(synthesizedWebMapping1).synthesize();
1419+
assertThat(synthesizedWebMapping1).isEqualTo(synthesizedAgainWebMapping);
1420+
// Synthesizing an already synthesized annotation results in the original synthesized annotation instance.
1421+
assertThat(synthesizedWebMapping1).isSameAs(synthesizedAgainWebMapping);
1422+
}
1423+
13791424
@Test
13801425
void synthesizeWhenAliasForIsMissingAttributeDeclaration() throws Exception {
13811426
AliasForWithMissingAttributeDeclaration annotation =
@@ -2951,6 +2996,18 @@ public void handleMappedWithDifferentPathAndValueAttributes() {
29512996
}
29522997
}
29532998

2999+
/**
3000+
* Mimics javax.persistence.Id
3001+
*/
3002+
@Retention(RUNTIME)
3003+
@interface Id {
3004+
}
3005+
3006+
@Id
3007+
private Long getId() {
3008+
return 42L;
3009+
}
3010+
29543011
@Retention(RetentionPolicy.RUNTIME)
29553012
@interface TestConfiguration {
29563013

0 commit comments

Comments
 (0)