From b24732b86e4321181f414dbf9e600dcfb00f3ecd Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 29 Jul 2026 02:44:41 +0200 Subject: [PATCH] refactor(portals): split the external-playback handoff out of the service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the service and its spec crossed the 400-line cap with the close-failure handling, so the handoff — deciding which process is ours, closing it, and surviving a close that rejects — now lives in `vod-details-external-session.ts` with its own spec file. Two tests had to start awaiting: replacing a running external player is a round-trip, and the handoff now yields once even when there is nothing to close, so the new playback is mounted a microtask later than before. Co-Authored-By: Claude Opus 5 --- .../vod-details-external-playback.spec.ts | 263 ++++++++++++++++++ .../vod-details-external-session.ts | 27 ++ .../vod-details-playback.service.spec.ts | 154 +--------- .../vod-details-playback.service.ts | 28 +- .../vod-details-route-caption.spec.ts | 5 +- 5 files changed, 308 insertions(+), 169 deletions(-) create mode 100644 libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-playback.spec.ts diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-playback.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-playback.spec.ts new file mode 100644 index 000000000..2826d6676 --- /dev/null +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-playback.spec.ts @@ -0,0 +1,263 @@ +import { signal } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { + PORTAL_EXTERNAL_PLAYBACK, + PORTAL_PLAYBACK_POSITIONS, + PORTAL_PLAYER, +} from '@iptvnator/portal/shared/util'; +import { XtreamStore } from '@iptvnator/portal/xtream/data-access'; +import { PlaybackPositionRuntimeBridgeService } from '@iptvnator/services'; +import type { + PlaybackPositionData, + PlayerContentInfo, +} from '@iptvnator/shared/interfaces'; +import { VodDetailsPlaybackService } from './vod-details-playback.service'; + +/** + * Which external session this page owns. + * + * Multi-source can launch MPV/VLC for a movie in ANOTHER playlist, and the + * session then carries that playlist's ids — so the matcher decides whether + * the primary button can stop it or silently launches a second player. + */ +/** + * Which external process this page owns, and what replacing it entails. + * Split from the position/ownership spec to keep both inside the file-size + * rule. + */ +describe('VodDetailsPlaybackService — external playback handoff', () => { + const ROUTE_PLAYLIST = 'playlist-1'; + const ROUTE_VOD_ID = 650020; + + let service: VodDetailsPlaybackService; + /** The bridge callback the service registers at construction. */ + let positionListener: ((data: PlaybackPositionData) => void) | undefined; + const addRecentItem = jest.fn(); + const activeSession = signal(null); + const closeSession = jest.fn().mockResolvedValue(undefined); + const openResolvedPlayback = jest.fn(); + const activeSource = signal(null); + + function sessionFor(playlistId: string, contentXtreamId: number) { + return { + player: 'mpv', + status: 'playing', + contentInfo: { + playlistId, + contentXtreamId, + contentType: 'vod' as const, + }, + }; + } + + beforeEach(() => { + activeSession.set(null); + activeSource.set(null); + positionListener = undefined; + addRecentItem.mockClear(); + + TestBed.configureTestingModule({ + providers: [ + VodDetailsPlaybackService, + { + provide: XtreamStore, + useValue: { + currentPlaylist: signal({ id: ROUTE_PLAYLIST }), + addRecentItem, + constructVodStreamUrl: jest + .fn() + .mockReturnValue('https://example.com/route.mkv'), + }, + }, + { + provide: PORTAL_EXTERNAL_PLAYBACK, + useValue: { activeSession, closeSession }, + }, + { + provide: PORTAL_PLAYBACK_POSITIONS, + useValue: { + getPlaybackPosition: jest.fn(), + savePlaybackPosition: jest.fn(), + }, + }, + { + provide: PORTAL_PLAYER, + useValue: { + isEmbeddedPlayer: jest.fn().mockReturnValue(false), + openResolvedPlayback, + }, + }, + { + provide: PlaybackPositionRuntimeBridgeService, + useValue: { + onPlaybackPositionUpdate: ( + listener: (data: PlaybackPositionData) => void + ) => { + positionListener = listener; + return () => undefined; + }, + }, + }, + ], + }); + + service = TestBed.inject(VodDetailsPlaybackService); + service.bind({ + vodId: signal(ROUTE_VOD_ID), + vodInfo: signal(null), + activeSource, + }); + }); + + it('closes the alternative it launched, once the badge has moved on', async () => { + // The controller marks the DESTINATION active before playback is + // handed over, so by the time the switch reaches the service the + // running process no longer looks like "ours" — and was left playing + // beside its replacement. + activeSession.set(sessionFor('playlist-2', 991)); + activeSource.set({ + playlistId: 'playlist-3', + contentXtreamId: 77, + contentType: 'vod', + }); + closeSession.mockClear(); + + // Pretend the alternative we are replacing is the one we started. + await service.startResolvedPlayback({ + streamUrl: 'https://example.com/first.mkv', + title: 'Example Movie', + contentInfo: { + playlistId: 'playlist-2', + contentXtreamId: 991, + contentType: 'vod', + }, + }); + closeSession.mockClear(); + + await service.startResolvedPlayback({ + streamUrl: 'https://example.com/second.mkv', + title: 'Example Movie', + contentInfo: { + playlistId: 'playlist-3', + contentXtreamId: 77, + contentType: 'vod', + }, + }); + + expect(closeSession).toHaveBeenCalledWith( + expect.objectContaining({ + contentInfo: expect.objectContaining({ + contentXtreamId: 991, + }), + }) + ); + }); + + it('still starts the replacement when closing the old player fails', async () => { + activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); + closeSession.mockRejectedValue(new Error('close ipc failed')); + openResolvedPlayback.mockClear(); + + await service.startResolvedPlayback({ + streamUrl: 'https://example.com/alt.mkv', + title: 'Example Movie', + }); + + // The switch is already committed — the badge names the new source. + // Bailing out here left the page claiming a source with nothing + // started at all. + expect(openResolvedPlayback).toHaveBeenCalledTimes(1); + + closeSession.mockResolvedValue(undefined); + }); + + it('launches only the newest source when two switches overlap', async () => { + activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); + // One shared promise: both calls see the same running session, so + // both await the same close rather than each making its own. + let releaseClose: (() => void) | undefined; + const closing = new Promise((resolve) => { + releaseClose = () => resolve(); + }); + closeSession.mockReturnValue(closing); + // These spies are module-level and never cleared between cases. + openResolvedPlayback.mockClear(); + + const first = service.startResolvedPlayback({ + streamUrl: 'https://example.com/one.mkv', + title: 'Example Movie', + }); + const second = service.startResolvedPlayback({ + streamUrl: 'https://example.com/two.mkv', + title: 'Example Movie', + }); + + releaseClose?.(); + await Promise.all([first, second]); + + // Both saw the same running session and awaited its close. Launching + // both afterwards is how two detached players appear again, with the + // older one holding a source the user has already moved on from. + expect(openResolvedPlayback).toHaveBeenCalledTimes(1); + expect(openResolvedPlayback).toHaveBeenCalledWith( + expect.objectContaining({ + streamUrl: 'https://example.com/two.mkv', + }), + true + ); + + closeSession.mockResolvedValue(undefined); + }); + + it('drops a switch that a plain Play overtook', async () => { + activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); + let releaseClose: (() => void) | undefined; + const closing = new Promise((resolve) => { + releaseClose = () => resolve(); + }); + closeSession.mockReturnValue(closing); + openResolvedPlayback.mockClear(); + + const switching = service.startResolvedPlayback({ + streamUrl: 'https://example.com/alt.mkv', + title: 'Example Movie', + }); + + // The user presses Play on the route copy while that close is pending. + service.playVod({ + movie_data: { stream_id: ROUTE_VOD_ID, name: 'Example Movie' }, + } as never); + releaseClose?.(); + await switching; + + // Only the route copy may be playing; the switch was overtaken. + expect(openResolvedPlayback).toHaveBeenCalledTimes(1); + expect(openResolvedPlayback).not.toHaveBeenCalledWith( + expect.objectContaining({ + streamUrl: 'https://example.com/alt.mkv', + }), + true + ); + + closeSession.mockResolvedValue(undefined); + }); + + it('stops the running external player before switching sources', async () => { + activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); + + await service.startResolvedPlayback({ + streamUrl: 'https://example.com/alt.mkv', + title: 'Example Movie', + contentInfo: { + playlistId: 'playlist-2', + contentXtreamId: 991, + contentType: 'vod', + }, + }); + + // A switch REPLACES what is playing. With MPV/VLC and instance reuse + // off the backend spawns a second detached player otherwise: both + // sources keep running, and Stop owns only the newer one. + expect(closeSession).toHaveBeenCalled(); + }); +}); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-session.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-session.ts index fa0d73eb3..ba0c085ad 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-session.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-external-session.ts @@ -75,3 +75,30 @@ export function runningExternalSession( return isLaunchedOne ? session : matched; } + +/** + * Close the running external player before its replacement starts. + * + * A failure is logged rather than propagated: the caller has already + * committed the switch, so refusing to launch would leave the page naming a + * source with nothing playing — worse than a possibly-lingering process, and + * a close that rejects usually means the session was gone already. + */ +export async function closeRunningExternalSession( + session: ExternalPlayerSession | null, + close: (session: ExternalPlayerSession) => Promise, + warn: (message: string, error: unknown) => void +): Promise { + if (!session) { + return; + } + + try { + await close(session); + } catch (error) { + warn( + 'Closing the previous external player failed; starting the replacement anyway.', + error + ); + } +} diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts index 2ea9153b4..687c99ecb 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts @@ -146,163 +146,19 @@ describe('VodDetailsPlaybackService — external session ownership', () => { expect(service.matchedExternalPlayback()).toBeNull(); }); - it('stops the running external player before switching sources', async () => { - activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); - await service.startResolvedPlayback({ - streamUrl: 'https://example.com/alt.mkv', - title: 'Example Movie', - contentInfo: { - playlistId: 'playlist-2', - contentXtreamId: 991, - contentType: 'vod', - }, - }); - // A switch REPLACES what is playing. With MPV/VLC and instance reuse - // off the backend spawns a second detached player otherwise: both - // sources keep running, and Stop owns only the newer one. - expect(closeSession).toHaveBeenCalled(); - }); - it('closes the alternative it launched, once the badge has moved on', async () => { - // The controller marks the DESTINATION active before playback is - // handed over, so by the time the switch reaches the service the - // running process no longer looks like "ours" — and was left playing - // beside its replacement. - activeSession.set(sessionFor('playlist-2', 991)); - activeSource.set({ - playlistId: 'playlist-3', - contentXtreamId: 77, - contentType: 'vod', - }); - closeSession.mockClear(); - // Pretend the alternative we are replacing is the one we started. - await service.startResolvedPlayback({ - streamUrl: 'https://example.com/first.mkv', - title: 'Example Movie', - contentInfo: { - playlistId: 'playlist-2', - contentXtreamId: 991, - contentType: 'vod', - }, - }); - closeSession.mockClear(); - await service.startResolvedPlayback({ - streamUrl: 'https://example.com/second.mkv', - title: 'Example Movie', - contentInfo: { - playlistId: 'playlist-3', - contentXtreamId: 77, - contentType: 'vod', - }, - }); - - expect(closeSession).toHaveBeenCalledWith( - expect.objectContaining({ - contentInfo: expect.objectContaining({ - contentXtreamId: 991, - }), - }) - ); - }); - - it('drops a switch that a plain Play overtook', async () => { - activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); - let releaseClose: (() => void) | undefined; - const closing = new Promise((resolve) => { - releaseClose = () => resolve(); - }); - closeSession.mockReturnValue(closing); - openResolvedPlayback.mockClear(); - - const switching = service.startResolvedPlayback({ - streamUrl: 'https://example.com/alt.mkv', - title: 'Example Movie', - }); - - // The user presses Play on the route copy while that close is pending. - service.playVod({ - movie_data: { stream_id: ROUTE_VOD_ID, name: 'Example Movie' }, - } as never); - releaseClose?.(); - await switching; - - // Only the route copy may be playing; the switch was overtaken. - expect(openResolvedPlayback).toHaveBeenCalledTimes(1); - expect(openResolvedPlayback).not.toHaveBeenCalledWith( - expect.objectContaining({ - streamUrl: 'https://example.com/alt.mkv', - }), - true - ); - - closeSession.mockResolvedValue(undefined); - }); - - it('still starts the replacement when closing the old player fails', async () => { - activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); - closeSession.mockRejectedValue(new Error('close ipc failed')); - openResolvedPlayback.mockClear(); - - await service.startResolvedPlayback({ - streamUrl: 'https://example.com/alt.mkv', - title: 'Example Movie', - }); - - // The switch is already committed — the badge names the new source. - // Bailing out here left the page claiming a source with nothing - // started at all. - expect(openResolvedPlayback).toHaveBeenCalledTimes(1); - - closeSession.mockResolvedValue(undefined); - }); - - it('launches only the newest source when two switches overlap', async () => { - activeSession.set(sessionFor(ROUTE_PLAYLIST, ROUTE_VOD_ID)); - // One shared promise: both calls see the same running session, so - // both await the same close rather than each making its own. - let releaseClose: (() => void) | undefined; - const closing = new Promise((resolve) => { - releaseClose = () => resolve(); - }); - closeSession.mockReturnValue(closing); - // These spies are module-level and never cleared between cases. - openResolvedPlayback.mockClear(); - - const first = service.startResolvedPlayback({ - streamUrl: 'https://example.com/one.mkv', - title: 'Example Movie', - }); - const second = service.startResolvedPlayback({ - streamUrl: 'https://example.com/two.mkv', - title: 'Example Movie', - }); - - releaseClose?.(); - await Promise.all([first, second]); - - // Both saw the same running session and awaited its close. Launching - // both afterwards is how two detached players appear again, with the - // older one holding a source the user has already moved on from. - expect(openResolvedPlayback).toHaveBeenCalledTimes(1); - expect(openResolvedPlayback).toHaveBeenCalledWith( - expect.objectContaining({ - streamUrl: 'https://example.com/two.mkv', - }), - true - ); - - closeSession.mockResolvedValue(undefined); - }); - - it('records a source started through multi-source as recently viewed', () => { + it('records a source started through multi-source as recently viewed', async () => { // Playing an alternative from the picker, or letting a pin decide the // primary Play, is still watching the movie — it belongs in Recently // Viewed exactly as an ordinary Play does. - service.startResolvedPlayback({ + // + // Awaited: the handoff always yields once now, because replacing a + // running external player is a round-trip even when there is none. + await service.startResolvedPlayback({ streamUrl: 'https://example.com/alt.mkv', title: 'Example Movie', contentInfo: { diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts index b496b7513..feb90c3a7 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts @@ -27,6 +27,7 @@ import { } from '@iptvnator/shared/interfaces'; import type { PlaybackFallbackRequest } from '@iptvnator/ui/playback'; import { + closeRunningExternalSession, ownsContent, runningExternalSession, } from './vod-details-external-session'; @@ -352,26 +353,15 @@ export class VodDetailsPlaybackService { // A switch REPLACES what is playing. With MPV or VLC and instance // reuse off, the backend spawns a second detached player otherwise — // both sources keep running and Stop owns only the newer one. - const running = runningExternalSession( - this.externalPlayback.activeSession(), - this.launchedExternally, - this.matchedExternalPlayback() + await closeRunningExternalSession( + runningExternalSession( + this.externalPlayback.activeSession(), + this.launchedExternally, + this.matchedExternalPlayback() + ), + (session) => this.externalPlayback.closeSession(session), + (message, error) => this.logger.warn(message, error) ); - if (running) { - try { - await this.externalPlayback.closeSession(running); - } catch (error) { - // The switch is already committed — the badge names the new - // source. Giving up here would leave the page claiming a - // source with nothing started, which is worse than the - // pre-existing risk of a lingering process (and a close that - // rejects usually means the session was gone anyway). - this.logger.warn( - 'Closing the previous external player failed; starting the replacement anyway.', - error - ); - } - } // Closing is a round-trip, and a second pick across it would otherwise // reach this line too: both would have seen the same session, closed diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts index 95e4c889e..539614c0f 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts @@ -138,7 +138,7 @@ describe('VodDetailsRouteComponent — source caption', () => { expect(component.activeSourceCaption()).toBeNull(); }); - it('waits again after a manual source switch', () => { + it('waits again after a manual source switch', async () => { currentPlaylist.set({ id: 'playlist-1' }); const component = fixture.componentInstance; const playback = fixture.debugElement.injector.get( @@ -170,6 +170,9 @@ describe('VodDetailsRouteComponent — source caption', () => { title: 'Example', startTime: 3, }); + // Replacing a running external player is a round-trip, so the handoff + // yields once before the new playback is mounted. + await Promise.resolve(); expect(playback.inlinePlayback()?.streamUrl).toBe( 'http://example.com/alt.mkv'