diff --git a/core/dr-sync-nextcloud/src/lib.rs b/core/dr-sync-nextcloud/src/lib.rs index 9c4ed61..c0e49ee 100644 --- a/core/dr-sync-nextcloud/src/lib.rs +++ b/core/dr-sync-nextcloud/src/lib.rs @@ -530,6 +530,20 @@ mod tests { )); } + #[test] + fn addressing_by_path_is_what_works() { + // The counterpart to the rejection above, and the reason trash and + // purge pass `RemoteId::Path` even when the catalog knows the fileid: + // every id-taking method routes through here, so a `Stable` turns a + // trash into "operation unsupported by this backend". + let b = backend(); + assert_eq!( + b.url_for_id(&RemoteId::Path(RemotePath::new("Photos/_MG_8154.dng"))) + .unwrap(), + "https://cloud.example/remote.php/dav/files/duncan/Photos/_MG_8154.dng" + ); + } + #[test] fn statuses_map_to_actionable_errors() { use reqwest::StatusCode; diff --git a/core/dr-sync/src/types.rs b/core/dr-sync/src/types.rs index ac12d0a..6b06d6f 100644 --- a/core/dr-sync/src/types.rs +++ b/core/dr-sync/src/types.rs @@ -66,6 +66,16 @@ impl std::fmt::Display for RemotePath { /// survives server-side rename and move, so a move is detected as a move /// rather than a delete plus a re-download of a large file (FR-NC-5). /// Otherwise it falls back to the path, and moves cost a transfer. +/// +/// **Identity is not an address.** `Stable` answers "is this the same object +/// as before?"; it does not necessarily name one the backend can address. +/// Nextcloud, the only backend, exposes no fileid-addressable endpoint, so it +/// serves [`RemoteBackend::get`], [`delete`](RemoteBackend::delete) and +/// [`move_to`](RemoteBackend::move_to) by path alone and rejects a `Stable` +/// with [`RemoteError::Unsupported`]. Callers holding both — the catalog keeps +/// the fileid beside `remote_path` — must pass the path to those three and +/// keep the fileid for what it is good for: matching moves across scans, and +/// keying the thumbnail shards. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum RemoteId { /// A server-assigned identifier, e.g. Nextcloud's `oc:fileid`. diff --git a/ui/dr-ui/src/trash.rs b/ui/dr-ui/src/trash.rs index 21de435..f2357ad 100644 --- a/ui/dr-ui/src/trash.rs +++ b/ui/dr-ui/src/trash.rs @@ -65,14 +65,20 @@ pub enum Direction { /// One image to move, resolved before the worker starts. /// -/// Carries the id the `MOVE` addresses and the destination path, so the worker -/// needs no catalog access to do its half — the catalog is not `Send`, and the -/// worker owns a separate connection only for the write-back. +/// Carries the paths the `MOVE` runs between, so the worker needs no catalog +/// access to do its half — the catalog is not `Send`, and the worker owns a +/// separate connection only for the write-back. #[derive(Debug, Clone)] pub struct Move { pub image_id: ImageId, - /// `oc:fileid` where known, else the path. A stable id survives the move and - /// keeps the thumbnail and sidecar mapping attached. + /// `oc:fileid` where the catalog knows one — the identity `MOVE` preserves, + /// which is what keeps the thumbnail and sidecar mapping attached across a + /// trash and restore. + /// + /// Not how the file is addressed: the `MOVE` goes from `from` to `to`, + /// because WebDAV exposes no fileid-addressable endpoint and the backend + /// rejects a bare `RemoteId::Stable`. Carried so the plan records the + /// identity it expects to survive. pub file_id: Option, pub from: String, pub to: String, @@ -199,13 +205,22 @@ pub fn spawn_move( let mut failed: Vec = Vec::new(); for (i, mv) in moves.iter().enumerate() { - let id = match mv.file_id { - Some(f) => RemoteId::Stable(f), - None => RemoteId::Path(RemotePath::new(&mv.from)), - }; + // Addressed by path, not by `mv.file_id`: WebDAV has no + // fileid-addressable endpoint, so a `RemoteId::Stable` here is + // rejected by the backend. The fileid is an identity that the + // MOVE preserves, not a way to name the source. + let id = RemoteId::Path(RemotePath::new(&mv.from)); match backend.move_to(&id, &RemotePath::new(&mv.to)).await { - Ok(()) => succeeded.push((mv.image_id, mv.to.clone())), + Ok(()) => { + // The fileid is logged, not sent: if a restore later + // shows a missing thumbnail, this is the record of which + // identity the MOVE was supposed to carry across. + if let Some(f) = mv.file_id { + log::debug!("moved {} to {} as fileid {f}", mv.from, mv.to); + } + succeeded.push((mv.image_id, mv.to.clone())); + } Err(e) => { // Named by file, not by id: the user recognises the // filename and cannot do anything with a row number. @@ -307,10 +322,9 @@ pub fn spawn_purge( let mut failed: Vec = Vec::new(); for (i, (image, file_id, path)) in paths.iter().enumerate() { - let id = match file_id { - Some(f) => RemoteId::Stable(*f), - None => RemoteId::Path(RemotePath::new(path)), - }; + // By path — see `spawn_move`. `file_id` still matters below, as + // the key the thumbnail shards are stored under. + let id = RemoteId::Path(RemotePath::new(path)); match backend.delete(&id, None).await { Ok(()) => {