Skip to content

Add forceCreate, forceUpdate, forceFill and Model.unguarded - #36

Merged
necessarylion merged 2 commits into
mainfrom
feat/force-fill
Nov 2, 2026
Merged

necessarylion merged 2 commits into
mainfrom
feat/force-fill

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem

A query update() on a model drops keys that fillable does not allow.
So User().where('id', 1).update({'is_admin': true}) changes nothing and returns 0.
With no fillable, it throws.
There was no way to set a guarded column from a map.

What changed

  • New forceCreate(), forceUpdate() and forceFill(). They ignore fillable and guarded, like Laravel.
  • New Model.unguarded(() async { ... }). It turns the check off for every write inside the callback. It uses a zone, so other requests at the same time still use fillable.
  • forceUpdate() is also on the query builder, so it works after where(). On QueryBuilder.table() it is the same as update().
  • A loaded model update({...}) now sets updated_at, like Laravel.

Why query update() keeps the fillable check

Laravel's query update ignores fillable. Dox does not copy this.
The docs and the doc comment show User().where('id', 1).update(req.body).
If the default changed, that code would let a user set is_admin.
So the default stays safe, and forceUpdate() gives the Laravel behaviour.

Tests

New tests in test/security_test.dart.
All tests pass on SQLite, Postgres 16 and MariaDB 11. dart analyze is clean.

Docs PR: dartondox/dox (docs/force-fill).

A model update() on a query drops keys that fillable does not allow.
There was no way to set a guarded column from a map. Add the Laravel
force methods and Model.unguarded(). Query update() keeps the fillable
check, so update(req.body) stays safe.

A loaded model update() now sets updated_at.
@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

Verdict: changes requested. The code looks right. One test gap blocks.

Checks run: SQLite tests (531 passed), dart analyze --fatal-infos (no issues), dart format (clean).

Blocking

  1. test/security_test.dart:228: no test proves that other requests keep fillable while Model.unguarded() runs. This is the main security promise of the PR. If someone later swaps the zone for a static flag with try/finally, every current test still passes. Please add a test that starts a normal create(body) (with is_admin) at the same time as an unguarded callback that awaits in the middle. The normal write must still throw or drop is_admin.
  2. test/security_test.dart:220: the forceFill test only sets name, which fillable already allows. Please also force-fill a guarded column (like is_admin) and check that save() writes it to the table.

What I checked and found OK

  • No path skips fillable unless the caller picks a force method or unguarded. _onlyFillable is the only gate, and it reads the zone value at call time.
  • Zone scope: the flag stays off after the callback ends, after an error thrown inside, and in a request that runs at the same time (I checked this with a small script). Nested calls work.
  • forceFill: save() sees the new values as changed and writes them. Relation names and unknown keys are skipped. hidden/appends only matter for JSON, so nothing to do there.
  • updated_at on loaded-model update(): it uses timestampsColumn['updated_at'], so a custom column name works. With timestamps: false the generator sets it to null, so nothing changes. It is a behaviour change, but it matches save() and Laravel, and a row update that also moves updated_at should not break apps. OK as a fix.
  • Changelog is under ## Unreleased. No version bump.

Nits

  • Work started inside unguarded that ends later (a Timer, an unawaited future, a stream listener) still runs with the check off. This is how zones work. Please say it in the doc comment at lib/src/model.dart:109.
  • forceFill (lib/src/model.dart:502) skips a value of the wrong type with no sign, for example an ISO string for a DateTime field. Maybe add a short line about this in the doc, or return the skipped keys.
  • Events fired inside forceCreate/forceUpdate (like creating) also run unguarded. Laravel works the same way. A one-line note in the doc would help.
  • No test for a model with timestamps: false or a custom updated_at name on loaded-model update().

…on a guarded column

The unguarded doc comment now says that work started inside the
callback that finishes later also runs with the check off.
@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Fixer reply

Fixed the blocking items in 1eff26c.

  1. New test a request at the same time keeps fillable. An unguarded callback waits on a completer. While it waits, a normal create and update with is_admin run outside the zone, and is_admin is dropped. Then the callback goes on and its create keeps is_admin. I checked it: with a static flag in place of the zone, this test fails.
  2. The forceFill test now force-fills is_admin (a guarded column), calls save(), reloads the row and checks the value.
  3. The unguarded doc comment no longer says "nothing outside it". It now says that work started inside the callback that finishes later (timers, futures not awaited, stream listeners) also runs with the check off, so await everything inside.

Tests pass on SQLite, Postgres 16 and MariaDB 11. dart analyze is clean. I did not change the other nits.

@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

Result: no blocking issues. Ready to merge.

Checked at 1eff26c:

  • dart analyze: no issues. Full SQLite suite: all pass.
  • Concurrency test is real. I swapped the zone for a static flag. The test then failed (Model.unguarded turns fillable off only inside it and a request at the same time keeps fillable). So it guards the zone.
  • forceFill needs setColumnValue. The base Model.setColumnValue returns false (it was added earlier for increment). The dox_builder generator always writes a real one for every @DoxModel model (_getCodeForSetColumnValue, all columns including timestamps). The generated test models have it. Only the hand-written DefaultUser test model lacked it, so the test override is correct. Normal app models work.
  • No breaking change found. New methods are opt-in. A query update() keeps fillable.

Nits (do not block):

  • A hand-written model with no setColumnValue makes forceFill do nothing, with no error. The docs only show generated models, so this is rare. A later PR could make forceFill throw when a key is skipped for this reason.
  • blog.update() on a loaded model now also sets updated_at. It is a small behavior change, and it is in the CHANGELOG as a fix.

@necessarylion
necessarylion merged commit df54dea into main Nov 2, 2026
2 checks passed
@necessarylion
necessarylion deleted the feat/force-fill branch November 2, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants