Plug.Test support for repeated request headers

[halostatue]

From a post I wrote on ElixirForum:

So, Conn.get_req_header/2 returns [binary()] and I’m trying to figure out how to properly test multiple instances of a header with Plug.Test. Using Conn.Test.put_req_header/3 twice overwrites the header, and doesn’t add the second instance of the header, and it accepts only a single binary() as the value.

I’ve looked at previous issues focussed on adding multiple instances of response headers, but I haven’t seen any issues addressing testing of multiple instances of request headers (where repeated headers are much more common).

Would a PR be entertained to add Plug.Test.add_req_header/3? It could be added to Plug.Conn, but that seems to be more ripe for a creeping scope request to add add_resp_header/3.

add_req_header/3 is fairly simple. The main functionality is %{conn | req_headers: [{key, value} | headers]}, but would obviously need to check %Conn{state: :sent} and would need to either make Plug.Conn.validate_req_header!/2 public or copy it and a whole host of other functions to Plug.Test.

[josevalim]

How do we add multiple response headers? I believe it is merge_response_headers? Whatever API we have there we should mirror here.

[halostatue]

Plug.Conn.merge_resp_headers/2 has a similar issue to put_resp_header, as it collapses headers down to a single instance value:

conn
|> put_resp_header("test", "123")
|> merge_resp_headers([{"test", "456"}, {"test", "789"}])
|> Map.get(:resp_headers)

# => [{"cache-control", "max-age=0, private, must-revalidate"}, {"test", "789"}]

On the other hand, prepend_resp_headers/2 seems to do the trick, but does them in a different order than I would personally expect:

conn
|> put_resp_header("test", "123")
|> prepend_resp_headers([{"test", "456"}, {"test", "789"}])
|> Map.get(:resp_headers)

# => [
  {"test", "456"},
  {"test", "789"},
  {"cache-control", "max-age=0, private, must-revalidate"},
  {"test", "123"}
]

At the same time, my add_req_header/3 implementation does the same thing (for the order that would be expected, it would need to be headers ++ [{key, value}]—which would have performance issues.

[josevalim]

So we can have prepend_req_headers and merge_req_headers with the same semantics. I don't think we should introduce a new verb. :) PRs welcome!

[halostatue]

Working on it.

The implementation of prepend_resp_headers does this: %{conn | resp_headers: headers ++ resp_headers}. Would it be worthwhile adding append_resp_headers and append_req_headers that does the opposite?

[josevalim]

No plans for new verbs for now, unless it is strongly needed.