From 95096cb1c4a5055de7c9e805c943902de8e6b447 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 22 Sep 2015 09:25:05 -0600 Subject: [PATCH] Always use instanceId if present. fixes gh-89 --- .../consul/discovery/ConsulLifecycle.java | 31 ++++---- .../ConsulDiscoveryClientCustomizedTests.java | 78 +++++++++++++++++++ 2 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientCustomizedTests.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 d73e424c..46854d28 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 @@ -16,19 +16,20 @@ package org.springframework.cloud.consul.discovery; +import java.util.LinkedList; +import java.util.List; + +import javax.servlet.ServletContext; + import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.agent.model.NewService; -import org.springframework.util.Assert; - -import javax.servlet.ServletContext; -import java.util.LinkedList; -import java.util.List; /** * @author Spencer Gibb @@ -69,13 +70,7 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { protected void register() { Assert.notNull(service.getPort(), "service.port has not been set"); String appName = getAppName(); - String id; - if (properties.getInstanceId() == null) { - id = getServiceId(); - } else { - id = normalizeForDns(properties.getInstanceId()); - } - service.setId(id); + service.setId(getServiceId()); service.setName(normalizeForDns(appName)); service.setTags(createTags()); @@ -110,7 +105,11 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { } public String getServiceId() { - return normalizeForDns(getContext().getId()); + if (!StringUtils.hasText(properties.getInstanceId())) { + return normalizeForDns(getContext().getId()); + } else { + return normalizeForDns(properties.getInstanceId()); + } } @Override @@ -155,8 +154,8 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { private List createTags() { List tags = new LinkedList<>(properties.getTags()); if(servletContext != null - && StringUtils.isNotBlank(servletContext.getContextPath()) - && StringUtils.isNotBlank(servletContext.getContextPath().replaceAll("/", ""))) { + && StringUtils.hasText(servletContext.getContextPath()) + && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { tags.add("contextPath=" + servletContext.getContextPath()); } return tags; diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientCustomizedTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientCustomizedTests.java new file mode 100644 index 00000000..7612368d --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientCustomizedTests.java @@ -0,0 +1,78 @@ +/* + * Copyright 2013-2015 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.discovery; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + +import java.util.List; + +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.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.EnableDiscoveryClient; +import org.springframework.cloud.consul.ConsulAutoConfiguration; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = ConsulDiscoveryClientCustomizedTests.MyTestConfig.class) +@WebIntegrationTest(value = {"spring.application.name=testConsulDiscovery2", "spring.cloud.consul.discovery.instanceId=testConsulDiscovery2Id"}, randomPort = true) +public class ConsulDiscoveryClientCustomizedTests { + + @Autowired + private ConsulDiscoveryClient discoveryClient; + + @Test + public void getInstancesForServiceWorks() { + List instances = discoveryClient.getInstances("consul"); + assertNotNull("instances was null", instances); + assertFalse("instances was empty", instances.isEmpty()); + + ServiceInstance instance = instances.get(0); + assertNotIpAddress(instance); + } + + private void assertNotIpAddress(ServiceInstance instance) { + assertFalse("host is an ip address", Character.isDigit(instance.getHost().charAt(0))); + } + + @Test + public void getLocalInstance() { + ServiceInstance instance = discoveryClient.getLocalServiceInstance(); + assertNotNull("instance was null", instance); + assertNotIpAddress(instance); + assertEquals("instance id was wrong", "testConsulDiscovery2Id", instance.getServiceId()); + } + + @Configuration + @EnableDiscoveryClient + @EnableAutoConfiguration + @Import({ ConsulAutoConfiguration.class, ConsulDiscoveryClientConfiguration.class }) + public static class MyTestConfig { + + } +}