Support @Cache* as merged composed annotations [SPR-13475]

[spring-projects-issues]

Nicolas Labrot opened SPR-13475 and commented

Given this composed annotation:

@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@Documented
@Cacheable
public @interface MyCacheable {
	String[] cacheNames() default {};
	String key() default "";
}

The resulting @Cacheable attributes are not merged. Issue seems to come from SpringCacheAnnotationParser#parseCacheAnnotations which does not use AnnotatedElementUtils.findMergedAnnotation(*, Cacheable.class);.

---

Affects: 4.2 GA

Issue Links:

• Support multiple composed annotations on a single annotated element [SPR-13486] #17490 Support multiple composed annotations on a single annotated element ("depends on")
• Introduce predefined composed annotations in core Spring [SPR-13442] #18022 Introduce predefined composed annotations in core Spring
• Use AnnotatedElementUtils instead of AnnotationUtils wherever feasible [SPR-13440] #18020 Use AnnotatedElementUtils instead of AnnotationUtils wherever feasible
• Support @ResponseStatus as a merged composed annotation [SPR-13441] #18021 Support @ResponseStatus as a merged composed annotation
• Support @CrossOrigin as a merged composed annotation [SPR-13468] #18047 Support @CrossOrigin as a merged composed annotation
• @CacheConfig(cacheNames) broken when used on interface [SPR-14781] #19347 @CacheConfig(cacheNames) broken when used on interface
• Cache annotation lookup in 4.3 finds more annotations than in 4.2 [SPR-14801] #19367 Cache annotation lookup in 4.3 finds more annotations than in 4.2
• Support repeatable annotations as composed annotations [SPR-13973] #18545 Support repeatable annotations as composed annotations
• AnnotationFormatterFactory should support @AliasFor [SPR-14844] #19410 AnnotationFormatterFactory should support @AliasFor
• Support @AliasFor for @JmsListener attributes [SPR-13803] #18376 Support @AliasFor for @JmsListener attributes

Referenced from: commits ea09e57, 59c88eb, 54703bf

1 votes, 7 watchers

[spring-projects-issues]

Stéphane Nicoll commented

Can you share a project that reproduces the problem because we have tests for that stuff.

Composable annotations may not work indeed but meta-annotations should.

[spring-projects-issues]

Stéphane Nicoll commented

Alright, scratch that. The merging thing isn't tested. I'll look at that in time for 4.2.2.RELEASE

[spring-projects-issues]

Nicolas Labrot commented

If you agree, I can give it a try to a PR.

[spring-projects-issues]

Sam Brannen commented

Nicolas Labrot, there's no need to create a PR for this, since it's straightforward.

Cheers,

Sam

[spring-projects-issues]

Sam Brannen commented

Hi guys,

I unfortunately spoke too soon: this is actually not straightforward.

The code in question is SpringCacheAnnotationParser.getAnnotations() which collects all annotations of the desired type (e.g., @Cacheable, @CacheEvict, etc.) present or meta-present on the annotated element.

In other words, even though Spring's caching annotations are not @Repeatable it is in fact possible to declare multiple instances of a given annotation if one or more such annotations are meta-present -- for example:

@Retention(RUNTIME)
@Target(METHOD)
@CacheEvict("foo")
public @interface EvictFoo {

	@AliasFor(annotation = Cacheable.class, attribute = "key")
	String key() default "";
}

@Retention(RUNTIME)
@Target(METHOD)
@CacheEvict("bar")
public @interface EvictBar {

	@AliasFor(annotation = Cacheable.class, attribute = "key")
	String key() default "";
}

@EvictFoo(key = "#id + 'foo'")
@EvictBar(key = "#id + 'bar'")
public void multipleEvictions(Long id) {}

Since AnnotatedElementUtils.findMergedAnnotation(...) finds a single target annotation, it is therefore (currently) impossible to support both @EvictFoo and @EvictBar as merged composed annotations on a single annotated element like the multipleEvictions() method above.

In conclusion: supporting @Cache\* annotations as merged composed annotations will require new functionality in AnnotatedElementUtils and extensive testing. As a consequence, I am pushing this off until Spring Framework 4.3.

Regards,

Sam

p.s. I have raised #17490 to address the missing functionality.

[spring-projects-issues]

Sam Brannen commented

FYI: #17490 has now been resolved.

[spring-projects-issues]

Sam Brannen commented

Completed as described in GitHub commit 59c88eb:

Support @Cache* as merged composed annotations

Prior to this commit, @Cacheable, @CacheEvict, @CachePut, and @Caching
could be used to create custom stereotype annotations with hardcoded
values for their attributes; however, it was not possible to create
composed annotations with attribute overrides.

This commit addresses this issue by refactoring
SpringCacheAnnotationParser to use the newly introduced
findAllMergedAnnotations() method in AnnotatedElementUtils. As a
result, @Cacheable, @CacheEvict, @CachePut, and @Caching can now be
used to create custom composed annotations with attribute overrides
configured via @AliasFor.

[spring-projects-issues]

Marc Vanbrabant commented

Sam Brannen could it be that the refactoring from 59c88eb has broken some @CacheConfig configurations? I have created a (hopefully clear enough) bug report at #19347