-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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
Split new HttpHeaders(...) constructor into static factory methods HttpHeaders.backedBy(...) and HttpHeaders.copyOf(...) #34340
Labels
status: superseded
An issue that has been superseded by another
Comments
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 30, 2025
… on the constructor. Fixes spring-projects#34340
spring-projects-issues
added
the
status: waiting-for-triage
An issue we've not yet triaged or decided on
label
Jan 30, 2025
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 30, 2025
… on the constructor. Fixes spring-projects#34340
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 30, 2025
… on the constructor. Fixes spring-projects#34340
Let's discuss this change on the PR directly. |
bclozel
added
status: superseded
An issue that has been superseded by another
and removed
status: waiting-for-triage
An issue we've not yet triaged or decided on
labels
Jan 30, 2025
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 30, 2025
… on the constructor. Fixes spring-projects#34340 chore: Update constructor references with backedBy method. Signed-off-by: Bryce J. Fisher <[email protected]>
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 31, 2025
… on the constructor. chore: Update constructor references with backedBy method. chore: Remove `@Deprecated` annotations. Fixes spring-projects#34340 Signed-off-by: Bryce J. Fisher <[email protected]>
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 31, 2025
… on the constructor. chore: Update constructor references with backedBy method. chore: Remove `@Deprecated` annotations. Fixes spring-projects#34340 Signed-off-by: Bryce J. Fisher <[email protected]>
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 31, 2025
… on the constructor. chore: Update constructor references with backedBy method. chore: Remove `@Deprecated` annotations. Fixes spring-projects#34340 Signed-off-by: Bryce J. Fisher <[email protected]>
craftmaster2190
added a commit
to craftmaster2190/spring-framework
that referenced
this issue
Jan 31, 2025
… on the constructor. chore: Update constructor references with backedBy method. chore: Remove `@Deprecated` annotations. Fixes spring-projects#34340 Signed-off-by: Bryce J. Fisher <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Enhancement Request
Replace
new HttpHeaders(...)
constructor with static factory methodHttpHeaders.backedBy(...)
and add also a method calledHttpHeaders.copyOf(...)
.Use cases
There are already use cases for a
backedBy
method that behaves the way the constructor does now.Use cases for a
copyOf
method that creates a copy and is not backed by the previous map.defaultHeader(...)
methods on WebClient, RestTemplate, etc.)Background
Unless one is a seasoned Spring maintainer; the behavior of the HttpHeaders(...) constructor seems very surprising.
Take the following example code.
The above code is incorrect in that the second instance of HttpHeaders is not a true deep copy.
Changes to the copy are also changed in the first instance.
If (and that is a big if) the developer reads the documentation and HttpHeadersTest unit test;
they might be led to write:
However, this is not intuitive and also does not protect the original headers object from being mutated since ReadOnlyHttpHeaders does not make the underlying MultiValueMap read-only.
So then the developer writes this code since there is not a
copy
/clone
method.I've read the following related issues and I recognize there are historical reasons for why the constructor is the way it is. #28336 #33789 #32116
This comment stuck out:
This comment, while explaining the desired behavior, acknowledges the root of the surprise in that there is an implied norm given by collection-related constructors. For developers who have trained on collections, the conflicting behavior of HttpHeaders is disorienting.
I'd ask that the
new HttpHeaders(...)
constructors be deprecated, made private, and eventually fully replaced with static factory methodsHttpHeaders.backedBy(...)
andHttpHeaders.copyOf(...)
to remove any ambiguity, keep the desired behavior for adapting existing structures, and also provide the functionality of creating deep clones.The text was updated successfully, but these errors were encountered: