From 6e7e8dc96c74e5916ec7afe218e942e6590a16d4 Mon Sep 17 00:00:00 2001 From: Kieran Date: Fri, 15 Mar 2024 18:29:02 -0700 Subject: [PATCH] Misc refactors 2024-03-15 (#90) * Fixed TV show output template * Discard jobs if record not found; hopefully addressed timing-related source deletion bug --- .../downloading/media_download_worker.ex | 5 +++++ .../fast_indexing/fast_indexing_worker.ex | 5 +++++ .../fast_indexing/media_indexing_worker.ex | 3 +++ lib/pinchflat/media/media.ex | 3 ++- .../metadata/source_metadata_storage_worker.ex | 5 +++++ .../media_collection_indexing_worker.ex | 5 +++++ lib/pinchflat/sources/sources.ex | 3 ++- .../media_profiles/media_profile_html.ex | 2 +- .../downloading/media_download_worker_test.exs | 4 ++++ .../fast_indexing/fast_indexing_worker_test.exs | 16 ++++++++++------ .../fast_indexing/media_indexing_worker_test.exs | 4 ++++ .../source_metadata_storage_worker_test.exs | 4 ++++ .../media_collection_indexing_worker_test.exs | 4 ++++ 13 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index 386c1f3..f87554a 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -6,6 +6,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do unique: [period: :infinity, states: [:available, :scheduled, :retryable, :executing]], tags: ["media_item", "media_fetching"] + require Logger + alias __MODULE__ alias Pinchflat.Tasks alias Pinchflat.Repo @@ -42,6 +44,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do else :ok end + rescue + Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} not found") + Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} stale") end defp download_media_and_schedule_jobs(media_item) do diff --git a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex index 5e16a68..1e0f700 100644 --- a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex +++ b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex @@ -6,6 +6,8 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do unique: [period: :infinity, states: [:available, :scheduled, :retryable]], tags: ["media_source", "fast_indexing"] + require Logger + alias __MODULE__ alias Pinchflat.Tasks alias Pinchflat.Sources @@ -41,6 +43,9 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do else :ok end + rescue + Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") + Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end defp reschedule_indexing(source) do diff --git a/lib/pinchflat/fast_indexing/media_indexing_worker.ex b/lib/pinchflat/fast_indexing/media_indexing_worker.ex index 8792f60..c4da1e1 100644 --- a/lib/pinchflat/fast_indexing/media_indexing_worker.ex +++ b/lib/pinchflat/fast_indexing/media_indexing_worker.ex @@ -62,5 +62,8 @@ defmodule Pinchflat.FastIndexing.MediaIndexingWorker do end :ok + rescue + Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") + Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end end diff --git a/lib/pinchflat/media/media.ex b/lib/pinchflat/media/media.ex index 170968e..e074ce1 100644 --- a/lib/pinchflat/media/media.ex +++ b/lib/pinchflat/media/media.ex @@ -196,13 +196,14 @@ defmodule Pinchflat.Media do def delete_media_item(%MediaItem{} = media_item, opts \\ []) do delete_files = Keyword.get(opts, :delete_files, false) + Tasks.delete_tasks_for(media_item) + if delete_files do {:ok, _} = delete_media_files(media_item) end # Should delete these no matter what delete_internal_metadata_files(media_item) - Tasks.delete_tasks_for(media_item) Repo.delete(media_item) end diff --git a/lib/pinchflat/metadata/source_metadata_storage_worker.ex b/lib/pinchflat/metadata/source_metadata_storage_worker.ex index c0bc885..7d298c2 100644 --- a/lib/pinchflat/metadata/source_metadata_storage_worker.ex +++ b/lib/pinchflat/metadata/source_metadata_storage_worker.ex @@ -9,6 +9,8 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do # in an infinite loop. unique: [period: 600] + require Logger + alias __MODULE__ alias Pinchflat.Repo alias Pinchflat.Tasks @@ -47,5 +49,8 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do }) :ok + rescue + Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") + Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end end diff --git a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex index 2c5cee3..6fdf8e2 100644 --- a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex +++ b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex @@ -6,6 +6,8 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do unique: [period: :infinity, states: [:available, :scheduled, :retryable]], tags: ["media_source", "media_collection_indexing"] + require Logger + alias __MODULE__ alias Pinchflat.Tasks alias Pinchflat.Sources @@ -90,6 +92,9 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do # perform a no-op :ok end + rescue + Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") + Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end defp reschedule_indexing(source) do diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index ea2b2d3..d7e387a 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -103,6 +103,8 @@ defmodule Pinchflat.Sources do def delete_source(%Source{} = source, opts \\ []) do delete_files = Keyword.get(opts, :delete_files, false) + Tasks.delete_tasks_for(source) + source |> Media.list_media_items_for() |> Enum.each(fn media_item -> @@ -110,7 +112,6 @@ defmodule Pinchflat.Sources do end) delete_source_metadata_files(source) - Tasks.delete_tasks_for(source) Repo.delete(source) end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex index 8f84700..7ce35f3 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex @@ -70,7 +70,7 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileHTML do end defp media_center_output_template do - "/shows/{{ source_custom_name }}/{{ season_from_date }}/{{ season_episode_from_date }} - {{ title }}.{{ ext }}" + "/shows/{{ source_custom_name }}/Season {{ season_from_date }}/{{ season_episode_from_date }} - {{ title }}.{{ ext }}" end defp audio_output_template do diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index aab984b..ad04c69 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -95,5 +95,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do assert media_item.media_size_bytes > 0 end + + test "does not blow up if the record doesn't exist" do + assert :ok = perform_job(MediaDownloadWorker, %{id: 0}) + end end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs index 4536fcd..a69a99d 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs @@ -31,13 +31,13 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do expect(HTTPClientMock, :get, fn _url -> {:ok, ""} end) source = source_fixture(fast_index: true) - perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) end test "reschedules itself if fast indexing is enabled" do expect(HTTPClientMock, :get, fn _url -> {:ok, ""} end) source = source_fixture(fast_index: true) - perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) assert_enqueued( worker: FastIndexingWorker, @@ -50,8 +50,8 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do stub(HTTPClientMock, :get, fn _url -> {:ok, ""} end) source = source_fixture(fast_index: true) - perform_job(FastIndexingWorker, %{"id" => source.id}) - perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) assert [_] = all_enqueued(worker: FastIndexingWorker) end @@ -60,14 +60,18 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do expect(HTTPClientMock, :get, 0, fn _url -> {:ok, ""} end) source = source_fixture(fast_index: false) - perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) end test "does not reschedule itself if fast indexing is disabled" do source = source_fixture(fast_index: false) - perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{id: source.id}) refute_enqueued(worker: FastIndexingWorker, args: %{"id" => source.id}) end + + test "does not blow up if the record doesn't exist" do + assert :ok = perform_job(FastIndexingWorker, %{id: 0}) + end end end diff --git a/test/pinchflat/fast_indexing/media_indexing_worker_test.exs b/test/pinchflat/fast_indexing/media_indexing_worker_test.exs index db31cc8..178e66a 100644 --- a/test/pinchflat/fast_indexing/media_indexing_worker_test.exs +++ b/test/pinchflat/fast_indexing/media_indexing_worker_test.exs @@ -53,5 +53,9 @@ defmodule Pinchflat.FastIndexing.MediaIndexingWorkerTest do assert [_] = all_enqueued(worker: MediaDownloadWorker) end + + test "does not blow up if the record doesn't exist" do + assert :ok = perform_job(MediaDownloadWorker, %{id: 0, media_url: @media_url}) + end end end diff --git a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs index 48047b5..41b191e 100644 --- a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs +++ b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs @@ -74,5 +74,9 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do assert [_, _] = all_enqueued(worker: SourceMetadataStorageWorker) end + + test "does not blow up if the record doesn't exist" do + assert :ok = perform_job(SourceMetadataStorageWorker, %{id: 0}) + end end end diff --git a/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs b/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs index 5cc07df..6037c54 100644 --- a/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs +++ b/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs @@ -161,5 +161,9 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do perform_job(MediaCollectionIndexingWorker, %{id: source.id}) end) end + + test "does not blow up if the record doesn't exist" do + assert :ok = perform_job(MediaCollectionIndexingWorker, %{id: 0}) + end end end