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

@CachePut evaluates key before cache condition #22769

Closed
wants to merge 6 commits into from

Conversation

lgxbslgx
Copy link
Contributor

@lgxbslgx lgxbslgx commented Apr 8, 2019

At present, @Cacheable, @CacheEvict and @CachePut evaluate their keys after condition but before unless. I think the keys should be evaluated after all the cache condition which contains the condition and unless.

I change the code, add the corresponding test cases and fix the JavaDoc for @Cacheable, @CacheEvict and @CachePut.

Fixes #22294.

@Cacheable, @CacheEvict and @CachePut evaluate their keys after
their cache condition which contains the `condition` and `unless`.
Add the corresponding test cases.
Fixes spring-projects#22294.
@Cacheable, @CacheEvict and @CachePut evaluate their conditions
and keys after they get the result of the method, so the `#result` can
be used.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 8, 2019
@jhoeller jhoeller requested a review from snicoll April 9, 2019 07:01
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 10, 2021
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 16, 2021
@snicoll snicoll added this to the 6.0.x milestone Nov 16, 2021
@capatcapworld
Copy link

When will this fix be merged in to master ? We have this problem in our code.

# Conflicts:
#	spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java
#	spring-context/src/test/java/org/springframework/cache/config/AbstractCacheAnnotationTests.java
#	spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/AnnotatedClassCacheableService.java
After merging main branch, some previous test cases missed.
@lgxbslgx
Copy link
Contributor Author

I merged the main branch and solved the conflicts just now.

@jhoeller jhoeller modified the milestones: 6.0.x, 6.x Backlog Jan 11, 2023
@jhoeller jhoeller modified the milestones: 6.x Backlog, 6.1.x Jul 13, 2023
@snicoll snicoll self-assigned this Jul 13, 2023
@snicoll
Copy link
Member

snicoll commented Jul 14, 2023

It took me a bit to understand why the evaluation of cacheable was moved as it felt unrelated but evaluating the unless expression earlier means that we need the result for cacheable as well.

@snicoll snicoll modified the milestones: 6.1.x, 6.1.0-M3 Jul 14, 2023
snicoll pushed a commit that referenced this pull request Jul 14, 2023
Prior to this commit a @CachePut operation would fail if the key
expression is invalid, but guarded with an unless condition as the
former was evaluated too early. This commit makes sure that key for
a put is only evaluated if the put operation is active.

Note that this does not apply for @Cacheable as the key needs to be
computed early to determine if a matching entry exists in the cache.

See gh-22769
snicoll added a commit that referenced this pull request Jul 14, 2023
@snicoll snicoll closed this in d61b9c6 Jul 14, 2023
@snicoll snicoll changed the title Evaluating key after cache condition @CachePut evaluates key before cache condition Jul 14, 2023
@snicoll snicoll added type: bug A general bug and removed type: enhancement A general enhancement labels Jul 14, 2023
@snicoll
Copy link
Member

snicoll commented Jul 14, 2023

@lgxbslgx thank you for making your first contribution to Spring Framework.

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) type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@CachePut evaluates key before cache condition
6 participants