Skip to content

Conversation

@quaff
Copy link
Contributor

@quaff quaff commented Jan 10, 2025

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 10, 2025
@quaff quaff force-pushed the patch-95 branch 2 times, most recently from 639b156 to 6edea41 Compare January 10, 2025 09:45
@bclozel
Copy link
Member

bclozel commented Jan 16, 2025

Thanks for reaching out, but we'd like to discuss first this enhancement proposal.
Similar to #34227, I'm not sure why we should consider this use case for HTTP interfaces.

We already support @RequestHeader for a single header, or Map/MultivalueMap for many headers. The goal of HTTP interface services is to provide a Java interface for developers with a meaningful API. We don't want to turn this feature into another way to call HTTP clients.

Here, I don't see what HttpHeaders would bring here, besides getting/setting transport-specific headers or copying entire headers from another call. Both these use cases are wrong in my opinion.

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 16, 2025
@quaff
Copy link
Contributor Author

quaff commented Jan 16, 2025

Here, I don't see what HttpHeaders would bring here, besides getting/setting transport-specific headers or copying entire headers from another call. Both these use cases are wrong in my opinion.

I agree it's a bit redundant, but I think the semantic is more accurate compare to @RequestHeader MultiValueMap<String, ?>, IMO most developers may expect HttpHeaders is resolved as well as HttpMethod.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 16, 2025
@bclozel
Copy link
Member

bclozel commented Jan 16, 2025

I think that this would confuse developers, since HttpHeaders contains the entire set of headers to be sent, including "Content-Length", "Content-Type" and more. This begs the question: which headers should be sent as-is or overwritten by the client on the way out?

Thanks for the proposal, but I'm declining this enhancement.

@bclozel bclozel closed this Jan 16, 2025
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 16, 2025
@quaff
Copy link
Contributor Author

quaff commented Jan 16, 2025

I think that this would confuse developers, since HttpHeaders contains the entire set of headers to be sent, including "Content-Length", "Content-Type" and more.

@RequestHeader MultiValueMap<String, ?> may contains those headers also.

This begs the question: which headers should be sent as-is or overwritten by the client on the way out?

Agree.

@ciscoo
Copy link

ciscoo commented Jan 23, 2025

I just noticed this PR on my issue. Note that @RequestHeader HttpHeaders works as is today without this PR since it implements MultiValueMap which in turn is a Map, so it is already handled:

May or may not work as expected, but it works. 🤷

@bclozel
Copy link
Member

bclozel commented Jan 23, 2025

This won't work in the future, see #33913.

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

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants