jsr107 / jsr107spec

JSR107 Cache Specification
Apache License 2.0
413 stars 164 forks source link

Interactions between declarative API and CacheLoader&CacheWriter? #375

Open jerrinot opened 7 years ago

jerrinot commented 7 years ago

When a declarative API (cache annotations) is used then a container generates a cache key.

When a cache is configured as read-through or write-through then the generated key is passed to a CacheLoader/CacheWriter.

However there is a mismatch: The key is generated by a container however the CacheLoader/CacheWriter are typically implemented by end-users -> the CacheLoader has no good way to access arguments used during invocation of the annotated method. All it has is just the generated key.

A possible workaround is to always always use an explicit CacheKeyGenerator and never use the default one -> a user has a full control of the generated cache key and can get back the arguments in the MapLoader code. Unfortunately this is tedious as CacheKeyGenerator is a very low-level API and it renders the default generator useless.

I'm not sure what's the best solution here:

  1. Perhaps the spec should mandate a concrete implementation of the default (composed?) key. The implementation could have an extra method to get back the arguments encapsulated by this key? However this approach goes against the idea of generated key relaxation.
  2. Alternatively the spec could provides a bunch of static method for introspection of the generated cache keys?
  3. Anything else?

Related to https://github.com/jsr107/jsr107spec/issues/313

cruftex commented 7 years ago

the CacheLoader has no good way to access arguments used during invocation of the annotated method. All it has is just the generated key.

Why should it? That is actually the very concept of a cache key and the CacheLoader/CacheWriter. If something else then the cache key influences the behavior of the loader, your cache would be inconsistent. Sometimes people try it anyways: http://stackoverflow.com/questions/37944398/how-to-pass-two-pass-one-more-parameter-other-than-key-in-google-guava-cache

IMHO the JavaDoc of the CacheLoader should have much more words of warning about this.

Of course the loader might want to get an additional setup that is immutable during the cache live time. But "passing down" such a setup by method parameters would be very error prone.

Is there really a useful scenario for CacheLoader/CacheWriter being used together with annotations? I think its two different concepts to wire in the cache.

jerrinot commented 7 years ago
@CacheResult
Person calculateArchenemy(Person person) {
  if (person.equals(Person.HOLMES)) {
    return Person.MORIARTY;
  } else {
    return expensiveCalculation(person.getId());
  }
}

Now imagine I have a database dump of archenemies of all people in the world. Then I could just plug the CacheLoader in and never ever need to invoke the expensiveCalculation(). Except I can't because the cache will pass me the GeneratedCacheKey and I have no way to access the person.getId()

cruftex commented 7 years ago

Got you. The main issue is the GeneratedCacheKey. Since the way the key is generated is not part of the spec it could be anything. Don't we have another issue on that? I think that problem is actually not only related to CacheLoader/Writer. For example one might also additionally use the bulk API Cache.getAll() which annotations don't have.

Mixing Loader/Writer and annotation will get you also in other troubles as well: Annotations have (defined) exception handling, while loaders don't.

jerrinot commented 7 years ago

this is related: https://github.com/jsr107/jsr107spec/issues/374 <-- if this is going to be implemented then the key could be truly everything.

I also found this and this is partially related as well. The original idea behind this issue comes from https://github.com/ben-manes/caffeine/issues/115

cruftex commented 6 years ago

Marking with 2.0. The key generation stuff can be improved but not in the 1.1 MR.