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 5ae1c6e0..fae7ec01 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 @@ -72,6 +72,7 @@ import org.springframework.util.StringUtils; * @author Hyeonmin Park * @author Felix Dittrich * @author Dominique Villard + * @athor Can Bezmen */ public class FeignClientFactoryBean implements FactoryBean, InitializingBean, ApplicationContextAware, BeanFactoryAware { @@ -448,7 +449,6 @@ public class FeignClientFactoryBean if (StringUtils.hasText(url) && !url.startsWith("http")) { url = "http://" + url; } - String url = this.url + cleanPath(); Client client = getOptional(feignClientFactory, Client.class); if (client != null) { if (client instanceof FeignBlockingLoadBalancerClient) { @@ -489,14 +489,14 @@ public class FeignClientFactoryBean @SuppressWarnings({ "unchecked", "rawtypes" }) private HardCodedTarget resolveTarget(FeignClientFactory context, String contextId, String url) { if (StringUtils.hasText(url)) { - return new HardCodedTarget(type, name, url); + return new HardCodedTarget(type, name, url + cleanPath()); } if (refreshableClient) { RefreshableUrl refreshableUrl = context.getInstance(contextId, RefreshableUrl.class.getCanonicalName() + "-" + contextId, RefreshableUrl.class); if (Objects.nonNull(refreshableUrl) && StringUtils.hasText(refreshableUrl.getUrl())) { - return new RefreshableHardCodedTarget<>(type, name, refreshableUrl); + return new RefreshableHardCodedTarget<>(type, name, refreshableUrl, cleanPath()); } } FeignClientProperties.FeignClientConfiguration config = findConfigByKey(contextId); @@ -505,7 +505,7 @@ public class FeignClientFactoryBean "Provide Feign client URL either in @FeignClient() or in config properties."); } - return new PropertyBasedTarget(type, name, config); + return new PropertyBasedTarget(type, name, config, cleanPath()); } private boolean isUrlAvailableInConfig(String contextId) { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/PropertyBasedTarget.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/PropertyBasedTarget.java index dd7b35ce..fdc471d2 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/PropertyBasedTarget.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/PropertyBasedTarget.java @@ -25,6 +25,7 @@ import feign.Target; * `spring.cloud.openfeign.client.config.[clientId].url`. * * @author Olga Maciaszek-Sharma + * @author Can Bezmen * @see FeignClientProperties.FeignClientConfiguration#getUrl() */ public class PropertyBasedTarget extends Target.HardCodedTarget { @@ -33,15 +34,25 @@ public class PropertyBasedTarget extends Target.HardCodedTarget { private final FeignClientProperties.FeignClientConfiguration config; + private final String path; + + public PropertyBasedTarget(Class type, String name, FeignClientProperties.FeignClientConfiguration config, + String path) { + super(type, name, config.getUrl()); + this.config = config; + this.path = path; + } + public PropertyBasedTarget(Class type, String name, FeignClientProperties.FeignClientConfiguration config) { super(type, name, config.getUrl()); this.config = config; + path = ""; } @Override public String url() { if (url == null) { - url = config.getUrl(); + url = config.getUrl() + path; } return url; } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/RefreshableHardCodedTarget.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/RefreshableHardCodedTarget.java index cd155b1f..a5cd3434 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/RefreshableHardCodedTarget.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/RefreshableHardCodedTarget.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -22,21 +22,32 @@ import feign.Target; * This target provides url wrapped under {@link Target}. * * @author Jasbir Singh + * @author Olga Maciaszek-Sharma * @since 4.0.0 */ public class RefreshableHardCodedTarget extends Target.HardCodedTarget { private final RefreshableUrl refreshableUrl; + private final String cleanPath; + @SuppressWarnings("unchecked") public RefreshableHardCodedTarget(Class type, String name, RefreshableUrl refreshableUrl) { super(type, name, refreshableUrl.getUrl()); this.refreshableUrl = refreshableUrl; + cleanPath = ""; + } + + @SuppressWarnings("unchecked") + public RefreshableHardCodedTarget(Class type, String name, RefreshableUrl refreshableUrl, String cleanPath) { + super(type, name, refreshableUrl.getUrl()); + this.refreshableUrl = refreshableUrl; + this.cleanPath = cleanPath; } @Override public String url() { - return refreshableUrl.getUrl(); + return refreshableUrl.getUrl() + cleanPath; } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/NonRefreshableFeignClientUrlTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/NonRefreshableFeignClientUrlTests.java index 1670e635..f9788c96 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/NonRefreshableFeignClientUrlTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/NonRefreshableFeignClientUrlTests.java @@ -34,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Jasbir Singh * @author Olga Maciaszek-Sharma + * @author Alex Elin */ @SpringBootTest @TestPropertySource("classpath:feign-properties.properties") @@ -56,12 +57,6 @@ class NonRefreshableFeignClientUrlTests { assertThat(response.getTargetType()).isEqualTo(Target.HardCodedTarget.class); } - @Test - void shouldInstantiateFeignClientWhenUrlFromFeignClientUrlGivenPreferenceOverProperties() { - UrlTestClient.UrlResponseForTests response = feignClientWithFixUrl.fixPath(); - assertThat(response.getUrl()).isEqualTo("http://localhost:8081/fixPath"); - } - @Test public void shouldInstantiateFeignClientWhenUrlFromProperties() { UrlTestClient.UrlResponseForTests response = configBasedClient.test(); @@ -76,11 +71,28 @@ class NonRefreshableFeignClientUrlTests { assertThat(response.getTargetType()).isEqualTo(Target.HardCodedTarget.class); } + @Test + void shouldInstantiateFeignClientWhenUrlAndPathAreInTheFeignClientAnnotation( + @Autowired Application.WithPathAndFixedUrlClient client) { + UrlTestClient.UrlResponseForTests response = client.test(); + assertThat(response.getUrl()).isEqualTo("http://localhost:7777/common/test"); + assertThat(response.getTargetType()).isEqualTo(Target.HardCodedTarget.class); + } + + @Test + void shouldInstantiateFeignClientWhenUrlFromPropertiesAndPathInTheFeignClientAnnotation( + @Autowired Application.WithPathAndUrlFromConfigClient client) { + UrlTestClient.UrlResponseForTests response = client.test(); + assertThat(response.getUrl()).isEqualTo("http://localhost:7777/common/test"); + assertThat(response.getTargetType()).isEqualTo(PropertyBasedTarget.class); + } + @Configuration @EnableAutoConfiguration @EnableConfigurationProperties(FeignClientProperties.class) @EnableFeignClients(clients = { Application.FeignClientWithFixUrl.class, Application.ConfigBasedClient.class, - Application.NameBasedUrlClient.class }) + Application.NameBasedUrlClient.class, Application.WithPathAndUrlFromConfigClient.class, + Application.WithPathAndFixedUrlClient.class }) protected static class Application { @Bean @@ -112,6 +124,22 @@ class NonRefreshableFeignClientUrlTests { } + @FeignClient(name = "withPathAndFixUrlClient", path = "/common", url = "http://localhost:7777") + protected interface WithPathAndFixedUrlClient { + + @GetMapping("/test") + UrlTestClient.UrlResponseForTests test(); + + } + + @FeignClient(name = "withPathAndUrlFromConfigClient", path = "/common") + protected interface WithPathAndUrlFromConfigClient { + + @GetMapping("/test") + UrlTestClient.UrlResponseForTests test(); + + } + } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/RefreshableFeignClientUrlTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/RefreshableFeignClientUrlTests.java index 836fa1f6..2b94f92d 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/RefreshableFeignClientUrlTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/RefreshableFeignClientUrlTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 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. @@ -24,7 +24,6 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.context.scope.refresh.RefreshScope; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.test.annotation.DirtiesContext; @@ -35,26 +34,24 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Jasbir Singh + * @author Olga Maciaszek-Sharma */ @SpringBootTest @TestPropertySource("classpath:feign-refreshable-properties.properties") @DirtiesContext class RefreshableFeignClientUrlTests { - @Autowired - private ApplicationContext applicationContext; - @Autowired private RefreshScope refreshScope; @Autowired - private RefreshableFeignClientUrlTests.Application.RefreshableClientWithFixUrl refreshableClientWithFixUrl; + private RefreshableFeignClientUrlTests.Application.RefreshableClientWithFixedUrl refreshableClientWithFixedUrl; @Autowired private RefreshableFeignClientUrlTests.Application.RefreshableUrlClient refreshableUrlClient; @Autowired - private Application.RefreshableUrlClientForContextRefreshCase refreshableUrlClientForContextRefreshCase; + private Application.RefreshableUrlClientForContextRefreshCase refreshableClientForContextRefreshCase; @Autowired private Application.NameBasedUrlClient nameBasedUrlClient; @@ -63,16 +60,18 @@ class RefreshableFeignClientUrlTests { private FeignClientProperties clientProperties; @Test - void shouldInstantiateFeignClientWhenUrlFromFeignClientUrl() { - UrlTestClient.UrlResponseForTests response = refreshableClientWithFixUrl.fixPath(); - assertThat(response.getUrl()).isEqualTo("http://localhost:8081/fixPath"); + void shouldInstantiateFeignClientWhenUrlFromAnnotation() { + UrlTestClient.UrlResponseForTests response = refreshableClientWithFixedUrl.fixedPath(); + assertThat(response.getUrl()).isEqualTo("http://localhost:8081/fixedPath"); assertThat(response.getTargetType()).isEqualTo(Target.HardCodedTarget.class); } @Test - void shouldInstantiateFeignClientWhenUrlFromFeignClientUrlGivenPreferenceOverProperties() { - UrlTestClient.UrlResponseForTests response = refreshableClientWithFixUrl.fixPath(); - assertThat(response.getUrl()).isEqualTo("http://localhost:8081/fixPath"); + void shouldInstantiateFeignClientWhenUrlAndPathFromAnnotation( + @Autowired Application.WithPathAndFixedUrlClient client) { + UrlTestClient.UrlResponseForTests response = client.test(); + assertThat(response.getUrl()).isEqualTo("http://localhost:7777/common/test"); + assertThat(response.getTargetType()).isEqualTo(Target.HardCodedTarget.class); } @Test @@ -84,15 +83,28 @@ class RefreshableFeignClientUrlTests { @Test void shouldInstantiateFeignClientWhenUrlFromPropertiesAndThenUpdateUrlWhenContextRefresh() { - UrlTestClient.UrlResponseForTests response = refreshableUrlClientForContextRefreshCase.refreshable(); + UrlTestClient.UrlResponseForTests response = refreshableClientForContextRefreshCase.refreshable(); assertThat(response.getUrl()).isEqualTo("http://localhost:8080/refreshable"); - clientProperties.getConfig().get("refreshableClient").setUrl("http://localhost:8888/"); + clientProperties.getConfig().get("refreshableClientForContextRefreshCase").setUrl("http://localhost:8888"); refreshScope.refreshAll(); - response = refreshableUrlClient.refreshable(); + response = refreshableClientForContextRefreshCase.refreshable(); assertThat(response.getUrl()).isEqualTo("http://localhost:8888/refreshable"); } + @Test + void shouldInstantiateFeignClientWhenUrlFromPropertiesAndPathFromAnnotationThenUpdateUrlWhenContextRefresh( + @Autowired Application.RefreshableUrlClientForContextRefreshCaseWithPath client) { + UrlTestClient.UrlResponseForTests response = client.refreshable(); + assertThat(response.getUrl()).isEqualTo("http://localhost:8080/common/refreshable"); + + clientProperties.getConfig().get("refreshableClientForContextRefreshCaseWithPath") + .setUrl("http://localhost:8888"); + refreshScope.refreshAll(); + response = client.refreshable(); + assertThat(response.getUrl()).isEqualTo("http://localhost:8888/common/refreshable"); + } + @Test void shouldInstantiateFeignClientWhenUrlFromFeignClientName() { UrlTestClient.UrlResponseForTests response = nameBasedUrlClient.nonRefreshable(); @@ -104,8 +116,9 @@ class RefreshableFeignClientUrlTests { @EnableAutoConfiguration @EnableConfigurationProperties(FeignClientProperties.class) @EnableFeignClients(clients = { Application.RefreshableUrlClient.class, Application.NameBasedUrlClient.class, - Application.RefreshableClientWithFixUrl.class, - Application.RefreshableUrlClientForContextRefreshCase.class }) + Application.RefreshableClientWithFixedUrl.class, + Application.RefreshableUrlClientForContextRefreshCase.class, Application.WithPathAndFixedUrlClient.class, + Application.RefreshableUrlClientForContextRefreshCaseWithPath.class }) protected static class Application { @Bean @@ -113,11 +126,11 @@ class RefreshableFeignClientUrlTests { return new UrlTestClient(); } - @FeignClient(name = "refreshableClientWithFixUrl", url = "http://localhost:8081") - protected interface RefreshableClientWithFixUrl { + @FeignClient(name = "refreshableClientWithFixedUrl", url = "http://localhost:8081") + protected interface RefreshableClientWithFixedUrl { - @GetMapping("/fixPath") - UrlTestClient.UrlResponseForTests fixPath(); + @GetMapping("/fixedPath") + UrlTestClient.UrlResponseForTests fixedPath(); } @@ -137,6 +150,14 @@ class RefreshableFeignClientUrlTests { } + @FeignClient(name = "refreshableClientForContextRefreshCaseWithPath", path = "/common") + protected interface RefreshableUrlClientForContextRefreshCaseWithPath { + + @GetMapping("/refreshable") + UrlTestClient.UrlResponseForTests refreshable(); + + } + @FeignClient(name = "nameBasedClient") protected interface NameBasedUrlClient { @@ -145,6 +166,14 @@ class RefreshableFeignClientUrlTests { } + @FeignClient(name = "withPathAndFixUrlClient", path = "/common", url = "http://localhost:7777") + protected interface WithPathAndFixedUrlClient { + + @GetMapping("/test") + UrlTestClient.UrlResponseForTests test(); + + } + } } diff --git a/spring-cloud-openfeign-core/src/test/resources/feign-properties.properties b/spring-cloud-openfeign-core/src/test/resources/feign-properties.properties index b4c6e9ef..305d2610 100644 --- a/spring-cloud-openfeign-core/src/test/resources/feign-properties.properties +++ b/spring-cloud-openfeign-core/src/test/resources/feign-properties.properties @@ -28,3 +28,4 @@ spring.cloud.openfeign.client.config.connectTimeout.connectTimeout=1000 spring.cloud.openfeign.client.config.default.followRedirects=false spring.cloud.openfeign.client.config.feignClientWithFixUrl.url=http://localhost:8888 spring.cloud.openfeign.client.config.configBasedClient.url=http://localhost:9999 +spring.cloud.openfeign.client.config.withPathAndUrlFromConfigClient.url=http://localhost:7777 diff --git a/spring-cloud-openfeign-core/src/test/resources/feign-refreshable-properties.properties b/spring-cloud-openfeign-core/src/test/resources/feign-refreshable-properties.properties index e79a9a6e..ef156736 100644 --- a/spring-cloud-openfeign-core/src/test/resources/feign-refreshable-properties.properties +++ b/spring-cloud-openfeign-core/src/test/resources/feign-refreshable-properties.properties @@ -11,3 +11,4 @@ spring.cloud.openfeign.client.config.readTimeout.readTimeout=2000 spring.cloud.openfeign.client.config.refreshableClient.url=http://localhost:8082 spring.cloud.openfeign.client.config.refreshableClientWithFixUrl.url=http://localhost:8888 spring.cloud.openfeign.client.config.refreshableClientForContextRefreshCase.url=http://localhost:8080 +spring.cloud.openfeign.client.config.refreshableClientForContextRefreshCaseWithPath.url=http://localhost:8080