From 1f453fd66b5dbfc09cfad344e91b5c68ddfc7bb6 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Mon, 13 May 2024 12:45:02 -0700 Subject: [PATCH] renamed media redownload worker to disambiguate it from similarly named methods --- config/config.exs | 2 +- lib/pinchflat/downloading/downloading_helpers.ex | 7 +++++-- lib/pinchflat/downloading/media_download_worker.ex | 14 ++++++-------- ...d_worker.ex => media_quality_upgrade_worker.ex} | 8 +++++--- .../downloading/media_download_worker_test.exs | 4 ++-- ...t.exs => media_quality_upgrade_worker_test.exs} | 10 +++++----- 6 files changed, 24 insertions(+), 21 deletions(-) rename lib/pinchflat/downloading/{media_redownload_worker.ex => media_quality_upgrade_worker.ex} (69%) rename test/pinchflat/downloading/{media_redownload_worker_test.exs => media_quality_upgrade_worker_test.exs} (81%) diff --git a/config/config.exs b/config/config.exs index ad5febe..508b9b6 100644 --- a/config/config.exs +++ b/config/config.exs @@ -54,7 +54,7 @@ config :pinchflat, Oban, {Oban.Plugins.Cron, crontab: [ {"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker}, - {"0 2 * * *", Pinchflat.Downloading.MediaRedownloadWorker} + {"0 2 * * *", Pinchflat.Downloading.MediaQualityUpgradeWorker} ]} ], # TODO: consider making this an env var or something? diff --git a/lib/pinchflat/downloading/downloading_helpers.ex b/lib/pinchflat/downloading/downloading_helpers.ex index 01c2393..9bfbc88 100644 --- a/lib/pinchflat/downloading/downloading_helpers.ex +++ b/lib/pinchflat/downloading/downloading_helpers.ex @@ -74,8 +74,11 @@ defmodule Pinchflat.Downloading.DownloadingHelpers do and you've changed the source to download them, you can use this to download all the thumbnails for existing media items. - NOTE: does not delete existing files whatsoever. Will cause a full redownload of everything if the - output template has changed. + NOTE: does not delete existing files whatsoever. Does not overwrite the existing media file if it exists + at the location it expects. Will cause a full redownload of everything if the output template has changed + + NOTE: unrelated to the MediaQualityUpgradeWorker, which is for redownloading media items for quality upgrades + or improved sponsorblock segments Returns [{:ok, %Task{}} | {:error, any()}] """ diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index 7bd7fe9..fc1bc99 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -36,7 +36,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do Options: - `force`: force download even if the source is set to not download media. Fully re-downloads media, including the video - - `redownload?`: re-downloads media, including the video. Does not force download + - `quality_upgrade?`: re-downloads media, including the video. Does not force download if the source is set to not download media Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any} @@ -44,9 +44,7 @@ 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) - # TODO: rename to `upgrade_quality?` or similar to disambiguate from the other redownload method - # that doesn't `force_overwrites` - is_redownload = Map.get(args, "redownload?", false) + is_quality_upgrade = Map.get(args, "quality_upgrade?", false) media_item = media_item_id @@ -55,7 +53,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do # 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) || should_force do - download_media_and_schedule_jobs(media_item, is_redownload, should_force) + download_media_and_schedule_jobs(media_item, is_quality_upgrade, should_force) else :ok end @@ -64,8 +62,8 @@ 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, is_redownload, should_force) do - overwrite_behaviour = if should_force || is_redownload, do: :force_overwrites, else: :no_force_overwrites + defp download_media_and_schedule_jobs(media_item, is_quality_upgrade, should_force) do + overwrite_behaviour = if should_force || is_quality_upgrade, do: :force_overwrites, else: :no_force_overwrites override_opts = [overwrite_behaviour: overwrite_behaviour] case MediaDownloader.download_for_media_item(media_item, override_opts) do @@ -73,7 +71,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do {:ok, updated_media_item} = Media.update_media_item(downloaded_media_item, %{ media_size_bytes: compute_media_filesize(downloaded_media_item), - media_redownloaded_at: get_redownloaded_at(is_redownload) + media_redownloaded_at: get_redownloaded_at(is_quality_upgrade) }) :ok = run_user_script(updated_media_item) diff --git a/lib/pinchflat/downloading/media_redownload_worker.ex b/lib/pinchflat/downloading/media_quality_upgrade_worker.ex similarity index 69% rename from lib/pinchflat/downloading/media_redownload_worker.ex rename to lib/pinchflat/downloading/media_quality_upgrade_worker.ex index b3b6d82..ec325ef 100644 --- a/lib/pinchflat/downloading/media_redownload_worker.ex +++ b/lib/pinchflat/downloading/media_quality_upgrade_worker.ex @@ -1,4 +1,4 @@ -defmodule Pinchflat.Downloading.MediaRedownloadWorker do +defmodule Pinchflat.Downloading.MediaQualityUpgradeWorker do @moduledoc false use Oban.Worker, @@ -12,7 +12,9 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorker do alias Pinchflat.Downloading.MediaDownloadWorker @doc """ - Redownloads media items that are eligible for redownload. + Redownloads media items that are eligible for redownload for the purpose + of upgrading the quality of the media or improving things like sponsorblock + segments. This worker is scheduled to run daily via the Oban Cron plugin and it should run _after_ the retention worker. @@ -25,7 +27,7 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorker do Logger.info("Redownloading #{length(redownloadable_media)} media items") Enum.each(redownloadable_media, fn media_item -> - MediaDownloadWorker.kickoff_with_task(media_item, %{redownload?: true}) + MediaDownloadWorker.kickoff_with_task(media_item, %{quality_upgrade?: true}) end) end end diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index 6afb574..e82b0b1 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -108,7 +108,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do end) Oban.Testing.with_testing_mode(:inline, fn -> - {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id, redownload?: true})) + {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id, quality_upgrade?: true})) assert job.state == "completed" end) @@ -219,7 +219,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do {:ok, render_metadata(:media_metadata)} end) - perform_job(MediaDownloadWorker, %{id: media_item.id, redownload?: true}) + perform_job(MediaDownloadWorker, %{id: media_item.id, quality_upgrade?: true}) media_item = Repo.reload(media_item) assert media_item.media_redownloaded_at != nil diff --git a/test/pinchflat/downloading/media_redownload_worker_test.exs b/test/pinchflat/downloading/media_quality_upgrade_worker_test.exs similarity index 81% rename from test/pinchflat/downloading/media_redownload_worker_test.exs rename to test/pinchflat/downloading/media_quality_upgrade_worker_test.exs index 70d9cc0..3b8a359 100644 --- a/test/pinchflat/downloading/media_redownload_worker_test.exs +++ b/test/pinchflat/downloading/media_quality_upgrade_worker_test.exs @@ -1,4 +1,4 @@ -defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do +defmodule Pinchflat.Downloading.MediaQualityUpgradeWorkerTest do use Pinchflat.DataCase import Pinchflat.MediaFixtures @@ -6,7 +6,7 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do import Pinchflat.ProfilesFixtures alias Pinchflat.Downloading.MediaDownloadWorker - alias Pinchflat.Downloading.MediaRedownloadWorker + alias Pinchflat.Downloading.MediaQualityUpgradeWorker describe "perform/1" do test "kicks off a task for redownloadable media items" do @@ -20,9 +20,9 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do media_downloaded_at: now_minus(5, :days) }) - perform_job(MediaRedownloadWorker, %{}) + perform_job(MediaQualityUpgradeWorker, %{}) - assert [_] = all_enqueued(worker: MediaDownloadWorker, args: %{id: media_item.id, redownload?: true}) + assert [_] = all_enqueued(worker: MediaDownloadWorker, args: %{id: media_item.id, quality_upgrade?: true}) end test "does not kickoff a task for non-redownloadable media items" do @@ -36,7 +36,7 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do media_downloaded_at: now_minus(1, :day) }) - perform_job(MediaRedownloadWorker, %{}) + perform_job(MediaQualityUpgradeWorker, %{}) assert [] = all_enqueued(worker: MediaDownloadWorker) end