mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-15 05:36:17 -08:00
fix(server-storage): refuse user state for deleted playlists
PUT /v1/user-state/:namespace/:playlistId answers 404 playlist_missing when the server has no such playlist, so an edit queued on another device (including its conflict replay with If-None-Match: *) cannot recreate the state of a playlist deleted meanwhile. The client drops such an edit and its cache entry instead of retrying; page-hide writes report it as gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
716580cb16
commit
f956758cce
8 files changed
+114
-8
No files matched your search
@@ -205,6 +205,25 @@ describe('server storage routes', () => {
|
||||
})
|
||||
));
|
||||
|
||||
it('refuses user state for a playlist it does not have', () =>
|
||||
withTempStorage((storage) =>
|
||||
withServer(app(storage), async (baseUrl) => {
|
||||
const entry = `${baseUrl}/v1/user-state/xtream-favorites/gone`;
|
||||
const put = await fetch(
|
||||
entry,
|
||||
json({ value: ['live:1'] }, { method: 'PUT' })
|
||||
);
|
||||
expect(put.status).toBe(404);
|
||||
await expect(put.json()).resolves.toMatchObject({
|
||||
code: 'playlist_missing',
|
||||
});
|
||||
// Removing what is not there stays harmless.
|
||||
expect((await fetch(entry, { method: 'DELETE' })).status).toBe(
|
||||
200
|
||||
);
|
||||
})
|
||||
));
|
||||
|
||||
it('refuses a playlist write built on a replaced version', () =>
|
||||
withTempStorage((storage) =>
|
||||
withServer(app(storage), async (baseUrl) => {
|
||||
@@ -235,6 +254,10 @@ describe('server storage routes', () => {
|
||||
it('writes user state only on the tag it was read at', () =>
|
||||
withTempStorage((storage) =>
|
||||
withServer(app(storage), async (baseUrl) => {
|
||||
await fetch(
|
||||
`${baseUrl}/v1/playlists/p1`,
|
||||
json(m3uDocument('p1'), { method: 'PUT' })
|
||||
);
|
||||
const entry = `${baseUrl}/v1/user-state/xtream-favorites/p1`;
|
||||
// Created only while absent.
|
||||
const created = await fetch(
|
||||
|
||||
@@ -25,9 +25,20 @@ export function createUserStateRouter(storage: ServerStorage): Router {
|
||||
});
|
||||
|
||||
const write = (req: Request, res: Response, value: unknown) => {
|
||||
const namespace = requireNamespace(String(req.params['namespace']));
|
||||
const playlistId = String(req.params['playlistId']);
|
||||
// An edit another device queued before the playlist was deleted
|
||||
// must not bring its user state back.
|
||||
if (value !== undefined && !storage.playlists.exists(playlistId)) {
|
||||
throw new StorageRequestError(
|
||||
404,
|
||||
'playlist_missing',
|
||||
'The playlist does not exist.'
|
||||
);
|
||||
}
|
||||
const result = storage.userState.writeIf(
|
||||
requireNamespace(String(req.params['namespace'])),
|
||||
String(req.params['playlistId']),
|
||||
namespace,
|
||||
playlistId,
|
||||
value,
|
||||
expectedTag(req)
|
||||
);
|
||||
|
||||
@@ -92,7 +92,9 @@ Server storage adds versioned routes under `/v1`, which the PWA uses in
|
||||
positions, one JSON value per namespace and playlist; `GET` adds a
|
||||
content tag per entry (`tags`), and a write with `If-Match: "<tag>"` or
|
||||
`If-None-Match: *` answers `412` with the current value and tag when the
|
||||
entry changed since
|
||||
entry changed since, and a `PUT` for a playlist the server does not have
|
||||
answers `404 playlist_missing` (the client drops that edit), so an edit
|
||||
queued on another device cannot bring a deleted playlist's state back
|
||||
- `GET|PUT /v1/app-state/:key`
|
||||
|
||||
Server storage lives in `apps/web-backend/src/app/storage/`. With
|
||||
|
||||
@@ -133,7 +133,7 @@ All JSON. Errors: `{ message, status, code? }`.
|
||||
| `DELETE /v1/playlists` | Delete all playlists and all user state. |
|
||||
| `GET/PUT /v1/app-state/:key` | `{ value }`. |
|
||||
| `GET /v1/user-state` | `{ entries: { [namespace]: { [playlistId]: value } }, tags }`, a content tag per entry. |
|
||||
| `PUT/DELETE /v1/user-state/:namespace/:playlistId` | One entry. Namespaces are an allow-list. `If-Match: "<tag>"` or `If-None-Match: *` makes it conditional: `412` with the current value and tag. |
|
||||
| `PUT/DELETE /v1/user-state/:namespace/:playlistId` | One entry. Namespaces are an allow-list. `If-Match: "<tag>"` or `If-None-Match: *` makes it conditional: `412` with the current value and tag. A `PUT` for a playlist the server does not have answers `404 playlist_missing`. |
|
||||
|
||||
ETags are `"<bootId>-<revision>"` from an in-memory per-playlist revision
|
||||
counter; the server is the only writer of its database.
|
||||
|
||||
@@ -107,6 +107,15 @@ describe('ServerApiClient', () => {
|
||||
current: { value: [2], tag: 't3' },
|
||||
});
|
||||
|
||||
const gone = jest
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
response(404, { code: 'playlist_missing', message: 'gone' })
|
||||
);
|
||||
await expect(
|
||||
client(gone).sendKeepalive('PUT', '/user-state/a/b', { value: 1 })
|
||||
).resolves.toEqual({ status: 'gone' });
|
||||
|
||||
const refused = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new TypeError('body too large'));
|
||||
|
||||
@@ -12,6 +12,9 @@ import type { ServerStorageStatusService } from '../server-storage-status.servic
|
||||
|
||||
const REQUEST_TIMEOUT_MS = 20_000;
|
||||
|
||||
/** A user-state write for a playlist the server no longer has. */
|
||||
export const PLAYLIST_MISSING = 'playlist_missing';
|
||||
|
||||
/** nginx answers these while the backend behind it is down. */
|
||||
const GATEWAY_UNAVAILABLE = new Set([502, 503, 504]);
|
||||
|
||||
@@ -32,6 +35,7 @@ export class ServerApiError extends Error {
|
||||
export type KeepaliveOutcome =
|
||||
| { readonly status: 'saved'; readonly tag?: string | null }
|
||||
| { readonly status: 'conflict'; readonly current: ServerUserStateCurrent }
|
||||
| { readonly status: 'gone' }
|
||||
| { readonly status: 'failed' };
|
||||
|
||||
export interface ServerApiClientOptions {
|
||||
@@ -204,8 +208,13 @@ export class ServerApiClient {
|
||||
)) as Partial<ServerUserStateWriteResponse>;
|
||||
return { status: 'saved', tag: body?.tag };
|
||||
}
|
||||
const { current } =
|
||||
response.status === 412 ? await readErrorBody(response) : {};
|
||||
const { code, current } =
|
||||
response.status === 412 || response.status === 404
|
||||
? await readErrorBody(response)
|
||||
: {};
|
||||
if (code === PLAYLIST_MISSING) {
|
||||
return { status: 'gone' };
|
||||
}
|
||||
return current
|
||||
? { status: 'conflict', current }
|
||||
: { status: 'failed' };
|
||||
|
||||
@@ -3,7 +3,11 @@ import type {
|
||||
ServerUserStateNamespace,
|
||||
ServerUserStateWriteResponse,
|
||||
} from '@iptvnator/shared/interfaces';
|
||||
import { ServerApiError, type ServerApiClient } from './server-api-client';
|
||||
import {
|
||||
PLAYLIST_MISSING,
|
||||
ServerApiError,
|
||||
type ServerApiClient,
|
||||
} from './server-api-client';
|
||||
import {
|
||||
entryKey,
|
||||
entryPath,
|
||||
@@ -353,6 +357,8 @@ export class UserDataWriteQueue {
|
||||
}
|
||||
} else if (outcome.status === 'conflict') {
|
||||
this.replayOnConflict(entry, outcome.current);
|
||||
} else if (outcome.status === 'gone') {
|
||||
this.gone(entry);
|
||||
} else {
|
||||
this.requeue(entry);
|
||||
}
|
||||
@@ -389,6 +395,11 @@ export class UserDataWriteQueue {
|
||||
error.current
|
||||
) {
|
||||
this.replayOnConflict(entry, error.current);
|
||||
} else if (
|
||||
error instanceof ServerApiError &&
|
||||
error.code === PLAYLIST_MISSING
|
||||
) {
|
||||
this.gone(entry);
|
||||
} else {
|
||||
this.requeue(entry);
|
||||
this.hooks.writeFailed(error);
|
||||
@@ -397,6 +408,15 @@ export class UserDataWriteQueue {
|
||||
this.hooks.changed();
|
||||
}
|
||||
|
||||
/** The playlist was deleted elsewhere: its edit goes with it. */
|
||||
private gone(entry: PendingEntry): void {
|
||||
const key = entryKey(entry);
|
||||
if (this.edits.get(key) === entry.edit) {
|
||||
this.edits.delete(key);
|
||||
this.hooks.show(entry.namespace, entry.playlistId, undefined);
|
||||
}
|
||||
}
|
||||
|
||||
/** The server took `entry`: the edit queued behind it builds on it. */
|
||||
private confirmed(entry: PendingEntry, tag: string | null): void {
|
||||
this.tags.update(entry.namespace, entry.playlistId, tag);
|
||||
|
||||
@@ -21,6 +21,8 @@ function fakeServer(initial: Record<string, Record<string, unknown>> = {}) {
|
||||
'xtream-playback-positions': {},
|
||||
...structuredClone(initial),
|
||||
};
|
||||
/** Playlists deleted on the server: their user state is refused. */
|
||||
const deleted = new Set<string>();
|
||||
const tagOf = (value: unknown) =>
|
||||
value === undefined
|
||||
? null
|
||||
@@ -33,6 +35,9 @@ function fakeServer(initial: Record<string, Record<string, unknown>> = {}) {
|
||||
value?: unknown
|
||||
) => {
|
||||
const [, , namespace, playlistId] = path.split('/');
|
||||
if (value !== undefined && deleted.has(playlistId)) {
|
||||
throw new ServerApiError(404, 'playlist_missing', 'gone');
|
||||
}
|
||||
const current = state[namespace][playlistId];
|
||||
const expected =
|
||||
headers['If-None-Match'] === '*'
|
||||
@@ -89,6 +94,12 @@ function fakeServer(initial: Record<string, Record<string, unknown>> = {}) {
|
||||
const { tag } = await response.json();
|
||||
return { status: 'saved' as const, tag };
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof ServerApiError &&
|
||||
error.code === 'playlist_missing'
|
||||
) {
|
||||
return { status: 'gone' as const };
|
||||
}
|
||||
return error instanceof ServerApiError && error.current
|
||||
? {
|
||||
status: 'conflict' as const,
|
||||
@@ -99,7 +110,7 @@ function fakeServer(initial: Record<string, Record<string, unknown>> = {}) {
|
||||
}
|
||||
),
|
||||
};
|
||||
return { state, api };
|
||||
return { state, api, deleted };
|
||||
}
|
||||
|
||||
const favorites = (storage: ServerUserDataStorage) =>
|
||||
@@ -935,4 +946,25 @@ describe('ServerUserDataStorage with other devices', () => {
|
||||
|
||||
expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'Z']);
|
||||
});
|
||||
|
||||
it('drops an edit of a playlist another device deleted', async () => {
|
||||
const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } });
|
||||
const phone = new ServerUserDataStorage(
|
||||
server.api as never,
|
||||
memoryStorage(),
|
||||
jest.fn()
|
||||
);
|
||||
await phone.hydrate();
|
||||
phone.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] }));
|
||||
// Another device deletes the playlist before this edit is sent.
|
||||
server.deleted.add('p1');
|
||||
delete server.state['xtream-favorites']['p1'];
|
||||
|
||||
await phone.flush();
|
||||
await phone.flush();
|
||||
|
||||
expect(server.state['xtream-favorites']).toEqual({});
|
||||
expect(phone.hasUnsentWrites()).toBe(false);
|
||||
expect(favorites(phone)).toEqual({});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user