Skip to content

Commit 73a8a1b

Browse files
author
Phillip Webb
committed
Detect cache hit with multiple @Cachables
Fix CacheAspectSupport to consider a cache hit from any of the multiple @Cachables that may have been specified using the @caching annotation. Prior to this commit the following scenario would never produce a hit: @caching(cacheable = { @Cacheable(value = "c1", unless = "#result.size() < 4"), @Cacheable(value = "c2", unless = "#result.size() > 3") }) Issue: SPR-11124
1 parent 0f26a77 commit 73a8a1b

File tree

2 files changed

+108
-8
lines changed

2 files changed

+108
-8
lines changed

spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ private void inspectCacheEvicts(Collection<CacheOperationContext> evictions, boo
269269

270270
private CacheStatus inspectCacheables(Collection<CacheOperationContext> cacheables) {
271271
Map<CacheOperationContext, Object> cacheUpdates = new LinkedHashMap<CacheOperationContext, Object>(cacheables.size());
272-
boolean updateRequired = false;
272+
boolean cacheHit = false;
273273
Object retVal = null;
274274

275275
if (!cacheables.isEmpty()) {
@@ -288,21 +288,17 @@ private CacheStatus inspectCacheables(Collection<CacheOperationContext> cacheabl
288288
}
289289
// add op/key (in case an update is discovered later on)
290290
cacheUpdates.put(context, key);
291-
boolean localCacheHit = false;
292291
// check whether the cache needs to be inspected or not (the method will be invoked anyway)
293-
if (!updateRequired) {
292+
if (!cacheHit) {
294293
for (Cache cache : context.getCaches()) {
295294
Cache.ValueWrapper wrapper = cache.get(key);
296295
if (wrapper != null) {
297296
retVal = wrapper.get();
298-
localCacheHit = true;
297+
cacheHit = true;
299298
break;
300299
}
301300
}
302301
}
303-
if (!localCacheHit) {
304-
updateRequired = true;
305-
}
306302
}
307303
else {
308304
if (log) {
@@ -313,7 +309,7 @@ private CacheStatus inspectCacheables(Collection<CacheOperationContext> cacheabl
313309

314310
// return a status only if at least one cacheable matched
315311
if (atLeastOnePassed) {
316-
return new CacheStatus(cacheUpdates, updateRequired, retVal);
312+
return new CacheStatus(cacheUpdates, !cacheHit, retVal);
317313
}
318314
}
319315

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Copyright 2002-2013 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cache;
18+
19+
import java.util.Collections;
20+
import java.util.List;
21+
22+
import org.junit.Test;
23+
import org.springframework.cache.annotation.Cacheable;
24+
import org.springframework.cache.annotation.Caching;
25+
import org.springframework.cache.annotation.EnableCaching;
26+
import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
27+
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
28+
import org.springframework.context.annotation.Bean;
29+
import org.springframework.context.annotation.Configuration;
30+
31+
import static org.junit.Assert.*;
32+
33+
/**
34+
* Tests to reproduce raised caching issues.
35+
*
36+
* @author Phillip Webb
37+
*/
38+
public class CacheReproTests {
39+
40+
@Test
41+
public void spr11124() throws Exception {
42+
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
43+
Spr11124Config.class);
44+
Spr11124Service bean = context.getBean(Spr11124Service.class);
45+
bean.single(2);
46+
bean.single(2);
47+
bean.multiple(2);
48+
bean.multiple(2);
49+
context.close();
50+
}
51+
52+
@Configuration
53+
@EnableCaching
54+
public static class Spr11124Config {
55+
56+
@Bean
57+
public CacheManager cacheManager() {
58+
return new ConcurrentMapCacheManager();
59+
}
60+
61+
@Bean
62+
public Spr11124Service service() {
63+
return new Spr11124ServiceImpl();
64+
}
65+
66+
}
67+
68+
public interface Spr11124Service {
69+
70+
public List<String> single(int id);
71+
72+
public List<String> multiple(int id);
73+
74+
}
75+
76+
public static class Spr11124ServiceImpl implements Spr11124Service {
77+
78+
private int multipleCount = 0;
79+
80+
@Override
81+
@Cacheable(value = "smallCache")
82+
public List<String> single(int id) {
83+
if (this.multipleCount > 0) {
84+
fail("Called too many times");
85+
}
86+
this.multipleCount++;
87+
return Collections.emptyList();
88+
}
89+
90+
@Override
91+
@Caching(cacheable = {
92+
@Cacheable(value = "bigCache", unless = "#result.size() < 4"),
93+
@Cacheable(value = "smallCache", unless = "#result.size() > 3") })
94+
public List<String> multiple(int id) {
95+
if (this.multipleCount > 0) {
96+
fail("Called too many times");
97+
}
98+
this.multipleCount++;
99+
return Collections.emptyList();
100+
}
101+
102+
}
103+
104+
}

0 commit comments

Comments
 (0)