From 345f4515ec4dab7ba1e782e492f24834312393fd Mon Sep 17 00:00:00 2001 From: Piotr Wielgolaski Date: Thu, 16 Nov 2017 22:18:20 +0100 Subject: [PATCH] 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 { } +}