feat(objects): insert-only saves — opt-in create that fails instead of overwriting by rubenvdlinde · Pull Request #2211 · ConductionNL/openregister

rubenvdlinde · 2026-07-30T22:13:19Z

Closes #2210.

The problem, measured

saveObject() is an upsert. Given an identifier that already exists it updates — silently and successfully:

POST /api/objects/13/213 {"id":"a1b2…0001","name":"probe-1"}  -> created
POST /api/objects/13/213 {"id":"a1b2…0001","name":"probe-2"}  -> 200, same id
SELECT count(*), string_agg(_name,'|')                        ->  1 | probe-2

That is right for almost every caller. It is wrong for a caller claiming something — a lock, a slot, a lease, a queue position. There "it already existed" is the entire answer, and swallowing it means two callers both believe they won while the loser is never told. The _uuid unique constraint exists on the table, but nothing surfaces it: the collision resolves into an update before it can fail.

What this adds — all opt-in


`ObjectExistsException`	409 Conflict, carries the uuid so a caller can tell "my claim lost" from other errors
`SaveObject::saveObject()`	`$failIfExists` (default false) + a guard on the existing-object branch
`ObjectService::saveObject()`	pass-through
`ObjectsController::create()`	`_failIfExists=true` → 409, caught before the generic `\Exception` handler that flattens to 403
`ObjectWriteNode`	`onConflict: fail` for flows

Defaulting to false is the load-bearing choice. Making create strictly-insert by default would change behaviour for every caller relying on today's upsert — and because that upsert is silent, there is no way to enumerate from the code who depends on it.

The controller catch placement matters too: a losing claim reported as "forbidden" is indistinguishable from a permissions problem, which defeats the purpose of asking for insert-only semantics.

Verified on a live instance, both directions

default (no flag):    create=201, create=201  -> 1 row, last write wins   (unchanged)
_failIfExists=true:   create=201, create=409  -> 1 row, FIRST claimant kept it

409 body: {"error":"An object with identifier \"…\" already exists.","uuid":"…"}

Re-run after the final phpcbf reformat, not just before it.

Gates

phpcs clean on all changed files
phpstan OK
15,516 unit tests green, plus 3 new tests for the exception

Consumer

hydra's flows-first port (hydra#425, decision D7): a concurrency cap expressed as slot objects, where ObjectWriteNode's findMatch + saveObject pair could not claim a slot without a lost update. That task stayed deliberately blocked rather than shipping a lock that silently loses.

⚠️ Worth a reviewer's eye on the SaveObject guard placement specifically — it sits on the hottest write path in the fleet.

…f overwriting saveObject() is an upsert: given an identifier that already exists it updates, silently and successfully. That is right for almost every caller and is unchanged here. It is wrong for a caller that is CLAIMING something — a lock, a slot, a lease, a queue position. There "it already existed" is the entire answer, and swallowing it means two callers both believe they won while the loser is never told. Measured before writing any of this: POST /api/objects/13/213 {"id":"a1b2…0001","name":"probe-1"} -> created POST /api/objects/13/213 {"id":"a1b2…0001","name":"probe-2"} -> 200, same id SELECT count(*), string_agg(_name,'|') -> 1 | probe-2 The _uuid unique constraint exists on the table but nothing surfaces it — the collision is resolved into an update before it can fail. WHAT THIS ADDS, all opt-in: ObjectExistsException 409 Conflict, carries the uuid so a caller can tell "my claim lost" from other errors SaveObject::saveObject() $failIfExists (default false) + a guard on the existing-object branch ObjectService::saveObject() pass-through ObjectsController::create() `_failIfExists=true` -> 409. Caught BEFORE the generic \Exception handler, which flattens everything to 403 — a losing claim reported as "forbidden" is indistinguishable from a permissions problem ObjectWriteNode `onConflict: fail` for flows DEFAULTING TO false IS THE LOAD-BEARING CHOICE. Making create strictly-insert by default would change behaviour for every caller relying on today's upsert — and because that upsert is silent, there is no way to enumerate from the code who depends on it. VERIFIED ON A LIVE INSTANCE, both directions, after the final reformat: default (no flag): create=201, create=201 -> 1 row, last write wins _failIfExists=true: create=201, create=409 -> 1 row, FIRST claimant kept it 409 body: {"error":"An object with identifier \"…\" already exists.","uuid":"…"} Gates: phpcs clean on all changed files, phpstan OK, 15,516 unit tests green. Closes #2210. The consuming case is hydra's flows-first port (hydra#425 D7): a concurrency cap expressed as slot objects, where the node's findMatch/saveObject pair could not claim a slot without a lost update.

…2213) #2211 shipped with a guarantee stronger than it delivers. I verified it sequentially (claim1 -> 201, claim2 -> 409) and merged. The concurrent test fails: 10 simultaneous claims on one identifier, three runs run1: 201=6 409=2 rows=1 run2: 201=4 409=6 rows=1 run3: 201=2 409=8 rows=1 Multiple callers receive 201. Exactly one row survives, so the extra 201s are lost updates reporting success. The guard sits between the existence lookup and the write — two separate operations — so N callers can all pass the lookup before any of them writes. It narrows the window; it does not close it. What holds: the DEFAULT path is untouched, so no existing caller is affected, and a sequential duplicate is still correctly refused. What does not: `_failIfExists` / `onConflict: fail` must not be relied on for mutual exclusion. Closing it means letting the database arbitrate — a real INSERT against the existing _uuid unique constraint, translated into ObjectExistsException. Adds that warning to all three places someone will read before trusting it: the exception's own docblock, the guard in SaveObject, and the node's onConflict constant. Tracked as #2212. The acceptance criterion in hydra task 3.5 said "prove this with two flows started simultaneously, not last". I wrote that criterion and then verified sequentially anyway.

github-actions · 2026-07-30T22:21:03Z

Quality Report — ConductionNL/openregister @ `9b7d613`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
composer			✅	✅ 174/174
npm			✅	✅ 555/555
PHPUnit					✅
Newman					✅
Playwright					⏭️

Quality workflow — 2026-07-30 22:21 UTC

Download the full PDF report from the workflow artifacts.

… silently updating (#2215) #2211's guard sat in SaveObject, one layer too high, and changed nothing under concurrency. Measured then: 12 simultaneous claims on one identifier produced up to 8 responses of 201, up to 8 audit `create` entries, and ONE surviving row. WHY. There are two stacked check-then-writes, not one: 1. SaveObject::saveObject() findAndValidateExistingObject -> create 2. MagicMapper::saveObjectToRegisterSchemaTable() findObject -> insert | update A losing writer passes (1), so the SERVICE labels the operation a create and the audit trail records `create`. By the time it reaches (2) the winner's row exists, so it quietly took the UPDATE branch — overwriting the winner's data, returning 201, and logging a create. That is why guarding only (1) had no effect: the common case never reaches an INSERT at all, so there is no constraint violation to catch. Pinned by a single observation: after a race the table held ONE row with ONE _id and three audit creates. A second INSERT would have produced a second _id or a violation. Neither happened, so the extra "creates" were updates. THE FIX, in MagicMapper where the branch actually is: - update branch + failIfExists -> throw ObjectExistsException (the common case) - INSERT collision + failIfExists -> catch DbException REASON_UNIQUE_CONSTRAINT_VIOLATION -> throw ObjectExistsException (the genuinely simultaneous case; portable across MySQL/PostgreSQL, matching NotificationDedupeStateMapper's existing pattern rather than emitting dialect-specific ON CONFLICT SQL) - INSERT collision without the flag -> fall through to update, so upsert intent still lands the caller's data VERIFIED, 12 racers per run: insert-only 6/6 runs 1x201 11x409 1 row, 1 _id (was up to 8x201) default 10 racers 10x201 1 row, no errors (unchanged) sequential upsert still last-write-wins Re-run after the final phpcbf reformat, not just before it. Gates: phpcs clean, phpstan OK, 15,516 unit tests green. Closes #2212. Unblocks hydra#425 decision D7 (task 3.5): a slot claim can now be expressed with object-write without a lost update.

rubenvdlinde merged commit a5419ba into development Jul 30, 2026
17 checks passed

rubenvdlinde deleted the feat/object-create-if-absent branch July 30, 2026 22:14

rubenvdlinde mentioned this pull request Jul 31, 2026

fix(objects): failIfExists is now a real lock — the losing writer was silently updating #2215

Merged

Navigation Menu

Uh oh!

feat(objects): insert-only saves — opt-in create that fails instead of overwriting - #2211

feat(objects): insert-only saves — opt-in create that fails instead of overwriting#2211
rubenvdlinde merged 1 commit into
developmentfrom
feat/object-create-if-absent

rubenvdlinde commented Jul 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

rubenvdlinde commented Jul 30, 2026

The problem, measured

What this adds — all opt-in

Verified on a live instance, both directions

Gates

Consumer

Uh oh!

Uh oh!

github-actions Bot commented Jul 30, 2026

Quality Report — ConductionNL/openregister @ 9b7d613

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Quality Report — ConductionNL/openregister @ `9b7d613`