validate_unique_tentatively -> unsafe_validate_unique

integration_test/cases/repo.exs

@@ -550,35 +550,20 @@ defmodule Ecto.Integration.RepoTest do
     assert %Ecto.Changeset{} = changeset.changes.item
   end
 
-  test "validate_unique_tentatively/3" do
+  test "unsafe_validate_unique/3" do
     {:ok, inserted_post} = TestRepo.insert(%Post{title: "Greetings", text: "hi"})
+    new_post_changeset = Post.changeset(%Post{}, %{title: "Greetings", text: "ho"})
 
-    new_post_changeset =
-      %Post{}
-      |> Post.changeset(%{title: "Greetings", text: "ho"})
-
-    assert Ecto.Changeset.validate_unique_tentatively(
-      new_post_changeset,
-      [:title],
-      TestRepo
-    ).errors[:title] ==
-      {"has already been taken", [validation: [:validate_unique_tentatively, [:title]]]}
-
-    assert Ecto.Changeset.validate_unique_tentatively(
-      new_post_changeset,
-      [:title, :text],
-      TestRepo
-    ).errors[:title] == nil
-
-    update_changeset =
-      inserted_post
-      |> Post.changeset(%{text: "ho"})
-
-    assert Ecto.Changeset.validate_unique_tentatively(
-      update_changeset,
-      [:title],
-      TestRepo
-    ).errors[:title] == nil # cannot conflict with itself
+    changeset = Ecto.Changeset.unsafe_validate_unique(new_post_changeset, [:title], TestRepo)
+    assert changeset.errors[:title] ==
+           {"has already been taken", validation: :unsafe_unique, fields: [:title]}
+
+    changeset = Ecto.Changeset.unsafe_validate_unique(new_post_changeset, [:title, :text], TestRepo)
+    assert changeset.errors[:title] == nil
+
+    update_changeset = Post.changeset(inserted_post, %{text: "ho"})
+    changeset = Ecto.Changeset.unsafe_validate_unique(update_changeset, [:title], TestRepo)
+    assert changeset.errors[:title] == nil # cannot conflict with itself
   end
 
   test "get(!)" do

lib/ecto/changeset.ex

@@ -31,15 +31,16 @@ defmodule Ecto.Changeset do
   Ecto changesets provide both validations and constraints which
   are ultimately turned into errors in case something goes wrong.
 
-  The difference between them is that validations (with the exception of
-  `validate_unique_tentatively/3`) can be executed without a need to interact
-  with the database and, therefore, are always executed before attempting to
-  insert or update the entry in the database.
+  The difference between them is that most validations can be
+  executed without a need to interact with the database and, therefore,
+  are always executed before attempting to insert or update the entry
+  in the database. Some validations may happen against the database but
+  they are inherently unsafe. Those validations start with a `unsafe_`
+  prefix, such as `unsafe_validate_unique/3`.
 
-  However, constraints can only be checked in a safe way when
-  performing the operation in the database. As a consequence,
-  validations are always checked before constraints. Constraints
-  won't even be checked in case validations failed.
+  On the other hand, constraints rely on the database and are always safe.
+  As a consequence, validations are always checked before constraints.
+  Constraints won't even be checked in case validations failed.
 
   Let's see an example:
 
@@ -1394,69 +1395,73 @@ defmodule Ecto.Changeset do
   end
 
   @doc """
-  Validates (tentatively) that no existing record with a different primary key
+  Validates that no existing record with a different primary key
   has the same values for these fields.
 
-  This is tentative because a race condition may occur. A unique constraint
-  should also be used to ensure uniqueness.
+  This function exists to provide quick feedback to users of your
+  application. It should not be relied on for any data guarantee as it
+  has race conditions and is inherently unsafe. For example, if this
+  check happens twice in the same time interval (because the user
+  submitted a form twice), both checks may pass and you may end-up with
+  duplicate entries in the database. Therefore, a `unique_constraint/3`
+  should also be used to ensure your data won't get corrupted.
 
-  However, in most cases where conflicting data exists, it will have been
-  inserted prior to the current validation phase. Noticing those conflicts
-  during validation gives the user a chance to correct them at the same time
-  as other validation errors.
+  However, because constraints are only checked if all validations
+  succeed, this function can be used as an early check to provide
+  early feedback to users, since most conflicting data will have been
+  inserted prior to the current validation phase.
 
   ## Examples
-      validate_unique_tentatively(changeset, [:email], repo)
-      validate_unique_tentatively(changeset, [:city_name, :state_name], repo)
-      validate_unique_tentatively(changeset, [:city_name, :state_name], repo, "city must be unique within state")
+
+      unsafe_validate_unique(changeset, [:email], repo)
+      unsafe_validate_unique(changeset, [:city_name, :state_name], repo)
+      unsafe_validate_unique(changeset, [:city_name, :state_name], repo, "city must be unique within state")
 
   """
-  def validate_unique_tentatively(changeset, field_names, repo, error_message \\ "has already been taken") do
-    field_names = List.wrap(field_names)
-    where_clause = Enum.map(field_names, fn (field_name) ->
-      {field_name, get_field(changeset, field_name)}
-    end)
+  def unsafe_validate_unique(changeset, fields, repo, opts \\ []) do
+    fields = List.wrap(fields)
+    %{validations: validations, data: %{__struct__: struct}} = changeset
+    changeset = %{changeset | validations: [{:unsafe_unique, fields} | validations]}
+
+    where_clause = for field <- fields do
+      {field, get_field(changeset, field)}
+    end
 
     # If we don't have values for all fields, we can't query for uniqueness
-    if Enum.any?(where_clause, fn (tuple) -> is_nil(elem(tuple, 1)) end) do
+    if Enum.any?(where_clause, &(&1 |> elem(1) |> is_nil())) do
       changeset
     else
-      dups_query = Ecto.Query.from q in changeset.data.__struct__, where: ^where_clause
-
-      # For updates, don't flag a record as a dup of itself
-      pk_fields_and_vals = pk_fields_and_vals(changeset)
-      incomplete_pk? = Enum.any?(
-        pk_fields_and_vals,
-        fn {_field, val} -> is_nil(val) end
-      )
-      dups_query = if incomplete_pk? do
-        dups_query
-      else
-        Enum.reduce(
-          pk_fields_and_vals,
-          dups_query, fn ({field_name, val}, query) ->
-            Ecto.Query.from q in query, where: field(q, ^field_name) != ^val
+      pk_pairs = pk_fields_and_values(changeset, struct)
+
+      query =
+        struct
+        |> Ecto.Query.where(^where_clause)
+        |> Ecto.Query.select(true)
+        |> Ecto.Query.limit(1)
+
+      query =
+        # It should not conflict with itself for updates
+        if Enum.any?(pk_pairs, &(&1 |> elem(1) |> is_nil())) do
+          query
+        else
+          Enum.reduce(pk_pairs, query, fn {field, value}, acc ->
+            Ecto.Query.where(acc, [q], field(q, ^field) != ^value)
           end)
-      end
+        end
 
-      dups_exist_query = Ecto.Query.from q in dups_query, select: true, limit: 1
-      case repo.one(dups_exist_query) do
-        true -> add_error(
-          changeset,
-          hd(field_names),
-          error_message,
-          [validation: [:validate_unique_tentatively, field_names]]
-        )
-        nil  -> changeset
+      if repo.one(query) do
+        add_error(changeset, hd(fields), message(opts, "has already been taken"),
+                  validation: :unsafe_unique, fields: fields)
+      else
+        changeset
       end
     end
   end
 
-  defp pk_fields_and_vals(%Changeset{} = changeset) do
-    primary_key_field_names = changeset.data.__struct__.__schema__(:primary_key)
-    Enum.map(primary_key_field_names, fn(field_name) ->
-      {field_name, get_field(changeset, field_name)}
-    end)
+  defp pk_fields_and_values(changeset, struct) do
+    for field <- struct.__schema__(:primary_key) do
+      {field, get_field(changeset, field)}
+    end
   end
 
   defp ensure_field_exists!(%Changeset{types: types, data: data}, field) do
@@ -1783,7 +1788,7 @@ defmodule Ecto.Changeset do
 
     case Ecto.Type.cast(:boolean, value) do
       {:ok, true} -> changeset
-      _ -> add_error(changeset, field, message(opts, "must be accepted"), [validation: :acceptance])
+      _ -> add_error(changeset, field, message(opts, "must be accepted"), validation: :acceptance)
     end
   end
   def validate_acceptance(%{params: nil} = changeset, _, _) do

test/ecto/changeset_test.exs

@@ -37,7 +37,7 @@ defmodule Ecto.ChangesetTest do
     use Ecto.Schema
 
     schema "posts" do
-      field :id_part_2, :integer, primary_key: true
+      field :token, :integer, primary_key: true
       field :title, :string, default: ""
       field :body
       field :uuid, :binary_id
@@ -55,7 +55,7 @@ defmodule Ecto.ChangesetTest do
   end
 
   defp changeset(schema \\ %Post{}, params) do
-    cast(schema, params, ~w(id id_part_2 title body upvotes decimal topics virtual))
+    cast(schema, params, ~w(id token title body upvotes decimal topics virtual))
   end
 
   ## cast/4
@@ -1047,56 +1047,39 @@ defmodule Ecto.ChangesetTest do
 
   alias Ecto.TestRepo
 
-  test "validate_unique_tentatively/3" do
+  test "unsafe_validate_unique/3" do
     dup_result = {1, [true]}
     no_dup_result = {0, []}
     base_changeset = changeset(%Post{}, %{"title" => "Hello World", "body" => "hi"})
 
-    Process.put(:test_repo_all_results, dup_result)
     # validate uniqueness of one field
-    changeset = validate_unique_tentatively(base_changeset, :title, TestRepo)
-    assert changeset.errors == [
-      title: {
-        "has already been taken",
-        [validation: [:validate_unique_tentatively, [:title]]],
-      }
-    ]
+    Process.put(:test_repo_all_results, dup_result)
+    changeset = unsafe_validate_unique(base_changeset, :title, TestRepo)
+    assert changeset.errors ==
+           [title: {"has already been taken", validation: :unsafe_unique, fields: [:title]}]
+
     Process.put(:test_repo_all_results, no_dup_result)
-    changeset = validate_unique_tentatively(base_changeset, :title, TestRepo)
+    changeset = unsafe_validate_unique(base_changeset, :title, TestRepo)
     assert changeset.valid?
 
-    Process.put(:test_repo_all_results, dup_result)
     # validate uniqueness of multiple fields
-    changeset = validate_unique_tentatively(
-      base_changeset, [:title, :body], TestRepo
-    )
-    assert changeset.errors == [
-      title: {
-        "has already been taken",
-        [validation: [:validate_unique_tentatively, [:title, :body]]],
-      }
-    ]
+    Process.put(:test_repo_all_results, dup_result)
+    changeset = unsafe_validate_unique(base_changeset, [:title, :body], TestRepo)
+    assert changeset.errors ==
+           [title: {"has already been taken", validation: :unsafe_unique, fields: [:title, :body]}]
+
     Process.put(:test_repo_all_results, no_dup_result)
-    changeset = validate_unique_tentatively(
-      base_changeset, [:title, :body], TestRepo
-    )
+    changeset = unsafe_validate_unique(base_changeset, [:title, :body], TestRepo)
     assert changeset.valid?
 
-    Process.put(:test_repo_all_results, dup_result)
     # custom error message
-    changeset = validate_unique_tentatively(
-      base_changeset, [:title], TestRepo, "is taken"
-    )
-    assert changeset.errors == [
-      title: {
-        "is taken",
-        [validation: [:validate_unique_tentatively, [:title]]],
-      }
-    ]
+    Process.put(:test_repo_all_results, dup_result)
+    changeset = unsafe_validate_unique(base_changeset, [:title], TestRepo, message: "is taken")
+    assert changeset.errors ==
+           [title: {"is taken", validation: :unsafe_unique, fields: [:title]}]
+
     Process.put(:test_repo_all_results, no_dup_result)
-    changeset = validate_unique_tentatively(
-      base_changeset, [:title], TestRepo, "is taken"
-    )
+    changeset = unsafe_validate_unique(base_changeset, [:title], TestRepo, "is taken")
     assert changeset.valid?
   end