Refactor decode_response/2

CHANGELOG.md

@@ -10,6 +10,8 @@
 * `Assent.Strategy.Twitch` added
 * `Assent.Strategy.OAuth2` now supports PKCE
 * `Assent.Strategy.OAuth2.Base.authorize_url/2` incomplete typespec fixed
+* `Assent.Strategy.decode_response/2` deprecated accepting result tuples and now accepts `Assent.HTTPAdapter.HTTPResponse` structs
+
 
 ## v0.2.10 (2024-04-11)
 

lib/assent.ex

@@ -8,6 +8,10 @@ defmodule Assent do
   defmodule CallbackCSRFError do
     defexception [:key]
 
+    @type t :: %__MODULE__{
+      key: binary()
+    }
+
     def message(exception) do
       "CSRF detected with param key #{inspect(exception.key)}"
     end
@@ -16,6 +20,11 @@ defmodule Assent do
   defmodule MissingParamError do
     defexception [:expected_key, :params]
 
+    @type t :: %__MODULE__{
+      expected_key: binary(),
+      params: map()
+    }
+
     def message(exception) do
       expected_key = inspect(exception.expected_key)
       params = inspect(Map.keys(exception.params))
@@ -29,6 +38,11 @@ defmodule Assent do
 
     alias Assent.HTTPAdapter.HTTPResponse
 
+    @type t :: %__MODULE__{
+      message: binary(),
+      response: HTTPResponse.t()
+    }
+
     def message(exception) do
       """
       #{exception.message}
@@ -43,6 +57,10 @@ defmodule Assent do
 
     alias Assent.HTTPAdapter.HTTPResponse
 
+    @type t :: %__MODULE__{
+      response: HTTPResponse.t()
+    }
+
     def message(exception) do
       """
       An invalid response was received.
@@ -57,6 +75,10 @@ defmodule Assent do
 
     alias Assent.HTTPAdapter.HTTPResponse
 
+    @type t :: %__MODULE__{
+      response: HTTPResponse.t()
+    }
+
     def message(exception) do
       """
       An unexpected response was received.
@@ -69,6 +91,12 @@ defmodule Assent do
   defmodule ServerUnreachableError do
     defexception [:http_adapter, :request_url, :reason]
 
+    @type t :: %__MODULE__{
+      http_adapter: module(),
+      request_url: binary(),
+      reason: term()
+    }
+
     def message(exception) do
       [url | _rest] = String.split(exception.request_url, "?", parts: 2)
 

lib/assent/strategy.ex

@@ -26,7 +26,7 @@ defmodule Assent.Strategy do
         end
       end
   """
-  alias Assent.{Config, HTTPAdapter.HTTPResponse, ServerUnreachableError}
+  alias Assent.{Config, HTTPAdapter.HTTPResponse, InvalidResponseError, ServerUnreachableError}
 
   @callback authorize_url(Config.t()) ::
               {:ok, %{:url => binary(), optional(atom()) => any()}} | {:error, term()}
@@ -45,7 +45,7 @@ defmodule Assent.Strategy do
     |> http_adapter.request(url, body, headers, opts)
     |> case do
       {:ok, response} ->
-        decode_response({:ok, response}, config)
+        decode_response(response, config)
 
       {:error, error} ->
         {:error,
@@ -87,23 +87,35 @@ defmodule Assent.Strategy do
   end
 
   @doc """
-  Decodes a request response.
+  Decodes request response body.
   """
-  @spec decode_response(
-          {:ok, HTTPResponse.t()} | {:error, HTTPResponse.t()} | {:error, term()},
-          Config.t()
-        ) :: {:ok, HTTPResponse.t()} | {:error, HTTPResponse.t()} | {:error, term()}
-  def decode_response({ok_or_error, %{body: body, headers: headers} = resp}, config)
-      when is_binary(body) do
-    case decode_body(headers, body, config) do
-      {:ok, body} -> {ok_or_error, %{resp | body: body}}
+  @spec decode_response(HTTPResponse.t(), Config.t()) ::
+          {:ok, HTTPResponse.t()} | {:error, InvalidResponseError.t()}
+  def decode_response(%HTTPResponse{} = response, config) do
+    case decode(response.headers, response.body, config) do
+      {:ok, body} -> {:ok, %{response | body: body}}
+      {:error, _error} -> {:error, InvalidResponseError.exception(response: response)}
+    end
+  end
+
+  # TODO: Remove in 0.3 release
+  def decode_response({res, %HTTPResponse{} = response}, config) do
+    IO.warn("Passing {:ok | :error, response} to decode_response/2 is deprecated")
+
+    case decode(response.headers, response.body, config) do
+      {:ok, body} -> {res, %{response | body: body}}
       {:error, error} -> {:error, error}
     end
   end
 
-  def decode_response(any, _config), do: any
+  # TODO: Remove in 0.3 release
+  def decode_response({:error, error}, _config) do
+    IO.warn("Passing {:error, error} to decode_response/2 is deprecated")
 
-  defp decode_body(headers, body, config) do
+    {:error, error}
+  end
+
+  defp decode(headers, body, config) when is_binary(body) do
     case List.keyfind(headers, "content-type", 0) do
       {"content-type", "application/json" <> _rest} ->
         decode_json(body, config)
@@ -119,6 +131,8 @@ defmodule Assent.Strategy do
     end
   end
 
+  defp decode(_headers, body, _config), do: {:ok, body}
+
   @doc """
   Decode a JSON response to a map
   """

test/assent/strategy_test.exs

@@ -2,42 +2,62 @@ defmodule Assent.StrategyTest do
   use Assent.TestCase
   doctest Assent.Strategy
 
-  alias Assent.{HTTPAdapter.HTTPResponse, Strategy}
+  alias Assent.{HTTPAdapter.HTTPResponse, InvalidResponseError, Strategy}
 
-  test "decode_response/2" do
-    expected = %{"a" => "1", "b" => "2"}
-
-    headers = [{"content-type", "application/json"}]
-    body = @json_library.encode!(expected)
-
-    assert Strategy.decode_response({:ok, %{body: body, headers: headers}}, []) ==
-             {:ok, %{body: expected, headers: headers}}
-
-    assert Strategy.decode_response({:error, %{body: body, headers: headers}}, []) ==
-             {:error, %{body: expected, headers: headers}}
-
-    headers = [{"content-type", "application/json; charset=utf-8"}]
-
-    assert Strategy.decode_response({:ok, %{body: body, headers: headers}}, []) ==
-             {:ok, %{body: expected, headers: headers}}
-
-    headers = [{"content-type", "text/javascript"}]
-
-    assert Strategy.decode_response({:ok, %{body: body, headers: headers}}, []) ==
-             {:ok, %{body: expected, headers: headers}}
+  @body %{"a" => "1", "b" => "2"}
+  @headers [{"content-type", "application/json"}]
+  @json_encoded_body @json_library.encode!(@body)
+  @uri_encoded_body URI.encode_query(@body)
 
-    headers = [{"content-type", "application/x-www-form-urlencoded"}]
-    body = URI.encode_query(expected)
-
-    assert Strategy.decode_response({:ok, %{body: body, headers: headers}}, []) ==
-             {:ok, %{body: expected, headers: headers}}
-
-    headers = [{"content-type", "application/x-www-form-urlencoded; charset=utf-8"}]
-
-    assert Strategy.decode_response({:ok, %{body: body, headers: headers}}, []) ==
-             {:ok, %{body: expected, headers: headers}}
-
-    assert Strategy.decode_response({:error, "error reason"}, []) == {:error, "error reason"}
+  test "decode_response/2" do
+    assert {:ok, response} =
+             Strategy.decode_response(
+               %HTTPResponse{body: @json_encoded_body, headers: @headers},
+               []
+             )
+
+    assert response.body == @body
+
+    assert {:ok, response} =
+             Strategy.decode_response(
+               %HTTPResponse{
+                 body: @json_encoded_body,
+                 headers: [{"content-type", "application/json; charset=utf-8"}]
+               },
+               []
+             )
+
+    assert response.body == @body
+
+    assert {:ok, response} =
+             Strategy.decode_response(
+               %HTTPResponse{
+                 body: @json_encoded_body,
+                 headers: [{"content-type", "text/javascript"}]
+               },
+               []
+             )
+
+    assert response.body == @body
+
+    assert {:ok, response} =
+             Strategy.decode_response(
+               %HTTPResponse{
+                 body: @uri_encoded_body,
+                 headers: [{"content-type", "application/x-www-form-urlencoded"}]
+               },
+               []
+             )
+
+    assert response.body == @body
+
+    assert {:ok, response} = Strategy.decode_response(%HTTPResponse{body: @body, headers: []}, [])
+    assert response.body == @body
+
+    assert {:error, %InvalidResponseError{} = error} =
+             Strategy.decode_response(%HTTPResponse{body: "%", headers: @headers}, [])
+
+    assert error.response.body == "%"
   end
 
   defmodule JSONMock do
@@ -172,15 +192,9 @@ defmodule Assent.StrategyTest do
                 request_url: "json-encoded-body-text/javascript-header"
               }}
 
-    assert {:error, error} =
+    assert {:error, %InvalidResponseError{}} =
              Strategy.request(:get, "invalid-json-body", nil, [], http_adapter: HTTPMock)
 
-    if unquote(@json_library == Jason) do
-      assert %Jason.DecodeError{} = error
-    else
-      assert error == {:invalid_byte, 0, 37}
-    end
-
     assert Strategy.request(:get, "json-no-headers", nil, [], http_adapter: HTTPMock) ==
              {:ok,
               %HTTPResponse{