Browse Source

Actually fix the feign client validation issue

We need to still assert that the FeignClient is valid (no name and
value specified together for instance). Added missing tests.
pull/6/head
Dave Syer 9 years ago
parent
commit
afb43c5883
  1. 9
      spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java
  2. 47
      spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java

9
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java

@ -155,8 +155,6 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, @@ -155,8 +155,6 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar,
Map<String, Object> attributes = annotationMetadata
.getAnnotationAttributes(
FeignClient.class.getCanonicalName());
// Spring 4.2 didn't do this for us. With 4.3 it's idempotent.
attributes = AnnotationAttributes.fromMap(attributes);
String name = getClientName(attributes);
registerClientConfiguration(registry, name,
@ -192,10 +190,9 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, @@ -192,10 +190,9 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar,
}
private void validate(Map<String, Object> attributes) {
if (StringUtils.hasText((String) attributes.get("value"))) {
Assert.isTrue(!StringUtils.hasText((String) attributes.get("serviceId")),
"Either name (serviceId) or value can be specified, but not both");
}
AnnotationAttributes annotation = AnnotationAttributes.fromMap(attributes);
// This blows up if an aliased property is overspecified
annotation.getAliasedString("name", FeignClient.class, null);
}
private String getName(Map<String, Object> attributes) {

47
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java

@ -39,6 +39,50 @@ public class FeignClientValidationTests { @@ -39,6 +39,50 @@ public class FeignClientValidationTests {
@Rule
public ExpectedException expected = ExpectedException.none();
@Test
public void testNameAndValue() {
this.expected.expectMessage("only one is permitted");
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
NameAndValueConfiguration.class);
assertNotNull(context.getBean(NameAndValueConfiguration.Client.class));
context.close();
}
@Configuration
@Import(FeignAutoConfiguration.class)
@EnableFeignClients(clients = NameAndValueConfiguration.Client.class)
protected static class NameAndValueConfiguration {
@FeignClient(value = "foo", name = "bar")
interface Client {
@RequestMapping(method = RequestMethod.GET, value = "/")
String get();
}
}
@Test
public void testServiceIdAndValue() {
this.expected.expectMessage("only one is permitted");
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
NameAndValueConfiguration.class);
assertNotNull(context.getBean(NameAndServiceIdConfiguration.Client.class));
context.close();
}
@Configuration
@Import(FeignAutoConfiguration.class)
@EnableFeignClients(clients = NameAndServiceIdConfiguration.Client.class)
protected static class NameAndServiceIdConfiguration {
@FeignClient(serviceId = "foo", name = "bar")
interface Client {
@RequestMapping(method = RequestMethod.GET, value = "/")
String get();
}
}
@Test
public void testNotLegalHostname() {
this.expected.expectMessage("not legal hostname (foo_bar)");
@ -115,7 +159,8 @@ public class FeignClientValidationTests { @@ -115,7 +159,8 @@ public class FeignClientValidationTests {
return new Dummy();
}
class Dummy { }
class Dummy {
}
}
}

Loading…
Cancel
Save