[atomic-reuse] Preserve concurrent updates arriving during an asynchronous row save #54

New issue

Closed

opened 2026-10-19 12:06:03 +02:00 by michielbdejong · 1 comment

michielbdejong commented

2026-10-19 12:06:03 +02:00

Owner

Affected path

Scope: atomic-reuse only for this reproduced defect.

Path	Affected by this finding?
`/atomic-reuse` (`AtomicLibDemo` + `AtomicTable`)	Yes; reproduced with pinned Atomic lib 0.40.0.
NG ORM table editor (`db-table-editor-native`)	Not this implementation defect; it uses a different adapter.
Main `db-table-editor-atomic` editor on NG ORM	Not this implementation defect; despite its name, its record-writing path also uses NG ORM, not `AtomicTable`.

This scope does not claim that the NG editors have no independent correctness problems. Reproductions used commit be8938078d97a70519a991ade8b9207d07067c5f, and current main's relevant source was checked to match. Temporary focused tests reproduced the defects; no fix or permanent regression test has been added.

Problem

The adapter checks the baseline and clones the current Resource before awaiting its field setters. An external update can arrive after that check. The demo persistence callback then inserts the complete older clone with skipCommitCompare: true, replacing the updated Resource and losing an unrelated field change.

Sources: baseline, clone and asynchronous setters, demo persistence callback.

The NG editor applies changed fields to an observable ORM row instead of persisting a complete Atomic clone through this callback. This reproduction is not evidence of the same mechanism in NG or a claim about Atomic Server commit behavior.

Deterministic reproduction

Using the setup/helper variables in tests/unit/atomic-table.test.ts:

const { table, store } = setup();
await table.create(base + 'one', { [title]: 'Before', [count]: 2 });
const row = table.records()[0];
const saving = row.save({ [title]: 'Local edit' }, row.values);

// Arrives after baseline checking, while the save yields in Resource.set().
const external = store.resources.get(row.id)!.clone();
external.setUnsafe(count, 99);
store.addResources(external, { skipCommitCompare: true });

await saving;
expect(table.records()[0].values[title]).toBe('Local edit');
expect(table.records()[0].values[count]).toBe(99);

Observed: Count becomes 99 on external insertion, then reverts to 2 after the title save finishes. The final assertion fails. This was reproduced with a controlled adapter-level interleaving, not by sequentially clicking two demo buttons. Existing tests cover an update arriving before save starts, not this window.

Acceptance

Preserve unrelated external changes arriving before or during save.
Detect/reconcile overlapping changes without silently replacing them; retain the draft on conflict/failure.
Use a coherent per-resource strategy for conflict checking and application. A second check followed by another asynchronous full replacement must not leave the same race.
Add deterministic tests for unrelated/overlapping updates during save, deletion during save and retry.
Keep the demo callback guarantee distinct from real backend commit, authorization and concurrency behavior (#45).

Related: #51 is membership; #52 is performance, not this save race.

## Affected path **Scope: atomic-reuse only for this reproduced defect.** | Path | Affected by this finding? | | --- | --- | | `/atomic-reuse` (`AtomicLibDemo` + `AtomicTable`) | Yes; reproduced with pinned Atomic lib 0.40.0. | | NG ORM table editor (`db-table-editor-native`) | Not this implementation defect; it uses a different adapter. | | Main `db-table-editor-atomic` editor on NG ORM | Not this implementation defect; despite its name, its record-writing path also uses NG ORM, not `AtomicTable`. | This scope does not claim that the NG editors have no independent correctness problems. Reproductions used commit `be8938078d97a70519a991ade8b9207d07067c5f`, and current main's relevant source was checked to match. Temporary focused tests reproduced the defects; no fix or permanent regression test has been added. ## Problem The adapter checks the baseline and clones the current Resource before awaiting its field setters. An external update can arrive after that check. The demo persistence callback then inserts the complete older clone with `skipCommitCompare: true`, replacing the updated Resource and losing an unrelated field change. Sources: [baseline, clone and asynchronous setters](https://git.elfaconsortium.eu/ELFA/database-apps/src/commit/be8938078d97a70519a991ade8b9207d07067c5f/shared/atomic/atomicTable.ts#L50), [demo persistence callback](https://git.elfaconsortium.eu/ELFA/database-apps/src/commit/be8938078d97a70519a991ade8b9207d07067c5f/apps/db-table-editor-atomic/AtomicLibDemo.tsx#L29). The NG editor applies changed fields to an observable ORM row instead of persisting a complete Atomic clone through this callback. This reproduction is not evidence of the same mechanism in NG or a claim about Atomic Server commit behavior. ## Deterministic reproduction Using the setup/helper variables in `tests/unit/atomic-table.test.ts`: ```ts const { table, store } = setup(); await table.create(base + 'one', { [title]: 'Before', [count]: 2 }); const row = table.records()[0]; const saving = row.save({ [title]: 'Local edit' }, row.values); // Arrives after baseline checking, while the save yields in Resource.set(). const external = store.resources.get(row.id)!.clone(); external.setUnsafe(count, 99); store.addResources(external, { skipCommitCompare: true }); await saving; expect(table.records()[0].values[title]).toBe('Local edit'); expect(table.records()[0].values[count]).toBe(99); ``` Observed: Count becomes 99 on external insertion, then reverts to 2 after the title save finishes. The final assertion fails. This was reproduced with a controlled adapter-level interleaving, not by sequentially clicking two demo buttons. Existing tests cover an update arriving before save starts, not this window. ## Acceptance - Preserve unrelated external changes arriving before or during save. - Detect/reconcile overlapping changes without silently replacing them; retain the draft on conflict/failure. - Use a coherent per-resource strategy for conflict checking and application. A second check followed by another asynchronous full replacement must not leave the same race. - Add deterministic tests for unrelated/overlapping updates during save, deletion during save and retry. - Keep the demo callback guarantee distinct from real backend commit, authorization and concurrency behavior (#45). Related: #51 is membership; #52 is performance, not this save race.

michielbdejong referenced this issue

2026-10-19 13:52:52 +02:00

Update active table editors when external changes arrive #57

michielbdejong commented

2026-10-19 13:53:00 +02:00

Author

Owner

The React update handling is proposed in #57 for both NG-backed table editors and /atomic-reuse. Incoming changes update untouched draft fields; overlapping scalar edits retain the draft and expose explicit resolution choices. Native controls preserve invalid intermediate text when an external edit arrives.

This is a partial mitigation, not a fix for the asynchronous save race described here. The gap after conflict checking and before persistence remains, so #54 stays open. Closing it still requires a concurrency-safe check-and-apply operation in the persistence path. All 336 unit tests, six typechecks, 12 production builds and 22 focused browser tests passed locally.

The React update handling is proposed in https://git.elfaconsortium.eu/ELFA/database-apps/pulls/57 for both NG-backed table editors and `/atomic-reuse`. Incoming changes update untouched draft fields; overlapping scalar edits retain the draft and expose explicit resolution choices. Native controls preserve invalid intermediate text when an external edit arrives. This is a partial mitigation, not a fix for the asynchronous save race described here. The gap after conflict checking and before persistence remains, so #54 stays open. Closing it still requires a concurrency-safe check-and-apply operation in the persistence path. All 336 unit tests, six typechecks, 12 production builds and 22 focused browser tests passed locally.

michielbdejong referenced this issue

2026-10-19 14:50:13 +02:00

Scope Atomic table rows by table parent and class #59

michielbdejong referenced this issue from a pull request that will close it,

2026-10-19 16:39:26 +02:00

Preserve concurrent updates during Atomic table saves #62

michielbdejong closed this issue

2026-10-28 13:04:50 +01:00