Skip to content

Commit f5e3ed8

Browse files
fix(storage): cache git nodes after blob writes (#208)
1 parent 0938d54 commit f5e3ed8

1 file changed

Lines changed: 29 additions & 3 deletions

File tree

src/storage/git.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,6 @@ impl<const N: usize> NodeStorage<N> for GitNodeStorage<N> {
199199
hash: ValueDigest<N>,
200200
node: ProllyNode<N>,
201201
) -> Result<(), StorageError> {
202-
// Store in cache
203-
self.cache.lock().put(hash.clone(), Arc::new(node.clone()));
204-
205202
// Store as Git blob (this is durable — the blob lives in the git object db)
206203
let blob_id = self
207204
.store_node_as_blob(&node)
@@ -219,6 +216,10 @@ impl<const N: usize> NodeStorage<N> for GitNodeStorage<N> {
219216
// full in-memory map — so the per-insert write is redundant.
220217
self.hash_to_object_id.lock().insert(hash.clone(), blob_id);
221218

219+
// Only cache after the durable write and mapping succeed; otherwise a
220+
// failed insert can be served in-process but disappear on reopen.
221+
self.cache.lock().put(hash, Arc::new(node));
222+
222223
Ok(())
223224
}
224225

@@ -438,4 +439,29 @@ mod tests {
438439
let cached = storage.get_node_by_hash(&hash1);
439440
assert!(cached.is_some());
440441
}
442+
443+
#[test]
444+
fn failed_insert_does_not_populate_cache() {
445+
let (temp_dir, repo) = create_test_repo();
446+
let mut storage = GitNodeStorage::<32>::new(repo, temp_dir.path().to_path_buf()).unwrap();
447+
let objects_path = temp_dir.path().join("objects");
448+
std::fs::remove_dir_all(&objects_path).unwrap();
449+
std::fs::write(&objects_path, b"not a directory").unwrap();
450+
451+
let node = create_test_node();
452+
let hash = node.get_hash();
453+
454+
assert!(
455+
storage.insert_node(hash.clone(), node).is_err(),
456+
"sabotaged object database should reject blob writes"
457+
);
458+
assert!(
459+
storage.get_node_by_hash(&hash).is_none(),
460+
"failed blob writes must not leave phantom nodes in cache"
461+
);
462+
assert!(
463+
!storage.get_hash_mappings().contains_key(&hash),
464+
"failed blob writes must not create hash mappings"
465+
);
466+
}
441467
}

0 commit comments

Comments
 (0)