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

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

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}.
 	 */

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;

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java

@@ -60,6 +60,8 @@ class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean,
 
 	private String url;
 
+	private String contextId;
+
 	private String path;
 
 	private boolean decode404;
@@ -72,6 +74,7 @@ class FeignClientFactoryBean implements FactoryBean<Object>, 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<Object>, 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<Object>, InitializingBean,
 			builder.options(options);
 		}
 		Map<String, RequestInterceptor> 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<Object>, InitializingBean,
 	}
 
 	protected <T> T get(FeignContext context, Class<T> 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> T getOptional(FeignContext context, Class<T> type) {
-		return context.getInstance(this.name, type);
+		return context.getInstance(this.contextId, type);
 	}
 
 	protected <T> T loadBalance(Feign.Builder builder, FeignContext context,
@@ -241,7 +244,6 @@ class FeignClientFactoryBean implements FactoryBean<Object>, 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<Object>, InitializingBean,
 		this.name = name;
 	}
 
+	public String getContextId() {
+		return contextId;
+	}
+
+	public void setContextId(String contextId) {
+		this.contextId = contextId;
+	}
+
 	public String getUrl() {
 		return url;
 	}

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<String, Object> 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");
 		}

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",

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);
 	}
 

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() {

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