Simplify :on_writable_violation handling

Author: green-david

lib/ecto/repo/schema.ex

@@ -434,7 +434,7 @@ defmodule Ecto.Repo.Schema do
     struct = struct_from_changeset!(:insert, changeset)
     schema = struct.__struct__
     dumper = schema.__schema__(:dump)
-    {insertable_fields, non_insertable} = schema.__schema__(:insertable)
+    {keep_fields, drop_fields} = schema.__schema__(:insertable_fields)
     assocs = schema.__schema__(:associations)
     embeds = schema.__schema__(:embeds)
 
@@ -452,8 +452,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 = update_in(changeset.changes, &drop_non_writable_changes!(&1, non_insertable, :insert))
+    changeset = Relation.surface_changes(changeset, struct, keep_fields ++ assocs)
+    changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :insert))
 
     wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn ->
       assoc_opts = assoc_opts(assocs, opts)
@@ -472,7 +472,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 =
@@ -544,7 +544,7 @@ defmodule Ecto.Repo.Schema do
     struct = struct_from_changeset!(:update, changeset)
     schema = struct.__struct__
     dumper = schema.__schema__(:dump)
-    {updatable_fields, non_updatable} = schema.__schema__(:updatable)
+    {keep_fields, drop_fields} = schema.__schema__(:updatable_fields)
     assocs = schema.__schema__(:associations)
     embeds = schema.__schema__(:embeds)
 
@@ -561,7 +561,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, &drop_non_writable_changes!(&1, non_updatable, :update))
+    changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :update))
 
     if changeset.changes != %{} or force? do
       wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn ->
@@ -576,7 +576,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)
 
@@ -625,20 +625,22 @@ defmodule Ecto.Repo.Schema do
     {:error, put_repo_and_action(changeset, :update, repo, tuplet)}
   end
 
-  defp drop_non_writable_changes!(changes, non_writable, action) do
-    Enum.reduce(non_writable, changes, fn {name, on_writable_violation}, changes ->
-      case Map.pop(changes, name) do
+  defp drop_non_writable_changes!(changes, non_writable_fields, schema, action) do
+    Enum.reduce(non_writable_fields, changes, fn field, changes ->
+      case Map.pop(changes, field) do
         {nil, changes} ->
           changes
         {_change, changes} ->
-          handle_writable_violation(name, on_writable_violation, action)
+          handle_writable_violation(field, schema, action)
           changes
       end
     end)
   end
 
-  defp handle_writable_violation(name, on_writable_violation, action) do
-    message = "attempted to write to non-writable field #{inspect(name)} during #{action}"
+  defp handle_writable_violation(field, schema, action) do
+    on_writable_violation = schema.__schema__(:on_writable_violation)[field]
+
+    message = "attempted to write to non-writable field #{inspect(field)} during #{action}"
 
     case on_writable_violation do
       :raise ->
@@ -990,7 +992,7 @@ defmodule Ecto.Repo.Schema do
   end
 
   defp replace_all_fields!(_kind, schema, to_remove) do
-    {updatable_fields, _} = schema.__schema__(:updatable)
+    {updatable_fields, _} = schema.__schema__(:updatable_fields)
     Enum.map(updatable_fields -- to_remove, &field_source!(schema, &1))
   end
 

lib/ecto/schema.ex

@@ -2439,30 +2439,36 @@ defmodule Ecto.Schema do
     embed_names = Enum.map(embeds, &elem(&1, 0))
 
     updatable =
-      for {name, {_, {writable, on_writable_violation}}} <- fields, reduce: {[], []} do
+      for {name, {_, {writable, _on_writable_violation}}} <- fields, reduce: {[], []} do
         {keep, drop} ->
           case writable do
             :always -> {[name | keep], drop}
-            _ -> {keep, [{name, on_writable_violation} | drop]}
+            _ -> {keep, [name | drop]}
           end
       end
 
     insertable =
-      for {name, {_, {writable, on_writable_violation}}} <- fields, reduce: {[], []} do
+      for {name, {_, {writable, _on_writable_violation}}} <- fields, reduce: {[], []} do
         {keep, drop} ->
           case writable do
-            :never -> {keep, [{name, on_writable_violation} | drop]}
+            :never -> {keep, [name | drop]}
             _ -> {[name | keep], drop}
           end
       end
 
+    on_writable_violation =
+      for {name, {_, {_writable, on_writable_violation}}} <- fields do
+        {name, on_writable_violation}
+      end
+
     single_arg = [
       {[:dump], dump |> Map.new() |> Macro.escape()},
       {[:load], load |> Macro.escape()},
       {[:associations], assoc_names},
       {[:embeds], embed_names},
-      {[:updatable], updatable},
-      {[:insertable], insertable},
+      {[:updatable_fields], updatable},
+      {[:insertable_fields], insertable},
+      {[:on_writable_violation], on_writable_violation |> Map.new() |> Macro.escape()},
       {[:redact_fields], redacted_fields},
       {[:autogenerate_fields], Enum.flat_map(autogenerate, &elem(&1, 0))},
       {[:virtual_fields], Enum.map(virtual_fields, &elem(&1, 0))},

test/ecto/schema_test.exs

@@ -41,7 +41,7 @@ defmodule Ecto.SchemaTest do
                :comment_id
              ]
 
-    assert Schema.__schema__(:insertable) ==
+    assert Schema.__schema__(:insertable_fields) ==
              {[
                 :comment_id,
                 :non_updatable,
@@ -53,11 +53,11 @@ defmodule Ecto.SchemaTest do
                 :email,
                 :name,
                 :id
-              ], [{:unwritable, :nothing}]}
+              ], [:unwritable]}
 
-    assert Schema.__schema__(:updatable) ==
+    assert Schema.__schema__(:updatable_fields) ==
              {[:comment_id, :no_query_load, :uuid, :array, :count, :password, :email, :name, :id],
-              [{:non_updatable, :nothing}, {:unwritable, :nothing}]}
+              [:non_updatable, :unwritable]}
 
     assert Schema.__schema__(:virtual_fields) == [:temp]