From 00ea88c67f3e68ab42531628c9275a43a84b2f4e Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Mon, 5 Jul 2021 11:55:41 +0200 Subject: [PATCH 1/3] Fix typo. Refactor. --- .../openfeign/valid/FeignClientTests.java | 214 +++++++++--------- 1 file changed, 103 insertions(+), 111 deletions(-) diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java index 84831252..c747eda0 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java @@ -17,7 +17,6 @@ package org.springframework.cloud.openfeign.valid; import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.text.ParseException; import java.time.LocalDate; @@ -49,7 +48,6 @@ import feign.Feign; import feign.Logger; import feign.RequestInterceptor; import feign.RequestTemplate; -import feign.Target; import feign.codec.EncodeException; import feign.hystrix.FallbackFactory; import feign.hystrix.SetterFactory; @@ -181,80 +179,80 @@ public class FeignClientTests { @Test public void testClient() { - assertThat(this.testClient).as("testClient was null").isNotNull(); - assertThat(Proxy.isProxyClass(this.testClient.getClass())) - .as("testClient is not a java Proxy").isTrue(); - InvocationHandler invocationHandler = Proxy.getInvocationHandler(this.testClient); + assertThat(testClient).as("testClient was null").isNotNull(); + assertThat(Proxy.isProxyClass(testClient.getClass())) + .as("testClient is not a java Proxy").isTrue(); + InvocationHandler invocationHandler = Proxy.getInvocationHandler(testClient); assertThat(invocationHandler).as("invocationHandler was null").isNotNull(); } @Test public void testRequestMappingClassLevelPropertyReplacement() { - Hello hello = this.testClient.getHelloUsingPropertyPlaceHolder(); + Hello hello = testClient.getHelloUsingPropertyPlaceHolder(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match").isEqualTo(new Hello(OI_TERRA_2)); } @Test public void testSimpleType() { - Hello hello = this.testClient.getHello(); + Hello hello = testClient.getHello(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match") - .isEqualTo(new Hello(HELLO_WORLD_1)); + .isEqualTo(new Hello(HELLO_WORLD_1)); } @Test public void testOptional() { - Optional hello = this.testClient.getOptionalHello(); + Optional hello = testClient.getOptionalHello(); assertThat(hello).isNotNull().isPresent().contains(new Hello(HELLO_WORLD_1)); } @Test public void testGenericType() { - List hellos = this.testClient.getHellos(); + List hellos = testClient.getHellos(); assertThat(hellos).as("hellos was null").isNotNull(); assertThat(getHelloList()).as("hellos didn't match").isEqualTo(hellos); } @Test public void testRequestInterceptors() { - List headers = this.testClient.getHelloHeaders(); + List headers = testClient.getHelloHeaders(); assertThat(headers).as("headers was null").isNotNull(); assertThat(headers.contains("myheader1value")) - .as("headers didn't contain myheader1value").isTrue(); + .as("headers didn't contain myheader1value").isTrue(); assertThat(headers.contains("myheader2value")) - .as("headers didn't contain myheader2value").isTrue(); + .as("headers didn't contain myheader2value").isTrue(); } @Test public void testHeaderPlaceholders() { - String header = this.testClient.getHelloHeadersPlaceholders(); + String header = testClient.getHelloHeadersPlaceholders(); assertThat(header).as("header was null").isNotNull(); assertThat(header).as("header was wrong").isEqualTo("myPlaceholderHeaderValue"); } @Test - public void testFeignClientType() throws IllegalAccessException { - assertThat(this.feignClient).isInstanceOf(LoadBalancerFeignClient.class); - LoadBalancerFeignClient client = (LoadBalancerFeignClient) this.feignClient; + public void testFeignClientType() { + assertThat(feignClient).isInstanceOf(LoadBalancerFeignClient.class); + LoadBalancerFeignClient client = (LoadBalancerFeignClient) feignClient; Client delegate = client.getDelegate(); assertThat(delegate).isInstanceOf(Client.Default.class); } @Test public void testServiceId() { - assertThat(this.testClientServiceId).as("testClientServiceId was null") - .isNotNull(); - final Hello hello = this.testClientServiceId.getHello(); + assertThat(testClientServiceId).as("testClientServiceId was null") + .isNotNull(); + final Hello hello = testClientServiceId.getHello(); assertThat(hello).as("The hello response was null").isNotNull(); assertThat(hello).as("first hello didn't match") - .isEqualTo(new Hello(HELLO_WORLD_1)); + .isEqualTo(new Hello(HELLO_WORLD_1)); } @Test public void testParams() { List list = Arrays.asList("a", "1", "test"); - List params = this.testClient.getParams(list); + List params = testClient.getParams(list); assertThat(params).as("params was null").isNotNull(); assertThat(params.size()).as("params size was wrong").isEqualTo(list.size()); } @@ -262,24 +260,24 @@ public class FeignClientTests { @Test public void testFormattedParams() { List list = Arrays.asList(LocalDate.of(2001, 1, 1), - LocalDate.of(2018, 6, 10)); - List params = this.testClient.getFormattedParams(list); + LocalDate.of(2018, 6, 10)); + List params = testClient.getFormattedParams(list); assertThat(params).as("params was null").isNotNull(); assertThat(params).as("params not converted correctly").isEqualTo(list); } @Test public void testHystrixCommand() throws NoSuchMethodException { - HystrixCommand> command = this.testClient.getHellosHystrix(); + HystrixCommand> command = testClient.getHellosHystrix(); assertThat(command).as("command was null").isNotNull(); assertThat(command.getCommandGroup().name()).as( - "Hystrix command group name should match the name of the feign client") - .isEqualTo("localapp"); + "Hystrix command group name should match the name of the feign client") + .isEqualTo("localapp"); String configKey = Feign.configKey(TestClient.class, - TestClient.class.getMethod("getHellosHystrix", (Class[]) null)); + TestClient.class.getMethod("getHellosHystrix", (Class[]) null)); assertThat(command.getCommandKey().name()) - .as("Hystrix command key name should match the feign config key") - .isEqualTo(configKey); + .as("Hystrix command key name should match the feign config key") + .isEqualTo(configKey); List hellos = command.execute(); assertThat(hellos).as("hellos was null").isNotNull(); assertThat(getHelloList()).as("hellos didn't match").isEqualTo(hellos); @@ -287,171 +285,171 @@ public class FeignClientTests { @Test public void testSingle() { - Single single = this.testClient.getHelloSingle(); + Single single = testClient.getHelloSingle(); assertThat(single).as("single was null").isNotNull(); Hello hello = single.toBlocking().value(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match") - .isEqualTo(new Hello(HELLO_WORLD_1)); + .isEqualTo(new Hello(HELLO_WORLD_1)); } @Test public void testNoContentResponse() { - ResponseEntity response = this.testClient.noContent(); + ResponseEntity response = testClient.noContent(); assertThat(response).as("response was null").isNotNull(); assertThat(response.getStatusCode()).as("status code was wrong") - .isEqualTo(HttpStatus.NO_CONTENT); + .isEqualTo(HttpStatus.NO_CONTENT); } @Test public void testHeadResponse() { - ResponseEntity response = this.testClient.head(); + ResponseEntity response = testClient.head(); assertThat(response).as("response was null").isNotNull(); assertThat(response.getStatusCode()).as("status code was wrong") - .isEqualTo(HttpStatus.OK); + .isEqualTo(HttpStatus.OK); } @Test public void testHttpEntity() { - HttpEntity entity = this.testClient.getHelloEntity(); + HttpEntity entity = testClient.getHelloEntity(); assertThat(entity).as("entity was null").isNotNull(); Hello hello = entity.getBody(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match") - .isEqualTo(new Hello(HELLO_WORLD_1)); + .isEqualTo(new Hello(HELLO_WORLD_1)); } @Test public void testMoreComplexHeader() { - String response = this.testClient.moreComplexContentType("{\"value\":\"OK\"}"); + String response = testClient.moreComplexContentType("{\"value\":\"OK\"}"); assertThat(response).as("response was null").isNotNull(); assertThat(response).as("didn't respond with {\"value\":\"OK\"}") - .isEqualTo("{\"value\":\"OK\"}"); + .isEqualTo("{\"value\":\"OK\"}"); } @Test public void testDecodeNotFound() { - ResponseEntity response = this.decodingTestClient.notFound(); + ResponseEntity response = decodingTestClient.notFound(); assertThat(response).as("response was null").isNotNull(); assertThat(response.getStatusCode()).as("status code was wrong") - .isEqualTo(HttpStatus.NOT_FOUND); + .isEqualTo(HttpStatus.NOT_FOUND); assertThat(response.getBody()).as("response body was not null").isNull(); } @Test public void testOptionalNotFound() { - Optional s = this.decodingTestClient.optional(); + Optional s = decodingTestClient.optional(); assertThat(s).isNotPresent(); } @Test public void testConvertingExpander() { - assertThat(this.testClient.getToString(Arg.A)).isEqualTo(Arg.A.toString()); - assertThat(this.testClient.getToString(Arg.B)).isEqualTo(Arg.B.toString()); + assertThat(testClient.getToString(Arg.A)).isEqualTo(Arg.A.toString()); + assertThat(testClient.getToString(Arg.B)).isEqualTo(Arg.B.toString()); - assertThat(this.testClient.getToString(new OtherArg("foo"))).isEqualTo("bar"); + assertThat(testClient.getToString(new OtherArg("foo"))).isEqualTo("bar"); List args = new ArrayList<>(); args.add(new OtherArg("foo")); args.add(new OtherArg("goo")); List expectedResult = new ArrayList<>(); expectedResult.add("bar"); expectedResult.add("goo"); - assertThat(this.testClient.getToString(args)).isEqualTo(expectedResult); + assertThat(testClient.getToString(args)).isEqualTo(expectedResult); } @Test public void testHystrixFallbackWorks() { - Hello hello = this.hystrixClient.fail(); + Hello hello = hystrixClient.fail(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("message was wrong").isEqualTo("fallback"); } @Test public void testHystrixFallbackSingle() { - Single single = this.hystrixClient.failSingle(); + Single single = hystrixClient.failSingle(); assertThat(single).as("single was null").isNotNull(); Hello hello = single.toBlocking().value(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("message was wrong") - .isEqualTo("fallbacksingle"); + .isEqualTo("fallbacksingle"); } @Test public void testHystrixFallbackCommand() { - HystrixCommand command = this.hystrixClient.failCommand(); + HystrixCommand command = hystrixClient.failCommand(); assertThat(command).as("command was null").isNotNull(); Hello hello = command.execute(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("message was wrong") - .isEqualTo("fallbackcommand"); + .isEqualTo("fallbackcommand"); } @Test public void testHystrixFallbackObservable() { - Observable observable = this.hystrixClient.failObservable(); + Observable observable = hystrixClient.failObservable(); assertThat(observable).as("observable was null").isNotNull(); Hello hello = observable.toBlocking().first(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("message was wrong") - .isEqualTo("fallbackobservable"); + .isEqualTo("fallbackobservable"); } @Test public void testHystrixFallbackFuture() throws Exception { - Future future = this.hystrixClient.failFuture(); + Future future = hystrixClient.failFuture(); assertThat(future).as("future was null").isNotNull(); Hello hello = future.get(1, TimeUnit.SECONDS); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("message was wrong") - .isEqualTo("fallbackfuture"); + .isEqualTo("fallbackfuture"); } @Test public void testHystrixClientWithFallBackFactory() throws Exception { - Hello hello = this.hystrixClientWithFallBackFactory.fail(); + Hello hello = hystrixClientWithFallBackFactory.fail(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello.getMessage()).as("hello#message was null").isNotNull(); assertThat(hello.getMessage().contains("500")).as( - "hello#message did not contain the cause (status code) of the fallback invocation") - .isTrue(); + "hello#message did not contain the cause (status code) of the fallback invocation") + .isTrue(); } @Test(expected = HystrixRuntimeException.class) public void testInvalidTypeHystrixFallbackFactory() throws Exception { - this.invalidTypeHystrixClientWithFallBackFactory.fail(); + invalidTypeHystrixClientWithFallBackFactory.fail(); } @Test(expected = HystrixRuntimeException.class) public void testNullHystrixFallbackFactory() throws Exception { - this.nullHystrixClientWithFallBackFactory.fail(); + nullHystrixClientWithFallBackFactory.fail(); } @Test public void testFormURLEncoded() { Hello hello = new Hello(HELLO_WORLD_1); - Hello response = testClient.postToUrlEncoded(hello); + Hello response = testClient.postFormUrlEncoded(hello); assertThat(response).isEqualTo(hello); } @Test public void namedFeignClientWorks() { - assertThat(this.namedHystrixClient).as("namedHystrixClient was null").isNotNull(); + assertThat(namedHystrixClient).as("namedHystrixClient was null").isNotNull(); } @Test public void testHystrixSetterFactory() { - HystrixCommand> command = this.hystrixSetterFactoryClient - .getHellosHystrix(); + HystrixCommand> command = hystrixSetterFactoryClient + .getHellosHystrix(); assertThat(command).as("command was null").isNotNull(); String setterPrefix = TestHystrixSetterFactoryClientConfig.SETTER_PREFIX; assertThat(command.getCommandGroup().name()).as( - "Hystrix command group name should match the name of the feign client with a prefix of " - + setterPrefix) - .isEqualTo(setterPrefix + "localapp5"); + "Hystrix command group name should match the name of the feign client with a prefix of " + + setterPrefix) + .isEqualTo(setterPrefix + "localapp5"); assertThat(command.getCommandKey().name()).as( - "Hystrix command key name should match the request method (space) request path with a prefix of " - + setterPrefix) - .isEqualTo(setterPrefix + "GET /hellos"); + "Hystrix command key name should match the request method (space) request path with a prefix of " + + setterPrefix) + .isEqualTo(setterPrefix + "GET /hellos"); List hellos = command.execute(); assertThat(hellos).as("hellos was null").isNotNull(); assertThat(getHelloList()).as("hellos didn't match").isEqualTo(hellos); @@ -459,13 +457,13 @@ public class FeignClientTests { @Test public void testSingleRequestPart() { - String response = this.multipartClient.singlePart("abc"); + String response = multipartClient.singlePart("abc"); assertThat(response).isEqualTo("abc"); } @Test public void testSinglePojoRequestPart() { - String response = this.multipartClient.singlePojoPart(new Hello(HELLO_WORLD_1)); + String response = multipartClient.singlePojoPart(new Hello(HELLO_WORLD_1)); assertThat(response).isEqualTo(HELLO_WORLD_1); } @@ -473,7 +471,7 @@ public class FeignClientTests { public void testMultipleRequestParts() { MockMultipartFile file = new MockMultipartFile("file", "hello.bin", null, "hello".getBytes()); - String response = this.multipartClient.multipart("abc", "123", file); + String response = multipartClient.multipart("abc", "123", file); assertThat(response).isEqualTo("abc123hello.bin"); } @@ -483,7 +481,7 @@ public class FeignClientTests { Hello pojo2 = new Hello(OI_TERRA_2); MockMultipartFile file = new MockMultipartFile("file", "hello.bin", null, "hello".getBytes()); - String response = this.multipartClient.multipartPojo("abc", "123", pojo1, pojo2, + String response = multipartClient.multipartPojo("abc", "123", pojo1, pojo2, file); assertThat(response).isEqualTo("abc123hello world 1oi terra 2hello.bin"); } @@ -493,10 +491,10 @@ public class FeignClientTests { List multipartFiles = Arrays.asList( new MockMultipartFile("file1", "hello1.bin", null, "hello".getBytes()), new MockMultipartFile("file2", "hello2.bin", null, "hello".getBytes())); - String partNames = this.multipartClient + String partNames = multipartClient .requestPartListOfMultipartFilesReturnsPartNames(multipartFiles); assertThat(partNames).isEqualTo("files,files"); - String fileNames = this.multipartClient + String fileNames = multipartClient .requestPartListOfMultipartFilesReturnsFileNames(multipartFiles); assertThat(fileNames).contains("hello1.bin", "hello2.bin"); } @@ -509,7 +507,7 @@ public class FeignClientTests { "hello".getBytes()); MockMultipartFile file2 = new MockMultipartFile("file2", "hello2.bin", null, "hello".getBytes()); - String response = this.multipartClient + String response = multipartClient .requestPartListOfPojosAndListOfMultipartFiles( Arrays.asList(pojo1, pojo2), Arrays.asList(file1, file2)); assertThat(response).isEqualTo("hello world 1oi terra 2hello1.binhello2.bin"); @@ -520,7 +518,7 @@ public class FeignClientTests { String partName = UUID.randomUUID().toString(); MockMultipartFile file1 = new MockMultipartFile(partName, "hello1.bin", null, "hello".getBytes()); - String response = this.multipartClient.requestBodySingleMultipartFile(file1); + String response = multipartClient.requestBodySingleMultipartFile(file1); assertThat(response).isEqualTo(partName); } @@ -530,7 +528,7 @@ public class FeignClientTests { "hello".getBytes()); MockMultipartFile file2 = new MockMultipartFile("file2", "hello2.bin", null, "hello".getBytes()); - String response = this.multipartClient + String response = multipartClient .requestBodyListOfMultipartFiles(Arrays.asList(file1, file2)); assertThat(response).contains("file1", "file2"); } @@ -545,7 +543,7 @@ public class FeignClientTests { form.put("file1", file1); form.put("file2", file2); form.put("hello", "world"); - String response = this.multipartClient.requestBodyMap(form); + String response = multipartClient.requestBodyMap(form); assertThat(response).contains("file1", "file2", "hello"); } @@ -554,7 +552,7 @@ public class FeignClientTests { MockMultipartFile file = new MockMultipartFile("file1", "hello1.bin", null, "hello".getBytes()); expected.expectCause(instanceOf(EncodeException.class)); - this.multipartClient.invalid(file); + multipartClient.invalid(file); } protected enum Arg { @@ -626,8 +624,8 @@ public class FeignClientTests { String getToString(@RequestParam("arg") Arg arg); @PostMapping(path = "/form-urlencoded", - consumes = APPLICATION_FORM_URLENCODED_VALUE) - Hello postToUrlEncoded(Hello hello); + consumes = APPLICATION_FORM_URLENCODED_VALUE) + Hello postFormUrlEncoded(Hello hello); @RequestMapping(method = RequestMethod.GET, path = "/tostring2") String getToString(@RequestParam("arg") OtherArg arg); @@ -790,7 +788,7 @@ public class FeignClientTests { @Override public String toString() { - return this.value; + return value; } } @@ -824,13 +822,10 @@ public class FeignClientTests { @Override public HystrixClientWithFallBackFactory create(final Throwable cause) { - return new HystrixClientWithFallBackFactory() { - @Override - public Hello fail() { - assertThat(cause).isNotNull().as("Cause was null"); - return new Hello( - "Hello from the fallback side: " + cause.getMessage()); - } + return () -> { + assertThat(cause).isNotNull().as("Cause was null"); + return new Hello( + "Hello from the fallback side: " + cause.getMessage()); }; } @@ -890,18 +885,15 @@ public class FeignClientTests { @Bean public SetterFactory commandKeyIsRequestLineSetterFactory() { - return new SetterFactory() { - @Override - public HystrixCommand.Setter create(Target target, Method method) { - String groupKey = SETTER_PREFIX + target.name(); - RequestMapping requestMapping = method - .getAnnotation(RequestMapping.class); - String commandKey = SETTER_PREFIX + requestMapping.method()[0] + " " - + requestMapping.path()[0]; - return HystrixCommand.Setter - .withGroupKey(HystrixCommandGroupKey.Factory.asKey(groupKey)) - .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey)); - } + return (target, method) -> { + String groupKey = SETTER_PREFIX + target.name(); + RequestMapping requestMapping = method + .getAnnotation(RequestMapping.class); + String commandKey = SETTER_PREFIX + requestMapping.method()[0] + " " + + requestMapping.path()[0]; + return HystrixCommand.Setter + .withGroupKey(HystrixCommandGroupKey.Factory.asKey(groupKey)) + .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey)); }; } @@ -1169,8 +1161,8 @@ public class FeignClientTests { } @PostMapping(path = "/form-urlencoded", - consumes = APPLICATION_FORM_URLENCODED_VALUE) - Hello postToFormUrlEncoded(Hello hello) { + consumes = APPLICATION_FORM_URLENCODED_VALUE) + Hello postFormUrlEncoded(Hello hello) { return hello; } @@ -1188,7 +1180,7 @@ public class FeignClientTests { } public String getMessage() { - return this.message; + return message; } public void setMessage(String message) { @@ -1204,12 +1196,12 @@ public class FeignClientTests { return false; } Hello that = (Hello) o; - return Objects.equals(this.message, that.message); + return Objects.equals(message, that.message); } @Override public int hashCode() { - return Objects.hash(this.message); + return Objects.hash(message); } } @@ -1233,7 +1225,7 @@ public class FeignClientTests { @Bean public ServerList ribbonServerList() { - return new StaticServerList<>(new Server("localhost", this.port)); + return new StaticServerList<>(new Server("localhost", port)); } } From baf7cf95f247d5b169fe7d11eeb04744cf0a35c6 Mon Sep 17 00:00:00 2001 From: Michal Domagala Date: Mon, 5 Jul 2021 12:43:59 +0200 Subject: [PATCH 2/3] Simplify Log.info logic (#556) --- .../cloud/openfeign/FeignClientFactoryBean.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 0b4c74d3..ac24747f 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 @@ -373,12 +373,9 @@ public class FeignClientFactoryBean implements FactoryBean, Initializing Feign.Builder builder = feign(context); if (!StringUtils.hasText(url)) { - if (url != null && LOG.isInfoEnabled()) { - LOG.info( - "The provided URL is empty. Will try picking an instance via load-balancing."); - } - else if (LOG.isDebugEnabled()) { - LOG.debug("URL not provided. Will use LoadBalancer."); + + if (LOG.isInfoEnabled()) { + LOG.info("For '" + name + "' URL not provided. Will try picking an instance via load-balancing."); } if (!name.startsWith("http")) { url = "http://" + name; From df1f982046fb7e36a0e1934f5a5729a63cff1db3 Mon Sep 17 00:00:00 2001 From: Michal Domagala Date: Mon, 5 Jul 2021 13:03:06 +0200 Subject: [PATCH 3/3] Fix for RetryableFeignBlockingLoadBalancerClient closes stream (#569) --- ...adBalancerResponseStatusCodeException.java | 48 +++++++++++++++++++ ...ryableFeignBlockingLoadBalancerClient.java | 8 ++-- ...eFeignBlockingLoadBalancerClientTests.java | 32 +++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerResponseStatusCodeException.java diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerResponseStatusCodeException.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerResponseStatusCodeException.java new file mode 100644 index 00000000..0e7017f8 --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerResponseStatusCodeException.java @@ -0,0 +1,48 @@ +/* + * Copyright 2013-2020 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 + * + * https://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.loadbalancer; + +import java.io.ByteArrayInputStream; +import java.net.URI; + +import feign.Response; + +import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; + +/** + * A {@link RetryableStatusCodeException} for {@link Response}s. + * + * @author Ryan Baxter + */ +public class LoadBalancerResponseStatusCodeException extends RetryableStatusCodeException { + + private final Response response; + + public LoadBalancerResponseStatusCodeException(String serviceId, Response response, byte[] body, URI uri) { + super(serviceId, response.status(), response, uri); + this.response = Response.builder() + .body(new ByteArrayInputStream(body), body.length) + .headers(response.headers()).reason(response.reason()) + .status(response.status()).request(response.request()).build(); + } + + @Override + public Response getResponse() { + return this.response; + } + +} diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java index d6bb1bf4..342bb534 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java @@ -36,7 +36,6 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRecoveryCallbac import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; -import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -46,6 +45,7 @@ import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; +import org.springframework.util.StreamUtils; /** * A {@link Client} implementation that provides Spring Retry support for requests @@ -112,9 +112,11 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { LOG.debug( String.format("Retrying on status code: %d", responseStatus)); } + byte[] byteArray = response.body() == null ? new byte[] {} + : StreamUtils.copyToByteArray(response.body().asInputStream()); response.close(); - throw new RetryableStatusCodeException(serviceId, responseStatus, - response, URI.create(request.url())); + throw new LoadBalancerResponseStatusCodeException(serviceId, response, + byteArray, URI.create(request.url())); } return response; }, new LoadBalancedRecoveryCallback() { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java index d6c6644d..ca175318 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java @@ -16,7 +16,10 @@ package org.springframework.cloud.openfeign.loadbalancer; +import java.io.BufferedInputStream; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.Collection; @@ -27,6 +30,7 @@ import java.util.Map; import feign.Client; import feign.Request; import feign.Response; +import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -113,6 +117,13 @@ class RetryableFeignBlockingLoadBalancerClientTests { return Response.builder().request(testRequest()).status(status).build(); } + private Response testResponse(int status, String body) { + // ByteArrayInputStream ignores close() and must be wrapped + InputStream reallyCloseable = new BufferedInputStream(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))); + return Response.builder().request(testRequest()).status(status).body(reallyCloseable, null).build(); + + } + @Test void shouldExecuteOriginalRequestIfInstanceNotFound() throws IOException { Request request = testRequest(); @@ -148,6 +159,27 @@ class RetryableFeignBlockingLoadBalancerClientTests { verify(delegate, times(2)).execute(any(), any()); } + @Test + void shouldExposeResponseBodyOnRetry() throws IOException { + properties.getRetryableStatusCodes().add(503); + Request request = testRequest(); + when(delegate.execute(any(), any())) + .thenReturn(testResponse(503, "foo"), testResponse(503, "foo")); + when(retryFactory.createRetryPolicy(any(), eq(loadBalancerClient))) + .thenReturn(new BlockingLoadBalancedRetryPolicy("test", + loadBalancerClient, properties)); + when(loadBalancerClient.reconstructURI(serviceInstance, + URI.create("http://test/path"))) + .thenReturn(URI.create("http://testhost:80/path")); + + Response response = feignBlockingLoadBalancerClient.execute(request, new Request.Options()); + + String bodyContent = IOUtils.toString(response.body().asReader(StandardCharsets.UTF_8)); + assertThat(bodyContent).isEqualTo("foo"); + } + + + @Test void shouldPassCorrectRequestToDelegate() throws IOException { Request request = testRequest();