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 eed7c81c..38552632 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 @@ -448,6 +448,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 dc357d76..eb8eaa1a 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,24 +18,30 @@ package org.springframework.cloud.openfeign; import java.util.Collections; +import feign.Target; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledForJreRange; +import org.junit.jupiter.api.condition.JRE; import org.springframework.beans.factory.support.DefaultListableBeanFactory; 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.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * @author Spencer Gibb * @author Gang Li * @author Michal Domagala * @author Szymon Linowski + * @author Olga Maciaszek-Sharma */ class FeignClientsRegistrarTests { @@ -97,14 +103,39 @@ class FeignClientsRegistrarTests { @Test void shouldPassSubLevelFeignClient() { - AnnotationConfigApplicationContext config = new AnnotationConfigApplicationContext(); - ((DefaultListableBeanFactory) config.getBeanFactory()).setAllowBeanDefinitionOverriding(false); - config.register(TopLevelSubLevelTestConfig.class); - assertThatCode(() -> config.refresh()) + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + ((DefaultListableBeanFactory) context.getBeanFactory()).setAllowBeanDefinitionOverriding(false); + context.register(TopLevelSubLevelTestConfig.class); + assertThatCode(context::refresh) .as("Case https://github.com/spring-cloud/spring-cloud-openfeign/issues/331 should be solved") .doesNotThrowAnyException(); } + @Test + @DisabledForJreRange(min = JRE.JAVA_16) + 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$3"); + assertThat(target.name()).isEqualTo("nullUrlFeignClient"); + assertThat(target.url()).isEqualTo("http://nullUrlFeignClient"); + } + + @Test + 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 { @@ -122,6 +153,16 @@ 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 }) @@ -142,4 +183,18 @@ 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 { + + } + }