Ask the server about a collection once per job, not once per file
Every `move_to` guaranteed its destination's parent with a `MKCOL` for each ancestor down from the account root, and a trash folder under a library root several levels deep meant three round trips answering `405 Method Not Allowed` before the one `MOVE` that did anything — for every image of a delete, on a connection built for that job. The backend now records the collections it has confirmed exist and asks about each once. It lives for one job, so a folder another client removes mid-batch is the one case this misses, and the `MOVE` then reports the `409` rather than hiding it.
This commit is contained in:
@@ -47,6 +47,20 @@ pub struct NextcloudBackend {
|
||||
/// `/remote.php/dav/files/<user>/` — the prefix stripped from hrefs.
|
||||
dav_base: String,
|
||||
caps: Capabilities,
|
||||
/// Collections this backend has seen exist, so [`create_dir`] asks the
|
||||
/// server about each one once.
|
||||
///
|
||||
/// Every `MOVE` guarantees its destination's parent, and did so with a
|
||||
/// `MKCOL` for each ancestor down from the account root — for a trash
|
||||
/// folder three levels deep that was three round trips of `405 Method
|
||||
/// Not Allowed` before the one request that moved anything, on every
|
||||
/// image of a batch. A backend lives for one job (`dr_ui::remote::
|
||||
/// connect` builds one per worker), so a folder deleted by another
|
||||
/// client mid-job is the one case this can get wrong, and it is reported
|
||||
/// as the `409` the `MOVE` then earns rather than hidden.
|
||||
///
|
||||
/// [`create_dir`]: RemoteBackend::create_dir
|
||||
known_dirs: std::sync::Mutex<std::collections::HashSet<String>>,
|
||||
}
|
||||
|
||||
impl NextcloudBackend {
|
||||
@@ -63,6 +77,7 @@ impl NextcloudBackend {
|
||||
login: creds.login_name.clone(),
|
||||
password: creds.app_password.clone(),
|
||||
dav_base,
|
||||
known_dirs: Default::default(),
|
||||
caps: Capabilities {
|
||||
// The property that makes a no-op sync one request (ARCH §8.1).
|
||||
change_detection: ChangeDetection::PropagatingEtags,
|
||||
@@ -567,6 +582,16 @@ impl RemoteBackend for NextcloudBackend {
|
||||
chain.reverse();
|
||||
|
||||
for dir in chain {
|
||||
// Asked once per backend — see `known_dirs`. The lock is held
|
||||
// across no await: it is taken to look, and again to record.
|
||||
let known = self
|
||||
.known_dirs
|
||||
.lock()
|
||||
.map(|k| k.contains(dir.as_str()))
|
||||
.unwrap_or(false);
|
||||
if known {
|
||||
continue;
|
||||
}
|
||||
let url = self.url_for(&dir);
|
||||
let resp = self
|
||||
.client
|
||||
@@ -581,10 +606,12 @@ impl RemoteBackend for NextcloudBackend {
|
||||
|
||||
// 405 is "already a collection here", which is exactly what the
|
||||
// caller wanted. Anything else is reported.
|
||||
if resp.status() == reqwest::StatusCode::METHOD_NOT_ALLOWED {
|
||||
continue;
|
||||
if resp.status() != reqwest::StatusCode::METHOD_NOT_ALLOWED {
|
||||
map_status(resp.status(), &url)?;
|
||||
}
|
||||
if let Ok(mut k) = self.known_dirs.lock() {
|
||||
k.insert(dir.as_str().to_string());
|
||||
}
|
||||
map_status(resp.status(), &url)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user