diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index 0605bcd..989668e 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/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 diff --git a/lib/pinchflat/media/file_deletion.ex b/lib/pinchflat/media/file_syncing.ex similarity index 55% rename from lib/pinchflat/media/file_deletion.ex rename to lib/pinchflat/media/file_syncing.ex index 7ed01a8..7fb5b65 100644 --- a/lib/pinchflat/media/file_deletion.ex +++ b/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 diff --git a/lib/pinchflat/media/file_syncing_worker.ex b/lib/pinchflat/media/file_syncing_worker.ex new file mode 100644 index 0000000..fde7e73 --- /dev/null +++ b/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 diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index 1573762..9c48cec 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/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) diff --git a/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex b/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex index 43b4c22..0495384 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex +++ b/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 @@ -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 @@ -70,6 +70,15 @@ Refresh Metadata + <: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 + + <:option>
diff --git a/lib/pinchflat_web/router.ex b/lib/pinchflat_web/router.ex index 1e1ee7d..9ba7068 100644 --- a/lib/pinchflat_web/router.ex +++ b/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 diff --git a/test/pinchflat/media/file_deletion_test.exs b/test/pinchflat/media/file_deletion_test.exs deleted file mode 100644 index 6494f21..0000000 --- a/test/pinchflat/media/file_deletion_test.exs +++ /dev/null @@ -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 diff --git a/test/pinchflat/media/file_syncing_test.exs b/test/pinchflat/media/file_syncing_test.exs new file mode 100644 index 0000000..c69c795 --- /dev/null +++ b/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 diff --git a/test/pinchflat/media/file_syncing_worker_test.exs b/test/pinchflat/media/file_syncing_worker_test.exs new file mode 100644 index 0000000..37ebafa --- /dev/null +++ b/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 diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index 5d532da..5a6e0e7 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/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})