Redo indexing mechanism (#16)

* Bumped up the line length because I fear no man

* 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.

* Updated media finders to respect format filters; Added credo file

lib/pinchflat/media.ex

@@ -19,15 +19,31 @@ defmodule Pinchflat.Media do
 
   @doc """
   Returns a list of pending media_items for a given source, where
-  pending means the `media_filepath` is `nil`.
+  pending means the `media_filepath` is `nil` AND the media_item
+  matches the format selection rules of the parent media_profile.
+
+  See `build_format_clauses` but tl;dr is it _may_ filter based
+  on shorts or livestreams depending on the media_profile settings.
 
   Returns [%MediaItem{}, ...].
   """
   def list_pending_media_items_for(%Source{} = source) do
-    from(
-      m in MediaItem,
-      where: m.source_id == ^source.id and is_nil(m.media_filepath)
-    )
+    media_profile = Repo.preload(source, :media_profile).media_profile
+
+    MediaItem
+    |> where([mi], mi.source_id == ^source.id and is_nil(mi.media_filepath))
+    |> where(^build_format_clauses(media_profile))
+    |> Repo.all()
+  end
+
+  @doc """
+  Returns a list of downloaded media_items for a given source.
+
+  Returns [%MediaItem{}, ...].
+  """
+  def list_downloaded_media_items_for(%Source{} = source) do
+    MediaItem
+    |> where([mi], mi.source_id == ^source.id and not is_nil(mi.media_filepath))
     |> Repo.all()
   end
 
@@ -72,4 +88,39 @@ defmodule Pinchflat.Media do
   def change_media_item(%MediaItem{} = media_item, attrs \\ %{}) do
     MediaItem.changeset(media_item, attrs)
   end
+
+  defp build_format_clauses(media_profile) do
+    mapped_struct = Map.from_struct(media_profile)
+
+    Enum.reduce(mapped_struct, dynamic(true), fn attr, dynamic ->
+      case {attr, media_profile} do
+        {{:shorts_behaviour, :only}, %{livestream_behaviour: :only}} ->
+          dynamic([mi], ^dynamic and (mi.livestream == true or fragment("? ILIKE ?", mi.original_url, "%/shorts/%")))
+
+        # Technically redundant, but makes the other clauses easier to parse
+        # (redundant because this condition is the same as the condition above, just flipped)
+        {{:livestream_behaviour, :only}, %{shorts_behaviour: :only}} ->
+          dynamic
+
+        {{:shorts_behaviour, :only}, _} ->
+          # return records with /shorts/ in the original_url
+          dynamic([mi], ^dynamic and fragment("? ILIKE ?", mi.original_url, "%/shorts/%"))
+
+        {{:livestream_behaviour, :only}, _} ->
+          # return records with livestream: true
+          dynamic([mi], ^dynamic and mi.livestream == true)
+
+        {{:shorts_behaviour, :exclude}, %{livestream_behaviour: lb}} when lb != :only ->
+          # return records without /shorts/ in the original_url
+          dynamic([mi], ^dynamic and fragment("? NOT ILIKE ?", mi.original_url, "%/shorts/%"))
+
+        {{:livestream_behaviour, :exclude}, %{shorts_behaviour: sb}} when sb != :only ->
+          # return records with livestream: false
+          dynamic([mi], ^dynamic and mi.livestream == false)
+
+        _ ->
+          dynamic
+      end
+    end)
+  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 """
-  Returns a list of strings representing the video ids in the collection.
+  alias Pinchflat.Utils.FunctionUtils
 
-  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
-      {:ok, output} -> {:ok, String.split(output, "\n", trim: true)}
-      res -> res
+    case runner.run(url, opts, "%(.{id,title,was_live,original_url})j") do
+      {:ok, output} ->
+        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
-  the `option_builder`.
-
-  Returns {:ok, list(binary())} | {:error, any, ...}.
+  Returns {:ok, [map()]} | {: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
-    media_profile = Repo.preload(source, :media_profile).media_profile
-    {:ok, options} = option_builder(backend).build(media_profile)
-
-    source_module(backend).get_video_ids(source.collection_id, options)
+  def get_media_attributes(%Source{} = source, backend) do
+    source_module(backend).get_media_attributes(source.collection_id)
   end
 
-  def get_video_ids(source_url, backend) when is_binary(source_url) do
-    source_module(backend).get_video_ids(source_url)
+  def get_media_attributes(source_url, backend) when is_binary(source_url) do
+    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/media_profile.ex

@@ -45,7 +45,9 @@ defmodule Pinchflat.Profiles.MediaProfile do
     # through the entire collection to determine if a video is a short or
     # a livestream.
     # NOTE: these can BOTH be set to :only which will download shorts and
-    # livestreams _only_ and ignore regular videos.
+    # livestreams _only_ and ignore regular videos. The redundant case
+    # is when one is set to :only and the other is set to :exclude.
+    # See `build_format_clauses` in the Media context for more.
     field :shorts_behaviour, Ecto.Enum, values: [:include, :exclude, :only], default: :include
     field :livestream_behaviour, Ecto.Enum, values: [:include, :exclude, :only], default: :include
 

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