-
Notifications
You must be signed in to change notification settings - Fork 120
feat: polymorphic helper for request body streaming #477
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,7 +126,7 @@ defmodule Mint.HTTP do | |
|
|
||
| @behaviour Mint.Core.Conn | ||
|
|
||
| @opaque t() :: Mint.HTTP1.t() | Mint.HTTP2.t() | ||
| @type t() :: Mint.HTTP1.t() | Mint.HTTP2.t() | ||
|
|
||
| defguardp is_data_message(message) | ||
| when elem(message, 0) in [:ssl, :tcp] and tuple_size(message) == 3 | ||
|
|
@@ -1065,6 +1065,12 @@ defmodule Mint.HTTP do | |
| @impl true | ||
| def put_proxy_headers(conn, headers), do: conn_apply(conn, :put_proxy_headers, [conn, headers]) | ||
|
|
||
| @impl true | ||
| def get_send_window(conn, ref), do: conn_apply(conn, :get_send_window, [conn, ref]) | ||
|
|
||
| @spec next_body_chunk_size(t(), term(), binary()) :: non_neg_integer() | ||
| def next_body_chunk_size(conn, ref, body), do: min(get_send_window(conn, ref), byte_size(body)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this function is unnecessary. |
||
|
|
||
| ## Helpers | ||
|
|
||
| defp conn_apply(%UnsafeProxy{}, fun, args), do: apply(UnsafeProxy, fun, args) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,7 +102,8 @@ defmodule Mint.HTTP1 do | |
| proxy_headers: [], | ||
| private: %{}, | ||
| log: false, | ||
| optional_responses: [] | ||
| optional_responses: [], | ||
| sndbuf: nil | ||
| ] | ||
|
|
||
| defmacrop log(conn, level, message) do | ||
|
|
@@ -204,6 +205,7 @@ defmodule Mint.HTTP1 do | |
| end | ||
|
|
||
| with :ok <- Util.inet_opts(transport, socket), | ||
| {:ok, sndbuf_opts} <- transport.getopts(socket, [:sndbuf]), | ||
| :ok <- if(mode == :active, do: transport.setopts(socket, active: :once), else: :ok) do | ||
| conn = %__MODULE__{ | ||
| transport: transport, | ||
|
|
@@ -216,7 +218,8 @@ defmodule Mint.HTTP1 do | |
| log: log?, | ||
| case_sensitive_headers: Keyword.get(opts, :case_sensitive_headers, false), | ||
| skip_target_validation: Keyword.get(opts, :skip_target_validation, false), | ||
| optional_responses: validate_optional_response_values(opts) | ||
| optional_responses: validate_optional_response_values(opts), | ||
| sndbuf: sndbuf_opts[:sndbuf] | ||
| } | ||
|
|
||
| {:ok, conn} | ||
|
|
@@ -667,6 +670,10 @@ defmodule Mint.HTTP1 do | |
| %{conn | proxy_headers: headers} | ||
| end | ||
|
|
||
| @impl true | ||
| def get_send_window(%__MODULE__{streaming_request: %{ref: ref}, sndbuf: sndbuf}, ref), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is semantically very different to what what the HTTP2 module does. There is no concept of send windows in HTTP/1 so we should not expose it on this module. |
||
| do: sndbuf | ||
|
|
||
| ## Helpers | ||
|
|
||
| defp decode(:status, %{request: request} = conn, data, responses) do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1039,6 +1039,10 @@ defmodule Mint.HTTP2 do | |
| %{conn | proxy_headers: headers} | ||
| end | ||
|
|
||
| @impl true | ||
| def get_send_window(%__MODULE__{} = conn, ref), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| do: min(get_window_size(conn, :connection), get_window_size(conn, {:request, ref})) | ||
|
|
||
| ## Helpers | ||
|
|
||
| defp handle_closed(conn) do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,44 @@ | ||
| defmodule Mint.HTTPTest do | ||
| use ExUnit.Case, async: true | ||
| doctest Mint.HTTP | ||
|
|
||
| alias Mint.{HTTP, HTTP1.TestServer} | ||
|
|
||
| setup do | ||
| {:ok, port, server_ref} = TestServer.start() | ||
| assert {:ok, conn} = HTTP.connect(:http, "localhost", port) | ||
| assert_receive {^server_ref, server_socket} | ||
|
|
||
| [conn: conn, server_socket: server_socket] | ||
| end | ||
|
|
||
| describe "get_send_window/2" do | ||
| test "returns a positive integer for an active streaming request", %{conn: conn} do | ||
| {:ok, conn, ref} = HTTP.request(conn, "GET", "/", [], :stream) | ||
|
|
||
| send_window = HTTP.get_send_window(conn, ref) | ||
| assert is_integer(send_window) | ||
| assert send_window > 0 | ||
| end | ||
| end | ||
|
|
||
| describe "next_body_chunk_size/3" do | ||
| test "returns body size when body is smaller than send window", %{conn: conn} do | ||
| {:ok, conn, ref} = HTTP.request(conn, "GET", "/", [], :stream) | ||
| small_body = "hello" | ||
| assert HTTP.next_body_chunk_size(conn, ref, small_body) == byte_size(small_body) | ||
| end | ||
|
|
||
| test "returns send window when body is larger than send window", %{conn: conn} do | ||
| {:ok, conn, ref} = HTTP.request(conn, "GET", "/", [], :stream) | ||
| send_window = HTTP.get_send_window(conn, ref) | ||
| large_body = :binary.copy(<<0>>, send_window + 1000) | ||
| assert HTTP.next_body_chunk_size(conn, ref, large_body) == send_window | ||
| end | ||
|
|
||
| test "returns 0 for empty body", %{conn: conn} do | ||
| {:ok, conn, ref} = HTTP.request(conn, "GET", "/", [], :stream) | ||
| assert HTTP.next_body_chunk_size(conn, ref, "") == 0 | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.