Fix: LocalResponseCacheGatewayFilterFactory recreates Caffeine cache on every route refresh

spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactory.java

@@ -18,7 +18,9 @@
 
 import java.time.Duration;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 
 import com.github.benmanes.caffeine.cache.Caffeine;
 import org.jspecify.annotations.Nullable;
@@ -63,6 +65,8 @@ public class LocalResponseCacheGatewayFilterFactory
 
 	private final CaffeineCacheManager caffeineCacheManager;
 
+	private final Map<String, CacheSettings> registeredCacheSettings = new ConcurrentHashMap<>();
+
 	public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheManagerFactory,
 			Duration defaultTimeToLive, DataSize defaultSize, RequestOptions requestOptions) {
 		this(cacheManagerFactory, defaultTimeToLive, defaultSize, requestOptions, new CaffeineCacheManager());
@@ -84,16 +88,33 @@ public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheM
 	public GatewayFilter apply(RouteCacheConfiguration config) {
 		LocalResponseCacheProperties cacheProperties = mapRouteCacheConfig(config);
 
-		Caffeine caffeine = LocalResponseCacheUtils.createCaffeine(cacheProperties);
-		String cacheName = config.getRouteId() + "-cache";
-		caffeineCacheManager.registerCustomCache(cacheName, caffeine.build());
-		Cache routeCache = caffeineCacheManager.getCache(cacheName);
-		Objects.requireNonNull(routeCache, "Cache " + cacheName + " not found");
+		Cache routeCache = registerOrReuseCache(config, cacheProperties);
 		return new ResponseCacheGatewayFilter(
 				cacheManagerFactory.create(routeCache, cacheProperties.getTimeToLive(), requestOptions));
 
 	}
 
+	/**
+	 * Returns the cache backing the route. A route refresh applies the filter again, so
+	 * the cache already registered is reused when the route cache configuration has not
+	 * changed, keeping the entries cached so far. A new cache is built when the
+	 * configuration changed, and also when the route has no id, because in that case the
+	 * cache name cannot tell one route from another.
+	 */
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	private Cache registerOrReuseCache(RouteCacheConfiguration config, LocalResponseCacheProperties cacheProperties) {
+		String cacheName = config.getRouteId() + "-cache";
+		CacheSettings settings = new CacheSettings(cacheProperties.getTimeToLive(), cacheProperties.getSize());
+		if (config.getRouteId() == null || !settings.equals(registeredCacheSettings.get(cacheName))) {
+			Caffeine caffeine = LocalResponseCacheUtils.createCaffeine(cacheProperties);
+			caffeineCacheManager.registerCustomCache(cacheName, caffeine.build());
+			registeredCacheSettings.put(cacheName, settings);
+		}
+		Cache routeCache = caffeineCacheManager.getCache(cacheName);
+		Objects.requireNonNull(routeCache, "Cache " + cacheName + " not found");
+		return routeCache;
+	}
+
 	private LocalResponseCacheProperties mapRouteCacheConfig(RouteCacheConfiguration config) {
 		Duration timeToLive = config.getTimeToLive() != null ? config.getTimeToLive() : defaultTimeToLive;
 		DataSize size = config.getSize() != null ? config.getSize() : defaultSize;
@@ -148,4 +169,7 @@ public void setRouteId(String routeId) {
 
 	}
 
+	private record CacheSettings(@Nullable Duration timeToLive, @Nullable DataSize size) {
+	}
+
 }

spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java

@@ -16,26 +16,40 @@
 
 package org.springframework.cloud.gateway.filter.factory.cache;
 
+import java.net.URI;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junitpioneer.jupiter.RetryingTest;
+import reactor.core.publisher.Flux;
 
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.cloud.gateway.event.RefreshRoutesEvent;
+import org.springframework.cloud.gateway.event.RefreshRoutesResultEvent;
+import org.springframework.cloud.gateway.filter.FilterDefinition;
+import org.springframework.cloud.gateway.handler.predicate.PredicateDefinition;
+import org.springframework.cloud.gateway.route.RouteDefinition;
+import org.springframework.cloud.gateway.route.RouteDefinitionLocator;
 import org.springframework.cloud.gateway.route.RouteLocator;
 import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder;
 import org.springframework.cloud.gateway.test.BaseWebClientTests;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Import;
 import org.springframework.http.CacheControl;
@@ -75,6 +89,9 @@ private static Long parseMaxAge(String cacheControlValue) {
 			webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 	public class UsingFilterParams extends BaseWebClientTests {
 
+		@Autowired
+		private ConfigurableApplicationContext applicationContext;
+
 		@RetryingTest(3)
 		void shouldNotCacheResponseWhenGetRequestHasBody() {
 			String uri = "/" + UUID.randomUUID() + "/cache/headers";
@@ -195,6 +212,51 @@ void shouldCacheResponseWhenOnlyNonVaryHeaderIsDifferent() {
 					.isEqualTo(customHeaderFromReq1));
 		}
 
+		@RetryingTest(3)
+		void shouldServeCachedResponseAfterRouteRefreshWhenCacheConfigurationIsUnchanged() {
+			String uri = "/" + UUID.randomUUID() + "/refreshable-cache/headers";
+
+			testClient.get()
+				.uri(uri)
+				.header("Host", "www.localresponsecache.org")
+				.header(CUSTOM_HEADER, "1")
+				.exchange()
+				.expectBody()
+				.jsonPath("$.headers." + CUSTOM_HEADER)
+				.isEqualTo("1");
+
+			// A route refresh re-applies the filter factory. The cache configuration is
+			// unchanged, so the entries cached before the refresh must survive it.
+			refreshRoutes();
+			refreshRoutes();
+
+			testClient.get()
+				.uri(uri)
+				.header("Host", "www.localresponsecache.org")
+				.header(CUSTOM_HEADER, "2")
+				.exchange()
+				.expectBody()
+				.jsonPath("$.headers." + CUSTOM_HEADER)
+				.isEqualTo("1");
+		}
+
+		private void refreshRoutes() {
+			CountDownLatch refreshed = new CountDownLatch(1);
+			ApplicationListener<RefreshRoutesResultEvent> listener = event -> refreshed.countDown();
+			applicationContext.addApplicationListener(listener);
+			try {
+				applicationContext.publishEvent(new RefreshRoutesEvent(this));
+				assertThat(refreshed.await(DURATION.toSeconds(), TimeUnit.SECONDS)).isTrue();
+			}
+			catch (InterruptedException ex) {
+				Thread.currentThread().interrupt();
+				throw new IllegalStateException("Interrupted while waiting for the route refresh", ex);
+			}
+			finally {
+				applicationContext.removeApplicationListener(listener);
+			}
+		}
+
 		@RetryingTest(3)
 		void shouldNotCacheResponseWhenVaryHeaderIsDifferent() {
 			String varyHeader = HttpHeaders.ORIGIN;
@@ -450,6 +512,28 @@ public RouteLocator testRouteLocator(RouteLocatorBuilder builder) {
 					.build();
 			}
 
+			/**
+			 * The Java DSL does not set the route id on the filter configuration, so
+			 * routes built with {@link RouteLocatorBuilder} always get a new cache. A
+			 * route definition carries its id all the way to the filter configuration,
+			 * which is what a discovery-driven deployment looks like, so the refresh
+			 * behaviour has to be asserted on a route defined this way.
+			 */
+			@Bean
+			public RouteDefinitionLocator refreshableCacheRouteDefinitionLocator() {
+				RouteDefinition routeDefinition = new RouteDefinition();
+				routeDefinition.setId("refreshable_local_response_cache_test");
+				routeDefinition.setUri(URI.create(uri));
+				routeDefinition.setPredicates(List.of(new PredicateDefinition("Path=/{namespace}/refreshable-cache/**"),
+						new PredicateDefinition("Host={sub}.localresponsecache.org")));
+				FilterDefinition localResponseCache = new FilterDefinition();
+				localResponseCache.setName("LocalResponseCache");
+				localResponseCache.addArg("timeToLive", "2m");
+				routeDefinition.setFilters(List.of(new FilterDefinition("StripPrefix=2"),
+						new FilterDefinition("PrefixPath=/httpbin"), localResponseCache));
+				return () -> Flux.just(routeDefinition);
+			}
+
 		}
 
 	}

spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryUnitTests.java

@@ -0,0 +1,117 @@
+/*
+ * Copyright 2013-present 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
+ *
+ *      https://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.cloud.gateway.filter.factory.cache;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.cache.Cache;
+import org.springframework.cache.caffeine.CaffeineCacheManager;
+import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheGatewayFilterFactory.RouteCacheConfiguration;
+import org.springframework.cloud.gateway.filter.factory.cache.keygenerator.CacheKeyGenerator;
+import org.springframework.util.unit.DataSize;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for {@link LocalResponseCacheGatewayFilterFactory}.
+ */
+public class LocalResponseCacheGatewayFilterFactoryUnitTests {
+
+	@Test
+	void applyReusesRegisteredCacheOnRouteRefresh() {
+		CaffeineCacheManager cacheManager = new CaffeineCacheManager();
+		LocalResponseCacheGatewayFilterFactory factory = createFactory(cacheManager);
+
+		RouteCacheConfiguration config = new RouteCacheConfiguration();
+		config.setRouteId("refreshed-route");
+
+		factory.apply(config);
+		Cache cache = cacheManager.getCache("refreshed-route-cache");
+		assertThat(cache).isNotNull();
+		cache.put("cached-key", "cached-value");
+
+		factory.apply(config);
+
+		Cache cacheAfterRefresh = cacheManager.getCache("refreshed-route-cache");
+		assertThat(cacheAfterRefresh).isSameAs(cache);
+		assertThat(cacheAfterRefresh.get("cached-key", String.class)).isEqualTo("cached-value");
+	}
+
+	@Test
+	void applyRegistersOneCachePerRoute() {
+		CaffeineCacheManager cacheManager = new CaffeineCacheManager();
+		LocalResponseCacheGatewayFilterFactory factory = createFactory(cacheManager);
+
+		RouteCacheConfiguration firstConfig = new RouteCacheConfiguration();
+		firstConfig.setRouteId("first-route");
+		RouteCacheConfiguration secondConfig = new RouteCacheConfiguration();
+		secondConfig.setRouteId("second-route");
+
+		factory.apply(firstConfig);
+		factory.apply(secondConfig);
+
+		Cache firstCache = cacheManager.getCache("first-route-cache");
+		Cache secondCache = cacheManager.getCache("second-route-cache");
+		assertThat(firstCache).isNotNull();
+		assertThat(secondCache).isNotNull();
+		assertThat(firstCache).isNotSameAs(secondCache);
+	}
+
+	@Test
+	void applyBuildsNewCacheWhenRouteCacheConfigurationChanges() {
+		CaffeineCacheManager cacheManager = new CaffeineCacheManager();
+		LocalResponseCacheGatewayFilterFactory factory = createFactory(cacheManager);
+
+		RouteCacheConfiguration config = new RouteCacheConfiguration();
+		config.setRouteId("reconfigured-route");
+		config.setTimeToLive(Duration.ofMinutes(2));
+
+		factory.apply(config);
+		Cache cache = cacheManager.getCache("reconfigured-route-cache");
+
+		factory.apply(config.setTimeToLive(Duration.ofMillis(100)));
+
+		assertThat(cacheManager.getCache("reconfigured-route-cache")).isNotSameAs(cache);
+	}
+
+	@Test
+	void applyBuildsNewCacheWhenRouteHasNoId() {
+		CaffeineCacheManager cacheManager = new CaffeineCacheManager();
+		LocalResponseCacheGatewayFilterFactory factory = createFactory(cacheManager);
+
+		RouteCacheConfiguration longLivedConfig = new RouteCacheConfiguration();
+		longLivedConfig.setTimeToLive(Duration.ofMinutes(2));
+		RouteCacheConfiguration shortLivedConfig = new RouteCacheConfiguration();
+		shortLivedConfig.setTimeToLive(Duration.ofMillis(100));
+
+		factory.apply(longLivedConfig);
+		Cache longLivedCache = cacheManager.getCache("null-cache");
+
+		factory.apply(shortLivedConfig);
+
+		assertThat(cacheManager.getCache("null-cache")).isNotSameAs(longLivedCache);
+	}
+
+	private LocalResponseCacheGatewayFilterFactory createFactory(CaffeineCacheManager cacheManager) {
+		return new LocalResponseCacheGatewayFilterFactory(new ResponseCacheManagerFactory(new CacheKeyGenerator()),
+				Duration.ofMinutes(5), DataSize.ofMegabytes(5), new LocalResponseCacheProperties().getRequest(),
+				cacheManager);
+	}
+
+}