diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 35a9cefe..eabcce06 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -548,6 +548,19 @@ fn sync_one( info!(" = Synced {} new events", new_events_count); } else { info!(" ✓ Already up to date!"); + // Resume-from-newest is one-way: events older than dest's newest are never + // fetched. If a short recent slice was imported first, a later pull of the + // complete history reports "up to date" while dest is missing most of it. + // Warn loudly so that case is diagnosable (ActivityWatch/aw-server-rust#683). + if resume_sync_at.is_some() { + let src_count = ds_from.get_event_count(bucket_from.id.as_str())?; + if src_count > eventcount_to_new { + warn!( + " ! Source bucket '{}' has {src_count} events but destination '{}' has {eventcount_to_new} after a resume-from-newest pull. Older history in the source was not imported. Delete the destination bucket and re-pull to recover.", + bucket_from.id, bucket_to.id + ); + } + } } Ok(()) diff --git a/aw-sync/src/sync_wrapper.rs b/aw-sync/src/sync_wrapper.rs index 47067a58..c5f438a6 100644 --- a/aw-sync/src/sync_wrapper.rs +++ b/aw-sync/src/sync_wrapper.rs @@ -1,20 +1,33 @@ use std::error::Error; use std::fs; +use std::path::Path; use crate::sync::{sync_run, SyncMode, SyncSpec}; use aw_client_rust::blocking::AwClient; pub fn pull_all(client: &AwClient) -> Result<(), Box> { - let hostnames = crate::util::get_remotes()?; - for host in hostnames { - pull(&host, client)? + let sync_root = crate::dirs::get_sync_dir().map_err(|_| "Could not get sync dir")?; + let dbs = crate::util::list_remote_dbs(&sync_root)?; + let selected = crate::util::select_remote_dbs_by_device_id(dbs); + if selected.is_empty() { + info!("No remote databases found in {:?}", sync_root); + return Ok(()); + } + info!( + "Pulling {} remote database(s): {:?}", + selected.len(), + selected + .iter() + .map(|d| d.path.display().to_string()) + .collect::>() + ); + for remote in selected { + pull_db(client, &remote.hostname, &remote.path)?; } Ok(()) } pub fn pull(host: &str, client: &AwClient) -> Result<(), Box> { - client.wait_for_start()?; - // Path to the sync folder // Sync folder is structured ./{hostname}/{device_id}/test.db let sync_root_dir = crate::dirs::get_sync_dir().map_err(|_| "Could not get sync dir")?; @@ -45,14 +58,20 @@ pub fn pull(host: &str, client: &AwClient) -> Result<(), Box> { .max_by_key(|entry| entry.metadata().map(|m| m.len()).unwrap_or(0)) .ok_or_else(|| format!("No db found in sync folder {:?}", sync_dir))?; + pull_db(client, host, &db.path()) +} + +fn pull_db(client: &AwClient, host: &str, db_path: &Path) -> Result<(), Box> { + client.wait_for_start()?; + let sync_root_dir = crate::dirs::get_sync_dir().map_err(|_| "Could not get sync dir")?; + let sync_dir = sync_root_dir.join(host); let sync_spec = SyncSpec { - path: sync_dir.clone(), - path_db: Some(db.path().clone()), + path: sync_dir, + path_db: Some(db_path.to_path_buf()), buckets: None, // Sync all buckets by default start: None, }; sync_run(client, &sync_spec, SyncMode::Pull)?; - Ok(()) } diff --git a/aw-sync/src/util.rs b/aw-sync/src/util.rs index a026123d..1caf5950 100644 --- a/aw-sync/src/util.rs +++ b/aw-sync/src/util.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::error::Error; use std::ffi::OsStr; use std::fs; @@ -166,57 +167,244 @@ mod tests { assert_eq!(host_for_url("127.0.0.1"), "127.0.0.1"); assert_eq!(host_for_url("localhost"), "localhost"); } + + fn temp_sync_root() -> std::path::PathBuf { + let p = std::env::temp_dir().join(format!( + "aw-sync-remotes-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&p).unwrap(); + p + } + + fn write_remote_db( + root: &std::path::Path, + host: &str, + device_id: &str, + size: usize, + ) -> std::path::PathBuf { + let dir = root.join(host).join(device_id); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("test.db"); + fs::write(&path, vec![0u8; size]).unwrap(); + path + } + + #[test] + fn list_remote_dbs_walks_hostname_device_layout() { + let root = temp_sync_root(); + let large = write_remote_db(&root, "poco_f8_ultra", "device-1", 64); + let small = write_remote_db(&root, "POCO F8 Ultra", "device-1", 8); + write_remote_db(&root, "other-host", "device-2", 16); + + let mut listed = super::list_remote_dbs(&root).unwrap(); + listed.sort_by(|a, b| a.path.cmp(&b.path)); + fs::remove_dir_all(&root).unwrap(); + + assert_eq!(listed.len(), 3); + assert!(listed.iter().any(|d| d.path == large && d.size == 64)); + assert!(listed.iter().any(|d| d.path == small && d.size == 8)); + assert!(listed.iter().any(|d| d.device_id == "device-2")); + } + + #[test] + fn list_remote_dbs_skips_legacy_two_level_root_dbs() { + // `{sync_root}/{device_id}/test.db` is the leftover daemon layout + // from #682. pull_all must not import it. + let root = temp_sync_root(); + let three = write_remote_db(&root, "poco_f8_ultra", "device-1", 64); + let two_level_dir = root.join("device-orphan"); + fs::create_dir_all(&two_level_dir).unwrap(); + let orphan = two_level_dir.join("test.db"); + fs::write(&orphan, vec![0u8; 128]).unwrap(); + + let listed = super::list_remote_dbs(&root).unwrap(); + fs::remove_dir_all(&root).unwrap(); + + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].path, three); + assert!(listed.iter().all(|d| d.path != orphan)); + } + + #[test] + fn select_remote_dbs_keeps_largest_per_device_id() { + let root = temp_sync_root(); + let large = write_remote_db(&root, "poco_f8_ultra", "device-1", 64); + write_remote_db(&root, "POCO F8 Ultra", "device-1", 8); + let other = write_remote_db(&root, "other-host", "device-2", 16); + + let listed = super::list_remote_dbs(&root).unwrap(); + // Reverse to prove selection does not depend on discovery order. + let mut reversed = listed.clone(); + reversed.reverse(); + let selected = super::select_remote_dbs_by_device_id(reversed); + fs::remove_dir_all(&root).unwrap(); + + assert_eq!(selected.len(), 2); + let device_1 = selected.iter().find(|d| d.device_id == "device-1").unwrap(); + assert_eq!(device_1.path, large); + assert_eq!(device_1.size, 64); + let device_2 = selected.iter().find(|d| d.device_id == "device-2").unwrap(); + assert_eq!(device_2.path, other); + } + + #[test] + fn select_remote_dbs_keeps_unique_device_ids() { + let a = super::RemoteDb { + hostname: "host-a".into(), + device_id: "aaa".into(), + path: std::path::PathBuf::from("/sync/host-a/aaa/test.db"), + size: 10, + }; + let b = super::RemoteDb { + hostname: "host-b".into(), + device_id: "bbb".into(), + path: std::path::PathBuf::from("/sync/host-b/bbb/test.db"), + size: 1, + }; + let selected = super::select_remote_dbs_by_device_id(vec![a.clone(), b.clone()]); + assert_eq!(selected, vec![a, b]); + } + + #[test] + fn select_db_paths_keeps_largest_per_device_id() { + let root = temp_sync_root(); + let large = write_remote_db(&root, "poco_f8_ultra", "device-1", 64); + write_remote_db(&root, "POCO F8 Ultra", "device-1", 8); + let other = write_remote_db(&root, "other-host", "device-2", 16); + let listed = super::list_remote_dbs(&root) + .unwrap() + .into_iter() + .map(|d| d.path) + .collect(); + let selected = super::select_db_paths_by_device_id(listed); + fs::remove_dir_all(&root).unwrap(); + assert_eq!(selected.len(), 2); + assert!(selected.contains(&large)); + assert!(selected.contains(&other)); + } } -/// Check if a directory contains a .db file -fn contains_db_file(dir: &std::path::Path) -> bool { - fs::read_dir(dir) - .ok() - .map(|entries| { - entries.filter_map(Result::ok).any(|entry| { - entry - .path() - .extension() - .map(|ext| ext == "db") - .unwrap_or(false) - }) - }) - .unwrap_or(false) +/// A peer database discovered under `{sync_root}/{hostname}/{device_id}/*.db`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct RemoteDb { + pub hostname: String, + pub device_id: String, + pub path: PathBuf, + pub size: u64, } -/// Check if a directory contains a subdirectory that contains a .db file -fn contains_subdir_with_db_file(dir: &std::path::Path) -> bool { - fs::read_dir(dir) - .ok() - .map(|entries| { - entries - .filter_map(Result::ok) - .any(|entry| entry.path().is_dir() && contains_db_file(&entry.path())) - }) - .unwrap_or(false) +/// List every `{hostname}/{device_id}/*.db` under `sync_root`. +/// +/// Returns device_id and file size so callers can collapse duplicate folders +/// for one device before importing. I/O errors are propagated rather than +/// skipped: dropping a host directory we failed to read would report a +/// successful sync that quietly omitted that host's data. +/// +/// 3-level-only by design. This is the `pull_all` walker. A leftover 2-level +/// root db (`{sync_root}/{device_id}/test.db`, no hostname folder) is **not** +/// a pull candidate. That is the correct outcome: the 1.19 GB root orphan +/// from ActivityWatch/aw-server-rust#682 must never be imported by a peer. +/// Do not "fix" this by broadening the walk — the advanced `sync_run` path +/// still uses [`find_remotes`] (2-level, relative to whatever directory it +/// is given). Two walkers, two code paths; that is intentional. +pub(crate) fn list_remote_dbs(sync_root: &Path) -> std::io::Result> { + let mut dbs = Vec::new(); + if !sync_root.exists() { + return Ok(dbs); + } + for host_ent in fs::read_dir(sync_root)? { + let host_ent = host_ent?; + let host_path = host_ent.path(); + if !host_path.is_dir() { + continue; + } + let Some(hostname) = host_ent.file_name().to_str().map(str::to_string) else { + continue; + }; + for device_ent in fs::read_dir(&host_path)? { + let device_ent = device_ent?; + let device_path = device_ent.path(); + if !device_path.is_dir() { + continue; + } + let Some(device_id) = device_ent.file_name().to_str().map(str::to_string) else { + continue; + }; + for file_ent in fs::read_dir(&device_path)? { + let file_ent = file_ent?; + let path = file_ent.path(); + if !(path.is_file() && path.extension().and_then(|e| e.to_str()) == Some("db")) { + continue; + } + let size = file_ent.metadata().map(|m| m.len()).unwrap_or(0); + dbs.push(RemoteDb { + hostname: hostname.clone(), + device_id: device_id.clone(), + path, + size, + }); + } + } + } + Ok(dbs) } -/// Return all remotes in the sync folder -/// Only returns folders that match ./{host}/{device_id}/*.db -// TODO: share logic with find_remotes and find_remotes_nonlocal -pub fn get_remotes() -> Result, Box> { - let sync_root_dir = crate::dirs::get_sync_dir()?; - fs::create_dir_all(&sync_root_dir)?; - let hostnames = fs::read_dir(sync_root_dir)? - .filter_map(Result::ok) - .filter(|entry| entry.path().is_dir() && contains_subdir_with_db_file(&entry.path())) - .filter_map(|entry| { - entry - .path() - .file_name() - .and_then(|os_str| os_str.to_str().map(String::from)) - }) - .collect(); - info!("Found remotes: {:?}", hostnames); - Ok(hostnames) +/// Keep one database per `device_id`, preferring the largest file. +/// +/// A hostname change (or sanitization) can leave the same device writing under +/// two folder names. Importing both is unsafe: provenance is derived from the +/// bucket hostname, so both land in the same destination bucket, and resume +/// then silently drops the older history. See ActivityWatch/aw-server-rust#683. +pub(crate) fn select_remote_dbs_by_device_id(dbs: Vec) -> Vec { + let mut by_device: HashMap> = HashMap::new(); + for db in dbs { + by_device.entry(db.device_id.clone()).or_default().push(db); + } + + let mut selected = Vec::with_capacity(by_device.len()); + for (device_id, mut group) in by_device { + group.sort_by(|a, b| b.size.cmp(&a.size).then_with(|| a.path.cmp(&b.path))); + let mut group = group.into_iter(); + let winner = group.next().expect("device_id group is non-empty"); + let skipped: Vec = group + .map(|d| format!("{} ({} bytes)", d.path.display(), d.size)) + .collect(); + if !skipped.is_empty() { + warn!( + "device_id {device_id} appears under {} folders; using largest {} ({} bytes), skipping: {:?}", + skipped.len() + 1, + winner.path.display(), + winner.size, + skipped + ); + } + selected.push(winner); + } + selected.sort_by(|a, b| { + a.hostname + .cmp(&b.hostname) + .then_with(|| a.device_id.cmp(&b.device_id)) + .then_with(|| a.path.cmp(&b.path)) + }); + selected } -/// Returns a list of all remote dbs +/// 2-level walker: `{sync_directory}/{x}/*.db`. +/// +/// Callers pass different roots: +/// - `sync_wrapper::pull` passes a host folder, so this finds +/// `{host}/{device_id}/*.db` +/// - advanced `sync_run` against the sync root finds the legacy +/// `{device_id}/*.db` layout +/// +/// Do not broaden this to 3-level at the sync root. That would make a +/// pull import the leftover root orphan from #682. Default-daemon pull +/// is [`list_remote_dbs`] (3-level-only), not this function. /// /// I/O errors are propagated rather than unwrapped (a panic here aborts the app /// on Android, ActivityWatch/aw-android#220) and rather than skipped: silently @@ -239,14 +427,17 @@ fn find_remotes(sync_directory: &Path) -> std::io::Result> { Ok(dbs) } -/// Returns a list of all remotes, excluding local ones +/// Returns a list of all remotes, excluding local ones. +/// +/// Duplicate folders for one `device_id` are collapsed to the largest db so +/// resume-from-newest cannot silently drop history (aw-server-rust#683). pub fn find_remotes_nonlocal( sync_directory: &Path, device_id: &str, sync_db: Option<&PathBuf>, ) -> std::io::Result> { let remotes_all = find_remotes(sync_directory)?; - Ok(remotes_all + let filtered: Vec = remotes_all .into_iter() // Filter out own remote .filter(|path| !path.to_string_lossy().contains(device_id)) @@ -258,5 +449,34 @@ pub fn find_remotes_nonlocal( true } }) - .collect()) + .collect(); + Ok(select_db_paths_by_device_id(filtered)) +} + +/// Collapse `{…}/{device_id}/*.db` paths to the largest file per device_id. +fn select_db_paths_by_device_id(paths: Vec) -> Vec { + let dbs: Vec = paths + .into_iter() + .filter_map(|path| { + let device_id = path.parent()?.file_name()?.to_str()?.to_string(); + let hostname = path + .parent() + .and_then(|p| p.parent()) + .and_then(|p| p.file_name()) + .and_then(|s| s.to_str()) + .unwrap_or("") + .to_string(); + let size = fs::metadata(&path).map(|m| m.len()).unwrap_or(0); + Some(RemoteDb { + hostname, + device_id, + path, + size, + }) + }) + .collect(); + select_remote_dbs_by_device_id(dbs) + .into_iter() + .map(|d| d.path) + .collect() }