renamed media redownload worker to disambiguate it from similarly named methods

This commit is contained in:
Kieran Eglin 2024-05-13 12:45:02 -07:00
parent 840eabe3bb
commit 1f453fd66b
No known key found for this signature in database
GPG key ID: 193984967FCF432D
6 changed files with 24 additions and 21 deletions

View file

@ -54,7 +54,7 @@ config :pinchflat, Oban,
{Oban.Plugins.Cron, {Oban.Plugins.Cron,
crontab: [ crontab: [
{"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker}, {"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker},
{"0 2 * * *", Pinchflat.Downloading.MediaRedownloadWorker} {"0 2 * * *", Pinchflat.Downloading.MediaQualityUpgradeWorker}
]} ]}
], ],
# TODO: consider making this an env var or something? # TODO: consider making this an env var or something?

View file

@ -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 and you've changed the source to download them, you can use this to download all the thumbnails for
existing media items. existing media items.
NOTE: does not delete existing files whatsoever. Will cause a full redownload of everything if the NOTE: does not delete existing files whatsoever. Does not overwrite the existing media file if it exists
output template has changed. 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()}] Returns [{:ok, %Task{}} | {:error, any()}]
""" """

View file

@ -36,7 +36,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
Options: Options:
- `force`: force download even if the source is set to not download media. Fully - `force`: force download even if the source is set to not download media. Fully
re-downloads media, including the video 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 if the source is set to not download media
Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any} Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any}
@ -44,9 +44,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
@impl Oban.Worker @impl Oban.Worker
def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do
should_force = Map.get(args, "force", false) should_force = Map.get(args, "force", false)
# TODO: rename to `upgrade_quality?` or similar to disambiguate from the other redownload method is_quality_upgrade = Map.get(args, "quality_upgrade?", false)
# that doesn't `force_overwrites`
is_redownload = Map.get(args, "redownload?", false)
media_item = media_item =
media_item_id 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 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 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 else
:ok :ok
end end
@ -64,8 +62,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} stale") Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} stale")
end end
defp download_media_and_schedule_jobs(media_item, is_redownload, should_force) do defp download_media_and_schedule_jobs(media_item, is_quality_upgrade, should_force) do
overwrite_behaviour = if should_force || is_redownload, do: :force_overwrites, else: :no_force_overwrites overwrite_behaviour = if should_force || is_quality_upgrade, do: :force_overwrites, else: :no_force_overwrites
override_opts = [overwrite_behaviour: overwrite_behaviour] override_opts = [overwrite_behaviour: overwrite_behaviour]
case MediaDownloader.download_for_media_item(media_item, override_opts) do case MediaDownloader.download_for_media_item(media_item, override_opts) do
@ -73,7 +71,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
{:ok, updated_media_item} = {:ok, updated_media_item} =
Media.update_media_item(downloaded_media_item, %{ Media.update_media_item(downloaded_media_item, %{
media_size_bytes: compute_media_filesize(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) :ok = run_user_script(updated_media_item)

View file

@ -1,4 +1,4 @@
defmodule Pinchflat.Downloading.MediaRedownloadWorker do defmodule Pinchflat.Downloading.MediaQualityUpgradeWorker do
@moduledoc false @moduledoc false
use Oban.Worker, use Oban.Worker,
@ -12,7 +12,9 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorker do
alias Pinchflat.Downloading.MediaDownloadWorker alias Pinchflat.Downloading.MediaDownloadWorker
@doc """ @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 This worker is scheduled to run daily via the Oban Cron plugin
and it should run _after_ the retention worker. 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") Logger.info("Redownloading #{length(redownloadable_media)} media items")
Enum.each(redownloadable_media, fn media_item -> 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 end
end end

View file

@ -108,7 +108,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end) end)
Oban.Testing.with_testing_mode(:inline, fn -> 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" assert job.state == "completed"
end) end)
@ -219,7 +219,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
{:ok, render_metadata(:media_metadata)} {:ok, render_metadata(:media_metadata)}
end) 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) media_item = Repo.reload(media_item)
assert media_item.media_redownloaded_at != nil assert media_item.media_redownloaded_at != nil

View file

@ -1,4 +1,4 @@
defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do defmodule Pinchflat.Downloading.MediaQualityUpgradeWorkerTest do
use Pinchflat.DataCase use Pinchflat.DataCase
import Pinchflat.MediaFixtures import Pinchflat.MediaFixtures
@ -6,7 +6,7 @@ defmodule Pinchflat.Downloading.MediaRedownloadWorkerTest do
import Pinchflat.ProfilesFixtures import Pinchflat.ProfilesFixtures
alias Pinchflat.Downloading.MediaDownloadWorker alias Pinchflat.Downloading.MediaDownloadWorker
alias Pinchflat.Downloading.MediaRedownloadWorker alias Pinchflat.Downloading.MediaQualityUpgradeWorker
describe "perform/1" do describe "perform/1" do
test "kicks off a task for redownloadable media items" 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) 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 end
test "does not kickoff a task for non-redownloadable media items" do 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) media_downloaded_at: now_minus(1, :day)
}) })
perform_job(MediaRedownloadWorker, %{}) perform_job(MediaQualityUpgradeWorker, %{})
assert [] = all_enqueued(worker: MediaDownloadWorker) assert [] = all_enqueued(worker: MediaDownloadWorker)
end end