Refactored indexing

Previously, indexing worked by collecting the video IDs of only videos
that matched indexing criteria. This new model instead stores ALL videos
for a given source, but will only _download_ videos that meet that criteria.
This lets us backfill without indexing, makes it easier to add in other
backends, lets us download one-off videos for a source that don't quite
meet criteria, you name it.

.iex.exs

@@ -44,7 +44,7 @@ defmodule IexHelpers do
         :channel -> channel_url()
       end
 
-    SourceDetails.get_video_ids(source)
+    SourceDetails.get_media_attributes(source)
   end
 end
 

lib/pinchflat/media/media_item.ex

@@ -13,17 +13,21 @@ defmodule Pinchflat.Media.MediaItem do
   @allowed_fields ~w(
     title
     media_id
+    original_url
+    livestream
     media_filepath
     source_id
     subtitle_filepaths
     thumbnail_filepath
     metadata_filepath
   )a
-  @required_fields ~w(media_id source_id)a
+  @required_fields ~w(title original_url livestream media_id source_id)a
 
   schema "media_items" do
     field :title, :string
     field :media_id, :string
+    field :original_url, :string
+    field :livestream, :boolean, default: false
     field :media_filepath, :string
     field :thumbnail_filepath, :string
     field :metadata_filepath, :string

lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex

@@ -4,18 +4,26 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollection do
   videos (aka: a source [ie: channels, playlists]).
   """
 
-  @doc """
+  alias Pinchflat.Utils.FunctionUtils
-  Returns a list of strings representing the video ids in the collection.
 
-  Returns {:ok, [binary()]} | {:error, any, ...}.
+  @doc """
+  Returns a list of maps representing the videos in the collection.
+
+  Returns {:ok, [map()]} | {:error, any, ...}.
   """
-  def get_video_ids(url, command_opts \\ []) do
+  def get_media_attributes(url, command_opts \\ []) do
     runner = Application.get_env(:pinchflat, :yt_dlp_runner)
     opts = command_opts ++ [:simulate, :skip_download]
 
-    case runner.run(url, opts, "%(id)s") do
+    case runner.run(url, opts, "%(.{id,title,was_live,original_url})j") do
-      {:ok, output} -> {:ok, String.split(output, "\n", trim: true)}
+      {:ok, output} ->
-      res -> res
+        output
+        |> String.split("\n", trim: true)
+        |> Enum.map(&Phoenix.json_library().decode!/1)
+        |> FunctionUtils.wrap_ok()
+
+      res ->
+        res
     end
   end
 

lib/pinchflat/media_client/source_details.ex

@@ -6,11 +6,8 @@ defmodule Pinchflat.MediaClient.SourceDetails do
   it open-ish for future expansion (just in case).
   """
 
-  alias Pinchflat.Repo
   alias Pinchflat.MediaSource.Source
-
   alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection, as: YtDlpSource
-  alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder, as: YtDlpIndexOptionBuilder
 
   @doc """
   Gets a source's ID and name from its URL using the given backend.
@@ -22,24 +19,19 @@ defmodule Pinchflat.MediaClient.SourceDetails do
   end
 
   @doc """
-  Returns a list of video IDs for the given source URL OR source record using the given backend.
+  Returns a list of basic video data mapsfor the given source URL OR
+  source record using the given backend.
 
-  If passing a source record, the call to the backend may have custom options applied based on
+  Returns {:ok, [map()]} | {:error, any, ...}.
-  the `option_builder`.
-
-  Returns {:ok, list(binary())} | {:error, any, ...}.
   """
-  def get_video_ids(sourceable, backend \\ :yt_dlp)
+  def get_media_attributes(sourceable, backend \\ :yt_dlp)
 
-  def get_video_ids(%Source{} = source, backend) do
+  def get_media_attributes(%Source{} = source, backend) do
-    media_profile = Repo.preload(source, :media_profile).media_profile
+    source_module(backend).get_media_attributes(source.collection_id)
-    {:ok, options} = option_builder(backend).build(media_profile)
-
-    source_module(backend).get_video_ids(source.collection_id, options)
   end
 
-  def get_video_ids(source_url, backend) when is_binary(source_url) do
+  def get_media_attributes(source_url, backend) when is_binary(source_url) do
-    source_module(backend).get_video_ids(source_url)
+    source_module(backend).get_media_attributes(source_url)
   end
 
   defp source_module(backend) do
@@ -47,10 +39,4 @@ defmodule Pinchflat.MediaClient.SourceDetails do
       :yt_dlp -> YtDlpSource
     end
   end
-
-  defp option_builder(backend) do
-    case backend do
-      :yt_dlp -> YtDlpIndexOptionBuilder
-    end
-  end
 end

lib/pinchflat/media_source.ex

@@ -7,7 +7,6 @@ defmodule Pinchflat.MediaSource do
   alias Pinchflat.Repo
 
   alias Pinchflat.Tasks
-  alias Pinchflat.Media
   alias Pinchflat.Tasks.SourceTasks
   alias Pinchflat.MediaSource.Source
   alias Pinchflat.MediaClient.SourceDetails
@@ -39,26 +38,6 @@ defmodule Pinchflat.MediaSource do
     |> commit_and_start_indexing()
   end
 
-  @doc """
-  Given a media source, creates (indexes) the media by creating media_items for each
-  media ID in the source.
-
-  Returns [%MediaItem{}, ...] | [%Ecto.Changeset{}, ...]
-  """
-  def index_media_items(%Source{} = source) do
-    {:ok, media_ids} = SourceDetails.get_video_ids(source.original_url)
-
-    media_ids
-    |> Enum.map(fn media_id ->
-      attrs = %{source_id: source.id, media_id: media_id}
-
-      case Media.create_media_item(attrs) do
-        {:ok, media_item} -> media_item
-        {:error, changeset} -> changeset
-      end
-    end)
-  end
-
   @doc """
   Updates a source. May attempt to pull additional source details from the
   original_url (if changed). May attempt to start indexing the source's
@@ -101,6 +80,9 @@ defmodule Pinchflat.MediaSource do
   This means that it'll go for it even if a changeset is otherwise invalid. This
   is pretty easy to change, but for MVP I'm not concerned.
 
+  NOTE: When operating in the ideal path, this effectively adds an API call
+  to the source creation/update process. Should be used only when needed.
+
   IDEA: Maybe I could discern `collection_type` based on the original URL?
   It also seems like it's a channel when the returned yt-dlp channel_id is the
   same as the playlist_id - maybe could use that?

lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex

@@ -1,52 +0,0 @@
-defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder do
-  @moduledoc """
-  Builds the options for yt-dlp to index a media source based on the given media profile.
-  """
-
-  alias Pinchflat.Profiles.MediaProfile
-
-  @doc """
-  Builds the options for yt-dlp to index a media source based on the given media profile.
-  """
-  def build(%MediaProfile{} = media_profile) do
-    built_options = release_type_options(media_profile)
-
-    {:ok, built_options}
-  end
-
-  defp release_type_options(media_profile) do
-    mapped_struct = Map.from_struct(media_profile)
-
-    # Appending multiple match filters treats them as an OR condition,
-    # so we have to be careful around combining `only` and `exclude` options.
-    # eg: only shorts + exclude livestreams = "any video that is a short OR is not a livestream"
-    # which will return all shorts AND normal videos.
-    Enum.reduce(mapped_struct, [], fn attr, acc ->
-      case {attr, media_profile} do
-        {{:shorts_behaviour, :only}, _} ->
-          acc ++ [match_filter: "original_url*=/shorts/"]
-
-        {{:livestream_behaviour, :only}, _} ->
-          acc ++ [match_filter: "was_live"]
-
-        # Since match_filter is an OR (see above), `exclude`s must be ignored entirely if the
-        # other type is set to `only`. There is also special behaviour if they're both excludes,
-        # hence why these check against `:include` alone.
-        {{:shorts_behaviour, :exclude}, %{livestream_behaviour: :include}} ->
-          acc ++ [match_filter: "original_url!*=/shorts/"]
-
-        {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :include}} ->
-          acc ++ [match_filter: "!was_live"]
-
-        # Again, since it's an OR, there's a special syntax if they're both excluded
-        # to make it an AND. Note that I'm not checking for the other permutation of
-        # both excluding since this MUST get hit so adding the other version would double up.
-        {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :exclude}} ->
-          acc ++ [match_filter: "!was_live & original_url!*=/shorts/"]
-
-        _ ->
-          acc
-      end
-    end)
-  end
-end

lib/pinchflat/tasks/source_tasks.ex

@@ -6,6 +6,7 @@ defmodule Pinchflat.Tasks.SourceTasks do
   alias Pinchflat.Media
   alias Pinchflat.Tasks
   alias Pinchflat.MediaSource.Source
+  alias Pinchflat.MediaClient.SourceDetails
   alias Pinchflat.Workers.MediaIndexingWorker
   alias Pinchflat.Workers.VideoDownloadWorker
 
@@ -33,6 +34,32 @@ defmodule Pinchflat.Tasks.SourceTasks do
     end
   end
 
+  @doc """
+  Given a media source, creates (indexes) the media by creating media_items for each
+  media ID in the source.
+
+  Returns [%MediaItem{}, ...] | [%Ecto.Changeset{}, ...]
+  """
+  def index_media_items(%Source{} = source) do
+    {:ok, media_attributes} = SourceDetails.get_media_attributes(source.original_url)
+
+    media_attributes
+    |> Enum.map(fn media_attrs ->
+      attrs = %{
+        source_id: source.id,
+        title: media_attrs["title"],
+        media_id: media_attrs["id"],
+        original_url: media_attrs["original_url"],
+        livestream: media_attrs["was_live"]
+      }
+
+      case Media.create_media_item(attrs) do
+        {:ok, media_item} -> media_item
+        {:error, changeset} -> changeset
+      end
+    end)
+  end
+
   @doc """
   Starts tasks for downloading videos for any of a sources _pending_ media items.
   Jobs are not enqueued if the source is set to not download media. This will return :ok.

lib/pinchflat/utils/function_utils.ex

@@ -0,0 +1,15 @@
+defmodule Pinchflat.Utils.FunctionUtils do
+  @moduledoc """
+  Utility functions for working with functions
+  """
+
+  @doc """
+  Wraps the provided term in an :ok tuple. Useful for fulfilling a contract, but
+  other usage should be assessed to see if it's the right fit.
+
+  Returns {:ok, term}
+  """
+  def wrap_ok(value) do
+    {:ok, value}
+  end
+end

lib/pinchflat/workers/media_indexing_worker.ex

@@ -47,7 +47,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
   end
 
   defp index_media_and_reschedule(source) do
-    MediaSource.index_media_items(source)
+    SourceTasks.index_media_items(source)
     SourceTasks.enqueue_pending_media_downloads(source)
 
     source

priv/repo/migrations/20240208194729_add_indexing_attributes_to_media_items.exs

@@ -0,0 +1,10 @@
+defmodule Pinchflat.Repo.Migrations.AddIndexingAttributesToMediaItems do
+  use Ecto.Migration
+
+  def change do
+    alter table(:media_items) do
+      add :livestream, :boolean, default: false, null: false
+      add :original_url, :string, null: false
+    end
+  end
+end

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

@@ -1,6 +1,7 @@
 defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
   use ExUnit.Case, async: true
   import Mox
+  import Pinchflat.MediaSourceFixtures
 
   alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection
 
@@ -8,22 +9,23 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
 
   setup :verify_on_exit!
 
-  describe "get_video_ids/2" do
+  describe "get_media_attributes/2" do
-    test "returns a list of video ids with no blank elements" do
+    test "returns a list of video attributes with no blank elements" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "id1\nid2\n\nid3\n"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture() <> "\n\n"} end)
 
-      assert {:ok, ["id1", "id2", "id3"]} = VideoCollection.get_video_ids(@channel_url)
+      assert {:ok, [%{"id" => "video1"}, %{"id" => "video2"}, %{"id" => "video3"}]} =
+               VideoCollection.get_media_attributes(@channel_url)
     end
 
     test "it passes the expected default args" do
       expect(YtDlpRunnerMock, :run, fn _url, opts, ot ->
         assert opts == [:simulate, :skip_download]
-        assert ot == "%(id)s"
+        assert ot == "%(.{id,title,was_live,original_url})j"
 
         {:ok, ""}
       end)
 
-      assert {:ok, _} = VideoCollection.get_video_ids(@channel_url)
+      assert {:ok, _} = VideoCollection.get_media_attributes(@channel_url)
     end
 
     test "it passes the expected custom args" do
@@ -33,13 +35,13 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
         {:ok, ""}
       end)
 
-      assert {:ok, _} = VideoCollection.get_video_ids(@channel_url, [:custom_arg])
+      assert {:ok, _} = VideoCollection.get_media_attributes(@channel_url, [:custom_arg])
     end
 
     test "returns the error straight through when the command fails" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "Big issue", 1} end)
 
-      assert {:error, "Big issue", 1} = VideoCollection.get_video_ids(@channel_url)
+      assert {:error, "Big issue", 1} = VideoCollection.get_media_attributes(@channel_url)
     end
   end
 

test/pinchflat/media_client/source_details_test.exs

@@ -22,7 +22,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do
       assert {:ok, _} = SourceDetails.get_source_details(@channel_url)
     end
 
-    test "it returns a struct composed of the returned data" do
+    test "it returns a map composed of the returned data" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
         Phoenix.json_library().encode(%{
           channel: "TheUselessTrials",
@@ -43,42 +43,42 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do
     end
   end
 
-  describe "get_video_ids/2 when passed a string" do
+  describe "get_media_attributes/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]
-        assert ot == "%(id)s"
+        assert ot == "%(.{id,title,was_live,original_url})j"
 
         {:ok, ""}
       end)
 
-      assert {:ok, _} = SourceDetails.get_video_ids(@channel_url)
+      assert {:ok, _} = SourceDetails.get_media_attributes(@channel_url)
     end
 
-    test "it returns a list of strings" do
+    test "it returns a list of maps" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
-        {:ok, "video1\nvideo2\nvideo3"}
+        {:ok, source_attributes_return_fixture()}
       end)
 
-      assert {:ok, ["video1", "video2", "video3"]} = SourceDetails.get_video_ids(@channel_url)
+      assert {:ok, [%{}, %{}, %{}]} = SourceDetails.get_media_attributes(@channel_url)
     end
   end
 
-  describe "get_video_ids/2 when passed a Source record" do
+  describe "get_media_attributes/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"}
+        {:ok, source_attributes_return_fixture()}
       end)
 
-      assert {:ok, _} = SourceDetails.get_video_ids(source)
+      assert {:ok, _} = SourceDetails.get_media_attributes(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]
+        assert opts == [:simulate, :skip_download]
         {:ok, ""}
       end)
 
@@ -89,7 +89,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do
         )
 
       source = source_fixture(media_profile_id: media_profile.id)
-      assert {:ok, _} = SourceDetails.get_video_ids(source)
+      assert {:ok, _} = SourceDetails.get_media_attributes(source)
     end
   end
 end

test/pinchflat/media_client/video_downloader_test.exs

@@ -10,7 +10,7 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
   setup do
     media_item =
       Repo.preload(
-        media_item_fixture(%{title: nil, media_filepath: nil}),
+        media_item_fixture(%{title: "Something", media_filepath: nil}),
         [:metadata, source: :media_profile]
       )
 
@@ -58,7 +58,6 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do
     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/pinchflat/media_source_test.exs

@@ -6,7 +6,6 @@ defmodule Pinchflat.MediaSourceTest do
   import Pinchflat.MediaSourceFixtures
 
   alias Pinchflat.MediaSource
-  alias Pinchflat.Media.MediaItem
   alias Pinchflat.MediaSource.Source
   alias Pinchflat.Workers.MediaIndexingWorker
 
@@ -117,59 +116,6 @@ defmodule Pinchflat.MediaSourceTest do
     end
   end
 
-  describe "index_media_items/1" do
-    setup do
-      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo2\nvideo3"} end)
-
-      {:ok, [source: source_fixture()]}
-    end
-
-    test "it creates a media_item record for each media ID returned", %{source: source} do
-      assert media_items = MediaSource.index_media_items(source)
-
-      assert Enum.count(media_items) == 3
-      assert ["video1", "video2", "video3"] == Enum.map(media_items, & &1.media_id)
-      assert Enum.all?(media_items, fn %MediaItem{} -> true end)
-    end
-
-    test "it attaches all media_items to the given source", %{source: source} do
-      source_id = source.id
-      assert media_items = MediaSource.index_media_items(source)
-
-      assert Enum.count(media_items) == 3
-      assert Enum.all?(media_items, fn %MediaItem{source_id: ^source_id} -> true end)
-    end
-
-    test "it won't duplicate media_items based on media_id and source", %{source: source} do
-      _first_run = MediaSource.index_media_items(source)
-      _duplicate_run = MediaSource.index_media_items(source)
-
-      media_items = Repo.preload(source, :media_items).media_items
-      assert Enum.count(media_items) == 3
-    end
-
-    test "it can duplicate media_ids for different sources", %{source: source} do
-      other_source = source_fixture()
-
-      media_items = MediaSource.index_media_items(source)
-      media_items_other_source = MediaSource.index_media_items(other_source)
-
-      assert Enum.count(media_items) == 3
-      assert Enum.count(media_items_other_source) == 3
-
-      assert Enum.map(media_items, & &1.media_id) ==
-               Enum.map(media_items_other_source, & &1.media_id)
-    end
-
-    test "it returns a list of media_items or changesets", %{source: source} do
-      first_run = MediaSource.index_media_items(source)
-      duplicate_run = MediaSource.index_media_items(source)
-
-      assert Enum.all?(first_run, fn %MediaItem{} -> true end)
-      assert Enum.all?(duplicate_run, fn %Ecto.Changeset{} -> true end)
-    end
-  end
-
   describe "update_source/2" do
     test "updates with valid data updates the source" do
       source = source_fixture()

test/pinchflat/media_test.exs

@@ -63,7 +63,8 @@ defmodule Pinchflat.MediaTest do
         media_id: Faker.String.base64(12),
         title: Faker.Commerce.product_name(),
         media_filepath: "/video/#{Faker.File.file_name(:video)}",
-        source_id: source_fixture().id
+        source_id: source_fixture().id,
+        original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}"
       }
 
       assert {:ok, %MediaItem{} = media_item} = Media.create_media_item(valid_attrs)

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

@@ -1,104 +0,0 @@
-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/tasks/source_tasks_test.exs

@@ -1,6 +1,7 @@
 defmodule Pinchflat.Tasks.SourceTasksTest do
   use Pinchflat.DataCase
 
+  import Mox
   import Pinchflat.TasksFixtures
   import Pinchflat.MediaFixtures
   import Pinchflat.MediaSourceFixtures
@@ -8,9 +9,12 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
   alias Pinchflat.Tasks
   alias Pinchflat.Tasks.Task
   alias Pinchflat.Tasks.SourceTasks
+  alias Pinchflat.Media.MediaItem
   alias Pinchflat.Workers.MediaIndexingWorker
   alias Pinchflat.Workers.VideoDownloadWorker
 
+  setup :verify_on_exit!
+
   describe "kickoff_indexing_task/1" do
     test "it does not schedule a job if the interval is <= 0" do
       source = source_fixture(index_frequency_minutes: -1)
@@ -46,6 +50,61 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
     end
   end
 
+  describe "index_media_items/1" do
+    setup do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
+
+      {:ok, [source: source_fixture()]}
+    end
+
+    test "it creates a media_item record for each media ID returned", %{source: source} do
+      assert media_items = SourceTasks.index_media_items(source)
+
+      assert Enum.count(media_items) == 3
+      assert ["video1", "video2", "video3"] == Enum.map(media_items, & &1.media_id)
+      assert ["Video 1", "Video 2", "Video 3"] == Enum.map(media_items, & &1.title)
+      assert Enum.all?(media_items, fn mi -> mi.original_url end)
+      assert Enum.all?(media_items, fn %MediaItem{} -> true end)
+    end
+
+    test "it attaches all media_items to the given source", %{source: source} do
+      source_id = source.id
+      assert media_items = SourceTasks.index_media_items(source)
+
+      assert Enum.count(media_items) == 3
+      assert Enum.all?(media_items, fn %MediaItem{source_id: ^source_id} -> true end)
+    end
+
+    test "it won't duplicate media_items based on media_id and source", %{source: source} do
+      _first_run = SourceTasks.index_media_items(source)
+      _duplicate_run = SourceTasks.index_media_items(source)
+
+      media_items = Repo.preload(source, :media_items).media_items
+      assert Enum.count(media_items) == 3
+    end
+
+    test "it can duplicate media_ids for different sources", %{source: source} do
+      other_source = source_fixture()
+
+      media_items = SourceTasks.index_media_items(source)
+      media_items_other_source = SourceTasks.index_media_items(other_source)
+
+      assert Enum.count(media_items) == 3
+      assert Enum.count(media_items_other_source) == 3
+
+      assert Enum.map(media_items, & &1.media_id) ==
+               Enum.map(media_items_other_source, & &1.media_id)
+    end
+
+    test "it returns a list of media_items or changesets", %{source: source} do
+      first_run = SourceTasks.index_media_items(source)
+      duplicate_run = SourceTasks.index_media_items(source)
+
+      assert Enum.all?(first_run, fn %MediaItem{} -> true end)
+      assert Enum.all?(duplicate_run, fn %Ecto.Changeset{} -> true end)
+    end
+  end
+
   describe "enqueue_pending_media_downloads/1" do
     test "it enqueues a job for each pending media item" do
       source = source_fixture()

test/pinchflat/utils/function_utils_test.exs

@@ -0,0 +1,11 @@
+defmodule Pinchflat.Utils.FunctionUtilsTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Utils.FunctionUtils
+
+  describe "wrap_ok/1" do
+    test "wraps the provided term in an :ok tuple" do
+      assert FunctionUtils.wrap_ok("hello") == {:ok, "hello"}
+    end
+  end
+end

test/pinchflat/workers/media_indexing_worker_test.exs

@@ -38,32 +38,33 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it kicks off a download job for each pending media item" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
 
       source = source_fixture(index_frequency_minutes: 10)
       perform_job(MediaIndexingWorker, %{id: source.id})
 
-      assert [_] = all_enqueued(worker: VideoDownloadWorker)
+      assert length(all_enqueued(worker: VideoDownloadWorker)) == 3
     end
 
     test "it starts a job for any pending media item even if it's from another run" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
 
       source = source_fixture(index_frequency_minutes: 10)
       media_item_fixture(%{source_id: source.id, media_filepath: nil})
       perform_job(MediaIndexingWorker, %{id: source.id})
 
-      assert [_, _] = all_enqueued(worker: VideoDownloadWorker)
+      assert length(all_enqueued(worker: VideoDownloadWorker)) == 4
     end
 
     test "it does not kick off a job for media items that could not be saved" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo1"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
 
       source = source_fixture(index_frequency_minutes: 10)
+      media_item_fixture(%{source_id: source.id, media_filepath: nil, media_id: "video1"})
       perform_job(MediaIndexingWorker, %{id: source.id})
 
-      # Only one job should be enqueued, since the second video is a duplicate
+      # Only 3 jobs should be enqueued, since the first video is a duplicate
-      assert [_] = all_enqueued(worker: VideoDownloadWorker)
+      assert length(all_enqueued(worker: VideoDownloadWorker))
     end
 
     test "it reschedules the job based on the index frequency" do
@@ -91,7 +92,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it creates the basic media_item records" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo2"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
 
       source = source_fixture(index_frequency_minutes: 10)
 
@@ -102,7 +103,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
         |> Enum.map(fn media_item -> media_item.media_id end)
       end
 
-      assert_changed([from: [], to: ["video1", "video2"]], media_item_fetcher, fn ->
+      assert_changed([from: [], to: ["video1", "video2", "video3"]], media_item_fetcher, fn ->
         perform_job(MediaIndexingWorker, %{id: source.id})
       end)
     end

test/support/fixtures/media_fixtures.ex

@@ -15,6 +15,8 @@ defmodule Pinchflat.MediaFixtures do
       |> Enum.into(%{
         media_id: Faker.String.base64(12),
         title: Faker.Commerce.product_name(),
+        original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}",
+        livestream: false,
         media_filepath: "/video/#{Faker.File.file_name(:video)}",
         source_id: MediaSourceFixtures.source_fixture().id
       })

test/support/fixtures/media_source_fixtures.ex

@@ -29,4 +29,30 @@ defmodule Pinchflat.MediaSourceFixtures do
 
     source
   end
+
+  def source_attributes_return_fixture do
+    source_attributes = [
+      %{
+        id: "video1",
+        title: "Video 1",
+        original_url: "https://example.com/video1",
+        was_live: false
+      },
+      %{
+        id: "video2",
+        title: "Video 2",
+        original_url: "https://example.com/video2",
+        was_live: true
+      },
+      %{
+        id: "video3",
+        title: "Video 3",
+        original_url: "https://example.com/video3",
+        was_live: false
+      }
+    ]
+
+    source_attributes
+    |> Enum.map_join("\n", &Phoenix.json_library().encode!(&1))
+  end
 end