fix(repositories) Make optimistic-lock updates race-free #23

Merged

araemer merged 1 commit from fix-optimistic-lock-race into stage

2026-10-04 06:03:35 +00:00

araemer commented

2026-10-03 12:07:33 +00:00

Owner

Fixes #20.

Problem

Two saves of the same entity that overlap in time could both pass the version check, and the later one silently overwrote the earlier one. The check (findOne with an optimistic lock) and the save() were separate statements with no row lock in between.

A second gap showed up while fixing it: save() only increments the root row's version when one of the root's own columns changed. A recipe edit that only touches ingredients or steps (or a day edit that only touches dishes) left the version unchanged, so a second stale edit passed the check even without overlap.

Change

AbstractRepository.updateWithVersionCheck() is now the one optimistic-lock update. In one transaction it locks the root row with SELECT ... FOR UPDATE, compares the version (404 if the row is gone, 409 if stale), saves, and increments the root's version if the save did not. Every successful update raises the version by exactly 1.
update(), RecipeRepository.updateRecipe(), MealPlanRepository.updateDay() and updateShare() all use it; the three copies of check-then-save are gone.
A missing version yields 400 VALIDATION_FAILED instead of 409.
AbstractRepository and its subclasses take an EntityManager (default AppDataSource.manager) and no longer call AppDataSource directly, so they can run inside a caller's transaction. ShoppingListRepository is unchanged.
AbstractEntity JSDoc no longer claims that save() rejects a stale version.
ADR docs/adr/2026-10-03-race-free-optimistic-locking.md; docs/architecture.md and CLAUDE.md updated. The shopping list item writes ADR is marked as partially superseded (only its statements about the racy update()).

Behavior changes to be aware of

A recipe's or meal plan day's version now changes on every successful update, including child-only edits.
Server-side load-then-update paths (change password, avatar, tag replacement on recipes) can now get a 409 if another write lands in between, instead of overwriting it.

Verification

npm test: 57 files, 562 tests pass, including new src/repositories/tests/AbstractRepository.test.ts.
Throwaway script against the local dev database (temp rows, removed afterwards):
- two overlapping tag updates with the same version: one succeeds, one gets 409
- two overlapping step-only recipe edits with the same version: one succeeds, one gets 409
- step-only recipe edit: version 1 -> 2 (a plain save() leaves it at 1)
- root + child edit: version rises by exactly 1
- stale version 409, missing version 400, missing row 404
- repository built on a caller's transaction manager: update runs inside it and is undone by the caller's rollback
npm run build reports one error in src/mappers/CompactShoppingListMapper.ts on my machine. It is not from this branch: the local recipe-platform checkout is on 002-shopping-list-overview-previews with uncommitted contract changes that add fields to CompactShoppingListDto. The same error appears on stage without this change.

🤖 Generated with Claude Code

Fixes #20. ## Problem Two saves of the same entity that overlap in time could both pass the version check, and the later one silently overwrote the earlier one. The check (`findOne` with an optimistic lock) and the `save()` were separate statements with no row lock in between. A second gap showed up while fixing it: `save()` only increments the root row's version when one of the root's own columns changed. A recipe edit that only touches ingredients or steps (or a day edit that only touches dishes) left the version unchanged, so a second stale edit passed the check even without overlap. ## Change - `AbstractRepository.updateWithVersionCheck()` is now the one optimistic-lock update. In one transaction it locks the root row with `SELECT ... FOR UPDATE`, compares the version (404 if the row is gone, 409 if stale), saves, and increments the root's version if the save did not. Every successful update raises the version by exactly 1. - `update()`, `RecipeRepository.updateRecipe()`, `MealPlanRepository.updateDay()` and `updateShare()` all use it; the three copies of check-then-save are gone. - A missing `version` yields 400 `VALIDATION_FAILED` instead of 409. - `AbstractRepository` and its subclasses take an `EntityManager` (default `AppDataSource.manager`) and no longer call `AppDataSource` directly, so they can run inside a caller's transaction. `ShoppingListRepository` is unchanged. - `AbstractEntity` JSDoc no longer claims that `save()` rejects a stale version. - ADR `docs/adr/2026-10-03-race-free-optimistic-locking.md`; `docs/architecture.md` and `CLAUDE.md` updated. The shopping list item writes ADR is marked as partially superseded (only its statements about the racy `update()`). ## Behavior changes to be aware of - A recipe's or meal plan day's `version` now changes on every successful update, including child-only edits. - Server-side load-then-update paths (change password, avatar, tag replacement on recipes) can now get a 409 if another write lands in between, instead of overwriting it. ## Verification - `npm test`: 57 files, 562 tests pass, including new `src/repositories/tests/AbstractRepository.test.ts`. - Throwaway script against the local dev database (temp rows, removed afterwards): - two overlapping tag updates with the same version: one succeeds, one gets 409 - two overlapping step-only recipe edits with the same version: one succeeds, one gets 409 - step-only recipe edit: version 1 -> 2 (a plain `save()` leaves it at 1) - root + child edit: version rises by exactly 1 - stale version 409, missing version 400, missing row 404 - repository built on a caller's transaction manager: update runs inside it and is undone by the caller's rollback - `npm run build` reports one error in `src/mappers/CompactShoppingListMapper.ts` on my machine. It is not from this branch: the local `recipe-platform` checkout is on `002-shopping-list-overview-previews` with uncommitted contract changes that add fields to `CompactShoppingListDto`. The same error appears on `stage` without this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

araemer added 1 commit

2026-10-03 12:07:33 +00:00

fix(repositories) Make optimistic-lock updates race-free

/ test (pull_request) Successful in 1m8s

Details

bdfa3e8590

Two overlapping saves of the same entity could both pass the version check
and the later one silently overwrote the earlier one (issue #20).

- AbstractRepository.updateWithVersionCheck() locks the row with
  SELECT ... FOR UPDATE, compares the version, saves, and increments the
  root version if the save did not, so child-only edits of a recipe or
  meal plan day also move the version
- update(), updateRecipe(), updateDay() and updateShare() share that one
  implementation
- A missing version yields 400, a vanished row 404
- AbstractRepository and its subclasses take an EntityManager
- Correct the AbstractEntity JSDoc: save() never rejects a stale version

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

araemer referenced this pull request

2026-10-03 14:07:30 +00:00

feat(shoppingList) Shopping list overview with item previews (spec 002) #25

araemer referenced this pull request

2026-10-03 17:40:06 +00:00

Release: shopping lists (spec 001) #30

araemer merged commit 3f697f9f1b into stage

2026-10-04 06:03:35 +00:00

araemer referenced this pull request from a commit

2026-10-04 06:03:36 +00:00

Merge pull request 'fix(repositories) Make optimistic-lock updates race-free' (#23) from fix-optimistic-lock-race into stage

araemer referenced this pull request