mirror of
https://github.com/jcreek/LivingDexTracker.git
synced 2026-09-16 02:22:17 +00:00
fix(sync): keep queued catch-record writes from crossing an account change
The queue held whole records and flushed whatever was in it, so edits made before a sign-out could be written against the account that signed in next. Queue patches and merge them per key, so two edits to one entry combine instead of one replacing the other; drop everything when the owning account is no longer the current one; and stop scheduling flushes while offline, where they could only fail and back off.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { writable, type Readable } from 'svelte/store';
|
import { writable, type Readable } from 'svelte/store';
|
||||||
import type { CatchRecord } from '$lib/models/CatchRecord';
|
import type { CatchRecordPatch } from '$lib/models/PokedexGridRow';
|
||||||
|
|
||||||
export type CatchRecordWriteQueueStatus = {
|
export type CatchRecordWriteQueueStatus = {
|
||||||
pending: number;
|
pending: number;
|
||||||
@@ -10,7 +10,7 @@ export type CatchRecordWriteQueueStatus = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
type QueueItem = {
|
type QueueItem = {
|
||||||
record: CatchRecord;
|
record: CatchRecordPatch;
|
||||||
attempts: number;
|
attempts: number;
|
||||||
notBefore: number; // unix ms
|
notBefore: number; // unix ms
|
||||||
debounceTimer: ReturnType<typeof setTimeout> | null;
|
debounceTimer: ReturnType<typeof setTimeout> | null;
|
||||||
@@ -31,6 +31,8 @@ export type CreateCatchRecordWriteQueueOptions = {
|
|||||||
batchSize?: number;
|
batchSize?: number;
|
||||||
/** Max number of concurrent in-flight requests. */
|
/** Max number of concurrent in-flight requests. */
|
||||||
concurrency?: number;
|
concurrency?: number;
|
||||||
|
/** Prevent queued work from crossing an account change. */
|
||||||
|
isCurrentUser?: () => boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type EnqueueCatchRecordWriteOptions = {
|
export type EnqueueCatchRecordWriteOptions = {
|
||||||
@@ -53,7 +55,7 @@ export type FlushOptions = {
|
|||||||
limit?: number;
|
limit?: number;
|
||||||
};
|
};
|
||||||
|
|
||||||
function keyFor(record: CatchRecord): string {
|
function keyFor(record: CatchRecordPatch): string {
|
||||||
return `${record.userId}:${record.pokedexId}:${record.pokemonId}`;
|
return `${record.userId}:${record.pokedexId}:${record.pokemonId}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -66,11 +68,12 @@ function backoffMs(attempts: number): number {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueueOptions): {
|
export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueueOptions): {
|
||||||
enqueue: (record: CatchRecord, opts?: EnqueueCatchRecordWriteOptions) => void;
|
enqueue: (record: CatchRecordPatch, opts?: EnqueueCatchRecordWriteOptions) => void;
|
||||||
flushNow: (opts?: FlushOptions) => Promise<void>;
|
flushNow: (opts?: FlushOptions) => Promise<void>;
|
||||||
getStatus: Readable<CatchRecordWriteQueueStatus>;
|
getStatus: Readable<CatchRecordWriteQueueStatus>;
|
||||||
getPendingCount: () => number;
|
getPendingCount: () => number;
|
||||||
clearError: () => void;
|
clearError: () => void;
|
||||||
|
getPendingPatch: (pokemonId: string) => CatchRecordPatch | undefined;
|
||||||
} {
|
} {
|
||||||
const { endpointUrl, fetchFn, batchSize = 100, concurrency = 1 } = options;
|
const { endpointUrl, fetchFn, batchSize = 100, concurrency = 1 } = options;
|
||||||
|
|
||||||
@@ -109,7 +112,7 @@ export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueue
|
|||||||
}
|
}
|
||||||
|
|
||||||
function scheduleFlush() {
|
function scheduleFlush() {
|
||||||
if (scheduled) return;
|
if (scheduled || (typeof navigator !== 'undefined' && navigator.onLine === false)) return;
|
||||||
const next = computeNextWakeup();
|
const next = computeNextWakeup();
|
||||||
if (next === null) return;
|
if (next === null) return;
|
||||||
const delay = Math.max(0, next - Date.now());
|
const delay = Math.max(0, next - Date.now());
|
||||||
@@ -120,6 +123,12 @@ export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueue
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function flushBatch(opts?: FlushOptions): Promise<void> {
|
async function flushBatch(opts?: FlushOptions): Promise<void> {
|
||||||
|
if (options.isCurrentUser && !options.isCurrentUser()) {
|
||||||
|
for (const item of items.values()) if (item.debounceTimer) clearTimeout(item.debounceTimer);
|
||||||
|
items.clear();
|
||||||
|
updateStatus({ lastError: null });
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (typeof navigator !== 'undefined' && navigator.onLine === false) {
|
if (typeof navigator !== 'undefined' && navigator.onLine === false) {
|
||||||
// Stay queued; caller can retry when online.
|
// Stay queued; caller can retry when online.
|
||||||
return;
|
return;
|
||||||
@@ -206,7 +215,7 @@ export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueue
|
|||||||
scheduleFlush();
|
scheduleFlush();
|
||||||
}
|
}
|
||||||
|
|
||||||
function enqueue(record: CatchRecord, opts?: EnqueueCatchRecordWriteOptions) {
|
function enqueue(record: CatchRecordPatch, opts?: EnqueueCatchRecordWriteOptions) {
|
||||||
const k = keyFor(record);
|
const k = keyFor(record);
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const existing = items.get(k);
|
const existing = items.get(k);
|
||||||
@@ -232,7 +241,7 @@ export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueue
|
|||||||
}
|
}
|
||||||
|
|
||||||
items.set(k, {
|
items.set(k, {
|
||||||
record,
|
record: { ...existing?.record, ...record },
|
||||||
attempts: existing?.attempts ?? 0,
|
attempts: existing?.attempts ?? 0,
|
||||||
notBefore,
|
notBefore,
|
||||||
debounceTimer,
|
debounceTimer,
|
||||||
@@ -255,6 +264,8 @@ export function createCatchRecordWriteQueue(options: CreateCatchRecordWriteQueue
|
|||||||
flushNow,
|
flushNow,
|
||||||
getStatus: { subscribe: statusStore.subscribe },
|
getStatus: { subscribe: statusStore.subscribe },
|
||||||
getPendingCount,
|
getPendingCount,
|
||||||
clearError
|
clearError,
|
||||||
|
getPendingPatch: (pokemonId: string) =>
|
||||||
|
[...items.values()].find((item) => item.record.pokemonId === pokemonId)?.record
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,6 +28,27 @@ describe('createCatchRecordWriteQueue()', () => {
|
|||||||
vi.unstubAllGlobals();
|
vi.unstubAllGlobals();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('merges partial notes/status patches without inventing omitted fields', async () => {
|
||||||
|
const fetchFn = vi.fn(async () => new Response('[]'));
|
||||||
|
const queue = createCatchRecordWriteQueue({ endpointUrl: '/api/catches', fetchFn });
|
||||||
|
const identity = { userId: 'u', pokedexId: 'd', pokemonId: '1' };
|
||||||
|
queue.enqueue({ ...identity, personalNotes: 'Keep' }, { flushSoon: false });
|
||||||
|
queue.enqueue({ ...identity, inHome: true }, { flushSoon: false });
|
||||||
|
expect(queue.getPendingPatch('1')).toEqual({
|
||||||
|
...identity,
|
||||||
|
personalNotes: 'Keep',
|
||||||
|
inHome: true
|
||||||
|
});
|
||||||
|
await queue.flushNow();
|
||||||
|
const payload = JSON.parse(
|
||||||
|
String((fetchFn.mock.calls[0] as unknown as [string, RequestInit])[1].body)
|
||||||
|
);
|
||||||
|
expect(payload).toEqual([{ ...identity, personalNotes: 'Keep', inHome: true }]);
|
||||||
|
queue.enqueue({ ...identity, personalNotes: '' }, { flushSoon: false });
|
||||||
|
expect(queue.getPendingPatch('1')?.personalNotes).toBe('');
|
||||||
|
await queue.flushNow();
|
||||||
|
});
|
||||||
|
|
||||||
it('coalesces multiple updates for the same key and flushes only the latest state', async () => {
|
it('coalesces multiple updates for the same key and flushes only the latest state', async () => {
|
||||||
const fetchFn = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
|
const fetchFn = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
|
||||||
return new Response(init?.body as string, { status: 200 });
|
return new Response(init?.body as string, { status: 200 });
|
||||||
@@ -141,6 +162,21 @@ describe('createCatchRecordWriteQueue()', () => {
|
|||||||
expect(queue.getPendingCount()).toBe(1);
|
expect(queue.getPendingCount()).toBe(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('discards queued edits after the owning account changes', async () => {
|
||||||
|
const fetchFn = vi.fn<typeof fetch>();
|
||||||
|
let current = true;
|
||||||
|
const queue = createCatchRecordWriteQueue({
|
||||||
|
endpointUrl: '/catch-records',
|
||||||
|
fetchFn,
|
||||||
|
isCurrentUser: () => current
|
||||||
|
});
|
||||||
|
queue.enqueue(mkRecord(), { debounceMs: 100, flushSoon: false });
|
||||||
|
current = false;
|
||||||
|
await queue.flushNow();
|
||||||
|
expect(fetchFn).not.toHaveBeenCalled();
|
||||||
|
expect(queue.getPendingCount()).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
it('does not discard a newer version enqueued during an in-flight request', async () => {
|
it('does not discard a newer version enqueued during an in-flight request', async () => {
|
||||||
let resolveFirst: ((response: Response) => void) | undefined;
|
let resolveFirst: ((response: Response) => void) | undefined;
|
||||||
const firstResponse = new Promise<Response>((resolve) => (resolveFirst = resolve));
|
const firstResponse = new Promise<Response>((resolve) => (resolveFirst = resolve));
|
||||||
|
|||||||
Reference in New Issue
Block a user