From a861ef94ac531dc441d899b9504e8285ab8ec26f Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 23:18:58 +0200 Subject: [PATCH] fix(sources): clean URL-shaped fragments; one registry for busy sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A fragment is cleaned whatever its shape: `#https://joe:pw@…` loses its userinfo, an unclassifiable secret is dropped, and the name check reads a URL in the fragment as well as a parameter list. - Every refresh holds its source in the root SourceActivityService, which deletes and the startup auto-refresh already use: Sources-row refreshes, the switcher's M3U refresh and the shared Xtream flow. The switcher no longer offers Refresh or Remove for a source the page is refreshing, the removal service ignores a held source, a second refresh from either entry point is skipped, and the page disables Remove while it is held. - The long-input timing spec allows 3 s: the inputs take about 0.25 s, a backtracking pattern takes minutes, and a loaded runner took 1.6 s. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/iptvnator-ui-guidelines.md | 9 ++--- .../playlist-refresh-action.service.spec.ts | 4 +++ .../lib/playlist-refresh-action.service.ts | 3 ++ .../playlist-switcher-panel.component.html | 2 +- .../playlist-switcher-panel.component.spec.ts | 8 ++--- .../playlist-switcher-panel.component.ts | 11 +++++-- .../recent-playlists.component.html | 1 + .../recent-playlists.component.spec.ts | 18 ++++++++++ .../recent-playlists.component.ts | 7 ++++ .../source-list-refresh.service.spec.ts | 31 +++++++++++++++++ .../source-list-refresh.service.ts | 16 +++++++-- .../source-removal.service.spec.ts | 14 +++----- .../source-removal/source-removal.service.ts | 10 +++--- .../lib/xtream-refresh-flow.service.spec.ts | 33 +++++++++++++++++++ .../ui/src/lib/xtream-refresh-flow.service.ts | 7 ++++ .../lib/source-display/source-display.spec.ts | 8 +++++ .../src/lib/source-display/source-display.ts | 6 ++-- .../strip-url-credentials.spec.ts | 26 ++++++++++++++- .../source-display/strip-url-credentials.ts | 14 ++++---- 19 files changed, 188 insertions(+), 40 deletions(-) diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index df1dfbe82..0b63995f1 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -1391,10 +1391,11 @@ in `@iptvnator/playlist/shared/util` (`source-display`, `source-format`, switcher closes the popover first, since its focus trap would keep the keyboard from Undo: Undo takes the focus and returns it to the trigger (`SourceRemovalOptions.returnFocusTo`). A source cannot be removed while - a refresh clears and refills it: Remove is disabled in both lists and the - service ignores it (`PlaylistRefreshActionService.isSourceBusy`), and the - Sources page shows a switcher refresh as busy. Bulk cleanup keeps its - confirmation dialog. + a refresh clears and refills it: every refresh (a Sources row, the + switcher, the Xtream flow, the startup auto-refresh) and every delete + holds the source in the root `SourceActivityService`, which both lists, + the removal service and a second refresh consult. The Sources page shows + a switcher refresh as busy. Bulk cleanup keeps its confirmation dialog. - **Side list.** Type and, on desktop, Status (Any / Responding / Not responding) combine and persist (`SourcesViewStateService`); each group counts within the other's choice. The page checks every source, not only diff --git a/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.spec.ts b/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.spec.ts index 81f84ec5f..7132ed481 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.spec.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.spec.ts @@ -349,13 +349,17 @@ describe('PlaylistRefreshActionService', () => { resolve = r; }) ); + const activity = TestBed.inject(SourceActivityService); service.refresh(playlist); expect(service.isSourceBusy('busy')).toBe(true); expect(service.isSourceBusy('other')).toBe(false); + // The Sources page and the removal guard see it too. + expect(activity.isBusy('busy')).toBe(true); resolve({ _id: 'busy', playlist: { items: [] } } as Playlist); await Promise.resolve(); await Promise.resolve(); expect(service.isSourceBusy('busy')).toBe(false); + expect(activity.isBusy('busy')).toBe(false); }); it('passes trusted TLS hosts to URL-backed M3U refreshes', async () => { diff --git a/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.ts b/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.ts index 26e298bb9..991460649 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-refresh-action.service.ts @@ -148,6 +148,8 @@ export class PlaylistRefreshActionService { this.m3uRefreshId.set(item._id); this.isRefreshing.set(true); + // Seen by the Sources page and the removal guard as well. + const release = this.activity.begin([item._id]); if (isActiveM3uRoute) { this.store.dispatch( ChannelActions.setChannelsLoading({ loading: true }) @@ -214,6 +216,7 @@ export class PlaylistRefreshActionService { } this.m3uRefreshId.set(null); this.isRefreshing.set(false); + release(); } } diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html index 33a443417..f5e368d28 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html @@ -157,7 +157,7 @@ [current]="option.playlist._id === currentId()" [highlighted]="index === highlight()" [canRefresh]="canRefresh(option.playlist)" - [removable]="!isRefreshing(option.playlist)" + [removable]="!isBusy(option.playlist)" (activated)="select(option.playlist)" (refreshRequested)="refresh(option.playlist)" (playlistInfoRequested)="openInfo(option.playlist)" diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts index b058d70df..fb0c85900 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts @@ -15,6 +15,7 @@ import { SortOptions, SortOrder, SortService, + SourceActivityService, } from '@iptvnator/services'; import { PlaylistMeta } from '@iptvnator/shared/interfaces'; import { PlaylistRefreshActionService } from '../playlist-refresh-action.service'; @@ -51,7 +52,6 @@ describe('PlaylistSwitcherPanelComponent', () => { let selectPlaylist: jest.Mock; let remove: jest.Mock; let navigate: jest.Mock; - let refreshing: ReturnType>; const input = () => (fixture.nativeElement as HTMLElement).querySelector( @@ -83,7 +83,6 @@ describe('PlaylistSwitcherPanelComponent', () => { selectPlaylist = jest.fn(); remove = jest.fn(); navigate = jest.fn(); - refreshing = signal(null); TestBed.configureTestingModule({ imports: [ PlaylistSwitcherPanelComponent, @@ -138,7 +137,6 @@ describe('PlaylistSwitcherPanelComponent', () => { useValue: { canRefresh: () => true, refresh: jest.fn(), - isSourceBusy: (id: string) => refreshing() === id, }, }, { provide: MatDialog, useValue: { open: jest.fn() } }, @@ -288,7 +286,8 @@ describe('PlaylistSwitcherPanelComponent', () => { }); it('offers no Remove while a refresh clears and refills the source', async () => { - refreshing.set('bravo'); + // Held by a refresh from any entry point, the Sources page included. + const release = TestBed.inject(SourceActivityService).begin(['bravo']); fixture.detectChanges(); const menuRemove = async (id: string) => { (fixture.nativeElement as HTMLElement) @@ -318,6 +317,7 @@ describe('PlaylistSwitcherPanelComponent', () => { ) ?.click(); expect(selectPlaylist).toHaveBeenCalled(); + release(); }); it('leaves out a source whose delete is pending or running', () => { diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts index 222e4a643..2c79baca6 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts @@ -32,7 +32,11 @@ import { sourceSortName, SourcesTypeFilter, } from '@iptvnator/playlist/shared/util'; -import { SortBy, SortService } from '@iptvnator/services'; +import { + SortBy, + SortService, + SourceActivityService, +} from '@iptvnator/services'; import { PlaylistMeta, PlaylistSourceKind, @@ -76,6 +80,7 @@ const KINDS: readonly PlaylistSourceKind[] = ['m3u', 'xtream', 'stalker']; changeDetection: ChangeDetectionStrategy.OnPush, }) export class PlaylistSwitcherPanelComponent { + private readonly activity = inject(SourceActivityService); private readonly context = inject(PlaylistContextFacade); private readonly document = inject(DOCUMENT); private readonly dialog = inject(MatDialog); @@ -284,8 +289,8 @@ export class PlaylistSwitcherPanelComponent { } /** A source cannot be removed while a refresh clears and refills it. */ - isRefreshing(playlist: PlaylistMeta): boolean { - return this.refreshAction.isSourceBusy(playlist._id); + isBusy(playlist: PlaylistMeta): boolean { + return this.activity.isBusy(playlist._id); } refresh(playlist: PlaylistMeta): void { diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.html b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.html index ad21653b5..fd7673956 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.html +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.html @@ -37,6 +37,7 @@ [draggable]="isCustomSort()" [canRefresh]="refresh.canRefresh(item)" [refreshing]="isRefreshing(item)" + [removable]="isRemovable(item)" [deleting]="removal.progress.isDeleting(item._id)" [busyMessage]="busyMessage(item)" [busyProgress]="busyProgress(item)" diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts index d8ab3530f..2c885a471 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; import { MatDialog } from '@angular/material/dialog'; import { Router } from '@angular/router'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; @@ -21,10 +22,12 @@ import { SortOptions, SortOrder, SortService, + SourceActivityService, } from '@iptvnator/services'; import { PlaylistMeta, SourceStatus } from '@iptvnator/shared/interfaces'; import { PlaylistRefreshActionService } from '../playlist-refresh-action.service'; import { SourceRemovalService } from '../source-removal/source-removal.service'; +import { SourceRowComponent } from '../source-row/source-row.component'; import { RecentPlaylistsComponent } from './recent-playlists.component'; import { SourceListRefreshService } from './source-list-refresh.service'; @@ -285,6 +288,21 @@ describe('RecentPlaylistsComponent', () => { ).toBe(true); }); + it('offers no Remove while something else holds a source', () => { + const removable = () => + fixture.debugElement + .queryAll(By.directive(SourceRowComponent)) + .map((row) => + (row.componentInstance as SourceRowComponent).removable() + ); + expect(removable()).toEqual([true, true, true]); + // The startup auto-refresh, or a refresh from the switcher. + const release = TestBed.inject(SourceActivityService).begin(['m3u']); + fixture.detectChanges(); + expect(removable()).toEqual([false, true, true]); + release(); + }); + it('hides sources whose delete is pending', () => { pending.set(new Set(['m3u'])); expect(ids()).toEqual(['xtream', 'file']); diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts index f249babf1..e126b3467 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts @@ -40,6 +40,7 @@ import { RuntimeCapabilitiesService, SortBy, SortService, + SourceActivityService, } from '@iptvnator/services'; import { PlaylistMeta, sourceStatusBucket } from '@iptvnator/shared/interfaces'; import type { PlaylistType } from '../add-playlist-menu/playlist-type'; @@ -82,6 +83,7 @@ export class RecentPlaylistsComponent { readonly removal = inject(SourceRemovalService); readonly refresh = inject(SourceListRefreshService); private readonly refreshAction = inject(PlaylistRefreshActionService); + private readonly activity = inject(SourceActivityService); readonly searchQueryInput = input(''); readonly playlistClicked = output(); @@ -228,6 +230,11 @@ export class RecentPlaylistsComponent { ); } + /** Nothing else (a refresh, the startup auto-refresh) holds it. */ + isRemovable(item: PlaylistMeta): boolean { + return !this.activity.isBusy(item._id); + } + busyMessage(item: PlaylistMeta): string { return ( this.removal.progress.message(item._id) || diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.spec.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.spec.ts index ca5a0f4ee..82f0e6928 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.spec.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.spec.ts @@ -24,6 +24,7 @@ import { SortBy, SortOrder, SortService, + SourceActivityService, } from '@iptvnator/services'; import { CONNECTIVITY_GUARD_RESET, @@ -502,6 +503,36 @@ describe('SourceListRefreshService', () => { expect(playlistRefreshService.refreshPlaylist).not.toHaveBeenCalled(); }); + it('holds a row refresh in the shared registry, and leaves a held source alone', async () => { + const activity = TestBed.inject(SourceActivityService); + const item = createPlaylistMeta({ + _id: 'playlist-m3u', + serverUrl: undefined, + username: undefined, + password: undefined, + url: 'https://source.test/list.m3u', + }); + let resolve!: (value: unknown) => void; + playlistRefreshService.refreshPlaylist.mockReturnValueOnce( + new Promise((r) => { + resolve = r; + }) + ); + component.refresh(item); + // The switcher and the removal guard see the row's refresh. + expect(activity.isBusy(item._id)).toBe(true); + resolve({ id: item._id, items: [] }); + await Promise.resolve(); + await Promise.resolve(); + expect(activity.isBusy(item._id)).toBe(false); + + // A refresh or delete started elsewhere holds it: no second run. + const release = activity.begin([item._id]); + component.refresh(item); + expect(playlistRefreshService.refreshPlaylist).toHaveBeenCalledTimes(1); + release(); + }); + it('re-evaluates refresh bridge availability when refreshing local M3U playlists', async () => { runtime.supportsPlaylistRefresh = false; const lateComponent = TestBed.inject(SourceListRefreshService); diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.ts index f526175f8..78c40523a 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/source-list-refresh.service.ts @@ -11,6 +11,7 @@ import { isDbAbortError, PlaylistRefreshService, RuntimeCapabilitiesService, + SourceActivityService, } from '@iptvnator/services'; import { PLAYLIST_UPDATE, @@ -57,6 +58,10 @@ export class SourceListRefreshService { private readonly dataService = inject(DataService); private readonly playlistRefreshService = inject(PlaylistRefreshService); private readonly runtime = inject(RuntimeCapabilitiesService); + // Root registry: the switcher, the removal guard and cleanup consult it, + // so a row refresh must not be visible only to this page. + private readonly activity = inject(SourceActivityService); + private readonly releases = new Map void>(); private readonly snackBar = inject(MatSnackBar); private readonly store = inject(Store); private readonly translate = inject(TranslateService); @@ -83,7 +88,7 @@ export class SourceListRefreshService { } refresh(item: PlaylistMeta): void { - if (this.isRefreshing(item._id)) { + if (this.isRefreshing(item._id) || this.activity.isBusy(item._id)) { return; } if (item.serverUrl && this.runtime.supportsXtreamSqliteDataSource) { @@ -166,7 +171,8 @@ export class SourceListRefreshService { /** The shared Xtream re-import reports into this list's row state. */ private readonly xtreamReporter: XtreamRefreshProgressReporter = { - isBusy: (playlistId) => this.isRefreshing(playlistId), + isBusy: (playlistId) => + this.isRefreshing(playlistId) || this.activity.isBusy(playlistId), begin: ({ playlistId }) => this.setRefreshing(playlistId, true), report: ({ playlistId }, event) => this.track(playlistId, event), end: ({ playlistId }) => { @@ -283,6 +289,12 @@ export class SourceListRefreshService { } private setRefreshing(id: string, refreshing: boolean): void { + if (refreshing && !this.releases.has(id)) { + this.releases.set(id, this.activity.begin([id])); + } else if (!refreshing) { + this.releases.get(id)?.(); + this.releases.delete(id); + } this.refreshingIds.update((current) => { const next = new Set(current); if (refreshing) { diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts index de78aa84c..3193c6af8 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts @@ -11,13 +11,13 @@ import { DatabaseService, PlaylistDeleteActionService, RuntimeCapabilitiesService, + SourceActivityService, } from '@iptvnator/services'; import { PlaylistMeta } from '@iptvnator/shared/interfaces'; import { RENDERER_PERFORMANCE_PHASE_HOOK_KEY, type RendererPerformancePhaseEvent, } from '@iptvnator/shared/logging'; -import { PlaylistRefreshActionService } from '../playlist-refresh-action.service'; import { SOURCE_REMOVAL_UNDO_MS, SourceRemovalService, @@ -52,7 +52,6 @@ describe('SourceRemovalService', () => { }; let router: { url: string; navigate: jest.Mock; navigateByUrl: jest.Mock }; let service: SourceRemovalService; - let refreshing: Set; const toast = (message: string, data: { commitNow?: () => void } = {}) => { const dismissed = new Subject(); @@ -85,7 +84,6 @@ describe('SourceRemovalService', () => { deletePlaylist = jest.fn().mockResolvedValue(true); cancelOperation = jest.fn().mockResolvedValue(true); dispatch = jest.fn(); - refreshing = new Set(); resolvedPlaylistId = signal(null); routePlaylistId = signal(null); context = { @@ -134,12 +132,6 @@ describe('SourceRemovalService', () => { provide: RuntimeCapabilitiesService, useValue: { supportsXtreamSqliteDataSource: true }, }, - { - provide: PlaylistRefreshActionService, - useValue: { - isSourceBusy: (id: string) => refreshing.has(id), - }, - }, ], }); service = TestBed.inject(SourceRemovalService); @@ -165,13 +157,15 @@ describe('SourceRemovalService', () => { }); it('does not remove a source while a refresh clears and refills it', async () => { - refreshing.add('a'); + // Any entry point's refresh, or the startup auto-refresh. + const release = TestBed.inject(SourceActivityService).begin(['a']); service.remove(playlist('a')); expireUndo(); await flush(); expect(service.isPending('a')).toBe(false); expect(toasts).toHaveLength(0); expect(deletePlaylist).not.toHaveBeenCalled(); + release(); }); it('restores the source on Undo without deleting anything', async () => { diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts index 2c3410878..d1028742c 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts @@ -11,9 +11,9 @@ import { import { PlaylistDeleteActionService, RuntimeCapabilitiesService, + SourceActivityService, } from '@iptvnator/services'; import { PlaylistMeta } from '@iptvnator/shared/interfaces'; -import { PlaylistRefreshActionService } from '../playlist-refresh-action.service'; import { SourceDeleteProgressService } from './source-delete-progress.service'; import { SourceRemovalToastComponent } from './source-removal-toast.component'; import { @@ -67,7 +67,7 @@ interface PendingRemoval { export class SourceRemovalService { private readonly deleteAction = inject(PlaylistDeleteActionService); private readonly playlistContext = inject(PlaylistContextFacade); - private readonly refreshAction = inject(PlaylistRefreshActionService); + private readonly activity = inject(SourceActivityService); private readonly router = inject(Router); private readonly runtime = inject(RuntimeCapabilitiesService); private readonly snackBar = inject(MatSnackBar); @@ -94,11 +94,11 @@ export class SourceRemovalService { next: PlaylistMeta | null = null, options: SourceRemovalOptions = {} ): void { - // A refresh started from the switcher is destructive too (it clears - // the catalog, then re-imports it): the two must not overlap. + // A refresh from any entry point (or the startup auto-refresh) is + // destructive too: it clears the catalog, then re-imports it. if ( this.isPending(playlist._id) || - this.refreshAction.isSourceBusy(playlist._id) + this.activity.isBusy(playlist._id) ) { return; } diff --git a/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.spec.ts b/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.spec.ts index 13c75c41c..b16d85461 100644 --- a/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.spec.ts +++ b/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.spec.ts @@ -9,6 +9,7 @@ import { DatabaseService, DbOperationEvent, PlaybackPositionService, + SourceActivityService, } from '@iptvnator/services'; import { CONNECTIVITY_GUARD_RESET, @@ -168,6 +169,38 @@ describe('XtreamRefreshFlowService', () => { expect(dataService.sendIpcEvent).not.toHaveBeenCalled(); }); + it('holds the source in the shared registry for the run, and skips a held one', async () => { + const activity = TestBed.inject(SourceActivityService); + const item = createPlaylistMeta(); + let heldDuringDelete = false; + databaseService.deleteXtreamPlaylistContent.mockImplementationOnce( + () => { + heldDuringDelete = activity.isBusy(item._id); + return Promise.resolve({ + success: true, + favorites: [], + recentlyViewed: [], + hiddenCategories: [], + }); + } + ); + service.confirmAndRefresh(item, createReporter().reporter); + await confirmPromise; + expect(heldDuringDelete).toBe(true); + expect(activity.isBusy(item._id)).toBe(false); + + // A delete or another refresh already holds it: nothing runs. + const release = activity.begin([item._id]); + const second = createReporter(); + service.confirmAndRefresh(item, second.reporter); + await confirmPromise; + expect(second.calls).toEqual([]); + expect( + databaseService.deleteXtreamPlaylistContent + ).toHaveBeenCalledTimes(1); + release(); + }); + it('serializes destructive runs for one playlist across entry points', async () => { const item = createPlaylistMeta(); const confirms: Array | undefined> = []; diff --git a/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.ts b/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.ts index c872a6263..9f405887d 100644 --- a/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.ts +++ b/libs/playlist/shared/ui/src/lib/xtream-refresh-flow.service.ts @@ -11,6 +11,7 @@ import { isDbAbortError, PlaybackPositionService, resetHostConnectivityGuard, + SourceActivityService, XtreamPendingRestoreService, } from '@iptvnator/services'; import { PlaylistActions } from '@iptvnator/m3u-state'; @@ -79,6 +80,7 @@ export class XtreamRefreshFlowService { private readonly pendingRestoreService = inject( XtreamPendingRestoreService ); + private readonly activity = inject(SourceActivityService); /** * Playlists with a destructive run in flight, across every entry point. @@ -119,8 +121,11 @@ export class XtreamRefreshFlowService { item: PlaylistMeta, reporter: XtreamRefreshProgressReporter ): Promise { + // The shared registry also holds deletes, the startup auto-refresh + // and M3U refreshes from either entry point. if ( this.activeRefreshPlaylistIds.has(item._id) || + this.activity.isBusy(item._id) || reporter.isBusy(item._id) ) { return; @@ -131,6 +136,7 @@ export class XtreamRefreshFlowService { const run: XtreamRefreshRun = { playlistId: item._id, operationId }; reporter.begin(run); this.activeRefreshPlaylistIds.add(item._id); + const release = this.activity.begin([item._id]); try { // Before anything destructive: this deletes the cached catalog and @@ -205,6 +211,7 @@ export class XtreamRefreshFlowService { } } finally { this.activeRefreshPlaylistIds.delete(item._id); + release(); reporter.end(run); } } diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts index 867fcfaae..b7777f214 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts @@ -110,6 +110,14 @@ describe('sourceDisplayName', () => { ).toBe('host.test'); }); + it('drops a name holding a secret of a URL in the fragment', () => { + const url = 'https://proxy.test/list#https://joe:hunter2@origin.test/a'; + for (const title of ['Mine hunter2', 'Mine joe']) { + expect(sourceDisplayName({ title, url })).toBe('proxy.test'); + } + expect(sourceDisplayName({ title: 'Mine', url })).toBe('Mine'); + }); + it('keeps a playlist file name from a path that names no account', () => { const url = 'https://cdn.example/live/news/list.m3u'; expect(sourceDisplayName({ title: 'list.m3u', url })).toBe('list.m3u'); diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.ts index 843c92b98..716381b5e 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.ts @@ -132,10 +132,8 @@ function collectUrlSecrets( visitNested(segment); }); visit(url.searchParams); - const fragment = url.hash.slice(1); - if (fragment.includes('=')) { - visit(new URLSearchParams(fragment)); - } + // A fragment may hold a parameter list or a whole URL. + visitNested(url.hash.slice(1)); } /** diff --git a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts index f08b95fe4..db70ab4ca 100644 --- a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts @@ -83,6 +83,28 @@ describe('stripUrlCredentials', () => { expect(stripUrlCredentials(nested)).not.toMatch(/hunter2|joe/); }); + it('cleans a fragment that holds a URL rather than parameters', () => { + expect( + stripUrlCredentials( + 'https://proxy.test/list#https://joe:hunter2@origin.test/a.m3u' + ) + ).toBe('https://proxy.test/list#https://origin.test/a.m3u'); + expect( + stripUrlCredentials( + `https://proxy.test/list#${encodeURIComponent( + 'https://joe:hunter2@origin.test/a.m3u' + )}` + ) + ).not.toMatch(/hunter2|joe/); + expect( + stripUrlCredentials('https://proxy.test/list#joe:hunter2@origin') + ).toBe('https://proxy.test/list'); + // A plain anchor stays as written. + expect(stripUrlCredentials('https://host.test/list.m3u#part-2')).toBe( + 'https://host.test/list.m3u#part-2' + ); + }); + it('keeps a playlist path that only starts like an Xtream stream', () => { // No stream follows `/news/list.m3u`: it names no account. expect(stripUrlCredentials('https://cdn.test/live/news/list.m3u')).toBe( @@ -163,7 +185,9 @@ describe('stripUrlCredentials', () => { `https://host.test/list?note=${encodeURIComponent(value)}` ); } - expect(performance.now() - started).toBeLessThan(1_000); + // About 0.25 s in total when idle; a backtracking pattern takes + // minutes. The margin absorbs a loaded CI runner. + expect(performance.now() - started).toBeLessThan(3_000); }); it('returns null for text that is not an absolute URL', () => { diff --git a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts index 2162b9505..9ab029646 100644 --- a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts +++ b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts @@ -169,13 +169,15 @@ function stripUrl(value: string, depth: number): string | null { const search = withoutSensitiveParams(url.searchParams, depth); url.search = search ? `?${search}` : ''; + // Any fragment, not only a parameter list: `#https://joe:pw@…` is a URL. const fragment = url.hash.slice(1); - if (looksLikeSearchParams(fragment)) { - const hash = withoutSensitiveParams( - new URLSearchParams(fragment), - depth - ); - url.hash = hash ? `#${hash}` : ''; + if (fragment) { + const clean = sanitizeNested(fragment, depth); + if (!clean) { + url.hash = ''; + } else if (clean !== fragment) { + url.hash = clean; + } } return url.toString();