Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions lib/endpoint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will have to check which other modules either make assumptions about the status code or pass it on to client code. This could produce a (good) breaking change in those cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't see anything on a cursory glance. Let me know if I missed anything though.

else
%SparkPost.Endpoint.Response{status_code: 200, results: decoded_body}
%SparkPost.Endpoint.Response{status_code: code, results: decoded_body}
end
end

Expand All @@ -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)
%{
Expand All @@ -100,6 +104,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
Expand Down
4 changes: 4 additions & 0 deletions lib/mockserver.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
53 changes: 52 additions & 1 deletion lib/suppression_list.ex
Original file line number Diff line number Diff line change
@@ -1,17 +1,68 @@
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.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`:
- {:ok, ""}

Returned by `SparkPost.SuppressionList.upsert_one/3`:
- {:ok, 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 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 upsert_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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should adopt the usual {:ok, %Sparkpost.Endpoint.Response{}} or {:error, %SparkPost.Endpoint.Error{}} here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to adopt this. Unfortunately I'm not sure I have enough time to go through and update the library everywhere to make sure everything still works. Might make sense to have this in a different PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. I wasn't suggesting you retrofit the whole library with this. These are the first endpoints with simple good/bad responses though so I thought we'd start here.

case response do
%SparkPost.Endpoint.Response{status_code: 200, results: results} ->
{:ok, Map.get(results, :message, "")}
_ -> {:error, 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
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment about idiomatic {:ok, ...} return values applies here.

response = Endpoint.request(:delete, "suppression-list/#{recipient}", %{}, %{}, [], false)
case response do
%SparkPost.Endpoint.Response{status_code: 204} ->
{:ok, ""}
_ -> {:error, response}
end
end

@doc """
Execute a search of the suppression list based on the provided
parameters.
Expand Down
7 changes: 7 additions & 0 deletions test/data/suppressiondelete_fail.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"errors": [
{
"message": "Recipient could not be found"
}
]
}
5 changes: 5 additions & 0 deletions test/data/suppressionlistupdate.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"results": {
"message": "Test response message"
}
}
7 changes: 7 additions & 0 deletions test/data/suppressionupdate_fail.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"errors": [
{
"message": "Type must be one of: 'transactional', 'non_transactional'"
}
]
}
10 changes: 10 additions & 0 deletions test/endpoint_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
44 changes: 44 additions & 0 deletions test/suppression_list_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,50 @@ defmodule SparkPost.SuppressionListTest do

import Mock

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
{:ok, resp} = SuppressionList.upsert_one("test@marketing.com", "non_transactional", "test description")
assert resp == "Test response message"
end

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
{: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'"}]
end

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
{:ok, 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
{: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"}]
end

test_with_mock "SuppressionList.search succeeds with SuppressionList.SearchResult",
HTTPoison, [request: fn (method, url, body, headers, opts) ->
assert method == :get
Expand Down