Add fix for explicit content-type header priority issue#1297
Conversation
| } | ||
|
|
||
| test("explicit Content-Type header should have priority over the codec") { | ||
| val ep = endpoint.out(stringBody and header("Content-Type", "text/csv")) |
There was a problem hiding this comment.
I think it also might be beneficial to
- move this definition to
tests/src/main/scala/sttp/tapir/tests/package.json(a reusable endpoint definition) - add a server test (in
ServerBasicTests) that when this endpoint is called, we get response with the correct content-type
There was a problem hiding this comment.
@adamw point 1. and 2. implemented, please review
| .out(stringBody) | ||
| .name("Query with default") | ||
|
|
||
| val out_overridden_content_type_header: Endpoint[Unit, Unit, String, Any] = |
There was a problem hiding this comment.
[minor, naming] this is in other places referred to as a fixed header - instead of overriden
| basicRequest.post(baseUri).send(backend).map(_.body shouldBe Right("")) | ||
| }, | ||
| testServer(out_overridden_content_type_header, "Overridden content-type header")((_: Unit) => pureResult("".asRight[Unit])) { (backend, baseUri) => | ||
| basicRequest.get(baseUri).send(backend).map(_.headers.map(h => h.name -> h.value).toSet should contain ("Content-Type" -> "text/csv")) |
There was a problem hiding this comment.
I'd rather run the .contains using a Header instance, as it properly checks headers using case-insentitive header name comparison
| RequestBody( | ||
| info.description, | ||
| codecToMediaType(codec, info.examples), | ||
| codecToMediaType(codec, info.examples, Option.empty), |
| .map(output => outputToResponses(output, ResponsesDefaultKey, None)) | ||
| .getOrElse(ListMap()) | ||
|
|
||
| private def extractForcedContentType(outputs: List[EndpointOutput[_]]): Option[String] = outputs match { |
There was a problem hiding this comment.
again, I'd call this extractFixedContentType
There was a problem hiding this comment.
another thing - outputs might be nested, e.g. using mapped pairs or simply pairs. The above will probably break given an output like .out(header("C-T", "...").and(header("another", "...")))
For this purposes, there's a traverseOutputs method which properly checks the entire structure
slabiakt
changed the title
| private def extractFixedContentType(outputs: List[EndpointOutput[_]]): Option[String] = { | ||
| outputs.flatMap(_.traverseOutputs { | ||
| case EndpointIO.FixedHeader(h, _, _) => | ||
| if (h.name.equalsIgnoreCase("Content-Type")) Vector(Option(h.value)) else Vector(None) |
There was a problem hiding this comment.
simpler: Vecotr(h.value) & Vector.empty. Then you just need to call .headOption on the result to get the first element (if any)
| private def extractFixedContentType(outputs: List[EndpointOutput[_]]): Option[String] = { | ||
| outputs.flatMap(_.traverseOutputs { | ||
| case EndpointIO.FixedHeader(h, _, _) => | ||
| if (h.name.equalsIgnoreCase("Content-Type")) Vector(h.value) else Vector.empty |
There was a problem hiding this comment.
you can use h.is for a case-insensitive check
| basicRequest.post(baseUri).send(backend).map(_.body shouldBe Right("")) | ||
| }, | ||
| testServer(out_fixed_content_type_header, "Fixed content-type header")((_: Unit) => pureResult("".asRight[Unit])) { (backend, baseUri) => | ||
| basicRequest.get(baseUri).send(backend).map(_.headers should contain only Header("Content-Type", "text/csv")) |
There was a problem hiding this comment.
the content-length header is also returned, so I think you'll have to drop the only in this test
|
Thanks! |
2 participants
Closes #1280