Add @OnCacheHit annotation to intercept the cache hit

[mipo256]

There is a common requirement to perform some actions in case cache was hit, for instance to log the actual hit, as in the mentioned issue.

For logging purposes, for instance:

1. Enable detailed logging for the org.springframework.cache, which is not very convenient, since the log is predefined and cannot be customized.
2. Or we can use Cache/CacheManager directly. For instance, something like that:

var resp = cache.get(dto.getKey(), DomainClass.class);
if (resp != null) {
  log.info("Return entities for client : {} from cache by key : {}", clientId, dto.getKey());
  return Either.right(resp);
}
log.info("Request entities for clientId {} with key {} from original service", clientId, dto.getKey());
return delegate
    .getEntities(dto, clientId)
    .peek(responseDto -> cache.put(dto.getKey(), responseDto));

That of course can be done, but if you think about it, this turmoil is required here because of one simple thing - we want to log the cache hit.

Proposal:

I think it would be a good idea to introduce an annotation, such as @OnCacheHit(cacheName="something", unless="SPEL")
This would've clearly made the code above unnecessary, becuase we can just do:

@OnCacheHit(cacheName="my-cache")
// args - original method arguments
void logHit(Object[] args) {
  log.info("Return entities for client : {} from cache by key : {}", args[0], args[1]);
}

[simonbasle]

hey @mipo256, thanks for the suggestion. I don't feel this is such a good idea to implement this in core, especially given how cache hits are on the "hot path". I think this argument has been made by others before, but I'd like to reiterate on a few pointers:

• if you're after logging all cache hits (whatever the entry point) then stats / metrics are probably a better bet. Boot has support for this if the underlying cache provider supports stats (I believe most do)
• Spring does logs at TRACE level (which is better than weaving in unconditional logging via CacheAspectSupport)
• if you're really in need of a specific behavior, you could implement a CacheResolver that returns a decorated version of Cache which logs. Note that this approach would also work if you're looking to log for specific Cacheable methods only (as could be the case for debugging purposes) since the CacheResolver API has access to the CacheOperationInvocationContext.

In any case, that's not something we think we can add to the framework.

[mipo256]

Hey, thanks for reply @simonbasle :)

Although, I do not think you got me - I am perfectly aware about everything that you're enumarating, that will of course work, and I have myself stated that there're a couple of ways to do so.

My point is to make it easier to accomplish such a simple task. Apring itself adheres to an annotation driven approach, so I do not think it is bad. Again, logging with TRACE does not solve my issue, and other solutions - yes, they work, I do not deny it, they're just too much for such a small task.

Hope we're on the same page 🙂

[simonbasle]

Adding this to the caching annotation model and CacheAspectSupport is not something we're inclined to do.