Merge pull request #36 from cheerfulstoic/add-errors-for-long-postgres-identifiers

fix: Add errors for when postgres identifiers would be too long

Author: cheerfulstoic

lib/ecto_watch/watcher_server.ex

@@ -69,7 +69,10 @@ defmodule EctoWatch.WatcherServer do
       [options.schema_definition.primary_key | options.extra_columns]
       |> Enum.map_join(",", &"'#{&1}',row.#{&1}")
 
-    details = watcher_details(%{unique_label: unique_label, repo_mod: repo_mod, options: options})
+    details =
+      watcher_details(%{unique_label: unique_label, repo_mod: repo_mod, options: options})
+
+    validate_watcher_details!(details, options)
 
     Ecto.Adapters.SQL.query!(
       repo_mod,
@@ -92,6 +95,7 @@ defmodule EctoWatch.WatcherServer do
     )
 
     # Can't use the "OR REPLACE" syntax before postgres v13.3.4, so using DROP TRIGGER IF EXISTS
+    # THOUGHT: Could messages be lost during the drop/re-create?
     Ecto.Adapters.SQL.query!(
       repo_mod,
       """
@@ -238,6 +242,43 @@ defmodule EctoWatch.WatcherServer do
     }
   end
 
+  defp validate_watcher_details!(watcher_details, watcher_options) do
+    case Ecto.Adapters.SQL.query!(watcher_details.repo_mod, "SHOW max_identifier_length", []) do
+      %Postgrex.Result{rows: [[max_identifier_length]]} ->
+        max_identifier_length = String.to_integer(max_identifier_length)
+
+        max_byte_size =
+          max(
+            byte_size(watcher_details.function_name),
+            byte_size(watcher_details.trigger_name)
+          )
+
+        if max_byte_size > max_identifier_length do
+          difference = max_byte_size - max_identifier_length
+
+          if watcher_options.label do
+            raise """
+              Error for watcher: #{inspect(identifier(watcher_options))}
+
+              Label is #{difference} character(s) too long for the auto-generated Postgres trigger name.
+            """
+          else
+            raise """
+              Error for watcher: #{inspect(identifier(watcher_options))}
+
+              Schema module name is #{difference} character(s) too long for the auto-generated Postgres trigger name.
+
+              You may want to use the `label` option
+
+            """
+          end
+        end
+
+      other ->
+        raise "Unexpected result when querying for max_identifier_length: #{inspect(other)}"
+    end
+  end
+
   def topics(update_type, unique_label, returned_values, identifier_columns)
       when update_type in ~w[inserted updated deleted]a do
     [

test/ecto_watch_test.exs

@@ -50,6 +50,30 @@ defmodule EctoWatchTest do
     end
   end
 
+  defmodule ModuleWithAReallyLongName do
+    use Ecto.Schema
+
+    @moduledoc """
+    A module that *just* barely fits when creating function/trigger names
+    """
+
+    schema "a_module_with_a_really_long_name" do
+      field(:the_string, :string)
+    end
+  end
+
+  defmodule ModuleWithJustTooLongAName do
+    use Ecto.Schema
+
+    @moduledoc """
+    A module that *just* barely fits when creating function/trigger names
+    """
+
+    schema "a_module_with_just_too_long_a_name" do
+      field(:the_string, :string)
+    end
+  end
+
   setup do
     start_supervised!(TestRepo)
 
@@ -91,6 +115,28 @@ defmodule EctoWatchTest do
       []
     )
 
+    Ecto.Adapters.SQL.query!(
+      TestRepo,
+      """
+        CREATE TABLE a_module_with_a_really_long_name (
+          id SERIAL PRIMARY KEY,
+          the_string TEXT
+        )
+      """,
+      []
+    )
+
+    Ecto.Adapters.SQL.query!(
+      TestRepo,
+      """
+        CREATE TABLE a_module_with_just_too_long_a_name (
+          id SERIAL PRIMARY KEY,
+          the_string TEXT
+        )
+      """,
+      []
+    )
+
     %Postgrex.Result{
       rows: [[already_existing_id1]]
     } =
@@ -486,6 +532,78 @@ defmodule EctoWatchTest do
       start_supervised!({EctoWatch, repo: TestRepo, pub_sub: TestPubSub, watchers: []})
     end
 
+    test "Errors should be given if the schema module is too long for creating the trigger name" do
+      assert_raise RuntimeError,
+                   ~r/Schema module name is 1 character\(s\) too long for the auto-generated Postgres trigger/,
+                   fn ->
+                     start_supervised!(
+                       {EctoWatch,
+                        repo: TestRepo,
+                        pub_sub: TestPubSub,
+                        watchers: [
+                          {ModuleWithJustTooLongAName, :inserted}
+                        ]}
+                     )
+                   end
+
+      # Everything works fine if you're just at the limit
+      start_supervised!(
+        {EctoWatch,
+         repo: TestRepo,
+         pub_sub: TestPubSub,
+         watchers: [
+           {ModuleWithAReallyLongName, :inserted}
+         ]}
+      )
+
+      EctoWatch.subscribe({ModuleWithAReallyLongName, :inserted})
+
+      Ecto.Adapters.SQL.query!(
+        TestRepo,
+        "INSERT INTO a_module_with_a_really_long_name (the_string) VALUES ('the value')",
+        []
+      )
+
+      assert_receive {{ModuleWithAReallyLongName, :inserted}, %{id: _}}
+    end
+
+    test "Errors should be given if the label is too long" do
+      assert_raise RuntimeError,
+                   ~r/Label is 1 character\(s\) too long for the auto-generated Postgres trigger name/,
+                   fn ->
+                     start_supervised!(
+                       {EctoWatch,
+                        repo: TestRepo,
+                        pub_sub: TestPubSub,
+                        watchers: [
+                          {ModuleWithJustTooLongAName, :inserted,
+                           label: :the_label_is_also_just_much_too_long_such_a_shame}
+                        ]}
+                     )
+                   end
+
+      # Everything works fine if you're just at the limit
+      start_supervised!(
+        {EctoWatch,
+         repo: TestRepo,
+         pub_sub: TestPubSub,
+         watchers: [
+           {ModuleWithJustTooLongAName, :inserted,
+            label: :the_label_is_also_just_much_too_long_such_a_sham}
+         ]}
+      )
+
+      EctoWatch.subscribe(:the_label_is_also_just_much_too_long_such_a_sham)
+
+      Ecto.Adapters.SQL.query!(
+        TestRepo,
+        "INSERT INTO a_module_with_just_too_long_a_name (the_string) VALUES ('the value')",
+        []
+      )
+
+      assert_receive {:the_label_is_also_just_much_too_long_such_a_sham, %{id: _}}
+    end
+
     test "subscribe requires proper Ecto schema", %{
       already_existing_id1: already_existing_id1
     } do