Open jerrinot opened 7 years ago
Eric
We now realise that is an interoperability problem with annotations where you are using a cache through the programmatic API AND the annotations API.
I think this is a use case people want to use. There are some bugs reported and general frustration with the spec over this.
GeneratedCacheKey has the same two methods as Object. It also extends Serializable, but I don’t think that is necessary.
It would be helpful if you could weigh in on this.
In JCache 2.0, we need to solve this problem. I am interested in how you would solve it.
Greg
and his answer
Man, it has been 3+ years since I've thought about any of this stuff :)
Honestly I don't remember why we used that vs just returning a Serializable object.
Proposal: replace GeneratedCacheKey with Object.
Returning Object
instead GeneratedCacheKey
breaks binary compatibility.
I believe it should be possible to use some cleverness to workaround it, especially if JCache 2.0 is going to target Java 8. If there are other places where compatibility is broken then it's probably not worth it.
If you have a clever idea then post it.
I was thinking this would break binary compatibility. However because we are loosening to Object I doubt in this case it is going to break a lot of code as:
So far I have not found anything in the Spec, that an implementation must use a CacheKeyGenerator
or wrap a key into a GeneratedCacheKey
, so that is up to the implementation.
The Spec could simply say, that a single key must be passed as-is. Annotation implementations will need to change. Applications are affected as well, since the cache contents change and the resolving of external configuration changes probably, too (different types, maybe names).
I don't know whether this could be acceptable for an MR. There is the danger that applications break and need slightly adoptions.
For 2.0 I am personally in favor for ditching everything around CacheKeyGenerator
and simplify radically: Only one key parameter which is the key. The whole problem arises around the need for multiple parameters forming a key and the configuration flexibility around it. That also has quite a runtime overhead since parameters and types need to be stored in an array for the CacheInvocationContext
and then wrapped again into the GeneratedCacheKey
.
IMO: with a new major version, you can break binary compatibility. Let's not cling to a ill-suited design for the sake of backwards compatibility.
I have read a few other issues about this topic but I am not 100% sure my question is related. Is this interop between annotations and programmatic API supposed to work?
//First invoke this
@CachePut(cacheName = "default")
public void put(@CacheKey String key, @CacheValue String data) {
}
//Then this with the same String key
public InvoiceData get(String key) {
//cache is @ApplicationScoped constructed from cacheManager.getCache("default");
if (cache.containsKey(key)) { //this is never true
return cache.get(key);
}
}
I did try to iterate over all cache entries in the getter and the value from PUT does appear to be in there. So.. what exactly is going on here?
After digging deeper I realized my sample above is pretty dumb. Since @CachePut needs to operate on variable length Object[] params to generate a key I can't just search for key like that, even though the code seems viable looking from high up.
Although, where there is a will there is a way..
Object[] obj = {key};
DefaultGeneratedCacheKey genKey = new DefaultGeneratedCacheKey(obj);
if (cache.containsKey(genKey)) {
return cache.get(genKey); //yay!
}
I guess this would make more sense with some specialized key generators but seems like a bad idea in general.
I complained about this issue years ago, but back then it was considered being just a nice to have: https://github.com/jsr107/jsr107spec/issues/313
When JCache is moved to the Jakarta EE umbrella, the DefaultGeneratedCacheKey must be put into the public API to resolve this issue. A key factory is IMO something that is a nice to have and could be done in a future version instead.
CacheKeyGenerator
has to return an instance ofGeneratedCacheKey
- this is cumbersome to use and does not play nicely with CacheLoader/CacheWriters. See discussion at here, here and also thisSpring Caching also has a concept of KeyGenerator and it allows to use an ordinary Object as a key.
This is JCache 2.0 material.