fix(playlists): serialize per-playlist collection writes to prevent lost updates (#1255)

* fix(playlists): serialize per-playlist collection writes to prevent lost updates All per-playlist mutations (portal favorites, recently viewed, playlist meta/favorites updates) used an uncoordinated read -> patch -> replace-whole-row pattern, so two overlapping mutations on the same playlist were last-write-wins and silently dropped each other's changes (flagged by Greptile on PR #1253). Chain every read-modify-write through a per-playlist promise queue (Map<playlistId, Promise>) inside defer(), covering both the SQLite upsert and IndexedDB update paths while keeping the Observable-based public API, laziness, and emitted values unchanged. A failed mutation does not wedge the queue, and different playlists are not serialized against each other. Regression coverage: overlapping favorite+recently-viewed adds, two rapid favorite adds, IndexedDB-path overlap (all three fail on the old code), plus queue-continues-after-error and cross-playlist independence guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playlists): close serialization gaps flagged by review bots Greptile P1 (writers bypassing the queue): route updateManyPlaylists and updatePlaylistPositions through the same per-playlist write queue. Auto-refresh batch writes now re-read the stored row inside the queue and preserve user-owned fields (favorites, recently viewed, position) instead of writing a pre-refresh snapshot over them; position updates re-read and patch inside the queue on both storage paths. Codex P1 (callers precompute stale snapshots): add an atomic PlaylistsService.transformPlaylistFavorites(playlistId, transform) that applies the favorites transform to the freshly-read row inside the queue, and convert every read-then-set call site to it: UnifiedFavoritesDataService M3U add/remove/clear/reorder and Stalker reorder/clear, GlobalFavoritesService M3U removal, DashboardDataService M3U removal. Reorders now keep concurrently added favorites (appended after the dragged order) instead of dropping them. New coverage: overlapping favorites transforms, auto-refresh batch write vs queued favorite add, position update vs queued favorite add, and a public addFavorite race through UnifiedFavoritesDataService; existing specs updated to the transform-based contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playlists): share the canonical refresh merge with the auto-refresh batch The batch path previously spread the stale refresh snapshot over the freshly read row and pinned only favorites/recently-viewed/position, so a queued metadata mutation (hiddenGroupTitles, curated EPG sources) finishing before the refresh write could be reverted. Extract updatePlaylist's merge into mergeRefreshedPlaylist() and use it for both the single-playlist update flow and updateManyPlaylists: refresh-owned data (parsed content, count, EPG detection) comes from the payload, user-owned state comes from the current row, and manual/disabled EPG configuration is resolved through resolvePlaylistEpgSourceState instead of being overwritten. Regression test: queued hiddenGroupTitles meta update overlapping an auto-refresh batch write keeps both the metadata change and the refreshed content, including preserved manualEpgUrls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playlists): let the current row decide autoRefresh during batch refresh The batch path force-set autoRefresh: true after the merge, so disabling auto-refresh while a refresh was in flight was reverted by the completing write. Drop the override — mergeRefreshedPlaylist already prefers the current row's autoRefresh over the snapshot — and add a count fallback to the snapshot value for rows without a stored copy. Regression test: disabling auto-refresh concurrently with the batch write keeps autoRefresh false while still applying the refreshed content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-10-08 17:06:15 -08:00 · 2026-07-25 19:24:42 +02:00
1 parent 270350c2e1
commit 6bdd6fd8a8
@@ -1,7 +1,7 @@
 import { TestBed } from '@angular/core/testing';
 import { Store } from '@ngrx/store';
 import { TranslateService } from '@ngx-translate/core';
-import { of } from 'rxjs';
+import { firstValueFrom, from, of } from 'rxjs';
 import { DatabaseService, PlaylistsService } from '@iptvnator/services';
 import {
    Channel,
@@ -49,8 +49,7 @@ describe('UnifiedFavoritesDataService', () => {
    let playlistsService: {
        addPortalFavorite: jest.Mock;
        getPlaylistById: jest.Mock;
-        setFavorites: jest.Mock;
-        setPortalFavorites: jest.Mock;
+        transformPlaylistFavorites: jest.Mock;
    };

    const m3uChannels: Channel[] = [
@@ -134,8 +133,31 @@ describe('UnifiedFavoritesDataService', () => {
        playlistsService = {
            addPortalFavorite: jest.fn().mockReturnValue(of({})),
            getPlaylistById: jest.fn(),
-            setFavorites: jest.fn().mockReturnValue(of({})),
-            setPortalFavorites: jest.fn().mockReturnValue(of({})),
+            // Faithful default: read the playlist through the configured
+            // getPlaylistById mock and apply the transform to its favorites,
+            // like the real queued PlaylistsService implementation.
+            transformPlaylistFavorites: jest.fn(
+                (
+                    playlistId: string,
+                    transform: (current: unknown[]) => unknown[]
+                ) =>
+                    from(
+                        (async () => {
+                            const playlist = (await firstValueFrom(
+                                playlistsService.getPlaylistById(playlistId) ??
+                                    of(undefined)
+                            )) as Partial<Playlist> | undefined;
+                            const current = Array.isArray(playlist?.favorites)
+                                ? playlist.favorites
+                                : [];
+                            return {
+                                ...playlist,
+                                _id: playlistId,
+                                favorites: transform(current),
+                            };
+                        })()
+                    )
+            ),
        };
        databaseService = {
            getAllGlobalFavorites: jest.fn().mockResolvedValue([]),
@@ -408,7 +430,7 @@ describe('UnifiedFavoritesDataService', () => {
        ]);
    });

-    it('persists M3U playlist reorders through setFavorites', async () => {
+    it('persists M3U playlist reorders through an atomic favorites transform', async () => {
        const reorderedItems = [
            {
                uid: 'm3u::m3u-1::https://example.com/2.m3u8',
@@ -438,13 +460,27 @@ describe('UnifiedFavoritesDataService', () => {
            portalType: 'm3u',
        });

-        expect(playlistsService.setFavorites).toHaveBeenCalledWith('m3u-1', [
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledWith('m3u-1', expect.any(Function));
+        const [, transform] =
+            playlistsService.transformPlaylistFavorites.mock.calls[0];
+        // The reordered ids come first; favorites added concurrently (not part
+        // of the drag list) are preserved at the end instead of being dropped.
+        expect(
+            transform([
+                'https://example.com/1.m3u8',
+                'https://example.com/2.m3u8',
+                'https://example.com/concurrent.m3u8',
+            ])
+        ).toEqual([
            'https://example.com/2.m3u8',
            'https://example.com/1.m3u8',
+            'https://example.com/concurrent.m3u8',
        ]);
    });

-    it('adds M3U favorites through setFavorites without duplicating existing entries', async () => {
+    it('adds M3U favorites through an atomic transform without duplicating existing entries', async () => {
        playlistsService.getPlaylistById.mockReturnValue(
            of({
                _id: 'm3u-1',
@@ -463,10 +499,9 @@ describe('UnifiedFavoritesDataService', () => {
            channelId: 'new-channel',
        } satisfies UnifiedCollectionItem);

-        expect(playlistsService.setFavorites).toHaveBeenCalledWith('m3u-1', [
-            'https://example.com/existing.m3u8',
-            'https://example.com/new.m3u8',
-        ]);
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledWith('m3u-1', expect.any(Function));
        expect(store.dispatch).toHaveBeenCalledWith(
            expect.objectContaining({
                type: '[Playlists] Update Playlist Meta',
@@ -490,7 +525,69 @@ describe('UnifiedFavoritesDataService', () => {
            streamUrl: 'https://example.com/existing.m3u8',
        } satisfies UnifiedCollectionItem);

-        expect(playlistsService.setFavorites).toHaveBeenCalledTimes(1);
+        // The duplicate add still runs atomically, but its transform leaves
+        // the favorites unchanged.
+        const [, duplicateTransform] =
+            playlistsService.transformPlaylistFavorites.mock.calls[1];
+        expect(
+            duplicateTransform(['https://example.com/existing.m3u8'])
+        ).toEqual(['https://example.com/existing.m3u8']);
+    });
+
+    it('does not lose a rapid favorite toggle through the public M3U add flow', async () => {
+        const backingStore = {
+            favorites: ['https://example.com/existing.m3u8'],
+        };
+        let queue: Promise<unknown> = Promise.resolve();
+        playlistsService.transformPlaylistFavorites.mockImplementation(
+            (
+                playlistId: string,
+                transform: (current: string[]) => string[]
+            ) => {
+                // Emulate the real queued read-transform-write semantics.
+                const run = queue.then(async () => {
+                    await Promise.resolve();
+                    backingStore.favorites = transform(backingStore.favorites);
+                    return {
+                        _id: playlistId,
+                        favorites: backingStore.favorites,
+                    };
+                });
+                queue = run.then(
+                    () => undefined,
+                    () => undefined
+                );
+                return from(run);
+            }
+        );
+
+        const baseItem = {
+            contentType: 'live',
+            sourceType: 'm3u',
+            playlistId: 'm3u-1',
+            playlistName: 'M3U List',
+        } as const;
+
+        await Promise.all([
+            service.addFavorite({
+                ...baseItem,
+                uid: 'm3u::m3u-1::https://example.com/a.m3u8',
+                name: 'Channel A',
+                streamUrl: 'https://example.com/a.m3u8',
+            } satisfies UnifiedCollectionItem),
+            service.addFavorite({
+                ...baseItem,
+                uid: 'm3u::m3u-1::https://example.com/b.m3u8',
+                name: 'Channel B',
+                streamUrl: 'https://example.com/b.m3u8',
+            } satisfies UnifiedCollectionItem),
+        ]);
+
+        expect(backingStore.favorites).toEqual([
+            'https://example.com/existing.m3u8',
+            'https://example.com/a.m3u8',
+            'https://example.com/b.m3u8',
+        ]);
    });

    it('adds Xtream favorites after resolving the content id', async () => {
@@ -605,7 +702,7 @@ describe('UnifiedFavoritesDataService', () => {
        );
    });

-    it('persists Stalker playlist reorders through setPortalFavorites', async () => {
+    it('persists Stalker playlist reorders through an atomic favorites transform', async () => {
        playlistsService.getPlaylistById.mockReturnValue(
            of({
                _id: 'stalker-1',
@@ -642,10 +739,15 @@ describe('UnifiedFavoritesDataService', () => {
            }
        );

-        expect(playlistsService.setPortalFavorites).toHaveBeenCalledWith(
-            'stalker-1',
-            [stalkerFavorites[1], stalkerFavorites[0]]
-        );
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledWith('stalker-1', expect.any(Function));
+        const [, transform] =
+            playlistsService.transformPlaylistFavorites.mock.calls[0];
+        expect(transform(stalkerFavorites)).toEqual([
+            stalkerFavorites[1],
+            stalkerFavorites[0],
+        ]);
    });

    it('clears M3U favorites once per playlist with the remaining favorites preserved', async () => {
@@ -683,10 +785,21 @@ describe('UnifiedFavoritesDataService', () => {
            },
        ] satisfies UnifiedCollectionItem[]);

-        expect(playlistsService.setFavorites).toHaveBeenCalledTimes(1);
-        expect(playlistsService.setFavorites).toHaveBeenCalledWith('m3u-1', [
-            'https://example.com/3.m3u8',
-        ]);
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledTimes(1);
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledWith('m3u-1', expect.any(Function));
+        expect(store.dispatch).toHaveBeenCalledWith(
+            expect.objectContaining({
+                type: '[Playlists] Update Playlist Meta',
+                playlist: expect.objectContaining({
+                    _id: 'm3u-1',
+                    favorites: ['https://example.com/3.m3u8'],
+                }),
+            })
+        );
    });

    it('clears Stalker favorites once per playlist with the remaining favorites preserved', async () => {
@@ -727,11 +840,15 @@ describe('UnifiedFavoritesDataService', () => {
            },
        ] satisfies UnifiedCollectionItem[]);

-        expect(playlistsService.setPortalFavorites).toHaveBeenCalledTimes(1);
-        expect(playlistsService.setPortalFavorites).toHaveBeenCalledWith(
-            'stalker-1',
-            [remainingFavorite]
-        );
+        expect(
+            playlistsService.transformPlaylistFavorites
+        ).toHaveBeenCalledTimes(1);
+        const [transformedPlaylistId, transform] =
+            playlistsService.transformPlaylistFavorites.mock.calls[0];
+        expect(transformedPlaylistId).toBe('stalker-1');
+        expect(transform([...stalkerFavorites, remainingFavorite])).toEqual([
+            remainingFavorite,
+        ]);
    });

    it('clears Xtream favorites through the bulk removal path', async () => {
@@ -82,17 +82,13 @@ export class UnifiedFavoritesDataService {
    async removeFavorite(item: UnifiedCollectionItem): Promise<void> {
        switch (item.sourceType) {
            case 'm3u': {
-                const playlist = await firstValueFrom(
-                    this.playlistsService.getPlaylistById(item.playlistId)
+                await this.transformM3uFavorites(item.playlistId, (current) =>
+                    current.filter(
+                        (favoriteId) =>
+                            favoriteId !== item.streamUrl &&
+                            favoriteId !== item.channelId
+                    )
                );
-                const filtered = (
-                    (playlist.favorites as string[]) ?? []
-                ).filter(
-                    (favoriteId) =>
-                        favoriteId !== item.streamUrl &&
-                        favoriteId !== item.channelId
-                );
-                await this.setM3uFavorites(item.playlistId, filtered);
                break;
            }
            case 'xtream': {
@@ -132,23 +128,9 @@ export class UnifiedFavoritesDataService {
            return;
        }

-        const playlist = (await firstValueFrom(
-            this.playlistsService.getPlaylistById(item.playlistId)
-        )) as Playlist | undefined;
-        const currentFavorites = Array.isArray(playlist?.favorites)
-            ? playlist.favorites.filter(
-                  (favorite): favorite is string => typeof favorite === 'string'
-              )
-            : [];
-
-        if (currentFavorites.includes(favoriteId)) {
-            return;
-        }
-
-        await this.setM3uFavorites(item.playlistId, [
-            ...currentFavorites,
-            favoriteId,
-        ]);
+        await this.transformM3uFavorites(item.playlistId, (current) =>
+            current.includes(favoriteId) ? current : [...current, favoriteId]
+        );
    }

    private async addXtreamFavorite(
@@ -249,12 +231,14 @@ export class UnifiedFavoritesDataService {
            options.playlistId &&
            options.portalType === 'm3u'
        ) {
-            await this.setM3uFavorites(
-                options.playlistId,
-                items
-                    .map((item) => item.streamUrl ?? item.channelId ?? '')
-                    .filter((value) => value.length > 0)
-            );
+            const orderedIds = items
+                .map((item) => item.streamUrl ?? item.channelId ?? '')
+                .filter((value) => value.length > 0);
+            const orderedIdSet = new Set(orderedIds);
+            await this.transformM3uFavorites(options.playlistId, (current) => [
+                ...orderedIds,
+                ...current.filter((favorite) => !orderedIdSet.has(favorite)),
+            ]);
            return;
        }

@@ -263,32 +247,43 @@ export class UnifiedFavoritesDataService {
            options.playlistId &&
            options.portalType === 'stalker'
        ) {
-            const playlist = (await firstValueFrom(
-                this.playlistsService.getPlaylistById(options.playlistId)
-            )) as Playlist | undefined;
-            const currentFavorites = Array.isArray(playlist?.favorites)
-                ? playlist.favorites.filter(isStalkerItem)
-                : [];
-            const favoritesById = new Map(
-                currentFavorites.map((favorite) => [
-                    this.getStalkerFavoriteId(favorite),
-                    favorite,
-                ])
-            );
-            const reorderedFavorites = items
-                .map(
-                    (item) =>
-                        favoritesById.get(this.getStalkerFavoriteId(item)) ??
-                        null
-                )
-                .filter(
-                    (favorite): favorite is StalkerPortalItem =>
-                        favorite !== null
-                );
            await firstValueFrom(
-                this.playlistsService.setPortalFavorites(
+                this.playlistsService.transformPlaylistFavorites(
                    options.playlistId,
-                    reorderedFavorites
+                    (current) => {
+                        const currentFavorites = current.filter(isStalkerItem);
+                        const favoritesById = new Map(
+                            currentFavorites.map((favorite) => [
+                                this.getStalkerFavoriteId(favorite),
+                                favorite,
+                            ])
+                        );
+                        const reorderedFavorites = items
+                            .map(
+                                (item) =>
+                                    favoritesById.get(
+                                        this.getStalkerFavoriteId(item)
+                                    ) ?? null
+                            )
+                            .filter(
+                                (favorite): favorite is StalkerPortalItem =>
+                                    favorite !== null
+                            );
+                        const reorderedIds = new Set(
+                            reorderedFavorites.map((favorite) =>
+                                this.getStalkerFavoriteId(favorite)
+                            )
+                        );
+                        return [
+                            ...reorderedFavorites,
+                            ...currentFavorites.filter(
+                                (favorite) =>
+                                    !reorderedIds.has(
+                                        this.getStalkerFavoriteId(favorite)
+                                    )
+                            ),
+                        ];
+                    }
                )
            );
            return;
@@ -334,9 +329,6 @@ export class UnifiedFavoritesDataService {
        await Promise.all(
            Array.from(groupedItems.entries()).map(
                async ([playlistId, playlistItems]) => {
-                    const playlist = (await firstValueFrom(
-                        this.playlistsService.getPlaylistById(playlistId)
-                    )) as Playlist | undefined;
                    const targetIds = new Set<string>();
                    playlistItems.forEach((item) => {
                        [item.streamUrl, item.channelId].forEach((value) => {
@@ -346,17 +338,12 @@ export class UnifiedFavoritesDataService {
                            }
                        });
                    });
-                    const currentFavorites = Array.isArray(playlist?.favorites)
-                        ? playlist.favorites.filter(
-                              (favorite): favorite is string =>
-                                  typeof favorite === 'string'
-                          )
-                        : [];
-                    const nextFavorites = currentFavorites.filter(
-                        (favorite) => !targetIds.has(favorite.trim())
-                    );

-                    await this.setM3uFavorites(playlistId, nextFavorites);
+                    await this.transformM3uFavorites(playlistId, (current) =>
+                        current.filter(
+                            (favorite) => !targetIds.has(favorite.trim())
+                        )
+                    );
                }
            )
        );
@@ -409,26 +396,26 @@ export class UnifiedFavoritesDataService {
        await Promise.all(
            Array.from(groupedItems.entries()).map(
                async ([playlistId, playlistItems]) => {
-                    const playlist = (await firstValueFrom(
-                        this.playlistsService.getPlaylistById(playlistId)
-                    )) as Playlist | undefined;
                    const targetIds = new Set(
                        playlistItems.map((item) =>
                            this.getStalkerFavoriteId(item)
                        )
                    );
-                    const currentFavorites = Array.isArray(playlist?.favorites)
-                        ? playlist.favorites.filter(isStalkerItem)
-                        : [];
-                    const nextFavorites = currentFavorites.filter(
-                        (favorite) =>
-                            !targetIds.has(this.getStalkerFavoriteId(favorite))
-                    );

                    await firstValueFrom(
-                        this.playlistsService.setPortalFavorites(
+                        this.playlistsService.transformPlaylistFavorites(
                            playlistId,
-                            nextFavorites
+                            (current) =>
+                                current
+                                    .filter(isStalkerItem)
+                                    .filter(
+                                        (favorite) =>
+                                            !targetIds.has(
+                                                this.getStalkerFavoriteId(
+                                                    favorite
+                                                )
+                                            )
+                                    )
                        )
                    );
                }
@@ -758,18 +745,27 @@ export class UnifiedFavoritesDataService {
        }
    }

-    private async setM3uFavorites(
+    private async transformM3uFavorites(
        playlistId: string,
-        favorites: string[]
+        transform: (currentFavorites: string[]) => string[]
    ): Promise<void> {
-        await firstValueFrom(
-            this.playlistsService.setFavorites(playlistId, favorites)
+        const updatedPlaylist = await firstValueFrom(
+            this.playlistsService.transformPlaylistFavorites(
+                playlistId,
+                (current) =>
+                    transform(
+                        current.filter(
+                            (favorite): favorite is string =>
+                                typeof favorite === 'string'
+                        )
+                    )
+            )
        );
        this.store.dispatch(
            PlaylistActions.updatePlaylistMeta({
                playlist: {
                    _id: playlistId,
-                    favorites,
+                    favorites: updatedPlaylist.favorites,
                } as PlaylistMeta,
            })
        );