mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-04 16:56:59 +01:00
fix(fuse,core): list and resolve conflict copies in synced folders, log each once (B99)
The conflict sweep flagged divergent copies anywhere in the tree but kept its dedupe set in memory, so every daemon start logged them again. The conflict listing only covered My Files, so copies in on-demand sync folders were never listed, and the Activity page showed them as resolved while they were still on disk. List copies in synced folders by their absolute local path, route resolves to the mount that serves them, flag only what the listing can show, and consult the activity log before flagging a copy again.
This commit is contained in:
1 parent
912acc760a
commit
e6cb52339d
8 files changed
+171
-21
No files matched your search
@@ -429,10 +429,12 @@ pub enum Request {
|
||||
/// Retry a backed-off queued op now instead of waiting out its backoff;
|
||||
/// every failed op when `id` is `None`. Replies with [`Response::Ok`].
|
||||
RetryPendingOp { id: Option<i64> },
|
||||
/// List the `(sync-conflict …)` copies under My Files. Replies with
|
||||
/// List the `(sync-conflict …)` copies under My Files and in synced
|
||||
/// folders. Replies with
|
||||
/// [`Response::Conflicts`].
|
||||
ListConflicts,
|
||||
/// Resolve one conflict copy (`path` mountpoint-relative or absolute).
|
||||
/// Resolve one conflict copy (`path` mountpoint-relative, or absolute for a
|
||||
/// copy in a synced folder).
|
||||
/// Everything removed goes to the Proton trash. Replies with
|
||||
/// [`Response::Ok`].
|
||||
ResolveConflict { path: String, keep: ConflictKeep },
|
||||
@@ -701,9 +703,10 @@ pub struct DeviceInfo {
|
||||
/// [`Response::Conflicts`]).
|
||||
#[derive(Serialize, Deserialize, Debug, Clone)]
|
||||
pub struct ConflictInfo {
|
||||
/// Mountpoint-relative path of the copy.
|
||||
/// Mountpoint-relative path of the copy, or its absolute local path when it
|
||||
/// sits in a synced folder.
|
||||
pub path: String,
|
||||
/// Mountpoint-relative path of the file it is a copy of.
|
||||
/// Path of the file it is a copy of, in the same form as `path`.
|
||||
pub original_path: String,
|
||||
/// False when the original is gone and only the copy is left.
|
||||
pub original_exists: bool,
|
||||
|
||||
@@ -29,6 +29,17 @@ impl Db {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether the kept log already holds an entry of `kind` for `target`. Lets
|
||||
/// the daemon report something once across restarts rather than once per
|
||||
/// run, without keeping a second table in step with the log.
|
||||
pub fn activity_has(&self, kind: ActivityKind, target: &str) -> Result<bool> {
|
||||
let kind = serde_json::to_string(&kind)?;
|
||||
let conn = self.read();
|
||||
let mut stmt =
|
||||
conn.prepare("SELECT 1 FROM activity WHERE kind = ?1 AND target = ?2 LIMIT 1")?;
|
||||
Ok(stmt.exists(params![kind, target])?)
|
||||
}
|
||||
|
||||
/// The most recent activity, newest first, capped at `limit` entries. Rows
|
||||
/// whose stored `kind` no longer parses (written by an older build) are
|
||||
/// skipped rather than failing the whole read.
|
||||
|
||||
@@ -1414,6 +1414,29 @@ fn activity_reads_back_newest_first() {
|
||||
assert_eq!(db.activity_list(1).unwrap().len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn activity_has_finds_only_matching_kind_and_target() {
|
||||
let db = Db::open_in_memory().unwrap();
|
||||
db.activity_add(&activity(
|
||||
"a (sync-conflict 1).txt",
|
||||
ActivityKind::Conflict,
|
||||
false,
|
||||
))
|
||||
.unwrap();
|
||||
db.activity_add(&activity("b.txt", ActivityKind::Upload, true))
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
db.activity_has(ActivityKind::Conflict, "a (sync-conflict 1).txt")
|
||||
.unwrap()
|
||||
);
|
||||
assert!(
|
||||
!db.activity_has(ActivityKind::Upload, "a (sync-conflict 1).txt")
|
||||
.unwrap()
|
||||
);
|
||||
assert!(!db.activity_has(ActivityKind::Conflict, "b.txt").unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn activity_prunes_to_the_keep_limit() {
|
||||
let db = Db::open_in_memory().unwrap();
|
||||
|
||||
@@ -14,15 +14,38 @@
|
||||
//! that is open, being written, or still owed an upload, for the same reason the
|
||||
//! sweep does: the node is about to change under the decision.
|
||||
//!
|
||||
//! Only copies under My Files are handled here. A copy inside a mirror folder is
|
||||
//! a local file first; the sync engine owns it.
|
||||
//! Copies under My Files are named by their mountpoint-relative path. Copies
|
||||
//! inside a synced folder are named by their absolute local path, so a resolve
|
||||
//! request routes to the on-demand mount that serves them. A copy inside a
|
||||
//! mirror folder is listed too, but it is a local file first: the sync engine
|
||||
//! owns it, so it is resolved by editing the folder, not through here.
|
||||
|
||||
use super::*;
|
||||
use pdfs_core::control::{ConflictInfo, ConflictKeep};
|
||||
use sweep::conflict_base_name;
|
||||
|
||||
impl Core {
|
||||
/// Every conflict copy under My Files, with the file it is a copy of.
|
||||
/// Where the user finds a conflict copy: its mountpoint-relative path under
|
||||
/// My Files, or its absolute local path inside a synced folder. `None` for a
|
||||
/// node no location of this machine shows, such as one in another
|
||||
/// computer's device tree.
|
||||
///
|
||||
/// The sweep only flags what this can name, and the listing uses it too. A
|
||||
/// flagged copy the listing cannot see reads as resolved in the activity
|
||||
/// feed while it still sits in the folder (`docs/BUGS.md` B99).
|
||||
pub(crate) fn conflict_path(&self, roots: &SearchRoots, uid: &NodeUid) -> Option<String> {
|
||||
let uid = uid.to_string();
|
||||
if let Ok(Some(path)) = self
|
||||
.db
|
||||
.path_relative_to(&self.primary_root_uid.to_string(), &uid)
|
||||
{
|
||||
return Some(path);
|
||||
}
|
||||
let drive_path = self.db.node_path(&uid).ok().flatten()?;
|
||||
roots.resolve(&drive_path)
|
||||
}
|
||||
|
||||
/// Every conflict copy this machine shows, with the file it is a copy of.
|
||||
pub(crate) fn list_conflicts(&self) -> CoreResult<Vec<ConflictInfo>> {
|
||||
let stored = self
|
||||
.db
|
||||
@@ -37,13 +60,13 @@ impl Core {
|
||||
for node in &nodes {
|
||||
by_parent_name.insert((node.parent_uid.as_ref(), node.name.as_str()), node);
|
||||
}
|
||||
let root = self.primary_root_uid.to_string();
|
||||
let roots = self.search_roots();
|
||||
let mut items = Vec::new();
|
||||
for node in &nodes {
|
||||
let Some(base) = conflict_base_name(&node.name) else {
|
||||
continue;
|
||||
};
|
||||
let Ok(Some(path)) = self.db.path_relative_to(&root, &node.uid.to_string()) else {
|
||||
let Some(path) = self.conflict_path(&roots, &node.uid) else {
|
||||
continue;
|
||||
};
|
||||
let original = by_parent_name
|
||||
|
||||
@@ -185,6 +185,38 @@ fn route_to_mount(core: &Core, mountpoint: &Path, path: &str) -> CoreResult<(Cor
|
||||
rel_to_mount(mountpoint, path).map(|rel| (core.clone(), rel))
|
||||
}
|
||||
|
||||
/// [`route_to_mount`] for a conflict copy. The listing names a copy inside a
|
||||
/// synced folder by its absolute local path (`docs/BUGS.md` B99), so the
|
||||
/// resolve has to run against the on-demand mount serving it. A mirror folder
|
||||
/// has no mount to run against: its copy is an ordinary local file.
|
||||
fn route_conflict(core: &Core, mountpoint: &Path, path: &str) -> CoreResult<(Core, PathBuf)> {
|
||||
let routed = route_to_mount(core, mountpoint, path).map_err(|e| {
|
||||
let p = Path::new(path);
|
||||
let mirrored = core
|
||||
.db
|
||||
.sync_folder_list()
|
||||
.unwrap_or_default()
|
||||
.iter()
|
||||
.any(|folder| folder.mode == "mirror" && p.starts_with(&folder.local_path));
|
||||
if mirrored {
|
||||
CoreError::invalid(format!(
|
||||
"{path} is in a mirrored folder; rename or delete the local file there"
|
||||
))
|
||||
} else {
|
||||
e
|
||||
}
|
||||
})?;
|
||||
if routed
|
||||
.1
|
||||
.components()
|
||||
.all(|component| matches!(component, Component::Normal(_) | Component::CurDir))
|
||||
{
|
||||
Ok(routed)
|
||||
} else {
|
||||
Err(CoreError::invalid(format!("{path} escapes the mountpoint")))
|
||||
}
|
||||
}
|
||||
|
||||
/// Turn a CLI-supplied path into a mountpoint-relative path. An absolute path
|
||||
/// must live under `mountpoint`; a relative path is taken as already relative to
|
||||
/// the mount root.
|
||||
@@ -1123,13 +1155,15 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
|
||||
Ok(items) => CtlResponse::Conflicts { items },
|
||||
Err(e) => CtlResponse::error(e),
|
||||
},
|
||||
Ok(CtlRequest::ResolveConflict { path, keep }) => match rel_to_mount(mountpoint, &path) {
|
||||
Ok(rel) => match core.resolve_conflict(&rel, &keep) {
|
||||
Ok(message) => CtlResponse::Ok { message },
|
||||
Ok(CtlRequest::ResolveConflict { path, keep }) => {
|
||||
match route_conflict(core, mountpoint, &path) {
|
||||
Ok((core, rel)) => match core.resolve_conflict(&rel, &keep) {
|
||||
Ok(message) => CtlResponse::Ok { message },
|
||||
Err(e) => CtlResponse::error(e),
|
||||
},
|
||||
Err(e) => CtlResponse::error(e),
|
||||
},
|
||||
Err(e) => CtlResponse::error(e),
|
||||
},
|
||||
}
|
||||
}
|
||||
Ok(CtlRequest::ListPendingOps) => match core.pending_op_infos() {
|
||||
Ok(items) => CtlResponse::PendingOps { items },
|
||||
Err(e) => CtlResponse::error(e),
|
||||
|
||||
@@ -558,9 +558,10 @@ struct Core {
|
||||
/// instead of polling or blocking for the whole run. See
|
||||
/// [`Core::await_trash_refresh`].
|
||||
trash_progress: Arc<tokio::sync::Notify>,
|
||||
/// Conflict copies the sweep has already flagged as needing attention this
|
||||
/// run, so a divergent `(sync-conflict …)` file is logged once rather than
|
||||
/// on every sweep pass. See [`Core::run_conflict_sweep_loop`].
|
||||
/// Conflict copies the sweep has already checked against the activity log
|
||||
/// this run, so each pass does not ask the database again. The log itself
|
||||
/// is what keeps a copy from being flagged again after a restart. See
|
||||
/// [`Core::run_conflict_sweep_loop`] and `docs/BUGS.md` B99.
|
||||
conflict_notified: Arc<Mutex<HashSet<NodeUid>>>,
|
||||
/// Whether the conflict sweep may actually trash the duplicates it finds, or
|
||||
/// only report them. Resolved once at mount from config + environment
|
||||
|
||||
@@ -109,6 +109,7 @@ impl Core {
|
||||
by_parent_name.insert((parent, node.name.as_str()), node);
|
||||
}
|
||||
|
||||
let roots = self.search_roots();
|
||||
let online = self.online.load(std::sync::atomic::Ordering::Relaxed);
|
||||
for node in &nodes {
|
||||
if node.trashed || node.is_folder() {
|
||||
@@ -132,7 +133,16 @@ impl Core {
|
||||
self.remove_conflict_copy(node, &base);
|
||||
}
|
||||
}
|
||||
SweepAction::Flag(detail) => self.flag_conflict(&node.uid, &node.name, detail),
|
||||
// Only flag what the conflict listing can show. Anything else
|
||||
// would sit in the feed as "resolved" with no way to act on
|
||||
// it (`docs/BUGS.md` B99).
|
||||
SweepAction::Flag(detail) => {
|
||||
if self.conflict_path(&roots, &node.uid).is_some() {
|
||||
self.flag_conflict(&node.uid, &node.name, detail);
|
||||
} else {
|
||||
debug!(uid = %node.uid, name = %node.name, "conflict sweep: copy is outside every location; not flagged");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -273,12 +283,21 @@ impl Core {
|
||||
busy
|
||||
}
|
||||
|
||||
/// Surface a conflict copy that needs the user's attention, at most once per
|
||||
/// daemon run so the activity feed is not spammed each pass.
|
||||
/// Surface a conflict copy that needs the user's attention, once. The
|
||||
/// in-memory set spares the database each pass; the activity log itself is
|
||||
/// what keeps a restart from logging the same copy again (`docs/BUGS.md`
|
||||
/// B99). A copy's name carries its own timestamp, so the name identifies it.
|
||||
fn flag_conflict(&self, uid: &NodeUid, name: &str, detail: String) {
|
||||
if !self.conflict_notified.lock().insert(uid.clone()) {
|
||||
return;
|
||||
}
|
||||
match self.db.activity_has(ActivityKind::Conflict, name) {
|
||||
Ok(false) => {}
|
||||
Ok(true) => return,
|
||||
// Cannot tell: a repeat entry is harmless, a missing one hides a
|
||||
// conflict.
|
||||
Err(e) => warn!(%uid, name, error = ?e, "conflict sweep: activity check failed"),
|
||||
}
|
||||
debug!(%uid, name, detail, "conflict sweep: flagged divergent conflict copy");
|
||||
self.log_activity(ActivityKind::Conflict, name, detail, false);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user