diff --git a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc index 816792c5..cac6f58f 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 965e86d0..f1536159 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) @@ -73,9 +74,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 fc88d077..42d4d35c 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 { @@ -243,7 +249,6 @@ class FeignClientsRegistrar definition.setLazyInit(true); validate(attributes); - String alias = contextId + "FeignClient"; AbstractBeanDefinition beanDefinition = definition.getBeanDefinition(); beanDefinition.setAttribute(FactoryBean.OBJECT_TYPE_ATTRIBUTE, className); beanDefinition.setAttribute("feignClientsRegistrarFactoryBean", factoryBean); @@ -253,13 +258,13 @@ class FeignClientsRegistrar 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 }); + qualifiers); BeanDefinitionReaderUtils.registerBeanDefinition(holder, registry); } @@ -379,6 +384,19 @@ class FeignClientsRegistrar 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 a44830d6..b7ec76e4 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. @@ -84,9 +84,9 @@ 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..5997ef4c --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarIntegrationTests.java @@ -0,0 +1,135 @@ +/* + * 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 b25f5a06..03e36deb 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. @@ -80,7 +80,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) @@ -102,7 +102,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/", 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 40f530a3..e1c51cd8 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. @@ -101,7 +101,8 @@ public class FeignPageableEncodingTests { Pageable pageable = PageRequest.of(0, 10, Sort.Direction.DESC, "sortProperty"); // when - final ResponseEntity> response = this.invoiceClient.getInvoicesPaged(pageable); + final ResponseEntity> response = this.invoiceClient + .getInvoicesPaged(pageable); // then assertThat(response).isNotNull(); @@ -125,11 +126,12 @@ public class FeignPageableEncodingTests { @Test public void testPageableWithMultipleSort() { // given - Pageable pageable = PageRequest.of(0, 10, - Sort.by(Sort.Order.desc("sortProperty1"), Sort.Order.asc("sortProperty2"))); + Pageable pageable = PageRequest.of(0, 10, Sort + .by(Sort.Order.desc("sortProperty1"), Sort.Order.asc("sortProperty2"))); // when - final ResponseEntity> response = this.invoiceClient.getInvoicesPaged(pageable); + final ResponseEntity> response = this.invoiceClient + .getInvoicesPaged(pageable); // then assertThat(response).isNotNull();