Skip to content

Commit dbccec1

Browse files
committed
Indexing: Add reconciler tests, fix dir→file orphan bug
- Add 9 unit tests for `reconciler.rs` and `event_loop.rs` covering FSEvents edge cases: atomic swaps, `MustScanSubDirs` preservation, false directory removal, `merge_fs_events` flag priority, and multi-level `reconcile_subtree` - Fix latent bug: `reconcile_subtree` now sends `DeleteSubtreeById` before upserting when a directory is replaced by a file, preventing orphaned children - Replace `thread::sleep`-based sync with `flush_blocking()` in 6 pre-existing tests, remove 9 unnecessary post-shutdown sleeps
1 parent 7319c5c commit dbccec1

3 files changed

Lines changed: 479 additions & 15 deletions

File tree

apps/desktop/src-tauri/src/indexing/CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ Key test files are alongside each module (test functions within `#[cfg(test)]` b
167167

168168
**ScanContext maps scan root to ROOT_ID**: Both `scan_volume` and `scan_subtree` create a `ScanContext` that maps the scan root directory to `ROOT_ID` (1). This means all top-level entries under any scan root get `parent_id = ROOT_ID` in the DB. For subtree scans, the root is resolved to its existing entry ID (not ROOT_ID), and `DeleteDescendantsById` is sent before the scan starts. The `ScanContext` opens a temporary read connection to the DB to fetch `next_id` via `get_next_id()`.
169169

170+
**Reconciler must delete old subtree on dir-to-file type changes**: When `reconcile_subtree` matches a filesystem entry to a DB entry by name, it must check if `is_directory` changed. If a directory became a file, `DeleteSubtreeById` must be sent before `UpsertEntryV2`. Without this, `INSERT OR REPLACE` keeps the same row ID (same `parent_id + name`), and the old directory's children become logical orphans — entries parented by a file.
171+
170172
**Never use `INSERT OR REPLACE` on entries without deleting descendants first**: `INSERT OR REPLACE` on the `idx_parent_name` unique index silently deletes the old row and inserts a new one with a new ID. This orphans all children (their `parent_id` points to the deleted old ID) and orphans the old `dir_stats` row. The scanner's `insert_entries_v2_batch` still uses `INSERT OR REPLACE` as a safety net, but it's always preceded by `DeleteDescendantsById` for subtree scans, so no conflicts should occur in practice.
171173

172174
**IndexWriter exposes `db_path()`**: The scanner needs the DB path to open a temporary connection for `ScanContext::new()`. This path is stored on the `IndexWriter` handle and accessible via `db_path()`. The temporary connection is short-lived (only used to read `MAX(id)`).

apps/desktop/src-tauri/src/indexing/event_loop.rs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,3 +927,114 @@ fn verify_affected_dirs(affected_paths: &HashSet<String>, writer: &IndexWriter)
927927
new_dir_paths,
928928
}
929929
}
930+
931+
// ── Tests ────────────────────────────────────────────────────────────
932+
933+
#[cfg(test)]
934+
mod tests {
935+
use super::*;
936+
937+
fn make_event(path: &str, event_id: u64, flags: watcher::FsEventFlags) -> watcher::FsChangeEvent {
938+
watcher::FsChangeEvent {
939+
path: path.to_string(),
940+
event_id,
941+
flags,
942+
}
943+
}
944+
945+
/// Merging created+removed: `item_removed` wins (priority-based merge),
946+
/// dropping `item_created`. The reconciler's stat-before-delete in
947+
/// `handle_removal` compensates: if the file still exists on disk, it
948+
/// upserts instead of deleting. Regression coverage for f0c225f.
949+
#[test]
950+
fn merge_created_then_removed_prioritizes_removed() {
951+
let created = make_event(
952+
"/test/file.txt",
953+
100,
954+
watcher::FsEventFlags {
955+
item_created: true,
956+
item_is_file: true,
957+
..Default::default()
958+
},
959+
);
960+
let removed = make_event(
961+
"/test/file.txt",
962+
200,
963+
watcher::FsEventFlags {
964+
item_removed: true,
965+
item_is_file: true,
966+
..Default::default()
967+
},
968+
);
969+
970+
let merged = merge_fs_events(&created, &removed);
971+
972+
assert!(merged.flags.item_removed, "item_removed should be set");
973+
assert!(!merged.flags.item_created, "item_created is dropped — removed wins");
974+
assert!(merged.flags.item_is_file, "item_is_file should be preserved");
975+
assert_eq!(merged.event_id, 200, "higher event_id should be kept");
976+
}
977+
978+
/// Same as above but with events in reverse order: removed first, then
979+
/// created. `item_removed` still wins.
980+
#[test]
981+
fn merge_removed_then_created_prioritizes_removed() {
982+
let removed = make_event(
983+
"/test/file.txt",
984+
100,
985+
watcher::FsEventFlags {
986+
item_removed: true,
987+
item_is_file: true,
988+
..Default::default()
989+
},
990+
);
991+
let created = make_event(
992+
"/test/file.txt",
993+
200,
994+
watcher::FsEventFlags {
995+
item_created: true,
996+
item_is_file: true,
997+
..Default::default()
998+
},
999+
);
1000+
1001+
let merged = merge_fs_events(&removed, &created);
1002+
1003+
assert!(merged.flags.item_removed, "item_removed should be set");
1004+
assert!(!merged.flags.item_created, "item_created is dropped — removed wins");
1005+
assert!(merged.flags.item_is_file, "item_is_file should be preserved");
1006+
assert_eq!(merged.event_id, 200, "higher event_id should be kept");
1007+
}
1008+
1009+
/// When merging, the higher event_id should always win regardless of
1010+
/// which event is "existing" vs "incoming".
1011+
#[test]
1012+
fn merge_keeps_higher_event_id() {
1013+
let older = make_event(
1014+
"/test/file.txt",
1015+
300,
1016+
watcher::FsEventFlags {
1017+
item_modified: true,
1018+
item_is_file: true,
1019+
..Default::default()
1020+
},
1021+
);
1022+
let newer = make_event(
1023+
"/test/file.txt",
1024+
100,
1025+
watcher::FsEventFlags {
1026+
item_modified: true,
1027+
item_is_file: true,
1028+
..Default::default()
1029+
},
1030+
);
1031+
1032+
// existing=older (300), incoming=newer (100)
1033+
let merged = merge_fs_events(&older, &newer);
1034+
assert_eq!(merged.event_id, 300, "higher event_id should be kept");
1035+
1036+
// existing=newer (100), incoming=older (300)
1037+
let merged = merge_fs_events(&newer, &older);
1038+
assert_eq!(merged.event_id, 300, "higher event_id should be kept");
1039+
}
1040+
}

0 commit comments

Comments
 (0)