Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add :constraint_handler and Ecto.Adapters.SQL.Constraint #627

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 180 additions & 11 deletions integration_test/myxql/constraints_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,30 @@ defmodule Ecto.Integration.ConstraintsTest do
import Ecto.Migrator, only: [up: 4]
alias Ecto.Integration.PoolRepo

defmodule ConstraintMigration do
defmodule CustomConstraintHandler do
@quotes ~w(" ' `)

# 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
with [_, quoted] <- :binary.split(message, "Overlapping values for key "),
[_, index | _] <- :binary.split(quoted, @quotes, [:global]) do
[exclusion: strip_source(index, opts[:source])]
else
_ -> []
end
end

def to_constraints(err, opts) do
# Falls back to default `ecto_sql` handler for all others
Ecto.Adapters.MyXQL.Connection.to_constraints(err, opts)
end

defp strip_source(name, nil), do: name
defp strip_source(name, source), do: String.trim_leading(name, "#{source}.")
end

defmodule ConstraintTableMigration do
use Ecto.Migration

@table table(:constraints_test)
Expand All @@ -15,12 +38,64 @@ defmodule Ecto.Integration.ConstraintsTest do
add :from, :integer
add :to, :integer
end
end
end

defmodule ConstraintMigration do
use Ecto.Migration

@table table(:constraints_test)

def change do
# Only valid after MySQL 8.0.19
create constraint(@table.name, :positive_price, check: "price > 0")
end
end

defmodule ProcedureEmulatingConstraintMigration do
use Ecto.Migration

@table_name :constraints_test

def up do
insert_trigger_sql = trigger_sql(@table_name, "INSERT")
update_trigger_sql = trigger_sql(@table_name, "UPDATE")

drop_triggers(@table_name)
repo().query!(insert_trigger_sql)
repo().query!(update_trigger_sql)
end

def down do
drop_triggers(@table_name)
end

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
BEGIN
DECLARE v_rowcount INT;
DECLARE v_msg VARCHAR(200);

SELECT COUNT(*) INTO v_rowcount FROM #{table_name}
WHERE (NEW.from <= `to` AND NEW.to >= `from`);

IF v_rowcount > 0 THEN
SET v_msg = CONCAT('Overlapping values for key \\'#{table_name}.cannot_overlap\\'');
SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = v_msg, MYSQL_ERRNO = 1644;
END IF;
END;
"""
end

defp drop_triggers(table_name) do
repo().query!("DROP TRIGGER IF EXISTS #{table_name}_insert_overlap")
repo().query!("DROP TRIGGER IF EXISTS #{table_name}_update_overlap")
end
end

defmodule Constraint do
use Ecto.Integration.Schema

Expand All @@ -36,20 +111,29 @@ defmodule Ecto.Integration.ConstraintsTest do
setup_all do
ExUnit.CaptureLog.capture_log(fn ->
num = @base_migration + System.unique_integer([:positive])
up(PoolRepo, num, ConstraintMigration, log: false)
up(PoolRepo, num, ConstraintTableMigration, log: false)
up(PoolRepo, num + 1, ProcedureEmulatingConstraintMigration, log: false)
end)

:ok
end

@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)
end)
Comment on lines +123 to +126
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than moving this migration here, the other changes to this test are from mix format


# When the changeset doesn't expect the db error
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)

exception =
assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
PoolRepo.insert(changeset)
end
assert_raise Ecto.ConstraintError,
~r/constraint error when attempting to insert struct/,
fn ->
PoolRepo.insert(changeset)
end

assert exception.message =~ "\"positive_price\" (check_constraint)"
assert exception.message =~ "The changeset has not defined any constraint."
Expand All @@ -60,24 +144,109 @@ defmodule Ecto.Integration.ConstraintsTest do
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price)
|> PoolRepo.insert()
assert changeset.errors == [price: {"is invalid", [constraint: :check, constraint_name: "positive_price"]}]

assert changeset.errors == [
price: {"is invalid", [constraint: :check, constraint_name: "positive_price"]}
]

assert changeset.data.__meta__.state == :built

# When the changeset does expect the db error and gives a custom message
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)

{:error, changeset} =
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
|> Ecto.Changeset.check_constraint(:price,
name: :positive_price,
message: "price must be greater than 0"
)
|> PoolRepo.insert()
assert changeset.errors == [price: {"price must be greater than 0", [constraint: :check, constraint_name: "positive_price"]}]

assert changeset.errors == [
price:
{"price must be greater than 0",
[constraint: :check, constraint_name: "positive_price"]}
]

assert changeset.data.__meta__.state == :built

# When the change does not violate the check constraint
changeset = Ecto.Changeset.change(%Constraint{}, price: 10, from: 100, to: 200)
{:ok, changeset} =

{:ok, result} =
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
|> Ecto.Changeset.check_constraint(:price,
name: :positive_price,
message: "price must be greater than 0"
)
|> PoolRepo.insert()

assert is_integer(result.id)
end

test "custom handled constraint" do
changeset = Ecto.Changeset.change(%Constraint{}, from: 0, to: 10)
{:ok, item} = PoolRepo.insert(changeset)

non_overlapping_changeset = Ecto.Changeset.change(%Constraint{}, from: 11, to: 12)
{:ok, _} = PoolRepo.insert(non_overlapping_changeset)

overlapping_changeset = Ecto.Changeset.change(%Constraint{}, from: 9, to: 12)

msg_re = ~r/constraint error when attempting to insert struct/

# When the changeset doesn't expect the db error
exception =
assert_raise Ecto.ConstraintError, msg_re, fn -> PoolRepo.insert(overlapping_changeset) end

assert exception.message =~ "\"cannot_overlap\" (exclusion_constraint)"
assert exception.message =~ "The changeset has not defined any constraint."
assert exception.message =~ "call `exclusion_constraint/3`"

# When the changeset does expect the db error
# but the key does not match the default generated by `exclusion_constraint`
exception =
assert_raise Ecto.ConstraintError, msg_re, fn ->
overlapping_changeset
|> 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.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")
|> PoolRepo.insert()
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)
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")
|> PoolRepo.insert()
assert is_integer(changeset.id)
assert changeset.errors == [from: {"must not overlap", [constraint: :exclusion, constraint_name: "cannot_overlap"]}]
assert changeset.data.__meta__.state == :loaded
end
end
7 changes: 6 additions & 1 deletion integration_test/myxql/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ Application.put_env(:ecto_sql, PoolRepo,
url: Application.get_env(:ecto_sql, :mysql_test_url) <> "/ecto_test",
pool_size: 5,
pool_count: String.to_integer(System.get_env("POOL_COUNT", "1")),
show_sensitive_data_on_connection_error: true
show_sensitive_data_on_connection_error: true,
# Passes through into adapter_meta
constraint_handler: Ecto.Integration.ConstraintsTest.CustomConstraintHandler
)

defmodule Ecto.Integration.PoolRepo do
Expand All @@ -84,6 +86,9 @@ _ = Ecto.Adapters.MyXQL.storage_down(TestRepo.config())
:ok = Ecto.Adapters.MyXQL.storage_up(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()

%{rows: [[version]]} = TestRepo.query!("SELECT @@version", [])
Expand Down
2 changes: 1 addition & 1 deletion lib/ecto/adapters/myxql.ex
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ defmodule Ecto.Adapters.MyXQL do
{:ok, last_insert_id(key, last_insert_id)}

{:error, err} ->
case @conn.to_constraints(err, source: source) do
case Ecto.Adapters.SQL.to_constraints(adapter_meta, opts, err, source: source) do
[] -> raise err
constraints -> {:invalid, constraints}
end
Expand Down
13 changes: 11 additions & 2 deletions lib/ecto/adapters/sql.ex
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,13 @@ 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
%{constraint_handler: constraint_handler} = adapter_meta
constraint_handler = Keyword.get(opts, :constraint_handler) || constraint_handler
constraint_handler.to_constraints(err, err_opts)
end

defp sql_call(adapter_meta, callback, args, params, opts) do
%{pid: pool, telemetry: telemetry, sql: sql, opts: default_opts} = adapter_meta
conn = get_conn_or_pool(pool, adapter_meta)
Expand Down Expand Up @@ -872,6 +879,7 @@ defmodule Ecto.Adapters.SQL do
"""
end

constraint_handler = Keyword.get(config, :constraint_handler, connection)
stacktrace = Keyword.get(config, :stacktrace, nil)
telemetry_prefix = Keyword.fetch!(config, :telemetry_prefix)
telemetry = {config[:repo], log, telemetry_prefix ++ [:query]}
Expand All @@ -884,6 +892,7 @@ defmodule Ecto.Adapters.SQL do
meta = %{
telemetry: telemetry,
sql: connection,
constraint_handler: constraint_handler,
stacktrace: stacktrace,
opts: Keyword.take(config, @pool_opts)
}
Expand Down Expand Up @@ -1130,7 +1139,7 @@ defmodule Ecto.Adapters.SQL do
@doc false
def struct(
adapter_meta,
conn,
_conn,
sql,
operation,
source,
Expand Down Expand Up @@ -1165,7 +1174,7 @@ defmodule Ecto.Adapters.SQL do
operation: operation

{:error, err} ->
case conn.to_constraints(err, source: source) do
case to_constraints(adapter_meta, opts, err, source: source) do
[] -> raise_sql_call_error(err)
constraints -> {:invalid, constraints}
end
Expand Down
19 changes: 19 additions & 0 deletions lib/ecto/adapters/sql/constraint.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
defmodule Ecto.Adapters.SQL.Constraint do
@moduledoc """
Specifies the constraint handling API
"""

@doc """
Receives the exception returned by `c:Ecto.Adapters.SQL.Connection.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()
end
Loading