-
Notifications
You must be signed in to change notification settings - Fork 20
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
Tesla Adapter #57
Open
jwarlander
wants to merge
18
commits into
inaka:master
Choose a base branch
from
jwarlander:tesla-adapter
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+310
−6
Open
Tesla Adapter #57
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
05ab12c
Make a working GET request with Tesla
jwarlander bd77999
Use custom JSON decoding due to special requirements
jwarlander 9006d2c
Add note about using Tesla for JSON decoding
jwarlander c20fbe6
Support query parameters for Tesla
jwarlander 967b4ce
Make sure that Tesla supports custom headers
jwarlander 2e4d786
Make sure that Tesla returns proper error response codes
jwarlander 74143b8
Translate Tesla errors to properly handle invalid server, etc
jwarlander 25fdfad
Encode to JSON with Tesla + use maps for headers
jwarlander 4a4e442
Make sure patch requests are properly handled
jwarlander b69bf5b
Convert empty response body to nil
jwarlander aff50ee
Use Tesla middleware for response translation
jwarlander b542bbe
Unify error translation and wrap all Tesla calls
jwarlander bd051db
Use Elixir 1.3 in CI, required for Tesla
jwarlander a903181
Update Tesla adapter docs + add remaining TODO items
jwarlander ec40041
Tesla always raises on error; don't handle {:error, _} results
jwarlander 3042954
Handle Hackney options for Tesla
jwarlander 10b0c50
Add test for passing a custom hackney option
jwarlander 5b68a56
Add information about Tesla adapter to the README
jwarlander File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
language: elixir | ||
elixir: | ||
- 1.2.3 | ||
- 1.3.4 | ||
otp_release: | ||
- 18.2.1 | ||
env: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
defmodule Dayron.TeslaAdapter do | ||
@moduledoc """ | ||
Makes http requests using Tesla library. | ||
Use this adapter to make http requests to an external Rest API. | ||
|
||
## Example config | ||
config :my_app, MyApp.Repo, | ||
adapter: Dayron.TeslaAdapter, | ||
url: "https://api.example.com" | ||
|
||
## TODO | ||
|
||
- Handle streaming | ||
""" | ||
@behaviour Dayron.Adapter | ||
|
||
defmodule Client do | ||
@moduledoc """ | ||
A Tesla Client implementation, sending json requests, parsing | ||
json responses to Maps or a List of Maps. Maps keys are also converted to | ||
atoms by default. | ||
""" | ||
use Tesla | ||
|
||
plug Tesla.Middleware.EncodeJson, engine: Poison | ||
plug Dayron.TeslaAdapter.Translator | ||
|
||
adapter Tesla.Adapter.Hackney | ||
end | ||
|
||
defmodule Translator do | ||
@moduledoc """ | ||
A Tesla Middleware implementation, translating responses to the format | ||
expected by Dayron. | ||
|
||
We're also doing JSON decoding of responses here, as the built-in JSON | ||
middleware in Tesla will only decode content-type `application/json`, as | ||
well as raise an error on decoding issues instead of returning the raw | ||
input. Both of these differences break the existing implicit contract, as | ||
implemented by `Dayron.HTTPoisonAdapter`. | ||
""" | ||
def call(env, next, _opts) do | ||
env | ||
|> Tesla.run(next) | ||
|> translate_response() | ||
end | ||
|
||
defp translate_response(%Tesla.Env{} = response) do | ||
{:ok, %Dayron.Response{ | ||
status_code: response.status, | ||
body: translate_response_body(response.body), | ||
headers: response.headers |> Map.to_list | ||
} | ||
} | ||
end | ||
|
||
defp translate_response_body(""), do: nil | ||
defp translate_response_body("ok"), do: %{} | ||
defp translate_response_body(body) do | ||
try do | ||
body |> Poison.decode!(keys: :atoms) | ||
rescue | ||
Poison.SyntaxError -> body | ||
end | ||
end | ||
|
||
def translate_error(%Tesla.Error{} = error) do | ||
data = error |> Map.from_struct | ||
{:error, struct(Dayron.ClientError, data)} | ||
end | ||
end | ||
|
||
@doc """ | ||
Implementation for `Dayron.Adapter.get/3`. | ||
""" | ||
def get(url, headers \\ [], opts \\ []) do | ||
tesla_call(:get, [url, build_options(headers, opts)]) | ||
end | ||
|
||
@doc """ | ||
Implementation for `Dayron.Adapter.post/4`. | ||
""" | ||
def post(url, body, headers \\ [], opts \\ []) do | ||
tesla_call(:post, [url, body, build_options(headers, opts)]) | ||
end | ||
|
||
@doc """ | ||
Implementation for `Dayron.Adapter.patch/4`. | ||
""" | ||
def patch(url, body, headers \\ [], opts \\ []) do | ||
tesla_call(:patch, [url, body, build_options(headers, opts)]) | ||
end | ||
|
||
@doc """ | ||
Implementation for `Dayron.Adapter.delete/3`. | ||
""" | ||
def delete(url, headers \\ [], opts \\ []) do | ||
tesla_call(:delete, [url, build_options(headers, opts)]) | ||
end | ||
|
||
defp tesla_call(method, args) do | ||
try do | ||
apply(Client, method, args) | ||
rescue | ||
e in Tesla.Error -> Translator.translate_error(e) | ||
end | ||
end | ||
|
||
def build_options([], opts), do: build_options(opts) | ||
def build_options(headers, opts) do | ||
build_options([{:headers, Enum.into(headers, %{})} | opts]) | ||
end | ||
|
||
defp build_options(opts) do | ||
Enum.reduce(opts, [{:opts, build_hackney_options(opts)}], fn | ||
{:headers, value}, options -> [{:headers, value} | options] | ||
{:params, value}, options -> [{:query, value} | options] | ||
_, options -> options | ||
end) | ||
end | ||
|
||
defp build_hackney_options(opts) do | ||
Enum.reduce(opts, [], fn | ||
{:hackney, extra_opts}, hn_opts -> hn_opts ++ extra_opts | ||
{:timeout, value}, hn_opts -> [{:connect_timeout, value} | hn_opts] | ||
{:recv_timeout, value}, hn_opts -> [{:recv_timeout, value} | hn_opts] | ||
{:proxy, value}, hn_opts -> [{:proxy, value} | hn_opts] | ||
{:proxy_auth, value}, hn_opts -> [{:proxy_auth, value} | hn_opts] | ||
{:ssl, value}, hn_opts -> [{:ssl_options, value} | hn_opts] | ||
{:follow_redirect, value}, hn_opts -> [{:follow_redirect, value} | hn_opts] | ||
{:max_redirect, value}, hn_opts -> [{:max_redirect, value} | hn_opts] | ||
#{:stream_to, arg}, hn_opts -> | ||
# [:async, {:stream_to, spawn(module, :transformer, [arg])} | hn_opts] | ||
_other, hn_opts -> hn_opts | ||
end) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
defmodule Dayron.TeslaAdapterTest do | ||
use ExUnit.Case, async: true | ||
alias Dayron.TeslaAdapter | ||
|
||
setup do | ||
bypass = Bypass.open | ||
{:ok, bypass: bypass, api_url: "http://localhost:#{bypass.port}"} | ||
end | ||
|
||
test "returns a decoded body for a valid get request", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, ~s<{"name": "Full Name", "address":{"street": "Elm Street", "zipcode": "88888"}}>) | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/id") | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
assert body[:name] == "Full Name" | ||
assert body[:address] == %{street: "Elm Street", zipcode: "88888"} | ||
end | ||
|
||
test "handles response body 'ok'", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, "ok") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/id") | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
assert body == %{} | ||
end | ||
|
||
test "handles invalid json body", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, "{invalid_json=1}") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/id") | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
assert body == "{invalid_json=1}" | ||
end | ||
|
||
test "returns a decoded body for a response list", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, ~s<[{"name": "First Resource"}, {"name": "Second Resource"}]>) | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources") | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
[first, second | _t] = body | ||
assert first[:name] == "First Resource" | ||
assert second[:name] == "Second Resource" | ||
end | ||
|
||
test "accepts query parameters and headers", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources" == conn.request_path | ||
assert "q=qu+ery&page=2" == conn.query_string | ||
assert [{"accept", "application/json"} | _] = conn.req_headers | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, "") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources", [{"accept", "application/json"}], [params: [{:q, "qu ery"}, {:page, 2}]]) | ||
assert {:ok, %Dayron.Response{status_code: 200, body: _}} = response | ||
end | ||
|
||
test "accepts custom headers", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert {"accesstoken", "token"} in conn.req_headers | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 200, "") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/id", [accesstoken: "token"]) | ||
assert {:ok, %Dayron.Response{status_code: 200, body: _}} = response | ||
end | ||
|
||
test "returns a 404 response", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/invalid" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 404, "") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/invalid") | ||
assert {:ok, %Dayron.Response{status_code: 404, body: _}} = response | ||
end | ||
|
||
test "returns a 500 error response", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/server-error" == conn.request_path | ||
assert "GET" == conn.method | ||
Plug.Conn.resp(conn, 500, "") | ||
end | ||
response = TeslaAdapter.get("#{api_url}/resources/server-error") | ||
assert {:ok, %Dayron.Response{status_code: 500, body: _}} = response | ||
end | ||
|
||
test "returns an error for invalid server" do | ||
response = TeslaAdapter.get("http://localhost:0001/resources/error") | ||
assert {:error, %Dayron.ClientError{reason: :econnrefused}} = response | ||
end | ||
|
||
test "returns a decoded body for a valid post request", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources" == conn.request_path | ||
assert [{"accept", "application/json"} | _] = conn.req_headers | ||
assert "POST" == conn.method | ||
Plug.Conn.resp(conn, 201, ~s<{"name": "Full Name", "age": 30}>) | ||
end | ||
response = TeslaAdapter.post("#{api_url}/resources", %{name: "Full Name", age: 30}, [{"accept", "application/json"}]) | ||
assert {:ok, %Dayron.Response{status_code: 201, body: body}} = response | ||
assert body[:name] == "Full Name" | ||
assert body[:age] == 30 | ||
end | ||
|
||
test "returns a decoded body for a valid patch request", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert [{"accept", "application/json"} | _] = conn.req_headers | ||
assert "PATCH" == conn.method | ||
Plug.Conn.resp(conn, 200, ~s<{"name": "Full Name", "age": 30}>) | ||
end | ||
response = TeslaAdapter.patch("#{api_url}/resources/id", %{name: "Full Name", age: 30}, [{"accept", "application/json"}]) | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
assert body[:name] == "Full Name" | ||
assert body[:age] == 30 | ||
end | ||
|
||
test "returns an empty body for a valid delete request", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
assert "/resources/id" == conn.request_path | ||
assert [{"content-type", "application/json"}|_] = conn.req_headers | ||
assert "DELETE" == conn.method | ||
Plug.Conn.resp(conn, 204, "") | ||
end | ||
response = TeslaAdapter.delete("#{api_url}/resources/id", [{"content-type", "application/json"}]) | ||
assert {:ok, %Dayron.Response{status_code: 204, body: nil}} = response | ||
end | ||
|
||
test "passing a custom hackney option works", %{bypass: bypass, api_url: api_url} do | ||
Bypass.expect bypass, fn conn -> | ||
case conn.request_path do | ||
"/old" -> | ||
conn | ||
|> Plug.Conn.put_resp_header("location", "/new") | ||
|> Plug.Conn.resp(301, "You are being redirected.") | ||
|> Plug.Conn.halt | ||
"/new" -> | ||
Plug.Conn.resp(conn, 200, "bar") | ||
end | ||
end | ||
response = TeslaAdapter.post("#{api_url}/old", "foo", [], [ | ||
{:follow_redirect, true}, | ||
{:hackney, [{:force_redirect, true}]} | ||
]) | ||
assert {:ok, %Dayron.Response{status_code: 200, body: body}} = response | ||
assert body == "bar" | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@jwarlander any reason to go low level and building hackney options instead of trying Tesla Dynamic Middlewares to include custom headers and params?
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.
It should definitely be possible to allow for custom middlewares; perhaps using application config? I could also see it as an option on the repo itself, something like:
However, what do you think about me raising a separate pull request for that, on top of the initial working adapter for Tesla?
As far as the current implementation goes, one could imagine just pushing headers into the options in the main client code, and having middleware perform the rest of the option translation.. Strictly speaking though, I'm not sure if it would make a big difference - I see middlewares mostly as an excellent way to support optional / new behavior, and translating the Dayron options to something that's suitable for Tesla's underlying Hackney adapter isn't really optional :)
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.
I should note though that the response translation ended up as a middleware module because I could find no good way to hook into the Tesla client itself.. then I ended up working outside of the client anyway, and implementing the
tesla_call/2
function that could as easily do that part, so I'll admit it's inconsistent ;)It would probably be pretty nice to split out option handling and response translation both into separate middlewares, put them in their own source files, and clean up the client module. I'll play around with it and see where it goes!