Skip to content
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

Add %Req.Response{}.trailers field #233

Merged
merged 4 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ jobs:
MIX_ENV: test
# TODO: Remove on Req 1.0
REQ_NOWARN_OUTPUT: true
# TODO: Remove on next finch
FINCH_REF: main
strategy:
fail-fast: false
matrix:
Expand Down
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ config :req, legacy_headers_as_lists: true

This legacy fallback will be removed on Req 1.0.

There are two other changes to headers in this release.

Header names are now case-insensitive in functions like
`Req.Response.get_header/2`.

Trailer headers, or more precisely trailer fields or simply trailers, are now stored
in a separate `trailers` field on the `%Req.Response{}` struct as long as you use Finch 0.17+.

### Add Request Body Streaming

Req v0.4 adds official support for request body streaming by setting the request body to an
Expand Down Expand Up @@ -134,6 +142,8 @@ resp.body #=> %File.Stream{}
Req headers are now stored internally downcased and all accessor functions
like [`Req.Response.get_header/2`] are downcasing the given header name.

* Add `trailers` field to [`Req.Response`] struct. This change requires Finch 0.17.

* Make `request.registered_options` internal representation private.

* Make `request.options` internal representation private.
Expand Down Expand Up @@ -721,6 +731,7 @@ See "Adapter" section in `Req.Request` module documentation for more information
[`Req.Request.fetch_option!/2`]: https://hexdocs.pm/req/Req.Request.html#fetch_option!/2
[`Req.Request.delete_option/2`]: https://hexdocs.pm/req/Req.Request.html#update_private/4

[`Req.Response`]: https://hexdocs.pm/req/Req.Response.html
[`Req.Response.get_header/2`]: https://hexdocs.pm/req/Req.Response.html#get_response/2
[`Req.Response.delete_header/2`]: https://hexdocs.pm/req/Req.Response.html#delete_header/2
[`Req.Response.update_private/4`]: https://hexdocs.pm/req/Req.Response.html#update_private/4
Expand Down
23 changes: 14 additions & 9 deletions lib/req/response.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ defmodule Req.Response do

* `:body` - the HTTP response body.

* `:trailers` - the HTTP response trailers. The trailer names must be downcased.

* `:private` - a map reserved for libraries and frameworks to use.
Prefix the keys with the name of your project to avoid any future
conflicts. Only accepts `t:atom/0` keys.
Expand All @@ -19,12 +21,14 @@ defmodule Req.Response do
status: non_neg_integer(),
headers: %{binary() => [binary()]},
body: binary() | term(),
trailers: %{binary() => [binary()]},
private: map()
}

defstruct status: 200,
headers: if(Req.MixProject.legacy_headers_as_lists?(), do: [], else: %{}),
body: "",
trailers: %{},
private: %{}

@doc """
Expand Down Expand Up @@ -55,15 +59,16 @@ defmodule Req.Response do
else
def new(options) do
options =
Map.take(options, [:status, :headers, :body])
|> Map.update(:headers, %{}, fn
map when is_map(map) ->
map

list when is_list(list) ->
Enum.reduce(list, %{}, fn {name, value}, acc ->
Map.update(acc, name, [value], &(&1 ++ [value]))
end)
Map.take(options, [:status, :headers, :body, :trailers])
|> Map.update(:headers, %{}, fn headers ->
Enum.reduce(headers, %{}, fn {name, value}, acc ->
Map.update(acc, name, List.wrap(value), &(&1 ++ List.wrap(value)))
end)
end)
|> Map.update(:trailers, %{}, fn trailers ->
Enum.reduce(trailers, %{}, fn {name, value}, acc ->
Map.update(acc, name, List.wrap(value), &(&1 ++ List.wrap(value)))
end)
end)

struct!(__MODULE__, options)
Expand Down
28 changes: 18 additions & 10 deletions lib/req/steps.ex
Original file line number Diff line number Diff line change
Expand Up @@ -735,11 +735,7 @@ defmodule Req.Steps do
{request, %{response | status: status}}

{:headers, fields}, {request, response} ->
fields =
Enum.reduce(fields, %{}, fn {name, value}, acc ->
Map.update(acc, name, [value], &(&1 ++ [value]))
end)

fields = finch_fields_to_map(fields)
response = update_in(response.headers, &Map.merge(&1, fields))
{request, response}

Expand All @@ -755,6 +751,11 @@ defmodule Req.Steps do
raise ArgumentError,
"expected {:cont, acc} or {:halt, acc}, got: #{inspect(other)}"
end

{:trailers, fields}, {request, response} ->
fields = finch_fields_to_map(fields)
response = update_in(response.trailers, &Map.merge(&1, fields))
{request, response}
end

try do
Expand Down Expand Up @@ -786,17 +787,18 @@ defmodule Req.Steps do
{acc, request, %{response | status: status}}

{:headers, fields}, {acc, request, response} ->
fields =
Enum.reduce(fields, %{}, fn {name, value}, acc ->
Map.update(acc, name, [value], &(&1 ++ [value]))
end)

fields = finch_fields_to_map(fields)
response = update_in(response.headers, &Map.merge(&1, fields))
{acc, request, response}

{:data, data}, {acc, request, response} ->
acc = collector.(acc, {:cont, data})
{acc, request, response}

{:trailers, fields}, {acc, request, response} ->
fields = finch_fields_to_map(fields)
response = update_in(response.trailers, &Map.merge(&1, fields))
{acc, request, response}
end

case Finch.stream(
Expand Down Expand Up @@ -824,6 +826,12 @@ defmodule Req.Steps do
end
end

defp finch_fields_to_map(fields) do
Enum.reduce(fields, %{}, fn {name, value}, acc ->
Map.update(acc, name, [value], &(&1 ++ [value]))
end)
end

defp finch_parse_message(ref, {ref, {:data, data}}) do
{:ok, [{:data, data}]}
end
Expand Down
13 changes: 9 additions & 4 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ defmodule Req.MixProject do
end

defp finch_opts do
if path = System.get_env("FINCH_PATH") do
[path: path]
else
[]
cond do
path = System.get_env("FINCH_PATH") ->
[path: path]

ref = System.get_env("FINCH_REF") ->
[github: "sneako/finch", ref: ref]

true ->
[]
end
end

Expand Down
24 changes: 20 additions & 4 deletions test/req/steps_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1618,14 +1618,22 @@
assert resp.status == 200
assert resp.headers["transfer-encoding"] == ["chunked"]
assert resp.headers["trailer"] == ["x-foo, x-bar"]
assert resp.headers["x-foo"] == ["foo"]
assert resp.headers["x-bar"] == ["bar"]

# TODO: Remove check on new Finch release
if Map.has_key?(%Finch.Response{}, :trailers) do
assert resp.trailers["x-foo"] == ["foo"]
assert resp.trailers["x-bar"] == ["bar"]
else
assert resp.headers["x-foo"] == ["foo"]
assert resp.headers["x-bar"] == ["bar"]
end

assert_receive {:data, "chunk1"}
assert_receive {:data, "chunk2"}
refute_receive _
end

test "into: fun with halt", %{bypass: bypass, url: url} do

Check failure on line 1636 in test/req/steps_test.exs

View workflow job for this annotation

GitHub Actions / test (1.13, 24.3.4.10)

test run_finch into: fun with halt (Req.StepsTest)
Bypass.expect(bypass, "GET", "/", fn conn ->
conn = Plug.Conn.send_chunked(conn, 200)
{:ok, conn} = Plug.Conn.chunk(conn, "foo")
Expand Down Expand Up @@ -1692,8 +1700,16 @@
assert resp.status == 200
assert resp.headers["transfer-encoding"] == ["chunked"]
assert resp.headers["trailer"] == ["x-foo, x-bar"]
assert resp.headers["x-foo"] == ["foo"]
assert resp.headers["x-bar"] == ["bar"]

# TODO: Remove check on new Finch release
if Map.has_key?(%Finch.Response{}, :trailers) do
assert resp.trailers["x-foo"] == ["foo"]
assert resp.trailers["x-bar"] == ["bar"]
else
assert resp.headers["x-foo"] == ["foo"]
assert resp.headers["x-bar"] == ["bar"]
end

assert resp.body == ["chunk1", "chunk2"]
end

Expand Down
Loading