From c8afa382fe8f85d58986d92c02caf7bd55e78508 Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 14 Oct 2026 00:30:40 +0200 Subject: [PATCH] fix(server-storage): adopted edits keep yielding through conflicts; newest view per item; same-token sign-ins A conflict replay collapsed every step into one edit, so a closed tab's adopted step lost `yields` after the first conflict and a second one replayed its old position over a newer one. Its steps now stay as they are, and only this page's own collapse into one edit on top of them. The move combines recently viewed items one per item, keeping the newest view and its artwork, then newest first; before, the server's older copy of an item won over a later browser view. A failed sign-in clears the stored token only on a 401: a network failure says nothing about the token, and another tab may have stored the same one meanwhile. Co-Authored-By: Claude Opus 5.5 --- .../server-sign-in.component.spec.ts | 14 +++++ .../server-sign-in.component.ts | 14 ++--- docs/architecture/pwa-self-hosted.md | 11 ++-- .../server-storage/server-migration-merge.ts | 17 +++--- .../server-migration.service.spec.ts | 9 ++-- .../server-migration.service.ts | 12 ++--- .../server-user-data-adopt.spec.ts | 53 +++++++++++++++++++ .../server-storage/server-user-data-queue.ts | 8 ++- 8 files changed, 108 insertions(+), 30 deletions(-) diff --git a/apps/web/src/app/server-storage/server-sign-in.component.spec.ts b/apps/web/src/app/server-storage/server-sign-in.component.spec.ts index ee3af0cec..f7f4ed605 100644 --- a/apps/web/src/app/server-storage/server-sign-in.component.spec.ts +++ b/apps/web/src/app/server-storage/server-sign-in.component.spec.ts @@ -98,6 +98,20 @@ describe('ServerSignInComponent', () => { expect(readServerAccessToken('i-1')).toBe('right'); }); + it('leaves a token another tab stored to it after a network failure', async () => { + session.fetchStatus.mockImplementation(async () => { + // Another tab signs in with the same token meanwhile. + writeServerAccessToken('right', 'i-1'); + throw new ServerApiError(0, 'network', 'offline'); + }); + + component.edit('right'); + await component.submit(new Event('submit')); + + expect(component.feedback()).toBe('unreachable'); + expect(readServerAccessToken('i-1')).toBe('right'); + }); + it('tells an unreachable server apart and clears the message on edit', async () => { session.fetchStatus.mockRejectedValue( new ServerApiError(0, undefined, 'offline') diff --git a/apps/web/src/app/server-storage/server-sign-in.component.ts b/apps/web/src/app/server-storage/server-sign-in.component.ts index a90943564..4fbc38baf 100644 --- a/apps/web/src/app/server-storage/server-sign-in.component.ts +++ b/apps/web/src/app/server-storage/server-sign-in.component.ts @@ -83,12 +83,14 @@ export class ServerSignInComponent { await this.session.fetchStatus(); this.openWorkspace(); } catch (error) { - forgetAttempt(token); - this.feedback.set( - error instanceof ServerApiError && error.status === 401 - ? 'rejected' - : 'unreachable' - ); + const rejected = + error instanceof ServerApiError && error.status === 401; + // A refused token is no one's. A network failure says nothing + // about it, and another tab may have stored the same one since. + if (rejected) { + forgetAttempt(token); + } + this.feedback.set(rejected ? 'rejected' : 'unreachable'); } finally { this.busy.set(false); } diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 5e3d4d590..02e31f6e0 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -248,9 +248,9 @@ settings and guards read them in every build. tab's record has gone stale (at its next re-read, or its 30 s check when it has nothing to sync and never re-reads); the result replays the closed tab's edits first and this tab's newer ones last. An adopted edit - changes only what is still as its page saw it (`EditStep.yields`): an - item or key changed since, by this tab or another device, keeps its - newer value. Tabs never clear each other's. + changes only what is still as its page saw it (`EditStep.yields`), also + at every later conflict: an item or key changed since, by this tab or + another device, keeps its newer value. Tabs never clear each other's. Cache and saved writes belong to one server database (`iptvnator-server:cache-owner`); another one behind the same address starts clean. Startup waits for the first read only a few seconds; @@ -357,8 +357,9 @@ settings and guards read them in every build. snapshots first (`typeLegacyXtreamRefs`; snapshots keep their bare key as an alias), then `combineUserData` unions lists (playback positions by content type and Xtream id, renamed to the - server playlist; recently viewed items newest first, since the app cuts - history from the end) and merges maps, the server's value winning on a + server playlist; recently viewed items newest first and one per item, + its newest view, since the app cuts history from the end) and merges + maps, the server's value winning on a conflict. Parental locks stay in the browser but move to the server playlist id with it; while the lock store or a browser user-data map cannot be read the move counts as failed. Browser data is never diff --git a/libs/services/src/lib/server-storage/server-migration-merge.ts b/libs/services/src/lib/server-storage/server-migration-merge.ts index e119ffb59..f48299087 100644 --- a/libs/services/src/lib/server-storage/server-migration-merge.ts +++ b/libs/services/src/lib/server-storage/server-migration-merge.ts @@ -137,14 +137,14 @@ export function retargetUserData( } /** - * Recently viewed items newest first, as the app keeps them: it cuts the - * list at its limit from the end, so a browser view combined after the - * server's older ones would be dropped by the next one. Undated items - * keep their order, after the dated ones. + * Recently viewed items as the app keeps them: newest first, since it cuts + * the list at its limit from the end, and one per item, its newest view + * with that view's artwork. Undated items follow the dated ones in order. */ -export function newestViewsFirst(items: unknown): unknown { - if (!Array.isArray(items)) { - return items; +export function combineRecentViews(values: readonly unknown[]): unknown { + const present = values.filter((value) => value !== undefined); + if (present.length === 0 || !present.every(Array.isArray)) { + return combineUserData(values); } const viewedAt = (item: unknown) => { const at = isPlainObject(item) @@ -152,7 +152,8 @@ export function newestViewsFirst(items: unknown): unknown { : NaN; return Number.isNaN(at) ? 0 : at; }; - return [...items].sort((a, b) => viewedAt(b) - viewedAt(a)); + const views = ([] as unknown[]).concat(...(present as unknown[][])); + return unionItems(views.sort((a, b) => viewedAt(b) - viewedAt(a))); } function isPlainObject(value: unknown): value is Record { 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 3c4ec69a4..14ec2b339 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 @@ -202,18 +202,21 @@ describe('ServerMigrationService', () => { }, }, }); + // vod:8 was watched again in this browser, with other artwork. + const again = { ...view('vod:8', 4), backdropUrl: 'b.jpg' }; localStorage.setItem( 'xtream-recent-items', - JSON.stringify({ x: [view('vod:1', 3)] }) + JSON.stringify({ x: [again, view('vod:1', 3)] }) ); await TestBed.inject(ServerMigrationService).moveBrowserData(); - // The app cuts history from the end: the newest view leads. + // The app cuts history from the end: the newest view leads, and an + // item keeps its newest view. expect(session.api.sendJson).toHaveBeenCalledWith( 'PUT', '/user-state/xtream-recent-items/server-x', - { value: [view('vod:1', 3), view('vod:9', 2), view('vod:8', 1)] }, + { value: [again, view('vod:1', 3), view('vod:9', 2)] }, {} ); }); 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 d505cefb2..2afca4930 100644 --- a/libs/services/src/lib/server-storage/server-migration.service.ts +++ b/libs/services/src/lib/server-storage/server-migration.service.ts @@ -10,8 +10,8 @@ import { withExplicitStalkerPortalFlag, } from '@iptvnator/shared/interfaces'; import { + combineRecentViews, combineUserData, - newestViewsFirst, remapChannelReferences, retargetUserData, unionItems, @@ -258,13 +258,11 @@ export class ServerMigrationService { } const typed = [stored, ...browserCopies].map(typeLegacyXtreamRefs); for (const namespace of SERVER_USER_STATE_NAMESPACES) { - const combined = combineUserData( - typed.map((copy) => copy[namespace]) - ); - const value = + const value = ( namespace === 'xtream-recent-items' - ? newestViewsFirst(combined) - : combined; + ? combineRecentViews + : combineUserData + )(typed.map((copy) => copy[namespace])); if ( value === undefined || JSON.stringify(value) === JSON.stringify(stored[namespace]) diff --git a/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts index c5ead1632..d711040f5 100644 --- a/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts @@ -156,4 +156,57 @@ describe('ServerUserDataStorage taking over other page loads', () => { expect(adopted.resumed()).toBe(false); expect(kept.resumed()).toBe(true); }); + + it("keeps a closed tab's edit giving way through repeated conflicts", async () => { + const at = (id: number, seconds: number) => ({ + contentType: 'vod', + contentXtreamId: id, + positionSeconds: seconds, + }); + const server = fakeServer({ + 'xtream-playback-positions': { p1: [at(1, 10), at(2, 5)] }, + }); + const positions = () => server.state['xtream-playback-positions']['p1']; + const shared = memoryStorage(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const closed = new ServerUserDataStorage( + { + ...server.api, + sendJson: jest.fn(() => + Promise.reject(new ServerApiError(0, 'network', 'offline')) + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await closed.hydrate(); + closed.setItem( + 'xtream-playback-positions', + JSON.stringify({ p1: [at(1, 20), at(2, 5)] }) + ); + await closed.flush(); + closed.close(); + clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); + const tab = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + + // Adopted; another device moves movie 2 before it is sent. + await tab.hydrate(); + server.state['xtream-playback-positions']['p1'] = [at(1, 10), at(2, 6)]; + await tab.flush(); + // And movie 1 before the replayed write lands. + server.state['xtream-playback-positions']['p1'] = [at(1, 30), at(2, 6)]; + await tab.flush(); + await tab.flush(); + clock.mockRestore(); + + expect(positions()).toEqual([at(1, 30), at(2, 6)]); + }); }); diff --git a/libs/services/src/lib/server-storage/server-user-data-queue.ts b/libs/services/src/lib/server-storage/server-user-data-queue.ts index df1f84da1..c2ff73528 100644 --- a/libs/services/src/lib/server-storage/server-user-data-queue.ts +++ b/libs/services/src/lib/server-storage/server-user-data-queue.ts @@ -458,11 +458,17 @@ export class UserDataWriteQueue { ) { return; } + // A closed tab's steps keep giving way to newer changes at the next + // conflict; this page's own become one edit on top of them. + const yielding = latest.steps.filter((step) => step.yields); this.pending.set(key, { ...latest, value: merged, tag: current.tag, - steps: [{ before: current.value, after: merged }], + steps: [ + ...yielding, + { before: replaySteps(current.value, yielding), after: merged }, + ], }); this.scheduleFlush(); }