diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index b0a83f5896..aedc99cf34 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -362,7 +362,7 @@ defmodule Ecto.Repo.Schema do struct = struct_from_changeset!(:insert, changeset) schema = struct.__struct__ dumper = schema.__schema__(:dump) - insertable_fields = schema.__schema__(:insertable_fields) + {keep_fields, drop_fields} = schema.__schema__(:insertable_fields) assocs = schema.__schema__(:associations) embeds = schema.__schema__(:embeds) @@ -379,7 +379,8 @@ defmodule Ecto.Repo.Schema do # On insert, we always merge the whole struct into the # changeset as changes, except the primary key if it is nil. changeset = put_repo_and_action(changeset, :insert, repo, tuplet) - changeset = Relation.surface_changes(changeset, struct, insertable_fields ++ assocs) + changeset = Relation.surface_changes(changeset, struct, keep_fields ++ assocs) + changeset = update_in(changeset.changes, &Map.drop(&1, drop_fields)) wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> assoc_opts = assoc_opts(assocs, opts) @@ -398,7 +399,7 @@ defmodule Ecto.Repo.Schema do {changes, cast_extra, dump_extra, return_types, return_sources} = autogenerate_id(autogen_id, changes, return_types, return_sources, adapter) - changes = Map.take(changes, insertable_fields) + changes = Map.take(changes, keep_fields) autogen = autogenerate_changes(schema, :insert, changes) dump_changes = @@ -454,7 +455,7 @@ defmodule Ecto.Repo.Schema do struct = struct_from_changeset!(:update, changeset) schema = struct.__struct__ dumper = schema.__schema__(:dump) - updatable_fields = schema.__schema__(:updatable_fields) + {keep_fields, drop_fields} = schema.__schema__(:updatable_fields) assocs = schema.__schema__(:associations) embeds = schema.__schema__(:embeds) @@ -471,6 +472,7 @@ defmodule Ecto.Repo.Schema do # fields into the changeset. All changes must be in the # changeset before hand. changeset = put_repo_and_action(changeset, :update, repo, tuplet) + changeset = update_in(changeset.changes, &Map.drop(&1, drop_fields)) if changeset.changes != %{} or force? do wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn -> @@ -483,7 +485,7 @@ defmodule Ecto.Repo.Schema do if changeset.valid? do embeds = Ecto.Embedded.prepare(changeset, embeds, adapter, :update) - changes = changeset.changes |> Map.merge(embeds) |> Map.take(updatable_fields) + changes = changeset.changes |> Map.merge(embeds) |> Map.take(keep_fields) autogen = autogenerate_changes(schema, :update, changes) dump_changes = dump_changes!(:update, changes, autogen, schema, [], dumper, adapter) @@ -797,7 +799,8 @@ defmodule Ecto.Repo.Schema do end defp replace_all_fields!(_kind, schema, to_remove) do - Enum.map(schema.__schema__(:updatable_fields) -- to_remove, &field_source!(schema, &1)) + {updatable_fields, _} = schema.__schema__(:updatable_fields) + Enum.map(updatable_fields -- to_remove, &field_source!(schema, &1)) end defp field_source!(nil, field) do diff --git a/lib/ecto/schema.ex b/lib/ecto/schema.ex index e7450aae03..842698573d 100644 --- a/lib/ecto/schema.ex +++ b/lib/ecto/schema.ex @@ -671,11 +671,29 @@ defmodule Ecto.Schema do def __schema__(:redact_fields), do: unquote(redacted_fields) def __schema__(:virtual_fields), do: unquote(Enum.map(virtual_fields, &elem(&1, 0))) - def __schema__(:updatable_fields), - do: unquote(for {name, {_, :always}} <- fields, do: name) + def __schema__(:updatable_fields) do + unquote( + for {name, {_, writable}} <- fields, reduce: {[], []} do + {keep, drop} -> + case writable do + :always -> {[name | keep], drop} + _ -> {keep, [name | drop]} + end + end + ) + end - def __schema__(:insertable_fields), - do: unquote(for {name, {_, writable}} when writable != :never <- fields, do: name) + def __schema__(:insertable_fields) do + unquote( + for {name, {_, writable}} <- fields, reduce: {[], []} do + {keep, drop} -> + case writable do + :never -> {keep, [name | drop]} + _ -> {[name | keep], drop} + end + end + ) + end def __schema__(:autogenerate_fields), do: unquote(Enum.flat_map(autogenerate, &elem(&1, 0))) diff --git a/test/ecto/repo_test.exs b/test/ecto/repo_test.exs index d9fb87e76f..bb6f0d6132 100644 --- a/test/ecto/repo_test.exs +++ b/test/ecto/repo_test.exs @@ -1839,13 +1839,13 @@ defmodule Ecto.RepoTest do describe "on conflict" do test "passes all fields on replace_all" do - fields = [:id, :x, :yyy, :z, :array, :map] + fields = [:map, :array, :z, :yyy, :x, :id] TestRepo.insert(%MySchema{id: 1}, on_conflict: :replace_all) assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], []}}} end test "passes all fields+embeds on replace_all" do - fields = [:id, :x, :embeds] + fields = [:embeds, :x, :id] TestRepo.insert(%MySchemaEmbedsMany{id: 1}, on_conflict: :replace_all) assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], []}}} end @@ -1875,7 +1875,7 @@ defmodule Ecto.RepoTest do end test "passes all fields except given fields" do - fields = [:x, :yyy, :z, :map] + fields = [:map, :z, :yyy, :x] TestRepo.insert(%MySchema{id: 1}, on_conflict: {:replace_all_except, [:id, :array]}) assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], []}}} end diff --git a/test/ecto/schema_test.exs b/test/ecto/schema_test.exs index c71c9171dc..ea6c1cd0db 100644 --- a/test/ecto/schema_test.exs +++ b/test/ecto/schema_test.exs @@ -30,10 +30,10 @@ defmodule Ecto.SchemaTest do [:id, :name, :email, :password, :count, :array, :uuid, :no_query_load, :unwritable, :non_updatable, :comment_id] assert Schema.__schema__(:insertable_fields) == - [:id, :name, :email, :password, :count, :array, :uuid, :no_query_load, :non_updatable, :comment_id] + {[:comment_id, :non_updatable, :no_query_load, :uuid, :array, :count, :password, :email, :name, :id], [:unwritable]} assert Schema.__schema__(:updatable_fields) == - [:id, :name, :email, :password, :count, :array, :uuid, :no_query_load, :comment_id] + {[:comment_id, :no_query_load, :uuid, :array, :count, :password, :email, :name, :id], [:non_updatable, :unwritable]} assert Schema.__schema__(:virtual_fields) == [:temp]