mirror of
https://github.com/nimbold/Firelink.git
synced 2026-07-26 12:08:27 +00:00
fix(downloads): close audit lifecycle gaps
Prevent duplicate Add Window submissions and preserve discard confirmation. Reject duplicate backend primary-path ownership and clear stale lifecycle progress.
This commit is contained in:
@@ -923,6 +923,20 @@ pub fn load_ownership(connection: &Connection) -> Result<Vec<(String, String)>,
|
||||
}
|
||||
|
||||
pub fn set_ownership(connection: &Connection, id: &str, path: &str) -> Result<(), String> {
|
||||
let existing_owner = connection
|
||||
.query_row(
|
||||
"SELECT id FROM download_ownership
|
||||
WHERE primary_path = ?1 AND id <> ?2
|
||||
LIMIT 1",
|
||||
params![path, id],
|
||||
|row| row.get::<_, String>(0),
|
||||
)
|
||||
.optional()
|
||||
.map_err(|error| format!("failed to check download ownership path: {error}"))?;
|
||||
if existing_owner.is_some() {
|
||||
return Err("Download destination is already owned by another Firelink download".to_string());
|
||||
}
|
||||
|
||||
connection
|
||||
.execute(
|
||||
"INSERT INTO download_ownership (id, primary_path) VALUES (?1, ?2)
|
||||
@@ -1668,4 +1682,22 @@ mod tests {
|
||||
acknowledge_pairing_token_notice(&connection).unwrap();
|
||||
assert!(!has_pending_notice(&connection, TOKEN_CHANGED_NOTICE).unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_two_download_ids_from_claiming_the_same_primary_path() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let state = init_at_path(temp.path()).unwrap();
|
||||
let connection = state.lock().unwrap();
|
||||
|
||||
set_ownership(&connection, "first", "/downloads/file.bin").unwrap();
|
||||
let error = set_ownership(&connection, "second", "/downloads/file.bin")
|
||||
.expect_err("a primary path must have one live owner");
|
||||
|
||||
assert!(error.contains("already owned"));
|
||||
assert_eq!(load_ownership(&connection).unwrap(), vec![(
|
||||
"first".to_string(),
|
||||
"/downloads/file.bin".to_string()
|
||||
)]);
|
||||
set_ownership(&connection, "first", "/downloads/renamed.bin").unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -101,6 +101,7 @@ export const AddDownloadsModal = () => {
|
||||
const [resolvedLocation, setResolvedLocation] = useState('');
|
||||
const [isQueueMenuOpen, setIsQueueMenuOpen] = useState(false);
|
||||
const [isSubmitting, setIsSubmitting] = useState(false);
|
||||
const isSubmittingRef = useRef(false);
|
||||
const actionMenuRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
// Right Form
|
||||
@@ -155,7 +156,7 @@ export const AddDownloadsModal = () => {
|
||||
};
|
||||
|
||||
const closeModalFromDismissAction = useCallback(() => {
|
||||
if (isSubmitting) return;
|
||||
if (isSubmitting || isSubmittingRef.current) return;
|
||||
const hasPendingInput = Boolean(
|
||||
urls.trim() || pendingAddUrls.trim() || parsedItems.length || headers.trim() || cookies.trim()
|
||||
);
|
||||
@@ -206,6 +207,7 @@ export const AddDownloadsModal = () => {
|
||||
cookiesManuallyEditedRef.current = false;
|
||||
setMirrors('');
|
||||
setIsQueueMenuOpen(false);
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
}, [
|
||||
isAddModalOpen,
|
||||
@@ -496,13 +498,14 @@ export const AddDownloadsModal = () => {
|
||||
};
|
||||
|
||||
const handleAction = async (action: AddDownloadAction) => {
|
||||
if (isSubmitting || !canSubmitMetadataRows(parsedItems)) {
|
||||
if (isSubmitting || isSubmittingRef.current || !canSubmitMetadataRows(parsedItems)) {
|
||||
return;
|
||||
}
|
||||
if (speedLimitEnabled && (!Number.isFinite(Number(speedLimit)) || Number(speedLimit) <= 0)) {
|
||||
addToast({ message: 'Speed limit must be greater than zero', variant: 'error', isActionable: true });
|
||||
return;
|
||||
}
|
||||
isSubmittingRef.current = true;
|
||||
setIsSubmitting(true);
|
||||
let finalLocation = saveLocation;
|
||||
let useSharedDestination = isSaveLocationManual;
|
||||
@@ -525,11 +528,13 @@ export const AddDownloadsModal = () => {
|
||||
const approvedPath = await useSettingsStore.getState().approveDownloadRoot(selected);
|
||||
destinationOverrides[index] = approvedPath;
|
||||
} else {
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
return;
|
||||
}
|
||||
} catch (e) {
|
||||
console.error("Failed to select folder:", e);
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
return;
|
||||
}
|
||||
@@ -622,6 +627,7 @@ export const AddDownloadsModal = () => {
|
||||
setPendingUseSharedDestination(useSharedDestination);
|
||||
setPendingDestinationOverrides(destinationOverrides);
|
||||
setShowingDuplicates(true);
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
return;
|
||||
}
|
||||
@@ -629,6 +635,7 @@ export const AddDownloadsModal = () => {
|
||||
try {
|
||||
await executeAddDownloads(action, finalLocation, useSharedDestination, undefined, destinationOverrides);
|
||||
} finally {
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
}
|
||||
};
|
||||
@@ -886,6 +893,8 @@ export const AddDownloadsModal = () => {
|
||||
<DuplicateResolutionModal
|
||||
conflicts={conflicts}
|
||||
onConfirm={(resolutions) => {
|
||||
if (isSubmittingRef.current) return;
|
||||
isSubmittingRef.current = true;
|
||||
setShowingDuplicates(false);
|
||||
setIsSubmitting(true);
|
||||
void executeAddDownloads(
|
||||
@@ -902,7 +911,10 @@ export const AddDownloadsModal = () => {
|
||||
isActionable: true
|
||||
});
|
||||
})
|
||||
.finally(() => setIsSubmitting(false));
|
||||
.finally(() => {
|
||||
isSubmittingRef.current = false;
|
||||
setIsSubmitting(false);
|
||||
});
|
||||
}}
|
||||
onCancel={() => setShowingDuplicates(false)}
|
||||
/>
|
||||
@@ -1238,7 +1250,7 @@ export const AddDownloadsModal = () => {
|
||||
{metadataSummaryMessage(parsedItems)}
|
||||
</div>
|
||||
<div className="flex gap-2.5">
|
||||
<button onClick={() => toggleAddModal(false)} disabled={isSubmitting} className="add-download-button add-download-button-cancel px-4 text-xs">
|
||||
<button onClick={closeModalFromDismissAction} disabled={isSubmitting} className="add-download-button add-download-button-cancel px-4 text-xs">
|
||||
Cancel
|
||||
</button>
|
||||
<div ref={actionMenuRef} className="relative flex gap-2.5">
|
||||
|
||||
@@ -113,4 +113,47 @@ describe('useDownloadProgressStore', () => {
|
||||
expect(useDownloadProgressStore.getState().progressMap).toEqual({});
|
||||
release();
|
||||
});
|
||||
|
||||
it('drops stale progress when a download returns to a queued lifecycle', async () => {
|
||||
const handlers: Record<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
handlers[event] = handler as (event: any) => void;
|
||||
return Promise.resolve(vi.fn());
|
||||
});
|
||||
useDownloadStore.setState({
|
||||
downloads: [{
|
||||
id: 'reused',
|
||||
url: 'https://example.com/file',
|
||||
fileName: 'file.bin',
|
||||
status: 'downloading',
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
}]
|
||||
});
|
||||
|
||||
const release = await initDownloadListener();
|
||||
handlers['download-progress']({ payload: {
|
||||
id: 'reused',
|
||||
fraction: 0.8,
|
||||
speed: '1 MB/s',
|
||||
eta: '2s',
|
||||
size: '8 MB',
|
||||
size_is_final: false
|
||||
} });
|
||||
handlers['download-state']({ payload: {
|
||||
id: 'reused',
|
||||
status: 'queued'
|
||||
} });
|
||||
handlers['download-progress']({ payload: {
|
||||
id: 'reused',
|
||||
fraction: 0.9,
|
||||
speed: '2 MB/s',
|
||||
eta: '1s',
|
||||
size: '9 MB',
|
||||
size_is_final: false
|
||||
} });
|
||||
|
||||
expect(useDownloadProgressStore.getState().progressMap).toEqual({});
|
||||
release();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,9 +38,10 @@ const startDownloadListeners = async () => {
|
||||
return;
|
||||
}
|
||||
// A sidecar can flush one last progress chunk after a pause, failure,
|
||||
// or completion event. Do not let that stale chunk repopulate the live
|
||||
// progress map or overwrite a later lifecycle's first frame.
|
||||
if (current && ['completed', 'failed', 'paused'].includes(current.status)) {
|
||||
// completion, or lifecycle reset. Do not let that stale chunk repopulate
|
||||
// the live progress map or overwrite a later lifecycle's first frame.
|
||||
if (!['downloading', 'processing'].includes(current.status)) {
|
||||
useDownloadProgressStore.getState().clearDownloadProgress(payload.id);
|
||||
return;
|
||||
}
|
||||
useDownloadProgressStore.getState().updateDownloadProgress(payload.id, payload);
|
||||
@@ -74,6 +75,9 @@ const startDownloadListeners = async () => {
|
||||
return;
|
||||
}
|
||||
|
||||
if (['queued', 'retrying', 'completed', 'failed', 'paused'].includes(status)) {
|
||||
useDownloadProgressStore.getState().clearDownloadProgress(payload.id);
|
||||
}
|
||||
const progress = useDownloadProgressStore.getState().progressMap[payload.id];
|
||||
const updates: Partial<DownloadItem> = {
|
||||
status,
|
||||
@@ -109,7 +113,7 @@ const startDownloadListeners = async () => {
|
||||
} else if (status === 'completed' || status === 'failed') {
|
||||
mainStore.unregisterBackendIds([payload.id]);
|
||||
}
|
||||
if (status === 'completed' || status === 'failed' || status === 'paused') {
|
||||
if (['queued', 'retrying', 'completed', 'failed', 'paused'].includes(status)) {
|
||||
useDownloadProgressStore.getState().clearDownloadProgress(payload.id);
|
||||
}
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user