From 24d9b08d5c3de3f45cd15c139330fefb113f150c Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Mon, 17 Jan 2022 12:41:20 +0100 Subject: [PATCH] Backport bugfix and resolve conflicts. --- .../openfeign/FeignClientFactoryBean.java | 3 ++ .../openfeign/FeignClientsRegistrar.java | 6 ++- .../openfeign/FeignClientsRegistrarTests.java | 53 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) 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 ef9971ee..004b9a0d 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 @@ -443,6 +443,9 @@ public class FeignClientFactoryBean } private String cleanPath() { + if (path == null) { + return ""; + } String path = this.path.trim(); if (StringUtils.hasLength(path)) { if (!path.startsWith("/")) { 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 fa1495be..4fe4b8d8 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 @@ -302,7 +302,11 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLo if (resolver == null) { return resolved; } - return String.valueOf(resolver.evaluate(resolved, new BeanExpressionContext(beanFactory, null))); + Object evaluateValue = resolver.evaluate(resolved, new BeanExpressionContext(beanFactory, null)); + if (evaluateValue != null) { + return String.valueOf(evaluateValue); + } + return null; } return value; } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java index 483c5bb7..fc310985 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java @@ -18,6 +18,7 @@ package org.springframework.cloud.openfeign; import java.util.Collections; +import feign.Target; import org.junit.Test; import org.springframework.beans.factory.support.DefaultListableBeanFactory; @@ -25,15 +26,19 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Configuration; import org.springframework.mock.env.MockEnvironment; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.bind.annotation.GetMapping; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * @author Spencer Gibb * @author Gang Li * @author Michal Domagala + * @author Szymon Linowski + * @author Olga Maciaszek-Sharma */ public class FeignClientsRegistrarTests { @@ -101,6 +106,30 @@ public class FeignClientsRegistrarTests { .doesNotThrowAnyException(); } + @Test + public void shouldResolveNullUrl() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(NullUrlFeignClientTestConfig.class); + context.refresh(); + + Object feignClientBean = context.getBean(NullUrlFeignClient.class); + + Object invocationHandlerLambda = ReflectionTestUtils.getField(feignClientBean, "h"); + Target.HardCodedTarget target = (Target.HardCodedTarget) ReflectionTestUtils + .getField(invocationHandlerLambda, "arg$4"); + assertThat(target.name()).isEqualTo("nullUrlFeignClient"); + assertThat(target.url()).isEqualTo("http://nullUrlFeignClient"); + } + + @Test + public void shouldResolveAndValidateNullName() { + assertThatIllegalStateException().isThrownBy(() -> { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(NullExpressionNameFeignClientTestConfig.class); + context.refresh(); + }); + } + @FeignClient(name = "fallbackTestClient", url = "http://localhost:8080/", fallback = FallbackClient.class) protected interface FallbackClient { @@ -118,6 +147,16 @@ public class FeignClientsRegistrarTests { } + @FeignClient(name = "nullUrlFeignClient", url = "${test.url:#{null}}", path = "${test.path:#{null}}") + protected interface NullUrlFeignClient { + + } + + @FeignClient(name = "${test.name:#{null}}") + protected interface NullExpressionNameFeignClient { + + } + @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration @EnableFeignClients(clients = { FeignClientsRegistrarTests.FallbackClient.class }) @@ -138,4 +177,18 @@ public class FeignClientsRegistrarTests { } + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @EnableFeignClients(clients = NullUrlFeignClient.class) + protected static class NullUrlFeignClientTestConfig { + + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @EnableFeignClients(clients = NullExpressionNameFeignClient.class) + protected static class NullExpressionNameFeignClientTestConfig { + + } + }