feat(playlist): clean up selected inactive desktop sources (#1593) (#1596)

libs/services/src/index.ts

@@ -28,3 +28,5 @@ export * from './lib/xtream-connection-test.service';
 export * from './lib/xtream-connection-test-state';
 
 export * from './lib/source-health-evidence.service';
+
+export * from './lib/source-activity.service';

libs/services/src/lib/playlist-delete-action.service.spec.ts

@@ -1,10 +1,11 @@
+import { SourceActivityService } from './source-activity.service';
 import {
     EnvironmentInjector,
     Injector,
     createEnvironmentInjector,
     runInInjectionContext,
 } from '@angular/core';
-import { of } from 'rxjs';
+import { of, throwError, Subject } from 'rxjs';
 import { PlaylistMeta } from '@iptvnator/shared/interfaces';
 import { DatabaseService } from './database-electron.service';
 import { PlaylistDeleteActionService } from './playlist-delete-action.service';
@@ -48,6 +49,10 @@ describe('PlaylistDeleteActionService', () => {
 
         injector = createEnvironmentInjector(
             [
+                {
+                    provide: SourceActivityService,
+                    useValue: new SourceActivityService(),
+                },
                 { provide: DatabaseService, useValue: databaseService },
                 { provide: PlaylistsService, useValue: playlistsService },
                 { provide: RuntimeCapabilitiesService, useValue: runtime },
@@ -67,6 +72,26 @@ describe('PlaylistDeleteActionService', () => {
         );
     }
 
+    it('keeps shared deletion activity reserved until persistence and cleanup finish', async () => {
+        const completion = new Subject<{ success: boolean }>();
+        playlistsService.deletePlaylist.mockReturnValue(completion);
+        const activity = injector.get(SourceActivityService);
+        const pending = createService().deletePlaylist(playlist);
+        expect(activity.isBusy(playlist._id)).toBe(true);
+        completion.next({ success: true });
+        await pending;
+        expect(activity.isBusy(playlist._id)).toBe(false);
+    });
+
+    it('preserves the boolean failure contract for single-source callers', async () => {
+        playlistsService.deletePlaylist.mockReturnValue(
+            throwError(() => new Error('worker failed'))
+        );
+        await expect(createService().deletePlaylist(playlist)).resolves.toBe(
+            false
+        );
+    });
+
     it('deletes browser playlists through PlaylistsService', async () => {
         const service = createService();
 
@@ -78,7 +103,7 @@ describe('PlaylistDeleteActionService', () => {
         expect(databaseService.deletePlaylist).not.toHaveBeenCalled();
     });
 
-    it('deletes SQLite-backed Xtream playlists through DatabaseService with progress options', async () => {
+    it('routes SQLite deletion through the serialized owner with progress options', async () => {
         runtime.supportsXtreamSqliteDataSource = true;
         const onEvent = jest.fn();
         const service = createService();
@@ -90,14 +115,14 @@ describe('PlaylistDeleteActionService', () => {
         expect(databaseService.createOperationId).toHaveBeenCalledWith(
             'playlist-delete'
         );
-        expect(databaseService.deletePlaylist).toHaveBeenCalledWith(
+        expect(playlistsService.deletePlaylist).toHaveBeenCalledWith(
             'playlist-1',
             {
                 operationId: 'playlist-delete-1',
                 onEvent,
             }
         );
-        expect(playlistsService.deletePlaylist).not.toHaveBeenCalled();
+        expect(databaseService.deletePlaylist).not.toHaveBeenCalled();
     });
 
     it('deletes SQLite-backed non-Xtream playlists without progress options', async () => {
@@ -112,10 +137,10 @@ describe('PlaylistDeleteActionService', () => {
         ).resolves.toBe(true);
 
         expect(databaseService.createOperationId).not.toHaveBeenCalled();
-        expect(databaseService.deletePlaylist).toHaveBeenCalledWith(
-            'playlist-1',
-            undefined
+        expect(playlistsService.deletePlaylist).toHaveBeenCalledWith(
+            'playlist-1'
         );
+        expect(databaseService.deletePlaylist).not.toHaveBeenCalled();
     });
 
     it('uses browser playlist storage when an Xtream playlist lacks Xtream SQLite support', async () => {

libs/services/src/lib/playlist-delete-action.service.ts

@@ -1,4 +1,4 @@
-import { SourceHealthEvidenceService } from './source-health-evidence.service';
+import { SourceActivityService } from './source-activity.service';
 import { inject, Injectable } from '@angular/core';
 import { firstValueFrom } from 'rxjs';
 import { PlaylistMeta } from '@iptvnator/shared/interfaces';
@@ -20,7 +20,7 @@ export interface PlaylistDeleteActionOptions {
 
 @Injectable({ providedIn: 'root' })
 export class PlaylistDeleteActionService {
-    private readonly healthEvidence = inject(SourceHealthEvidenceService, {
+    private readonly activity = inject(SourceActivityService, {
         optional: true,
     });
     private readonly databaseService = inject(DatabaseService);
@@ -31,42 +31,45 @@ export class PlaylistDeleteActionService {
         playlist: PlaylistMeta,
         options: PlaylistDeleteActionOptions = {}
     ): Promise<boolean> {
-        const supportsElectronDelete = playlist.serverUrl
-            ? this.runtime.supportsXtreamSqliteDataSource
-            : this.runtime.supportsSqlite;
-
-        if (supportsElectronDelete) {
-            const deleted = await this.deletePlaylistInElectron(
+        try {
+            const result = await this.deletePlaylistWithResult(
                 playlist,
                 options
             );
-            if (deleted)
-                this.healthEvidence?.connections.next({ id: playlist._id });
-            return deleted;
+            return result.success;
+        } catch {
+            return false;
         }
-
-        const result = await firstValueFrom(
-            this.playlistsService.deletePlaylist(playlist._id)
-        );
-        return result.success;
     }
 
-    private deletePlaylistInElectron(
+    async deletePlaylistWithResult(
         playlist: PlaylistMeta,
-        options: PlaylistDeleteActionOptions
-    ): Promise<boolean> {
-        const operationId = playlist.serverUrl
-            ? this.databaseService.createOperationId('playlist-delete')
-            : undefined;
-
-        return this.databaseService.deletePlaylist(
-            playlist._id,
-            operationId
+        options: PlaylistDeleteActionOptions = {}
+    ): Promise<{ success: boolean; cleanupWarnings?: number }> {
+        const workerOptions =
+            playlist.serverUrl && this.runtime.supportsXtreamSqliteDataSource
                 ? {
-                      operationId,
+                      operationId:
+                          this.databaseService.createOperationId(
+                              'playlist-delete'
+                          ),
                       onEvent: options.onEvent,
                   }
-                : undefined
-        );
+                : undefined;
+        // Persistence owns serialization, the single worker invocation, and
+        // post-delete cleanup. UI callers only commit the resulting state.
+        const release = this.activity?.begin([playlist._id]);
+        try {
+            return await firstValueFrom(
+                workerOptions
+                    ? this.playlistsService.deletePlaylist(
+                          playlist._id,
+                          workerOptions
+                      )
+                    : this.playlistsService.deletePlaylist(playlist._id)
+            );
+        } finally {
+            release?.();
+        }
     }
 }

libs/services/src/lib/playlists.service.spec.ts

@@ -125,6 +125,61 @@ describe('PlaylistsService', () => {
         expect(cleanup).toHaveBeenCalledWith('playlist-1');
     });
 
+    it('owns one worker deletion and waits for cleanup before completion', async () => {
+        const service = createService();
+        const deleteWorker = jest.fn().mockResolvedValue(true);
+        let finishCleanup!: () => void;
+        const cleanup = jest.fn(
+            () =>
+                new Promise<void>((resolve) => {
+                    finishCleanup = resolve;
+                })
+        );
+        Object.defineProperties(service, {
+            isElectronStorageAvailable: { value: true },
+            electronApi: { value: { dbDeletePlaylist: jest.fn() } },
+        });
+        Object.assign(service, {
+            ensureElectronPlaylistMigrations: jest
+                .fn()
+                .mockResolvedValue(undefined),
+            databaseService: { deletePlaylist: deleteWorker },
+            playlistDeleteCleanups: [cleanup],
+        });
+        let completed = false;
+        const deletion = firstValueFrom(
+            service.deletePlaylist('a', { operationId: 'delete-a' })
+        ).then((result) => {
+            completed = true;
+            return result;
+        });
+        for (let i = 0; i < 20 && !finishCleanup; i++) await Promise.resolve();
+        expect(deleteWorker).toHaveBeenCalledTimes(1);
+        expect(completed).toBe(false);
+        finishCleanup();
+        await expect(deletion).resolves.toEqual({ success: true });
+        expect(cleanup).toHaveBeenCalledTimes(1);
+    });
+
+    it('reports cleanup warnings without resurrecting an already deleted row', async () => {
+        const service = createService();
+        Object.assign(service, {
+            playlistDeleteCleanups: [
+                () => Promise.reject(new Error('cleanup failed')),
+            ],
+        });
+        const warning = jest
+            .spyOn(console, 'warn')
+            .mockImplementation(() => undefined);
+        try {
+            await expect(
+                firstValueFrom(service.deletePlaylist('a'))
+            ).resolves.toEqual({ success: true, cleanupWarnings: 1 });
+        } finally {
+            warning.mockRestore();
+        }
+    });
+
     it('returns browser playlist summaries without embedded playlist payloads', async () => {
         const dbService = {
             getAll: jest.fn(() =>
@@ -982,7 +1037,7 @@ describe('PlaylistsService', () => {
         jest.spyOn(Date, 'now').mockReturnValue(1770000000000);
         // Auto-refresh snapshots always come from playlists that had
         // autoRefresh enabled; the batch write preserves that flag from the
-        // current row (or the snapshot when the row is missing) instead of
+        // current row instead of
         // force-enabling it.
         const playlists = [
             {
@@ -1004,6 +1059,7 @@ describe('PlaylistsService', () => {
         ] as Playlist[];
         const dbService = {
             getAll: jest.fn(() => of([])),
+            getByID: jest.fn((_store: string, id: string) => of(playlists.find((p) => p._id === id))),
             update: jest.fn((_storeName: string, playlist: Playlist) =>
                 of(playlist)
             ),
@@ -1036,6 +1092,20 @@ describe('PlaylistsService', () => {
         );
     });
 
+    it.each([false, true])('does not resurrect a deleted source when startup refresh finishes (SQLite=%s)', async (sqlite) => {
+        const upsert = jest.fn();
+        testWindow.electron = sqlite ? {
+            dbGetAppState: jest.fn().mockResolvedValue('1'), dbSetAppState: jest.fn(),
+            dbGetAppPlaylists: jest.fn().mockResolvedValue([]), dbGetAppPlaylist: jest.fn().mockResolvedValue(null),
+            dbUpsertAppPlaylist: upsert,
+        } : undefined;
+        const update = jest.fn(() => of(undefined));
+        const service = createService({ update });
+        expect(await firstValueFrom(service.updateManyPlaylists([{ _id: 'deleted', autoRefresh: true } as Playlist]))).toEqual([]);
+        expect(update).not.toHaveBeenCalled();
+        expect(upsert).not.toHaveBeenCalled();
+    });
+
     it('short-circuits updateManyPlaylists when no playlists are provided', async () => {
         const dbService = {
             update: jest.fn(),
@@ -1693,6 +1763,19 @@ describe('PlaylistsService', () => {
         );
     });
 
+    it.each([false, true])('does not recreate a removed source on singular refresh (SQLite: %s)', async (sqlite) => {
+        const upsert = jest.fn();
+        testWindow.electron = sqlite ? {
+            dbGetAllPlaylists: jest.fn().mockResolvedValue([]),
+            dbGetAppPlaylist: jest.fn().mockResolvedValue(null),
+            dbUpsertAppPlaylist: upsert,
+        } as unknown as typeof testWindow.electron : undefined;
+        const update = jest.fn(() => of(undefined));
+        const service = createService({ update });
+        await expect(firstValueFrom(service.updatePlaylist('deleted', { _id: 'deleted' } as Playlist))).rejects.toThrow('Playlist no longer exists');
+        expect(update).not.toHaveBeenCalled();
+        expect(upsert).not.toHaveBeenCalled();
+    });
     it('keeps hiddenGroupTitles when refreshing a playlist payload', async () => {
         const existingPlaylist: Playlist = {
             _id: 'playlist-2',

libs/services/src/lib/playlists.service.ts

@@ -1,3 +1,7 @@
+import {
+    DatabaseService,
+    type DbOperationOptions,
+} from './database-electron.service';
 import { SourceHealthEvidenceService } from './source-health-evidence.service';
 import { inject, Injectable } from '@angular/core';
 import { MatSnackBar } from '@angular/material/snack-bar';
@@ -105,6 +109,7 @@ export function resolvePlaylistParser(parserModule: PlaylistParserModule) {
 export class PlaylistsService {
     private readonly healthEvidence = inject(SourceHealthEvidenceService, { optional: true });
     private readonly dbService = inject(NgxIndexedDBService);
+    private readonly databaseService = inject(DatabaseService);
     private readonly snackBar = inject(MatSnackBar);
     private readonly translateService = inject(TranslateService);
     private readonly runtime = inject(RuntimeCapabilitiesService);
@@ -534,7 +539,10 @@ export class PlaylistsService {
         );
     }
 
-    deletePlaylist(playlistId: string): Observable<{ success: boolean }> {
+    deletePlaylist(
+        playlistId: string,
+        options?: DbOperationOptions
+    ): Observable<{ success: boolean; cleanupWarnings?: number }> {
         // Deletion goes through the SAME per-playlist queue as every write:
         // a queued mutation (e.g. the Stalker portal repair's conditional
         // transform) landing after an unserialized delete would upsert the
@@ -547,7 +555,17 @@ export class PlaylistsService {
                         await this.ensureElectronPlaylistMigrations();
                         const electron = this.electronApi;
                         if (electron) {
-                            await electron.dbDeletePlaylist(playlistId);
+                            if (options) {
+                                const deleted =
+                                    await this.databaseService.deletePlaylist(
+                                        playlistId,
+                                        options
+                                    );
+                                if (!deleted)
+                                    throw new Error(
+                                        'Playlist deletion did not complete'
+                                    );
+                            } else await electron.dbDeletePlaylist(playlistId);
                         }
                         return undefined;
                     }
@@ -561,13 +579,19 @@ export class PlaylistsService {
         return delete$.pipe(
             tap(() => this.healthEvidence?.connections.next({ id: playlistId })),
             switchMap(() => from(this.runPlaylistDeleteCleanups(playlistId))),
-            map(() => ({ success: true }))
+            map((cleanupWarnings) =>
+                cleanupWarnings
+                    ? { success: true, cleanupWarnings }
+                    : { success: true }
+            )
         );
     }
 
-    private async runPlaylistDeleteCleanups(playlistId: string): Promise<void> {
+    private async runPlaylistDeleteCleanups(
+        playlistId: string
+    ): Promise<number> {
         if (this.playlistDeleteCleanups.length === 0) {
-            return;
+            return 0;
         }
 
         const failures = (
@@ -589,6 +613,7 @@ export class PlaylistsService {
                 failure
             );
         }
+        return failures.length;
     }
 
     /**
@@ -650,6 +675,7 @@ export class PlaylistsService {
             const currentPlaylist = await firstValueFrom(
                 this.getPlaylistById(playlistId, operationId)
             );
+            if (!currentPlaylist) throw new Error('Playlist no longer exists');
             const mergedPlaylist = this.mergeRefreshedPlaylist(
                 currentPlaylist,
                 updatedPlaylist,
@@ -1004,6 +1030,7 @@ export class PlaylistsService {
                     const current = await firstValueFrom(
                         this.getPlaylistById(playlist._id)
                     );
+                    if (!current) return null;
                     // The merge takes autoRefresh from the current row first,
                     // so disabling auto-refresh while a refresh is in flight
                     // is not reverted by the completing batch write.
@@ -1017,7 +1044,7 @@ export class PlaylistsService {
                     return nextPlaylist;
                 })
             )
-        );
+        ).pipe(map((rows) => rows.filter((row): row is Playlist => row !== null)));
     }
 
     getFavoriteChannels(playlistId: string) {

libs/services/src/lib/source-activity.service.ts

@@ -0,0 +1,32 @@
+import { Injectable, signal } from '@angular/core';
+
+/** Shared renderer work that source cleanup must not interrupt. */
+@Injectable({ providedIn: 'root' })
+export class SourceActivityService {
+    private readonly counts = signal<ReadonlyMap<string, number>>(new Map());
+    isBusy(id: string): boolean {
+        return (this.counts().get(id) ?? 0) > 0;
+    }
+    begin(ids: readonly string[]): () => void {
+        const unique = [...new Set(ids)];
+        this.counts.update((value) => {
+            const next = new Map(value);
+            unique.forEach((id) => next.set(id, (next.get(id) ?? 0) + 1));
+            return next;
+        });
+        let released = false;
+        return () => {
+            if (released) return;
+            released = true;
+            this.counts.update((value) => {
+                const next = new Map(value);
+                unique.forEach((id) => {
+                    const remaining = (next.get(id) ?? 1) - 1;
+                    if (remaining) next.set(id, remaining);
+                    else next.delete(id);
+                });
+                return next;
+            });
+        };
+    }
+}