mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-10 18:36:15 -08:00
fix(downloads): fail closed on stale episode state
This commit is contained in:
1 parent
e977791126
commit
d26df7acb4
11 files changed
+145
-43
No files matched your search
@@ -943,7 +943,10 @@ engine` (restart required) or
|
||||
for provider URLs, headers, and metadata; the backend still runs one active
|
||||
transfer with a FIFO queue. `DOWNLOADS_START` remains the sole start IPC; its
|
||||
stable `reason: 'already-in-progress'` result is counted as skipped, and no
|
||||
batch IPC or schema migration is introduced.
|
||||
batch IPC or schema migration is introduced. Episode and season download
|
||||
actions require the latest global list request to have succeeded; a failed
|
||||
refresh leaves loading/empty-state resolution intact but disables starts
|
||||
until an authoritative snapshot arrives.
|
||||
- Episode ownership uses normalized `episode.id` as the canonical `xtreamId`
|
||||
for both providers; Stalker playback identifiers only resolve the URL. Exact
|
||||
`(playlistId, contentType, xtreamId)` matches are authoritative, while
|
||||
|
||||
@@ -55,10 +55,14 @@ variants, contextual buttons, and theme-aware styling.
|
||||
`loadDownloads()` therefore always invokes the Electron list IPC without its
|
||||
legacy optional playlist scope. Route scope, category, and search must never
|
||||
replace or narrow that signal. Overlapping loads are request-ordered so a
|
||||
late response cannot replace a newer snapshot. Before each fresh download or
|
||||
resume the service asks the main process for an authorized folder and calls
|
||||
the corresponding IPC command. The `onDownloadsUpdate` broadcast triggers a
|
||||
new global load.
|
||||
late response cannot replace a newer snapshot. `hasLoadedDownloads` records
|
||||
that the latest attempt completed, including an error, while
|
||||
`hasAuthoritativeDownloadList` is true only after the latest request
|
||||
succeeds. Series download actions require both so a failed refresh cannot
|
||||
restart rows missing from a stale or empty renderer snapshot. Before each
|
||||
fresh download or resume the service asks the main process for an authorized
|
||||
folder and calls the corresponding IPC command. The `onDownloadsUpdate`
|
||||
broadcast triggers a new global load.
|
||||
- **Series season queueing**
|
||||
`SeasonDownloadCoordinator` owns synchronous, per-identity pending
|
||||
reservations and submits an individual episode or selected-season snapshot
|
||||
|
||||
+12
-2
@@ -12,6 +12,7 @@ import { SeasonDownloadCoordinator } from './season-download-coordinator.service
|
||||
|
||||
interface DownloadsServiceStub {
|
||||
readonly downloads: WritableSignal<DownloadItem[]>;
|
||||
readonly hasAuthoritativeDownloadList: WritableSignal<boolean>;
|
||||
readonly hasLoadedDownloads: WritableSignal<boolean>;
|
||||
readonly isAvailable: WritableSignal<boolean>;
|
||||
readonly startDownload: jest.MockedFunction<
|
||||
@@ -44,6 +45,7 @@ describe('SeasonDownloadCoordinator', () => {
|
||||
beforeEach(() => {
|
||||
downloadsService = {
|
||||
downloads: signal<DownloadItem[]>([]),
|
||||
hasAuthoritativeDownloadList: signal(true),
|
||||
hasLoadedDownloads: signal(true),
|
||||
isAvailable: signal(true),
|
||||
startDownload: jest.fn().mockResolvedValue({ success: true }),
|
||||
@@ -210,10 +212,18 @@ describe('SeasonDownloadCoordinator', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('blocks eligibility and reservation until downloads have loaded', async () => {
|
||||
downloadsService.hasLoadedDownloads.set(false);
|
||||
it('blocks eligibility and reservation without an authoritative download list', async () => {
|
||||
downloadsService.hasAuthoritativeDownloadList.set(false);
|
||||
const item = candidate(FIRST_IDENTITY);
|
||||
|
||||
expect(downloadsService.hasLoadedDownloads()).toBe(true);
|
||||
expect(coordinator.isEligible(item)).toBe(false);
|
||||
await expect(coordinator.enqueueOne(item)).resolves.toBe('skipped');
|
||||
expect(item.prepare).not.toHaveBeenCalled();
|
||||
|
||||
downloadsService.hasAuthoritativeDownloadList.set(true);
|
||||
downloadsService.hasLoadedDownloads.set(false);
|
||||
|
||||
expect(coordinator.isEligible(item)).toBe(false);
|
||||
await expect(coordinator.enqueueOne(item)).resolves.toBe('skipped');
|
||||
expect(item.prepare).not.toHaveBeenCalled();
|
||||
|
||||
+1
@@ -36,6 +36,7 @@ export class SeasonDownloadCoordinator {
|
||||
isEligible(candidate: EpisodeDownloadCandidate): boolean {
|
||||
return (
|
||||
this.downloadsService.isAvailable() &&
|
||||
this.downloadsService.hasAuthoritativeDownloadList() &&
|
||||
this.downloadsService.hasLoadedDownloads() &&
|
||||
!this.isPending(candidate.identity) &&
|
||||
isEpisodeDownloadEligible(this.findDownload(candidate.identity))
|
||||
|
||||
@@ -35,15 +35,27 @@ function row(
|
||||
describe('episode download identity', () => {
|
||||
it('prefers a canonical match over an earlier coordinate fallback', () => {
|
||||
const coordinateFallback = row({ xtreamId: 999 });
|
||||
const canonical = row({
|
||||
const canonical = row();
|
||||
|
||||
expect(
|
||||
findEpisodeDownload(identity, [coordinateFallback, canonical])
|
||||
).toBe(canonical);
|
||||
});
|
||||
|
||||
it('fails closed when a canonical match has conflicting complete coordinates', () => {
|
||||
const coordinateFallback = row({ xtreamId: 999 });
|
||||
const conflictingCanonical = row({
|
||||
seriesXtreamId: 50,
|
||||
seasonNumber: 4,
|
||||
episodeNumber: 8,
|
||||
});
|
||||
|
||||
expect(
|
||||
findEpisodeDownload(identity, [coordinateFallback, canonical])
|
||||
).toBe(canonical);
|
||||
findEpisodeDownload(identity, [
|
||||
coordinateFallback,
|
||||
conflictingCanonical,
|
||||
])
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to complete matching episode coordinates', () => {
|
||||
|
||||
@@ -26,6 +26,29 @@ export interface EpisodeDownloadRecord {
|
||||
readonly filePath?: string;
|
||||
}
|
||||
|
||||
function hasConflictingCompleteCoordinates(
|
||||
download: EpisodeDownloadRecord,
|
||||
identity: EpisodeDownloadIdentity
|
||||
): boolean {
|
||||
const { seriesXtreamId, seasonNumber, episodeNumber } = download;
|
||||
if (
|
||||
seriesXtreamId === undefined ||
|
||||
seasonNumber === undefined ||
|
||||
episodeNumber === undefined
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return (
|
||||
!Number.isSafeInteger(seriesXtreamId) ||
|
||||
!Number.isSafeInteger(seasonNumber) ||
|
||||
!Number.isSafeInteger(episodeNumber) ||
|
||||
seriesXtreamId !== identity.seriesXtreamId ||
|
||||
seasonNumber !== identity.seasonNumber ||
|
||||
episodeNumber !== identity.episodeNumber
|
||||
);
|
||||
}
|
||||
|
||||
export function findEpisodeDownload<T extends EpisodeDownloadRecord>(
|
||||
identity: EpisodeDownloadIdentity,
|
||||
downloads: readonly T[]
|
||||
@@ -37,7 +60,9 @@ export function findEpisodeDownload<T extends EpisodeDownloadRecord>(
|
||||
download.xtreamId === identity.xtreamId
|
||||
);
|
||||
if (canonicalMatch) {
|
||||
return canonicalMatch;
|
||||
return hasConflictingCompleteCoordinates(canonicalMatch, identity)
|
||||
? undefined
|
||||
: canonicalMatch;
|
||||
}
|
||||
|
||||
return downloads.find(
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import { signal } from '@angular/core';
|
||||
|
||||
export class DownloadListLoadState {
|
||||
private requestId = 0;
|
||||
private readonly loading = signal(false);
|
||||
private readonly loaded = signal(false);
|
||||
private readonly authoritative = signal(false);
|
||||
|
||||
readonly isLoading = this.loading.asReadonly();
|
||||
readonly hasLoaded = this.loaded.asReadonly();
|
||||
readonly hasAuthoritativeList = this.authoritative.asReadonly();
|
||||
|
||||
begin(): number {
|
||||
this.loading.set(true);
|
||||
this.authoritative.set(false);
|
||||
return ++this.requestId;
|
||||
}
|
||||
|
||||
isLatest(requestId: number): boolean {
|
||||
return requestId === this.requestId;
|
||||
}
|
||||
|
||||
markSucceeded(): void {
|
||||
this.authoritative.set(true);
|
||||
this.loaded.set(true);
|
||||
}
|
||||
|
||||
markFailed(): void {
|
||||
this.loaded.set(true);
|
||||
}
|
||||
|
||||
finish(): void {
|
||||
this.loading.set(false);
|
||||
}
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
ElectronBridgeDownloadStartPayload,
|
||||
ElectronBridgeDownloadStartResult,
|
||||
} from '@iptvnator/shared/interfaces';
|
||||
import { DownloadListLoadState } from './download-list-load-state';
|
||||
import { DownloadItem, DownloadsService } from './downloads.service';
|
||||
import { RuntimeCapabilitiesService } from './runtime-capabilities.service';
|
||||
|
||||
@@ -20,6 +21,7 @@ type TestDownloadsService = {
|
||||
downloadFolder: WritableSignal<string>;
|
||||
isAvailable: () => boolean;
|
||||
isLoadingDownloads: Signal<boolean>;
|
||||
hasAuthoritativeDownloadList: Signal<boolean>;
|
||||
hasLoadedDownloads: Signal<boolean>;
|
||||
getDownload: DownloadsService['getDownload'];
|
||||
loadDownloads: DownloadsService['loadDownloads'];
|
||||
@@ -30,9 +32,7 @@ type TestDownloadsService = {
|
||||
selectFolder: DownloadsService['selectFolder'];
|
||||
startDownload: DownloadsService['startDownload'];
|
||||
updateMetadata: DownloadsService['updateMetadata'];
|
||||
_isLoadingDownloads: WritableSignal<boolean>;
|
||||
_hasLoadedDownloads: WritableSignal<boolean>;
|
||||
loadDownloadsRequestId: number;
|
||||
downloadListLoadState: DownloadListLoadState;
|
||||
};
|
||||
|
||||
type DownloadsElectronStub = {
|
||||
@@ -117,8 +117,7 @@ describe('DownloadsService', () => {
|
||||
|
||||
function createService(initialDownloads: DownloadItem[] = []) {
|
||||
const downloads = signal(initialDownloads);
|
||||
const isLoadingDownloads = signal(false);
|
||||
const hasLoadedDownloads = signal(false);
|
||||
const downloadListLoadState = new DownloadListLoadState();
|
||||
const downloadFolder = signal('');
|
||||
const service = Object.create(
|
||||
DownloadsService.prototype
|
||||
@@ -128,11 +127,11 @@ describe('DownloadsService', () => {
|
||||
downloads,
|
||||
downloadFolder,
|
||||
isAvailable: () => true,
|
||||
_isLoadingDownloads: isLoadingDownloads,
|
||||
isLoadingDownloads: isLoadingDownloads.asReadonly(),
|
||||
_hasLoadedDownloads: hasLoadedDownloads,
|
||||
hasLoadedDownloads: hasLoadedDownloads.asReadonly(),
|
||||
loadDownloadsRequestId: 0,
|
||||
downloadListLoadState,
|
||||
isLoadingDownloads: downloadListLoadState.isLoading,
|
||||
hasAuthoritativeDownloadList:
|
||||
downloadListLoadState.hasAuthoritativeList,
|
||||
hasLoadedDownloads: downloadListLoadState.hasLoaded,
|
||||
});
|
||||
|
||||
return service;
|
||||
@@ -183,6 +182,7 @@ describe('DownloadsService', () => {
|
||||
const request = service.loadDownloads();
|
||||
|
||||
expect(service.isLoadingDownloads()).toBe(true);
|
||||
expect(service.hasAuthoritativeDownloadList()).toBe(false);
|
||||
expect(service.hasLoadedDownloads()).toBe(false);
|
||||
expect(electron.downloadsGetList).toHaveBeenCalledWith();
|
||||
|
||||
@@ -191,6 +191,7 @@ describe('DownloadsService', () => {
|
||||
|
||||
expect(service.downloads()).toEqual([item]);
|
||||
expect(service.isLoadingDownloads()).toBe(false);
|
||||
expect(service.hasAuthoritativeDownloadList()).toBe(true);
|
||||
expect(service.hasLoadedDownloads()).toBe(true);
|
||||
});
|
||||
|
||||
@@ -498,7 +499,7 @@ describe('DownloadsService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('marks downloads as loaded after a failed request while preserving existing data', async () => {
|
||||
it('marks a failed request complete while making the preserved list non-authoritative', async () => {
|
||||
const existing = createDownload(1);
|
||||
const error = new Error('download query failed');
|
||||
jest.spyOn(console, 'error').mockImplementation(() => undefined);
|
||||
@@ -508,11 +509,13 @@ describe('DownloadsService', () => {
|
||||
}),
|
||||
};
|
||||
const service = createService([existing]);
|
||||
service.downloadListLoadState.markSucceeded();
|
||||
|
||||
await service.loadDownloads();
|
||||
|
||||
expect(service.downloads()).toEqual([existing]);
|
||||
expect(service.isLoadingDownloads()).toBe(false);
|
||||
expect(service.hasAuthoritativeDownloadList()).toBe(false);
|
||||
expect(service.hasLoadedDownloads()).toBe(true);
|
||||
expect(console.error).toHaveBeenCalledWith(
|
||||
'[DownloadsService] Error loading downloads:',
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { computed, inject, Injectable, OnDestroy, signal } from '@angular/core';
|
||||
import type { DownloadMetadataSnapshot } from '@iptvnator/shared/interfaces';
|
||||
import type { ElectronBridgeDownloadStartResult } from '@iptvnator/shared/interfaces';
|
||||
import { DownloadListLoadState } from './download-list-load-state';
|
||||
import { updateDownloadMetadata } from './downloads-metadata-update';
|
||||
import type { DownloadItem, DownloadStartInput } from './downloads.models';
|
||||
import { formatDownloadBytes } from './downloads.utils';
|
||||
@@ -16,19 +17,20 @@ export type {
|
||||
export class DownloadsService implements OnDestroy {
|
||||
private readonly runtime = inject(RuntimeCapabilitiesService);
|
||||
private unsubscribe?: () => void;
|
||||
private loadDownloadsRequestId = 0;
|
||||
|
||||
private readonly _isLoadingDownloads = signal(false);
|
||||
private readonly _hasLoadedDownloads = signal(false);
|
||||
private readonly downloadListLoadState = new DownloadListLoadState();
|
||||
|
||||
/** Signal for the list of downloads */
|
||||
readonly downloads = signal<DownloadItem[]>([]);
|
||||
|
||||
/** Whether the download list is currently being loaded */
|
||||
readonly isLoadingDownloads = this._isLoadingDownloads.asReadonly();
|
||||
readonly isLoadingDownloads = this.downloadListLoadState.isLoading;
|
||||
|
||||
/** Whether the first download list request has completed */
|
||||
readonly hasLoadedDownloads = this._hasLoadedDownloads.asReadonly();
|
||||
readonly hasLoadedDownloads = this.downloadListLoadState.hasLoaded;
|
||||
|
||||
/** Whether the latest download list request completed successfully */
|
||||
readonly hasAuthoritativeDownloadList =
|
||||
this.downloadListLoadState.hasAuthoritativeList;
|
||||
|
||||
/** Whether the download feature is available (Electron only) */
|
||||
readonly isAvailable = computed(() => this.runtime.supportsDownloads);
|
||||
@@ -91,23 +93,22 @@ export class DownloadsService implements OnDestroy {
|
||||
async loadDownloads(): Promise<void> {
|
||||
if (!this.isAvailable()) return;
|
||||
|
||||
const requestId = ++this.loadDownloadsRequestId;
|
||||
this._isLoadingDownloads.set(true);
|
||||
const requestId = this.downloadListLoadState.begin();
|
||||
|
||||
try {
|
||||
const list = await window.electron.downloadsGetList();
|
||||
if (requestId === this.loadDownloadsRequestId) {
|
||||
if (this.downloadListLoadState.isLatest(requestId)) {
|
||||
this.downloads.set(list);
|
||||
this._hasLoadedDownloads.set(true);
|
||||
this.downloadListLoadState.markSucceeded();
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('[DownloadsService] Error loading downloads:', error);
|
||||
if (requestId === this.loadDownloadsRequestId) {
|
||||
this._hasLoadedDownloads.set(true);
|
||||
if (this.downloadListLoadState.isLatest(requestId)) {
|
||||
this.downloadListLoadState.markFailed();
|
||||
}
|
||||
} finally {
|
||||
if (requestId === this.loadDownloadsRequestId) {
|
||||
this._isLoadingDownloads.set(false);
|
||||
if (this.downloadListLoadState.isLatest(requestId)) {
|
||||
this.downloadListLoadState.finish();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,6 +27,7 @@ import { SeasonContainerComponent } from './season-container.component';
|
||||
|
||||
interface DownloadsServiceStub {
|
||||
readonly isAvailable: WritableSignal<boolean>;
|
||||
readonly hasAuthoritativeDownloadList: WritableSignal<boolean>;
|
||||
readonly hasLoadedDownloads: WritableSignal<boolean>;
|
||||
readonly downloads: WritableSignal<DownloadItem[]>;
|
||||
readonly startDownload: jest.MockedFunction<
|
||||
@@ -173,6 +174,7 @@ describe('SeasonContainerComponent', () => {
|
||||
adapter: SeasonEpisodeDownloadAdapter | null = downloadAdapter
|
||||
) => {
|
||||
downloadsServiceStub.isAvailable.set(true);
|
||||
downloadsServiceStub.hasAuthoritativeDownloadList.set(true);
|
||||
downloadsServiceStub.hasLoadedDownloads.set(true);
|
||||
fixture.componentRef.setInput('downloadAdapter', adapter);
|
||||
};
|
||||
@@ -189,6 +191,7 @@ describe('SeasonContainerComponent', () => {
|
||||
localStorage.removeItem('iptvnator_episode_view_mode');
|
||||
downloadsServiceStub = {
|
||||
isAvailable: signal(false),
|
||||
hasAuthoritativeDownloadList: signal(false),
|
||||
hasLoadedDownloads: signal(false),
|
||||
downloads: signal<DownloadItem[]>([]),
|
||||
startDownload: jest.fn().mockResolvedValue({ success: true }),
|
||||
@@ -637,7 +640,7 @@ describe('SeasonContainerComponent', () => {
|
||||
it('disables the season action until authoritative eligible work exists and while work is in flight', async () => {
|
||||
const first = createEpisode();
|
||||
enableDownloads();
|
||||
downloadsServiceStub.hasLoadedDownloads.set(false);
|
||||
downloadsServiceStub.hasAuthoritativeDownloadList.set(false);
|
||||
setRequiredInputs({ '1': [first] });
|
||||
fixture.detectChanges();
|
||||
|
||||
@@ -646,8 +649,10 @@ describe('SeasonContainerComponent', () => {
|
||||
'[data-test-id="download-season"]'
|
||||
) as HTMLButtonElement;
|
||||
expect(seasonButton().disabled).toBe(true);
|
||||
expect(downloadsServiceStub.hasLoadedDownloads()).toBe(true);
|
||||
expect(episodeAction(first.id).disabled).toBe(true);
|
||||
|
||||
downloadsServiceStub.hasLoadedDownloads.set(true);
|
||||
downloadsServiceStub.hasAuthoritativeDownloadList.set(true);
|
||||
fixture.componentRef.setInput('isLoading', true);
|
||||
fixture.detectChanges();
|
||||
expect(seasonButton().disabled).toBe(true);
|
||||
|
||||
@@ -118,7 +118,9 @@ export class SeasonDownloadPresenter {
|
||||
const adapter = sources.adapter();
|
||||
const seasonKey = sources.selectedSeason();
|
||||
const downloadsAvailable = this.downloadsService.isAvailable();
|
||||
const downloadsLoaded = this.downloadsService.hasLoadedDownloads();
|
||||
const downloadsReady =
|
||||
this.downloadsService.hasLoadedDownloads() &&
|
||||
this.downloadsService.hasAuthoritativeDownloadList();
|
||||
this.downloadsService.downloads();
|
||||
|
||||
return sources.selectedEpisodes().map((episode) => {
|
||||
@@ -138,7 +140,7 @@ export class SeasonDownloadPresenter {
|
||||
eligible: Boolean(
|
||||
candidate &&
|
||||
downloadsAvailable &&
|
||||
downloadsLoaded &&
|
||||
downloadsReady &&
|
||||
!pending &&
|
||||
isEpisodeDownloadEligible(download)
|
||||
),
|
||||
@@ -147,7 +149,7 @@ export class SeasonDownloadPresenter {
|
||||
candidate,
|
||||
download,
|
||||
pending,
|
||||
downloadsLoaded
|
||||
downloadsReady
|
||||
),
|
||||
};
|
||||
});
|
||||
@@ -168,6 +170,7 @@ export class SeasonDownloadPresenter {
|
||||
sources.isLoading() ||
|
||||
this.batchRunning() ||
|
||||
!this.downloadsService.hasLoadedDownloads() ||
|
||||
!this.downloadsService.hasAuthoritativeDownloadList() ||
|
||||
!sources.selectedSeason() ||
|
||||
this.rows().length === 0 ||
|
||||
this.eligibleEpisodeCount() === 0
|
||||
@@ -278,9 +281,9 @@ export class SeasonDownloadPresenter {
|
||||
candidate: EpisodeDownloadCandidate | null,
|
||||
download: DownloadItem | undefined,
|
||||
pending: boolean,
|
||||
downloadsLoaded: boolean
|
||||
downloadsReady: boolean
|
||||
): EpisodeDownloadPresentation {
|
||||
if (!candidate || !downloadsLoaded) {
|
||||
if (!candidate || !downloadsReady) {
|
||||
return EPISODE_DOWNLOAD_STATES.blocked;
|
||||
}
|
||||
if (
|
||||
|
||||
Reference in new issue
Block a user