diff --git a/config/config.exs b/config/config.exs index 742b3c1..aab4d10 100644 --- a/config/config.exs +++ b/config/config.exs @@ -65,7 +65,7 @@ config :pinchflat, Oban, media_indexing: 2, media_collection_indexing: 2, media_fetching: 2, - local_metadata: 8, + local_data: 8, remote_metadata: 4 ] diff --git a/lib/pinchflat/downloading/media_retention_worker.ex b/lib/pinchflat/downloading/media_retention_worker.ex index 0835438..3a36100 100644 --- a/lib/pinchflat/downloading/media_retention_worker.ex +++ b/lib/pinchflat/downloading/media_retention_worker.ex @@ -2,9 +2,9 @@ defmodule Pinchflat.Downloading.MediaRetentionWorker do @moduledoc false use Oban.Worker, - queue: :local_metadata, + queue: :local_data, unique: [period: :infinity, states: [:available, :scheduled, :retryable, :executing]], - tags: ["media_item", "local_metadata"] + tags: ["media_item", "local_data"] use Pinchflat.Media.MediaQuery diff --git a/lib/pinchflat/profiles/media_profile.ex b/lib/pinchflat/profiles/media_profile.ex index 3111640..da86df3 100644 --- a/lib/pinchflat/profiles/media_profile.ex +++ b/lib/pinchflat/profiles/media_profile.ex @@ -28,6 +28,7 @@ defmodule Pinchflat.Profiles.MediaProfile do livestream_behaviour preferred_resolution redownload_delay_days + marked_for_deletion_at )a @required_fields ~w(name output_path_template)a @@ -65,6 +66,8 @@ defmodule Pinchflat.Profiles.MediaProfile do field :livestream_behaviour, Ecto.Enum, values: ~w(include exclude only)a, default: :include field :preferred_resolution, Ecto.Enum, values: ~w(4320p 2160p 1080p 720p 480p 360p audio)a, default: :"1080p" + field :marked_for_deletion_at, :utc_datetime + has_many :sources, Source timestamps(type: :utc_datetime) diff --git a/lib/pinchflat/profiles/media_profile_deletion_worker.ex b/lib/pinchflat/profiles/media_profile_deletion_worker.ex new file mode 100644 index 0000000..529474d --- /dev/null +++ b/lib/pinchflat/profiles/media_profile_deletion_worker.ex @@ -0,0 +1,38 @@ +defmodule Pinchflat.Profiles.MediaProfileDeletionWorker do + @moduledoc false + + use Oban.Worker, + queue: :local_data, + tags: ["media_profiles", "local_data"] + + require Logger + + alias __MODULE__ + alias Pinchflat.Profiles + + @doc """ + Starts the profile deletion worker. Does not attach it to a task like `kickoff_with_task/2` + since deletion also cancels all tasks for the profile + + Returns {:ok, %Task{}} | {:error, %Ecto.Changeset{}} + """ + def kickoff(profile, job_args \\ %{}, job_opts \\ []) do + %{id: profile.id} + |> Map.merge(job_args) + |> MediaProfileDeletionWorker.new(job_opts) + |> Oban.insert() + end + + @doc """ + Deletes a profile and optionally deletes its files + + Returns :ok + """ + @impl Oban.Worker + def perform(%Oban.Job{args: %{"id" => profile_id} = args}) do + delete_files = Map.get(args, "delete_files", false) + profile = Profiles.get_media_profile!(profile_id) + + Profiles.delete_media_profile(profile, delete_files: delete_files) + end +end diff --git a/lib/pinchflat/sources/source.ex b/lib/pinchflat/sources/source.ex index 418c828..b6d9154 100644 --- a/lib/pinchflat/sources/source.ex +++ b/lib/pinchflat/sources/source.ex @@ -35,6 +35,7 @@ defmodule Pinchflat.Sources.Source do title_filter_regex media_profile_id output_path_template_override + marked_for_deletion_at )a # Expensive API calls are made when a source is inserted/updated so @@ -87,6 +88,8 @@ defmodule Pinchflat.Sources.Source do field :fanart_filepath, :string field :banner_filepath, :string + field :marked_for_deletion_at, :utc_datetime + belongs_to :media_profile, MediaProfile has_one :metadata, SourceMetadata, on_replace: :update diff --git a/lib/pinchflat/sources/source_deletion_worker.ex b/lib/pinchflat/sources/source_deletion_worker.ex new file mode 100644 index 0000000..9c36837 --- /dev/null +++ b/lib/pinchflat/sources/source_deletion_worker.ex @@ -0,0 +1,38 @@ +defmodule Pinchflat.Sources.SourceDeletionWorker do + @moduledoc false + + use Oban.Worker, + queue: :local_data, + tags: ["sources", "local_data"] + + require Logger + + alias __MODULE__ + alias Pinchflat.Sources + + @doc """ + Starts the source deletion worker. Does not attach it to a task like `kickoff_with_task/2` + since deletion also cancels all tasks for the source + + Returns {:ok, %Task{}} | {:error, %Ecto.Changeset{}} + """ + def kickoff(source, job_args \\ %{}, job_opts \\ []) do + %{id: source.id} + |> Map.merge(job_args) + |> SourceDeletionWorker.new(job_opts) + |> Oban.insert() + end + + @doc """ + Deletes a source and optionally deletes its files + + Returns :ok + """ + @impl Oban.Worker + def perform(%Oban.Job{args: %{"id" => source_id} = args}) do + delete_files = Map.get(args, "delete_files", false) + source = Sources.get_source!(source_id) + + Sources.delete_source(source, delete_files: delete_files) + end +end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex index 22a4916..34b7bf8 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex @@ -5,10 +5,12 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do alias Pinchflat.Repo alias Pinchflat.Profiles alias Pinchflat.Profiles.MediaProfile + alias Pinchflat.Profiles.MediaProfileDeletionWorker def index(conn, _params) do media_profiles = MediaProfile + |> where([mp], is_nil(mp.marked_for_deletion_at)) |> order_by(asc: :name) |> Repo.all() @@ -70,19 +72,15 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do end def delete(conn, %{"id" => id} = params) do - delete_files = Map.get(params, "delete_files", false) + # This awkward comparison converts the string to a boolean + delete_files = Map.get(params, "delete_files", "") == "true" media_profile = Profiles.get_media_profile!(id) - {:ok, _media_profile} = Profiles.delete_media_profile(media_profile, delete_files: delete_files) - flash_message = - if delete_files do - "Media profile, its sources, and its files deleted successfully." - else - "Media profile and its sources deleted successfully. Files were not deleted." - end + {:ok, _} = Profiles.update_media_profile(media_profile, %{marked_for_deletion_at: DateTime.utc_now()}) + MediaProfileDeletionWorker.kickoff(media_profile, %{delete_files: delete_files}) conn - |> put_flash(:info, flash_message) + |> put_flash(:info, "Media Profile deletion started. This may take a while to complete.") |> redirect(to: ~p"/media_profiles") end diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index d11dd08..ef00392 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.Sources.SourceDeletionWorker alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.SlowIndexing.SlowIndexingHelpers alias Pinchflat.Metadata.SourceMetadataStorageWorker @@ -17,6 +18,7 @@ defmodule PinchflatWeb.Sources.SourceController do from s in Source, as: :source, inner_join: mp in assoc(s, :media_profile), + where: is_nil(s.marked_for_deletion_at) and is_nil(mp.marked_for_deletion_at), preload: [media_profile: mp], order_by: [asc: s.custom_name], select: map(s, ^Source.__schema__(:fields)), @@ -124,19 +126,15 @@ defmodule PinchflatWeb.Sources.SourceController do end def delete(conn, %{"id" => id} = params) do - delete_files = Map.get(params, "delete_files", false) + # This awkward comparison converts the string to a boolean + delete_files = Map.get(params, "delete_files", "") == "true" source = Sources.get_source!(id) - {:ok, _source} = Sources.delete_source(source, delete_files: delete_files) - flash_message = - if delete_files do - "Source and files deleted successfully." - else - "Source deleted successfully. Files were not deleted." - end + {:ok, _} = Sources.update_source(source, %{marked_for_deletion_at: DateTime.utc_now()}) + SourceDeletionWorker.kickoff(source, %{delete_files: delete_files}) conn - |> put_flash(:info, flash_message) + |> put_flash(:info, "Source deletion started. This may take a while to complete.") |> redirect(to: ~p"/sources") end diff --git a/priv/repo/erd.png b/priv/repo/erd.png index 50e0ec4..70f395c 100644 Binary files a/priv/repo/erd.png and b/priv/repo/erd.png differ diff --git a/priv/repo/migrations/20240722183656_add_marked_for_deletion_at_to_sources_and_profiles.exs b/priv/repo/migrations/20240722183656_add_marked_for_deletion_at_to_sources_and_profiles.exs new file mode 100644 index 0000000..81a99e0 --- /dev/null +++ b/priv/repo/migrations/20240722183656_add_marked_for_deletion_at_to_sources_and_profiles.exs @@ -0,0 +1,13 @@ +defmodule Pinchflat.Repo.Migrations.AddMarkedForDeletionAtToSources do + use Ecto.Migration + + def change do + alter table(:sources) do + add :marked_for_deletion_at, :utc_datetime + end + + alter table(:media_profiles) do + add :marked_for_deletion_at, :utc_datetime + end + end +end diff --git a/test/pinchflat/profiles/media_profile_deletion_worker_test.exs b/test/pinchflat/profiles/media_profile_deletion_worker_test.exs new file mode 100644 index 0000000..b91aace --- /dev/null +++ b/test/pinchflat/profiles/media_profile_deletion_worker_test.exs @@ -0,0 +1,57 @@ +defmodule Pinchflat.Profiles.MediaProfileDeletionWorkerTest do + use Pinchflat.DataCase + + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + import Pinchflat.ProfilesFixtures + + alias Pinchflat.Profiles.MediaProfileDeletionWorker + + setup do + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + + {:ok, %{profile: media_profile_fixture()}} + end + + describe "kickoff/3" do + test "starts the worker", %{profile: profile} do + assert [] = all_enqueued(worker: MediaProfileDeletionWorker) + assert {:ok, _} = MediaProfileDeletionWorker.kickoff(profile) + assert [_] = all_enqueued(worker: MediaProfileDeletionWorker) + end + + test "can be called with additional job arguments", %{profile: profile} do + job_args = %{"delete_files" => true} + + assert {:ok, _} = MediaProfileDeletionWorker.kickoff(profile, job_args) + + assert_enqueued(worker: MediaProfileDeletionWorker, args: %{"id" => profile.id, "delete_files" => true}) + end + end + + describe "perform/1" do + test "deletes the profile, sources, and media but leaves the files", %{profile: profile} do + source = source_fixture(%{media_profile_id: profile.id}) + media_item = media_item_with_attachments(%{source_id: source.id}) + + perform_job(MediaProfileDeletionWorker, %{"id" => profile.id}) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(profile) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + assert File.exists?(media_item.media_filepath) + end + + test "deletes the profile, sources, and media and files if specified", %{profile: profile} do + source = source_fixture(%{media_profile_id: profile.id}) + media_item = media_item_with_attachments(%{source_id: source.id}) + + perform_job(MediaProfileDeletionWorker, %{"id" => profile.id, "delete_files" => true}) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(profile) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + refute File.exists?(media_item.media_filepath) + end + end +end diff --git a/test/pinchflat/sources/source_deletion_worker_test.exs b/test/pinchflat/sources/source_deletion_worker_test.exs new file mode 100644 index 0000000..fe2ec3d --- /dev/null +++ b/test/pinchflat/sources/source_deletion_worker_test.exs @@ -0,0 +1,52 @@ +defmodule Pinchflat.Sources.SourceDeletionWorkerTest do + use Pinchflat.DataCase + + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + + alias Pinchflat.Sources.SourceDeletionWorker + + setup do + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + + {:ok, %{source: source_fixture()}} + end + + describe "kickoff/3" do + test "starts the worker", %{source: source} do + assert [] = all_enqueued(worker: SourceDeletionWorker) + assert {:ok, _} = SourceDeletionWorker.kickoff(source) + assert [_] = all_enqueued(worker: SourceDeletionWorker) + end + + test "can be called with additional job arguments", %{source: source} do + job_args = %{"delete_files" => true} + + assert {:ok, _} = SourceDeletionWorker.kickoff(source, job_args) + + assert_enqueued(worker: SourceDeletionWorker, args: %{"id" => source.id, "delete_files" => true}) + end + end + + describe "perform/1" do + test "deletes the source but leaves the files", %{source: source} do + media_item = media_item_with_attachments(%{source_id: source.id}) + + perform_job(SourceDeletionWorker, %{"id" => source.id}) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + assert File.exists?(media_item.media_filepath) + end + + test "deletes the source and files if specified", %{source: source} do + media_item = media_item_with_attachments(%{source_id: source.id}) + + perform_job(SourceDeletionWorker, %{"id" => source.id, "delete_files" => true}) + + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end + assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end + refute File.exists?(media_item.media_filepath) + end + end +end diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index 03e9a8f..8c908de 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -1,12 +1,11 @@ defmodule PinchflatWeb.MediaProfileControllerTest do use PinchflatWeb.ConnCase - import Pinchflat.MediaFixtures - import Pinchflat.SourcesFixtures import Pinchflat.ProfilesFixtures alias Pinchflat.Repo alias Pinchflat.Settings + alias Pinchflat.Profiles.MediaProfileDeletionWorker @create_attrs %{name: "some name", output_path_template: "output_template.{{ ext }}"} @update_attrs %{ @@ -23,8 +22,17 @@ defmodule PinchflatWeb.MediaProfileControllerTest do describe "index" do test "lists all media_profiles", %{conn: conn} do + profile = media_profile_fixture() conn = get(conn, ~p"/media_profiles") + assert html_response(conn, 200) =~ "Media Profiles" + assert html_response(conn, 200) =~ profile.name + end + + test "omits profiles that have marked_for_deletion_at set", %{conn: conn} do + profile = media_profile_fixture(marked_for_deletion_at: DateTime.utc_now()) + conn = get(conn, ~p"/media_profiles") + refute html_response(conn, 200) =~ profile.name end end @@ -102,34 +110,28 @@ defmodule PinchflatWeb.MediaProfileControllerTest do end end - describe "delete media_profile when just deleting the records" do + describe "delete media_profile in all cases" do setup [:create_media_profile] - test "deletes chosen media_profile and its associations", %{conn: conn, media_profile: media_profile} do - source = source_fixture(media_profile_id: media_profile.id) - media_item = media_item_with_attachments(%{source_id: source.id}) - - conn = delete(conn, ~p"/media_profiles/#{media_profile}") - assert redirected_to(conn) == ~p"/media_profiles" - - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end - end - test "redirects to the media_profiles page", %{conn: conn, media_profile: media_profile} do conn = delete(conn, ~p"/media_profiles/#{media_profile}") assert redirected_to(conn) == ~p"/media_profiles" end - test "doesn't delete any files", %{conn: conn, media_profile: media_profile} do - source = source_fixture(media_profile_id: media_profile.id) - media_item = media_item_with_attachments(%{source_id: source.id}) + test "sets marked_for_deletion_at", %{conn: conn, media_profile: media_profile} do + delete(conn, ~p"/media_profiles/#{media_profile}") + assert Repo.reload!(media_profile).marked_for_deletion_at + end + end + describe "delete media_profile when just deleting the records" do + setup [:create_media_profile] + + test "enqueues a job without the delete_files arg", %{conn: conn, media_profile: media_profile} do delete(conn, ~p"/media_profiles/#{media_profile}") - assert File.exists?(media_item.media_filepath) + assert [%{args: %{"delete_files" => false}}] = all_enqueued(worker: MediaProfileDeletionWorker) end end @@ -142,31 +144,10 @@ defmodule PinchflatWeb.MediaProfileControllerTest do :ok end - test "deletes chosen media_profile and its associations", %{conn: conn, media_profile: media_profile} do - source = source_fixture(media_profile_id: media_profile.id) - media_item = media_item_with_attachments(%{source_id: source.id}) - - conn = delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") - assert redirected_to(conn) == ~p"/media_profiles" - - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_profile) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end - end - - test "redirects to the media_profiles page", %{conn: conn, media_profile: media_profile} do - conn = delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") - - assert redirected_to(conn) == ~p"/media_profiles" - end - - test "deletes the files", %{conn: conn, media_profile: media_profile} do - source = source_fixture(media_profile_id: media_profile.id) - media_item = media_item_with_attachments(%{source_id: source.id}) - + test "enqueues a job with the delete_files arg", %{conn: conn, media_profile: media_profile} do delete(conn, ~p"/media_profiles/#{media_profile}?delete_files=true") - refute File.exists?(media_item.media_filepath) + assert [%{args: %{"delete_files" => true}}] = all_enqueued(worker: MediaProfileDeletionWorker) end end diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index 011b5eb..a4a19ba 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.Sources.SourceDeletionWorker alias Pinchflat.Downloading.MediaDownloadWorker alias Pinchflat.Metadata.SourceMetadataStorageWorker alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker @@ -33,8 +34,26 @@ defmodule PinchflatWeb.SourceControllerTest do describe "index" do test "lists all sources", %{conn: conn} do + source = source_fixture() conn = get(conn, ~p"/sources") + assert html_response(conn, 200) =~ "Sources" + assert html_response(conn, 200) =~ source.custom_name + end + + test "omits sources that have marked_for_deletion_at set", %{conn: conn} do + source = source_fixture(marked_for_deletion_at: DateTime.utc_now()) + conn = get(conn, ~p"/sources") + + refute html_response(conn, 200) =~ source.custom_name + end + + test "omits sources who's media profile has marked_for_deletion_at set", %{conn: conn} do + media_profile = media_profile_fixture(marked_for_deletion_at: DateTime.utc_now()) + source = source_fixture(media_profile_id: media_profile.id) + conn = get(conn, ~p"/sources") + + refute html_response(conn, 200) =~ source.custom_name end end @@ -127,51 +146,37 @@ defmodule PinchflatWeb.SourceControllerTest do end end - describe "delete source when just deleting the records" do + describe "delete source in all cases" do setup [:create_source] - test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do - delete(conn, ~p"/sources/#{source}") - - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end - end - test "redirects to the sources page", %{conn: conn, source: source} do conn = delete(conn, ~p"/sources/#{source}") assert redirected_to(conn) == ~p"/sources" end - test "does not delete the files", %{conn: conn, source: source, media_item: media_item} do + test "sets marked_for_deletion_at", %{conn: conn, source: source} do delete(conn, ~p"/sources/#{source}") - assert File.exists?(media_item.media_filepath) + assert Repo.reload!(source).marked_for_deletion_at + end + end + + describe "delete source when just deleting the records" do + setup [:create_source] + + test "enqueues a job without the delete_files arg", %{conn: conn, source: source} do + delete(conn, ~p"/sources/#{source}") + + assert [%{args: %{"delete_files" => false}}] = all_enqueued(worker: SourceDeletionWorker) end end describe "delete source when deleting the records and files" do setup [:create_source] - setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) - - :ok - end - - test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do + test "enqueues a job without the delete_files arg", %{conn: conn, source: source} do delete(conn, ~p"/sources/#{source}?delete_files=true") - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(source) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(media_item) end - end - - test "redirects to the sources page", %{conn: conn, source: source} do - conn = delete(conn, ~p"/sources/#{source}?delete_files=true") - assert redirected_to(conn) == ~p"/sources" - end - - test "deletes the files", %{conn: conn, source: source, media_item: media_item} do - delete(conn, ~p"/sources/#{source}?delete_files=true") - refute File.exists?(media_item.media_filepath) + assert [%{args: %{"delete_files" => true}}] = all_enqueued(worker: SourceDeletionWorker) end end