diff --git a/CHANGELOG.md b/CHANGELOG.md index 41eb316..d12853d 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/assent.ex b/lib/assent.ex index 7d02a94..5fad63d 100644 --- a/lib/assent.ex +++ b/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) diff --git a/lib/assent/strategy.ex b/lib/assent/strategy.ex index b715cad..cded5c3 100644 --- a/lib/assent/strategy.ex +++ b/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 """ diff --git a/test/assent/strategy_test.exs b/test/assent/strategy_test.exs index bce9279..c6c4f34 100644 --- a/test/assent/strategy_test.exs +++ b/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{