diff --git a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc index 79cdb065..57b1159b 100644 --- a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc +++ b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc @@ -59,7 +59,7 @@ In the `@FeignClient` annotation the String value ("stores" above) is an arbitra You can also specify a URL using the `url` attribute (absolute value or just a hostname). The name of the bean in the application context is the fully qualified name of the interface. -To specify your own alias value you can use the `qualifier` value +To specify your own alias value you can use the `qualifiers` value of the `@FeignClient` annotation. The load-balancer client above will want to discover the physical addresses diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClient.java index dee564cf..14637340 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -33,6 +33,7 @@ import org.springframework.core.annotation.AliasFor; * * @author Spencer Gibb * @author Venil Noronha + * @author Olga Maciaszek-Sharma */ @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) @@ -65,9 +66,28 @@ public @interface FeignClient { /** * @return the @Qualifier value for the feign client. + * @deprecated in favour of {@link #qualifiers()}. + * + * If both {@link #qualifier()} and {@link #qualifiers()} are present, we will use the + * latter, unless the array returned by {@link #qualifiers()} is empty or only + * contains null or whitespace values, in which case we'll fall back + * first to {@link #qualifier()} and, if that's also not present, to the default = + * contextId + "FeignClient". */ + @Deprecated String qualifier() default ""; + /** + * @return the @Qualifiers value for the feign client. + * + * If both {@link #qualifier()} and {@link #qualifiers()} are present, we will use the + * latter, unless the array returned by {@link #qualifiers()} is empty or only + * contains null or whitespace values, in which case we'll fall back + * first to {@link #qualifier()} and, if that's also not present, to the default = + * contextId + "FeignClient". + */ + String[] qualifiers() default {}; + /** * @return an absolute URL or resolvable hostname (the protocol is optional). */ diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java index ee4dd13a..7cd7d75b 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -20,8 +20,12 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -48,6 +52,7 @@ import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.filter.AnnotationTypeFilter; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -57,6 +62,7 @@ import org.springframework.util.StringUtils; * @author Gang Li * @author Michal Domagala * @author Marcin Grzejszczak + * @author Olga Maciaszek-Sharma */ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLoaderAware, EnvironmentAware { @@ -227,7 +233,6 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLo definition.setLazyInit(true); validate(attributes); - String alias = contextId + "FeignClient"; AbstractBeanDefinition beanDefinition = definition.getBeanDefinition(); beanDefinition.setAttribute(FactoryBean.OBJECT_TYPE_ATTRIBUTE, className); beanDefinition.setAttribute("feignClientsRegistrarFactoryBean", factoryBean); @@ -237,12 +242,12 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLo beanDefinition.setPrimary(primary); - String qualifier = getQualifier(attributes); - if (StringUtils.hasText(qualifier)) { - alias = qualifier; + String[] qualifiers = getQualifiers(attributes); + if (ObjectUtils.isEmpty(qualifiers)) { + qualifiers = new String[] { contextId + "FeignClient" }; } - BeanDefinitionHolder holder = new BeanDefinitionHolder(beanDefinition, className, new String[] { alias }); + BeanDefinitionHolder holder = new BeanDefinitionHolder(beanDefinition, className, qualifiers); BeanDefinitionReaderUtils.registerBeanDefinition(holder, registry); } @@ -356,6 +361,18 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLo return null; } + private String[] getQualifiers(Map client) { + if (client == null) { + return null; + } + List qualifierList = new ArrayList<>(Arrays.asList((String[]) client.get("qualifiers"))); + qualifierList.removeIf(qualifier -> !StringUtils.hasText(qualifier)); + if (qualifierList.isEmpty() && getQualifier(client) != null) { + qualifierList = Collections.singletonList(getQualifier(client)); + } + return !qualifierList.isEmpty() ? qualifierList.toArray(new String[0]) : null; + } + private String getClientName(Map client) { if (client == null) { return null; diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java index c30a40b9..b90af1d6 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -80,8 +80,8 @@ public class FeignClientBuilderTests { for (final Method method : FeignClient.class.getMethods()) { methodNames.add(method.getName()); } - methodNames.removeAll(Arrays.asList("annotationType", "value", "serviceId", "qualifier", "configuration", - "primary", "equals", "hashCode", "toString")); + methodNames.removeAll(Arrays.asList("annotationType", "value", "serviceId", "qualifier", "qualifiers", + "configuration", "primary", "equals", "hashCode", "toString")); Collections.sort(methodNames); // If this safety check fails the Builder has to be updated. // (1) Either a field was removed from the FeignClient annotation and so it has to diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarIntegrationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarIntegrationTests.java new file mode 100644 index 00000000..0eae6c5a --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarIntegrationTests.java @@ -0,0 +1,130 @@ +/* + * Copyright 2013-2021 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.openfeign; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.openfeign.test.NoSecurityConfiguration; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Tests for {@link FeignClientsRegistrar}. + * + * @author Olga Maciaszek-Sharma + */ +@SpringBootTest(classes = FeignClientsRegistrarIntegrationTests.QualifiersTestConfig.class) +class FeignClientsRegistrarIntegrationTests { + + @Autowired + ConfigurableApplicationContext context; + + @Test + void shouldUseQualifiersIfPresent() { + assertThat(context.getBean("qualifier1")).isNotNull(); + assertThat(context.getBean("qualifier2")).isNotNull(); + assertThatExceptionOfType(NoSuchBeanDefinitionException.class).isThrownBy(() -> context.getBean("qualifier3")); + } + + @Test + void shouldUseQualifierIfQualifiersArrayNotPresent() { + assertThat(context.getBean("qualifier4")).isNotNull(); + } + + @Test + void shouldUseDefaultQualifierWhenNonePresent() { + assertThat(context.getBean("noQualifiersFeignClient")).isNotNull(); + } + + @Test + void shouldUseQualifierWhenEmptyQualifiers() { + assertThat(context.getBean("test1")).isNotNull(); + assertThatExceptionOfType(NoSuchBeanDefinitionException.class) + .isThrownBy(() -> context.getBean("emptyQualifiersFeignClient")); + } + + @Test + void shouldUseQualifierWhenWhitespaceQualifiers() { + assertThat(context.getBean("test2")).isNotNull(); + assertThatExceptionOfType(NoSuchBeanDefinitionException.class) + .isThrownBy(() -> context.getBean("whitespaceQualifiersFeignClient")); + } + + @Test + void shouldUseDefaultQualifierWhenEmptyQualifiers() { + assertThat(context.getBean("emptyQualifiersNoQualifierFeignClient")).isNotNull(); + } + + @Test + void shouldUseDefaultQualifierWhenWhitespaceQualifiers() { + assertThat(context.getBean("whitespaceQualifiersNoQualifierFeignClient")).isNotNull(); + } + + @FeignClient(name = "qualifiersClient", qualifiers = { "qualifier1", "qualifier2" }, qualifier = "qualifier3") + protected interface QualifiersClient { + + } + + @FeignClient(name = "qualifierClient", qualifier = "qualifier4") + protected interface QualifierClient { + + } + + @FeignClient(name = "noQualifiers") + protected interface NoQualifiersClient { + + } + + @FeignClient(name = "emptyQualifiers", qualifier = "test1", qualifiers = {}) + protected interface EmptyQualifiersClient { + + } + + @FeignClient(name = "whitespaceQualifiers", qualifier = "test2", qualifiers = { " " }) + protected interface WhitespaceQualifiersClient { + + } + + @FeignClient(name = "emptyQualifiersNoQualifier", qualifiers = {}) + protected interface EmptyQualifiersNoQualifierClient { + + } + + @FeignClient(name = "whitespaceQualifiersNoQualifier", qualifiers = { " " }) + protected interface WhitespaceQualifiersNoQualifierClient { + + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import(NoSecurityConfiguration.class) + @EnableFeignClients(clients = { QualifiersClient.class, QualifierClient.class, NoQualifiersClient.class, + EmptyQualifiersClient.class, WhitespaceQualifiersClient.class, EmptyQualifiersNoQualifierClient.class, + WhitespaceQualifiersNoQualifierClient.class }) + protected static class QualifiersTestConfig { + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java index ab5745c1..11cb4009 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -79,7 +79,7 @@ public class FeignClientsRegistrarTests { private String testGetName(String name) { FeignClientsRegistrar registrar = new FeignClientsRegistrar(); registrar.setEnvironment(new MockEnvironment()); - return registrar.getName(Collections.singletonMap("name", name)); + return registrar.getName(Collections.singletonMap("name", name)); } @Test(expected = IllegalArgumentException.class) @@ -100,7 +100,6 @@ public class FeignClientsRegistrarTests { assertThatCode(() -> config.refresh()) .as("Case https://github.com/spring-cloud/spring-cloud-openfeign/issues/331 should be solved") .doesNotThrowAnyException(); - } @FeignClient(name = "fallbackTestClient", url = "http://localhost:8080/", fallback = FallbackClient.class) diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java index b69776d4..e856a68c 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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.