mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(offline): a tab waits for the flush lock instead of skipping its flush
The cross-tab lock was taken with ifAvailable, so a tab that found another tab replaying returned from flush() at once. Its callers read that as "the queue is sent": the online trigger then re-seeds every trip from the server and rehydrates the open ones, and the socket refetches after its pre-reconnect flush. All tabs see the online event together, so the tabs that lost the race laid the server's rows from before the replay over the shared Dexie, and over the optimistic edits still on their way, while the winning tab was mid-flush. The lock is now requested without ifAvailable. A tab that finds it held waits, then runs its own pass, which finds nothing or what the holder left behind, so flush() resolves only after the replay and the re-seed chained onto it reads the state after it. The holder's requests are bounded by the client timeout, so the wait is too.
This commit is contained in:
+1
-1
@@ -35,7 +35,7 @@ The layering is **component → feature hook → store/slice → `repo/` → `ap
|
||||
- **`src/repo/`** — per-entity repositories mediating between the network and the offline cache: online, call REST and upsert into Dexie; offline, read Dexie. Always go through a repo for trip data.
|
||||
- **`src/api/client.ts`** — the single Axios instance, typed from `@trek/shared`. New API surface goes in a per-domain `api/<domain>.ts`, not the god module.
|
||||
- **`src/db/offlineDb.ts`** — the Dexie schema. Database and table names are the on-device contract — renaming one orphans every user's cache.
|
||||
- **`src/sync/`** — `mutationQueue.ts`: offline writes do an optimistic Dexie write (temporary **negative id** for creates), then `flush()` replays REST on reconnect with the mutation's UUID as the **`X-Idempotency-Key`** header (matching the server interceptor) and reconciles the row. Writes to one entity keep their order: a write parked as `failed` or `conflict` holds back the later writes to the same `resource` + entity id until the user retries, resolves or discards it, and only that action deletes a parked row (trip eviction never does). `flush()` takes the Web Lock `trek-mutation-flush:<db name>` so only one tab replays at a time (the in-tab flag alone where the API is missing). The rest of the directory drives reconnection sync, the auth gate that stops background loops before logout swaps the DB, offline preferences, and map pre-download (raster and vector prefetchers).
|
||||
- **`src/sync/`** — `mutationQueue.ts`: offline writes do an optimistic Dexie write (temporary **negative id** for creates), then `flush()` replays REST on reconnect with the mutation's UUID as the **`X-Idempotency-Key`** header (matching the server interceptor) and reconciles the row. Writes to one entity keep their order: a write parked as `failed` or `conflict` holds back the later writes to the same `resource` + entity id until the user retries, resolves or discards it, and only that action deletes a parked row (trip eviction never does). `flush()` takes the Web Lock `trek-mutation-flush:<db name>` so only one tab replays at a time; a tab that finds it taken waits for it, so `flush()` never resolves while another tab is still replaying and a re-seed chained onto it cannot lay older server rows over those writes (the in-tab flag alone where the API is missing). The rest of the directory drives reconnection sync, the auth gate that stops background loops before logout swaps the DB, offline preferences, and map pre-download (raster and vector prefetchers).
|
||||
|
||||
## Rules for new code
|
||||
|
||||
|
||||
@@ -43,28 +43,27 @@ export function generateUUID(): string {
|
||||
let _flushing = false
|
||||
|
||||
/**
|
||||
* Take the cross-tab flush lock, or learn that another tab holds it. Every open
|
||||
* Take the cross-tab flush lock, waiting while another tab holds it. Every open
|
||||
* tab of the account shares the queue and runs its own triggers, and the
|
||||
* _flushing flag only covers its own tab: two tabs read the same pending rows
|
||||
* and replay each of them twice, leaning on the server's replay cache to answer
|
||||
* the second. Resolves to the release function, or null when another tab is
|
||||
* flushing (that flush picks the rows up). Where the Web Locks API is missing
|
||||
* or refuses the request, the flush runs as before, guarded by the flag alone.
|
||||
* The lock goes with the tab, so a tab killed mid-flush never strands it.
|
||||
* the second. Waiting rather than skipping matters to the callers: the online
|
||||
* trigger re-seeds Dexie from the server once flush() resolves, and the socket
|
||||
* refetches the trip, so a flush that returned while another tab was still
|
||||
* replaying would let them lay the server's older rows over edits that are
|
||||
* about to land. The waiting tab then runs its own pass, which finds what the
|
||||
* holder left. Resolves to the release function. Where the Web Locks API is
|
||||
* missing or refuses the request, the flush runs as before, guarded by the
|
||||
* flag alone. The lock goes with the tab, so a tab killed mid-flush never
|
||||
* strands it.
|
||||
*/
|
||||
function acquireFlushLock(): Promise<(() => void) | null> {
|
||||
function acquireFlushLock(): Promise<() => void> {
|
||||
const noop = () => {}
|
||||
const locks = typeof navigator === 'undefined' ? undefined : navigator.locks
|
||||
if (!locks || typeof locks.request !== 'function') return Promise.resolve(noop)
|
||||
return new Promise(resolve => {
|
||||
locks
|
||||
.request(`trek-mutation-flush:${offlineDb.name}`, { ifAvailable: true }, lock => {
|
||||
if (!lock) {
|
||||
resolve(null)
|
||||
return undefined
|
||||
}
|
||||
return new Promise<void>(release => resolve(release))
|
||||
})
|
||||
.request(`trek-mutation-flush:${offlineDb.name}`, () => new Promise<void>(release => resolve(release)))
|
||||
.catch(() => resolve(noop))
|
||||
})
|
||||
}
|
||||
@@ -244,10 +243,6 @@ export const mutationQueue = {
|
||||
if (_flushing || isEffectivelyOffline() || !isAuthed()) return
|
||||
_flushing = true
|
||||
const release = await acquireFlushLock()
|
||||
if (!release) {
|
||||
_flushing = false
|
||||
return
|
||||
}
|
||||
// tempId → realId learned during this flush, so a dependent edit/delete
|
||||
// queued against an offline-created entity (still holding the negative id)
|
||||
// can be rewritten to the server id before it is replayed.
|
||||
|
||||
@@ -716,42 +716,70 @@ describe('mutationQueue: writes to one entity keep their order around a parked o
|
||||
});
|
||||
|
||||
describe('mutationQueue.flush: one tab at a time', () => {
|
||||
/** A Web Locks stand-in: one holder per name, `ifAvailable` answers null when taken. */
|
||||
/** A Web Locks stand-in: one holder per name, later requests wait their turn. */
|
||||
function fakeLocks() {
|
||||
const held = new Set<string>();
|
||||
const names: string[] = [];
|
||||
const request = vi.fn(async (name: string, _opts: { ifAvailable?: boolean }, cb: (lock: { name: string } | null) => unknown) => {
|
||||
names.push(name);
|
||||
if (held.has(name)) return cb(null);
|
||||
held.add(name);
|
||||
try { return await cb({ name }); } finally { held.delete(name); }
|
||||
const tails = new Map<string, Promise<unknown>>();
|
||||
const request = vi.fn((name: string, cb: (lock: { name: string }) => unknown) => {
|
||||
const run = (tails.get(name) ?? Promise.resolve()).then(() => cb({ name }));
|
||||
tails.set(name, run.catch(() => {}));
|
||||
return run;
|
||||
});
|
||||
Object.defineProperty(navigator, 'locks', { value: { request }, configurable: true });
|
||||
return { held, names, request };
|
||||
/** Another tab takes the lock and keeps it until the returned function runs. */
|
||||
const holdElsewhere = () => {
|
||||
let release = () => {};
|
||||
void request(`trek-mutation-flush:${offlineDb.name}`, () => new Promise<void>(r => { release = r; }));
|
||||
return () => release();
|
||||
};
|
||||
return { request, holdElsewhere };
|
||||
}
|
||||
|
||||
const settle = () => new Promise(resolve => setTimeout(resolve, 20));
|
||||
|
||||
afterEach(() => {
|
||||
Reflect.deleteProperty(navigator, 'locks');
|
||||
});
|
||||
|
||||
it('leaves the queue to the tab that holds the lock', async () => {
|
||||
it('waits for the tab that holds the lock, so what the caller does next sees its replay', async () => {
|
||||
const locks = fakeLocks();
|
||||
locks.held.add(`trek-mutation-flush:${offlineDb.name}`);
|
||||
const releaseOther = locks.holdElsewhere();
|
||||
const id = generateUUID();
|
||||
await mutationQueue.enqueue(makeMutation({ id }));
|
||||
let sent = 0;
|
||||
server.use(http.post('/api/trips/1/places', () => { sent++; return HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 61 }) }); }));
|
||||
|
||||
await mutationQueue.flush();
|
||||
let done = false;
|
||||
const flushed = mutationQueue.flush().then(() => { done = true; });
|
||||
await settle();
|
||||
// Still waiting: a caller chaining a re-seed onto this flush must not run yet.
|
||||
expect(done).toBe(false);
|
||||
expect(sent).toBe(0);
|
||||
expect(await offlineDb.mutationQueue.get(id)).toMatchObject({ status: 'pending' });
|
||||
|
||||
// The other tab is done: the next trigger here takes the lock and sends it.
|
||||
locks.held.clear();
|
||||
await mutationQueue.flush();
|
||||
// The other tab sends the row and lets go of the lock.
|
||||
await offlineDb.mutationQueue.delete(id);
|
||||
releaseOther();
|
||||
await flushed;
|
||||
|
||||
expect(done).toBe(true);
|
||||
expect(sent).toBe(0);
|
||||
expect(locks.request).toHaveBeenCalledWith(`trek-mutation-flush:${offlineDb.name}`, expect.any(Function));
|
||||
});
|
||||
|
||||
it('sends what the other tab left once it has the lock', async () => {
|
||||
const locks = fakeLocks();
|
||||
const releaseOther = locks.holdElsewhere();
|
||||
const id = generateUUID();
|
||||
await mutationQueue.enqueue(makeMutation({ id }));
|
||||
let sent = 0;
|
||||
server.use(http.post('/api/trips/1/places', () => { sent++; return HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 63 }) }); }));
|
||||
|
||||
const flushed = mutationQueue.flush();
|
||||
await settle();
|
||||
releaseOther();
|
||||
await flushed;
|
||||
|
||||
expect(sent).toBe(1);
|
||||
expect(await offlineDb.mutationQueue.get(id)).toBeUndefined();
|
||||
expect(locks.held.size).toBe(0);
|
||||
});
|
||||
|
||||
it('flushes under the flag alone when the lock request is refused', async () => {
|
||||
|
||||
Reference in New Issue
Block a user