Scan struct data for prepare callbacks without surfacing

The prepare callback check surfaced embeds from the data on insert,
building a changeset for every nested embed, and to_struct/4 then
surfaced them again.

Only a changeset stored in the data can have prepare callbacks, so
look for those directly instead of building new ones. Any other value
that surfacing would build a changeset from, such as a map, opens a
transaction, so its callbacks still run inside one.

Author: vanderhoop

lib/ecto/embedded.ex

@@ -145,8 +145,7 @@ defmodule Ecto.Embedded do
   # prepare callbacks, in which case a transaction is required.
   #
   # It must mirror the traversal done by prepare/4 below,
-  # including surfacing embeds (but not other fields, which
-  # are not traversed) from the data on insert.
+  # including the embeds surfaced from the data on insert.
   @doc false
   def prepare?(changeset, embeds, repo_action) do
     %{changes: changes, types: types} = changeset
@@ -172,12 +171,43 @@ defmodule Ecto.Embedded do
 
   defp to_struct_prepare?(%Changeset{}, :delete, _embed), do: false
 
-  defp to_struct_prepare?(%Changeset{data: data} = changeset, action, %{related: schema}) do
+  defp to_struct_prepare?(%Changeset{} = changeset, action, %{related: schema}) do
+    %{data: data, changes: changes} = changeset
     embeds = schema.__schema__(:embeds)
-    changeset = maybe_surface_changes(changeset, data, embeds, action)
-    prepare?(changeset, embeds, action)
+    surfaces_data? = action == :insert
+
+    prepare?(changeset, embeds, action) or
+      (surfaces_data? and data_prepare?(data, schema, embeds, changes))
+  end
+
+  # On insert, to_struct/4 surfaces the embeds left in the data. Only a changeset
+  # stored there can have prepare callbacks, so we look for those directly
+  # instead of building a changeset for every embed.
+  defp data_prepare?(data, schema, embeds, changes) do
+    Enum.any?(embeds, fn name ->
+      not is_map_key(changes, name) and
+        data_value_prepare?(Map.get(data, name), schema.__schema__(:embed, name))
+    end)
+  end
+
+  defp data_value_prepare?(nil, _embed), do: false
+
+  defp data_value_prepare?(values, embed) when is_list(values) do
+    Enum.any?(values, &data_value_prepare?(&1, embed))
+  end
+
+  defp data_value_prepare?(%Changeset{action: action} = changeset, embed) do
+    surfaced_changeset = %{changeset | action: action || :insert}
+    prepare_each?(embed, surfaced_changeset, :insert)
   end
 
+  defp data_value_prepare?(%{__struct__: schema} = struct, %{related: schema}) do
+    no_changes = %{}
+    data_prepare?(struct, schema, schema.__schema__(:embeds), no_changes)
+  end
+
+  defp data_value_prepare?(_unknown_value, _embed), do: true
+
   # Callback invoked by repository to prepare embeds.
   #
   # It replaces the changesets for embeds inside changes

test/ecto/repo/embedded_test.exs

@@ -42,6 +42,23 @@ defmodule Ecto.Repo.EmbeddedTest do
     end
   end
 
+  defmodule DeepEmbed do
+    use Ecto.Schema
+
+    embedded_schema do
+      embeds_one :embed, MyEmbed
+      embeds_many :embeds, MyEmbed
+    end
+  end
+
+  defmodule DeepSchema do
+    use Ecto.Schema
+
+    schema "deep_schema" do
+      embeds_one :deep_embed, DeepEmbed
+    end
+  end
+
   @uuid "30313233-3435-4637-9839-616263646566"
 
   ## insert
@@ -203,6 +220,9 @@ defmodule Ecto.Repo.EmbeddedTest do
     TestRepo.insert!(%MySchema{embed: %MyEmbed{x: "xyz"}, embeds: [%MyEmbed{x: "xyz"}]})
     refute_received {:transaction, _, _}
 
+    TestRepo.insert!(%MySchema{embed: %MyEmbed{x: "xyz", sub_embed: %SubEmbed{y: "xyz"}}})
+    refute_received {:transaction, _, _}
+
     embed =
       %MyEmbed{x: "xyz"}
       |> Ecto.Changeset.change()
@@ -292,6 +312,57 @@ defmodule Ecto.Repo.EmbeddedTest do
     assert_received {:prepared_in_transaction?, true}
   end
 
+  test "runs embed prepare callbacks surfaced from a map in struct data in transaction" do
+    sub_embed = embed_changeset_with_prepare(%SubEmbed{y: "xyz"})
+    deep_embed = %DeepEmbed{embed: %{x: "xyz", sub_embed: sub_embed}}
+
+    TestRepo.insert!(%DeepSchema{deep_embed: deep_embed})
+    assert_received {:transaction, _, _}
+    assert_received {:prepared_in_transaction?, true}
+  end
+
+  test "runs embed prepare callbacks surfaced from a struct in struct data in transaction" do
+    sub_embed = embed_changeset_with_prepare(%SubEmbed{y: "xyz"})
+    deep_embed = %DeepEmbed{embed: %MyEmbed{x: "xyz", sub_embed: sub_embed}}
+
+    TestRepo.insert!(%DeepSchema{deep_embed: deep_embed})
+    assert_received {:transaction, _, _}
+    assert_received {:prepared_in_transaction?, true}
+  end
+
+  test "runs embed prepare callbacks surfaced from a list in struct data in transaction" do
+    embed_with_prepare = embed_changeset_with_prepare(%MyEmbed{x: "xyz"})
+    deep_embed = %DeepEmbed{embeds: [%MyEmbed{x: "abc"}, embed_with_prepare]}
+
+    TestRepo.insert!(%DeepSchema{deep_embed: deep_embed})
+    assert_received {:transaction, _, _}
+    assert_received {:prepared_in_transaction?, true}
+  end
+
+  test "runs embed prepare callbacks surfaced from a changeset without action in struct data in transaction" do
+    sub_embed = embed_changeset_with_prepare(%SubEmbed{y: "xyz"})
+    embed_without_action = Ecto.Changeset.change(%MyEmbed{x: "xyz", sub_embed: sub_embed})
+    deep_embed = %DeepEmbed{embed: embed_without_action}
+
+    TestRepo.insert!(%DeepSchema{deep_embed: deep_embed})
+    assert_received {:transaction, _, _}
+    assert_received {:prepared_in_transaction?, true}
+  end
+
+  test "does not run embed prepare callbacks in struct data overridden by changes" do
+    overridden_embed = embed_changeset_with_prepare(%MyEmbed{x: "xyz"})
+    deep_embed = Ecto.Changeset.change(%DeepEmbed{embed: overridden_embed})
+    deep_embed = %{deep_embed | changes: %{embed: nil}}
+
+    %DeepSchema{}
+    |> Ecto.Changeset.change()
+    |> Ecto.Changeset.put_embed(:deep_embed, deep_embed)
+    |> TestRepo.insert!()
+
+    refute_received {:transaction, _, _}
+    refute_received {:prepared_in_transaction?, _}
+  end
+
   test "raises on embed changesets for another struct" do
     changeset = Ecto.Changeset.change(%MySchema{})
     embed = %Ecto.Changeset{data: %URI{}, valid?: true}