feat!: a clearing request now clears, and an empty list is one by AlexandreBernard · Pull Request #7 · moduloTech/model_mapper

AlexandreBernard · 2026-10-12T13:18:59Z

Rewritten on top of #16. The previous content of this branch re-implemented nullable itself;
#14 shipped that, better, so what is left here is only the default flip it announced.

What changes

#14 shipped the deprecation step: an explicit null on a param declaring no nullable is still
treated like an absent key — skipped, or replaced by its default — and warns that a future release
will clear the value. This is that release.

nullable now defaults to true and the warning is gone:

{ nickname: 'Bob' }   # sets it
{ status: 'active' }  # nickname untouched — the key is absent
{ nickname: null }    # CLEARS it

nullable false is unchanged and refuses the null. An app that already declared nullable on a
param sees no change at all — that is exactly what the previous release's warning asked for.

An empty list is a clearing request too

On a multi-valued param (many: true, type :array), [] now clears, like null. It is what a
client sends once it has removed every element (JSON.stringify({ parts: [] })), and until now it
did nothing while null emptied the param — two spellings of one intention with opposite outcomes,
and the silent one is the one a JSON client produces naturally. Measured before this PR:

{ "parts": null }    -> 0 part left
{ "parts": [] }      -> 1 part left      ← the client believes it emptied the list
nothing sent         -> 1 part left

On a scalar param [] still says nothing and changes nothing.

A `default` answers an absent key only

Erasing a value and falling back to a default are opposite intentions, so a null wins over a
default. This aligns with what master already does for nullable true.

Why it is stacked on #16

The flip is what makes the reference-write refusal load-bearing, so it must not land before it.

Clearing a has_one/has_many reference goes through the ActiveRecord association writer, which
applies it to the database at once (#13). Until now a null only reached it through an explicit
nullable true. After the flip it would happen by default, for every undeclared reference receiving
a null — so { "manual": null, "price": "abc" } would detach the child, then answer 422 for the
price, with nothing to roll it back. Measured on master:

422: price: Invalid format for field price: expected format float.
manual still attached? widget_id=nil      ← already gone
widget saved?          price=nil          ← nothing was persisted

Stacked on #16, the same payload:

422: manual: the associated records cannot be changed here, as it would write to the database
     before the request is validated; change them in the caller.
     price: Invalid format for field price: expected format float.
manual still attached? widget_id=64       ← untouched

#16's guard already covered the clearing path, so the stacking is all it took. Two
TestImmediateWrite cases that asserted "a null is ignored, so nothing is written" now assert "a
null is refused, and nothing is written".

Base: refuse-reference-write (#16), itself on fix/reference-write-during-map (#15). Merge those
first, or retarget once they land.

Upgrading

A param that relied on "a null means leave it alone" needs nullable false. Anything that already
declares nullable is unaffected. Clearing a has_one/has_many reference needs
immediate_write true, which is #16's contract, not a new one here.

Tests

308 runs, 0 failures. The warning-machinery tests are gone with the warning; the undeclared-null
cases now assert the clear (including over a default, and under a lambda required answering
false), and new cases cover [] clearing a multi-valued param, [] saying nothing on a scalar, and
nullable false refusing an empty list.

🤖 Generated with Claude Code

Ezveus

Review of 47f9870. All 105 tests pass. Findings 1–5 below are bugs reproduced with a throwaway script; the rest are cleanup/docs/test.

Blocking: an explicit null on a reference association writes to the DB during map (inline comments on L289).

Bug — [] is silently ignored (lib/model_mapper.rb:572, outside the diff so posted here): smart_presence turns an empty array into nil, and since the raw value is not nil, explicit_null? returns false and the param is skipped. Repro: a Widget with one Part, then { parts: [] } → w.reload.parts.count is still 1, while { parts: nil } empties it. The README says a multi-valued param empties to [] on null; a client sending [] — the natural JSON for "none left" — gets no error and no change.

Ezveus · 2026-10-15T09:03:06Z

+        if value.nil?
+          # Cleared by an explicit null. A param holding several values empties to `[]` — assigning
+          # nil to a collection association raises, and an empty list is what "none left" means.
+          (param_config.multiple? || param_config.array?) ? [] : nil


Bug (reproduced) — has_one reference: explicit null writes to the DB during map. Assigning manual = nil on a persisted parent makes ActiveRecord nullify the current child immediately, which breaks the "mapping is write-free" rule the README and b1bea5f/87769ef rely on.

Repro: persisted Widget with a Manual, then map_to_model! with { manual: nil, price: 'abc' } → raises ValidationError, yet Manual.find(m.id).widget_id is already nil. Before this PR the null was skipped.

Reframed after #14 / #13. The root cause predates this PR: assigning any value to a has_one/has_many reference on a persisted record writes to the database during map (measured on master a759e6d with a non-null replacement), tracked in #13. On master, a null only reaches it through an explicit nullable true, and the README warns about it. What this PR changes is that it would happen by default, for every undeclared reference receiving a null. So it is still blocking here: either #13 is fixed first, or has_one/has_many references are left out of the default flip.

Ezveus · 2026-10-15T09:03:06Z

+        if value.nil?
+          # Cleared by an explicit null. A param holding several values empties to `[]` — assigning
+          # nil to a collection association raises, and an empty list is what "none left" means.
+          (param_config.multiple? || param_config.array?) ? [] : nil


Bug (reproduced) — many: true reference: null assigns parts = [] during map. For a persisted has_many, ActiveRecord nullifies (or with dependent: :destroy, deletes) the rows immediately, before validation completes and without any save.

Repro: { parts: nil, price: 'abc' } through map_to_model! → raises ValidationError, but Part.find(pt.id).widget_id is already nil. The caller has no way to undo it.

Reframed after #14 / #13. The root cause predates this PR: assigning any value to a has_one/has_many reference on a persisted record writes to the database during map (measured on master a759e6d with a non-null replacement), tracked in #13. On master, a null only reaches it through an explicit nullable true, and the README warns about it. What this PR changes is that it would happen by default, for every undeclared reference receiving a null. So it is still blocking here: either #13 is fixed first, or has_one/has_many references are left out of the default flip.

Ezveus · 2026-10-15T09:03:06Z

+      # null, which CLEARS the value (RFC 7386 merge-patch semantics: absent means "leave alone",
+      # null means "erase"). `nullable false` refuses that null rather than writing it.
+      if value.nil? && allow_nil
+        next unless explicit_null?(source_params, param_config)


Bug (reproduced) — an attribute with a default can never be cleared. An explicit null is replaced by the default. README/CHANGELOG say "a key sent with an explicit null clears the value".

Repro: attribute :status do default 'dflt' end, then { status: nil } → status is 'dflt', not nil.

Also (altitude): key presence is checked again by a second dig in explicit_null?, separate from validate_param's extraction, and the two lookups disagree (string/symbol, smart_presence on []/{}/"") — that's the root of the string-key and [] bugs. Having validate_param return a single read (e.g. [value, present]) and branching on it would remove both.

Still relevant — and now decided on master. #14 (merged as 857a9c5) settled the target semantics: an explicit null clears a param that has a default; default only answers an absent key (issue #6, RFC 7396). On master, nullable true on such a param clears it (test_nullable_true_does_not_apply_the_default). If this PR keeps applying the default to a null, an app that declared nullable true today changes behaviour when this merges — please align.

The altitude point in this comment (two divergent reads of the key) is resolved by #14: there is a single read_path now.

Ezveus · 2026-10-15T09:03:06Z

+  def test_explicit_null_empties_an_array_of_references
+    category = Category.create!(name: 'Tools')
+    widget = Widget.create!
+    widget.categories = [category] if widget.respond_to?(:categories=)


Test — dead setup. Widget has no categories= (only attr_accessor :category_ids), so widget.categories = [category] does nothing; category_ids starts out nil and the test only proves nil → []. It would not catch a regression where a populated list is not emptied. Suggest widget.category_ids = [category.id].

Ezveus · 2026-10-15T09:03:06Z

+
+**Upgrading:** an attribute that relied on "a null means leave it alone" needs `nullable false`.
+
+## 0.5.0


Changelog — phantom 0.5.0. The former "Unreleased" section is renamed 0.5.0, but the gemspec goes from 0.4.1 straight to 0.6.0 and there is no 0.5.0 tag, so the changelog lists a release that never existed and the upgrade notes point at the wrong version.

A key sent with an explicit null was skipped exactly like an absent key, so a payload could never clear a value (issue #6). PR #7 makes a null clear by default, which is breaking for clients that send null to mean "leave it alone"; this change prepares it without changing any default behaviour. - An explicit null (key present in its parent, raw value nil) on a param that declares no `nullable` is still skipped, but warns once per mapper class and param per process. - `nullable true` clears the value now (nil, or [] for a collection) and skips `default`; `nullable false` refuses the null with InvalidNilValueError. Both silence the warning. - `required` still wins; "", [], {} and a nil/empty parent section are not nulls; the key is matched with the same keys `validate_param` digs with, so a string key cannot clear what it cannot set. - `nullable` on a param mapped by a sub-mapper raises ConfigurationError. 226 runs, 0 failures (190 before). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Ezveus · 2026-10-15T09:54:00Z

Update after #14 (merged into master as 857a9c5).

#14 ships the deprecation step for this change without touching the default:

An explicit null on a param that does not declare nullable is still skipped, but warns once per mapper and param.
nullable true clears and nullable false refuses, with the same nullable false semantics as here.
Payload keys are read through a single symbol/string-indifferent read_path.

This PR now conflicts with master in README.md, lib/model_mapper.rb and lib/model_mapper/param_config.rb, because it re-implements nullable. Rebased, it should reduce to the default flip:

treat an undeclared nullable as true;
drop the deprecation warning;
update the README, CHANGELOG and upgrade notes. Apps that already silenced the warning by declaring nullable see no change.

Where the earlier review stands:

Thread	Status
has_one / `many: true` reference writes during map	Still blocking. The root cause predates this PR (#13), but this PR would trigger it by default. Fix #13 first, or leave these references out of the flip.
String keys clear but never set	Resolved by #14 (`read_path`). Use `master`'s `explicit_null?` after rebasing.
`[]` silently ignored	Open decision for this PR. On `master`, `[]` is not a null, and the docs leave the question to this PR. Decide whether `[]` empties a collection.
`default` blocks clearing	Still relevant, now decided on `master`. A null clears and `default` only answers an absent key. Please align.
Two divergent key reads / `reference_id` duplication	Resolved by #14.
Dead `categories=` test setup	Still relevant, unless the test goes away in the rebase.
Phantom 0.5.0 in the CHANGELOG	Still relevant until the version bump is dropped from this branch.

Second step of moduloTech#13, after the deprecation warning of the previous PR (the model is moduloTech#14 then moduloTech#7). A reference `association` whose assignment ActiveRecord would write to the database at once (a has_one, has_many or HABTM of a persisted record, given a value that differs from the current one) is now refused with an InvalidValueError on the param's path unless it declares `immediate_write true`. Nothing is assigned or written: the map is write-free again, including when it then fails validation. `immediate_write false` now says the same as leaving it undeclared; it stays a distinct state so a child mapper can override its parent either way. The deprecation warning is removed. Related to moduloTech#13. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… test The upgrade note pointed at the step-1 warning, which no release has shipped yet (both steps sit under Unreleased), so it is dropped. The breaking entry now says the deprecated `type :array, of: :referential` onto a `*_ids` writer is not covered and still writes during the map. A test pins the undeclared refusal of a `nullable true` clear on a has_one, the case where the cleared value is nil. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

An explicit null on a param declaring no `nullable` was treated like an absent key — skipped, or replaced by its `default` — while warning that a future release would clear the value. This is that release: `nullable` defaults to true, the warning is gone, and `{ name: null }` erases the value where a payload without `name` still leaves it alone (RFC 7396). `nullable false` is unchanged and refuses the null. An app that already declared `nullable` sees no change, which is what the warning asked for. An empty list on a multi-valued param is a clearing request too. It is what a client sends once it has removed every element, and it used to do nothing at all while a null emptied the param — two spellings of one intention with opposite outcomes, and the silent one is the one a JSON client produces naturally. On a scalar param `[]` still says nothing. A `default` now answers an absent key only: erasing a value and falling back to a default are opposite intentions, so the null wins. This sits on top of the reference-write refusal because the flip is what makes that refusal load-bearing. Clearing a has_one/has_many reference goes through the ActiveRecord writer, which applies it to the database at once; until now a null only reached it through an explicit `nullable true`, so a payload like `{ manual: null, price: 'abc' }` would detach the child and then answer 422 for the price, with nothing to roll it back. The clear is now refused unless the reference declares `immediate_write true`, so the default flip cannot cost anyone their data.

PR #7 makes `[]` on a multi-valued param (`many: true`, `type :array`) clear the value, like a null: it is what a client sends once it has removed every element. Master releases before #7, and until now it left `[]` outside `nullable` entirely, so #7's promise that "an app that already declared `nullable` sees no change" did not hold. Measured on master 857a9c5, `type :array` holding [1, 2], payload { numbers: [] }: nullable true -> [1, 2] (#7: []) nullable false -> [1, 2] (#7: ValidationError on numbers) and the undeclared case emitted no warning, so `[]` would have gone straight to breaking with no deprecation step. `[]` on a multi-valued param is now governed exactly like a null: undeclared keeps skipping it (or applying the `default`) and warns once per mapper and param, whichever spelling arrives first; `nullable true` clears it, weighed by `authorize` like a null clear; `nullable false` refuses it with InvalidNilValueError. `required` still wins. `""`, `{}` and `[]` on a scalar param are not clearing requests, and a sub-mapper association keeps skipping both. Tests: 322 -> 334 runs, 0 failures. Six mutations of the new branch (collection check, emptiness check, no `[]` clearing, warning wording, assign gate, required gate) each turn at least one test red. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Second step of #13, after the deprecation warning of the previous PR (the model is #14 then #7). A reference `association` whose assignment ActiveRecord would write to the database at once (a has_one, has_many or HABTM of a persisted record, given a value that differs from the current one) is now refused with an InvalidValueError on the param's path unless it declares `immediate_write true`. Nothing is assigned or written: the map is write-free again, including when it then fails validation. `immediate_write false` now says the same as leaving it undeclared; it stays a distinct state so a child mapper can override its parent either way. The deprecation warning is removed. Related to #13. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

AlexandreBernard mentioned this pull request Oct 12, 2026

feat: a mapper can authorize a write field by field #8

Merged

AlexandreBernard force-pushed the explicit-null-clears branch from 6085ea1 to 47f9870 Compare October 13, 2026 14:29

Ezveus reviewed Oct 15, 2026

View reviewed changes

This was referenced Oct 15, 2026

Assigning a has_one/has_many reference writes to the database during map #13

Open

feat: warn on an ignored explicit null; add the nullable opt-in #14

Merged

This was referenced Oct 15, 2026

feat: warn on a reference written during the map; add immediate_write #15

Merged

feat!: refuse a reference written during the map by default #16

Closed

Matthieu Ciappara and others added 3 commits October 15, 2026 12:48

AlexandreBernard force-pushed the explicit-null-clears branch from 47f9870 to 900e11c Compare October 15, 2026 13:53

AlexandreBernard changed the title ~~feat!: an explicit null clears the value, an absent key still does not~~ feat!: a clearing request now clears, and an empty list is one Oct 15, 2026

AlexandreBernard changed the base branch from master to refuse-reference-write October 15, 2026 13:53

Ezveus mentioned this pull request Oct 15, 2026

feat: an empty list on a multi-valued param is a clearing request #17

Merged

Ezveus force-pushed the refuse-reference-write branch from 45951d2 to bc06fbc Compare October 15, 2026 14:23

Ezveus force-pushed the refuse-reference-write branch from bc06fbc to 8a6396a Compare October 15, 2026 14:26

Ezveus mentioned this pull request Oct 19, 2026

fix: release review of master since 0.4.1 (7 tickets) #18

Merged

Navigation Menu

Repository navigation

feat!: a clearing request now clears, and an empty list is one - #7

feat!: a clearing request now clears, and an empty list is one#7
AlexandreBernard wants to merge 3 commits into
moduloTech:refuse-reference-writefrom
AlexandreBernard:explicit-null-clears

AlexandreBernard commented Oct 12, 2026 •

edited

Loading

Uh oh!

Ezveus left a comment

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus Oct 15, 2026

Uh oh!

Ezveus commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		Upgrading: an attribute that relied on "a null means leave it alone" needs `nullable false`.

		## 0.5.0

Conversation

AlexandreBernard commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes

An empty list is a clearing request too

A default answers an absent key only

Why it is stacked on #16

Upgrading

Tests

Uh oh!

Ezveus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ezveus commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AlexandreBernard commented Oct 12, 2026 •

edited

Loading

A `default` answers an absent key only