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

feat: allow to specify encoding strategy for query params #558

Merged
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: 1 addition & 1 deletion guides/explanations/3.adapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ defmodule Tesla.Adapter.Req do
@impl Tesla.Adapter
def run(env, _opts) do
req = Req.new(
url: Tesla.build_url(env.url, env.query),
url: Tesla.build_url(env),
method: env.method,
headers: env.headers,
body: env.body
Expand Down
62 changes: 48 additions & 14 deletions lib/tesla.ex
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ defmodule Tesla do
MyApi.get_something(client, 42)
```
"""
if Version.match?(System.version(), "~> 1.7"), do: @doc(since: "1.2.0")
@doc since: "1.2.0"
@spec client([Tesla.Client.middleware()], Tesla.Client.adapter()) :: Tesla.Client.t()
def client(middleware, adapter \\ nil), do: Tesla.Builder.client(middleware, [], adapter)

Expand All @@ -291,34 +291,68 @@ defmodule Tesla do
@deprecated "Use client/1 or client/2 instead"
def build_adapter(fun), do: Tesla.Builder.client([], [], fun)

@type encoding_strategy :: :rfc3986 | :www_form

@doc """
Builds URL with the given query params.
Builds URL with the given URL and query params.

Useful when you need to create a URL with dynamic query params from a Keyword
list

Useful when you need to create an URL with dynamic query params from a Keyword list
Allows to specify the `encoding` strategy to be one either `:www_form` or
`:rfc3986`. Read more about encoding at `URI.encode_query/2`.

- `url` - the base URL to which the query params will be appended.
- `query` - a list of key-value pairs to be encoded as query params.
- `encoding` - the encoding strategy to use. Defaults to `:www_form`

## Examples

iex> Tesla.build_url("http://api.example.com", [user: 3, page: 2])
"http://api.example.com?user=3&page=2"
iex> Tesla.build_url("https://api.example.com", [user: 3, page: 2])
"https://api.example.com?user=3&page=2"

URL that already contains query params:

# URL that already contains query params
iex> url = "http://api.example.com?user=3"
iex> url = "https://api.example.com?user=3"
iex> Tesla.build_url(url, [page: 2, status: true])
"http://api.example.com?user=3&page=2&status=true"
"https://api.example.com?user=3&page=2&status=true"

Default encoding `:www_form`:

iex> Tesla.build_url("https://api.example.com", [user_name: "John Smith"])
"https://api.example.com?user_name=John+Smith"

Specified encoding strategy `:rfc3986`:

iex> Tesla.build_url("https://api.example.com", [user_name: "John Smith"], :rfc3986)
"https://api.example.com?user_name=John%20Smith"
"""
@spec build_url(Tesla.Env.url(), Tesla.Env.query()) :: binary
def build_url(url, []), do: url
@spec build_url(Tesla.Env.url(), Tesla.Env.query(), encoding_strategy) :: binary
def build_url(url, query, encoding \\ :www_form)

def build_url(url, query) do
def build_url(url, [], _encoding), do: url

def build_url(url, query, encoding) do
join = if String.contains?(url, "?"), do: "&", else: "?"
url <> join <> encode_query(query)
url <> join <> encode_query(query, encoding)
end

@doc """
Builds a URL from the given `t:Tesla.Env.t/0` struct.

Combines the `url` and `query` fields, and allows specifying the `encoding`
strategy before calling `build_url/3`.
"""
@spec build_url(Tesla.Env.t()) :: String.t()
def build_url(%Tesla.Env{} = env) do
query_encoding = Keyword.get(env.opts, :query_encoding, :www_form)
Tesla.build_url(env.url, env.query, query_encoding)
end

def encode_query(query) do
def encode_query(query, encoding \\ :www_form) do
query
|> Enum.flat_map(&encode_pair/1)
|> URI.encode_query()
|> URI.encode_query(encoding)
end

@doc false
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/finch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ if Code.ensure_loaded?(Finch) do
opts = Tesla.Adapter.opts(@defaults, env, opts)

name = Keyword.fetch!(opts, :name)
url = Tesla.build_url(env.url, env.query)
url = Tesla.build_url(env)
req_opts = Keyword.take(opts, [:pool_timeout, :receive_timeout])
req = build(env.method, url, env.headers, env.body)

Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/gun.ex
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ if Code.ensure_loaded?(:gun) do
defp request(env, opts) do
request(
Tesla.Adapter.Shared.format_method(env.method),
Tesla.build_url(env.url, env.query),
Tesla.build_url(env),
format_headers(env.headers),
env.body || "",
Tesla.Adapter.opts(
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/hackney.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ if Code.ensure_loaded?(:hackney) do
defp request(env, opts) do
request(
env.method,
Tesla.build_url(env.url, env.query),
Tesla.build_url(env),
env.headers,
env.body,
Tesla.Adapter.opts(env, opts)
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/httpc.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ defmodule Tesla.Adapter.Httpc do
handle(
request(
env.method,
Tesla.build_url(env.url, env.query) |> to_charlist,
Tesla.build_url(env) |> to_charlist(),
Enum.map(env.headers, fn {k, v} -> {to_charlist(k), to_charlist(v)} end),
content_type,
env.body,
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/ibrowse.ex
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ if Code.ensure_loaded?(:ibrowse) do

handle(
request(
Tesla.build_url(env.url, env.query) |> to_charlist,
Tesla.build_url(env) |> to_charlist(),
env.headers,
env.method,
body,
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/adapter/mint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ if Code.ensure_loaded?(Mint.HTTP) do
defp request(env, opts) do
request(
format_method(env.method),
Tesla.build_url(env.url, env.query),
Tesla.build_url(env),
env.headers,
env.body,
Enum.into(opts, %{})
Expand Down
2 changes: 1 addition & 1 deletion lib/tesla/error.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ defmodule Tesla.Error do
defexception env: nil, stack: [], reason: nil

def message(%Tesla.Error{env: %{url: url, method: method}, reason: reason}) do
"#{inspect(reason)} (#{method |> to_string |> String.upcase()} #{url})"
"#{inspect(reason)} (#{method |> to_string() |> String.upcase()} #{url})"
end
end
7 changes: 6 additions & 1 deletion lib/tesla/middleware/logger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ defmodule Tesla.Middleware.Logger.Formatter do
Enum.map(format, &output(&1, request, response, time))
end

defp output(:query, env, _, _), do: env.query |> Tesla.encode_query()
defp output(:query, env, _, _) do
encoding = Keyword.get(env.opts, :query_encoding, :www_form)

Tesla.encode_query(env.query, encoding)
end

defp output(:method, env, _, _), do: env.method |> to_string() |> String.upcase()
defp output(:url, env, _, _), do: env.url
defp output(:status, _, {:ok, env}, _), do: to_string(env.status)
Expand Down
27 changes: 27 additions & 0 deletions test/support/adapter_case/basic.ex
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,33 @@ defmodule Tesla.AdapterCase.Basic do
assert args["user[age]"] == "20"
end

test "encoding query params with www_form by default" do
request = %Env{
method: :get,
url: "#{@http}/get",
query: [user_name: "John Smith"]
}

assert {:ok, %Env{} = response} = call(request)
assert {:ok, %Env{} = response} = Tesla.Middleware.JSON.decode(response, [])

assert response.body["url"] == "#{@http}/get?user_name=John+Smith"
end

test "encoding query params with rfc3986 optionally" do
request = %Env{
method: :get,
url: "#{@http}/get",
query: [user_name: "John Smith"],
opts: [query_encoding: :rfc3986]
}

assert {:ok, %Env{} = response} = call(request)
assert {:ok, %Env{} = response} = Tesla.Middleware.JSON.decode(response, [])

assert response.body["url"] == "#{@http}/get?user_name=John%20Smith"
end

test "autoredirects disabled by default" do
request = %Env{
method: :get,
Expand Down
16 changes: 15 additions & 1 deletion test/tesla/middleware/logger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Tesla.Middleware.LoggerTest do
defmodule Client do
use Tesla

plug Tesla.Middleware.Logger
plug Tesla.Middleware.Logger, format: "$query $url -> $status"

adapter fn env ->
env = Tesla.put_header(env, "content-type", "text/plain")
Expand Down Expand Up @@ -61,6 +61,20 @@ defmodule Tesla.Middleware.LoggerTest do
log = capture_log(fn -> Client.get("/ok") end)
assert log =~ "/ok -> 200"
end

test "default encoding strategy www_form" do
log = capture_log(fn -> Client.get("/ok", query: [test: "foo bar"]) end)
assert log =~ "test=foo+bar"
end

test "encodes with specified strategy" do
log =
capture_log(fn ->
Client.get("/ok", query: %{"test" => "foo bar"}, opts: [query_encoding: :rfc3986])
end)

assert log =~ "test=foo%20bar"
end
end

describe "Debug mode" do
Expand Down
2 changes: 1 addition & 1 deletion test/tesla/middleware/retry_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ defmodule Tesla.Middleware.RetryTest do
end

test "finally pass on laggy request" do
assert {:ok, %Tesla.Env{url: "/maybe", method: :get}} = Client.get("/maybe") |> dbg()
assert {:ok, %Tesla.Env{url: "/maybe", method: :get}} = Client.get("/maybe")
end

test "pass retry_count opt" do
Expand Down
60 changes: 43 additions & 17 deletions test/tesla_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ defmodule TeslaTest do
require Tesla

@url "http://localhost:#{Application.compile_env(:httparrot, :http_port)}"
@api_url "http://api.example.com"

describe "Adapters" do
defmodule ModuleAdapter do
Expand Down Expand Up @@ -280,38 +281,38 @@ defmodule TeslaTest do
end

describe "build_url/2" do
setup do
{:ok, url: "http://api.example.com"}
end

test "returns URL with query params from keyword list", %{url: url} do
test "returns URL with query params from keyword list" do
query_params = [user: 3, page: 2]
assert build_url(url, query_params) === url <> "?user=3&page=2"
assert build_url(@api_url, query_params) == @api_url <> "?user=3&page=2"
end

test "returns URL with query params from nested keyword list", %{url: url} do
test "returns URL with query params from nested keyword list" do
query_params = [nested: [more_nested: [argument: 1]]]
assert build_url(url, query_params) === url <> "?nested%5Bmore_nested%5D%5Bargument%5D=1"

assert build_url(@api_url, query_params) ==
@api_url <> "?nested%5Bmore_nested%5D%5Bargument%5D=1"
end

test "returns URL with query params from tuple list", %{url: url} do
test "returns URL with query params from tuple list" do
query_params = [{"user", 3}, {"page", 2}]
assert build_url(url, query_params) === url <> "?user=3&page=2"
assert build_url(@api_url, query_params) == @api_url <> "?user=3&page=2"
end

test "returns URL with query params from nested tuple list", %{url: url} do
test "returns URL with query params from nested tuple list" do
query_params = [{"nested", [{"more_nested", [{"argument", 1}]}]}]
assert build_url(url, query_params) === url <> "?nested%5Bmore_nested%5D%5Bargument%5D=1"

assert build_url(@api_url, query_params) ==
@api_url <> "?nested%5Bmore_nested%5D%5Bargument%5D=1"
end

test "returns URL with new query params concated from keyword list", %{url: url} do
url_with_param = url <> "?user=4"
test "returns URL with new query params concated from keyword list" do
url_with_param = @api_url <> "?user=4"
query_params = [page: 2, status: true]
assert build_url(url_with_param, query_params) === url <> "?user=4&page=2&status=true"
assert build_url(url_with_param, query_params) == @api_url <> "?user=4&page=2&status=true"
end

test "returns normal URL when query list is empty", %{url: url} do
assert build_url(url, []) == url
test "returns normal URL when query list is empty" do
assert build_url(@api_url, []) == @api_url
end

test "returns error when passing wrong params" do
Expand All @@ -323,4 +324,29 @@ defmodule TeslaTest do
end
end
end

describe "build_url/3" do
test "encoding www_form" do
query_params = [name: "foo bar", page: 2]
assert build_url(@api_url, query_params, :www_form) == @api_url <> "?name=foo+bar&page=2"
assert build_url(@api_url, query_params, :www_form) == build_url(@api_url, query_params)
end

test "encoding rfc3986" do
query_params = [name: "foo bar", page: 2]
assert build_url(@api_url, query_params, :rfc3986) == @api_url <> "?name=foo%20bar&page=2"
end

test "default encoding www_form" do
query_params = [name: "foo bar", page: 2]
assert build_url(@api_url, query_params, :www_form) == build_url(@api_url, query_params)
end
end

describe "build_url/1" do
test "returns URL with query params from Tesla.Env struct" do
env = %Tesla.Env{url: @api_url, query: [user: 3, page: 2]}
assert Tesla.build_url(env) == @api_url <> "?user=3&page=2"
end
end
end
Loading