From 9fce110c425cd7c8f58ac0416fcdd05908347a45 Mon Sep 17 00:00:00 2001 From: buildmaster Date: Wed, 20 Sep 2023 23:19:19 +0000 Subject: [PATCH] Correct LocalResponseCache behaviour w/ "no-cache", "must-revalidate", "max-age" and "no-store" directives summary: 1. Inputs / expected result in order assuming Cache TTL configured to 120s * Request contains `no-store` or `private` * (done) Cache is skipped without storing response and `Cache-Control` will come from upstream * Request contains `no-cache` * The cached response is ignored by SCG dispatching to upstream(we cannot return 304 - imagine a client sends no-cache waiting for the server to revalidate and returns fresh data but instead it receives a bodyless response 304) * A) Fresh response coming from upstream is renewed in the SCG cache and `Cache-Control: max-age=120s`. Other clients will see max-age increased * B) Fresh response coming from upstream is not stored. Other clients will not be affected. * Otherwise (ignoring `ETag`, `If-Modified-Since` and other request directives * if Response is not cached in SCG yet * `Cache-Control: max-age=120` (no-cache, no-store and must-revalidate doesn't make sense) * if Response is cached in SCG with remaining TTL = 30s * `Cache-Control: max-age=30` (no-cache, no-store and must-revalidate doesn't make sense) * if Response is cached in SCG with remaining TTL = 0s * `Cache-Control: max-age=0,no-cache,must-revalidate` (max-age=0+must-revalidate is equivalent to no-cache) 2. Revalidation is out of scope but it can be added in the future (ETag, If-Modified-Since, Modified-Since) --- README.adoc | 8 +- .../LocalResponseCacheAutoConfiguration.java | 4 +- ...GlobalLocalResponseCacheGatewayFilter.java | 4 +- ...ocalResponseCacheGatewayFilterFactory.java | 25 +- .../cache/LocalResponseCacheProperties.java | 13 +- .../LocalResponseCacheRequestOptions.java | 31 +++ .../cache/LocalResponseCacheUtils.java | 33 +++ .../RequestNoCacheDirectiveStrategy.java | 37 +++ .../cache/ResponseCacheGatewayFilter.java | 21 +- .../factory/cache/ResponseCacheManager.java | 29 ++- .../cache/ResponseCacheManagerFactory.java | 5 +- ...ivesByMaxAgeAfterCacheExchangeMutator.java | 73 ++++++ ...MaxAgeHeaderAfterCacheExchangeMutator.java | 36 ++- ...etStatusCodeAfterCacheExchangeMutator.java | 21 +- ...itional-spring-configuration-metadata.json | 5 +- ...esponseCacheGatewayFilterFactoryTests.java | 225 +++++++++++++++--- .../cache/LocalResponseCacheUtilsTests.java | 50 ++++ .../cache/ResponseCacheGatewayFilterTest.java | 2 +- ...yMaxAgeAfterCacheExchangeMutatorTests.java | 89 +++++++ ...geHeaderAfterCacheExchangeMutatorTest.java | 53 +++-- ...atusCodeAfterCacheExchangeMutatorTest.java | 40 +--- 21 files changed, 652 insertions(+), 152 deletions(-) create mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheRequestOptions.java create mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java create mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/RequestNoCacheDirectiveStrategy.java create mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator.java create mode 100644 spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtilsTests.java create mode 100644 spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutatorTests.java diff --git a/README.adoc b/README.adoc index 1e6a67412..944db2b62 100644 --- a/README.adoc +++ b/README.adoc @@ -6,7 +6,7 @@ Edit the files in the src/main/asciidoc/ directory instead. image::https://github.com/spring-cloud/spring-cloud-gateway/workflows/Build/badge.svg?style=svg["Actions Status", link="https://github.com/spring-cloud/spring-cloud-gateway/actions"] -image::https://codecov.io/gh/spring-cloud/spring-cloud-gateway/branch/main/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-gateway/branch/main"] +image::https://codecov.io/gh/spring-cloud/spring-cloud-gateway/branch/4.0.x/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-gateway/branch/main"] This project provides an API Gateway built on top of the Spring Ecosystem, including: Spring 6, Spring Boot 3 and Project Reactor. Spring Cloud Gateway aims to provide a simple, yet effective way to route to APIs and provide cross cutting concerns to them such as: security, monitoring/metrics, and resiliency. @@ -109,7 +109,7 @@ from the `file` menu. == Contributing -:spring-cloud-build-branch: 4.0.x +:spring-cloud-build-branch: master Spring Cloud is released under the non-restrictive Apache 2.0 license, and follows a very standard Github development process, using Github @@ -126,7 +126,7 @@ author credit if we do. Active contributors might be asked to join the core tea given the ability to merge pull requests. === Code of Conduct -This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/master/docs/src/main/asciidoc/code-of-conduct.adoc[code of +This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/{spring-cloud-build-branch}/docs/src/main/asciidoc/code-of-conduct.adoc[code of conduct]. By participating, you are expected to uphold this code. Please report unacceptable behavior to spring-code-of-conduct@pivotal.io. @@ -137,7 +137,7 @@ added after the original pull request but before a merge. * Use the Spring Framework code format conventions. If you use Eclipse you can import formatter settings using the `eclipse-code-formatter.xml` file from the - https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-dependencies-parent/eclipse-code-formatter.xml[Spring + https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/{spring-cloud-build-branch}/spring-cloud-dependencies-parent/eclipse-code-formatter.xml[Spring Cloud Build] project. If using IntelliJ, you can use the https://plugins.jetbrains.com/plugin/6546[Eclipse Code Formatter Plugin] to import the same file. diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/LocalResponseCacheAutoConfiguration.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/LocalResponseCacheAutoConfiguration.java index 233e93e89..ddeaafd27 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/LocalResponseCacheAutoConfiguration.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/LocalResponseCacheAutoConfiguration.java @@ -65,7 +65,7 @@ public class LocalResponseCacheAutoConfiguration { @Qualifier(RESPONSE_CACHE_MANAGER_NAME) CacheManager cacheManager, LocalResponseCacheProperties properties) { return new GlobalLocalResponseCacheGatewayFilter(responseCacheManagerFactory, responseCache(cacheManager), - properties.getTimeToLive()); + properties.getTimeToLive(), properties.getRequest()); } @Bean(name = RESPONSE_CACHE_MANAGER_NAME) @@ -78,7 +78,7 @@ public class LocalResponseCacheAutoConfiguration { public LocalResponseCacheGatewayFilterFactory localResponseCacheGatewayFilterFactory( ResponseCacheManagerFactory responseCacheManagerFactory, LocalResponseCacheProperties properties) { return new LocalResponseCacheGatewayFilterFactory(responseCacheManagerFactory, properties.getTimeToLive(), - properties.getSize()); + properties.getSize(), properties.getRequest()); } @Bean diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/GlobalLocalResponseCacheGatewayFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/GlobalLocalResponseCacheGatewayFilter.java index 03acc42a4..e2667b7bf 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/GlobalLocalResponseCacheGatewayFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/GlobalLocalResponseCacheGatewayFilter.java @@ -41,9 +41,9 @@ public class GlobalLocalResponseCacheGatewayFilter implements GlobalFilter, Orde private final ResponseCacheGatewayFilter responseCacheGatewayFilter; public GlobalLocalResponseCacheGatewayFilter(ResponseCacheManagerFactory cacheManagerFactory, Cache globalCache, - Duration configuredTimeToLive) { + Duration configuredTimeToLive, LocalResponseCacheRequestOptions requestOptions) { responseCacheGatewayFilter = new ResponseCacheGatewayFilter( - cacheManagerFactory.create(globalCache, configuredTimeToLive)); + cacheManagerFactory.create(globalCache, configuredTimeToLive, requestOptions)); } @Override diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactory.java index cdeef6967..bd91ddbbf 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactory.java @@ -30,11 +30,9 @@ import org.springframework.validation.annotation.Validated; /** * {@link org.springframework.cloud.gateway.filter.factory.GatewayFilterFactory} of - * {@link ResponseCacheGatewayFilter}. - * - * By default, a global cache (defined as properties in the application) is used. For - * specific route configuration, parameters can be added following - * {@link RouteCacheConfiguration} class. + * {@link ResponseCacheGatewayFilter}. By default, a global cache (defined as properties + * in the application) is used. For specific route configuration, parameters can be added + * following {@link RouteCacheConfiguration} class. * * @author Marta Medio * @author Ignacio Lozano @@ -49,23 +47,21 @@ public class LocalResponseCacheGatewayFilterFactory */ public static final String LOCAL_RESPONSE_CACHE_FILTER_APPLIED = "LocalResponseCacheGatewayFilter-Applied"; - private ResponseCacheManagerFactory cacheManagerFactory; + private final ResponseCacheManagerFactory cacheManagerFactory; - private Duration defaultTimeToLive; + private final Duration defaultTimeToLive; - private DataSize defaultSize; + private final DataSize defaultSize; - public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheManagerFactory, - Duration defaultTimeToLive) { - this(cacheManagerFactory, defaultTimeToLive, null); - } + private final LocalResponseCacheRequestOptions requestOptions; public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheManagerFactory, - Duration defaultTimeToLive, DataSize defaultSize) { + Duration defaultTimeToLive, DataSize defaultSize, LocalResponseCacheRequestOptions requestOptions) { super(RouteCacheConfiguration.class); this.cacheManagerFactory = cacheManagerFactory; this.defaultTimeToLive = defaultTimeToLive; this.defaultSize = defaultSize; + this.requestOptions = requestOptions; } @Override @@ -74,7 +70,8 @@ public class LocalResponseCacheGatewayFilterFactory Cache routeCache = LocalResponseCacheAutoConfiguration.createGatewayCacheManager(cacheProperties) .getCache(config.getRouteId() + "-cache"); - return new ResponseCacheGatewayFilter(cacheManagerFactory.create(routeCache, cacheProperties.getTimeToLive())); + return new ResponseCacheGatewayFilter( + cacheManagerFactory.create(routeCache, cacheProperties.getTimeToLive(), requestOptions)); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheProperties.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheProperties.java index 96e76da16..71827b2d0 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheProperties.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheProperties.java @@ -40,6 +40,8 @@ public class LocalResponseCacheProperties { private Duration timeToLive; + private LocalResponseCacheRequestOptions request = new LocalResponseCacheRequestOptions(); + public DataSize getSize() { return size; } @@ -64,9 +66,18 @@ public class LocalResponseCacheProperties { this.timeToLive = timeToLive; } + public LocalResponseCacheRequestOptions getRequest() { + return request; + } + + public void setRequest(LocalResponseCacheRequestOptions request) { + this.request = request; + } + @Override public String toString() { - return "LocalResponseCacheProperties{" + "timeToLive=" + getTimeToLive() + '\'' + ", size='" + getSize() + '}'; + return "LocalResponseCacheProperties{" + "size=" + size + ", timeToLive=" + timeToLive + ", request=" + request + + '}'; } } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheRequestOptions.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheRequestOptions.java new file mode 100644 index 000000000..36c30a2ec --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheRequestOptions.java @@ -0,0 +1,31 @@ +/* + * Copyright 2013-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. + * 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; + +public class LocalResponseCacheRequestOptions { + + private RequestNoCacheDirectiveStrategy noCache = RequestNoCacheDirectiveStrategy.SKIP_UPDATE_CACHE_ENTRY; + + public RequestNoCacheDirectiveStrategy getNoCache() { + return noCache; + } + + public void setNoCache(RequestNoCacheDirectiveStrategy noCache) { + this.noCache = noCache; + } + +} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java new file mode 100644 index 000000000..9850a4fce --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java @@ -0,0 +1,33 @@ +/* + * Copyright 2013-2020 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.util.Optional; + +import org.springframework.http.server.reactive.ServerHttpRequest; + +public final class LocalResponseCacheUtils { + + private LocalResponseCacheUtils() { + } + + public static boolean isNoCacheRequest(ServerHttpRequest request) { + return Optional.ofNullable(request.getHeaders().getCacheControl()) + .filter(cc -> cc.matches(".*(\s|,|^)no-cache(\\s|,|$).*")).isPresent(); + } + +} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/RequestNoCacheDirectiveStrategy.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/RequestNoCacheDirectiveStrategy.java new file mode 100644 index 000000000..ef4b07bbd --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/RequestNoCacheDirectiveStrategy.java @@ -0,0 +1,37 @@ +/* + * Copyright 2013-2022 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; + +/** + * When client sends "no-cache" directive in "Cache-Control" header, the response should + * be re-validated from upstream. There are several strategies that indicates what to do + * with the new fresh response. + */ +public enum RequestNoCacheDirectiveStrategy { + + /** + * Update the cache entry by the fresh response coming from upstream with a new time + * to live. + */ + UPDATE_CACHE_ENTRY, + /** + * Skip the update. The client will receive the fresh response, other clients will + * receive the old entry in cache. + */ + SKIP_UPDATE_CACHE_ENTRY + +} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilter.java index a7769df1e..5fec016d1 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilter.java @@ -66,7 +66,7 @@ public class ResponseCacheGatewayFilter implements GatewayFilter, Ordered { private Mono filterWithCache(ServerWebExchange exchange, GatewayFilterChain chain) { final String metadataKey = responseCacheManager.resolveMetadataKey(exchange); - Optional cached = responseCacheManager.getFromCache(exchange.getRequest(), metadataKey); + Optional cached = getCachedResponse(exchange, metadataKey); if (cached.isPresent()) { return responseCacheManager.processFromCache(exchange, metadataKey, cached.get()); @@ -77,6 +77,22 @@ public class ResponseCacheGatewayFilter implements GatewayFilter, Ordered { } } + private Optional getCachedResponse(ServerWebExchange exchange, String metadataKey) { + Optional cached; + if (shouldRevalidate(exchange)) { + cached = Optional.empty(); + } + else { + cached = responseCacheManager.getFromCache(exchange.getRequest(), metadataKey); + } + + return cached; + } + + private boolean shouldRevalidate(ServerWebExchange exchange) { + return LocalResponseCacheUtils.isNoCacheRequest(exchange.getRequest()); + } + private class CachingResponseDecorator extends ServerHttpResponseDecorator { private final String metadataKey; @@ -94,7 +110,8 @@ public class ResponseCacheGatewayFilter implements GatewayFilter, Ordered { final ServerHttpResponse response = exchange.getResponse(); Flux decoratedBody; - if (responseCacheManager.isResponseCacheable(response)) { + if (responseCacheManager.isResponseCacheable(response) + && !responseCacheManager.isNoCacheRequestWithoutUpdate(exchange.getRequest())) { decoratedBody = responseCacheManager.processFromUpstream(metadataKey, exchange, Flux.from(body)); } else { diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java index 4ad1a82d2..53b126e78 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java @@ -32,6 +32,7 @@ import reactor.core.publisher.Mono; import org.springframework.cache.Cache; import org.springframework.cloud.gateway.filter.factory.cache.keygenerator.CacheKeyGenerator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.AfterCacheExchangeMutator; +import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetMaxAgeHeaderAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetResponseHeadersAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetStatusCodeAfterCacheExchangeMutator; @@ -63,12 +64,23 @@ public class ResponseCacheManager { private final Cache cache; - public ResponseCacheManager(CacheKeyGenerator cacheKeyGenerator, Cache cache, Duration configuredTimeToLive) { + private final boolean ignoreNoCacheUpdate; + + public ResponseCacheManager(CacheKeyGenerator cacheKeyGenerator, Cache cache, Duration configuredTimeToLive, + LocalResponseCacheRequestOptions requestOptions) { this.cacheKeyGenerator = cacheKeyGenerator; this.cache = cache; + this.ignoreNoCacheUpdate = isSkipNoCacheUpdateActive(requestOptions); this.afterCacheExchangeMutators = List.of(new SetResponseHeadersAfterCacheExchangeMutator(), new SetStatusCodeAfterCacheExchangeMutator(), - new SetMaxAgeHeaderAfterCacheExchangeMutator(configuredTimeToLive, Clock.systemDefaultZone())); + new SetMaxAgeHeaderAfterCacheExchangeMutator(configuredTimeToLive, Clock.systemDefaultZone(), + ignoreNoCacheUpdate), + new SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator()); + } + + private static boolean isSkipNoCacheUpdateActive(LocalResponseCacheRequestOptions requestOptions) { + return Optional.ofNullable(requestOptions).map(LocalResponseCacheRequestOptions::getNoCache) + .filter(RequestNoCacheDirectiveStrategy.SKIP_UPDATE_CACHE_ENTRY::equals).isPresent(); } private static final List statusesToCache = Arrays.asList(HttpStatus.OK, HttpStatus.PARTIAL_CONTENT, @@ -132,13 +144,8 @@ public class ResponseCacheManager { afterCacheExchangeMutators.forEach(processor -> processor.accept(exchange, cachedResponse)); saveMetadataInCache(metadataKey, new CachedResponseMetadata(cachedResponse.headers().getVary())); - if (HttpStatus.NOT_MODIFIED.equals(response.getStatusCode())) { - return response.writeWith(Mono.empty()); - } - else { - return response.writeWith( - Flux.fromIterable(cachedResponse.body()).map(data -> response.bufferFactory().wrap(data))); - } + return response + .writeWith(Flux.fromIterable(cachedResponse.body()).map(data -> response.bufferFactory().wrap(data))); } private CachedResponseMetadata retrieveMetadata(String metadataKey) { @@ -157,6 +164,10 @@ public class ResponseCacheManager { return isStatusCodeToCache(response) && isCacheControlAllowed(response) && !isVaryWildcard(response); } + boolean isNoCacheRequestWithoutUpdate(ServerHttpRequest request) { + return LocalResponseCacheUtils.isNoCacheRequest(request) && ignoreNoCacheUpdate; + } + private boolean isStatusCodeToCache(ServerHttpResponse response) { return statusesToCache.contains(response.getStatusCode()); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManagerFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManagerFactory.java index 005920684..827526189 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManagerFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManagerFactory.java @@ -33,8 +33,9 @@ public class ResponseCacheManagerFactory { this.cacheKeyGenerator = cacheKeyGenerator; } - public ResponseCacheManager create(Cache cache, Duration timeToLive) { - return new ResponseCacheManager(cacheKeyGenerator, cache, timeToLive); + public ResponseCacheManager create(Cache cache, Duration timeToLive, + LocalResponseCacheRequestOptions requestOptions) { + return new ResponseCacheManager(cacheKeyGenerator, cache, timeToLive, requestOptions); } } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator.java new file mode 100644 index 000000000..cd72b13a8 --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator.java @@ -0,0 +1,73 @@ +/* + * Copyright 2013-2020 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.postprocessor; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; +import org.springframework.web.server.ServerWebExchange; + +public class SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator implements AfterCacheExchangeMutator { + + final Pattern MAX_AGE_PATTERN = Pattern.compile("(?:,|^)\\s*max-age=(\\d+)"); + + @Override + public void accept(ServerWebExchange exchange, CachedResponse cachedResponse) { + Optional maxAge = Optional.ofNullable(exchange.getResponse().getHeaders().getCacheControl()) + .map(MAX_AGE_PATTERN::matcher).filter(Matcher::find).map(matcher -> matcher.group(1)) + .map(Integer::parseInt); + + if (maxAge.isPresent()) { + if (maxAge.get() > 0) { + removeNoCacheHeaders(exchange); + } + else { + keepNoCacheHeaders(exchange); + } + } + } + + private void keepNoCacheHeaders(ServerWebExchange exchange) { + // at least it contains 'max-age' so we can append items with commas safely + String cacheControl = exchange.getResponse().getHeaders().getCacheControl(); + StringBuilder newCacheControl = new StringBuilder(cacheControl); + + if (!cacheControl.contains("no-cache")) { + newCacheControl.append(",no-cache"); + } + + if (!cacheControl.contains("must-revalidate")) { + newCacheControl.append(",must-revalidate"); + } + exchange.getResponse().getHeaders().setCacheControl(newCacheControl.toString()); + } + + private void removeNoCacheHeaders(ServerWebExchange exchange) { + String cacheControl = exchange.getResponse().getHeaders().getCacheControl(); + List cacheControlValues = Arrays.asList(cacheControl.split("\\s*,\\s*")); + + String newCacheControl = cacheControlValues.stream() + .filter(s -> !s.matches("must-revalidate|no-cache|no-store")).collect(Collectors.joining(",")); + exchange.getResponse().getHeaders().setCacheControl(newCacheControl); + } + +} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutator.java index 7b9b5dc09..2bb553194 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutator.java @@ -25,7 +25,9 @@ import java.util.List; import java.util.stream.Collectors; import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; +import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheUtils; import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.web.server.ServerWebExchange; @@ -45,25 +47,33 @@ public class SetMaxAgeHeaderAfterCacheExchangeMutator implements AfterCacheExcha private final Clock clock; - public SetMaxAgeHeaderAfterCacheExchangeMutator(Duration configuredTimeToLive, Clock clock) { + private final boolean ignoreNoCacheUpdate; + + public SetMaxAgeHeaderAfterCacheExchangeMutator(Duration configuredTimeToLive, Clock clock, + boolean ignoreNoCacheUpdate) { this.configuredTimeToLive = configuredTimeToLive; this.clock = clock; + this.ignoreNoCacheUpdate = ignoreNoCacheUpdate; } @Override public void accept(ServerWebExchange exchange, CachedResponse cachedResponse) { ServerHttpResponse response = exchange.getResponse(); - long calculatedMaxAgeInSeconds = calculateMaxAgeInSeconds(cachedResponse, configuredTimeToLive); + long calculatedMaxAgeInSeconds = calculateMaxAgeInSeconds(exchange.getRequest(), cachedResponse, + configuredTimeToLive); rewriteCacheControlMaxAge(response.getHeaders(), calculatedMaxAgeInSeconds); } - private long calculateMaxAgeInSeconds(CachedResponse cachedResponse, Duration configuredTimeToLive) { + private long calculateMaxAgeInSeconds(ServerHttpRequest request, CachedResponse cachedResponse, + Duration configuredTimeToLive) { + boolean noCache = LocalResponseCacheUtils.isNoCacheRequest(request); long maxAge; - if (configuredTimeToLive.getSeconds() == -1) { - maxAge = -1; + if (noCache && ignoreNoCacheUpdate || configuredTimeToLive.getSeconds() < 0) { + maxAge = 0; } else { - maxAge = Math.max(0, configuredTimeToLive.minus(getElapsedTimeInSeconds(cachedResponse)).getSeconds()); + long calculatedMaxAge = configuredTimeToLive.minus(getElapsedTimeInSeconds(cachedResponse)).getSeconds(); + maxAge = Math.max(0, calculatedMaxAge); } return maxAge; @@ -77,10 +87,10 @@ public class SetMaxAgeHeaderAfterCacheExchangeMutator implements AfterCacheExcha boolean isMaxAgePresent = headers.getCacheControl() != null && headers.getCacheControl().contains(MAX_AGE_PREFIX); + List newCacheControlDirectives = new ArrayList<>(); if (isMaxAgePresent) { List cacheControlHeaders = headers.get(HttpHeaders.CACHE_CONTROL); cacheControlHeaders = cacheControlHeaders == null ? Collections.emptyList() : cacheControlHeaders; - List replacedCacheControlHeaders = new ArrayList<>(); for (String value : cacheControlHeaders) { if (value.contains(MAX_AGE_PREFIX)) { if (seconds == -1) { @@ -92,11 +102,17 @@ public class SetMaxAgeHeaderAfterCacheExchangeMutator implements AfterCacheExcha value = value.replaceFirst("\\bmax-age=\\d+\\b", MAX_AGE_PREFIX + seconds); } } - replacedCacheControlHeaders.add(value); + newCacheControlDirectives.add(value); } - headers.remove(HttpHeaders.CACHE_CONTROL); - headers.addAll(HttpHeaders.CACHE_CONTROL, replacedCacheControlHeaders); } + else { + List cacheControlHeaders = headers.get(HttpHeaders.CACHE_CONTROL); + newCacheControlDirectives = cacheControlHeaders == null ? new ArrayList<>() + : new ArrayList<>(cacheControlHeaders); + newCacheControlDirectives.add("max-age=" + seconds); + } + headers.remove(HttpHeaders.CACHE_CONTROL); + headers.addAll(HttpHeaders.CACHE_CONTROL, newCacheControlDirectives); } } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutator.java index 41b2c817c..ff7246170 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutator.java @@ -17,38 +17,21 @@ package org.springframework.cloud.gateway.filter.factory.cache.postprocessor; import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpResponse; -import org.springframework.util.CollectionUtils; import org.springframework.web.server.ServerWebExchange; /** - * It sets HTTP Status Code depending {@literal no-cache} - * {@link HttpHeaders#CACHE_CONTROL} header. + * It sets HTTP Status Code. * * @author Marta Medio * @author Ignacio Lozano */ public class SetStatusCodeAfterCacheExchangeMutator implements AfterCacheExchangeMutator { - private static final String NO_CACHE_VALUE = "no-cache"; - @Override public void accept(ServerWebExchange exchange, CachedResponse cachedResponse) { - HttpHeaders requestHeaders = exchange.getRequest().getHeaders(); ServerHttpResponse response = exchange.getResponse(); - - if (!CollectionUtils.isEmpty(cachedResponse.body()) && isRequestNoCache(requestHeaders)) { - response.setStatusCode(HttpStatus.NOT_MODIFIED); - } - else { - response.setStatusCode(cachedResponse.statusCode()); - } - } - - private boolean isRequestNoCache(HttpHeaders requestHeaders) { - return requestHeaders.getCacheControl() != null && requestHeaders.getCacheControl().contains(NO_CACHE_VALUE); + response.setStatusCode(cachedResponse.statusCode()); } } diff --git a/spring-cloud-gateway-server/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-gateway-server/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 90105d6b7..4921a1fb0 100644 --- a/spring-cloud-gateway-server/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-cloud-gateway-server/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -46,12 +46,13 @@ "name": "spring.cloud.gateway.filter.local-response-cache.size", "type": "org.springframework.util.unit.DataSize", "description": "Maximum size of the cache to evict entries for this route (in KB, MB and GB).", - "defaultValue": "5m" + "defaultValue": "null" }, { "name": "spring.cloud.gateway.filter.local-response-cache.time-to-live", "type": "java.time.Duration", - "description": "Time to expire a cache entry (expressed in s for seconds, m for minutes, and h for hours)." + "description": "Time to expire a cache entry (expressed in s for seconds, m for minutes, and h for hours).", + "defaultValue": "5m" }, { "name": "spring.cloud.gateway.filter.dedupe-response-header.enabled", diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java index eb538345b..6a133b812 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.gateway.filter.factory.cache; import java.time.Duration; +import java.time.Instant; import java.util.Map; import java.util.Objects; import java.util.UUID; @@ -44,6 +45,7 @@ import org.springframework.util.StringUtils; import org.springframework.util.unit.DataSize; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.data.Offset.offset; /** * @author Ignacio Lozano @@ -53,12 +55,23 @@ import static org.assertj.core.api.Assertions.assertThat; @ActiveProfiles(profiles = "local-cache-filter") public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTests { - private static final String CUSTOM_HEADER = "X-Custom-Date"; + private static final String CUSTOM_HEADER = "X-Custom-Header"; + + private static Long parseMaxAge(String cacheControlValue) { + if (StringUtils.hasText(cacheControlValue)) { + Pattern maxAgePattern = Pattern.compile("\\bmax-age=(\\d+)\\b"); + Matcher matcher = maxAgePattern.matcher(cacheControlValue); + if (matcher.find()) { + return Long.parseLong(matcher.group(1)); + } + } + return null; + } @Nested @SpringBootTest(properties = { "spring.cloud.gateway.filter.local-response-cache.enabled=true" }, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) - public class LocalResponseCacheUsingFilterParams extends BaseWebClientTests { + public class UsingFilterParams extends BaseWebClientTests { @Test void shouldNotCacheResponseWhenGetRequestHasBody() { @@ -101,17 +114,18 @@ public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTe } @Test - void shouldCacheAndReturnNotModifiedStatusWhenCacheControlIsNoCache() { + void shouldNotIncludeMustRevalidateNoStoreAndNoCacheDirectivesWhenMaxAgeIsPositive() { String uri = "/" + UUID.randomUUID() + "/cache/headers"; - testClient.get().uri(uri).header("Host", "www.localresponsecache.org").header(CUSTOM_HEADER, "1").exchange() - .expectBody().jsonPath("$.headers." + CUSTOM_HEADER); + var response = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() + .expectBody().returnResult(); + var maxAge = response.getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); - testClient.get().uri(uri).header("Host", "www.localresponsecache.org").header(CUSTOM_HEADER, "2") - // Cache-Control asks to not return cached content because it is - // HttpHeaders.NotModified - .header(HttpHeaders.CACHE_CONTROL, CacheControl.noCache().getHeaderValue()).exchange() - .expectStatus().isNotModified().expectBody().isEmpty(); + assertThat(maxAge).isGreaterThan(0L); + assertThat(response.getResponseHeaders().get(HttpHeaders.CACHE_CONTROL)).doesNotContain("no-store", + "must-revalidate", "no-cache"); } @Test @@ -163,11 +177,13 @@ public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTe String uri = "/" + UUID.randomUUID() + "/cache/headers"; Long maxAgeRequest1 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() - .map(this::parseMaxAge).filter(Objects::nonNull).findAny().orElse(null); + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); Thread.sleep(2000); Long maxAgeRequest2 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() - .map(this::parseMaxAge).filter(Objects::nonNull).findAny().orElse(null); + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); assertThat(maxAgeRequest2).isLessThan(maxAgeRequest1); } @@ -213,17 +229,6 @@ public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTe .jsonPath("$.headers." + CUSTOM_HEADER, "2"); } - private Long parseMaxAge(String cacheControlValue) { - if (StringUtils.hasText(cacheControlValue)) { - Pattern maxAgePattern = Pattern.compile("\\bmax-age=(\\d+)\\b"); - Matcher matcher = maxAgePattern.matcher(cacheControlValue); - if (matcher.find()) { - return Long.parseLong(matcher.group(1)); - } - } - return null; - } - void assertNonVaryHeaderInContent(String uri, String varyHeader, String varyHeaderValue, String nonVaryHeader, String nonVaryHeaderValue, String expectedNonVaryResponse) { testClient.get().uri(uri).header("Host", "www.localresponsecache.org").header("X-Request-Vary", varyHeader) @@ -275,34 +280,186 @@ public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTe properties = { "spring.cloud.gateway.filter.local-response-cache.enabled=true", "spring.cloud.gateway.filter.local-response-cache.timeToLive=20s" }, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) - public class LocalResponseCacheUsingDefaultProperties extends BaseWebClientTests { + public class UsingPropertiesAsDefault extends BaseWebClientTests { @Test void shouldApplyMaxAgeFromPropertiesWhenFilterHasNoParams() throws InterruptedException { String uri = "/" + UUID.randomUUID() + "/cache/headers"; Long maxAgeRequest1 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() - .map(this::parseMaxAge).filter(Objects::nonNull).findAny().orElse(null); + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); assertThat(maxAgeRequest1).isLessThanOrEqualTo(20L); Thread.sleep(2000); Long maxAgeRequest2 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() - .map(this::parseMaxAge).filter(Objects::nonNull).findAny().orElse(null); + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); assertThat(maxAgeRequest2).isLessThan(maxAgeRequest1); } - private Long parseMaxAge(String cacheControlValue) { - if (StringUtils.hasText(cacheControlValue)) { - Pattern maxAgePattern = Pattern.compile("\\bmax-age=(\\d+)\\b"); - Matcher matcher = maxAgePattern.matcher(cacheControlValue); - if (matcher.find()) { - return Long.parseLong(matcher.group(1)); - } + @Test + void shouldNotCacheWhenPrivateDirectiveIsInRequest() { + testClient = testClient.mutate().responseTimeout(Duration.ofHours(1)).build(); + + String uri = "/" + UUID.randomUUID() + "/cache/headers"; + + testClient.get().uri(uri).header("Host", "www.localresponsecache.org") + .header(HttpHeaders.CACHE_CONTROL, CacheControl.noStore().getHeaderValue()) + .header(CUSTOM_HEADER, "1").exchange().expectBody().jsonPath("$.headers." + CUSTOM_HEADER); + + testClient.get().uri(uri).header("Host", "www.localresponsecache.org").header(CUSTOM_HEADER, "2").exchange() + .expectBody().jsonPath("$.headers." + CUSTOM_HEADER).isEqualTo("2"); + + testClient.get().uri(uri).header("Host", "www.localresponsecache.org").header(CUSTOM_HEADER, "3") // second + // request + // cached + // "2" + // -> + // "3" + // will + // be + // ignored + .exchange().expectBody().jsonPath("$.headers." + CUSTOM_HEADER).isEqualTo("2"); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + public static class TestConfig { + + @Value("${test.uri}") + String uri; + + @Bean + public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { + return builder.routes().route("local_response_cache_java_test", + r -> r.path("/{namespace}/cache/**").and().host("{sub}.localresponsecache.org") + .filters(f -> f.stripPrefix(2).prefixPath("/httpbin").localResponseCache(null, null)) + .uri(uri)) + .build(); + } + + } + + } + + @Nested + @SpringBootTest( + properties = { "spring.cloud.gateway.filter.local-response-cache.enabled=true", + "spring.cloud.gateway.filter.local-response-cache.time-to-live=2m", + "spring.cloud.gateway.filter.local-response-cache.request.no-cache=skip-update-cache-entry" }, + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) + public class DirectiveNoCacheSkippingUpdate extends BaseWebClientTests { + + @Test + void shouldNotCacheWhenCacheControlAsksToValidateWithNotCache_refreshCacheWhenDirectiveNoCache() + throws InterruptedException { + String uri = "/" + UUID.randomUUID() + "/cache/headers"; + + // 1. Store in cache - max-age ~= 2m AND NOT + // (must-revalidate,no-cache,no-store) + final Instant when1stRequest = Instant.now(); + var firstResponse = testClient.get().uri(uri).header("Host", "www.localresponsecache.org") + .header(CUSTOM_HEADER, "1").exchange().expectBody().jsonPath("$.headers." + CUSTOM_HEADER) + .isEqualTo("1").returnResult(); + var maxAge1st = firstResponse.getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAge1st).isCloseTo(Duration.ofMinutes(2).toSeconds(), offset(10L)); + assertThat(firstResponse.getResponseHeaders().getCacheControl()).doesNotContain("must-revalidate", + "no-cache", "no-store"); + + // 2. "no-cache" should return max-age=0 & must-revalidate,no-cache,no-store + var secondResponse = testClient.get().uri(uri).header("Host", "www.localresponsecache.org") + .header(CUSTOM_HEADER, "2") + // Cache-Control asks to not use the cached content + .header(HttpHeaders.CACHE_CONTROL, CacheControl.noCache().getHeaderValue()).exchange().expectBody() + .jsonPath("$.headers." + CUSTOM_HEADER).isEqualTo("2").returnResult(); + var maxAge2nd = secondResponse.getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAge2nd).isZero(); + + // 3. After 2s, max-age = (when1stRequest) - 1s - offset_delay + var waitDuration = Duration.ofSeconds(1); + Thread.sleep(waitDuration.toMillis()); // Wait 2s to check max-age renewed + final Instant when3rdRequest = Instant.now(); + var thirdResponseCached = testClient.get().uri(uri).header("Host", "www.localresponsecache.org") + .header(CUSTOM_HEADER, "3").exchange().expectBody().jsonPath("$.headers." + CUSTOM_HEADER) + .isEqualTo("1").returnResult(); + var maxAge3rd = thirdResponseCached.getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAge3rd).isCloseTo( + Duration.ofMinutes(2).minus(Duration.between(when1stRequest, when3rdRequest)).getSeconds(), + offset(10L)); + assertThat(maxAge3rd).isNotZero(); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + public static class TestConfig { + + @Value("${test.uri}") + String uri; + + @Bean + public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { + return builder.routes().route("local_response_cache_java_test", + r -> r.path("/{namespace}/cache/**").and().host("{sub}.localresponsecache.org") + .filters(f -> f.stripPrefix(2).prefixPath("/httpbin").localResponseCache(null, null)) + .uri(uri)) + .build(); } - return null; + + } + + } + + @Nested + @SpringBootTest( + properties = { "spring.cloud.gateway.filter.local-response-cache.enabled=true", + "spring.cloud.gateway.filter.local-response-cache.time-to-live=2m", + "spring.cloud.gateway.filter.local-response-cache.request.no-cache=update-cache-entry" }, + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) + public class DirectiveNoCacheWithUpdate extends BaseWebClientTests { + + @Test + void oldMaxAgeWhenNoCacheRequest() throws InterruptedException { + testClient = testClient.mutate().responseTimeout(Duration.ofHours(1)).build(); + + String uri = "/" + UUID.randomUUID() + "/cache/headers"; + // First request -> cache miss + final Instant when1stRequest = Instant.now(); + Long maxAgeRequest1 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() + .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAgeRequest1).isCloseTo(Duration.ofMinutes(2).toSeconds(), offset(10L)); + + // Second request + no-cache -> skip cache and ignore update + Thread.sleep(1000); + final Duration between1stAnd2ndRequest = Duration.between(when1stRequest, Instant.now()); + Long maxAgeRequest2 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org") + .header(HttpHeaders.CACHE_CONTROL, CacheControl.noCache().getHeaderValue()).exchange().expectBody() + .returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAgeRequest2).isCloseTo(Duration.ofMinutes(2).minus(between1stAnd2ndRequest).getSeconds(), + offset(10L)); + + // Third request -> cache hit -> entry (and max-age) is updated + Thread.sleep(1000); + Long maxAgeRequest3 = testClient.get().uri(uri).header("Host", "www.localresponsecache.org").exchange() + .expectBody().returnResult().getResponseHeaders().get(HttpHeaders.CACHE_CONTROL).stream() + .map(LocalResponseCacheGatewayFilterFactoryTests::parseMaxAge).filter(Objects::nonNull).findAny() + .orElse(null); + assertThat(maxAgeRequest3).isCloseTo(Duration.ofMinutes(2).toSeconds(), offset(10L)); } @EnableAutoConfiguration diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtilsTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtilsTests.java new file mode 100644 index 000000000..711672b89 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtilsTests.java @@ -0,0 +1,50 @@ +/* + * Copyright 2013-2020 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 org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; + +import static org.assertj.core.api.Assertions.assertThat; + +class LocalResponseCacheUtilsTests { + + @ParameterizedTest + @ValueSource(strings = { "", "no-store", "no-store, wrong-no-cache", "s-no-cache" }) + void shouldNotIdentifyRequestAsNoCacheRequest(String cacheControl) { + MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this") + .header("Cache-Control", cacheControl).build(); + + boolean result = LocalResponseCacheUtils.isNoCacheRequest(httpRequest); + + assertThat(result).isFalse(); + } + + @ParameterizedTest + @ValueSource(strings = { "no-cache", "s-no-cache, no-cache", "private,no-cache", " no-cache", "no-cache " }) + void shouldIdentifyRequestAsNoCacheRequest(String cacheControl) { + MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this") + .header("Cache-Control", cacheControl).build(); + + boolean result = LocalResponseCacheUtils.isNoCacheRequest(httpRequest); + + assertThat(result).isTrue(); + } + +} diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilterTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilterTest.java index 64616cc8c..cb2506dd5 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilterTest.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheGatewayFilterTest.java @@ -30,7 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; */ class ResponseCacheGatewayFilterTest { - ResponseCacheManager cacheManagerToTest = new ResponseCacheManager(null, null, null); + ResponseCacheManager cacheManagerToTest = new ResponseCacheManager(null, null, null, null); @Test void requestShouldBeCacheable() { diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutatorTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutatorTests.java new file mode 100644 index 000000000..2911a2130 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetCacheDirectivesByMaxAgeAfterCacheExchangeMutatorTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2013-2020 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.postprocessor; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.http.server.reactive.MockServerHttpResponse; +import org.springframework.mock.web.server.MockServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; + +class SetCacheDirectivesByMaxAgeAfterCacheExchangeMutatorTests { + + private MockServerWebExchange inputExchange; + + private SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator toTest; + + @BeforeEach + void setUp() { + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setCacheControl("max-age=1234"); + MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this").build(); + + inputExchange = MockServerWebExchange.from(httpRequest); + MockServerHttpResponse httpResponse = inputExchange.getResponse(); + httpResponse.setStatusCode(HttpStatus.OK); + httpResponse.getHeaders().putAll(responseHeaders); + + toTest = new SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator(); + } + + @ParameterizedTest + @ValueSource(strings = { "", "ETag=1234-123", "s-max-age=20" }) + void doesntModifyCacheControlWhenNoMaxAge(String cacheControlValue) { + inputExchange.getResponse().getHeaders().setCacheControl(cacheControlValue); + + toTest.accept(inputExchange, null); + + assertThat(inputExchange.getResponse().getHeaders().getCacheControl()).isEqualTo(cacheControlValue); + } + + @ParameterizedTest + @ValueSource( + strings = { "max-age=0", "ETag=1234-123,max-age=0", "s-max-age=20,max-age=0", "ETag=with-spaces, max-age=0", + "ETag=with-spaces, max-age=0,Expires=123123123", " max-age=0, ETag=with-spaces" }) + void directivesNoCacheAreAddedWhenMaxAgeIsZero(String cacheControlValue) { + inputExchange.getResponse().getHeaders().setCacheControl(cacheControlValue); + + toTest.accept(inputExchange, null); + + assertThat(inputExchange.getResponse().getHeaders().getCacheControl()).doesNotContainPattern(",\\s*,") + .contains("max-age=0").contains("must-revalidate").contains("no-cache"); + } + + @ParameterizedTest + @ValueSource(strings = { "max-age=10,must-revalidate", "must-revalidate,ETag=1234-123,max-age=10", + "must-revalidate,s-max-age=0,max-age=10", " max-age=10, must-revalidate,ETag=with-spaces", + "ETag=with-spaces,must-revalidate, max-age=10,Expires=123123123", + "ETag=with-spaces,must-revalidate, max-age=10", "max-age=10,no-store" }) + void directivesNoCacheAreRemovedWhenMaxAgePositive(String cacheControlValue) { + inputExchange.getResponse().getHeaders().setCacheControl(cacheControlValue); + + toTest.accept(inputExchange, null); + + assertThat(inputExchange.getResponse().getHeaders().getCacheControl()).contains("max-age=10") + .doesNotContainPattern(",\\s*,").doesNotContain("no-store").doesNotContain("must-revalidate") + .doesNotContain("no-cache"); + } + +} diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutatorTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutatorTest.java index b2adcd647..780ea1114 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutatorTest.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetMaxAgeHeaderAfterCacheExchangeMutatorTest.java @@ -34,7 +34,7 @@ import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpResponse; import org.springframework.mock.web.server.MockServerWebExchange; -import org.springframework.util.StringUtils; +import org.springframework.util.CollectionUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -67,16 +67,29 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { } @Test - void maxAgeIsNotAdded_whenMaxAgeIsNotPresent() { + void maxAgeIsTimeToLive_whenMaxAgeIsNotPresent() { inputExchange.getResponse().getHeaders().setCacheControl((String) null); Duration timeToLive = Duration.ofSeconds(30); CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, - clock); + clock, false); toTest.accept(inputExchange, inputCachedResponse); - assertThat(parseMaxAge(inputExchange.getResponse())).isEmpty(); + assertThat(parseMaxAge(inputExchange.getResponse()).get()).isEqualTo(timeToLive.getSeconds()); + } + + @Test + void maxAgeIsZero_whenTimeToLiveIsNegative() { + inputExchange.getResponse().getHeaders().setCacheControl((String) null); + + Duration timeToLive = Duration.ofSeconds(-1); + CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); + + SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, + clock, false); + toTest.accept(inputExchange, inputCachedResponse); + assertThat(parseMaxAge(inputExchange.getResponse()).get()).isEqualTo(0L); } @Test @@ -85,12 +98,12 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, - clock); + clock, false); toTest.accept(inputExchange, inputCachedResponse); Optional firstMaxAgeSeconds = parseMaxAge(inputExchange.getResponse()); SetMaxAgeHeaderAfterCacheExchangeMutator toTestSecondsLater = new SetMaxAgeHeaderAfterCacheExchangeMutator( - timeToLive, clockSecondsLater); + timeToLive, clockSecondsLater, false); toTestSecondsLater.accept(inputExchange, inputCachedResponse); Optional secondMaxAgeSeconds = parseMaxAge(inputExchange.getResponse()); @@ -106,12 +119,12 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, - clock); + clock, false); toTest.accept(inputExchange, inputCachedResponse); Optional firstMaxAgeSeconds = parseMaxAge(inputExchange.getResponse()); SetMaxAgeHeaderAfterCacheExchangeMutator toTestSecondsLater = new SetMaxAgeHeaderAfterCacheExchangeMutator( - timeToLive, clockSecondsLater); + timeToLive, clockSecondsLater, false); toTestSecondsLater.accept(inputExchange, inputCachedResponse); Optional secondMaxAgeSeconds = parseMaxAge(inputExchange.getResponse()); @@ -126,7 +139,7 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, - clock); + clock, false); toTest.accept(inputExchange, inputCachedResponse); String[] cacheControlValues = Optional.ofNullable(inputExchange.getResponse().getHeaders().getCacheControl()) @@ -141,7 +154,7 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { CachedResponse inputCachedResponse = CachedResponse.create(HttpStatus.OK).timestamp(clock.instant()).build(); SetMaxAgeHeaderAfterCacheExchangeMutator toTest = new SetMaxAgeHeaderAfterCacheExchangeMutator(timeToLive, - clock); + clock, false); toTest.accept(inputExchange, inputCachedResponse); List cacheControlValues = inputExchange.getResponse().getHeaders().get("X-Custom-Header"); @@ -149,18 +162,22 @@ class SetMaxAgeHeaderAfterCacheExchangeMutatorTest { } private Optional parseMaxAge(ServerHttpResponse response) { - return parseMaxAge(response.getHeaders().getCacheControl()); + return parseMaxAge(response.getHeaders().get("Cache-Control")); } - private Optional parseMaxAge(String cacheControlValue) { - if (StringUtils.hasText(cacheControlValue)) { - Pattern maxAgePattern = Pattern.compile("\\bmax-age=(\\d+)\\b"); - Matcher matcher = maxAgePattern.matcher(cacheControlValue); + private Optional parseMaxAge(List cacheControlValues) { + if (CollectionUtils.isEmpty(cacheControlValues)) { + return Optional.empty(); + } + + final Pattern maxAgePattern = Pattern.compile("\\bmax-age=(\\d+)\\b"); + return cacheControlValues.stream().map(cacheControlDirective -> { + Matcher matcher = maxAgePattern.matcher(cacheControlDirective); if (matcher.find()) { - return Optional.of(Long.parseLong(matcher.group(1))); + return Long.parseLong(matcher.group(1)); } - } - return Optional.empty(); + return null; + }).filter(maxAge -> maxAge != null).findFirst(); } } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutatorTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutatorTest.java index 9ad1e802b..d052ad204 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutatorTest.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/SetStatusCodeAfterCacheExchangeMutatorTest.java @@ -17,7 +17,8 @@ package org.springframework.cloud.gateway.filter.factory.cache.postprocessor; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; import org.springframework.http.HttpHeaders; @@ -47,42 +48,17 @@ class SetStatusCodeAfterCacheExchangeMutatorTest { httpResponse.getHeaders().putAll(responseHeaders); } - @Test - void statusCodeIs304_whenCacheHitsAndNoCacheHeaderIsPresent() { - CachedResponse cachedResponse = CachedResponse.create(HttpStatus.OK).body("some-data").build(); - MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this") - .header("Cache-Control", "no-cache").build(); - - inputExchange = MockServerWebExchange.from(httpRequest); - - SetStatusCodeAfterCacheExchangeMutator toTest = new SetStatusCodeAfterCacheExchangeMutator(); - toTest.accept(inputExchange, cachedResponse); - - assertThat(inputExchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.NOT_MODIFIED); - } - - @Test - void statusCodeIs200_whenCacheHitsAndNoCacheHeaderIsNotPresent() { - CachedResponse cachedResponse = CachedResponse.create(HttpStatus.OK).body("some-data").build(); - - SetStatusCodeAfterCacheExchangeMutator toTest = new SetStatusCodeAfterCacheExchangeMutator(); - toTest.accept(inputExchange, cachedResponse); - - assertThat(inputExchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.OK); - } - - @Test - void statusCodeIs200_whenNoCacheHitsAndEvenNoCacheHeaderIsPresent() { - CachedResponse cachedResponse = CachedResponse.create(HttpStatus.OK).build(); - MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this") - .header("Cache-Control", "no-cache").build(); + @ParameterizedTest + @ValueSource(ints = {200, 400, 404, 500}) + void statusCodeIsSetFromCachedResponse(int statusCode) { + CachedResponse cachedResponse = CachedResponse.create(HttpStatus.valueOf(statusCode)).body("some-data").build(); + MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this").build(); inputExchange = MockServerWebExchange.from(httpRequest); SetStatusCodeAfterCacheExchangeMutator toTest = new SetStatusCodeAfterCacheExchangeMutator(); toTest.accept(inputExchange, cachedResponse); - assertThat(inputExchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(inputExchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.valueOf(statusCode)); } - }