From c0d8177b782b62309e27d70114bc0a2272ea08f1 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Thu, 21 Mar 2024 14:35:13 +0100 Subject: [PATCH] Override default dismiss404 config with client-specific config. Fixes gh-1005. --- .../openfeign/FeignClientFactoryBean.java | 27 +++++---- ...eignClientFactoryBeanIntegrationTests.java | 59 ++++++++++++------- .../resources/application-defaultstest.yml | 2 + 3 files changed, 57 insertions(+), 31 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 3b768145a..e0d65e3a7 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 @@ -171,12 +171,12 @@ protected void configureFeign(FeignClientFactory context, Feign.Builder builder) if (properties != null && inheritParentContext) { if (properties.isDefaultToProperties()) { configureUsingConfiguration(context, builder); - configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); - configureUsingProperties(properties.getConfig().get(contextId), builder); + configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), + properties.getConfig().get(contextId), builder); } else { - configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); - configureUsingProperties(properties.getConfig().get(contextId), builder); + configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), + properties.getConfig().get(contextId), builder); configureUsingConfiguration(context, builder); } configureDefaultRequestElements(properties.getConfig().get(properties.getDefaultConfig()), @@ -249,6 +249,19 @@ protected void configureUsingConfiguration(FeignClientFactory context, Feign.Bui } } + protected void configureUsingProperties(FeignClientProperties.FeignClientConfiguration baseConfig, + FeignClientProperties.FeignClientConfiguration finalConfig, Feign.Builder builder) { + configureUsingProperties(baseConfig, builder); + configureUsingProperties(finalConfig, builder); + Boolean dismiss404 = finalConfig != null && finalConfig.getDismiss404() != null ? finalConfig.getDismiss404() + : (baseConfig != null && baseConfig.getDismiss404() != null ? baseConfig.getDismiss404() : null); + if (dismiss404 != null) { + if (dismiss404) { + builder.dismiss404(); + } + } + } + protected void configureUsingProperties(FeignClientProperties.FeignClientConfiguration config, Feign.Builder builder) { if (config == null) { @@ -291,12 +304,6 @@ protected void configureUsingProperties(FeignClientProperties.FeignClientConfigu builder.responseInterceptor(getOrInstantiate(config.getResponseInterceptor())); } - if (config.getDismiss404() != null) { - if (config.getDismiss404()) { - builder.dismiss404(); - } - } - if (Objects.nonNull(config.getEncoder())) { builder.encoder(getOrInstantiate(config.getEncoder())); } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java index 5091754d0..f8617684e 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; +import feign.FeignException; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -35,6 +36,8 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestHeader; @@ -44,6 +47,8 @@ import static java.util.Map.entry; 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.springframework.http.MediaType.APPLICATION_JSON_VALUE; /** @@ -52,7 +57,7 @@ * @author Olga Maciaszek-Sharma */ @SpringBootTest(classes = FeignClientFactoryBeanIntegrationTests.Application.class, - webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @ActiveProfiles("defaultstest") class FeignClientFactoryBeanIntegrationTests { @@ -64,37 +69,41 @@ class FeignClientFactoryBeanIntegrationTests { @Test void shouldProcessDefaultRequestHeadersPerClient() { - assertThat(testClientA.headers()).isNotNull() - .contains(entry("x-custom-header-2", List.of("2 from default")), + assertThat(testClientA.headers()).isNotNull().contains(entry("x-custom-header-2", List.of("2 from default")), entry("x-custom-header", List.of("from client A"))); - assertThat(testClientB.headers()).isNotNull() - .contains(entry("x-custom-header-2", List.of("2 from default")), + assertThat(testClientB.headers()).isNotNull().contains(entry("x-custom-header-2", List.of("2 from default")), entry("x-custom-header", List.of("from client B"))); } @Test void shouldProcessDefaultQueryParamsPerClient() { - assertThat(testClientA.params()).isNotNull() - .contains(entry("customParam2", "2 from default"), + assertThat(testClientA.params()).isNotNull().contains(entry("customParam2", "2 from default"), entry("customParam1", "from client A")); - assertThat(testClientB.params()).isNotNull() - .contains(entry("customParam2", "2 from default"), + assertThat(testClientB.params()).isNotNull().contains(entry("customParam2", "2 from default"), entry("customParam1", "from client B")); } + @Test + void shouldProcessDismiss404PerClient() { + assertThatExceptionOfType(FeignException.FeignClientException.class).isThrownBy(() -> testClientA.test404()); + assertThatCode(() -> { + ResponseEntity response404 = testClientB.test404(); + assertThat(response404.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response404.getBody()).isNull(); + }).doesNotThrowAnyException(); + } + @FeignClient("testClientA") - public interface TestClientA { + public interface TestClientA extends TestClient { - @GetMapping("/headers") - Map> headers(); + } - @GetMapping("/params") - Map params(); + @FeignClient("testClientB") + public interface TestClientB extends TestClient { } - @FeignClient("testClientB") - public interface TestClientB { + public interface TestClient { @GetMapping("/headers") Map> headers(); @@ -102,13 +111,16 @@ public interface TestClientB { @GetMapping("/params") Map params(); + @GetMapping + ResponseEntity test404(); + } @EnableAutoConfiguration - @EnableFeignClients(clients = {TestClientA.class, TestClientB.class}) + @EnableFeignClients(clients = { TestClientA.class, TestClientB.class }) @RestController - @LoadBalancerClients({@LoadBalancerClient(name = "testClientA", configuration = TestClientAConfiguration.class), - @LoadBalancerClient(name = "testClientB", configuration = TestClientBConfiguration.class)}) + @LoadBalancerClients({ @LoadBalancerClient(name = "testClientA", configuration = TestClientAConfiguration.class), + @LoadBalancerClient(name = "testClientB", configuration = TestClientBConfiguration.class) }) @RequestMapping @Import(NoSecurityConfiguration.class) protected static class Application { @@ -123,6 +135,11 @@ public Map headersB(@RequestParam Map params) { return params; } + @GetMapping + ResponseEntity test404() { + return ResponseEntity.notFound().build(); + } + } // LoadBalancer with fixed server list for "testClientA" pointing to localhost @@ -134,7 +151,7 @@ static class TestClientAConfiguration { @Bean public ServiceInstanceListSupplier testClientAServiceInstanceListSupplier() { return ServiceInstanceListSuppliers.from("testClientA", - new DefaultServiceInstance("local-1", "testClientA", "localhost", port, false)); + new DefaultServiceInstance("local-1", "testClientA", "localhost", port, false)); } } @@ -148,7 +165,7 @@ static class TestClientBConfiguration { @Bean public ServiceInstanceListSupplier testClientBServiceInstanceListSupplier() { return ServiceInstanceListSuppliers.from("testClientB", - new DefaultServiceInstance("local-1", "testClientB", "localhost", port, false)); + new DefaultServiceInstance("local-1", "testClientB", "localhost", port, false)); } } diff --git a/spring-cloud-openfeign-core/src/test/resources/application-defaultstest.yml b/spring-cloud-openfeign-core/src/test/resources/application-defaultstest.yml index 5e11b73a5..f3b466199 100644 --- a/spring-cloud-openfeign-core/src/test/resources/application-defaultstest.yml +++ b/spring-cloud-openfeign-core/src/test/resources/application-defaultstest.yml @@ -5,6 +5,7 @@ spring: config: default: logger-level: FULL + dismiss404: true default-request-headers: x-custom-header: - "default" @@ -16,6 +17,7 @@ spring: customParam2: - "2 from default" testClientA: + dismiss404: false default-request-headers: x-custom-header: - "from client A"