From e556ccc7ed490e7d6fc12b0c4bb23eb3524e2522 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Fri, 15 May 2020 18:24:49 -0400 Subject: [PATCH] Moves DiscoveryClientOptionalArgsConfiguration to auto-configuration. Throws an error if eureka.client.webclient.enabled is true and webflux is not on the classpath. --- .../eureka/EurekaClientAutoConfiguration.java | 4 +- ...coveryClientOptionalArgsConfiguration.java | 21 +++++- .../main/resources/META-INF/spring.factories | 1 + .../EurekaClientAutoConfigurationTests.java | 10 +-- ...ptionalArgsConfigurationNoWebfluxTest.java | 64 +++++++++++++++++ ...pClientsOptionalArgsConfigurationTest.java | 68 +++++++------------ ...tiveDiscoveryClientConfigurationTests.java | 2 + ...bonClientPreprocessorIntegrationTests.java | 2 + 8 files changed, 118 insertions(+), 54 deletions(-) create mode 100644 spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationNoWebfluxTest.java diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index 3b7dad1c8..3850358b6 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -55,7 +55,6 @@ import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationP import org.springframework.cloud.client.serviceregistry.ServiceRegistryAutoConfiguration; import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.context.scope.refresh.RefreshScope; -import org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration; import org.springframework.cloud.netflix.eureka.metadata.DefaultManagementMetadataProvider; import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadata; import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadataProvider; @@ -67,7 +66,6 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Lazy; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.util.StringUtils; @@ -87,12 +85,12 @@ import static org.springframework.cloud.commons.util.IdUtils.getDefaultInstanceI @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties @ConditionalOnClass(EurekaClientConfig.class) -@Import(DiscoveryClientOptionalArgsConfiguration.class) @ConditionalOnProperty(value = "eureka.client.enabled", matchIfMissing = true) @ConditionalOnDiscoveryEnabled @AutoConfigureBefore({ NoopDiscoveryClientAutoConfiguration.class, CommonsClientAutoConfiguration.class, ServiceRegistryAutoConfiguration.class }) @AutoConfigureAfter(name = { + "org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration", "org.springframework.cloud.autoconfigure.RefreshAutoConfiguration", "org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration", "org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationAutoConfiguration" }) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientOptionalArgsConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientOptionalArgsConfiguration.java index 4c3bf3fcb..be5d14cc7 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientOptionalArgsConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientOptionalArgsConfiguration.java @@ -43,7 +43,7 @@ public class DiscoveryClientOptionalArgsConfiguration { @ConditionalOnMissingClass("com.sun.jersey.api.client.filter.ClientFilter") @ConditionalOnMissingBean(value = { AbstractDiscoveryClientOptionalArgs.class }, search = SearchStrategy.CURRENT) - @ConditionalOnProperty(prefix = "eureka.client", name = { "webclient.enabled" }, + @ConditionalOnProperty(prefix = "eureka.client", name = "webclient.enabled", matchIfMissing = true, havingValue = "false") public RestTemplateDiscoveryClientOptionalArgs restTemplateDiscoveryClientOptionalArgs() { logger.info("Eureka HTTP Client uses RestTemplate."); @@ -52,11 +52,13 @@ public class DiscoveryClientOptionalArgsConfiguration { @Bean @ConditionalOnMissingClass("com.sun.jersey.api.client.filter.ClientFilter") + @ConditionalOnClass( + name = "org.springframework.web.reactive.function.client.WebClient") @ConditionalOnMissingBean( value = { AbstractDiscoveryClientOptionalArgs.class, RestTemplateDiscoveryClientOptionalArgs.class }, search = SearchStrategy.CURRENT) - @ConditionalOnProperty(prefix = "eureka.client", name = { "webclient.enabled" }, + @ConditionalOnProperty(prefix = "eureka.client", name = "webclient.enabled", havingValue = "true") public WebClientDiscoveryClientOptionalArgs webClientDiscoveryClientOptionalArgs() { logger.info("Eureka HTTP Client uses WebClient."); @@ -71,4 +73,19 @@ public class DiscoveryClientOptionalArgsConfiguration { return new MutableDiscoveryClientOptionalArgs(); } + @Configuration + @ConditionalOnMissingClass({ "com.sun.jersey.api.client.filter.ClientFilter", + "org.springframework.web.reactive.function.client.WebClient" }) + @ConditionalOnProperty(prefix = "eureka.client", name = "webclient.enabled", + havingValue = "true") + protected static class WebClientNotFoundConfiguration { + + public WebClientNotFoundConfiguration() { + throw new IllegalStateException("eureka.client.webclient.enabled is true, " + + "but WebClient is not on the classpath. Please add " + + "spring-boot-starter-webflux as a dependency."); + } + + } + } diff --git a/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/spring.factories b/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/spring.factories index 54839c7e5..674a4f0df 100644 --- a/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/spring.factories @@ -1,5 +1,6 @@ org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ org.springframework.cloud.netflix.eureka.config.EurekaClientConfigServerAutoConfiguration,\ +org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration,\ org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration,\ org.springframework.cloud.netflix.ribbon.eureka.RibbonEurekaAutoConfiguration,\ org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration,\ diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index 0d5e17973..1c67d08e8 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -48,6 +48,7 @@ import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationP import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.context.refresh.ContextRefresher; import org.springframework.cloud.context.scope.GenericScope; +import org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration; import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaServiceRegistry; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -82,6 +83,7 @@ public class EurekaClientAutoConfigurationTests { private void setupContext(Class... config) { ConfigurationPropertySources.attach(this.context.getEnvironment()); this.context.register(PropertyPlaceholderAutoConfiguration.class, + DiscoveryClientOptionalArgsConfiguration.class, EurekaDiscoveryClientConfiguration.class); for (Class value : config) { this.context.register(value); @@ -591,6 +593,7 @@ public class EurekaClientAutoConfigurationTests { public void shouldNotHaveDiscoveryClientWhenBlockingDiscoveryDisabled() { new ApplicationContextRunner() .withConfiguration(AutoConfigurations.of(UtilAutoConfiguration.class, + DiscoveryClientOptionalArgsConfiguration.class, EurekaClientAutoConfiguration.class, EurekaDiscoveryClientConfiguration.class)) .withPropertyValues("spring.cloud.discovery.blocking.enabled=false") @@ -677,13 +680,6 @@ public class EurekaClientAutoConfigurationTests { @Configuration(proxyBeanMethods = false) protected static class MockClientConfiguration { - @Bean - public MutableDiscoveryClientOptionalArgs mutableDiscoveryClientOptionalArgs() { - MutableDiscoveryClientOptionalArgs args = new MutableDiscoveryClientOptionalArgs(); - args.setEurekaJerseyClient(jerseyClient()); - return args; - } - @Bean public EurekaJerseyClient jerseyClient() { EurekaJerseyClient mock = Mockito.mock(EurekaJerseyClient.class); diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationNoWebfluxTest.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationNoWebfluxTest.java new file mode 100644 index 000000000..76d7866fc --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationNoWebfluxTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2017-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.netflix.eureka.config; + +import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.cloud.netflix.eureka.sample.EurekaSampleApplication; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; +import org.springframework.context.ConfigurableApplicationContext; + +import static org.assertj.core.api.AssertionsForClassTypes.fail; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +/** + * @author Daniel Lavoie + */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "jersey-client-*", "jersey-core-*", "jersey-apache-client4-*", + "spring-webflux-*" }) +@SpringBootTest(classes = EurekaSampleApplication.class, + webEnvironment = WebEnvironment.RANDOM_PORT) +public class EurekaHttpClientsOptionalArgsConfigurationNoWebfluxTest { + + @Test + @SuppressWarnings("unchecked") + public void contextFailsWithoutWebClient() { + + ConfigurableApplicationContext ctx = null; + try { + TomcatURLStreamHandlerFactory.disable(); + ctx = new SpringApplicationBuilder(EurekaSampleApplication.class) + .properties("eureka.client.webclient.enabled=true").run(); + fail("exception not thrown"); + } + catch (Exception e) { + // this is the desired state + assertThat(e).hasMessageContaining("WebClient is not on the classpath"); + } + if (ctx != null) { + ctx.close(); + } + } + +} diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationTest.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationTest.java index a4cbbdd82..1fb5babd5 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationTest.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/EurekaHttpClientsOptionalArgsConfigurationTest.java @@ -19,16 +19,14 @@ package org.springframework.cloud.netflix.eureka.config; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.boot.WebApplicationType; -import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.cloud.netflix.eureka.http.RestTemplateDiscoveryClientOptionalArgs; import org.springframework.cloud.netflix.eureka.http.WebClientDiscoveryClientOptionalArgs; import org.springframework.cloud.netflix.eureka.sample.EurekaSampleApplication; import org.springframework.cloud.test.ClassPathExclusions; import org.springframework.cloud.test.ModifiedClassPathRunner; -import org.springframework.context.ConfigurableApplicationContext; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; @@ -43,53 +41,39 @@ public class EurekaHttpClientsOptionalArgsConfigurationTest { @Test public void contextLoadsWithRestTemplate() { - try (ConfigurableApplicationContext context = new SpringApplicationBuilder() - .web(WebApplicationType.NONE).sources(EurekaSampleApplication.class) - .properties(new String[] { "eureka.client.webclient.enabled=false" }) - .run()) { - assertThat(context.getBean(RestTemplateDiscoveryClientOptionalArgs.class)) - .isNotNull(); - try { - Object bean = context.getBean(WebClientDiscoveryClientOptionalArgs.class); - assertThat(bean).isNull(); - } - catch (Exception ex) { - } - } + new WebApplicationContextRunner() + .withUserConfiguration(EurekaSampleApplication.class) + .withPropertyValues("eureka.client.webclient.enabled=false") + .run(context -> { + assertThat(context) + .hasSingleBean(RestTemplateDiscoveryClientOptionalArgs.class); + assertThat(context) + .doesNotHaveBean(WebClientDiscoveryClientOptionalArgs.class); + }); } @Test public void contextLoadsWithWebClient() { - try (ConfigurableApplicationContext context = new SpringApplicationBuilder() - .web(WebApplicationType.NONE).sources(EurekaSampleApplication.class) - .properties(new String[] { "eureka.client.webclient.enabled=true" }) - .run()) { - assertThat(context.getBean(WebClientDiscoveryClientOptionalArgs.class)) - .isNotNull(); - try { - Object bean = context - .getBean(RestTemplateDiscoveryClientOptionalArgs.class); - assertThat(bean).isNull(); - } - catch (Exception ex) { - } - } + new WebApplicationContextRunner() + .withUserConfiguration(EurekaSampleApplication.class) + .withPropertyValues("eureka.client.webclient.enabled=true") + .run(context -> { + assertThat(context).doesNotHaveBean( + RestTemplateDiscoveryClientOptionalArgs.class); + assertThat(context) + .hasSingleBean(WebClientDiscoveryClientOptionalArgs.class); + }); } @Test public void contextLoadsWithRestTemplateAsDefault() { - try (ConfigurableApplicationContext context = new SpringApplicationBuilder() - .web(WebApplicationType.NONE).sources(EurekaSampleApplication.class) - .run()) { - assertThat(context.getBean(RestTemplateDiscoveryClientOptionalArgs.class)) - .isNotNull(); - try { - Object bean = context.getBean(WebClientDiscoveryClientOptionalArgs.class); - assertThat(bean).isNull(); - } - catch (Exception ex) { - } - } + new WebApplicationContextRunner() + .withUserConfiguration(EurekaSampleApplication.class).run(context -> { + assertThat(context) + .hasSingleBean(RestTemplateDiscoveryClientOptionalArgs.class); + assertThat(context) + .doesNotHaveBean(WebClientDiscoveryClientOptionalArgs.class); + }); } } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/reactive/EurekaReactiveDiscoveryClientConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/reactive/EurekaReactiveDiscoveryClientConfigurationTests.java index 4576c1810..8c5cc2b98 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/reactive/EurekaReactiveDiscoveryClientConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/reactive/EurekaReactiveDiscoveryClientConfigurationTests.java @@ -26,6 +26,7 @@ import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient; import org.springframework.cloud.client.discovery.health.reactive.ReactiveDiscoveryClientHealthIndicator; import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration; +import org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration; import static org.assertj.core.api.Assertions.assertThat; @@ -38,6 +39,7 @@ class EurekaReactiveDiscoveryClientConfigurationTests { .withConfiguration(AutoConfigurations.of(UtilAutoConfiguration.class, ReactiveCommonsClientAutoConfiguration.class, EurekaClientAutoConfiguration.class, + DiscoveryClientOptionalArgsConfiguration.class, EurekaReactiveDiscoveryClientConfiguration.class)); @Test diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientPreprocessorIntegrationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientPreprocessorIntegrationTests.java index 21f567652..65e67847e 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientPreprocessorIntegrationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientPreprocessorIntegrationTests.java @@ -30,6 +30,7 @@ import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration; import org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration; +import org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; @@ -81,6 +82,7 @@ public class EurekaRibbonClientPreprocessorIntegrationTests { @RibbonClient("foo") @Import({ UtilAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class, RibbonAutoConfiguration.class, + DiscoveryClientOptionalArgsConfiguration.class, EurekaDiscoveryClientConfiguration.class, EurekaClientAutoConfiguration.class, RibbonEurekaAutoConfiguration.class }) protected static class TestConfiguration {