swap to mfargs for :constraint_handler option

- adds initial docs
- updates behaviours and built-in connections

Author: petermueller

integration_test/myxql/constraints_test.exs

@@ -5,8 +5,11 @@ defmodule Ecto.Integration.ConstraintsTest do
   alias Ecto.Integration.PoolRepo
 
   defmodule CustomConstraintHandler do
+    @behaviour Ecto.Adapters.SQL.Constraint
+
     @quotes ~w(" ' `)
 
+    @impl Ecto.Adapters.SQL.Constraint
     # An example of a custom handler a user might write
     def to_constraints(%MyXQL.Error{mysql: %{name: :ER_SIGNAL_EXCEPTION}, message: message}, opts) do
       # Assumes this is the only use-case of `ER_SIGNAL_EXCEPTION` the user has implemented custom errors for
@@ -41,7 +44,7 @@ defmodule Ecto.Integration.ConstraintsTest do
     end
   end
 
-  defmodule ConstraintMigration do
+  defmodule CheckConstraintMigration do
     use Ecto.Migration
 
     @table table(:constraints_test)
@@ -52,7 +55,7 @@ defmodule Ecto.Integration.ConstraintsTest do
     end
   end
 
-  defmodule ProcedureEmulatingConstraintMigration do
+  defmodule TriggerEmulatingConstraintMigration do
     use Ecto.Migration
 
     @table_name :constraints_test
@@ -70,11 +73,14 @@ defmodule Ecto.Integration.ConstraintsTest do
       drop_triggers(@table_name)
     end
 
+    # FOR EACH ROW, not a great example performance-wise,
+    # but demonstrates the feature
     defp trigger_sql(table_name, before_type) do
       ~s"""
       CREATE TRIGGER #{table_name}_#{String.downcase(before_type)}_overlap
         BEFORE #{String.upcase(before_type)}
-        ON #{table_name} FOR EACH ROW
+        ON #{table_name}
+        FOR EACH ROW
       BEGIN
         DECLARE v_rowcount INT;
         DECLARE v_msg VARCHAR(200);
@@ -112,7 +118,6 @@ defmodule Ecto.Integration.ConstraintsTest do
     ExUnit.CaptureLog.capture_log(fn ->
       num = @base_migration + System.unique_integer([:positive])
       up(PoolRepo, num, ConstraintTableMigration, log: false)
-      up(PoolRepo, num + 1, ProcedureEmulatingConstraintMigration, log: false)
     end)
 
     :ok
@@ -121,8 +126,9 @@ defmodule Ecto.Integration.ConstraintsTest do
   @tag :create_constraint
   test "check constraint" do
     num = @base_migration + System.unique_integer([:positive])
+
     ExUnit.CaptureLog.capture_log(fn ->
-      :ok = up(PoolRepo, num, ConstraintMigration, log: false)
+      :ok = up(PoolRepo, num, CheckConstraintMigration, log: false)
     end)
 
     # When the changeset doesn't expect the db error
@@ -131,9 +137,7 @@ defmodule Ecto.Integration.ConstraintsTest do
     exception =
       assert_raise Ecto.ConstraintError,
                    ~r/constraint error when attempting to insert struct/,
-                   fn ->
-                     PoolRepo.insert(changeset)
-                   end
+                   fn -> PoolRepo.insert(changeset) end
 
     assert exception.message =~ "\"positive_price\" (check_constraint)"
     assert exception.message =~ "The changeset has not defined any constraint."
@@ -184,7 +188,14 @@ defmodule Ecto.Integration.ConstraintsTest do
     assert is_integer(result.id)
   end
 
+  @tag :constraint_handler
   test "custom handled constraint" do
+    num = @base_migration + System.unique_integer([:positive])
+
+    ExUnit.CaptureLog.capture_log(fn ->
+      :ok = up(PoolRepo, num, TriggerEmulatingConstraintMigration, log: false)
+    end)
+
     changeset = Ecto.Changeset.change(%Constraint{}, from: 0, to: 10)
     {:ok, item} = PoolRepo.insert(changeset)
 
@@ -211,42 +222,67 @@ defmodule Ecto.Integration.ConstraintsTest do
         |> Ecto.Changeset.exclusion_constraint(:from)
         |> PoolRepo.insert()
       end
+
     assert exception.message =~ "\"cannot_overlap\" (exclusion_constraint)"
 
     # When the changeset does expect the db error, but doesn't give a custom message
     {:error, changeset} =
       overlapping_changeset
       |> Ecto.Changeset.exclusion_constraint(:from, name: :cannot_overlap)
       |> PoolRepo.insert()
-    assert changeset.errors == [from: {"violates an exclusion constraint", [constraint: :exclusion, constraint_name: "cannot_overlap"]}]
+
+    assert changeset.errors == [
+             from:
+               {"violates an exclusion constraint",
+                [constraint: :exclusion, constraint_name: "cannot_overlap"]}
+           ]
+
     assert changeset.data.__meta__.state == :built
 
     # When the changeset does expect the db error and gives a custom message
     {:error, changeset} =
       overlapping_changeset
-      |> Ecto.Changeset.exclusion_constraint(:from, name: :cannot_overlap, message: "must not overlap")
+      |> Ecto.Changeset.exclusion_constraint(:from,
+        name: :cannot_overlap,
+        message: "must not overlap"
+      )
       |> PoolRepo.insert()
-    assert changeset.errors == [from: {"must not overlap", [constraint: :exclusion, constraint_name: "cannot_overlap"]}]
-    assert changeset.data.__meta__.state == :built
 
+    assert changeset.errors == [
+             from:
+               {"must not overlap", [constraint: :exclusion, constraint_name: "cannot_overlap"]}
+           ]
+
+    assert changeset.data.__meta__.state == :built
 
     # When the changeset does expect the db error, but a different handler is used
     exception =
       assert_raise MyXQL.Error, fn ->
         overlapping_changeset
         |> Ecto.Changeset.exclusion_constraint(:from, name: :cannot_overlap)
-        |> PoolRepo.insert(constraint_handler: Ecto.Adapters.MyXQL.Connection)
+        |> PoolRepo.insert(
+          constraint_handler: {Ecto.Adapters.MyXQL.Connection, :to_constraints, []}
+        )
       end
+
     assert exception.message =~ "Overlapping values for key 'constraints_test.cannot_overlap'"
 
     # When custom error is coming from an UPDATE
     overlapping_update_changeset = Ecto.Changeset.change(item, from: 0, to: 9)
 
     {:error, changeset} =
       overlapping_update_changeset
-      |> Ecto.Changeset.exclusion_constraint(:from, name: :cannot_overlap, message: "must not overlap")
+      |> Ecto.Changeset.exclusion_constraint(:from,
+        name: :cannot_overlap,
+        message: "must not overlap"
+      )
       |> PoolRepo.insert()
-    assert changeset.errors == [from: {"must not overlap", [constraint: :exclusion, constraint_name: "cannot_overlap"]}]
+
+    assert changeset.errors == [
+             from:
+               {"must not overlap", [constraint: :exclusion, constraint_name: "cannot_overlap"]}
+           ]
+
     assert changeset.data.__meta__.state == :loaded
   end
 end

integration_test/myxql/test_helper.exs

@@ -59,7 +59,7 @@ Application.put_env(:ecto_sql, PoolRepo,
   pool_count: String.to_integer(System.get_env("POOL_COUNT", "1")),
   show_sensitive_data_on_connection_error: true,
   # Passes through into adapter_meta
-  constraint_handler: Ecto.Integration.ConstraintsTest.CustomConstraintHandler
+  constraint_handler: {Ecto.Integration.ConstraintsTest.CustomConstraintHandler, :to_constraints, []}
 )
 
 defmodule Ecto.Integration.PoolRepo do
@@ -88,7 +88,7 @@ _ = Ecto.Adapters.MyXQL.storage_down(TestRepo.config())
 {:ok, _pid} = TestRepo.start_link()
 
 # Passes through into adapter_meta, overrides Application config
-# {:ok, _pid} = PoolRepo.start_link([constraint_handler: Ecto.Integration.ConstraintsTest.CustomConstraintHandler])
+# {:ok, _pid} = PoolRepo.start_link([constraint_handler: {Ecto.Integration.ConstraintsTest.CustomConstraintHandler, :to_constraints, []}])
 {:ok, _pid} = PoolRepo.start_link()
 
 %{rows: [[version]]} = TestRepo.query!("SELECT @@version", [])

lib/ecto/adapters/myxql.ex

@@ -341,7 +341,7 @@ defmodule Ecto.Adapters.MyXQL do
         {:ok, last_insert_id(key, last_insert_id)}
 
       {:error, err} ->
-        case Ecto.Adapters.SQL.to_constraints(adapter_meta, opts, err, source: source) do
+        case Ecto.Adapters.SQL.to_constraints(adapter_meta, err, opts, source: source) do
           [] -> raise err
           constraints -> {:invalid, constraints}
         end

lib/ecto/adapters/myxql/connection.ex

@@ -4,6 +4,7 @@ if Code.ensure_loaded?(MyXQL) do
     alias Ecto.Adapters.SQL
 
     @behaviour Ecto.Adapters.SQL.Connection
+    @behaviour Ecto.Adapters.SQL.Constraint
 
     ## Connection
 

lib/ecto/adapters/postgres/connection.ex

@@ -4,6 +4,7 @@ if Code.ensure_loaded?(Postgrex) do
 
     @default_port 5432
     @behaviour Ecto.Adapters.SQL.Connection
+    @behaviour Ecto.Adapters.SQL.Constraint
     @explain_prepared_statement_name "ecto_explain_statement"
 
     ## Module and Options

lib/ecto/adapters/sql.ex

@@ -37,6 +37,9 @@ defmodule Ecto.Adapters.SQL do
     * `to_sql(type, query)` -
        shortcut for `Ecto.Adapters.SQL.to_sql/3`
 
+    * `to_constraints(exception, opts, error_opts)` -
+       shortcut for `Ecto.Adapters.SQL.to_constraints/4`
+
   Generally speaking, you must invoke those functions directly from
   your repository, for example: `MyApp.Repo.query("SELECT true")`.
   You can also invoke them directly from `Ecto.Adapters.SQL`, but
@@ -393,6 +396,45 @@ defmodule Ecto.Adapters.SQL do
       {"SELECT p.id, p.title, p.inserted_at, p.created_at FROM posts as p", []}
   """
 
+  # TODO - add docs here and/or somewhere for how to pass `constraint_handler` per call
+  @to_constraints_doc """
+  Handles adapter-specific exceptions, converting them to
+  the corresponding contraint errors.
+
+  The constraints are in the keyword list and must return the
+  constraint type, like `:unique`, and the constraint name as
+  a string, for example:
+
+      [unique: "posts_title_index"]
+
+  Returning an empty list signifies the error does not come
+  from any constraint, and should continue with the default
+  exception handling path (i.e. raise or further handling).
+
+  ## Options
+    * `:constraint_handler` - A module, function, and list of arguments (`mfargs`)
+
+  The `:constraint_handler` option defaults to the adapter's connection module.
+  For the built-in adapters this would be:
+
+    * `Ecto.Adapters.Postgres.Connection.to_constraints/2`
+    * `Ecto.Adapters.MyXQL.Connection.to_constraints/2`
+    * `Ecto.Adapters.Tds.Connection.to_constraints/2`
+
+  See `Ecto.Adapters.SQL.Constraint` if you need to fully
+  customize the handling of constraints for all operations.
+
+  ## Examples
+
+      # Postgres
+      iex> MyRepo.to_constraints(%Postgrex.Error{code: :unique, constraint: "posts_title_index"}, [])
+      [unique: "posts_title_index"]
+
+      # MySQL
+      iex> MyRepo.to_constraints(%MyXQL.Error{mysql: %{name: :ER_CHECK_CONSTRAINT_VIOLATED}, message: "Check constraint 'positive_price' is violated."}, [])
+      [check: "positive_price"]
+  """
+
   @explain_doc """
   Executes an EXPLAIN statement or similar for the given query according to its kind and the
   adapter in the given repository.
@@ -668,11 +710,22 @@ defmodule Ecto.Adapters.SQL do
     sql_call(adapter_meta, :query_many, [sql], params, opts)
   end
 
-  @doc false
-  def to_constraints(adapter_meta, opts, err, err_opts) do
+  @doc @to_constraints_doc
+  @spec to_constraints(
+          pid() | Ecto.Repo.t() | Ecto.Adapter.adapter_meta(),
+          exception :: Exception.t(),
+          options :: Keyword.t(),
+          error_options :: Keyword.t()
+        ) :: Keyword.t()
+  def to_constraints(repo, err, opts, err_opts) when is_atom(repo) or is_pid(repo) do
+    to_constraints(Ecto.Adapter.lookup_meta(repo), err, opts, err_opts)
+  end
+
+  def to_constraints(adapter_meta, err, opts, err_opts) do
     %{constraint_handler: constraint_handler} = adapter_meta
-    constraint_handler = Keyword.get(opts, :constraint_handler) || constraint_handler
-    constraint_handler.to_constraints(err, err_opts)
+    {constraint_mod, fun, args} = Keyword.get(opts, :constraint_handler) || constraint_handler
+    args = [err, err_opts | args]
+    apply(constraint_mod, fun, args)
   end
 
   defp sql_call(adapter_meta, callback, args, params, opts) do
@@ -794,6 +847,7 @@ defmodule Ecto.Adapters.SQL do
     query_many_doc = @query_many_doc
     query_many_bang_doc = @query_many_bang_doc
     to_sql_doc = @to_sql_doc
+    to_constraints_doc = @to_constraints_doc
     explain_doc = @explain_doc
     disconnect_all_doc = @disconnect_all_doc
 
@@ -833,6 +887,16 @@ defmodule Ecto.Adapters.SQL do
         Ecto.Adapters.SQL.to_sql(operation, get_dynamic_repo(), queryable)
       end
 
+      @doc unquote(to_constraints_doc)
+      @spec to_constraints(
+              exception :: Exception.t(),
+              options :: Keyword.t(),
+              error_options :: Keyword.t()
+            ) :: Keyword.t()
+      def to_constraints(err, opts, err_opts) do
+        Ecto.Adapters.SQL.to_constraints(get_dynamic_repo(), err, opts, err_opts)
+      end
+
       @doc unquote(explain_doc)
       @spec explain(:all | :update_all | :delete_all, Ecto.Queryable.t(), opts :: Keyword.t()) ::
               String.t() | Exception.t() | list(map)
@@ -890,7 +954,9 @@ defmodule Ecto.Adapters.SQL do
       """
     end
 
-    constraint_handler = Keyword.get(config, :constraint_handler, connection)
+    constraint_handler =
+      Keyword.get(config, :constraint_handler, {connection, :to_constraints, []})
+
     stacktrace = Keyword.get(config, :stacktrace)
     telemetry_prefix = Keyword.fetch!(config, :telemetry_prefix)
     telemetry = {config[:repo], log, telemetry_prefix ++ [:query]}
@@ -1200,7 +1266,7 @@ defmodule Ecto.Adapters.SQL do
           operation: operation
 
       {:error, err} ->
-        case to_constraints(adapter_meta, opts, err, source: source) do
+        case to_constraints(adapter_meta, err, opts, source: source) do
           [] -> raise_sql_call_error(err)
           constraints -> {:invalid, constraints}
         end

lib/ecto/adapters/sql/connection.ex

@@ -52,20 +52,6 @@ defmodule Ecto.Adapters.SQL.Connection do
   @callback stream(connection, statement, params, options :: Keyword.t()) ::
               Enum.t()
 
-  @doc """
-  Receives the exception returned by `c:query/4`.
-
-  The constraints are in the keyword list and must return the
-  constraint type, like `:unique`, and the constraint name as
-  a string, for example:
-
-      [unique: "posts_title_index"]
-
-  Must return an empty list if the error does not come
-  from any constraint.
-  """
-  @callback to_constraints(exception :: Exception.t(), options :: Keyword.t()) :: Keyword.t()
-
   ## Queries
 
   @doc """

lib/ecto/adapters/sql/constraint.ex

@@ -1,4 +1,6 @@
 defmodule Ecto.Adapters.SQL.Constraint do
+  # TODO - add more docs around setting `:constraint_handler` globally
+
   @moduledoc """
   Specifies the constraint handling API
   """

lib/ecto/adapters/tds/connection.ex

@@ -8,6 +8,7 @@ if Code.ensure_loaded?(Tds) do
     require Ecto.Schema
 
     @behaviour Ecto.Adapters.SQL.Connection
+    @behaviour Ecto.Adapters.SQL.Constraint
 
     @impl true
     def child_spec(opts) do