diff --git a/platform/dr-plat/src/lib.rs b/platform/dr-plat/src/lib.rs index a889172..5225c40 100644 --- a/platform/dr-plat/src/lib.rs +++ b/platform/dr-plat/src/lib.rs @@ -10,4 +10,7 @@ pub mod storage; pub use secrets::{ EphemeralSecretStore, PlatformSecretStore, SecretError, SecretKind, SecretRef, SecretStore, }; -pub use storage::{DirRef, Entry, LocalStorage, Node, SeekableRead, Storage, StorageError}; +pub use storage::{ + DirRef, Entry, LocalStorage, NewFile, Node, SeekableRead, Storage, StorageError, + WritableStorage, +}; diff --git a/platform/dr-plat/src/storage.rs b/platform/dr-plat/src/storage.rs index 9817646..326cb6a 100644 --- a/platform/dr-plat/src/storage.rs +++ b/platform/dr-plat/src/storage.rs @@ -38,7 +38,7 @@ use std::collections::BTreeMap; use std::fmt; -use std::io::{Read, Seek}; +use std::io::{Read, Seek, Write}; use std::path::{Component, Path, PathBuf}; use dr_types::{ByteRange, DirEntry, DirState, RootId, SourceRef}; @@ -193,6 +193,22 @@ pub enum StorageError { #[error("range reads unsupported by this storage")] RangeUnsupported, + /// A name that is already taken. + /// + /// Distinguished from a plain io error because it is the one failure an + /// import can recover from on its own, by resolving a different name. + #[error("already exists: {0}")] + AlreadyExists(String), + + /// A name no destination could hold — empty, `.`, `..`, or containing a + /// separator. + /// + /// A separator is refused rather than created as nested directories: the + /// caller asked for one child and would get a tree, and on SAF the call + /// would create a single document with a slash in its display name. + #[error("invalid name: {0:?}")] + InvalidName(String), + #[error("io error: {0}")] Io(String), } @@ -245,6 +261,86 @@ pub trait Storage: Send + Sync { fn read_range(&self, src: &SourceRef, range: ByteRange) -> Result, StorageError>; } +/// A file that has been created but not yet filled. +/// +/// The reference comes back *with* the sink because on SAF those are one +/// operation and two results: `createDocument` returns the new document's URI, +/// and only that URI can open the stream. A caller that had to ask for the +/// reference afterwards would have to find the file by name — the one thing +/// [`WritableStorage::create_file`] exists to avoid. +pub struct NewFile { + /// How to reach the file once it is written. This is the provider's own + /// answer, not a name the caller composed. + pub source: SourceRef, + /// Where the bytes go. Dropping it closes the file. + pub sink: Box, +} + +impl std::fmt::Debug for NewFile { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("NewFile") + .field("source", &self.source) + .finish_non_exhaustive() + } +} + +/// TRACES: FR-CAT-10 | NFR-PORT-1 +/// Create directories and files under a granted root. +/// +/// Separate from [`Storage`] rather than folded into it, because the two are +/// not granted together. A library root may be read-only — a card mounted +/// `ro`, a share the user has view rights on — and the type system should say +/// so at the point a caller needs to write rather than at the point one +/// discovers `EROFS`. Ingest asks for a `&dyn WritableStorage` destination and +/// a plain `&dyn Storage` source, which is exactly the asymmetry of copying +/// off a card. +/// +/// # Why this is not `write(path, bytes)` +/// +/// Same reason [`Storage::list`] hands back references (see the module docs): +/// a SAF document id is not composable, so a destination is reached by +/// creating each level and keeping what the provider returned. Every method +/// here therefore takes a *parent reference plus one name* and returns the +/// reference the provider produced. +/// +/// The other constraint SAF imposes is that `createDocument` renames on +/// collision by itself, appending ` (1)` and returning a URI for a name nobody +/// asked for. So an implementation must decide the name before creating +/// anything, and callers must read [`NewFile::source`] rather than assume the +/// file is called what they asked for. +pub trait WritableStorage: Storage { + /// Get or create a child directory. + /// + /// Idempotent: an existing directory of that name is returned as it is, + /// never duplicated. That is load-bearing rather than a convenience — + /// importing a second card from the same day must land in the same folder, + /// and on SAF the naive call would produce `2026-08-22 (1)`. + fn create_dir(&self, parent: &DirRef, name: &str) -> Result; + + /// Create a new file and open it for writing. + /// + /// Fails with [`StorageError::AlreadyExists`] rather than truncating. + /// Overwriting is never what an import wants, and the caller that *does* + /// want a distinct name resolves one first with [`Self::exists`] — the + /// same shape `dr_export::resolve_name` uses. + fn create_file(&self, parent: &DirRef, name: &str) -> Result; + + /// Whether a name is already taken in this directory. + /// + /// Answers for files and directories alike: a destination cannot hold both + /// a folder and an image called `2026-08-22`, so a collision check that + /// only looked at one would be wrong half the time. + fn exists(&self, parent: &DirRef, name: &str) -> Result; + + /// Remove a file this storage created. + /// + /// Needed for the failure path rather than as a feature: an import that + /// dies partway through verification has written a file that no catalog + /// knows about, and leaving it behind means the next run sees a duplicate + /// of something that was never successfully imported (FR-CAT-10). + fn remove_file(&self, src: &SourceRef) -> Result<(), StorageError>; +} + /// TRACES: FR-PLAT-LIN-1 | NFR-PORT-1 /// The filesystem implementation: a root is a directory the user picked. /// @@ -487,6 +583,99 @@ impl Storage for LocalStorage { /// A file whose mtime is unreadable compares equal to itself forever and so is /// never re-read. That is the better failure: the alternative, a value that /// changes each time it is asked for, would re-process the file on every scan. +/// TRACES: FR-CAT-10 +/// A name that is one ordinary child, and not a way out of its directory. +/// +/// Checked here rather than left to `resolve`, because the failure is +/// different in kind: `resolve` guards against a *stored key* the catalog +/// carries, where this guards against a name a *template* just produced. The +/// user typing `{make}/{model}` into a filename field should be told the name +/// is invalid, not have two directories appear. +fn check_name(name: &str) -> Result<(), StorageError> { + if name.is_empty() + || name == "." + || name == ".." + || name.contains('/') + || name.contains('\\') + || name.contains('\0') + { + return Err(StorageError::InvalidName(name.to_string())); + } + Ok(()) +} + +impl WritableStorage for LocalStorage { + fn create_dir(&self, parent: &DirRef, name: &str) -> Result { + check_name(name)?; + let key = child_key(parent.key(), name); + let path = self.resolve(parent.root_id(), &key)?; + match std::fs::create_dir(&path) { + Ok(()) => {} + // Already there is success, not a collision: see the trait docs + // on why importing twice in a day depends on it. A *file* of that + // name is a real conflict and is reported as one. + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + if !path.is_dir() { + return Err(StorageError::AlreadyExists(path.display().to_string())); + } + } + Err(e) => return Err(map_io(&path, e)), + } + Ok(DirRef::from_parts(parent.root_id(), key)) + } + + fn create_file(&self, parent: &DirRef, name: &str) -> Result { + check_name(name)?; + let key = child_key(parent.key(), name); + let path = self.resolve(parent.root_id(), &key)?; + // `create_new` rather than `create`: the check and the creation are + // one syscall, so two imports running at once cannot both decide a + // name is free and have the second silently truncate the first. + let file = std::fs::File::options() + .write(true) + .create_new(true) + .open(&path) + .map_err(|e| match e.kind() { + std::io::ErrorKind::AlreadyExists => { + StorageError::AlreadyExists(path.display().to_string()) + } + _ => map_io(&path, e), + })?; + Ok(NewFile { + source: SourceRef::Local { + root: parent.root_id(), + relative: key, + }, + sink: Box::new(std::io::BufWriter::new(file)), + }) + } + + fn exists(&self, parent: &DirRef, name: &str) -> Result { + check_name(name)?; + let path = self.resolve(parent.root_id(), &child_key(parent.key(), name))?; + // `symlink_metadata` rather than `exists()`: a broken symlink is a + // name that is taken, and `exists()` reports it as free — after which + // `create_new` fails and the import stops on a name it was told was + // available. + match std::fs::symlink_metadata(&path) { + Ok(_) => Ok(true), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(map_io(&path, e)), + } + } + + fn remove_file(&self, src: &SourceRef) -> Result<(), StorageError> { + let path = self.file_path(src)?; + match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + // Already gone is the state the caller wanted. This runs on a + // failure path, where a second error would mask the first. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(map_io(&path, e)), + } + } +} + fn modified_millis(meta: &std::fs::Metadata) -> i64 { let Ok(t) = meta.modified() else { return 0; @@ -557,6 +746,115 @@ mod tests { const ROOT: RootId = RootId(1); + // ---- the write half (FR-CAT-10) ------------------------------------- + + #[test] + fn a_created_file_is_reachable_by_the_reference_it_returned() { + let t = Tree::new("create-file"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + let mut nf = st.create_file(&root, "IMG_0001.CR3").unwrap(); + nf.sink.write_all(b"raw bytes").unwrap(); + drop(nf.sink); + + // The point of handing the reference back: the caller reads what it + // wrote without ever composing a key. + let got = st.read_range(&nf.source, 0..64).unwrap(); + assert_eq!(got, b"raw bytes"); + } + + #[test] + fn creating_a_directory_twice_returns_the_same_one() { + let t = Tree::new("create-dir-twice"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + // Two cards imported on the same day. The second must land in the + // folder the first made, not beside it. + let a = st.create_dir(&root, "2026").unwrap(); + let b = st.create_dir(&root, "2026").unwrap(); + assert_eq!(a, b); + + let day = st.create_dir(&a, "2026-08-22").unwrap(); + assert_eq!(day.key(), "2026/2026-08-22"); + } + + #[test] + fn an_existing_file_is_refused_rather_than_truncated() { + let t = Tree::new("no-truncate"); + t.file("keep.CR3", b"the original"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + let err = st.create_file(&root, "keep.CR3").unwrap_err(); + assert!(matches!(err, StorageError::AlreadyExists(_)), "{err:?}"); + // And the bytes are still there — the failure happened before any + // truncation, which is the whole reason `create_new` is used. + assert_eq!(fs::read(t.0.join("keep.CR3")).unwrap(), b"the original"); + } + + #[test] + fn a_directory_of_that_name_is_not_mistaken_for_a_file() { + let t = Tree::new("dir-not-file"); + t.dir("2026-08-22"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + // A *file* where a directory is wanted is a genuine conflict... + t.file("taken", b"x"); + let err = st.create_dir(&root, "taken").unwrap_err(); + assert!(matches!(err, StorageError::AlreadyExists(_)), "{err:?}"); + } + + #[test] + fn a_name_with_a_separator_is_refused_rather_than_creating_a_tree() { + let t = Tree::new("no-separators"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + for bad in ["a/b", "..", ".", "", "a\\b"] { + let err = st.create_file(&root, bad).unwrap_err(); + assert!( + matches!(err, StorageError::InvalidName(_)), + "{bad:?} gave {err:?}" + ); + } + // And the escape does not happen by another route: nothing was made + // outside the root. + assert!(!t.0.parent().unwrap().join("b").exists()); + } + + #[test] + fn a_taken_name_is_reported_before_anything_is_created() { + let t = Tree::new("exists"); + t.file("IMG_0001.CR3", b"x").dir("2026"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + assert!(st.exists(&root, "IMG_0001.CR3").unwrap()); + // Directories count too: a destination cannot hold both. + assert!(st.exists(&root, "2026").unwrap()); + assert!(!st.exists(&root, "IMG_0002.CR3").unwrap()); + } + + #[test] + fn a_half_written_file_can_be_taken_back() { + let t = Tree::new("remove"); + let st = t.storage(); + let root = st.root_dir(ROOT).unwrap(); + + let nf = st.create_file(&root, "partial.CR3").unwrap(); + let src = nf.source.clone(); + drop(nf.sink); + st.remove_file(&src).unwrap(); + assert!(!t.0.join("partial.CR3").exists()); + // Removing what is already gone is the state the caller wanted, and + // this runs on a failure path where a second error would mask the + // first. + st.remove_file(&src).unwrap(); + } + #[test] fn a_listing_names_files_and_directories_apart() { let t = Tree::new("listing");