From ee9173d21880be96e5d3877cd48e0afd8ccf75b2 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Thu, 30 May 2024 15:13:41 -0700 Subject: [PATCH] [WIP] started on source deletion --- config/config.exs | 2 +- .../downloading/media_retention_worker.ex | 4 +- .../sources/source_deletion_worker.ex | 38 +++++++++++++ .../controllers/sources/source_controller.ex | 15 +++--- .../sources/source_deletion_worker_test.exs | 54 +++++++++++++++++++ .../controllers/source_controller_test.exs | 16 +++--- 6 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 lib/pinchflat/sources/source_deletion_worker.ex create mode 100644 test/pinchflat/sources/source_deletion_worker_test.exs diff --git a/config/config.exs b/config/config.exs index 508b9b6..2407975 100644 --- a/config/config.exs +++ b/config/config.exs @@ -64,7 +64,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 3e5c0d0..354be2b 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"] require Logger 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/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index 2df192f..52ca4a6 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 @@ -109,17 +110,13 @@ defmodule PinchflatWeb.Sources.SourceController do def delete(conn, %{"id" => id} = params) do delete_files = Map.get(params, "delete_files", false) 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 + SourceDeletionWorker.kickoff(source, %{delete_files: delete_files}) conn - |> put_flash(:info, flash_message) + |> put_flash( + :info, + "Started deletion job in background. May take a while to complete depending on the number of media items." + ) |> redirect(to: ~p"/sources") 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..fb145da --- /dev/null +++ b/test/pinchflat/sources/source_deletion_worker_test.exs @@ -0,0 +1,54 @@ +defmodule Pinchflat.Sources.SourceDeletionWorkerTest do + use Pinchflat.DataCase + + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + + alias Pinchflat.Media + alias Pinchflat.Sources + 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 -> Sources.get_source!(source.id) end + assert_raise Ecto.NoResultsError, fn -> Media.get_media_item!(media_item.id) 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 -> Sources.get_source!(source.id) end + assert_raise Ecto.NoResultsError, fn -> Media.get_media_item!(media_item.id) end + refute File.exists?(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 b9d6882..d24791f 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -119,6 +119,12 @@ defmodule PinchflatWeb.SourceControllerTest do end describe "delete source when just deleting the records" do + setup do + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + + :ok + end + setup [:create_source] test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do @@ -137,16 +143,6 @@ defmodule PinchflatWeb.SourceControllerTest do delete(conn, ~p"/sources/#{source}") assert File.exists?(media_item.media_filepath) 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 end) - - :ok - end test "deletes chosen source and media_items", %{conn: conn, source: source, media_item: media_item} do delete(conn, ~p"/sources/#{source}?delete_files=true")