fix(backup): don't let a stale export pause a reconnected backup

- An export now pauses a revoked integration only if its row is
  unchanged since the export read it, using the trigger-maintained
  updatedAt column as the row version. If the user reconnected in the
  meantime, the stale failure no longer disables the fresh credentials
  or asks the user to reconnect again. updateExportStatus now reports
  whether a row was written.
- The backup status store ignores a response overtaken by a newer
  refresh, or by the status being flagged, cleared or set directly, so
  a slow response can't overwrite newer state.
- Unit tests cover the whole integration repository, the guarded pause
  and stale status responses. Coverage thresholds are raised to the
  new baseline.
This commit is contained in:
Josh Creek
2026-09-14 19:03:29 +01:00
parent fb95b43b30
commit 7786d46078
8 changed files with 398 additions and 26 deletions
+58
View File
@@ -104,6 +104,64 @@ describe('refreshBackupStatus', () => {
expect(get(backupsNeedingReconnect)).toEqual(['google_drive']);
});
function deferredResponse() {
let resolve!: (response: Response) => void;
const promise = new Promise<Response>((r) => (resolve = r));
return { promise, resolve };
}
const json = (body: unknown) => new Response(JSON.stringify(body));
it('ignores a response overtaken by a newer refresh', async () => {
const slow = deferredResponse();
fetchMock
.mockReturnValueOnce(slow.promise)
.mockResolvedValueOnce(json([{ provider: 'google_drive', enabled: true }]));
const first = refreshBackupStatus();
await refreshBackupStatus();
slow.resolve(json([{ provider: 'google_drive', enabled: false }]));
await first;
expect(get(backupsNeedingReconnect)).toEqual([]);
});
it('ignores a response that arrives after the status was flagged directly', async () => {
const slow = deferredResponse();
fetchMock.mockReturnValueOnce(slow.promise);
const pending = refreshBackupStatus();
markReconnectNeeded(['dropbox']);
slow.resolve(json([{ provider: 'dropbox', enabled: true }]));
await pending;
expect(get(backupsNeedingReconnect)).toEqual(['dropbox']);
});
it('ignores a response that arrives after the status was cleared', async () => {
const slow = deferredResponse();
fetchMock.mockReturnValueOnce(slow.promise);
const pending = refreshBackupStatus();
clearBackupStatus();
slow.resolve(json([{ provider: 'google_drive', enabled: false }]));
await pending;
expect(get(backupsNeedingReconnect)).toEqual([]);
});
it('ignores a response that arrives after the status was set from another source', async () => {
const slow = deferredResponse();
fetchMock.mockReturnValueOnce(slow.promise);
const pending = refreshBackupStatus();
setBackupStatus([{ provider: 'google_drive', enabled: true }]);
slow.resolve(json([{ provider: 'google_drive', enabled: false }]));
await pending;
expect(get(backupsNeedingReconnect)).toEqual([]);
});
it('keeps the last known status when the request fails', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
markReconnectNeeded(['dropbox']);
@@ -0,0 +1,257 @@
import { describe, expect, it, vi } from 'vitest';
import type { SupabaseClient } from '@supabase/supabase-js';
import PokedexExportIntegrationRepository from '$lib/repositories/PokedexExportIntegrationRepository';
type Result = { data: unknown; error: unknown };
/** A chainable stand-in for the Supabase query builder that records each call made on it. */
function fakeSupabase(result: Result) {
const calls: unknown[][] = [];
const builder: Record<string, unknown> = {};
for (const method of ['select', 'update', 'upsert', 'single', 'eq', 'is', 'or']) {
builder[method] = (...args: unknown[]) => {
calls.push([method, ...args]);
return builder;
};
}
// Awaiting the builder runs the query, as it does in supabase-js.
builder.then = (resolve: (value: Result) => unknown, reject?: (reason: unknown) => unknown) =>
Promise.resolve(result).then(resolve, reject);
const from = vi.fn(() => builder);
return { supabase: { from } as unknown as SupabaseClient, calls, from };
}
const VERSION = '2026-09-14T12:00:00.123456+00:00';
const WRITTEN: Result = { data: [{ id: 'int-1' }], error: null };
describe('PokedexExportIntegrationRepository.updateExportStatus', () => {
it('writes only while the row still has the version the caller read', async () => {
const { supabase, calls, from } = fakeSupabase(WRITTEN);
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
const applied = await repo.updateExportStatus('int-1', { enabled: false }, VERSION);
expect(applied).toBe(true);
expect(from).toHaveBeenCalledWith('pokedex_export_integrations');
expect(calls).toEqual([
['update', { enabled: false }],
['eq', 'id', 'int-1'],
['eq', 'userId', 'user-1'],
['eq', 'updatedAt', VERSION],
['is', 'pokedexId', null],
['select', 'id']
]);
});
it('writes unconditionally without a version, scoped to the Pokédex', async () => {
const { supabase, calls } = fakeSupabase(WRITTEN);
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', 'dex-1');
expect(await repo.updateExportStatus('int-1', { lastError: null })).toBe(true);
expect(calls).toEqual([
['update', { lastError: null }],
['eq', 'id', 'int-1'],
['eq', 'userId', 'user-1'],
['eq', 'pokedexId', 'dex-1'],
['select', 'id']
]);
});
it.each([
['no row matched, e.g. after a reconnect changed it', { data: [], error: null }],
['the response carries no rows', { data: null, error: null }]
])('reports nothing written when %s', async (_label, result) => {
const { supabase } = fakeSupabase(result);
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
expect(await repo.updateExportStatus('int-1', { enabled: false }, VERSION)).toBe(false);
});
it('logs and reports nothing written when the update fails', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const { supabase } = fakeSupabase({ data: null, error: { message: 'boom' } });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
expect(await repo.updateExportStatus('int-1', { enabled: false })).toBe(false);
expect(consoleError).toHaveBeenCalled();
consoleError.mockRestore();
});
});
describe('PokedexExportIntegrationRepository.listAll', () => {
it('maps each row version, defaulting a missing one to null', async () => {
const row = {
userId: 'user-1',
pokedexId: null,
provider: 'google_drive',
enabled: true,
fileName: null,
folderId: null,
path: null,
accessToken: 'access',
refreshToken: 'refresh',
accessTokenExpiresAt: null,
metadata: null,
lastExportedAt: null,
lastError: null
};
const { supabase } = fakeSupabase({
data: [
{ ...row, id: 'with-version', updatedAt: VERSION },
{ ...row, id: 'without-version' }
],
error: null
});
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
const [withVersion, withoutVersion] = await repo.listAll();
expect(withVersion).toMatchObject({ _id: 'with-version', updatedAt: VERSION });
expect(withoutVersion).toMatchObject({ _id: 'without-version', updatedAt: null });
});
});
function dbRow(id: string) {
return {
id,
userId: 'user-1',
pokedexId: null,
provider: 'dropbox',
enabled: true,
fileName: null,
folderId: null,
path: '/Backups',
accessToken: 'access',
refreshToken: 'refresh',
accessTokenExpiresAt: null,
metadata: null,
lastExportedAt: null,
lastError: null,
updatedAt: VERSION
};
}
describe('PokedexExportIntegrationRepository queries', () => {
it('lists enabled integrations scoped to one Pokédex', async () => {
const { supabase, calls } = fakeSupabase({ data: [dbRow('int-1')], error: null });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', 'dex-1');
const [integration] = await repo.listEnabled();
expect(integration).toMatchObject({ _id: 'int-1', provider: 'dropbox', path: '/Backups' });
expect(calls).toEqual([
['select', '*'],
['eq', 'userId', 'user-1'],
['eq', 'pokedexId', 'dex-1'],
['eq', 'enabled', true]
]);
});
it.each([
['dex-1', ['or', 'pokedexId.eq.dex-1,pokedexId.is.null']],
[null, ['is', 'pokedexId', null]]
])('lists enabled integrations for Pokédex %j or the whole account', async (pokedexId, scope) => {
const { supabase, calls } = fakeSupabase({ data: [dbRow('int-1')], error: null });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', pokedexId);
expect(await repo.listEnabledForPokedexOrUser()).toHaveLength(1);
expect(calls).toEqual([
['select', '*'],
['eq', 'userId', 'user-1'],
['eq', 'enabled', true],
scope
]);
});
it.each(['listAll', 'listEnabled', 'listEnabledForPokedexOrUser'] as const)(
'%s returns nothing when no rows come back',
async (method) => {
const { supabase } = fakeSupabase({ data: null, error: null });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
expect(await repo[method]()).toEqual([]);
}
);
it.each(['listAll', 'listEnabled', 'listEnabledForPokedexOrUser'] as const)(
'%s throws when the query fails',
async (method) => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const { supabase } = fakeSupabase({ data: null, error: { message: 'boom' } });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
await expect(repo[method]()).rejects.toThrow('Failed to load export integrations: boom');
consoleError.mockRestore();
}
);
});
describe('PokedexExportIntegrationRepository.upsert', () => {
it('saves one account-wide integration per provider', async () => {
const { supabase, calls } = fakeSupabase({ data: dbRow('int-1'), error: null });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', 'dex-1');
const saved = await repo.upsert({ provider: 'dropbox', enabled: true, lastError: null });
expect(saved).toMatchObject({ _id: 'int-1', updatedAt: VERSION });
expect(calls).toEqual([
[
'upsert',
{
userId: 'user-1',
pokedexId: null,
provider: 'dropbox',
enabled: true,
lastError: null
},
{ onConflict: 'userId,provider' }
],
['select'],
['single']
]);
});
it.each([
['the save fails', { data: null, error: { message: 'boom' } }, 'boom'],
['nothing comes back', { data: null, error: null }, 'No result returned']
])('throws when %s', async (_label, result, message) => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const { supabase } = fakeSupabase(result);
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
await expect(repo.upsert({ provider: 'dropbox' })).rejects.toThrow(
`Failed to save export integration: ${message}`
);
consoleError.mockRestore();
});
});
describe('PokedexExportIntegrationRepository.updateTokens', () => {
it.each([
['dex-1', ['eq', 'pokedexId', 'dex-1']],
[null, ['is', 'pokedexId', null]]
])('stores refreshed tokens scoped to Pokédex %j', async (pokedexId, scope) => {
const { supabase, calls } = fakeSupabase({ data: null, error: null });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', pokedexId);
const patch = { accessToken: 'new-access', accessTokenExpiresAt: null };
await repo.updateTokens('int-1', patch);
expect(calls).toEqual([
['update', patch],
['eq', 'id', 'int-1'],
['eq', 'userId', 'user-1'],
scope
]);
});
it('logs instead of throwing when the token update fails', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const { supabase } = fakeSupabase({ data: null, error: { message: 'boom' } });
const repo = new PokedexExportIntegrationRepository(supabase, 'user-1', null);
await expect(repo.updateTokens('int-1', { accessToken: 'x' })).resolves.toBeUndefined();
expect(consoleError).toHaveBeenCalled();
consoleError.mockRestore();
});
});
+33 -8
View File
@@ -35,6 +35,8 @@ import { exportPokedexIfConfigured } from '$lib/services/PokedexExportService';
const supabase = {} as SupabaseClient;
const EXPIRED = () => new Date(Date.now() - 60_000).toISOString();
const FRESH = () => new Date(Date.now() + 3_600_000).toISOString();
/** The row version an export read; a guarded write only applies while it still matches. */
const VERSION = '2026-09-14T12:00:00.123456+00:00';
function integration(overrides: Partial<PokedexExportIntegration> = {}): PokedexExportIntegration {
return {
@@ -53,6 +55,7 @@ function integration(overrides: Partial<PokedexExportIntegration> = {}): Pokedex
metadata: null,
lastExportedAt: null,
lastError: null,
updatedAt: VERSION,
...overrides
};
}
@@ -105,6 +108,8 @@ beforeEach(() => {
mocks.pokedex = DEX;
mocks.integrations = [];
mocks.updateExportStatus.mockReset();
// The repository reports whether a row was written; by default every write applies.
mocks.updateExportStatus.mockResolvedValue(true);
mocks.updateTokens.mockReset();
fetchMock.mockReset();
vi.stubGlobal('fetch', fetchMock);
@@ -138,10 +143,11 @@ describe('exportPokedexIfConfigured when a provider revokes access', () => {
reconnectRequired: true
}
]);
expect(mocks.updateExportStatus).toHaveBeenCalledWith(id, {
lastError: expect.stringMatching(message),
enabled: false
});
expect(mocks.updateExportStatus).toHaveBeenCalledWith(
id,
{ lastError: expect.stringMatching(message), enabled: false },
VERSION
);
expect(uploadCalls()).toHaveLength(0);
expect(mocks.updateTokens).not.toHaveBeenCalled();
});
@@ -152,13 +158,32 @@ describe('exportPokedexIfConfigured when a provider revokes access', () => {
const result = await exportPokedexIfConfigured(supabase, 'user-1', 'dex-1');
expect(result.failed[0]).toMatchObject({ reconnectRequired: true });
expect(mocks.updateExportStatus).toHaveBeenCalledWith('google-1', {
lastError: expect.stringMatching(/Reconnect Google Drive/),
enabled: false
});
expect(mocks.updateExportStatus).toHaveBeenCalledWith(
'google-1',
{ lastError: expect.stringMatching(/Reconnect Google Drive/), enabled: false },
VERSION
);
expect(fetchMock).not.toHaveBeenCalled();
});
it('leaves a backup reconnected during the export enabled and unflagged', async () => {
mocks.integrations = [integration()];
stubProvider(REVOKED);
// The guarded pause matches no row: a reconnect changed it after this export read it.
mocks.updateExportStatus.mockResolvedValue(false);
const result = await exportPokedexIfConfigured(supabase, 'user-1', 'dex-1');
expect(mocks.updateExportStatus).toHaveBeenCalledTimes(1);
expect(mocks.updateExportStatus).toHaveBeenCalledWith(
'google-1',
{ lastError: expect.stringMatching(/Reconnect Google Drive/), enabled: false },
VERSION
);
// The client must not tell the user to reconnect a connection that is already fresh.
expect(result.failed[0]).toMatchObject({ reconnectRequired: false });
});
it.each([
['a server error', { status: 500, body: 'upstream down' }],
['a different OAuth error', { status: 400, body: { error: 'invalid_client' } }]