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

Extend caching for collections #23221

Closed
neiser opened this issue Jul 1, 2019 · 18 comments
Closed

Extend caching for collections #23221

neiser opened this issue Jul 1, 2019 · 18 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@neiser
Copy link
Contributor

neiser commented Jul 1, 2019

I'd like to use the Spring Caching abstraction in a way that I can also ask for a bunch of items via a list of provided IDs, and then put the result into the cache, as if I had asked for each item one by one.

As an example, consider the following snippet:

    @Cacheable("myCache")
    public String findById(String id) {
        // ... access DB backend and return item
    }

    @CollectionCacheable("myCache") // let's suppose this exists and does the magic :D
    public Map<String, String> findByIds(Collection<String> ids) {
        // ... access DB backend, return map of id to item
    }

Then usage would be:

// suppose the following call has happened before the examples below
findById("key1")

// example1: uses "key1" from cache, never calls findByIds
findByIds(Arrays.asList("key1")) 

// example2: calls findByIds(["key2"]) and uses value for "key1" from cache,
//           puts value for "key2" into cache
findByIds(Arrays.asList("key1", "key2"))

// example3: calls findByIds(["key3", "key4"]) as there's nothing in the cache
//           puts value for "key3" and for "key4" separately into cache
findByIds(Arrays.asList("key3", "key4")) 

I've implemented this functionality in a proof-of-concept Spring Boot application here: https://github.com/neiser/spring-collection-cacheable The main implementation can be found in CollectionCacheInterceptor#handleCollectionCacheable

The PoC lacks the following:

  1. Modifying the BeanFactoryCacheOperationSourceAdvisor to account for my extra CollectionCacheableCacheAnnotationParser and CollectionCacheInterceptor only worked via a BeanFactoryPostProcessor
  2. No consisderation of sync (might be possible, but is harder to implement)
  3. The CacheAspectSupport (which I extended in CollectionCacheInterceptor) has only some methods protected, which required me to copy some of the code from CacheAspectSupport in CollectionCacheInterceptor

Still, the tests in MyRepositoryIntTest show that the PoC works 👍

My questions to you folks are:

  1. Do you have a better idea how to "modify" the BeanFactoryCacheOperationSourceAdvisor in order to support my custom @CollectionCacheable?
  2. Do you think the annotation @CollectionCacheable could be officially integrated in Spring's caching layer (of course with some more polishing)? I've seen some discussions on Stackoverflow and in particular in Insert all elements of a returned list into a Cache [SPR-15213] #19777 but they lack the "partial" invocation feature as in example 2 above (they proposes to use the Cache/CacheManager interfaces to achieve the desired functionality).
  3. If you don't want to integrate something like @CollectionCacheable: The CacheAspectSupport class is hard to extend for my PoC. The same holds for CollectionCacheableCacheAnnotationParser, which is extending SpringCacheAnnotationParser. Would you mind to make this easier for my use case?

I'm happy to contribute a PR once I know what your opinion on this feature is. I can also polish the PoC further following your recommendations. I appreciate any feedback!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 1, 2019
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Jul 1, 2019
@snicoll
Copy link
Member

snicoll commented Jul 9, 2019

Thanks for sharing the idea and building a prototype. I don't think such contract is a good fit for a low-level framework abstraction and I've already expressed some concern in #19777 indeed (thought your use case looks different to me).

Having said that, we can look at what makes it hard for you to build such aspect in your own code. Can you share the specifics?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jul 9, 2019
@neiser
Copy link
Contributor Author

neiser commented Jul 10, 2019

Hi @snicoll, thanks for your feedback. I can totally understand that integrating @CollectionCacheable completely would be too much. So, let's go for making the Caching Abstraction better to extend for my use case:

The first step would be to make changing the provided ProxyCachingConfiguration easier. I've introduced a BeanFactoryPostProcessor (BFPP) which adds a BeanPostProcessor to modify the AOP advisor BeanFactoryCacheOperationSourceAdvisor, see https://github.com/neiser/spring-collection-cacheable/blob/master/src/main/java/com/example/springcollectioncacheable/CollectionCacheableProxyCachingConfiguration.java As the method emitting the BFPP is not static, I get a warning when running the application that some features like autowiring don't work. That indicates to me that this modification is rather hacky, and making the method static is hard as AbstractCachingConfiguration is used as a base class for this configuration. Also replacing the CacheInterceptor was not simply done by defining a bean implementing the interface. I suppose it has to do with the fact that the beans I'm trying to replace are infrastructure beans? Maybe I'm also missing the point how to extend this correctly and you have some better idea.

It would be nice if not only the the SpringCacheAnnotationParser is used for finding caching annotations, but all beans implementing CacheAnnotationParser. It looks like there's some support in the code for this, but I did not find a way to achieve getting my own caching annotation and operation added easily.

Last part is reusing and extending the CacheAspectSupport class to write your own CacheInterceptor. Some things were simply private where I would have wished they were protected. Those are little things I guess and I could do them once we have a plan for a PR I'm also happy to work on.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 10, 2019
@snicoll snicoll added for: team-attention and removed type: enhancement A general enhancement labels Jul 10, 2019
@szantopeter
Copy link

@neiser this looks really promising (I am not a Spring team member, just a developer who found this ticket). While the Spring team is deciding to include it or not would you mind creating a library, that can be accessed from maven central?

@neiser
Copy link
Contributor Author

neiser commented Aug 2, 2019

@szantopeter I'm reluctant to create a library without having discussed the integration parts as mentioned above. The final solution would be that some of Spring's caching abstraction are changed such that my code can integrate a bit more seamlessly, and then a library could be created I guess.

I hope that the Spring team will find some time soon to work on this.

If you desperately need this feature now, feel free to simply copy the code in the PoC project I mentioned above.

@neiser
Copy link
Contributor Author

neiser commented Oct 15, 2019

@snicoll Are you going to have a look at this or should this issue be closed?

@snicoll
Copy link
Member

snicoll commented Oct 15, 2019

@neiser thanks for the nudge and sorry for the late reply. We discussed this one at the team meeting and I forgot to share the outcome of the discussion.

CacheAspectSupport is internal and is not really meant to be reused externally so I am afraid I'd recommend rolling out your own arrangement for this.

@snicoll snicoll closed this as completed Oct 15, 2019
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 15, 2019
@szantopeter
Copy link

@neisen if you happen to create a library out of this, please share the URL. I it would be highly useful for me as a user

@chimw
Copy link

chimw commented May 7, 2020

mark to learn

@mxmlnglt
Copy link

mxmlnglt commented Sep 16, 2020

@snicoll ... so what are we supposed to do then?

Today I just discovered Spring's caching feature for the needs of our project, and this usage of the cache (i.e. easier handling of Collection of cached/cacheable objets - particularly for pre-loading the cache) seems repeatedly asked for by many, seeing the continuous amount of questions on SO over the years (and probably more I didn't found):

https://stackoverflow.com/questions/13178723/spring-cache-abstraction-with-multi-value-queries
https://stackoverflow.com/questions/26242492/how-to-cache-results-of-a-spring-data-jpa-query-method-without-using-query-cache
https://stackoverflow.com/questions/27940704/how-to-load-cache-on-startup-in-spring
https://stackoverflow.com/questions/31451994/how-to-pre-populate-data-in-cache-with-spring-and-ehcache
https://stackoverflow.com/questions/33657881/what-strategies-exist-for-using-spring-cache-on-methods-that-take-an-array-or-co
https://stackoverflow.com/questions/33657881/what-strategies-exist-for-using-spring-cache-on-methods-that-take-an-array-or-co
https://stackoverflow.com/questions/37342548/spring-cache-jsr107-list-collection-argument-as-part-of-the-key
https://stackoverflow.com/questions/41966690/putting-all-returned-elements-into-a-spring-boot-cache-using-annotations
https://stackoverflow.com/questions/44529029/spring-cache-with-collection-of-items-entities
https://stackoverflow.com/questions/46330052/how-to-cache-the-list-of-entires-based-on-its-primary-key-using-spring-caching
https://stackoverflow.com/questions/56471247/how-to-cache-data-during-application-startup-in-spring-boot-application

@mxmlnglt
Copy link

mxmlnglt commented Sep 16, 2020

@neiser
Copy link
Contributor Author

neiser commented Sep 16, 2020

@mxmlnglt I'd still be happy if I could get some advice from @snicoll how the PoC I shared in the original post can be turned into something worth building a library from, as @szantopeter asked me to do. I don't think the adjustments necessary in the Spring caching layer are actually that large. Just make it a bit more "interceptable" at the right places, and I could build on top of this in a clean way. The current solution is, as outlined, rather crude.

@neiser
Copy link
Contributor Author

neiser commented Jan 1, 2021

@mxmlnglt @szantopeter I've created a library here: https://github.com/qaware/collection-cacheable-for-spring It will be available on mvn central soon.

@szantopeter
Copy link

@neiser Thanks a lot, the library looks great!

@ms100
Copy link

ms100 commented Feb 3, 2023

You can use Cache As Multi

@maxxyme
Copy link

maxxyme commented Feb 3, 2023

You can use Cache As Multi

Too bad it's entirely in... Chinese.

@ms100
Copy link

ms100 commented Apr 26, 2023

You can use Cache As Multi

Too bad it's entirely in... Chinese.

@maxxyme I have added English documents.

@maxxyme
Copy link

maxxyme commented Apr 28, 2023

You can use Cache As Multi

Too bad it's entirely in... Chinese.

@maxxyme I have added English documents.

Thanks for your work!! https://github.com/ms100/cache-as-multi/blob/main/README-EN.md

@sureshkmit
Copy link

Cache As Multi looks promising. Can we add similar functionality into the spring cache library?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

10 participants