Add tests for multi error cases

Author: JohnDoneth

lib/upload.ex

@@ -98,7 +98,7 @@ defmodule Upload do
     |> Upload.Multi.download_and_insert_variant(original_blob, variant, transform_fn)
     |> repo.transaction()
     |> case do
-      {:ok, multi_result} -> Map.fetch!(multi_result, "download_and_insert_#{variant}")
+      {:ok, multi_result} -> {:ok, Map.fetch!(multi_result, "download_and_insert_#{variant}")}
       {:error, error} -> {:error, error}
     end
   end
@@ -113,7 +113,7 @@ defmodule Upload do
       {:ok, [%Blob{...}, %Blob{...}]}
   """
   @spec create_multiple_variants(Blob.t(), [variant_id()], any()) ::
-          {:ok, [Blob.t()]} | {:error, any()}
+          {:ok, [Blob.t()]} | {:error, String.t(), any()}
   def create_multiple_variants(original_blob, variants, transform_fn)
       when is_function(transform_fn, 3) do
     variants = Enum.map(variants, &to_string/1)
@@ -129,13 +129,10 @@ defmodule Upload do
     |> repo.transaction()
     |> case do
       {:ok, multi_result} ->
-        {:ok,
-         multi_result
-         |> Map.values()
-         |> Enum.map(fn {:ok, value} -> value end)}
+        {:ok, Map.values(multi_result)}
 
-      {:error, error} ->
-        {:error, error}
+      {:error, stage, error, _} ->
+        {:error, stage, error}
     end
   end
 

lib/upload/errors.ex

@@ -6,3 +6,12 @@ defmodule Upload.RandomFileError do
     "Failed to create temporary file: #{inspect(reason)}"
   end
 end
+
+defmodule Upload.DownloadError do
+  defexception [:reason, :path, :key]
+
+  @impl true
+  def message(%{reason: reason, path: path, key: key}) do
+    "Failed to download '#{key}' to '#{path}': #{inspect(reason)}"
+  end
+end

lib/upload/multi.ex

@@ -76,13 +76,15 @@ defmodule Upload.Multi do
   def download_and_insert_variant(multi, original_blob, variant, transform_fn) do
     Multi.run(multi, "download_and_insert_#{variant}", fn repo, _ ->
       with {:ok, blob_path} <- create_random_file(),
-           :ok <- Upload.Storage.download(original_blob.key, blob_path),
+           :ok <- download_file(original_blob.key, blob_path),
            {:ok, variant_path} <- create_random_file(),
            :ok <- transform_fn.(blob_path, variant_path, variant),
            :ok <- cleanup(blob_path),
            {:ok, blob} <- insert_variant(repo, original_blob, variant, variant_path),
            :ok <- cleanup(variant_path) do
         {:ok, blob}
+      else
+        {:error, reason} -> {:error, reason}
       end
     end)
   end
@@ -105,7 +107,7 @@ defmodule Upload.Multi do
 
     changeset = Blob.changeset(%Blob{}, params)
 
-    {:ok, repo.insert(changeset)}
+    repo.insert(changeset)
   end
 
   defp variant_filename(original_blob, variant) do
@@ -123,8 +125,19 @@ defmodule Upload.Multi do
   end
 
   defp cleanup(path) do
-    with {:error, reason} <- File.rm(path) do
-      %File.Error{path: path, reason: reason, action: "remove temporary file"}
+    case File.rm(path) do
+      :ok ->
+        :ok
+
+      {:error, reason} ->
+        {:error, %File.Error{path: path, reason: reason, action: "remove temporary file"}}
+    end
+  end
+
+  defp download_file(key, path) do
+    case Upload.Storage.download(key, path) do
+      :ok -> :ok
+      {:error, reason} -> {:error, %Upload.DownloadError{reason: reason, key: key, path: path}}
     end
   end
 end

mix.exs

@@ -56,6 +56,7 @@ defmodule Upload.Mixfile do
       {:plug, "~> 1.13"},
       {:postgrex, ">= 0.0.0"},
       {:sweet_xml, ">= 0.0.0"},
+      {:mock, "~> 0.3"},
 
       # Test dependencies for this package
       {:credo, "~> 1.7", only: [:dev, :test], runtime: false},

mix.lock

@@ -31,10 +31,12 @@
   "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"},
+  "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
   "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
   "mimerl": {:hex, :mimerl, "1.3.0", "d0cd9fc04b9061f82490f6581e0128379830e78535e017f7780f37fea7545726", [:rebar3], [], "hexpm", "a1e15a50d1887217de95f0b9b0793e32853f7c258a5cd227650889b38839fe9d"},
   "mix_test_watch": {:hex, :mix_test_watch, "1.2.0", "1f9acd9e1104f62f280e30fc2243ae5e6d8ddc2f7f4dc9bceb454b9a41c82b42", [:mix], [{:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}], "hexpm", "278dc955c20b3fb9a3168b5c2493c2e5cffad133548d307e0a50c7f2cfbf34f6"},
+  "mock": {:hex, :mock, "0.3.8", "7046a306b71db2488ef54395eeb74df0a7f335a7caca4a3d3875d1fc81c884dd", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "7fa82364c97617d79bb7d15571193fc0c4fe5afd0c932cef09426b3ee6fe2022"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
   "phoenix_html": {:hex, :phoenix_html, "4.1.1", "4c064fd3873d12ebb1388425a8f2a19348cef56e7289e1998e2d2fa758aa982e", [:mix], [], "hexpm", "f2f2df5a72bc9a2f510b21497fd7d2b86d932ec0598f0210fed4114adc546c6f"},

test/support/data_case.ex

@@ -10,6 +10,7 @@ defmodule Upload.DataCase do
   using do
     quote do
       import Upload.DataCase
+      import Mock
     end
   end
 

test/upload_test.exs

@@ -78,6 +78,80 @@ defmodule UploadTest do
       assert small_avif_variant.key == "uploads/users/avatars/123/variant/small_avif.avif"
       assert small_avif_variant.key in list_uploaded_keys()
     end
+
+    test "returns an error when a temp file cannot be created" do
+      changeset = change_person(%{avatar: @upload})
+
+      assert {:ok, %{person: person}} = upload_person(changeset)
+      assert person.avatar
+
+      with_mock(Plug.Upload, random_file: fn _ -> {:error, :boom} end) do
+        {:error, "download_and_insert_small", %Upload.RandomFileError{reason: {:error, :boom}}} =
+          Upload.create_multiple_variants(
+            person.avatar,
+            [
+              "small"
+            ],
+            &transform_image/3
+          )
+      end
+    end
+
+    test "returns an error when the original file cannot be downloaded" do
+      changeset = change_person(%{avatar: @upload})
+
+      assert {:ok, %{person: person}} = upload_person(changeset)
+      assert person.avatar
+
+      with_mock(Upload.Storage, download: fn _key, _ -> {:error, :boom} end) do
+        {:error, "download_and_insert_small",
+         %Upload.DownloadError{reason: :boom, key: "uploads/users/avatars/123.jpg"}} =
+          Upload.create_multiple_variants(
+            person.avatar,
+            [
+              "small"
+            ],
+            &transform_image/3
+          )
+      end
+    end
+
+    test "returns an error when a temp file cannot be deleted" do
+      changeset = change_person(%{avatar: @upload})
+
+      assert {:ok, %{person: person}} = upload_person(changeset)
+      assert person.avatar
+
+      with_mock(File, [:passthrough], rm: fn _path -> {:error, :enoent} end) do
+        {:error, "download_and_insert_small",
+         %File.Error{reason: :enoent, action: "remove temporary file"}} =
+          Upload.create_multiple_variants(
+            person.avatar,
+            [
+              "small"
+            ],
+            fn _, _, _ -> :ok end
+          )
+      end
+    end
+
+    test "returns an error when attempting to insert a bad key caused by a variant name" do
+      changeset = change_person(%{avatar: @upload})
+
+      assert {:ok, %{person: person}} = upload_person(changeset)
+      assert person.avatar
+
+      {:error, "download_and_insert_.", changeset} =
+        Upload.create_multiple_variants(
+          person.avatar,
+          [
+            "."
+          ],
+          fn _, _, _ -> :ok end
+        )
+
+      assert errors_on(changeset)[:key] == ["has invalid format"]
+    end
   end
 
   describe "put_access_control_list/2" do