Reuse generated key for get+put within same cacheable operation

Closes gh-31789

Author: jhoeller

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

@@ -616,7 +616,7 @@ private void performCacheEvicts(List<CacheOperationContext> contexts, @Nullable
 		for (CacheOperationContext context : contexts) {
 			CacheEvictOperation operation = (CacheEvictOperation) context.metadata.operation;
 			if (isConditionPassing(context, result)) {
-				Object key = null;
+				Object key = context.getGeneratedKey();
 				for (Cache cache : context.getCaches()) {
 					if (operation.isCacheWide()) {
 						logInvalidating(context, operation, null);
@@ -673,7 +673,7 @@ private Object generateKey(CacheOperationContext context, @Nullable Object resul
 			throw new IllegalArgumentException("""
 					Null key returned for cache operation [%s]. If you are using named parameters, \
 					ensure that the compiler uses the '-parameters' flag."""
-						.formatted(context.metadata.operation));
+					.formatted(context.metadata.operation));
 		}
 		if (logger.isTraceEnabled()) {
 			logger.trace("Computed cache key '" + key + "' for operation " + context.metadata.operation);
@@ -798,6 +798,9 @@ protected class CacheOperationContext implements CacheOperationInvocationContext
 		@Nullable
 		private Boolean conditionPassing;
 
+		@Nullable
+		private Object key;
+
 		public CacheOperationContext(CacheOperationMetadata metadata, Object[] args, Object target) {
 			this.metadata = metadata;
 			this.args = extractArgs(metadata.method, args);
@@ -873,9 +876,17 @@ else if (this.metadata.operation instanceof CachePutOperation cachePutOperation)
 		protected Object generateKey(@Nullable Object result) {
 			if (StringUtils.hasText(this.metadata.operation.getKey())) {
 				EvaluationContext evaluationContext = createEvaluationContext(result);
-				return evaluator.key(this.metadata.operation.getKey(), this.metadata.methodKey, evaluationContext);
+				this.key = evaluator.key(this.metadata.operation.getKey(), this.metadata.methodKey, evaluationContext);
+			}
+			else {
+				this.key = this.metadata.keyGenerator.generate(this.target, this.metadata.method, this.args);
 			}
-			return this.metadata.keyGenerator.generate(this.target, this.metadata.method, this.args);
+			return this.key;
+		}
+
+		@Nullable
+		protected Object getGeneratedKey() {
+			return this.key;
 		}
 
 		private EvaluationContext createEvaluationContext(@Nullable Object result) {
@@ -969,7 +980,10 @@ public Object apply(@Nullable Object result) {
 
 		public void performCachePut(@Nullable Object value) {
 			if (this.context.canPutToCache(value)) {
-				Object key = generateKey(this.context, value);
+				Object key = this.context.getGeneratedKey();
+				if (key == null) {
+					key = generateKey(this.context, value);
+				}
 				if (logger.isTraceEnabled()) {
 					logger.trace("Creating cache entry for key '" + key + "' in cache(s) " +
 							this.context.getCacheNames());

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -16,11 +16,16 @@
 
 package org.springframework.cache.config;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
 import org.junit.jupiter.api.Test;
 
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
 import org.springframework.cache.CacheManager;
+import org.springframework.cache.annotation.Cacheable;
 import org.springframework.cache.annotation.CachingConfigurer;
 import org.springframework.cache.annotation.EnableCaching;
 import org.springframework.cache.interceptor.CacheErrorHandler;
@@ -139,6 +144,17 @@ void bothSetOnlyResolverIsUsed() {
 		context.close();
 	}
 
+	@Test
+	void mutableKey() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(EnableCachingConfig.class, ServiceWithMutableKey.class);
+		ctx.refresh();
+
+		ServiceWithMutableKey service = ctx.getBean(ServiceWithMutableKey.class);
+		String result = service.find(new ArrayList<>(List.of("id")));
+		assertThat(service.find(new ArrayList<>(List.of("id")))).isSameAs(result);
+	}
+
 
 	@Configuration
 	@EnableCaching
@@ -277,4 +293,14 @@ public CacheResolver cacheResolver() {
 		}
 	}
 
+
+	static class ServiceWithMutableKey {
+
+		@Cacheable(value = "testCache", keyGenerator = "customKeyGenerator")
+		public String find(Collection<String> id) {
+			id.add("other");
+			return id.toString();
+		}
+	}
+
 }