From 568bc47b3e61aae13e834bb946679050c95d97b6 Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Fri, 9 Oct 2015 12:47:04 -0500 Subject: [PATCH] Fix feign scanning that fails if clients attr is not use fixes gh-584 --- .../netflix/feign/FeignClientsRegistrar.java | 10 +- .../netflix/feign/valid/FeignClientTests.java | 20 ++-- .../scanning/FeignClientScanningTests.java | 110 ++++++++++++++++++ 3 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/scanning/FeignClientScanningTests.java 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 4fdb8529..61365b31 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 @@ -109,8 +109,11 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, Map attrs = metadata.getAnnotationAttributes( EnableFeignClients.class.getName()); AnnotationTypeFilter annotationTypeFilter = new AnnotationTypeFilter(FeignClient.class); - if (attrs != null && attrs.containsKey("clients")) { - final Class[] clients = (Class[]) attrs.get("clients"); + final Class[] clients = attrs == null ? null : (Class[]) attrs.get("clients"); + if (clients == null || clients.length == 0) { + scanner.addIncludeFilter(annotationTypeFilter); + basePackages = getBasePackages(metadata); + } else { final Set clientClasses = new HashSet<>(); basePackages = new HashSet<>(); for (Class clazz : clients) { @@ -125,9 +128,6 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, } }; scanner.addIncludeFilter(new AllTypeFilter(Arrays.asList(filter, annotationTypeFilter))); - } else { - scanner.addIncludeFilter(annotationTypeFilter); - basePackages = getBasePackages(metadata); } for (String basePackage : basePackages) { 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 dd6d3215..9d3b2e6a 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 @@ -247,18 +247,18 @@ public class FeignClientTests { public static class Hello { private String message; } -} -// Load balancer with fixed server list for "local" pointing to localhost -@Configuration -class LocalRibbonClientConfiguration { + // Load balancer with fixed server list for "local" pointing to localhost + @Configuration + public static class LocalRibbonClientConfiguration { - @Value("${local.server.port}") - private int port = 0; + @Value("${local.server.port}") + private int port = 0; - @Bean - public ServerList ribbonServerList() { - return new StaticServerList<>(new Server("localhost", this.port)); - } + @Bean + public ServerList ribbonServerList() { + return new StaticServerList<>(new Server("localhost", this.port)); + } + } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/scanning/FeignClientScanningTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/scanning/FeignClientScanningTests.java new file mode 100644 index 00000000..d516b15e --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/scanning/FeignClientScanningTests.java @@ -0,0 +1,110 @@ +/* + * Copyright 2013-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.feign.valid.scanning; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.netflix.ribbon.RibbonClient; +import org.springframework.cloud.netflix.ribbon.StaticServerList; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; + +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerList; + +import feign.Client; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = FeignClientScanningTests.Application.class) +@WebIntegrationTest(randomPort = true, value = { "spring.application.name=feignclienttest", + "feign.httpclient.enabled=false"}) +@DirtiesContext +public class FeignClientScanningTests { + + @Value("${local.server.port}") + private int port = 0; + + @Autowired + private TestClient testClient; + + @Autowired + private Client feignClient; + + @FeignClient("localapp") + protected interface TestClient { + @RequestMapping(method = RequestMethod.GET, value = "/hello") + String getHello(); + } + + @Configuration + @EnableAutoConfiguration + @RestController + @EnableFeignClients // NO clients attribute. That's what this class is testing! + @RibbonClient(name = "localapp", configuration = LocalRibbonClientConfiguration.class) + protected static class Application { + @RequestMapping(method = RequestMethod.GET, value = "/hello") + public String getHello() { + return "hello world 1"; + } + + public static void main(String[] args) { + new SpringApplicationBuilder(Application.class).properties( + "spring.application.name=feignclienttest", + "management.contextPath=/admin").run(args); + } + } + + @Test + public void testSimpleType() { + String hello = this.testClient.getHello(); + assertNotNull("hello was null", hello); + assertEquals("first hello didn't match", "hello world 1", hello); + } + + // Load balancer with fixed server list for "local" pointing to localhost + @Configuration + public static class LocalRibbonClientConfiguration { + + @Value("${local.server.port}") + private int port = 0; + + @Bean + public ServerList ribbonServerList() { + return new StaticServerList<>(new Server("localhost", this.port)); + } + + } +}