diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index 370ed53..c7ea232 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -39,7 +39,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do - `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} + Returns :ok | {:error, any, ...any} """ @impl Oban.Worker def perform(%Oban.Job{args: %{"id" => media_item_id} = args}) do @@ -59,12 +59,14 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: media item #{media_item_id} stale") end + # If a user script exists and, when run, returns a non-zero exit code, prevent this and all future downloads + # of the media item. defp fetch_and_run_prevent_download_user_script(media_item_id) do media_item = Media.get_media_item!(media_item_id) {:ok, media_item} = case run_user_script(:media_pre_download, media_item) do - {:ok, _, exit_code} when exit_code > 0 -> Media.update_media_item(media_item, %{prevent_download: true}) + {:ok, _, exit_code} when exit_code != 0 -> Media.update_media_item(media_item, %{prevent_download: true}) _ -> {:ok, media_item} end @@ -83,9 +85,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do media_redownloaded_at: get_redownloaded_at(is_quality_upgrade) }) - :ok = run_user_script(:media_downloaded, updated_media_item) + run_user_script(:media_downloaded, updated_media_item) - {:ok, updated_media_item} + :ok {:recovered, _} -> {:error, :retry} diff --git a/lib/pinchflat/lifecycle/user_scripts/command_runner.ex b/lib/pinchflat/lifecycle/user_scripts/command_runner.ex index 2bdc739..54aa38c 100644 --- a/lib/pinchflat/lifecycle/user_scripts/command_runner.ex +++ b/lib/pinchflat/lifecycle/user_scripts/command_runner.ex @@ -23,13 +23,14 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunner do This function will succeed in almost all cases, even if the user script command failed - this is because I don't want bad scripts to stop the whole process. - If something fails, it'll be logged. + If something fails, it'll be logged and returned BUT the tuple will always + start with {:ok, ...}. The only things that can cause a true failure are passing in an invalid event type or if the passed data cannot be encoded into JSON - both indicative of failures in the development process. - Returns :ok + Returns {:ok, :no_executable} | {:ok, output, exit_code} """ @impl UserScriptCommandRunner def run(event_type, encodable_data) when event_type in @event_types do @@ -63,7 +64,7 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunner do if FilesystemUtils.exists_and_nonempty?(filepath) do {:ok, filepath} else - Logger.warning("User scripts lifecyle file either not present or is empty. Skipping.") + Logger.info("User scripts lifecyle file either not present or is empty. Skipping.") {:ok, :no_executable} end diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index f7dfc23..35eabeb 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -162,20 +162,6 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do assert media_item.media_redownloaded_at == nil end - test "calls the user script runner", %{media_item: media_item} do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> - {:ok, render_metadata(:media_metadata)} - end) - - expect(UserScriptRunnerMock, :run, fn :media_downloaded, data -> - assert data.id == media_item.id - - {:ok, "", 0} - end) - - perform_job(MediaDownloadWorker, %{id: media_item.id}) - end - test "does not blow up if the record doesn't exist" do assert :ok = perform_job(MediaDownloadWorker, %{id: 0}) end @@ -237,4 +223,59 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do perform_job(MediaDownloadWorker, %{id: media_item.id, force: true}) end end + + describe "perform/1 when testing user script callbacks" do + setup do + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> + {:ok, render_metadata(:media_metadata)} + end) + + :ok + end + + test "calls the media_pre_download user script runner", %{media_item: media_item} do + expect(UserScriptRunnerMock, :run, fn :media_pre_download, data -> + assert data.id == media_item.id + + {:ok, "", 0} + end) + + expect(UserScriptRunnerMock, :run, fn :media_downloaded, _ -> {:ok, "", 0} end) + + perform_job(MediaDownloadWorker, %{id: media_item.id}) + end + + test "does not download the media if the pre-download script returns an error", %{media_item: media_item} do + expect(UserScriptRunnerMock, :run, fn :media_pre_download, _ -> {:ok, "", 1} end) + + assert :ok = perform_job(MediaDownloadWorker, %{id: media_item.id}) + media_item = Repo.reload!(media_item) + + refute media_item.media_filepath + assert media_item.prevent_download + end + + test "downloads media if the pre-download script is not present", %{media_item: media_item} do + expect(UserScriptRunnerMock, :run, fn :media_pre_download, _ -> {:ok, :no_executable} end) + expect(UserScriptRunnerMock, :run, fn :media_downloaded, _ -> {:ok, :no_executable} end) + + assert :ok = perform_job(MediaDownloadWorker, %{id: media_item.id}) + media_item = Repo.reload!(media_item) + + assert media_item.media_filepath + refute media_item.prevent_download + end + + test "calls the media_downloaded user script runner", %{media_item: media_item} do + expect(UserScriptRunnerMock, :run, fn :media_pre_download, _ -> {:ok, "", 0} end) + + expect(UserScriptRunnerMock, :run, fn :media_downloaded, data -> + assert data.id == media_item.id + + {:ok, "", 0} + end) + + perform_job(MediaDownloadWorker, %{id: media_item.id}) + end + end end