diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index c269dc44..1bfea73c 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up JDK uses: actions/setup-java@v3 with: diff --git a/pom.xml b/pom.xml index 208dfbd9..1a71272a 100644 --- a/pom.xml +++ b/pom.xml @@ -54,7 +54,7 @@ 8.3.0 1.0.6.RELEASE 17 - 1.6.1 + 1.6.2 3.1.0-SNAPSHOT 4.1.0-SNAPSHOT 1.17.3 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 233e93e8..ddeaafd2 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 03acc42a..75f74ca3 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 @@ -40,12 +40,19 @@ public class GlobalLocalResponseCacheGatewayFilter implements GlobalFilter, Orde private final ResponseCacheGatewayFilter responseCacheGatewayFilter; + @Deprecated public GlobalLocalResponseCacheGatewayFilter(ResponseCacheManagerFactory cacheManagerFactory, Cache globalCache, Duration configuredTimeToLive) { responseCacheGatewayFilter = new ResponseCacheGatewayFilter( cacheManagerFactory.create(globalCache, configuredTimeToLive)); } + public GlobalLocalResponseCacheGatewayFilter(ResponseCacheManagerFactory cacheManagerFactory, Cache globalCache, + Duration configuredTimeToLive, LocalResponseCacheProperties.RequestOptions requestOptions) { + responseCacheGatewayFilter = new ResponseCacheGatewayFilter( + cacheManagerFactory.create(globalCache, configuredTimeToLive, requestOptions)); + } + @Override public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { if (exchange.getAttributes().get(LOCAL_RESPONSE_CACHE_FILTER_APPLIED) == null) { 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 cdeef696..652de96f 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 @@ -24,17 +24,16 @@ import org.springframework.cache.Cache; import org.springframework.cloud.gateway.config.LocalResponseCacheAutoConfiguration; import org.springframework.cloud.gateway.filter.GatewayFilter; import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory; +import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheProperties.RequestOptions; import org.springframework.cloud.gateway.support.HasRouteId; import org.springframework.util.unit.DataSize; 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 +48,27 @@ 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; + private final RequestOptions requestOptions; + + @Deprecated public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheManagerFactory, - Duration defaultTimeToLive) { - this(cacheManagerFactory, defaultTimeToLive, null); + Duration defaultTimeToLive, DataSize defaultSize) { + this(cacheManagerFactory, defaultTimeToLive, defaultSize, new RequestOptions()); } public LocalResponseCacheGatewayFilterFactory(ResponseCacheManagerFactory cacheManagerFactory, - Duration defaultTimeToLive, DataSize defaultSize) { + Duration defaultTimeToLive, DataSize defaultSize, RequestOptions requestOptions) { super(RouteCacheConfiguration.class); this.cacheManagerFactory = cacheManagerFactory; this.defaultTimeToLive = defaultTimeToLive; this.defaultSize = defaultSize; + this.requestOptions = requestOptions; } @Override @@ -74,7 +77,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 96e76da1..33a27b03 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 RequestOptions request = new RequestOptions(); + public DataSize getSize() { return size; } @@ -64,9 +66,57 @@ public class LocalResponseCacheProperties { this.timeToLive = timeToLive; } + public RequestOptions getRequest() { + return request; + } + + public void setRequest(RequestOptions request) { + this.request = request; + } + @Override public String toString() { - return "LocalResponseCacheProperties{" + "timeToLive=" + getTimeToLive() + '\'' + ", size='" + getSize() + '}'; + return "LocalResponseCacheProperties{" + "size=" + size + ", timeToLive=" + timeToLive + ", request=" + request + + '}'; + } + + public static class RequestOptions { + + private NoCacheStrategy noCacheStrategy = NoCacheStrategy.SKIP_UPDATE_CACHE_ENTRY; + + public NoCacheStrategy getNoCacheStrategy() { + return noCacheStrategy; + } + + public void setNoCacheStrategy(NoCacheStrategy noCacheStrategy) { + this.noCacheStrategy = noCacheStrategy; + } + + @Override + public String toString() { + return "RequestOptions{" + "noCacheStrategy=" + noCacheStrategy + '}'; + } + + } + + /** + * 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 NoCacheStrategy { + + /** + * 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/LocalResponseCacheUtils.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java new file mode 100644 index 00000000..6de54ac5 --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheUtils.java @@ -0,0 +1,32 @@ +/* + * 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.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.util.StringUtils; + +public final class LocalResponseCacheUtils { + + private LocalResponseCacheUtils() { + } + + public static boolean isNoCacheRequest(ServerHttpRequest request) { + String cacheControl = request.getHeaders().getCacheControl(); + return StringUtils.hasText(cacheControl) && cacheControl.matches(".*(\s|,|^)no-cache(\\s|,|$).*"); + } + +} 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 a7769df1..5fec016d 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 4ad1a82d..90d8a42d 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 @@ -30,8 +30,11 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.cache.Cache; +import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheProperties.NoCacheStrategy; +import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheProperties.RequestOptions; 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 +66,28 @@ public class ResponseCacheManager { private final Cache cache; + private final boolean ignoreNoCacheUpdate; + + @Deprecated public ResponseCacheManager(CacheKeyGenerator cacheKeyGenerator, Cache cache, Duration configuredTimeToLive) { + this(cacheKeyGenerator, cache, configuredTimeToLive, new RequestOptions()); + } + + public ResponseCacheManager(CacheKeyGenerator cacheKeyGenerator, Cache cache, Duration configuredTimeToLive, + RequestOptions 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(RequestOptions requestOptions) { + return requestOptions != null + && requestOptions.getNoCacheStrategy().equals(NoCacheStrategy.SKIP_UPDATE_CACHE_ENTRY); } private static final List statusesToCache = Arrays.asList(HttpStatus.OK, HttpStatus.PARTIAL_CONTENT, @@ -132,13 +151,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 +171,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 00592068..1030070b 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,14 @@ public class ResponseCacheManagerFactory { this.cacheKeyGenerator = cacheKeyGenerator; } + @Deprecated public ResponseCacheManager create(Cache cache, Duration timeToLive) { return new ResponseCacheManager(cacheKeyGenerator, cache, timeToLive); } + public ResponseCacheManager create(Cache cache, Duration timeToLive, + LocalResponseCacheProperties.RequestOptions 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 00000000..cd72b13a --- /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 7b9b5dc0..2bb55319 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 41b2c817..ff724617 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 90105d6b..753c5efa 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 @@ -45,13 +45,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" + "description": "Maximum size of the cache to evict entries for this route (in KB, MB and GB)." }, { "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 eb538345..a161ec54 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,185 @@ 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(); } - 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-strategy=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(); + } + + } + + } + + @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-strategy=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 00000000..711672b8 --- /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 64616cc8..cb2506dd 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 00000000..2911a213 --- /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 b2adcd64..fab93d26 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,6 +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.CollectionUtils; import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -67,16 +68,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 +99,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 +120,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,11 +140,11 @@ 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()) - .map(s -> s.split("\\s*,\\s*")).orElse(null); + String[] cacheControlValues = StringUtils + .tokenizeToStringArray(inputExchange.getResponse().getHeaders().getCacheControl(), ","); assertThat(cacheControlValues).contains("max-stale=12", "min-stale=1"); } @@ -141,7 +155,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 +163,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); - if (matcher.find()) { - return Optional.of(Long.parseLong(matcher.group(1))); - } + private Optional parseMaxAge(List cacheControlValues) { + if (CollectionUtils.isEmpty(cacheControlValues)) { + return Optional.empty(); } - 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 Long.parseLong(matcher.group(1)); + } + 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 9ad1e802..9ff03362 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,18 @@ 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(); + @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.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(); - - 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)); } }