From 750cb35e6afdff6e6a07ac28853df717a78e6696 Mon Sep 17 00:00:00 2001 From: sdoeringNew Date: Wed, 19 Sep 2018 15:37:51 +0200 Subject: [PATCH] Add builder for feign clients. (#56) Utilizes the logic from the registrar and the factory bean. The changes to them have been made minimal invasive. Fixes gh-42 --- .../cloud/openfeign/FeignClientBuilder.java | 93 +++++++++++ .../openfeign/FeignClientFactoryBean.java | 12 +- .../openfeign/FeignClientsRegistrar.java | 25 ++- .../openfeign/FeignClientBuilderTests.java | 158 ++++++++++++++++++ .../openfeign/beans/FeignClientTests.java | 37 +++- .../cloud/openfeign/beans/TestClient.java | 2 + 6 files changed, 320 insertions(+), 7 deletions(-) create mode 100644 spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java new file mode 100644 index 00000000..3c0908ad --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java @@ -0,0 +1,93 @@ +/* + * Copyright 2013-2018 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.openfeign; + +import org.springframework.context.ApplicationContext; + +/** + * A builder for creating Feign clients without using the {@link FeignClient} annotation. + *

+ * This builder builds the Feign client exactly like it would be created by using the + * {@link FeignClient} annotation. + * + * @author Sven Döring + */ +public class FeignClientBuilder { + + private final ApplicationContext applicationContext; + + public FeignClientBuilder(final ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + public Builder forType(final Class type, final String name) { + return new Builder<>(applicationContext, type, name); + } + + public static class Builder { + + private FeignClientFactoryBean feignClientFactoryBean; + + private Builder(final ApplicationContext applicationContext, final Class type, + final String name) { + this.feignClientFactoryBean = new FeignClientFactoryBean(); + + this.feignClientFactoryBean.setApplicationContext(applicationContext); + this.feignClientFactoryBean.setType(type); + this.feignClientFactoryBean.setName(FeignClientsRegistrar.getName(name)); + // preset default values - these values resemble the default values on the + // FeignClient annotation + this.url("").path("").decode404(false).fallback(void.class) + .fallbackFactory(void.class); + } + + public Builder url(final String url) { + this.feignClientFactoryBean.setUrl(FeignClientsRegistrar.getUrl(url)); + return this; + } + + public Builder path(final String path) { + this.feignClientFactoryBean.setPath(FeignClientsRegistrar.getPath(path)); + return this; + } + + public Builder decode404(final boolean decode404) { + this.feignClientFactoryBean.setDecode404(decode404); + return this; + } + + public Builder fallback(final Class fallback) { + FeignClientsRegistrar.validateFallback(fallback); + this.feignClientFactoryBean.setFallback(fallback); + return this; + } + + public Builder fallbackFactory(final Class fallbackFactory) { + FeignClientsRegistrar.validateFallbackFactory(fallbackFactory); + this.feignClientFactoryBean.setFallbackFactory(fallbackFactory); + return this; + } + + /** + * @param the target type of the Feign client to be created + * @return the created Feign client + */ + public T build() { + return this.feignClientFactoryBean.getTarget(); + } + } +} diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java index f8186101..d8fab63f 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java @@ -229,6 +229,14 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, @Override public Object getObject() throws Exception { + return getTarget(); + } + + /** + * @param the target type of the Feign client + * @return a {@link Feign} client created with the specified data and the context information + */ + T getTarget() { FeignContext context = applicationContext.getBean(FeignContext.class); Feign.Builder builder = feign(context); @@ -241,7 +249,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, url = this.name; } url += cleanPath(); - return loadBalance(builder, context, new HardCodedTarget<>(this.type, + return (T) loadBalance(builder, context, new HardCodedTarget<>(this.type, this.name, url)); } if (StringUtils.hasText(this.url) && !this.url.startsWith("http")) { @@ -258,7 +266,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, builder.client(client); } Targeter targeter = get(context, Targeter.class); - return targeter.target(this, builder, context, new HardCodedTarget<>( + return (T) targeter.target(this, builder, context, new HardCodedTarget<>( this.type, this.name, url)); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java index 9427d431..7e458887 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java @@ -198,12 +198,19 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, AnnotationAttributes annotation = AnnotationAttributes.fromMap(attributes); // This blows up if an aliased property is overspecified // FIXME annotation.getAliasedString("name", FeignClient.class, null); + validateFallback(annotation.getClass("fallback")); + validateFallbackFactory(annotation.getClass("fallbackFactory")); + } + + static void validateFallback(final Class clazz) { Assert.isTrue( - !annotation.getClass("fallback").isInterface(), + !clazz.isInterface(), "Fallback class must implement the interface annotated by @FeignClient" ); - Assert.isTrue( - !annotation.getClass("fallbackFactory").isInterface(), + } + + static void validateFallbackFactory(final Class clazz) { + Assert.isTrue(!clazz.isInterface(), "Fallback factory must produce instances of fallback classes that implement the interface annotated by @FeignClient" ); } @@ -217,6 +224,10 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, name = (String) attributes.get("value"); } name = resolve(name); + return getName(name); + } + + static String getName(String name) { if (!StringUtils.hasText(name)) { return ""; } @@ -247,6 +258,10 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, private String getUrl(Map attributes) { String url = resolve((String) attributes.get("url")); + return getUrl(url); + } + + static String getUrl(String url) { if (StringUtils.hasText(url) && !(url.startsWith("#{") && url.contains("}"))) { if (!url.contains("://")) { url = "http://" + url; @@ -263,6 +278,10 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, private String getPath(Map attributes) { String path = resolve((String) attributes.get("path")); + return getPath(path); + } + + static String getPath(String path) { if (StringUtils.hasText(path)) { path = path.trim(); if (!path.startsWith("/")) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java new file mode 100644 index 00000000..cf750073 --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java @@ -0,0 +1,158 @@ +/* + * Copyright 2013-2018 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.openfeign; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.file.ClosedFileSystemException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.Mockito; +import org.springframework.cloud.openfeign.testclients.TestClient; +import org.springframework.context.ApplicationContext; +import org.springframework.util.ReflectionUtils; + +/** + * @author Sven Döring + */ +public class FeignClientBuilderTests { + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private FeignClientBuilder feignClientBuilder; + + private ApplicationContext applicationContext; + + private static Object getDefaultValueFromFeignClientAnnotation( + final String methodName) { + final Method method = ReflectionUtils.findMethod(FeignClient.class, methodName); + return method.getDefaultValue(); + } + + private static void assertFactoryBeanField(final FeignClientBuilder.Builder builder, + final String fieldName, final Object expectedValue) { + final Field factoryBeanField = ReflectionUtils + .findField(FeignClientBuilder.Builder.class, "feignClientFactoryBean"); + ReflectionUtils.makeAccessible(factoryBeanField); + final FeignClientFactoryBean factoryBean = (FeignClientFactoryBean) ReflectionUtils + .getField(factoryBeanField, builder); + + final Field field = ReflectionUtils.findField(FeignClientFactoryBean.class, + fieldName); + ReflectionUtils.makeAccessible(field); + final Object value = ReflectionUtils.getField(field, factoryBean); + Assert.assertEquals("Expected value for the field '" + fieldName + "':", + expectedValue, value); + } + + @Before + public void setUp() { + this.applicationContext = Mockito.mock(ApplicationContext.class); + this.feignClientBuilder = new FeignClientBuilder(applicationContext); + } + + @Test + public void safetyCheckForNewFieldsOnTheFeignClientAnnotation() { + final List methodNames = new ArrayList(); + for (final Method method : FeignClient.class.getMethods()) { + methodNames.add(method.getName()); + } + methodNames.removeAll( + Arrays.asList("annotationType", "value", "serviceId", "qualifier", + "configuration", "primary", "equals", "hashCode", "toString")); + Collections.sort(methodNames); + // If this safety check fails the Builder has to be updated. + // (1) Either a field was removed from the FeignClient annotation and so it has to + // be removed + // 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", + "fallbackFactory", "name", "path", "url")); + } + + @Test + public void forType_preinitializedBuilder() { + // when: + final FeignClientBuilder.Builder builder = feignClientBuilder + .forType(FeignClientBuilderTests.class, "TestClient"); + + // then: + assertFactoryBeanField(builder, "applicationContext", applicationContext); + assertFactoryBeanField(builder, "type", FeignClientBuilderTests.class); + assertFactoryBeanField(builder, "name", "TestClient"); + + // and: + assertFactoryBeanField(builder, "url", + getDefaultValueFromFeignClientAnnotation("url")); + assertFactoryBeanField(builder, "path", + getDefaultValueFromFeignClientAnnotation("path")); + assertFactoryBeanField(builder, "decode404", + getDefaultValueFromFeignClientAnnotation("decode404")); + assertFactoryBeanField(builder, "fallback", + getDefaultValueFromFeignClientAnnotation("fallback")); + assertFactoryBeanField(builder, "fallbackFactory", + getDefaultValueFromFeignClientAnnotation("fallbackFactory")); + } + + @Test + public void forType_allFieldsSetOnBuilder() { + // when: + final FeignClientBuilder.Builder builder = feignClientBuilder + .forType(FeignClientBuilderTests.class, "TestClient").decode404(true) + .fallback(Object.class).fallbackFactory(Object.class).path("Path/") + .url("Url/"); + + // then: + assertFactoryBeanField(builder, "applicationContext", applicationContext); + assertFactoryBeanField(builder, "type", FeignClientBuilderTests.class); + assertFactoryBeanField(builder, "name", "TestClient"); + + // and: + assertFactoryBeanField(builder, "url", "http://Url/"); + assertFactoryBeanField(builder, "path", "/Path"); + assertFactoryBeanField(builder, "decode404", true); + assertFactoryBeanField(builder, "fallback", Object.class); + assertFactoryBeanField(builder, "fallbackFactory", Object.class); + } + + @Test + public void forType_build() { + // given: + Mockito.when(applicationContext.getBean(FeignContext.class)) + .thenThrow(new ClosedFileSystemException()); // throw an unusual exception + // in the + // FeignClientFactoryBean + final FeignClientBuilder.Builder builder = feignClientBuilder + .forType(TestClient.class, "TestClient"); + + // expect: 'the build will fail right after calling build() with the mocked + // unusual exception' + thrown.expect(Matchers.isA(ClosedFileSystemException.class)); + builder.build(); + } +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/FeignClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/FeignClientTests.java index a2f9180f..ee45e43c 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/FeignClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/FeignClientTests.java @@ -31,8 +31,11 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.cloud.openfeign.EnableFeignClients; import org.springframework.cloud.openfeign.FeignClient; +import org.springframework.cloud.openfeign.FeignClientBuilder; import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.web.bind.annotation.RequestMapping; @@ -66,12 +69,24 @@ public class FeignClientTests { @Autowired private org.springframework.cloud.openfeign.beans.extra.TestClient extraClient; + @Qualifier("build-by-builder") + @Autowired + private TestClient buildByBuilder; + @Configuration @EnableAutoConfiguration @RestController @EnableFeignClients + @Import(FeignClientBuilder.class) protected static class Application { + @Bean("build-by-builder") + public TestClient buildByBuilder(final FeignClientBuilder feignClientBuilder) { + return feignClientBuilder + .forType(TestClient.class, "builderapp") + .build(); + } + @RequestMapping(method = RequestMethod.GET, value = "/hello") public Hello getHello() { return new Hello("hello world 1"); @@ -112,7 +127,7 @@ public class FeignClientTests { } @Test - public void testAnnnotations() throws Exception { + public void testAnnotations() { Map beans = this.context .getBeansWithAnnotation(FeignClient.class); assertTrue("Wrong clients: " + beans, @@ -122,13 +137,31 @@ public class FeignClientTests { @Test public void testClient() { assertNotNull("testClient was null", this.testClient); - assertNotNull("testClient was null", this.extraClient); + assertNotNull("extraClient was null", this.extraClient); assertTrue("testClient is not a java Proxy", Proxy.isProxyClass(this.testClient.getClass())); InvocationHandler invocationHandler = Proxy.getInvocationHandler(this.testClient); assertNotNull("invocationHandler was null", invocationHandler); } + @Test + public void extraClient() { + assertNotNull("extraClient was null", this.extraClient); + assertTrue("extraClient is not a java Proxy", + Proxy.isProxyClass(this.extraClient.getClass())); + InvocationHandler invocationHandler = Proxy.getInvocationHandler(this.extraClient); + assertNotNull("invocationHandler was null", invocationHandler); + } + + @Test + public void buildByBuilder() { + assertNotNull("buildByBuilder was null", this.buildByBuilder); + assertTrue("buildByBuilder is not a java Proxy", + Proxy.isProxyClass(this.buildByBuilder.getClass())); + InvocationHandler invocationHandler = Proxy.getInvocationHandler(this.buildByBuilder); + assertNotNull("invocationHandler was null", invocationHandler); + } + @Configuration public static class TestDefaultFeignConfig { } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/TestClient.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/TestClient.java index e00200e5..7f3ac65c 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/TestClient.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/beans/TestClient.java @@ -18,9 +18,11 @@ package org.springframework.cloud.openfeign.beans; import org.springframework.cloud.openfeign.FeignClient; import org.springframework.cloud.openfeign.beans.FeignClientTests.Hello; +import org.springframework.context.annotation.Primary; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +@Primary @FeignClient(value = "localapp") public interface TestClient { @RequestMapping(method = RequestMethod.GET, value = "/hello")