Refactored all query methods to use dynamic snippets

lib/pinchflat/downloading/downloading_helpers.ex

@@ -85,10 +85,16 @@ defmodule Pinchflat.Downloading.DownloadingHelpers do
   """
   def kickoff_redownload_for_existing_media(%Source{} = source) do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> MediaQuery.require_assoc(:media_profile)
-    |> MediaQuery.with_media_downloaded_at()
+    |> where(
-    |> MediaQuery.where_download_not_prevented()
+      ^dynamic(
-    |> MediaQuery.where_not_culled()
+        [m, s, mp],
+        ^MediaQuery.for_source(source) and
+          ^MediaQuery.downloaded() and
+          not (^MediaQuery.download_prevented()) and
+          not (^MediaQuery.culled())
+      )
+    )
     |> Repo.all()
     |> Enum.map(&MediaDownloadWorker.kickoff_with_task/1)
   end

lib/pinchflat/fast_indexing/fast_indexing_helpers.ex

@@ -49,8 +49,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpers do
 
   defp list_media_items_by_media_id_for(source, media_ids) do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> where(^dynamic([mi], ^MediaQuery.for_source(source) and mi.media_id in ^media_ids))
-    |> MediaQuery.with_media_ids(media_ids)
     |> Repo.all()
   end
 

lib/pinchflat/lifecycle/notifications/source_notifications.ex

@@ -63,15 +63,14 @@ defmodule Pinchflat.Lifecycle.Notifications.SourceNotifications do
 
   defp pending_media_item_count(source) do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> MediaQuery.require_assoc(:media_profile)
-    |> MediaQuery.where_pending_download()
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.pending()))
     |> Repo.aggregate(:count)
   end
 
   defp downloaded_media_item_count(source) do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.downloaded()))
-    |> MediaQuery.with_media_filepath()
     |> Repo.aggregate(:count)
   end
 

lib/pinchflat/media/media.ex

@@ -32,9 +32,8 @@ defmodule Pinchflat.Media do
   """
   def list_cullable_media_items do
     MediaQuery.new()
-    |> MediaQuery.with_media_filepath()
+    |> MediaQuery.require_assoc(:source)
-    |> MediaQuery.where_past_retention_period()
+    |> where(^MediaQuery.cullable())
-    |> MediaQuery.where_culling_not_prevented()
     |> Repo.all()
   end
 
@@ -54,18 +53,14 @@ defmodule Pinchflat.Media do
   """
   def list_redownloadable_media_items do
     MediaQuery.new()
-    |> MediaQuery.with_media_downloaded_at()
+    |> MediaQuery.require_assoc(:media_profile)
-    |> MediaQuery.where_download_not_prevented()
+    |> where(^MediaQuery.redownloadable())
-    |> MediaQuery.where_not_culled()
-    |> MediaQuery.where_media_not_redownloaded()
-    |> MediaQuery.where_past_redownload_delay()
     |> Repo.all()
   end
 
   @doc """
   Returns a list of pending media_items for a given source, where
-  pending means the `media_filepath` is `nil` AND the media_item
+  pending means the media_item satisfies `MediaQuery.pending`. You
-  matches satisfies `MediaQuery.where_pending_download`. You
   should really check out that function if you need to know more
   because it has a lot going on.
 
@@ -73,16 +68,14 @@ defmodule Pinchflat.Media do
   """
   def list_pending_media_items_for(%Source{} = source) do
     MediaQuery.new()
-    |> join(:inner, [m], s in assoc(m, :source))
+    |> MediaQuery.require_assoc(:media_profile)
-    |> join(:inner, [m, s], mp in assoc(s, :media_profile))
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.pending()))
-    |> where([m, s, mp], ^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.pending?()))
     |> Repo.all()
   end
 
   @doc """
   For a given media_item, tells you if it is pending download. This is defined as
-  the media_item having a `media_filepath` of `nil` and matching the format selection
+  the media_item satisfying `MediaQuery.pending` which you should really check out.
-  rules of the parent media_profile.
 
   Intentionally does not take the `download_media` setting of the source into account.
 
@@ -92,8 +85,8 @@ defmodule Pinchflat.Media do
     media_item = Repo.preload(media_item, source: :media_profile)
 
     MediaQuery.new()
-    |> MediaQuery.with_id(media_item.id)
+    |> MediaQuery.require_assoc(:media_profile)
-    |> MediaQuery.where_pending_download()
+    |> where(^dynamic([m, s, mp], m.id == ^media_item.id and ^MediaQuery.pending()))
     |> Repo.exists?()
   end
 

lib/pinchflat/media/media_item.ex

@@ -142,8 +142,7 @@ defmodule Pinchflat.Media.MediaItem do
 
     current_max =
       MediaQuery.new()
-      |> MediaQuery.for_source(source_id)
+      |> where(^dynamic([mi], mi.upload_date == ^changes.upload_date and ^MediaQuery.for_source(source)))
-      |> MediaQuery.where_uploaded_on_date(changes.upload_date)
       |> Repo.aggregate(aggregator, :upload_date_index)
 
     case current_max do

lib/pinchflat/media/media_query.ex

@@ -23,42 +23,20 @@ defmodule Pinchflat.Media.MediaQuery do
     end
   end
 
-  # Prefixes:
-  # - for_* - belonging to a certain record
-  # - join_* - for joining on a certain record
-  # - with_*, where_* - for filtering based on full, concrete attributes
-  # - matching_* - for filtering based on partial attributes (e.g. LIKE, regex, full-text search)
-  #
-  # Suffixes:
-  # - _for - the arg passed is an association record
-
-  # NOTE: that dyanmic query approach kinda rocked - should refactor in future
-
   def new do
     MediaItem
   end
 
   # Queries below this line are dynamic query methods (which I want to move to)
 
-  def for_source(source_id) when is_integer(source_id) do
+  def for_source(source_id) when is_integer(source_id), do: dynamic([mi], mi.source_id == ^source_id)
-    dynamic([mi], mi.source_id == ^source_id)
+  def for_source(source), do: dynamic([mi], mi.source_id == ^source.id)
-  end
 
-  def for_source(source) do
+  def downloaded, do: dynamic([mi], not is_nil(mi.media_filepath))
-    dynamic([mi], mi.source_id == ^source.id)
+  def download_prevented, do: dynamic([mi], mi.prevent_download == true)
-  end
+  def culling_prevented, do: dynamic([mi], mi.prevent_culling == true)
-
+  def culled, do: dynamic([mi], not is_nil(mi.culled_at))
-  def downloaded? do
+  def redownloaded, do: dynamic([mi], not is_nil(mi.media_redownloaded_at))
-    dynamic([mi], not is_nil(mi.media_downloaded_at))
-  end
-
-  def download_not_prevented do
-    dynamic([mi], mi.prevent_download == false)
-  end
-
-  def no_media_filepath do
-    dynamic([mi], is_nil(mi.media_filepath))
-  end
 
   def upload_date_after_source_cutoff do
     dynamic([mi, source], is_nil(source.download_cutoff_date) or mi.upload_date >= source.download_cutoff_date)
@@ -95,50 +73,19 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
-  # TODO: figure out how to do something like `require_assoc` here
+  def past_retention_period do
-  def pending? do
     dynamic(
-      [mi],
-      ^download_not_prevented() and
-        ^no_media_filepath() and
-        ^upload_date_after_source_cutoff() and
-        ^format_matching_profile_preference() and
-        ^matches_source_title_regex()
-    )
-  end
-
-  # Queries below this line are the "legacy" query methods (which I want to move from)
-
-  def for_source(query, source_id) when is_integer(source_id) do
-    where(query, [mi], mi.source_id == ^source_id)
-  end
-
-  def for_source(query, source) do
-    where(query, [mi], mi.source_id == ^source.id)
-  end
-
-  def join_sources(query) do
-    from(mi in query, join: s in assoc(mi, :source), as: :sources)
-  end
-
-  def where_past_retention_period(query) do
-    query
-    |> require_assoc(:source)
-    |> where(
       [mi, source],
       fragment("""
-      IFNULL(retention_period_days, 0) > 0 AND
+        IFNULL(retention_period_days, 0) > 0 AND
-      DATETIME('now', '-' || retention_period_days || ' day') > media_downloaded_at
+        DATETIME('now', '-' || retention_period_days || ' day') > media_downloaded_at
       """)
     )
   end
 
-  def where_past_redownload_delay(query) do
+  def past_redownload_delay do
-    query
+    dynamic(
-    |> require_assoc(:source)
+      [mi, source, media_profile],
-    |> require_assoc(:media_profile)
-    |> where(
-      [_mi, _source, _media_profile],
       # Returns media items where the upload_date is at least redownload_delay_days ago AND
       # downloaded_at minus the redownload_delay_days is before the upload date
       fragment("""
@@ -149,61 +96,57 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
-  def where_culling_not_prevented(query) do
+  def cullable do
-    where(query, [mi], mi.prevent_culling == false)
+    dynamic(
-  end
-
-  def where_not_culled(query) do
-    where(query, [mi], is_nil(mi.culled_at))
-  end
-
-  def where_media_not_redownloaded(query) do
-    where(query, [mi], is_nil(mi.media_redownloaded_at))
-  end
-
-  def with_id(query, id) do
-    where(query, [mi], mi.id == ^id)
-  end
-
-  def with_media_ids(query, media_ids) do
-    where(query, [mi], mi.media_id in ^media_ids)
-  end
-
-  def with_media_downloaded_at(query) do
-    where(query, [mi], not is_nil(mi.media_downloaded_at))
-  end
-
-  def with_media_filepath(query) do
-    where(query, [mi], not is_nil(mi.media_filepath))
-  end
-
-  def with_no_media_filepath(query) do
-    where(query, [mi], is_nil(mi.media_filepath))
-  end
-
-  def with_upload_date_after_source_cutoff(query) do
-    query
-    |> require_assoc(:source)
-    |> where([mi, source], is_nil(source.download_cutoff_date) or mi.upload_date >= source.download_cutoff_date)
-  end
-
-  def where_uploaded_on_date(query, date) do
-    where(query, [mi], mi.upload_date == ^date)
-  end
-
-  def where_download_not_prevented(query) do
-    where(query, [mi], mi.prevent_download == false)
-  end
-
-  def matching_source_title_regex(query) do
-    query
-    |> require_assoc(:source)
-    |> where(
       [mi, source],
-      is_nil(source.title_filter_regex) or fragment("regexp_like(?, ?)", mi.title, source.title_filter_regex)
+      ^downloaded() and
+        ^past_retention_period() and
+        not (^culling_prevented())
     )
   end
 
+  def pending do
+    dynamic(
+      [mi],
+      not (^downloaded()) and
+        not (^download_prevented()) and
+        ^upload_date_after_source_cutoff() and
+        ^format_matching_profile_preference() and
+        ^matches_source_title_regex()
+    )
+  end
+
+  def redownloadable do
+    dynamic(
+      [mi, source],
+      ^downloaded() and
+        not (^download_prevented()) and
+        not (^culled()) and
+        not (^redownloaded()) and
+        ^past_redownload_delay()
+    )
+  end
+
+  def require_assoc(query, identifier) do
+    if has_named_binding?(query, identifier) do
+      query
+    else
+      do_require_assoc(query, identifier)
+    end
+  end
+
+  defp do_require_assoc(query, :source) do
+    from(mi in query, join: s in assoc(mi, :source), as: :source)
+  end
+
+  defp do_require_assoc(query, :media_profile) do
+    query
+    |> require_assoc(:source)
+    |> join(:inner, [mi, source], mp in assoc(source, :media_profile), as: :media_profile)
+  end
+
+  # This needs to be a non-dynamic query because it alone should control things like
+  # ordering and `snippets` for full-text search
   def matching_search_term(query, nil), do: query
 
   def matching_search_term(query, term) do
@@ -221,62 +164,4 @@ defmodule Pinchflat.Media.MediaQuery do
       order_by: [desc: fragment("rank")]
     )
   end
-
-  def with_format_matching_profile_preference(query) do
-    query
-    |> require_assoc(:media_profile)
-    |> where(
-      fragment("""
-        CASE
-          WHEN shorts_behaviour = 'only' AND livestream_behaviour = 'only' THEN
-            livestream = true OR short_form_content = true
-          WHEN shorts_behaviour = 'only' THEN
-            short_form_content = true
-          WHEN livestream_behaviour = 'only' THEN
-            livestream = true
-          WHEN shorts_behaviour = 'exclude' AND livestream_behaviour = 'exclude' THEN
-            short_form_content = false AND livestream = false
-          WHEN shorts_behaviour = 'exclude' THEN
-            short_form_content = false
-          WHEN livestream_behaviour = 'exclude' THEN
-            livestream = false
-          ELSE
-            true
-        END
-      """)
-    )
-  end
-
-  def where_pending_download(query) do
-    query
-    |> where_download_not_prevented()
-    |> with_no_media_filepath()
-    |> with_upload_date_after_source_cutoff()
-    |> with_format_matching_profile_preference()
-    |> matching_source_title_regex()
-  end
-
-  def where_pending_or_downloaded(query) do
-    query
-    |> where_pending_download()
-    |> or_where([mi], not is_nil(mi.media_downloaded_at))
-  end
-
-  defp require_assoc(query, identifier) do
-    if has_named_binding?(query, identifier) do
-      query
-    else
-      do_require_assoc(query, identifier)
-    end
-  end
-
-  defp do_require_assoc(query, :source) do
-    from(mi in query, join: s in assoc(mi, :source), as: :source)
-  end
-
-  defp do_require_assoc(query, :media_profile) do
-    query
-    |> require_assoc(:source)
-    |> join(:inner, [mi, source], mp in assoc(source, :media_profile), as: :media_profile)
-  end
 end

lib/pinchflat/podcasts/podcast_helpers.ex

@@ -27,8 +27,7 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do
     limit = Keyword.get(opts, :limit, 1_000)
 
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.downloaded()))
-    |> MediaQuery.with_media_filepath()
     |> order_by(desc: :upload_date)
     |> Repo.maybe_limit(limit)
     |> Repo.all()

lib/pinchflat/sources/sources.ex

@@ -127,7 +127,7 @@ defmodule Pinchflat.Sources do
     Tasks.delete_tasks_for(source)
 
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> where(^MediaQuery.for_source(source))
     |> Repo.all()
     |> Enum.each(fn media_item ->
       Media.delete_media_item(media_item, delete_files: delete_files)

lib/pinchflat/sources/sources_query.ex

@@ -12,20 +12,26 @@ defmodule Pinchflat.Sources.SourcesQuery do
 
   alias Pinchflat.Sources.Source
 
-  # Prefixes:
+  # This allows the module to be aliased and query methods to be used
-  # - for_* - belonging to a certain record
+  # all in one go
-  # - join_* - for joining on a certain record
+  # usage: use Pinchflat.Sources.SourcesQuery
-  # - with_* - for filtering based on full, concrete attributes
+  defmacro __using__(_opts) do
-  # - matching_* - for filtering based on partial attributes (e.g. LIKE, regex, full-text search)
+    quote do
-  #
+      import Ecto.Query, warn: false
-  # Suffixes:
+
-  # - _for - the arg passed is an association record
+      alias unquote(__MODULE__)
+    end
+  end
 
   def new do
     Source
   end
 
-  def for_media_profile(query, media_profile) do
+  def for_media_profile(media_profile_id) when is_integer(media_profile_id) do
-    where(query, [s], s.media_profile_id == ^media_profile.id)
+    dynamic([s], s.media_profile_id == ^media_profile_id)
+  end
+
+  def for_media_profile(media_profile) do
+    dynamic([s], s.media_profile_id == ^media_profile.id)
   end
 end

lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex

@@ -1,11 +1,9 @@
 defmodule PinchflatWeb.MediaProfiles.MediaProfileController do
   use PinchflatWeb, :controller
-
+  use Pinchflat.Sources.SourcesQuery
-  import Ecto.Query, warn: false
 
   alias Pinchflat.Repo
   alias Pinchflat.Profiles
-  alias Pinchflat.Sources.SourcesQuery
   alias Pinchflat.Profiles.MediaProfile
 
   def index(conn, _params) do
@@ -43,7 +41,7 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do
 
     sources =
       SourcesQuery.new()
-      |> SourcesQuery.for_media_profile(media_profile)
+      |> where(^SourcesQuery.for_media_profile(media_profile))
       |> order_by(asc: :custom_name)
       |> Repo.all()
 

lib/pinchflat_web/controllers/pages/page_controller.ex

@@ -26,7 +26,7 @@ defmodule PinchflatWeb.Pages.PageController do
       source_count: Repo.aggregate(Source, :count, :id),
       media_item_count:
         MediaQuery.new()
-        |> MediaQuery.with_media_downloaded_at()
+        |> where(^MediaQuery.downloaded())
         |> Repo.aggregate(:count, :id)
     )
   end

lib/pinchflat_web/controllers/pages/page_html/history_table_live.ex

@@ -97,7 +97,8 @@ defmodule Pinchflat.Pages.HistoryTableLive do
 
   defp generate_base_query do
     MediaQuery.new()
-    |> MediaQuery.where_pending_or_downloaded()
+    |> MediaQuery.require_assoc(:media_profile)
+    |> where(^dynamic(^MediaQuery.downloaded() or ^MediaQuery.pending()))
     |> order_by(desc: :id)
   end
 

lib/pinchflat_web/controllers/podcasts/podcast_controller.ex

@@ -26,8 +26,7 @@ defmodule PinchflatWeb.Podcasts.PodcastController do
     # if the source doesn't have any usable images
     media_items =
       MediaQuery.new()
-      |> MediaQuery.for_source(source)
+      |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.downloaded()))
-      |> MediaQuery.with_media_filepath()
       |> Repo.maybe_limit(1)
       |> Repo.all()
 

lib/pinchflat_web/controllers/sources/source_controller.ex

@@ -25,7 +25,7 @@ defmodule PinchflatWeb.Sources.SourceController do
             subquery(
               from m in MediaItem,
                 where: m.source_id == parent_as(:source).id,
-                where: ^MediaQuery.downloaded?(),
+                where: ^MediaQuery.downloaded(),
                 select: count(m.id)
             ),
           pending_count:
@@ -33,7 +33,7 @@ defmodule PinchflatWeb.Sources.SourceController do
               from m in MediaItem,
                 join: s in assoc(m, :source),
                 where: m.source_id == parent_as(:source).id,
-                where: ^MediaQuery.pending?(),
+                where: ^MediaQuery.pending(),
                 select: count(m.id)
             )
         }

lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex

@@ -86,15 +86,14 @@ defmodule Pinchflat.Sources.MediaItemTableLive do
 
   defp generate_base_query(source, "pending") do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> MediaQuery.require_assoc(:media_profile)
-    |> MediaQuery.where_pending_download()
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.pending()))
     |> order_by(desc: :id)
   end
 
   defp generate_base_query(source, "downloaded") do
     MediaQuery.new()
-    |> MediaQuery.for_source(source)
+    |> where(^dynamic(^MediaQuery.for_source(source) and ^MediaQuery.downloaded()))
-    |> MediaQuery.with_media_filepath()
     |> order_by(desc: :id)
   end
 end

test/pinchflat/downloading/downloading_helpers_test.exs

@@ -114,7 +114,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
   describe "kickoff_redownload_for_existing_media/1" do
     test "enqueues a download job for each downloaded media item" do
       source = source_fixture()
-      media_item = media_item_fixture(source_id: source.id, media_downloaded_at: now())
+      media_item = media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4")
 
       assert [{:ok, _}] = DownloadingHelpers.kickoff_redownload_for_existing_media(source)
 
@@ -124,14 +124,14 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
     test "doesn't enqueue jobs for media that should be ignored" do
       source = source_fixture()
       other_source = source_fixture()
-      _not_downloaded = media_item_fixture(source_id: source.id, media_downloaded_at: nil)
+      _not_downloaded = media_item_fixture(source_id: source.id, media_filepath: nil)
-      _other_source = media_item_fixture(source_id: other_source.id, media_downloaded_at: now())
+      _other_source = media_item_fixture(source_id: other_source.id, media_filepath: "some/filepath.mp4")
 
       _download_prevented =
-        media_item_fixture(source_id: source.id, media_downloaded_at: now(), prevent_download: true)
+        media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4", prevent_download: true)
 
       _culled =
-        media_item_fixture(source_id: source.id, media_downloaded_at: now(), culled_at: now())
+        media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4", culled_at: now())
 
       assert [] = DownloadingHelpers.kickoff_redownload_for_existing_media(source)
 

test/support/fixtures/media_fixtures.ex

@@ -17,7 +17,7 @@ defmodule Pinchflat.MediaFixtures do
       attrs
       |> Enum.into(%{
         media_id: media_id,
-        title: Faker.Commerce.product_name(),
+        title: Faker.Commerce.product_name() <> " #{media_id}",
         original_url: "https://www.youtube.com/watch?v=#{media_id}",
         livestream: false,
         short_form_content: false,