diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 31b7e9d0..4abf2aa2 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -72,7 +72,12 @@ pub fn sync_run( let device_id = info.device_id.as_str(); // FIXME: Bad device_id assumption? - let ds_localremote = setup_local_remote(sync_spec.path.as_path(), device_id)?; + // Only stage a local db when this pass actually pushes. Pull-only + // `sync_run` is how `sync_wrapper::pull` walks a *peer's* host folder; + // creating `{peer_host}/{our_device_id}/test.db` there breaks the + // "each device only writes files it owns" invariant (see + // ActivityWatch/aw-server-rust#682). + let ds_localremote = maybe_setup_local_remote(sync_spec.path.as_path(), device_id, mode)?; let remote_dbfiles = crate::util::find_remotes_nonlocal( sync_spec.path.as_path(), device_id, @@ -130,16 +135,18 @@ pub fn sync_run( } // Push local server buckets to sync folder - if mode == SyncMode::Push || mode == SyncMode::Both { + if let Some(ds_localremote) = &ds_localremote { info!("Pushing..."); - sync_datastores(client, &ds_localremote, true, Some(device_id), sync_spec)?; + sync_datastores(client, ds_localremote, true, Some(device_id), sync_spec)?; } // Close open database connections for ds_from in &ds_remotes { ds_from.close(); } - ds_localremote.close(); + if let Some(ds_localremote) = &ds_localremote { + ds_localremote.close(); + } // Dropping also works to close the database connections, weirdly enough. // Probably because once the database is dropped, the thread will stop, @@ -182,6 +189,23 @@ pub fn list_buckets(client: &AwClient) -> Result<(), Box> { Ok(()) } +fn maybe_setup_local_remote( + path: &Path, + device_id: &str, + mode: SyncMode, +) -> Result, Box> { + if mode == SyncMode::Push || mode == SyncMode::Both { + Ok(Some(setup_local_remote(path, device_id)?)) + } else { + // `get_sync_dir()` is path construction only. Push/Both used to create + // the root as a side effect of staging; pull-only still needs the root + // so `find_remotes` does not NotFound (advanced CLI / pull-only daemon + // on a fresh machine). Do not create `{path}/{device_id}/`. + fs::create_dir_all(path)?; + Ok(None) + } +} + fn setup_local_remote(path: &Path, device_id: &str) -> Result> { // FIXME: Don't run twice if already exists fs::create_dir_all(path)?; @@ -596,3 +620,69 @@ fn log_buckets(ds: &dyn AccessMethod) -> Result<(), String> { } Ok(()) } + +#[cfg(test)] +mod pull_only_staging_tests { + use super::*; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn temp_dir() -> PathBuf { + let p = std::env::temp_dir().join(format!( + "aw-sync-pull-only-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&p).unwrap(); + p + } + + #[test] + fn pull_does_not_create_local_staging_db() { + let dir = temp_dir(); + let staged = maybe_setup_local_remote(&dir, "device-local", SyncMode::Pull).unwrap(); + assert!(staged.is_none()); + assert!( + !dir.join("device-local").exists(), + "pull-only must not create {{peer}}/{{our_device_id}}/" + ); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn pull_creates_missing_sync_root_without_local_staging() { + let dir = temp_dir(); + fs::remove_dir_all(&dir).unwrap(); + assert!(!dir.exists()); + let staged = maybe_setup_local_remote(&dir, "device-local", SyncMode::Pull).unwrap(); + assert!(staged.is_none()); + assert!( + dir.is_dir(), + "pull-only must create the sync root so discovery does not NotFound" + ); + assert!( + !dir.join("device-local").exists(), + "pull-only must not create {{peer}}/{{our_device_id}}/" + ); + let remotes = crate::util::find_remotes_nonlocal(&dir, "device-local", None).unwrap(); + assert!(remotes.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn push_creates_local_staging_dir() { + let dir = temp_dir(); + let staged = maybe_setup_local_remote(&dir, "device-local", SyncMode::Push).unwrap(); + assert!(staged.is_some()); + // Datastore::new opens sqlite on a worker thread, so test.db may not + // exist yet; the directory is created synchronously and is the leak + // pull-only used to leave in a peer folder. + assert!(dir.join("device-local").is_dir()); + if let Some(ds) = staged { + ds.close(); + } + let _ = fs::remove_dir_all(&dir); + } +}