Skip to content

Commit 0ee8de2

Browse files
OlgaMaciaszekOlgaMaciaszek
committed
Fix merging default headers and params. Fixes gh-1001.
1 parent 55a78d9 commit 0ee8de2

File tree

3 files changed

+201
-7
lines changed

3 files changed

+201
-7
lines changed

‎spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java

+14-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2023 the original author or authors.
2+
* Copyright 2013-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -324,19 +324,26 @@ protected void configureUsingProperties(FeignClientProperties.FeignClientConfigu
324324

325325
protected void configureDefaultRequestElements(FeignClientProperties.FeignClientConfiguration defaultConfig,
326326
FeignClientProperties.FeignClientConfiguration clientConfig, Feign.Builder builder) {
327-
Map<String, Collection<String>> defaultRequestHeaders = defaultConfig != null
328-
? defaultConfig.getDefaultRequestHeaders() : new HashMap<>();
327+
Map<String, Collection<String>> defaultRequestHeaders = new HashMap<>();
328+
if (defaultConfig != null) {
329+
defaultConfig.getDefaultRequestHeaders()
330+
.forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
331+
}
329332
if (clientConfig != null) {
330-
defaultRequestHeaders.putAll(clientConfig.getDefaultRequestHeaders());
333+
clientConfig.getDefaultRequestHeaders().forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
331334
}
332335
if (!defaultRequestHeaders.isEmpty()) {
333336
addDefaultRequestHeaders(defaultRequestHeaders, builder);
334337
}
335338

336-
Map<String, Collection<String>> defaultQueryParameters = defaultConfig != null
337-
? defaultConfig.getDefaultQueryParameters() : new HashMap<>();
339+
Map<String, Collection<String>> defaultQueryParameters = new HashMap<>();
340+
if (defaultConfig != null) {
341+
defaultConfig.getDefaultQueryParameters()
342+
.forEach((k, v) -> defaultQueryParameters.put(k, new ArrayList<>(v)));
343+
}
338344
if (clientConfig != null) {
339-
defaultQueryParameters.putAll(clientConfig.getDefaultQueryParameters());
345+
clientConfig.getDefaultQueryParameters()
346+
.forEach((k, v) -> defaultQueryParameters.put(k, new ArrayList<>(v)));
340347
}
341348
if (!defaultQueryParameters.isEmpty()) {
342349
addDefaultQueryParams(defaultQueryParameters, builder);

‎spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryBeanIntegrationTests.java

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright 2013-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.openfeign;
18+
19+
import java.util.HashMap;
20+
import java.util.List;
21+
import java.util.Map;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
27+
import org.springframework.boot.test.context.SpringBootTest;
28+
import org.springframework.boot.test.web.server.LocalServerPort;
29+
import org.springframework.cloud.client.DefaultServiceInstance;
30+
import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient;
31+
import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients;
32+
import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
33+
import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers;
34+
import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
35+
import org.springframework.context.annotation.Bean;
36+
import org.springframework.context.annotation.Import;
37+
import org.springframework.http.HttpHeaders;
38+
import org.springframework.test.context.ActiveProfiles;
39+
import org.springframework.web.bind.annotation.GetMapping;
40+
import org.springframework.web.bind.annotation.RequestHeader;
41+
import org.springframework.web.bind.annotation.RequestMapping;
42+
import org.springframework.web.bind.annotation.RequestParam;
43+
import org.springframework.web.bind.annotation.RestController;
44+
45+
import static java.util.Map.entry;
46+
import static org.assertj.core.api.Assertions.assertThat;
47+
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
48+
49+
/**
50+
* Integration tests for {@link FeignClientFactoryBean}.
51+
*
52+
* @author Olga Maciaszek-Sharma
53+
*/
54+
@SpringBootTest(classes = FeignClientFactoryBeanIntegrationTests.Application.class,
55+
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
56+
@ActiveProfiles("defaultstest")
57+
class FeignClientFactoryBeanIntegrationTests {
58+
59+
@Autowired
60+
TestClientA testClientA;
61+
62+
@Autowired
63+
TestClientB testClientB;
64+
65+
@Test
66+
void shouldProcessDefaultRequestHeadersPerClient() {
67+
assertThat(testClientA.headers()).isNotNull()
68+
.contains(entry("x-custom-header-2", List.of("2 from default")),
69+
entry("x-custom-header", List.of("from client A")));
70+
assertThat(testClientB.headers()).isNotNull()
71+
.contains(entry("x-custom-header-2", List.of("2 from default")),
72+
entry("x-custom-header", List.of("from client B")));
73+
}
74+
75+
@Test
76+
void shouldProcessDefaultQueryParamsPerClient() {
77+
assertThat(testClientA.params()).isNotNull()
78+
.contains(entry("customParam2", "2 from default"),
79+
entry("customParam1", "from client A"));
80+
assertThat(testClientB.params()).isNotNull()
81+
.contains(entry("customParam2", "2 from default"),
82+
entry("customParam1", "from client B"));
83+
}
84+
85+
@FeignClient("testClientA")
86+
public interface TestClientA {
87+
88+
@GetMapping("/headers")
89+
Map<String, List<String>> headers();
90+
91+
@GetMapping("/params")
92+
Map<String, String> params();
93+
94+
}
95+
96+
@FeignClient("testClientB")
97+
public interface TestClientB {
98+
99+
@GetMapping("/headers")
100+
Map<String, List<String>> headers();
101+
102+
@GetMapping("/params")
103+
Map<String, String> params();
104+
105+
}
106+
107+
@EnableAutoConfiguration
108+
@EnableFeignClients(clients = {TestClientA.class, TestClientB.class})
109+
@RestController
110+
@LoadBalancerClients({@LoadBalancerClient(name = "testClientA", configuration = TestClientAConfiguration.class),
111+
@LoadBalancerClient(name = "testClientB", configuration = TestClientBConfiguration.class)})
112+
@RequestMapping
113+
@Import(NoSecurityConfiguration.class)
114+
protected static class Application {
115+
116+
@GetMapping(value = "/headers", produces = APPLICATION_JSON_VALUE)
117+
public Map<String, List<String>> headers(@RequestHeader HttpHeaders headers) {
118+
return new HashMap<>(headers);
119+
}
120+
121+
@GetMapping(value = "/params", produces = APPLICATION_JSON_VALUE)
122+
public Map<String, String> headersB(@RequestParam Map<String, String> params) {
123+
return params;
124+
}
125+
126+
}
127+
128+
// LoadBalancer with fixed server list for "testClientA" pointing to localhost
129+
static class TestClientAConfiguration {
130+
131+
@LocalServerPort
132+
private int port = 0;
133+
134+
@Bean
135+
public ServiceInstanceListSupplier testClientAServiceInstanceListSupplier() {
136+
return ServiceInstanceListSuppliers.from("testClientA",
137+
new DefaultServiceInstance("local-1", "testClientA", "localhost", port, false));
138+
}
139+
140+
}
141+
142+
// LoadBalancer with fixed server list for "testClientB" pointing to localhost
143+
static class TestClientBConfiguration {
144+
145+
@LocalServerPort
146+
private int port = 0;
147+
148+
@Bean
149+
public ServiceInstanceListSupplier testClientBServiceInstanceListSupplier() {
150+
return ServiceInstanceListSuppliers.from("testClientB",
151+
new DefaultServiceInstance("local-1", "testClientB", "localhost", port, false));
152+
}
153+
154+
}
155+
156+
}

‎spring-cloud-openfeign-core/src/test/resources/application-defaultstest.yml

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
spring:
2+
cloud:
3+
openfeign:
4+
client:
5+
config:
6+
default:
7+
logger-level: FULL
8+
default-request-headers:
9+
x-custom-header:
10+
- "default"
11+
x-custom-header-2:
12+
- "2 from default"
13+
default-query-parameters:
14+
customParam1:
15+
- "default"
16+
customParam2:
17+
- "2 from default"
18+
testClientA:
19+
default-request-headers:
20+
x-custom-header:
21+
- "from client A"
22+
default-query-parameters:
23+
customParam1:
24+
- "from client A"
25+
testClientB:
26+
default-request-headers:
27+
x-custom-header:
28+
- "from client B"
29+
default-query-parameters:
30+
customParam1:
31+
- "from client B"

0 commit comments

Comments
 (0)