From 798a19e468e2d9e9915027360fc0111d993f4bcc Mon Sep 17 00:00:00 2001 From: Jakub Narloch Date: Sat, 25 Jul 2015 18:25:56 +0200 Subject: [PATCH] Specifing only the serviceId causes NPE --- .../netflix/feign/FeignClientsRegistrar.java | 7 ++++--- .../netflix/feign/valid/FeignClientTests.java | 21 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) 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 66910602..484eced9 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 @@ -43,6 +43,7 @@ import org.springframework.util.StringUtils; /** * @author Spencer Gibb + * @author Jakub Narloch */ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLoaderAware, BeanClassLoaderAware { @@ -115,13 +116,13 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, 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"); } } private String getServiceId(Map attributes) { - String name = (String) attributes.get("name"); + String name = (String) attributes.get("serviceId"); if (!StringUtils.hasText(name)) { name = (String) attributes.get("value"); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java index 253bee64..66d9de61 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java @@ -16,7 +16,8 @@ package org.springframework.cloud.netflix.feign.valid; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; @@ -65,6 +66,7 @@ import feign.RequestTemplate; /** * @author Spencer Gibb + * @author Jakub Narloch */ @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = FeignClientTests.Application.class) @@ -80,6 +82,9 @@ public class FeignClientTests { @Autowired private TestClient testClient; + @Autowired + private TestClientServiceId testClientServiceId; + @Autowired private Client feignClient; @@ -99,6 +104,12 @@ public class FeignClientTests { public List getHelloHeaders(); } + @FeignClient(serviceId = "localapp") + protected static interface TestClientServiceId { + @RequestMapping(method = RequestMethod.GET, value = "/hello") + public Hello getHello(); + } + @Configuration @EnableAutoConfiguration @RestController @@ -210,6 +221,14 @@ public class FeignClientTests { assertThat(delegate, is(instanceOf(feign.Client.Default.class))); } + @Test + public void testServiceId() { + assertNotNull("testClientServiceId was null", testClientServiceId); + final Hello hello = testClientServiceId.getHello(); + assertNotNull("The hello response was null", hello); + assertEquals("first hello didn't match", new Hello("hello world 1"), hello); + } + @Data @AllArgsConstructor @NoArgsConstructor