mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-11 02:46:16 -08:00
fix(portals): serialize destructive Xtream refreshes per playlist
The two refresh entry points cannot see each other: the header action tracks one global flag, the sources row tracks its own id set, and neither disables the other's button. Both can be confirmed for the same playlist inside the delete window, and the second run then collects an already-emptied catalog and parks it over the first run's snapshot — `XtreamPendingRestoreService.set()` overwrites unconditionally — so favorites, history, hidden categories and playback positions are lost on re-import. The race predates this PR: on master the two guards are already independent (`isRefreshing()` against the per-row pending sets), so neither entry point could ever have fixed it alone. Extracting the flow is what makes it fixable once. `XtreamRefreshFlowService` now holds the set of playlists with a run in flight and refuses a second one before it resets the guard, releasing it in the same finally that ends the reporter, so a finished run never strands a playlist. Found by Greptile on #1431. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e3fc3c8802
commit
5165300981
2 files changed
+82
-1
No files matched your search
@@ -168,6 +168,69 @@ describe('XtreamRefreshFlowService', () => {
|
||||
expect(dataService.sendIpcEvent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('serializes destructive runs for one playlist across entry points', async () => {
|
||||
const item = createPlaylistMeta();
|
||||
const confirms: Array<Promise<void> | undefined> = [];
|
||||
let deleteStarted!: () => void;
|
||||
const deleteHasStarted = new Promise<void>((resolve) => {
|
||||
deleteStarted = resolve;
|
||||
});
|
||||
let releaseDelete!: (state: unknown) => void;
|
||||
const deleteSettles = new Promise((resolve) => {
|
||||
releaseDelete = resolve;
|
||||
});
|
||||
|
||||
databaseService.deleteXtreamPlaylistContent.mockImplementation(() => {
|
||||
order.push('delete');
|
||||
deleteStarted();
|
||||
return deleteSettles;
|
||||
});
|
||||
dialogService.openConfirmDialog.mockImplementation(
|
||||
({ onConfirm }: { onConfirm?: () => Promise<void> }) => {
|
||||
confirms.push(onConfirm?.());
|
||||
}
|
||||
);
|
||||
|
||||
const header = createReporter();
|
||||
const sourcesRow = createReporter();
|
||||
|
||||
service.confirmAndRefresh(item, header.reporter);
|
||||
await deleteHasStarted;
|
||||
|
||||
// The row's reporter honestly reports "not busy": it tracks its own id
|
||||
// set and never saw the header action start. Only the flow can refuse
|
||||
// this, and it must refuse before the guard reset — a second run would
|
||||
// park an already-emptied catalog over the first run's snapshot.
|
||||
service.confirmAndRefresh(item, sourcesRow.reporter);
|
||||
await Promise.resolve();
|
||||
|
||||
expect(sourcesRow.calls).toEqual([]);
|
||||
expect(
|
||||
databaseService.deleteXtreamPlaylistContent
|
||||
).toHaveBeenCalledTimes(1);
|
||||
expect(dataService.sendIpcEvent).toHaveBeenCalledTimes(1);
|
||||
|
||||
releaseDelete({
|
||||
success: true,
|
||||
favorites: [],
|
||||
recentlyViewed: [],
|
||||
hiddenCategories: [],
|
||||
});
|
||||
await Promise.all(confirms);
|
||||
|
||||
expect(header.calls).toEqual(['begin', 'end']);
|
||||
|
||||
// The block lifts with the run: a refresh that already finished must
|
||||
// not strand the playlist.
|
||||
service.confirmAndRefresh(item, sourcesRow.reporter);
|
||||
await confirms[confirms.length - 1];
|
||||
|
||||
expect(sourcesRow.calls).toEqual(['begin', 'end']);
|
||||
expect(
|
||||
databaseService.deleteXtreamPlaylistContent
|
||||
).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('marks the run busy before the guard reset and any destructive work', async () => {
|
||||
const { reporter, runs } = createReporter({
|
||||
begin: (run) => {
|
||||
|
||||
@@ -80,6 +80,19 @@ export class XtreamRefreshFlowService {
|
||||
XtreamPendingRestoreService
|
||||
);
|
||||
|
||||
/**
|
||||
* Playlists with a destructive run in flight, across every entry point.
|
||||
*
|
||||
* A reporter only ever knows its own: the header action tracks one global
|
||||
* flag, the sources row tracks its own id set, and neither disables the
|
||||
* other's button. Both can therefore be confirmed for the same playlist
|
||||
* inside the delete window, and the second run would collect an
|
||||
* already-emptied catalog and park it over the first run's snapshot —
|
||||
* losing favorites, history, hidden categories and playback positions on
|
||||
* re-import. This service is the only place that sees both.
|
||||
*/
|
||||
private readonly activeRefreshPlaylistIds = new Set<string>();
|
||||
|
||||
/**
|
||||
* Asks for confirmation, then deletes and re-imports the playlist. Callers
|
||||
* keep their own entry-point guard (a disabled button, a pending row) —
|
||||
@@ -105,7 +118,10 @@ export class XtreamRefreshFlowService {
|
||||
item: PlaylistMeta,
|
||||
reporter: XtreamRefreshProgressReporter
|
||||
): Promise<void> {
|
||||
if (reporter.isBusy(item._id)) {
|
||||
if (
|
||||
this.activeRefreshPlaylistIds.has(item._id) ||
|
||||
reporter.isBusy(item._id)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -113,6 +129,7 @@ export class XtreamRefreshFlowService {
|
||||
this.databaseService.createOperationId('xtream-refresh');
|
||||
const run: XtreamRefreshRun = { playlistId: item._id, operationId };
|
||||
reporter.begin(run);
|
||||
this.activeRefreshPlaylistIds.add(item._id);
|
||||
|
||||
try {
|
||||
// Before anything destructive: this deletes the cached catalog and
|
||||
@@ -186,6 +203,7 @@ export class XtreamRefreshFlowService {
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
this.activeRefreshPlaylistIds.delete(item._id);
|
||||
reporter.end(run);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user