From 9f8c0dd8ef38bdff72385791b0c4c5878a7bef8e Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 20 Apr 2020 12:53:18 -0400 Subject: [PATCH] Makes disabling service registry by properties consistent. (#634) Fixes gh-619 --- .../ConsulCatalogWatchAutoConfiguration.java | 2 + ...oServiceRegistrationAutoConfiguration.java | 40 +++++++++- ...onsulServiceRegistryAutoConfiguration.java | 26 ++++++- ...itional-spring-configuration-metadata.json | 28 +++++++ ...lAutoServiceRegistrationDisabledTests.java | 72 +++++++++++------- .../ConsulServiceRegistryDisabledTests.java | 73 +++++++++++++++++++ 6 files changed, 209 insertions(+), 32 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/main/resources/META-INF/additional-spring-configuration-metadata.json create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryDisabledTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulCatalogWatchAutoConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulCatalogWatchAutoConfiguration.java index deab32f1..ec5a3190 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulCatalogWatchAutoConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulCatalogWatchAutoConfiguration.java @@ -20,6 +20,7 @@ import com.ecwid.consul.v1.ConsulClient; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.ConditionalOnDiscoveryEnabled; @@ -41,6 +42,7 @@ import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; matchIfMissing = true) @ConditionalOnDiscoveryEnabled @AutoConfigureAfter({ ConsulDiscoveryClientConfiguration.class }) +@ConditionalOnBean(ConsulDiscoveryProperties.class) public class ConsulCatalogWatchAutoConfiguration { /** diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java index 3888503a..40277b48 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java @@ -23,6 +23,7 @@ import javax.servlet.ServletContext; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.condition.AllNestedConditions; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -34,6 +35,7 @@ import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties; import org.springframework.cloud.consul.discovery.HeartbeatProperties; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; /** @@ -44,8 +46,7 @@ import org.springframework.context.annotation.Configuration; @ConditionalOnMissingBean( type = "org.springframework.cloud.consul.discovery.ConsulLifecycle") @ConditionalOnConsulEnabled -@ConditionalOnProperty(value = "spring.cloud.service-registry.auto-registration.enabled", - matchIfMissing = true) +@Conditional(ConsulAutoServiceRegistrationAutoConfiguration.OnConsulRegistrationEnabledCondition.class) @AutoConfigureAfter({ AutoServiceRegistrationConfiguration.class, ConsulServiceRegistryAutoConfiguration.class }) public class ConsulAutoServiceRegistrationAutoConfiguration { @@ -95,4 +96,39 @@ public class ConsulAutoServiceRegistrationAutoConfiguration { } + protected static class OnConsulRegistrationEnabledCondition + extends AllNestedConditions { + + OnConsulRegistrationEnabledCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnProperty( + value = "spring.cloud.service-registry.auto-registration.enabled", + matchIfMissing = true) + static class AutoRegistrationEnabledClass { + + } + + @ConditionalOnProperty( + value = "spring.cloud.consul.service-registry.auto-registration.enabled", + matchIfMissing = true) + static class ConsulAutoRegistrationEnabledClass { + + } + + @ConditionalOnProperty(value = "spring.cloud.service-registry.enabled", + matchIfMissing = true) + static class ServiceRegistryEnabledClass { + + } + + @ConditionalOnProperty(value = "spring.cloud.consul.service-registry.enabled", + matchIfMissing = true) + static class ConsulServiceRegistryEnabledClass { + + } + + } + } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryAutoConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryAutoConfiguration.java index ff1a648f..a33a0426 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryAutoConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryAutoConfiguration.java @@ -20,6 +20,7 @@ import com.ecwid.consul.v1.ConsulClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureBefore; +import org.springframework.boot.autoconfigure.condition.AllNestedConditions; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.serviceregistry.ServiceRegistryAutoConfiguration; @@ -29,6 +30,7 @@ import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties; import org.springframework.cloud.consul.discovery.HeartbeatProperties; import org.springframework.cloud.consul.discovery.TtlScheduler; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; /** @@ -37,8 +39,7 @@ import org.springframework.context.annotation.Configuration; */ @Configuration(proxyBeanMethods = false) @ConditionalOnConsulEnabled -@ConditionalOnProperty(value = "spring.cloud.service-registry.enabled", - matchIfMissing = true) +@Conditional(ConsulServiceRegistryAutoConfiguration.OnConsulRegistrationEnabledCondition.class) @AutoConfigureBefore(ServiceRegistryAutoConfiguration.class) public class ConsulServiceRegistryAutoConfiguration { @@ -64,4 +65,25 @@ public class ConsulServiceRegistryAutoConfiguration { return new ConsulDiscoveryProperties(inetUtils); } + protected static class OnConsulRegistrationEnabledCondition + extends AllNestedConditions { + + OnConsulRegistrationEnabledCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnProperty(value = "spring.cloud.service-registry.enabled", + matchIfMissing = true) + static class ServiceRegistryEnabledClass { + + } + + @ConditionalOnProperty(value = "spring.cloud.consul.service-registry.enabled", + matchIfMissing = true) + static class ConsulServiceRegistryEnabledClass { + + } + + } + } diff --git a/spring-cloud-consul-discovery/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-consul-discovery/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 00000000..648968be --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,28 @@ +{ + "properties": [ + { + "name": "spring.cloud.service-registry.enabled", + "type": "java.lang.Boolean", + "description": "Enables Service Registry functionality.", + "defaultValue": "true" + }, + { + "name": "spring.cloud.consul.service-registry.enabled", + "type": "java.lang.Boolean", + "description": "Enables Consul Service Registry functionality.", + "defaultValue": "true" + }, + { + "name": "spring.cloud.service-registry.auto-registration.enabled", + "type": "java.lang.Boolean", + "description": "Enables Service Registry Auto-registration.", + "defaultValue": "true" + }, + { + "name": "spring.cloud.consul.service-registry.auto-registration.enabled", + "type": "java.lang.Boolean", + "description": "Enables Consul Service Registry Auto-registration.", + "defaultValue": "true" + } + ] +} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDisabledTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDisabledTests.java index 415579be..77e0b739 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDisabledTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDisabledTests.java @@ -22,52 +22,68 @@ import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.Response; import com.ecwid.consul.v1.agent.model.Service; import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.boot.autoconfigure.ImportAutoConfiguration; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationConfiguration; -import org.springframework.cloud.consul.ConsulAutoConfiguration; +import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.context.annotation.Configuration; -import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; -import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; /** * @author Spencer Gibb */ -@RunWith(SpringRunner.class) -@SpringBootTest(classes = ConsulAutoServiceRegistrationDisabledTests.TestConfig.class, - properties = { "spring.application.name=myTestNotRegisteredService2", - "spring.cloud.service-registry.auto-registration.enabled=false" }, - webEnvironment = RANDOM_PORT) public class ConsulAutoServiceRegistrationDisabledTests { - @Autowired - private ConsulClient consul; - - @Autowired(required = false) - private ConsulAutoServiceRegistration autoServiceRegistration; + @Test + public void disabledViaSpringCloudProperty() { + testAutoRegistrationDisabled("myTestNotRegisteredService2", + "spring.cloud.service-registry.auto-registration.enabled"); + } @Test - public void contextLoads() { - assertThat(this.autoServiceRegistration) - .as("ConsulAutoServiceRegistration was created").isNull(); + public void disabledViaConsulProperty() { + testAutoRegistrationDisabled("myTestNotRegisteredService3", + "spring.cloud.consul.service-registry.auto-registration.enabled"); + } - Response> response = this.consul.getAgentServices(); - Map services = response.getValue(); - Service service = services.get("myTestNotRegisteredService2"); - assertThat(service).as("service was registered").isNull(); + @Test + public void disabledViaSpringCloudServiceRegistryProperty() { + testAutoRegistrationDisabled("myTestNotRegisteredService4", + "spring.cloud.service-registry.enabled"); + } + + @Test + public void disabledViaConsulServiceRegistryProperty() { + testAutoRegistrationDisabled("myTestNotRegisteredService5", + "spring.cloud.consul.service-registry.enabled"); + } + + private void testAutoRegistrationDisabled(String testName, String disableProperty) { + new WebApplicationContextRunner().withUserConfiguration(TestConfig.class) + .withPropertyValues("spring.application.name=" + testName, + disableProperty + "=false", "server.port=0") + .run(context -> { + + assertThat(context) + .doesNotHaveBean(ConsulAutoServiceRegistration.class); + assertThat(context) + .doesNotHaveBean(ConsulAutoServiceRegistrationListener.class); + assertThat(context).doesNotHaveBean(ConsulAutoRegistration.class); + assertThat(context) + .doesNotHaveBean(ConsulRegistrationCustomizer.class); + + ConsulClient consul = context.getBean(ConsulClient.class); + + Response> response = consul.getAgentServices(); + Map services = response.getValue(); + Service service = services.get(testName); + assertThat(service).as("service was registered").isNull(); + + }); } @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration - @ImportAutoConfiguration({ AutoServiceRegistrationConfiguration.class, - ConsulAutoConfiguration.class, - ConsulAutoServiceRegistrationAutoConfiguration.class }) public static class TestConfig { } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryDisabledTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryDisabledTests.java new file mode 100644 index 00000000..a54c996a --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryDisabledTests.java @@ -0,0 +1,73 @@ +/* + * Copyright 2013-2019 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.consul.serviceregistry; + +import java.util.Map; + +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.agent.model.Service; +import org.junit.Test; + +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.runner.WebApplicationContextRunner; +import org.springframework.cloud.consul.discovery.HeartbeatProperties; +import org.springframework.context.annotation.Configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Spencer Gibb + */ +public class ConsulServiceRegistryDisabledTests { + + @Test + public void disabledViaSpringCloudProperty() { + testAutoRegistrationDisabled("serviceRegistryDisabledTest1", + "spring.cloud.service-registry.enabled"); + } + + @Test + public void disabledViaConsulProperty() { + testAutoRegistrationDisabled("serviceRegistryDisabledTest2", + "spring.cloud.consul.service-registry.enabled"); + } + + private void testAutoRegistrationDisabled(String testName, String disableProperty) { + new WebApplicationContextRunner().withUserConfiguration(TestConfig.class) + .withPropertyValues("spring.application.name=" + testName, + disableProperty + "=false", "server.port=0") + .run(context -> { + assertThat(context).doesNotHaveBean(ConsulServiceRegistry.class); + assertThat(context).doesNotHaveBean(HeartbeatProperties.class); + + ConsulClient consul = context.getBean(ConsulClient.class); + + Response> response = consul.getAgentServices(); + Map services = response.getValue(); + Service service = services.get(testName); + assertThat(service).as("service was registered").isNull(); + }); + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + public static class TestConfig { + + } + +}