Deprecate validate_unique/3 in favor of unique_constraint/3

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## v0.16.0-dev
+
+* Deprecations
+  * `Ecto.Changeset.validate_unique/3` is deprecate in favor of `Ecto.Changeset.unique_constraint/3`. Read the documentation for the latter for more information on updating
+
+* Backwards incompatible changes
+  * `Ecto.Adapters.SQL.query/4` now returns `{:ok, result}`. Use `Ecto.Adapters.SQL.query!/4` for the previous behaviour
+
 ## v0.15.0
 
 * Enhancements

integration_test/cases/repo.exs

@@ -1,3 +1,5 @@
+Code.require_file "../support/types.exs", __DIR__
+
 defmodule Ecto.Integration.RepoTest do
   use Ecto.Integration.Case
 
@@ -189,58 +191,34 @@ defmodule Ecto.Integration.RepoTest do
     assert_raise Ecto.StaleModelError, fn -> TestRepo.delete!(base_post) end
   end
 
-  test "validate_unique/3" do
-    import Ecto.Changeset
-    post = TestRepo.insert!(%Post{title: "HELLO"})
-
-    on_insert = cast(%Post{}, %{"title" => "HELLO"}, ~w(title), ~w())
-    assert validate_unique(on_insert, :title, on: TestRepo).errors != []
-
-    on_update = cast(post, %{"title" => "HELLO"}, ~w(title), ~w())
-    assert validate_unique(on_update, :title, on: TestRepo).errors == []
-
-    on_update = cast(post, %{"title" => nil}, ~w(), ~w(title))
-    assert validate_unique(on_update, :title, on: TestRepo).errors == []
-
-    on_update = cast(%{post | id: nil, title: nil}, %{"title" => "HELLO"}, ~w(title), ~w())
-    assert validate_unique(on_update, :title, on: TestRepo).errors != []
-  end
-
-  @tag :sql_fragments
-  test "validate_unique/3 with downcasing" do
-    import Ecto.Changeset
-    TestRepo.insert!(%Post{title: "HELLO"})
+  test "unique constraint" do
+    changeset = Ecto.Changeset.change(%Post{}, uuid: Ecto.UUID.generate())
+    {:ok, _}  = TestRepo.insert(changeset)
 
-    on_insert = cast(%Post{}, %{"title" => "hello"}, ~w(title), ~w())
-    assert validate_unique(on_insert, :title, on: TestRepo, downcase: true).errors != []
-  end
-
-  @tag :case_sensitive
-  test "validate_unique/3 case sensitive" do
-    import Ecto.Changeset
-    post = TestRepo.insert!(%Post{title: "HELLO"})
-
-    on_insert = cast(%Post{}, %{"title" => "hello"}, ~w(title), ~w())
-    assert validate_unique(on_insert, :title, on: TestRepo).errors == []
-
-    on_update = cast(%{post | id: nil}, %{"title" => "hello"}, ~w(title), ~w())
-    assert validate_unique(on_update, :title, on: TestRepo).errors == []
-  end
+    exception =
+      assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert model/, fn ->
+        changeset
+        |> TestRepo.insert()
+      end
 
-  test "validate_unique/3 with scope" do
-    import Ecto.Changeset
-    TestRepo.insert!(%Post{title: "hello", text: "world"})
+    assert exception.message =~ "unique: posts_uuid_index"
+    assert exception.message =~ "The changeset has not defined any constraint."
 
-    on_insert = cast(%Post{}, %{"title" => "hello", "text" => "elixir"}, ~w(title), ~w(text))
-    assert validate_unique(on_insert, :title, on: TestRepo).errors == [title: "has already been taken"]
-    assert validate_unique(on_insert, :title, scope: [:text], on: TestRepo).errors == []
+    message = ~r/constraint error when attempting to insert model/
+    exception =
+      assert_raise Ecto.ConstraintError, message, fn ->
+        changeset
+        |> Ecto.Changeset.unique_constraint(:uuid, name: :posts_email_changeset)
+        |> TestRepo.insert()
+      end
 
-    on_insert = cast(%Post{}, %{"title" => "hello", "text" => nil}, ~w(title), ~w(text))
-    assert validate_unique(on_insert, :title, scope: [:text], on: TestRepo).errors == []
+    assert exception.message =~ "unique: posts_email_changeset"
 
-    assert_raise(Ecto.QueryError, fn ->
-      validate_unique(on_insert, :title, scope: [:non_existent], on: TestRepo).errors == []
-    end)
+    {:error, changeset} =
+      changeset
+      |> Ecto.Changeset.unique_constraint(:uuid)
+      |> TestRepo.insert()
+    assert changeset.errors == [uuid: "has already been taken"]
   end
 
   test "get(!)" do

integration_test/mysql/test_helper.exs

@@ -1,7 +1,6 @@
 Logger.configure(level: :info)
-ExUnit.start exclude: [:array_type, :read_after_writes, :case_sensitive,
-                       :uses_usec, :uses_msec, :strict_savepoint,
-                       :create_index_if_not_exists]
+ExUnit.start exclude: [:array_type, :read_after_writes, :uses_usec, :uses_msec,
+                       :strict_savepoint, :create_index_if_not_exists]
 
 # Configure Ecto for support and tests
 Application.put_env(:ecto, :lock_for_update, "FOR UPDATE")

integration_test/support/migration.exs

@@ -27,7 +27,7 @@ defmodule Ecto.Integration.Migration do
     # Add a unique index on uuid. We use this
     # to verify the behaviour that the index
     # only matters if the UUID column is not NULL.
-    create index(:posts, [:uuid], unique: true)
+    create unique_index(:posts, [:uuid])
 
     create table(:permalinks) do
       add :url

lib/ecto.ex

@@ -151,12 +151,11 @@ defmodule Ecto do
           field :age, :integer
         end
 
-        def changeset(user, params \\ nil) do
+        def changeset(user, params \\ :empty) do
           user
           |> cast(params, ~w(name email), ~w(age))
           |> validate_format(:email, ~r/@/)
-          |> validate_inclusion(:age, 0..130)
-          |> validate_unique(:email, on: Repo)
+          |> validate_inclusion(:age, 18..100)
         end
       end
 

lib/ecto/adapter.ex

@@ -10,6 +10,7 @@ defmodule Ecto.Adapter do
   @type source :: {prefix :: binary, table :: binary, model :: atom}
   @type fields :: Keyword.t
   @type filters :: Keyword.t
+  @type constraints :: Keyword.t
   @type returning :: [atom]
   @type prepared :: term
   @type preprocess :: (Macro.t, term -> term)
@@ -144,7 +145,7 @@ defmodule Ecto.Adapter do
   will be a non `nil` value.
   """
   defcallback insert(repo, source, fields, autogenerate_id, returning, options) ::
-                    {:ok, Keyword.t} | no_return
+                    {:ok, Keyword.t} | {:invalid, constraints} | no_return
 
   @doc """
   Updates a single model with the given filters.
@@ -168,7 +169,8 @@ defmodule Ecto.Adapter do
   will be a non `nil` value.
   """
   defcallback update(repo, source, fields, filters, autogenerate_id, returning, options) ::
-                    {:ok, Keyword.t} | {:error, :stale} | no_return
+                    {:ok, Keyword.t} | {:invalid, constraints} |
+                    {:error, :stale} | no_return
 
   @doc """
   Deletes a sigle model with the given filters.
@@ -188,5 +190,6 @@ defmodule Ecto.Adapter do
   If the value is `nil`, it means there is no autogenerate primary key.
   """
   defcallback delete(repo, source, filters, autogenerate_id, options) ::
-                     {:ok, Keyword.t} | {:error, :stale} | no_return
+                     {:ok, Keyword.t} | {:invalid, constraints} |
+                     {:error, :stale} | no_return
 end

lib/ecto/adapters/mysql.ex

@@ -159,39 +159,24 @@ defmodule Ecto.Adapters.MySQL do
   @doc false
   def insert(_repo, {_prefix, source, _model}, _params, _autogen, [_|_] = returning, _opts) do
     raise ArgumentError, "MySQL does not support :read_after_writes in models. " <>
-                         "The following fields in #{inspect source} have tagged as such: #{inspect returning}"
+                         "The following fields in #{inspect source} are tagged as such: #{inspect returning}"
   end
 
   def insert(repo, {prefix, source, _model}, params, {pk, :id, nil}, [], opts) do
     {fields, values} = :lists.unzip(params)
     sql = @conn.insert(prefix, source, fields, [])
-    case Ecto.Adapters.SQL.query!(repo, sql, values, opts) do
-      %{num_rows: 1, last_insert_id: last_insert_id} ->
+    case Ecto.Adapters.SQL.query(repo, sql, values, opts) do
+      {:ok, %{num_rows: 1, last_insert_id: last_insert_id}} ->
         {:ok, [{pk, last_insert_id}]}
+      {:error, err} ->
+        case @conn.to_constraints(err) do
+          []          -> raise err
+          constraints -> {:invalid, constraints}
+        end
     end
   end
 
   def insert(repo, source, params, autogen, [], opts) do
     super(repo, source, params, autogen, [], opts)
   end
-
-  @doc false
-  def update(repo, {prefix, source, _model}, fields, filter, _autogenerate, returning, opts) do
-    {fields, values1} = :lists.unzip(fields)
-    {filter, values2} = :lists.unzip(filter)
-    sql = @conn.update(prefix, source, fields, filter, returning)
-    case Ecto.Adapters.SQL.query!(repo, sql, values1 ++ values2, opts) do
-      %{num_rows: 0} -> {:error, :stale}
-      %{num_rows: _} -> {:ok, []}
-    end
-  end
-
-  @doc false
-  def delete(repo, {prefix, source, _model}, filter, _autogenerate, opts) do
-    {filter, values} = :lists.unzip(filter)
-    case Ecto.Adapters.SQL.query!(repo, @conn.delete(prefix, source, filter, []), values, opts) do
-      %{num_rows: 0} -> {:error, :stale}
-      %{num_rows: _} -> {:ok, []}
-    end
-  end
 end

lib/ecto/adapters/mysql/connection.ex

@@ -35,6 +35,19 @@ if Code.ensure_loaded?(Mariaex.Connection) do
       Application.get_env(:ecto, :json_library)
     end
 
+    def to_constraints(%Mariaex.Error{mariadb: %{code: 1062, message: message}}) do
+      case :binary.split(message, " for key ") do
+        [_, quoted_index] ->
+          size = byte_size(quoted_index) - 2
+          <<?', index::binary-size(size), ?'>> = quoted_index
+          [unique: index]
+        _ ->
+          []
+      end
+    end
+    def to_constraints(%Mariaex.Error{} = error),
+      do: error
+
     ## Transaction
 
     def begin_transaction do

lib/ecto/adapters/postgres/connection.ex

@@ -37,6 +37,11 @@ if Code.ensure_loaded?(Postgrex.Connection) do
     defp normalize_port(port) when is_binary(port), do: String.to_integer(port)
     defp normalize_port(port) when is_integer(port), do: port
 
+    def to_constraints(%Postgrex.Error{postgres: %{code: :unique_violation, constraint: constraint}}),
+      do: [unique: constraint]
+    def to_constraints(%Postgrex.Error{}),
+      do: []
+
     ## Transaction
 
     def begin_transaction do

lib/ecto/adapters/sql.ex

@@ -74,21 +74,22 @@ defmodule Ecto.Adapters.SQL do
       def insert(repo, {prefix, source, _model}, params, _autogenerate, returning, opts) do
         {fields, values} = :lists.unzip(params)
         sql = @conn.insert(prefix, source, fields, returning)
-        Ecto.Adapters.SQL.model(repo, sql, values, returning, opts)
+        Ecto.Adapters.SQL.model(repo, @conn, sql, values, returning, opts)
       end
 
       @doc false
       def update(repo, {prefix, source, _model}, fields, filter, _autogenerate, returning, opts) do
         {fields, values1} = :lists.unzip(fields)
         {filter, values2} = :lists.unzip(filter)
         sql = @conn.update(prefix, source, fields, filter, returning)
-        Ecto.Adapters.SQL.model(repo, sql, values1 ++ values2, returning, opts)
+        Ecto.Adapters.SQL.model(repo, @conn, sql, values1 ++ values2, returning, opts)
       end
 
       @doc false
       def delete(repo, {prefix, source, _model}, filter, _autogenarate, opts) do
         {filter, values} = :lists.unzip(filter)
-        Ecto.Adapters.SQL.model(repo, @conn.delete(prefix, source, filter, []), values, [], opts)
+        sql = @conn.delete(prefix, source, filter, [])
+        Ecto.Adapters.SQL.model(repo, @conn, sql, values, [], opts)
       end
 
       ## Transaction
@@ -155,6 +156,22 @@ defmodule Ecto.Adapters.SQL do
     {prepared, params}
   end
 
+  @doc """
+  Same as `query/4` but raises on invalid queries.
+  """
+  @spec query!(Ecto.Repo.t, String.t, [term], Keyword.t) ::
+               %{rows: nil | [tuple], num_rows: non_neg_integer} | no_return
+  def query!(repo, sql, params, opts \\ []) do
+    query!(repo, sql, params, nil, opts)
+  end
+
+  defp query!(repo, sql, params, mapper, opts) do
+    case query(repo, sql, params, mapper, opts) do
+      {:ok, result} -> result
+      {:error, err} -> raise err
+    end
+  end
+
   @doc """
   Runs custom SQL query on given repo.
 
@@ -177,24 +194,21 @@ defmodule Ecto.Adapters.SQL do
 
   ## Examples
 
-      iex> Ecto.Adapters.SQL.query!(MyRepo, "SELECT $1::integer + $2", [40, 2])
-      %{rows: [{42}], num_rows: 1}
+      iex> Ecto.Adapters.SQL.query(MyRepo, "SELECT $1::integer + $2", [40, 2])
+      {:ok, %{rows: [{42}], num_rows: 1}}
 
   """
-  @spec query!(Ecto.Repo.t, String.t, [term], Keyword.t) ::
-               %{rows: nil | [tuple], num_rows: non_neg_integer} | no_return
-  def query!(repo, sql, params, opts \\ []) do
-    query!(repo, sql, params, nil, opts)
+  @spec query(Ecto.Repo.t, String.t, [term], Keyword.t) ::
+              {:ok, %{rows: nil | [tuple], num_rows: non_neg_integer}} | {:error, Exception.t}
+  def query(repo, sql, params, opts \\ []) do
+    query(repo, sql, params, nil, opts)
   end
 
-  defp query!(repo, sql, params, mapper, opts) do
+  defp query(repo, sql, params, mapper, opts) do
     case query(repo, sql, params, nil, mapper, opts) do
-      {{:ok, result}, entry} ->
+      {result, entry} ->
         log(repo, entry)
         result
-      {{:error, err}, entry} ->
-        log(repo, entry)
-        raise err
       :noconnect ->
         # :noconnect can never be the reason a call fails because
         # it is converted to {:nodedown, node}. This means the exit
@@ -452,14 +466,19 @@ defmodule Ecto.Adapters.SQL do
   end
 
   @doc false
-  def model(repo, sql, values, returning, opts) do
-    case query!(repo, sql, values, opts) do
-      %{rows: nil, num_rows: 1} ->
+  def model(repo, conn, sql, values, returning, opts) do
+    case query(repo, sql, values, nil, opts) do
+      {:ok, %{rows: nil, num_rows: 1}} ->
         {:ok, []}
-      %{rows: [values], num_rows: 1} ->
+      {:ok, %{rows: [values], num_rows: 1}} ->
         {:ok, Enum.zip(returning, values)}
-      %{num_rows: 0} ->
+      {:ok, %{num_rows: 0}} ->
         {:error, :stale}
+      {:error, err} ->
+        case conn.to_constraints(err) do
+          []          -> raise err
+          constraints -> {:invalid, constraints}
+        end
     end
   end
 

lib/ecto/adapters/sql/query.ex

@@ -24,6 +24,20 @@ defmodule Ecto.Adapters.SQL.Query do
   """
   defcallback query(pid, query :: binary, params :: list(), opts :: Keyword.t) :: result
 
+  @doc """
+  Receives the exception returned by `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.
+  """
+  defcallback to_constraints(Exception.t) :: Keyword.t
+
   ## Queries
 
   @doc """