From a2fe8de377bbeda51932b0606232e8ecadf984c9 Mon Sep 17 00:00:00 2001 From: bono007 Date: Fri, 12 Mar 2021 14:27:07 -0600 Subject: [PATCH] Adds support for multiple query tags (#684) --- .../discovery/ConsulDiscoveryClient.java | 10 +- .../discovery/ConsulDiscoveryProperties.java | 44 +++- .../ConsulReactiveDiscoveryClient.java | 13 +- ...ulDiscoveryClientDefaultQueryTagTests.java | 110 --------- .../ConsulDiscoveryClientQueryTagsTests.java | 216 ++++++++++++++++++ ...iscoveryClientServerListQueryTagTests.java | 111 --------- .../ConsulDiscoveryPropertiesTests.java | 102 +++++++-- .../ConsulReactiveDiscoveryClientTests.java | 7 +- 8 files changed, 354 insertions(+), 259 deletions(-) delete mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientDefaultQueryTagTests.java create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientQueryTagsTests.java delete mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientServerListQueryTagTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java index 00149726..f722fdf0 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java @@ -67,13 +67,13 @@ public class ConsulDiscoveryClient implements DiscoveryClient { private void addInstancesToList(List instances, String serviceId, QueryParams queryParams) { HealthServicesRequest.Builder requestBuilder = HealthServicesRequest.newBuilder() - .setPassing(this.properties.isQueryPassing()).setQueryParams(queryParams) - .setToken(this.properties.getAclToken()); - String queryTag = this.properties.getQueryTagForService(serviceId); - if (queryTag != null) { - requestBuilder.setTag(queryTag); + .setPassing(properties.isQueryPassing()).setQueryParams(queryParams).setToken(properties.getAclToken()); + String[] queryTags = properties.getQueryTagsForService(serviceId); + if (queryTags != null) { + requestBuilder.setTags(queryTags); } HealthServicesRequest request = requestBuilder.build(); + Response> services = this.client.getHealthServices(serviceId, request); for (HealthService service : services.getValue()) { diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java index 96da9b21..03455e1d 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java @@ -30,6 +30,8 @@ import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.commons.util.InetUtils.HostInfo; import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.core.style.ToStringCreator; +import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; /** * Defines configuration for service discovery and registration. @@ -160,7 +162,8 @@ public class ConsulDiscoveryProperties { /** * Map of serviceId's -> tag to query for in server list. This allows filtering - * services by a single tag. + * services by one more tags. Multiple tags can be specified with a comma separated + * value. */ private Map serverListQueryTags = new HashMap<>(); @@ -170,7 +173,10 @@ public class ConsulDiscoveryProperties { */ private Map datacenters = new HashMap<>(); - /** Tag to query for in service list if one is not listed in serverListQueryTags. */ + /** + * Tag to query for in service list if one is not listed in serverListQueryTags. + * Multiple tags can be specified with a comma separated value. + */ private String defaultQueryTag; /** @@ -219,14 +225,44 @@ public class ConsulDiscoveryProperties { } /** - * @param serviceId The service who's filtering tag is being looked up - * @return The tag the given service id should be filtered by, or null. + * Gets the tag to use when looking up the instances for a particular service. If the + * service has an entry in {@link #serverListQueryTags} that will be used. Otherwise + * the content of {@link #defaultQueryTag} will be used. + * @param serviceId the service whose instances are being looked up + * @return the tag to filter the service instances by or null if no tags are + * configured for the service and the default query tag is not configured */ public String getQueryTagForService(String serviceId) { String tag = this.serverListQueryTags.get(serviceId); return tag != null ? tag : this.defaultQueryTag; } + /** + * Gets the array of tags to use when looking up the instances for a particular + * service. If the service has an entry in {@link #serverListQueryTags} that will be + * used. Otherwise the content of {@link #defaultQueryTag} will be used. This differs + * from {@link #getQueryTagForService(String)} in that it assumes the configured tag + * property value may represent multiple tags when separated by commas. When the tag + * property is set to a single tag then this method behaves identical to its + * aforementioned counterpart except that it returns a single element array with the + * single tag value. + *

+ * The expected format of the tag property value is {@code tag1,tag2,..,tagN}. + * Whitespace will be trimmed off each entry. + * @param serviceId the service whose instances are being looked up + * @return the array of tags to filter the service instances by - it will be null if + * no tags are configured for the service and the default query tag is not configured + * or if a single tag is configured and it is the empty string + */ + @Nullable + public String[] getQueryTagsForService(String serviceId) { + String queryTagStr = getQueryTagForService(serviceId); + if (queryTagStr == null || queryTagStr.isEmpty()) { + return null; + } + return StringUtils.tokenizeToStringArray(queryTagStr, ","); + } + public String getHostname() { return this.preferIpAddress ? this.ipAddress : this.hostname; } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClient.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClient.java index 5a4ff0a2..2fb42938 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClient.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClient.java @@ -41,6 +41,7 @@ import org.springframework.cloud.consul.discovery.ConsulServiceInstance; * Consul version of {@link ReactiveDiscoveryClient}. * * @author Tim Ysewyn + * @author Chris Bono */ public class ConsulReactiveDiscoveryClient implements ReactiveDiscoveryClient { @@ -75,11 +76,17 @@ public class ConsulReactiveDiscoveryClient implements ReactiveDiscoveryClient { } private List getHealthServices(String serviceId) { - HealthServicesRequest request = HealthServicesRequest.newBuilder().setTag(this.properties.getDefaultQueryTag()) - .setPassing(this.properties.isQueryPassing()).setQueryParams(QueryParams.DEFAULT) - .setToken(this.properties.getAclToken()).build(); + HealthServicesRequest.Builder requestBuilder = HealthServicesRequest.newBuilder() + .setPassing(properties.isQueryPassing()).setQueryParams(QueryParams.DEFAULT) + .setToken(properties.getAclToken()); + String[] queryTags = properties.getQueryTagsForService(serviceId); + if (queryTags != null) { + requestBuilder.setTags(queryTags); + } + HealthServicesRequest request = requestBuilder.build(); Response> services = client.getHealthServices(serviceId, request); + return services == null ? Collections.emptyList() : services.getValue(); } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientDefaultQueryTagTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientDefaultQueryTagTests.java deleted file mode 100644 index 69fbc070..00000000 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientDefaultQueryTagTests.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * 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.discovery; - -import java.util.Arrays; -import java.util.List; - -import com.ecwid.consul.v1.ConsulClient; -import com.ecwid.consul.v1.agent.model.NewService; -import org.junit.After; -import org.junit.Before; -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.ServiceInstance; -import org.springframework.cloud.consul.test.ConsulTestcontainers; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; -import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringRunner; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.MOCK; - -/** - * @author Piotr Wielgolaski - */ -@RunWith(SpringRunner.class) -@SpringBootTest(webEnvironment = MOCK, classes = ConsulDiscoveryClientDefaultQueryTagTests.TestConfig.class, - properties = { "spring.application.name=consulServiceDefaultTag", - "spring.cloud.consul.discovery.catalogServicesWatch.enabled=false", - "spring.cloud.consul.discovery.defaultQueryTag=intg" }) -@DirtiesContext -@ContextConfiguration(initializers = ConsulTestcontainers.class) -public class ConsulDiscoveryClientDefaultQueryTagTests { - - public static final String NAME = "consulServiceDefaultTag"; - - @Autowired - private ConsulDiscoveryClient discoveryClient; - - @Autowired - private ConsulClient consulClient; - - private NewService intgService = serviceForEnvironment("intg", 9081); - - private NewService uatService = serviceForEnvironment("uat", 9080); - - @Before - public void setUp() throws Exception { - this.consulClient.agentServiceRegister(this.intgService); - this.consulClient.agentServiceRegister(this.uatService); - } - - @After - public void tearDown() throws Exception { - this.consulClient.agentServiceDeregister(this.intgService.getId()); - this.consulClient.agentServiceDeregister(this.uatService.getId()); - } - - @Test - public void shouldReturnOnlyIntgInstance() { - List instances = this.discoveryClient.getInstances(NAME); - assertThat(instances).as("instances was wrong size").hasSize(1); - ServiceInstance serviceInstance = instances.get(0); - assertThat(serviceInstance.getPort()).isEqualTo(intgService.getPort()); - assertThat(serviceInstance.getServiceId()).isEqualTo(intgService.getName()); - assertThat(serviceInstance.getInstanceId()).isEqualTo(intgService.getId()); - assertThat(serviceInstance).isInstanceOf(ConsulServiceInstance.class); - ConsulServiceInstance consulInstance = (ConsulServiceInstance) serviceInstance; - assertThat(consulInstance.getTags()).containsOnly("intg"); - assertThat(consulInstance.getHealthService()).isNotNull(); - } - - private NewService serviceForEnvironment(String env, int port) { - NewService service = new NewService(); - service.setAddress("localhost"); - service.setId(NAME + env); - service.setName(NAME); - service.setPort(port); - service.setTags(Arrays.asList(env)); - return service; - } - - @Configuration(proxyBeanMethods = false) - @EnableAutoConfiguration - @Import({ ConsulDiscoveryClientConfiguration.class }) - protected static class TestConfig { - - } - -} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientQueryTagsTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientQueryTagsTests.java new file mode 100644 index 00000000..77b5b721 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientQueryTagsTests.java @@ -0,0 +1,216 @@ +/* + * 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.discovery; + +import java.util.Arrays; +import java.util.List; + +import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; + +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.agent.model.NewService; +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.consul.test.ConsulTestcontainers; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Lightweight integration tests to verify Consul query tags usage in + * {@link ConsulDiscoveryClient#getInstances(String)}. + * + * @author Piotr Wielgolaski + * @author Chris Bono + */ +class ConsulDiscoveryClientQueryTagsTests { + + private static final String NAME = "query-tags-test-services"; + static NewService QA_WEST_SERVICE = serviceForEnvironmentAndRegion("qa", "us-west", 9080); + static NewService QA_EAST_SERVICE = serviceForEnvironmentAndRegion("qa", "us-east", 9080); + static NewService PROD_WEST_SERVICE = serviceForEnvironmentAndRegion("prod", "us-west", 9082); + static NewService PROD_EAST_SERVICE = serviceForEnvironmentAndRegion("prod", "us-east", 9082); + + private ApplicationContextRunner appContextRunner = new ApplicationContextRunner() + .withInitializer(new ConsulTestcontainers()).withConfiguration(AutoConfigurations.of(TestConfig.class)) + .withPropertyValues("spring.application.name=consulServiceQueryTags", + "spring.cloud.consul.discovery.catalogServicesWatch.enabled=false"); + + private static NewService serviceForEnvironmentAndRegion(String env, String region, int port) { + NewService service = new NewService(); + service.setAddress("localhost"); + service.setId(String.format("%s-%s-%s", NAME, env, region)); + service.setName(NAME); + service.setPort(port); + service.setTags(Arrays.asList(env, region)); + return service; + } + + @Test + void singleMatchingTagSpecifiedOnDefaultQueryTagProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa").run( + context -> assertThatGetInstancesReturnsExpectedServices(context, QA_WEST_SERVICE, QA_EAST_SERVICE)); + } + + @Test + void singleNonMatchingTagSpecifiedOnDefaultQueryTagProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=foo") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, new NewService[0])); + } + + @Test + void multipleMatchingTagsSpecifiedOnDefaultQueryTagProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=prod,us-west") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, PROD_WEST_SERVICE)); + } + + @Test + void multipleNonMatchingTagsSpecifiedOnDefaultQueryTagProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=prod,foo") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, new NewService[0])); + } + + @Test + void multipleConflictingMatchingTagsSpecifiedOnDefaultQueryTagsProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=prod,qa") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context)); + } + + @Test + void emptyTagSpecifiedOnDefaultQueryTagProperty() { + appContextRunner.withPropertyValues("spring.cloud.consul.discovery.default-query-tag=") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, QA_WEST_SERVICE, QA_EAST_SERVICE, + PROD_WEST_SERVICE, PROD_EAST_SERVICE)); + } + + @Test + void singleTagSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=prod") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, PROD_WEST_SERVICE, + PROD_EAST_SERVICE)); + } + + @Test + void singleNonMatchingTagSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=foo") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, new NewService[0])); + } + + @Test + void multipleMatchingTagsSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=prod,us-west") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, PROD_WEST_SERVICE)); + } + + @Test + void multipleNotAllMatchingTagsSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=prod,foo") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, new NewService[0])); + } + + @Test + void multipleConflictingMatchingTagsSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=prod,qa") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, new NewService[0])); + } + + @Test + void emptyTagSpecifiedOnServerListQueryTagsProperty() { + appContextRunner + .withPropertyValues("spring.cloud.consul.discovery.default-query-tag=qa", + "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=") + .run(context -> assertThatGetInstancesReturnsExpectedServices(context, QA_WEST_SERVICE, QA_EAST_SERVICE, + PROD_WEST_SERVICE, PROD_EAST_SERVICE)); + } + + @Test + void noTagsSpecifiedOnAnyProperties() { + appContextRunner.run(context -> assertThatGetInstancesReturnsExpectedServices(context, QA_WEST_SERVICE, + QA_EAST_SERVICE, PROD_WEST_SERVICE, PROD_EAST_SERVICE)); + } + + private void assertThatGetInstancesReturnsExpectedServices(AssertableApplicationContext context, + NewService... expectedServices) { + if (expectedServices == null) { + expectedServices = new NewService[0]; + } + assertThat(context).hasNotFailed(); + ConsulDiscoveryClient consulDiscoveryClient = context.getBean(ConsulDiscoveryClient.class); + List serviceInstances = consulDiscoveryClient.getInstances(NAME); + assertThat(serviceInstances).hasSize(expectedServices.length) + .hasOnlyElementsOfType(ConsulServiceInstance.class); + for (NewService expectedService : expectedServices) { + assertThat(serviceInstances) + .anySatisfy(serviceInstance -> assertThatServicesMatch((ConsulServiceInstance) serviceInstance, + expectedService)); + } + } + + private void assertThatServicesMatch(ConsulServiceInstance serviceInstance, NewService expectedService) { + assertThat(serviceInstance.getPort()).isEqualTo(expectedService.getPort()); + assertThat(serviceInstance.getServiceId()).isEqualTo(expectedService.getName()); + assertThat(serviceInstance.getInstanceId()).isEqualTo(expectedService.getId()); + assertThat(serviceInstance).isInstanceOf(ConsulServiceInstance.class); + assertThat(serviceInstance.getTags()).containsExactlyElementsOf(expectedService.getTags()); + assertThat(serviceInstance.getHealthService()).isNotNull(); + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import(ConsulDiscoveryClientConfiguration.class) + protected static class TestConfig { + + @Autowired + private ConsulClient consulClient; + + @PostConstruct + void init() { + consulClient.agentServiceRegister(ConsulDiscoveryClientQueryTagsTests.QA_WEST_SERVICE); + consulClient.agentServiceRegister(ConsulDiscoveryClientQueryTagsTests.QA_EAST_SERVICE); + consulClient.agentServiceRegister(ConsulDiscoveryClientQueryTagsTests.PROD_WEST_SERVICE); + consulClient.agentServiceRegister(ConsulDiscoveryClientQueryTagsTests.PROD_EAST_SERVICE); + } + + @PreDestroy + void destroy() { + consulClient.agentServiceDeregister(ConsulDiscoveryClientQueryTagsTests.QA_WEST_SERVICE.getId()); + consulClient.agentServiceDeregister(ConsulDiscoveryClientQueryTagsTests.QA_EAST_SERVICE.getId()); + consulClient.agentServiceDeregister(ConsulDiscoveryClientQueryTagsTests.PROD_WEST_SERVICE.getId()); + consulClient.agentServiceDeregister(ConsulDiscoveryClientQueryTagsTests.PROD_EAST_SERVICE.getId()); + } + + } + +} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientServerListQueryTagTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientServerListQueryTagTests.java deleted file mode 100644 index eeff7754..00000000 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientServerListQueryTagTests.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * 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.discovery; - -import java.util.Collections; -import java.util.List; - -import com.ecwid.consul.v1.ConsulClient; -import com.ecwid.consul.v1.agent.model.NewService; -import org.junit.After; -import org.junit.Before; -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.ServiceInstance; -import org.springframework.cloud.consul.test.ConsulTestcontainers; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; -import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringRunner; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.MOCK; -import static org.springframework.cloud.consul.discovery.ConsulDiscoveryClientServerListQueryTagTests.NAME; - -/** - * Integration test to verify the - * {@link ConsulDiscoveryProperties#getServerListQueryTags()} is respected. - * - * @author Chris Bono - */ -@RunWith(SpringRunner.class) -@SpringBootTest(webEnvironment = MOCK, classes = ConsulDiscoveryClientServerListQueryTagTests.TestConfig.class, - properties = { "spring.application.name=" + NAME, - "spring.cloud.consul.discovery.catalogServicesWatch.enabled=false", - "spring.cloud.consul.discovery.server-list-query-tags[" + NAME + "]=uat", - "spring.cloud.consul.discovery.defaultQueryTag=intg" }) -@DirtiesContext -@ContextConfiguration(initializers = ConsulTestcontainers.class) -public class ConsulDiscoveryClientServerListQueryTagTests { - - public static final String NAME = "consulServiceServerListQueryTags"; - - @Autowired - private ConsulDiscoveryClient discoveryClient; - - @Autowired - private ConsulClient consulClient; - - private NewService intgService = serviceForEnvironment("intg", 9081); - - private NewService uatService = serviceForEnvironment("uat", 9080); - - @Before - public void setUp() { - consulClient.agentServiceRegister(intgService); - consulClient.agentServiceRegister(uatService); - } - - @After - public void tearDown() { - consulClient.agentServiceDeregister(intgService.getId()); - consulClient.agentServiceDeregister(uatService.getId()); - } - - @Test - public void shouldReturnInstanceWithMatchingServerListQueryTags() { - List instances = discoveryClient.getInstances(NAME); - assertThat(instances).as("instances was wrong size").hasSize(1); - ServiceInstance serviceInstance = instances.get(0); - assertThat(serviceInstance.getPort()).isEqualTo(uatService.getPort()); - assertThat(serviceInstance.getServiceId()).isEqualTo(uatService.getName()); - assertThat(serviceInstance.getInstanceId()).isEqualTo(uatService.getId()); - } - - private NewService serviceForEnvironment(String env, int port) { - NewService service = new NewService(); - service.setAddress("localhost"); - service.setId(NAME + env); - service.setName(NAME); - service.setPort(port); - service.setTags(Collections.singletonList(env)); - return service; - } - - @Configuration(proxyBeanMethods = false) - @EnableAutoConfiguration - @Import({ ConsulDiscoveryClientConfiguration.class }) - protected static class TestConfig { - - } - -} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryPropertiesTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryPropertiesTests.java index 05227b75..9764f168 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryPropertiesTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryPropertiesTests.java @@ -19,15 +19,20 @@ package org.springframework.cloud.consul.discovery; import java.util.Collections; import java.util.Map; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.commons.util.InetUtilsProperties; import static org.assertj.core.api.Assertions.assertThat; -public class ConsulDiscoveryPropertiesTests { +/** + * Unit tests for {@link ConsulDiscoveryProperties}. + * + * @author Chris Bono + */ +class ConsulDiscoveryPropertiesTests { private static final String DEFAULT_TAG = "defaultTag"; @@ -45,45 +50,96 @@ public class ConsulDiscoveryPropertiesTests { private ConsulDiscoveryProperties properties; - @Before - public void setUp() { + @BeforeEach + void setUp() { this.properties = new ConsulDiscoveryProperties(new InetUtils(new InetUtilsProperties())); - this.properties.setDefaultQueryTag(DEFAULT_TAG); - this.properties.setServerListQueryTags(this.serverListQueryTags); - this.properties.setDatacenters(this.datacenters); + properties.setDefaultQueryTag(DEFAULT_TAG); + properties.setServerListQueryTags(this.serverListQueryTags); + properties.setDatacenters(this.datacenters); } @Test - public void testReturnsNullWhenNoDefaultAndNotInMap() { - this.properties.setDefaultQueryTag(null); - - assertThat(this.properties.getQueryTagForService(SERVICE_NAME_NOT_IN_MAP)).isNull(); + void getTagReturnsNullWhenNoDefaultAndNotInMap() { + properties.setDefaultQueryTag(null); + assertThat(properties.getQueryTagForService(SERVICE_NAME_NOT_IN_MAP)).isNull(); } @Test - public void testGetTagReturnsDefaultWhenNotInMap() { - assertThat(this.properties.getQueryTagForService(SERVICE_NAME_NOT_IN_MAP)).isEqualTo(DEFAULT_TAG); + void getTagReturnsDefaultWhenNotInMap() { + assertThat(properties.getQueryTagForService(SERVICE_NAME_NOT_IN_MAP)).isEqualTo(DEFAULT_TAG); } @Test - public void testGetTagReturnsMapValueWhenInMap() { - assertThat(this.properties.getQueryTagForService(SERVICE_NAME_IN_MAP)).isEqualTo(MAP_TAG); + void getTagReturnsMapValueWhenInMap() { + assertThat(properties.getQueryTagForService(SERVICE_NAME_IN_MAP)).isEqualTo(MAP_TAG); } @Test - public void testGetDcReturnsNullWhenNotInMap() { - assertThat(this.properties.getDatacenters().get(SERVICE_NAME_NOT_IN_MAP)).isNull(); + void getTagsReturnsNullWhenNoDefaultAndNotInMap() { + properties.setDefaultQueryTag(null); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_NOT_IN_MAP)).isNull(); } @Test - public void testGetDcReturnsMapValueWhenInMap() { - assertThat(this.properties.getDatacenters().get(SERVICE_NAME_IN_MAP)).isEqualTo(MAP_DC); + void getTagsReturnsNullWhenDefaultIsSetToEmptyStringAndNotInMap() { + properties.setDefaultQueryTag(""); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_NOT_IN_MAP)).isNull(); } @Test - public void testAddManagementTag() { - this.properties.getManagementTags().add("newTag"); - assertThat(this.properties.getManagementTags()).containsOnly(ConsulDiscoveryProperties.MANAGEMENT, "newTag"); + void getTagsReturnsDefaultWhenNotInMap() { + assertThat(properties.getQueryTagsForService(SERVICE_NAME_NOT_IN_MAP)).containsExactly(DEFAULT_TAG); + } + + @Test + void getTagsReturnsMapValueWhenInMap() { + assertThat(properties.getQueryTagsForService(SERVICE_NAME_IN_MAP)).containsExactly(MAP_TAG); + } + + @Test + void getTagsReturnsNullWhenMapValueIsSetToEmptyStringAndInMap() { + properties.setServerListQueryTags(Collections.singletonMap(SERVICE_NAME_IN_MAP, "")); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_IN_MAP)).isNull(); + } + + @Test + void getTagsReturnsMultipleFromDefaultQueryTag() { + properties.setDefaultQueryTag("foo,bar"); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_NOT_IN_MAP)).containsExactly("foo", "bar"); + } + + @Test + void getTagsReturnsMultipleFromServerListMapEntry() { + properties.setServerListQueryTags(Collections.singletonMap(SERVICE_NAME_IN_MAP, "foo,bar")); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_IN_MAP)).containsExactly("foo", "bar"); + } + + @Test + void getDcReturnsNullWhenNotInMap() { + assertThat(properties.getDatacenters().get(SERVICE_NAME_NOT_IN_MAP)).isNull(); + } + + @Test + void getTagsReturnsSingleTrimmedEntryFromTagWithExtraWhitespace() { + properties.setServerListQueryTags(Collections.singletonMap(SERVICE_NAME_IN_MAP, " foo ")); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_IN_MAP)).containsExactly("foo"); + } + + @Test + void getTagsReturnsMultipleTrimmedEntriesFromTagsWithExtraWhitespace() { + properties.setServerListQueryTags(Collections.singletonMap(SERVICE_NAME_IN_MAP, " foo , bar ")); + assertThat(properties.getQueryTagsForService(SERVICE_NAME_IN_MAP)).containsExactly("foo", "bar"); + } + + @Test + void getDcReturnsMapValueWhenInMap() { + assertThat(properties.getDatacenters().get(SERVICE_NAME_IN_MAP)).isEqualTo(MAP_DC); + } + + @Test + void addManagementTag() { + properties.getManagementTags().add("newTag"); + assertThat(properties.getManagementTags()).containsOnly(ConsulDiscoveryProperties.MANAGEMENT, "newTag"); } } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClientTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClientTests.java index 35009845..29988904 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClientTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/reactive/ConsulReactiveDiscoveryClientTests.java @@ -40,6 +40,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -127,7 +128,7 @@ class ConsulReactiveDiscoveryClientTests { Flux instances = client.getInstances("existing-service"); StepVerifier.create(instances).expectNextCount(1).expectComplete().verify(); verify(properties).getAclToken(); - verify(properties).getDefaultQueryTag(); + verify(properties).getQueryTagsForService("existing-service"); verify(properties).isQueryPassing(); verify(consulClient).getHealthServices(eq("existing-service"), any()); } @@ -142,7 +143,7 @@ class ConsulReactiveDiscoveryClientTests { Flux instances = client.getInstances("existing-service"); StepVerifier.create(instances).expectNextCount(1).expectComplete().verify(); verify(properties, times(1)).getAclToken(); - verify(properties).getDefaultQueryTag(); + verify(properties).getQueryTagsForService("existing-service"); verify(properties).isQueryPassing(); verify(consulClient).getHealthServices(eq("existing-service"), any()); } @@ -152,7 +153,7 @@ class ConsulReactiveDiscoveryClientTests { } private void configureCommonProperties() { - when(properties.getDefaultQueryTag()).thenReturn("queryTag"); + when(properties.getQueryTagsForService(anyString())).thenReturn(new String[] { "queryTag" }); when(properties.isQueryPassing()).thenReturn(false); }