From a8ef24246ff6f30e9c697b5f0e56f95d9fba4ad3 Mon Sep 17 00:00:00 2001 From: Preetha Date: Thu, 16 Nov 2017 13:42:19 -0600 Subject: [PATCH 1/3] Fix ConsulHealthIndicator to stop reading internal config values from v1/agent/self and switch to a more stable API (#370) --- .../cloud/consul/ConsulHealthIndicator.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java b/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java index 314aac95..476bbd07 100644 --- a/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java +++ b/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java @@ -41,19 +41,13 @@ public class ConsulHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Health.Builder builder) throws Exception { - try { - Response self = consul.getAgentSelf(); - Config config = self.getValue().getConfig(); + try { + Response leaderStatus = consul.getStatusLeader(); Response>> services = consul .getCatalogServices(QueryParams.DEFAULT); builder.up() - .withDetail("services", services.getValue()) - .withDetail("advertiseAddress", config.getAdvertiseAddress()) - .withDetail("datacenter", config.getDatacenter()) - .withDetail("domain", config.getDomain()) - .withDetail("nodeName", config.getNodeName()) - .withDetail("bindAddress", config.getBindAddress()) - .withDetail("clientAddress", config.getClientAddress()); + .withDetail("leader", leaderStatus.getValue()) + .withDetail("services", services.getValue()); } catch (Exception e) { builder.down(e); From 345f4515ec4dab7ba1e782e492f24834312393fd Mon Sep 17 00:00:00 2001 From: Piotr Wielgolaski Date: Thu, 16 Nov 2017 22:18:20 +0100 Subject: [PATCH 2/3] Extract servlet logic to be included conditionally only when ServletContext is available in classpath (#372) fixes gh-314 --- .../consul/discovery/ConsulLifecycle.java | 9 ++- .../ConsulAutoRegistration.java | 27 +++---- ...oServiceRegistrationAutoConfiguration.java | 17 ++++- .../ConsulRegistrationCustomizer.java | 26 +++++++ .../ConsulServletRegistrationCustomizer.java | 42 +++++++++++ ...strationCustomizedServletContextTests.java | 72 +++++++++++++++++++ 6 files changed, 177 insertions(+), 16 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java create mode 100644 spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java index cfe8e152..388cf269 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java @@ -18,10 +18,15 @@ package org.springframework.cloud.consul.discovery; import javax.servlet.ServletContext; +import java.util.Collections; +import java.util.List; + import org.springframework.beans.BeansException; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle; import org.springframework.cloud.consul.serviceregistry.ConsulAutoRegistration; +import org.springframework.cloud.consul.serviceregistry.ConsulRegistrationCustomizer; +import org.springframework.cloud.consul.serviceregistry.ConsulServletRegistrationCustomizer; import org.springframework.context.ApplicationContext; import org.springframework.retry.annotation.Retryable; import org.springframework.util.Assert; @@ -108,8 +113,10 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { return; } Assert.notNull(service.getPort(), "service.port has not been set"); + List registrationCustomizers = Collections + .singletonList(new ConsulServletRegistrationCustomizer(servletContext)); ConsulAutoRegistration registration = ConsulAutoRegistration.lifecycleRegistration(service.getPort(), - getServiceId(), this.properties, getContext(), this.servletContext, this.ttlConfig); + getServiceId(), this.properties, getContext(), registrationCustomizers, this.ttlConfig); if (registration.getService().getPort() == null) { // not set by properties registration.initializePort(service.getPort()); } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java index 11d52045..fe73a77f 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java @@ -19,8 +19,6 @@ package org.springframework.cloud.consul.serviceregistry; import java.util.LinkedList; import java.util.List; -import javax.servlet.ServletContext; - import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.cloud.client.discovery.ManagementServerPortUtils; import org.springframework.cloud.client.serviceregistry.ServiceRegistry; @@ -64,7 +62,8 @@ public class ConsulAutoRegistration extends ConsulRegistration { } public static ConsulAutoRegistration registration(ConsulDiscoveryProperties properties, ApplicationContext context, - ServletContext servletContext, HeartbeatProperties heartbeatProperties) { + List registrationCustomizers, + HeartbeatProperties heartbeatProperties) { RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); NewService service = new NewService(); @@ -74,7 +73,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { service.setAddress(properties.getHostname()); } service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, servletContext)); + service.setTags(createTags(properties, registrationCustomizers)); if (properties.getPort() != null) { service.setPort(properties.getPort()); @@ -87,7 +86,8 @@ public class ConsulAutoRegistration extends ConsulRegistration { @Deprecated //TODO: do I need this here, or should I just copy what I need back into lifecycle? public static ConsulAutoRegistration lifecycleRegistration(Integer port, String instanceId, ConsulDiscoveryProperties properties, ApplicationContext context, - ServletContext servletContext, HeartbeatProperties heartbeatProperties) { + List registrationCustomizers, + HeartbeatProperties heartbeatProperties) { RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); NewService service = new NewService(); @@ -97,7 +97,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { service.setAddress(properties.getHostname()); } service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, servletContext)); + service.setTags(createTags(properties, registrationCustomizers)); // If an alternate external port is specified, register using it instead if (properties.getPort() != null) { @@ -173,20 +173,21 @@ public class ConsulAutoRegistration extends ConsulRegistration { return normalized.toString(); } - - public static List createTags(ConsulDiscoveryProperties properties, ServletContext servletContext) { + public static List createTags(ConsulDiscoveryProperties properties, + List registrationCustomizers) { List tags = new LinkedList<>(properties.getTags()); - if(servletContext != null - && StringUtils.hasText(servletContext.getContextPath()) - && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { - tags.add("contextPath=" + servletContext.getContextPath()); - } + if (!StringUtils.isEmpty(properties.getInstanceZone())) { tags.add(properties.getDefaultZoneMetadataName() + "=" + properties.getInstanceZone()); } if (!StringUtils.isEmpty(properties.getInstanceGroup())) { tags.add("group=" + properties.getInstanceGroup()); } + if (registrationCustomizers != null) { + for (ConsulRegistrationCustomizer customizer : registrationCustomizers) { + customizer.customizeTags(tags); + } + } return tags; } 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 08de5812..c083d75f 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 @@ -17,9 +17,12 @@ package org.springframework.cloud.consul.serviceregistry; import javax.servlet.ServletContext; +import java.util.List; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationProperties; @@ -50,8 +53,18 @@ public class ConsulAutoServiceRegistrationAutoConfiguration { @Bean @ConditionalOnMissingBean public ConsulAutoRegistration consulRegistration(ConsulDiscoveryProperties properties, ApplicationContext applicationContext, - ServletContext servletContext, HeartbeatProperties heartbeatProperties) { - return ConsulAutoRegistration.registration(properties, applicationContext, servletContext, heartbeatProperties); + ObjectProvider> registrationCustomizers, HeartbeatProperties heartbeatProperties) { + return ConsulAutoRegistration.registration(properties, applicationContext, registrationCustomizers.getIfAvailable(), heartbeatProperties); } + @Configuration + @ConditionalOnClass(ServletContext.class) + protected static class ConsulServletConfiguration { + @Bean + public ConsulRegistrationCustomizer servletConsulCustomizer(final ServletContext servletContext) { + return new ConsulServletRegistrationCustomizer(servletContext); + } + } + + } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java new file mode 100644 index 00000000..1e648c9e --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java @@ -0,0 +1,26 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.List; + +/** + * @author Piotr Wielgolaski + */ +public interface ConsulRegistrationCustomizer { + void customizeTags(List tags); +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java new file mode 100644 index 00000000..14196ac2 --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java @@ -0,0 +1,42 @@ +/* + * Copyright 2013-2016 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 + * + * http://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 javax.servlet.ServletContext; +import java.util.List; + +import org.springframework.util.StringUtils; + +/** + * @author Piotr Wielgolaski + */ +public class ConsulServletRegistrationCustomizer implements ConsulRegistrationCustomizer { + private ServletContext servletContext; + + public ConsulServletRegistrationCustomizer(ServletContext servletContext) { + this.servletContext = servletContext; + } + + @Override + public void customizeTags(List tags) { + if(servletContext != null + && StringUtils.hasText(servletContext.getContextPath()) + && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { + tags.add("contextPath=" + servletContext.getContextPath()); + } + } +} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java new file mode 100644 index 00000000..832fc3b0 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java @@ -0,0 +1,72 @@ +/* + * Copyright 2013-2016 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 + * + * http://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 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.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringRunner; + +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.agent.model.Service; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Piotr Wielgolaski + */ +@RunWith(SpringRunner.class) +@SpringBootTest(classes = ConsulAutoServiceRegistrationCustomizedServletContextTests.TestConfig.class, + properties = { "spring.application.name=myTestService-WithServletContext", + "spring.cloud.consul.discovery.instanceId=myTestService1-WithServletContext", + "server.contextPath=/customContext"}, + webEnvironment = RANDOM_PORT) +public class ConsulAutoServiceRegistrationCustomizedServletContextTests { + + @Autowired + private ConsulClient consul; + + @Test + public void contextLoads() { + Response> response = consul.getAgentServices(); + Map services = response.getValue(); + Service service = services.get("myTestService1-WithServletContext"); + assertNotNull("service was null", service); + assertNotEquals("service port is 0", 0, service.getPort().intValue()); + assertEquals("service id was wrong", "myTestService1-WithServletContext", service.getId()); + assertTrue("service context was wrong", service.getTags().contains("contextPath=/customContext")); + } + + + @Configuration + @EnableAutoConfiguration + @ImportAutoConfiguration({ AutoServiceRegistrationConfiguration.class, ConsulAutoConfiguration.class, ConsulAutoServiceRegistrationAutoConfiguration.class }) + public static class TestConfig { } +} From 364e63e2ae2dc70fb4a203c06665b3f7b471b48c Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 16 Nov 2017 22:01:56 -0500 Subject: [PATCH 3/3] Change ConsulRegistrationCustomizer to take a ConsulRegistration --- .../consul/discovery/ConsulLifecycle.java | 5 +- .../TestConsulLifecycleConfiguration.java | 3 +- .../ConsulAutoRegistration.java | 29 +++++--- ...oServiceRegistrationAutoConfiguration.java | 5 +- .../ConsulRegistrationCustomizer.java | 4 +- .../ConsulServletRegistrationCustomizer.java | 25 +++++-- ...strationCustomizedServletContextTests.java | 7 +- ...sulAutoServiceRegistrationNonWebTests.java | 67 +++++++++++++++++++ 8 files changed, 114 insertions(+), 31 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationNonWebTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java index 388cf269..24ffc1c2 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.List; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle; import org.springframework.cloud.consul.serviceregistry.ConsulAutoRegistration; @@ -59,7 +60,7 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { private TtlScheduler ttlScheduler; - private ServletContext servletContext; + private ObjectProvider servletContext; private NewService service = new NewService(); @@ -76,7 +77,7 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { this.ttlScheduler = ttlScheduler; } - public void setServletContext(ServletContext servletContext) { + public void setServletContext(ObjectProvider servletContext) { this.servletContext = servletContext; } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TestConsulLifecycleConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TestConsulLifecycleConfiguration.java index c9cb26c4..53d7a505 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TestConsulLifecycleConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TestConsulLifecycleConfiguration.java @@ -18,6 +18,7 @@ package org.springframework.cloud.consul.discovery; import javax.servlet.ServletContext; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.context.annotation.Bean; @@ -37,7 +38,7 @@ public class TestConsulLifecycleConfiguration { private TtlScheduler ttlScheduler; @Autowired(required = false) - private ServletContext servletContext; + private ObjectProvider servletContext; @Bean public ConsulLifecycle consulLifecycle(ConsulClient consulClient, ConsulDiscoveryProperties discoveryProperties, diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java index fe73a77f..3aa2900f 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java @@ -73,7 +73,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { service.setAddress(properties.getHostname()); } service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, registrationCustomizers)); + service.setTags(createTags(properties)); if (properties.getPort() != null) { service.setPort(properties.getPort()); @@ -81,7 +81,17 @@ public class ConsulAutoRegistration extends ConsulRegistration { setCheck(service, properties, context, heartbeatProperties); } - return new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + ConsulAutoRegistration registration = new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + customize(registrationCustomizers, registration); + return registration; + } + + public static void customize(List registrationCustomizers, ConsulAutoRegistration registration) { + if (registrationCustomizers != null) { + for (ConsulRegistrationCustomizer customizer : registrationCustomizers) { + customizer.customize(registration); + } + } } @Deprecated //TODO: do I need this here, or should I just copy what I need back into lifecycle? @@ -97,7 +107,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { service.setAddress(properties.getHostname()); } service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, registrationCustomizers)); + service.setTags(createTags(properties)); // If an alternate external port is specified, register using it instead if (properties.getPort() != null) { @@ -110,7 +120,9 @@ public class ConsulAutoRegistration extends ConsulRegistration { setCheck(service, properties, context, heartbeatProperties); - return new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + ConsulAutoRegistration registration = new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + customize(registrationCustomizers, registration); + return registration; } public static void setCheck(NewService service, ConsulDiscoveryProperties properties, ApplicationContext context, HeartbeatProperties heartbeatProperties) { @@ -173,8 +185,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { return normalized.toString(); } - public static List createTags(ConsulDiscoveryProperties properties, - List registrationCustomizers) { + public static List createTags(ConsulDiscoveryProperties properties) { List tags = new LinkedList<>(properties.getTags()); if (!StringUtils.isEmpty(properties.getInstanceZone())) { @@ -183,11 +194,7 @@ public class ConsulAutoRegistration extends ConsulRegistration { if (!StringUtils.isEmpty(properties.getInstanceGroup())) { tags.add("group=" + properties.getInstanceGroup()); } - if (registrationCustomizers != null) { - for (ConsulRegistrationCustomizer customizer : registrationCustomizers) { - customizer.customizeTags(tags); - } - } + return tags; } 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 c083d75f..8c4bf234 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 @@ -16,9 +16,10 @@ package org.springframework.cloud.consul.serviceregistry; -import javax.servlet.ServletContext; import java.util.List; +import javax.servlet.ServletContext; + import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -61,7 +62,7 @@ public class ConsulAutoServiceRegistrationAutoConfiguration { @ConditionalOnClass(ServletContext.class) protected static class ConsulServletConfiguration { @Bean - public ConsulRegistrationCustomizer servletConsulCustomizer(final ServletContext servletContext) { + public ConsulRegistrationCustomizer servletConsulCustomizer(ObjectProvider servletContext) { return new ConsulServletRegistrationCustomizer(servletContext); } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java index 1e648c9e..2496e313 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistrationCustomizer.java @@ -16,11 +16,9 @@ package org.springframework.cloud.consul.serviceregistry; -import java.util.List; - /** * @author Piotr Wielgolaski */ public interface ConsulRegistrationCustomizer { - void customizeTags(List tags); + void customize(ConsulRegistration registration); } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java index 14196ac2..416df70d 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServletRegistrationCustomizer.java @@ -17,26 +17,37 @@ package org.springframework.cloud.consul.serviceregistry; import javax.servlet.ServletContext; +import java.util.ArrayList; import java.util.List; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.util.StringUtils; /** * @author Piotr Wielgolaski */ public class ConsulServletRegistrationCustomizer implements ConsulRegistrationCustomizer { - private ServletContext servletContext; + private ObjectProvider servletContext; - public ConsulServletRegistrationCustomizer(ServletContext servletContext) { + public ConsulServletRegistrationCustomizer(ObjectProvider servletContext) { this.servletContext = servletContext; } @Override - public void customizeTags(List tags) { - if(servletContext != null - && StringUtils.hasText(servletContext.getContextPath()) - && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { - tags.add("contextPath=" + servletContext.getContextPath()); + public void customize(ConsulRegistration registration) { + if (servletContext == null) { + return; + } + ServletContext sc = servletContext.getIfAvailable(); + if(sc != null + && StringUtils.hasText(sc.getContextPath()) + && StringUtils.hasText(sc.getContextPath().replaceAll("/", ""))) { + List tags = registration.getService().getTags(); + if (tags == null) { + tags = new ArrayList<>(); + } + tags.add("contextPath=" + sc.getContextPath()); + registration.getService().setTags(tags); } } } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java index 832fc3b0..83e7970d 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedServletContextTests.java @@ -22,10 +22,8 @@ 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.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.context.annotation.Configuration; import org.springframework.test.context.junit4.SpringRunner; @@ -64,9 +62,8 @@ public class ConsulAutoServiceRegistrationCustomizedServletContextTests { assertTrue("service context was wrong", service.getTags().contains("contextPath=/customContext")); } - + @EnableDiscoveryClient @Configuration @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/ConsulAutoServiceRegistrationNonWebTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationNonWebTests.java new file mode 100644 index 00000000..e06ddc83 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationNonWebTests.java @@ -0,0 +1,67 @@ +/* + * Copyright 2013-2017 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 + * + * http://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 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.test.context.SpringBootTest; +import org.springframework.cloud.client.discovery.EnableDiscoveryClient; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringRunner; + +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.agent.model.Service; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.NONE; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringRunner.class) +@SpringBootTest(classes = ConsulAutoServiceRegistrationNonWebTests.TestConfig.class, + properties = { "spring.application.name=consulNonWebTest", "server.port=32111" }, + webEnvironment = NONE) +public class ConsulAutoServiceRegistrationNonWebTests { + + @Autowired + private ConsulClient consul; + + @Autowired(required = false) + private ConsulAutoServiceRegistration autoServiceRegistration; + + @Test + public void contextLoads() { + assertNotNull("ConsulAutoServiceRegistration was created", autoServiceRegistration); + + Response> response = consul.getAgentServices(); + Map services = response.getValue(); + Service service = services.get("consulNonWebTest"); + assertNull("service was registered", service); //no port to listen, hence no registration + } + + @EnableDiscoveryClient + @Configuration + @EnableAutoConfiguration + public static class TestConfig { } +} \ No newline at end of file