From 26d457e65634a5b8e4b61215bad2145806c23277 Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 9 Apr 2024 17:45:39 -0700 Subject: [PATCH 001/240] [Enhancement] Add Apprise support (#170) * [WIP] add settings sidebar entry and placeholder page * [WIP] added placeholder UI and logic for settings form * Added column and UI for apprise server * Add some tests * Added placeholder command runner for apprise * [WIP] Adding apprise package * Added apprise command runner * Hooked up apprise notification module * Ensured apprise was running in verbose mode * Updated wording of apprise notification * Added apprise to README --- README.md | 1 + config/config.exs | 2 + config/test.exs | 1 + dev.Dockerfile | 22 ++-- lib/pinchflat/boot/pre_job_startup_tasks.ex | 15 ++- .../fast_indexing/fast_indexing_helpers.ex | 4 +- .../fast_indexing/fast_indexing_worker.ex | 12 ++- .../notifications/apprise_command_runner.ex | 12 +++ lib/pinchflat/notifications/command_runner.ex | 65 ++++++++++++ .../notifications/source_notifications.ex | 77 ++++++++++++++ lib/pinchflat/settings/setting.ex | 6 +- lib/pinchflat/settings/settings.ex | 21 +++- .../media_collection_indexing_worker.ex | 16 ++- .../slow_indexing/slow_indexing_helpers.ex | 8 +- lib/pinchflat/utils/cli_utils.ex | 48 +++++++++ lib/pinchflat/yt_dlp/command_runner.ex | 47 ++------ ...and_runner.ex => yt_dlp_command_runner.ex} | 2 +- .../components/core_components.ex | 13 +-- .../layouts/partials/sidebar.html.heex | 8 +- .../settings/setting_controller.ex | 26 +++++ .../controllers/settings/setting_html.ex | 20 ++++ .../setting_html/setting_form.html.heex | 21 ++++ .../settings/setting_html/show.html.heex | 12 +++ lib/pinchflat_web/router.ex | 1 + ...221551_add_apprise_servers_to_settings.exs | 9 ++ ...181121_add_apprise_version_to_settings.exs | 9 ++ selfhosted.Dockerfile | 6 +- .../boot/pre_job_startup_tasks_test.exs | 26 ++++- .../fast_indexing_helpers_test.exs | 10 +- .../fast_indexing_worker_test.exs | 25 +++++ .../notifications/command_runner_test.exs | 65 ++++++++++++ .../source_notifications_test.exs | 100 ++++++++++++++++++ test/pinchflat/settings_test.exs | 18 ++++ .../media_collection_indexing_worker_test.exs | 33 ++++++ test/pinchflat/utils/cli_utils_test.exs | 23 ++++ test/pinchflat/yt_dlp/command_runner_test.exs | 25 ----- .../controllers/setting_controller_test.exs | 23 ++++ test/test_helper.exs | 5 +- 38 files changed, 730 insertions(+), 107 deletions(-) create mode 100644 lib/pinchflat/notifications/apprise_command_runner.ex create mode 100644 lib/pinchflat/notifications/command_runner.ex create mode 100644 lib/pinchflat/notifications/source_notifications.ex create mode 100644 lib/pinchflat/utils/cli_utils.ex rename lib/pinchflat/yt_dlp/{backend_command_runner.ex => yt_dlp_command_runner.ex} (90%) create mode 100644 lib/pinchflat_web/controllers/settings/setting_controller.ex create mode 100644 lib/pinchflat_web/controllers/settings/setting_html.ex create mode 100644 lib/pinchflat_web/controllers/settings/setting_html/setting_form.html.heex create mode 100644 lib/pinchflat_web/controllers/settings/setting_html/show.html.heex create mode 100644 priv/repo/migrations/20240404221551_add_apprise_servers_to_settings.exs create mode 100644 priv/repo/migrations/20240408181121_add_apprise_version_to_settings.exs create mode 100644 test/pinchflat/notifications/command_runner_test.exs create mode 100644 test/pinchflat/notifications/source_notifications_test.exs create mode 100644 test/pinchflat/utils/cli_utils_test.exs create mode 100644 test/pinchflat_web/controllers/setting_controller_test.exs diff --git a/README.md b/README.md index be086d3..bdf24a5 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ If it doesn't work for your use case, please make a feature request! You can als - Uses a novel approach to download new content more quickly than other apps - Supports downloading audio content - Custom rules for handling YouTube Shorts and livestreams +- Apprise support for notifications - Optionally automatically delete old content ([docs](https://github.com/kieraneglin/pinchflat/wiki/Automatically-Delete-Media)) - Advanced options like setting cutoff dates and filtering by title - Reliable hands-off operation diff --git a/config/config.exs b/config/config.exs index 4e7a854..b395950 100644 --- a/config/config.exs +++ b/config/config.exs @@ -12,7 +12,9 @@ config :pinchflat, generators: [timestamp_type: :utc_datetime], # Specifying backend data here makes mocking and local testing SUPER easy yt_dlp_executable: System.find_executable("yt-dlp"), + apprise_executable: System.find_executable("apprise"), yt_dlp_runner: Pinchflat.YtDlp.CommandRunner, + apprise_runner: Pinchflat.Notifications.CommandRunner, media_directory: "/downloads", # The user may or may not store metadata for their needs, but the app will always store its copy metadata_directory: "/config/metadata", diff --git a/config/test.exs b/config/test.exs index 6c7409e..ce61044 100644 --- a/config/test.exs +++ b/config/test.exs @@ -3,6 +3,7 @@ import Config config :pinchflat, # Specifying backend data here makes mocking and local testing SUPER easy yt_dlp_executable: Path.join([File.cwd!(), "/test/support/scripts/yt-dlp-mocks/repeater.sh"]), + apprise_executable: Path.join([File.cwd!(), "/test/support/scripts/yt-dlp-mocks/repeater.sh"]), media_directory: Path.join([System.tmp_dir!(), "test", "media"]), metadata_directory: Path.join([System.tmp_dir!(), "test", "metadata"]), tmpfile_directory: Path.join([System.tmp_dir!(), "test", "tmpfiles"]), diff --git a/dev.Dockerfile b/dev.Dockerfile index 517ca63..bbf2683 100644 --- a/dev.Dockerfile +++ b/dev.Dockerfile @@ -5,15 +5,10 @@ ARG DEV_IMAGE="hexpm/elixir:${ELIXIR_VERSION}-erlang-${OTP_VERSION}-debian-${DEB FROM ${DEV_IMAGE} -# Set the locale deets -ENV LANG en_US.UTF-8 -ENV LANGUAGE en_US:en -ENV LC_ALL en_US.UTF-8 - # Install debian packages RUN apt-get update -qq RUN apt-get install -y inotify-tools ffmpeg curl git openssh-client \ - python3 python3-pip python3-setuptools python3-wheel python3-dev + python3 python3-pip python3-setuptools python3-wheel python3-dev locales # Install nodejs RUN curl -sL https://deb.nodesource.com/setup_20.x -o nodesource_setup.sh @@ -25,9 +20,20 @@ RUN npm install -g yarn RUN mix local.hex --force RUN mix local.rebar --force -# Download YT-DLP +# Download and update YT-DLP # NOTE: If you're seeing weird issues, consider using the FFMPEG released by yt-dlp -RUN python3 -m pip install -U --pre yt-dlp --break-system-packages +RUN curl -L https://github.com/yt-dlp/yt-dlp/releases/latest/download/yt-dlp -o /usr/local/bin/yt-dlp +RUN chmod a+rx /usr/local/bin/yt-dlp +RUN yt-dlp -U + +# Download Apprise +RUN python3 -m pip install -U apprise --break-system-packages + +# Set the locale +RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && locale-gen +ENV LANG en_US.UTF-8 +ENV LANGUAGE en_US:en +ENV LC_ALL en_US.UTF-8 # Create app directory and copy the Elixir projects into it. WORKDIR /app diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index d4f4168..74b96d3 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -14,7 +14,6 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do alias Pinchflat.Repo alias Pinchflat.Settings - alias Pinchflat.YtDlp.CommandRunner alias Pinchflat.Filesystem.FilesystemHelpers def start_link(opts \\ []) do @@ -56,15 +55,25 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do filepath = Path.join(base_dir, "cookies.txt") if !File.exists?(filepath) do - Logger.info("Cookies does not exist - creating it") + Logger.info("yt-dlp cookie file does not exist - creating it") FilesystemHelpers.write_p!(filepath, "") end end defp apply_default_settings do - {:ok, yt_dlp_version} = CommandRunner.version() + {:ok, yt_dlp_version} = yt_dlp_runner().version() + {:ok, apprise_version} = apprise_runner().version() Settings.set(yt_dlp_version: yt_dlp_version) + Settings.set(apprise_version: apprise_version) + end + + defp yt_dlp_runner do + Application.get_env(:pinchflat, :yt_dlp_runner) + end + + defp apprise_runner do + Application.get_env(:pinchflat, :apprise_runner) end end diff --git a/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex b/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex index a38ccf7..b9d5a44 100644 --- a/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex +++ b/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex @@ -25,7 +25,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpers do function starts individual indexing tasks for each new media item. I think it does make sense grammatically, but I could see how that's confusing. - Returns :ok + Returns [binary()] where each binary is the media ID of a new media item. """ def kickoff_indexing_tasks_from_youtube_rss_feed(%Source{} = source) do {:ok, media_ids} = YoutubeRss.get_recent_media_ids_from_rss(source) @@ -37,6 +37,8 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpers do MediaIndexingWorker.kickoff_with_task(source, url) end) + + new_media_ids end @doc """ diff --git a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex index 1e0f700..15d287f 100644 --- a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex +++ b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex @@ -11,8 +11,10 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do alias __MODULE__ alias Pinchflat.Tasks alias Pinchflat.Sources + alias Pinchflat.Settings alias Pinchflat.Sources.Source alias Pinchflat.FastIndexing.FastIndexingHelpers + alias Pinchflat.Notifications.SourceNotifications @doc """ Starts the source fast indexing worker and creates a task for the source. @@ -37,8 +39,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do source = Sources.get_source!(source_id) if source.fast_index do - FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) - + perform_indexing_and_notification(source) reschedule_indexing(source) else :ok @@ -48,6 +49,13 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end + defp perform_indexing_and_notification(source) do + apprise_server = Settings.get!(:apprise_server) + new_media_items = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) + + SourceNotifications.send_new_media_notification(apprise_server, source, length(new_media_items)) + end + defp reschedule_indexing(source) do next_run_in = Source.fast_index_frequency() * 60 diff --git a/lib/pinchflat/notifications/apprise_command_runner.ex b/lib/pinchflat/notifications/apprise_command_runner.ex new file mode 100644 index 0000000..13ad327 --- /dev/null +++ b/lib/pinchflat/notifications/apprise_command_runner.ex @@ -0,0 +1,12 @@ +defmodule Pinchflat.Notifications.AppriseCommandRunner do + @moduledoc """ + A behaviour for running CLI commands against a notification backend (apprise). + + Used so we can implement Mox for testing without actually running the + apprise command. + """ + + @callback run(binary(), keyword()) :: :ok | {:error, binary()} + @callback run(List.t(), keyword()) :: :ok | {:error, binary()} + @callback version() :: {:ok, binary()} | {:error, binary()} +end diff --git a/lib/pinchflat/notifications/command_runner.ex b/lib/pinchflat/notifications/command_runner.ex new file mode 100644 index 0000000..8f1a806 --- /dev/null +++ b/lib/pinchflat/notifications/command_runner.ex @@ -0,0 +1,65 @@ +defmodule Pinchflat.Notifications.CommandRunner do + @moduledoc """ + Runs apprise commands using the `System.cmd/3` function + """ + + require Logger + + alias Pinchflat.Utils.CliUtils + alias Pinchflat.Utils.FunctionUtils + alias Pinchflat.Notifications.AppriseCommandRunner + + @behaviour AppriseCommandRunner + + @doc """ + Runs an apprise command and returns the string output. + Can take a single server string or a list of servers as well as additional + arguments to pass to the command. + + Returns {:ok, binary()} | {:error, :no_servers} | {:error, binary()} + """ + @impl AppriseCommandRunner + def run(nil, _), do: {:error, :no_servers} + def run("", _), do: {:error, :no_servers} + def run([], _), do: {:error, :no_servers} + + def run(endpoints, command_opts) do + endpoints = List.wrap(endpoints) + default_opts = [:verbose] + parsed_opts = CliUtils.parse_options(default_opts ++ command_opts) + + Logger.info("[apprise] called with: #{Enum.join(parsed_opts ++ endpoints, " ")}") + {output, return_code} = System.cmd(backend_executable(), parsed_opts ++ endpoints) + Logger.info("[apprise] response: #{output}") + + case return_code do + 0 -> {:ok, String.trim(output)} + _ -> {:error, String.trim(output)} + end + end + + @doc """ + Returns the version of apprise as a string. + + Returns {:ok, binary()} | {:error, binary()} + """ + @impl AppriseCommandRunner + def version do + case System.cmd(backend_executable(), ["--version"]) do + {output, 0} -> + output + |> String.split(~r{\r?\n}) + |> List.first() + |> String.replace("Apprise", "") + |> String.trim() + |> FunctionUtils.wrap_ok() + + {output, _} -> + {:error, output} + end + end + + defp backend_executable do + Application.get_env(:pinchflat, :apprise_executable) + end +end diff --git a/lib/pinchflat/notifications/source_notifications.ex b/lib/pinchflat/notifications/source_notifications.ex new file mode 100644 index 0000000..e39841d --- /dev/null +++ b/lib/pinchflat/notifications/source_notifications.ex @@ -0,0 +1,77 @@ +defmodule Pinchflat.Notifications.SourceNotifications do + @moduledoc """ + Contains utilities for sending notifications about sources + """ + + require Logger + + alias Pinchflat.Repo + alias Pinchflat.Media.MediaQuery + + @doc """ + Wraps a function that may change the number of pending or downloaded + media items for a source, sending an apprise notification if + the count changes. + + Returns the return value of the provided function + """ + def wrap_new_media_notification(servers, source, func) do + before_count = relevant_media_item_count(source) + retval = func.() + after_count = relevant_media_item_count(source) + + send_new_media_notification(servers, source, after_count - before_count) + + retval + end + + @doc """ + Sends a notification if the count of new media items has changed + + Returns :ok + """ + def send_new_media_notification(_, _, count) when count <= 0, do: :ok + + def send_new_media_notification(servers, source, changed_count) do + opts = [ + title: "[Pinchflat] New media found", + body: "Found #{changed_count} new media item(s) for #{source.custom_name}. Downloading them now" + ] + + case backend_runner().run(servers, opts) do + {:ok, _} -> + Logger.info("Sent new media notification for source #{source.id}") + + {:error, :no_servers} -> + Logger.info("No notification servers provided for source #{source.id}") + + {:error, err} -> + Logger.error("Failed to send new media notification for source #{source.id}: #{err}") + end + + :ok + end + + defp relevant_media_item_count(source) do + pending_media_item_count(source) + downloaded_media_item_count(source) + end + + defp pending_media_item_count(source) do + MediaQuery.new() + |> MediaQuery.for_source(source) + |> MediaQuery.with_media_pending_download() + |> Repo.aggregate(:count) + end + + defp downloaded_media_item_count(source) do + MediaQuery.new() + |> MediaQuery.for_source(source) + |> MediaQuery.with_media_filepath() + |> Repo.aggregate(:count) + end + + defp backend_runner do + # This approach lets us mock the command for testing + Application.get_env(:pinchflat, :apprise_runner) + end +end diff --git a/lib/pinchflat/settings/setting.ex b/lib/pinchflat/settings/setting.ex index 5fb7cf6..f9eb386 100644 --- a/lib/pinchflat/settings/setting.ex +++ b/lib/pinchflat/settings/setting.ex @@ -9,7 +9,9 @@ defmodule Pinchflat.Settings.Setting do @allowed_fields [ :onboarding, :pro_enabled, - :yt_dlp_version + :yt_dlp_version, + :apprise_version, + :apprise_server ] @required_fields ~w( @@ -21,6 +23,8 @@ defmodule Pinchflat.Settings.Setting do field :onboarding, :boolean, default: true field :pro_enabled, :boolean, default: false field :yt_dlp_version, :string + field :apprise_version, :string + field :apprise_server, :string end @doc false diff --git a/lib/pinchflat/settings/settings.ex b/lib/pinchflat/settings/settings.ex index 4b133c8..25a8d68 100644 --- a/lib/pinchflat/settings/settings.ex +++ b/lib/pinchflat/settings/settings.ex @@ -20,6 +20,17 @@ defmodule Pinchflat.Settings do |> Repo.one() end + @doc """ + Updates the setting record. + + Returns {:ok, %Setting{}} | {:error, %Ecto.Changeset{}} + """ + def update_setting(%Setting{} = setting, attrs) do + setting + |> Setting.changeset(attrs) + |> Repo.update() + end + @doc """ Updates a setting, returning the new value. Is setup to take a keyword list argument so you @@ -29,8 +40,7 @@ defmodule Pinchflat.Settings do """ def set([{attr, value}]) do record() - |> Setting.changeset(%{attr => value}) - |> Repo.update() + |> update_setting(%{attr => value}) |> case do {:ok, %{^attr => _}} -> {:ok, value} {:ok, _} -> {:error, :invalid_key} @@ -61,4 +71,11 @@ defmodule Pinchflat.Settings do {:error, _} -> raise "Setting `#{name}` not found" end end + + @doc """ + Returns `%Ecto.Changeset{}` + """ + def change_setting(%Setting{} = setting, attrs \\ %{}) do + Setting.changeset(setting, attrs) + end end diff --git a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex index 70f9c2e..0e982eb 100644 --- a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex +++ b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex @@ -11,9 +11,11 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do alias __MODULE__ alias Pinchflat.Tasks alias Pinchflat.Sources + alias Pinchflat.Settings alias Pinchflat.Sources.Source alias Pinchflat.FastIndexing.FastIndexingWorker alias Pinchflat.SlowIndexing.SlowIndexingHelpers + alias Pinchflat.Notifications.SourceNotifications @doc """ Starts the source slow indexing worker and creates a task for the source. @@ -78,21 +80,21 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do case {source.index_frequency_minutes, source.last_indexed_at} do {index_freq, _} when index_freq > 0 -> # If the indexing is on a schedule simply run indexing and reschedule - SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + perform_indexing_and_notification(source) maybe_enqueue_fast_indexing_task(source) reschedule_indexing(source) {_, nil} -> # If the source has never been indexed, index it once # even if it's not meant to reschedule - SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + perform_indexing_and_notification(source) :ok _ -> # If the source HAS been indexed and is not meant to reschedule, # perform a no-op (unless forced) if args["force"] do - SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + perform_indexing_and_notification(source) end :ok @@ -102,6 +104,14 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end + defp perform_indexing_and_notification(source) do + apprise_server = Settings.get!(:apprise_server) + + SourceNotifications.wrap_new_media_notification(apprise_server, source, fn -> + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + end) + end + defp reschedule_indexing(source) do next_run_in = source.index_frequency_minutes * 60 diff --git a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex index c95306c..b6b118a 100644 --- a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex +++ b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex @@ -60,7 +60,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do def index_and_enqueue_download_for_media_items(%Source{} = source) do # See the method definition below for more info on how file watchers work # (important reading if you're not familiar with it) - {:ok, media_attributes} = get_media_attributes_for_collection_and_setup_file_watcher(source) + {:ok, media_attributes} = setup_file_watcher_and_kickoff_indexing(source) # Reload because the source may have been updated during the (long-running) indexing process # and important settings like `download_media` may have changed. source = Repo.reload!(source) @@ -84,15 +84,15 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do # lines (ie: you should gracefully fail if you can't parse a line). # # This works in-tandem with the normal (blocking) media indexing behaviour. When - # the `get_media_attributes_for_collection` method completes it'll return the FULL result to - # the caller for parsing. Ideally, every item in the list will have already + # the `setup_file_watcher_and_kickoff_indexing` method completes it'll return the + # FULL result to the caller for parsing. Ideally, every item in the list will have already # been processed by the file follower, but if not, the caller handles creation # of any media items that were missed/initially failed. # # It attempts a graceful shutdown of the file follower after the indexing is done, # but the FileFollowerServer will also stop itself if it doesn't see any activity # for a sufficiently long time. - defp get_media_attributes_for_collection_and_setup_file_watcher(source) do + defp setup_file_watcher_and_kickoff_indexing(source) do {:ok, pid} = FileFollowerServer.start_link() handler = fn filepath -> setup_file_follower_watcher(pid, filepath, source) end diff --git a/lib/pinchflat/utils/cli_utils.ex b/lib/pinchflat/utils/cli_utils.ex new file mode 100644 index 0000000..4e2a488 --- /dev/null +++ b/lib/pinchflat/utils/cli_utils.ex @@ -0,0 +1,48 @@ +defmodule Pinchflat.Utils.CliUtils do + @moduledoc """ + Utility methods for working with CLI executables + """ + + alias Pinchflat.Utils.StringUtils + + @doc """ + Parses a list of command options into a list of strings suitable for passing to + `System.cmd/3`. + + We want to satisfy the following behaviours: + 1. If the key is an atom, convert it to a string and convert it to kebab case (for convenience) + 2. If the key is a string, assume we want it as-is and don't convert it + 3. If the key is accompanied by a value, append the value to the list + 4. If the key is not accompanied by a value, assume it's a flag and PREpend it to the list + + Returns [binary()] + """ + def parse_options(command_opts) do + command_opts + |> List.wrap() + |> Enum.reduce([], &parse_option/2) + end + + defp parse_option({k, v}, acc) when is_atom(k) do + stringified_key = StringUtils.to_kebab_case(Atom.to_string(k)) + + parse_option({"--#{stringified_key}", v}, acc) + end + + defp parse_option({k, v}, acc) when is_binary(k) do + acc ++ [k, to_string(v)] + end + + defp parse_option(arg, acc) when is_atom(arg) do + stringified_arg = + arg + |> Atom.to_string() + |> StringUtils.to_kebab_case() + + parse_option("--#{stringified_arg}", acc) + end + + defp parse_option(arg, acc) when is_binary(arg) do + acc ++ [arg] + end +end diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index 537dbd6..7fc19bf 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -5,11 +5,11 @@ defmodule Pinchflat.YtDlp.CommandRunner do require Logger - alias Pinchflat.Utils.StringUtils + alias Pinchflat.Utils.CliUtils + alias Pinchflat.YtDlp.YtDlpCommandRunner alias Pinchflat.Filesystem.FilesystemHelpers, as: FSUtils - alias Pinchflat.YtDlp.BackendCommandRunner - @behaviour BackendCommandRunner + @behaviour YtDlpCommandRunner @doc """ Runs a yt-dlp command and returns the string output. Saves the output to @@ -23,7 +23,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do Returns {:ok, binary()} | {:error, output, status}. """ - @impl BackendCommandRunner + @impl YtDlpCommandRunner def run(url, command_opts, output_template, addl_opts \\ []) do # This approach lets us mock the command for testing command = backend_executable() @@ -32,7 +32,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do 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) + formatted_command_opts = [url] ++ CliUtils.parse_options(command_opts ++ print_to_file_opts ++ cookie_opts) Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}") @@ -48,7 +48,12 @@ defmodule Pinchflat.YtDlp.CommandRunner do end end - @impl BackendCommandRunner + @doc """ + Returns the version of yt-dlp as a string + + Returns {:ok, binary()} | {:error, binary()} + """ + @impl YtDlpCommandRunner def version do command = backend_executable() @@ -81,36 +86,6 @@ defmodule Pinchflat.YtDlp.CommandRunner do end end - # We want to satisfy the following behaviours: - # - # 1. If the key is an atom, convert it to a string and convert it to kebab case (for convenience) - # 2. If the key is a string, assume we want it as-is and don't convert it - # 3. If the key is accompanied by a value, append the value to the list - # 4. If the key is not accompanied by a value, assume it's a flag and PREpend it to the list - defp parse_options(command_opts) do - Enum.reduce(command_opts, [], &parse_option/2) - end - - defp parse_option({k, v}, acc) when is_atom(k) do - stringified_key = StringUtils.to_kebab_case(Atom.to_string(k)) - - parse_option({"--#{stringified_key}", v}, acc) - end - - defp parse_option({k, v}, acc) when is_binary(k) do - acc ++ [k, to_string(v)] - end - - defp parse_option(arg, acc) when is_atom(arg) do - stringified_arg = StringUtils.to_kebab_case(Atom.to_string(arg)) - - parse_option("--#{stringified_arg}", acc) - end - - defp parse_option(arg, acc) when is_binary(arg) do - acc ++ [arg] - end - defp backend_executable do Application.get_env(:pinchflat, :yt_dlp_executable) end diff --git a/lib/pinchflat/yt_dlp/backend_command_runner.ex b/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex similarity index 90% rename from lib/pinchflat/yt_dlp/backend_command_runner.ex rename to lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex index e09eaeb..9b46a32 100644 --- a/lib/pinchflat/yt_dlp/backend_command_runner.ex +++ b/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex @@ -1,4 +1,4 @@ -defmodule Pinchflat.YtDlp.BackendCommandRunner do +defmodule Pinchflat.YtDlp.YtDlpCommandRunner do @moduledoc """ A behaviour for running CLI commands against a downloader backend (yt-dlp). diff --git a/lib/pinchflat_web/components/core_components.ex b/lib/pinchflat_web/components/core_components.ex index f124b16..654b743 100644 --- a/lib/pinchflat_web/components/core_components.ex +++ b/lib/pinchflat_web/components/core_components.ex @@ -247,6 +247,7 @@ defmodule PinchflatWeb.CoreComponents do attr :label_suffix, :string, default: nil attr :value, :any attr :help, :string, default: nil + attr :html_help, :boolean, default: false attr :type, :string, default: "text", @@ -298,7 +299,7 @@ defmodule PinchflatWeb.CoreComponents do <%= @label %> <%= @label_suffix %> - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> """ @@ -325,7 +326,7 @@ defmodule PinchflatWeb.CoreComponents do - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> """ @@ -356,7 +357,7 @@ defmodule PinchflatWeb.CoreComponents do > - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> @@ -387,7 +388,7 @@ defmodule PinchflatWeb.CoreComponents do <%= render_slot(@inner_block) %> - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> """ @@ -411,7 +412,7 @@ defmodule PinchflatWeb.CoreComponents do ]} {@rest} ><%= Phoenix.HTML.Form.normalize_value("textarea", @value) %> - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> """ @@ -438,7 +439,7 @@ defmodule PinchflatWeb.CoreComponents do ]} {@rest} /> - <.help :if={@help}><%= @help %> + <.help :if={@help}><%= if @html_help, do: Phoenix.HTML.raw(@help), else: @help %> <.error :for={msg <- @errors}><%= msg %> """ diff --git a/lib/pinchflat_web/components/layouts/partials/sidebar.html.heex b/lib/pinchflat_web/components/layouts/partials/sidebar.html.heex index 978c471..deae097 100644 --- a/lib/pinchflat_web/components/layouts/partials/sidebar.html.heex +++ b/lib/pinchflat_web/components/layouts/partials/sidebar.html.heex @@ -24,6 +24,7 @@ <.sidebar_item icon="hero-home" text="Home" href={~p"/"} /> <.sidebar_item icon="hero-tv" text="Sources" href={~p"/sources"} /> <.sidebar_item icon="hero-adjustments-vertical" text="Media Profiles" href={~p"/media_profiles"} /> + <.sidebar_item icon="hero-cog-6-tooth" text="Settings" href={~p"/settings"} /> @@ -38,12 +39,7 @@ target="_blank" href="https://github.com/kieraneglin/pinchflat/wiki" /> - <.sidebar_item - icon="hero-code-bracket" - text="Github" - target="_blank" - href="https://github.com/kieraneglin/pinchflat" - /> + <.sidebar_item icon="hero-cog" text="Github" target="_blank" href="https://github.com/kieraneglin/pinchflat" />
  • setting_params}) do + setting = Settings.record() + + case Settings.update_setting(setting, setting_params) do + {:ok, _} -> + conn + |> put_flash(:info, "Settings updated successfully.") + |> redirect(to: ~p"/settings") + + {:error, %Ecto.Changeset{} = changeset} -> + render(conn, "show.html", changeset: changeset) + end + end +end diff --git a/lib/pinchflat_web/controllers/settings/setting_html.ex b/lib/pinchflat_web/controllers/settings/setting_html.ex new file mode 100644 index 0000000..47ad97c --- /dev/null +++ b/lib/pinchflat_web/controllers/settings/setting_html.ex @@ -0,0 +1,20 @@ +defmodule PinchflatWeb.Settings.SettingHTML do + use PinchflatWeb, :html + + embed_templates "setting_html/*" + + @doc """ + Renders a setting form. + """ + attr :changeset, Ecto.Changeset, required: true + attr :action, :string, required: true + + def setting_form(assigns) + + def apprise_server_help do + url = "https://github.com/caronc/apprise/wiki/URLBasics" + classes = "underline decoration-bodydark decoration-1 hover:decoration-white" + + ~s(Server endpoint for Apprise notifications when new media is found. See Apprise docs for more information) + end +end diff --git a/lib/pinchflat_web/controllers/settings/setting_html/setting_form.html.heex b/lib/pinchflat_web/controllers/settings/setting_html/setting_form.html.heex new file mode 100644 index 0000000..f3ba48e --- /dev/null +++ b/lib/pinchflat_web/controllers/settings/setting_html/setting_form.html.heex @@ -0,0 +1,21 @@ +<.simple_form :let={f} for={@changeset} action={@action}> + <.error :if={@changeset.action}> + Oops, something went wrong! Please check the errors below. + + +

    + Notification Settings +

    + + <.input + field={f[:apprise_server]} + type="text" + label="Apprise Server" + help={apprise_server_help()} + html_help={true} + inputclass="font-mono text-sm" + placeholder="https://discordapp.com/api/webhooks/{WebhookID}/{WebhookToken}" + /> + + <.button class="my-10 sm:mb-7.5 w-full sm:w-auto" rounding="rounded-lg">Save Settings + diff --git a/lib/pinchflat_web/controllers/settings/setting_html/show.html.heex b/lib/pinchflat_web/controllers/settings/setting_html/show.html.heex new file mode 100644 index 0000000..db90e00 --- /dev/null +++ b/lib/pinchflat_web/controllers/settings/setting_html/show.html.heex @@ -0,0 +1,12 @@ +
    +
    +

    + Settings +

    +
    +
    +
    +
    + <.setting_form changeset={@changeset} action={~p"/settings"} /> +
    +
    diff --git a/lib/pinchflat_web/router.ex b/lib/pinchflat_web/router.ex index b83e6d8..04edebe 100644 --- a/lib/pinchflat_web/router.ex +++ b/lib/pinchflat_web/router.ex @@ -30,6 +30,7 @@ defmodule PinchflatWeb.Router do resources "/media_profiles", MediaProfiles.MediaProfileController resources "/search", Searches.SearchController, only: [:show], singleton: true + resources "/settings", Settings.SettingController, only: [:show, :update], singleton: true resources "/sources", Sources.SourceController do post "/force_download", Sources.SourceController, :force_download diff --git a/priv/repo/migrations/20240404221551_add_apprise_servers_to_settings.exs b/priv/repo/migrations/20240404221551_add_apprise_servers_to_settings.exs new file mode 100644 index 0000000..bba04c6 --- /dev/null +++ b/priv/repo/migrations/20240404221551_add_apprise_servers_to_settings.exs @@ -0,0 +1,9 @@ +defmodule Pinchflat.Repo.Migrations.AddAppriseServersToSettings do + use Ecto.Migration + + def change do + alter table(:settings) do + add :apprise_server, :string + end + end +end diff --git a/priv/repo/migrations/20240408181121_add_apprise_version_to_settings.exs b/priv/repo/migrations/20240408181121_add_apprise_version_to_settings.exs new file mode 100644 index 0000000..2a624b4 --- /dev/null +++ b/priv/repo/migrations/20240408181121_add_apprise_version_to_settings.exs @@ -0,0 +1,9 @@ +defmodule Pinchflat.Repo.Migrations.AddAppriseVersionToSettings do + use Ecto.Migration + + def change do + alter table(:settings) do + add :apprise_version, :string + end + end +end diff --git a/selfhosted.Dockerfile b/selfhosted.Dockerfile index 0dfc70a..62b2c70 100644 --- a/selfhosted.Dockerfile +++ b/selfhosted.Dockerfile @@ -79,7 +79,7 @@ ARG PORT=8945 RUN apt-get update -y RUN apt-get install -y libstdc++6 openssl libncurses5 locales ca-certificates \ - ffmpeg curl git openssh-client nano + ffmpeg curl git openssh-client nano python3 python3-pip RUN apt-get clean && rm -f /var/lib/apt/lists/*_* # Download and update YT-DLP @@ -87,9 +87,11 @@ RUN curl -L https://github.com/yt-dlp/yt-dlp/releases/latest/download/yt-dlp -o RUN chmod a+rx /usr/local/bin/yt-dlp RUN yt-dlp -U +# Download Apprise +RUN python3 -m pip install -U apprise --break-system-packages + # Set the locale RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && locale-gen - ENV LANG en_US.UTF-8 ENV LANGUAGE en_US:en ENV LC_ALL en_US.UTF-8 diff --git a/test/pinchflat/boot/pre_job_startup_tasks_test.exs b/test/pinchflat/boot/pre_job_startup_tasks_test.exs index f2cdd66..8fd81bb 100644 --- a/test/pinchflat/boot/pre_job_startup_tasks_test.exs +++ b/test/pinchflat/boot/pre_job_startup_tasks_test.exs @@ -1,11 +1,19 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do use Pinchflat.DataCase + import Mox import Pinchflat.JobFixtures alias Pinchflat.Settings alias Pinchflat.Boot.PreJobStartupTasks + setup do + stub(YtDlpRunnerMock, :version, fn -> {:ok, "1"} end) + stub(AppriseRunnerMock, :version, fn -> {:ok, "2"} end) + + :ok + end + describe "reset_executing_jobs" do test "resets executing jobs" do job = job_fixture() @@ -13,7 +21,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do assert Repo.reload!(job).state == "executing" - PreJobStartupTasks.start_link() + PreJobStartupTasks.init(%{}) assert Repo.reload!(job).state == "retryable" end @@ -27,21 +35,31 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do refute File.exists?(filepath) - PreJobStartupTasks.start_link() + PreJobStartupTasks.init(%{}) assert File.exists?(filepath) end end describe "apply_default_settings" do - test "sets default settings" do + test "sets yt_dlp version" do Settings.set(yt_dlp_version: nil) refute Settings.get!(:yt_dlp_version) - PreJobStartupTasks.start_link() + PreJobStartupTasks.init(%{}) assert Settings.get!(:yt_dlp_version) end + + test "sets apprise version" do + Settings.set(apprise_version: nil) + + refute Settings.get!(:apprise_version) + + PreJobStartupTasks.init(%{}) + + assert Settings.get!(:apprise_version) + end end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs index 6c55051..23350c0 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs @@ -24,7 +24,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do test "enqueues a new worker for each new media_id in the source's RSS feed", %{source: source} do expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) - assert :ok = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) + assert [_] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) assert [worker] = all_enqueued(worker: MediaIndexingWorker) assert worker.args["id"] == source.id @@ -35,10 +35,16 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) media_item_fixture(source_id: source.id, media_id: "test_1") - assert :ok = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) + assert [] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) refute_enqueued(worker: MediaIndexingWorker) end + + test "returns the IDs of the found media items", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + assert ["test_1"] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) + end end describe "index_and_enqueue_download_for_media_item/2" do diff --git a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs index a69a99d..55007ba 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs @@ -4,6 +4,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do import Mox import Pinchflat.SourcesFixtures + alias Pinchflat.Settings alias Pinchflat.Sources.Source alias Pinchflat.FastIndexing.FastIndexingWorker @@ -74,4 +75,28 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do assert :ok = perform_job(FastIndexingWorker, %{id: 0}) end end + + describe "perform/1 when testing notifications" do + setup do + Settings.set(apprise_server: "server_1") + + :ok + end + + test "sends a notification if new media was found" do + source = source_fixture(fast_index: true) + + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + expect(AppriseRunnerMock, :run, fn servers, opts -> + assert "server_1" = servers + assert is_binary(Keyword.get(opts, :title)) + assert is_binary(Keyword.get(opts, :body)) + + {:ok, ""} + end) + + perform_job(FastIndexingWorker, %{id: source.id}) + end + end end diff --git a/test/pinchflat/notifications/command_runner_test.exs b/test/pinchflat/notifications/command_runner_test.exs new file mode 100644 index 0000000..0ed4cc6 --- /dev/null +++ b/test/pinchflat/notifications/command_runner_test.exs @@ -0,0 +1,65 @@ +defmodule Pinchflat.Notifications.CommandRunnerTest do + use ExUnit.Case, async: true + + alias Pinchflat.Notifications.CommandRunner, as: Runner + + @original_executable Application.compile_env(:pinchflat, :apprise_executable) + + setup do + on_exit(&reset_executable/0) + end + + describe "run/2" do + test "returns :ok when the command succeeds" do + assert {:ok, _} = Runner.run("server_1", []) + end + + test "includes the servers as the first argument" do + assert {:ok, output} = Runner.run(["server_1", "server_2"], []) + + assert String.contains?(output, "server_1 server_2") + end + + test "lets you pass a single server as a string" do + assert {:ok, output} = Runner.run("server_1", []) + + assert String.contains?(output, "server_1") + end + + test "passes all arguments to the command" do + assert {:ok, output} = Runner.run("server_1", ["--dry-run"]) + + assert String.contains?(output, "--dry-run") + end + + test "returns the output when the command fails" do + wrap_executable("/bin/false", fn -> + assert {:error, ""} = Runner.run("server_1", []) + end) + end + + test "returns a relevant error if no servers are provided" do + assert {:error, :no_servers} = Runner.run(nil, []) + assert {:error, :no_servers} = Runner.run("", []) + assert {:error, :no_servers} = Runner.run([], []) + end + end + + describe "version/0" do + test "adds the version arg" do + assert {:ok, output} = Runner.version() + + assert String.contains?(output, "--version") + end + end + + defp wrap_executable(new_executable, fun) do + Application.put_env(:pinchflat, :apprise_executable, new_executable) + fun.() + reset_executable() + end + + def reset_executable do + Application.put_env(:pinchflat, :apprise_executable, @original_executable) + end +end diff --git a/test/pinchflat/notifications/source_notifications_test.exs b/test/pinchflat/notifications/source_notifications_test.exs new file mode 100644 index 0000000..0bf897e --- /dev/null +++ b/test/pinchflat/notifications/source_notifications_test.exs @@ -0,0 +1,100 @@ +defmodule Pinchflat.Notifications.SourceNotificationsTest do + use Pinchflat.DataCase + + import Mox + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + + alias Pinchflat.Notifications.SourceNotifications + + @apprise_servers ["server_1", "server_2"] + + setup :verify_on_exit! + + describe "wrap_new_media_notification/3" do + test "sends a notification when the pending count changes" do + source = source_fixture() + + expect(AppriseRunnerMock, :run, fn servers, opts -> + assert servers == @apprise_servers + + assert opts == [ + title: "[Pinchflat] New media found", + body: "Found 1 new media item(s) for #{source.custom_name}. Downloading them now" + ] + + {:ok, ""} + end) + + SourceNotifications.wrap_new_media_notification(@apprise_servers, source, fn -> + media_item_fixture(%{source_id: source.id, media_filepath: nil}) + end) + end + + test "sends a notification when the downloaded count changes" do + source = source_fixture() + + expect(AppriseRunnerMock, :run, fn servers, opts -> + assert servers == @apprise_servers + + assert opts == [ + title: "[Pinchflat] New media found", + body: "Found 1 new media item(s) for #{source.custom_name}. Downloading them now" + ] + + {:ok, ""} + end) + + SourceNotifications.wrap_new_media_notification(@apprise_servers, source, fn -> + media_item_fixture(%{source_id: source.id, media_filepath: "file.mp4"}) + end) + end + + test "does not send a notification when the count does not change" do + source = source_fixture() + + expect(AppriseRunnerMock, :run, 0, fn _, _ -> {:ok, ""} end) + + SourceNotifications.wrap_new_media_notification(@apprise_servers, source, fn -> + media_item_fixture(%{source_id: source.id, prevent_download: true, media_filepath: nil}) + end) + end + + test "returns the value of the function" do + source = source_fixture() + expect(AppriseRunnerMock, :run, 0, fn _, _ -> {:ok, ""} end) + + retval = SourceNotifications.wrap_new_media_notification(@apprise_servers, source, fn -> "value" end) + + assert retval == "value" + end + end + + describe "send_new_media_notification/3" do + test "sends a notification when count is positive" do + source = source_fixture() + + expect(AppriseRunnerMock, :run, fn servers, opts -> + assert servers == @apprise_servers + + assert opts == [ + title: "[Pinchflat] New media found", + body: "Found 1 new media item(s) for #{source.custom_name}. Downloading them now" + ] + + {:ok, ""} + end) + + :ok = SourceNotifications.send_new_media_notification(@apprise_servers, source, 1) + end + + test "does not send a notification when count not positive" do + source = source_fixture() + + expect(AppriseRunnerMock, :run, 0, fn _, _ -> {:ok, ""} end) + + :ok = SourceNotifications.send_new_media_notification(@apprise_servers, source, 0) + :ok = SourceNotifications.send_new_media_notification(@apprise_servers, source, -1) + end + end +end diff --git a/test/pinchflat/settings_test.exs b/test/pinchflat/settings_test.exs index bcfb97f..944eaf1 100644 --- a/test/pinchflat/settings_test.exs +++ b/test/pinchflat/settings_test.exs @@ -24,6 +24,16 @@ defmodule Pinchflat.SettingsTest do end end + describe "update_setting/2" do + test "updates the setting" do + setting = Settings.record() + + assert {:ok, false} = Settings.get(:onboarding) + assert {:ok, %Setting{}} = Settings.update_setting(setting, %{onboarding: true}) + assert {:ok, true} = Settings.get(:onboarding) + end + end + describe "set/1" do test "updates the setting" do assert {:ok, true} = Settings.set(onboarding: true) @@ -60,4 +70,12 @@ defmodule Pinchflat.SettingsTest do end end end + + describe "change_setting/2" do + test "returns a changeset" do + setting = Settings.record() + + assert %Ecto.Changeset{} = Settings.change_setting(setting, %{onboarding: true}) + end + end end diff --git a/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs b/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs index 6a3c140..0830435 100644 --- a/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs +++ b/test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs @@ -7,6 +7,7 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do import Pinchflat.SourcesFixtures alias Pinchflat.Tasks + alias Pinchflat.Settings alias Pinchflat.Sources.Source alias Pinchflat.FastIndexing.FastIndexingWorker alias Pinchflat.Downloading.MediaDownloadWorker @@ -51,6 +52,12 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do end describe "perform/1" do + setup do + stub(AppriseRunnerMock, :run, fn _, _ -> {:ok, ""} end) + + :ok + end + test "it indexes the source if it should be indexed" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> {:ok, ""} end) @@ -210,4 +217,30 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do assert :ok = perform_job(MediaCollectionIndexingWorker, %{id: 0}) end end + + describe "perform/1 when testing apprise notifications" do + setup do + Settings.set(apprise_server: "server_1") + + :ok + end + + test "sends a notification if new media was found" do + source = source_fixture() + + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> + {:ok, source_attributes_return_fixture()} + end) + + expect(AppriseRunnerMock, :run, fn servers, opts -> + assert "server_1" = servers + assert is_binary(Keyword.get(opts, :title)) + assert is_binary(Keyword.get(opts, :body)) + + {:ok, ""} + end) + + perform_job(MediaCollectionIndexingWorker, %{id: source.id}) + end + end end diff --git a/test/pinchflat/utils/cli_utils_test.exs b/test/pinchflat/utils/cli_utils_test.exs new file mode 100644 index 0000000..b053158 --- /dev/null +++ b/test/pinchflat/utils/cli_utils_test.exs @@ -0,0 +1,23 @@ +defmodule Pinchflat.Utils.CliUtilsTest do + use ExUnit.Case, async: true + + alias Pinchflat.Utils.CliUtils + + describe "parse_options/1" do + test "it converts symbol k-v arg keys to kebab case" do + assert ["--buffer-size", "1024"] = CliUtils.parse_options(buffer_size: 1024) + end + + test "it keeps string k-v arg keys untouched" do + assert ["--under_score", "1024"] = CliUtils.parse_options({"--under_score", 1024}) + end + + test "it converts symbol arg keys to kebab case" do + assert ["--ignore-errors"] = CliUtils.parse_options(:ignore_errors) + end + + test "it keeps string arg keys untouched" do + assert ["-v"] = CliUtils.parse_options("-v") + end + end +end diff --git a/test/pinchflat/yt_dlp/command_runner_test.exs b/test/pinchflat/yt_dlp/command_runner_test.exs index 889b030..ac06a48 100644 --- a/test/pinchflat/yt_dlp/command_runner_test.exs +++ b/test/pinchflat/yt_dlp/command_runner_test.exs @@ -17,31 +17,6 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do assert {:ok, _output} = Runner.run(@media_url, [], "") end - test "it converts symbol k-v arg keys to kebab case" do - assert {:ok, output} = Runner.run(@media_url, [buffer_size: 1024], "") - - assert String.contains?(output, "--buffer-size 1024") - end - - test "it keeps string k-v arg keys untouched" do - assert {:ok, output} = Runner.run(@media_url, [{"--under_score", 1024}], "") - - assert String.contains?(output, "--under_score 1024") - end - - test "it converts symbol arg keys to kebab case" do - assert {:ok, output} = Runner.run(@media_url, [:ignore_errors], "") - - assert String.contains?(output, "--ignore-errors") - end - - test "it keeps string arg keys untouched" do - assert {:ok, output} = Runner.run(@media_url, ["-v"], "") - - assert String.contains?(output, "-v") - refute String.contains?(output, "--v") - end - test "it includes the media url as the first argument" do assert {:ok, output} = Runner.run(@media_url, [:ignore_errors], "") diff --git a/test/pinchflat_web/controllers/setting_controller_test.exs b/test/pinchflat_web/controllers/setting_controller_test.exs new file mode 100644 index 0000000..0063ebc --- /dev/null +++ b/test/pinchflat_web/controllers/setting_controller_test.exs @@ -0,0 +1,23 @@ +defmodule PinchflatWeb.SettingControllerTest do + use PinchflatWeb.ConnCase + + describe "show settings" do + test "renders the page", %{conn: conn} do + conn = get(conn, ~p"/settings") + + assert html_response(conn, 200) =~ "Settings" + end + end + + describe "update settings" do + test "saves and redirects when data is valid", %{conn: conn} do + update_attrs = %{apprise_server: "test://server"} + + conn = put(conn, ~p"/settings", setting: update_attrs) + assert redirected_to(conn) == ~p"/settings" + + conn = get(conn, ~p"/settings") + assert html_response(conn, 200) =~ update_attrs[:apprise_server] + end + end +end diff --git a/test/test_helper.exs b/test/test_helper.exs index bea0589..e6cd91e 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,6 +1,9 @@ -Mox.defmock(YtDlpRunnerMock, for: Pinchflat.YtDlp.BackendCommandRunner) +Mox.defmock(YtDlpRunnerMock, for: Pinchflat.YtDlp.YtDlpCommandRunner) Application.put_env(:pinchflat, :yt_dlp_runner, YtDlpRunnerMock) +Mox.defmock(AppriseRunnerMock, for: Pinchflat.Notifications.AppriseCommandRunner) +Application.put_env(:pinchflat, :apprise_runner, AppriseRunnerMock) + Mox.defmock(HTTPClientMock, for: Pinchflat.HTTP.HTTPBehaviour) Application.put_env(:pinchflat, :http_client, HTTPClientMock) From 318d6a7594efb23590a58aee1cc4f54bb11db294 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 9 Apr 2024 17:51:45 -0700 Subject: [PATCH 002/240] updated help text for fast indexing --- .../controllers/sources/source_html/source_form.html.heex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex index e94ebe6..a36e2d8 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex @@ -56,7 +56,7 @@ type="toggle" label="Use Fast Indexing" label_suffix="(pro)" - help="Experimental. Overrides 'Index Frequency'. Recommended for large channels that upload frequently. See below for more info" + help="Experimental. Overrides 'Index Frequency'. Recommended for large channels that upload frequently. Does not work with private playlists. See below for more info" x-init=" // `enabled` is the data attribute that the toggle uses internally fastIndexingEnabled = enabled From b0c2a3364482180019240ecb69ee5cd38245e130 Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 9 Apr 2024 18:24:07 -0700 Subject: [PATCH 003/240] Added more custom source attributes to output template (#172) --- lib/pinchflat/downloading/download_option_builder.ex | 2 ++ .../controllers/media_profiles/media_profile_html.ex | 3 +++ 2 files changed, 5 insertions(+) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 8c1af52..479e643 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -144,6 +144,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do defp output_options_map(source) do %{ "source_custom_name" => source.custom_name, + "source_collection_id" => source.collection_id, + "source_collection_name" => source.collection_name, "source_collection_type" => source.collection_type } end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex index 8f4a67a..6395214 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex @@ -59,6 +59,9 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileHTML do upload_year: nil, upload_yyyy_mm_dd: "the upload date in the format YYYY-MM-DD", source_custom_name: "the name of the sources that use this profile", + source_collection_id: "the YouTube ID of the sources that use this profile", + source_collection_name: + "the YouTube name of the sources that use this profile (often the same as source_custom_name)", source_collection_type: "the collection type of the sources using this profile. Either 'channel' or 'playlist'", artist_name: "the name of the artist with fallbacks to other uploader fields" } From cec9e3c7ff014535f7a487269ad8769df5e8b00f Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 9 Apr 2024 18:45:38 -0700 Subject: [PATCH 004/240] bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index e65a92b..434a0e0 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.9", + version: "0.1.10", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From a2bcd454c7f9315bbda9bd352494fd567d7dddc1 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 9 Apr 2024 19:07:49 -0700 Subject: [PATCH 005/240] Added apprise to runtime --- config/runtime.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/config/runtime.exs b/config/runtime.exs index a496f55..df022fd 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -57,6 +57,7 @@ if config_env() == :prod do config :pinchflat, yt_dlp_executable: System.find_executable("yt-dlp"), + apprise_executable: System.find_executable("apprise"), media_directory: "/downloads", metadata_directory: metadata_path, extras_directory: extras_path, From e841f39cf2e2447059cc2cc8396162cbbb6c3a9e Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 10 Apr 2024 17:54:45 -0700 Subject: [PATCH 006/240] [Enhancement] Redownload new media after a delay (#173) * Added redownload-related columns * Added methods for fetching re-downloadable media items * Filled out redownload worker + tests * Added redownload worker to config.exs cron * Added to UI and README --- README.md | 2 + config/config.exs | 3 +- .../downloading/media_download_worker.ex | 28 +++- .../downloading/media_redownload_worker.ex | 31 +++++ lib/pinchflat/media/media.ex | 34 ++++- lib/pinchflat/media/media_item.ex | 4 +- lib/pinchflat/media/media_query.ex | 50 +++++-- .../notifications/source_notifications.ex | 2 +- lib/pinchflat/profiles/media_profile.ex | 4 +- .../media_profile_form.html.heex | 11 ++ .../controllers/sources/source_controller.ex | 2 +- ...20240409224152_add_redownloaded_fields.exs | 13 ++ .../media_download_worker_test.exs | 26 +++- .../media_redownload_worker_test.exs | 44 ++++++ test/pinchflat/media_test.exs | 125 ++++++++++++++++++ 15 files changed, 349 insertions(+), 30 deletions(-) create mode 100644 lib/pinchflat/downloading/media_redownload_worker.ex create mode 100644 priv/repo/migrations/20240409224152_add_redownloaded_fields.exs create mode 100644 test/pinchflat/downloading/media_redownload_worker_test.exs diff --git a/README.md b/README.md index bdf24a5..0b0a10c 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,8 @@ If it doesn't work for your use case, please make a feature request! You can als - Supports downloading audio content - Custom rules for handling YouTube Shorts and livestreams - Apprise support for notifications +- Allows automatically redownloading new media after a set period + - This can help improve the download quality of new content or improve SponsorBlock tags - Optionally automatically delete old content ([docs](https://github.com/kieraneglin/pinchflat/wiki/Automatically-Delete-Media)) - Advanced options like setting cutoff dates and filtering by title - Reliable hands-off operation diff --git a/config/config.exs b/config/config.exs index b395950..c157fab 100644 --- a/config/config.exs +++ b/config/config.exs @@ -52,7 +52,8 @@ config :pinchflat, Oban, {Oban.Plugins.Pruner, max_age: 30 * 24 * 60 * 60}, {Oban.Plugins.Cron, crontab: [ - {"@daily", Pinchflat.Downloading.MediaRetentionWorker} + {"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker}, + {"0 2 * * *", Pinchflat.Downloading.MediaRedownloadWorker} ]} ], # TODO: consider making this an env var or something? diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index 17ab5b5..0e12067 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -35,14 +35,17 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do """ @impl Oban.Worker def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do + should_force = Map.get(args, "force", false) + is_redownload = Map.get(args, "redownload?", false) + media_item = media_item_id |> Media.get_media_item!() |> Repo.preload(:source) # If the source or media item is set to not download media, perform a no-op unless forced - if (media_item.source.download_media && !media_item.prevent_download) || args["force"] do - download_media_and_schedule_jobs(media_item) + if (media_item.source.download_media && !media_item.prevent_download) || should_force do + download_media_and_schedule_jobs(media_item, is_redownload) else :ok end @@ -51,10 +54,13 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} stale") end - defp download_media_and_schedule_jobs(media_item) do + defp download_media_and_schedule_jobs(media_item, is_redownload) do case MediaDownloader.download_for_media_item(media_item) do {:ok, updated_media_item} -> - compute_and_save_media_filesize(updated_media_item) + Media.update_media_item(updated_media_item, %{ + media_size_bytes: compute_media_filesize(updated_media_item), + media_redownloaded_at: get_redownloaded_at(is_redownload) + }) {:ok, updated_media_item} @@ -66,13 +72,21 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do end end - defp compute_and_save_media_filesize(media_item) do + defp compute_media_filesize(media_item) do case File.stat(media_item.media_filepath) do {:ok, %{size: size}} -> - Media.update_media_item(media_item, %{media_size_bytes: size}) + size _ -> - :ok + nil + end + end + + defp get_redownloaded_at(is_redownload) do + if is_redownload do + DateTime.utc_now() + else + nil end end end diff --git a/lib/pinchflat/downloading/media_redownload_worker.ex b/lib/pinchflat/downloading/media_redownload_worker.ex new file mode 100644 index 0000000..b3b6d82 --- /dev/null +++ b/lib/pinchflat/downloading/media_redownload_worker.ex @@ -0,0 +1,31 @@ +defmodule Pinchflat.Downloading.MediaRedownloadWorker do + @moduledoc false + + use Oban.Worker, + queue: :media_fetching, + unique: [period: :infinity, states: [:available, :scheduled, :retryable, :executing]], + tags: ["media_item", "media_fetching"] + + require Logger + + alias Pinchflat.Media + alias Pinchflat.Downloading.MediaDownloadWorker + + @doc """ + Redownloads media items that are eligible for redownload. + + This worker is scheduled to run daily via the Oban Cron plugin + and it should run _after_ the retention worker. + + Returns :ok + """ + @impl Oban.Worker + def perform(%Oban.Job{}) do + redownloadable_media = Media.list_redownloadable_media_items() + Logger.info("Redownloading #{length(redownloadable_media)} media items") + + Enum.each(redownloadable_media, fn media_item -> + MediaDownloadWorker.kickoff_with_task(media_item, %{redownload?: true}) + end) + end +end diff --git a/lib/pinchflat/media/media.ex b/lib/pinchflat/media/media.ex index ee8531d..d458af4 100644 --- a/lib/pinchflat/media/media.ex +++ b/lib/pinchflat/media/media.ex @@ -31,15 +31,39 @@ defmodule Pinchflat.Media do def list_cullable_media_items do MediaQuery.new() |> MediaQuery.with_media_filepath() - |> MediaQuery.with_passed_retention_period() - |> MediaQuery.with_no_culling_prevention() + |> MediaQuery.where_past_retention_period() + |> MediaQuery.where_culling_not_prevented() + |> Repo.all() + end + + @doc """ + Returns a list of media_items that are redownloadable based on the redownload delay + of the media_profile their source belongs to. + + The logic is that a media_item is past_redownload_delay if the media_item's + upload_date is at least redownload_delay_days ago AND + `media_downloaded_at` - `redownload_delay_days` is before the media_item's `upload_date`. + This logic grabs media that we've recently downloaded AND is recently uploaded, but + doesn't grab media that we've recently downloaded and was uploaded a long time ago. + This also makes things work as expected when downloading media from a source for the + first time. + + Returns [%MediaItem{}, ...] + """ + def list_redownloadable_media_items do + MediaQuery.new() + |> MediaQuery.with_media_downloaded_at() + |> MediaQuery.where_download_not_prevented() + |> 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 - matches satisfies `MediaQuery.with_media_pending_download`. 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. @@ -48,7 +72,7 @@ defmodule Pinchflat.Media do def list_pending_media_items_for(%Source{} = source) do MediaQuery.new() |> MediaQuery.for_source(source) - |> MediaQuery.with_media_pending_download() + |> MediaQuery.where_pending_download() |> Repo.all() end @@ -66,7 +90,7 @@ defmodule Pinchflat.Media do MediaQuery.new() |> MediaQuery.with_id(media_item.id) - |> MediaQuery.with_media_pending_download() + |> MediaQuery.where_pending_download() |> Repo.exists?() end diff --git a/lib/pinchflat/media/media_item.ex b/lib/pinchflat/media/media_item.ex index 52633b2..a671d58 100644 --- a/lib/pinchflat/media/media_item.ex +++ b/lib/pinchflat/media/media_item.ex @@ -34,7 +34,8 @@ defmodule Pinchflat.Media.MediaItem do # These are user or system controlled fields :prevent_download, :prevent_culling, - :culled_at + :culled_at, + :media_redownloaded_at ] # Pretty much all the fields captured at index are required. @required_fields ~w( @@ -61,6 +62,7 @@ defmodule Pinchflat.Media.MediaItem do field :livestream, :boolean, default: false field :short_form_content, :boolean, default: false field :media_downloaded_at, :utc_datetime + field :media_redownloaded_at, :utc_datetime field :upload_date, :date field :duration_seconds, :integer diff --git a/lib/pinchflat/media/media_query.ex b/lib/pinchflat/media/media_query.ex index 6751c28..024f745 100644 --- a/lib/pinchflat/media/media_query.ex +++ b/lib/pinchflat/media/media_query.ex @@ -15,7 +15,7 @@ defmodule Pinchflat.Media.MediaQuery do # Prefixes: # - for_* - belonging to a certain record # - join_* - for joining on a certain record - # - with_* - for filtering based on full, concrete attributes + # - with_*, where_* - for filtering based on full, concrete attributes # - matching_* - for filtering based on partial attributes (e.g. LIKE, regex, full-text search) # # Suffixes: @@ -33,24 +33,46 @@ defmodule Pinchflat.Media.MediaQuery do from(mi in query, join: s in assoc(mi, :source), as: :sources) end - def with_passed_retention_period(query) do + def where_past_retention_period(query) do query |> require_assoc(:source) |> where( [mi, source], - fragment( - "IFNULL(?, 0) > 0 AND DATETIME('now', '-' || ? || ' day') > ?", - source.retention_period_days, - source.retention_period_days, - mi.media_downloaded_at - ) + fragment(""" + IFNULL(retention_period_days, 0) > 0 AND + DATETIME('now', '-' || retention_period_days || ' day') > media_downloaded_at + """) ) end - def with_no_culling_prevention(query) do + def where_past_redownload_delay(query) do + query + |> require_assoc(:source) + |> 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(""" + IFNULL(redownload_delay_days, 0) > 0 AND + DATETIME('now', '-' || redownload_delay_days || ' day') > upload_date AND + DATETIME(media_downloaded_at, '-' || redownload_delay_days || ' day') < upload_date + """) + ) + end + + def where_culling_not_prevented(query) do where(query, [mi], mi.prevent_culling == false) 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 @@ -59,6 +81,10 @@ defmodule Pinchflat.Media.MediaQuery 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 @@ -73,7 +99,7 @@ defmodule Pinchflat.Media.MediaQuery do |> 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 + def where_download_not_prevented(query) do where(query, [mi], mi.prevent_download == false) end @@ -129,9 +155,9 @@ defmodule Pinchflat.Media.MediaQuery do ) end - def with_media_pending_download(query) do + def where_pending_download(query) do query - |> with_no_prevented_download() + |> where_download_not_prevented() |> with_no_media_filepath() |> with_upload_date_after_source_cutoff() |> with_format_matching_profile_preference() diff --git a/lib/pinchflat/notifications/source_notifications.ex b/lib/pinchflat/notifications/source_notifications.ex index e39841d..1205858 100644 --- a/lib/pinchflat/notifications/source_notifications.ex +++ b/lib/pinchflat/notifications/source_notifications.ex @@ -59,7 +59,7 @@ defmodule Pinchflat.Notifications.SourceNotifications do defp pending_media_item_count(source) do MediaQuery.new() |> MediaQuery.for_source(source) - |> MediaQuery.with_media_pending_download() + |> MediaQuery.where_pending_download() |> Repo.aggregate(:count) end diff --git a/lib/pinchflat/profiles/media_profile.ex b/lib/pinchflat/profiles/media_profile.ex index 57a3b7f..0b351a9 100644 --- a/lib/pinchflat/profiles/media_profile.ex +++ b/lib/pinchflat/profiles/media_profile.ex @@ -26,12 +26,14 @@ defmodule Pinchflat.Profiles.MediaProfile do shorts_behaviour livestream_behaviour preferred_resolution + redownload_delay_days )a @required_fields ~w(name output_path_template)a schema "media_profiles" do field :name, :string + field :redownload_delay_days, :integer field :output_path_template, :string, default: "/{{ source_custom_name }}/{{ upload_yyyy_mm_dd }} {{ title }}/{{ title }} [{{ id }}].{{ ext }}" @@ -60,7 +62,6 @@ defmodule Pinchflat.Profiles.MediaProfile do # See `build_format_clauses` in the Media context for more. field :shorts_behaviour, Ecto.Enum, values: ~w(include exclude only)a, default: :include field :livestream_behaviour, Ecto.Enum, values: ~w(include exclude only)a, default: :include - field :preferred_resolution, Ecto.Enum, values: ~w(2160p 1080p 720p 480p 360p audio)a, default: :"1080p" has_many :sources, Source @@ -75,6 +76,7 @@ defmodule Pinchflat.Profiles.MediaProfile do |> validate_required(@required_fields) # Ensures it ends with `.{{ ext }}` or `.%(ext)s` or similar (with a little wiggle room) |> validate_format(:output_path_template, ext_regex(), message: "must end with .{{ ext }}") + |> validate_number(:redownload_delay_days, greater_than_or_equal_to: 0) |> unique_constraint(:name) end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/media_profile_form.html.heex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/media_profile_form.html.heex index 36d99cc..78a6c5f 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/media_profile_form.html.heex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/media_profile_form.html.heex @@ -203,6 +203,17 @@ /> +
    + <.input + field={f[:redownload_delay_days]} + type="number" + label="Redownload Delay (days)" + min="0" + help="Delay in days until new media is redownloaded. Redownloading new media can improve its quality or SponsorBlock tags. Leave blank to not redownload" + x-init="$watch('selectedPreset', p => p && ($el.value = presets[p]))" + /> +
    +

    Media Center Options

    diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index d8e904d..ff12534 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -63,7 +63,7 @@ defmodule PinchflatWeb.Sources.SourceController do pending_media = MediaQuery.new() |> MediaQuery.for_source(source) - |> MediaQuery.with_media_pending_download() + |> MediaQuery.where_pending_download() |> order_by(desc: :id) |> limit(100) |> Repo.all() diff --git a/priv/repo/migrations/20240409224152_add_redownloaded_fields.exs b/priv/repo/migrations/20240409224152_add_redownloaded_fields.exs new file mode 100644 index 0000000..11c528c --- /dev/null +++ b/priv/repo/migrations/20240409224152_add_redownloaded_fields.exs @@ -0,0 +1,13 @@ +defmodule Pinchflat.Repo.Migrations.AddRedownloadedFields do + use Ecto.Migration + + def change do + alter table(:media_profiles) do + add :redownload_delay_days, :integer + end + + alter table(:media_items) do + add :media_redownloaded_at, :utc_datetime + end + end +end diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index b227c31..4f3b72e 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -62,7 +62,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do assert media_item.media_filepath == nil perform_job(MediaDownloadWorker, %{id: media_item.id}) - assert Repo.reload(media_item).media_filepath != nil + media_item = Repo.reload(media_item) + + assert media_item.media_filepath != nil end test "it saves the metadata to the media_item", %{media_item: media_item} do @@ -149,6 +151,28 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do assert media_item.media_size_bytes > 0 end + test "saves redownloaded_at if this is for a redownload", %{media_item: media_item} do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> + {:ok, render_metadata(:media_metadata)} + end) + + perform_job(MediaDownloadWorker, %{id: media_item.id, redownload?: true}) + media_item = Repo.reload(media_item) + + assert media_item.media_redownloaded_at != nil + end + + test "doesn't save redownloaded_at if this is not for a redownload", %{media_item: media_item} do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> + {:ok, render_metadata(:media_metadata)} + end) + + perform_job(MediaDownloadWorker, %{id: media_item.id}) + media_item = Repo.reload(media_item) + + assert media_item.media_redownloaded_at == nil + end + test "does not blow up if the record doesn't exist" do assert :ok = perform_job(MediaDownloadWorker, %{id: 0}) end diff --git a/test/pinchflat/downloading/media_redownload_worker_test.exs b/test/pinchflat/downloading/media_redownload_worker_test.exs new file mode 100644 index 0000000..70d9cc0 --- /dev/null +++ b/test/pinchflat/downloading/media_redownload_worker_test.exs @@ -0,0 +1,44 @@ +defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do + use Pinchflat.DataCase + + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + import Pinchflat.ProfilesFixtures + + alias Pinchflat.Downloading.MediaDownloadWorker + alias Pinchflat.Downloading.MediaRedownloadWorker + + describe "perform/1" do + test "kicks off a task for redownloadable media items" do + media_profile = media_profile_fixture(%{redownload_delay_days: 4}) + source = source_fixture(%{media_profile_id: media_profile.id, inserted_at: now_minus(10, :days)}) + + media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(6, :days), + media_downloaded_at: now_minus(5, :days) + }) + + perform_job(MediaRedownloadWorker, %{}) + + assert [_] = all_enqueued(worker: MediaDownloadWorker, args: %{id: media_item.id, redownload?: true}) + end + + test "does not kickoff a task for non-redownloadable media items" do + media_profile = media_profile_fixture(%{redownload_delay_days: 4}) + source = source_fixture(%{media_profile_id: media_profile.id, inserted_at: now_minus(10, :days)}) + + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(6, :days), + media_downloaded_at: now_minus(1, :day) + }) + + perform_job(MediaRedownloadWorker, %{}) + + assert [] = all_enqueued(worker: MediaDownloadWorker) + end + end +end diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index 52eff4d..fad36c0 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -130,6 +130,131 @@ defmodule Pinchflat.MediaTest do end end + describe "list_redownloadable_media_items/0" do + setup do + media_profile = media_profile_fixture(%{redownload_delay_days: 4}) + source = source_fixture(%{media_profile_id: media_profile.id, inserted_at: now_minus(10, :days)}) + + {:ok, %{media_profile: media_profile, source: source}} + end + + test "returns media eligible for redownload", %{source: source} do + media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(6, :days), + media_downloaded_at: now_minus(5, :days) + }) + + assert Media.list_redownloadable_media_items() == [media_item] + end + + test "returns media items that were downloaded in past but still meet redownload delay", %{source: source} do + media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(20, :days), + media_downloaded_at: now_minus(19, :days) + }) + + assert Media.list_redownloadable_media_items() == [media_item] + end + + test "does not return media items without a media_downloaded_at", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(5, :days), + media_downloaded_at: nil + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items that are set to prevent download", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(5, :days), + media_downloaded_at: now(), + prevent_download: true + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items that have been culled", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(5, :days), + media_downloaded_at: now(), + culled_at: now() + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items before the download delay", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(3, :days), + media_downloaded_at: now_minus(3, :days) + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items that have already been redownloaded", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(5, :days), + media_downloaded_at: now(), + media_redownloaded_at: now() + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items that were first downloaded well after the upload_date", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + media_downloaded_at: now(), + upload_date: now_minus(20, :days) + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items that were recently uploaded", %{source: source} do + _media_item = + media_item_fixture(%{ + source_id: source.id, + media_downloaded_at: now(), + upload_date: now_minus(2, :days) + }) + + assert Media.list_redownloadable_media_items() == [] + end + + test "does not return media items without a redownload delay" do + media_profile = media_profile_fixture(%{redownload_delay_days: nil}) + source = source_fixture(%{media_profile_id: media_profile.id}) + + _media_item = + media_item_fixture(%{ + source_id: source.id, + upload_date: now_minus(6, :days), + media_downloaded_at: now_minus(5, :days) + }) + + assert Media.list_redownloadable_media_items() == [] + end + end + describe "list_pending_media_items_for/1" do test "it returns pending without a filepath for a given source" do source = source_fixture() From 0fcdd1df84670b8a9b9eb18e52270801709d848f Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 18:02:26 -0700 Subject: [PATCH 007/240] Renamed FS Helpers module to FS Utils --- lib/pinchflat/boot/pre_job_startup_tasks.ex | 4 +- lib/pinchflat/downloading/media_downloader.ex | 4 +- lib/pinchflat/media/media.ex | 6 +-- .../metadata/metadata_file_helpers.ex | 6 +-- lib/pinchflat/metadata/nfo_builder.ex | 6 +-- lib/pinchflat/metadata/source_image_parser.ex | 4 +- lib/pinchflat/release.ex | 4 +- lib/pinchflat/sources/sources.ex | 6 +-- .../filesystem_utils.ex} | 2 +- lib/pinchflat/yt_dlp/command_runner.ex | 2 +- lib/pinchflat/yt_dlp/media_collection.ex | 4 +- .../media_download_worker_test.exs | 4 +- test/pinchflat/metadata/nfo_builder_test.exs | 4 +- .../file_follower_server_test.exs | 4 +- test/pinchflat/sources_test.exs | 6 +-- .../filesystem_utils_test.exs} | 48 +++++++++---------- test/pinchflat/yt_dlp/command_runner_test.exs | 6 +-- test/support/fixtures/media_fixtures.ex | 8 ++-- test/support/fixtures/sources_fixtures.ex | 8 ++-- 19 files changed, 68 insertions(+), 68 deletions(-) rename lib/pinchflat/{filesystem/filesystem_helpers.ex => utils/filesystem_utils.ex} (97%) rename test/pinchflat/{filesystem/filesystem_helpers_test.exs => utils/filesystem_utils_test.exs} (70%) diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index 74b96d3..e36d1ca 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -14,7 +14,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do alias Pinchflat.Repo alias Pinchflat.Settings - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils def start_link(opts \\ []) do GenServer.start_link(__MODULE__, %{}, opts) @@ -57,7 +57,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do if !File.exists?(filepath) do Logger.info("yt-dlp cookie file does not exist - creating it") - FilesystemHelpers.write_p!(filepath, "") + FilesystemUtils.write_p!(filepath, "") end end diff --git a/lib/pinchflat/downloading/media_downloader.ex b/lib/pinchflat/downloading/media_downloader.ex index 8d5be4d..b1fb91a 100644 --- a/lib/pinchflat/downloading/media_downloader.ex +++ b/lib/pinchflat/downloading/media_downloader.ex @@ -13,7 +13,7 @@ defmodule Pinchflat.Downloading.MediaDownloader do alias Pinchflat.Metadata.NfoBuilder alias Pinchflat.Metadata.MetadataParser alias Pinchflat.Metadata.MetadataFileHelpers - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Downloading.DownloadOptionBuilder alias Pinchflat.YtDlp.Media, as: YtDlpMedia @@ -30,7 +30,7 @@ defmodule Pinchflat.Downloading.MediaDownloader do Returns {:ok, %MediaItem{}} | {:error, any, ...any} """ def download_for_media_item(%MediaItem{} = media_item) do - output_filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + output_filepath = FilesystemUtils.generate_metadata_tmpfile(:json) media_with_preloads = Repo.preload(media_item, [:metadata, source: :media_profile]) case download_with_options(media_item.original_url, media_with_preloads, output_filepath) do diff --git a/lib/pinchflat/media/media.ex b/lib/pinchflat/media/media.ex index d458af4..43f7b6e 100644 --- a/lib/pinchflat/media/media.ex +++ b/lib/pinchflat/media/media.ex @@ -11,7 +11,7 @@ defmodule Pinchflat.Media do alias Pinchflat.Media.MediaItem alias Pinchflat.Media.MediaQuery alias Pinchflat.Metadata.MediaMetadata - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Returns the list of media_items. @@ -223,7 +223,7 @@ defmodule Pinchflat.Media do end) |> List.flatten() |> Enum.filter(&is_binary/1) - |> Enum.each(&FilesystemHelpers.delete_file_and_remove_empty_directories/1) + |> Enum.each(&FilesystemUtils.delete_file_and_remove_empty_directories/1) {:ok, media_item} end @@ -235,6 +235,6 @@ defmodule Pinchflat.Media do MediaMetadata.filepath_attributes() |> Enum.map(fn field -> mapped_struct[field] end) |> Enum.filter(&is_binary/1) - |> Enum.each(&FilesystemHelpers.delete_file_and_remove_empty_directories/1) + |> Enum.each(&FilesystemUtils.delete_file_and_remove_empty_directories/1) end end diff --git a/lib/pinchflat/metadata/metadata_file_helpers.ex b/lib/pinchflat/metadata/metadata_file_helpers.ex index 8d4da24..959d1ca 100644 --- a/lib/pinchflat/metadata/metadata_file_helpers.ex +++ b/lib/pinchflat/metadata/metadata_file_helpers.ex @@ -9,7 +9,7 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do needed """ - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Returns the directory where metadata for a database record should be stored. @@ -36,7 +36,7 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do filepath = generate_filepath_for(database_record, "metadata.json.gz") {:ok, json} = Phoenix.json_library().encode(metadata_map) - :ok = FilesystemHelpers.write_p!(filepath, json, [:compressed]) + :ok = FilesystemUtils.write_p!(filepath, json, [:compressed]) filepath end @@ -62,7 +62,7 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url)) thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url) - :ok = FilesystemHelpers.write_p!(filepath, thumbnail_blob) + :ok = FilesystemUtils.write_p!(filepath, thumbnail_blob) filepath end diff --git a/lib/pinchflat/metadata/nfo_builder.ex b/lib/pinchflat/metadata/nfo_builder.ex index ad42331..3db237f 100644 --- a/lib/pinchflat/metadata/nfo_builder.ex +++ b/lib/pinchflat/metadata/nfo_builder.ex @@ -7,7 +7,7 @@ defmodule Pinchflat.Metadata.NfoBuilder do import Pinchflat.Utils.XmlUtils, only: [safe: 1] alias Pinchflat.Metadata.MetadataFileHelpers - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Builds an NFO file for a media item (read: single "episode") and @@ -18,7 +18,7 @@ defmodule Pinchflat.Metadata.NfoBuilder do def build_and_store_for_media_item(filepath, metadata) do nfo = build_for_media_item(metadata) - FilesystemHelpers.write_p!(filepath, nfo) + FilesystemUtils.write_p!(filepath, nfo) filepath end @@ -32,7 +32,7 @@ defmodule Pinchflat.Metadata.NfoBuilder do def build_and_store_for_source(filepath, metadata) do nfo = build_for_source(metadata) - FilesystemHelpers.write_p!(filepath, nfo) + FilesystemUtils.write_p!(filepath, nfo) filepath end diff --git a/lib/pinchflat/metadata/source_image_parser.ex b/lib/pinchflat/metadata/source_image_parser.ex index a207462..9a0370c 100644 --- a/lib/pinchflat/metadata/source_image_parser.ex +++ b/lib/pinchflat/metadata/source_image_parser.ex @@ -2,7 +2,7 @@ defmodule Pinchflat.Metadata.SourceImageParser do @moduledoc """ Functions for parsing and storing source images. """ - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Given a base directory and source metadata, look for the appropriate images @@ -62,7 +62,7 @@ defmodule Pinchflat.Metadata.SourceImageParser do extension = Path.extname(tmp_filepath) final_filepath = Path.join([base_directory, "#{filename}#{extension}"]) - FilesystemHelpers.cp_p!(tmp_filepath, final_filepath) + FilesystemUtils.cp_p!(tmp_filepath, final_filepath) {source_attr_name, final_filepath} end diff --git a/lib/pinchflat/release.ex b/lib/pinchflat/release.ex index 7648efe..158d5dc 100644 --- a/lib/pinchflat/release.ex +++ b/lib/pinchflat/release.ex @@ -7,7 +7,7 @@ defmodule Pinchflat.Release do require Logger - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils def migrate do load_app() @@ -39,7 +39,7 @@ defmodule Pinchflat.Release do Logger.info("Checking permissions for #{dir}") filepath = Path.join([dir, ".keep"]) - case FilesystemHelpers.write_p(filepath, "") do + case FilesystemUtils.write_p(filepath, "") do :ok -> Logger.info("Permissions OK") diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index a65d75b..27d16b8 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -13,7 +13,7 @@ defmodule Pinchflat.Sources do alias Pinchflat.Profiles.MediaProfile alias Pinchflat.YtDlp.MediaCollection alias Pinchflat.Metadata.SourceMetadata - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.FastIndexing.FastIndexingWorker alias Pinchflat.SlowIndexing.SlowIndexingHelpers @@ -153,7 +153,7 @@ defmodule Pinchflat.Sources do Source.filepath_attributes() |> Enum.map(fn field -> mapped_struct[field] end) |> Enum.filter(&is_binary/1) - |> Enum.each(&FilesystemHelpers.delete_file_and_remove_empty_directories/1) + |> Enum.each(&FilesystemUtils.delete_file_and_remove_empty_directories/1) end defp delete_internal_metadata_files(source) do @@ -163,7 +163,7 @@ defmodule Pinchflat.Sources do SourceMetadata.filepath_attributes() |> Enum.map(fn field -> mapped_struct[field] end) |> Enum.filter(&is_binary/1) - |> Enum.each(&FilesystemHelpers.delete_file_and_remove_empty_directories/1) + |> Enum.each(&FilesystemUtils.delete_file_and_remove_empty_directories/1) end defp add_source_details_to_changeset(source, changeset) do diff --git a/lib/pinchflat/filesystem/filesystem_helpers.ex b/lib/pinchflat/utils/filesystem_utils.ex similarity index 97% rename from lib/pinchflat/filesystem/filesystem_helpers.ex rename to lib/pinchflat/utils/filesystem_utils.ex index 349c4b7..8315218 100644 --- a/lib/pinchflat/filesystem/filesystem_helpers.ex +++ b/lib/pinchflat/utils/filesystem_utils.ex @@ -1,4 +1,4 @@ -defmodule Pinchflat.Filesystem.FilesystemHelpers do +defmodule Pinchflat.Utils.FilesystemUtils do @moduledoc """ Utility methods for working with the filesystem """ diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index 7fc19bf..00dfebf 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -7,7 +7,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do alias Pinchflat.Utils.CliUtils alias Pinchflat.YtDlp.YtDlpCommandRunner - alias Pinchflat.Filesystem.FilesystemHelpers, as: FSUtils + alias Pinchflat.Utils.FilesystemUtils, as: FSUtils @behaviour YtDlpCommandRunner diff --git a/lib/pinchflat/yt_dlp/media_collection.ex b/lib/pinchflat/yt_dlp/media_collection.ex index 249a703..a0b3328 100644 --- a/lib/pinchflat/yt_dlp/media_collection.ex +++ b/lib/pinchflat/yt_dlp/media_collection.ex @@ -6,7 +6,7 @@ defmodule Pinchflat.YtDlp.MediaCollection do require Logger - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.YtDlp.Media, as: YtDlpMedia @doc """ @@ -26,7 +26,7 @@ defmodule Pinchflat.YtDlp.MediaCollection do # available formats since we're just getting the media details command_opts = [:simulate, :skip_download, :ignore_no_formats_error] output_template = YtDlpMedia.indexing_output_template() - output_filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + output_filepath = FilesystemUtils.generate_metadata_tmpfile(:json) file_listener_handler = Keyword.get(addl_opts, :file_listener_handler, false) if file_listener_handler do diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index 4f3b72e..7d0e616 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -6,7 +6,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do alias Pinchflat.Media alias Pinchflat.Sources - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Downloading.MediaDownloadWorker setup :verify_on_exit! @@ -140,7 +140,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do test "it saves the file's size to the database", %{media_item: media_item} do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> metadata = render_parsed_metadata(:media_metadata) - FilesystemHelpers.write_p!(metadata["filepath"], "test") + FilesystemUtils.write_p!(metadata["filepath"], "test") {:ok, Phoenix.json_library().encode!(metadata)} end) diff --git a/test/pinchflat/metadata/nfo_builder_test.exs b/test/pinchflat/metadata/nfo_builder_test.exs index 14ea1fa..c8b032a 100644 --- a/test/pinchflat/metadata/nfo_builder_test.exs +++ b/test/pinchflat/metadata/nfo_builder_test.exs @@ -2,10 +2,10 @@ defmodule Pinchflat.Metadata.NfoBuilderTest do use Pinchflat.DataCase alias Pinchflat.Metadata.NfoBuilder - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils setup do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + filepath = FilesystemUtils.generate_metadata_tmpfile(:json) on_exit(fn -> File.rm!(filepath) end) diff --git a/test/pinchflat/slow_indexing/file_follower_server_test.exs b/test/pinchflat/slow_indexing/file_follower_server_test.exs index 52104fe..ff09695 100644 --- a/test/pinchflat/slow_indexing/file_follower_server_test.exs +++ b/test/pinchflat/slow_indexing/file_follower_server_test.exs @@ -1,12 +1,12 @@ defmodule Pinchflat.SlowIndexing.FileFollowerServerTest do use ExUnit.Case, async: true - alias alias Pinchflat.Filesystem.FilesystemHelpers + alias alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.SlowIndexing.FileFollowerServer setup do {:ok, pid} = FileFollowerServer.start_link() - tmpfile = FilesystemHelpers.generate_metadata_tmpfile(:txt) + tmpfile = FilesystemUtils.generate_metadata_tmpfile(:txt) {:ok, %{pid: pid, tmpfile: tmpfile}} end diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 8ce107d..175a945 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -8,7 +8,7 @@ defmodule Pinchflat.SourcesTest do alias Pinchflat.Sources alias Pinchflat.Sources.Source - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Metadata.MetadataFileHelpers alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.FastIndexing.FastIndexingWorker @@ -561,7 +561,7 @@ defmodule Pinchflat.SourcesTest do end test "does not delete the source's non-metadata files" do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:nfo) + filepath = FilesystemUtils.generate_metadata_tmpfile(:nfo) source = source_fixture(%{nfo_filepath: filepath}) assert {:ok, _} = Sources.delete_source(source) @@ -592,7 +592,7 @@ defmodule Pinchflat.SourcesTest do end test "deletes the source's non-metadata files" do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:nfo) + filepath = FilesystemUtils.generate_metadata_tmpfile(:nfo) source = source_fixture(%{nfo_filepath: filepath}) assert {:ok, _} = Sources.delete_source(source, delete_files: true) diff --git a/test/pinchflat/filesystem/filesystem_helpers_test.exs b/test/pinchflat/utils/filesystem_utils_test.exs similarity index 70% rename from test/pinchflat/filesystem/filesystem_helpers_test.exs rename to test/pinchflat/utils/filesystem_utils_test.exs index 03f2106..7d86cbc 100644 --- a/test/pinchflat/filesystem/filesystem_helpers_test.exs +++ b/test/pinchflat/utils/filesystem_utils_test.exs @@ -1,13 +1,13 @@ -defmodule Pinchflat.Filesystem.FilesystemHelpersTest do +defmodule Pinchflat.Utils.FilesystemUtilsTest do use Pinchflat.DataCase import Pinchflat.MediaFixtures - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils describe "generate_metadata_tmpfile/1" do test "creates a tmpfile and returns its path" do - res = FilesystemHelpers.generate_metadata_tmpfile(:json) + res = FilesystemUtils.generate_metadata_tmpfile(:json) assert String.ends_with?(res, ".json") assert File.exists?(res) @@ -22,7 +22,7 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do refute media_item.media_size_bytes - assert {:ok, media_item} = FilesystemHelpers.compute_and_save_media_filesize(media_item) + assert {:ok, media_item} = FilesystemUtils.compute_and_save_media_filesize(media_item) assert Repo.reload!(media_item).media_size_bytes end @@ -30,16 +30,16 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do test "returns the error if operation fails" do media_item = media_item_fixture(%{media_filepath: "/nonexistent/file.mkv"}) - assert {:error, _} = FilesystemHelpers.compute_and_save_media_filesize(media_item) + assert {:error, _} = FilesystemUtils.compute_and_save_media_filesize(media_item) end end describe "write_p/3" do test "writes content to a file" do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + filepath = FilesystemUtils.generate_metadata_tmpfile(:json) content = "{}" - assert :ok = FilesystemHelpers.write_p(filepath, content) + assert :ok = FilesystemUtils.write_p(filepath, content) assert File.read!(filepath) == content File.rm!(filepath) @@ -50,7 +50,7 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do filepath = Path.join([tmpfile_directory, "foo", "bar", "file.json"]) content = "{}" - assert :ok = FilesystemHelpers.write_p(filepath, content) + assert :ok = FilesystemUtils.write_p(filepath, content) assert File.read!(filepath) == content File.rm!(filepath) @@ -59,10 +59,10 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do describe "write_p!/3" do test "writes content to a file" do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + filepath = FilesystemUtils.generate_metadata_tmpfile(:json) content = "{}" - assert :ok = FilesystemHelpers.write_p!(filepath, content) + assert :ok = FilesystemUtils.write_p!(filepath, content) assert File.read!(filepath) == content File.rm!(filepath) @@ -73,7 +73,7 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do filepath = Path.join([tmpfile_directory, "foo", "bar", "file.json"]) content = "{}" - assert :ok = FilesystemHelpers.write_p!(filepath, content) + assert :ok = FilesystemUtils.write_p!(filepath, content) assert File.read!(filepath) == content File.rm!(filepath) @@ -82,11 +82,11 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do describe "delete_file_and_remove_empty_directories/1" do test "deletes file at the provided filepath" do - filepath = FilesystemHelpers.generate_metadata_tmpfile(:json) + filepath = FilesystemUtils.generate_metadata_tmpfile(:json) assert File.exists?(filepath) - assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath) + assert :ok = FilesystemUtils.delete_file_and_remove_empty_directories(filepath) refute File.exists?(filepath) end @@ -94,9 +94,9 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do test "deletes empty directories" do tmpfile_directory = Application.get_env(:pinchflat, :tmpfile_directory) filepath = Path.join([tmpfile_directory, "foo", "bar", "baz", "qux.json"]) - FilesystemHelpers.write_p!(filepath, "") + FilesystemUtils.write_p!(filepath, "") - assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath) + assert :ok = FilesystemUtils.delete_file_and_remove_empty_directories(filepath) refute File.exists?(filepath) refute File.exists?(Path.join([tmpfile_directory, "foo", "bar", "baz"])) @@ -108,10 +108,10 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do tmpfile_directory = Application.get_env(:pinchflat, :tmpfile_directory) filepath_1 = Path.join([tmpfile_directory, "foo", "bar", "baz", "qux.json"]) filepath_2 = Path.join([tmpfile_directory, "foo", "baz.json"]) - FilesystemHelpers.write_p!(filepath_1, "") - FilesystemHelpers.write_p!(filepath_2, "") + FilesystemUtils.write_p!(filepath_1, "") + FilesystemUtils.write_p!(filepath_2, "") - assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath_1) + assert :ok = FilesystemUtils.delete_file_and_remove_empty_directories(filepath_1) refute File.exists?(filepath_1) refute File.exists?(Path.join([tmpfile_directory, "foo", "bar", "baz"])) @@ -121,24 +121,24 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do assert File.exists?(Path.join([tmpfile_directory, "foo"])) # cleanup - FilesystemHelpers.delete_file_and_remove_empty_directories(filepath_2) + FilesystemUtils.delete_file_and_remove_empty_directories(filepath_2) end test "returns an error if file could not be deleted" do filepath = "/nonexistent/file.json" - assert {:error, _} = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath) + assert {:error, _} = FilesystemUtils.delete_file_and_remove_empty_directories(filepath) end end describe "cp_p!/2" do test "copies a file from source to destination" do source = "#{tmpfile_directory()}/source.json" - FilesystemHelpers.write_p!(source, "TEST") + FilesystemUtils.write_p!(source, "TEST") destination = "#{tmpfile_directory()}/destination.json" refute File.exists?(destination) - FilesystemHelpers.cp_p!(source, destination) + FilesystemUtils.cp_p!(source, destination) assert File.exists?(destination) assert File.read!(destination) == "TEST" @@ -148,11 +148,11 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do test "creates directories as needed" do source = "#{tmpfile_directory()}/source.json" - FilesystemHelpers.write_p!(source, "TEST") + FilesystemUtils.write_p!(source, "TEST") destination = "#{tmpfile_directory()}/foo/bar/destination.json" refute File.exists?(destination) - FilesystemHelpers.cp_p!(source, destination) + FilesystemUtils.cp_p!(source, destination) assert File.exists?(destination) File.rm!(source) diff --git a/test/pinchflat/yt_dlp/command_runner_test.exs b/test/pinchflat/yt_dlp/command_runner_test.exs index ac06a48..decd6e5 100644 --- a/test/pinchflat/yt_dlp/command_runner_test.exs +++ b/test/pinchflat/yt_dlp/command_runner_test.exs @@ -1,7 +1,7 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do use ExUnit.Case, async: true - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.YtDlp.CommandRunner, as: Runner @@ -51,7 +51,7 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do end test "includes cookie options when cookies.txt exists", %{cookie_file: cookie_file} do - FilesystemHelpers.write_p!(cookie_file, "cookie data") + FilesystemUtils.write_p!(cookie_file, "cookie data") assert {:ok, output} = Runner.run(@media_url, [], "") @@ -59,7 +59,7 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do end test "doesn't include cookie options when cookies.txt blank", %{cookie_file: cookie_file} do - FilesystemHelpers.write_p!(cookie_file, " \n \n ") + FilesystemUtils.write_p!(cookie_file, " \n \n ") assert {:ok, output} = Runner.run(@media_url, [], "") diff --git a/test/support/fixtures/media_fixtures.ex b/test/support/fixtures/media_fixtures.ex index f724039..f80f494 100644 --- a/test/support/fixtures/media_fixtures.ex +++ b/test/support/fixtures/media_fixtures.ex @@ -5,7 +5,7 @@ defmodule Pinchflat.MediaFixtures do """ alias Pinchflat.SourcesFixtures - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Generate a media_item. @@ -52,8 +52,8 @@ defmodule Pinchflat.MediaFixtures do json_gz_filepath = Path.join(metadata_dir, "metadata.json.gz") thumbnail_filepath = Path.join(metadata_dir, "thumbnail.jpg") - FilesystemHelpers.cp_p!(media_metadata_filepath_fixture(), json_gz_filepath) - FilesystemHelpers.cp_p!(thumbnail_filepath_fixture(), thumbnail_filepath) + FilesystemUtils.cp_p!(media_metadata_filepath_fixture(), json_gz_filepath) + FilesystemUtils.cp_p!(thumbnail_filepath_fixture(), thumbnail_filepath) merged_attrs = Map.merge(attrs, %{ @@ -74,7 +74,7 @@ defmodule Pinchflat.MediaFixtures do "#{:rand.uniform(1_000_000)}_media.mp4" ]) - FilesystemHelpers.cp_p!(media_filepath_fixture(), stored_media_filepath) + FilesystemUtils.cp_p!(media_filepath_fixture(), stored_media_filepath) merged_attrs = Map.merge(attrs, %{media_filepath: stored_media_filepath}) media_item_fixture(merged_attrs) diff --git a/test/support/fixtures/sources_fixtures.ex b/test/support/fixtures/sources_fixtures.ex index 55375e5..bb768db 100644 --- a/test/support/fixtures/sources_fixtures.ex +++ b/test/support/fixtures/sources_fixtures.ex @@ -8,7 +8,7 @@ defmodule Pinchflat.SourcesFixtures do alias Pinchflat.MediaFixtures alias Pinchflat.Sources.Source alias Pinchflat.ProfilesFixtures - alias Pinchflat.Filesystem.FilesystemHelpers + alias Pinchflat.Utils.FilesystemUtils @doc """ Generate a source. @@ -59,9 +59,9 @@ defmodule Pinchflat.SourcesFixtures do poster_filepath = Path.join(metadata_dir, "poster.jpg") fanart_filepath = Path.join(metadata_dir, "fanart.jpg") - FilesystemHelpers.cp_p!(MediaFixtures.media_metadata_filepath_fixture(), json_gz_filepath) - FilesystemHelpers.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), poster_filepath) - FilesystemHelpers.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), fanart_filepath) + FilesystemUtils.cp_p!(MediaFixtures.media_metadata_filepath_fixture(), json_gz_filepath) + FilesystemUtils.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), poster_filepath) + FilesystemUtils.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), fanart_filepath) merged_attrs = Map.merge(attrs, %{ From 8fbcc8b28916f1aeebc9f98616f39a8a068d31f6 Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 10 Apr 2024 20:17:22 -0700 Subject: [PATCH 008/240] [Enhancement] Allow custom yt-dlp options (#176) * Added option for yt-dlp config file usage * renamed yt-dlp config file * refactored to use a precedence-based approach * Updated README --- README.md | 1 + lib/pinchflat/boot/pre_job_startup_tasks.ex | 18 ++-- .../downloading/download_option_builder.ex | 35 ++++++- lib/pinchflat/yt_dlp/command_runner.ex | 33 ++++--- .../boot/pre_job_startup_tasks_test.exs | 14 ++- .../download_option_builder_test.exs | 91 +++++++++++++++++++ test/pinchflat/yt_dlp/command_runner_test.exs | 5 +- 7 files changed, 172 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 0b0a10c..143623c 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ If it doesn't work for your use case, please make a feature request! You can als - Reliable hands-off operation - Can pass cookies to YouTube to download your private playlists ([docs](https://github.com/kieraneglin/pinchflat/wiki/YouTube-Cookies)) - Sponsorblock integration +- \[Advanced\] allows custom `yt-dlp` options ([docs](https://github.com/kieraneglin/pinchflat/wiki/%5BAdvanced%5D-Custom-yt%E2%80%90dlp-options)) ## Screenshots diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index e36d1ca..9f6bcf7 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -32,7 +32,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do @impl true def init(state) do reset_executing_jobs() - create_blank_cookie_file() + create_blank_yt_dlp_files() apply_default_settings() {:ok, state} @@ -50,15 +50,19 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do Logger.info("Reset #{count} executing jobs") end - defp create_blank_cookie_file do + defp create_blank_yt_dlp_files do + files = ["cookies.txt", "yt-dlp-configs/base-config.txt"] base_dir = Application.get_env(:pinchflat, :extras_directory) - filepath = Path.join(base_dir, "cookies.txt") - if !File.exists?(filepath) do - Logger.info("yt-dlp cookie file does not exist - creating it") + Enum.each(files, fn file -> + filepath = Path.join(base_dir, file) - FilesystemUtils.write_p!(filepath, "") - end + if !File.exists?(filepath) do + Logger.info("Creating blank file: #{filepath}") + + FilesystemUtils.write_p!(filepath, "") + end + end) end defp apply_default_settings do diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 479e643..9e3a5c4 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -10,8 +10,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do @doc """ Builds the options for yt-dlp to download media based on the given media's profile. - IDEA: consider adding the ability to pass in a second argument to override - these options + Returns {:ok, [Keyword.t()]} """ def build(%MediaItem{} = media_item_with_preloads) do media_profile = media_item_with_preloads.source.media_profile @@ -23,7 +22,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do metadata_options(media_profile) ++ quality_options(media_profile) ++ sponsorblock_options(media_profile) ++ - output_options(media_item_with_preloads) + output_options(media_item_with_preloads) ++ + config_file_options(media_item_with_preloads) {:ok, built_options} end @@ -128,6 +128,35 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end end + # This is put here instead of the CommandRunner module because it should only + # be applied to downloading - if it were in CommandRunner it would apply to + # all yt-dlp commands (like indexing) + defp config_file_options(media_item) do + base_dir = Path.join(Application.get_env(:pinchflat, :extras_directory), "yt-dlp-configs") + # Ordered by priority - the first file has the highest priority + filenames = [ + "media-item-#{media_item.id}-config.txt", + "source-#{media_item.source_id}-config.txt", + "media-profile-#{media_item.source.media_profile_id}-config.txt", + "base-config.txt" + ] + + config_filepaths = + Enum.reduce(filenames, [], fn filename, acc -> + filepath = Path.join(base_dir, filename) + + case File.read(filepath) do + {:ok, file_data} -> + if String.trim(file_data) != "", do: [filepath | acc], else: acc + + {:error, _} -> + acc + end + end) + + Enum.map(config_filepaths, fn filepath -> {:config_locations, filepath} end) + end + defp output_options(media_item_with_preloads) do [ output: build_output_path_for(media_item_with_preloads.source) diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index 00dfebf..ed13d33 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -27,13 +27,14 @@ defmodule Pinchflat.YtDlp.CommandRunner do def run(url, command_opts, output_template, addl_opts \\ []) do # This approach lets us mock the command for testing 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 = 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] ++ CliUtils.parse_options(command_opts ++ print_to_file_opts ++ cookie_opts) + external_file_opts = build_external_file_options() + # These must stay in exactly this order, hence why I'm giving it its own variable. + all_opts = command_opts ++ print_to_file_opts ++ external_file_opts + formatted_command_opts = [url] ++ CliUtils.parse_options(all_opts) Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}") case System.cmd(command, formatted_command_opts, stderr_to_stdout: true) do @@ -73,17 +74,25 @@ defmodule Pinchflat.YtDlp.CommandRunner do end end - defp build_cookie_options do + defp build_external_file_options do base_dir = Application.get_env(:pinchflat, :extras_directory) - cookie_file = Path.join(base_dir, "cookies.txt") + filename_options_map = %{cookies: "cookies.txt"} - case File.read(cookie_file) do - {:ok, cookie_data} -> - if String.trim(cookie_data) != "", do: [cookies: cookie_file], else: [] + Enum.reduce(filename_options_map, [], fn {opt_name, filename}, acc -> + filepath = Path.join(base_dir, filename) - {:error, _} -> - [] - end + case File.read(filepath) do + {:ok, file_data} -> + if String.trim(file_data) != "" do + [{opt_name, filepath} | acc] + else + acc + end + + {:error, _} -> + acc + end + end) end defp backend_executable do diff --git a/test/pinchflat/boot/pre_job_startup_tasks_test.exs b/test/pinchflat/boot/pre_job_startup_tasks_test.exs index 8fd81bb..66ecfce 100644 --- a/test/pinchflat/boot/pre_job_startup_tasks_test.exs +++ b/test/pinchflat/boot/pre_job_startup_tasks_test.exs @@ -27,7 +27,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do end end - describe "create_blank_cookie_file" do + describe "create_blank_yt_dlp_files" do test "creates a blank cookie file" do base_dir = Application.get_env(:pinchflat, :extras_directory) filepath = Path.join(base_dir, "cookies.txt") @@ -39,6 +39,18 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do assert File.exists?(filepath) end + + test "creates a blank yt-dlp config file" do + base_dir = Application.get_env(:pinchflat, :extras_directory) + filepath = Path.join([base_dir, "yt-dlp-configs", "base-config.txt"]) + File.rm(filepath) + + refute File.exists?(filepath) + + PreJobStartupTasks.init(%{}) + + assert File.exists?(filepath) + end end describe "apply_default_settings" do diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index d7bc631..22ca4b2 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -5,6 +5,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do import Pinchflat.ProfilesFixtures alias Pinchflat.Profiles + alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Downloading.DownloadOptionBuilder setup do @@ -261,6 +262,96 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do end end + describe "build/1 when testing config file options" do + setup do + base_dir = Path.join(Application.get_env(:pinchflat, :extras_directory), "yt-dlp-configs") + + {:ok, %{base_dir: base_dir}} + end + + test "includes base config file if it's present", %{media_item: media_item, base_dir: base_dir} do + filepath = Path.join(base_dir, "base-config.txt") + + FilesystemUtils.write_p!(filepath, "base config") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + assert {:config_locations, filepath} in res + end + + test "includes media profile config file if it's present", %{media_item: media_item, base_dir: base_dir} do + media_profile = media_item.source.media_profile + filepath = Path.join(base_dir, "media-profile-#{media_profile.id}-config.txt") + + FilesystemUtils.write_p!(filepath, "profile config") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + assert {:config_locations, filepath} in res + end + + test "includes source config file if it's present", %{media_item: media_item, base_dir: base_dir} do + source = media_item.source + filepath = Path.join(base_dir, "source-#{source.id}-config.txt") + + FilesystemUtils.write_p!(filepath, "profile config") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + assert {:config_locations, filepath} in res + end + + test "includes media item config file if it's present", %{media_item: media_item, base_dir: base_dir} do + filepath = Path.join(base_dir, "media-item-#{media_item.id}-config.txt") + + FilesystemUtils.write_p!(filepath, "media item config") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + assert {:config_locations, filepath} in res + end + + test "does not include config file options if they are not present", %{media_item: media_item} do + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + refute :config_locations in res + end + + test "does not return a config file if it's blank", %{media_item: media_item, base_dir: base_dir} do + filepath = Path.join(base_dir, "base-config.txt") + + FilesystemUtils.write_p!(filepath, " \n \n ") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + refute :config_locations in res + end + + test "returns config files in order of precedence", %{media_item: media_item, base_dir: base_dir} do + source = media_item.source + media_profile = source.media_profile + + base_filepath = Path.join(base_dir, "base-config.txt") + source_filepath = Path.join(base_dir, "source-#{source.id}-config.txt") + media_item_filepath = Path.join(base_dir, "media-item-#{media_item.id}-config.txt") + media_profile_filepath = Path.join(base_dir, "media-profile-#{media_profile.id}-config.txt") + + FilesystemUtils.write_p!(base_filepath, "config") + FilesystemUtils.write_p!(source_filepath, "config") + FilesystemUtils.write_p!(media_item_filepath, "config") + FilesystemUtils.write_p!(media_profile_filepath, "config") + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + expected_order = [ + {:config_locations, base_filepath}, + {:config_locations, media_profile_filepath}, + {:config_locations, source_filepath}, + {:config_locations, media_item_filepath} + ] + + assert Enum.filter(res, fn + {:config_locations, _} -> true + _ -> false + end) == expected_order + end + end + describe "build_output_path_for/1" do test "builds an output path for a source", %{media_item: media_item} do path = DownloadOptionBuilder.build_output_path_for(media_item.source) diff --git a/test/pinchflat/yt_dlp/command_runner_test.exs b/test/pinchflat/yt_dlp/command_runner_test.exs index decd6e5..17cf61c 100644 --- a/test/pinchflat/yt_dlp/command_runner_test.exs +++ b/test/pinchflat/yt_dlp/command_runner_test.exs @@ -42,12 +42,13 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do end end - describe "run/4 when testing cookie options" do + describe "run/4 when testing external file options" do setup do base_dir = Application.get_env(:pinchflat, :extras_directory) cookie_file = Path.join(base_dir, "cookies.txt") + yt_dlp_file = Path.join([base_dir, "yt-dlp-configs", "main.txt"]) - {:ok, cookie_file: cookie_file} + {:ok, cookie_file: cookie_file, yt_dlp_file: yt_dlp_file} end test "includes cookie options when cookies.txt exists", %{cookie_file: cookie_file} do From 2a9677df51641788a02b39c97e21492d84d44022 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 20:35:45 -0700 Subject: [PATCH 009/240] Bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 434a0e0..2e7cec7 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.10", + version: "0.1.11", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From 1a699223fec9ecfc2c49aeb10574197f244721f5 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 21:01:50 -0700 Subject: [PATCH 010/240] Updated README blurb on WAL issues --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 143623c..685d70f 100644 --- a/README.md +++ b/README.md @@ -114,10 +114,10 @@ It's recommended to not run the container as root. Doing so can create permissio ### Advanced: storing Pinchflat config directory on a network share -README: This is currently in the testing phase and not a recommended option (yet). The implications of changing this setting isn't clear and this could, conceivably, result in data loss. Only change this setting if you know what you're doing, why this is important, and are okay with possible data loss or DB corruption. This may become the default in the future once it's been tested more thoroughly. - As pointed out in [#137](https://github.com/kieraneglin/pinchflat/issues/137), SQLite doesn't like being run in WAL mode on network shares. If you're running Pinchflat on a network share, you can disable WAL mode by setting the `JOURNAL_MODE` environment variable to `delete`. This will make Pinchflat run in rollback journal mode which is less performant but should work on network shares. +Changing this setting from WAL to `delete` on an existing Pinchflat instance could, conceivably, result in data loss. Only change this setting if you know what you're doing, why this is important, and are okay with possible data loss or DB corruption. Backup your database first! + If you change this setting and it works well for you, please leave a comment on [#137](https://github.com/kieraneglin/pinchflat/issues/137)! Doubly so if it does _not_ work well. ## EFF donations From 25aaef7da435b3de49864725bea533daa6d2c8c5 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 21:43:15 -0700 Subject: [PATCH 011/240] Fixed bug with redownloading not forcing download of the video --- lib/pinchflat/downloading/download_option_builder.ex | 3 ++- test/pinchflat/downloading/download_option_builder_test.exs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 9e3a5c4..dec34a7 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -41,7 +41,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end defp default_options do - [:no_progress, :windows_filenames] + # Add force-overwrites to make sure redownloading works + [:no_progress, :windows_filenames, :force_overwrites] end defp subtitle_options(media_profile) do diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 22ca4b2..a7a30cb 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -39,6 +39,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert :no_progress in res assert :windows_filenames in res + assert :force_overwrites in res end end From 96c65012cae9f28b4f28476232ed58ae7e61675a Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 21:59:01 -0700 Subject: [PATCH 012/240] Bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 2e7cec7..1f281fb 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.11", + version: "0.1.12", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From e984c0529889797099c4361a86c5ffd3ef4f0aba Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 10 Apr 2024 22:02:19 -0700 Subject: [PATCH 013/240] [Enhancement] Allow overriding output templates on a per-source basis (#179) * Added output path override to table and download option builder * Added output template override to UI --- .../downloading/download_option_builder.ex | 7 +- lib/pinchflat/profiles/media_profile.ex | 3 +- lib/pinchflat/sources/source.ex | 4 + lib/pinchflat/sources/sources.ex | 13 ++ .../controllers/sources/source_html.ex | 16 ++ .../sources/source_html/source_form.html.heex | 212 ++++++++++-------- ...1154245_add_output_template_to_sources.exs | 9 + .../download_option_builder_test.exs | 38 ++++ test/pinchflat/sources_test.exs | 22 ++ 9 files changed, 227 insertions(+), 97 deletions(-) create mode 100644 priv/repo/migrations/20240411154245_add_output_template_to_sources.exs diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index dec34a7..180c4bc 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -3,6 +3,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do Builds the options for yt-dlp to download media based on the given media profile. """ + alias Pinchflat.Sources alias Pinchflat.Sources.Source alias Pinchflat.Media.MediaItem alias Pinchflat.Downloading.OutputPathBuilder @@ -30,12 +31,12 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do @doc """ Builds the output path for yt-dlp to download media based on the given source's - media profile. + media profile. Uses the source's override output path template if it exists. Returns binary() """ def build_output_path_for(%Source{} = source_with_preloads) do - output_path_template = source_with_preloads.media_profile.output_path_template + output_path_template = Sources.output_path_template(source_with_preloads) build_output_path(output_path_template, source_with_preloads) end @@ -184,7 +185,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do # It's dependent on the output_path_template being a string ending `.{{ ext }}` # (or equivalent), but that's validated by the MediaProfile schema. defp determine_thumbnail_location(media_item_with_preloads) do - output_path_template = media_item_with_preloads.source.media_profile.output_path_template + output_path_template = Sources.output_path_template(media_item_with_preloads.source) output_path_template |> String.split(~r{\.}, include_captures: true) diff --git a/lib/pinchflat/profiles/media_profile.ex b/lib/pinchflat/profiles/media_profile.ex index 0b351a9..be6c6a0 100644 --- a/lib/pinchflat/profiles/media_profile.ex +++ b/lib/pinchflat/profiles/media_profile.ex @@ -80,7 +80,8 @@ defmodule Pinchflat.Profiles.MediaProfile do |> unique_constraint(:name) end - defp ext_regex do + @doc false + def ext_regex do ~r/\.({{ ?ext ?}}|%\( ?ext ?\)[sS])$/ end end diff --git a/lib/pinchflat/sources/source.ex b/lib/pinchflat/sources/source.ex index c2ef216..a02c8c0 100644 --- a/lib/pinchflat/sources/source.ex +++ b/lib/pinchflat/sources/source.ex @@ -32,6 +32,7 @@ defmodule Pinchflat.Sources.Source do retention_period_days title_filter_regex media_profile_id + output_path_template_override )a # Expensive API calls are made when a source is inserted/updated so @@ -76,6 +77,7 @@ defmodule Pinchflat.Sources.Source do field :retention_period_days, :integer field :original_url, :string field :title_filter_regex, :string + field :output_path_template_override, :string field :series_directory, :string field :nfo_filepath, :string @@ -109,6 +111,8 @@ defmodule Pinchflat.Sources.Source do |> dynamic_default(:uuid, fn _ -> Ecto.UUID.generate() end) |> validate_required(required_fields) |> validate_number(:retention_period_days, greater_than_or_equal_to: 0) + # Ensures it ends with `.{{ ext }}` or `.%(ext)s` or similar (with a little wiggle room) + |> validate_format(:output_path_template_override, MediaProfile.ext_regex(), message: "must end with .{{ ext }}") |> cast_assoc(:metadata, with: &SourceMetadata.changeset/2, required: false) |> unique_constraint([:collection_id, :media_profile_id, :title_filter_regex], error_key: :original_url) end diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index 27d16b8..b57a628 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -19,6 +19,19 @@ defmodule Pinchflat.Sources do alias Pinchflat.SlowIndexing.SlowIndexingHelpers alias Pinchflat.Metadata.SourceMetadataStorageWorker + @doc """ + Returns the relevant output path template for a source. + Pulls from the source's override if present, otherwise uses the media profile's. + + Returns binary() + """ + def output_path_template(source) do + source = Repo.preload(source, :media_profile) + media_profile = source.media_profile + + source.output_path_template_override || media_profile.output_path_template + end + @doc """ Returns the list of sources. Returns [%Source{}, ...] """ diff --git a/lib/pinchflat_web/controllers/sources/source_html.ex b/lib/pinchflat_web/controllers/sources/source_html.ex index b93b754..a5a7545 100644 --- a/lib/pinchflat_web/controllers/sources/source_html.ex +++ b/lib/pinchflat_web/controllers/sources/source_html.ex @@ -28,4 +28,20 @@ defmodule PinchflatWeb.Sources.SourceHTML do def rss_feed_url(conn, source) do url(conn, ~p"/sources/#{source.uuid}/feed") <> ".xml" end + + def output_path_template_override_placeholders(media_profiles) do + media_profiles + |> Enum.map(&{&1.id, &1.output_path_template}) + |> Map.new() + |> Phoenix.json_library().encode!() + end + + def output_path_template_override_help do + help_button_classes = "underline decoration-bodydark decoration-1 hover:decoration-white cursor-pointer" + help_button = ~s{Click here} + + """ + Must end with .{{ ext }}. Same rules as Media Profile output path templates. #{help_button} to load your media profile's output template + """ + end end diff --git a/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex index a36e2d8..33ea259 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/source_form.html.heex @@ -9,113 +9,139 @@ Oops, something went wrong! Please check the errors below. -
    -

    - General Options -

    - - Editing Mode: - -
    +
    +
    +

    + General Options +

    + + Editing Mode: + +
    - <.input - field={f[:custom_name]} - type="text" - label="Custom Name" - help="Something descriptive. Does not impact indexing or downloading" - /> - - <.input field={f[:original_url]} type="text" label="Source URL" help="URL of a channel or playlist (required)" /> - - <.input - field={f[:media_profile_id]} - options={Enum.map(@media_profiles, &{&1.name, &1.id})} - type="select" - label="Media Profile" - help="Sets your preferences for what media to look for and how to store it" - /> - -

    - Indexing Options -

    - -
    <.input - field={f[:index_frequency_minutes]} - options={friendly_index_frequencies()} - type="select" - label="Index Frequency" - x-bind:disabled="fastIndexingEnabled == true" - x-init="$watch('fastIndexingEnabled', v => v && ($el.value = 30 * 24 * 60))" - help="Indexing is the process of checking for media to download. Sets the time between one index of this source finishing and the next one starting" + field={f[:custom_name]} + type="text" + label="Custom Name" + help="Something descriptive. Does not impact indexing or downloading" /> -
    + <.input field={f[:original_url]} type="text" label="Source URL" help="URL of a channel or playlist (required)" /> + + <.input + field={f[:media_profile_id]} + options={Enum.map(@media_profiles, &{&1.name, &1.id})} + type="select" + label="Media Profile" + help="Sets your preferences for what media to look for and how to store it" + x-model.fill="mediaProfileId" + /> + +

    + Indexing Options +

    + +
    <.input - field={f[:fast_index]} - type="toggle" - label="Use Fast Indexing" - label_suffix="(pro)" - help="Experimental. Overrides 'Index Frequency'. Recommended for large channels that upload frequently. Does not work with private playlists. See below for more info" - x-init=" + field={f[:index_frequency_minutes]} + options={friendly_index_frequencies()} + type="select" + label="Index Frequency" + x-bind:disabled="fastIndexingEnabled == true" + x-init="$watch('fastIndexingEnabled', v => v && ($el.value = 30 * 24 * 60))" + help="Indexing is the process of checking for media to download. Sets the time between one index of this source finishing and the next one starting" + /> + +
    + <.input + field={f[:fast_index]} + type="toggle" + label="Use Fast Indexing" + label_suffix="(pro)" + help="Experimental. Overrides 'Index Frequency'. Recommended for large channels that upload frequently. Does not work with private playlists. See below for more info" + x-init=" // `enabled` is the data attribute that the toggle uses internally fastIndexingEnabled = enabled $watch('enabled', value => fastIndexingEnabled = !!value) " - /> -
    -
    + /> +
    +
    -

    - Downloading Options -

    - - <.input - field={f[:download_media]} - type="toggle" - label="Download Media" - help="Unchecking still indexes media but it won't be downloaded until you enable this option" - /> - - <.input - field={f[:download_cutoff_date]} - type="text" - label="Download Cutoff Date" - placeholder="YYYY-MM-DD" - maxlength="10" - pattern="((?:19|20)[0-9][0-9])-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])" - title="YYYY-MM-DD" - help="Only download media uploaded after this date. Leave blank to download all media. Must be in YYYY-MM-DD format" - /> - - <.input - field={f[:retention_period_days]} - type="number" - label="Retention Period (days)" - min="0" - help="Days between when media is *downloaded* and when it's deleted. Leave blank to keep media indefinitely" - /> - -

    - Advanced Options + Downloading Options

    -

    - Tread carefully -

    <.input - field={f[:title_filter_regex]} - type="text" - label="Title Filter Regex" - placeholder="(?i)^How to Bike$" - help="A PCRE-compatible regex. Only media with titles that match this regex will be downloaded. Look up 'SQLean Regex docs' for more" + field={f[:download_media]} + type="toggle" + label="Download Media" + help="Unchecking still indexes media but it won't be downloaded until you enable this option" /> + + <.input + field={f[:download_cutoff_date]} + type="text" + label="Download Cutoff Date" + placeholder="YYYY-MM-DD" + maxlength="10" + pattern="((?:19|20)[0-9][0-9])-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])" + title="YYYY-MM-DD" + help="Only download media uploaded after this date. Leave blank to download all media. Must be in YYYY-MM-DD format" + /> + + <.input + field={f[:retention_period_days]} + type="number" + label="Retention Period (days)" + min="0" + help="Days between when media is *downloaded* and when it's deleted. Leave blank to keep media indefinitely" + /> + +
    +

    + Advanced Options +

    +

    + Tread carefully +

    + + <.input + field={f[:title_filter_regex]} + type="text" + label="Title Filter Regex" + placeholder="(?i)^How to Bike$" + help="A PCRE-compatible regex. Only media with titles that match this regex will be downloaded. Look up 'SQLean Regex docs' for more" + /> + +
    + <.input + field={f[:output_path_template_override]} + type="text" + inputclass="font-mono" + label="Output path template override" + help={output_path_template_override_help()} + html_help={true} + x-bind:placeholder="placeholders[mediaProfileId]" + x-model.fill="inputValue" + /> +
    +
    + + <.button class="my-10 sm:mb-7.5 w-full sm:w-auto" rounding="rounded-lg">Save Source + +
    + <.fast_indexing_help /> +
    - - <.button class="my-10 sm:mb-7.5 w-full sm:w-auto" rounding="rounded-lg">Save Source - -
    - <.fast_indexing_help /> -
    diff --git a/priv/repo/migrations/20240411154245_add_output_template_to_sources.exs b/priv/repo/migrations/20240411154245_add_output_template_to_sources.exs new file mode 100644 index 0000000..8237996 --- /dev/null +++ b/priv/repo/migrations/20240411154245_add_output_template_to_sources.exs @@ -0,0 +1,9 @@ +defmodule Pinchflat.Repo.Migrations.AddOutputTemplateToSources do + use Ecto.Migration + + def change do + alter table(:sources) do + add :output_path_template_override, :string + end + end +end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index a7a30cb..5314f89 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -4,6 +4,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do import Pinchflat.SourcesFixtures import Pinchflat.ProfilesFixtures + alias Pinchflat.Sources alias Pinchflat.Profiles alias Pinchflat.Utils.FilesystemUtils alias Pinchflat.Downloading.DownloadOptionBuilder @@ -31,6 +32,20 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert {:output, "/tmp/test/media/#{media_item.source.custom_name}.%(ext)s"} in res end + + test "uses source's output override if present", %{media_item: media_item} do + source = media_item.source + {:ok, _} = Sources.update_source(source, %{output_path_template_override: "override.%(ext)s"}) + + media_item = + media_item + |> Repo.reload() + |> Repo.preload(source: :media_profile) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:output, "/tmp/test/media/override.%(ext)s"} in res + end end describe "build/1 when testing default options" do @@ -135,6 +150,20 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert {:output, "thumbnail:/tmp/test/media/%(title)S-thumb.%(ext)s"} in res end + test "appends -thumb to source's output path override, if present", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true}) + {:ok, _} = Sources.update_source(media_item.source, %{output_path_template_override: "override.%(ext)s"}) + + media_item = + media_item + |> Repo.reload() + |> Repo.preload(source: :media_profile) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:output, "thumbnail:/tmp/test/media/override-thumb.%(ext)s"} in res + end + test "converts thumbnail to jpg when download_thumbnail is true", %{media_item: media_item} do media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true}) @@ -359,6 +388,15 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert path == "/tmp/test/media/%(title)S.%(ext)s" end + + test "uses source's output override if present", %{media_item: media_item} do + source = media_item.source + {:ok, source} = Sources.update_source(source, %{output_path_template_override: "override.%(ext)s"}) + + path = DownloadOptionBuilder.build_output_path_for(source) + + assert path == "/tmp/test/media/override.%(ext)s" + end end defp update_media_profile_attribute(media_item_with_preloads, attrs) do diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 175a945..3b8091d 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -35,6 +35,28 @@ defmodule Pinchflat.SourcesTest do end end + describe "output_path_template/1" do + test "returns the source's override if present" do + source = source_fixture(%{output_path_template_override: "/override/{{ title }}.{{ ext }}"}) + + assert Sources.output_path_template(source) == "/override/{{ title }}.{{ ext }}" + end + + test "returns the media profile's template if no override is present" do + media_profile = media_profile_fixture(%{output_path_template: "/profile/{{ title }}.{{ ext }}"}) + source = source_fixture(%{media_profile_id: media_profile.id}) + + assert Sources.output_path_template(source) == "/profile/{{ title }}.{{ ext }}" + end + + test "Treats empty strings as being blank" do + media_profile = media_profile_fixture(%{output_path_template: "/profile/{{ title }}.{{ ext }}"}) + source = source_fixture(%{media_profile_id: media_profile.id, output_path_template_override: " "}) + + assert Sources.output_path_template(source) == "/profile/{{ title }}.{{ ext }}" + end + end + describe "list_sources/0" do test "it returns all sources" do source = source_fixture() From c36e33e1fdb2d111ab2cfab2fd7a9556e6b5b18d Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 10 Apr 2024 22:13:19 -0700 Subject: [PATCH 014/240] [Housekeeping] Close system port when jobs are cancelled (#182) * Created a test setup that works * Refactored test setup into real-world fixes --- dev.Dockerfile | 2 +- lib/pinchflat/notifications/command_runner.ex | 2 +- lib/pinchflat/utils/cli_utils.ex | 17 ++++++++++++ lib/pinchflat/yt_dlp/command_runner.ex | 2 +- priv/cmd_wrapper.sh | 26 +++++++++++++++++++ test/pinchflat/utils/cli_utils_test.exs | 6 +++++ 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100755 priv/cmd_wrapper.sh diff --git a/dev.Dockerfile b/dev.Dockerfile index bbf2683..9acbd3c 100644 --- a/dev.Dockerfile +++ b/dev.Dockerfile @@ -8,7 +8,7 @@ FROM ${DEV_IMAGE} # Install debian packages RUN apt-get update -qq RUN apt-get install -y inotify-tools ffmpeg curl git openssh-client \ - python3 python3-pip python3-setuptools python3-wheel python3-dev locales + python3 python3-pip python3-setuptools python3-wheel python3-dev locales procps # Install nodejs RUN curl -sL https://deb.nodesource.com/setup_20.x -o nodesource_setup.sh diff --git a/lib/pinchflat/notifications/command_runner.ex b/lib/pinchflat/notifications/command_runner.ex index 8f1a806..5f51823 100644 --- a/lib/pinchflat/notifications/command_runner.ex +++ b/lib/pinchflat/notifications/command_runner.ex @@ -45,7 +45,7 @@ defmodule Pinchflat.Notifications.CommandRunner do """ @impl AppriseCommandRunner def version do - case System.cmd(backend_executable(), ["--version"]) do + case CliUtils.wrap_cmd(backend_executable(), ["--version"]) do {output, 0} -> output |> String.split(~r{\r?\n}) diff --git a/lib/pinchflat/utils/cli_utils.ex b/lib/pinchflat/utils/cli_utils.ex index 4e2a488..8d6c1d4 100644 --- a/lib/pinchflat/utils/cli_utils.ex +++ b/lib/pinchflat/utils/cli_utils.ex @@ -5,6 +5,23 @@ defmodule Pinchflat.Utils.CliUtils do alias Pinchflat.Utils.StringUtils + @doc """ + Wraps a command in a shell script that will terminate + the command if stdin is closed. Useful for stopping + commands if the job runner is cancelled. + + Delegates to `System.cmd/3` and any options/output + are passed through. + + Returns {binary(), integer()} + """ + def wrap_cmd(command, args, opts \\ []) do + wrapper_command = Path.join(:code.priv_dir(:pinchflat), "cmd_wrapper.sh") + actual_command = [command] ++ args + + System.cmd(wrapper_command, actual_command, opts) + end + @doc """ Parses a list of command options into a list of strings suitable for passing to `System.cmd/3`. diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index ed13d33..572c3ac 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -37,7 +37,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do formatted_command_opts = [url] ++ CliUtils.parse_options(all_opts) Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}") - case System.cmd(command, formatted_command_opts, stderr_to_stdout: true) do + case CliUtils.wrap_cmd(command, formatted_command_opts, stderr_to_stdout: true) do {_, 0} -> # IDEA: consider deleting the file after reading it. It's in the tmp dir, so it's not # a huge deal, but it's still a good idea to clean up after ourselves. diff --git a/priv/cmd_wrapper.sh b/priv/cmd_wrapper.sh new file mode 100755 index 0000000..fe5df29 --- /dev/null +++ b/priv/cmd_wrapper.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +# This script is a wrapper for other programs +# that ensures they are killed when stdin closes +# (eg: a job terminates) + +# Start the program in the background +exec "$@" & +pid1=$! + +# Silence warnings from here on +exec >/dev/null 2>&1 + +# Read from stdin in the background and +# kill running program when stdin closes +exec 0<&0 $( + while read; do :; done + kill -KILL $pid1 +) & +pid2=$! + +# Clean up +wait $pid1 +ret=$? +kill -KILL $pid2 +exit $ret diff --git a/test/pinchflat/utils/cli_utils_test.exs b/test/pinchflat/utils/cli_utils_test.exs index b053158..0c676b7 100644 --- a/test/pinchflat/utils/cli_utils_test.exs +++ b/test/pinchflat/utils/cli_utils_test.exs @@ -3,6 +3,12 @@ defmodule Pinchflat.Utils.CliUtilsTest do alias Pinchflat.Utils.CliUtils + describe "wrap_cmd/3" do + test "delegates to System.cmd/3" do + assert {"output\n", 0} = CliUtils.wrap_cmd("echo", ["output"]) + end + end + describe "parse_options/1" do test "it converts symbol k-v arg keys to kebab case" do assert ["--buffer-size", "1024"] = CliUtils.parse_options(buffer_size: 1024) From f2a7463ff32f510d2f981f5b0e165c4bf0a31322 Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 10 Apr 2024 22:22:17 -0700 Subject: [PATCH 015/240] [Enhancement] Improve support for 4k videos with Plex (#181) * Added WIP 4k MP4 fix [skip ci] * Added tests for new remux options --- .../downloading/download_option_builder.ex | 14 ++++++++------ .../downloading/download_option_builder_test.exs | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 180c4bc..69fd061 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -106,16 +106,18 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end defp quality_options(media_profile) do - video_codec_options = "+codec:avc:m4a" + video_codec_option = fn res -> + [format_sort: "res:#{res},+codec:avc:m4a", remux_video: "mp4"] + end case media_profile.preferred_resolution do # Also be aware that :audio disabled all embedding options for subtitles :audio -> [:extract_audio, format: "bestaudio[ext=m4a]"] - :"360p" -> [format_sort: "res:360,#{video_codec_options}"] - :"480p" -> [format_sort: "res:480,#{video_codec_options}"] - :"720p" -> [format_sort: "res:720,#{video_codec_options}"] - :"1080p" -> [format_sort: "res:1080,#{video_codec_options}"] - :"2160p" -> [format_sort: "res:2160,#{video_codec_options}"] + :"360p" -> video_codec_option.("360") + :"480p" -> video_codec_option.("480") + :"720p" -> video_codec_option.("720") + :"1080p" -> video_codec_option.("1080") + :"2160p" -> video_codec_option.("2160") end end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 5314f89..d5c4dab 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -240,6 +240,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert {:ok, res} = DownloadOptionBuilder.build(media_item) assert {:format_sort, "res:#{resolution},+codec:avc:m4a"} in res + assert {:remux_video, "mp4"} in res end) end @@ -250,6 +251,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert :extract_audio in res assert {:format, "bestaudio[ext=m4a]"} in res + + refute {:remux_video, "mp4"} in res end end From 8ca19ba076c3faf8ab9402d09708ba90e8f40e25 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 10 Apr 2024 22:28:02 -0700 Subject: [PATCH 016/240] Bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 1f281fb..3f13148 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.12", + version: "0.1.13", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From b2e5e9b880d6a6c0fe9e2cc34d65756eddc7ac34 Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 16 Apr 2024 13:47:56 -0700 Subject: [PATCH 017/240] Resolved bug where non-pending media would notify when fast indexing (#187) --- .../downloading/downloading_helpers.ex | 21 ++++ .../fast_indexing/fast_indexing_helpers.ex | 69 +++++------- .../fast_indexing/fast_indexing_worker.ex | 33 ++++-- .../fast_indexing/media_indexing_worker.ex | 69 ------------ .../media_collection_indexing_worker.ex | 5 +- .../slow_indexing/slow_indexing_helpers.ex | 8 +- lib/pinchflat/sources/sources.ex | 1 - .../downloading/downloading_helpers_test.exs | 41 +++++++ .../fast_indexing_helpers_test.exs | 102 +++++++----------- .../fast_indexing_worker_test.exs | 30 ++++++ .../media_indexing_worker_test.exs | 61 ----------- .../slow_indexing_helpers_test.exs | 3 +- test/pinchflat/sources_test.exs | 6 +- 13 files changed, 190 insertions(+), 259 deletions(-) delete mode 100644 lib/pinchflat/fast_indexing/media_indexing_worker.ex delete mode 100644 test/pinchflat/fast_indexing/media_indexing_worker_test.exs diff --git a/lib/pinchflat/downloading/downloading_helpers.ex b/lib/pinchflat/downloading/downloading_helpers.ex index 1e61545..a7f64e3 100644 --- a/lib/pinchflat/downloading/downloading_helpers.ex +++ b/lib/pinchflat/downloading/downloading_helpers.ex @@ -7,9 +7,11 @@ defmodule Pinchflat.Downloading.DownloadingHelpers do require Logger + alias Pinchflat.Repo alias Pinchflat.Media alias Pinchflat.Tasks alias Pinchflat.Sources.Source + alias Pinchflat.Media.MediaItem alias Pinchflat.Downloading.MediaDownloadWorker @doc """ @@ -43,4 +45,23 @@ defmodule Pinchflat.Downloading.DownloadingHelpers do |> Media.list_pending_media_items_for() |> Enum.each(&Tasks.delete_pending_tasks_for/1) end + + @doc """ + Takes a single media item and enqueues a download job if the media should be + downloaded, based on the source's download settings and whether media is + considered pending. + + Returns {:ok, %Task{}} | {:error, :should_not_download} | {:error, any()} + """ + def kickoff_download_if_pending(%MediaItem{} = media_item) do + media_item = Repo.preload(media_item, :source) + + if media_item.source.download_media && Media.pending_download?(media_item) do + Logger.info("Kicking off download for media item ##{media_item.id} (#{media_item.media_id})") + + MediaDownloadWorker.kickoff_with_task(media_item) + else + {:error, :should_not_download} + end + end end diff --git a/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex b/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex index b9d5a44..b12eff2 100644 --- a/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex +++ b/lib/pinchflat/fast_indexing/fast_indexing_helpers.ex @@ -5,64 +5,45 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpers do Many of these methods are made to be kickoff or be consumed by workers. """ + require Logger + alias Pinchflat.Repo alias Pinchflat.Media alias Pinchflat.Sources.Source alias Pinchflat.Media.MediaQuery alias Pinchflat.FastIndexing.YoutubeRss - alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.FastIndexing.MediaIndexingWorker + alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.YtDlp.Media, as: YtDlpMedia @doc """ - Fetches new media IDs from a source's YouTube RSS feed and kicks off indexing tasks - for any new media items. See comments in `MediaIndexingWorker` for more info on the + Fetches new media IDs from a source's YouTube RSS feed, indexes them, and kicks off downloading + tasks for any pending media items. See comments in `FastIndexingWorker` for more info on the order of operations and how this fits into the indexing process. - Despite the similar name to `kickoff_fast_indexing_task`, this does work differently. - `kickoff_fast_indexing_task` starts a task that _calls_ this function whereas this - function starts individual indexing tasks for each new media item. I think it does - make sense grammatically, but I could see how that's confusing. - - Returns [binary()] where each binary is the media ID of a new media item. + Returns [%MediaItem{}] where each item is a new media item that was created _but not necessarily + downloaded_. """ - def kickoff_indexing_tasks_from_youtube_rss_feed(%Source{} = source) do + def kickoff_download_tasks_from_youtube_rss_feed(%Source{} = source) do {:ok, media_ids} = YoutubeRss.get_recent_media_ids_from_rss(source) existing_media_items = list_media_items_by_media_id_for(source, media_ids) new_media_ids = media_ids -- Enum.map(existing_media_items, & &1.media_id) - Enum.each(new_media_ids, fn media_id -> - url = "https://www.youtube.com/watch?v=#{media_id}" + maybe_new_media_items = + Enum.map(new_media_ids, fn media_id -> + case create_media_item_from_media_id(source, media_id) do + {:ok, media_item} -> + media_item - MediaIndexingWorker.kickoff_with_task(source, url) - end) - - new_media_ids - end - - @doc """ - Indexes a single media item for a source and enqueues a download job if the - media should be downloaded. This method creates the media item record so it's - the one-stop-shop for adding a media item (and possibly downloading it) just - by a URL and source. - - Returns {:ok, media_item} | {:error, any()} - """ - def index_and_enqueue_download_for_media_item(%Source{} = source, url) do - maybe_media_item = create_media_item_from_url(source, url) - - case maybe_media_item do - {:ok, media_item} -> - if source.download_media && Media.pending_download?(media_item) do - MediaDownloadWorker.kickoff_with_task(media_item) + err -> + Logger.error("Error creating media item '#{media_id}' from URL: #{inspect(err)}") + nil end + end) - {:ok, media_item} + DownloadingHelpers.enqueue_pending_download_tasks(source) - err -> - err - end + Enum.filter(maybe_new_media_items, & &1) end defp list_media_items_by_media_id_for(source, media_ids) do @@ -72,9 +53,15 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpers do |> Repo.all() end - defp create_media_item_from_url(source, url) do - {:ok, media_attrs} = YtDlpMedia.get_media_attributes(url) + defp create_media_item_from_media_id(source, media_id) do + url = "https://www.youtube.com/watch?v=#{media_id}" - Media.create_media_item_from_backend_attrs(source, media_attrs) + case YtDlpMedia.get_media_attributes(url) do + {:ok, media_attrs} -> + Media.create_media_item_from_backend_attrs(source, media_attrs) + + err -> + err + end end end diff --git a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex index 15d287f..0f85d40 100644 --- a/lib/pinchflat/fast_indexing/fast_indexing_worker.ex +++ b/lib/pinchflat/fast_indexing/fast_indexing_worker.ex @@ -10,6 +10,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do alias __MODULE__ alias Pinchflat.Tasks + alias Pinchflat.Media alias Pinchflat.Sources alias Pinchflat.Settings alias Pinchflat.Sources.Source @@ -28,9 +29,21 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do end @doc """ - Kicks off the fast indexing process for a source, reschedules the job to run again - once complete. See `MediaCollectionIndexingWorker` and `MediaIndexingWorker` comments - for more + Similar to `MediaCollectionIndexingWorker`, but for working with RSS feeds. + `MediaCollectionIndexingWorker` should be preferred in general, but this is + useful for downloading small batches of media items via fast indexing. + + Only kicks off downloads for media that _should_ be downloaded + (ie: the source is set to download and the media matches the profile's format preferences) + + Order of operations: + 1. FastIndexingWorker (this module) periodically checks the YouTube RSS feed for new media. + with `FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed` + 2. If the above `kickoff_download_tasks_from_youtube_rss_feed` finds new media items in the RSS feed, + it indexes them with a yt-dlp call to create the media item records then kicks off downloading + tasks (MediaDownloadWorker) for any new media items _that should be downloaded_. + 3. Once downloads are kicked off, this worker sends a notification to the apprise server if applicable + then reschedules itself to run again in the future. Returns :ok | {:ok, :job_exists} | {:ok, %Task{}} """ @@ -39,7 +52,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do source = Sources.get_source!(source_id) if source.fast_index do - perform_indexing_and_notification(source) + perform_indexing_and_send_notification(source) reschedule_indexing(source) else :ok @@ -49,11 +62,17 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorker do Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end - defp perform_indexing_and_notification(source) do + defp perform_indexing_and_send_notification(source) do apprise_server = Settings.get!(:apprise_server) - new_media_items = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) - SourceNotifications.send_new_media_notification(apprise_server, source, length(new_media_items)) + new_media_items = + source + |> FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed() + |> Enum.filter(&Media.pending_download?(&1)) + + if source.download_media do + SourceNotifications.send_new_media_notification(apprise_server, source, length(new_media_items)) + end end defp reschedule_indexing(source) do diff --git a/lib/pinchflat/fast_indexing/media_indexing_worker.ex b/lib/pinchflat/fast_indexing/media_indexing_worker.ex deleted file mode 100644 index c4da1e1..0000000 --- a/lib/pinchflat/fast_indexing/media_indexing_worker.ex +++ /dev/null @@ -1,69 +0,0 @@ -defmodule Pinchflat.FastIndexing.MediaIndexingWorker do - @moduledoc false - - use Oban.Worker, - queue: :media_indexing, - unique: [period: :infinity, states: [:available, :scheduled, :retryable]], - tags: ["media_source", "media_indexing"] - - require Logger - - alias __MODULE__ - alias Pinchflat.Tasks - alias Pinchflat.Sources - alias Pinchflat.FastIndexing.FastIndexingHelpers - - @doc """ - Starts the fast media indexing worker and creates a task for the source. - - Returns {:ok, %Task{}} | {:error, :duplicate_job} | {:error, %Ecto.Changeset{}} - """ - def kickoff_with_task(source, media_url, opts \\ []) do - %{id: source.id, media_url: media_url} - |> MediaIndexingWorker.new(opts) - |> Tasks.create_job_with_task(source) - end - - @doc """ - Similar to `MediaCollectionIndexingWorker`, but for individual media items. - Does not reschedule or check anything to do with a source's indexing - frequency - only collects initial metadata then kicks off a download. - `MediaCollectionIndexingWorker` should be preferred in general, but this is - useful for downloading one-off media items based on a URL (like for fast indexing). - - Only downloads media that _should_ be downloaded (ie: the source is set to download - and the media matches the profile's format preferences) - - Order of operations: - 1. FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed/1 (which is running - in its own worker) periodically checks the YouTube RSS feed for new media - 2. If new media is found, it enqueues a MediaIndexingWorker (this module) for each new media - item - 3. This worker fetches the media metadata and uses that to determine if it should be - downloaded. If so, it enqueues a MediaDownloadWorker - - Each is a worker because they all either need to be scheduled periodically or call out to - an external service and will be long-running. They're split into different jobs to separate - retry logic for each step and allow us to better optimize various queues (eg: the indexing - steps can keep running while the slow download steps are worked through). - - Returns :ok - """ - @impl Oban.Worker - def perform(%Oban.Job{args: %{"id" => source_id, "media_url" => media_url}}) do - source = Sources.get_source!(source_id) - - case FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, media_url) do - {:ok, media_item} -> - Logger.debug("Indexed and enqueued download for url: #{media_url} (media item: #{media_item.id})") - - {:error, reason} -> - Logger.debug("Failed to index and enqueue download for url: #{media_url} (reason: #{inspect(reason)})") - end - - :ok - rescue - Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") - Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") - end -end diff --git a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex index 0e982eb..678a9a5 100644 --- a/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex +++ b/lib/pinchflat/slow_indexing/media_collection_indexing_worker.ex @@ -61,9 +61,8 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorker do 5. If the source uses fast indexing, that job is kicked off as well. It uses RSS to run a smaller, faster, and more frequent index. That job handles rescheduling itself but largely has a similar behaviour to this - job in that it kicks off index and maybe download jobs. The biggest difference - is that an index job is kicked off _for each new media item_ as opposed - to one larger index job. Check out `MediaIndexingWorker` comments for more. + job in that it runs and index and maybe kicks off media download jobs. + Check out `FastIndexingWorker` comments for more. 6. If the job reschedules, the cycle from step 3 repeats until the heat death of the universe. The user changing things like the index frequency can dequeue or reschedule jobs as well diff --git a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex index b6b118a..2434b92 100644 --- a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex +++ b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex @@ -16,7 +16,6 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do alias Pinchflat.YtDlp.MediaCollection alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.SlowIndexing.FileFollowerServer - alias Pinchflat.Downloading.MediaDownloadWorker alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker alias Pinchflat.YtDlp.Media, as: YtDlpMedia @@ -29,7 +28,6 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers 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, job_args, job_opts) @@ -127,11 +125,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do case Media.create_media_item_from_backend_attrs(source, media_attrs) do {:ok, %MediaItem{} = media_item} -> - if source.download_media && Media.pending_download?(media_item) do - Logger.debug("FileFollowerServer Handler: Enqueuing download task for #{inspect(media_attrs)}") - - MediaDownloadWorker.kickoff_with_task(media_item) - end + DownloadingHelpers.kickoff_download_if_pending(media_item) {:error, changeset} -> changeset diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index b57a628..f9a3658 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -288,7 +288,6 @@ defmodule Pinchflat.Sources do %{index_frequency_minutes: _} -> Tasks.delete_pending_tasks_for(source, "FastIndexingWorker") - Tasks.delete_pending_tasks_for(source, "MediaIndexingWorker") Tasks.delete_pending_tasks_for(source, "MediaCollectionIndexingWorker") _ -> diff --git a/test/pinchflat/downloading/downloading_helpers_test.exs b/test/pinchflat/downloading/downloading_helpers_test.exs index b54a5af..1c3c95b 100644 --- a/test/pinchflat/downloading/downloading_helpers_test.exs +++ b/test/pinchflat/downloading/downloading_helpers_test.exs @@ -4,6 +4,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do import Mox import Pinchflat.MediaFixtures import Pinchflat.SourcesFixtures + import Pinchflat.ProfilesFixtures alias Pinchflat.Tasks alias Pinchflat.Downloading.DownloadingHelpers @@ -72,4 +73,44 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do assert [] = Tasks.list_tasks_for(media_item) end end + + describe "kickoff_download_if_pending/1" do + setup do + media_item = media_item_fixture(media_filepath: nil) + + {:ok, media_item: media_item} + end + + test "enqueues a download job", %{media_item: media_item} do + assert {:ok, _} = DownloadingHelpers.kickoff_download_if_pending(media_item) + + assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id}) + end + + test "creates and returns a download task record", %{media_item: media_item} do + assert {:ok, task} = DownloadingHelpers.kickoff_download_if_pending(media_item) + + assert [found_task] = Tasks.list_tasks_for(media_item, "MediaDownloadWorker") + assert task.id == found_task.id + end + + test "does not enqueue a download job if the source does not allow it" do + source = source_fixture(%{download_media: false}) + media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + assert {:error, :should_not_download} = DownloadingHelpers.kickoff_download_if_pending(media_item) + + refute_enqueued(worker: MediaDownloadWorker) + end + + test "does not enqueue a download job if the media item does not match the format rules" do + profile = media_profile_fixture(%{livestream_behaviour: :exclude}) + source = source_fixture(%{media_profile_id: profile.id}) + media_item = media_item_fixture(source_id: source.id, media_filepath: nil, livestream: true) + + assert {:error, :should_not_download} = DownloadingHelpers.kickoff_download_if_pending(media_item) + + refute_enqueued(worker: MediaDownloadWorker) + end + end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs index 23350c0..f065b7e 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs @@ -9,45 +9,11 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do alias Pinchflat.Tasks alias Pinchflat.Media.MediaItem alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.FastIndexing.MediaIndexingWorker alias Pinchflat.FastIndexing.FastIndexingHelpers setup :verify_on_exit! - @media_url "https://www.youtube.com/watch?v=test_1" - - describe "kickoff_indexing_tasks_from_youtube_rss_feed/1" do - setup do - {:ok, [source: source_fixture()]} - end - - test "enqueues a new worker for each new media_id in the source's RSS feed", %{source: source} do - expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) - - assert [_] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) - - assert [worker] = all_enqueued(worker: MediaIndexingWorker) - assert worker.args["id"] == source.id - assert worker.args["media_url"] == "https://www.youtube.com/watch?v=test_1" - end - - test "does not enqueue a new worker for the source's media IDs we already know about", %{source: source} do - expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) - media_item_fixture(source_id: source.id, media_id: "test_1") - - assert [] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) - - refute_enqueued(worker: MediaIndexingWorker) - end - - test "returns the IDs of the found media items", %{source: source} do - expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) - - assert ["test_1"] = FastIndexingHelpers.kickoff_indexing_tasks_from_youtube_rss_feed(source) - end - end - - describe "index_and_enqueue_download_for_media_item/2" do + describe "kickoff_download_tasks_from_youtube_rss_feed/1" do setup do stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, media_attributes_return_fixture()} @@ -56,41 +22,50 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do {:ok, [source: source_fixture()]} end - test "creates a new media item based on the URL", %{source: source} do - assert Repo.aggregate(MediaItem, :count) == 0 - assert {:ok, _} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) - assert Repo.aggregate(MediaItem, :count) == 1 + test "enqueues a new worker for each new media_id in the source's RSS feed", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + assert [media_item] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + + assert [worker] = all_enqueued(worker: MediaDownloadWorker) + assert worker.args["id"] == media_item.id end - test "won't duplicate media_items based on media_id and source", %{source: source} do - assert {:ok, mi_1} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) - assert {:ok, mi_2} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) + test "does not enqueue a new worker for the source's media IDs we already know about", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + media_item_fixture(source_id: source.id, media_id: "test_1") - assert Repo.aggregate(MediaItem, :count) == 1 - assert mi_1.id == mi_2.id - end - - test "enqueues a download job", %{source: source} do - assert {:ok, media_item} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) - - assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id}) - end - - test "creates a download task record", %{source: source} do - assert {:ok, media_item} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) - - assert [_] = Tasks.list_tasks_for(media_item, "MediaDownloadWorker") - end - - test "does not enqueue a download job if the source does not allow it" do - source = source_fixture(%{download_media: false}) - - assert {:ok, _} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) + assert [] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) refute_enqueued(worker: MediaDownloadWorker) end + test "returns the found media items", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + assert [%MediaItem{}] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + end + + test "does not enqueue a download job if the source does not allow it" do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + source = source_fixture(%{download_media: false}) + + assert [%MediaItem{}] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + + refute_enqueued(worker: MediaDownloadWorker) + end + + test "creates a download task record", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + assert [media_item] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + + assert [_] = Tasks.list_tasks_for(media_item, "MediaDownloadWorker") + end + test "does not enqueue a download job if the media item does not match the format rules" do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + profile = media_profile_fixture(%{shorts_behaviour: :exclude}) source = source_fixture(%{media_profile_id: profile.id}) @@ -110,7 +85,8 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do {:ok, output} end) - assert {:ok, _media_item} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url) + assert [%MediaItem{}] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + refute_enqueued(worker: MediaDownloadWorker) end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs index 55007ba..d2772d7 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs @@ -87,6 +87,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do source = source_fixture(fast_index: true) expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, render_metadata(:media_metadata)} end) expect(AppriseRunnerMock, :run, fn servers, opts -> assert "server_1" = servers @@ -98,5 +99,34 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do perform_job(FastIndexingWorker, %{id: source.id}) end + + test "doesn't send a notification if new media is not found" do + source = source_fixture(fast_index: true) + + expect(HTTPClientMock, :get, fn _url -> {:ok, ""} end) + expect(AppriseRunnerMock, :run, 0, fn _servers, _opts -> {:ok, ""} end) + + perform_job(FastIndexingWorker, %{id: source.id}) + end + + test "doesn't send a notification if the source doesn't download media" do + source = source_fixture(fast_index: true, download_media: false) + + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, render_metadata(:media_metadata)} end) + expect(AppriseRunnerMock, :run, 0, fn _servers, _opts -> {:ok, ""} end) + + perform_job(FastIndexingWorker, %{id: source.id}) + end + + test "doesn't send a notification if the media isn't pending download" do + source = source_fixture(fast_index: true, title_filter_regex: "foobar") + + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, render_metadata(:media_metadata)} end) + expect(AppriseRunnerMock, :run, 0, fn _servers, _opts -> {:ok, ""} end) + + perform_job(FastIndexingWorker, %{id: source.id}) + end end end diff --git a/test/pinchflat/fast_indexing/media_indexing_worker_test.exs b/test/pinchflat/fast_indexing/media_indexing_worker_test.exs deleted file mode 100644 index 178e66a..0000000 --- a/test/pinchflat/fast_indexing/media_indexing_worker_test.exs +++ /dev/null @@ -1,61 +0,0 @@ -defmodule Pinchflat.FastIndexing.MediaIndexingWorkerTest do - use Pinchflat.DataCase - - import Mox - import Pinchflat.MediaFixtures - import Pinchflat.SourcesFixtures - - alias Pinchflat.Media.MediaItem - alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.FastIndexing.MediaIndexingWorker - - @media_url "https://www.youtube.com/watch?v=1234567890" - - setup :verify_on_exit! - - setup do - source = source_fixture() - - {:ok, source: source} - end - - describe "kickoff_with_task/2" do - test "starts the worker", %{source: source} do - assert [] = all_enqueued(worker: MediaIndexingWorker) - assert {:ok, _} = MediaIndexingWorker.kickoff_with_task(source, @media_url) - assert [_] = all_enqueued(worker: MediaIndexingWorker) - end - - test "attaches a task", %{source: source} do - assert {:ok, task} = MediaIndexingWorker.kickoff_with_task(source, @media_url) - assert task.source_id == source.id - end - end - - describe "perform/1" do - test "indexes the media item and saves it to the database", %{source: source} do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> - {:ok, media_attributes_return_fixture()} - end) - - before = Repo.aggregate(MediaItem, :count, :id) - perform_job(MediaIndexingWorker, %{id: source.id, media_url: @media_url}) - - assert Repo.aggregate(MediaItem, :count, :id) == before + 1 - end - - test "enqueues a download job for the media item", %{source: source} do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> - {:ok, media_attributes_return_fixture()} - end) - - perform_job(MediaIndexingWorker, %{id: source.id, media_url: @media_url}) - - assert [_] = all_enqueued(worker: MediaDownloadWorker) - end - - test "does not blow up if the record doesn't exist" do - assert :ok = perform_job(MediaDownloadWorker, %{id: 0, media_url: @media_url}) - end - end -end diff --git a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs index 80e7959..4bc72ee 100644 --- a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs +++ b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs @@ -12,7 +12,6 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do alias Pinchflat.Media.MediaItem alias Pinchflat.FastIndexing.FastIndexingWorker alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.FastIndexing.MediaIndexingWorker alias Pinchflat.SlowIndexing.SlowIndexingHelpers alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker @@ -47,7 +46,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do test "it deletes any pending media tasks for the source" do source = source_fixture() - {:ok, job} = Oban.insert(MediaIndexingWorker.new(%{"id" => source.id})) + {:ok, job} = Oban.insert(FastIndexingWorker.new(%{"id" => source.id})) task = task_fixture(source_id: source.id, job_id: job.id) assert {:ok, _} = SlowIndexingHelpers.kickoff_indexing_task(source) diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 3b8091d..d6589ff 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -13,7 +13,6 @@ defmodule Pinchflat.SourcesTest do alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.FastIndexing.FastIndexingWorker alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.FastIndexing.MediaIndexingWorker alias Pinchflat.Metadata.SourceMetadataStorageWorker alias Pinchflat.SlowIndexing.MediaCollectionIndexingWorker @@ -415,16 +414,13 @@ defmodule Pinchflat.SourcesTest do {:ok, job_1} = Oban.insert(FastIndexingWorker.new(%{"id" => source.id})) task_1 = task_fixture(source_id: source.id, job_id: job_1.id) - {:ok, job_2} = Oban.insert(MediaIndexingWorker.new(%{"id" => source.id})) + {:ok, job_2} = Oban.insert(MediaCollectionIndexingWorker.new(%{"id" => source.id})) task_2 = task_fixture(source_id: source.id, job_id: job_2.id) - {:ok, job_3} = Oban.insert(MediaCollectionIndexingWorker.new(%{"id" => source.id})) - task_3 = task_fixture(source_id: source.id, job_id: job_3.id) assert {:ok, %Source{}} = Sources.update_source(source, update_attrs) assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task_1) end assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task_2) end - assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task_3) end end test "not updating the index frequency will not re-schedule the indexing task or delete tasks" do From 47219578754e6098016726495745f98d2a9e6202 Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 16 Apr 2024 16:54:55 -0700 Subject: [PATCH 018/240] Added yt-dlp options for year formatting (#188) --- lib/pinchflat/downloading/download_option_builder.ex | 10 ++++++++-- .../downloading/download_option_builder_test.exs | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 69fd061..5dc1bb2 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -42,8 +42,14 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end defp default_options do - # Add force-overwrites to make sure redownloading works - [:no_progress, :windows_filenames, :force_overwrites] + [ + :no_progress, + :windows_filenames, + # Add force-overwrites to make sure redownloading works + :force_overwrites, + # This makes the date metadata conform to what jellyfin expects + parse_metadata: "%(upload_date>%Y-%m-%d)s:(?P.+)" + ] end defp subtitle_options(media_profile) do diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index d5c4dab..8ce35a2 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -55,6 +55,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert :no_progress in res assert :windows_filenames in res assert :force_overwrites in res + assert {:parse_metadata, "%(upload_date>%Y-%m-%d)s:(?P.+)"} in res end end From 618711691b9f08bf0545d22c82aa1da89c52a31e Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 16 Apr 2024 17:37:39 -0700 Subject: [PATCH 019/240] [Bugfix]: Misc. bugfixes 2024-04-16 (#189) * Manually installed mutagen * Stopped upgrade form from submitting on enter * Gracefully handle duplicate enqueued downloads * Update metadata thumbnail fetcher to use the best jpg available --- assets/js/app.js | 11 ++++++++++ dev.Dockerfile | 3 +++ .../metadata/metadata_file_helpers.ex | 9 +++++++- .../layouts/partials/upgrade_button_live.ex | 3 ++- .../media_items/media_item_controller.ex | 8 ++++++- selfhosted.Dockerfile | 3 +++ test/pinchflat/media_test.exs | 12 +++------- .../metadata/metadata_file_helpers_test.exs | 22 ++++++++++++------- .../media_item_controller_test.exs | 9 ++++++++ 9 files changed, 60 insertions(+), 20 deletions(-) diff --git a/assets/js/app.js b/assets/js/app.js index f161731..90fa638 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -36,6 +36,17 @@ let liveSocket = new LiveSocket('/live', Socket, { window.Alpine.clone(from, to) } } + }, + hooks: { + supressEnterSubmission: { + mounted() { + this.el.addEventListener('keypress', (event) => { + if (event.key === 'Enter') { + event.preventDefault() + } + }) + } + } } }) diff --git a/dev.Dockerfile b/dev.Dockerfile index 9acbd3c..c6e6132 100644 --- a/dev.Dockerfile +++ b/dev.Dockerfile @@ -29,6 +29,9 @@ RUN yt-dlp -U # Download Apprise RUN python3 -m pip install -U apprise --break-system-packages +# Download Mutagen for music thumbnail generation +RUN python3 -m pip install -U mutagen --break-system-packages + # Set the locale RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && locale-gen ENV LANG en_US.UTF-8 diff --git a/lib/pinchflat/metadata/metadata_file_helpers.ex b/lib/pinchflat/metadata/metadata_file_helpers.ex index 959d1ca..a804ecd 100644 --- a/lib/pinchflat/metadata/metadata_file_helpers.ex +++ b/lib/pinchflat/metadata/metadata_file_helpers.ex @@ -54,11 +54,18 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do @doc """ Downloads and stores a thumbnail for a media item, returning the filepath. + Chooses the highest quality jpg thumbnail available. Returns binary() """ def download_and_store_thumbnail_for(database_record, metadata_map) do - thumbnail_url = metadata_map["thumbnail"] + thumbnail_url = + metadata_map["thumbnails"] + |> Enum.filter(&(&1["preference"] && String.ends_with?(&1["url"], ".jpg"))) + |> Enum.sort(&(&1["preference"] >= &2["preference"])) + |> List.first() + |> Map.get("url") + filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url)) thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url) diff --git a/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex b/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex index 959edae..17e1ac2 100644 --- a/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex +++ b/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex @@ -3,10 +3,11 @@ defmodule Pinchflat.UpgradeButtonLive do def render(assigns) do ~H""" -
    + <.input type="text" name="unlock-pro-textbox" value="" />
    + <%!-- The setTimeout is so the modal has time to disappear before it's removed --%> <.button class="w-full mt-4" type="button" diff --git a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex index d348fee..278ef23 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_controller.ex +++ b/lib/pinchflat_web/controllers/media_items/media_item_controller.ex @@ -50,7 +50,13 @@ defmodule PinchflatWeb.MediaItems.MediaItemController do 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}) + + :ok = + case MediaDownloadWorker.kickoff_with_task(media_item, %{force: true}) do + {:ok, _} -> :ok + {:error, :duplicate_job} -> :ok + err -> err + end conn |> put_flash(:info, "Download task enqueued.") diff --git a/selfhosted.Dockerfile b/selfhosted.Dockerfile index 62b2c70..fd3bd91 100644 --- a/selfhosted.Dockerfile +++ b/selfhosted.Dockerfile @@ -90,6 +90,9 @@ RUN yt-dlp -U # Download Apprise RUN python3 -m pip install -U apprise --break-system-packages +# Download Mutagen for music thumbnail generation +RUN python3 -m pip install -U mutagen --break-system-packages + # Set the locale RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && locale-gen ENV LANG en_US.UTF-8 diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index fad36c0..f972259 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -731,9 +731,7 @@ defmodule Pinchflat.MediaTest do metadata: %{ metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}), thumbnail_filepath: - MetadataFileHelpers.download_and_store_thumbnail_for(media_item, %{ - "thumbnail" => "https://example.com/thumbnail.jpg" - }) + MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata)) } } @@ -760,9 +758,7 @@ defmodule Pinchflat.MediaTest do metadata: %{ metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}), thumbnail_filepath: - MetadataFileHelpers.download_and_store_thumbnail_for(media_item, %{ - "thumbnail" => "https://example.com/thumbnail.jpg" - }) + MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata)) } } @@ -831,9 +827,7 @@ defmodule Pinchflat.MediaTest do metadata: %{ metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}), thumbnail_filepath: - MetadataFileHelpers.download_and_store_thumbnail_for(media_item, %{ - "thumbnail" => "https://example.com/thumbnail.jpg" - }) + MetadataFileHelpers.download_and_store_thumbnail_for(media_item, render_parsed_metadata(:media_metadata)) } } diff --git a/test/pinchflat/metadata/metadata_file_helpers_test.exs b/test/pinchflat/metadata/metadata_file_helpers_test.exs index 4e2c73b..554afd2 100644 --- a/test/pinchflat/metadata/metadata_file_helpers_test.exs +++ b/test/pinchflat/metadata/metadata_file_helpers_test.exs @@ -54,13 +54,11 @@ defmodule Pinchflat.Metadata.MetadataFileHelpersTest do describe "download_and_store_thumbnail_for/2" do setup do # This tests that the HTTP endpoint is being called with every test - expect(HTTPClientMock, :get, fn url, _headers, _opts -> - assert url =~ "example.com" - + expect(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, "thumbnail data"} end) - metadata = %{"thumbnail" => "example.com/thumbnail.jpg"} + metadata = render_parsed_metadata(:media_metadata) {:ok, %{metadata: metadata}} end @@ -68,7 +66,7 @@ defmodule Pinchflat.Metadata.MetadataFileHelpersTest do test "returns the filepath", %{media_item: media_item, metadata: metadata} do filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata) - assert filepath =~ ~r{/media_items/#{media_item.id}/thumbnail.jpg} + assert filepath =~ ~r{/media_items/#{media_item.id}/maxresdefault.jpg} end test "creates folder structure based on passed record", %{media_item: media_item, metadata: metadata} do @@ -77,11 +75,19 @@ defmodule Pinchflat.Metadata.MetadataFileHelpersTest do assert File.exists?(Path.dirname(filepath)) end - test "the filename and extension is based on the URL", %{media_item: media_item} do - metadata = %{"thumbnail" => "example.com/maxres.webp"} + test "chooses the highest preference jpg thumbnail available", %{media_item: media_item} do + metadata = %{ + "thumbnails" => [ + %{"url" => "https://i.ytimg.com/vi/ABC123/img_1.jpg", "preference" => -1}, + %{"url" => "https://i.ytimg.com/vi/ABC123/img_2.jpg", "preference" => 1}, + %{"url" => "https://i.ytimg.com/vi/ABC123/img_3.jpg", "preference" => -10}, + %{"url" => "https://i.ytimg.com/vi/ABC123/img_4.webp", "preference" => 10} + ] + } + filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata) - assert Path.basename(filepath) == "maxres.webp" + assert filepath =~ ~r{/media_items/#{media_item.id}/img_2.jpg} end end diff --git a/test/pinchflat_web/controllers/media_item_controller_test.exs b/test/pinchflat_web/controllers/media_item_controller_test.exs index b0c8847..5a2dbd9 100644 --- a/test/pinchflat_web/controllers/media_item_controller_test.exs +++ b/test/pinchflat_web/controllers/media_item_controller_test.exs @@ -97,6 +97,15 @@ defmodule PinchflatWeb.MediaItemControllerTest do assert [_] = all_enqueued(worker: MediaDownloadWorker) end + test "doesn't freak out if the task is a duplicate", %{conn: conn} do + media_item = media_item_fixture() + + MediaDownloadWorker.kickoff_with_task(media_item, %{force: true}) + + 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}) From a4d5f45edcd739f22d35bc4da49138b8c89f8246 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 16 Apr 2024 17:38:59 -0700 Subject: [PATCH 020/240] Bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 3f13148..86f814d 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.13", + version: "0.1.14", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From aea40a3f92cedba0d155a00ef0bf97f4e7b1bce7 Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 17 Apr 2024 10:22:55 -0700 Subject: [PATCH 021/240] [Enhancement] Add pagination for a source's media (#190) * [WIP] added first attempt at pagination component * Hooked up pagination for downloaded media --- lib/pinchflat/utils/number_utils.ex | 16 ++++ .../custom_components/table_components.ex | 50 ++++++++++ .../controllers/sources/source_controller.ex | 33 +------ .../source_html/media_item_table_live.ex | 91 +++++++++++++++++++ .../sources/source_html/show.html.heex | 50 ++-------- test/pinchflat/utils/number_utils_test.exs | 19 ++++ .../sources/media_item_table_live_test.exs | 60 ++++++++++++ 7 files changed, 247 insertions(+), 72 deletions(-) create mode 100644 lib/pinchflat/utils/number_utils.ex create mode 100644 lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex create mode 100644 test/pinchflat/utils/number_utils_test.exs create mode 100644 test/pinchflat_web/controllers/sources/media_item_table_live_test.exs diff --git a/lib/pinchflat/utils/number_utils.ex b/lib/pinchflat/utils/number_utils.ex new file mode 100644 index 0000000..4a8404b --- /dev/null +++ b/lib/pinchflat/utils/number_utils.ex @@ -0,0 +1,16 @@ +defmodule Pinchflat.Utils.NumberUtils do + @moduledoc """ + Utility methods for working with numbers + """ + + @doc """ + Clamps a number between a minimum and maximum value + + Returns integer() | float() + """ + def clamp(num, minimum, maximum) do + num + |> max(minimum) + |> min(maximum) + end +end diff --git a/lib/pinchflat_web/components/custom_components/table_components.ex b/lib/pinchflat_web/components/custom_components/table_components.ex index fbe897c..532fba1 100644 --- a/lib/pinchflat_web/components/custom_components/table_components.ex +++ b/lib/pinchflat_web/components/custom_components/table_components.ex @@ -2,6 +2,8 @@ defmodule PinchflatWeb.CustomComponents.TableComponents do @moduledoc false use Phoenix.Component + alias PinchflatWeb.CoreComponents + @doc """ Renders a table component with the given rows and columns. @@ -50,4 +52,52 @@ defmodule PinchflatWeb.CustomComponents.TableComponents do """ end + + @doc """ + Renders simple pagination controls for a table in a liveview. + + ## Examples + + <.live_pagination_controls page_number={@page} total_pages={@total_pages} /> + """ + attr :page_number, :integer, default: 1 + attr :total_pages, :integer, default: 1 + + def live_pagination_controls(assigns) do + ~H""" + + """ + end end diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index ff12534..6e4b215 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -6,9 +6,7 @@ defmodule PinchflatWeb.Sources.SourceController do alias Pinchflat.Repo alias Pinchflat.Tasks alias Pinchflat.Sources - alias Pinchflat.MediaQuery alias Pinchflat.Sources.Source - alias Pinchflat.Media.MediaQuery alias Pinchflat.Profiles.MediaProfile alias Pinchflat.Downloading.DownloadingHelpers alias Pinchflat.SlowIndexing.SlowIndexingHelpers @@ -60,29 +58,7 @@ defmodule PinchflatWeb.Sources.SourceController do |> Tasks.list_tasks_for(nil, [:executing, :available, :scheduled, :retryable]) |> Repo.preload(:job) - pending_media = - MediaQuery.new() - |> MediaQuery.for_source(source) - |> MediaQuery.where_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, - pending_tasks: pending_tasks, - pending_media: pending_media, - downloaded_media: downloaded_media, - total_downloaded: total_downloaded_for(source) - ) + render(conn, :show, source: source, pending_tasks: pending_tasks) end def edit(conn, %{"id" => id}) do @@ -151,13 +127,6 @@ defmodule PinchflatWeb.Sources.SourceController do |> Repo.all() end - defp total_downloaded_for(source) do - MediaQuery.new() - |> MediaQuery.for_source(source) - |> MediaQuery.with_media_filepath() - |> Repo.aggregate(:count, :id) - end - defp get_onboarding_layout do if Settings.get!(:onboarding) do {Layouts, :onboarding} diff --git a/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex b/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex new file mode 100644 index 0000000..bc2705c --- /dev/null +++ b/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex @@ -0,0 +1,91 @@ +defmodule Pinchflat.Sources.MediaItemTableLive do + use PinchflatWeb, :live_view + import Ecto.Query, warn: false + + alias Pinchflat.Repo + alias Pinchflat.Sources + alias Pinchflat.Media.MediaQuery + alias Pinchflat.Utils.NumberUtils + + @limit 10 + + def render(%{records: []} = assigns) do + ~H""" +

    Nothing Here!

    + """ + end + + def render(assigns) do + ~H""" +
    + + Showing <%= length(@records) %> of <%= @total_record_count %> + + <.table rows={@records} table_class="text-black dark:text-white"> + <:col :let={media_item} label="Title"> + <.subtle_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"}> + <%= StringUtils.truncate(media_item.title, 50) %> + + + <:col :let={media_item} label="" class="flex place-content-evenly"> + <.icon_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"} icon="hero-eye" class="mx-1" /> + <.icon_link href={~p"/sources/#{@source.id}/media/#{media_item.id}/edit"} icon="hero-pencil-square" class="mx-1" /> + + +
    + <.live_pagination_controls page_number={@page} total_pages={@total_pages} /> +
    +
    + """ + end + + def mount(_params, session, socket) do + page = 1 + media_state = session["media_state"] + source = Sources.get_source!(session["source_id"]) + base_query = generate_base_query(source, media_state) + pagination_attrs = fetch_pagination_attributes(base_query, page) + + {:ok, assign(socket, Map.merge(pagination_attrs, %{base_query: base_query, source: source}))} + end + + def handle_event("page_change", %{"direction" => direction}, %{assigns: assigns} = socket) do + direction = if direction == "inc", do: 1, else: -1 + new_page = assigns.page + direction + new_assigns = fetch_pagination_attributes(assigns.base_query, new_page) + + {:noreply, assign(socket, new_assigns)} + end + + defp fetch_pagination_attributes(base_query, page) do + total_record_count = Repo.aggregate(base_query, :count, :id) + total_pages = max(ceil(total_record_count / @limit), 1) + page = NumberUtils.clamp(page, 1, total_pages) + records = fetch_records(base_query, page) + + %{page: page, total_pages: total_pages, records: records, total_record_count: total_record_count} + end + + defp fetch_records(base_query, page) do + offset = (page - 1) * @limit + + base_query + |> limit(^@limit) + |> offset(^offset) + |> Repo.all() + end + + defp generate_base_query(source, "pending") do + MediaQuery.new() + |> MediaQuery.for_source(source) + |> MediaQuery.where_pending_download() + |> order_by(desc: :id) + end + + defp generate_base_query(source, "downloaded") do + MediaQuery.new() + |> MediaQuery.for_source(source) + |> MediaQuery.with_media_filepath() + |> order_by(desc: :id) + end +end diff --git a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex index d9687a1..3093bcf 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex @@ -37,48 +37,18 @@ <:tab title="Pending Media"> - <%= if match?([_|_], @pending_media) do %> -

    Shows a maximum of 100 media items

    - <.table rows={@pending_media} table_class="text-black dark:text-white"> - <:col :let={media_item} label="Title"> - <.subtle_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"}> - <%= StringUtils.truncate(media_item.title, 50) %> - - - <:col :let={media_item} label="" class="flex place-content-evenly"> - <.icon_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"} icon="hero-eye" class="mx-1" /> - <.icon_link - href={~p"/sources/#{@source.id}/media/#{media_item.id}/edit"} - icon="hero-pencil-square" - class="mx-1" - /> - - - <% else %> -

    Nothing Here!

    - <% end %> + <%= live_render( + @conn, + Pinchflat.Sources.MediaItemTableLive, + session: %{"source_id" => @source.id, "media_state" => "pending"} + ) %> <:tab title="Downloaded Media"> - <%= if match?([_|_], @downloaded_media) do %> -

    Shows a maximum of 100 media items (<%= @total_downloaded %> total)

    - <.table rows={@downloaded_media} table_class="text-black dark:text-white"> - <:col :let={media_item} label="Title"> - <.subtle_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"}> - <%= StringUtils.truncate(media_item.title, 50) %> - - - <:col :let={media_item} label="" class="flex place-content-evenly"> - <.icon_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"} icon="hero-eye" class="mx-1" /> - <.icon_link - href={~p"/sources/#{@source.id}/media/#{media_item.id}/edit"} - icon="hero-pencil-square" - class="mx-1" - /> - - - <% else %> -

    Nothing Here!

    - <% end %> + <%= live_render( + @conn, + Pinchflat.Sources.MediaItemTableLive, + session: %{"source_id" => @source.id, "media_state" => "downloaded"} + ) %> <:tab title="Pending Tasks"> <%= if match?([_|_], @pending_tasks) do %> diff --git a/test/pinchflat/utils/number_utils_test.exs b/test/pinchflat/utils/number_utils_test.exs new file mode 100644 index 0000000..48efe41 --- /dev/null +++ b/test/pinchflat/utils/number_utils_test.exs @@ -0,0 +1,19 @@ +defmodule Pinchflat.Utils.NumberUtilsTest do + use ExUnit.Case, async: true + + alias Pinchflat.Utils.NumberUtils + + describe "clamp/3" do + test "returns the minimum when the number is less than the minimum" do + assert NumberUtils.clamp(1, 2, 3) == 2 + end + + test "returns the maximum when the number is greater than the maximum" do + assert NumberUtils.clamp(4, 2, 3) == 3 + end + + test "returns the number when it is between the minimum and maximum" do + assert NumberUtils.clamp(2, 1, 3) == 2 + end + end +end diff --git a/test/pinchflat_web/controllers/sources/media_item_table_live_test.exs b/test/pinchflat_web/controllers/sources/media_item_table_live_test.exs new file mode 100644 index 0000000..555738f --- /dev/null +++ b/test/pinchflat_web/controllers/sources/media_item_table_live_test.exs @@ -0,0 +1,60 @@ +defmodule PinchflatWeb.Sources.MediaItemTableLiveTest do + use PinchflatWeb.ConnCase + + import Phoenix.LiveViewTest + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + + alias Pinchflat.Sources.MediaItemTableLive + + setup do + source = source_fixture() + + {:ok, source: source} + end + + describe "initial rendering" do + test "shows message when no records", %{conn: conn, source: source} do + {:ok, _view, html} = live_isolated(conn, MediaItemTableLive, session: create_session(source)) + + assert html =~ "Nothing Here!" + refute html =~ "Showing" + end + + test "shows records when present", %{conn: conn, source: source} do + media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + {:ok, _view, html} = live_isolated(conn, MediaItemTableLive, session: create_session(source)) + + assert html =~ "Showing 1 of 1" + assert html =~ "Title" + assert html =~ media_item.title + end + end + + describe "media_state" do + test "shows pending media when pending", %{conn: conn, source: source} do + downloaded_media_item = media_item_fixture(source_id: source.id) + pending_media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + {:ok, _view, html} = live_isolated(conn, MediaItemTableLive, session: create_session(source, "pending")) + + assert html =~ pending_media_item.title + refute html =~ downloaded_media_item.title + end + + test "shows downloaded media when downloaded", %{conn: conn, source: source} do + downloaded_media_item = media_item_fixture(source_id: source.id) + pending_media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + {:ok, _view, html} = live_isolated(conn, MediaItemTableLive, session: create_session(source, "downloaded")) + + assert html =~ downloaded_media_item.title + refute html =~ pending_media_item.title + end + end + + defp create_session(source, media_state \\ "pending") do + %{"source_id" => source.id, "media_state" => media_state} + end +end From 6f78ec40d731c8557ed33048af94d65e87404298 Mon Sep 17 00:00:00 2001 From: Kieran Date: Wed, 17 Apr 2024 11:31:50 -0700 Subject: [PATCH 022/240] [Enhancement] Improve layout of media item page on large displays (#192) * Improved layout of media items page * Removed useless anchor tag --- .../components/core_components.ex | 7 +++--- .../custom_components/text_components.ex | 21 +++++++++++++++- .../media_item_html/media_preview.heex | 4 ++-- .../media_item_html/show.html.heex | 24 +++++++++++++------ .../media_profile_html/show.html.heex | 4 ++-- .../pages/page_html/home.html.heex | 4 ++-- .../sources/source_html/show.html.heex | 4 ++-- 7 files changed, 49 insertions(+), 19 deletions(-) diff --git a/lib/pinchflat_web/components/core_components.ex b/lib/pinchflat_web/components/core_components.ex index 654b743..ccdf0b9 100644 --- a/lib/pinchflat_web/components/core_components.ex +++ b/lib/pinchflat_web/components/core_components.ex @@ -19,7 +19,6 @@ defmodule PinchflatWeb.CoreComponents do import PinchflatWeb.Gettext alias Phoenix.LiveView.JS - alias PinchflatWeb.CustomComponents.TextComponents @doc """ Renders a modal. @@ -638,9 +637,11 @@ defmodule PinchflatWeb.CoreComponents do ~H"""
      -
    • +
    • <%= k %>: - <%= v %> + + <%= v %> +
    """ diff --git a/lib/pinchflat_web/components/custom_components/text_components.ex b/lib/pinchflat_web/components/custom_components/text_components.ex index 1433bd3..1a577f5 100644 --- a/lib/pinchflat_web/components/custom_components/text_components.ex +++ b/lib/pinchflat_web/components/custom_components/text_components.ex @@ -35,11 +35,12 @@ defmodule PinchflatWeb.CustomComponents.TextComponents do Renders a subtle link with the given href and content. """ attr :href, :string, required: true + attr :target, :string, default: "_self" slot :inner_block def subtle_link(assigns) do ~H""" - <.link href={@href} class="underline decoration-bodydark decoration-1 hover:decoration-white"> + <.link href={@href} target={@target} class="underline decoration-bodydark decoration-1 hover:decoration-white"> <%= render_slot(@inner_block) %> """ @@ -59,4 +60,22 @@ defmodule PinchflatWeb.CustomComponents.TextComponents do """ end + + @doc """ + Renders a block of text with each line broken into a separate span. + """ + attr :text, :string, required: true + + def break_on_newline(assigns) do + broken_text = + assigns.text + |> String.split("\n", trim: false) + |> Enum.intersperse(Phoenix.HTML.Tag.tag(:span, class: "inline-block mt-2")) + + assigns = Map.put(assigns, :text, broken_text) + + ~H""" + <%= @text %> + """ + end end diff --git a/lib/pinchflat_web/controllers/media_items/media_item_html/media_preview.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/media_preview.heex index 2146a98..5ad0533 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_html/media_preview.heex +++ b/lib/pinchflat_web/controllers/media_items/media_item_html/media_preview.heex @@ -1,12 +1,12 @@ <%= if media_type(@media_item) == :video do %> -
  • - - Pinchflat v<%= Application.spec(:pinchflat)[:vsn] %> + + Pinchflat v<%= Application.spec(:pinchflat)[:vsn] %> + + NEW + - + yt-dlp <%= Settings.get!(:yt_dlp_version) %>
    • From f2c9c10437c6696b7444dd93942cd4ad1f8e0689 Mon Sep 17 00:00:00 2001 From: Kieran Date: Thu, 2 May 2024 12:09:55 -0700 Subject: [PATCH 035/240] Added reload button to live tables (#223) --- .../source_html/media_item_table_live.ex | 30 ++++++++++++++++--- .../sources/source_html/show.html.heex | 2 +- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex b/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex index bc2705c..8c56d97 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex +++ b/lib/pinchflat_web/controllers/sources/source_html/media_item_table_live.ex @@ -11,17 +11,33 @@ defmodule Pinchflat.Sources.MediaItemTableLive do def render(%{records: []} = assigns) do ~H""" -

    Nothing Here!

    +
    + +

    Nothing Here!

    +
    """ end def render(assigns) do ~H"""
    - - Showing <%= length(@records) %> of <%= @total_record_count %> + + + Showing <%= length(@records) %> of <%= @total_record_count %> - <.table rows={@records} table_class="text-black dark:text-white"> + <.table rows={@records} table_class="text-white"> <:col :let={media_item} label="Title"> <.subtle_link href={~p"/sources/#{@source.id}/media/#{media_item.id}"}> <%= StringUtils.truncate(media_item.title, 50) %> @@ -57,6 +73,12 @@ defmodule Pinchflat.Sources.MediaItemTableLive do {:noreply, assign(socket, new_assigns)} end + def handle_event("reload_page", _params, %{assigns: assigns} = socket) do + new_assigns = fetch_pagination_attributes(assigns.base_query, assigns.page) + + {:noreply, assign(socket, new_assigns)} + end + defp fetch_pagination_attributes(base_query, page) do total_record_count = Repo.aggregate(base_query, :count, :id) total_pages = max(ceil(total_record_count / @limit), 1) diff --git a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex index 2d110c4..0f8869d 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/show.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/show.html.heex @@ -64,7 +64,7 @@ <% else %> -

    Nothing Here!

    +

    Nothing Here!

    <% end %> From 7090349abd21b1b0a5c339d33ba9e26a8fe30056 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Thu, 2 May 2024 12:13:14 -0700 Subject: [PATCH 036/240] Bumped version --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index dfa6a1c..13b9930 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pinchflat.MixProject do def project do [ app: :pinchflat, - version: "0.1.15", + version: "0.1.16", elixir: "~> 1.16", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From f655a8ae01628848fddf4d99f5eb2418b21b13e4 Mon Sep 17 00:00:00 2001 From: Kieran Date: Fri, 3 May 2024 09:15:12 -0700 Subject: [PATCH 037/240] Improved robustness of file downloads (#225) --- .../downloading/download_option_builder.ex | 11 ++++- .../metadata/metadata_file_helpers.ex | 36 ++++++++++------ lib/pinchflat/metadata/metadata_parser.ex | 2 +- lib/pinchflat/yt_dlp/media.ex | 2 +- .../media_item_html/show.html.heex | 2 +- .../download_option_builder_test.exs | 2 +- .../metadata/metadata_file_helpers_test.exs | 42 +++++++++++++++++++ .../metadata/metadata_parser_test.exs | 8 ++++ test/pinchflat/yt_dlp/media_test.exs | 11 +++++ 9 files changed, 99 insertions(+), 17 deletions(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 0627bd6..5745012 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -124,9 +124,18 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do [format_sort: "res:#{res},+codec:avc:m4a", remux_video: "mp4"] end + audio_format_precedence = [ + "bestaudio[ext=m4a]", + "bestaudio[ext=mp3]", + "bestaudio", + "best[ext=m4a]", + "best[ext=mp3]", + "best" + ] + case media_profile.preferred_resolution do # Also be aware that :audio disabled all embedding options for subtitles - :audio -> [:extract_audio, format: "bestaudio[ext=m4a]"] + :audio -> [:extract_audio, format: Enum.join(audio_format_precedence, "/")] :"360p" -> video_codec_option.("360") :"480p" -> video_codec_option.("480") :"720p" -> video_codec_option.("720") diff --git a/lib/pinchflat/metadata/metadata_file_helpers.ex b/lib/pinchflat/metadata/metadata_file_helpers.ex index a804ecd..f45d556 100644 --- a/lib/pinchflat/metadata/metadata_file_helpers.ex +++ b/lib/pinchflat/metadata/metadata_file_helpers.ex @@ -54,24 +54,36 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do @doc """ Downloads and stores a thumbnail for a media item, returning the filepath. - Chooses the highest quality jpg thumbnail available. + Chooses the highest quality thumbnail available (preferring jpg). Returns + nil if no thumbnails are available. - Returns binary() + Returns binary() | nil """ def download_and_store_thumbnail_for(database_record, metadata_map) do - thumbnail_url = - metadata_map["thumbnails"] - |> Enum.filter(&(&1["preference"] && String.ends_with?(&1["url"], ".jpg"))) - |> Enum.sort(&(&1["preference"] >= &2["preference"])) - |> List.first() - |> Map.get("url") + thumbnails = + (metadata_map["thumbnails"] || []) + # Give it a low preference if the `preference` key doesn't exist + |> Enum.map(&Map.put_new(&1, "preference", -1000)) + # Give it a low preference if image isn't a jpg + |> Enum.map(fn t -> + preference_weight = if String.ends_with?(t["url"], ".jpg"), do: t["preference"], else: t["preference"] - 1000 - filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url)) - thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url) + Map.put(t, "preference", preference_weight) + end) - :ok = FilesystemUtils.write_p!(filepath, thumbnail_blob) + case Enum.sort_by(thumbnails, & &1["preference"], :desc) do + [thumbnail_map | _] -> + thumbnail_url = thumbnail_map["url"] + filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url)) + thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url) - filepath + :ok = FilesystemUtils.write_p!(filepath, thumbnail_blob) + + filepath + + _ -> + nil + end end @doc """ diff --git a/lib/pinchflat/metadata/metadata_parser.ex b/lib/pinchflat/metadata/metadata_parser.ex index 1173001..ef048b8 100644 --- a/lib/pinchflat/metadata/metadata_parser.ex +++ b/lib/pinchflat/metadata/metadata_parser.ex @@ -30,7 +30,7 @@ defmodule Pinchflat.Metadata.MetadataParser do original_url: metadata["original_url"], description: metadata["description"], media_filepath: metadata["filepath"], - livestream: metadata["was_live"], + livestream: !!metadata["was_live"], duration_seconds: metadata["duration"] && round(metadata["duration"]) } end diff --git a/lib/pinchflat/yt_dlp/media.ex b/lib/pinchflat/yt_dlp/media.ex index 7a6c1c5..70a3e97 100644 --- a/lib/pinchflat/yt_dlp/media.ex +++ b/lib/pinchflat/yt_dlp/media.ex @@ -87,7 +87,7 @@ defmodule Pinchflat.YtDlp.Media do title: response["title"], description: response["description"], original_url: response["webpage_url"], - livestream: response["was_live"], + livestream: !!response["was_live"], duration_seconds: response["duration"] && round(response["duration"]), short_form_content: response["webpage_url"] && short_form_content?(response), upload_date: response["upload_date"] && MetadataFileHelpers.parse_upload_date(response["upload_date"]) diff --git a/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex b/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex index 8c481de..0ac630b 100644 --- a/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex +++ b/lib/pinchflat_web/controllers/media_items/media_item_html/show.html.heex @@ -32,7 +32,7 @@