chore: cleaned up bang-Operator usage

lib/bds/media/rebuilder.ex

@@ -19,64 +19,77 @@ defmodule BDS.Media.Rebuilder do
 
   @type rebuild_opts :: keyword()
 
-  @spec rebuild_media_from_files(String.t(), rebuild_opts()) :: {:ok, [Media.t()]}
+  @spec rebuild_media_from_files(String.t(), rebuild_opts()) :: {:ok, [Media.t()]} | {:error, term()}
   def rebuild_media_from_files(project_id, opts \\ []) do
     project = Projects.get_project!(project_id)
     on_progress = progress_callback(opts)
 
-    canonical_sidecars =
-      project
-      |> Projects.project_data_dir()
-      |> Path.join("media")
+    media_dir = project |> Projects.project_data_dir() |> Path.join("media")
+
+    canonical_results =
+      media_dir
       |> list_matching_files("*.meta")
       |> Enum.filter(&Sidecars.canonical_sidecar?/1)
       |> Enum.filter(&Sidecars.binary_exists_for_sidecar?/1)
       |> Rebuild.parallel_map(&Sidecars.parse_canonical_sidecar(project, &1))
 
-    translation_sidecars =
-      project
-      |> Projects.project_data_dir()
-      |> Path.join("media")
-      |> list_matching_files("*.meta")
-      |> Enum.filter(&Sidecars.translation_sidecar?/1)
-      |> Rebuild.parallel_map(&Sidecars.parse_translation_sidecar(&1))
+    with {:ok, canonical_sidecars} <- collect_sidecars(canonical_results) do
+      translation_results =
+        media_dir
+        |> list_matching_files("*.meta")
+        |> Enum.filter(&Sidecars.translation_sidecar?/1)
+        |> Rebuild.parallel_map(&Sidecars.parse_translation_sidecar(&1))
 
-    total_files = length(canonical_sidecars) + length(translation_sidecars)
-    :ok = report_rebuild_started(on_progress, total_files, "media files")
+      with {:ok, translation_sidecars} <- collect_sidecars(translation_results) do
+        total_files = length(canonical_sidecars) + length(translation_sidecars)
+        :ok = report_rebuild_started(on_progress, total_files, "media files")
 
-    media_items =
-      canonical_sidecars
-      |> Enum.with_index(1)
-      |> Enum.map(fn {sidecar, index} ->
-        media = Sidecars.upsert_media_from_sidecar(project, sidecar, sync_search: false)
-        :ok = report_rebuild_progress(on_progress, index, total_files, "media files")
-        media
-      end)
+        media_items =
+          canonical_sidecars
+          |> Enum.with_index(1)
+          |> Enum.map(fn {sidecar, index} ->
+            media = Sidecars.upsert_media_from_sidecar(project, sidecar, sync_search: false)
+            :ok = report_rebuild_progress(on_progress, index, total_files, "media files")
+            media
+          end)
 
-    canonical_media_by_binary_path =
-      Map.new(media_items, fn media ->
-        {Path.join(Projects.project_data_dir(project), media.file_path), media}
-      end)
+        canonical_media_by_binary_path =
+          Map.new(media_items, fn media ->
+            {Path.join(Projects.project_data_dir(project), media.file_path), media}
+          end)
 
-    translation_sidecars
-    |> Enum.with_index(length(canonical_sidecars) + 1)
-    |> Enum.each(fn {sidecar, index} ->
-      Sidecars.upsert_translation_from_sidecar(project, canonical_media_by_binary_path, sidecar,
-        sync_search: false
-      )
+        translation_sidecars
+        |> Enum.with_index(length(canonical_sidecars) + 1)
+        |> Enum.each(fn {sidecar, index} ->
+          Sidecars.upsert_translation_from_sidecar(project, canonical_media_by_binary_path, sidecar,
+            sync_search: false
+          )
 
-      :ok = report_rebuild_progress(on_progress, index, total_files, "media files")
-    end)
+          :ok = report_rebuild_progress(on_progress, index, total_files, "media files")
+        end)
 
-    if Keyword.get(opts, :reindex_search, true) do
-      :ok = report_rebuild_phase(on_progress, 0.99, "Refreshing media search index")
+        if Keyword.get(opts, :reindex_search, true) do
+          :ok = report_rebuild_phase(on_progress, 0.99, "Refreshing media search index")
 
-      :ok =
-        Search.reindex_media(project.id,
-          on_progress: scaled_progress_reporter(on_progress, 0.99, 1.0)
-        )
+          :ok =
+            Search.reindex_media(project.id,
+              on_progress: scaled_progress_reporter(on_progress, 0.99, 1.0)
+            )
+        end
+
+        {:ok, media_items}
+      end
     end
+  end
 
-    {:ok, media_items}
+  defp collect_sidecars(results) do
+    Enum.reduce_while(results, {:ok, []}, fn
+      {:ok, sidecar}, {:ok, sidecars} -> {:cont, {:ok, [sidecar | sidecars]}}
+      {:error, reason}, {:ok, _sidecars} -> {:halt, {:error, reason}}
+    end)
+    |> case do
+      {:ok, sidecars} -> {:ok, Enum.reverse(sidecars)}
+      {:error, reason} -> {:error, reason}
+    end
   end
 end

lib/bds/media/sidecars.ex

@@ -67,28 +67,35 @@ defmodule BDS.Media.Sidecars do
     )
   end
 
-  @spec parse_canonical_sidecar(BDS.Projects.Project.t(), Path.t()) :: map()
+  @spec parse_canonical_sidecar(BDS.Projects.Project.t(), Path.t()) ::
+          {:ok, map()} | {:error, {:read_sidecar, Path.t(), File.posix()}}
   def parse_canonical_sidecar(project, sidecar_path) do
-    {:ok, fields} = sidecar_path |> File.read!() |> Sidecar.parse_document()
-    relative_sidecar_path = Path.relative_to(sidecar_path, Projects.project_data_dir(project))
-    relative_file_path = String.trim_trailing(relative_sidecar_path, ".meta")
+    with {:ok, contents} <- read_sidecar(sidecar_path),
+         {:ok, fields} <- Sidecar.parse_document(contents) do
+      relative_sidecar_path = Path.relative_to(sidecar_path, Projects.project_data_dir(project))
+      relative_file_path = String.trim_trailing(relative_sidecar_path, ".meta")
 
-    %{
-      fields: fields,
-      relative_sidecar_path: relative_sidecar_path,
-      relative_file_path: relative_file_path,
-      filename: Path.basename(relative_file_path)
-    }
+      {:ok,
+       %{
+         fields: fields,
+         relative_sidecar_path: relative_sidecar_path,
+         relative_file_path: relative_file_path,
+         filename: Path.basename(relative_file_path)
+       }}
+    end
   end
 
-  @spec parse_translation_sidecar(Path.t()) :: map()
+  @spec parse_translation_sidecar(Path.t()) ::
+          {:ok, map()} | {:error, {:read_sidecar, Path.t(), File.posix()}}
   def parse_translation_sidecar(sidecar_path) do
-    {:ok, fields} = sidecar_path |> File.read!() |> Sidecar.parse_document()
-
-    %{
-      fields: fields,
-      binary_path: binary_path_for_translation_sidecar(sidecar_path)
-    }
+    with {:ok, contents} <- read_sidecar(sidecar_path),
+         {:ok, fields} <- Sidecar.parse_document(contents) do
+      {:ok,
+       %{
+         fields: fields,
+         binary_path: binary_path_for_translation_sidecar(sidecar_path)
+       }}
+    end
   end
 
   @spec upsert_media_from_sidecar(BDS.Projects.Project.t(), map(), keyword()) :: Media.t()
@@ -192,10 +199,12 @@ defmodule BDS.Media.Sidecars do
         project = Projects.get_project!(media.project_id)
         sidecar_path = Path.join(Projects.project_data_dir(project), media.sidecar_path)
 
-        if File.exists?(sidecar_path) do
-          {:ok, upsert_media_from_sidecar(project, parse_canonical_sidecar(project, sidecar_path), sync_search: true)}
-        else
-          {:error, :not_found}
+        case parse_existing_canonical_sidecar(project, sidecar_path) do
+          {:ok, sidecar} ->
+            {:ok, upsert_media_from_sidecar(project, sidecar, sync_search: true)}
+
+          {:error, reason} ->
+            {:error, reason}
         end
     end
   end
@@ -232,23 +241,23 @@ defmodule BDS.Media.Sidecars do
             translation_sidecar_path(media, translation.language)
           )
 
-        if File.exists?(sidecar_path) do
-          sidecar = parse_translation_sidecar(sidecar_path)
+        case parse_existing_translation_sidecar(sidecar_path) do
+          {:ok, sidecar} ->
+            case BDS.Media.upsert_media_translation(
+                   media.id,
+                   DocumentFields.fetch!(sidecar.fields, "language"),
+                   %{
+                     title: DocumentFields.get(sidecar.fields, "title"),
+                     alt: DocumentFields.get(sidecar.fields, "alt"),
+                     caption: DocumentFields.get(sidecar.fields, "caption")
+                   }
+                 ) do
+              {:ok, updated_translation} -> {:ok, updated_translation}
+              error -> error
+            end
 
-          case BDS.Media.upsert_media_translation(
-                 media.id,
-                 DocumentFields.fetch!(sidecar.fields, "language"),
-                 %{
-                   title: DocumentFields.get(sidecar.fields, "title"),
-                   alt: DocumentFields.get(sidecar.fields, "alt"),
-                   caption: DocumentFields.get(sidecar.fields, "caption")
-                 }
-               ) do
-            {:ok, updated_translation} -> {:ok, updated_translation}
-            error -> error
-          end
-        else
-          {:error, :not_found}
+          {:error, reason} ->
+            {:error, reason}
         end
     end
   end
@@ -259,10 +268,12 @@ defmodule BDS.Media.Sidecars do
     project = Projects.get_project!(project_id)
     sidecar_path = Path.join(Projects.project_data_dir(project), relative_path)
 
-    if File.exists?(sidecar_path) do
-      {:ok, upsert_media_from_sidecar(project, parse_canonical_sidecar(project, sidecar_path), sync_search: true)}
-    else
-      {:error, :not_found}
+    case parse_existing_canonical_sidecar(project, sidecar_path) do
+      {:ok, sidecar} ->
+        {:ok, upsert_media_from_sidecar(project, sidecar, sync_search: true)}
+
+      {:error, reason} ->
+        {:error, reason}
     end
   end
 
@@ -272,35 +283,35 @@ defmodule BDS.Media.Sidecars do
     project = Projects.get_project!(project_id)
     sidecar_path = Path.join(Projects.project_data_dir(project), relative_path)
 
-    if File.exists?(sidecar_path) do
-      sidecar = parse_translation_sidecar(sidecar_path)
+    case parse_existing_translation_sidecar(sidecar_path) do
+      {:ok, sidecar} ->
+        case Repo.get(Media, DocumentFields.get(sidecar.fields, "translationFor")) do
+          nil ->
+            {:error, :not_found}
 
-      case Repo.get(Media, DocumentFields.get(sidecar.fields, "translationFor")) do
-        nil ->
-          {:error, :not_found}
+          media ->
+            case Repo.get_by(Translation,
+                   translation_for: media.id,
+                   language: DocumentFields.fetch!(sidecar.fields, "language")
+                 ) do
+              nil ->
+                BDS.Media.upsert_media_translation(
+                  media.id,
+                  DocumentFields.fetch!(sidecar.fields, "language"),
+                  %{
+                    title: DocumentFields.get(sidecar.fields, "title"),
+                    alt: DocumentFields.get(sidecar.fields, "alt"),
+                    caption: DocumentFields.get(sidecar.fields, "caption")
+                  }
+                )
 
-        media ->
-          case Repo.get_by(Translation,
-                 translation_for: media.id,
-                 language: DocumentFields.fetch!(sidecar.fields, "language")
-               ) do
-            nil ->
-              BDS.Media.upsert_media_translation(
-                media.id,
-                DocumentFields.fetch!(sidecar.fields, "language"),
-                %{
-                  title: DocumentFields.get(sidecar.fields, "title"),
-                  alt: DocumentFields.get(sidecar.fields, "alt"),
-                  caption: DocumentFields.get(sidecar.fields, "caption")
-                }
-              )
+              _translation ->
+                {:error, :conflict}
+            end
+        end
 
-            _translation ->
-              {:error, :conflict}
-          end
-      end
-    else
-      {:error, :not_found}
+      {:error, reason} ->
+        {:error, reason}
     end
   end
 
@@ -326,4 +337,27 @@ defmodule BDS.Media.Sidecars do
     |> String.trim_trailing(".meta")
     |> File.exists?()
   end
+
+  defp parse_existing_canonical_sidecar(project, sidecar_path) do
+    if File.exists?(sidecar_path) do
+      parse_canonical_sidecar(project, sidecar_path)
+    else
+      {:error, :not_found}
+    end
+  end
+
+  defp parse_existing_translation_sidecar(sidecar_path) do
+    if File.exists?(sidecar_path) do
+      parse_translation_sidecar(sidecar_path)
+    else
+      {:error, :not_found}
+    end
+  end
+
+  defp read_sidecar(sidecar_path) do
+    case File.read(sidecar_path) do
+      {:ok, contents} -> {:ok, contents}
+      {:error, reason} -> {:error, {:read_sidecar, sidecar_path, reason}}
+    end
+  end
 end