Fix remaining parameter bugs, fix attachment download, some cleanup

Author: skosch

lib/mailman/attachment.ex

@@ -666,17 +666,18 @@ defmodule Mailman.Attachment do
   ]
 
   defp get_attachment_data_via_http(url) do
-    case is_valid_url?(url) do
-      {:ok, _} ->
-        response = HTTPotion.get(url, timeout: 60_000)
-
+    if is_valid_url?(url) do
+      with {:ok, response} <- HTTPoison.get(url, recv_timeout: 60_000) do
         case response.status_code do
           200 -> {:ok, response.body}
           _ -> {:error, :invalid_http_response}
         end
-
-      {:error, message} ->
-        {:error, message}
+      else
+        {:error, message} ->
+          {:error, message}
+      end
+    else
+      {:error, :invalid_url}
     end
   end
 
@@ -749,14 +750,13 @@ defmodule Mailman.Attachment do
   def mime_type_and_subtype_from_extension(path) do
     extension = Path.extname(path)
 
-    type =
-      Enum.find(mime_types(), fn {ext, _} ->
-        ext == extension
-      end)
-
-    case type do
+    mime_types()
+    |> Enum.find(fn {ext, _} ->
+      ext == extension
+    end)
+    |> case do
       nil -> "application/octet-stream"
-      _ -> elem(type, 1)
+      {_ext, mimetype} -> mimetype
     end
     |> String.split("/")
     |> List.to_tuple()

lib/mailman/parsing.ex

@@ -23,21 +23,27 @@ defmodule Mailman.Parsing do
 
   @doc "Parses the message and returns Email"
   def parse!(message_text) do
-    case parse message_text do
-      { :ok, parsed }    -> parsed
-      { :error, reason } -> throw "Couldn't parse given message. #{reason}"
+    case parse(message_text) do
+      {:ok, parsed} -> parsed
+      {:error, reason} -> throw("Couldn't parse given message. #{reason}")
     end
   end
 
   def get_header(raw, name) do
-    header = Enum.find all_headers(raw), fn({header_name, _}) ->
-      header_name == name
-    end
+    header =
+      Enum.find(get_headers(raw), fn {header_name, _} ->
+        header_name == name
+      end)
+
     if header != nil do
       value = elem(header, 1)
+
       cond do
-        name == "To" || name == "Cc" || name == "Bcc" -> String.split(value, ",") |> Enum.map(&String.trim(&1))
-        true -> value
+        name == "To" || name == "Cc" || name == "Bcc" ->
+          String.split(value, ",") |> Enum.map(&String.trim(&1))
+
+        true ->
+          value
       end
     else
       if name == "To" || name == "Cc" || name == "Bcc" do
@@ -48,32 +54,34 @@ defmodule Mailman.Parsing do
     end
   end
 
-  def all_headers(raw) do
-    elem(raw, 2)
+  def get_headers({_mime_type, _mime_subtype, headers, _parameters, _content}) do
+      headers
   end
 
   def filename_from_raw(raw_part) do
-    maybe_param = raw_parameters_for(raw_part) |>
-      List.last |>
-      elem(1) |>
-      Enum.find(fn(p) ->
+    maybe_param =
+      raw_part
+      |> get_parameters
+      |> Map.get(:disposition_params)
+      |> Enum.find(fn p ->
         elem(p, 0) == "filename"
       end)
+
     if maybe_param != nil do
       elem(maybe_param, 1)
     else
       nil
     end
   end
 
-  def raw_parameters_for(raw_part) do
-    elem(raw_part, 3)
+  def get_parameters({_mime_type, _mime_subtype, _headers, parameters, _content}) do
+    parameters
   end
 
-  def is_raw_attachement(raw_part) do
+  def is_raw_attachment(raw_part) do
     case filename_from_raw(raw_part) do
       nil -> false
-      _   -> true
+      _ -> true
     end
   end
 
@@ -88,11 +96,11 @@ defmodule Mailman.Parsing do
   def get_attachments(raw) do
     raw
     |> content_parts
-    |> Enum.filter(&is_raw_attachement(&1))
-    |> Enum.map(&raw_to_attachement(&1))
+    |> Enum.filter(&is_raw_attachment(&1))
+    |> Enum.map(&raw_to_attachment(&1))
   end
 
-  def raw_to_attachement(raw_part) do
+  def raw_to_attachment(raw_part) do
     %Mailman.Attachment{
       file_name: filename_from_raw(raw_part),
       mime_type: get_type(raw_part),
@@ -103,11 +111,12 @@ defmodule Mailman.Parsing do
 
   def content_parts(raw) when is_tuple(raw) do
     body = get_raw_body(raw)
+
     cond do
       is_binary(body) -> [raw]
-      is_list(body)   -> Enum.map(body, &content_parts(&1))
+      is_list(body) -> Enum.map(body, &content_parts(&1))
     end
-    |> List.flatten
+    |> List.flatten()
   end
 
   def get_type(raw) when is_tuple(raw) do

lib/mailman/render.ex

@@ -16,7 +16,7 @@ defmodule Mailman.Render do
       top_mime_type,
       top_mime_sub_type,
       headers_for(email) ++ extra_headers,
-      [],
+      %{},
       top_content_part
     }
   end
@@ -31,20 +31,20 @@ defmodule Mailman.Render do
       html_part_tuple
     else
       if is_nil(html_part_tuple), do: nil, else:
-        {"multipart", "related", [], [], [html_part_tuple | inline_attachment_part_tuples]}
+        {"multipart", "related", [], %{}, [html_part_tuple | inline_attachment_part_tuples]}
     end
 
     alternative_or_plain_tuple = if is_nil(related_or_html_part_tuple) do
       plain_part_tuple
     else
-      {"multipart", "alternative", [], [], [plain_part_tuple, related_or_html_part_tuple]}
+      {"multipart", "alternative", [], %{}, [plain_part_tuple, related_or_html_part_tuple]}
     end
 
     mixed_or_alternative_tuple = if Enum.empty?(attached_attachment_part_tuples) do
       alternative_or_plain_tuple
     else
       if is_nil(alternative_or_plain_tuple), do: nil, else:
-        {"multipart", "mixed", [], [], [alternative_or_plain_tuple | attached_attachment_part_tuples]}
+        {"multipart", "mixed", [], %{}, [alternative_or_plain_tuple | attached_attachment_part_tuples]}
     end
 
     mixed_or_alternative_tuple

mix.exs

@@ -17,7 +17,7 @@ defmodule Mailman.Mixfile do
 
   # Configuration for the OTP application
   def application do
-    [applications: [:ssl, :crypto, :eiconv, :gen_smtp, :httpotion]]
+    [applications: [:ssl, :crypto, :eiconv, :gen_smtp, :httpoison]]
   end
 
   # Note that :eiconv encoder/decoder is used by gen_smtp as well,
@@ -34,9 +34,10 @@ defmodule Mailman.Mixfile do
   defp deps do
     [
       {:eiconv, "~> 1.0.0"},
-      {:gen_smtp, "~> 0.15.0"},
+    # {:gen_smtp, "~> 0.15.0"},
+      {:gen_smtp, github: "gen-smtp/gen_smtp", override: true},
       {:ex_doc, ">= 0.19.1", only: :dev},
-      {:httpotion, "~> 3.1.0"},
+      {:httpoison, "~> 1.6"},
       {:credo, "~> 0.10", only: [:dev, :test], runtime: false}
     ]
   end

mix.lock

@@ -1,14 +1,25 @@
 %{
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []},
+  "certifi": {:hex, :certifi, "2.5.2", "b7cfeae9d2ed395695dd8201c57a2d019c0c43ecaf8b8bcb9320b40d6662f340", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"},
   "credo": {:hex, :credo, "0.10.0", "66234a95effaf9067edb19fc5d0cd5c6b461ad841baac42467afed96c78e5e9e", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, optional: false]}, {:jason, "~> 1.0", [hex: :jason, optional: false]}]},
   "earmark": {:hex, :earmark, "1.3.0", "17f0c38eaafb4800f746b457313af4b2442a8c2405b49c645768680f900be603", [:mix], [], "hexpm"},
   "eiconv": {:hex, :eiconv, "1.0.0", "ee1e47ee37799a05beff7a68d61f63cccc93101833c4fb94b454c23b12a21629", [:rebar3], []},
   "ex_doc": {:hex, :ex_doc, "0.19.1", "519bb9c19526ca51d326c060cb1778d4a9056b190086a8c6c115828eaccea6cf", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.7", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm"},
-  "gen_smtp": {:hex, :gen_smtp, "0.15.0", "9f51960c17769b26833b50df0b96123605a8024738b62db747fece14eb2fbfcc", [:rebar3], [], "hexpm"},
+  "gen_smtp": {:git, "https://github.com/gen-smtp/gen_smtp.git", "62f585bf8d630908c9f958c978140f1012be0343", []},
+  "hackney": {:hex, :hackney, "1.16.0", "5096ac8e823e3a441477b2d187e30dd3fff1a82991a806b2003845ce72ce2d84", [:rebar3], [{:certifi, "2.5.2", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.1", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.0", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.6", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
+  "httpoison": {:hex, :httpoison, "1.7.0", "abba7d086233c2d8574726227b6c2c4f6e53c4deae7fe5f6de531162ce9929a0", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
   "httpotion": {:hex, :httpotion, "3.1.0", "14d20d9b0ce4e86e253eb91e4af79e469ad949f57a5d23c0a51b2f86559f6589", [:mix], [{:ibrowse, "~> 4.4", [hex: :ibrowse, optional: false]}]},
+  "hut": {:hex, :hut, "1.3.0", "71f2f054e657c03f959cf1acc43f436ea87580696528ca2a55c8afb1b06c85e7", [:"erlang.mk", :rebar, :rebar3], [], "hexpm"},
   "ibrowse": {:hex, :ibrowse, "4.4.0", "2d923325efe0d2cb09b9c6a047b2835a5eda69d8a47ed6ff8bc03628b764e991", [:rebar3], []},
+  "idna": {:hex, :idna, "6.0.1", "1d038fb2e7668ce41fbf681d2c45902e52b3cb9e9c77b55334353b222c2ee50c", [:rebar3], [{:unicode_util_compat, "0.5.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
   "jason": {:hex, :jason, "1.1.1", "d3ccb840dfb06f2f90a6d335b536dd074db748b3e7f5b11ab61d239506585eb2", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, optional: true]}]},
   "makeup": {:hex, :makeup, "0.5.5", "9e08dfc45280c5684d771ad58159f718a7b5788596099bdfb0284597d368a882", [:mix], [{:nimble_parsec, "~> 0.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.10.0", "0f09c2ddf352887a956d84f8f7e702111122ca32fbbc84c2f0569b8b65cbf7fa", [:mix], [{:makeup, "~> 0.5.5", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm"},
+  "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"},
+  "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm"},
   "nimble_parsec": {:hex, :nimble_parsec, "0.4.0", "ee261bb53214943679422be70f1658fff573c5d0b0a1ecd0f18738944f818efe", [:mix], [], "hexpm"},
+  "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm"},
+  "ranch": {:hex, :ranch, "1.6.1", "2609724c9a7e163ca716a46c9087e037db4262935f5d866122ba158ed917c233", [:rebar3], [], "hexpm"},
+  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm"},
+  "unicode_util_compat": {:hex, :unicode_util_compat, "0.5.0", "8516502659002cec19e244ebd90d312183064be95025a319a6c7e89f4bccd65b", [:rebar3], [], "hexpm"},
 }

test/attachments_test.exs

@@ -6,6 +6,12 @@ defmodule AttachmentsTest do
     assert is_map(attachment)
   end
 
+  test "Attachment via HTTPoison works" do
+    {:ok, attachment} = Mailman.Attachment.inline("https://www.w3.org/")
+    assert is_map(attachment)
+  end
+
+
   test "#inline returns {:error, message} when file doesn't exist" do
     file_path = "test/data/idontexist.png"
     {:error, _} = Mailman.Attachment.inline(file_path)

test/mailman_test.exs

@@ -120,7 +120,7 @@ defmodule MailmanTest do
     assert bcc_email.bcc |> length == 1
   end
 
-  test "encodes attachements properly" do
+  test "encodes attachments properly" do
     email_with_attachments = email_with_attachments()
     {:ok, message} = MyApp.Mailer.deliver(email_with_attachments)
     email = Mailman.Email.parse!(message)