Skip to content

Commit e1d4be8

Browse files
authored
Add ability to configure the behavior of writing to a non-writable field (#4734)
1 parent 129c2ea commit e1d4be8

3 files changed

Lines changed: 167 additions & 6 deletions

File tree

lib/ecto/repo/schema.ex

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ defmodule Ecto.Repo.Schema do
66
alias Ecto.Changeset
77
alias Ecto.Changeset.Relation
88
require Ecto.Query
9+
require Logger
910

1011
import Ecto.Query.Planner, only: [attach_prefix: 2]
1112

@@ -451,8 +452,8 @@ defmodule Ecto.Repo.Schema do
451452
# On insert, we always merge the whole struct into the
452453
# changeset as changes, except the primary key if it is nil.
453454
changeset = put_repo_and_action(changeset, :insert, repo, tuplet)
454-
changeset = Relation.surface_changes(changeset, struct, keep_fields ++ assocs)
455-
changeset = update_in(changeset.changes, &Map.drop(&1, drop_fields))
455+
changeset = Relation.surface_changes(changeset, struct, keep_fields ++ drop_fields ++ assocs)
456+
changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :insert))
456457

457458
wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn ->
458459
assoc_opts = assoc_opts(assocs, opts)
@@ -560,7 +561,7 @@ defmodule Ecto.Repo.Schema do
560561
# fields into the changeset. All changes must be in the
561562
# changeset before hand.
562563
changeset = put_repo_and_action(changeset, :update, repo, tuplet)
563-
changeset = update_in(changeset.changes, &Map.drop(&1, drop_fields))
564+
changeset = update_in(changeset.changes, &drop_non_writable_changes!(&1, drop_fields, schema, :update))
564565

565566
if changeset.changes != %{} or force? do
566567
wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn ->
@@ -624,6 +625,33 @@ defmodule Ecto.Repo.Schema do
624625
{:error, put_repo_and_action(changeset, :update, repo, tuplet)}
625626
end
626627

628+
defp drop_non_writable_changes!(changes, non_writable_fields, schema, action) do
629+
Enum.reduce(non_writable_fields, changes, fn field, changes ->
630+
case changes do
631+
%{^field => _change} ->
632+
handle_writable_violation(field, schema, action)
633+
changes
634+
%{} ->
635+
changes
636+
end
637+
end)
638+
end
639+
640+
defp handle_writable_violation(field, schema, action) do
641+
on_writable_violation = schema.__schema__(:on_writable_violation)[field]
642+
643+
message = "attempted to write to non-writable field #{inspect(field)} during #{action}"
644+
645+
case on_writable_violation do
646+
:raise ->
647+
raise ArgumentError, message
648+
:warn ->
649+
Logger.warning(message)
650+
_ ->
651+
:ok
652+
end
653+
end
654+
627655
@doc """
628656
Implementation for `Ecto.Repo.insert_or_update/2`.
629657
"""

lib/ecto/schema.ex

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@ defmodule Ecto.Schema do
525525
Module.register_attribute(__MODULE__, :ecto_autogenerate, accumulate: true)
526526
Module.register_attribute(__MODULE__, :ecto_autoupdate, accumulate: true)
527527
Module.register_attribute(__MODULE__, :ecto_redact_fields, accumulate: true)
528+
Module.register_attribute(__MODULE__, :ecto_on_writable_violation, accumulate: true)
528529
end
529530
end
530531

@@ -544,7 +545,8 @@ defmodule Ecto.Schema do
544545
:where,
545546
:references,
546547
:skip_default_validation,
547-
:writable
548+
:writable,
549+
:on_writable_violation
548550
]
549551

550552
@doc """
@@ -700,6 +702,13 @@ defmodule Ecto.Schema do
700702
be further modified, even in an upsert. If set to `:never`, the field becomes
701703
read only. Defaults to `:always`.
702704
705+
* `:on_writable_violation` - Defines what action to take when performing an insert or update
706+
attempts to modify a field that should not be modified according to it's `:writable` value.
707+
Must be one of `:nothing`, `:warn`, or `:raise`. If set to `:nothing`, the modification is
708+
silently ignored. If set to `:warn`, the modification is ignored and a warning is logged. If set
709+
to `:raise`, an exception is raised and the operation is aborted. If `:writable` is set to `:always`,
710+
`:on_writable_violation` must be set to `:nothing`. Defaults to `:nothing`.
711+
703712
"""
704713
defmacro field(name, type \\ :string, opts \\ []) do
705714
quote do
@@ -2013,6 +2022,7 @@ defmodule Ecto.Schema do
20132022
virtual? = opts[:virtual] || false
20142023
pk? = opts[:primary_key] || false
20152024
writable = opts[:writable] || :always
2025+
on_writable_violation = opts[:on_writable_violation] || :nothing
20162026
put_struct_field(mod, name, Keyword.get(opts, :default))
20172027

20182028
redact_field? =
@@ -2069,6 +2079,8 @@ defmodule Ecto.Schema do
20692079
raise ArgumentError, "autogenerated fields must always be writable"
20702080
end
20712081

2082+
Module.put_attribute(mod, :ecto_on_writable_violation, {name, on_writable_violation})
2083+
20722084
if pk? do
20732085
Module.put_attribute(mod, :ecto_primary_keys, name)
20742086
end
@@ -2367,6 +2379,7 @@ defmodule Ecto.Schema do
23672379
autoupdate = Module.get_attribute(module, :ecto_autoupdate) |> Enum.reverse()
23682380
read_after_writes = Module.get_attribute(module, :ecto_raw) |> Enum.reverse()
23692381
autogenerate_id = Module.get_attribute(module, :ecto_autogenerate_id)
2382+
on_writable_violation = Module.get_attribute(module, :ecto_on_writable_violation)
23702383

23712384
struct_fields = Module.get_attribute(module, :ecto_struct_fields) |> Enum.reverse()
23722385
derive = Module.get_attribute(module, :derive)
@@ -2450,6 +2463,7 @@ defmodule Ecto.Schema do
24502463
{[:embeds], embed_names},
24512464
{[:updatable_fields], updatable},
24522465
{[:insertable_fields], insertable},
2466+
{[:on_writable_violation], on_writable_violation |> Map.new() |> Macro.escape()},
24532467
{[:redact_fields], redacted_fields},
24542468
{[:autogenerate_fields], Enum.flat_map(autogenerate, &elem(&1, 0))},
24552469
{[:virtual_fields], Enum.map(virtual_fields, &elem(&1, 0))},

test/ecto/repo_test.exs

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ defmodule Ecto.RepoTest do
33

44
import Ecto.Query
55
import Ecto, only: [put_meta: 2]
6+
import ExUnit.CaptureLog
67
alias Ecto.TestRepo
78

89
defmodule MyParent do
@@ -181,6 +182,26 @@ defmodule Ecto.RepoTest do
181182
end
182183
end
183184

185+
defmodule MySchemaWritableWarn do
186+
use Ecto.Schema
187+
188+
schema "my_schema" do
189+
field :never, :integer, writable: :never, on_writable_violation: :warn
190+
field :always, :integer, writable: :always
191+
field :insert, :integer, writable: :insert, on_writable_violation: :warn
192+
end
193+
end
194+
195+
defmodule MySchemaWritableRaise do
196+
use Ecto.Schema
197+
198+
schema "my_schema" do
199+
field :never, :integer, writable: :never, on_writable_violation: :raise
200+
field :always, :integer, writable: :always
201+
field :insert, :integer, writable: :insert, on_writable_violation: :raise
202+
end
203+
end
204+
184205
defmodule MySchemaOneField do
185206
use Ecto.Schema
186207

@@ -2418,14 +2439,47 @@ defmodule Ecto.RepoTest do
24182439
]
24192440
end
24202441

2421-
test "update only saves changes for writable: :always" do
2442+
test "update with on_writable_violation: :nothing saves changes for writable: :always and ignores changes for writable: :insert/:never" do
24222443
%MySchemaWritable{id: 1}
24232444
|> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12})
24242445
|> TestRepo.update()
24252446

24262447
assert_received {:update, %{changes: [always: 10]}}
24272448
end
24282449

2450+
test "update with on_writable_violation: :warn saves changes for writable: :always, ignores changes for writable: :insert/:never, and logs a warning" do
2451+
log = capture_log(fn ->
2452+
%MySchemaWritableWarn{id: 1}
2453+
|> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12})
2454+
|> TestRepo.update()
2455+
2456+
assert_received {:update, %{changes: [always: 10]}}
2457+
end)
2458+
2459+
assert log =~ "attempted to write to non-writable field :insert during update"
2460+
assert log =~ "attempted to write to non-writable field :never during update"
2461+
end
2462+
2463+
test "update with on_writable_violation: :raise saves changes for writable: :always and raises for changes for writable: :insert/:never" do
2464+
assert_raise ArgumentError, "attempted to write to non-writable field :never during update", fn ->
2465+
%MySchemaWritableRaise{id: 1}
2466+
|> Ecto.Changeset.change(%{never: 10})
2467+
|> TestRepo.update()
2468+
end
2469+
2470+
assert_raise ArgumentError, "attempted to write to non-writable field :insert during update", fn ->
2471+
%MySchemaWritableRaise{id: 2}
2472+
|> Ecto.Changeset.change(%{insert: 11})
2473+
|> TestRepo.update()
2474+
end
2475+
2476+
%MySchemaWritableRaise{id: 3}
2477+
|> Ecto.Changeset.change(%{always: 12})
2478+
|> TestRepo.update()
2479+
2480+
assert_received {:update, %{changes: [always: 12]}}
2481+
end
2482+
24292483
test "update is a no-op when updatable fields are not changed" do
24302484
%MySchemaWritable{id: 1}
24312485
|> Ecto.Changeset.change(%{never: "can't update", insert: "can't update either"})
@@ -2459,7 +2513,16 @@ defmodule Ecto.RepoTest do
24592513
end
24602514
end
24612515

2462-
test "insert only saves changes for writable: :always/:insert" do
2516+
test "insert with surfaced changes on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do
2517+
%MySchemaWritable{id: 1, always: 10, never: 11, insert: 12}
2518+
|> Ecto.Changeset.change(%{})
2519+
|> TestRepo.insert()
2520+
2521+
assert_received {:insert, %{fields: inserted_fields}}
2522+
assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12]
2523+
end
2524+
2525+
test "insert with on_writable_violation: :nothing saves changes for writable: :always/:insert and ignores changes for writable: :never" do
24632526
%MySchemaWritable{id: 1}
24642527
|> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12})
24652528
|> TestRepo.insert()
@@ -2468,6 +2531,62 @@ defmodule Ecto.RepoTest do
24682531
assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12]
24692532
end
24702533

2534+
test "insert with with surfaced changes and on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do
2535+
log = capture_log(fn ->
2536+
%MySchemaWritableWarn{id: 1, always: 10, never: 11, insert: 12}
2537+
|> Ecto.Changeset.change(%{})
2538+
|> TestRepo.insert()
2539+
2540+
assert_received {:insert, %{fields: inserted_fields}}
2541+
assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12]
2542+
end)
2543+
2544+
assert log =~ "attempted to write to non-writable field :never during insert"
2545+
end
2546+
2547+
test "insert with on_writable_violation: :warn saves changes for writable: :always/:insert, ignores changes for writable: :never, and logs a warning" do
2548+
log = capture_log(fn ->
2549+
%MySchemaWritableWarn{id: 1}
2550+
|> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12})
2551+
|> TestRepo.insert()
2552+
2553+
assert_received {:insert, %{fields: inserted_fields}}
2554+
assert Enum.sort(inserted_fields) == [always: 10, id: 1, insert: 12]
2555+
end)
2556+
2557+
assert log =~ "attempted to write to non-writable field :never during insert"
2558+
end
2559+
2560+
test "insert with surfaced changes and on_writable_violation: :raise saves changes for writable: :always/:insert and raises for changes for writable: :never" do
2561+
assert_raise ArgumentError, "attempted to write to non-writable field :never during insert", fn ->
2562+
%MySchemaWritableRaise{id: 1, never: 10}
2563+
|> Ecto.Changeset.change(%{})
2564+
|> TestRepo.insert()
2565+
end
2566+
2567+
%MySchemaWritableRaise{id: 2, insert: 11, always: 12}
2568+
|> Ecto.Changeset.change(%{})
2569+
|> TestRepo.insert()
2570+
2571+
assert_received {:insert, %{fields: inserted_fields}}
2572+
assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11]
2573+
end
2574+
2575+
test "insert with on_writable_violation: :raise saves changes for writable: :always/:insert and raises for changes for writable: :never" do
2576+
assert_raise ArgumentError, "attempted to write to non-writable field :never during insert", fn ->
2577+
%MySchemaWritableRaise{id: 1}
2578+
|> Ecto.Changeset.change(%{never: 10})
2579+
|> TestRepo.insert()
2580+
end
2581+
2582+
%MySchemaWritableRaise{id: 2}
2583+
|> Ecto.Changeset.change(%{insert: 11, always: 12})
2584+
|> TestRepo.insert()
2585+
2586+
assert_received {:insert, %{fields: inserted_fields}}
2587+
assert Enum.sort(inserted_fields) == [always: 12, id: 2, insert: 11]
2588+
end
2589+
24712590
test "insert with returning" do
24722591
%MySchemaWritable{id: 1}
24732592
|> Ecto.Changeset.change(%{always: 10, never: 11, insert: 12})

0 commit comments

Comments
 (0)