diff --git a/lib/ecto/multi.ex b/lib/ecto/multi.ex index 4d92f64d98..5b063b0a15 100644 --- a/lib/ecto/multi.ex +++ b/lib/ecto/multi.ex @@ -124,7 +124,7 @@ defmodule Ecto.Multi do alias __MODULE__ alias Ecto.Changeset - defstruct operations: [], names: MapSet.new() + defstruct operations: [], names: %{} @typedoc """ Map of changes made so far during the current transaction. For any Multi @@ -147,7 +147,10 @@ defmodule Ecto.Multi do | {:insert_all, schema_or_source, [map | Keyword.t()], Keyword.t()} @typep operations :: [{name, operation}] - @typep names :: MapSet.t() + # A plain map is used instead of MapSet, as MapSet's internal :sets + # representation is opaque and Dialyzer reports opaqueness violations + # when the struct default above gets inlined into callers. + @typep names :: %{optional(name) => true} @typedoc """ Name of an operation in the Multi. Can be any term, as long as it is unique @@ -219,9 +222,9 @@ defmodule Ecto.Multi do %{names: lhs_names, operations: lhs_ops} = lhs %{names: rhs_names, operations: rhs_ops} = rhs - case MapSet.intersection(lhs_names, rhs_names) |> MapSet.to_list() do + case common_names(lhs_names, rhs_names) do [] -> - %Multi{names: MapSet.union(lhs_names, rhs_names), operations: joiner.(lhs_ops, rhs_ops)} + %Multi{names: Map.merge(lhs_names, rhs_names), operations: joiner.(lhs_ops, rhs_ops)} common -> raise ArgumentError, """ @@ -746,10 +749,10 @@ defmodule Ecto.Multi do defp add_operation(%Multi{} = multi, name, operation) do %{operations: operations, names: names} = multi - if MapSet.member?(names, name) do + if Map.has_key?(names, name) do raise "#{Kernel.inspect(name)} is already a member of the Ecto.Multi: \n#{Kernel.inspect(multi)}" else - %{multi | operations: [{name, operation} | operations], names: MapSet.put(names, name)} + %{multi | operations: [{name, operation} | operations], names: Map.put(names, name, true)} end end @@ -942,11 +945,11 @@ defmodule Ecto.Multi do defp apply_run_fun(fun, repo, acc), do: apply(fun, [repo, acc]) defp merge_results(changes, new_changes, names) do - new_names = new_changes |> Map.keys() |> MapSet.new() + new_names = Map.new(new_changes, fn {name, _} -> {name, true} end) - case MapSet.intersection(names, new_names) |> MapSet.to_list() do + case common_names(names, new_names) do [] -> - {Map.merge(changes, new_changes), MapSet.union(names, new_names)} + {Map.merge(changes, new_changes), Map.merge(names, new_names)} common -> raise "cannot merge Multi; the following operations were found in " <> @@ -954,6 +957,10 @@ defmodule Ecto.Multi do end end + defp common_names(lhs_names, rhs_names) do + for {name, true} <- lhs_names, Map.has_key?(rhs_names, name), do: name + end + defp operation_fun({:update_all, queryable_fun, updates}, opts) do fn repo, changes -> {:ok, repo.update_all(queryable_fun.(changes), updates, opts)} diff --git a/test/ecto/multi_test.exs b/test/ecto/multi_test.exs index 5aaef41903..3fb3c35366 100644 --- a/test/ecto/multi_test.exs +++ b/test/ecto/multi_test.exs @@ -31,7 +31,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert(:comment, changeset) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}] end @@ -43,7 +43,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert(:comment, struct) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}] end @@ -55,7 +55,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations end @@ -66,7 +66,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.update(:comment, changeset) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :update}, []}}] end @@ -78,7 +78,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.update(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations end @@ -89,7 +89,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert_or_update(:comment, changeset) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}] end @@ -98,7 +98,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.inspect() - assert multi.names == MapSet.new([]) + assert multi.names == %{} assert multi.operations == [{:inspect, {:inspect, []}}] end @@ -110,7 +110,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert_or_update(:comment, changeset) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :update}, []}}] end @@ -122,7 +122,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert_or_update(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations end @@ -133,7 +133,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.delete(:comment, changeset) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :delete}, []}}] end @@ -145,7 +145,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.delete(:comment, struct) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert multi.operations == [{:comment, {:changeset, %{changeset | action: :delete}, []}}] end @@ -157,7 +157,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.delete(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations end @@ -166,7 +166,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.one(:comment, Comment) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert [{:comment, {:run, _fun}}] = multi.operations end @@ -177,7 +177,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.one(:comment, fun) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert [{:comment, {:run, _fun}}] = multi.operations end @@ -186,7 +186,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.all(:comments, Comment) - assert multi.names == MapSet.new([:comments]) + assert multi.names == %{:comments => true} assert [{:comments, {:run, _fun}}] = multi.operations end @@ -197,7 +197,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.all(:comments, fun) - assert multi.names == MapSet.new([:comments]) + assert multi.names == %{:comments => true} assert [{:comments, {:run, _fun}}] = multi.operations end @@ -206,7 +206,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.exists?(:comment, Comment) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert [{:comment, {:run, _fun}}] = multi.operations end @@ -217,7 +217,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.exists?(:comment, fun) - assert multi.names == MapSet.new([:comment]) + assert multi.names == %{:comment => true} assert [{:comment, {:run, _fun}}] = multi.operations end @@ -226,7 +226,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.error(:oops, :value) - assert multi.names == MapSet.new([:oops]) + assert multi.names == %{:oops => true} assert multi.operations == [{:oops, {:error, :value}}] end @@ -237,7 +237,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.run(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert multi.operations == [{:fun, {:run, fun}}] end @@ -248,7 +248,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.run({:fun, 3}, fun) - assert multi.names == MapSet.new([{:fun, 3}]) + assert multi.names == %{{:fun, 3} => true} assert multi.operations == [{{:fun, 3}, {:run, fun}}] end @@ -259,7 +259,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.run(~c"myFunction", fun) - assert multi.names == MapSet.new([~c"myFunction"]) + assert multi.names == %{~c"myFunction" => true} assert multi.operations == [{~c"myFunction", {:run, fun}}] end @@ -268,7 +268,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.run(:fun, __MODULE__, :run_ok, []) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert multi.operations == [{:fun, {:run, {__MODULE__, :run_ok, []}}}] end @@ -277,7 +277,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert_all(:comments, Comment, [[x: 2]]) - assert multi.names == MapSet.new([:comments]) + assert multi.names == %{:comments => true} assert [{:comments, {:insert_all, Comment, [[x: 2]], []}}] = multi.operations end @@ -288,7 +288,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.insert_all(:fun, Comment, fun_entries) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations assert {:ok, changes} = TestRepo.transact(multi) @@ -302,7 +302,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.update_all(:comments, Comment, set: [x: 2]) - assert multi.names == MapSet.new([:comments]) + assert multi.names == %{:comments => true} assert [{:comments, {:update_all, query, updates, []}}] = multi.operations assert updates == [set: [x: 2]] assert query == Ecto.Queryable.to_query(Comment) @@ -315,7 +315,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.update_all(:fun, fun_queryable, []) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations assert {:ok, changes} = TestRepo.transact(multi) @@ -329,7 +329,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.delete_all(:comments, Comment) - assert multi.names == MapSet.new([:comments]) + assert multi.names == %{:comments => true} assert [{:comments, {:delete_all, query, []}}] = multi.operations assert query == Ecto.Queryable.to_query(Comment) end @@ -341,7 +341,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.delete_all(:fun, fun) - assert multi.names == MapSet.new([:fun]) + assert multi.names == %{:fun => true} assert [{:fun, {:run, _fun}}] = multi.operations assert {:ok, changes} = TestRepo.transact(multi) @@ -357,12 +357,12 @@ defmodule Ecto.MultiTest do merged = Multi.append(lhs, rhs) operations = Keyword.keys(merged.operations) - assert merged.names == MapSet.new([:one, :two, :three, :four]) + assert merged.names == %{:one => true, :two => true, :three => true, :four => true} assert operations == [:four, :three, :two, :one] merged = Multi.prepend(lhs, rhs) operations = Keyword.keys(merged.operations) - assert merged.names == MapSet.new([:one, :two, :three, :four]) + assert merged.names == %{:one => true, :two => true, :three => true, :four => true} assert operations == [:two, :one, :four, :three] end @@ -419,7 +419,7 @@ defmodule Ecto.MultiTest do Multi.new() |> Multi.put(name, value) - assert multi.names == MapSet.new([name]) + assert multi.names == %{name => true} assert multi.operations == [{name, {:put, value}}] end