Skip to content

Commit

Permalink
Fix merging default headers and params. Fixes gh-1001.
Browse files Browse the repository at this point in the history
  • Loading branch information
OlgaMaciaszek committed Mar 20, 2024
1 parent 55a78d9 commit 0ee8de2
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 the original author or authors.
* Copyright 2013-2024 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.
Expand Down Expand Up @@ -324,19 +324,26 @@ protected void configureUsingProperties(FeignClientProperties.FeignClientConfigu

protected void configureDefaultRequestElements(FeignClientProperties.FeignClientConfiguration defaultConfig,
FeignClientProperties.FeignClientConfiguration clientConfig, Feign.Builder builder) {
Map<String, Collection<String>> defaultRequestHeaders = defaultConfig != null
? defaultConfig.getDefaultRequestHeaders() : new HashMap<>();
Map<String, Collection<String>> defaultRequestHeaders = new HashMap<>();
if (defaultConfig != null) {
defaultConfig.getDefaultRequestHeaders()
.forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
}
if (clientConfig != null) {
defaultRequestHeaders.putAll(clientConfig.getDefaultRequestHeaders());
clientConfig.getDefaultRequestHeaders().forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
}
if (!defaultRequestHeaders.isEmpty()) {
addDefaultRequestHeaders(defaultRequestHeaders, builder);
}

Map<String, Collection<String>> defaultQueryParameters = defaultConfig != null
? defaultConfig.getDefaultQueryParameters() : new HashMap<>();
Map<String, Collection<String>> defaultQueryParameters = new HashMap<>();
if (defaultConfig != null) {
defaultConfig.getDefaultQueryParameters()
.forEach((k, v) -> defaultQueryParameters.put(k, new ArrayList<>(v)));
}
if (clientConfig != null) {
defaultQueryParameters.putAll(clientConfig.getDefaultQueryParameters());
clientConfig.getDefaultQueryParameters()
.forEach((k, v) -> defaultQueryParameters.put(k, new ArrayList<>(v)));
}
if (!defaultQueryParameters.isEmpty()) {
addDefaultQueryParams(defaultQueryParameters, builder);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
* Copyright 2013-2024 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.openfeign;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.cloud.client.DefaultServiceInstance;
import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient;
import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients;
import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers;
import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Import;
import org.springframework.http.HttpHeaders;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

import static java.util.Map.entry;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;

/**
* Integration tests for {@link FeignClientFactoryBean}.
*
* @author Olga Maciaszek-Sharma
*/
@SpringBootTest(classes = FeignClientFactoryBeanIntegrationTests.Application.class,
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles("defaultstest")
class FeignClientFactoryBeanIntegrationTests {

@Autowired
TestClientA testClientA;

@Autowired
TestClientB testClientB;

@Test
void shouldProcessDefaultRequestHeadersPerClient() {
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")),
entry("x-custom-header", List.of("from client B")));
}

@Test
void shouldProcessDefaultQueryParamsPerClient() {
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"),
entry("customParam1", "from client B"));
}

@FeignClient("testClientA")
public interface TestClientA {

@GetMapping("/headers")
Map<String, List<String>> headers();

@GetMapping("/params")
Map<String, String> params();

}

@FeignClient("testClientB")
public interface TestClientB {

@GetMapping("/headers")
Map<String, List<String>> headers();

@GetMapping("/params")
Map<String, String> params();

}

@EnableAutoConfiguration
@EnableFeignClients(clients = {TestClientA.class, TestClientB.class})
@RestController
@LoadBalancerClients({@LoadBalancerClient(name = "testClientA", configuration = TestClientAConfiguration.class),
@LoadBalancerClient(name = "testClientB", configuration = TestClientBConfiguration.class)})
@RequestMapping
@Import(NoSecurityConfiguration.class)
protected static class Application {

@GetMapping(value = "/headers", produces = APPLICATION_JSON_VALUE)
public Map<String, List<String>> headers(@RequestHeader HttpHeaders headers) {
return new HashMap<>(headers);
}

@GetMapping(value = "/params", produces = APPLICATION_JSON_VALUE)
public Map<String, String> headersB(@RequestParam Map<String, String> params) {
return params;
}

}

// LoadBalancer with fixed server list for "testClientA" pointing to localhost
static class TestClientAConfiguration {

@LocalServerPort
private int port = 0;

@Bean
public ServiceInstanceListSupplier testClientAServiceInstanceListSupplier() {
return ServiceInstanceListSuppliers.from("testClientA",
new DefaultServiceInstance("local-1", "testClientA", "localhost", port, false));
}

}

// LoadBalancer with fixed server list for "testClientB" pointing to localhost
static class TestClientBConfiguration {

@LocalServerPort
private int port = 0;

@Bean
public ServiceInstanceListSupplier testClientBServiceInstanceListSupplier() {
return ServiceInstanceListSuppliers.from("testClientB",
new DefaultServiceInstance("local-1", "testClientB", "localhost", port, false));
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
spring:
cloud:
openfeign:
client:
config:
default:
logger-level: FULL
default-request-headers:
x-custom-header:
- "default"
x-custom-header-2:
- "2 from default"
default-query-parameters:
customParam1:
- "default"
customParam2:
- "2 from default"
testClientA:
default-request-headers:
x-custom-header:
- "from client A"
default-query-parameters:
customParam1:
- "from client A"
testClientB:
default-request-headers:
x-custom-header:
- "from client B"
default-query-parameters:
customParam1:
- "from client B"

0 comments on commit 0ee8de2

Please sign in to comment.