From 1689a98a83f39e22c9c6ae0a8691ede9880ed089 Mon Sep 17 00:00:00 2001 From: Ashu Goel Date: Wed, 22 Mar 2017 11:45:59 -0700 Subject: [PATCH 1/4] [suppression_list] add API to delete an entry --- lib/endpoint.ex | 9 +++++---- lib/suppression_list.ex | 24 +++++++++++++++++++++++- test/data/suppressiondelete_fail.json | 7 +++++++ test/suppression_list_test.exs | 22 ++++++++++++++++++++++ 4 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 test/data/suppressiondelete_fail.json diff --git a/lib/endpoint.ex b/lib/endpoint.ex index 8f63d2b..3f7441c 100644 --- a/lib/endpoint.ex +++ b/lib/endpoint.ex @@ -36,9 +36,9 @@ defmodule SparkPost.Endpoint do url = Application.get_env(:sparkpost, :api_endpoint, @default_endpoint) <> endpoint {:ok, request_body} = encode_request_body(body) - + request_headers = if method in [:get, :delete] do - headers + headers else Map.merge(headers, %{"Content-Type": "application/json"}) end @@ -73,9 +73,9 @@ defmodule SparkPost.Endpoint do defp handle_response({:ok, %HTTPoison.Response{status_code: code, body: body}}, decode_results) when code >= 200 and code < 300 do decoded_body = decode_response_body(body) if decode_results do - %SparkPost.Endpoint.Response{status_code: 200, results: decoded_body.results} + %SparkPost.Endpoint.Response{status_code: code, results: decoded_body.results} else - %SparkPost.Endpoint.Response{status_code: 200, results: decoded_body} + %SparkPost.Endpoint.Response{status_code: code, results: decoded_body} end end @@ -100,6 +100,7 @@ defmodule SparkPost.Endpoint do body |> Washup.filter |> Poison.encode end + defp decode_response_body(body) when byte_size(body) == 0, do: "" defp decode_response_body(body) do # TODO: [key: :atoms] is unsafe for open-ended structures such as # metadata and substitution_data diff --git a/lib/suppression_list.ex b/lib/suppression_list.ex index d17bfa5..9b8d8e1 100644 --- a/lib/suppression_list.ex +++ b/lib/suppression_list.ex @@ -1,17 +1,39 @@ defmodule SparkPost.SuppressionList do @moduledoc """ The SparkPost Suppression List API for working with suppression lists. - Use `SparkPost.SuppressionList.search/1` to search through your account's suppression list. + Use `SparkPost.SuppressionList.delete/1` to delete a single entry from a list, + `SparkPost.SuppressionList.search/1` to search through your account's suppression list. Check out the documentation for each function or use the [SparkPost API reference](https://developers.sparkpost.com/api/suppression_list.html) for details. + Returned by `SparkPost.SuppressionList.delete/1`: + - nothing (empty body) + Returned by `SparkPost.SuppressionList.search/1`. - %SparkPost.SuppressionList.SearchResult{} """ alias SparkPost.Endpoint + @doc """ + Deletes a specific entry from the list. Returns an empty string if + the deletion was successful. Returns a %SparkPost.Endpoint.Error{} with a 404 + if the specified entry is not in the list. Returns a %SparkPost.Endpoint.Error{} + with a 403 if the entry could not be removed for any reason (such as Compliance). + + Parameters: + recipient: the entry to delete from the suppression list. + """ + def delete(recipient) do + response = Endpoint.request(:delete, "suppression-list/#{recipient}", %{}, %{}, [], false) + case response do + %SparkPost.Endpoint.Response{status_code: 204} -> + "" + _ -> response + end + end + @doc """ Execute a search of the suppression list based on the provided parameters. diff --git a/test/data/suppressiondelete_fail.json b/test/data/suppressiondelete_fail.json new file mode 100644 index 0000000..b5a05be --- /dev/null +++ b/test/data/suppressiondelete_fail.json @@ -0,0 +1,7 @@ +{ + "errors": [ + { + "message": "Recipient could not be found" + } + ] +} diff --git a/test/suppression_list_test.exs b/test/suppression_list_test.exs index 2a4b238..e080de9 100644 --- a/test/suppression_list_test.exs +++ b/test/suppression_list_test.exs @@ -8,6 +8,28 @@ defmodule SparkPost.SuppressionListTest do import Mock + test_with_mock "SuppressionList.delete succeeds with empty body", + HTTPoison, [request: fn (method, url, body, headers, opts) -> + assert method == :delete + fun = MockServer.mk_http_resp(204, "") + fun.(method, url, body, headers, opts) + end] do + resp = SuppressionList.delete("test@marketing.com") + assert resp == "" + end + + test_with_mock "SuppressionList.delete fails 404", + HTTPoison, [request: fn (method, url, body, headers, opts) -> + assert method == :delete + fun = MockServer.mk_http_resp(404, MockServer.get_json("suppressiondelete_fail")) + fun.(method, url, body, headers, opts) + end] do + resp = SuppressionList.delete("test@marketing.com") + assert %SparkPost.Endpoint.Error{} = resp + assert resp.status_code == 404 + assert resp.errors == [%{message: "Recipient could not be found"}] + end + test_with_mock "SuppressionList.search succeeds with SuppressionList.SearchResult", HTTPoison, [request: fn (method, url, body, headers, opts) -> assert method == :get From fc0c4cc8226f58e78f2f2854d026b28f5a00d95d Mon Sep 17 00:00:00 2001 From: Ashu Goel Date: Fri, 24 Mar 2017 20:51:36 -0700 Subject: [PATCH 2/4] [endpoint] gracefully handle httpoison errors --- lib/endpoint.ex | 4 ++++ lib/mockserver.ex | 4 ++++ test/endpoint_test.exs | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/lib/endpoint.ex b/lib/endpoint.ex index 3f7441c..728c7ba 100644 --- a/lib/endpoint.ex +++ b/lib/endpoint.ex @@ -86,6 +86,10 @@ defmodule SparkPost.Endpoint do end end + defp handle_response({:error, %HTTPoison.Error{reason: reason}}, _decode_results) do + %SparkPost.Endpoint.Error{status_code: nil, errors: [reason]} + end + defp base_request_headers() do {:ok, version} = :application.get_key(:sparkpost, :vsn) %{ diff --git a/lib/mockserver.ex b/lib/mockserver.ex index a10c2fa..6094ccd 100644 --- a/lib/mockserver.ex +++ b/lib/mockserver.ex @@ -36,4 +36,8 @@ defmodule SparkPost.MockServer do def mk_http_resp(status_code, body) do fn (_method, _url, _body, _headers, _opts) -> {:ok, %HTTPoison.Response{status_code: status_code, body: body}} end end + + def mk_error(reason) do + fn (_method, _url, _body, _headers, _opts) -> {:error, %HTTPoison.Error{reason: reason}} end + end end diff --git a/test/endpoint_test.exs b/test/endpoint_test.exs index 1c89a8b..2d44d60 100644 --- a/test/endpoint_test.exs +++ b/test/endpoint_test.exs @@ -102,4 +102,14 @@ defmodule SparkPost.EndpointTest do Endpoint.request(:get, "transmissions", %{}, %{}, []) end end + + test_with_mock "Endpoint request can handle httpoison timeouts", HTTPoison, + [request: fn (method, url, body, headers, opts) -> + fun = MockServer.mk_error(:timeout) + fun.(method, url, body, headers, opts) + end + ] do + assert %Endpoint.Error{errors: [:timeout], status_code: nil, results: nil} == + Endpoint.request(:post, "transmissions", %{}, %{}, []) + end end From 9035124ee94dc116c3e528afb1a73f252562489c Mon Sep 17 00:00:00 2001 From: Ashu Goel Date: Fri, 24 Mar 2017 21:05:23 -0700 Subject: [PATCH 3/4] [suppression_list] add ability to insert/update a single entry --- lib/suppression_list.ex | 31 ++++++++++++++++++++++++++- test/data/suppressionlistupdate.json | 5 +++++ test/data/suppressionupdate_fail.json | 7 ++++++ test/suppression_list_test.exs | 22 +++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/data/suppressionlistupdate.json create mode 100644 test/data/suppressionupdate_fail.json diff --git a/lib/suppression_list.ex b/lib/suppression_list.ex index 9b8d8e1..346d901 100644 --- a/lib/suppression_list.ex +++ b/lib/suppression_list.ex @@ -2,7 +2,8 @@ defmodule SparkPost.SuppressionList do @moduledoc """ The SparkPost Suppression List API for working with suppression lists. Use `SparkPost.SuppressionList.delete/1` to delete a single entry from a list, - `SparkPost.SuppressionList.search/1` to search through your account's suppression list. + `SparkPost.SuppressionList.update_one/3` to insert or update a single list entry, + or `SparkPost.SuppressionList.search/1` to search through your account's suppression list. Check out the documentation for each function or use the [SparkPost API reference](https://developers.sparkpost.com/api/suppression_list.html) for details. @@ -10,12 +11,40 @@ defmodule SparkPost.SuppressionList do Returned by `SparkPost.SuppressionList.delete/1`: - nothing (empty body) + Returned by `SparkPost.SuppressionList.update_one/3`: + - message (A success message string) + Returned by `SparkPost.SuppressionList.search/1`. - %SparkPost.SuppressionList.SearchResult{} """ alias SparkPost.Endpoint + @doc """ + Insert or update a single entry in the suppression list. + Returns a single string with the success message if the entry + was updated or inserted. Returns a %SparkPost.Endpoint.Error{} with a 400 + if the type is invalid. + + Parameters: + - recipient: the email to insert or update in the suppression list + - type: one of "transactional" or "non_transactional" + - description (optional): optional description of this entry in the suppression list + """ + def update_one(recipient, type, description \\ nil) do + body = if description == nil do + %{type: type} + else + %{type: type, description: description} + end + response = Endpoint.request(:put, "suppression-list/#{recipient}", body) + case response do + %SparkPost.Endpoint.Response{status_code: 200, results: results} -> + Map.get(results, :message, "") + _ -> response + end + end + @doc """ Deletes a specific entry from the list. Returns an empty string if the deletion was successful. Returns a %SparkPost.Endpoint.Error{} with a 404 diff --git a/test/data/suppressionlistupdate.json b/test/data/suppressionlistupdate.json new file mode 100644 index 0000000..b53ba33 --- /dev/null +++ b/test/data/suppressionlistupdate.json @@ -0,0 +1,5 @@ +{ + "results": { + "message": "Test response message" + } +} diff --git a/test/data/suppressionupdate_fail.json b/test/data/suppressionupdate_fail.json new file mode 100644 index 0000000..cd63df1 --- /dev/null +++ b/test/data/suppressionupdate_fail.json @@ -0,0 +1,7 @@ +{ + "errors": [ + { + "message": "Type must be one of: 'transactional', 'non_transactional'" + } + ] +} diff --git a/test/suppression_list_test.exs b/test/suppression_list_test.exs index e080de9..58a4711 100644 --- a/test/suppression_list_test.exs +++ b/test/suppression_list_test.exs @@ -8,6 +8,28 @@ defmodule SparkPost.SuppressionListTest do import Mock + test_with_mock "SuppressionList.update_one succeeds with message", + HTTPoison, [request: fn (method, url, body, headers, opts) -> + assert method == :put + fun = MockServer.mk_http_resp(200, MockServer.get_json("suppressionlistupdate")) + fun.(method, url, body, headers, opts) + end] do + resp = SuppressionList.update_one("test@marketing.com", "non_transactional", "test description") + assert resp == "Test response message" + end + + test_with_mock "SuppressionList.update_one fails with invalid type", + HTTPoison, [request: fn (method, url, body, headers, opts) -> + assert method == :put + fun = MockServer.mk_http_resp(400, MockServer.get_json("suppressionupdate_fail")) + fun.(method, url, body, headers, opts) + end] do + resp = SuppressionList.update_one("test@marketing.com", "bad_type") + assert %SparkPost.Endpoint.Error{} = resp + assert resp.status_code == 400 + assert resp.errors == [%{message: "Type must be one of: 'transactional', 'non_transactional'"}] + end + test_with_mock "SuppressionList.delete succeeds with empty body", HTTPoison, [request: fn (method, url, body, headers, opts) -> assert method == :delete From d8efa11823225ab1f62406760221c84191d66610 Mon Sep 17 00:00:00 2001 From: Ashu Goel Date: Tue, 28 Mar 2017 12:02:43 -0700 Subject: [PATCH 4/4] [suppression_list] minor PR updates. start moving towards more idiomatic {:ok | :error, resp} tuples --- lib/suppression_list.ex | 20 ++++++++++---------- test/suppression_list_test.exs | 12 ++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/suppression_list.ex b/lib/suppression_list.ex index 346d901..1e158ac 100644 --- a/lib/suppression_list.ex +++ b/lib/suppression_list.ex @@ -2,17 +2,17 @@ defmodule SparkPost.SuppressionList do @moduledoc """ The SparkPost Suppression List API for working with suppression lists. Use `SparkPost.SuppressionList.delete/1` to delete a single entry from a list, - `SparkPost.SuppressionList.update_one/3` to insert or update a single list entry, + `SparkPost.SuppressionList.upsert_one/3` to insert or update a single list entry, or `SparkPost.SuppressionList.search/1` to search through your account's suppression list. Check out the documentation for each function or use the [SparkPost API reference](https://developers.sparkpost.com/api/suppression_list.html) for details. Returned by `SparkPost.SuppressionList.delete/1`: - - nothing (empty body) + - {:ok, ""} - Returned by `SparkPost.SuppressionList.update_one/3`: - - message (A success message string) + Returned by `SparkPost.SuppressionList.upsert_one/3`: + - {:ok, message} (A success message string) Returned by `SparkPost.SuppressionList.search/1`. - %SparkPost.SuppressionList.SearchResult{} @@ -24,14 +24,14 @@ defmodule SparkPost.SuppressionList do Insert or update a single entry in the suppression list. Returns a single string with the success message if the entry was updated or inserted. Returns a %SparkPost.Endpoint.Error{} with a 400 - if the type is invalid. + if there was an issue with the request format. Parameters: - recipient: the email to insert or update in the suppression list - type: one of "transactional" or "non_transactional" - description (optional): optional description of this entry in the suppression list """ - def update_one(recipient, type, description \\ nil) do + def upsert_one(recipient, type, description \\ nil) do body = if description == nil do %{type: type} else @@ -40,8 +40,8 @@ defmodule SparkPost.SuppressionList do response = Endpoint.request(:put, "suppression-list/#{recipient}", body) case response do %SparkPost.Endpoint.Response{status_code: 200, results: results} -> - Map.get(results, :message, "") - _ -> response + {:ok, Map.get(results, :message, "")} + _ -> {:error, response} end end @@ -58,8 +58,8 @@ defmodule SparkPost.SuppressionList do response = Endpoint.request(:delete, "suppression-list/#{recipient}", %{}, %{}, [], false) case response do %SparkPost.Endpoint.Response{status_code: 204} -> - "" - _ -> response + {:ok, ""} + _ -> {:error, response} end end diff --git a/test/suppression_list_test.exs b/test/suppression_list_test.exs index 58a4711..175631f 100644 --- a/test/suppression_list_test.exs +++ b/test/suppression_list_test.exs @@ -8,23 +8,23 @@ defmodule SparkPost.SuppressionListTest do import Mock - test_with_mock "SuppressionList.update_one succeeds with message", + test_with_mock "SuppressionList.upsert_one succeeds with message", HTTPoison, [request: fn (method, url, body, headers, opts) -> assert method == :put fun = MockServer.mk_http_resp(200, MockServer.get_json("suppressionlistupdate")) fun.(method, url, body, headers, opts) end] do - resp = SuppressionList.update_one("test@marketing.com", "non_transactional", "test description") + {:ok, resp} = SuppressionList.upsert_one("test@marketing.com", "non_transactional", "test description") assert resp == "Test response message" end - test_with_mock "SuppressionList.update_one fails with invalid type", + test_with_mock "SuppressionList.upsert_one fails with invalid type", HTTPoison, [request: fn (method, url, body, headers, opts) -> assert method == :put fun = MockServer.mk_http_resp(400, MockServer.get_json("suppressionupdate_fail")) fun.(method, url, body, headers, opts) end] do - resp = SuppressionList.update_one("test@marketing.com", "bad_type") + {:error, resp} = SuppressionList.upsert_one("test@marketing.com", "bad_type") assert %SparkPost.Endpoint.Error{} = resp assert resp.status_code == 400 assert resp.errors == [%{message: "Type must be one of: 'transactional', 'non_transactional'"}] @@ -36,7 +36,7 @@ defmodule SparkPost.SuppressionListTest do fun = MockServer.mk_http_resp(204, "") fun.(method, url, body, headers, opts) end] do - resp = SuppressionList.delete("test@marketing.com") + {:ok, resp} = SuppressionList.delete("test@marketing.com") assert resp == "" end @@ -46,7 +46,7 @@ defmodule SparkPost.SuppressionListTest do fun = MockServer.mk_http_resp(404, MockServer.get_json("suppressiondelete_fail")) fun.(method, url, body, headers, opts) end] do - resp = SuppressionList.delete("test@marketing.com") + {:error, resp} = SuppressionList.delete("test@marketing.com") assert %SparkPost.Endpoint.Error{} = resp assert resp.status_code == 404 assert resp.errors == [%{message: "Recipient could not be found"}]