diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java index c82cb500..8c585eb7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java @@ -38,21 +38,20 @@ import org.springframework.core.annotation.AliasFor; public @interface FeignClient { /** - * The serviceId with optional protocol prefix. Synonym for {@link #serviceId() - * serviceId}. Either serviceId or url must be specified but not both. Can be - * specified as property key, eg: ${propertyKey}. + * The name of the service with optional protocol prefix. Synonym for {@link #name() + * name}. A name must be specified for all clients, whether or not a url is provided. + * Can be specified as property key, eg: ${propertyKey}. */ @AliasFor("name") String value() default ""; /** - * The serviceId with optional protocol prefix. Synonym for {@link #value() value}. - * Either serviceId or url must be specified but not both. Can be - * specified as property key, eg: ${propertyKey}. + * The service id with optional protocol prefix. Synonym for {@link #value() value}. * * @deprecated use {@link #name() name} instead */ @Deprecated + @AliasFor("value") String serviceId() default ""; @AliasFor("value") diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java index d100810e..eb868f0a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java @@ -181,15 +181,21 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, String alias = name + "FeignClient"; AbstractBeanDefinition beanDefinition = definition.getBeanDefinition(); beanDefinition.setPrimary(true); - BeanDefinitionHolder holder = new BeanDefinitionHolder( - beanDefinition, className, new String[]{alias}); + BeanDefinitionHolder holder = new BeanDefinitionHolder(beanDefinition, className, + new String[] { alias }); BeanDefinitionReaderUtils.registerBeanDefinition(holder, registry); } private void validate(Map attributes) { if (StringUtils.hasText((String) attributes.get("value"))) { + Assert.isTrue(!StringUtils.hasText((String) attributes.get("name")), + "Either name or value can be specified, but not both"); Assert.isTrue(!StringUtils.hasText((String) attributes.get("serviceId")), - "Either serviceId or value can be specified, but not both"); + "Either name (serviceId) or value can be specified, but not both"); + } + if (StringUtils.hasText((String) attributes.get("name"))) { + Assert.isTrue(!StringUtils.hasText((String) attributes.get("serviceId")), + "Either name or serviceId can be specified, but not both"); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java index 32b73efa..df12e615 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java @@ -34,10 +34,10 @@ import org.springframework.web.bind.annotation.RequestMapping; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; -import feign.Request; - import static org.junit.Assert.assertEquals; +import feign.Request; + /** * @author Spencer Gibb */ @@ -45,7 +45,7 @@ import static org.junit.Assert.assertEquals; @SpringApplicationConfiguration(classes = LoadBalancerFeignClientOverrideTests.TestConfiguration.class) @WebIntegrationTest(randomPort = true, value = { "spring.application.name=loadBalancerFeignClientTests", - "feign.httpclient.enabled=false", "feign.okhttp.enabled=false"}) + "feign.httpclient.enabled=false", "feign.okhttp.enabled=false" }) @DirtiesContext public class LoadBalancerFeignClientOverrideTests { @@ -55,27 +55,34 @@ public class LoadBalancerFeignClientOverrideTests { @Test public void overrideRequestOptions() { // specific ribbon 'bar' configuration via spring bean - Request.Options barOptions = this.context.getInstance("bar", Request.Options.class); + Request.Options barOptions = this.context.getInstance("bar", + Request.Options.class); assertEquals(1, barOptions.connectTimeoutMillis()); assertEquals(2, barOptions.readTimeoutMillis()); assertOptions(barOptions, "bar", 1, 2); - // specific ribbon 'foo' configuration - Request.Options fooOptions = this.context.getInstance("foo", Request.Options.class); + // specific ribbon 'foo' configuration via application.yml + Request.Options fooOptions = this.context.getInstance("foo", + Request.Options.class); assertEquals(LoadBalancerFeignClient.DEFAULT_OPTIONS, fooOptions); assertOptions(fooOptions, "foo", 7, 17); // generic ribbon default configuration - Request.Options bazOptions = this.context.getInstance("baz", Request.Options.class); - assertEquals(LoadBalancerFeignClient.DEFAULT_OPTIONS, fooOptions); + Request.Options bazOptions = this.context.getInstance("baz", + Request.Options.class); + assertEquals(LoadBalancerFeignClient.DEFAULT_OPTIONS, bazOptions); assertOptions(bazOptions, "baz", 3001, 60001); } - void assertOptions(Request.Options options, String name, int expectedConnect, int expectedRead) { - LoadBalancerFeignClient client = this.context.getInstance(name, LoadBalancerFeignClient.class); + void assertOptions(Request.Options options, String name, int expectedConnect, + int expectedRead) { + LoadBalancerFeignClient client = this.context.getInstance(name, + LoadBalancerFeignClient.class); IClientConfig config = client.getClientConfig(options, name); - assertEquals("connect was wrong for "+name, expectedConnect, config.get(CommonClientConfigKey.ConnectTimeout, -1).intValue()); - assertEquals("read was wrong for "+name, expectedRead, config.get(CommonClientConfigKey.ReadTimeout, -1).intValue()); + assertEquals("connect was wrong for " + name, expectedConnect, + config.get(CommonClientConfigKey.ConnectTimeout, -1).intValue()); + assertEquals("read was wrong for " + name, expectedRead, + config.get(CommonClientConfigKey.ReadTimeout, -1).intValue()); } @Configuration @@ -107,10 +114,9 @@ public class LoadBalancerFeignClientOverrideTests { } } - @FeignClient(value = "baz") + @FeignClient("baz") interface BazClient { @RequestMapping("/") String get(); - } }