From e227808b9d47c8c35d2a60414cb1c83564e72e5c Mon Sep 17 00:00:00 2001 From: Piotr Smolarski Date: Mon, 7 Jan 2019 15:35:48 +0100 Subject: [PATCH] Support Multiple Clients Using The Same Service (#90) * Use serviceId as url target if present. Fixes gh-67 * Code review fixes and improvements * Fix test * Add contextId to override bean name of feign client and its configuration. * Add documentation * Add contextId example to documentation --- .../main/asciidoc/spring-cloud-openfeign.adoc | 27 ++++++++++ .../cloud/openfeign/FeignClient.java | 5 ++ .../cloud/openfeign/FeignClientBuilder.java | 6 +++ .../openfeign/FeignClientFactoryBean.java | 24 ++++++--- .../openfeign/FeignClientsRegistrar.java | 19 ++++++- .../openfeign/FeignClientBuilderTests.java | 3 +- .../FeignClientUsingPropertiesTests.java | 6 +-- .../cloud/openfeign/SpringDecoderTests.java | 1 + .../invalid/FeignClientValidationTests.java | 49 +++++++++++++++++-- 9 files changed, 123 insertions(+), 17 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc index 39df827f..86aa25a8 100644 --- a/docs/src/main/asciidoc/spring-cloud-openfeign.adoc +++ b/docs/src/main/asciidoc/spring-cloud-openfeign.adoc @@ -73,6 +73,8 @@ in your external configuration (see A central concept in Spring Cloud's Feign support is that of the named client. Each feign client is part of an ensemble of components that work together to contact a remote server on demand, and the ensemble has a name that you give it as an application developer using the `@FeignClient` annotation. Spring Cloud creates a new ensemble as an `ApplicationContext` on demand for each named client using `FeignClientsConfiguration`. This contains (amongst other things) an `feign.Decoder`, a `feign.Encoder`, and a `feign.Contract`. +It is possible to override the name of that ensemble by using the `contextId` +attribute of the `@FeignClient` annotation. Spring Cloud lets you take full control of the feign client by declaring additional configuration (on top of the `FeignClientsConfiguration`) using `@FeignClient`. Example: @@ -90,6 +92,10 @@ NOTE: `FooConfiguration` does not need to be annotated with `@Configuration`. Ho NOTE: The `serviceId` attribute is now deprecated in favor of the `name` attribute. +NOTE: Using `contextId` attribute of the `@FeignClient` annotation in addition to changing the name of +the `ApplicationContext` ensemble, it will override the alias of the client name +and it will be used as part of the name of the configuration bean created for that client. + WARNING: Previously, using the `url` attribute, did not require the `name` attribute. Using `name` is now required. Placeholders are supported in the `name` and `url` attributes. @@ -206,6 +212,27 @@ hystrix: strategy: SEMAPHORE ---- +If we want to create multiple feign clients with the same name or url +so that they would point to the same server but each with a different custom configuration then +we have to use `contextId` attribute of the `@FeignClient` in order to avoid name +collision of these configuration beans. + +[source,java,indent=0] +---- +@FeignClient(contextId = "fooClient", name = "stores", configuration = FooConfiguration.class) +public interface FooClient { + //.. +} +---- + +[source,java,indent=0] +---- +@FeignClient(contextId = "barClient", name = "stores", configuration = BarConfiguration.class) +public interface BarClient { + //.. +} +---- + === Creating Feign Clients Manually In some cases it might be necessary to customize your Feign Clients in a way that is not 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 59d58bb1..17c63bed 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 @@ -54,6 +54,11 @@ public @interface FeignClient { @Deprecated String serviceId() default ""; + /** + * This will be used as the bean name instead of name if present, but will not be used as a service id. + */ + String contextId() default ""; + /** * The service id with optional protocol prefix. Synonym for {@link #value() value}. */ diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java index 3c0908ad..626a8284 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java @@ -49,6 +49,7 @@ public class FeignClientBuilder { this.feignClientFactoryBean.setApplicationContext(applicationContext); this.feignClientFactoryBean.setType(type); this.feignClientFactoryBean.setName(FeignClientsRegistrar.getName(name)); + this.feignClientFactoryBean.setContextId(FeignClientsRegistrar.getName(name)); // preset default values - these values resemble the default values on the // FeignClient annotation this.url("").path("").decode404(false).fallback(void.class) @@ -60,6 +61,11 @@ public class FeignClientBuilder { return this; } + public Builder contextId(final String contextId) { + this.feignClientFactoryBean.setContextId(contextId); + return this; + } + public Builder path(final String path) { this.feignClientFactoryBean.setPath(FeignClientsRegistrar.getPath(path)); return this; diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java index d8fab63f..2b0f1b08 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java @@ -60,6 +60,8 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, private String url; + private String contextId; + private String path; private boolean decode404; @@ -72,6 +74,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, @Override public void afterPropertiesSet() throws Exception { + Assert.hasText(this.contextId, "Context id must be set"); Assert.hasText(this.name, "Name must be set"); } @@ -104,10 +107,10 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, if (properties.isDefaultToProperties()) { configureUsingConfiguration(context, builder); configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); - configureUsingProperties(properties.getConfig().get(this.name), builder); + configureUsingProperties(properties.getConfig().get(this.contextId), builder); } else { configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); - configureUsingProperties(properties.getConfig().get(this.name), builder); + configureUsingProperties(properties.getConfig().get(this.contextId), builder); configureUsingConfiguration(context, builder); } } else { @@ -133,7 +136,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, builder.options(options); } Map requestInterceptors = context.getInstances( - this.name, RequestInterceptor.class); + this.contextId, RequestInterceptor.class); if (requestInterceptors != null) { builder.requestInterceptors(requestInterceptors.values()); } @@ -202,16 +205,16 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, } protected T get(FeignContext context, Class type) { - T instance = context.getInstance(this.name, type); + T instance = context.getInstance(this.contextId, type); if (instance == null) { throw new IllegalStateException("No bean found of type " + type + " for " - + this.name); + + this.contextId); } return instance; } protected T getOptional(FeignContext context, Class type) { - return context.getInstance(this.name, type); + return context.getInstance(this.contextId, type); } protected T loadBalance(Feign.Builder builder, FeignContext context, @@ -241,7 +244,6 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, Feign.Builder builder = feign(context); if (!StringUtils.hasText(this.url)) { - String url; if (!this.name.startsWith("http")) { url = "http://" + this.name; } @@ -309,6 +311,14 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, this.name = name; } + public String getContextId() { + return contextId; + } + + public void setContextId(String contextId) { + this.contextId = contextId; + } + public String getUrl() { return url; } 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 7e458887..02beddb8 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 @@ -171,13 +171,15 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, definition.addPropertyValue("path", getPath(attributes)); String name = getName(attributes); definition.addPropertyValue("name", name); + String contextId = getContextId(attributes); + definition.addPropertyValue("contextId", contextId); definition.addPropertyValue("type", className); definition.addPropertyValue("decode404", attributes.get("decode404")); definition.addPropertyValue("fallback", attributes.get("fallback")); definition.addPropertyValue("fallbackFactory", attributes.get("fallbackFactory")); definition.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE); - String alias = name + "FeignClient"; + String alias = contextId + "FeignClient"; AbstractBeanDefinition beanDefinition = definition.getBeanDefinition(); boolean primary = (Boolean)attributes.get("primary"); // has a default, won't be null @@ -227,6 +229,16 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, return getName(name); } + private String getContextId(Map attributes) { + String contextId = (String) attributes.get("contextId"); + if (!StringUtils.hasText(contextId)) { + return getName(attributes); + } + + contextId = resolve(contextId); + return getName(contextId); + } + static String getName(String name) { if (!StringUtils.hasText(name)) { return ""; @@ -350,7 +362,10 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, if (client == null) { return null; } - String value = (String) client.get("value"); + String value = (String) client.get("contextId"); + if (!StringUtils.hasText(value)) { + value = (String) client.get("value"); + } if (!StringUtils.hasText(value)) { value = (String) client.get("name"); } 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 cf750073..1555eb9b 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 @@ -91,7 +91,7 @@ public class FeignClientBuilderTests { // on this builder class. // (2) Or a new field was added and the builder class has to be extended with this // new field. - Assert.assertThat(methodNames, Matchers.contains("decode404", "fallback", + Assert.assertThat(methodNames, Matchers.contains("contextId", "decode404", "fallback", "fallbackFactory", "name", "path", "url")); } @@ -105,6 +105,7 @@ public class FeignClientBuilderTests { assertFactoryBeanField(builder, "applicationContext", applicationContext); assertFactoryBeanField(builder, "type", FeignClientBuilderTests.class); assertFactoryBeanField(builder, "name", "TestClient"); + assertFactoryBeanField(builder, "contextId", "TestClient"); // and: assertFactoryBeanField(builder, "url", diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientUsingPropertiesTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientUsingPropertiesTests.java index 6eca2eaa..06ebd16f 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientUsingPropertiesTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientUsingPropertiesTests.java @@ -81,15 +81,15 @@ public class FeignClientUsingPropertiesTests { public FeignClientUsingPropertiesTests() { fooFactoryBean = new FeignClientFactoryBean(); - fooFactoryBean.setName("foo"); + fooFactoryBean.setContextId("foo"); fooFactoryBean.setType(FeignClientFactoryBean.class); barFactoryBean = new FeignClientFactoryBean(); - barFactoryBean.setName("bar"); + barFactoryBean.setContextId("bar"); barFactoryBean.setType(FeignClientFactoryBean.class); formFactoryBean = new FeignClientFactoryBean(); - formFactoryBean.setName("form"); + formFactoryBean.setContextId("form"); formFactoryBean.setType(FeignClientFactoryBean.class); } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java index 0800b7fd..f16c6bb2 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java @@ -62,6 +62,7 @@ public class SpringDecoderTests extends FeignClientFactoryBean { public SpringDecoderTests() { setName("test"); + setContextId("test"); } public TestClient testClient() { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java index 84dfe1c2..a67d80e1 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java @@ -23,9 +23,13 @@ import feign.hystrix.HystrixFeign; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; +import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.openfeign.EnableFeignClients; import org.springframework.cloud.openfeign.FeignAutoConfiguration; import org.springframework.cloud.openfeign.FeignClient; +import org.springframework.cloud.openfeign.ribbon.FeignRibbonClientAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -64,24 +68,61 @@ public class FeignClientValidationTests { @Test public void testServiceIdAndValue() { - this.expected.expectMessage("only one is permitted"); AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - NameAndValueConfiguration.class); + LoadBalancerAutoConfiguration.class, + RibbonAutoConfiguration.class, + FeignRibbonClientAutoConfiguration.class, + NameAndServiceIdConfiguration.class); assertNotNull(context.getBean(NameAndServiceIdConfiguration.Client.class)); context.close(); } @Configuration - @Import(FeignAutoConfiguration.class) + @Import({FeignAutoConfiguration.class, HttpClientConfiguration.class}) @EnableFeignClients(clients = NameAndServiceIdConfiguration.Client.class) protected static class NameAndServiceIdConfiguration { - @FeignClient(serviceId = "foo", name = "bar") + @FeignClient(name = "bar", serviceId = "foo") interface Client { @RequestMapping(method = RequestMethod.GET, value = "/") String get(); } + } + + @Test + public void testDuplicatedClientNames() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.setAllowBeanDefinitionOverriding(false); + context.register( + LoadBalancerAutoConfiguration.class, + RibbonAutoConfiguration.class, + FeignRibbonClientAutoConfiguration.class, + DuplicatedFeignClientNamesConfiguration.class + ); + context.refresh(); + assertNotNull(context.getBean(DuplicatedFeignClientNamesConfiguration.FooClient.class)); + assertNotNull(context.getBean(DuplicatedFeignClientNamesConfiguration.BarClient.class)); + context.close(); + } + + @Configuration + @Import({FeignAutoConfiguration.class, HttpClientConfiguration.class}) + @EnableFeignClients(clients = {DuplicatedFeignClientNamesConfiguration.FooClient.class, + DuplicatedFeignClientNamesConfiguration.BarClient.class}) + protected static class DuplicatedFeignClientNamesConfiguration { + + @FeignClient(contextId = "foo", name = "bar") + interface FooClient { + @RequestMapping(method = RequestMethod.GET, value = "/") + String get(); + } + + @FeignClient(name = "bar") + interface BarClient { + @RequestMapping(method = RequestMethod.GET, value = "/") + String get(); + } } @Test