[Enhancement] Added ability to detect when files have been deleted (#399)

* Added function for updating a media item's filepaths on-disk

* Added placeholder action to source page

* Turned the file sync into a job and properly hooked it up to the controller

lib/pinchflat/downloading/media_download_worker.ex

@@ -12,7 +12,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
   alias Pinchflat.Tasks
   alias Pinchflat.Repo
   alias Pinchflat.Media
-  alias Pinchflat.Media.FileDeletion
+  alias Pinchflat.Media.FileSyncing
   alias Pinchflat.Downloading.MediaDownloader
 
   alias Pinchflat.Lifecycle.UserScripts.CommandRunner, as: UserScriptRunner
@@ -86,7 +86,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
             media_redownloaded_at: get_redownloaded_at(is_quality_upgrade)
           })
 
-        :ok = FileDeletion.delete_outdated_files(media_item, updated_media_item)
+        :ok = FileSyncing.delete_outdated_files(media_item, updated_media_item)
         run_user_script(:media_downloaded, updated_media_item)
 
         :ok

lib/pinchflat/media/file_syncing.ex

@@ -1,8 +1,9 @@
-defmodule Pinchflat.Media.FileDeletion do
+defmodule Pinchflat.Media.FileSyncing do
   @moduledoc """
-  Functions for deleting files that are no longer needed by media items.
+  Functions for ensuring file state is accurately reflected in the database.
   """
 
+  alias Pinchflat.Media
   alias Pinchflat.Utils.MapUtils
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Utils.FilesystemUtils, as: FSUtils
@@ -34,6 +35,22 @@ defmodule Pinchflat.Media.FileDeletion do
     :ok
   end
 
+  @doc """
+  Nillifies any media item filepaths that don't exist on disk for a list of media items
+
+  returns [%MediaItem{}]
+  """
+  def sync_file_presence_on_disk(media_items) do
+    Enum.map(media_items, fn media_item ->
+      new_attributes = sync_media_item_files(media_item)
+      # Doing this one-by-one instead of batching since this process
+      # can take time and a batch could let MediaItem state get out of sync
+      {:ok, updated_media_item} = Media.update_media_item(media_item, new_attributes)
+
+      updated_media_item
+    end)
+  end
+
   defp handle_file_deletion(old_attributes, new_attributes) do
     # The logic:
     #   - A file should only be deleted if it exists and the new file is different
@@ -51,4 +68,26 @@ defmodule Pinchflat.Media.FileDeletion do
       end
     end)
   end
+
+  defp sync_media_item_files(media_item) do
+    non_subtitle_keys = MediaItem.filepath_attributes() -- [:subtitle_filepaths]
+    subtitle_keys = MapUtils.from_nested_list(media_item.subtitle_filepaths)
+    non_subtitles = Map.take(media_item, non_subtitle_keys)
+
+    # This one is checking for the negative (ie: only update if the file doesn't exist)
+    new_non_subtitle_attrs =
+      Enum.reduce(non_subtitles, %{}, fn {key, filepath}, acc ->
+        if filepath && File.exists?(filepath), do: acc, else: Map.put(acc, key, nil)
+      end)
+
+    # This one is checking for the positive (ie: only update if the file exists)
+    # This is because subtitles, being an array type in the DB, are most easily updated
+    # by a full replacement rather than finding the actual diff
+    new_subtitle_attrs =
+      Enum.reduce(subtitle_keys, [], fn {key, filepath}, acc ->
+        if filepath && File.exists?(filepath), do: acc ++ [[key, filepath]], else: acc
+      end)
+
+    Map.put(new_non_subtitle_attrs, :subtitle_filepaths, new_subtitle_attrs)
+  end
 end

lib/pinchflat/media/file_syncing_worker.ex

@@ -0,0 +1,38 @@
+defmodule Pinchflat.Media.FileSyncingWorker do
+  @moduledoc false
+
+  use Oban.Worker,
+    queue: :local_data,
+    tags: ["sources", "local_data"]
+
+  alias __MODULE__
+  alias Pinchflat.Repo
+  alias Pinchflat.Tasks
+  alias Pinchflat.Sources
+  alias Pinchflat.Media.FileSyncing
+
+  @doc """
+  Starts the source file syncing worker.
+
+  Returns {:ok, %Task{}} | {:error, %Ecto.Changeset{}}
+  """
+  def kickoff_with_task(source, opts \\ []) do
+    %{id: source.id}
+    |> FileSyncingWorker.new(opts)
+    |> Tasks.create_job_with_task(source)
+  end
+
+  @doc """
+  Deletes a profile and optionally deletes its files
+
+  Returns :ok
+  """
+  @impl Oban.Worker
+  def perform(%Oban.Job{args: %{"id" => source_id}}) do
+    source = Repo.preload(Sources.get_source!(source_id), :media_items)
+
+    FileSyncing.sync_file_presence_on_disk(source.media_items)
+
+    :ok
+  end
+end

lib/pinchflat_web/controllers/sources/source_controller.ex

@@ -8,6 +8,7 @@ defmodule PinchflatWeb.Sources.SourceController do
   alias Pinchflat.Sources.Source
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Media.FileSyncingWorker
   alias Pinchflat.Sources.SourceDeletionWorker
   alias Pinchflat.Downloading.DownloadingHelpers
   alias Pinchflat.SlowIndexing.SlowIndexingHelpers
@@ -175,6 +176,15 @@ defmodule PinchflatWeb.Sources.SourceController do
     )
   end
 
+  def sync_files_on_disk(conn, %{"source_id" => id}) do
+    wrap_forced_action(
+      conn,
+      id,
+      "File sync enqueued.",
+      &FileSyncingWorker.kickoff_with_task/1
+    )
+  end
+
   defp wrap_forced_action(conn, source_id, message, fun) do
     source = Sources.get_source!(source_id)
     fun.(source)

lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex

@@ -38,7 +38,7 @@
     <.link
       href={~p"/sources/#{@source}/force_download_pending"}
       method="post"
-      data-confirm="Are you sure you want to force a download of all *pending* media items? This isn't normally needed."
+      data-confirm="Are you sure you want to force a download of all pending media items? This isn't normally needed."
     >
       Download Pending
     </.link>
@@ -47,7 +47,7 @@
     <.link
       href={~p"/sources/#{@source}/force_redownload"}
       method="post"
-      data-confirm="Are you sure you want to re-download all currently downloaded media items? This isn't normally needed and won't change anything if the files already exist."
+      data-confirm="Are you sure you want to re-download all currently downloaded media items? This doesn't upgrade your media, but will download any missing files if your settings have changed. This isn't normally needed."
     >
       Redownload Existing
     </.link>
@@ -70,6 +70,15 @@
       Refresh Metadata
     </.link>
   </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@source}/sync_files_on_disk"}
+      method="post"
+      data-confirm="Are you sure you want to sync files? This will update media items if their files have been deleted. File addition or moves are not detected. This isn't normally needed."
+    >
+      Sync Files on Disk
+    </.link>
+  </:option>
   <:option>
     <div class="h-px w-full bg-bodydark2"></div>
   </:option>

lib/pinchflat_web/router.ex

@@ -40,6 +40,7 @@ defmodule PinchflatWeb.Router do
       post "/force_redownload", Sources.SourceController, :force_redownload
       post "/force_index", Sources.SourceController, :force_index
       post "/force_metadata_refresh", Sources.SourceController, :force_metadata_refresh
+      post "/sync_files_on_disk", Sources.SourceController, :sync_files_on_disk
 
       resources "/media", MediaItems.MediaItemController, only: [:show, :edit, :update, :delete] do
         post "/force_download", MediaItems.MediaItemController, :force_download

test/pinchflat/media/file_deletion_test.exs

@@ -1,77 +0,0 @@
-defmodule Pinchflat.Media.FileDeletionTest do
-  use Pinchflat.DataCase
-
-  import Pinchflat.MediaFixtures
-
-  alias Pinchflat.Media.FileDeletion
-
-  describe "delete_outdated_files/2" do
-    test "deletes outdated non-subtitle files" do
-      new_media_item = media_item_with_attachments()
-      old_media_item = media_item_with_attachments()
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(new_media_item.media_filepath)
-      refute File.exists?(old_media_item.media_filepath)
-    end
-
-    test "doesn't delete non-subtitle files if the new file is the same" do
-      new_media_item = media_item_with_attachments()
-      old_media_item = media_item_fixture(%{media_filepath: new_media_item.media_filepath})
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(new_media_item.media_filepath)
-      assert File.exists?(old_media_item.media_filepath)
-    end
-
-    test "doesn't delete the old file if the new file is missing that key" do
-      new_media_item = media_item_fixture(%{media_filepath: nil})
-      old_media_item = media_item_with_attachments()
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(old_media_item.media_filepath)
-    end
-
-    test "deletes outdated subtitle files" do
-      new_media_item = media_item_with_attachments()
-      old_media_item = media_item_with_attachments()
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(get_subtitle_filepath(new_media_item, "en"))
-      refute File.exists?(get_subtitle_filepath(old_media_item, "en"))
-    end
-
-    test "keeps old subtitle files if the new file is the same" do
-      new_media_item = media_item_with_attachments()
-      old_media_item = media_item_fixture(%{subtitle_filepaths: new_media_item.subtitle_filepaths})
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(get_subtitle_filepath(new_media_item, "en"))
-      assert File.exists?(get_subtitle_filepath(old_media_item, "en"))
-    end
-
-    test "doesn't delete old subtitle files if the new file is missing that key" do
-      new_media_item = media_item_fixture(%{subtitle_filepaths: []})
-      old_media_item = media_item_with_attachments()
-
-      assert :ok = FileDeletion.delete_outdated_files(old_media_item, new_media_item)
-
-      assert File.exists?(get_subtitle_filepath(old_media_item, "en"))
-    end
-  end
-
-  defp get_subtitle_filepath(media_item, language) do
-    Enum.reduce_while(media_item.subtitle_filepaths, nil, fn [lang, filepath], acc ->
-      if lang == language do
-        {:halt, filepath}
-      else
-        {:cont, acc}
-      end
-    end)
-  end
-end

test/pinchflat/media/file_syncing_test.exs

@@ -0,0 +1,122 @@
+defmodule Pinchflat.Media.FileSyncingTest do
+  use Pinchflat.DataCase
+
+  import Pinchflat.MediaFixtures
+
+  alias Pinchflat.Media.FileSyncing
+
+  describe "delete_outdated_files/2" do
+    test "deletes outdated non-subtitle files" do
+      new_media_item = media_item_with_attachments()
+      old_media_item = media_item_with_attachments()
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(new_media_item.media_filepath)
+      refute File.exists?(old_media_item.media_filepath)
+    end
+
+    test "doesn't delete non-subtitle files if the new file is the same" do
+      new_media_item = media_item_with_attachments()
+      old_media_item = media_item_fixture(%{media_filepath: new_media_item.media_filepath})
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(new_media_item.media_filepath)
+      assert File.exists?(old_media_item.media_filepath)
+    end
+
+    test "doesn't delete the old file if the new file is missing that key" do
+      new_media_item = media_item_fixture(%{media_filepath: nil})
+      old_media_item = media_item_with_attachments()
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(old_media_item.media_filepath)
+    end
+
+    test "deletes outdated subtitle files" do
+      new_media_item = media_item_with_attachments()
+      old_media_item = media_item_with_attachments()
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(get_subtitle_filepath(new_media_item, "en"))
+      refute File.exists?(get_subtitle_filepath(old_media_item, "en"))
+    end
+
+    test "keeps old subtitle files if the new file is the same" do
+      new_media_item = media_item_with_attachments()
+      old_media_item = media_item_fixture(%{subtitle_filepaths: new_media_item.subtitle_filepaths})
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(get_subtitle_filepath(new_media_item, "en"))
+      assert File.exists?(get_subtitle_filepath(old_media_item, "en"))
+    end
+
+    test "doesn't delete old subtitle files if the new file is missing that key" do
+      new_media_item = media_item_fixture(%{subtitle_filepaths: []})
+      old_media_item = media_item_with_attachments()
+
+      assert :ok = FileSyncing.delete_outdated_files(old_media_item, new_media_item)
+
+      assert File.exists?(get_subtitle_filepath(old_media_item, "en"))
+    end
+  end
+
+  describe "sync_file_presence_on_disk/1" do
+    test "removes attributes whose files are missing" do
+      media_item = media_item_fixture(%{media_filepath: "/tmp/missing_file.mp4"})
+
+      assert media_item.media_filepath
+      assert [updated_media_item] = FileSyncing.sync_file_presence_on_disk([media_item])
+      refute updated_media_item.media_filepath
+    end
+
+    test "doesn't remove attributes where the files still exist" do
+      media_item = media_item_with_attachments()
+
+      assert media_item.media_filepath
+      assert [updated_media_item] = FileSyncing.sync_file_presence_on_disk([media_item])
+      assert updated_media_item.media_filepath
+    end
+
+    test "doesn't touch other attributes if some are missing and some aren't" do
+      media_item = media_item_with_attachments()
+      File.rm(media_item.media_filepath)
+
+      assert media_item.thumbnail_filepath
+      assert media_item.media_filepath
+      assert [updated_media_item] = FileSyncing.sync_file_presence_on_disk([media_item])
+      assert updated_media_item.thumbnail_filepath
+      refute updated_media_item.media_filepath
+    end
+
+    test "removes subtitle files that are missing" do
+      media_item = media_item_fixture(%{subtitle_filepaths: [["en", "/tmp/missing_file.srt"]]})
+
+      assert get_subtitle_filepath(media_item, "en")
+      assert [updated_media_item] = FileSyncing.sync_file_presence_on_disk([media_item])
+      refute get_subtitle_filepath(updated_media_item, "en")
+    end
+
+    test "doesn't remove subtitle files that still exist" do
+      media_item = media_item_with_attachments()
+
+      assert get_subtitle_filepath(media_item, "en")
+      assert [updated_media_item] = FileSyncing.sync_file_presence_on_disk([media_item])
+      assert get_subtitle_filepath(updated_media_item, "en")
+    end
+  end
+
+  defp get_subtitle_filepath(media_item, language) do
+    Enum.reduce_while(media_item.subtitle_filepaths, nil, fn [lang, filepath], acc ->
+      if lang == language do
+        {:halt, filepath}
+      else
+        {:cont, acc}
+      end
+    end)
+  end
+end

test/pinchflat/media/file_syncing_worker_test.exs

@@ -0,0 +1,37 @@
+defmodule Pinchflat.Media.FileSyncingWorkerTest do
+  use Pinchflat.DataCase
+
+  import Pinchflat.MediaFixtures
+  import Pinchflat.SourcesFixtures
+
+  alias Pinchflat.Media.FileSyncingWorker
+
+  describe "kickoff_with_task/3" do
+    test "starts the worker" do
+      source = source_fixture()
+
+      assert [] = all_enqueued(worker: FileSyncingWorker)
+      assert {:ok, _} = FileSyncingWorker.kickoff_with_task(source)
+      assert [_] = all_enqueued(worker: FileSyncingWorker)
+    end
+
+    test "attaches a task" do
+      source = source_fixture()
+
+      assert {:ok, task} = FileSyncingWorker.kickoff_with_task(source)
+      assert task.source_id == source.id
+    end
+  end
+
+  describe "perform/1" do
+    test "syncs file presence on disk" do
+      source = source_fixture()
+      media_item = media_item_fixture(%{media_filepath: "/tmp/missing.mp4", source_id: source.id})
+
+      perform_job(FileSyncingWorker, %{"id" => source.id})
+      updated_media_item = Repo.reload!(media_item)
+
+      refute updated_media_item.media_filepath
+    end
+  end
+end

test/pinchflat_web/controllers/source_controller_test.exs

@@ -7,6 +7,7 @@ defmodule PinchflatWeb.SourceControllerTest do
 
   alias Pinchflat.Repo
   alias Pinchflat.Settings
+  alias Pinchflat.Media.FileSyncingWorker
   alias Pinchflat.Sources.SourceDeletionWorker
   alias Pinchflat.Downloading.MediaDownloadWorker
   alias Pinchflat.Metadata.SourceMetadataStorageWorker
@@ -268,6 +269,23 @@ defmodule PinchflatWeb.SourceControllerTest do
     end
   end
 
+  describe "sync_files_on_disk" do
+    test "forces a file sync", %{conn: conn} do
+      source = source_fixture()
+
+      assert [] = all_enqueued(worker: FileSyncingWorker)
+      post(conn, ~p"/sources/#{source.id}/sync_files_on_disk")
+      assert [_] = all_enqueued(worker: FileSyncingWorker)
+    end
+
+    test "redirects to the source page", %{conn: conn} do
+      source = source_fixture()
+
+      conn = post(conn, ~p"/sources/#{source.id}/sync_files_on_disk")
+      assert redirected_to(conn) == ~p"/sources/#{source.id}"
+    end
+  end
+
   defp create_source(_) do
     source = source_fixture()
     media_item = media_item_with_attachments(%{source_id: source.id})