mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-09 09:16:57 +00:00
fix(download): prevent persistence wipeout on startup and recover schema v3 downloads
- Prevent premature persistence activation by installing the subscription only after download state initialization finishes - Guard persistence subscription and flush handlers against emitting snapshots before database hydration completes - Preserve array and set reference equality in applyRemovalJob when target job ID is absent, avoiding spurious persistence triggers - Merge removal jobs monotonically during initDB and suppress redundant identical removal events - Recover missing downloads, ownership records, and custom queues from schema v3 migration backup when SQLite downloads table is empty - Add resilience against corrupt backup candidates and maintain referential integrity by excluding tombstoned download paths - Add unit and regression tests covering persistence hydration barriers, removal job deduplication, and backup recovery
This commit is contained in:
+357
-4
@@ -105,6 +105,7 @@ fn init_at_path_internal(
|
||||
migrate_schema(&mut connection, version)?;
|
||||
|
||||
import_legacy_data(&mut connection, app_data_dir, portable)?;
|
||||
recover_downloads_from_migration_backup(&mut connection, app_data_dir, portable)?;
|
||||
if portable {
|
||||
sanitize_persisted_downloads(&mut connection)?;
|
||||
}
|
||||
@@ -228,10 +229,13 @@ fn migrate_schema(connection: &mut Connection, from_version: i64) -> Result<(),
|
||||
}
|
||||
|
||||
if from_version < 4 {
|
||||
transaction.execute_batch("CREATE TABLE download_removal_jobs (
|
||||
id TEXT PRIMARY KEY, data TEXT NOT NULL
|
||||
);
|
||||
CREATE TABLE download_removal_assets (id TEXT PRIMARY KEY, data TEXT NOT NULL);").map_err(|error| format!("failed to migrate removal jobs: {error}"))?;
|
||||
transaction.execute_batch("
|
||||
CREATE TABLE IF NOT EXISTS metadata (key TEXT PRIMARY KEY, value TEXT NOT NULL);
|
||||
CREATE TABLE IF NOT EXISTS download_removal_jobs (
|
||||
id TEXT PRIMARY KEY, data TEXT NOT NULL
|
||||
);
|
||||
CREATE TABLE IF NOT EXISTS download_removal_assets (id TEXT PRIMARY KEY, data TEXT NOT NULL);
|
||||
").map_err(|error| format!("failed to migrate removal jobs: {error}"))?;
|
||||
}
|
||||
|
||||
transaction
|
||||
@@ -316,6 +320,213 @@ fn import_legacy_data(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn recover_downloads_from_migration_backup(
|
||||
connection: &mut Connection,
|
||||
app_data_dir: &Path,
|
||||
portable: bool,
|
||||
) -> Result<(), String> {
|
||||
const RECOVERY_MARKER: &str = "migration-backup-recovered:schema-v3";
|
||||
if !table_exists(connection, "metadata")? {
|
||||
return Ok(());
|
||||
}
|
||||
if metadata_exists(connection, RECOVERY_MARKER)? {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if !table_exists(connection, "downloads")? {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let current_downloads_count: i64 = connection
|
||||
.query_row("SELECT COUNT(*) FROM downloads", [], |row| row.get(0))
|
||||
.map_err(|error| format!("failed to count downloads for recovery: {error}"))?;
|
||||
if current_downloads_count > 0 {
|
||||
connection
|
||||
.execute(
|
||||
"INSERT INTO metadata (key, value) VALUES (?1, 'skipped')
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||
params![RECOVERY_MARKER],
|
||||
)
|
||||
.map_err(|error| format!("failed to record recovery status: {error}"))?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let entries = fs::read_dir(app_data_dir)
|
||||
.map_err(|error| format!("failed to read app data directory for recovery: {error}"))?;
|
||||
let mut backup_candidates: Vec<PathBuf> = Vec::new();
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
if let Some(file_name) = path.file_name().and_then(|n| n.to_str()) {
|
||||
if file_name.starts_with(&format!("{DATABASE_NAME}.backup-schema-v3")) {
|
||||
if let Ok(metadata) = fs::symlink_metadata(&path) {
|
||||
if metadata.is_file() && !metadata.file_type().is_symlink() {
|
||||
backup_candidates.push(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
backup_candidates.sort_by(|a, b| b.cmp(a));
|
||||
|
||||
for candidate in backup_candidates {
|
||||
let Ok(backup_conn) = Connection::open(&candidate) else {
|
||||
continue;
|
||||
};
|
||||
if !table_exists(&backup_conn, "downloads").unwrap_or(false) {
|
||||
continue;
|
||||
}
|
||||
let backup_count: i64 = backup_conn
|
||||
.query_row("SELECT COUNT(*) FROM downloads", [], |row| row.get(0))
|
||||
.unwrap_or(0);
|
||||
if backup_count == 0 {
|
||||
continue;
|
||||
}
|
||||
|
||||
let Ok(mut backup_downloads) =
|
||||
query_string_column(&backup_conn, "SELECT data FROM downloads ORDER BY rowid")
|
||||
else {
|
||||
log::warn!(
|
||||
"Failed to read downloads from migration backup candidate '{}'",
|
||||
candidate.display()
|
||||
);
|
||||
continue;
|
||||
};
|
||||
if portable {
|
||||
if let Err(error) = sanitize_download_strings(&mut backup_downloads) {
|
||||
log::warn!(
|
||||
"Failed to sanitize migration backup downloads from '{}': {error}",
|
||||
candidate.display()
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
let transaction = connection
|
||||
.transaction()
|
||||
.map_err(|error| format!("failed to begin recovery transaction: {error}"))?;
|
||||
|
||||
let has_removal_jobs_table = table_exists(&transaction, "download_removal_jobs").unwrap_or(false);
|
||||
let mut restored_ids = std::collections::HashSet::new();
|
||||
let mut restored_count = 0;
|
||||
for data in &backup_downloads {
|
||||
let Ok(value) = serde_json::from_str::<Value>(data) else {
|
||||
continue;
|
||||
};
|
||||
let Some(id) = value.get("id").and_then(Value::as_str) else {
|
||||
continue;
|
||||
};
|
||||
if has_removal_jobs_table
|
||||
&& transaction
|
||||
.query_row(
|
||||
"SELECT EXISTS(SELECT 1 FROM download_removal_jobs WHERE id = ?1)",
|
||||
[id],
|
||||
|row| row.get::<_, bool>(0),
|
||||
)
|
||||
.unwrap_or(false)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let status = value.get("status").and_then(Value::as_str).unwrap_or("completed");
|
||||
let queue_id = value.get("queueId").and_then(Value::as_str);
|
||||
let inserted = transaction
|
||||
.execute(
|
||||
"INSERT INTO downloads (id, status, queue_id, data) VALUES (?1, ?2, ?3, ?4)
|
||||
ON CONFLICT(id) DO NOTHING",
|
||||
params![id, status, queue_id, data],
|
||||
)
|
||||
.map_err(|error| format!("failed to restore download '{id}': {error}"))?;
|
||||
if inserted > 0 {
|
||||
restored_ids.insert(id.to_string());
|
||||
restored_count += 1;
|
||||
}
|
||||
}
|
||||
|
||||
if table_exists(&backup_conn, "download_ownership").unwrap_or(false)
|
||||
&& table_exists(&transaction, "download_ownership").unwrap_or(false)
|
||||
{
|
||||
if let Ok(mut stmt) = backup_conn.prepare("SELECT id, primary_path FROM download_ownership") {
|
||||
if let Ok(rows) = stmt.query_map([], |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))) {
|
||||
for (id, primary_path) in rows.flatten() {
|
||||
if restored_ids.contains(&id) {
|
||||
let _ = transaction.execute(
|
||||
"INSERT INTO download_ownership (id, primary_path) VALUES (?1, ?2) ON CONFLICT(id) DO NOTHING",
|
||||
params![id, primary_path],
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if table_exists(&backup_conn, "download_owned_paths").unwrap_or(false)
|
||||
&& table_exists(&transaction, "download_owned_paths").unwrap_or(false)
|
||||
{
|
||||
if let Ok(mut stmt) = backup_conn.prepare("SELECT id, paths FROM download_owned_paths") {
|
||||
if let Ok(rows) = stmt.query_map([], |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))) {
|
||||
for (id, paths) in rows.flatten() {
|
||||
if restored_ids.contains(&id) {
|
||||
let _ = transaction.execute(
|
||||
"INSERT INTO download_owned_paths (id, paths) VALUES (?1, ?2) ON CONFLICT(id) DO NOTHING",
|
||||
params![id, paths],
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if table_exists(&backup_conn, "download_removal_paths").unwrap_or(false)
|
||||
&& table_exists(&transaction, "download_removal_paths").unwrap_or(false)
|
||||
{
|
||||
if let Ok(mut stmt) = backup_conn.prepare("SELECT id, paths FROM download_removal_paths") {
|
||||
if let Ok(rows) = stmt.query_map([], |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))) {
|
||||
for (id, paths) in rows.flatten() {
|
||||
if restored_ids.contains(&id) {
|
||||
let _ = transaction.execute(
|
||||
"INSERT INTO download_removal_paths (id, paths) VALUES (?1, ?2) ON CONFLICT(id) DO NOTHING",
|
||||
params![id, paths],
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if table_exists(&backup_conn, "queues").unwrap_or(false)
|
||||
&& table_exists(&transaction, "queues").unwrap_or(false)
|
||||
{
|
||||
if let Ok(mut stmt) = backup_conn.prepare("SELECT id, data FROM queues") {
|
||||
if let Ok(rows) = stmt.query_map([], |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))) {
|
||||
for (id, data) in rows.flatten() {
|
||||
let _ = transaction.execute(
|
||||
"INSERT INTO queues (id, data) VALUES (?1, ?2) ON CONFLICT(id) DO NOTHING",
|
||||
params![id, data],
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
transaction
|
||||
.execute(
|
||||
"INSERT INTO metadata (key, value) VALUES (?1, 'complete')
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||
params![RECOVERY_MARKER],
|
||||
)
|
||||
.map_err(|error| format!("failed to record recovery completion: {error}"))?;
|
||||
|
||||
transaction
|
||||
.commit()
|
||||
.map_err(|error| format!("failed to commit recovery: {error}"))?;
|
||||
|
||||
log::info!(
|
||||
"Restored {restored_count} download(s) from migration backup '{}'",
|
||||
candidate.display()
|
||||
);
|
||||
break;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn sanitize_legacy_source(path: &Path, remove_pairing_token: bool) -> Result<(), String> {
|
||||
match fs::symlink_metadata(path) {
|
||||
Ok(metadata) if metadata.file_type().is_symlink() => {
|
||||
@@ -3897,4 +4108,146 @@ mod tests {
|
||||
3
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recover_downloads_from_migration_backup_restores_empty_downloads_and_excludes_tombstones() {
|
||||
let root = TempDir::new().unwrap();
|
||||
let db = init_at_path(root.path()).unwrap();
|
||||
let mut connection = db.lock().unwrap();
|
||||
|
||||
let backup_path = root.path().join("firelink.sqlite.backup-schema-v3-20260907T120000Z-unit");
|
||||
{
|
||||
let backup_conn = Connection::open(&backup_path).unwrap();
|
||||
backup_conn.execute_batch("
|
||||
CREATE TABLE downloads (id TEXT PRIMARY KEY, status TEXT NOT NULL, queue_id TEXT, data TEXT NOT NULL);
|
||||
INSERT INTO downloads VALUES ('keep-1', 'completed', 'main', '{\"id\":\"keep-1\",\"fileName\":\"keep1.bin\",\"status\":\"completed\"}');
|
||||
INSERT INTO downloads VALUES ('keep-2', 'completed', 'main', '{\"id\":\"keep-2\",\"fileName\":\"keep2.bin\",\"status\":\"completed\"}');
|
||||
INSERT INTO downloads VALUES ('deleted-tombstone', 'completed', 'main', '{\"id\":\"deleted-tombstone\",\"fileName\":\"deleted.bin\",\"status\":\"completed\"}');
|
||||
CREATE TABLE download_ownership (id TEXT PRIMARY KEY, primary_path TEXT NOT NULL);
|
||||
INSERT INTO download_ownership VALUES ('keep-1', '/path/to/keep1.bin');
|
||||
INSERT INTO download_ownership VALUES ('deleted-tombstone', '/path/to/deleted.bin');
|
||||
CREATE TABLE queues (id TEXT PRIMARY KEY, data TEXT NOT NULL);
|
||||
INSERT INTO queues VALUES ('custom-queue', '{\"id\":\"custom-queue\",\"name\":\"Custom\"}');
|
||||
").unwrap();
|
||||
}
|
||||
|
||||
// Simulate that a removal job already exists in download_removal_jobs
|
||||
connection.execute(
|
||||
"INSERT INTO download_removal_jobs (id, data) VALUES (?1, ?2)",
|
||||
params![
|
||||
"deleted-tombstone",
|
||||
r#"{"id":"deleted-tombstone","revision":1,"deleteAssets":true,"phase":"completed","error":null}"#
|
||||
],
|
||||
).unwrap();
|
||||
|
||||
recover_downloads_from_migration_backup(&mut connection, root.path(), false).unwrap();
|
||||
|
||||
let loaded = load_downloads(&connection).unwrap();
|
||||
assert_eq!(loaded.len(), 2);
|
||||
assert!(loaded.iter().any(|d| d.contains("keep-1")));
|
||||
assert!(loaded.iter().any(|d| d.contains("keep-2")));
|
||||
assert!(!loaded.iter().any(|d| d.contains("deleted-tombstone")));
|
||||
|
||||
// Ownership should be restored for keep-1 but not for deleted-tombstone
|
||||
assert_eq!(
|
||||
connection.query_row::<String, _, _>(
|
||||
"SELECT primary_path FROM download_ownership WHERE id = 'keep-1'",
|
||||
[],
|
||||
|r| r.get(0)
|
||||
).unwrap(),
|
||||
"/path/to/keep1.bin"
|
||||
);
|
||||
assert_eq!(
|
||||
connection.query_row::<i64, _, _>(
|
||||
"SELECT COUNT(*) FROM download_ownership WHERE id = 'deleted-tombstone'",
|
||||
[],
|
||||
|r| r.get(0)
|
||||
).unwrap(),
|
||||
0
|
||||
);
|
||||
|
||||
// Custom queue should be restored
|
||||
assert!(load_queues(&connection).unwrap().iter().any(|q| q.contains("custom-queue")));
|
||||
|
||||
assert_eq!(
|
||||
connection.query_row::<String, _, _>(
|
||||
"SELECT value FROM metadata WHERE key = 'migration-backup-recovered:schema-v3'",
|
||||
[],
|
||||
|r| r.get(0)
|
||||
).unwrap(),
|
||||
"complete"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recover_downloads_from_migration_backup_skips_when_downloads_exist() {
|
||||
let root = TempDir::new().unwrap();
|
||||
let db = init_at_path(root.path()).unwrap();
|
||||
let mut connection = db.lock().unwrap();
|
||||
|
||||
let backup_path = root.path().join("firelink.sqlite.backup-schema-v3-20260907T120000Z-unit");
|
||||
{
|
||||
let backup_conn = Connection::open(&backup_path).unwrap();
|
||||
backup_conn.execute_batch("
|
||||
CREATE TABLE downloads (id TEXT PRIMARY KEY, status TEXT NOT NULL, queue_id TEXT, data TEXT NOT NULL);
|
||||
INSERT INTO downloads VALUES ('from-backup', 'completed', 'main', '{\"id\":\"from-backup\"}');
|
||||
").unwrap();
|
||||
}
|
||||
|
||||
connection.execute(
|
||||
"INSERT INTO downloads (id, status, queue_id, data) VALUES ('existing', 'completed', 'main', '{\"id\":\"existing\"}')",
|
||||
[]
|
||||
).unwrap();
|
||||
connection.execute("DELETE FROM metadata WHERE key = 'migration-backup-recovered:schema-v3'", []).unwrap();
|
||||
|
||||
recover_downloads_from_migration_backup(&mut connection, root.path(), false).unwrap();
|
||||
|
||||
let loaded = load_downloads(&connection).unwrap();
|
||||
assert_eq!(loaded.len(), 1);
|
||||
assert!(loaded[0].contains("existing"));
|
||||
assert_eq!(
|
||||
connection.query_row::<String, _, _>(
|
||||
"SELECT value FROM metadata WHERE key = 'migration-backup-recovered:schema-v3'",
|
||||
[],
|
||||
|r| r.get(0)
|
||||
).unwrap(),
|
||||
"skipped"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recover_downloads_from_migration_backup_skips_corrupt_candidate_and_continues_to_valid() {
|
||||
let root = TempDir::new().unwrap();
|
||||
let corrupt_path = root.path().join("firelink.sqlite.backup-schema-v3-20260907T130000Z-corrupt");
|
||||
fs::write(&corrupt_path, b"not a valid sqlite file").unwrap();
|
||||
|
||||
let valid_path = root.path().join("firelink.sqlite.backup-schema-v3-20260907T110000Z-valid");
|
||||
{
|
||||
let valid_conn = Connection::open(&valid_path).unwrap();
|
||||
valid_conn.execute_batch("
|
||||
CREATE TABLE downloads (id TEXT PRIMARY KEY, status TEXT NOT NULL, queue_id TEXT, data TEXT NOT NULL);
|
||||
INSERT INTO downloads VALUES ('valid-1', 'completed', 'main', '{\"id\":\"valid-1\"}');
|
||||
").unwrap();
|
||||
}
|
||||
|
||||
let db = init_at_path(root.path()).unwrap();
|
||||
let mut connection = db.lock().unwrap();
|
||||
|
||||
connection.execute("DELETE FROM downloads", []).unwrap();
|
||||
connection.execute("DELETE FROM metadata WHERE key = 'migration-backup-recovered:schema-v3'", []).unwrap();
|
||||
|
||||
recover_downloads_from_migration_backup(&mut connection, root.path(), false).unwrap();
|
||||
|
||||
let loaded = load_downloads(&connection).unwrap();
|
||||
assert_eq!(loaded.len(), 1);
|
||||
assert!(loaded[0].contains("valid-1"));
|
||||
assert_eq!(
|
||||
connection.query_row::<String, _, _>(
|
||||
"SELECT value FROM metadata WHERE key = 'migration-backup-recovered:schema-v3'",
|
||||
[],
|
||||
|r| r.get(0)
|
||||
).unwrap(),
|
||||
"complete"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+4
-2
@@ -540,7 +540,7 @@ function App() {
|
||||
}, [sidebarWidth]);
|
||||
|
||||
useEffect(() => {
|
||||
const disposePersistence = initializeDownloadPersistence(getCurrentWindow().label);
|
||||
let disposePersistence: (() => void) | null = null;
|
||||
let active = true;
|
||||
let exitRequested = false;
|
||||
let exiting = false;
|
||||
@@ -779,6 +779,7 @@ function App() {
|
||||
try {
|
||||
await initializeDownloadState();
|
||||
if (!active) return;
|
||||
disposePersistence = initializeDownloadPersistence(getCurrentWindow().label);
|
||||
} catch (error) {
|
||||
disposeListeners();
|
||||
cleanupListeners = null;
|
||||
@@ -806,7 +807,8 @@ function App() {
|
||||
unlistenExit = null;
|
||||
unlistenSettingsHydration?.();
|
||||
mainWindowSizePersistence.dispose();
|
||||
disposePersistence();
|
||||
disposePersistence?.();
|
||||
disposePersistence = null;
|
||||
};
|
||||
}, [addToast, enqueueAddInput, processExtensionDownload, queueFrontendReadyUpdate]);
|
||||
|
||||
|
||||
@@ -172,6 +172,137 @@ describe('useDownloadStore', () => {
|
||||
expect(useDownloadStore.getState().downloads).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not trigger persistence or wipe existing downloads when initDB hydrates with completed removal jobs', async () => {
|
||||
const disposePersistence = initializeDownloadPersistence('main');
|
||||
const commitCalls: unknown[] = [];
|
||||
const completed = { id: 'tombstoned-1', revision: 2, deleteAssets: true, phase: 'completed' as const, error: null };
|
||||
const keepDownload = {
|
||||
id: 'keep-1',
|
||||
url: 'https://example.com/file.bin',
|
||||
fileName: 'file.bin',
|
||||
status: 'completed' as const,
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
};
|
||||
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async command => {
|
||||
if (command === 'list_download_removals') {
|
||||
return [completed] as never;
|
||||
}
|
||||
if (command === 'db_get_all_queues') return [] as never;
|
||||
if (command === 'db_get_all_downloads') {
|
||||
return [JSON.stringify(keepDownload)] as never;
|
||||
}
|
||||
if (command === 'db_commit_download_state') {
|
||||
commitCalls.push(command);
|
||||
return undefined as never;
|
||||
}
|
||||
return undefined as never;
|
||||
});
|
||||
|
||||
try {
|
||||
await useDownloadStore.getState().initDB();
|
||||
expect(commitCalls).toHaveLength(0);
|
||||
expect(useDownloadStore.getState().downloads).toHaveLength(1);
|
||||
expect(useDownloadStore.getState().downloads[0].id).toBe('keep-1');
|
||||
} finally {
|
||||
disposePersistence();
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves store collection reference equality in applyRemovalJob when the job ID is not present', () => {
|
||||
const stateBefore = useDownloadStore.getState();
|
||||
stateBefore.applyRemovalJob({
|
||||
id: 'non-existent-job',
|
||||
revision: 1,
|
||||
deleteAssets: true,
|
||||
phase: 'completed',
|
||||
error: null
|
||||
});
|
||||
const stateAfter = useDownloadStore.getState();
|
||||
expect(stateAfter.downloads).toBe(stateBefore.downloads);
|
||||
expect(stateAfter.pendingOrder).toBe(stateBefore.pendingOrder);
|
||||
expect(stateAfter.allocationPendingIds).toBe(stateBefore.allocationPendingIds);
|
||||
expect(stateAfter.backendRegisteredIds).toBe(stateBefore.backendRegisteredIds);
|
||||
});
|
||||
|
||||
it('ignores premature flushDownloadPersistence before hydration completes', async () => {
|
||||
let commitCalled = false;
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async command => {
|
||||
if (command === 'db_commit_download_state') {
|
||||
commitCalled = true;
|
||||
}
|
||||
return undefined as never;
|
||||
});
|
||||
|
||||
await flushDownloadPersistence();
|
||||
expect(commitCalled).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores duplicate identical removal jobs and stale revisions without mutating state', () => {
|
||||
const job = {
|
||||
id: 'job-1',
|
||||
revision: 2,
|
||||
deleteAssets: true,
|
||||
phase: 'failed' as const,
|
||||
error: 'disk error'
|
||||
};
|
||||
useDownloadStore.getState().applyRemovalJob(job);
|
||||
const stateAfterFirst = useDownloadStore.getState();
|
||||
|
||||
// Identical duplicate should be a no-op
|
||||
useDownloadStore.getState().applyRemovalJob(job);
|
||||
const stateAfterDuplicate = useDownloadStore.getState();
|
||||
expect(stateAfterDuplicate.removalJobs).toBe(stateAfterFirst.removalJobs);
|
||||
|
||||
// Stale revision should be ignored
|
||||
useDownloadStore.getState().applyRemovalJob({
|
||||
id: 'job-1',
|
||||
revision: 1,
|
||||
deleteAssets: true,
|
||||
phase: 'running' as const,
|
||||
error: null
|
||||
});
|
||||
const stateAfterStale = useDownloadStore.getState();
|
||||
expect(stateAfterStale.removalJobs).toBe(stateAfterFirst.removalJobs);
|
||||
expect(stateAfterStale.removalJobs['job-1'].revision).toBe(2);
|
||||
});
|
||||
|
||||
it('preserves newer in-memory removal jobs during initDB', async () => {
|
||||
useDownloadStore.setState({
|
||||
removalJobs: {
|
||||
'concurrent-1': {
|
||||
id: 'concurrent-1',
|
||||
revision: 3,
|
||||
deleteAssets: true,
|
||||
phase: 'completed',
|
||||
error: null
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async command => {
|
||||
if (command === 'list_download_removals') {
|
||||
return [
|
||||
{
|
||||
id: 'concurrent-1',
|
||||
revision: 1,
|
||||
deleteAssets: true,
|
||||
phase: 'running',
|
||||
error: null
|
||||
}
|
||||
] as never;
|
||||
}
|
||||
if (command === 'db_get_all_queues') return [] as never;
|
||||
if (command === 'db_get_all_downloads') return [] as never;
|
||||
return undefined as never;
|
||||
});
|
||||
|
||||
await useDownloadStore.getState().initDB();
|
||||
expect(useDownloadStore.getState().removalJobs['concurrent-1'].revision).toBe(3);
|
||||
expect(useDownloadStore.getState().removalJobs['concurrent-1'].phase).toBe('completed');
|
||||
});
|
||||
|
||||
it('invalidates in-flight Add-modal handoffs when the modal is toggled', () => {
|
||||
const initialVersion = useDownloadStore.getState().pendingAddRequestVersion;
|
||||
|
||||
|
||||
@@ -1783,20 +1783,37 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
applyRemovalJob: (job) => {
|
||||
if (!job || typeof job.id !== 'string' || !Number.isSafeInteger(job.revision)
|
||||
|| job.revision < 0 || !['pending', 'running', 'failed', 'completed'].includes(job.phase)) return;
|
||||
if ((get().removalJobs[job.id]?.revision ?? -1) > job.revision) return;
|
||||
set(state => ({
|
||||
removalJobs: { ...state.removalJobs, [job.id]: job },
|
||||
downloads: job.phase === 'completed'
|
||||
? state.downloads.filter(item => item.id !== job.id)
|
||||
const currentJob = get().removalJobs[job.id];
|
||||
if (currentJob) {
|
||||
if (currentJob.revision > job.revision) return;
|
||||
if (currentJob.revision === job.revision && currentJob.phase === job.phase && currentJob.error === job.error) return;
|
||||
}
|
||||
set(state => {
|
||||
const hasDownload = state.downloads.some(item => item.id === job.id);
|
||||
const nextDownloads = job.phase === 'completed'
|
||||
? (hasDownload ? state.downloads.filter(item => item.id !== job.id) : state.downloads)
|
||||
: job.phase === 'failed' && job.revision > 0
|
||||
? state.downloads.map(item => item.id === job.id ? { ...item, status: 'failed' as const, speed: '-', eta: '-' } : item)
|
||||
: state.downloads,
|
||||
pendingOrder: state.pendingOrder.filter(id => id !== job.id),
|
||||
allocationPendingIds: new Set([...state.allocationPendingIds].filter(id => id !== job.id)),
|
||||
backendRegisteredIds: job.phase === 'completed'
|
||||
? (hasDownload ? state.downloads.map(item => item.id === job.id ? { ...item, status: 'failed' as const, speed: '-', eta: '-' } : item) : state.downloads)
|
||||
: state.downloads;
|
||||
const hasPending = state.pendingOrder.includes(job.id);
|
||||
const nextPendingOrder = hasPending ? state.pendingOrder.filter(id => id !== job.id) : state.pendingOrder;
|
||||
const hasAllocation = state.allocationPendingIds.has(job.id);
|
||||
const nextAllocationPendingIds = hasAllocation
|
||||
? new Set([...state.allocationPendingIds].filter(id => id !== job.id))
|
||||
: state.allocationPendingIds;
|
||||
const hasRegistered = job.phase === 'completed' && state.backendRegisteredIds.has(job.id);
|
||||
const nextBackendRegisteredIds = hasRegistered
|
||||
? new Set([...state.backendRegisteredIds].filter(id => id !== job.id))
|
||||
: state.backendRegisteredIds,
|
||||
}));
|
||||
: state.backendRegisteredIds;
|
||||
|
||||
return {
|
||||
removalJobs: { ...state.removalJobs, [job.id]: job },
|
||||
downloads: nextDownloads,
|
||||
pendingOrder: nextPendingOrder,
|
||||
allocationPendingIds: nextAllocationPendingIds,
|
||||
backendRegisteredIds: nextBackendRegisteredIds,
|
||||
};
|
||||
});
|
||||
if (job.phase === 'completed') useDownloadProgressStore.getState().resetDownloadProgress(job.id);
|
||||
syncSystemIntegrations();
|
||||
},
|
||||
@@ -3216,10 +3233,20 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
},
|
||||
initDB: async () => {
|
||||
try {
|
||||
downloadPersistenceReady = false;
|
||||
// Register before recovery starts; no worker runs until snapshots hydrate.
|
||||
if (!removalListener) removalListener = await listenEvent('download-removal', event => get().applyRemovalJob(event.payload));
|
||||
const removalJobs = await invoke('list_download_removals');
|
||||
for (const job of removalJobs) get().applyRemovalJob(job);
|
||||
set(state => {
|
||||
const merged: Record<string, DownloadRemovalJob> = { ...state.removalJobs };
|
||||
for (const job of removalJobs) {
|
||||
const existing = merged[job.id];
|
||||
if (!existing || job.revision >= existing.revision) {
|
||||
merged[job.id] = job;
|
||||
}
|
||||
}
|
||||
return { removalJobs: merged };
|
||||
});
|
||||
const persistedQueues = (await invoke('db_get_all_queues')).flatMap(value => {
|
||||
try {
|
||||
return [JSON.parse(value) as PersistedQueue];
|
||||
@@ -3283,6 +3310,9 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
: d
|
||||
))
|
||||
}));
|
||||
if (downloadPersistenceUnsubscribe) {
|
||||
downloadPersistenceReady = true;
|
||||
}
|
||||
|
||||
} catch (e) {
|
||||
console.error("Failed to init DB", e);
|
||||
@@ -3465,6 +3495,7 @@ export const initializeDownloadPersistence = (windowLabel: string): (() => void)
|
||||
if (windowLabel !== 'main' || downloadPersistenceUnsubscribe) return () => undefined;
|
||||
|
||||
downloadPersistenceUnsubscribe = useDownloadStore.subscribe((state, prevState) => {
|
||||
if (!downloadPersistenceReady) return;
|
||||
if (state.queues !== prevState.queues || state.downloads !== prevState.downloads) {
|
||||
void queuePersistenceSnapshot(persistenceSnapshotForState(state)).catch(error => {
|
||||
console.error('Failed to persist download state:', error);
|
||||
|
||||
Reference in New Issue
Block a user