MockMvc request builders should expose HttpHeaders #35588
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
Signed-off-by: Clemens Nafe <clemens.nafe@otto.de>
mieseprem
force-pushed
the
fixes_#35576
branch
from
4095bc8 to
23d7684
Compare
| */ | ||
| public B header(String name, Object... values) { | ||
| this.headers.addAll(name, Arrays.asList(values)); | ||
| this.headers.addAll(name, Arrays.stream(values).map(String::valueOf).toList()); |
There was a problem hiding this comment.
The MultiValueMap<String, Object> matches to how MockHttpServletRequest manages header values, allowing Collections, arrays, as well as date and number values, which it then formats when headers are obtained. So this change is unlikely to go without causing regressions.
The code in this method could match the behavior of getDateHeader and getIntHeader of MockHttpServletRequest, in reverse by formatting to String values in the same way, to ensure backwards compatibility.
There was a problem hiding this comment.
To avoid heading in the wrong direction, I’ll briefly confirm what I’ve understood:
Working internally with HttpHeaders isn’t the best idea because of potential regressions. It’s better to continue using MultiValueMap internally.
So, if we still want to offer a method headers(Consumer<HttpHeaders>), we should consume the HttpHeaders and then call the existing headers(HttpHeaders) method. I can change it in that direction.
There was a problem hiding this comment.
So, if we still want to offer a method
headers(Consumer<HttpHeaders>), we should consume theHttpHeadersand then call the existingheaders(HttpHeaders)method.
That unfortunately does not work.
If you create a new instance of HttpHeaders and provide that to the Consumer<HttpHeaders>, the consumer can only add new headers. It cannot remove or process existing headers.
rstoyanchev
added
in: test
rstoyanchev
changed the title
Signed-off-by: Clemens Nafe <clemens.nafe@otto.de>
4 participants
Does it fix #35576 ?
If that's the case: here you go — enjoy