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

Co-authored-by: Yordis Prieto <yordis.prieto@gmail.com>

guides/explanations/3.adapter.md

@@ -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

lib/tesla.ex

@@ -253,7 +253,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)
 
@@ -263,34 +263,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

lib/tesla/adapter/finch.ex

@@ -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)
 

lib/tesla/adapter/gun.ex

@@ -179,7 +179,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(

lib/tesla/adapter/hackney.ex

@@ -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)

lib/tesla/adapter/httpc.ex

@@ -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,

lib/tesla/adapter/ibrowse.ex

@@ -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,

lib/tesla/adapter/mint.ex

@@ -94,7 +94,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, %{})

lib/tesla/error.ex

@@ -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

lib/tesla/middleware/logger.ex

@@ -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)

test/support/adapter_case/basic.ex

@@ -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,

test/tesla/middleware/logger_test.exs

@@ -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")
@@ -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

test/tesla/middleware/retry_test.exs

@@ -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

test/tesla_test.exs

@@ -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
@@ -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
@@ -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