diff --git a/lib/mongo_ecto/conversions.ex b/lib/mongo_ecto/conversions.ex index c32a385..4aaeb25 100644 --- a/lib/mongo_ecto/conversions.ex +++ b/lib/mongo_ecto/conversions.ex @@ -74,13 +74,25 @@ defmodule Mongo.Ecto.Conversions do defp document(doc, pk) do map(doc, fn {key, value} -> - pair(key, value, pk, &from_ecto_pk(&1, pk)) + # Recurse with `nil` so the parent schema's primary key isn't + # accidentally rewritten when it appears inside a nested document + # (e.g. an `embeds_one` whose embedded schema has its own `:id` + # field). Without this guard, an embed like + # + # %{jurisdiction: %{id: "MD", title: "Maryland"}} + # + # gets serialized as `jurisdiction._id`, which breaks queries that + # filter on `jurisdiction.id`. The pk → _id remap is only meaningful + # at the top of the document. + pair(key, value, pk, &from_ecto_pk(&1, nil)) end) end defp document(doc, params, pk) do map(doc, fn {key, value} -> - pair(key, value, pk, &inject_params(&1, params, pk)) + # Same rationale as `document/2`: scope the pk → _id rename to the + # outer document. See the comment in `document/2`. + pair(key, value, pk, &inject_params(&1, params, nil)) end) end diff --git a/test/mongo_ecto/conversions_test.exs b/test/mongo_ecto/conversions_test.exs new file mode 100644 index 0000000..c933ee8 --- /dev/null +++ b/test/mongo_ecto/conversions_test.exs @@ -0,0 +1,72 @@ +defmodule Mongo.Ecto.ConversionsTest do + use ExUnit.Case, async: true + + alias Mongo.Ecto.Conversions + + describe "from_ecto_pk/2" do + test "renames the primary key to :_id at the top level" do + assert {:ok, encoded} = Conversions.from_ecto_pk(%{id: "abc", title: "t"}, :id) + + assert Map.new(encoded) == %{_id: "abc", title: "t"} + end + + test "does not rename :id inside a nested map when the parent pk is :id" do + input = %{ + id: "abc", + title: "outer", + jurisdiction: %{id: "MD", title: "Maryland"} + } + + assert {:ok, encoded} = Conversions.from_ecto_pk(input, :id) + + encoded_map = Map.new(encoded) + assert encoded_map[:_id] == "abc" + refute Map.has_key?(encoded_map, :id) + + # The nested :id field must be passed through unchanged — it belongs to + # the embedded sub-document, not the parent schema. + assert Map.new(encoded_map[:jurisdiction]) == %{id: "MD", title: "Maryland"} + end + + test "does not rename :id inside a list of nested maps" do + input = %{id: "abc", items: [%{id: "one"}, %{id: "two"}]} + + assert {:ok, encoded} = Conversions.from_ecto_pk(input, :id) + + encoded_map = Map.new(encoded) + assert encoded_map[:_id] == "abc" + + assert Enum.map(encoded_map[:items], &Map.new/1) == [ + %{id: "one"}, + %{id: "two"} + ] + end + + test "leaves nested maps alone when the pk is nil" do + input = %{id: "abc", jurisdiction: %{id: "MD"}} + + assert {:ok, encoded} = Conversions.from_ecto_pk(input, nil) + + encoded_map = Map.new(encoded) + assert encoded_map[:id] == "abc" + assert Map.new(encoded_map[:jurisdiction]) == %{id: "MD"} + end + end + + describe "inject_params/3" do + test "renames the primary key to :_id only at the top level" do + input = %{ + id: "abc", + jurisdiction: %{id: "MD", title: "Maryland"} + } + + assert {:ok, encoded} = Conversions.inject_params(input, {}, :id) + + encoded_map = Map.new(encoded) + assert encoded_map[:_id] == "abc" + refute Map.has_key?(encoded_map, :id) + + assert Map.new(encoded_map[:jurisdiction]) == %{id: "MD", title: "Maryland"} + end + end +end