diff --git a/Pinchflat_improvements.md b/Pinchflat_improvements.md index 61d3b90..84fb7b0 100644 --- a/Pinchflat_improvements.md +++ b/Pinchflat_improvements.md @@ -2,7 +2,7 @@ Issues discovered while debugging Jellyfin artwork problems. -## Issue 1: Source images not downloaded for early sources +## Issue 1: Source images not downloaded for early sources [COMPLETED] **Problem:** Sources created before `download_source_images` was added to the media profile have empty `poster_filepath`, `fanart_filepath`, and `banner_filepath` in the `sources` table. The `source_metadata` table may have stale references to files that were never actually downloaded. @@ -19,7 +19,7 @@ Issues discovered while debugging Jellyfin artwork problems. 2. Downloads missing source images for sources that don't have them 3. Updates both `sources` and `source_metadata` tables -## Issue 2: Source images not downloaded for some new sources +## Issue 2: Source images not downloaded for some new sources [COMPLETED] **Problem:** Some recently added sources have no `source_metadata` entry at all, even though they use a profile with `download_source_images: true`. diff --git a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex index 4fcb19b..52ffcb5 100644 --- a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex +++ b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex @@ -21,6 +21,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do alias Pinchflat.SlowIndexing.FileFollowerServer alias Pinchflat.Downloading.DownloadOptionBuilder alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker + alias Pinchflat.Metadata.SourceMetadataStorageWorker alias Pinchflat.YtDlp.Media, as: YtDlpMedia @@ -245,6 +246,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do # Updates the source after a successful indexing run. This includes: # - Setting `last_indexed_at` to the current time # - Advancing `download_cutoff_date` to 7 days ago if it's older (or nil) + # - Kicking off metadata storage if source images are missing but should be downloaded # # Advancing the cutoff date prevents yt-dlp from scanning through months of old videos # on every index. We use 7 days as a buffer to ensure we don't miss any videos that @@ -260,6 +262,8 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do |> maybe_advance_cutoff_date(source.download_cutoff_date, new_cutoff_date) Sources.update_source(source, update_attrs, run_post_commit_tasks: false) + + maybe_kickoff_metadata_storage_for_missing_images(source) end defp maybe_advance_cutoff_date(attrs, nil, new_cutoff_date) do @@ -273,4 +277,24 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do attrs end end + + # Kicks off metadata storage if source images should be downloaded but are missing. + # This handles the case where: + # 1. A source was created before download_source_images was enabled on the profile + # 2. The source metadata worker failed or was interrupted + # 3. The profile's download_source_images setting was later enabled + defp maybe_kickoff_metadata_storage_for_missing_images(source) do + source = Repo.preload(source, :media_profile) + + if source.media_profile.download_source_images && source_images_missing?(source) do + Logger.info("Source #{source.id} is missing images, kicking off metadata storage") + SourceMetadataStorageWorker.kickoff_with_task(source) + end + + :ok + end + + defp source_images_missing?(source) do + is_nil(source.poster_filepath) || is_nil(source.fanart_filepath) + end end diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index edd37f1..dc90dda 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -336,6 +336,11 @@ defmodule Pinchflat.Sources do {_, %{original_url: _}} -> SourceMetadataStorageWorker.kickoff_with_task(source) + # If the media_profile_id has changed, re-fetch metadata to potentially + # download source images if the new profile has download_source_images enabled + {_, %{media_profile_id: _}} -> + SourceMetadataStorageWorker.kickoff_with_task(source) + _ -> :ok end diff --git a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs index e71c812..2895dc1 100644 --- a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs +++ b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs @@ -13,6 +13,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do alias Pinchflat.Downloading.MediaDownloadWorker alias Pinchflat.SlowIndexing.SlowIndexingHelpers alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker + alias Pinchflat.Metadata.SourceMetadataStorageWorker setup do {:ok, %{source: source_fixture()}} @@ -259,6 +260,32 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do assert source.download_cutoff_date == recent_cutoff end + test "kicks off metadata storage if source images are missing but should be downloaded" do + profile = media_profile_fixture(%{download_source_images: true}) + source = source_fixture(%{media_profile_id: profile.id, poster_filepath: nil, fanart_filepath: nil}) + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + + assert_enqueued(worker: SourceMetadataStorageWorker, args: %{"id" => source.id}) + end + + test "does not kick off metadata storage if source images exist" do + source = source_with_metadata_attachments() + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + + refute_enqueued(worker: SourceMetadataStorageWorker) + end + + test "does not kick off metadata storage if profile does not download source images" do + profile = media_profile_fixture(%{download_source_images: false}) + source = source_fixture(%{media_profile_id: profile.id, poster_filepath: nil}) + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + + refute_enqueued(worker: SourceMetadataStorageWorker) + end + test "enqueues a job for each pending media item" do source = source_fixture() media_item = media_item_fixture(source_id: source.id, media_filepath: nil) diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 44bae14..3963a99 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -565,6 +565,16 @@ defmodule Pinchflat.SourcesTest do refute_enqueued(worker: SourceMetadataStorageWorker) end + + test "updating will kickoff a metadata storage worker if the media_profile_id changes" do + source = source_fixture() + new_profile = media_profile_fixture() + update_attrs = %{media_profile_id: new_profile.id} + + assert {:ok, %Source{} = source} = Sources.update_source(source, update_attrs) + + assert_enqueued(worker: SourceMetadataStorageWorker, args: %{"id" => source.id}) + end end describe "update_source/3 when testing media download tasks" do