diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index c7cf3e596..dd06d4b8d 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -499,6 +499,13 @@ downloads sit in their own component-provided services beside it. ## Backup +`playPinnedSource` reports `played` / `superseded` / `unavailable`, and only +`unavailable` may fall through to the route source. Collapsing the first two +meant a double-click on Play — where the second attempt supersedes the first — +had the losing attempt conclude "no usable pin" and start the route stream over +the playback the winning one had just begun. Same distinction as `runFailover`'s, +for the same reason. + A pin is written and its old aliases retired in ONE transaction (`setVodSourcePin(db, pin, retireKeys)`). Split in two there is no honest outcome for a half-failure: a surviving alias is read before the canonical key diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts index b6faadfb9..4e5e0cd09 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts @@ -252,12 +252,31 @@ describe('VodDetailsRouteComponent — playback actions', () => { expect(reported).toHaveBeenLastCalledWith(120); }); + it('does not start the route source when a newer action owns the screen', async () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + withActiveSource('playlist-1', 650020); + // Double-clicking Play while the pin is resolving: the second click + // supersedes the first, and the first must not conclude "no usable + // pin" and start the route source over what the second just began. + jest.spyOn(component.multiSource, 'playPinnedSource').mockResolvedValue( + 'superseded' + ); + const reported = jest.spyOn(component.multiSource, 'reportPosition'); + + await component.onPrimaryAction({ + movie_data: { stream_id: 650020, name: 'Example' }, + } as never); + + expect(reported).not.toHaveBeenCalled(); + }); + it('takes the route wrappers when the primary button falls through', () => { currentPlaylist.set({ id: 'playlist-1' }); const component = fixture.componentInstance; withActiveSource('playlist-1', 650020); jest.spyOn(component.multiSource, 'playPinnedSource').mockResolvedValue( - false + 'unavailable' ); const reported = jest.spyOn(component.multiSource, 'reportPosition'); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts index deab57b3f..7bbc57931 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts @@ -443,9 +443,13 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { // A pinned source is an explicit "play this movie from here", so it // outranks the playlist the route happens to be on. Falls through to // the normal path when nothing is pinned or the pin cannot resolve. - if ( - await this.multiSource.playPinnedSource(this.msUi.resumeSecondsFor) - ) { + const pinned = await this.multiSource.playPinnedSource( + this.msUi.resumeSecondsFor + ); + // Only "no usable pin" falls through. A superseded attempt means a + // newer action already owns the screen — starting the route source + // here would override the playback that action just began. + if (pinned !== 'unavailable') { return; } @@ -460,7 +464,6 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { this.playVod(vodItem); } - stopExternalPlayback(): Promise { return this.playback.stopExternalPlayback(); } diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts index 90d52ddad..1dc3d6a17 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts @@ -103,7 +103,7 @@ describe('VodMultiSourceHostService — pinning', () => { // Without this the persisted preference is only an icon: reopening the // movie would still start the playlist the route is on. - await expect(service.playPinnedSource()).resolves.toBe(true); + await expect(service.playPinnedSource()).resolves.toBe('played'); expect(rowFor(ALT_TWO.id)?.isActive).toBe(true); }); @@ -121,7 +121,7 @@ describe('VodMultiSourceHostService — pinning', () => { // the user is about to press says "Resume". service.seedResumePosition(2538); - await expect(service.playPinnedSource()).resolves.toBe(true); + await expect(service.playPinnedSource()).resolves.toBe('played'); expect(resolver.resolve).toHaveBeenCalledWith( expect.objectContaining({ id: ALT_TWO.id }), @@ -179,7 +179,9 @@ describe('VodMultiSourceHostService — pinning', () => { service.seedResumePosition(12); const resumeFor = jest.fn().mockResolvedValue(3600); - await expect(service.playPinnedSource(resumeFor)).resolves.toBe(true); + await expect(service.playPinnedSource(resumeFor)).resolves.toBe( + 'played' + ); expect(resumeFor).toHaveBeenCalledWith( expect.objectContaining({ id: ALT_TWO.id }) @@ -225,7 +227,7 @@ describe('VodMultiSourceHostService — pinning', () => { await expect( service.playPinnedSource(jest.fn().mockResolvedValue(null)) - ).resolves.toBe(true); + ).resolves.toBe('played'); // Positions are keyed by (playlist, stream). Carrying the route copy's // timecode into a copy the user has never opened drops them 42 minutes @@ -249,7 +251,7 @@ describe('VodMultiSourceHostService — pinning', () => { // No lookup was performed, so "never watched" was never established — // zeroing here would throw away a position nobody contradicted. - await expect(service.playPinnedSource()).resolves.toBe(true); + await expect(service.playPinnedSource()).resolves.toBe('played'); expect(resolver.resolve).toHaveBeenCalledWith( expect.objectContaining({ id: ALT_TWO.id }), @@ -319,14 +321,14 @@ describe('VodMultiSourceHostService — pinning', () => { slow.resolve({ sources: [ALT_TWO], matchKind: 'title-year' }); await loading; - await expect(playing).resolves.toBe(true); + await expect(playing).resolves.toBe('played'); expect(rowFor(ALT_TWO.id)?.isActive).toBe(true); }); it('leaves Play alone when nothing is pinned', async () => { await loadMovie([ALT_TWO]); - await expect(service.playPinnedSource()).resolves.toBe(false); + await expect(service.playPinnedSource()).resolves.toBe('unavailable'); expect(startPlayback).not.toHaveBeenCalled(); }); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts index fbbccac95..e250e537c 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts @@ -111,6 +111,34 @@ describe('VodMultiSourceHostService — stale resolutions', () => { expect(rowFor(ALT_TWO.id)?.isActive).toBe(false); }); + it('tells a superseded pinned play apart from an unusable pin', async () => { + pins.get.mockResolvedValue({ + matchKey: 'title:the matrix:1999', + playlistId: ALT_TWO.playlistId, + contentId: ALT_TWO.contentId, + portalType: 'xtream', + }); + await loadMovie([ALT_TWO, ALT_THREE]); + + const slow = createDeferred>(); + resolver.resolve.mockReturnValueOnce(slow.promise); + const pinnedPlay = service.playPinnedSource(); + + // Wait until the pinned play is genuinely inside its resolve, so the + // second click is what supersedes it rather than a race on setup. + while (resolver.resolve.mock.calls.length === 0) { + await Promise.resolve(); + } + + // A second Play (double-click) supersedes the first while it resolves. + await service.play(ALT_THREE.id); + slow.resolve(resolvedFor(ALT_TWO, 0)); + + // Reporting "unavailable" here would have the caller start the ROUTE + // source over the playback the newer click just began. + await expect(pinnedPlay).resolves.toBe('superseded'); + }); + it('drops a slower switch that a newer selection already superseded', async () => { await loadMovie([ALT_TWO, ALT_THREE]); @@ -289,7 +317,7 @@ describe('VodMultiSourceHostService — stale resolutions', () => { lookup.resolve(3600); - await expect(playing).resolves.toBe(false); + await expect(playing).resolves.toBe('superseded'); expect(startPlayback).not.toHaveBeenCalled(); }); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts index 462c5b10b..09b484f81 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts @@ -24,6 +24,7 @@ import { import { runDiscovery, runFailover, + toPinnedOutcome, switchToSource, type SwitchOutcome, } from './vod-multi-source-session'; @@ -33,11 +34,12 @@ import { currentSourceRow } from './vod-multi-source-current-row'; import { probeSource } from './vod-multi-source-probe'; import { pinnedSourceAwaitingPlay, + type PinnedPlayOutcome, pinnedSourceIdOf, - playPinned, + startPinnedSource, pinKeysFor, readPin, - togglePinnedSource, + commitPinToggle, type PinKeySets, } from './vod-multi-source-pin'; import { @@ -236,50 +238,39 @@ export class VodMultiSourceHostService { ); /** - * Start from the pinned source if there is one. Returns false when there is - * nothing pinned to honour, leaving the caller's own Play path in charge. + * Start from the pinned source if there is one. `unavailable` means there + * is nothing pinned to honour, leaving the caller's own Play path in + * charge; a superseded attempt must NOT fall through that way. */ - async playPinnedSource( + playPinnedSource( resumeFor?: (source: VodSourceCandidate) => Promise - ): Promise { - // The pin arrives with discovery; answering before the lookup lands - // would make a persisted preference lose to worker latency. - if (!(await this.stillOwnsScreen())) { - return false; - } - + ): Promise { const session = this.sessionToken; - return playPinned({ + return startPinnedSource({ controller: this.controller, - pinnedSourceId: this.pendingPinnedSourceId(), + loadInFlight: this.loadInFlight, + pinnedSourceId: () => this.pendingPinnedSourceId(), resumeFor, isCurrent: () => session === this.sessionToken, - play: (sourceId) => this.play(sourceId), + play: (sourceId) => this.runPlay(sourceId), }); } - /** - * Wait for a discovery still in flight, then say whether this film is - * still the one on screen. Both callers touch the controller afterwards, - * and the user can navigate during the wait — acting then would answer - * one film's question with another film's sources. - */ - private async stillOwnsScreen(): Promise { - const session = this.sessionToken; - await this.loadInFlight; - return session === this.sessionToken; - } - /** Play from a specific source once; does not change the pin. */ async play(sourceId: string): Promise { + return (await this.runPlay(sourceId)) === 'played'; + } + + /** As `play`, but keeping the distinction the pinned path needs. */ + private async runPlay(sourceId: string): Promise { const candidate = this.controller.findSource(sourceId); if (!candidate || !this.bindings) { - return false; + return 'unavailable'; } this._busySourceId.set(sourceId); try { - return (await this.switchTo(candidate)) === 'switched'; + return toPinnedOutcome(await this.switchTo(candidate)); } finally { // Only while this attempt still owns the spinner, or a slower // pick would clear the row that is still resolving. @@ -292,21 +283,17 @@ export class VodMultiSourceHostService { /** Pin or unpin this source as the movie's preferred one. */ async togglePin(sourceId: string): Promise { const session = this.sessionToken; - const isPinned = this._sources().some( - (source) => source.id === sourceId && source.isPinned - ); - const pinned = await togglePinnedSource( - this.pins, - this.pinKeys, - this.controller.findSource(sourceId), - isPinned - ); + const pinned = await commitPinToggle({ + pins: this.pins, + keys: this.pinKeys, + candidate: this.controller.findSource(sourceId), + isPinned: this._sources().some( + (source) => source.id === sourceId && source.isPinned + ), + isCurrent: () => session === this.sessionToken, + }); - // The write is an IPC round-trip, and another movie can own the screen - // by the time it returns. Committing then would apply THIS film's - // answer to THAT film's controller — an unpin would clear the pin it - // just loaded, and its Play button would stop honouring it. - if (pinned === undefined || session !== this.sessionToken) { + if (pinned === undefined) { return; } @@ -315,8 +302,8 @@ export class VodMultiSourceHostService { } /** User-triggered availability check for one row. */ - async check(sourceId: string): Promise { - await probeSource(sourceId, { + check(sourceId: string): Promise { + return probeSource(sourceId, { controller: this.controller, resolver: this.resolver, probes: this.probes, @@ -340,7 +327,9 @@ export class VodMultiSourceHostService { // "nowhere to go" against a controller whose discovery has not landed // yet would strand the user on the error screen with alternatives // arriving a moment later and nothing left to retry them. - if (!(await this.stillOwnsScreen())) { + const session = this.sessionToken; + await this.loadInFlight; + if (session !== this.sessionToken) { return null; } diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts index 67348ae7e..6b5d05876 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-pin.ts @@ -151,6 +151,38 @@ export async function togglePinnedSource( return (await writePin(pins, keys, candidate)) ? candidate.id : undefined; } +/** What committing a pin toggle needs from the host. */ +export interface PinToggleDeps { + pins: Pick; + keys: PinKeySets; + candidate: VodSourceCandidate | null; + isPinned: boolean; + /** False once another movie owns the screen — the write is awaited. */ + isCurrent: () => boolean; +} + +/** + * Toggle the pin and report the id the controller should hold, or `undefined` + * when nothing should change. + * + * The write is an IPC round-trip and another movie can own the screen by the + * time it returns. Committing then would apply THIS film's answer to THAT + * film's controller — an unpin would clear the pin it just loaded, and its + * Play button would stop honouring it. + */ +export async function commitPinToggle( + deps: PinToggleDeps +): Promise { + const pinned = await togglePinnedSource( + deps.pins, + deps.keys, + deps.candidate, + deps.isPinned + ); + + return deps.isCurrent() ? pinned : undefined; +} + /** Which keys a pin may be looked up under, stored under, and removed from. */ export interface PinKeySets { /** Every alias the pin may be sitting under, most-trusted first. */ @@ -189,6 +221,28 @@ function retirablePinKeys(keys: PinKeySets): string[] { return retire; } +/** + * The full pinned-play errand: wait out a discovery still in flight, confirm + * this film still owns the screen, then start the pinned source. + * + * The wait matters — the pin arrives WITH discovery, so answering before the + * lookup lands would make a persisted preference lose to worker latency — and + * so does the re-check after it, because the user can navigate across it. + */ +export async function startPinnedSource( + deps: Omit & { + loadInFlight: Promise | null; + pinnedSourceId: () => string | null; + } +): Promise { + await deps.loadInFlight; + if (!deps.isCurrent()) { + return 'superseded'; + } + + return playPinned({ ...deps, pinnedSourceId: deps.pinnedSourceId() }); +} + /** What starting the pinned source needs from the host. */ export interface PinnedPlayDeps { controller: VodMultiSourceController; @@ -197,9 +251,19 @@ export interface PinnedPlayDeps { resumeFor?: (source: VodSourceCandidate) => Promise; /** False once another movie owns the screen — the lookup is awaited. */ isCurrent: () => boolean; - play: (sourceId: string) => Promise; + play: (sourceId: string) => Promise; } +/** + * Why a pinned play ended. + * + * `unavailable` is the ONLY outcome the caller may fall through on. A + * superseded attempt means something newer already owns the screen — treating + * that as "no usable pin" and starting the route source overrides the playback + * the newer action just began. + */ +export type PinnedPlayOutcome = 'played' | 'superseded' | 'unavailable'; + /** * Start the movie from its pinned source, at the position that source was * last left at. @@ -209,10 +273,12 @@ export interface PinnedPlayDeps { * row, which for this source is stale or missing entirely — resuming from it * would restart the film or jump to where a different copy was left. */ -export async function playPinned(deps: PinnedPlayDeps): Promise { +export async function playPinned( + deps: PinnedPlayDeps +): Promise { const pinned = deps.controller.findSource(deps.pinnedSourceId ?? ''); if (!pinned) { - return false; + return 'unavailable'; } const looked = !!deps.resumeFor; @@ -221,7 +287,7 @@ export async function playPinned(deps: PinnedPlayDeps): Promise { // it. Playing then would hand THIS film's source id to whatever movie now // owns the screen — starting it, if the id happens to exist there. if (!deps.isCurrent()) { - return false; + return 'superseded'; } if (looked) { diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts index ecc62dd24..9ac20f0fd 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts @@ -85,6 +85,22 @@ export function applyDiscoveredSources( controller.markPlaying(switchedTo.id); } +/** + * A switch outcome in the pinned-play vocabulary. + * + * Only `unresolvable` may fall through to the route source; a superseded + * attempt means something newer already owns the screen. + */ +export function toPinnedOutcome( + outcome: SwitchOutcome +): 'played' | 'superseded' | 'unavailable' { + if (outcome === 'switched') { + return 'played'; + } + + return outcome === 'superseded' ? 'superseded' : 'unavailable'; +} + /** What `switchToSource` needs from the host, without reaching into it. */ export interface SwitchDeps { controller: VodMultiSourceController;