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

Support sync property in @Cacheable annotation #323

Open
TheHett opened this issue Dec 2, 2021 · 11 comments
Open

Support sync property in @Cacheable annotation #323

TheHett opened this issue Dec 2, 2021 · 11 comments
Assignees

Comments

@TheHett
Copy link

TheHett commented Dec 2, 2021

Hi, configured Spring Boot like this article:
https://medium.com/aerospike-developer-blog/caching-with-spring-boot-and-aerospike-17b91267d6c

But have NPE:

java.lang.NullPointerException: null
	at org.springframework.data.aerospike.cache.AerospikeCache.get(AerospikeCache.java:116) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.cache.interceptor.CacheAspectSupport.handleSynchronizedGet(CacheAspectSupport.java:442) ~[spring-context-5.3.12.jar:5.3.12]
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:382) ~[spring-context-5.3.12.jar:5.3.12]
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:345) ~[spring-context-5.3.12.jar:5.3.12]
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64) ~[spring-context-5.3.12.jar:5.3.12]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.3.12.jar:5.3.12]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:753) ~[spring-aop-5.3.12.jar:5.3.12]
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:698) ~[spring-aop-5.3.12.jar:5.3.12]

This line on AerospikeCache:166 looks strange:

		T value = (T) client.get(null, getKey(key)).getValue(VALUE);

because the contract of method client.get says that null is possible.

\--- com.aerospike:spring-data-aerospike:3.3.0
     +--- com.aerospike:aerospike-client:5.1.9
@roimenashe
Copy link
Collaborator

Hi @TheHett,

Please share your project (github repo?)/code examples including the configuration classes, service/logic layer and any usages of @Cacheable, @CachePut and @CacheEvict annotations.
I'll happily review your code and if indeed there is an issue with the caching mechanism of Spring Data Aerospike we will fix it as soon as possible.

Thanks for showing interest.

@TheHett
Copy link
Author

TheHett commented Dec 2, 2021

@roimenashe thanks,
I created a small demo project that reproduces the problem https://github.com/TheHett/aerospike-demo
While creating I figured out the problem, It happens when the flag sync = true is in annotation:

    @Cacheable(cacheNames = ["test"], sync = true)

@roimenashe
Copy link
Collaborator

@TheHett You're right, we currently don't support sync = true option in @Cacheable annotation.
I'll change the name of this issue to "Support sync property in @Cacheable annotation" and we will add it into our backlog.
Thanks for noticing!

@roimenashe roimenashe changed the title AerospikeCache.get throws NPE Support sync property in @Cacheable annotation Dec 2, 2021
@roimenashe roimenashe self-assigned this Dec 2, 2021
@TheHett
Copy link
Author

TheHett commented Dec 2, 2021

@roimenashe without this option NPE still occurs, but in anoter the class:

java.lang.NullPointerException: null
	at org.springframework.data.aerospike.convert.MappingAerospikeWriteConverter.write(MappingAerospikeWriteConverter.java:74) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.convert.MappingAerospikeConverter.write(MappingAerospikeConverter.java:80) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.convert.MappingAerospikeConverter.write(MappingAerospikeConverter.java:39) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.cache.AerospikeCache.serializeAndPut(AerospikeCache.java:199) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.cache.AerospikeCache.put(AerospikeCache.java:171) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.cache.interceptor.AbstractCacheInvoker.doPut(AbstractCacheInvoker.java:87) ~[spring-context-5.3.13.jar:5.3.13]
	at org.springframework.cache.interceptor.CacheAspectSupport$CachePutRequest.apply(CacheAspectSupport.java:837) ~[spring-context-5.3.13.jar:5.3.13]
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:430) ~[spring-context-5.3.13.jar:5.3.13]
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:345) ~[spring-context-5.3.13.jar:5.3.13]
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64) ~[spring-context-5.3.13.jar:5.3.13]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.3.13.jar:5.3.13]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:753) ~[spring-aop-5.3.13.jar:5.3.13]
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:698) ~[spring-aop-5.3.13.jar:5.3.13]

@roimenashe
Copy link
Collaborator

@TheHett
Try to specify a key when using @Cacheable, look at the example in the article you mentioned:
https://medium.com/aerospike-developer-blog/caching-with-spring-boot-and-aerospike-17b91267d6c

In order for @Cacheable to work it requires an id, when the next execution of the method annotated with @Cacheable it checks wether the id exists in the cache - if so it fetches the data from the cache, if the id doesn't exist in the cache - a "cache miss" will occur, the data will be fetched from the main store and will also be inserted to the cache for next executions.

@Aloren
Copy link
Collaborator

Aloren commented Dec 6, 2021

@roimenashe can we please fix NPE with validation that id is present and throw illegalargumentexception if it is not?

@TheHett
Copy link
Author

TheHett commented Dec 7, 2021

@Aloren
Information about id it's not true. It will be automatically generated if not passed and not necessary specify it manually.
You can watch it using the debugger. The problem is different: just missed null-checkings.

@roimenashe
Copy link
Collaborator

@TheHett
The digest is generated using an empty user id which is not recommended, you should specify id per object you want to save in the cache/data store.

But you are right, this isn't the reason for the null pointer exception, the NPE occurs due to lack of persistent entity which is mentioned in the NPE:

java.lang.NullPointerException: Cannot invoke "org.springframework.data.aerospike.mapping.AerospikePersistentEntity.getPropertyAccessor(Object)" because "entity" is null
	at org.springframework.data.aerospike.convert.MappingAerospikeWriteConverter.write(MappingAerospikeWriteConverter.java:74) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.convert.MappingAerospikeConverter.write(MappingAerospikeConverter.java:80) ~[spring-data-aerospike-3.3.0.jar:na]
	at org.springframework.data.aerospike.convert.MappingAerospikeConverter.write(MappingAerospikeConverter.java:39) ~[spring-data-aerospike-3.3.0.jar:na]
...

The idea of Spring Data Aerospike is to use @document annotated classes for persistent.
I forked your demo project and made the necessary changes and it seems to work:
https://github.com/roimenashe/aerospike-demo

Screen Shot 2021-12-07 at 12 40 33

@Aloren
As @TheHett mentioned, missing id should work as well so validation isn't necessary in this case and as for the persistent entity exception I think the exception description is detailed and self explanatory - but I guess it can be something other the NPE lets think about it.

@Aloren
Copy link
Collaborator

Aloren commented Dec 10, 2021

@roimenashe in MappingAerospikeWriteConverter line 73:
‘’’
AerospikePersistentEntity<?> entity = mappingContext.getPersistentEntity(source.getClass());
‘’’
needs to be replaced with ‘getRequiredPersistentEntity’.

@roimenashe
Copy link
Collaborator

@roimenashe in MappingAerospikeWriteConverter line 73: ‘’’ AerospikePersistentEntity<?> entity = mappingContext.getPersistentEntity(source.getClass()); ‘’’ needs to be replaced with ‘getRequiredPersistentEntity’.

Thanks @Aloren,
I'll test it and let you know/open a PR.

@Aloren
Copy link
Collaborator

Aloren commented Dec 10, 2021

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants