Bumped version

* Updated downloader and runner to handle sponsorblock failures more gracefully

* stopped download worker from running if a media item is preventing download

config/config.exs

@@ -85,7 +85,7 @@ config :esbuild,
 
 # Configure tailwind (the version is required)
 config :tailwind,
-  version: "3.3.2",
+  version: "3.4.3",
   default: [
     args: ~w(
       --config=tailwind.config.js

lib/pinchflat/boot/nfo_backfill_worker.ex

@@ -0,0 +1,70 @@
+defmodule Pinchflat.Boot.NfoBackfillWorker do
+  @moduledoc false
+
+  use Oban.Worker,
+    queue: :local_metadata,
+    # This should have it running once _ever_ (until the job is pruned, anyway)
+    # NOTE: remove within the next month
+    unique: [period: :infinity, states: Oban.Job.states()],
+    tags: ["media_item", "media_metadata", "local_metadata", "data_backfill"]
+
+  import Ecto.Query, warn: false
+  require Logger
+
+  alias Pinchflat.Repo
+  alias Pinchflat.Media
+  alias Pinchflat.Media.MediaItem
+  alias Pinchflat.Metadata.NfoBuilder
+  alias Pinchflat.Metadata.MetadataFileHelpers
+
+  @doc """
+  Runs a one-off backfill job to regenerate NFO files for media items that have
+  both an NFO file and a metadata file. This is needed because NFO files weren't
+  escaping characters properly so we need to regenerate them.
+
+  This job will only run once as long as I remove it before the jobs are pruned in a month.
+
+  Returns :ok
+  """
+  @impl Oban.Worker
+  def perform(%Oban.Job{}) do
+    Logger.info("Running NFO backfill worker")
+
+    media_items = get_media_items_to_backfill()
+
+    Enum.each(media_items, fn media_item ->
+      nfo_exists = File.exists?(media_item.nfo_filepath)
+      metadata_exists = File.exists?(media_item.metadata.metadata_filepath)
+
+      if nfo_exists && metadata_exists do
+        Logger.info("NFO and metadata exist for media item #{media_item.id} - proceeding")
+
+        regenerate_nfo_for_media_item(media_item)
+      end
+    end)
+
+    :ok
+  end
+
+  defp get_media_items_to_backfill do
+    from(m in MediaItem, where: not is_nil(m.nfo_filepath))
+    |> Repo.all()
+    |> Repo.preload([:metadata, source: :media_profile])
+  end
+
+  defp regenerate_nfo_for_media_item(media_item) do
+    try do
+      case MetadataFileHelpers.read_compressed_metadata(media_item.metadata.metadata_filepath) do
+        {:ok, metadata} ->
+          Media.update_media_item(media_item, %{
+            nfo_filepath: NfoBuilder.build_and_store_for_media_item(media_item.nfo_filepath, metadata)
+          })
+
+        _err ->
+          Logger.error("Failed to read metadata for media item #{media_item.id}")
+      end
+    rescue
+      e -> Logger.error("Unknown error regenerating NFO file for MI ##{media_item.id}: #{inspect(e)}")
+    end
+  end
+end

lib/pinchflat/boot/post_job_startup_tasks.ex

@@ -7,6 +7,9 @@ defmodule Pinchflat.Boot.PostJobStartupTasks do
   Phoenix supervision tree.
   """
 
+  alias Pinchflat.Repo
+  alias Pinchflat.Boot.NfoBackfillWorker
+
   # restart: :temporary means that this process will never be restarted (ie: will run once and then die)
   use GenServer, restart: :temporary
   import Ecto.Query, warn: false
@@ -26,7 +29,8 @@ defmodule Pinchflat.Boot.PostJobStartupTasks do
   """
   @impl true
   def init(state) do
-    # Empty for now, keeping because tasks _will_ be added in future
+    Repo.insert_unique_job(NfoBackfillWorker.new(%{}))
+
     {:ok, state}
   end
 end

lib/pinchflat/boot/pre_job_startup_tasks.ex

@@ -65,8 +65,6 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do
   defp apply_default_settings do
     {:ok, yt_dlp_version} = CommandRunner.version()
 
-    Settings.fetch!(:onboarding, true)
+    Settings.set(yt_dlp_version: yt_dlp_version)
-    Settings.fetch!(:pro_enabled, false)
-    Settings.set!(:yt_dlp_version, yt_dlp_version)
   end
 end

lib/pinchflat/downloading/media_download_worker.ex

@@ -19,27 +19,29 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
 
   Returns {:ok, %Task{}} | {:error, :duplicate_job} | {:error, %Ecto.Changeset{}}
   """
-  def kickoff_with_task(media_item, opts \\ []) do
+  def kickoff_with_task(media_item, job_args \\ %{}, job_opts \\ []) do
     %{id: media_item.id}
-    |> MediaDownloadWorker.new(opts)
+    |> Map.merge(job_args)
+    |> MediaDownloadWorker.new(job_opts)
     |> Tasks.create_job_with_task(media_item)
   end
 
   @doc """
   For a given media item, download the media alongside any options.
-  Does not download media if its source is set to not download media.
+  Does not download media if its source is set to not download media
+  (unless forced).
 
   Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any}
   """
   @impl Oban.Worker
-  def perform(%Oban.Job{args: %{"id" => media_item_id}}) do
+  def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do
     media_item =
       media_item_id
       |> Media.get_media_item!()
       |> Repo.preload(:source)
 
-    # If the source is set to not download media, perform a no-op
+    # If the source or media item is set to not download media, perform a no-op unless forced
-    if media_item.source.download_media do
+    if (media_item.source.download_media && !media_item.prevent_download) || args["force"] do
       download_media_and_schedule_jobs(media_item)
     else
       :ok
@@ -56,9 +58,10 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
 
         {:ok, updated_media_item}
 
-      err ->
+      {:recovered, _} ->
-        Logger.error("Failed to download media for media item #{media_item.id}: #{inspect(err)}")
+        {:error, :retry}
 
+      {:error, _message} ->
         {:error, :download_failed}
     end
   end

lib/pinchflat/downloading/media_downloader.ex

@@ -5,12 +5,15 @@ defmodule Pinchflat.Downloading.MediaDownloader do
   to download the media with the desired options.
   """
 
+  require Logger
+
   alias Pinchflat.Repo
   alias Pinchflat.Media
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Metadata.NfoBuilder
   alias Pinchflat.Metadata.MetadataParser
   alias Pinchflat.Metadata.MetadataFileHelpers
+  alias Pinchflat.Filesystem.FilesystemHelpers
   alias Pinchflat.Downloading.DownloadOptionBuilder
 
   alias Pinchflat.YtDlp.Media, as: YtDlpMedia
@@ -27,33 +30,69 @@ defmodule Pinchflat.Downloading.MediaDownloader do
   Returns {:ok, %MediaItem{}} | {:error, any, ...any}
   """
   def download_for_media_item(%MediaItem{} = media_item) do
-    item_with_preloads = Repo.preload(media_item, [:metadata, source: :media_profile])
+    output_filepath = FilesystemHelpers.generate_metadata_tmpfile(:json)
+    media_with_preloads = Repo.preload(media_item, [:metadata, source: :media_profile])
 
-    case download_with_options(media_item.original_url, item_with_preloads) do
+    case download_with_options(media_item.original_url, media_with_preloads, output_filepath) do
       {:ok, parsed_json} ->
-        parsed_attrs =
+        update_media_item_from_parsed_json(media_with_preloads, parsed_json)
-          parsed_json
-          |> MetadataParser.parse_for_media_item()
-          |> Map.merge(%{
-            media_downloaded_at: DateTime.utc_now(),
-            nfo_filepath: determine_nfo_filepath(item_with_preloads, parsed_json),
-            metadata: %{
-              # IDEA: might be worth kicking off a job for this since thumbnail fetching
-              # could fail and I want to handle that in isolation
-              metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, parsed_json),
-              thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_item, parsed_json)
-            }
-          })
 
-        # Don't forgor to use preloaded associations or updates to
+      {:error, message, _exit_code} ->
-        # associations won't work!
+        Logger.error("yt-dlp download error for media item ##{media_with_preloads.id}: #{inspect(message)}")
-        Media.update_media_item(item_with_preloads, parsed_attrs)
+
+        if String.contains?(to_string(message), recoverable_errors()) do
+          attempt_update_media_item(media_with_preloads, output_filepath)
+
+          {:recovered, message}
+        else
+          {:error, message}
+        end
 
       err ->
-        err
+        Logger.error("Unknown error downloading media item ##{media_with_preloads.id}: #{inspect(err)}")
+
+        {:error, "Unknown error: #{inspect(err)}"}
     end
   end
 
+  defp attempt_update_media_item(media_with_preloads, output_filepath) do
+    with {:ok, contents} <- File.read(output_filepath),
+         {:ok, parsed_json} <- Phoenix.json_library().decode(contents) do
+      Logger.info("""
+      Recovery from yt-dlp error seems possible. Updating media item ##{media_with_preloads.id}
+      with parsed JSON from partial download attempt. Full download will be re-attemted in future
+      anyway
+      """)
+
+      update_media_item_from_parsed_json(media_with_preloads, parsed_json)
+    else
+      err ->
+        Logger.error("Unable to recover error for media item ##{media_with_preloads.id}: #{inspect(err)}")
+
+        {:error, :retry_failed}
+    end
+  end
+
+  defp update_media_item_from_parsed_json(media_with_preloads, parsed_json) do
+    parsed_attrs =
+      parsed_json
+      |> MetadataParser.parse_for_media_item()
+      |> Map.merge(%{
+        media_downloaded_at: DateTime.utc_now(),
+        nfo_filepath: determine_nfo_filepath(media_with_preloads, parsed_json),
+        metadata: %{
+          # IDEA: might be worth kicking off a job for this since thumbnail fetching
+          # could fail and I want to handle that in isolation
+          metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_with_preloads, parsed_json),
+          thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_with_preloads, parsed_json)
+        }
+      })
+
+    # Don't forgor to use preloaded associations or updates to
+    # associations won't work!
+    Media.update_media_item(media_with_preloads, parsed_attrs)
+  end
+
   defp determine_nfo_filepath(media_item, parsed_json) do
     if media_item.source.media_profile.download_nfo do
       filepath = Path.rootname(parsed_json["filepath"]) <> ".nfo"
@@ -64,9 +103,15 @@ defmodule Pinchflat.Downloading.MediaDownloader do
     end
   end
 
-  defp download_with_options(url, item_with_preloads) do
+  defp download_with_options(url, item_with_preloads, output_filepath) do
     {:ok, options} = DownloadOptionBuilder.build(item_with_preloads)
 
-    YtDlpMedia.download(url, options)
+    YtDlpMedia.download(url, options, output_filepath: output_filepath)
+  end
+
+  defp recoverable_errors do
+    [
+      "Unable to communicate with SponsorBlock"
+    ]
   end
 end

lib/pinchflat/media/media.ex

@@ -30,7 +30,6 @@ defmodule Pinchflat.Media do
   """
   def list_cullable_media_items do
     MediaQuery.new()
-    |> MediaQuery.join_sources()
     |> MediaQuery.with_media_filepath()
     |> MediaQuery.with_passed_retention_period()
     |> MediaQuery.with_no_culling_prevention()
@@ -40,36 +39,16 @@ defmodule Pinchflat.Media do
   @doc """
   Returns a list of pending media_items for a given source, where
   pending means the `media_filepath` is `nil` AND the media_item
-  matches the format selection rules of the parent media_profile.
+  matches satisfies `MediaQuery.with_media_pending_download`. You
-
+  should really check out that function if you need to know more
-  See `matching_download_criteria_for` but tl;dr is it _may_ filter based
+  because it has a lot going on.
-  on shorts livestreams depending on the media_profile settings.
 
   Returns [%MediaItem{}, ...].
   """
-  def list_pending_media_items_for(%Source{} = source, opts \\ []) do
+  def list_pending_media_items_for(%Source{} = source) do
-    limit = Keyword.get(opts, :limit, nil)
-    source = Repo.preload(source, :media_profile)
-
     MediaQuery.new()
     |> MediaQuery.for_source(source)
-    |> matching_download_criteria_for(source)
+    |> MediaQuery.with_media_pending_download()
-    |> Repo.maybe_limit(limit)
-    |> 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, opts \\ []) do
-    limit = Keyword.get(opts, :limit, nil)
-
-    MediaQuery.new()
-    |> MediaQuery.for_source(source)
-    |> MediaQuery.with_media_filepath()
-    |> Repo.maybe_limit(limit)
     |> Repo.all()
   end
 
@@ -87,7 +66,7 @@ defmodule Pinchflat.Media do
 
     MediaQuery.new()
     |> MediaQuery.with_id(media_item.id)
-    |> matching_download_criteria_for(media_item.source)
+    |> MediaQuery.with_media_pending_download()
     |> Repo.exists?()
   end
 
@@ -234,13 +213,4 @@ defmodule Pinchflat.Media do
     |> Enum.filter(&is_binary/1)
     |> Enum.each(&FilesystemHelpers.delete_file_and_remove_empty_directories/1)
   end
-
-  defp matching_download_criteria_for(query, source_with_preloads) do
-    query
-    |> MediaQuery.with_no_prevented_download()
-    |> MediaQuery.with_no_media_filepath()
-    |> MediaQuery.with_upload_date_after(source_with_preloads.download_cutoff_date)
-    |> MediaQuery.with_format_preference(source_with_preloads.media_profile)
-    |> MediaQuery.matching_title_regex(source_with_preloads.title_filter_regex)
-  end
 end

lib/pinchflat/media/media_query.ex

@@ -3,13 +3,10 @@ defmodule Pinchflat.Media.MediaQuery do
   Query helpers for the Media context.
 
   These methods are made to be one-ish liners used
-  to compose queries for media items. Each method should
+  to compose queries. Each method should strive to do
-  strive to do _one_ thing. These don't need to be tested
+  _one_ thing. These don't need to be tested as
-  as they are just building blocks for other functionality
+  they are just building blocks for other functionality
   which, itself, will be tested.
-
-  ALSO, this is me trying something new. If I like it,
-  I'll refactor other contexts to use this pattern.
   """
   import Ecto.Query, warn: false
 
@@ -37,13 +34,14 @@ defmodule Pinchflat.Media.MediaQuery do
   end
 
   def with_passed_retention_period(query) do
-    where(
+    query
-      query,
+    |> require_assoc(:source)
-      [mi, sources],
+    |> where(
+      [mi, source],
       fragment(
         "IFNULL(?, 0) > 0 AND DATETIME('now', '-' || ? || ' day') > ?",
-        sources.retention_period_days,
+        source.retention_period_days,
-        sources.retention_period_days,
+        source.retention_period_days,
         mi.media_downloaded_at
       )
     )
@@ -69,20 +67,23 @@ defmodule Pinchflat.Media.MediaQuery do
     where(query, [mi], is_nil(mi.media_filepath))
   end
 
-  def with_upload_date_after(query, nil), do: query
+  def with_upload_date_after_source_cutoff(query) do
-
+    query
-  def with_upload_date_after(query, date) do
+    |> require_assoc(:source)
-    where(query, [mi], mi.upload_date >= ^date)
+    |> where([mi, source], is_nil(source.download_cutoff_date) or mi.upload_date >= source.download_cutoff_date)
   end
 
   def with_no_prevented_download(query) do
     where(query, [mi], mi.prevent_download == false)
   end
 
-  def matching_title_regex(query, nil), do: query
+  def matching_source_title_regex(query) do
-
+    query
-  def matching_title_regex(query, regex) do
+    |> require_assoc(:source)
-    where(query, [mi], fragment("regexp_like(?, ?)", mi.title, ^regex))
+    |> where(
+      [mi, source],
+      is_nil(source.title_filter_regex) or fragment("regexp_like(?, ?)", mi.title, source.title_filter_regex)
+    )
   end
 
   def matching_search_term(query, nil), do: query
@@ -103,44 +104,55 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
-  # NOTE: this method breaks the contract set by other methods in that it
+  def with_format_matching_profile_preference(query) do
-  # takes a media_profile struct instead of taking just the attributes it
+    query
-  # cares about. Consider refactoring but low priority.
+    |> require_assoc(:media_profile)
-  def with_format_preference(query, media_profile) do
+    |> where(
-    mapped_struct = Map.from_struct(media_profile)
+      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
 
-    finders =
+  def with_media_pending_download(query) do
-      Enum.reduce(mapped_struct, dynamic(true), fn attr, dynamic ->
+    query
-        case {attr, media_profile} do
+    |> with_no_prevented_download()
-          {{:shorts_behaviour, :only}, %{livestream_behaviour: :only}} ->
+    |> with_no_media_filepath()
-            dynamic(
+    |> with_upload_date_after_source_cutoff()
-              [mi],
+    |> with_format_matching_profile_preference()
-              ^dynamic and (mi.livestream == true or mi.short_form_content == true)
+    |> matching_source_title_regex()
-            )
+  end
 
-          # Technically redundant, but makes the other clauses easier to parse
+  defp require_assoc(query, identifier) do
-          # (redundant because this condition is the same as the condition above, just flipped)
+    if has_named_binding?(query, identifier) do
-          {{:livestream_behaviour, :only}, %{shorts_behaviour: :only}} ->
+      query
-            dynamic
+    else
+      do_require_assoc(query, identifier)
+    end
+  end
 
-          {{:shorts_behaviour, :only}, _} ->
+  defp do_require_assoc(query, :source) do
-            dynamic([mi], ^dynamic and mi.short_form_content == true)
+    from(mi in query, join: s in assoc(mi, :source), as: :source)
+  end
 
-          {{:livestream_behaviour, :only}, _} ->
+  defp do_require_assoc(query, :media_profile) do
-            dynamic([mi], ^dynamic and mi.livestream == true)
+    query
-
+    |> require_assoc(:source)
-          {{:shorts_behaviour, :exclude}, %{livestream_behaviour: lb}} when lb != :only ->
+    |> join(:inner, [mi, source], mp in assoc(source, :media_profile), as: :media_profile)
-            dynamic([mi], ^dynamic and mi.short_form_content == false)
-
-          {{:livestream_behaviour, :exclude}, %{shorts_behaviour: sb}} when sb != :only ->
-            # return records with livestream: false
-            dynamic([mi], ^dynamic and mi.livestream == false)
-
-          _ ->
-            dynamic
-        end
-      end)
-
-    where(query, ^finders)
   end
 end

lib/pinchflat/metadata/nfo_builder.ex

@@ -4,6 +4,8 @@ defmodule Pinchflat.Metadata.NfoBuilder do
   use by Kodi/Jellyfin and other media center software.
   """
 
+  import Pinchflat.Utils.XmlUtils, only: [safe: 1]
+
   alias Pinchflat.Metadata.MetadataFileHelpers
   alias Pinchflat.Filesystem.FilesystemHelpers
 
@@ -42,12 +44,12 @@ defmodule Pinchflat.Metadata.NfoBuilder do
     """
     <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
     <episodedetails>
-      <title>#{metadata["title"]}</title>
+      <title>#{safe(metadata["title"])}</title>
-      <showtitle>#{metadata["uploader"]}</showtitle>
+      <showtitle>#{safe(metadata["uploader"])}</showtitle>
-      <uniqueid type="youtube" default="true">#{metadata["id"]}</uniqueid>
+      <uniqueid type="youtube" default="true">#{safe(metadata["id"])}</uniqueid>
-      <plot>#{metadata["description"]}</plot>
+      <plot>#{safe(metadata["description"])}</plot>
-      <aired>#{upload_date}</aired>
+      <aired>#{safe(upload_date)}</aired>
-      <season>#{upload_date.year}</season>
+      <season>#{safe(upload_date.year)}</season>
       <episode>#{Calendar.strftime(upload_date, "%m%d")}</episode>
       <genre>YouTube</genre>
     </episodedetails>
@@ -58,9 +60,9 @@ defmodule Pinchflat.Metadata.NfoBuilder do
     """
     <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
     <tvshow>
-      <title>#{metadata["title"]}</title>
+      <title>#{safe(metadata["title"])}</title>
-      <plot>#{metadata["description"]}</plot>
+      <plot>#{safe(metadata["description"])}</plot>
-      <uniqueid type="youtube" default="true">#{metadata["id"]}</uniqueid>
+      <uniqueid type="youtube" default="true">#{safe(metadata["id"])}</uniqueid>
       <genre>YouTube</genre>
     </tvshow>
     """

lib/pinchflat/podcasts/podcast_helpers.ex

@@ -5,7 +5,7 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do
   """
 
   alias Pinchflat.Repo
-  alias Pinchflat.Media
+  alias Pinchflat.Media.MediaQuery
   alias Pinchflat.Metadata.MediaMetadata
   alias Pinchflat.Metadata.SourceMetadata
 
@@ -25,8 +25,11 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do
   def persisted_media_items_for(source, opts \\ []) do
     limit = Keyword.get(opts, :limit, 500)
 
-    source
+    MediaQuery.new()
-    |> Media.list_downloaded_media_items_for(limit: limit)
+    |> MediaQuery.for_source(source)
+    |> MediaQuery.with_media_filepath()
+    |> Repo.maybe_limit(limit)
+    |> Repo.all()
     |> Enum.filter(fn media_item -> File.exists?(media_item.media_filepath) end)
   end
 

lib/pinchflat/podcasts/rss_feed_builder.ex

@@ -5,6 +5,8 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do
 
   @datetime_format "%a, %d %b %Y %H:%M:%S %z"
 
+  import Pinchflat.Utils.XmlUtils, only: [safe: 1]
+
   alias Pinchflat.Utils.DatetimeUtils
   alias Pinchflat.Podcasts.PodcastHelpers
   alias PinchflatWeb.Router.Helpers, as: Routes
@@ -94,14 +96,6 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do
     """
   end
 
-  defp safe(nil), do: ""
-
-  defp safe(value) do
-    value
-    |> Phoenix.HTML.html_escape()
-    |> Phoenix.HTML.safe_to_string()
-  end
-
   defp generate_self_link(url_base, source) do
     Path.join(url_base, "#{podcast_route(:rss_feed, source.uuid)}.xml")
   end

lib/pinchflat/settings/setting.ex

@@ -1,24 +1,32 @@
 defmodule Pinchflat.Settings.Setting do
   @moduledoc """
-  A Setting is a key-value pair with a datatype used to track user-level settings.
+  The Setting schema.
   """
 
   use Ecto.Schema
   import Ecto.Changeset
 
-  schema "settings" do
+  @allowed_fields [
-    field :name, :string
+    :onboarding,
-    field :value, :string
+    :pro_enabled,
-    field :datatype, Ecto.Enum, values: ~w(boolean string integer float)a
+    :yt_dlp_version
+  ]
 
-    timestamps(type: :utc_datetime)
+  @required_fields ~w(
+    onboarding
+    pro_enabled
+  )a
+
+  schema "settings" do
+    field :onboarding, :boolean, default: true
+    field :pro_enabled, :boolean, default: false
+    field :yt_dlp_version, :string
   end
 
   @doc false
   def changeset(setting, attrs) do
     setting
-    |> cast(attrs, [:name, :value, :datatype])
+    |> cast(attrs, @allowed_fields)
-    |> validate_required([:name, :value, :datatype])
+    |> validate_required(@required_fields)
-    |> unique_constraint([:name])
   end
 end

lib/pinchflat/settings/settings.ex

@@ -2,94 +2,63 @@ defmodule Pinchflat.Settings do
   @moduledoc """
   The Settings context.
   """
-
   import Ecto.Query, warn: false
-  alias Pinchflat.Repo
 
+  alias Pinchflat.Repo
   alias Pinchflat.Settings.Setting
 
   @doc """
-  Returns the list of settings.
+  Returns the only setting record. It _should_ be impossible
+  to create or delete this record, so it's assertive about
+  assuming it's the only one.
 
-  Returns [%Setting{}, ...]
+  Returns %Setting{}
   """
-  def list_settings do
+  def record do
-    Repo.all(Setting)
+    Setting
+    |> limit(1)
+    |> Repo.one()
   end
 
   @doc """
-  Creates or updates a setting, returning the parsed value.
+  Updates a setting, returning the new value.
-  Raises if an unsupported datatype is used. Optionally allows
+  Is setup to take a keyword list argument so you
-  specifying the datatype.
+  can call it like `Settings.set(onboarding: true)`
 
-  Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)`
+  Returns {:ok, value} | {:error, :invalid_key} | {:error, %Ecto.Changeset{}}
   """
-  def set!(name, value) do
+  def set([{attr, value}]) do
-    set!(name, value, infer_datatype(value))
+    record()
-  end
+    |> Setting.changeset(%{attr => value})
-
+    |> Repo.update()
-  def set!(name, value, datatype) do
+    |> case do
-    # Only create if doesn't exist
+      {:ok, %{^attr => _}} -> {:ok, value}
-    case Repo.get_by(Setting, name: to_string(name)) do
+      {:ok, _} -> {:error, :invalid_key}
-      nil -> create_setting!(name, value, datatype)
+      {:error, changeset} -> {:error, changeset}
-      setting -> update_setting!(setting, value, datatype)
     end
   end
 
   @doc """
-  Gets the parsed value of a setting. Raises if the setting does not exist.
+  Gets the value of a setting.
 
-  Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)`
+  Returns {:ok, value} | {:error, :invalid_key}
+  """
+  def get(name) do
+    case Map.fetch(record(), name) do
+      {:ok, value} -> {:ok, value}
+      :error -> {:error, :invalid_key}
+    end
+  end
+
+  @doc """
+  Gets the value of a setting, raising if it doesn't exist.
+
+  Returns value
   """
   def get!(name) do
-    Setting
+    case get(name) do
-    |> Repo.get_by!(name: to_string(name))
+      {:ok, value} -> value
-    |> read_setting()
+      {:error, _} -> raise "Setting `#{name}` not found"
-  end
-
-  @doc """
-  Attempts to find a setting by name or creates a setting with value
-  if one doesn't exist, returning the parsed value. Optionally allows
-  specifying the datatype.
-
-  Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)`
-  """
-  def fetch!(name, value) do
-    fetch!(name, value, infer_datatype(value))
-  end
-
-  def fetch!(name, value, datatype) do
-    case Repo.get_by(Setting, name: to_string(name)) do
-      nil -> create_setting!(name, value, datatype)
-      setting -> read_setting(setting)
     end
   end
-
-  defp change_setting(setting, attrs) do
-    Setting.changeset(setting, attrs)
-  end
-
-  defp create_setting!(name, value, datatype) do
-    %Setting{}
-    |> change_setting(%{name: to_string(name), value: to_string(value), datatype: datatype})
-    |> Repo.insert!()
-    |> read_setting()
-  end
-
-  defp update_setting!(setting, value, datatype) do
-    setting
-    |> change_setting(%{value: to_string(value), datatype: datatype})
-    |> Repo.update!()
-    |> read_setting()
-  end
-
-  defp read_setting(%{value: value, datatype: :string}), do: value
-  defp read_setting(%{value: value, datatype: :boolean}), do: value in ["true", "t", "1"]
-  defp read_setting(%{value: value, datatype: :integer}), do: String.to_integer(value)
-  defp read_setting(%{value: value, datatype: :float}), do: String.to_float(value)
-
-  defp infer_datatype(value) when is_boolean(value), do: :boolean
-  defp infer_datatype(value) when is_integer(value), do: :integer
-  defp infer_datatype(value) when is_float(value), do: :float
-  defp infer_datatype(value) when is_binary(value), do: :string
 end

lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex

@@ -20,9 +20,10 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do
 
   Returns {:ok, %Task{}} | {:error, :duplicate_job} | {:error, %Ecto.Changeset{}}
   """
-  def kickoff_with_task(source, opts \\ []) do
+  def kickoff_with_task(source, job_args \\ %{}, job_opts \\ []) do
     %{id: source.id}
-    |> MediaCollectionIndexingWorker.new(opts)
+    |> Map.merge(job_args)
+    |> MediaCollectionIndexingWorker.new(job_opts)
     |> Tasks.create_job_with_task(source)
   end
 
@@ -30,8 +31,8 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do
   The ID is that of a source _record_, not a YouTube channel/playlist ID. Indexes
   the provided source, kicks off downloads for each new MediaItem, and
   reschedules the job to run again in the future. It will ALWAYS index a source
-  if it's never been indexed before, but rescheduling is determined by the
+  if it's never been indexed before or if `force` is set to `true`, but rescheduling
-  `index_frequency_minutes` field.
+  is determined by the `index_frequency_minutes` field.
 
   README: Re-scheduling here works a little different than you may expect.
   The reschedule time is relative to the time the job has actually _completed_.
@@ -71,7 +72,7 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do
   Returns :ok | {:ok, %Task{}}
   """
   @impl Oban.Worker
-  def perform(%Oban.Job{args: %{"id" => source_id}}) do
+  def perform(%Oban.Job{args: %{"id" => source_id} = args}) do
     source = Sources.get_source!(source_id)
 
     case {source.index_frequency_minutes, source.last_indexed_at} do
@@ -89,7 +90,11 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do
 
       _ ->
         # If the source HAS been indexed and is not meant to reschedule,
-        # perform a no-op
+        # perform a no-op (unless forced)
+        if args["force"] do
+          SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source)
+        end
+
         :ok
     end
   rescue

lib/pinchflat/slow_indexing/slow_indexing_helpers.ex

@@ -27,12 +27,12 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do
 
   Returns {:ok, %Task{}}.
   """
-  def kickoff_indexing_task(%Source{} = source) do
+  def kickoff_indexing_task(%Source{} = source, job_args \\ %{}, job_opts \\ []) do
     Tasks.delete_pending_tasks_for(source, "FastIndexingWorker")
     Tasks.delete_pending_tasks_for(source, "MediaIndexingWorker")
     Tasks.delete_pending_tasks_for(source, "MediaCollectionIndexingWorker")
 
-    MediaCollectionIndexingWorker.kickoff_with_task(source)
+    MediaCollectionIndexingWorker.kickoff_with_task(source, job_args, job_opts)
   end
 
   @doc """

lib/pinchflat/sources/sources_query.ex

@@ -0,0 +1,31 @@
+defmodule Pinchflat.Sources.SourcesQuery do
+  @moduledoc """
+  Query helpers for the Sources context.
+
+  These methods are made to be one-ish liners used
+  to compose queries. Each method should strive to do
+  _one_ thing. These don't need to be tested as
+  they are just building blocks for other functionality
+  which, itself, will be tested.
+  """
+  import Ecto.Query, warn: false
+
+  alias Pinchflat.Sources.Source
+
+  # Prefixes:
+  # - for_* - belonging to a certain record
+  # - join_* - for joining on a certain record
+  # - with_* - 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
+
+  def new do
+    Source
+  end
+
+  def for_media_profile(query, media_profile) do
+    where(query, [s], s.media_profile_id == ^media_profile.id)
+  end
+end

lib/pinchflat/utils/xml_utils.ex

@@ -0,0 +1,17 @@
+defmodule Pinchflat.Utils.XmlUtils do
+  @moduledoc """
+  Utility methods for working with XML documents
+  """
+
+  @doc """
+  Escapes invalid XML characters in a string
+
+  Returns binary()
+  """
+  def safe(value) do
+    value
+    |> to_string()
+    |> Phoenix.HTML.html_escape()
+    |> Phoenix.HTML.safe_to_string()
+  end
+end

lib/pinchflat/yt_dlp/command_runner.ex

@@ -29,7 +29,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do
     command = backend_executable()
     # These must stay in exactly this order, hence why I'm giving it its own variable.
     # Also, can't use RAM file since yt-dlp needs a concrete filepath.
-    output_filepath = Keyword.get(addl_opts, :output_filepath, FSUtils.generate_metadata_tmpfile(:json))
+    output_filepath = generate_output_filepath(addl_opts)
     print_to_file_opts = [{:print_to_file, output_template}, output_filepath]
     cookie_opts = build_cookie_options()
     formatted_command_opts = [url] ++ parse_options(command_opts ++ print_to_file_opts ++ cookie_opts)
@@ -61,6 +61,13 @@ defmodule Pinchflat.YtDlp.CommandRunner do
     end
   end
 
+  defp generate_output_filepath(addl_opts) do
+    case Keyword.get(addl_opts, :output_filepath) do
+      nil -> FSUtils.generate_metadata_tmpfile(:json)
+      path -> path
+    end
+  end
+
   defp build_cookie_options do
     base_dir = Application.get_env(:pinchflat, :extras_directory)
     cookie_file = Path.join(base_dir, "cookies.txt")

lib/pinchflat/yt_dlp/media.ex

@@ -35,10 +35,10 @@ defmodule Pinchflat.YtDlp.Media do
 
   Returns {:ok, map()} | {:error, any, ...}.
   """
-  def download(url, command_opts \\ []) do
+  def download(url, command_opts \\ [], addl_opts \\ []) do
     opts = [:no_simulate] ++ command_opts
 
-    with {:ok, output} <- backend_runner().run(url, opts, "after_move:%()j"),
+    with {:ok, output} <- backend_runner().run(url, opts, "after_move:%()j", addl_opts),
          {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
       {:ok, parsed_json}
     else

lib/pinchflat_web/components/custom_components/tab_components.ex

@@ -34,7 +34,7 @@ defmodule PinchflatWeb.CustomComponents.TabComponents do
           <%= render_slot(@tab_append) %>
         </div>
       </header>
-      <div class="mt-4">
+      <div class="mt-4 min-h-60">
         <div :for={{tab, idx} <- Enum.with_index(@tab)} x-show={"openTab === #{idx}"} class="font-medium leading-relaxed">
           <%= render_slot(tab) %>
         </div>

lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex

@@ -30,7 +30,7 @@ defmodule Pinchflat.UpgradeButtonLive do
       |> String.downcase()
 
     if normalized_text == "got it!" do
-      Settings.set!(:pro_enabled, true)
+      Settings.set(pro_enabled: true)
 
       {:noreply, update(socket, :button_disabled, fn _ -> false end)}
     else

lib/pinchflat_web/controllers/media_items/media_item_controller.ex

@@ -6,6 +6,7 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do
   alias Pinchflat.Repo
   alias Pinchflat.Media
   alias Pinchflat.Media.MediaItem
+  alias Pinchflat.Downloading.MediaDownloadWorker
 
   def show(conn, %{"id" => id}) do
     media_item =
@@ -47,6 +48,15 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do
     |> redirect(to: ~p"/sources/#{media_item.source_id}")
   end
 
+  def force_download(conn, %{"media_item_id" => id}) do
+    media_item = Media.get_media_item!(id)
+    {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, %{force: true})
+
+    conn
+    |> put_flash(:info, "Download task enqueued.")
+    |> redirect(to: ~p"/sources/#{media_item.source_id}/media/#{media_item}")
+  end
+
   # See here for details on streaming files and range requests:
   # https://www.zeng.dev/post/2023-http-range-and-play-mp4-in-browser/
   #

lib/pinchflat_web/controllers/media_items/media_item_html/actions_dropdown.html.heex

@@ -0,0 +1,32 @@
+<.button_dropdown text="Actions" class="justify-center w-full sm:w-50">
+  <:option>
+    <.link
+      href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}/force_download"}
+      method="post"
+      data-confirm="Are you sure you force a download of this media?"
+    >
+      Force Download
+    </.link>
+  </:option>
+  <:option>
+    <div class="h-px w-full bg-bodydark2"></div>
+  </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}"}
+      method="delete"
+      data-confirm="Are you sure you want to delete all files for this media item? This cannot be undone."
+    >
+      Delete Files
+    </.link>
+  </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?prevent_download=true"}
+      method="delete"
+      data-confirm="Are you sure you want to delete all files for this media item and prevent it from re-downloading in the future? This cannot be undone."
+    >
+      Delete and Ignore
+    </.link>
+  </:option>
+</.button_dropdown>

lib/pinchflat_web/controllers/media_items/media_item_html/media_item_form.html.heex

@@ -17,7 +17,7 @@
     field={f[:prevent_download]}
     type="toggle"
     label="Prevent Download"
-    help="Checking excludes this media item from being downloaded"
+    help="Checking excludes this media item from automatic download. Download can still be manually forced"
   />
 
   <.input

lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex

@@ -20,26 +20,7 @@
   <div class="max-w-full overflow-x-auto">
     <.tabbed_layout>
       <:tab_append>
-        <.button_dropdown text="Actions" class="justify-center w-full sm:w-50">
+        <.actions_dropdown media_item={@media_item} />
-          <:option>
-            <.link
-              href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}"}
-              method="delete"
-              data-confirm="Are you sure you want to delete all files for this media item? This cannot be undone."
-            >
-              Delete Files
-            </.link>
-          </:option>
-          <:option>
-            <.link
-              href={~p"/sources/#{@media_item.source_id}/media/#{@media_item}?prevent_download=true"}
-              method="delete"
-              data-confirm="Are you sure you want to delete all files for this media item and prevent it from re-downloading in the future? This cannot be undone."
-            >
-              Delete and Ignore
-            </.link>
-          </:option>
-        </.button_dropdown>
       </:tab_append>
 
       <:tab title="Attributes">
@@ -53,9 +34,9 @@
           <h3 class="font-bold text-xl">Attributes</h3>
           <section>
             <strong>Source:</strong>
-            <.inline_link href={~p"/sources/#{@media_item.source_id}"}>
+            <.subtle_link href={~p"/sources/#{@media_item.source_id}"}>
               <%= @media_item.source.custom_name %>
-            </.inline_link>
+            </.subtle_link>
           </section>
 
           <.list_items_from_map map={Map.from_struct(@media_item)} />

lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex

@@ -1,12 +1,19 @@
 defmodule PinchflatWeb.MediaProfiles.MediaProfileController do
   use PinchflatWeb, :controller
 
+  import Ecto.Query, warn: false
+
   alias Pinchflat.Repo
   alias Pinchflat.Profiles
+  alias Pinchflat.Sources.SourcesQuery
   alias Pinchflat.Profiles.MediaProfile
 
   def index(conn, _params) do
-    media_profiles = Profiles.list_media_profiles()
+    media_profiles =
+      MediaProfile
+      |> order_by(asc: :name)
+      |> Repo.all()
+
     render(conn, :index, media_profiles: media_profiles)
   end
 
@@ -32,12 +39,15 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do
   end
 
   def show(conn, %{"id" => id}) do
-    media_profile =
+    media_profile = Profiles.get_media_profile!(id)
-      id
-      |> Profiles.get_media_profile!()
-      |> Repo.preload(:sources)
 
-    render(conn, :show, media_profile: media_profile)
+    sources =
+      SourcesQuery.new()
+      |> SourcesQuery.for_media_profile(media_profile)
+      |> order_by(asc: :custom_name)
+      |> Repo.all()
+
+    render(conn, :show, media_profile: media_profile, sources: sources)
   end
 
   def edit(conn, %{"id" => id}) do

lib/pinchflat_web/controllers/media_profiles/media_profile_html/show.html.heex

@@ -50,7 +50,7 @@
         </div>
       </:tab>
       <:tab title="Sources">
-        <.table rows={@media_profile.sources} table_class="text-black dark:text-white">
+        <.table rows={@sources} table_class="text-black dark:text-white">
           <:col :let={source} label="Name">
             <.subtle_link href={~p"/sources/#{source.id}"}>
               <%= source.custom_name || source.collection_name %>

lib/pinchflat_web/controllers/pages/page_controller.ex

@@ -11,7 +11,7 @@ defmodule PinchflatWeb.Pages.PageController do
     done_onboarding = params["onboarding"] == "0"
     force_onboarding = params["onboarding"] == "1"
 
-    if done_onboarding, do: Settings.set!(:onboarding, false)
+    if done_onboarding, do: Settings.set(onboarding: false)
 
     if force_onboarding || Settings.get!(:onboarding) do
       render_onboarding_page(conn)
@@ -30,7 +30,7 @@ defmodule PinchflatWeb.Pages.PageController do
   end
 
   defp render_onboarding_page(conn) do
-    Settings.set!(:onboarding, true)
+    Settings.set(onboarding: true)
 
     conn
     |> render(:onboarding_checklist,

lib/pinchflat_web/controllers/podcasts/podcast_controller.ex

@@ -2,7 +2,7 @@ defmodule PinchflatWeb.Podcasts.PodcastController do
   use PinchflatWeb, :controller
 
   alias Pinchflat.Repo
-  alias Pinchflat.Media
+  alias Pinchflat.Media.MediaQuery
   alias Pinchflat.Sources.Source
   alias Pinchflat.Podcasts.RssFeedBuilder
   alias Pinchflat.Podcasts.PodcastHelpers
@@ -20,10 +20,15 @@ defmodule PinchflatWeb.Podcasts.PodcastController do
 
   def feed_image(conn, %{"uuid" => uuid}) do
     source = Repo.get_by!(Source, uuid: uuid)
-    # This provides a fallback image if the source has none.
+
-    # We only need one since we're using the internal metadata image which
+    # This is used to fetch a fallback cover image
-    # we know exists.
+    # if the source doesn't have any usable images
-    media_items = Media.list_downloaded_media_items_for(source, limit: 1)
+    media_items =
+      MediaQuery.new()
+      |> MediaQuery.for_source(source)
+      |> MediaQuery.with_media_filepath()
+      |> Repo.maybe_limit(1)
+      |> Repo.all()
 
     case PodcastHelpers.select_cover_image(source, media_items) do
       {:error, _} ->

lib/pinchflat_web/controllers/sources/source_controller.ex

@@ -4,15 +4,21 @@ defmodule PinchflatWeb.Sources.SourceController do
   import Ecto.Query, warn: false
 
   alias Pinchflat.Repo
-  alias Pinchflat.Media
   alias Pinchflat.Tasks
   alias Pinchflat.Sources
-  alias Pinchflat.Profiles
+  alias Pinchflat.MediaQuery
   alias Pinchflat.Sources.Source
   alias Pinchflat.Media.MediaQuery
+  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Downloading.DownloadingHelpers
+  alias Pinchflat.SlowIndexing.SlowIndexingHelpers
 
   def index(conn, _params) do
-    sources = Repo.preload(Sources.list_sources(), :media_profile)
+    sources =
+      Source
+      |> order_by(asc: :custom_name)
+      |> Repo.all()
+      |> Repo.preload(:media_profile)
 
     render(conn, :index, sources: sources)
   end
@@ -49,9 +55,26 @@ defmodule PinchflatWeb.Sources.SourceController do
   def show(conn, %{"id" => id}) do
     source = Repo.preload(Sources.get_source!(id), :media_profile)
 
-    pending_tasks = Repo.preload(Tasks.list_pending_tasks_for(source), :job)
+    pending_tasks =
-    pending_media = Media.list_pending_media_items_for(source, limit: 100)
+      source
-    downloaded_media = Media.list_downloaded_media_items_for(source, limit: 100)
+      |> Tasks.list_tasks_for(nil, [:executing, :available, :scheduled, :retryable])
+      |> Repo.preload(:job)
+
+    pending_media =
+      MediaQuery.new()
+      |> MediaQuery.for_source(source)
+      |> MediaQuery.with_media_pending_download()
+      |> order_by(desc: :id)
+      |> limit(100)
+      |> Repo.all()
+
+    downloaded_media =
+      MediaQuery.new()
+      |> MediaQuery.for_source(source)
+      |> MediaQuery.with_media_filepath()
+      |> order_by(desc: :id)
+      |> limit(100)
+      |> Repo.all()
 
     render(conn, :show,
       source: source,
@@ -104,12 +127,30 @@ defmodule PinchflatWeb.Sources.SourceController do
     |> redirect(to: ~p"/sources")
   end
 
+  def force_download(conn, %{"source_id" => id}) do
+    source = Sources.get_source!(id)
+    DownloadingHelpers.enqueue_pending_download_tasks(source)
+
+    conn
+    |> put_flash(:info, "Forced download of pending media items.")
+    |> redirect(to: ~p"/sources/#{source}")
+  end
+
+  def force_index(conn, %{"source_id" => id}) do
+    source = Sources.get_source!(id)
+    SlowIndexingHelpers.kickoff_indexing_task(source, %{force: true})
+
+    conn
+    |> put_flash(:info, "Index enqueued.")
+    |> redirect(to: ~p"/sources/#{source}")
+  end
+
   defp media_profiles do
-    Profiles.list_media_profiles()
+    MediaProfile
+    |> order_by(asc: :name)
+    |> Repo.all()
   end
 
-  # NOTE: should move this out of the controller
-  # once I finally add some query fragment layer
   defp total_downloaded_for(source) do
     MediaQuery.new()
     |> MediaQuery.for_source(source)

lib/pinchflat_web/controllers/sources/source_html/actions_dropdown.html.heex

@@ -0,0 +1,55 @@
+<.button_dropdown text="Actions" class="justify-center w-full sm:w-50">
+  <:option>
+    <span
+      x-data="{ copied: false }"
+      x-on:click={"
+                window.copyTextToClipboard('#{rss_feed_url(@conn, @source)}')
+                copied = true
+                setTimeout(() => copied = false, 4000)
+              "}
+    >
+      Copy RSS Feed
+      <span x-show="copied" x-transition.duration.150ms><.icon name="hero-check" class="ml-2 h-4 w-4" /></span>
+    </span>
+  </:option>
+  <:option :if={@source.download_media}>
+    <.link
+      href={~p"/sources/#{@source}/force_download"}
+      method="post"
+      data-confirm="Are you sure you want to force a download of all *pending* media items? This isn't normally needed."
+    >
+      Force Download
+    </.link>
+  </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@source}/force_index"}
+      method="post"
+      data-confirm="Are you sure you want to force an index of this source? This isn't normally needed."
+    >
+      Force Index
+    </.link>
+  </:option>
+  <:option>
+    <div class="h-px w-full bg-bodydark2"></div>
+  </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@source}"}
+      method="delete"
+      data-confirm="Are you sure you want to delete this source (leaving files in place)? This cannot be undone."
+    >
+      Delete Source
+    </.link>
+  </:option>
+  <:option>
+    <.link
+      href={~p"/sources/#{@source}?delete_files=true"}
+      method="delete"
+      data-confirm="Are you sure you want to delete this source and it's files on disk? This cannot be undone."
+      class="mt-5 md:mt-0"
+    >
+      Delete Source + Files
+    </.link>
+  </:option>
+</.button_dropdown>

lib/pinchflat_web/controllers/sources/source_html/show.html.heex

@@ -20,43 +20,7 @@
   <div class="max-w-full overflow-x-auto">
     <.tabbed_layout>
       <:tab_append>
-        <.button_dropdown text="Actions" class="justify-center w-full sm:w-50">
+        <.actions_dropdown source={@source} conn={@conn} />
-          <:option>
-            <span
-              x-data="{ copied: false }"
-              x-on:click={"
-                window.copyTextToClipboard('#{rss_feed_url(@conn, @source)}')
-                copied = true
-                setTimeout(() => copied = false, 4000)
-              "}
-            >
-              Copy RSS Feed
-              <span x-show="copied" x-transition.duration.150ms><.icon name="hero-check" class="ml-2 h-4 w-4" /></span>
-            </span>
-          </:option>
-          <:option>
-            <div class="h-px w-full bg-bodydark2"></div>
-          </:option>
-          <:option>
-            <.link
-              href={~p"/sources/#{@source}"}
-              method="delete"
-              data-confirm="Are you sure you want to delete this source (leaving files in place)? This cannot be undone."
-            >
-              Delete Source
-            </.link>
-          </:option>
-          <:option>
-            <.link
-              href={~p"/sources/#{@source}?delete_files=true"}
-              method="delete"
-              data-confirm="Are you sure you want to delete this source and it's files on disk? This cannot be undone."
-              class="mt-5 md:mt-0"
-            >
-              Delete Source + Files
-            </.link>
-          </:option>
-        </.button_dropdown>
       </:tab_append>
 
       <:tab title="Attributes">
@@ -64,9 +28,9 @@
           <h3 class="font-bold text-lg">Attributes</h3>
           <section>
             <strong>Media Profile:</strong>
-            <.inline_link href={~p"/media_profiles/#{@source.media_profile_id}"}>
+            <.subtle_link href={~p"/media_profiles/#{@source.media_profile_id}"}>
               <%= @source.media_profile.name %>
-            </.inline_link>
+            </.subtle_link>
           </section>
 
           <.list_items_from_map map={Map.from_struct(@source)} />

lib/pinchflat_web/endpoint.ex

@@ -20,7 +20,7 @@ defmodule PinchflatWeb.Endpoint do
   plug Plug.Static,
     at: "/",
     from: :pinchflat,
-    gzip: false,
+    gzip: Mix.env() == :prod,
     only: PinchflatWeb.static_paths()
 
   # Code reloading can be explicitly enabled under the

lib/pinchflat_web/router.ex

@@ -32,7 +32,12 @@ defmodule PinchflatWeb.Router do
     resources "/search", Searches.SearchController, only: [:show], singleton: true
 
     resources "/sources", Sources.SourceController do
-      resources "/media", MediaItems.MediaItemController, only: [:show, :edit, :update, :delete]
+      post "/force_download", Sources.SourceController, :force_download
+      post "/force_index", Sources.SourceController, :force_index
+
+      resources "/media", MediaItems.MediaItemController, only: [:show, :edit, :update, :delete] do
+        post "/force_download", MediaItems.MediaItemController, :force_download
+      end
     end
   end
 

mix.exs

@@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do
   def project do
     [
       app: :pinchflat,
-      version: "0.1.7",
+      version: "0.1.9",
       elixir: "~> 1.16",
       elixirc_paths: elixirc_paths(Mix.env()),
       start_permanent: Mix.env() == :prod,

priv/repo/migrations/20240404172256_rename_settings_table.exs

@@ -0,0 +1,7 @@
+defmodule Pinchflat.Repo.Migrations.RenameSettingsTable do
+  use Ecto.Migration
+
+  def change do
+    rename table(:settings), to: table(:settings_backup)
+  end
+end

priv/repo/migrations/20240404174144_create_new_settings.exs

@@ -0,0 +1,29 @@
+defmodule Pinchflat.Repo.Migrations.CreateNewSettings do
+  use Ecto.Migration
+
+  def up do
+    create table(:settings) do
+      add :onboarding, :boolean, default: true, null: false
+      add :pro_enabled, :boolean, default: false, null: false
+      add :yt_dlp_version, :string
+    end
+
+    # Make an initial record because this will be the only one ever inserted
+    execute "INSERT INTO settings (onboarding, pro_enabled, yt_dlp_version) VALUES (true, false, NULL)"
+
+    # Set the value of onboarding to the previous version set in `settings_backup`
+    execute """
+      UPDATE settings
+      SET onboarding = COALESCE((SELECT value = 'true' FROM settings_backup WHERE name = 'onboarding'), true)
+    """
+
+    execute """
+      UPDATE settings
+      SET pro_enabled = COALESCE((SELECT value = 'true' FROM settings_backup WHERE name = 'pro_enabled'), false)
+    """
+  end
+
+  def down do
+    drop table(:settings)
+  end
+end

selfhosted.Dockerfile

@@ -108,6 +108,17 @@ EXPOSE ${PORT}
 # Only copy the final release from the build stage
 COPY --from=builder /app/_build/${MIX_ENV}/rel/pinchflat ./
 
+# NEVER do this if you're running in an environment where you don't trust the user
+# (ie: most environments). This is only acceptable in a self-hosted environment.
+# The user could just run the whole container as root and bypass this anyway so
+# it's not a huge deal.
+# This removes the root password to allow users to assume root if needed. This is
+# preferrable to running the whole container as root so that the files/directories
+# created by the app aren't owned by root and are therefore easier for other users
+# and processes to interact with. If you want to just run the whole container as
+# root, use --user 0:0 or something.
+RUN passwd -d root
+
 # If using an environment that doesn't automatically reap zombie processes, it is
 # advised to add an init process such as tini via `apt-get install`
 # above and adding an entrypoint. See https://github.com/krallin/tini for details

test/pinchflat/boot/pre_job_startup_tasks_test.exs

@@ -1,25 +1,47 @@
 defmodule Pinchflat.Boot.PreJobStartupTasksTest do
   use Pinchflat.DataCase
 
+  import Pinchflat.JobFixtures
+
   alias Pinchflat.Settings
-  alias Pinchflat.Settings.Setting
   alias Pinchflat.Boot.PreJobStartupTasks
 
-  describe "apply_default_settings" do
+  describe "reset_executing_jobs" do
-    setup do
+    test "resets executing jobs" do
-      Repo.delete_all(Setting)
+      job = job_fixture()
+      Repo.update_all(Oban.Job, set: [state: "executing"])
 
-      :ok
+      assert Repo.reload!(job).state == "executing"
-    end
-
-    test "sets default settings" do
-      assert_raise Ecto.NoResultsError, fn -> Settings.get!(:onboarding) end
-      assert_raise Ecto.NoResultsError, fn -> Settings.get!(:pro_enabled) end
 
       PreJobStartupTasks.start_link()
 
-      assert Settings.get!(:onboarding)
+      assert Repo.reload!(job).state == "retryable"
-      refute Settings.get!(:pro_enabled)
+    end
+  end
+
+  describe "create_blank_cookie_file" do
+    test "creates a blank cookie file" do
+      base_dir = Application.get_env(:pinchflat, :extras_directory)
+      filepath = Path.join(base_dir, "cookies.txt")
+      File.rm(filepath)
+
+      refute File.exists?(filepath)
+
+      PreJobStartupTasks.start_link()
+
+      assert File.exists?(filepath)
+    end
+  end
+
+  describe "apply_default_settings" do
+    test "sets default settings" do
+      Settings.set(yt_dlp_version: nil)
+
+      refute Settings.get!(:yt_dlp_version)
+
+      PreJobStartupTasks.start_link()
+
+      assert Settings.get!(:yt_dlp_version)
     end
   end
 end

test/pinchflat/downloading/media_download_worker_test.exs

@@ -4,6 +4,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
   import Mox
   import Pinchflat.MediaFixtures
 
+  alias Pinchflat.Media
   alias Pinchflat.Sources
   alias Pinchflat.Filesystem.FilesystemHelpers
   alias Pinchflat.Downloading.MediaDownloadWorker
@@ -34,11 +35,28 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
       assert {:ok, task} = MediaDownloadWorker.kickoff_with_task(media_item)
       assert task.media_item_id == media_item.id
     end
+
+    test "can be called with additional job arguments", %{media_item: media_item} do
+      job_args = %{"force" => true}
+
+      assert {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, job_args)
+
+      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id, "force" => true})
+    end
+
+    test "can be called with additional job options", %{media_item: media_item} do
+      job_opts = [max_attempts: 5]
+
+      assert {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item, %{}, job_opts)
+
+      [job] = all_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
+      assert job.max_attempts == 5
+    end
   end
 
   describe "perform/1" do
     test "it saves attributes to the media_item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
@@ -48,7 +66,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it saves the metadata to the media_item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
@@ -65,7 +83,19 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it sets the job to retryable if the download fails", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "error"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:error, "error"} end)
+
+      Oban.Testing.with_testing_mode(:inline, fn ->
+        {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))
+
+        assert job.state == "retryable"
+      end)
+    end
+
+    test "sets the job to retryable if the download failed and was retried", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+        {:error, "Unable to communicate with SponsorBlock", 1}
+      end)
 
       Oban.Testing.with_testing_mode(:inline, fn ->
         {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))
@@ -75,21 +105,38 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it ensures error are returned in a 2-item tuple", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "error", 1} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:error, "error", 1} end)
 
       assert {:error, :download_failed} = perform_job(MediaDownloadWorker, %{id: media_item.id})
     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)
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> :ok end)
 
       Sources.update_source(media_item.source, %{download_media: false})
 
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
+    test "does not download if the media item is set to not download", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> :ok end)
+
+      Media.update_media_item(media_item, %{prevent_download: true})
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id})
+    end
+
+    test "downloads anyway if forced", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> :ok end)
+
+      Sources.update_source(media_item.source, %{download_media: false})
+      Media.update_media_item(media_item, %{prevent_download: true})
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
+    end
+
     test "it saves the file's size to the database", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         metadata = render_parsed_metadata(:media_metadata)
         FilesystemHelpers.write_p!(metadata["filepath"], "test")
 

test/pinchflat/downloading/media_downloader_test.exs

@@ -25,9 +25,11 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
 
   describe "download_for_media_item/3" do
     test "it calls the backend runner", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn url, _opts, ot ->
+      expect(YtDlpRunnerMock, :run, fn url, _opts, ot, addl ->
         assert url == media_item.original_url
         assert ot == "after_move:%()j"
+        assert [{:output_filepath, filepath}] = addl
+        assert is_binary(filepath)
 
         {:ok, render_metadata(:media_metadata)}
       end)
@@ -36,7 +38,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
     end
 
     test "it saves the metadata filepath to the database", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
@@ -47,18 +49,56 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
       assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/maxresdefault.jpg"
     end
 
-    test "errors are passed through", %{media_item: media_item} do
+    test "non-recoverable errors are passed through", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
-        {:error, :some_error}
+        {:error, :some_error, 1}
       end)
 
       assert {:error, :some_error} = MediaDownloader.download_for_media_item(media_item)
     end
+
+    test "unknown errors are passed through", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+        {:error, :some_error}
+      end)
+
+      assert {:error, message} = MediaDownloader.download_for_media_item(media_item)
+      assert message == "Unknown error: {:error, :some_error}"
+    end
+  end
+
+  describe "download_for_media_item/3 when testing retries" do
+    test "returns a recovered tuple on recoverable errors", %{media_item: media_item} do
+      message = "Unable to communicate with SponsorBlock"
+
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+        {:error, message, 1}
+      end)
+
+      assert {:recovered, ^message} = MediaDownloader.download_for_media_item(media_item)
+    end
+
+    test "attempts to update the media item on recoverable errors", %{media_item: media_item} do
+      message = "Unable to communicate with SponsorBlock"
+
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, addl ->
+        [{:output_filepath, filepath}] = addl
+        File.write(filepath, render_metadata(:media_metadata))
+
+        {:error, message, 1}
+      end)
+
+      assert {:recovered, ^message} = MediaDownloader.download_for_media_item(media_item)
+      media_item = Repo.reload(media_item)
+
+      assert DateTime.diff(DateTime.utc_now(), media_item.media_downloaded_at) < 2
+      assert String.ends_with?(media_item.media_filepath, ".mkv")
+    end
   end
 
   describe "download_for_media_item/3 when testing media_item attributes" do
     setup do
-      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
@@ -100,7 +140,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
     end
 
     test "it extracts the thumbnail_filepath", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         metadata = render_parsed_metadata(:media_metadata)
 
         thumbnail_filepath =
@@ -124,7 +164,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
     end
 
     test "it extracts the metadata_filepath", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         metadata = render_parsed_metadata(:media_metadata)
 
         infojson_filepath = metadata["infojson_filename"]
@@ -143,7 +183,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
 
   describe "download_for_media_item/3 when testing NFO generation" do
     setup do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 

test/pinchflat/media_test.exs

@@ -149,14 +149,6 @@ defmodule Pinchflat.MediaTest do
 
       assert Media.list_pending_media_items_for(source) == []
     end
-
-    test "optionally accepts a limit" do
-      source = source_fixture()
-      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-
-      assert Media.list_pending_media_items_for(source, limit: 1) == [media_item]
-      assert Media.list_pending_media_items_for(source, limit: 0) == []
-    end
   end
 
   describe "list_pending_media_items_for/1 when testing shorts" do
@@ -335,25 +327,6 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
-  describe "list_downloaded_media_items_for/1" do
-    test "returns only media items with a media_filepath" do
-      source = source_fixture()
-      _media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-      media_item = media_item_fixture(%{source_id: source.id, media_filepath: "/video/#{Faker.File.file_name(:video)}"})
-
-      assert Media.list_downloaded_media_items_for(source) == [media_item]
-    end
-
-    test "optionally accepts a limit" do
-      source = source_fixture()
-      _media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil})
-      media_item = media_item_fixture(%{source_id: source.id, media_filepath: "/video/#{Faker.File.file_name(:video)}"})
-
-      assert Media.list_downloaded_media_items_for(source, limit: 1) == [media_item]
-      assert Media.list_downloaded_media_items_for(source, limit: 0) == []
-    end
-  end
-
   describe "pending_download?/1" do
     test "returns true when the media hasn't been downloaded" do
       media_item = media_item_fixture(%{media_filepath: nil})

test/pinchflat/metadata/nfo_builder_test.exs

@@ -30,6 +30,21 @@ defmodule Pinchflat.Metadata.NfoBuilderTest do
       assert String.contains?(nfo, ~S(<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>))
       assert String.contains?(nfo, "<title>#{metadata["title"]}</title>")
     end
+
+    test "escapes invalid characters", %{filepath: filepath} do
+      metadata = %{
+        "title" => "hello' & <world>",
+        "uploader" => "uploader",
+        "id" => "id",
+        "description" => "description",
+        "upload_date" => "20210101"
+      }
+
+      result = NfoBuilder.build_and_store_for_media_item(filepath, metadata)
+      nfo = File.read!(result)
+
+      assert String.contains?(nfo, "hello&#39; &amp; &lt;world&gt;")
+    end
   end
 
   describe "build_and_store_for_source/2" do
@@ -46,5 +61,18 @@ defmodule Pinchflat.Metadata.NfoBuilderTest do
       assert String.contains?(nfo, ~S(<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>))
       assert String.contains?(nfo, "<title>#{metadata["title"]}</title>")
     end
+
+    test "escapes invalid characters", %{filepath: filepath} do
+      metadata = %{
+        "title" => "hello' & <world>",
+        "description" => "description",
+        "id" => "id"
+      }
+
+      result = NfoBuilder.build_and_store_for_source(filepath, metadata)
+      nfo = File.read!(result)
+
+      assert String.contains?(nfo, "hello&#39; &amp; &lt;world&gt;")
+    end
   end
 end

test/pinchflat/settings_test.exs

@@ -9,100 +9,55 @@ defmodule Pinchflat.SettingsTest do
   # are always created on app boot (including in the test env),
   # so we can't treat these like a clean slate.
 
-  describe "list_settings/0" do
+  setup do
-    test "returns all settings" do
+    # Ensure we have a clean slate
-      Settings.set!("foo", "bar")
+    Settings.set(onboarding: false)
-      results = Settings.list_settings()
+    Settings.set(pro_enabled: false)
+    Settings.set(yt_dlp_version: nil)
 
-      assert Enum.all?(results, fn setting -> match?(%Setting{}, setting) end)
+    :ok
+  end
+
+  describe "record/0" do
+    test "returns the only setting" do
+      assert %Setting{} = Settings.record()
     end
   end
 
-  describe "set/2" do
+  describe "set/1" do
-    test "creates a new setting if one does not exist" do
+    test "updates the setting" do
-      original = Repo.aggregate(Setting, :count, :id)
+      assert {:ok, true} = Settings.set(onboarding: true)
-      Settings.set!("foo", "bar")
+      assert {:ok, true} = Settings.get(:onboarding)
-      assert Repo.aggregate(Setting, :count, :id) == original + 1
     end
 
-    test "updates an existing setting if one exists" do
+    test "returns an error if the setting key doesn't exist" do
-      Settings.set!("foo", "bar")
+      assert {:error, :invalid_key} = Settings.set(foo: "bar")
-      original = Repo.aggregate(Setting, :count, :id)
-      Settings.set!("foo", "baz")
-      assert Repo.aggregate(Setting, :count, :id) == original
-      assert Settings.get!("foo") == "baz"
     end
 
-    test "returns the parsed value" do
+    test "returns an error if the setting value is invalid" do
-      assert Settings.set!("foo", true) == true
+      assert {:error, %Ecto.Changeset{}} = Settings.set(onboarding: "bar")
-      assert Settings.set!("foo", false) == false
-      assert Settings.set!("foo", 123) == 123
-      assert Settings.set!("foo", 12.34) == 12.34
-      assert Settings.set!("foo", "bar") == "bar"
-    end
-
-    test "allows for atom keys" do
-      assert Settings.set!(:foo, "bar") == "bar"
-    end
-
-    test "blows up when an unsupported datatype is used" do
-      assert_raise FunctionClauseError, fn ->
-        Settings.set!("foo", nil)
-      end
-    end
-  end
-
-  describe "set/3" do
-    test "allows manual specification of datatype" do
-      assert Settings.set!("foo", "true", :boolean) == true
-      assert Settings.set!("foo", "false", :boolean) == false
-      assert Settings.set!("foo", "123", :integer) == 123
-      assert Settings.set!("foo", "12.34", :float) == 12.34
     end
   end
 
   describe "get/1" do
-    test "returns the value of the setting" do
+    test "returns the setting value" do
-      Settings.set!("str", "bar")
+      assert {:ok, false} = Settings.get(:onboarding)
-      Settings.set!("bool", true)
-      Settings.set!("int", 123)
-      Settings.set!("float", 12.34)
-
-      assert Settings.get!("str") == "bar"
-      assert Settings.get!("bool") == true
-      assert Settings.get!("int") == 123
-      assert Settings.get!("float") == 12.34
     end
 
-    test "allows for atom keys" do
+    test "returns an error if the setting key doesn't exist" do
-      Settings.set!("str", "bar")
+      assert {:error, :invalid_key} = Settings.get(:foo)
-      assert Settings.get!(:str) == "bar"
+    end
+  end
+
+  describe "get!/1" do
+    test "returns the setting value" do
+      assert Settings.get!(:onboarding) == false
     end
 
-    test "blows up when the setting does not exist" do
+    test "raises an error if the setting key doesn't exist" do
-      assert_raise Ecto.NoResultsError, fn ->
+      assert_raise RuntimeError, "Setting `foo` not found", fn ->
-        Settings.get!("foo")
+        Settings.get!(:foo)
       end
     end
   end
-
-  describe "fetch/2" do
-    test "creates a setting if one doesn't exist" do
-      original = Repo.aggregate(Setting, :count, :id)
-      assert Settings.fetch!("foo", "bar") == "bar"
-      assert Repo.aggregate(Setting, :count, :id) == original + 1
-    end
-
-    test "returns an existing setting if one does exist" do
-      Settings.set!("foo", "bar")
-
-      assert Settings.fetch!("foo", "baz") == "bar"
-    end
-  end
-
-  describe "fetch/3" do
-    test "allows manual specification of datatype" do
-      assert Settings.fetch!("foo", "true", :boolean) == true
-    end
-  end
 end

test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs

@@ -14,6 +14,42 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do
 
   setup :verify_on_exit!
 
+  describe "kickoff_with_task/3" do
+    setup do
+      source = source_fixture(index_frequency_minutes: 10)
+
+      {:ok, %{source: source}}
+    end
+
+    test "starts the worker", %{source: source} do
+      assert [] = all_enqueued(worker: MediaCollectionIndexingWorker)
+      assert {:ok, _} = MediaCollectionIndexingWorker.kickoff_with_task(source)
+      assert [_] = all_enqueued(worker: MediaCollectionIndexingWorker)
+    end
+
+    test "attaches a task", %{source: source} do
+      assert {:ok, task} = MediaCollectionIndexingWorker.kickoff_with_task(source)
+      assert task.source_id == source.id
+    end
+
+    test "can be called with additional job arguments", %{source: source} do
+      job_args = %{"force" => true}
+
+      assert {:ok, _} = MediaCollectionIndexingWorker.kickoff_with_task(source, job_args)
+
+      assert_enqueued(worker: MediaCollectionIndexingWorker, args: %{"id" => source.id, "force" => true})
+    end
+
+    test "can be called with additional job options", %{source: source} do
+      job_opts = [max_attempts: 5]
+
+      assert {:ok, _} = MediaCollectionIndexingWorker.kickoff_with_task(source, %{}, job_opts)
+
+      [job] = all_enqueued(worker: MediaCollectionIndexingWorker, args: %{"id" => source.id})
+      assert job.max_attempts == 5
+    end
+  end
+
   describe "perform/1" do
     test "it indexes the source if it should be indexed" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> {:ok, ""} end)
@@ -31,6 +67,14 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do
       perform_job(MediaCollectionIndexingWorker, %{id: source.id})
     end
 
+    test "it indexes the source no matter what if the 'force' arg is passed" do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> {:ok, ""} end)
+
+      source = source_fixture(index_frequency_minutes: 0, last_indexed_at: DateTime.utc_now())
+
+      perform_job(MediaCollectionIndexingWorker, %{id: source.id, force: true})
+    end
+
     test "it does not do any indexing if the source has been indexed and shouldn't be rescheduled" do
       expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl_opts -> {:ok, ""} end)
 

test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs

@@ -18,7 +18,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do
 
   setup :verify_on_exit!
 
-  describe "kickoff_indexing_task/1" do
+  describe "kickoff_indexing_task/3" do
     test "it schedules a job" do
       source = source_fixture(index_frequency_minutes: 1)
 
@@ -64,6 +64,25 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do
 
       assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end
     end
+
+    test "can be called with additional job arguments" do
+      source = source_fixture(index_frequency_minutes: 1)
+      job_args = %{"force" => true}
+
+      assert {:ok, _} = SlowIndexingHelpers.kickoff_indexing_task(source, job_args)
+
+      assert_enqueued(worker: MediaCollectionIndexingWorker, args: %{"id" => source.id, "force" => true})
+    end
+
+    test "can be called with additional job options" do
+      source = source_fixture(index_frequency_minutes: 1)
+      job_opts = [max_attempts: 5]
+
+      assert {:ok, _} = SlowIndexingHelpers.kickoff_indexing_task(source, %{}, job_opts)
+
+      [job] = all_enqueued(worker: MediaCollectionIndexingWorker, args: %{"id" => source.id})
+      assert job.max_attempts == 5
+    end
   end
 
   describe "index_and_enqueue_download_for_media_items/1" do

test/pinchflat/utils/xml_utils_test.exs

@@ -0,0 +1,16 @@
+defmodule Pinchflat.Utils.XmlUtilsTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Utils.XmlUtils
+
+  describe "safe/1" do
+    test "escapes invalid characters" do
+      assert XmlUtils.safe("hello' & <world>") == "hello&#39; &amp; &lt;world&gt;"
+    end
+
+    test "converts input to string" do
+      assert XmlUtils.safe(42) == "42"
+      assert XmlUtils.safe(nil) == ""
+    end
+  end
+end

test/pinchflat/yt_dlp/media_test.exs

@@ -11,9 +11,10 @@ defmodule Pinchflat.YtDlp.MediaTest do
 
   describe "download/2" do
     test "it calls the backend runner with the expected arguments" do
-      expect(YtDlpRunnerMock, :run, fn @media_url, opts, ot ->
+      expect(YtDlpRunnerMock, :run, fn @media_url, opts, ot, addl ->
         assert [:no_simulate] = opts
         assert "after_move:%()j" = ot
+        assert addl == []
 
         {:ok, render_metadata(:media_metadata)}
       end)
@@ -22,17 +23,18 @@ defmodule Pinchflat.YtDlp.MediaTest do
     end
 
     test "it passes along additional options" do
-      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, addl ->
         assert [:no_simulate, :custom_arg] = opts
+        assert [addl_arg: true] = addl
 
         {:ok, "{}"}
       end)
 
-      assert {:ok, _} = Media.download(@media_url, [:custom_arg])
+      assert {:ok, _} = Media.download(@media_url, [:custom_arg], addl_arg: true)
     end
 
     test "it parses and returns the generated file as JSON" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
@@ -41,7 +43,7 @@ defmodule Pinchflat.YtDlp.MediaTest do
     end
 
     test "it returns errors" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opt, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opt, _ot, _addl ->
         {:error, "something"}
       end)
 

test/pinchflat_web/controllers/media_item_controller_test.exs

@@ -4,6 +4,7 @@ defmodule PinchflatWeb.MediaItemControllerTest do
   import Pinchflat.MediaFixtures
 
   alias Pinchflat.Repo
+  alias Pinchflat.Downloading.MediaDownloadWorker
 
   describe "show media" do
     setup [:create_media_item]
@@ -87,6 +88,31 @@ defmodule PinchflatWeb.MediaItemControllerTest do
     end
   end
 
+  describe "force_download" do
+    test "enqueues download task", %{conn: conn} do
+      media_item = media_item_fixture()
+
+      assert [] = all_enqueued(worker: MediaDownloadWorker)
+      post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download")
+      assert [_] = all_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "forces a download even if one wouldn't normally run", %{conn: conn} do
+      media_item = media_item_fixture(%{media_filepath: nil})
+
+      post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download")
+      assert [_] = all_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id, "force" => true})
+    end
+
+    test "redirects to the show page", %{conn: conn} do
+      media_item = media_item_fixture()
+
+      conn = post(conn, ~p"/sources/#{media_item.source_id}/media/#{media_item.id}/force_download")
+
+      assert redirected_to(conn) == ~p"/sources/#{media_item.source_id}/media/#{media_item.id}"
+    end
+  end
+
   describe "streaming media" do
     test "returns 404 if the media isn't found", %{conn: conn} do
       media_item = media_item_fixture()

test/pinchflat_web/controllers/media_profile_controller_test.exs

@@ -16,7 +16,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do
   @invalid_attrs %{name: nil, output_path_template: nil}
 
   setup do
-    Settings.set!(:onboarding, false)
+    Settings.set(onboarding: false)
 
     :ok
   end
@@ -35,7 +35,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do
     end
 
     test "renders correct layout when onboarding", %{conn: conn} do
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = get(conn, ~p"/media_profiles/new")
 
       refute html_response(conn, 200) =~ "MENU"
@@ -59,14 +59,14 @@ defmodule PinchflatWeb.MediaProfileControllerTest do
     end
 
     test "redirects to onboarding when onboarding", %{conn: conn} do
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = post(conn, ~p"/media_profiles", media_profile: @create_attrs)
 
       assert redirected_to(conn) == ~p"/?onboarding=1"
     end
 
     test "renders correct layout on error when onboarding", %{conn: conn} do
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = post(conn, ~p"/media_profiles", media_profile: @invalid_attrs)
 
       refute html_response(conn, 200) =~ "MENU"

test/pinchflat_web/controllers/page_controller_test.exs

@@ -10,7 +10,7 @@ defmodule PinchflatWeb.PageControllerTest do
     end
 
     test "displays the onboarding page when onboarding is forced", %{conn: conn} do
-      Settings.set!(:onboarding, false)
+      Settings.set(onboarding: false)
 
       conn = get(conn, ~p"/?onboarding=1")
       assert html_response(conn, 200) =~ "Welcome to Pinchflat"
@@ -25,7 +25,7 @@ defmodule PinchflatWeb.PageControllerTest do
     end
 
     test "displays the home page when not onboarding", %{conn: conn} do
-      Settings.set!(:onboarding, false)
+      Settings.set(onboarding: false)
 
       conn = get(conn, ~p"/")
       assert html_response(conn, 200) =~ "MENU"

test/pinchflat_web/controllers/source_controller_test.exs

@@ -8,10 +8,12 @@ defmodule PinchflatWeb.SourceControllerTest do
 
   alias Pinchflat.Repo
   alias Pinchflat.Settings
+  alias Pinchflat.Downloading.MediaDownloadWorker
+  alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker
 
   setup do
     media_profile = media_profile_fixture()
-    Settings.set!(:onboarding, false)
+    Settings.set(onboarding: false)
 
     {
       :ok,
@@ -45,7 +47,7 @@ defmodule PinchflatWeb.SourceControllerTest do
     end
 
     test "renders correct layout when onboarding", %{conn: conn} do
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = get(conn, ~p"/sources/new")
 
       refute html_response(conn, 200) =~ "MENU"
@@ -72,14 +74,14 @@ defmodule PinchflatWeb.SourceControllerTest do
     test "redirects to onboarding when onboarding", %{conn: conn, create_attrs: create_attrs} do
       expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/3)
 
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = post(conn, ~p"/sources", source: create_attrs)
 
       assert redirected_to(conn) == ~p"/?onboarding=1"
     end
 
     test "renders correct layout on error when onboarding", %{conn: conn, invalid_attrs: invalid_attrs} do
-      Settings.set!(:onboarding, true)
+      Settings.set(onboarding: true)
       conn = post(conn, ~p"/sources", source: invalid_attrs)
 
       refute html_response(conn, 200) =~ "MENU"
@@ -160,6 +162,59 @@ defmodule PinchflatWeb.SourceControllerTest do
     end
   end
 
+  describe "force_download" do
+    test "enqueues pending download tasks", %{conn: conn} do
+      source = source_fixture()
+      _media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil})
+
+      assert [] = all_enqueued(worker: MediaDownloadWorker)
+      post(conn, ~p"/sources/#{source.id}/force_download")
+      assert [_] = all_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "redirects to the source page", %{conn: conn} do
+      source = source_fixture()
+
+      conn = post(conn, ~p"/sources/#{source.id}/force_download")
+      assert redirected_to(conn) == ~p"/sources/#{source.id}"
+    end
+  end
+
+  describe "force_index" do
+    test "forces an index", %{conn: conn} do
+      source = source_fixture()
+
+      assert [] = all_enqueued(worker: MediaCollectionIndexingWorker)
+      post(conn, ~p"/sources/#{source.id}/force_index")
+      assert [_] = all_enqueued(worker: MediaCollectionIndexingWorker)
+    end
+
+    test "forces an index even if one wouldn't normally run", %{conn: conn} do
+      source = source_fixture(index_frequency_minutes: 0, last_indexed_at: DateTime.utc_now())
+
+      post(conn, ~p"/sources/#{source.id}/force_index")
+      assert [job] = all_enqueued(worker: MediaCollectionIndexingWorker)
+      assert job.args == %{"id" => source.id, "force" => true}
+    end
+
+    test "deletes pending indexing tasks", %{conn: conn} do
+      source = source_fixture()
+      {:ok, task} = MediaCollectionIndexingWorker.kickoff_with_task(source)
+      job = Repo.preload(task, :job).job
+
+      assert job.state == "available"
+      post(conn, ~p"/sources/#{source.id}/force_index")
+      assert Repo.reload!(job).state == "cancelled"
+    end
+
+    test "redirects to the source page", %{conn: conn} do
+      source = source_fixture()
+
+      conn = post(conn, ~p"/sources/#{source.id}/force_index")
+      assert redirected_to(conn) == ~p"/sources/#{source.id}"
+    end
+  end
+
   defp create_source(_) do
     source = source_fixture()
     media_item = media_item_with_attachments(%{source_id: source.id})

test/support/conn_case.ex

@@ -21,8 +21,10 @@ defmodule PinchflatWeb.ConnCase do
     quote do
       # The default endpoint for testing
       @endpoint PinchflatWeb.Endpoint
+      alias Pinchflat.Repo
 
       use PinchflatWeb, :verified_routes
+      use Oban.Testing, repo: Repo
 
       # Import conveniences for testing with connections
       import Plug.Conn