Fix unique check for many_to_many

integration_test/cases/repo.exs

@@ -297,8 +297,7 @@ defmodule Ecto.Integration.RepoTest do
     assert changeset.data.__meta__.state == :built
   end
 
-  @tag :unique_constraint
-  test "unique constraint violation error message with join table in single changeset" do
+  test "unique pseudo-constraint violation error message with join table at the repository" do
     post =
       TestRepo.insert!(%Post{title: "some post"})
       |> TestRepo.preload(:unique_users)
@@ -311,13 +310,34 @@ defmodule Ecto.Integration.RepoTest do
       post
       |> Ecto.Changeset.change
       |> Ecto.Changeset.put_assoc(:unique_users, [user, user])
+      |> TestRepo.update
+
+    errors = Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end)
+    assert errors == %{unique_users: [%{}, %{id: ["has already been taken"]}]}
+    refute changeset.valid?
+  end
+
+  @tag :unique_constraint
+  test "unique constraint violation error message with join table in single changeset" do
+    post =
+      TestRepo.insert!(%Post{title: "some post"})
+      |> TestRepo.preload(:constraint_users)
+
+    user =
+      TestRepo.insert!(%User{name: "some user"})
+
+    # Violate the unique composite index
+    {:error, changeset} =
+      post
+      |> Ecto.Changeset.change
+      |> Ecto.Changeset.put_assoc(:constraint_users, [user, user])
       |> Ecto.Changeset.unique_constraint(:user,
           name: :posts_users_composite_pk_post_id_user_id_index,
           message: "has already been assigned")
       |> TestRepo.update
 
     errors = Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end)
-    assert errors == %{unique_users: [%{}, %{user: ["has already been assigned"]}]}
+    assert errors == %{constraint_users: [%{}, %{user: ["has already been assigned"]}]}
 
     refute changeset.valid?
   end
@@ -326,27 +346,27 @@ defmodule Ecto.Integration.RepoTest do
   test "unique constraint violation error message with join table and separate changesets" do
     post =
       TestRepo.insert!(%Post{title: "some post"})
-      |> TestRepo.preload(:unique_users)
+      |> TestRepo.preload(:constraint_users)
 
     user = TestRepo.insert!(%User{name: "some user"})
 
     post
     |> Ecto.Changeset.change
-    |> Ecto.Changeset.put_assoc(:unique_users, [user])
+    |> Ecto.Changeset.put_assoc(:constraint_users, [user])
     |> TestRepo.update
 
     # Violate the unique composite index
     {:error, changeset} =
       post
       |> Ecto.Changeset.change
-      |> Ecto.Changeset.put_assoc(:unique_users, [user])
+      |> Ecto.Changeset.put_assoc(:constraint_users, [user])
       |> Ecto.Changeset.unique_constraint(:user,
           name: :posts_users_composite_pk_post_id_user_id_index,
           message: "has already been assigned")
       |> TestRepo.update
 
     errors = Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end)
-    assert errors == %{unique_users: [%{user: ["has already been assigned"]}]}
+    assert errors == %{constraint_users: [%{user: ["has already been assigned"]}]}
 
     refute changeset.valid?
   end

integration_test/support/schemas.exs

@@ -48,6 +48,8 @@ defmodule Ecto.Integration.Post do
     many_to_many :users, Ecto.Integration.User,
       join_through: "posts_users", on_delete: :delete_all, on_replace: :delete
     many_to_many :unique_users, Ecto.Integration.User,
+      join_through: "posts_users", unique: true
+    many_to_many :constraint_users, Ecto.Integration.User,
       join_through: Ecto.Integration.PostUserCompositePk
     has_many :users_comments, through: [:users, :comments]
     has_many :comments_authors_permalinks, through: [:comments_authors, :permalink]

lib/ecto/association.ex

@@ -30,7 +30,8 @@ defmodule Ecto.Association do
                relationship: :parent | :child,
                owner: atom,
                owner_key: atom,
-               field: atom}
+               field: atom,
+               unique: boolean}
 
   alias Ecto.Query.{BooleanExpr, JoinExpr, QueryExpr}
 
@@ -448,7 +449,7 @@ defmodule Ecto.Association.Has do
   @on_replace_opts [:raise, :mark_as_invalid, :delete, :nilify]
   @has_one_on_replace_opts @on_replace_opts ++ [:update]
   defstruct [:cardinality, :field, :owner, :related, :owner_key, :related_key, :on_cast,
-             :queryable, :on_delete, :on_replace, defaults: [], relationship: :child]
+             :queryable, :on_delete, :on_replace, unique: true, defaults: [], relationship: :child]
 
   @doc false
   def struct(module, name, opts) do
@@ -635,7 +636,7 @@ defmodule Ecto.Association.HasThrough do
 
   @behaviour Ecto.Association
   defstruct [:cardinality, :field, :owner, :owner_key, :through, :on_cast,
-             relationship: :child]
+             relationship: :child, unique: true]
 
   @doc false
   def struct(module, name, opts) do
@@ -715,7 +716,7 @@ defmodule Ecto.Association.BelongsTo do
   @behaviour Ecto.Association
   @on_replace_opts [:raise, :mark_as_invalid, :delete, :nilify, :update]
   defstruct [:field, :owner, :related, :owner_key, :related_key, :queryable, :on_cast,
-             :on_replace, defaults: [], cardinality: :one, relationship: :parent]
+             :on_replace, defaults: [], cardinality: :one, relationship: :parent, unique: true]
 
   @doc false
   def struct(module, name, opts) do
@@ -837,7 +838,7 @@ defmodule Ecto.Association.ManyToMany do
   @on_replace_opts [:raise, :mark_as_invalid, :delete]
   defstruct [:field, :owner, :related, :owner_key, :queryable, :on_delete,
              :on_replace, :join_keys, :join_through, :on_cast,
-             defaults: [], relationship: :child, cardinality: :many]
+             defaults: [], relationship: :child, cardinality: :many, unique: false]
 
   @doc false
   def struct(module, name, opts) do
@@ -902,7 +903,8 @@ defmodule Ecto.Association.ManyToMany do
       queryable: queryable,
       on_delete: on_delete,
       on_replace: on_replace,
-      defaults: opts[:defaults] || []
+      defaults: opts[:defaults] || [],
+      unique: Keyword.get(opts, :unique, false)
     }
   end
 

lib/ecto/changeset/relation.ex

@@ -204,8 +204,8 @@ defmodule Ecto.Changeset.Relation do
     {:ok, [], true, false}
   end
 
-  defp cast_or_change(%{cardinality: :many}, value, current, current_pks, new_pks, fun) when is_list(value) do
-    map_changes(value, new_pks, fun, process_current(current, current_pks), [], true, true, %{})
+  defp cast_or_change(%{cardinality: :many, unique: unique}, value, current, current_pks, new_pks, fun) when is_list(value) do
+    map_changes(value, new_pks, fun, process_current(current, current_pks), [], true, true, unique && %{})
   end
 
   defp cast_or_change(_, _, _, _, _, _), do: :error
@@ -252,7 +252,7 @@ defmodule Ecto.Changeset.Relation do
         changeset = maybe_add_error_on_pk(changeset, pk_values, acc_pk_values)
         map_changes(rest, new_pks, fun, current, [changeset | acc],
                     valid? and changeset.valid?, (struct != nil) and skip? and skip?(changeset),
-                    Map.put(acc_pk_values, pk_values, true))
+                    acc_pk_values && Map.put(acc_pk_values, pk_values, true))
       :error ->
         :error
     end
@@ -274,7 +274,7 @@ defmodule Ecto.Changeset.Relation do
         Enum.reduce(schema.__schema__(:primary_key), changeset, fn pk, acc ->
           Changeset.add_error(acc, pk, "has already been taken")
         end)
-      %{} ->
+      _ ->
         changeset
     end
   end

lib/ecto/embedded.ex

@@ -8,12 +8,13 @@ defmodule Ecto.Embedded do
                        field: atom,
                        owner: atom,
                        on_cast: nil | fun,
-                       related: atom}
+                       related: atom,
+                       unique: boolean}
 
   @behaviour Ecto.Changeset.Relation
   @on_replace_opts [:raise, :mark_as_invalid, :delete]
   @embeds_one_on_replace_opts @on_replace_opts ++ [:update]
-  defstruct [:cardinality, :field, :owner, :related, :on_cast, on_replace: :raise]
+  defstruct [:cardinality, :field, :owner, :related, :on_cast, on_replace: :raise, unique: true]
 
   @doc """
   Builds the embedded struct.

lib/ecto/schema.ex

@@ -1464,7 +1464,7 @@ defmodule Ecto.Schema do
     Module.put_attribute(mod, :changeset_fields, {name, {:assoc, struct}})
   end
 
-  @valid_many_to_many_options [:join_through, :join_keys, :on_delete, :defaults, :on_replace]
+  @valid_many_to_many_options [:join_through, :join_keys, :on_delete, :defaults, :on_replace, :unique]
 
   @doc false
   def __many_to_many__(mod, name, queryable, opts) do