Media downloading indexing options (round 2) (#14)

* Adds options + option builder + metadata parsing for media thumbnails

* Added release-type options to media profile; built option parser for indexing operations

* Added new media_profile options to creation form; made show helper for rendering database items

* Added options for downloading/embedding metadata

* Adds option on sources to not download media (index only)

* reformatted docs

test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs

@@ -47,7 +47,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do
     test "extracts the subtitle filepaths", %{metadata: metadata} do
       result = Parser.parse_for_media_item(metadata)
 
-      assert [["de", german_filepath], ["en", english_filepath]] = result.subtitle_filepaths
+      assert [["de", german_filepath], ["en", english_filepath] | _rest] = result.subtitle_filepaths
 
       assert String.ends_with?(english_filepath, ".en.srt")
       assert String.ends_with?(german_filepath, ".de.srt")
@@ -83,4 +83,44 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do
       assert result.subtitle_filepaths == []
     end
   end
+
+  describe "parse_for_media_item/1 when testing thumbnail metadata" do
+    test "extracts the thumbnail filepath", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert String.ends_with?(result.thumbnail_filepath, ".webp")
+    end
+
+    test "doesn't freak out if the video has no thumbnails", %{metadata: metadata} do
+      metadata = Map.put(metadata, "thumbnails", %{})
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.thumbnail_filepath == nil
+    end
+
+    test "doesn't freak out if the thumbnails key is missing", %{metadata: metadata} do
+      metadata = Map.delete(metadata, "thumbnails")
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.thumbnail_filepath == nil
+    end
+  end
+
+  describe "parse_for_media_item/1 when testing infojson metadata" do
+    test "extracts the metadata filepath", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert String.ends_with?(result.metadata_filepath, ".info.json")
+    end
+
+    test "doesn't freak out if the video has no infojson", %{metadata: metadata} do
+      metadata = Map.put(metadata, "infojson_filename", nil)
+
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.metadata_filepath == nil
+    end
+  end
 end

test/pinchflat/media_client/source_details_test.exs

@@ -1,6 +1,8 @@
 defmodule Pinchflat.MediaClient.SourceDetailsTest do
-  use ExUnit.Case, async: true
+  use Pinchflat.DataCase
   import Mox
+  import Pinchflat.ProfilesFixtures
+  import Pinchflat.MediaSourceFixtures
 
   alias Pinchflat.MediaClient.SourceDetails
 
@@ -41,7 +43,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do
     end
   end
 
-  describe "get_video_ids/2" do
+  describe "get_video_ids/2 when passed a string" do
     test "it passes the expected arguments to the backend" do
       expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot ->
         assert opts == [:simulate, :skip_download]
@@ -61,4 +63,33 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do
       assert {:ok, ["video1", "video2", "video3"]} = SourceDetails.get_video_ids(@channel_url)
     end
   end
+
+  describe "get_video_ids/2 when passed a Source record" do
+    test "it calls the backend with the source's collection ID" do
+      source = source_fixture()
+
+      expect(YtDlpRunnerMock, :run, fn url, _opts, _ot ->
+        assert source.collection_id == url
+        {:ok, "video1\nvideo2\nvideo3"}
+      end)
+
+      assert {:ok, _} = SourceDetails.get_video_ids(source)
+    end
+
+    test "it builds options based on the source's media profile" do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot ->
+        assert opts == [{:match_filter, "!was_live"}, :simulate, :skip_download]
+        {:ok, ""}
+      end)
+
+      media_profile =
+        media_profile_fixture(
+          shorts_behaviour: :include,
+          livestream_behaviour: :exclude
+        )
+
+      source = source_fixture(media_profile_id: media_profile.id)
+      assert {:ok, _} = SourceDetails.get_video_ids(source)
+    end
+  end
 end

test/pinchflat/media_client/video_downloader_test.exs

@@ -28,18 +28,6 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
       assert {:ok, _} = VideoDownloader.download_for_media_item(media_item)
     end
 
-    test "it writes attributes to the media item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
-        {:ok, render_metadata(:media_metadata)}
-      end)
-
-      assert %{media_filepath: nil, title: nil, subtitle_filepaths: []} = media_item
-      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
-      assert updated_media_item.media_filepath
-      assert updated_media_item.title
-      assert length(updated_media_item.subtitle_filepaths) > 0
-    end
-
     test "it saves the metadata to the database", %{media_item: media_item} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
         {:ok, render_metadata(:media_metadata)}
@@ -59,4 +47,44 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
       assert {:error, :some_error} = VideoDownloader.download_for_media_item(media_item)
     end
   end
+
+  describe "download_for_media_item/3 when testing media_item attributes" do
+    setup do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      :ok
+    end
+
+    test "it extracts the title", %{media_item: media_item} do
+      assert media_item.title == nil
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert updated_media_item.title == "Trying to Wheelie Without the Rear Brake"
+    end
+
+    test "it extracts the media_filepath", %{media_item: media_item} do
+      assert media_item.media_filepath == nil
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert String.ends_with?(updated_media_item.media_filepath, ".mkv")
+    end
+
+    test "it extracts the subtitle_filepaths", %{media_item: media_item} do
+      assert media_item.subtitle_filepaths == []
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert [["de", _], ["en", _] | _rest] = updated_media_item.subtitle_filepaths
+    end
+
+    test "it extracts the thumbnail_filepath", %{media_item: media_item} do
+      assert media_item.thumbnail_filepath == nil
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert String.ends_with?(updated_media_item.thumbnail_filepath, ".webp")
+    end
+
+    test "it extracts the metadata_filepath", %{media_item: media_item} do
+      assert media_item.metadata_filepath == nil
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert String.ends_with?(updated_media_item.metadata_filepath, ".info.json")
+    end
+  end
 end

test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs

@@ -0,0 +1,161 @@
+defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder
+
+  @media_profile %MediaProfile{
+    output_path_template: "{{ title }}.%(ext)s"
+  }
+
+  describe "build/1" do
+    test "it generates an expanded output path based on the given template" do
+      assert {:ok, res} = DownloadOptionBuilder.build(@media_profile)
+
+      assert {:output, "/tmp/videos/%(title)S.%(ext)s"} in res
+    end
+  end
+
+  describe "build/1 when testing subtitle options" do
+    test "includes :write_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :write_subs in res
+    end
+
+    test "forces SRT format when download_subs is true" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert {:convert_subs, "srt"} in res
+    end
+
+    test "includes :write_auto_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true, download_auto_subs: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :write_auto_subs in res
+    end
+
+    test "doesn't include :write_auto_subs option when download_subs is false" do
+      media_profile = %MediaProfile{@media_profile | download_subs: false, download_auto_subs: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      refute :write_auto_subs in res
+    end
+
+    test "includes :embed_subs option when specified" do
+      media_profile = %MediaProfile{@media_profile | embed_subs: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :embed_subs in res
+    end
+
+    test "includes sub_langs option when download_subs is true" do
+      media_profile = %MediaProfile{@media_profile | download_subs: true, sub_langs: "en"}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert {:sub_langs, "en"} in res
+    end
+
+    test "includes sub_langs option when embed_subs is true" do
+      media_profile = %MediaProfile{@media_profile | embed_subs: true, sub_langs: "en"}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert {:sub_langs, "en"} in res
+    end
+
+    test "doesn't include sub_langs option when neither downloading nor embedding" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | embed_subs: false,
+          download_subs: false,
+          sub_langs: "en"
+      }
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      refute {:sub_langs, "en"} in res
+    end
+
+    test "other struct attributes are ignored" do
+      media_profile = %MediaProfile{@media_profile | id: -1}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      refute {:id, -1} in res
+    end
+  end
+
+  describe "build/1 when testing thumbnail options" do
+    test "includes :write_thumbnail option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_thumbnail: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :write_thumbnail in res
+    end
+
+    test "includes :embed_thumbnail option when specified" do
+      media_profile = %MediaProfile{@media_profile | embed_thumbnail: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :embed_thumbnail in res
+    end
+
+    test "doesn't include these options when not specified" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | embed_thumbnail: false,
+          download_thumbnail: false
+      }
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      refute :write_thumbnail in res
+      refute :embed_thumbnail in res
+    end
+  end
+
+  describe "build/1 when testing metadata options" do
+    test "includes :write_info_json option when specified" do
+      media_profile = %MediaProfile{@media_profile | download_metadata: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :write_info_json in res
+      assert :clean_info_json in res
+    end
+
+    test "includes :embed_metadata option when specified" do
+      media_profile = %MediaProfile{@media_profile | embed_metadata: true}
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      assert :embed_metadata in res
+    end
+
+    test "doesn't include these options when not specified" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | embed_metadata: false,
+          download_metadata: false
+      }
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+
+      refute :write_info_json in res
+      refute :clean_info_json in res
+      refute :embed_metadata in res
+    end
+  end
+end

test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs

@@ -0,0 +1,104 @@
+defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilderTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder
+
+  @media_profile %MediaProfile{
+    output_path_template: "{{ title }}.%(ext)s",
+    shorts_behaviour: :include,
+    livestream_behaviour: :include
+  }
+
+  describe "build/1 when testing release type options" do
+    test "adds correct filter when shorts_behaviour is :only" do
+      media_profile = %MediaProfile{@media_profile | shorts_behaviour: :only}
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "original_url*=/shorts/"} in res
+      refute {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "!was_live"} in res
+      refute {:match_filter, "was_live"} in res
+    end
+
+    test "adds correct filter when livestream_behaviour is :only" do
+      media_profile = %MediaProfile{@media_profile | livestream_behaviour: :only}
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "was_live"} in res
+      refute {:match_filter, "!was_live"} in res
+      refute {:match_filter, "!original_url*=/shorts/"} in res
+      refute {:match_filter, "original_url*=/shorts/"} in res
+    end
+
+    test "adds correct filter when both livestreams and shorts are :only" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | shorts_behaviour: :only,
+          livestream_behaviour: :only
+      }
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "original_url*=/shorts/"} in res
+      assert {:match_filter, "was_live"} in res
+      refute {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "!was_live"} in res
+    end
+
+    test "adds correct filter when shorts_behaviour is :exclude" do
+      media_profile = %MediaProfile{@media_profile | shorts_behaviour: :exclude}
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "original_url*=/shorts/"} in res
+      refute {:match_filter, "was_live"} in res
+      refute {:match_filter, "!was_live"} in res
+    end
+
+    test "adds correct filter when livestream_behaviour is :exclude" do
+      media_profile = %MediaProfile{@media_profile | livestream_behaviour: :exclude}
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "!was_live"} in res
+      refute {:match_filter, "was_live"} in res
+      refute {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "original_url*=/shorts/"} in res
+    end
+
+    test "adds correct filter when shorts and livestreams are both exclude" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | shorts_behaviour: :exclude,
+          livestream_behaviour: :exclude
+      }
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "!was_live & original_url!*=/shorts/"} in res
+      refute {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "!was_live"} in res
+      refute {:match_filter, "original_url*=/shorts/"} in res
+      refute {:match_filter, "was_live"} in res
+    end
+
+    test "does not add exclusion filter if one is excluded and the other is only" do
+      media_profile = %MediaProfile{
+        @media_profile
+        | shorts_behaviour: :exclude,
+          livestream_behaviour: :only
+      }
+
+      assert {:ok, res} = IndexOptionBuilder.build(media_profile)
+
+      assert {:match_filter, "was_live"} in res
+      refute {:match_filter, "original_url!*=/shorts/"} in res
+      refute {:match_filter, "original_url*=/shorts/"} in res
+      refute {:match_filter, "!was_live"} in res
+    end
+  end
+end

test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs

@@ -1,97 +0,0 @@
-defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do
-  use ExUnit.Case, async: true
-
-  alias Pinchflat.Profiles.MediaProfile
-  alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder
-
-  @media_profile %MediaProfile{
-    output_path_template: "{{ title }}.%(ext)s"
-  }
-
-  describe "build/1" do
-    test "it generates an expanded output path based on the given template" do
-      assert {:ok, res} = OptionBuilder.build(@media_profile)
-
-      assert {:output, "/tmp/videos/%(title)S.%(ext)s"} in res
-    end
-  end
-
-  describe "build/1 when testing subtitle options" do
-    test "includes :write_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert :write_subs in res
-    end
-
-    test "forces SRT format when download_subs is true" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert {:convert_subs, "srt"} in res
-    end
-
-    test "includes :write_auto_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true, download_auto_subs: true}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert :write_auto_subs in res
-    end
-
-    test "doesn't include :write_auto_subs option when download_subs is false" do
-      media_profile = %MediaProfile{@media_profile | download_subs: false, download_auto_subs: true}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      refute :write_auto_subs in res
-    end
-
-    test "includes :embed_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | embed_subs: true}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert :embed_subs in res
-    end
-
-    test "includes sub_langs option when download_subs is true" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true, sub_langs: "en"}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert {:sub_langs, "en"} in res
-    end
-
-    test "includes sub_langs option when embed_subs is true" do
-      media_profile = %MediaProfile{@media_profile | embed_subs: true, sub_langs: "en"}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      assert {:sub_langs, "en"} in res
-    end
-
-    test "doesn't include sub_langs option when neither downloading nor embedding" do
-      media_profile = %MediaProfile{
-        @media_profile
-        | embed_subs: false,
-          download_subs: false,
-          sub_langs: "en"
-      }
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      refute {:sub_langs, "en"} in res
-    end
-
-    test "other struct attributes are ignored" do
-      media_profile = %MediaProfile{@media_profile | id: -1}
-
-      assert {:ok, res} = OptionBuilder.build(media_profile)
-
-      refute {:id, -1} in res
-    end
-  end
-end

test/pinchflat/tasks/source_tasks_test.exs

@@ -75,5 +75,21 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
 
       assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id)
     end
+
+    test "it does not create a job if the source is set to not download" do
+      source = source_fixture(download_media: false)
+
+      assert :ok = SourceTasks.enqueue_pending_media_downloads(source)
+
+      refute_enqueued(worker: VideoDownloadWorker)
+    end
+
+    test "it does not attach tasks if the source is set to not download" do
+      source = source_fixture(download_media: false)
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+
+      assert :ok = SourceTasks.enqueue_pending_media_downloads(source)
+      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+    end
   end
 end

test/pinchflat/workers/video_download_worker_test.exs

@@ -4,6 +4,7 @@ defmodule Pinchflat.Workers.VideoDownloadWorkerTest do
   import Mox
   import Pinchflat.MediaFixtures
 
+  alias Pinchflat.MediaSource
   alias Pinchflat.Workers.VideoDownloadWorker
 
   setup :verify_on_exit!
@@ -55,5 +56,13 @@ defmodule Pinchflat.Workers.VideoDownloadWorkerTest do
         assert job.state == "retryable"
       end)
     end
+
+    test "it does not download if the source is set to not download", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> :ok end)
+
+      MediaSource.update_source(media_item.source, %{download_media: false})
+
+      perform_job(VideoDownloadWorker, %{id: media_item.id})
+    end
   end
 end