[Housekeeping] Pass the current action when calling the yt-dlp runner (…

…#514)

* Updated yt-dlp runner to take an action type

* Added actions to all callers of the yt-dlp runner

* [SQUASH] updated test files to use new mocking strategy

* Removed unneeded alias

lib/pinchflat/yt_dlp/command_runner.ex

@@ -3,6 +3,8 @@ defmodule Pinchflat.YtDlp.CommandRunner do
   Runs yt-dlp commands using the `System.cmd/3` function
   """
 
+  require Logger
+
   alias Pinchflat.Utils.CliUtils
   alias Pinchflat.YtDlp.YtDlpCommandRunner
   alias Pinchflat.Utils.FilesystemUtils, as: FSUtils
@@ -24,7 +26,8 @@ defmodule Pinchflat.YtDlp.CommandRunner do
   Returns {:ok, binary()} | {:error, output, status}.
   """
   @impl YtDlpCommandRunner
-  def run(url, command_opts, output_template, addl_opts \\ []) do
+  def run(url, action_name, command_opts, output_template, addl_opts \\ []) do
+    Logger.debug("Running yt-dlp command for action: #{action_name}")
     # This approach lets us mock the command for testing
     command = backend_executable()
 

lib/pinchflat/yt_dlp/media.ex

@@ -41,7 +41,7 @@ defmodule Pinchflat.YtDlp.Media do
   def download(url, command_opts \\ [], addl_opts \\ []) do
     all_command_opts = [:no_simulate] ++ command_opts
 
-    with {:ok, output} <- backend_runner().run(url, all_command_opts, "after_move:%()j", addl_opts),
+    with {:ok, output} <- backend_runner().run(url, :download, all_command_opts, "after_move:%()j", addl_opts),
          {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
       {:ok, parsed_json}
     else
@@ -56,7 +56,7 @@ defmodule Pinchflat.YtDlp.Media do
   Returns {:ok, :downloadable | :ignorable} | {:error, any}
   """
   def get_downloadable_status(url) do
-    case backend_runner().run(url, [:simulate, :skip_download], "%(.{live_status})j") do
+    case backend_runner().run(url, :get_downloadable_status, [:simulate, :skip_download], "%(.{live_status})j") do
       {:ok, output} ->
         output
         |> Phoenix.json_library().decode!()
@@ -78,7 +78,7 @@ defmodule Pinchflat.YtDlp.Media do
 
     # NOTE: it doesn't seem like this command actually returns anything in `after_move` since
     # we aren't downloading the main media file
-    backend_runner().run(url, all_command_opts, "after_move:%()j", addl_opts)
+    backend_runner().run(url, :download_thumbnail, all_command_opts, "after_move:%()j", addl_opts)
   end
 
   @doc """
@@ -92,7 +92,7 @@ defmodule Pinchflat.YtDlp.Media do
     all_command_opts = [:simulate, :skip_download] ++ command_opts
     output_template = indexing_output_template()
 
-    case backend_runner().run(url, all_command_opts, output_template, addl_opts) do
+    case backend_runner().run(url, :get_media_attributes, all_command_opts, output_template, addl_opts) do
       {:ok, output} ->
         output
         |> Phoenix.json_library().decode!()

lib/pinchflat/yt_dlp/media_collection.ex

@@ -33,12 +33,13 @@ defmodule Pinchflat.YtDlp.MediaCollection do
     output_filepath = FilesystemUtils.generate_metadata_tmpfile(:json)
     file_listener_handler = Keyword.get(addl_opts, :file_listener_handler, false)
     runner_opts = [output_filepath: output_filepath, use_cookies: use_cookies]
+    action = :get_media_attributes_for_collection
 
     if file_listener_handler do
       file_listener_handler.(output_filepath)
     end
 
-    case runner.run(url, all_command_opts, output_template, runner_opts) do
+    case runner.run(url, action, all_command_opts, output_template, runner_opts) do
       {:ok, output} ->
         parsed_lines =
           output
@@ -82,8 +83,9 @@ defmodule Pinchflat.YtDlp.MediaCollection do
 
     all_command_opts = default_opts ++ command_opts
     output_template = "%(.{channel,channel_id,playlist_id,playlist_title,filename})j"
+    action = :get_source_details
 
-    with {:ok, output} <- backend_runner().run(source_url, all_command_opts, output_template, addl_opts),
+    with {:ok, output} <- backend_runner().run(source_url, action, all_command_opts, output_template, addl_opts),
          {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
       {:ok, format_source_details(parsed_json)}
     else
@@ -121,8 +123,9 @@ defmodule Pinchflat.YtDlp.MediaCollection do
 
     all_command_opts = [:skip_download] ++ command_opts
     output_template = "playlist:%()j"
+    action = :get_source_metadata
 
-    with {:ok, output} <- backend_runner().run(source_url, all_command_opts, output_template, addl_opts),
+    with {:ok, output} <- backend_runner().run(source_url, action, all_command_opts, output_template, addl_opts),
          {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
       {:ok, parsed_json}
     else

lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex

@@ -6,7 +6,7 @@ defmodule Pinchflat.YtDlp.YtDlpCommandRunner do
   yt-dlp command.
   """
 
-  @callback run(binary(), keyword(), binary()) :: {:ok, binary()} | {:error, binary(), integer()}
-  @callback run(binary(), keyword(), binary(), keyword()) :: {:ok, binary()} | {:error, binary(), integer()}
+  @callback run(binary(), atom(), keyword(), binary()) :: {:ok, binary()} | {:error, binary(), integer()}
+  @callback run(binary(), atom(), keyword(), binary(), keyword()) :: {:ok, binary()} | {:error, binary(), integer()}
   @callback version() :: {:ok, binary()} | {:error, binary()}
 end

test/pinchflat/downloading/media_download_worker_test.exs

@@ -9,8 +9,12 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
   alias Pinchflat.Downloading.MediaDownloadWorker
 
   setup do
-    stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
-    stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+    stub(YtDlpRunnerMock, :run, fn
+      _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""}
+      _url, :download, _opts, _ot, _addl -> {:ok, ""}
+    end)
+
+    stub(YtDlpRunnerMock, :run, fn _url, :get_downloadable_status, _opts, _ot -> {:ok, "{}"} end)
     stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end)
     stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
 
@@ -54,11 +58,11 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
 
   describe "perform/1" do
     test "it saves attributes to the media_item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       assert media_item.media_filepath == nil
       perform_job(MediaDownloadWorker, %{id: media_item.id})
@@ -68,11 +72,11 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it saves the metadata to the media_item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       assert media_item.metadata == nil
       perform_job(MediaDownloadWorker, %{id: media_item.id})
@@ -87,7 +91,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it sets the job to retryable if the download fails", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:error, "error"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> {:error, "error"} end)
 
       Oban.Testing.with_testing_mode(:inline, fn ->
         {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))
@@ -97,7 +101,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "sets the job to retryable if the download failed and was retried", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl ->
         {:error, "Unable to communicate with SponsorBlock", 1}
       end)
 
@@ -109,7 +113,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "does not set the job to retryable if retrying wouldn't fix the issue", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl ->
         {:error, "Something something Video unavailable something something", 1}
       end)
 
@@ -121,36 +125,36 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "it ensures error are returned in a 2-item tuple", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:error, "error", 1} end)
+      expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> {:error, "error", 1} end)
 
       assert {:error, :download_failed} = perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
     test "it does not download if the source is set to not download", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> :ok end)
+      expect(YtDlpRunnerMock, :run, 0, fn _url, :download, _opts, _ot, _addl -> :ok end)
 
       Sources.update_source(media_item.source, %{download_media: false})
 
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
     test "does not download if the media item is set to not download", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> :ok end)
+      expect(YtDlpRunnerMock, :run, 0, fn _url, :download, _opts, _ot, _addl -> :ok end)
 
       Media.update_media_item(media_item, %{prevent_download: true})
 
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
     test "it saves the file's size to the database", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         metadata = render_parsed_metadata(:media_metadata)
         FilesystemUtils.write_p!(metadata["filepath"], "test")
 
         {:ok, Phoenix.json_library().encode!(metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       perform_job(MediaDownloadWorker, %{id: media_item.id})
       media_item = Repo.reload(media_item)
@@ -159,10 +163,12 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "does not set redownloaded_at by default", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
+
       perform_job(MediaDownloadWorker, %{id: media_item.id})
       media_item = Repo.reload(media_item)
 
@@ -174,22 +180,22 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "sets the no_force_overwrites runner option", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
         assert :no_force_overwrites in opts
         refute :force_overwrites in opts
 
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
   end
 
   describe "perform/1 when testing non-downloadable media" do
     test "does not retry the job if the media is currently not downloadable", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+      expect(YtDlpRunnerMock, :run, fn _url, :get_downloadable_status, _opts, _ot ->
         {:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
       end)
 
@@ -203,7 +209,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
 
   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)
+      expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> :ok end)
 
       Sources.update_source(media_item.source, %{download_media: false})
       Media.update_media_item(media_item, %{prevent_download: true})
@@ -212,26 +218,26 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "sets force_overwrites runner option", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
         assert :force_overwrites in opts
         refute :no_force_overwrites in opts
 
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} 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, 1, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       perform_job(MediaDownloadWorker, %{id: media_item.id, quality_upgrade?: true})
       media_item = Repo.reload(media_item)
@@ -240,28 +246,28 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
     end
 
     test "sets force_overwrites runner option", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
         assert :force_overwrites in opts
         refute :no_force_overwrites in opts
 
         {:ok, render_metadata(:media_metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
     end
 
     test "deletes old files if the media item has been updated" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
         tmp_media_item = media_item_with_attachments()
         metadata = render_parsed_metadata(:media_metadata)
         metadata = Map.put(metadata, "filepath", tmp_media_item.media_filepath)
 
         {:ok, Phoenix.json_library().encode!(metadata)}
       end)
 
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
 
       old_media_item = media_item_with_attachments()
       perform_job(MediaDownloadWorker, %{id: old_media_item.id, force: true})
@@ -275,8 +281,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
 
   describe "perform/1 when testing user script callbacks" do
     setup do
-      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
-        {:ok, render_metadata(:media_metadata)}
+      stub(YtDlpRunnerMock, :run, fn
+        _url, :download, _opts, _ot, _addl -> {:ok, render_metadata(:media_metadata)}
+        _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""}
       end)
 
       :ok