diff --git a/aw-sync/README.md b/aw-sync/README.md index 69cebfc5..ff06c042 100644 --- a/aw-sync/README.md +++ b/aw-sync/README.md @@ -80,7 +80,7 @@ We also avoid having to implement complex features such as conflict resolution, - It doesn't support Android, yet. - It mirrors events to all devices, - If you have a lot of devices you'll get a lot of duplicates, taking up a lot of space and potentially impacting performance. -- It doesn't support modifying/deleting events, yet. +- Event deletion is not synced. Owner-originated edits (same timestamp, new data) are reconciled within 7 days of the destination resume cursor; older edits are skipped to keep peak memory bounded. --- diff --git a/aw-sync/src/accessmethod.rs b/aw-sync/src/accessmethod.rs index be01d782..7bc72c32 100644 --- a/aw-sync/src/accessmethod.rs +++ b/aw-sync/src/accessmethod.rs @@ -22,6 +22,7 @@ pub trait AccessMethod: std::fmt::Debug { fn insert_events(&self, bucket_id: &str, events: Vec) -> Result<(), String>; fn get_event_count(&self, bucket_id: &str) -> Result; fn heartbeat(&self, bucket_id: &str, event: Event, duration: f64) -> Result<(), String>; + fn delete_events_by_id(&self, bucket_id: &str, event_ids: Vec) -> Result<(), String>; } /// Every method here returns a `Result`, so a datastore failure must be reported @@ -65,6 +66,11 @@ impl AccessMethod for Datastore { fn get_event_count(&self, bucket_id: &str) -> Result { Datastore::get_event_count(self, bucket_id, None, None).map_err(|e| format!("{e:?}")) } + fn delete_events_by_id(&self, bucket_id: &str, event_ids: Vec) -> Result<(), String> { + Datastore::delete_events_by_id(self, bucket_id, event_ids).map_err(|e| format!("{e:?}"))?; + self.force_commit().map_err(|e| format!("{e:?}"))?; + Ok(()) + } } impl AccessMethod for AwClient { @@ -111,4 +117,10 @@ impl AccessMethod for AwClient { fn heartbeat(&self, bucket_id: &str, event: Event, duration: f64) -> Result<(), String> { AwClient::heartbeat(self, bucket_id, &event, duration).map_err(|e| format!("{e:?}")) } + fn delete_events_by_id(&self, bucket_id: &str, event_ids: Vec) -> Result<(), String> { + for event_id in event_ids { + AwClient::delete_event(self, bucket_id, event_id).map_err(|e| e.to_string())?; + } + Ok(()) + } } diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 4abf2aa2..99a4b7a1 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -9,6 +9,7 @@ extern crate chrono; extern crate reqwest; extern crate serde_json; +use std::collections::HashMap; use std::error::Error; use std::fs; use std::path::{Path, PathBuf}; @@ -312,6 +313,11 @@ const BATCH_SIZE: usize = 5000; #[cfg(test)] const BATCH_SIZE: usize = 5; +/// How far before the resume cursor to look for owner-originated edits of +/// already-synced events (ActivityWatch/aw-android#253). Bounded so a full +/// historical bucket is never loaded into memory on Android. +const EDIT_RECONCILE_LOOKBACK: Duration = Duration::days(7); + /// Whether a bucket holds data synced from another host, rather than data /// collected on this host. /// @@ -432,6 +438,130 @@ pub fn sync_datastores( Ok(()) } +fn event_identity(event: &Event) -> (DateTime, i64) { + ( + event.timestamp, + event.duration.num_nanoseconds().unwrap_or(0), + ) +} + +/// Replace dest events whose timestamp+duration still exist on the source but +/// whose data changed. +/// +/// WebUI/Android title edits are delete+insert at the same timestamp and +/// duration. The resume cursor starts at the destination's latest event end, +/// so those replacements are outside the incremental fetch window. Under the +/// single-writer model the source is authoritative for its own buckets, so a +/// same-identity row with different data is an owner-originated edit, not a +/// conflict. Identity includes duration so two events that share a timestamp +/// are not collapsed into one HashMap slot. +/// +/// Duration-only updates of the live last event stay on the incremental +/// heartbeat path. Must run *before* that copy: a latest-event title edit is +/// also re-fetched as a start-clipped fragment, and heartbeat() refuses to +/// merge different data, which would insert a duplicate unless dest already +/// holds the new payload. +fn reconcile_updated_events( + ds_from: &dyn AccessMethod, + ds_to: &dyn AccessMethod, + bucket_from: &Bucket, + bucket_to: &Bucket, + resume_sync_at: Option>, +) -> Result<(), String> { + let Some(resume) = resume_sync_at else { + return Ok(()); + }; + let lookback_start = resume - EDIT_RECONCILE_LOOKBACK; + + // Bound both fetches to the lookback window ending at `resume`. + // end=None would load every newer source event when dest is far behind, + // exhausting Android RAM and bypassing the paginated incremental copy. + // get_events clips to the query range; dest-latest ends at `resume`, so + // that clip is a no-op on its (timestamp, duration) identity. Title edits + // keep duration; duration-only updates stay on the heartbeat path. Do not + // also cap by count — a newest-first cap silently skips older in-window + // edits. + // + // Datastore errors return rather than unwrap: a panic here aborts the + // whole pass (and on Android, the JNI frame). The per-bucket skip in + // ActivityWatch/aw-server-rust#697 then drops this bucket, not the daemon. + let source_events = ds_from.get_events( + bucket_from.id.as_str(), + Some(lookback_start), + Some(resume), + None, + )?; + let dest_events = ds_to.get_events( + bucket_to.id.as_str(), + Some(lookback_start), + Some(resume), + None, + )?; + + let mut dest_by_identity: HashMap<(DateTime, i64), Vec> = HashMap::new(); + for event in dest_events { + if event.timestamp >= lookback_start && event.timestamp < resume { + dest_by_identity + .entry(event_identity(&event)) + .or_default() + .push(event); + } + } + + let mut src_by_identity: HashMap<(DateTime, i64), Vec> = HashMap::new(); + for src in source_events { + // Skip events that start at/after the dest cursor; the incremental + // copy owns those. Do not use end>resume: the dest-latest event starts + // before resume and must still be title-reconciled. + if src.timestamp < lookback_start || src.timestamp >= resume { + continue; + } + src_by_identity + .entry(event_identity(&src)) + .or_default() + .push(src); + } + + for (identity, srcs) in src_by_identity { + let mut dsts = match dest_by_identity.remove(&identity) { + Some(dsts) if !dsts.is_empty() => dsts, + _ => continue, + }; + let mut to_insert = Vec::new(); + for src in srcs { + if let Some(idx) = dsts.iter().position(|dst| dst.data == src.data) { + // Still present on dest — keep it, including same-identity + // siblings a later source row must not treat as stale. + dsts.remove(idx); + } else { + to_insert.push(src); + } + } + if to_insert.is_empty() && dsts.is_empty() { + continue; + } + let ts = identity.0; + // Insert before delete so a crash cannot drop the row. A later pass + // sees matching data, skips insert, and still removes remaining stale ids. + if !to_insert.is_empty() { + let replacements: Vec = to_insert + .into_iter() + .map(|mut src| { + src.id = None; + src + }) + .collect(); + ds_to.insert_events(bucket_to.id.as_str(), replacements)?; + } + let stale: Vec = dsts.into_iter().filter_map(|dst| dst.id).collect(); + if !stale.is_empty() { + ds_to.delete_events_by_id(bucket_to.id.as_str(), stale)?; + } + info!(" ~ Reconciled edited event at {:?}", ts); + } + Ok(()) +} + /// Syncs a single bucket from one datastore to another fn sync_one( ds_from: &dyn AccessMethod, @@ -461,6 +591,8 @@ fn sync_one( info!(" + Starting from beginning"); } + reconcile_updated_events(ds_from, ds_to, &bucket_from, &bucket_to, resume_sync_at)?; + // Fetch events in bounded chunks to avoid OOM on devices with limited RAM (e.g. Android). // get_events returns events in descending order (newest first), so we paginate backwards // using the `end` parameter. Each chunk is written to `ds_to` as soon as it is fetched, diff --git a/aw-sync/tests/historical_edit.rs b/aw-sync/tests/historical_edit.rs new file mode 100644 index 00000000..99e3e863 --- /dev/null +++ b/aw-sync/tests/historical_edit.rs @@ -0,0 +1,459 @@ +/// Owner-originated historical edits (ActivityWatch/aw-android#253). +/// +/// WebUI/Android title edits are delete+insert at the same timestamp. aw-sync +/// resumes at the destination's latest event end, so those replacements sit +/// outside the fetch window unless a reconcile pass updates matching timestamps. +use chrono::{DateTime, Duration, Utc}; +use serde_json::json; + +use aw_datastore::Datastore; +use aw_models::{Bucket, Event}; +use aw_sync::SyncSpec; + +fn memory_pair() -> (Datastore, Datastore) { + ( + Datastore::new_in_memory(false), + Datastore::new_in_memory(false), + ) +} + +fn create_bucket(ds: &Datastore, id: &str, hostname: &str) -> String { + let bucket_json = format!( + r#"{{ + "id": "{id}", + "type": "currentwindow", + "hostname": "{hostname}", + "client": "test" + }}"# + ); + let bucket: Bucket = serde_json::from_str(&bucket_json).unwrap(); + ds.create_bucket(&bucket).unwrap(); + ds.force_commit().unwrap(); + id.to_string() +} + +fn event_at(ts: DateTime, duration: Duration, title: &str) -> Event { + let mut data = serde_json::Map::new(); + data.insert("app".to_string(), json!("VLC")); + data.insert("title".to_string(), json!(title)); + Event { + id: None, + timestamp: ts, + duration, + data, + } +} + +fn titles(ds: &Datastore, bucket_id: &str) -> Vec<(DateTime, String)> { + let mut events = ds.get_events(bucket_id, None, None, None).unwrap(); + events.sort_by_key(|e| e.timestamp); + events + .into_iter() + .map(|e| { + let title = e + .data + .get("title") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + (e.timestamp, title) + }) + .collect() +} + +/// Match the WebUI/Android edit path: delete the local row, insert a replacement +/// with the same timestamp and duration. +fn edit_title(ds: &Datastore, bucket_id: &str, ts: DateTime, new_title: &str) { + let events = ds.get_events(bucket_id, None, None, None).unwrap(); + let target = events + .iter() + .find(|e| e.timestamp == ts) + .expect("event to edit"); + let id = target.id.expect("source event id"); + ds.delete_events_by_id(bucket_id, vec![id]).unwrap(); + let mut data = target.data.clone(); + data.insert("title".to_string(), json!(new_title)); + ds.insert_events( + bucket_id, + &[Event { + id: None, + timestamp: target.timestamp, + duration: target.duration, + data, + }], + ) + .unwrap(); + ds.force_commit().unwrap(); +} + +fn sync_push(src: &Datastore, dest: &Datastore) { + aw_sync::sync_datastores(src, dest, true, Some("device-phone"), &SyncSpec::default()); +} + +fn sync_pull(src: &Datastore, dest: &Datastore) { + aw_sync::sync_datastores(src, dest, false, None, &SyncSpec::default()); +} + +#[test] +fn historical_title_edit_reaches_staging() { + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-test", "phone"); + let t0 = Utc::now() - Duration::hours(2); + let t1 = t0 + Duration::minutes(30); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(20), "sanitized.mp4"), + event_at(t1, Duration::minutes(5), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + + sync_push(&src, &dest); + assert_eq!( + titles(&dest, &bucket), + vec![ + (t0, "sanitized.mp4".to_string()), + (t1, "later window".to_string()), + ] + ); + + edit_title(&src, &bucket, t0, "Real Video Title"); + sync_push(&src, &dest); + + assert_eq!( + titles(&dest, &bucket), + vec![ + (t0, "Real Video Title".to_string()), + (t1, "later window".to_string()), + ], + "older title edit must update staging without duplicating or dropping the later event" + ); + assert_eq!(dest.get_event_count(&bucket, None, None).unwrap(), 2); +} + +#[test] +fn latest_title_edit_reaches_staging() { + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-latest", "phone"); + let t0 = Utc::now() - Duration::hours(1); + let t1 = t0 + Duration::minutes(40); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(10), "older"), + event_at(t1, Duration::minutes(15), "sanitized-latest.mp4"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + + sync_push(&src, &dest); + edit_title(&src, &bucket, t1, "Latest Real Title"); + sync_push(&src, &dest); + + assert_eq!( + titles(&dest, &bucket), + vec![ + (t0, "older".to_string()), + (t1, "Latest Real Title".to_string()), + ] + ); + assert_eq!( + dest.get_event_count(&bucket, None, None).unwrap(), + 2, + "latest-event title edit must not insert a clipped duplicate" + ); +} + +#[test] +fn historical_title_edit_reaches_peer() { + let (src, staging) = memory_pair(); + let peer = Datastore::new_in_memory(false); + let bucket = create_bucket(&src, "aw-watcher-android-peer", "phone"); + let t0 = Utc::now() - Duration::hours(3); + let t1 = t0 + Duration::hours(1); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(25), "sanitized.mp4"), + event_at(t1, Duration::minutes(8), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + + sync_push(&src, &staging); + sync_pull(&staging, &peer); + + let peer_bucket = format!("{bucket}-synced-from-phone"); + assert_eq!( + titles(&peer, &peer_bucket)[0].1, + "sanitized.mp4", + "precondition: peer imported the original title" + ); + + edit_title(&src, &bucket, t0, "Real Video Title"); + sync_push(&src, &staging); + sync_pull(&staging, &peer); + + assert_eq!( + titles(&staging, &bucket), + vec![ + (t0, "Real Video Title".to_string()), + (t1, "later window".to_string()), + ] + ); + assert_eq!( + titles(&peer, &peer_bucket), + vec![ + (t0, "Real Video Title".to_string()), + (t1, "later window".to_string()), + ], + "peer pull must pick up the owner-originated edit from staging" + ); +} + +#[test] +fn same_identity_siblings_survive_noop_sync() { + // Datastore does not require (timestamp, duration) to be unique. A + // per-source-row pass against a frozen dest snapshot treated every other + // sibling as stale and deleted them all, including on a no-op second sync. + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-identity-sib", "phone"); + let t0 = Utc::now() - Duration::hours(2); + let t1 = t0 + Duration::minutes(40); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(20), "alpha"), + event_at(t0, Duration::minutes(20), "bravo"), + event_at(t1, Duration::minutes(3), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + sync_push(&src, &dest); + + let got = titles(&dest, &bucket); + assert_eq!( + got.len(), + 3, + "noop reconcile must not drop same-identity siblings: {got:?}" + ); + assert!(got.contains(&(t0, "alpha".to_string()))); + assert!(got.contains(&(t0, "bravo".to_string()))); + assert!(got.contains(&(t1, "later window".to_string()))); +} + +#[test] +fn same_identity_sibling_edit_keeps_the_other() { + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-identity-edit", "phone"); + let t0 = Utc::now() - Duration::hours(2); + let t1 = t0 + Duration::minutes(40); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(20), "alpha"), + event_at(t0, Duration::minutes(20), "bravo"), + event_at(t1, Duration::minutes(3), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + + let target = src + .get_events(&bucket, None, None, None) + .unwrap() + .into_iter() + .find(|e| e.data.get("title").and_then(|v| v.as_str()) == Some("alpha")) + .unwrap(); + src.delete_events_by_id(&bucket, vec![target.id.unwrap()]) + .unwrap(); + let mut data = target.data.clone(); + data.insert("title".to_string(), json!("alpha-edited")); + src.insert_events( + &bucket, + &[Event { + id: None, + timestamp: target.timestamp, + duration: target.duration, + data, + }], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + + let got = titles(&dest, &bucket); + assert_eq!( + got.len(), + 3, + "editing one sibling must not drop the other: {got:?}" + ); + assert!(got.contains(&(t0, "alpha-edited".to_string()))); + assert!( + got.contains(&(t0, "bravo".to_string())), + "same timestamp+duration sibling must keep its title: {got:?}" + ); + assert!(got.contains(&(t1, "later window".to_string()))); +} + +#[test] +fn far_behind_dest_still_copies_post_cursor_backlog() { + // Reconcile must not load the post-cursor source backlog (end=resume). + // Those events belong to the bounded incremental copy. + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-far-behind", "phone"); + let t_old = Utc::now() - Duration::days(10); + let t_cursor = t_old + Duration::hours(2); + src.insert_events( + &bucket, + &[ + event_at(t_old, Duration::minutes(20), "sanitized.mp4"), + event_at(t_cursor, Duration::minutes(5), "cursor window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + + edit_title(&src, &bucket, t_old, "Real Video Title"); + let mut later = Vec::new(); + for i in 0..12 { + later.push(event_at( + t_cursor + Duration::days(1) + Duration::minutes(i), + Duration::minutes(1), + &format!("later-{i}"), + )); + } + src.insert_events(&bucket, &later).unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + + let got = titles(&dest, &bucket); + assert_eq!( + got.len(), + 14, + "lookback edit plus paginated backlog: {got:?}" + ); + assert!(got.contains(&(t_old, "Real Video Title".to_string()))); + assert!(got.contains(&(t_cursor, "cursor window".to_string()))); + assert!(got.contains(&(later[0].timestamp, "later-0".to_string()))); + assert!(got.contains(&(later[11].timestamp, "later-11".to_string()))); +} + +#[test] +fn same_timestamp_sibling_is_not_clobbered() { + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-sibling", "phone"); + let t0 = Utc::now() - Duration::hours(2); + let t1 = t0 + Duration::minutes(40); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(20), "sanitized.mp4"), + event_at(t0, Duration::minutes(5), "unrelated sibling"), + event_at(t1, Duration::minutes(3), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + let target = src + .get_events(&bucket, None, None, None) + .unwrap() + .into_iter() + .find(|e| e.data.get("title").and_then(|v| v.as_str()) == Some("sanitized.mp4")) + .unwrap(); + src.delete_events_by_id(&bucket, vec![target.id.unwrap()]) + .unwrap(); + let mut data = target.data.clone(); + data.insert("title".to_string(), json!("Real Video Title")); + src.insert_events( + &bucket, + &[Event { + id: None, + timestamp: target.timestamp, + duration: target.duration, + data, + }], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + + let got = titles(&dest, &bucket); + assert_eq!(got.len(), 3); + assert!(got.contains(&(t0, "Real Video Title".to_string()))); + assert!( + got.contains(&(t0, "unrelated sibling".to_string())), + "a same-timestamp event with a different duration must keep its title: {got:?}" + ); + assert!(got.contains(&(t1, "later window".to_string()))); +} + +#[test] +fn edits_older_than_lookback_are_not_reconciled() { + // Peak-memory bound: only the 7 days before the resume cursor are compared. + let (src, dest) = memory_pair(); + let bucket = create_bucket(&src, "aw-watcher-android-lookback", "phone"); + let t_old = Utc::now() - Duration::days(10); + let t_new = Utc::now() - Duration::minutes(5); + src.insert_events( + &bucket, + &[ + event_at(t_old, Duration::minutes(20), "sanitized-old.mp4"), + event_at(t_new, Duration::minutes(5), "recent window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + edit_title(&src, &bucket, t_old, "Too Old To Reconcile"); + sync_push(&src, &dest); + assert_eq!( + titles(&dest, &bucket), + vec![ + (t_old, "sanitized-old.mp4".to_string()), + (t_new, "recent window".to_string()), + ], + "edits older than 7 days stay on the resume-cursor path" + ); +} + +#[test] +fn wiping_destination_reexports_source_edits() { + // Control: a truly empty destination (internal staging gone) re-reads source + // from the beginning. Deleting only the Android SAF mirror does not do this. + let src = Datastore::new_in_memory(false); + let dest = Datastore::new_in_memory(false); + let bucket = create_bucket(&src, "aw-watcher-android-wipe", "phone"); + let t0 = Utc::now() - Duration::hours(2); + let t1 = t0 + Duration::minutes(30); + src.insert_events( + &bucket, + &[ + event_at(t0, Duration::minutes(20), "sanitized.mp4"), + event_at(t1, Duration::minutes(5), "later window"), + ], + ) + .unwrap(); + src.force_commit().unwrap(); + sync_push(&src, &dest); + edit_title(&src, &bucket, t0, "Real Video Title"); + + let fresh = Datastore::new_in_memory(false); + sync_push(&src, &fresh); + assert_eq!( + titles(&fresh, &bucket), + vec![ + (t0, "Real Video Title".to_string()), + (t1, "later window".to_string()), + ] + ); +}