From 28aa587aa4dd30ddaaec6ed87a127ee3cbacc0a1 Mon Sep 17 00:00:00 2001 From: reta Date: Wed, 27 Feb 2019 09:05:22 -0500 Subject: [PATCH] Addressing code review comments, made assertions more strict --- .../web/SkipPatternProviderConfigTest.java | 210 ++++++++---------- 1 file changed, 90 insertions(+), 120 deletions(-) diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java index 864010504..45495e65b 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProviderConfigTest.java @@ -17,9 +17,11 @@ package org.springframework.cloud.sleuth.instrument.web; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Optional; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.junit.Test; @@ -34,13 +36,13 @@ import org.springframework.boot.actuate.autoconfigure.web.server.ManagementServe import org.springframework.boot.actuate.endpoint.EndpointsSupplier; import org.springframework.boot.actuate.endpoint.web.ExposableWebEndpoint; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration; import org.springframework.boot.context.annotation.UserConfigurations; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.BDDAssertions.then; @@ -51,17 +53,18 @@ import static org.assertj.core.api.BDDAssertions.then; public class SkipPatternProviderConfigTest { private final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(TraceAutoConfiguration.class, + .withConfiguration(AutoConfigurations.of(DispatcherServletAutoConfiguration.class, + InfoEndpointAutoConfiguration.class, HealthIndicatorAutoConfiguration.class, + HealthEndpointAutoConfiguration.class, EndpointAutoConfiguration.class, + WebEndpointAutoConfiguration.class, TraceAutoConfiguration.class, TraceWebAutoConfiguration.class)); @Test public void should_pick_skip_pattern_from_sleuth_properties() throws Exception { contextRunner.withPropertyValues("spring.sleuth.web.skip-pattern=foo.*|bar.*") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()).isEqualTo("foo.*|bar.*"); + final String pattern = extractPattern(context); + then(pattern).isEqualTo("foo.*|bar.*"); }); } @@ -71,10 +74,8 @@ public class SkipPatternProviderConfigTest { .withPropertyValues("spring.sleuth.web.skip-pattern=foo.*|bar.*", "spring.sleuth.web.additional-skip-pattern=baz.*|faz.*") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()).isEqualTo("foo.*|bar.*|baz.*|faz.*"); + final String pattern = extractPattern(context); + then(pattern).isEqualTo("foo.*|bar.*|baz.*|faz.*"); }); } @@ -93,13 +94,12 @@ public class SkipPatternProviderConfigTest { public void should_return_management_context_with_context_path() throws Exception { contextRunner .withConfiguration(UserConfigurations.of( - ManagementContextAutoConfiguration.class, EndpointConfig.class)) + ManagementContextAutoConfiguration.class, ServerPropertiesConfig.class)) .withPropertyValues("management.server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()).contains("|foo.*"); + final String pattern = extractPattern(context); + then(pattern).contains("|foo.*"); + then(extractAllPatterns(context)).contains("foo.*"); }); } @@ -116,162 +116,114 @@ public class SkipPatternProviderConfigTest { @Test public void should_return_endpoints_without_context_path() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("/actuator/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/actuator/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/actuator/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_context_path() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("foo/actuator/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("foo/actuator/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("foo/actuator/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_without_context_path_and_base_path_set_to_root() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.endpoints.web.base-path=/") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()).contains("/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_context_path_and_base_path_set_to_root() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.endpoints.web.base-path=/", + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("foo/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("foo/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("foo/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_context_path_and_base_path_set_to_root_different_port() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.endpoints.web.base-path=/", - "management.servet.port=0", "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/", + "management.server.port=0", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()).contains("/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_actuator_context_path_only() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.endpoints.web.base-path=/mgt", + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/mgt", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("foo/mgt/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("foo/mgt/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("foo/mgt/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_actuator_default_context_path() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("foo/actuator/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("foo/actuator/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("foo/actuator/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_actuator_default_context_path_different_port() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.servet.port=0", "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.server.port=0", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("/actuator/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/actuator/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/actuator/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_actuator_context_path_only_different_port() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.endpoints.web.base-path=/mgt", - "management.servet.port=0", "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.endpoints.web.base-path=/mgt", + "management.server.port=0", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("/mgt/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/mgt/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/mgt/(health|health/.*|info|info/.*)"); }); } @Test public void should_return_endpoints_with_context_path_different_port() { - contextRunner.withConfiguration(UserConfigurations.of(EndpointConfig.class)) - .withPropertyValues("management.endpoint.health.enabled=true", - "management.endpoint.info.enabled=true", - "management.endpoints.web.exposure.include=info,health", - "management.servet.port=0", "server.servlet.context-path=foo") + contextRunner.withConfiguration(UserConfigurations.of(ServerPropertiesConfig.class)) + .withPropertyValues("management.server.port=0", "server.servlet.context-path=foo") .run(context -> { - SkipPatternProvider skipPatternProvider = context - .getBean(SkipPatternProvider.class); - Pattern pattern = skipPatternProvider.skipPattern(); - then(pattern.pattern()) - .contains("/actuator/(health|health/.*|info|info/.*)"); + final String pattern = extractPattern(context); + then(pattern).contains("/actuator/(health|health/.*|info|info/.*)"); + then(extractAllPatterns(context)).contains("/actuator/(health|health/.*|info|info/.*)"); }); } @@ -292,14 +244,32 @@ public class SkipPatternProviderConfigTest { private SingleSkipPattern bar() { return () -> Optional.of(Pattern.compile("bar")); } + + /** + * Extracts the patterns from pattern provider + */ + private String extractPattern(ApplicationContext context) { + SkipPatternProvider skipPatternProvider = context.getBean(SkipPatternProvider.class); + return skipPatternProvider.skipPattern().pattern(); + } + + /** + * Extracts all single patterns + */ + private Collection extractAllPatterns(ApplicationContext context) { + return context + .getBeansOfType(SingleSkipPattern.class) + .values() + .stream() + .map(SingleSkipPattern::skipPattern) + .filter(Optional::isPresent) + .map(Optional::get) + .map(Pattern::pattern) + .collect(Collectors.toList()); + } @Configuration - @ImportAutoConfiguration({ DispatcherServletAutoConfiguration.class, - InfoEndpointAutoConfiguration.class, HealthIndicatorAutoConfiguration.class, - HealthEndpointAutoConfiguration.class, EndpointAutoConfiguration.class, - WebEndpointAutoConfiguration.class }) @EnableConfigurationProperties(ServerProperties.class) - static class EndpointConfig { - + static class ServerPropertiesConfig { } }