Webhook refactoring: GenServer queue and consolidate duplicated helpers

**GenServer-based Webhook Processor**
- New WebhookProcessor GenServer at lib/reencodarr_web/webhook_processor.ex
- Processes webhook tasks sequentially to prevent SQLite lock contention
- Uses compile-time conditional to execute synchronously in test (avoiding sandbox issues)
- Spawns linked processes in production for true async execution
- Wraps all tasks with Retry.retry_on_db_busy for transient busy error handling

**Controller Cleanup**
- Created WebhookHelpers module consolidating duplicate validation and update logic
- Extracted: validate_file_path/1, validate_file_size/1, validate_file_id/1,
  update_or_upsert_video/2, handle_update_result/6
- Both Sonarr and Radarr controllers now use WebhookHelpers for shared functions
- Removed dead process_single_episode_download/1 (duplicate of process_validated_episode_file/1)

**Test Fix**
- Fixed exclude_patterns_test.exs: changed video_stats_query() to video_savings_query()
- The test was querying the wrong function; savings were split in prior refactor

**Verification**
- All 2492 tests pass (previously 1 failure)
- All 34 webhook controller tests pass
- Compilation clean with no warnings
- ~53 lines of duplication removed from controllers

Note: Explicit try/catch is necessary to catch :exit signals from OTP processes;
credo's implicit-try recommendation does not apply to this pattern.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

Author: mjc

config/test.exs

@@ -29,9 +29,6 @@ config :reencodarr, Reencodarr.Mailer, adapter: Swoosh.Adapters.Test
 # Disable swoosh api client as it is only required for production adapters
 config :swoosh, :api_client, false
 
-# In tests, run webhook handlers synchronously to avoid test sandbox issues with async tasks
-config :reencodarr, :webhook_async, false
-
 # Print only warnings and errors during test
 config :logger, level: :warning
 

lib/reencodarr/application.ex

@@ -60,6 +60,8 @@ defmodule Reencodarr.Application do
       {Phoenix.PubSub, name: Reencodarr.PubSub},
       # Start the Finch HTTP client for sending emails
       {Finch, name: Reencodarr.Finch},
+      # Webhook processor GenServer - queues webhook tasks to prevent SQLite lock contention
+      ReencodarrWeb.WebhookProcessor,
       # Start to serve requests, typically the last entry
       ReencodarrWeb.Endpoint,
       # DynamicSupervisor for port-holder processes (AbAv1.Encoder, AbAv1.CrfSearcher).

lib/reencodarr_web/controllers/radarr_webhook_controller.ex

@@ -2,6 +2,7 @@ defmodule ReencodarrWeb.RadarrWebhookController do
   use ReencodarrWeb, :controller
   require Logger
   alias Reencodarr.Core.Retry
+  alias ReencodarrWeb.WebhookHelpers
 
   def radarr(conn, %{"eventType" => "Test"} = params), do: handle_test(conn, params)
   def radarr(conn, %{"eventType" => "Grab"} = params), do: handle_grab(conn, params)
@@ -17,11 +18,7 @@ defmodule ReencodarrWeb.RadarrWebhookController do
   def radarr(conn, params), do: handle_unknown(conn, params)
 
   defp run_async(fun) do
-    if Application.get_env(:reencodarr, :webhook_async, true) do
-      Task.start(fun)
-    else
-      fun.()
-    end
+    ReencodarrWeb.WebhookProcessor.process(fun)
   end
 
   defp handle_test(conn, _params) do
@@ -59,51 +56,6 @@ defmodule ReencodarrWeb.RadarrWebhookController do
     send_resp(conn, :no_content, "")
   end
 
-  defp update_or_upsert_video(%{"previousPath" => old_path, "path" => new_path} = file, source) do
-    case Reencodarr.Media.get_video_by_path(old_path) do
-      {:error, :not_found} ->
-        Logger.warning("No video found for old path: #{old_path}, upserting as new")
-        Reencodarr.Sync.upsert_video_from_file(file, source)
-
-      {:ok, video} ->
-        video
-        |> Reencodarr.Media.update_video(%{path: new_path})
-        |> handle_update_result(video, old_path, new_path, file, source)
-    end
-  end
-
-  defp handle_update_result({:ok, _}, _video, old_path, new_path, _file, _source) do
-    Logger.info("Updated video path from #{old_path} to #{new_path}")
-  end
-
-  defp handle_update_result(
-         {:error, %Ecto.Changeset{errors: [path: {"has already been taken", _}]}},
-         video,
-         old_path,
-         new_path,
-         file,
-         source
-       ) do
-    Logger.warning(
-      "Video with path #{new_path} already exists, removing old entry and updating existing video"
-    )
-
-    case Reencodarr.Media.delete_video_with_vmafs(video) do
-      {:ok, _} ->
-        Logger.info("Successfully removed old video entry at #{old_path}")
-        Reencodarr.Sync.upsert_video_from_file(file, source)
-
-      {:error, reason} ->
-        Logger.error("Failed to remove old video entry at #{old_path}: #{inspect(reason)}")
-    end
-  end
-
-  defp handle_update_result({:error, changeset}, _video, old_path, new_path, _file, _source) do
-    Logger.error(
-      "Failed to update video path from #{old_path} to #{new_path}: #{inspect(changeset.errors)}"
-    )
-  end
-
   defp handle_moviefile(conn, %{"movieFile" => movie_file}) do
     Logger.info("Received new MovieFile event from Radarr!")
     run_async(fn -> process_movie_file(movie_file) end)
@@ -141,9 +93,9 @@ defmodule ReencodarrWeb.RadarrWebhookController do
   # Validation functions
 
   defp validate_movie_file(file) when is_map(file) do
-    with {:ok, path} <- validate_file_path(file["path"]),
-         {:ok, size} <- validate_file_size(file["size"]),
-         {:ok, id} <- validate_file_id(file["id"] || file["movieFileId"]) do
+    with {:ok, path} <- WebhookHelpers.validate_file_path(file["path"]),
+         {:ok, size} <- WebhookHelpers.validate_file_size(file["size"]),
+         {:ok, id} <- WebhookHelpers.validate_file_id(file["id"] || file["movieFileId"]) do
       {:ok, %{path: path, size: size, id: id, raw_file: file}}
     else
       {:error, reason} -> {:error, reason}
@@ -152,24 +104,6 @@ defmodule ReencodarrWeb.RadarrWebhookController do
 
   defp validate_movie_file(_), do: {:error, "movie file must be a map"}
 
-  defp validate_file_path(path) when is_binary(path) and path != "" do
-    if String.trim(path) != "" do
-      {:ok, path}
-    else
-      {:error, "path cannot be empty"}
-    end
-  end
-
-  defp validate_file_path(nil), do: {:error, "path is required"}
-  defp validate_file_path(_), do: {:error, "path must be a string"}
-
-  defp validate_file_size(size) when is_integer(size) and size > 0, do: {:ok, size}
-  defp validate_file_size(nil), do: {:error, "size is required"}
-  defp validate_file_size(_), do: {:error, "size must be a positive integer"}
-
-  defp validate_file_id(id) when is_binary(id) or is_integer(id), do: {:ok, id}
-  defp validate_file_id(_), do: {:error, "file id is required"}
-
   defp process_valid_movie_file(%{path: path, size: size, id: id, raw_file: file}) do
     scene_name = file["sceneName"] || Path.basename(path)
     Logger.info("Processing file #{scene_name}...")
@@ -245,7 +179,7 @@ defmodule ReencodarrWeb.RadarrWebhookController do
   defp process_movie_renames(files) do
     Enum.each(files, fn file ->
       Retry.retry_on_db_busy(fn ->
-        update_or_upsert_video(file, :radarr)
+        WebhookHelpers.update_or_upsert_video(file, :radarr)
       end)
     end)
   end

lib/reencodarr_web/controllers/sonarr_webhook_controller.ex

@@ -2,6 +2,7 @@ defmodule ReencodarrWeb.SonarrWebhookController do
   use ReencodarrWeb, :controller
   require Logger
   alias Reencodarr.Core.Retry
+  alias ReencodarrWeb.WebhookHelpers
 
   def sonarr(conn, %{"eventType" => "Test"} = params), do: handle_test(conn, params)
   def sonarr(conn, %{"eventType" => "Grab"} = params), do: handle_grab(conn, params)
@@ -20,11 +21,7 @@ defmodule ReencodarrWeb.SonarrWebhookController do
   def sonarr(conn, params), do: handle_unknown(conn, params)
 
   defp run_async(fun) do
-    if Application.get_env(:reencodarr, :webhook_async, true) do
-      Task.start(fun)
-    else
-      fun.()
-    end
+    ReencodarrWeb.WebhookProcessor.process(fun)
   end
 
   defp handle_test(conn, _params) do
@@ -45,7 +42,7 @@ defmodule ReencodarrWeb.SonarrWebhookController do
 
   defp handle_download(conn, %{"episodeFile" => episode_file} = _params)
        when is_map(episode_file) do
-    run_async(fn -> process_single_episode_download(episode_file) end)
+    run_async(fn -> process_validated_episode_file(episode_file) end)
     send_resp(conn, :no_content, "")
   end
 
@@ -77,56 +74,11 @@ defmodule ReencodarrWeb.SonarrWebhookController do
   defp process_episode_renames(files) do
     Enum.each(files, fn file ->
       Retry.retry_on_db_busy(fn ->
-        update_or_upsert_video(file, :sonarr)
+        WebhookHelpers.update_or_upsert_video(file, :sonarr)
       end)
     end)
   end
 
-  defp update_or_upsert_video(%{"previousPath" => old_path, "path" => new_path} = file, source) do
-    case Reencodarr.Media.get_video_by_path(old_path) do
-      {:error, :not_found} ->
-        Logger.warning("No video found for old path: #{old_path}, upserting as new")
-        Reencodarr.Sync.upsert_video_from_file(file, source)
-
-      {:ok, video} ->
-        video
-        |> Reencodarr.Media.update_video(%{path: new_path})
-        |> handle_update_result(video, old_path, new_path, file, source)
-    end
-  end
-
-  defp handle_update_result({:ok, _}, _video, old_path, new_path, _file, _source) do
-    Logger.info("Updated video path from #{old_path} to #{new_path}")
-  end
-
-  defp handle_update_result(
-         {:error, %Ecto.Changeset{errors: [path: {"has already been taken", _}]}},
-         video,
-         old_path,
-         new_path,
-         file,
-         source
-       ) do
-    Logger.warning(
-      "Video with path #{new_path} already exists, removing old entry and updating existing video"
-    )
-
-    case Reencodarr.Media.delete_video_with_vmafs(video) do
-      {:ok, _} ->
-        Logger.info("Successfully removed old video entry at #{old_path}")
-        Reencodarr.Sync.upsert_video_from_file(file, source)
-
-      {:error, reason} ->
-        Logger.error("Failed to remove old video entry at #{old_path}: #{inspect(reason)}")
-    end
-  end
-
-  defp handle_update_result({:error, changeset}, _video, old_path, new_path, _file, _source) do
-    Logger.error(
-      "Failed to update video path from #{old_path} to #{new_path}: #{inspect(changeset.errors)}"
-    )
-  end
-
   defp handle_episodefile(conn, %{"episodeFile" => episode_file}) do
     Logger.info("Received new episodefile event from Sonarr!")
     run_async(fn -> process_episode_file(episode_file) end)
@@ -164,9 +116,9 @@ defmodule ReencodarrWeb.SonarrWebhookController do
   # Validation functions
 
   defp validate_episode_file(file) when is_map(file) do
-    with {:ok, path} <- validate_file_path(file["path"]),
-         {:ok, size} <- validate_file_size(file["size"]),
-         {:ok, id} <- validate_file_id(file["id"]) do
+    with {:ok, path} <- WebhookHelpers.validate_file_path(file["path"]),
+         {:ok, size} <- WebhookHelpers.validate_file_size(file["size"]),
+         {:ok, id} <- WebhookHelpers.validate_file_id(file["id"]) do
       scene_name = file["sceneName"] || Path.basename(path)
       {:ok, %{path: path, size: size, id: id, scene_name: scene_name, raw_file: file}}
     else
@@ -176,24 +128,6 @@ defmodule ReencodarrWeb.SonarrWebhookController do
 
   defp validate_episode_file(_), do: {:error, "episode file must be a map"}
 
-  defp validate_file_path(path) when is_binary(path) and path != "" do
-    if String.trim(path) != "" do
-      {:ok, path}
-    else
-      {:error, "path cannot be empty"}
-    end
-  end
-
-  defp validate_file_path(nil), do: {:error, "path is required"}
-  defp validate_file_path(_), do: {:error, "path must be a string"}
-
-  defp validate_file_size(size) when is_integer(size) and size > 0, do: {:ok, size}
-  defp validate_file_size(nil), do: {:error, "size is required"}
-  defp validate_file_size(_), do: {:error, "size must be a positive integer"}
-
-  defp validate_file_id(id) when is_binary(id) or is_integer(id), do: {:ok, id}
-  defp validate_file_id(_), do: {:error, "file id is required"}
-
   defp reconcile_waiting_bad_file_issues({:ok, {:ok, video}}) do
     Reencodarr.Media.reconcile_replacement_video(video, :sonarr)
   end
@@ -223,23 +157,6 @@ defmodule ReencodarrWeb.SonarrWebhookController do
     end
   end
 
-  defp process_single_episode_download(file) do
-    case validate_episode_file(file) do
-      {:ok, validated_file} ->
-        scene_name = validated_file.scene_name
-        Logger.info("Received download event from Sonarr for #{scene_name}!")
-
-        Retry.retry_on_db_busy(fn ->
-          validated_file.raw_file
-          |> Reencodarr.Sync.upsert_video_from_file(:sonarr)
-          |> reconcile_waiting_bad_file_issues()
-        end)
-
-      {:error, reason} ->
-        Logger.error("Invalid episode file data from Sonarr: #{reason}")
-    end
-  end
-
   defp process_episode_file(file) do
     Retry.retry_on_db_busy(fn ->
       file

lib/reencodarr_web/webhook_helpers.ex

@@ -0,0 +1,75 @@
+defmodule ReencodarrWeb.WebhookHelpers do
+  @moduledoc """
+  Shared helpers for webhook processing across both Sonarr and Radarr controllers.
+  Consolidates duplicate validation and update logic that was copied between controllers.
+  """
+
+  require Logger
+
+  # Path, size, and ID validation
+
+  def validate_file_path(path) when is_binary(path) and path != "" do
+    if String.trim(path) != "" do
+      {:ok, path}
+    else
+      {:error, "path cannot be empty"}
+    end
+  end
+
+  def validate_file_path(nil), do: {:error, "path is required"}
+  def validate_file_path(_), do: {:error, "path must be a string"}
+
+  def validate_file_size(size) when is_integer(size) and size > 0, do: {:ok, size}
+  def validate_file_size(nil), do: {:error, "size is required"}
+  def validate_file_size(_), do: {:error, "size must be a positive integer"}
+
+  def validate_file_id(id) when is_binary(id) or is_integer(id), do: {:ok, id}
+  def validate_file_id(_), do: {:error, "file id is required"}
+
+  # Video update and rename handling
+
+  def update_or_upsert_video(%{"previousPath" => old_path, "path" => new_path} = file, source) do
+    case Reencodarr.Media.get_video_by_path(old_path) do
+      {:error, :not_found} ->
+        Logger.warning("No video found for old path: #{old_path}, upserting as new")
+        Reencodarr.Sync.upsert_video_from_file(file, source)
+
+      {:ok, video} ->
+        video
+        |> Reencodarr.Media.update_video(%{path: new_path})
+        |> handle_update_result(video, old_path, new_path, file, source)
+    end
+  end
+
+  def handle_update_result({:ok, _}, _video, old_path, new_path, _file, _source) do
+    Logger.info("Updated video path from #{old_path} to #{new_path}")
+  end
+
+  def handle_update_result(
+        {:error, %Ecto.Changeset{errors: [path: {"has already been taken", _}]}},
+        video,
+        old_path,
+        new_path,
+        file,
+        source
+      ) do
+    Logger.warning(
+      "Video with path #{new_path} already exists, removing old entry and updating existing video"
+    )
+
+    case Reencodarr.Media.delete_video_with_vmafs(video) do
+      {:ok, _} ->
+        Logger.info("Successfully removed old video entry at #{old_path}")
+        Reencodarr.Sync.upsert_video_from_file(file, source)
+
+      {:error, reason} ->
+        Logger.error("Failed to remove old video entry at #{old_path}: #{inspect(reason)}")
+    end
+  end
+
+  def handle_update_result({:error, changeset}, _video, old_path, new_path, _file, _source) do
+    Logger.error(
+      "Failed to update video path from #{old_path} to #{new_path}: #{inspect(changeset.errors)}"
+    )
+  end
+end