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(); }