Added ability to specify overwrite behaviour when downloading media

lib/pinchflat/downloading/download_option_builder.ex

@@ -15,11 +15,11 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
 
   Returns {:ok, [Keyword.t()]}
   """
-  def build(%MediaItem{} = media_item_with_preloads) do
+  def build(%MediaItem{} = media_item_with_preloads, override_opts \\ []) do
     media_profile = media_item_with_preloads.source.media_profile
 
     built_options =
-      default_options() ++
+      default_options(override_opts) ++
         subtitle_options(media_profile) ++
         thumbnail_options(media_item_with_preloads) ++
         metadata_options(media_profile) ++
@@ -50,11 +50,12 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
     build_output_path_for(%MediaItem{source: source_with_preloads})
   end
 
-  defp default_options do
+  defp default_options(override_opts) do
+    overwrite_behaviour = Keyword.get(override_opts, :overwrite_behaviour, :force_overwrites)
+
     [
       :no_progress,
-      # Add force-overwrites to make sure redownloading works
+      overwrite_behaviour,
-      :force_overwrites,
       # This makes the date metadata conform to what jellyfin expects
       parse_metadata: "%(upload_date>%Y-%m-%d)s:(?P<meta_date>.+)"
     ]

lib/pinchflat/downloading/media_download_worker.ex

@@ -33,6 +33,12 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
   Does not download media if its source is set to not download media
   (unless forced).
 
+  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
+      if the source is set to not download media
+
   Returns :ok | {:ok, %MediaItem{}} | {:error, any, ...any}
   """
   @impl Oban.Worker
@@ -47,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)
+      download_media_and_schedule_jobs(media_item, is_redownload, should_force)
     else
       :ok
     end
@@ -56,8 +62,11 @@ 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) do
+  defp download_media_and_schedule_jobs(media_item, is_redownload, should_force) do
-    case MediaDownloader.download_for_media_item(media_item) do
+    overwrite_behaviour = if should_force || is_redownload, do: :force_overwrites, else: :no_force_overwrites
+    override_opts = [overwrite_behaviour: overwrite_behaviour]
+
+    case MediaDownloader.download_for_media_item(media_item, override_opts) do
       {:ok, downloaded_media_item} ->
         {:ok, updated_media_item} =
           Media.update_media_item(downloaded_media_item, %{

lib/pinchflat/downloading/media_downloader.ex

@@ -29,11 +29,11 @@ defmodule Pinchflat.Downloading.MediaDownloader do
 
   Returns {:ok, %MediaItem{}} | {:error, any, ...any}
   """
-  def download_for_media_item(%MediaItem{} = media_item) do
+  def download_for_media_item(%MediaItem{} = media_item, override_opts \\ []) do
     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
+    case download_with_options(media_item.original_url, media_with_preloads, output_filepath, override_opts) do
       {:ok, parsed_json} ->
         update_media_item_from_parsed_json(media_with_preloads, parsed_json)
 
@@ -103,8 +103,8 @@ defmodule Pinchflat.Downloading.MediaDownloader do
     end
   end
 
-  defp download_with_options(url, item_with_preloads, output_filepath) do
+  defp download_with_options(url, item_with_preloads, output_filepath, override_opts) do
-    {:ok, options} = DownloadOptionBuilder.build(item_with_preloads)
+    {:ok, options} = DownloadOptionBuilder.build(item_with_preloads, override_opts)
 
     YtDlpMedia.download(url, options, output_filepath: output_filepath)
   end

test/pinchflat/downloading/download_option_builder_test.exs

@@ -65,6 +65,13 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
       assert :force_overwrites in res
       assert {:parse_metadata, "%(upload_date>%Y-%m-%d)s:(?P<meta_date>.+)"} in res
     end
+
+    test "includes override options if specified", %{media_item: media_item} do
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item, overwrite_behaviour: :no_force_overwrites)
+
+      refute :force_overwrites in res
+      assert :no_force_overwrites in res
+    end
   end
 
   describe "build/1 when testing subtitle options" do

test/pinchflat/downloading/media_download_worker_test.exs

@@ -136,15 +136,6 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
-    test "downloads anyway if forced", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> :ok end)
-
-      Sources.update_source(media_item.source, %{download_media: false})
-      Media.update_media_item(media_item, %{prevent_download: true})
-
-      perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
-    end
-
     test "it saves the file's size to the database", %{media_item: media_item} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
         metadata = render_parsed_metadata(:media_metadata)
@@ -159,18 +150,7 @@ 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
+    test "does not set redownloaded_at by default", %{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)
@@ -198,5 +178,62 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     test "does not blow up if the record doesn't exist" do
       assert :ok = perform_job(MediaDownloadWorker, %{id: 0})
     end
+
+    test "sets the no_force_overwrites runner option", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, _addl ->
+        assert :no_force_overwrites in opts
+        refute :force_overwrites in opts
+
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id})
+    end
+  end
+
+  describe "perform/1 when testing forced downloads" do
+    test "ignores 'prevent_download' if forced", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> :ok end)
+
+      Sources.update_source(media_item.source, %{download_media: false})
+      Media.update_media_item(media_item, %{prevent_download: true})
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
+    end
+
+    test "sets force_overwrites runner option", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, _addl ->
+        assert :force_overwrites in opts
+        refute :no_force_overwrites in opts
+
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
+    end
+  end
+
+  describe "perform/1 when testing re-downloads" do
+    test "sets redownloaded_at on the media_item", %{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 "sets force_overwrites runner option", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, _addl ->
+        assert :force_overwrites in opts
+        refute :no_force_overwrites in opts
+
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
+    end
   end
 end

test/pinchflat/downloading/media_downloader_test.exs

@@ -65,6 +65,21 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
     end
   end
 
+  describe "download_for_media_item/3 when testing override options" do
+    test "includes override opts if specified", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot, _addl ->
+        refute :force_overwrites in opts
+        assert :no_force_overwrites in opts
+
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      override_opts = [overwrite_behaviour: :no_force_overwrites]
+
+      assert {:ok, _} = MediaDownloader.download_for_media_item(media_item, override_opts)
+    end
+  end
+
   describe "download_for_media_item/3 when testing retries" do
     test "returns a recovered tuple on recoverable errors", %{media_item: media_item} do
       message = "Unable to communicate with SponsorBlock"