dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Let storage write as well as read

Browse Source 
Storage enumerates and reads, which is all a scan ever needed. An import
writes, and there was nothing to write through.

WritableStorage is separate from Storage rather than folded into it, because
the two are not granted together: a card mounted read-only, or a share the
user has view rights on, should fail to typecheck as a destination rather
than fail with EROFS halfway through a copy. Ingest takes a &dyn Storage
source and a &dyn WritableStorage destination, which is exactly the asymmetry
of copying off a card.

Shaped for SAF throughout, for the same reason the read side is: a document
id is not composable, so every call takes a parent reference plus one name
and hands back the reference the provider itself produced. create_dir is
idempotent because importing a second card on the same day must land in the
folder the first made, and the naive SAF call would produce "2026-08-22 (1)".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-22 14:00:26 +02:00
co-authored by Claude Opus 5
parent c36d4c80c8
commit 37d8744db8
2 changed files with 303 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
platform/dr-plat/src/storage.rs
+299 -1
View File
@@ -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<Vec<u8>, 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<dyn Write + Send>,
}
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<DirRef, StorageError>;
/// 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<NewFile, StorageError>;
/// 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<bool, StorageError>;
/// 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<DirRef, StorageError> {
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<NewFile, StorageError> {
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<bool, StorageError> {
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");