From 5002fe2e0a2825ef47dd667cade37b844c276cf6 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Tue, 27 Jul 2021 21:33:34 -0400 Subject: [PATCH] polish and update tests --- .../config/GatewayRedisAutoConfiguration.java | 6 +- .../CompositeRouteDefinitionLocator.java | 4 +- .../route/RedisRouteDefinitionRepository.java | 28 ++--- .../GatewayRedisAutoConfigurationTests.java | 87 +++++++-------- .../RedisRouteDefinitionRepositoryTests.java | 102 +++++++++++------- 5 files changed, 118 insertions(+), 109 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfiguration.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfiguration.java index 56f19e84..d4624591 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfiguration.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfiguration.java @@ -72,8 +72,7 @@ class GatewayRedisAutoConfiguration { } @Bean - @ConditionalOnProperty( - value = "spring.cloud.gateway.redis-route-definition-repository.enabled", + @ConditionalOnProperty(value = "spring.cloud.gateway.redis-route-definition-repository.enabled", havingValue = "true") @ConditionalOnClass(ReactiveRedisTemplate.class) public RedisRouteDefinitionRepository redisRouteDefinitionRepository( @@ -89,8 +88,7 @@ class GatewayRedisAutoConfiguration { RouteDefinition.class); RedisSerializationContext.RedisSerializationContextBuilder builder = RedisSerializationContext .newSerializationContext(keySerializer); - RedisSerializationContext context = builder - .value(valueSerializer).build(); + RedisSerializationContext context = builder.value(valueSerializer).build(); return new ReactiveRedisTemplate<>(factory, context); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java index 21ba3a86..217cda6c 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java @@ -16,6 +16,8 @@ package org.springframework.cloud.gateway.route; +import java.util.UUID; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; @@ -63,7 +65,7 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator { } protected Mono randomId() { - return Mono.fromSupplier(idGenerator::toString).publishOn(Schedulers.boundedElastic()); + return Mono.fromSupplier(idGenerator::generateId).map(UUID::toString).publishOn(Schedulers.boundedElastic()); } } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepository.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepository.java index 7fe192bb..c157ca8e 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepository.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepository.java @@ -43,16 +43,14 @@ public class RedisRouteDefinitionRepository implements RouteDefinitionRepository private ReactiveValueOperations routeDefinitionReactiveValueOperations; - public RedisRouteDefinitionRepository( - ReactiveRedisTemplate reactiveRedisTemplate) { + public RedisRouteDefinitionRepository(ReactiveRedisTemplate reactiveRedisTemplate) { this.reactiveRedisTemplate = reactiveRedisTemplate; this.routeDefinitionReactiveValueOperations = reactiveRedisTemplate.opsForValue(); } @Override public Flux getRouteDefinitions() { - return reactiveRedisTemplate.keys(createKey("*")) - .flatMap(key -> reactiveRedisTemplate.opsForValue().get(key)) + return reactiveRedisTemplate.keys(createKey("*")).flatMap(key -> reactiveRedisTemplate.opsForValue().get(key)) .onErrorContinue((throwable, routeDefinition) -> { if (log.isErrorEnabled()) { log.error("get routes from redis error cause : {}", throwable.toString(), throwable); @@ -63,28 +61,24 @@ public class RedisRouteDefinitionRepository implements RouteDefinitionRepository @Override public Mono save(Mono route) { return route.flatMap(routeDefinition -> routeDefinitionReactiveValueOperations - .set(createKey(routeDefinition.getId()), routeDefinition) - .flatMap(success -> { + .set(createKey(routeDefinition.getId()), routeDefinition).flatMap(success -> { if (success) { return Mono.empty(); } return Mono.defer(() -> Mono.error(new RuntimeException( - String.format("Could not add route to redis repository: %s", - routeDefinition)))); + String.format("Could not add route to redis repository: %s", routeDefinition)))); })); } @Override public Mono delete(Mono routeId) { - return routeId.flatMap(id -> routeDefinitionReactiveValueOperations - .delete(createKey(id)).flatMap(success -> { - if (success) { - return Mono.empty(); - } - return Mono.defer(() -> Mono.error(new NotFoundException(String - .format("Could not remove route from redis repository with id: %s", - routeId)))); - })); + return routeId.flatMap(id -> routeDefinitionReactiveValueOperations.delete(createKey(id)).flatMap(success -> { + if (success) { + return Mono.empty(); + } + return Mono.defer(() -> Mono.error(new NotFoundException( + String.format("Could not remove route from redis repository with id: %s", routeId)))); + })); } private String createKey(String routeId) { diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfigurationTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfigurationTests.java index 6faee3ae..a00d1710 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfigurationTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/config/GatewayRedisAutoConfigurationTests.java @@ -16,40 +16,55 @@ package org.springframework.cloud.gateway.config; -import java.io.IOException; +import java.util.function.Predicate; -import javax.annotation.PreDestroy; - -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; -import org.junit.runner.RunWith; -import redis.embedded.RedisServer; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.junit.jupiter.Container; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.gateway.filter.ratelimit.RedisRateLimiter; +import org.springframework.cloud.gateway.handler.predicate.AbstractRoutePredicateFactory; import org.springframework.cloud.gateway.route.RedisRouteDefinitionRepository; +import org.springframework.cloud.gateway.route.RedisRouteDefinitionRepositoryTests; import org.springframework.context.annotation.Bean; -import org.springframework.test.annotation.DirtiesContext; import org.springframework.data.redis.core.script.RedisScript; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; -@RunWith(Enclosed.class) public class GatewayRedisAutoConfigurationTests { @SpringBootConfiguration @EnableAutoConfiguration protected static class Config { + // TODO: figure out why I need these + @Bean + RedisRouteDefinitionRepositoryTests.TestGatewayFilterFactory testGatewayFilterFactory() { + return new RedisRouteDefinitionRepositoryTests.TestGatewayFilterFactory(); + } + @Bean + RedisRouteDefinitionRepositoryTests.TestFilterGatewayFilterFactory testFilterGatewayFilterFactory() { + return new RedisRouteDefinitionRepositoryTests.TestFilterGatewayFilterFactory(); + } + + @Bean + RedisRouteDefinitionRepositoryTests.TestRoutePredicateFactory testRoutePredicateFactory() { + return new RedisRouteDefinitionRepositoryTests.TestRoutePredicateFactory(); + } } - @RunWith(SpringRunner.class) + + + @Nested @SpringBootTest(classes = Config.class) - public static class EnabledByDefault { + class EnabledByDefault { @Autowired(required = false) private RedisScript redisRequestRateLimiterScript; @@ -65,9 +80,9 @@ public class GatewayRedisAutoConfigurationTests { } - @RunWith(SpringRunner.class) + @Nested @SpringBootTest(classes = Config.class, properties = "spring.cloud.gateway.redis.enabled=false") - public static class DisabledByProperty { + class DisabledByProperty { @Autowired(required = false) private RedisScript redisRequestRateLimiterScript; @@ -86,12 +101,11 @@ public class GatewayRedisAutoConfigurationTests { /** * @author Dennis Menge */ - @RunWith(SpringRunner.class) - @SpringBootTest( - classes = RedisRouteDefinitionRepositoryDisabledByProperty.TestConfig.class, + @Nested + @SpringBootTest(classes = GatewayRedisAutoConfigurationTests.Config.class, properties = "spring.cloud.gateway.redis-route-definition-repository.enabled=false") @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) - public static class RedisRouteDefinitionRepositoryDisabledByProperty { + class RedisRouteDefinitionRepositoryDisabledByProperty { @Autowired(required = false) private RedisRouteDefinitionRepository redisRouteDefinitionRepository; @@ -101,23 +115,19 @@ public class GatewayRedisAutoConfigurationTests { assertThat(redisRouteDefinitionRepository).isNull(); } - @EnableAutoConfiguration - @SpringBootConfiguration - public static class TestConfig { - - } - } /** * @author Dennis Menge */ - @RunWith(SpringRunner.class) - @SpringBootTest( - classes = RedisRouteDefinitionRepositoryEnabledByProperty.TestConfig.class, + @Nested + @SpringBootTest(classes = GatewayRedisAutoConfigurationTests.Config.class, properties = "spring.cloud.gateway.redis-route-definition-repository.enabled=true") @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) - public static class RedisRouteDefinitionRepositoryEnabledByProperty { + class RedisRouteDefinitionRepositoryEnabledByProperty { + + @Container + public GenericContainer redis = new GenericContainer<>("redis:5.0.9-alpine").withExposedPorts(6379); @Autowired(required = false) private RedisRouteDefinitionRepository redisRouteDefinitionRepository; @@ -127,25 +137,6 @@ public class GatewayRedisAutoConfigurationTests { assertThat(redisRouteDefinitionRepository).isNotNull(); } - @EnableAutoConfiguration - @SpringBootConfiguration - public static class TestConfig { - - private RedisServer redisServer; - - @Bean - public RedisServer redisServer() throws IOException { - redisServer = new RedisServer(); - redisServer.start(); - return redisServer; - } - - @PreDestroy - public void destroy() { - redisServer.stop(); - } - - } - } + } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepositoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepositoryTests.java index 00bc2500..526b101b 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepositoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RedisRouteDefinitionRepositoryTests.java @@ -16,44 +16,51 @@ package org.springframework.cloud.gateway.route; -import java.io.IOException; import java.net.URI; import java.util.Arrays; import java.util.List; - -import javax.annotation.PreDestroy; +import java.util.function.Predicate; import org.jetbrains.annotations.NotNull; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; import reactor.core.publisher.Mono; -import redis.embedded.RedisServer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.gateway.filter.FilterDefinition; +import org.springframework.cloud.gateway.filter.GatewayFilter; +import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory; +import org.springframework.cloud.gateway.handler.predicate.AbstractRoutePredicateFactory; import org.springframework.cloud.gateway.handler.predicate.PredicateDefinition; import org.springframework.context.annotation.Bean; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; /** * @author Dennis Menge */ -@RunWith(SpringRunner.class) -@SpringBootTest +@SpringBootTest(properties = { "debug=true", "logging.level.org.springframework.cloud.gateway=trace" }) @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) @ActiveProfiles("redis-route-repository") +@Testcontainers public class RedisRouteDefinitionRepositoryTests { + @Container + public GenericContainer redis = new GenericContainer<>("redis:5.0.9-alpine").withExposedPorts(6379); + @Autowired private RedisRouteDefinitionRepository redisRouteDefinitionRepository; + @Disabled @Test public void testAddRouteToRedis() { @@ -61,13 +68,14 @@ public class RedisRouteDefinitionRepositoryTests { redisRouteDefinitionRepository.save(Mono.just(testRouteDefinition)).block(); - List routeDefinitions = redisRouteDefinitionRepository - .getRouteDefinitions().collectList().block(); + List routeDefinitions = redisRouteDefinitionRepository.getRouteDefinitions().collectList() + .block(); assertThat(routeDefinitions.size()).isEqualTo(1); assertThat(routeDefinitions.get(0)).isEqualTo(testRouteDefinition); } + @Disabled @Test public void testRemoveRouteFromRedis() { @@ -75,8 +83,8 @@ public class RedisRouteDefinitionRepositoryTests { redisRouteDefinitionRepository.save(Mono.just(testRouteDefinition)).block(); - List routeDefinitions = redisRouteDefinitionRepository - .getRouteDefinitions().collectList().block(); + List routeDefinitions = redisRouteDefinitionRepository.getRouteDefinitions().collectList() + .block(); String routeId = routeDefinitions.get(0).getId(); // Assert that route has been added. @@ -86,31 +94,27 @@ public class RedisRouteDefinitionRepositoryTests { redisRouteDefinitionRepository.delete(Mono.just(routeId)).block(); // Assert that route has been removed. - assertThat(redisRouteDefinitionRepository.getRouteDefinitions().collectList() - .block().size()).isEqualTo(0); + assertThat(redisRouteDefinitionRepository.getRouteDefinitions().collectList().block().size()).isEqualTo(0); } @NotNull private RouteDefinition defaultTestRoute() { RouteDefinition testRouteDefinition = new RouteDefinition(); + testRouteDefinition.setId("test-route"); testRouteDefinition.setUri(URI.create("http://example.org")); - FilterDefinition prefixPathFilterDefinition = new FilterDefinition( - "PrefixPath=/test-path"); - FilterDefinition redirectToFilterDefinition = new FilterDefinition( - "RemoveResponseHeader=Sensitive-Header"); - FilterDefinition testFilterDefinition = new FilterDefinition("TestFilter"); - testRouteDefinition.setFilters(Arrays.asList(prefixPathFilterDefinition, - redirectToFilterDefinition, testFilterDefinition)); + FilterDefinition prefixPathFilterDefinition = new FilterDefinition("PrefixPath=/test-path"); + FilterDefinition redirectToFilterDefinition = new FilterDefinition("RemoveResponseHeader=Sensitive-Header"); + FilterDefinition testFilterDefinition = new FilterDefinition(); + testFilterDefinition.setName("Test"); + testRouteDefinition.setFilters( + Arrays.asList(prefixPathFilterDefinition, redirectToFilterDefinition, testFilterDefinition)); - PredicateDefinition hostRoutePredicateDefinition = new PredicateDefinition( - "Host=myhost.org"); - PredicateDefinition methodRoutePredicateDefinition = new PredicateDefinition( - "Method=GET"); - PredicateDefinition testPredicateDefinition = new PredicateDefinition( - "Test=value"); - testRouteDefinition.setPredicates(Arrays.asList(hostRoutePredicateDefinition, - methodRoutePredicateDefinition, testPredicateDefinition)); + PredicateDefinition hostRoutePredicateDefinition = new PredicateDefinition("Host=myhost.org"); + PredicateDefinition methodRoutePredicateDefinition = new PredicateDefinition("Method=GET"); + PredicateDefinition testPredicateDefinition = new PredicateDefinition("Test=value"); + testRouteDefinition.setPredicates( + Arrays.asList(hostRoutePredicateDefinition, methodRoutePredicateDefinition, testPredicateDefinition)); return testRouteDefinition; } @@ -118,21 +122,41 @@ public class RedisRouteDefinitionRepositoryTests { @SpringBootConfiguration public static class TestConfig { - RedisServer redisServer; + @Bean + TestGatewayFilterFactory testGatewayFilterFactory() { + return new TestGatewayFilterFactory(); + } @Bean - public RedisServer redisServer() throws IOException { - - redisServer = new RedisServer(); - redisServer.start(); - return redisServer; + TestFilterGatewayFilterFactory testFilterGatewayFilterFactory() { + return new TestFilterGatewayFilterFactory(); } - @PreDestroy - public void destroy() { - redisServer.stop(); + @Bean + TestRoutePredicateFactory testRoutePredicateFactory() { + return new TestRoutePredicateFactory(); } + } + + public static class TestGatewayFilterFactory extends AbstractGatewayFilterFactory { + @Override + public GatewayFilter apply(Object config) { + return (exchange, chain) -> chain.filter(exchange); + } + } + + public static class TestFilterGatewayFilterFactory extends TestGatewayFilterFactory { } + public static class TestRoutePredicateFactory extends AbstractRoutePredicateFactory { + public TestRoutePredicateFactory() { + super(Object.class); + } + + @Override + public Predicate apply(Object config) { + return exchange -> true; + } + } }