mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-14 13:16:16 -08:00
fix(sources): own Undo timer, per-connection health history, race-free perf waits
Local review round 2 (Codex + Greptile): - The delete committed whenever the shared snack bar was dismissed, so another message (a refresh finishing elsewhere) cut the four-second Undo window short and deleted the source early. The window is now the service's own timer; only the toast's close button (commitNow) or a second delete commits sooner. - The health history was keyed by playlist ID only, so an edited address or account inherited the old connection's result, failing run included. Entries now keep a 32-bit FNV-1a fingerprint of the source health key (no URL or credential); a result for another connection starts over and a stale entry no longer shows. - The M3U refresh-cancellation probe still queried the removed row's .cancel-btn, .busy-state__message and .action-spinner, so its cancel never fired; it now uses the source row's selectors (smoke run passes, cancelEffect=true). - The Xtream delete benchmark waited to catch the "Removing…" row coming back before it left, which a fast delete could finish before the wait began. The driver now arms a delete-playlist operation window before the commit click and ends on its completed event and the absent row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
366745f16d
commit
123bc170e9
20 files changed
+403
-97
No files matched your search
@@ -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) {
|
||||
|
||||
@@ -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*\(/);
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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<XtreamOperationWindowSnapshot>;
|
||||
export interface XtreamOperationWindow<
|
||||
Operation extends XtreamObservedOperation = typeof DELETE_OPERATION,
|
||||
> {
|
||||
assertActive(): Promise<XtreamOperationWindowSnapshot<Operation>>;
|
||||
dispose(): Promise<void>;
|
||||
waitForActive(): Promise<XtreamOperationWindowSnapshot>;
|
||||
waitForTerminal(): Promise<XtreamOperationWindowSnapshot>;
|
||||
waitForActive(): Promise<XtreamOperationWindowSnapshot<Operation>>;
|
||||
waitForTerminal(): Promise<XtreamOperationWindowSnapshot<Operation>>;
|
||||
}
|
||||
|
||||
export async function installXtreamOperationWindow(
|
||||
page: Page
|
||||
): Promise<XtreamOperationWindow> {
|
||||
export async function installXtreamOperationWindow<
|
||||
Operation extends XtreamObservedOperation = typeof DELETE_OPERATION,
|
||||
>(
|
||||
page: Page,
|
||||
operation: Operation = DELETE_OPERATION as Operation
|
||||
): Promise<XtreamOperationWindow<Operation>> {
|
||||
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<XtreamOperationWindowSnapshot> {
|
||||
const snapshot = await readOperationSnapshot(page);
|
||||
async function assertOperationActive<Operation extends XtreamObservedOperation>(
|
||||
page: Page,
|
||||
operation: Operation
|
||||
): Promise<XtreamOperationWindowSnapshot<Operation>> {
|
||||
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<XtreamOperationWindowSnapshot> {
|
||||
async function readOperationSnapshot<Operation extends XtreamObservedOperation>(
|
||||
page: Page,
|
||||
operation: Operation
|
||||
): Promise<XtreamOperationWindowSnapshot<Operation>> {
|
||||
const value = await page.evaluate(
|
||||
({ stateKey }) => {
|
||||
const target = globalThis as unknown as Record<string, unknown>;
|
||||
@@ -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<void> {
|
||||
);
|
||||
}
|
||||
|
||||
function isOperationSnapshot(
|
||||
value: unknown
|
||||
): value is XtreamOperationWindowSnapshot {
|
||||
function isOperationSnapshot<Operation extends XtreamObservedOperation>(
|
||||
value: unknown,
|
||||
operation: Operation
|
||||
): value is XtreamOperationWindowSnapshot<Operation> {
|
||||
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']) &&
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<HTMLButtonElement>('.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();
|
||||
|
||||
@@ -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"
|
||||
>
|
||||
<mat-icon>close</mat-icon>
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<void>;
|
||||
dismissed: Subject<void>;
|
||||
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<void>();
|
||||
const entry: (typeof toasts)[number] = {
|
||||
message,
|
||||
data,
|
||||
action: new Subject<void>(),
|
||||
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);
|
||||
|
||||
@@ -34,13 +34,18 @@ export interface SourceRemovalOptions {
|
||||
|
||||
interface PendingRemoval {
|
||||
playlist: PlaylistMeta;
|
||||
snackBar: MatSnackBarRef<SourceRemovalToastComponent>;
|
||||
/** Its Undo toast, set once opened. */
|
||||
snackBar?: MatSnackBarRef<SourceRemovalToastComponent>;
|
||||
/** 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<typeof setTimeout> | 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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -28,11 +28,12 @@ function readHistory(): Map<string, SourceHealthHistoryEntry> {
|
||||
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);
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
@@ -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([
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -84,6 +84,24 @@ export function sourceHealthKey(p: Partial<PlaylistMeta>): 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<PlaylistMeta>): string {
|
||||
return fingerprintSourceHealthKey(sourceHealthKey(p));
|
||||
}
|
||||
|
||||
export function accountHealth(
|
||||
status: unknown,
|
||||
expiration?: number,
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user