diff --git a/apps/web-backend/src/app/storage/playlists-router.ts b/apps/web-backend/src/app/storage/playlists-router.ts index d755fe4f2..ebf85b955 100644 --- a/apps/web-backend/src/app/storage/playlists-router.ts +++ b/apps/web-backend/src/app/storage/playlists-router.ts @@ -64,6 +64,15 @@ export function createPlaylistsRouter( 'The playlist changed since it was read.' ); } + // A create-only upload (a move from browser storage) that another + // tab or device made first. Checked and written in one handler. + if (req.get('if-none-match')?.trim() === '*' && playlists.exists(id)) { + throw new StorageRequestError( + 412, + 'conflict', + 'A playlist with this id exists already.' + ); + } playlists.upsert( { ...requireDocument(req.body), _id: id }, { keepPayload: req.query['keepPayload'] === '1' } diff --git a/apps/web-backend/src/app/storage/storage-routes.spec.ts b/apps/web-backend/src/app/storage/storage-routes.spec.ts index 9b915b154..5059c680e 100644 --- a/apps/web-backend/src/app/storage/storage-routes.spec.ts +++ b/apps/web-backend/src/app/storage/storage-routes.spec.ts @@ -251,6 +251,35 @@ describe('server storage routes', () => { }) )); + it('creates a playlist create-only, refused once it exists', () => + withTempStorage((storage) => + withServer(app(storage), async (baseUrl) => { + const create = (title: string) => + fetch( + `${baseUrl}/v1/playlists/p1`, + json( + { ...m3uDocument('p1'), title }, + { + method: 'PUT', + headers: { 'if-none-match': '*' }, + } + ) + ); + + expect((await create('First')).status).toBe(200); + // Another tab or device moved the same playlist first. + const second = await create('Second'); + expect(second.status).toBe(412); + await expect(second.json()).resolves.toMatchObject({ + code: 'conflict', + }); + const stored = await fetch(`${baseUrl}/v1/playlists/p1`); + await expect(stored.json()).resolves.toMatchObject({ + title: 'First', + }); + }) + )); + it('writes user state only on the tag it was read at', () => withTempStorage((storage) => withServer(app(storage), async (baseUrl) => { diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 9488624c8..c3e7fab5b 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -86,7 +86,8 @@ Server storage adds versioned routes under `/v1`, which the PWA uses in `header` fields (`409 payload_missing` without a stored row); every `PUT` answers with the new `ETag` and `X-Replaced-ETag`, the version it replaced, and one with `If-Match` answers `412 conflict` when another - write replaced that version + write replaced that version, one with `If-None-Match: *` when the + playlist exists already - `GET /v1/user-state`, `PUT|DELETE /v1/user-state/:namespace/:playlistId`: Xtream favorites, recently viewed, collection snapshots and playback positions, one JSON value per namespace and playlist; `GET` adds a @@ -363,7 +364,8 @@ settings and guards read them in every build. a conditional mutation, a conflict retried at the next start). A playlist the server already has, by id or by source (Xtream server and user, Stalker portal and MAC, M3U URL), merges into the server's copy; the - others upload with their ids. A merged playlist gains the browser + others upload with their ids, create-only (`If-None-Match: *`): one that + another tab or device moved first is merged into instead of overwritten. A merged playlist gains the browser document's favorites and history (M3U favorites name channels by stream URL, as the app looks them up; older ones by a channel id that is random per import, which becomes its URL on both sides; history entries keep diff --git a/libs/services/src/lib/server-storage/server-migration.service.spec.ts b/libs/services/src/lib/server-storage/server-migration.service.spec.ts index 472b3d952..3bec5bcff 100644 --- a/libs/services/src/lib/server-storage/server-migration.service.spec.ts +++ b/libs/services/src/lib/server-storage/server-migration.service.spec.ts @@ -22,6 +22,7 @@ describe('ServerMigrationService', () => { dbGetAppPlaylist: jest.Mock; dbGetAppPlaylistMetas: jest.Mock; dbUpsertAppPlaylist: jest.Mock; + dbCreateAppPlaylist: jest.Mock; }; api: { getJson: jest.Mock; sendJson: jest.Mock }; userData: { @@ -52,6 +53,7 @@ describe('ServerMigrationService', () => { dbUpsertAppPlaylist: jest .fn() .mockResolvedValue({ success: true }), + dbCreateAppPlaylist: jest.fn().mockResolvedValue(true), }, api: { getJson: jest.fn().mockResolvedValue({ entries: {} }), @@ -117,7 +119,7 @@ describe('ServerMigrationService', () => { const result = await migration.moveBrowserData(); expect(result).toEqual({ imported: 2, merged: 0, failed: 0 }); - expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledTimes( + expect(session.documentStore.dbCreateAppPlaylist).toHaveBeenCalledTimes( 2 ); expect(session.api.sendJson).toHaveBeenCalledWith( @@ -353,10 +355,38 @@ describe('ServerMigrationService', () => { }); }); + it('merges into a copy another tab or device uploaded first', async () => { + TestBed.overrideProvider(NgxIndexedDBService, { + useValue: { + getAll: () => + of([{ ...browserPlaylists[0], favorites: ['http://s/2'] }]), + }, + }); + // The id was free when planned, taken when uploaded. + session.documentStore.dbCreateAppPlaylist.mockResolvedValue(false); + const first = { + ...browserPlaylists[0], + favorites: ['http://s/1'], + recentlyViewed: [], + }; + session.documentStore.dbGetAppPlaylist.mockResolvedValue(first); + + const result = await TestBed.inject( + ServerMigrationService + ).moveBrowserData(); + + expect(result).toEqual({ imported: 0, merged: 1, failed: 0 }); + // Never overwritten: this browser's favorites join the first copy's. + expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledWith({ + ...first, + favorites: ['http://s/1', 'http://s/2'], + }); + }); + it('keeps the move on offer when a playlist could not be uploaded', async () => { - session.documentStore.dbUpsertAppPlaylist + session.documentStore.dbCreateAppPlaylist .mockRejectedValueOnce(new Error('413')) - .mockResolvedValue({ success: true }); + .mockResolvedValue(true); const migration = TestBed.inject(ServerMigrationService); const result = await migration.moveBrowserData(); @@ -382,7 +412,7 @@ describe('ServerMigrationService', () => { await expect(migration.moveBrowserData()).rejects.toThrow('Unsent'); expect( - session.documentStore.dbUpsertAppPlaylist + session.documentStore.dbCreateAppPlaylist ).not.toHaveBeenCalled(); expect(session.api.sendJson).not.toHaveBeenCalled(); expect(migration.browserDataMoved()).toBe(false); @@ -398,7 +428,7 @@ describe('ServerMigrationService', () => { ]), }, }); - session.documentStore.dbUpsertAppPlaylist.mockRejectedValue( + session.documentStore.dbCreateAppPlaylist.mockRejectedValue( new Error('500') ); localStorage.setItem( @@ -501,7 +531,7 @@ describe('ServerMigrationService', () => { await TestBed.inject(ServerMigrationService).moveBrowserData(); - expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledWith( + expect(session.documentStore.dbCreateAppPlaylist).toHaveBeenCalledWith( expect.objectContaining({ _id: 's1', isFullStalkerPortal: true }) ); }); diff --git a/libs/services/src/lib/server-storage/server-migration.service.ts b/libs/services/src/lib/server-storage/server-migration.service.ts index f82d16383..e6e25438f 100644 --- a/libs/services/src/lib/server-storage/server-migration.service.ts +++ b/libs/services/src/lib/server-storage/server-migration.service.ts @@ -130,10 +130,21 @@ export class ServerMigrationService { merged += 1; targets.set(step.browserId, step.serverId); } else { - await this.session.documentStore.dbUpsertAppPlaylist( - step.playlist - ); - imported += 1; + // Create-only: another tab or device that moved the + // same playlist first keeps its copy, merged into. + const created = + await this.session.documentStore.dbCreateAppPlaylist( + step.playlist + ); + if (created) { + imported += 1; + } else { + await this.mergeDocumentUserData( + step.playlist, + step.playlist._id + ); + merged += 1; + } onServer.add(step.playlist._id); targets.set(step.playlist._id, step.playlist._id); } diff --git a/libs/services/src/lib/server-storage/server-playlist-document-store.spec.ts b/libs/services/src/lib/server-storage/server-playlist-document-store.spec.ts index ae5188c43..3a71fe0d7 100644 --- a/libs/services/src/lib/server-storage/server-playlist-document-store.spec.ts +++ b/libs/services/src/lib/server-storage/server-playlist-document-store.spec.ts @@ -170,6 +170,19 @@ describe('ServerPlaylistDocumentStore', () => { expect(api.sendJson.mock.calls[1][3]).toEqual({}); }); + it('creates a playlist only while the server has none of that id', async () => { + const { api, store } = setup(); + api.sendJson.mockResolvedValueOnce(putResponse('"b.0.1"', '')); + + await expect(store.dbCreateAppPlaylist(doc)).resolves.toBe(true); + expect(api.sendJson.mock.calls[0][3]).toEqual({ 'If-None-Match': '*' }); + + api.sendJson.mockRejectedValueOnce( + new ServerApiError(412, 'conflict', 'exists') + ); + await expect(store.dbCreateAppPlaylist(doc)).resolves.toBe(false); + }); + it('writes a portal document on the version it read', async () => { const { api, store } = setup(); const portal = { _id: 'p2', title: 'Portal', macAddress: 'm' }; diff --git a/libs/services/src/lib/server-storage/server-playlist-document-store.ts b/libs/services/src/lib/server-storage/server-playlist-document-store.ts index 7e41995b4..4e9bb7228 100644 --- a/libs/services/src/lib/server-storage/server-playlist-document-store.ts +++ b/libs/services/src/lib/server-storage/server-playlist-document-store.ts @@ -191,6 +191,29 @@ export class ServerPlaylistDocumentStore implements PlaylistDocumentStore { } } + /** + * A create-only upload (the move from browser storage): false when the + * server has a playlist with that id already, uploaded meanwhile by + * another tab or device, which the caller merges into instead. + */ + async dbCreateAppPlaylist(playlist: Playlist): Promise { + try { + const response = await this.api.sendJson( + 'PUT', + `/playlists/${encodeURIComponent(playlist._id)}`, + playlist, + { 'If-None-Match': '*' } + ); + this.remember(playlist._id, response, playlist); + return true; + } catch (error) { + if (error instanceof ServerApiError && error.status === 412) { + return false; + } + throw error; + } + } + /** `null` when the server has no payload to keep (write it in full). */ private async keepPayload( playlist: Playlist,