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

Recognise a card whose photographs are already held

Browse Source 
Re-inserting a card that has already been imported produced a folder full of
`-1` copies. Both halves of the placement logic treated a taken name as a
collision to rename around, which is right for the case they were written for
— two cameras both writing IMG_0001.CR3 — and exactly wrong for the far more
common one, where the taken name is the same photograph.

Locally this cannot be answered from the catalog. On a library whose catalog
describes a *server*, a file sitting in the local destination has no row to be
found by, so the only way to know whether it has already been copied is to
look. The destination folder is listed once per folder rather than probed per
file: a card is two thousand frames landing in a handful of days.

Remotely the same question is one PROPFIND that was already being made to
resolve the name, so `upload_original` now returns `Placed::AlreadyThere`
instead of inventing a second copy of work that is already safe. The count is
reported apart from `uploaded`, because "12 already on the server" and "12
uploaded" are different answers to whether this run backed anything up — and
apart from the local duplicate count, because these files *were* copied here.

Name plus length decides it, not a digest: this runs before any transfer, and
hashing to answer it would read the whole card to avoid reading the whole card.
A camera reusing a filename after IMG_9999 writes a different number of bytes
essentially always, which leaves the rename for the case it is really for. The
digest tier still catches the same frame under a different name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-22 19:19:59 +02:00
co-authored by Claude Opus 5
parent b1bf68022d
commit e3dd1526e0
3 changed files with 408 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-sync/src/lib.rs
+1 -1
View File
@@ -35,7 +35,7 @@ pub use types::{
Cursor, EntryKind, Identity, Precondition, RemoteChange, RemoteEntry, RemoteId, RemotePath,
Validator,
};
pub use upload::{destination, upload_original};
pub use upload::{destination, upload_original, Placed};
/// TRACES: FR-NC-12
/// A remote storage backend.
core/dr-sync/src/upload.rs
+157 -30
View File
@@ -37,64 +37,123 @@ pub fn destination(library: &RemotePath, folders: &[String]) -> RemotePath {
.fold(library.clone(), |acc, seg| acc.join(seg))
}
/// Upload one original into its dated folder.
/// TRACES: FR-NC-7a | FR-CAT-11
/// What became of an original the caller offered.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Placed {
/// Sent now.
Uploaded {
path: RemotePath,
validator: Validator,
},
/// The server already held this photograph, so nothing was transferred.
///
/// The case a re-inserted card produces: the shoot was imported and
/// uploaded last week, and the card has not been formatted since. Worth a
/// variant of its own rather than folding into `Uploaded`, because the two
/// are different answers to "is my work backed up" — and because a caller
/// counting bytes moved must not count these.
AlreadyThere { path: RemotePath },
}
impl Placed {
/// Where the photograph is on the server, however it got there.
pub fn path(&self) -> &RemotePath {
match self {
Placed::Uploaded { path, .. } | Placed::AlreadyThere { path } => path,
}
}
}
/// Upload one original into its dated folder, unless it is already there.
///
/// Creates the folders if they are missing — `create_dir` makes parents and
/// succeeds on one that already exists, so the second import of a day costs
/// one request rather than a conflict.
///
/// Returns where it actually went, which may not be the name that was asked
/// Returns where the photograph is, which may not be the name that was asked
/// for: two cameras produce `IMG_0001.CR3` and the second must not overwrite
/// the first. The caller records the returned path, never the one it passed.
///
/// `size` is what decides between "already there" and "a different photograph
/// with the same name". See [`resolve`].
pub async fn upload_original(
backend: &dyn RemoteBackend,
library: &RemotePath,
folders: &[String],
name: &str,
body: Vec<u8>,
) -> Result<(RemotePath, Validator), RemoteError> {
) -> Result<Placed, RemoteError> {
let dir = destination(library, folders);
backend.create_dir(&dir).await?;
let name = free_name(backend, &dir, name).await?;
let path = dir.join(&name);
let validator = backend.put(&path, body, None).await?;
Ok((path, validator))
match resolve(backend, &dir, name, body.len() as u64).await? {
Resolution::AlreadyThere => Ok(Placed::AlreadyThere {
path: dir.join(name),
}),
Resolution::Use(name) => {
let path = dir.join(&name);
let validator = backend.put(&path, body, None).await?;
Ok(Placed::Uploaded { path, validator })
}
}
}
/// A name not already taken in a remote folder.
/// What to do about a name in the destination folder.
#[derive(Debug, Clone, PartialEq, Eq)]
enum Resolution {
/// Write under this name.
Use(String),
/// The photograph is already on the server. Send nothing.
AlreadyThere,
}
/// Decide what to call an original in a remote folder, or that it is already
/// there.
///
/// One listing rather than a probe per candidate: a day folder is listed in a
/// single `PROPFIND` and the answer covers every collision in it, where
/// probing would be a round trip per attempt over a link that may be mobile
/// data.
///
/// **Name plus length decides "already there".** A card that was imported and
/// uploaded last week and has not been formatted since is the ordinary case,
/// and re-uploading it would double the storage and fill the folder with `-1`
/// copies of a shoot that is already safe. The same length under the same name
/// in the folder this photograph belongs in is that photograph in every case
/// that is not deliberately constructed — and a camera that reuses a filename
/// after `IMG_9999` writes a *different* number of bytes essentially always,
/// which is what leaves the rename below for the case it is actually for.
///
/// A folder that cannot be listed yields the original name rather than an
/// error. That is the safe direction on a server that has just been asked to
/// create the folder: the alternative is failing an upload because a listing
/// raced the `MKCOL`, and a genuine collision still fails at the `PUT` if the
/// caller asked for a precondition.
async fn free_name(
/// raced the `MKCOL`.
async fn resolve(
backend: &dyn RemoteBackend,
dir: &RemotePath,
name: &str,
) -> Result<String, RemoteError> {
let taken: std::collections::HashSet<String> = match backend.list(dir, None).await {
size: u64,
) -> Result<Resolution, RemoteError> {
let taken: std::collections::HashMap<String, u64> = match backend.list(dir, None).await {
Ok(entries) => entries
.into_iter()
.filter(|e| e.kind == EntryKind::File)
.map(|e| e.path.name().to_string())
.map(|e| (e.path.name().to_string(), e.size))
.collect(),
Err(RemoteError::NotFound(_)) => Default::default(),
Err(e) => return Err(e),
};
if !taken.contains(name) {
return Ok(name.to_string());
match taken.get(name) {
None => return Ok(Resolution::Use(name.to_string())),
Some(&there) if there == size => return Ok(Resolution::AlreadyThere),
Some(_) => {}
}
// The same suffix convention the local import uses, so a photograph that
// collided on both sides carries the same name in both places.
// A different photograph that happens to share a name. The same suffix
// convention the local import uses, so one that collided on both sides
// carries the same name in both places.
let (stem, ext) = match name.rsplit_once('.') {
Some((s, e)) if !s.is_empty() => (s, Some(e)),
_ => (name, None),
@@ -104,8 +163,12 @@ async fn free_name(
Some(ext) => format!("{stem}-{n}.{ext}"),
None => format!("{stem}-{n}"),
};
if !taken.contains(&candidate) {
return Ok(candidate);
match taken.get(&candidate) {
None => return Ok(Resolution::Use(candidate)),
// The renamed copy is itself already up there: this is the second
// re-import of a card that legitimately collided once.
Some(&there) if there == size => return Ok(Resolution::AlreadyThere),
Some(_) => {}
}
}
Err(RemoteError::Unsupported(
@@ -147,7 +210,8 @@ mod tests {
// fake in `dr-sync-nextcloud`'s tests and against a real server in S8.
// What is worth pinning here is the collision rule, since it decides
// whether a second camera's `IMG_0001.CR3` overwrites the first.
struct Listing(Vec<&'static str>);
/// A folder holding files of given name and length.
struct Listing(Vec<(&'static str, u64)>);
#[async_trait::async_trait]
impl RemoteBackend for Listing {
@@ -165,12 +229,12 @@ mod tests {
Ok(self
.0
.iter()
.map(|n| RemoteEntry {
.map(|(n, size)| RemoteEntry {
id: RemoteId::Path(dir.join(n)),
path: dir.join(n),
kind: EntryKind::File,
validator: Validator::new("v"),
size: 0,
size: *size,
modified: None,
has_preview: false,
})
@@ -237,9 +301,12 @@ mod tests {
}
#[test]
fn a_second_cameras_img_0001_does_not_overwrite_the_first() {
let backend = Listing(vec!["IMG_0001.CR3"]);
let (path, _) = block_on(upload_original(
fn a_card_already_on_the_server_transfers_nothing() {
// The ordinary case: last week's shoot was imported and uploaded, and
// the card has not been formatted since. Re-uploading would double the
// storage and litter the folder with `-1` copies of safe work.
let backend = Listing(vec![("IMG_0001.CR3", 3)]);
let placed = block_on(upload_original(
&backend,
&RemotePath::new("PhotosRaw"),
&segs(&["2026", "2026-08-22"]),
@@ -247,13 +314,38 @@ mod tests {
vec![1, 2, 3],
))
.unwrap();
assert_eq!(path.as_str(), "PhotosRaw/2026/2026-08-22/IMG_0001-1.CR3");
assert_eq!(
placed,
Placed::AlreadyThere {
path: RemotePath::new("PhotosRaw/2026/2026-08-22/IMG_0001.CR3")
}
);
}
#[test]
fn a_second_cameras_img_0001_does_not_overwrite_the_first() {
// Same name, different length: a genuinely different photograph, which
// is what the rename is for.
let backend = Listing(vec![("IMG_0001.CR3", 99)]);
let placed = block_on(upload_original(
&backend,
&RemotePath::new("PhotosRaw"),
&segs(&["2026", "2026-08-22"]),
"IMG_0001.CR3",
vec![1, 2, 3],
))
.unwrap();
assert_eq!(
placed.path().as_str(),
"PhotosRaw/2026/2026-08-22/IMG_0001-1.CR3"
);
assert!(matches!(placed, Placed::Uploaded { .. }));
}
#[test]
fn a_free_name_is_used_as_it_is() {
let backend = Listing(vec!["IMG_0002.CR3"]);
let (path, _) = block_on(upload_original(
let backend = Listing(vec![("IMG_0002.CR3", 3)]);
let placed = block_on(upload_original(
&backend,
&RemotePath::new("PhotosRaw"),
&segs(&["2026", "2026-08-22"]),
@@ -261,6 +353,41 @@ mod tests {
vec![1, 2, 3],
))
.unwrap();
assert_eq!(path.as_str(), "PhotosRaw/2026/2026-08-22/IMG_0001.CR3");
assert_eq!(
placed.path().as_str(),
"PhotosRaw/2026/2026-08-22/IMG_0001.CR3"
);
assert!(matches!(placed, Placed::Uploaded { .. }));
}
#[test]
fn a_renamed_copy_that_is_already_up_there_is_not_sent_twice() {
// The card that collided once and is now being re-imported: both the
// original name and the renamed copy are on the server, and this
// photograph is the renamed one.
let backend = Listing(vec![("IMG_0001.CR3", 99), ("IMG_0001-1.CR3", 3)]);
let placed = block_on(upload_original(
&backend,
&RemotePath::new("PhotosRaw"),
&segs(&["2026", "2026-08-22"]),
"IMG_0001.CR3",
vec![1, 2, 3],
))
.unwrap();
assert!(matches!(placed, Placed::AlreadyThere { .. }), "{placed:?}");
}
#[test]
fn an_empty_folder_takes_the_name_as_offered() {
let backend = Listing(vec![]);
let placed = block_on(upload_original(
&backend,
&RemotePath::new("PhotosRaw"),
&segs(&["2026", "2026-08-22"]),
"IMG_0001.CR3",
vec![1, 2, 3],
))
.unwrap();
assert!(matches!(placed, Placed::Uploaded { .. }));
}
}