From f779d16e8ee646b74a1dd085ff5c9cae8f4255dc Mon Sep 17 00:00:00 2001 From: spencergibb Date: Tue, 25 Oct 2022 17:06:35 -0400 Subject: [PATCH] Polishes Add LocalResponseCache filter gh-2759 --- .../cache/keygenerator/CacheKeyGenerator.java | 11 +++-- .../keygenerator/CommonKeyValueGenerator.java | 46 ------------------- .../CookiesKeyValueGenerator.java | 9 +--- .../keygenerator/HeaderKeyValueGenerator.java | 4 +- .../cache/keygenerator/KeyValueGenerator.java | 17 +++++-- .../keygenerator/UriKeyValueGenerator.java | 2 +- .../DisableBuiltInFiltersTests.java | 2 +- .../DisableBuiltInGlobalFiltersTests.java | 4 +- ...ava => DefaultKeyValueGeneratorTests.java} | 16 +++++-- 9 files changed, 42 insertions(+), 69 deletions(-) delete mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGenerator.java rename spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/{CommonKeyValueGeneratorTest.java => DefaultKeyValueGeneratorTests.java} (82%) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CacheKeyGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CacheKeyGenerator.java index 37012377..addb1d20 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CacheKeyGenerator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CacheKeyGenerator.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.List; import java.util.stream.Stream; +import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.ServerHttpRequest; /** @@ -33,11 +34,15 @@ import org.springframework.http.server.reactive.ServerHttpRequest; */ public class CacheKeyGenerator { - private static final byte[] KEY_SEPARATOR_BYTES = ";".getBytes(); + /* for testing */ static final String KEY_SEPARATOR = ";"; + + private static final byte[] KEY_SEPARATOR_BYTES = KEY_SEPARATOR.getBytes(); private final MessageDigest messageDigest; - private static final CommonKeyValueGenerator COMMON_KEY_VALUE_GENERATOR = new CommonKeyValueGenerator(); + /* for testing */ static final List DEFAULT_KEY_VALUE_GENERATORS = List.of( + new UriKeyValueGenerator(), new HeaderKeyValueGenerator(HttpHeaders.AUTHORIZATION, KEY_SEPARATOR), + new CookiesKeyValueGenerator(KEY_SEPARATOR)); public CacheKeyGenerator() { try { @@ -64,7 +69,7 @@ public class CacheKeyGenerator { } private Stream getKeyValueGenerators(List varyHeaders) { - return Stream.concat(Stream.of(COMMON_KEY_VALUE_GENERATOR), + return Stream.concat(DEFAULT_KEY_VALUE_GENERATORS.stream(), varyHeaders.stream().sorted().map(header -> new HeaderKeyValueGenerator(header, ","))); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGenerator.java deleted file mode 100644 index 53575b9b..00000000 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGenerator.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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.keygenerator; - -import java.util.List; -import java.util.stream.Collectors; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.server.reactive.ServerHttpRequest; - -/** - * @author Marta Medio - */ -public class CommonKeyValueGenerator implements KeyValueGenerator { - - private static final String JOINING_DELIMITER = ";"; - - private final List keyValueGenerators; - - public CommonKeyValueGenerator() { - keyValueGenerators = List.of(new UriKeyValueGenerator(), - new HeaderKeyValueGenerator(HttpHeaders.AUTHORIZATION, JOINING_DELIMITER), - new CookiesKeyValueGenerator(JOINING_DELIMITER)); - } - - @Override - public String apply(ServerHttpRequest request) { - return keyValueGenerators.stream().map(generator -> generator.apply(request)) - .collect(Collectors.joining(JOINING_DELIMITER)); - } - -} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CookiesKeyValueGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CookiesKeyValueGenerator.java index b3d6b17c..14d86541 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CookiesKeyValueGenerator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CookiesKeyValueGenerator.java @@ -37,13 +37,8 @@ class CookiesKeyValueGenerator implements KeyValueGenerator { this.valueSeparator = Objects.requireNonNull(valueSeparator); } - @Override - public String apply(ServerHttpRequest request) { - return calculateCookiesData(request); - } - - private String calculateCookiesData(ServerHttpRequest request) { - String cookiesData = ""; + public String getKeyValue(ServerHttpRequest request) { + String cookiesData = null; MultiValueMap cookies = request.getCookies(); if (!CollectionUtils.isEmpty(cookies)) { cookiesData = cookies.values().stream().flatMap(Collection::stream) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/HeaderKeyValueGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/HeaderKeyValueGenerator.java index ed6ba285..d5ea55ca 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/HeaderKeyValueGenerator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/HeaderKeyValueGenerator.java @@ -43,7 +43,7 @@ class HeaderKeyValueGenerator implements KeyValueGenerator { } @Override - public String apply(ServerHttpRequest request) { + public String getKeyValue(ServerHttpRequest request) { HttpHeaders headers = request.getHeaders(); if (headers.get(header) != null) { StringBuilder keyVaryHeaders = new StringBuilder(); @@ -51,7 +51,7 @@ class HeaderKeyValueGenerator implements KeyValueGenerator { .append(getHeaderValues(headers).sorted().collect(Collectors.joining(valueSeparator))); return keyVaryHeaders.toString(); } - return ""; + return null; } private Stream getHeaderValues(HttpHeaders headers) { diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/KeyValueGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/KeyValueGenerator.java index 4e688bc8..a0665e9d 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/KeyValueGenerator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/KeyValueGenerator.java @@ -16,8 +16,6 @@ package org.springframework.cloud.gateway.filter.factory.cache.keygenerator; -import java.util.function.Function; - import org.springframework.http.server.reactive.ServerHttpRequest; /** @@ -26,6 +24,19 @@ import org.springframework.http.server.reactive.ServerHttpRequest; * @author Marta Medio * @author Ignacio Lozano */ -interface KeyValueGenerator extends Function { +interface KeyValueGenerator { + + /* + * Calls getKeyValue() and guards against null. + */ + default String apply(ServerHttpRequest request) { + String key = getKeyValue(request); + if (key == null) { + return ""; + } + return key; + } + + String getKeyValue(ServerHttpRequest request); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/UriKeyValueGenerator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/UriKeyValueGenerator.java index 60a0e744..a30f15ee 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/UriKeyValueGenerator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/UriKeyValueGenerator.java @@ -27,7 +27,7 @@ import org.springframework.http.server.reactive.ServerHttpRequest; public class UriKeyValueGenerator implements KeyValueGenerator { @Override - public String apply(ServerHttpRequest request) { + public String getKeyValue(ServerHttpRequest request) { return request.getURI().toString(); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInFiltersTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInFiltersTests.java index 2e70bf00..9cb6601a 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInFiltersTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInFiltersTests.java @@ -78,7 +78,7 @@ public class DisableBuiltInFiltersTests { @RunWith(SpringRunner.class) @SpringBootTest(classes = Config.class, - properties = {"spring.cloud.gateway.filter.add-request-header.enabled=false", + properties = { "spring.cloud.gateway.filter.add-request-header.enabled=false", "spring.cloud.gateway.filter.map-request-header.enabled=false", "spring.cloud.gateway.filter.add-request-headers-if-not-present.enabled=false", "spring.cloud.gateway.filter.add-request-parameter.enabled=false", diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInGlobalFiltersTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInGlobalFiltersTests.java index db55ccb0..0db80831 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInGlobalFiltersTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/conditional/DisableBuiltInGlobalFiltersTests.java @@ -59,8 +59,8 @@ public class DisableBuiltInGlobalFiltersTests { @RunWith(SpringRunner.class) @SpringBootTest(classes = Config.class, - properties = {"spring.cloud.gateway.global-filter.remove-cached-body.enabled=false", - "spring.cloud.gateway.global-filter.route-to-request-url.enabled=false"}) + properties = { "spring.cloud.gateway.global-filter.remove-cached-body.enabled=false", + "spring.cloud.gateway.global-filter.route-to-request-url.enabled=false" }) @ActiveProfiles("disable-components") public static class DisableSpecificsFiltersByProperty { diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGeneratorTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/DefaultKeyValueGeneratorTests.java similarity index 82% rename from spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGeneratorTest.java rename to spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/DefaultKeyValueGeneratorTests.java index 81b8bea6..f22a5cee 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/CommonKeyValueGeneratorTest.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/keygenerator/DefaultKeyValueGeneratorTests.java @@ -16,10 +16,13 @@ package org.springframework.cloud.gateway.filter.factory.cache.keygenerator; +import java.util.stream.Collectors; + import org.junit.jupiter.api.Test; import org.springframework.http.HttpCookie; import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -27,7 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Ignacio Lozano */ -class CommonKeyValueGeneratorTest { +class DefaultKeyValueGeneratorTests { @Test void uriAuthorizationAndCookiesArePresent() { @@ -40,7 +43,7 @@ class CommonKeyValueGeneratorTest { HttpCookie cookie = new HttpCookie(cookieName, cookieValue); MockServerHttpRequest request = MockServerHttpRequest.get(uri).cookie(cookie).headers(headers).build(); - String result = new CommonKeyValueGenerator().apply(request); + String result = apply(request); assertThat(result).isEqualTo(uri + ";Authorization=" + authorization + ";" + cookieName + "=" + cookieValue); } @@ -54,7 +57,7 @@ class CommonKeyValueGeneratorTest { HttpCookie cookie = new HttpCookie(cookieName, cookieValue); MockServerHttpRequest request = MockServerHttpRequest.get(uri).cookie(cookie).headers(headers).build(); - String result = new CommonKeyValueGenerator().apply(request); + String result = apply(request); assertThat(result).isEqualTo(uri + ";" + "" + ";" + cookieName + "=" + cookieValue); } @@ -64,9 +67,14 @@ class CommonKeyValueGeneratorTest { String uri = "http://myuri"; MockServerHttpRequest request = MockServerHttpRequest.get(uri).build(); - String result = new CommonKeyValueGenerator().apply(request); + String result = apply(request); assertThat(result).isEqualTo(uri + ";" + "" + ";" + ""); } + public String apply(ServerHttpRequest request) { + return CacheKeyGenerator.DEFAULT_KEY_VALUE_GENERATORS.stream().map(generator -> generator.apply(request)) + .collect(Collectors.joining(CacheKeyGenerator.KEY_SEPARATOR)); + } + }