Override default dismiss404 config with client-specific config. Fixes g…

…h-1005.
spring-cloud · Mar 21, 2024 · c0d8177 · c0d8177
1 parent d2a7d4f
commit c0d8177
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 31 deletions.
diff --git a/...nfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java b/...nfeign-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/...test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java b/...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,51 +69,58 @@ 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<String> 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<String, List<String>> headers();
+	}
 
-		@GetMapping("/params")
-		Map<String, String> params();
+	@FeignClient("testClientB")
+	public interface TestClientB extends TestClient {
 
 	}
 
-	@FeignClient("testClientB")
-	public interface TestClientB {
+	public interface TestClient {
 
 		@GetMapping("/headers")
 		Map<String, List<String>> headers();
 
 		@GetMapping("/params")
 		Map<String, String> params();
 
+		@GetMapping
+		ResponseEntity<String> 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<String, String> headersB(@RequestParam Map<String, String> params) {
 			return params;
 		}
 
+		@GetMapping
+		ResponseEntity<String> 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
@@ -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"