Allow @cACHEpUT 'key' SpEL to refer to #result

Allow the @cACHEpUT 'key' SpEL to refer to the result of the method call
via a '#result' variable.

This change is particularly useful when working with JPA entities that
have generated @id values since the ID will often not be available
until the entity has been saved.

Issue: SPR-10664

Author: philwebb

spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java

@@ -163,4 +163,11 @@ public Object multiConditionalCacheAndEvict(Object arg1) {
 	public Object multiUpdate(Object arg1) {
 		return arg1;
 	}
+
+	@Override
+	@CachePut(value="primary", key="#result.id")
+	public TestEntity putRefersToResult(TestEntity arg1) {
+		arg1.setId(Long.MIN_VALUE);
+		return arg1;
+	}
 }

spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java

@@ -70,4 +70,6 @@ public interface CacheableService<T> {
 	T multiConditionalCacheAndEvict(Object arg1);
 
 	T multiUpdate(Object arg1);
+
+	TestEntity putRefersToResult(TestEntity arg1);
 }

spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java

@@ -171,4 +171,11 @@ public Long multiConditionalCacheAndEvict(Object arg1) {
 	public Long multiUpdate(Object arg1) {
 		return Long.valueOf(arg1.toString());
 	}
+
+	@Override
+	@CachePut(value="primary", key="#result.id")
+	public TestEntity putRefersToResult(TestEntity arg1) {
+		arg1.setId(Long.MIN_VALUE);
+		return arg1;
+	}
 }

spring-aspects/src/test/java/org/springframework/cache/config/TestEntity.java

@@ -0,0 +1,56 @@
+/*
+ * 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.config;
+
+import org.springframework.util.ObjectUtils;
+
+/**
+ * Simple test entity for use with caching tests.
+ *
+ * @author Michael Plšd
+ */
+public class TestEntity {
+
+	private Long id;
+
+	public Long getId() {
+		return this.id;
+	}
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	@Override
+	public int hashCode() {
+		return ObjectUtils.nullSafeHashCode(this.id);
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj == this) {
+			return true;
+		}
+		if (obj == null) {
+			return false;
+		}
+		if (obj instanceof TestEntity) {
+			return ObjectUtils.nullSafeEquals(this.id, ((TestEntity) obj).id);
+		}
+		return false;
+	}
+}

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

@@ -148,15 +148,15 @@ private Object execute(Invoker invoker, CacheOperationContexts contexts) {
 		// Process any early evictions
 		processCacheEvicts(contexts.get(CacheEvictOperation.class), true, ExpressionEvaluator.NO_RESULT);
 
-		// Collect puts, either explicit @CachePuts or from a @Cachable miss
+		// Collect puts from any @Cachable miss
 		List<CachePutRequest> cachePutRequests = new ArrayList<CachePutRequest>();
-		collectPutRequests(contexts.get(CachePutOperation.class), cachePutRequests, false);
-		collectPutRequests(contexts.get(CacheableOperation.class), cachePutRequests, true);
+		collectPutRequests(contexts.get(CacheableOperation.class),
+				ExpressionEvaluator.NO_RESULT, cachePutRequests, true);
 
 		ValueWrapper result = null;
 
 		// We only attempt to get a cached result if there are no put requests
-		if(cachePutRequests.isEmpty()) {
+		if(cachePutRequests.isEmpty() && contexts.get(CachePutOperation.class).isEmpty()) {
 			result = findCachedResult(contexts.get(CacheableOperation.class));
 		}
 
@@ -165,6 +165,10 @@ private Object execute(Invoker invoker, CacheOperationContexts contexts) {
 			result = new SimpleValueWrapper(invoker.invoke());
 		}
 
+		// Collect any explicit @CachePuts
+		collectPutRequests(contexts.get(CachePutOperation.class), result.get(),
+				cachePutRequests, false);
+
 		// Process any collected put requests, either from @CachePut or a @Cacheable miss
 		for (CachePutRequest cachePutRequest : cachePutRequests) {
 			cachePutRequest.apply(result.get());
@@ -182,21 +186,21 @@ private void processCacheEvicts(Collection<CacheOperationContext> contexts,
 			CacheEvictOperation operation = (CacheEvictOperation) context.operation;
 			if (beforeInvocation == operation.isBeforeInvocation() &&
 					isConditionPassing(context, result)) {
-				performCacheEvict(context, operation);
+				performCacheEvict(context, operation, result);
 			}
 		}
 	}
 
 	private void performCacheEvict(CacheOperationContext context,
-			CacheEvictOperation operation) {
+			CacheEvictOperation operation, Object result) {
 		Object key = null;
 		for (Cache cache : context.getCaches()) {
 			if (operation.isCacheWide()) {
 				logInvalidating(context, operation, null);
 				cache.clear();
 			} else {
 				if(key == null) {
-					key = context.generateKey();
+					key = context.generateKey(result);
 				}
 				logInvalidating(context, operation, key);
 				cache.evict(key);
@@ -213,10 +217,11 @@ private void logInvalidating(CacheOperationContext context,
 		}
 	}
 
-	private void collectPutRequests(Collection<CacheOperationContext> contexts, Collection<CachePutRequest> putRequests, boolean whenNotInCache) {
+	private void collectPutRequests(Collection<CacheOperationContext> contexts,
+			Object result, Collection<CachePutRequest> putRequests, boolean whenNotInCache) {
 		for (CacheOperationContext context : contexts) {
-			if (isConditionPassing(context, ExpressionEvaluator.NO_RESULT)) {
-				Object key = generateKey(context);
+			if (isConditionPassing(context, result)) {
+				Object key = generateKey(context, result);
 				if (!whenNotInCache || findInCaches(context, key) == null) {
 					putRequests.add(new CachePutRequest(context, key));
 				}
@@ -229,7 +234,8 @@ private Cache.ValueWrapper findCachedResult(Collection<CacheOperationContext> co
 		for (CacheOperationContext context : contexts) {
 			if (isConditionPassing(context, ExpressionEvaluator.NO_RESULT)) {
 				if(result == null) {
-					result = findInCaches(context, generateKey(context));
+					result = findInCaches(context,
+							generateKey(context, ExpressionEvaluator.NO_RESULT));
 				}
 			}
 		}
@@ -254,8 +260,8 @@ private boolean isConditionPassing(CacheOperationContext context, Object result)
 		return passing;
 	}
 
-	private Object generateKey(CacheOperationContext context) {
-		Object key = context.generateKey();
+	private Object generateKey(CacheOperationContext context, Object result) {
+		Object key = context.generateKey(result);
 		Assert.notNull(key, "Null key returned for cache operation (maybe you "
 				+ "are using named params on classes without debug info?) "
 				+ context.operation);
@@ -393,9 +399,9 @@ else if (this.operation instanceof CachePutOperation) {
 		 * Computes the key for the given caching operation.
 		 * @return generated key (null if none can be generated)
 		 */
-		protected Object generateKey() {
+		protected Object generateKey(Object result) {
 			if (StringUtils.hasText(this.operation.getKey())) {
-				EvaluationContext evaluationContext = createEvaluationContext(ExpressionEvaluator.NO_RESULT);
+				EvaluationContext evaluationContext = createEvaluationContext(result);
 				return CacheAspectSupport.this.evaluator.key(this.operation.getKey(), this.method, evaluationContext);
 			}
 			return CacheAspectSupport.this.keyGenerator.generate(this.target, this.method, this.args);

spring-context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java

@@ -367,6 +367,16 @@ public void testMultiPut(CacheableService<?> service) {
 		assertSame(r2, secondary.get(o).get());
 	}
 
+	public void testPutRefersToResult(CacheableService<?> service) throws Exception {
+		Long id = Long.MIN_VALUE;
+		TestEntity entity = new TestEntity();
+		Cache primary = cm.getCache("primary");
+		assertNull(primary.get(id));
+		assertNull(entity.getId());
+		service.putRefersToResult(entity);
+		assertSame(entity, primary.get(id).get());
+	}
+
 	public void testMultiCacheAndEvict(CacheableService<?> service) {
 		String methodName = "multiCacheAndEvict";
 
@@ -621,6 +631,16 @@ public void testClassMultiPut() {
 		testMultiPut(ccs);
 	}
 
+	@Test
+	public void testPutRefersToResult() throws Exception {
+		testPutRefersToResult(cs);
+	}
+
+	@Test
+	public void testClassPutRefersToResult() throws Exception {
+		testPutRefersToResult(ccs);
+	}
+
 	@Test
 	public void testMultiCacheAndEvict() {
 		testMultiCacheAndEvict(cs);

spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java

@@ -31,6 +31,7 @@
 public class AnnotatedClassCacheableService implements CacheableService<Object> {
 
 	private final AtomicLong counter = new AtomicLong();
+
 	public static final AtomicLong nullInvocations = new AtomicLong();
 
 	@Override
@@ -164,4 +165,11 @@ public Object multiConditionalCacheAndEvict(Object arg1) {
 	public Object multiUpdate(Object arg1) {
 		return arg1;
 	}
+
+	@Override
+	@CachePut(value="primary", key="#result.id")
+	public TestEntity putRefersToResult(TestEntity arg1) {
+		arg1.setId(Long.MIN_VALUE);
+		return arg1;
+	}
 }

spring-context/src/test/java/org/springframework/cache/config/CacheableService.java

@@ -70,4 +70,6 @@ public interface CacheableService<T> {
 	T multiConditionalCacheAndEvict(Object arg1);
 
 	T multiUpdate(Object arg1);
+
+	TestEntity putRefersToResult(TestEntity arg1);
 }

spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java

@@ -170,4 +170,11 @@ public Long multiConditionalCacheAndEvict(Object arg1) {
 	public Long multiUpdate(Object arg1) {
 		return Long.valueOf(arg1.toString());
 	}
+
+	@Override
+	@CachePut(value="primary", key="#result.id")
+	public TestEntity putRefersToResult(TestEntity arg1) {
+		arg1.setId(Long.MIN_VALUE);
+		return arg1;
+	}
 }

spring-context/src/test/java/org/springframework/cache/config/TestEntity.java

@@ -0,0 +1,56 @@
+/*
+ * 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.config;
+
+import org.springframework.util.ObjectUtils;
+
+/**
+ * Simple test entity for use with caching tests.
+ *
+ * @author Michael Plšd
+ */
+public class TestEntity {
+
+	private Long id;
+
+	public Long getId() {
+		return this.id;
+	}
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	@Override
+	public int hashCode() {
+		return ObjectUtils.nullSafeHashCode(this.id);
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj == this) {
+			return true;
+		}
+		if (obj == null) {
+			return false;
+		}
+		if (obj instanceof TestEntity) {
+			return ObjectUtils.nullSafeEquals(this.id, ((TestEntity) obj).id);
+		}
+		return false;
+	}
+}