mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(services): share only the startup inventory read
A pending read was shared with any concurrent caller, but not every playlist write goes through PlaylistsService: the settings reset deletes all playlists through DatabaseService, so a caller could join a read taken before that deletion (Codex review). Share only the first read, which the startup pair (playlist effect and XMLTV reconciliation) needs while the startup screen still hides every writing action; sharing ends when it settles or a PlaylistsService write starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e82d46c051
commit
8515b10cc4
5 files changed
+54
-28
No files matched your search
@@ -184,13 +184,14 @@ harness, which is what the ratchet needs. The main process start
|
||||
`renderer.ipcCallsToFirstCard` counts what the renderer asks of the main
|
||||
process before the first card.
|
||||
|
||||
- `PlaylistsService.getAllPlaylists()` shares one pending SQLite read between
|
||||
concurrent callers: at startup the playlist effect and the XMLTV source
|
||||
reconciliation both read the inventory, and the second caller joins the
|
||||
first read and receives a `structuredClone` of its result. A settled read
|
||||
is never reused, and every SQLite write detaches the pending read, so a
|
||||
caller that follows a write reads again. `dbGetAppPlaylistMetas` before the
|
||||
first card: 2 → 1.
|
||||
- `PlaylistsService.getAllPlaylists()` shares its first SQLite read: at
|
||||
startup the playlist effect and the XMLTV source reconciliation both read
|
||||
the inventory, and the second caller joins the first read and receives a
|
||||
`structuredClone` of its result. Sharing ends when that read settles or a
|
||||
`PlaylistsService` write starts. It is limited to startup on purpose:
|
||||
other services write playlists too (the settings reset deletes them
|
||||
through `DatabaseService`), and while the startup screen is up no such
|
||||
action can run. `dbGetAppPlaylistMetas` before the first card: 2 → 1.
|
||||
- `reconcileEpgSources` stays before the first card on purpose: its
|
||||
completion bumps `EpgSourceSettingsService.revision()`, the fence that
|
||||
keeps XMLTV lookups from returning data of a removed source.
|
||||
|
||||
@@ -38,7 +38,6 @@ describe('Electron legacy playlist migration', () => {
|
||||
dbService,
|
||||
runtime: { supportsSqlite: true },
|
||||
electronMigrationPromise: null,
|
||||
pendingMetas: null,
|
||||
});
|
||||
return { playlists, dbService, electron, service };
|
||||
}
|
||||
|
||||
@@ -18,6 +18,13 @@ describe('PlaylistsService inventory reads', () => {
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
async function until(ready: () => boolean, what: string) {
|
||||
for (let turn = 0; !ready(); turn += 1) {
|
||||
if (turn > 100) throw new Error(`${what} never started`);
|
||||
await Promise.resolve();
|
||||
}
|
||||
}
|
||||
|
||||
function setup() {
|
||||
const reads: ReturnType<typeof deferred<Playlist[]>>[] = [];
|
||||
const electron = {
|
||||
@@ -43,12 +50,11 @@ describe('PlaylistsService inventory reads', () => {
|
||||
dbService: { getAll: jest.fn(() => of([])) },
|
||||
runtime: { supportsSqlite: true },
|
||||
electronMigrationPromise: null,
|
||||
pendingMetas: null,
|
||||
playlistWriteQueues: new Map(),
|
||||
});
|
||||
const settle = async (index: number, playlists: Playlist[]) => {
|
||||
// The read starts after the memoized migration's awaits.
|
||||
while (!reads[index]) await Promise.resolve();
|
||||
await until(() => !!reads[index], `metadata read ${index}`);
|
||||
reads[index].resolve(playlists);
|
||||
};
|
||||
return { electron, service, settle };
|
||||
@@ -85,13 +91,33 @@ describe('PlaylistsService inventory reads', () => {
|
||||
expect(electron.dbGetAppPlaylistMetas).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('stops sharing once the startup read settled', async () => {
|
||||
const { electron, service, settle } = setup();
|
||||
|
||||
const startup = firstValueFrom(service.getAllPlaylists());
|
||||
await settle(0, [source('startup')]);
|
||||
await startup;
|
||||
// Writes that bypass this service (e.g. the settings reset through
|
||||
// DatabaseService) are possible from here on, so every caller reads.
|
||||
const first = firstValueFrom(service.getAllPlaylists());
|
||||
const second = firstValueFrom(service.getAllPlaylists());
|
||||
await settle(1, [source('one')]);
|
||||
await settle(2, [source('two')]);
|
||||
|
||||
await expect(first).resolves.toEqual([source('one')]);
|
||||
await expect(second).resolves.toEqual([source('two')]);
|
||||
expect(electron.dbGetAppPlaylistMetas).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('starts a fresh read for callers that arrive after a write', async () => {
|
||||
const { electron, service, settle } = setup();
|
||||
const added = source('added');
|
||||
|
||||
const staleRead = firstValueFrom(service.getAllPlaylists());
|
||||
while (electron.dbGetAppPlaylistMetas.mock.calls.length === 0)
|
||||
await Promise.resolve();
|
||||
await until(
|
||||
() => electron.dbGetAppPlaylistMetas.mock.calls.length > 0,
|
||||
'metadata read 0'
|
||||
);
|
||||
await firstValueFrom(service.addPlaylist(added));
|
||||
const freshRead = firstValueFrom(service.getAllPlaylists());
|
||||
await settle(0, []);
|
||||
|
||||
@@ -60,7 +60,6 @@ describe('PlaylistsService', () => {
|
||||
},
|
||||
electronMigrationPromise: null,
|
||||
indexedDbMigrationPromise: null,
|
||||
pendingMetas: null,
|
||||
playlistWriteQueues: new Map(),
|
||||
playlistDeleteCleanups: [],
|
||||
});
|
||||
|
||||
@@ -117,9 +117,11 @@ export class PlaylistsService {
|
||||
inject(PLAYLIST_DELETE_CLEANUP, { optional: true }) ?? [];
|
||||
private electronMigrationPromise: Promise<void> | null = null;
|
||||
// Startup reads the inventory twice at once (the playlist effect and the
|
||||
// XMLTV source reconciliation); both share one worker round trip. Only a
|
||||
// pending read is shared, and every SQLite write detaches it.
|
||||
private pendingMetas: Promise<Playlist[]> | null = null;
|
||||
// XMLTV source reconciliation); both share one worker round trip. Only the
|
||||
// first read is shared, while the startup screen still hides every action
|
||||
// that writes playlists (other services write them too, e.g. the settings
|
||||
// reset); it ends when that read settles or a write here starts (null).
|
||||
private startupMetas?: Promise<Playlist[]> | null;
|
||||
private indexedDbMigrationPromise: Promise<void> | null = null;
|
||||
private readonly playlistWriteQueues = new Map<string, Promise<unknown>>();
|
||||
|
||||
@@ -401,7 +403,7 @@ export class PlaylistsService {
|
||||
return playlist;
|
||||
}
|
||||
|
||||
this.pendingMetas = null;
|
||||
this.startupMetas = null;
|
||||
if (operationId === undefined) {
|
||||
await electron.dbUpsertAppPlaylist(playlist);
|
||||
} else {
|
||||
@@ -419,7 +421,7 @@ export class PlaylistsService {
|
||||
return playlists;
|
||||
}
|
||||
|
||||
this.pendingMetas = null;
|
||||
this.startupMetas = null;
|
||||
await electron.dbUpsertAppPlaylists(playlists);
|
||||
playlists.forEach((playlist) => this.healthEvidence?.connections.next({ id: playlist._id, playlist }));
|
||||
return playlists;
|
||||
@@ -498,9 +500,9 @@ export class PlaylistsService {
|
||||
|
||||
getAllPlaylists() {
|
||||
if (this.isElectronStorageAvailable) {
|
||||
const pending = this.pendingMetas;
|
||||
const shared = this.startupMetas;
|
||||
// A joining caller gets its own copy of the shared result.
|
||||
if (pending) return from(pending.then((p) => structuredClone(p)));
|
||||
if (shared) return from(shared.then((p) => structuredClone(p)));
|
||||
const read = firstValueFrom(
|
||||
this.runOnSqlite(async () => {
|
||||
const electron = this.electronApi;
|
||||
@@ -513,12 +515,11 @@ export class PlaylistsService {
|
||||
);
|
||||
})
|
||||
);
|
||||
this.pendingMetas = read;
|
||||
const settle = () => {
|
||||
if (this.pendingMetas === read)
|
||||
this.pendingMetas = null;
|
||||
};
|
||||
read.then(settle, settle);
|
||||
if (shared === undefined) {
|
||||
this.startupMetas = read;
|
||||
const end = () => (this.startupMetas = null);
|
||||
read.then(end, end);
|
||||
}
|
||||
return from(read);
|
||||
}
|
||||
|
||||
@@ -573,7 +574,7 @@ export class PlaylistsService {
|
||||
await this.ensureElectronPlaylistMigrations();
|
||||
const electron = this.electronApi;
|
||||
if (electron) {
|
||||
this.pendingMetas = null;
|
||||
this.startupMetas = null;
|
||||
if (options) {
|
||||
const deleted =
|
||||
await this.databaseService.deletePlaylist(
|
||||
@@ -1345,7 +1346,7 @@ export class PlaylistsService {
|
||||
await this.ensureElectronPlaylistMigrations();
|
||||
const electron = this.electronApi;
|
||||
if (electron) {
|
||||
this.pendingMetas = null;
|
||||
this.startupMetas = null;
|
||||
await electron.dbDeleteAllPlaylists();
|
||||
this.healthEvidence?.connections.next({});
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user