Skip to content

Commit 407ff32

Browse files
committed
gave Reader a new(); added Display and from_str to Version, serde calls into it now; added in-memory ONLY variant of SaveContext and LoadContext --> moved to backend/; added an enum Backend to choose between Disk*Context and InMemory*Context; moved Disk*Context to backend/
1 parent 14ceecc commit 407ff32

10 files changed

Lines changed: 753 additions & 405 deletions

File tree

diskann-record/src/backend/disk.rs

Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
/*
2+
* Copyright (c) Microsoft Corporation.
3+
* Licensed under the MIT license.
4+
*/
5+
6+
//! Disk-backed save/load contexts.
7+
//!
8+
//! [`DiskSaveContext`] writes the manifest as JSON plus side-car artifact files into a
9+
//! directory; [`DiskLoadContext`] reads them back. The two halves are independent and
10+
//! communicate only through the filesystem. Both are available under the `disk` feature.
11+
12+
use std::{
13+
collections::HashSet,
14+
io::BufReader,
15+
path::{Path, PathBuf},
16+
sync::Mutex,
17+
};
18+
19+
use crate::{
20+
load::{self, LoadContext, Reader},
21+
save::{self, SaveContext, Value, Writer},
22+
};
23+
24+
/// The disk-backed [`SaveContext`].
25+
///
26+
/// Holds the manifest directory, the manifest path, and the set of artifact file names
27+
/// registered so far. Lookup and insertion go through a [`Mutex`] so that concurrent
28+
/// [`Save`](crate::save::Save) impls cannot accidentally hand out the same artifact name
29+
/// twice.
30+
#[derive(Debug)]
31+
pub(crate) struct DiskSaveContext {
32+
dir: PathBuf,
33+
metadata: PathBuf,
34+
files: Mutex<HashSet<String>>,
35+
}
36+
37+
#[derive(serde::Serialize)]
38+
struct Final<'a> {
39+
files: Vec<&'a str>,
40+
value: &'a Value<'a>,
41+
}
42+
43+
impl DiskSaveContext {
44+
/// Create a disk-backed save context targeting `dir` for side-car artifacts and
45+
/// `metadata` for the manifest. Validates that `dir` is an actual directory.
46+
///
47+
/// # Errors
48+
///
49+
/// Returns [`save::Error`] if `dir` does not exist, cannot be inspected, or exists but
50+
/// is not a directory.
51+
pub(crate) fn new(dir: PathBuf, metadata: PathBuf) -> save::Result<Self> {
52+
match std::fs::metadata(&dir) {
53+
Ok(meta) if meta.is_dir() => {}
54+
Ok(_) => {
55+
return Err(save::Error::message(format!(
56+
"path {} exists but is not a directory",
57+
dir.display()
58+
)));
59+
}
60+
Err(err) => {
61+
return Err(save::Error::new(err)
62+
.context(format!("while validating path {}", dir.display())));
63+
}
64+
}
65+
66+
Ok(Self {
67+
dir,
68+
metadata,
69+
files: Mutex::new(HashSet::new()),
70+
})
71+
}
72+
}
73+
74+
impl SaveContext for DiskSaveContext {
75+
type Output = ();
76+
77+
fn write(&self, key: Option<&str>) -> save::Result<Writer<'_>> {
78+
// When a human-readable hint is supplied it must be a simple relative file name:
79+
// reject absolute paths, parent traversal, and multi-component paths so the prefix
80+
// below produces a single, well-formed file name in the manifest directory.
81+
if let Some(key) = key {
82+
let mut components = std::path::Path::new(key).components();
83+
match components.next() {
84+
Some(std::path::Component::Normal(_)) if components.next().is_none() => {}
85+
_ => {
86+
return Err(save::Error::message(format!(
87+
"artifact file name hint {:?} must be a relative file name with no path \
88+
separators",
89+
key,
90+
)));
91+
}
92+
}
93+
}
94+
95+
let mut files = self
96+
.files
97+
.lock()
98+
.unwrap_or_else(|poison| poison.into_inner());
99+
100+
// Prefix each artifact with the count of artifacts written so far, so that reusing
101+
// the same `key` (or omitting it) still yields a unique file name.
102+
let name = match key {
103+
Some(key) => format!("{:03}-{}", files.len(), key),
104+
None => format!("{:03}", files.len()),
105+
};
106+
107+
if !files.insert(name.clone()) {
108+
return Err(save::Error::message(format!(
109+
"generated artifact name {:?} collides with an existing artifact",
110+
name,
111+
)));
112+
}
113+
let full = self.dir.join(&name);
114+
if full.exists() {
115+
return Err(save::Error::message(format!(
116+
"file {} already exists",
117+
full.display()
118+
)));
119+
}
120+
let file = std::fs::File::create_new(&full).map_err(|err| {
121+
save::Error::new(err).context(format!("while creating new file {}", full.display()))
122+
})?;
123+
Ok(Writer::file(name, file))
124+
}
125+
126+
/// Finalize the manifest.
127+
///
128+
/// Writes the manifest JSON atomically: serializes to a `<metadata>.temp` file first,
129+
/// then renames it into place. Fails if the temp file already exists (an in-flight
130+
/// save is in progress, or a previous run aborted between rename steps).
131+
fn finish(self, value: Value<'_>) -> save::Result<()> {
132+
let files = self
133+
.files
134+
.into_inner()
135+
.unwrap_or_else(|poison| poison.into_inner());
136+
let f = Final {
137+
files: files.iter().map(|k| &**k).collect(),
138+
value: &value,
139+
};
140+
141+
// Fail if the temp file already exists
142+
let mut temp = self.metadata.clone().into_os_string();
143+
temp.push(".temp");
144+
let temp = PathBuf::from(temp);
145+
let buffer = std::fs::File::create_new(&temp).map_err(|err| {
146+
if err.kind() == std::io::ErrorKind::AlreadyExists {
147+
save::Error::message(format!(
148+
"Temporary file {} already exists. Aborting!",
149+
temp.display()
150+
))
151+
} else {
152+
save::Error::new(err).context(format!(
153+
"while creating temp manifest file {}",
154+
temp.display()
155+
))
156+
}
157+
})?;
158+
159+
serde_json::to_writer_pretty(buffer, &f)
160+
.map_err(|err| save::Error::new(err).context("while serializing manifest to JSON"))?;
161+
std::fs::rename(&temp, &self.metadata).map_err(|err| {
162+
save::Error::new(err).context(format!(
163+
"while renaming temp manifest {} to final path {}",
164+
temp.display(),
165+
self.metadata.display()
166+
))
167+
})?;
168+
Ok(())
169+
}
170+
}
171+
172+
/// The disk-backed [`LoadContext`].
173+
///
174+
/// Reads the manifest produced by [`DiskSaveContext`] and resolves side-car artifact
175+
/// handles against the manifest directory.
176+
#[derive(Debug)]
177+
pub(crate) struct DiskLoadContext {
178+
dir: PathBuf,
179+
files: HashSet<PathBuf>,
180+
value: Value<'static>,
181+
}
182+
183+
#[derive(serde::Deserialize)]
184+
struct FileRepr {
185+
files: HashSet<PathBuf>,
186+
value: Value<'static>,
187+
}
188+
189+
impl DiskLoadContext {
190+
pub(crate) fn new(metadata: &Path, dir: &Path) -> load::Result<Self> {
191+
let file = std::fs::File::open(metadata).map_err(|e| {
192+
load::Error::new(e).context(format!("while trying to open {}", metadata.display()))
193+
})?;
194+
195+
let reader = BufReader::new(file);
196+
let repr: FileRepr = serde_json::from_reader(reader)
197+
.map_err(|e| load::Error::new(e).context("could not deserialize manifest"))?;
198+
199+
Ok(Self {
200+
dir: dir.into(),
201+
files: repr.files,
202+
value: repr.value,
203+
})
204+
}
205+
}
206+
207+
impl LoadContext for DiskLoadContext {
208+
fn value(&self) -> load::Result<&Value<'_>> {
209+
Ok(&self.value)
210+
}
211+
212+
fn read(&self, key: &str) -> load::Result<Reader<'_>> {
213+
let key_as_path: &Path = key.as_ref();
214+
let mut components = key_as_path.components();
215+
match components.next() {
216+
Some(std::path::Component::Normal(_)) if components.next().is_none() => {}
217+
_ => {
218+
return Err(
219+
load::Error::from(load::error::Kind::MissingFile).context(format!(
220+
"handle references file {:?} which escapes the manifest directory",
221+
key,
222+
)),
223+
);
224+
}
225+
}
226+
if !self.files.contains(key_as_path) {
227+
return Err(
228+
load::Error::from(load::error::Kind::MissingFile).context(format!(
229+
"handle references file {:?} which is not registered in the manifest",
230+
key,
231+
)),
232+
);
233+
}
234+
235+
let full = self.dir.join(key);
236+
let file = std::fs::File::open(&full).map_err(|err| {
237+
load::Error::new(err).context(format!("while opening artifact file {}", full.display()))
238+
})?;
239+
240+
Ok(Reader::new(Box::new(file)))
241+
}
242+
}
243+
244+
#[cfg(test)]
245+
mod tests {
246+
use std::path::{Path, PathBuf};
247+
248+
use super::*;
249+
250+
#[test]
251+
fn new_rejects_nonexistent_directory() {
252+
let missing = PathBuf::from("does/not/exist/anywhere/at/all");
253+
let err = DiskSaveContext::new(missing, "meta.json".into())
254+
.expect_err("a nonexistent directory must be rejected");
255+
assert!(format!("{err}").contains("while validating path"));
256+
}
257+
258+
#[test]
259+
fn new_rejects_file_as_directory() {
260+
let dir = tempfile::tempdir().unwrap();
261+
let file = dir.path().join("not_a_dir");
262+
std::fs::write(&file, b"hi").unwrap();
263+
let err = DiskSaveContext::new(file, dir.path().join("meta.json"))
264+
.expect_err("a file path must be rejected as a directory");
265+
assert!(format!("{err}").contains("is not a directory"));
266+
}
267+
268+
#[test]
269+
fn write_rejects_path_separators_and_traversal() {
270+
let dir = tempfile::tempdir().unwrap();
271+
let ctx = DiskSaveContext::new(dir.path().into(), dir.path().join("meta.json")).unwrap();
272+
for bad in ["sub/dir.bin", "../escape.bin", "/abs.bin"] {
273+
SaveContext::write(&ctx, Some(bad))
274+
.expect_err("keys with path separators must be rejected");
275+
}
276+
}
277+
278+
#[test]
279+
fn write_allows_duplicate_key() {
280+
let dir = tempfile::tempdir().unwrap();
281+
let ctx = DiskSaveContext::new(dir.path().into(), dir.path().join("meta.json")).unwrap();
282+
let first = SaveContext::write(&ctx, Some("artifact.bin"))
283+
.unwrap()
284+
.finish()
285+
.unwrap();
286+
let second = SaveContext::write(&ctx, Some("artifact.bin"))
287+
.unwrap()
288+
.finish()
289+
.unwrap();
290+
assert_ne!(
291+
first.as_str(),
292+
second.as_str(),
293+
"duplicate keys must be disambiguated by the count prefix"
294+
);
295+
assert_eq!(first.as_str(), "000-artifact.bin");
296+
assert_eq!(second.as_str(), "001-artifact.bin");
297+
}
298+
299+
#[test]
300+
fn write_allows_anonymous_artifact() {
301+
let dir = tempfile::tempdir().unwrap();
302+
let ctx = DiskSaveContext::new(dir.path().into(), dir.path().join("meta.json")).unwrap();
303+
let handle = SaveContext::write(&ctx, None).unwrap().finish().unwrap();
304+
assert!(!handle.as_str().is_empty());
305+
}
306+
307+
fn write_manifest(dir: &Path, files: &[&str]) -> PathBuf {
308+
let manifest = serde_json::json!({
309+
"files": files,
310+
"value": { "$version": "0.0.0" },
311+
});
312+
let metadata = dir.join("metadata.json");
313+
std::fs::write(&metadata, serde_json::to_vec(&manifest).unwrap()).unwrap();
314+
metadata
315+
}
316+
317+
#[test]
318+
fn read_rejects_unregistered_file() {
319+
let dir = tempfile::tempdir().unwrap();
320+
let metadata = write_manifest(dir.path(), &[]);
321+
let ctx = DiskLoadContext::new(&metadata, dir.path()).unwrap();
322+
let Err(err) = ctx.read("artifact.bin") else {
323+
panic!("an unregistered file must be rejected");
324+
};
325+
assert!(format!("{err}").contains("not registered in the manifest"));
326+
}
327+
328+
#[test]
329+
fn read_rejects_escaping_handle() {
330+
let dir = tempfile::tempdir().unwrap();
331+
// Register the escaping name so only the path-shape check can reject it.
332+
let metadata = write_manifest(dir.path(), &["../escape.bin"]);
333+
let ctx = DiskLoadContext::new(&metadata, dir.path()).unwrap();
334+
let Err(err) = ctx.read("../escape.bin") else {
335+
panic!("a handle escaping the manifest directory must be rejected");
336+
};
337+
assert!(format!("{err}").contains("escapes the manifest directory"));
338+
}
339+
}

0 commit comments

Comments
 (0)