From b0b40dade132577533bfbab34dbe8e03d9c613b6 Mon Sep 17 00:00:00 2001
From: Phillip Webb <pwebb@gopivotal.com>
Date: Tue, 26 Nov 2013 17:04:31 -0800
Subject: [PATCH] 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
---
 .../cache/interceptor/CacheAspectSupport.java |  12 +-
 .../cache/CacheReproTests.java                | 104 ++++++++++++++++++
 2 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 spring-context/src/test/java/org/springframework/cache/CacheReproTests.java

diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java
index 16eba6a8e7d..45476730110 100644
--- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java
+++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java
@@ -261,7 +261,7 @@ public abstract class CacheAspectSupport implements InitializingBean {
 		for (CacheOperationContext context : contexts) {
 			if (isConditionPassing(context, result)) {
 				Object key = generateKey(context, result);
-				if (!whenNotInCache || findInCaches(context, key) == null) {
+				if (!whenNotInCache || findInAnyCaches(contexts, key) == null) {
 					putRequests.add(new CachePutRequest(context, key));
 				}
 			}
@@ -280,6 +280,16 @@ public abstract class CacheAspectSupport implements InitializingBean {
 		return result;
 	}
 
+	private Cache.ValueWrapper  findInAnyCaches(Collection<CacheOperationContext> contexts, Object key) {
+		for (CacheOperationContext context : contexts) {
+			ValueWrapper wrapper = findInCaches(context, key);
+			if (wrapper != null) {
+				return wrapper;
+			}
+		}
+		return null;
+	}
+
 	private Cache.ValueWrapper findInCaches(CacheOperationContext context, Object key) {
 		for (Cache cache : context.getCaches()) {
 			Cache.ValueWrapper wrapper = cache.get(key);
diff --git a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java
new file mode 100644
index 00000000000..c89520a0a41
--- /dev/null
+++ b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java
@@ -0,0 +1,104 @@
+/*
+ * Copyright 2002-2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cache;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Test;
+import org.springframework.cache.annotation.Cacheable;
+import org.springframework.cache.annotation.Caching;
+import org.springframework.cache.annotation.EnableCaching;
+import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+import static org.junit.Assert.*;
+
+/**
+ * Tests to reproduce raised caching issues.
+ *
+ * @author Phillip Webb
+ */
+public class CacheReproTests {
+
+	@Test
+	public void spr11124() throws Exception {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
+				Spr11124Config.class);
+		Spr11124Service bean = context.getBean(Spr11124Service.class);
+		bean.single(2);
+		bean.single(2);
+		bean.multiple(2);
+		bean.multiple(2);
+		context.close();
+	}
+
+	@Configuration
+	@EnableCaching
+	public static class Spr11124Config {
+
+		@Bean
+		public CacheManager cacheManager() {
+			return new ConcurrentMapCacheManager();
+		}
+
+		@Bean
+		public Spr11124Service service() {
+			return new Spr11124ServiceImpl();
+		}
+
+	}
+
+	public interface Spr11124Service {
+
+		public List<String> single(int id);
+
+		public List<String> multiple(int id);
+
+	}
+
+	public static class Spr11124ServiceImpl implements Spr11124Service {
+
+		private int multipleCount = 0;
+
+		@Override
+		@Cacheable(value = "smallCache")
+		public List<String> single(int id) {
+			if (this.multipleCount > 0) {
+				fail("Called too many times");
+			}
+			this.multipleCount++;
+			return Collections.emptyList();
+		}
+
+		@Override
+		@Caching(cacheable = {
+			@Cacheable(value = "bigCache", unless = "#result.size() < 4"),
+			@Cacheable(value = "smallCache", unless = "#result.size() > 3") })
+		public List<String> multiple(int id) {
+			if (this.multipleCount > 0) {
+				fail("Called too many times");
+			}
+			this.multipleCount++;
+			return Collections.emptyList();
+		}
+
+	}
+
+}