Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sending multiple headers or query parameters to Feign client without knowing names #1360

Closed
asarkar opened this issue Sep 23, 2016 · 11 comments

Comments

@asarkar
Copy link
Contributor

asarkar commented Sep 23, 2016

We've a use case where the request may contain any number of headers starting with a predefined prefix. These headers should then be sent with the Feign client request.
For example, assume that the prefix is X-MyCompany. The request may contain 2 headers X-MyCompany-A and X-MyCompany-B. We need to pass both to the Feign client.
Since Feign requires all headers to be explicitly specified, this is currently not possible. It'd be nice to be able to just send a ? extends Map<String, ? extends List<String>> to Feign and have it add all the entries to the request. The same can be done for query parameters, where the type would be ? extends Map<String, String>. Being able to send a HttpEntity is also acceptable.

Using RestTemplate, this is almost trivial to do. If Feign has to compete with RestTemplate, it has to be smarter than it currently is.

P.S. Posted a similar SO question.

Edit 9/23/16:
One attempt using @HeaderMap based on @ryanjbaxter suggestion:

@SpringCloudApplication
@EnableFeignClients
public class DemoApplication {
    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @FeignClient(name = "httpbin", url = "httpbin.org")
    public interface HttpBinClient {
        @RequestMapping(path = "/post", method = POST, produces = APPLICATION_JSON_VALUE)
        public String echo(@HeaderMap MultiValueMap<String, String> headers);
    }
}
@RunWith(SpringRunner.class)
@SpringBootTest
public class DemoApplicationTest {
    @Autowired
    private DemoApplication.HttpBinClient client;
    private final ObjectReader objectReader = new ObjectMapper()
            .enable(ACCEPT_SINGLE_VALUE_AS_ARRAY)
            .readerFor(Request.class);

    @Test
    public void testEcho() throws IOException {
        MultiValueMap<String, String> headers = new LinkedMultiValueMap<>();
        headers.put(ACCEPT, singletonList(APPLICATION_JSON_VALUE));
        String lang = ENGLISH.toLanguageTag();
        headers.put(ACCEPT_LANGUAGE, singletonList(lang));
        String response = client.echo(headers);
        System.out.println(response);
        Request request = objectReader.readValue(response);

        assertThat(request.getUrl(), is("http://httpbin.org/post"));
        MultiValueMap<String, String> requestHeaders = request.getHeaders();
        assertThat(requestHeaders.keySet(), hasSize(greaterThanOrEqualTo(2)));
        assertThat(requestHeaders, hasEntry(is(ACCEPT), containsInAnyOrder(APPLICATION_JSON_VALUE)));
        assertThat(requestHeaders, hasEntry(is(ACCEPT_LANGUAGE), containsInAnyOrder(lang)));
    }

    @Data
    @JsonIgnoreProperties(ignoreUnknown = true)
    public static class Request {
        Map<String, String> args;
        @JsonDeserialize(as = LinkedMultiValueMap.class)
        MultiValueMap<String, String> headers;
        String url;
    }
}
@ryanjbaxter
Copy link
Contributor

Please try to avoid posting the same question in multiple places.

Does my answer to your question on SO help?

@asarkar
Copy link
Contributor Author

asarkar commented Sep 23, 2016

Please try to avoid posting the same question in multiple places.

Those are different things. On SO, I'm looking for an answer. Here, I'm looking for a code change. Besides, I did mention it in my post.

Anyway, your answer in SO didn't help. I've updated my original post to include the code that shows this problem.

@spencergibb
Copy link
Member

spencergibb commented Sep 23, 2016

@HeaderMap is a feign annotation and we've (foolishly) decided support Spring MVC annotations.

If Feign has to compete with RestTemplate

Feign will never be as flexible as RestTemplate.

Looks like we don't support this. Looks like RequestHeaderParameterProcessor needs to be updated to support @RequestHeader.

RequestHeader docs:

If the method parameter is a Map<String, String>, MultiValueMap<String, String>,
or HttpHeaders then the map is populated with all header names and values.

@asarkar
Copy link
Contributor Author

asarkar commented Sep 23, 2016

@spencergibb Currently the @RequestHeader annotation requires a name or the annotation processor throws an exception at runtime. You'll probably need to change that too.
Also, consider the same use case for query params. Apart from dynamic headers/query params, this change will also keep the feign client small such that if there're many headers/query params, the method signature won't have to have 20 parameters.

@spencergibb
Copy link
Member

Any other features would need a separate issue.

@asarkar
Copy link
Contributor Author

asarkar commented Sep 23, 2016

RequestParamParameterProcessor and RequesHeaderParameterProcessor are in the same package and very much related. In fact, the code is very similar too. So is the use case ("I want to send a map, please use it, thanks"). I can create a separate issue if you want but that'll only be more paperwork.

@ryanjbaxter
Copy link
Contributor

Sorry about that forgot we are using spring mvc annotations

@asarkar
Copy link
Contributor Author

asarkar commented Sep 25, 2016

Created #1361

@mydata
Copy link

mydata commented Jun 8, 2018

@asarkar @spencergibb I am trying to use the Multi header feature
ApiResponse performApiCall( @RequestHeader MultiValueMap<String, String> headerMap, @RequestBody ApiRequest apiReq);

It gives me the error: RequestHeader.value() was empty on parameter 0, while starting the spring boot App and feign client is never being created. Any suggestions?

@asarkar
Copy link
Contributor Author

asarkar commented Jun 8, 2018

@mydata, suggestion is that you create a new ticket or SO question instead of piggybacking on a ticket closed more than a year ago.

@sheetal-hemrom
Copy link

@mydata how did you solve the issue?? I am facing the same empty on parameter issue when using RequestHeader and passing map. Help!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants