Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is the CacheKeyGenerator resolved as a CDI bean in cache-annotations-ri-cdi? #73

Open
cen1 opened this issue Oct 10, 2019 · 3 comments

Comments

@cen1
Copy link

cen1 commented Oct 10, 2019

@CachePut(cacheName = "default", cacheKeyGenerator=MyKeyGenerator.class)

How would one know when using cache-annotations-ri-cdi MyKeyGenerator must be a bean instance and not just a plain class? There is no mention of this in the spec javadoc. I assume because spec does not want to narrow it down but ri-cdi specifically looks up the bean instance instead of instantiating the class.

I could not find documentation on this kind of cdi-ri specifics. Maybe I missed some document section that mentions which classes need to be bean instances?

And as a side topic, is it correct assumption that 3rd party implementations are reimplementing cache-ri-impl only? Is there a 3rd party implementation of cache-annotations-ri-cdi or is this module de-facto standard implementation for CDI activation?

Best regards

@cruftex
Copy link
Member

cruftex commented Oct 11, 2019

How would one know when using cache-annotations-ri-cdi MyKeyGenerator must be a bean instance and not just a plain class?

I don't understand that question.

It would be better to post this on stackoverflow, since not so many people are monitoring this issue tracker here. But please add some more details: What problem are you trying to solve? What have you done? What is the error output you get?

And as a side topic, is it correct assumption that 3rd party implementations are reimplementing cache-ri-impl only?

Cache vendors only implement the cache API. The annotations are implemented by application server or framework vendors. E.g. Spring isn't CDI but implements the JSR107 cache annotations as well.

Is there a 3rd party implementation of cache-annotations-ri-cdi or is this module de-facto standard implementation for CDI activation?

That is hard to tell. It seems not to be used that much:
https://mvnrepository.com/artifact/org.jsr107.ri/cache-annotations-ri-cdi/usages

The cause may also be that the usage JSR107 annotations is not so popular.

@cen1
Copy link
Author

cen1 commented Oct 11, 2019

I am playing around in a sample project, implemented key generator like this:

@CachePut(cacheName = "default", cacheKeyGenerator=MyKeyGenerator.class)
public void someMethod(...) {}
public class MyKeyGenerator implements CacheKeyGenerator {

@Override
public GeneratedCacheKey generateCacheKey(CacheKeyInvocationContext<? extends Annotation> cacheKeyInvocationContext) {
        return new SingleStringKey("test"); //just a mock key implementation
    }
}

But in runtime I got:

Caused by: java.lang.IllegalArgumentException: cacheKeyGenerator cannot be null
	at org.jsr107.ri.annotations.StaticCacheKeyInvocationContext.<init>(StaticCacheKeyInvocationContext.java:52)
	at org.jsr107.ri.annotations.CachePutMethodDetails.<init>(CachePutMethodDetails.java:46)
	at org.jsr107.ri.annotations.AbstractCacheLookupUtil.createCachePutMethodDetails(AbstractCacheLookupUtil.java:335)
	at org.jsr107.ri.annotations.AbstractCacheLookupUtil.getMethodDetails(AbstractCacheLookupUtil.java:194)
	at org.jsr107.ri.annotations.AbstractCacheLookupUtil.getCacheKeyInvocationContext(AbstractCacheLookupUtil.java:72)
	at org.jsr107.ri.annotations.AbstractCachePutInterceptor.cachePut(AbstractCachePutInterceptor.java:49)

That is because, behind the scenes, instance of key generator is acquired with CacheLookupUtil

protected <T> T getObjectByType(Class<T> type) {
    return beanManagerUtil.getBeanByType(type);
}

so since I am in CDI context, the fix is, for example:

@ApplicationScoped
public class MyKeyGenerator implements CacheKeyGenerator {

    @Override
    public GeneratedCacheKey generateCacheKey(CacheKeyInvocationContext<? extends Annotation> cacheKeyInvocationContext) {
        return new SingleStringKey("loled");
    }
}

Point: it is impossible for developer to know from the spec documentation that MyKeyGenerator needs to be a bean instance and not just a plain class because the spec does not mandate this. So my question is whether there is some documentation specific to cdi-ri where one could look up details like this. If not I guess it's up to tutorials and self-exploration, that is good enough for me.

@cruftex
Copy link
Member

cruftex commented Oct 11, 2019

Thanks for the details. The code makes your point clear now.

Giving it a bit of thought, I tend to consider this as a bug in cache-annotations-ri-cdi. Code that is using the caching annotations should behave the same, regardless whether its used CDI context or not.

OTOH, the behavior is useful. "Correcting" the behavior at that late time, probably is not a good idea. Maybe some people rely on the way it is now.

Hopefully people with the same problem will find this issue.

@cruftex cruftex changed the title How would one know that cacheKeyGenerator needs to be a bean instance? Why is the CacheKeyGenerator resolved as a CDI bean in cache-annotations-ri-cdi? Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants