Hook up user script event to media download

lib/pinchflat/downloading/download_option_builder.ex

@@ -8,6 +8,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Downloading.OutputPathBuilder
 
+  alias Pinchflat.Utils.FilesystemUtils, as: FSUtils
+
   @doc """
   Builds the options for yt-dlp to download media based on the given media's profile.
 
@@ -154,12 +156,10 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
       Enum.reduce(filenames, [], fn filename, acc ->
         filepath = Path.join(base_dir, filename)
 
-        case File.read(filepath) do
+        if FSUtils.exists_and_nonempty?(filepath) do
-          {:ok, file_data} ->
+          [filepath | acc]
-            if String.trim(file_data) != "", do: [filepath | acc], else: acc
+        else
-
+          acc
-          {:error, _} ->
-            acc
         end
       end)
 

lib/pinchflat/downloading/media_download_worker.ex

@@ -14,6 +14,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
   alias Pinchflat.Media
   alias Pinchflat.Downloading.MediaDownloader
 
+  alias Pinchflat.Lifecycle.UserScripts.CommandRunner, as: UserScriptRunner
+
   @doc """
   Starts the media_item media download worker and creates a task for the media_item.
 
@@ -56,11 +58,14 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
 
   defp download_media_and_schedule_jobs(media_item, is_redownload) do
     case MediaDownloader.download_for_media_item(media_item) do
-      {:ok, updated_media_item} ->
+      {:ok, downloaded_media_item} ->
-        Media.update_media_item(updated_media_item, %{
+        {:ok, updated_media_item} =
-          media_size_bytes: compute_media_filesize(updated_media_item),
+          Media.update_media_item(downloaded_media_item, %{
-          media_redownloaded_at: get_redownloaded_at(is_redownload)
+            media_size_bytes: compute_media_filesize(downloaded_media_item),
-        })
+            media_redownloaded_at: get_redownloaded_at(is_redownload)
+          })
+
+        :ok = run_user_script(updated_media_item)
 
         {:ok, updated_media_item}
 
@@ -74,21 +79,13 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
 
   defp compute_media_filesize(media_item) do
     case File.stat(media_item.media_filepath) do
-      {:ok, %{size: size}} ->
+      {:ok, %{size: size}} -> size
-        size
+      _ -> nil
-
-      _ ->
-        nil
     end
   end
 
-  defp get_redownloaded_at(is_redownload) do
+  defp get_redownloaded_at(true), do: DateTime.utc_now()
-    if is_redownload do
+  defp get_redownloaded_at(_), do: nil
-      DateTime.utc_now()
-    else
-      nil
-    end
-  end
 
   defp action_on_error(message) do
     # This will attempt re-download at the next indexing, but it won't be retried
@@ -103,4 +100,12 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
       {:error, :download_failed}
     end
   end
+
+  # NOTE: I like this pattern of using the default value so that I don't have to
+  # define it in config.exs (and friends). Consider using this elsewhere.
+  defp run_user_script(media_item) do
+    runner = Application.get_env(:pinchflat, :user_script_runner, UserScriptRunner)
+
+    runner.run(:media_downloaded, media_item)
+  end
 end

lib/pinchflat/lifecycle/notifications/command_runner.ex

@@ -28,10 +28,10 @@ defmodule Pinchflat.Lifecycle.Notifications.CommandRunner do
     default_opts = [:verbose]
     parsed_opts = CliUtils.parse_options(default_opts ++ command_opts)
 
-    {output, return_code} = CliUtils.wrap_cmd(backend_executable(), parsed_opts ++ endpoints)
+    {output, exit_code} = CliUtils.wrap_cmd(backend_executable(), parsed_opts ++ endpoints)
     Logger.info("[apprise] response: #{output}")
 
-    case return_code do
+    case exit_code do
       0 -> {:ok, String.trim(output)}
       _ -> {:error, String.trim(output)}
     end

lib/pinchflat/lifecycle/user_scripts/command_runner.ex

@@ -0,0 +1,76 @@
+defmodule Pinchflat.Lifecycle.UserScripts.CommandRunner do
+  @moduledoc """
+  Runs custom user commands commands using the `System.cmd/3` function
+  """
+
+  require Logger
+
+  alias Pinchflat.Utils.CliUtils
+  alias Pinchflat.Utils.FilesystemUtils
+  alias Pinchflat.Lifecycle.UserScripts.UserScriptCommandRunner
+
+  @behaviour UserScriptCommandRunner
+
+  @event_types [
+    :media_downloaded,
+    :media_deleted
+  ]
+
+  @doc """
+  Runs the user script command for the given event type. Passes the event
+  and the encoded data to the user script command.
+
+  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.
+
+  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
+  """
+  @impl UserScriptCommandRunner
+  def run(event_type, encodable_data) when event_type in @event_types do
+    case backend_executable() do
+      {:ok, :no_executable} ->
+        :ok
+
+      {:ok, executable_path} ->
+        {:ok, encoded_data} = Phoenix.json_library().encode(encodable_data)
+
+        {output, exit_code} =
+          CliUtils.wrap_cmd(
+            executable_path,
+            [to_string(event_type), encoded_data],
+            [],
+            logging_arg_override: "[supressed]"
+          )
+
+        handle_output(output, exit_code)
+    end
+  end
+
+  def run(event_type, _encodable_data) do
+    raise ArgumentError, "Invalid event type: #{inspect(event_type)}"
+  end
+
+  defp handle_output(output, exit_code) do
+    Logger.debug("Custom lifecycle script had an exit code of #{exit_code} output: #{output}")
+
+    :ok
+  end
+
+  defp backend_executable do
+    base_dir = Application.get_env(:pinchflat, :extras_directory)
+    filepath = Path.join([base_dir, "user-scripts", "lifecycle"])
+
+    if FilesystemUtils.exists_and_nonempty?(filepath) do
+      {:ok, filepath}
+    else
+      Logger.warning("User scripts lifecyle file either not present or is empty. Skipping.")
+
+      {:ok, :no_executable}
+    end
+  end
+end

lib/pinchflat/lifecycle/user_scripts/user_script_command_runner.ex

@@ -0,0 +1,10 @@
+defmodule Pinchflat.Lifecycle.UserScripts.UserScriptCommandRunner do
+  @moduledoc """
+  A behaviour for running custom user scripts on certain events.
+
+  Used so we can implement Mox for testing without actually running the
+  user's command.
+  """
+
+  @callback run(atom(), map()) :: :ok | {:error, binary()}
+end

lib/pinchflat/utils/cli_utils.ex

@@ -13,17 +13,22 @@ defmodule Pinchflat.Utils.CliUtils do
   commands if the job runner is cancelled.
 
   Delegates to `System.cmd/3` and any options/output
-  are passed through.
+  are passed through. Custom options can be passed in.
+
+  Custom options:
+    - logging_arg_override: if set, the passed value will be logged in place of
+      the actual arguments passed to the command
 
   Returns {binary(), integer()}
   """
-  def wrap_cmd(command, args, opts \\ []) do
+  def wrap_cmd(command, args, passthrough_opts \\ [], opts \\ []) do
     wrapper_command = Path.join(:code.priv_dir(:pinchflat), "cmd_wrapper.sh")
     actual_command = [command] ++ args
+    logging_arg_override = Keyword.get(opts, :logging_arg_override, Enum.join(args, " "))
 
-    Logger.info("[command_wrapper]: #{command} called with: #{Enum.join(args, " ")}")
+    Logger.info("[command_wrapper]: #{command} called with: #{logging_arg_override}")
 
-    System.cmd(wrapper_command, actual_command, opts)
+    System.cmd(wrapper_command, actual_command, passthrough_opts)
   end
 
   @doc """

lib/pinchflat/utils/filesystem_utils.ex

@@ -5,6 +5,21 @@ defmodule Pinchflat.Utils.FilesystemUtils do
   alias Pinchflat.Media
   alias Pinchflat.Utils.StringUtils
 
+  @doc """
+  Checks if a file exists and has non-whitespace contents.
+
+  Returns boolean()
+  """
+  def exists_and_nonempty?(filepath) do
+    case File.read(filepath) do
+      {:ok, contents} ->
+        String.trim(contents) != ""
+
+      _ ->
+        false
+    end
+  end
+
   @doc """
   Generates a temporary file and returns its path. The file is empty and has the given type.
   Generates all the directories in the path if they don't exist.

lib/pinchflat/yt_dlp/command_runner.ex

@@ -81,16 +81,10 @@ defmodule Pinchflat.YtDlp.CommandRunner do
     Enum.reduce(filename_options_map, [], fn {opt_name, filename}, acc ->
       filepath = Path.join(base_dir, filename)
 
-      case File.read(filepath) do
+      if FSUtils.exists_and_nonempty?(filepath) do
-        {:ok, file_data} ->
+        [{opt_name, filepath} | acc]
-          if String.trim(file_data) != "" do
+      else
-            [{opt_name, filepath} | acc]
+        acc
-          else
-            acc
-          end
-
-        {:error, _} ->
-          acc
       end
     end)
   end

test/pinchflat/downloading/media_download_worker_test.exs

@@ -12,9 +12,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
   setup :verify_on_exit!
 
   setup do
-    stub(HTTPClientMock, :get, fn _url, _headers, _opts ->
+    stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end)
-      {:ok, ""}
+    stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
-    end)
 
     media_item =
       %{media_filepath: nil}
@@ -185,6 +184,20 @@ 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
+      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

test/pinchflat/downloading/output_path/parser_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Downloading.OutputPath.ParserTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Downloading.OutputPath.Parser
 

test/pinchflat/lifecycle/notifications/command_runner_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Lifecycle.Notifications.CommandRunnerTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Lifecycle.Notifications.CommandRunner, as: Runner
 

test/pinchflat/lifecycle/user_scripts/command_runner_test.exs

@@ -0,0 +1,79 @@
+defmodule Pinchflat.Lifecycle.UserScripts.CommandRunnerTest do
+  use ExUnit.Case, async: false
+
+  alias Pinchflat.Utils.FilesystemUtils
+  alias Pinchflat.Lifecycle.UserScripts.CommandRunner, as: Runner
+
+  setup do
+    FilesystemUtils.write_p!(filepath(), "")
+    File.chmod(filepath(), 0o755)
+
+    :ok
+  end
+
+  describe "run/2" do
+    test "runs the provided lifecycle file if present" do
+      # We *love* indirectly testing side effects
+      tmp_dir = Application.get_env(:pinchflat, :tmpfile_directory)
+      File.write(filepath(), "#!/bin/bash\ntouch #{tmp_dir}/test_file\n")
+
+      refute File.exists?("#{tmp_dir}/test_file")
+      assert :ok = Runner.run(:media_downloaded, %{})
+      assert File.exists?("#{tmp_dir}/test_file")
+    end
+
+    test "passes the event name to the script" do
+      tmp_dir = Application.get_env(:pinchflat, :tmpfile_directory)
+      File.write(filepath(), "#!/bin/bash\necho $1 > #{tmp_dir}/event_name\n")
+
+      assert :ok = Runner.run(:media_downloaded, %{})
+      assert File.read!("#{tmp_dir}/event_name") == "media_downloaded\n"
+    end
+
+    test "passes the encoded data to the script" do
+      tmp_dir = Application.get_env(:pinchflat, :tmpfile_directory)
+      File.write(filepath(), "#!/bin/bash\necho $2 > #{tmp_dir}/encoded_data\n")
+
+      assert :ok = Runner.run(:media_downloaded, %{foo: "bar"})
+      assert File.read!("#{tmp_dir}/encoded_data") == "{\"foo\":\"bar\"}\n"
+    end
+
+    test "does nothing if the lifecycle file is not present" do
+      :ok = File.rm(filepath())
+
+      assert :ok = Runner.run(:media_downloaded, %{})
+    end
+
+    test "does nothing if the lifecycle file is empty" do
+      File.write(filepath(), "")
+
+      assert :ok = Runner.run(:media_downloaded, %{})
+    end
+
+    test "returns :ok if the command exits with a non-zero status" do
+      File.write(filepath(), "#!/bin/bash\nexit 1\n")
+
+      assert :ok = Runner.run(:media_downloaded, %{})
+    end
+
+    test "gets upset if you pass an invalid event type" do
+      assert_raise ArgumentError, "Invalid event type: :invalid_event", fn ->
+        Runner.run(:invalid_event, %{})
+      end
+    end
+
+    test "gets upset if the record cannot be decoded" do
+      File.write(filepath(), "#!/bin/bash")
+
+      assert_raise MatchError, fn ->
+        Runner.run(:media_downloaded, %Ecto.Changeset{})
+      end
+    end
+  end
+
+  defp filepath do
+    base_dir = Application.get_env(:pinchflat, :extras_directory)
+
+    Path.join([base_dir, "user-scripts", "lifecycle"])
+  end
+end

test/pinchflat/slow_indexing/file_follower_server_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.SlowIndexing.FileFollowerServerTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias alias Pinchflat.Utils.FilesystemUtils
   alias Pinchflat.SlowIndexing.FileFollowerServer

test/pinchflat/utils/changeset_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.ChangesetUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   defmodule MockSchema do
     use Ecto.Schema

test/pinchflat/utils/cli_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.CliUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.CliUtils
 

test/pinchflat/utils/datetime_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.DatetimeUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.DatetimeUtils
 

test/pinchflat/utils/filesystem_utils_test.exs

@@ -5,6 +5,38 @@ defmodule Pinchflat.Utils.FilesystemUtilsTest do
 
   alias Pinchflat.Utils.FilesystemUtils
 
+  describe "exists_and_nonempty?" do
+    test "returns true if a file exists and has contents" do
+      filepath = FilesystemUtils.generate_metadata_tmpfile(:json)
+      File.write(filepath, "{}")
+
+      assert FilesystemUtils.exists_and_nonempty?(filepath)
+
+      File.rm!(filepath)
+    end
+
+    test "returns false if a file doesn't exist" do
+      refute FilesystemUtils.exists_and_nonempty?("/nonexistent/file.json")
+    end
+
+    test "returns false if a file exists but is empty" do
+      filepath = FilesystemUtils.generate_metadata_tmpfile(:json)
+
+      refute FilesystemUtils.exists_and_nonempty?(filepath)
+
+      File.rm!(filepath)
+    end
+
+    test "trims the contents before checking" do
+      filepath = FilesystemUtils.generate_metadata_tmpfile(:json)
+      File.write(filepath, "  \n\n  \r\n  ")
+
+      refute FilesystemUtils.exists_and_nonempty?(filepath)
+
+      File.rm!(filepath)
+    end
+  end
+
   describe "generate_metadata_tmpfile/1" do
     test "creates a tmpfile and returns its path" do
       res = FilesystemUtils.generate_metadata_tmpfile(:json)

test/pinchflat/utils/function_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.FunctionUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.FunctionUtils
 

test/pinchflat/utils/number_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.NumberUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.NumberUtils
 

test/pinchflat/utils/string_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.StringUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.StringUtils
 

test/pinchflat/utils/xml_utils_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.Utils.XmlUtilsTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.XmlUtils
 

test/pinchflat/yt_dlp/command_runner_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.YtDlp.CommandRunnerTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   alias Pinchflat.Utils.FilesystemUtils
 
@@ -75,6 +75,9 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do
 
       refute String.contains?(output, "--cookies")
       refute String.contains?(output, cookie_file)
+
+      # Cleanup
+      FilesystemUtils.write_p!(cookie_file, "")
     end
   end
 

test/pinchflat_web/controllers/error_html_test.exs

@@ -1,5 +1,5 @@
 defmodule PinchflatWeb.ErrorHTMLTest do
-  use PinchflatWeb.ConnCase, async: true
+  use PinchflatWeb.ConnCase, async: false
 
   # Bring render_to_string/4 for testing custom views
   import Phoenix.Template

test/pinchflat_web/controllers/error_json_test.exs

@@ -1,5 +1,5 @@
 defmodule PinchflatWeb.ErrorJSONTest do
-  use PinchflatWeb.ConnCase, async: true
+  use PinchflatWeb.ConnCase, async: false
 
   test "renders 404" do
     assert PinchflatWeb.ErrorJSON.render("404.json", %{}) == %{errors: %{detail: "Not Found"}}

test/test_helper.exs

@@ -7,6 +7,9 @@ Application.put_env(:pinchflat, :apprise_runner, AppriseRunnerMock)
 Mox.defmock(HTTPClientMock, for: Pinchflat.HTTP.HTTPBehaviour)
 Application.put_env(:pinchflat, :http_client, HTTPClientMock)
 
+Mox.defmock(UserScriptRunnerMock, for: Pinchflat.Lifecycle.UserScripts.UserScriptCommandRunner)
+Application.put_env(:pinchflat, :user_script_runner, UserScriptRunnerMock)
+
 ExUnit.start()
 Ecto.Adapters.SQL.Sandbox.mode(Pinchflat.Repo, :manual)
 Faker.start()