diff --git a/apps/electron-backend-e2e/src/performance/m3u-refresh-renderer-capture.ts b/apps/electron-backend-e2e/src/performance/m3u-refresh-renderer-capture.ts index b7889cdf7..22fab9ebd 100644 --- a/apps/electron-backend-e2e/src/performance/m3u-refresh-renderer-capture.ts +++ b/apps/electron-backend-e2e/src/performance/m3u-refresh-renderer-capture.ts @@ -330,7 +330,7 @@ async function installRendererProbe( const row = sourceRow(); const message = row - ?.querySelector('.busy-state__message') + ?.querySelector('.source-row__busy-message') ?.textContent?.trim() .toLowerCase() ?? ''; for (const phase of ['fetch', 'pars', 'sav']) { @@ -345,9 +345,10 @@ async function installRendererProbe( state.cancelClickEpochMs !== null && state.uiSettledEpochMs === null && row !== undefined && - row.querySelector('.busy-state__message') === null && - row.querySelector('.action-spinner') === null && - row.querySelector('.cancel-btn') === null + row.querySelector('.source-row__busy-message') === null && + row.querySelector('.source-row__refresh mat-spinner') === + null && + row.querySelector('.source-row__cancel') === null ) { state.uiSettledEpochMs = epoch(); state.uiPaintFrameRequestId = requestAnimationFrame(() => { @@ -421,7 +422,9 @@ async function installRendererProbe( event.phase === 'parsing' && state.cancelClickEpochMs === null ) { - const button = sourceRow()?.querySelector('.cancel-btn'); + const button = sourceRow()?.querySelector( + '.source-row__cancel' + ); state.cancelButtonFound = button instanceof HTMLButtonElement; if (button instanceof HTMLButtonElement) { diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver-wiring.spec.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver-wiring.spec.ts index dd2d6ac52..97e293dce 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver-wiring.spec.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver-wiring.spec.ts @@ -68,6 +68,7 @@ describe('Xtream scenario driver source wiring', () => { ); assert.ok(driver.includes('installXtreamCancellationObserver')); assert.ok(driver.includes('installXtreamOperationWindow')); + assert.ok(driver.includes("'delete-playlist'")); assert.ok(driver.includes('runBackgroundProbe')); assert.doesNotMatch(combined, /waitForTimeout\s*\(/); assert.doesNotMatch(combined, /setTimeout\s*\(/); diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts index 51d2b5f98..e8bf5c2e9 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.spec.ts @@ -191,6 +191,50 @@ describe('Xtream Playwright scenario driver', () => { } }); + it('arms the playlist delete before its commit click and ends on its completion', async () => { + const fake = new FakePage(); + const completed = { + active: false, + operation: 'delete-playlist', + operationId: 'delete-playlist-1', + playlistId: 'playlist-1', + startedEpochMs: 100, + terminalEpochMs: 150, + terminalStatus: 'completed', + }; + fake.operationSnapshot = completed; + const prepared = await prepareXtreamScenario( + fake.asPage(), + scenarioOptions(XTREAM_SCENARIO_ID.DELETE_LARGE) + ); + await prepared.trigger(); + const armIndex = fake.log.indexOf('evaluate:install-operation'); + const clickIndex = fake.log.findIndex( + (entry) => + entry.startsWith('click:') && + entry.includes('.source-removal-toast__close') + ); + assert.ok(armIndex >= 0, 'the delete window is armed'); + assert.ok(armIndex < clickIndex, 'before the commit click'); + + await prepared.waitForTerminal(); + assert.ok( + fake.log.indexOf('waitForFunction:operation-terminal') > clickIndex + ); + + const failed = new FakePage(); + failed.operationSnapshot = { ...completed, terminalStatus: 'error' }; + const failing = await prepareXtreamScenario( + failed.asPage(), + scenarioOptions(XTREAM_SCENARIO_ID.DELETE_LARGE) + ); + await failing.trigger(); + await assert.rejects( + () => failing.waitForTerminal(), + /xtream-delete-did-not-complete/ + ); + }); + it('arms cancellation after preparation and immediately before the final Add trigger', async () => { const fake = new FakePage(); const prepared = await prepareXtreamScenario( diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts index eb1d8f2ac..2846e6034 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-driver.ts @@ -19,7 +19,7 @@ import { prepareXtreamSourceActionTrigger, waitForXtreamCancellationTerminal, waitForXtreamCatalogTerminal, - waitForXtreamDeleteTerminal, + waitForXtreamSourceRowGone, } from './xtream-scenario-ui'; import { assertXtreamMockOrigin } from './xtream-control-client'; import { assertXtreamBackgroundRefreshActive } from './xtream-renderer-probe'; @@ -106,6 +106,7 @@ export async function prepareXtreamScenario( return createPreparedScenario({ backgroundOperation, cancellationObserver: null, + deleteOperation: null, options, page, trigger, @@ -119,6 +120,8 @@ export async function prepareXtreamScenario( interface PreparedState { readonly backgroundOperation: XtreamOperationWindow | null; cancellationObserver: XtreamCancellationObserver | null; + /** The playlist delete, armed before its commit click. */ + deleteOperation: XtreamOperationWindow<'delete-playlist'> | null; readonly options: PrepareXtreamScenarioOptions; readonly page: Page; readonly trigger: Locator; @@ -136,6 +139,7 @@ function createPreparedScenario(state: PreparedState): PreparedXtreamScenario { await Promise.allSettled([ state.cancellationObserver?.dispose(), state.backgroundOperation?.dispose(), + state.deleteOperation?.dispose(), ]); }; const prepared: PreparedXtreamScenario = { @@ -189,6 +193,15 @@ function createPreparedScenario(state: PreparedState): PreparedXtreamScenario { state.cancellationObserver = await installXtreamCancellationObserver(state.page); } + if (state.options.scenarioId === XTREAM_SCENARIO_ID.DELETE_LARGE) { + // A fast delete can bring its row back and drop it again + // before a UI wait starts; the operation's own events cannot + // be missed once armed. + state.deleteOperation = await installXtreamOperationWindow( + state.page, + 'delete-playlist' + ); + } const triggerEpochMs = Date.now(); await state.trigger.click(); return { @@ -224,12 +237,20 @@ async function waitForScenarioTerminal( cancellation = await state.cancellationObserver.waitForClick(); await waitForXtreamCancellationTerminal(state.page); break; - case XTREAM_SCENARIO_ID.DELETE_LARGE: - await waitForXtreamDeleteTerminal( + case XTREAM_SCENARIO_ID.DELETE_LARGE: { + if (!state.deleteOperation) { + throw new Error('xtream-delete-operation-unavailable'); + } + const deleted = await state.deleteOperation.waitForTerminal(); + if (deleted.terminalStatus !== 'completed') { + throw new Error('xtream-delete-did-not-complete'); + } + await waitForXtreamSourceRowGone( state.page, state.options.sourceTitle ); break; + } case XTREAM_SCENARIO_ID.BACKGROUND_UI: if (readBackgroundProbe() === null) { throw new Error('xtream-background-probe-incomplete'); diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-operation-observer.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-operation-observer.ts index e582ab632..4ddc900e4 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-operation-observer.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-operation-observer.ts @@ -2,11 +2,16 @@ import type { Page } from '@playwright/test'; const OPERATION_STATE_KEY = '__iptvnatorXtreamScenarioOperationWindow'; const DELETE_OPERATION = 'delete-xtream-content'; +/** A content delete before a refresh, or a whole playlist's delete. */ +export type XtreamObservedOperation = + typeof DELETE_OPERATION | 'delete-playlist'; const OPERATION_TIMEOUT_MS = 120_000; -export interface XtreamOperationWindowSnapshot { +export interface XtreamOperationWindowSnapshot< + Operation extends XtreamObservedOperation = typeof DELETE_OPERATION, +> { readonly active: boolean; - readonly operation: typeof DELETE_OPERATION; + readonly operation: Operation; readonly operationId: string; readonly playlistId: string; readonly startedEpochMs: number; @@ -14,16 +19,21 @@ export interface XtreamOperationWindowSnapshot { readonly terminalStatus: 'cancelled' | 'completed' | 'error' | null; } -export interface XtreamOperationWindow { - assertActive(): Promise; +export interface XtreamOperationWindow< + Operation extends XtreamObservedOperation = typeof DELETE_OPERATION, +> { + assertActive(): Promise>; dispose(): Promise; - waitForActive(): Promise; - waitForTerminal(): Promise; + waitForActive(): Promise>; + waitForTerminal(): Promise>; } -export async function installXtreamOperationWindow( - page: Page -): Promise { +export async function installXtreamOperationWindow< + Operation extends XtreamObservedOperation = typeof DELETE_OPERATION, +>( + page: Page, + operation: Operation = DELETE_OPERATION as Operation +): Promise> { await page.evaluate( ({ command, operation, stateKey }) => { interface State { @@ -104,20 +114,20 @@ export async function installXtreamOperationWindow( }, { command: 'install-operation', - operation: DELETE_OPERATION, + operation, stateKey: OPERATION_STATE_KEY, } ); return { - assertActive: () => assertOperationActive(page), + assertActive: () => assertOperationActive(page, operation), dispose: () => disposeOperationState(page), async waitForActive() { await waitForOperationState(page, 'operation-active'); - return assertOperationActive(page); + return assertOperationActive(page, operation); }, async waitForTerminal() { await waitForOperationState(page, 'operation-terminal'); - return readOperationSnapshot(page); + return readOperationSnapshot(page, operation); }, }; } @@ -144,19 +154,21 @@ async function waitForOperationState( ); } -async function assertOperationActive( - page: Page -): Promise { - const snapshot = await readOperationSnapshot(page); +async function assertOperationActive( + page: Page, + operation: Operation +): Promise> { + const snapshot = await readOperationSnapshot(page, operation); if (!snapshot.active || snapshot.terminalEpochMs !== null) { throw new Error('xtream-background-action-outside-delete-window'); } return snapshot; } -async function readOperationSnapshot( - page: Page -): Promise { +async function readOperationSnapshot( + page: Page, + operation: Operation +): Promise> { const value = await page.evaluate( ({ stateKey }) => { const target = globalThis as unknown as Record; @@ -183,7 +195,7 @@ async function readOperationSnapshot( }, { command: 'snapshot-operation', stateKey: OPERATION_STATE_KEY } ); - if (!isOperationSnapshot(value)) { + if (!isOperationSnapshot(value, operation)) { throw new Error('xtream-operation-window-invalid'); } return value; @@ -202,16 +214,17 @@ async function disposeOperationState(page: Page): Promise { ); } -function isOperationSnapshot( - value: unknown -): value is XtreamOperationWindowSnapshot { +function isOperationSnapshot( + value: unknown, + operation: Operation +): value is XtreamOperationWindowSnapshot { if (!isRecord(value)) return false; const active = value['active']; const startedEpochMs = value['startedEpochMs']; const terminalEpochMs = value['terminalEpochMs']; const terminalStatus = value['terminalStatus']; return ( - value['operation'] === DELETE_OPERATION && + value['operation'] === operation && typeof active === 'boolean' && finitePositive(startedEpochMs) && nonEmptyString(value['operationId']) && diff --git a/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts b/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts index 4d5b97cc6..aa254c241 100644 --- a/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts +++ b/apps/electron-backend-e2e/src/performance/xtream-scenario-ui.ts @@ -139,14 +139,15 @@ export async function waitForXtreamCancellationTerminal( ); } -export async function waitForXtreamDeleteTerminal( +/** + * After the database reports the delete complete: the row, back as + * "Removing…" while it ran or never shown again for a fast delete, leaves. + */ +export async function waitForXtreamSourceRowGone( page: Page, sourceTitle: string ): Promise { - const row = xtreamSourceRow(page, sourceTitle); - // The row returns as "Removing…" once the delete starts, then leaves. - await row.waitFor({ state: 'attached' }); - await row.waitFor({ state: 'detached' }); + await xtreamSourceRow(page, sourceTitle).waitFor({ state: 'detached' }); } async function selectXtreamMethod(dialog: Locator): Promise { diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index 80458581b..4ecdda46a 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -1369,8 +1369,10 @@ in `@iptvnator/playlist/shared/util` (`source-display`, `source-format`, the full address on request. - **Delete with Undo.** A single delete has no confirmation: the row leaves every list at once (`SourceRemovalService.pendingIds`) and the delete runs - when the toast closes; its close button commits at once, and quitting - within the four seconds keeps the source. Deleting the active source + when the four-second Undo window ends. The window is the service's own + timer, not the toast's: another message replacing the toast (a refresh + finishing elsewhere) does not shorten it. The toast's close button commits + at once, and quitting within the four seconds keeps the source. Deleting the active source switches to the next row; Undo switches back only if no other source was chosen since. A desktop Xtream delete brings its row back as "Removing…" with progress and Cancel on the Sources page; the switcher leaves the @@ -1405,9 +1407,11 @@ The status dot on a source tile reads `SourceStatusService`: green answers, red does not, orange is an expired account, grey is not checked yet; local files have no dot. On desktop the live `SourceHealthService` result wins; the last conclusive result per source is kept across restarts -(`SourceHealthHistoryService`, no URL or credential stored), so a source that -failed yesterday shows as failing before it is checked again, with "Not -responding · 3 days". The dot's tooltip and accessible name give the +(`SourceHealthHistoryService`, no URL or credential stored, only a 32-bit +fingerprint of the connection), so a source that failed yesterday shows as +failing before it is checked again, with "Not responding · 3 days". An +edited address or account is a new connection: its kept result no longer +counts, and its history starts over. The dot's tooltip and accessible name give the check's cause (`SourceStatus.reason`: access denied, account disabled, timeout…, kept with the history) and check time; M3U checks say they cover the playlist URL, not its channels. Retry and Check diff --git a/docs/architecture/m3u-playlist-module.md b/docs/architecture/m3u-playlist-module.md index 60052054e..e0f38ecb6 100644 --- a/docs/architecture/m3u-playlist-module.md +++ b/docs/architecture/m3u-playlist-module.md @@ -1858,9 +1858,13 @@ An M3U success describes the playlist URL, not every channel. Local files and text imports have no network indicator. PWA retains its existing Xtream path. Lists read one status per source through `SourceStatusService`, which falls back to the last conclusive result kept by `SourceHealthHistoryService` -(localStorage, by playlist ID: state, check time and the start of a failing -run; no URL or credential). Deleting a source or resetting the library clears -its entry. See the [UI guidelines](iptvnator-ui-guidelines.md#network-source-indicators-electron). +(localStorage, by playlist ID: state, check time, the start of a failing +run, the check's cause and a 32-bit fingerprint of the checked connection; +no URL or credential). A result kept for an edited address or account is +ignored, and the next check starts the history over. Deleting a source or +resetting the library clears its entry. The Sources page keeps every +library source checked while it is open (`keepSourcesChecked`), so a status +filter never hides the sources it is waiting for. See the [UI guidelines](iptvnator-ui-guidelines.md#network-source-indicators-electron). Checks are demand-driven, never a startup readiness barrier: four at a time, two per origin, shared between surfaces. Known results remain visible during diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts index 048f15c4c..0ee342918 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts @@ -65,6 +65,16 @@ describe('SourceRemovalToastComponent', () => { ).toBe('Close'); }); + it('commits through the service from the close button when it can', () => { + const commitNow = jest.fn(); + data['commitNow'] = commitNow; + render() + .querySelector('.source-removal-toast__close') + ?.click(); + expect(commitNow).toHaveBeenCalled(); + expect(ref.dismiss).not.toHaveBeenCalled(); + }); + it('takes the focus when the delete left it nowhere', () => { const element = render(); (document.activeElement as HTMLElement | null)?.blur(); diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts index eb79a4224..a888c28a8 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts @@ -22,6 +22,8 @@ export interface SourceRemovalToastData { closeLabel: string; /** Undo takes the focus, which returns here when the toast closes. */ returnFocusTo?: HTMLElement | null; + /** The close button: commit the delete now rather than at the end. */ + commitNow?: () => void; } /** @@ -59,7 +61,7 @@ export interface SourceRemovalToastData { mat-icon-button type="button" class="source-removal-toast__close" - (click)="ref.dismiss()" + (click)="close()" [attr.aria-label]="data.closeLabel" > close @@ -97,4 +99,13 @@ export class SourceRemovalToastComponent { } }); } + + /** Commits the delete at once; the toast closes with it. */ + close(): void { + if (this.data.commitNow) { + this.data.commitNow(); + } else { + this.ref.dismiss(); + } + } } 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 8e143aaec..2f5b82f00 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 @@ -17,7 +17,10 @@ import { RENDERER_PERFORMANCE_PHASE_HOOK_KEY, type RendererPerformancePhaseEvent, } from '@iptvnator/shared/logging'; -import { SourceRemovalService } from './source-removal.service'; +import { + SOURCE_REMOVAL_UNDO_MS, + SourceRemovalService, +} from './source-removal.service'; const playlist = (id: string): PlaylistMeta => ({ _id: id, @@ -30,6 +33,7 @@ const playlist = (id: string): PlaylistMeta => ({ describe('SourceRemovalService', () => { let toasts: Array<{ message: string; + data: { commitNow?: () => void }; action: Subject; dismissed: Subject; dismiss: jest.Mock; @@ -48,10 +52,11 @@ describe('SourceRemovalService', () => { let router: { url: string; navigate: jest.Mock; navigateByUrl: jest.Mock }; let service: SourceRemovalService; - const toast = (message: string) => { + const toast = (message: string, data: { commitNow?: () => void } = {}) => { const dismissed = new Subject(); const entry: (typeof toasts)[number] = { message, + data, action: new Subject(), dismissed, dismiss: jest.fn(() => dismissed.next()), @@ -64,12 +69,16 @@ describe('SourceRemovalService', () => { }; }; + /** The Undo window ends: the service's own timer commits. */ + const expireUndo = () => jest.advanceTimersByTime(SOURCE_REMOVAL_UNDO_MS); + const flush = async () => { await Promise.resolve(); await Promise.resolve(); }; beforeEach(() => { + jest.useFakeTimers(); toasts = []; deletePlaylist = jest.fn().mockResolvedValue(true); cancelOperation = jest.fn().mockResolvedValue(true); @@ -100,8 +109,13 @@ describe('SourceRemovalService', () => { openFromComponent: jest.fn( ( _component: unknown, - config: { data: { message: string } } - ) => toast(config.data.message) + config: { + data: { + message: string; + commitNow?: () => void; + }; + } + ) => toast(config.data.message, config.data) ), }, }, @@ -121,15 +135,16 @@ describe('SourceRemovalService', () => { }); service = TestBed.inject(SourceRemovalService); }); + afterEach(() => jest.useRealTimers()); - it('hides the source and deletes it only when the toast closes', async () => { + it('hides the source and deletes it only when the Undo window ends', async () => { const a = playlist('a'); service.remove(a); expect(service.isPending('a')).toBe(true); expect(deletePlaylist).not.toHaveBeenCalled(); expect(toasts[0].message).toBe('WORKSPACE.SOURCES.REMOVED'); - toasts[0].dismissed.next(); + expireUndo(); await flush(); expect(deletePlaylist).toHaveBeenCalledWith(a, { onEvent: expect.any(Function), @@ -144,11 +159,37 @@ describe('SourceRemovalService', () => { service.remove(playlist('a')); toasts[0].action.next(); toasts[0].dismissed.next(); + expireUndo(); await flush(); expect(service.isPending('a')).toBe(false); expect(deletePlaylist).not.toHaveBeenCalled(); }); + it('keeps the full Undo window when another message replaces the toast', async () => { + service.remove(playlist('a')); + // A refresh finishing elsewhere opens its own snack bar. + toasts[0].dismissed.next(); + jest.advanceTimersByTime(SOURCE_REMOVAL_UNDO_MS - 1); + await flush(); + expect(deletePlaylist).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1); + await flush(); + expect(deletePlaylist).toHaveBeenCalledTimes(1); + }); + + it('commits at once from the close button, and only once', async () => { + service.remove(playlist('a')); + toasts[0].data.commitNow?.(); + await flush(); + expect(deletePlaylist).toHaveBeenCalledTimes(1); + expect(toasts[0].dismiss).toHaveBeenCalled(); + + expireUndo(); + await flush(); + expect(deletePlaylist).toHaveBeenCalledTimes(1); + }); + it('commits an earlier removal when a second one starts', async () => { service.remove(playlist('a')); service.remove(playlist('b')); @@ -216,7 +257,7 @@ describe('SourceRemovalService', () => { deletePlaylist.mockReturnValue(new Promise(() => undefined)); service.remove({ ...playlist('x'), serverUrl: 'http://x.test' }); expect(service.isPending('x')).toBe(true); - toasts[0].dismissed.next(); + expireUndo(); await flush(); expect(service.isPending('x')).toBe(false); expect(service.progress.isDeleting('x')).toBe(true); @@ -233,7 +274,7 @@ describe('SourceRemovalService', () => { events.push(event); try { service.remove({ ...playlist('x'), serverUrl: 'http://x.test' }); - toasts[0].dismissed.next(); + expireUndo(); await flush(); } finally { delete target[hook]; @@ -263,7 +304,7 @@ describe('SourceRemovalService', () => { } ); service.remove(playlist('x')); - toasts[0].dismissed.next(); + expireUndo(); await flush(); // The row is back, in its busy state. @@ -287,7 +328,7 @@ describe('SourceRemovalService', () => { it('brings the source back and says so when the delete fails', async () => { deletePlaylist.mockResolvedValue(false); service.remove(playlist('a')); - toasts[0].dismissed.next(); + expireUndo(); await flush(); expect(dispatch).not.toHaveBeenCalled(); expect(service.isPending('a')).toBe(false); 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 c87954ff1..54839e20f 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 @@ -34,13 +34,18 @@ export interface SourceRemovalOptions { interface PendingRemoval { playlist: PlaylistMeta; - snackBar: MatSnackBarRef; + /** Its Undo toast, set once opened. */ + snackBar?: MatSnackBarRef; /** The playlist was the active one; `next` replaced it, if any. */ wasActive: boolean; wasOnItsRoute: boolean; previousUrl: string; next: PlaylistMeta | null; undone: boolean; + /** Undone or committed: nothing more happens to it. */ + settled: boolean; + /** Commits when the Undo window ends, whatever happened to the toast. */ + timer: ReturnType | undefined; } /** @@ -50,7 +55,9 @@ interface PendingRemoval { * nothing is deleted while Undo is offered: the source only leaves the * lists (`pendingIds`), and the real delete runs when the toast closes * without Undo. Quitting the app inside those four seconds keeps the - * source. One removal is pending at a time; a second delete commits the + * source. The window is the service's own timer, not the toast's: another + * message can replace the toast early without shortening it. One removal + * is pending at a time; a second delete commits the * first. A delete that reports progress (a large Xtream catalog) brings its * row back as "Removing…" with Cancel, which keeps the source. Bulk cleanup * keeps its confirmation dialog instead. @@ -88,8 +95,10 @@ export class SourceRemovalService { if (this.isPending(playlist._id)) { return; } - // The toast for an earlier removal closes here and commits it. - this.pending?.snackBar.dismiss(); + // A second delete commits the first at once. + if (this.pending) { + this.settle(this.pending); + } const wasActive = this.playlistContext.resolvedPlaylistId() === playlist._id; @@ -112,6 +121,16 @@ export class SourceRemovalService { next: this.nameOf(replacement), }) : this.translate.instant('WORKSPACE.SOURCES.REMOVED', { name }); + const removal: PendingRemoval = { + playlist, + wasActive, + wasOnItsRoute, + previousUrl, + next: replacement, + undone: false, + settled: false, + timer: undefined, + }; const snackBar = this.snackBar.openFromComponent( SourceRemovalToastComponent, { @@ -122,35 +141,43 @@ export class SourceRemovalService { ), closeLabel: this.translate.instant('CLOSE'), returnFocusTo: options.returnFocusTo ?? null, + commitNow: () => this.settle(removal), }, duration: SOURCE_REMOVAL_UNDO_MS, panelClass: 'app-toast', } ); - const removal: PendingRemoval = { - playlist, - snackBar, - wasActive, - wasOnItsRoute, - previousUrl, - next: replacement, - undone: false, - }; + removal.snackBar = snackBar; + removal.timer = setTimeout( + () => this.settle(removal), + SOURCE_REMOVAL_UNDO_MS + ); this.pending = removal; - snackBar.onAction().subscribe(() => this.undo(removal)); - snackBar.afterDismissed().subscribe(() => { - if (this.pending === removal) { - this.pending = null; - } - if (!removal.undone) { - void this.commit(removal); - } - }); + } + + /** Ends the Undo window: commits the delete unless it was undone. */ + private settle(removal: PendingRemoval): void { + if (removal.settled) { + return; + } + removal.settled = true; + clearTimeout(removal.timer); + if (this.pending === removal) { + this.pending = null; + } + removal.snackBar?.dismiss(); + if (!removal.undone) { + void this.commit(removal); + } } private undo(removal: PendingRemoval): void { + if (removal.settled) { + return; + } removal.undone = true; + this.settle(removal); this.setPending(removal.playlist._id, false); if (!removal.wasActive) { return; diff --git a/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts b/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts index 842fd9729..d33367074 100644 --- a/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts +++ b/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts @@ -47,6 +47,19 @@ describe('SourceHealthHistoryService', () => { ); }); + it('keeps the fingerprint of the checked connection, and starts over for another', () => { + const history = create(); + history.record(['a'], failed(100), 'aaaa1111'); + history.record(['a'], failed(900), 'bbbb2222'); + expect(history.entries().get('a')).toEqual({ + state: 'bad', + checkedAt: 900, + failingSince: 900, + reason: 'network', + connection: 'bbbb2222', + }); + }); + it('drops a stored cause this build cannot describe', () => { localStorage.setItem( SOURCE_HEALTH_HISTORY_STORAGE_KEY, diff --git a/libs/portal/shared/data-access/src/lib/source-health-history.service.ts b/libs/portal/shared/data-access/src/lib/source-health-history.service.ts index 519b27632..3d251f63d 100644 --- a/libs/portal/shared/data-access/src/lib/source-health-history.service.ts +++ b/libs/portal/shared/data-access/src/lib/source-health-history.service.ts @@ -28,11 +28,12 @@ function readHistory(): Map { continue; } // A reason this build cannot describe would show as a raw key. - const { reason, ...rest } = entry; - entries.set( - id, - isSourceHealthReason(reason) ? { ...rest, reason } : rest - ); + const { reason, connection, ...rest } = entry; + entries.set(id, { + ...rest, + ...(isSourceHealthReason(reason) ? { reason } : {}), + ...(typeof connection === 'string' ? { connection } : {}), + }); } return entries; } catch { @@ -52,8 +53,9 @@ function toRecord( * The last conclusive health result of each source, by playlist ID, kept * across restarts. Live checks stay in memory (`SourceHealthService`); this * keeps only what a list needs before they run again: the state, when it - * was checked, since when a source has failed and why. No URL or credential is - * stored, so the entries need no redaction. + * was checked, since when a source has failed and why, and a fingerprint + * of the connection checked. No URL or credential is stored, so the + * entries need no redaction. */ @Injectable({ providedIn: 'root' }) export class SourceHealthHistoryService { @@ -73,7 +75,16 @@ export class SourceHealthHistoryService { ); } - record(ids: readonly string[], snapshot: SourceHealthSnapshot): void { + /** + * @param connection Fingerprint of the checked connection + * (`fingerprintSourceHealthKey`); a result for another connection than + * the kept one starts the history over. + */ + record( + ids: readonly string[], + snapshot: SourceHealthSnapshot, + connection?: string + ): void { const reachability = sourceReachabilityFromHealth(snapshot); const next = new Map(this.state()); let changed = false; @@ -83,7 +94,7 @@ export class SourceHealthHistoryService { previous, reachability, snapshot.checkedAt, - snapshot.reason + { reason: snapshot.reason, connection } ); if (entry && entry !== previous) { next.set(id, entry); diff --git a/libs/portal/shared/data-access/src/lib/source-health.service.ts b/libs/portal/shared/data-access/src/lib/source-health.service.ts index 884073f5d..ccaed69f4 100644 --- a/libs/portal/shared/data-access/src/lib/source-health.service.ts +++ b/libs/portal/shared/data-access/src/lib/source-health.service.ts @@ -4,6 +4,7 @@ import { RuntimeCapabilitiesService, } from '@iptvnator/services'; import { + fingerprintSourceHealthKey, PlaylistMeta, SourceHealthSnapshot, SourceHealthResult, @@ -307,6 +308,7 @@ export class SourceHealthService { const ids = [...this.identities] .filter(([, identity]) => identity === key) .map(([id]) => id); - if (ids.length) this.history.record(ids, snapshot); + if (ids.length) + this.history.record(ids, snapshot, fingerprintSourceHealthKey(key)); } } diff --git a/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts b/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts index f4b2f7191..cb5400f23 100644 --- a/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts +++ b/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts @@ -7,6 +7,7 @@ import { import { PlaylistMeta, SourceHealthHistoryEntry, + sourceHealthFingerprint, SourceHealthSnapshot, } from '@iptvnator/shared/interfaces'; import { SourceHealthHistoryService } from './source-health-history.service'; @@ -104,6 +105,40 @@ describe('SourceStatusService', () => { }); }); + it('ignores a result kept for the source before its address changed', () => { + history.set( + new Map([ + [ + 'm', + { + state: 'bad', + checkedAt: 50, + failingSince: 10, + connection: 'another-address', + }, + ], + ]) + ); + const service = create(true); + expect(service.statusOf(m3u)).toEqual({ + reachability: 'unknown', + }); + + history.set( + new Map([ + [ + 'm', + { + state: 'bad', + checkedAt: 50, + connection: sourceHealthFingerprint(m3u), + }, + ], + ]) + ); + expect(service.statusOf(m3u)?.reachability).toBe('bad'); + }); + it("keeps a failed check's cause across a restart", () => { history.set( new Map([ diff --git a/libs/portal/shared/data-access/src/lib/source-status.service.ts b/libs/portal/shared/data-access/src/lib/source-status.service.ts index 24bd0cfbb..481628879 100644 --- a/libs/portal/shared/data-access/src/lib/source-status.service.ts +++ b/libs/portal/shared/data-access/src/lib/source-status.service.ts @@ -8,6 +8,7 @@ import { isConclusiveReachability, PlaylistMeta, SourceHealthReason, + sourceHealthFingerprint, sourceHealthType, sourceReachabilityFromHealth, sourceReachabilityFromPortalStatus, @@ -86,7 +87,13 @@ export class SourceStatusService { } const live = this.health.get(playlist); const reachability = sourceReachabilityFromHealth(live); - const kept = this.history.entries().get(playlist._id); + const entry = this.history.entries().get(playlist._id); + // A result kept for the source's old address or account is not its. + const kept = + entry?.connection && + entry.connection !== sourceHealthFingerprint(playlist) + ? undefined + : entry; if (isConclusiveReachability(reachability)) { return { reachability, diff --git a/libs/shared/interfaces/src/lib/source-health.ts b/libs/shared/interfaces/src/lib/source-health.ts index 89ce05dca..73f504871 100644 --- a/libs/shared/interfaces/src/lib/source-health.ts +++ b/libs/shared/interfaces/src/lib/source-health.ts @@ -84,6 +84,24 @@ export function sourceHealthKey(p: Partial): string { p.stalkerSignature2, ]); } +/** + * A 32-bit FNV-1a fingerprint of a source health key: enough to tell that a + * source now points somewhere else, without storing where (or with which + * credentials). Kept with the persisted health history. + */ +export function fingerprintSourceHealthKey(key: string): string { + let hash = 0x811c9dc5; + for (let index = 0; index < key.length; index += 1) { + hash ^= key.charCodeAt(index); + hash = Math.imul(hash, 0x01000193); + } + return (hash >>> 0).toString(16).padStart(8, '0'); +} + +export function sourceHealthFingerprint(p: Partial): string { + return fingerprintSourceHealthKey(sourceHealthKey(p)); +} + export function accountHealth( status: unknown, expiration?: number, diff --git a/libs/shared/interfaces/src/lib/source-reachability.spec.ts b/libs/shared/interfaces/src/lib/source-reachability.spec.ts index a533713a4..5931702e9 100644 --- a/libs/shared/interfaces/src/lib/source-reachability.spec.ts +++ b/libs/shared/interfaces/src/lib/source-reachability.spec.ts @@ -73,11 +73,37 @@ describe('nextSourceHealthHistory', () => { }); it('keeps the cause of a failure, never of a success', () => { - const failed = nextSourceHealthHistory(undefined, 'bad', 100, 'auth'); + const failed = nextSourceHealthHistory(undefined, 'bad', 100, { + reason: 'auth', + }); expect(failed?.reason).toBe('auth'); - expect(nextSourceHealthHistory(failed, 'ok', 200, 'available')).toEqual( - { state: 'ok', checkedAt: 200, lastSuccessAt: 200 } - ); + expect( + nextSourceHealthHistory(failed, 'ok', 200, { reason: 'available' }) + ).toEqual({ state: 'ok', checkedAt: 200, lastSuccessAt: 200 }); + }); + + it('starts over for an edited connection, keeps the run for the same one', () => { + const failed = nextSourceHealthHistory(undefined, 'bad', 100, { + connection: 'old', + }); + expect( + nextSourceHealthHistory(failed, 'bad', 900, { connection: 'old' }) + ?.failingSince + ).toBe(100); + expect( + nextSourceHealthHistory(failed, 'bad', 900, { connection: 'new' }) + ).toEqual({ + state: 'bad', + checkedAt: 900, + failingSince: 900, + connection: 'new', + }); + // An inconclusive check of the new connection keeps nothing old. + expect( + nextSourceHealthHistory(failed, 'unknown', 900, { + connection: 'new', + }) + ).toBeUndefined(); }); it('ends the run when the source answers again', () => { diff --git a/libs/shared/interfaces/src/lib/source-reachability.ts b/libs/shared/interfaces/src/lib/source-reachability.ts index c51a6905d..65a082ecc 100644 --- a/libs/shared/interfaces/src/lib/source-reachability.ts +++ b/libs/shared/interfaces/src/lib/source-reachability.ts @@ -106,6 +106,12 @@ export interface SourceHealthHistoryEntry { lastSuccessAt?: number; /** The cause of the last failed check; absent while `ok`. */ reason?: SourceHealthReason; + /** + * Fingerprint of the connection that was checked + * (`sourceHealthFingerprint`): a result for an edited address or account + * is not this source's any more. + */ + connection?: string; } /** @@ -115,16 +121,23 @@ export interface SourceHealthHistoryEntry { * every later failed check. */ export function nextSourceHealthHistory( - previous: SourceHealthHistoryEntry | undefined, + kept: SourceHealthHistoryEntry | undefined, reachability: SourceReachability, checkedAt: number, - reason?: SourceHealthReason + detail: { reason?: SourceHealthReason; connection?: string } = {} ): SourceHealthHistoryEntry | undefined { + const { reason, connection } = detail; + // A check of another connection starts the history over. + const previous = + kept?.connection && connection && kept.connection !== connection + ? undefined + : kept; if (!isConclusiveReachability(reachability)) { return previous; } + const checked = connection ? { connection } : {}; if (reachability === 'ok') { - return { state: 'ok', checkedAt, lastSuccessAt: checkedAt }; + return { state: 'ok', checkedAt, lastSuccessAt: checkedAt, ...checked }; } const wasFailing = previous !== undefined && previous.state !== 'ok'; return { @@ -137,5 +150,6 @@ export function nextSourceHealthHistory( ? { lastSuccessAt: previous.lastSuccessAt } : {}), ...(reason ? { reason } : {}), + ...checked, }; }