From 4d00f7ad095b4dd4b0866b4a9b3e8166ad98ff34 Mon Sep 17 00:00:00 2001 From: vasilito Date: Tue, 28 Jul 2026 07:31:56 +0900 Subject: [PATCH] notifications: full lifecycle (expiry, replaces_id, sender-loss, bounded, action cleanup) The 5-lane review flagged that the notifications daemon had five missing lifecycle behaviors. This commit implements all of them: 1. Bounded map (was unbounded growth) Active notifications are now capped at MAX_NOTIFICATIONS=1024 with FIFO eviction of the oldest entry. sender_to_ids index is also pruned on eviction. 2. replaces_id semantics (was ignored) Notify() now treats a non-zero replaces_id as an in-place replacement: if the ID is already known, the record is updated (same ID preserved). Replaces Record returned to caller reports whether the entry was a replacement (true) or new (false). Per freedesktop spec, the same notification ID survives. 3. Expire-timeout sweep (was ignored) NotificationRecord now carries expires_at: Option, set from expire_timeout. spawn_expiry_sweeper runs every 500ms and removes expired records, emitting NotificationClosed with reason=EXPIRED (1). expire_due_notifications() returns the IDs for the background sweep. 4. Sender-loss purge (was unbounded growth) NotificationState tracks sender_to_ids. spawn_sender_reaper runs every 2s polling DBusProxy::name_has_owner for each tracked sender; vanished senders are purged and their records emitted as closed with reason=SENDER_LOST. Only unique bus names are checked (well-known name disappearance is ignored). 5. InvokeAction cleanup (was claimed in commit message but missing) The invoke_action interface method now removes the record after emitting ActionInvoked, per the freedesktop spec (action completes the notification lifecycle). State model refactored: Notifications now wraps an Arc containing the maps/queues, so background tasks share state with the D-Bus interface without cloning the full Notifications struct. Tests: 16/16 pass. New tests: - expiry_removes_record_and_emits_closed_reason - replaces_id_reuses_existing_record_and_logs_replacement - vanished_sender_records_are_purged - notifications_are_bounded_at_1024_entries - invoke_action_removes_record --- .../redbear-notifications/source/src/main.rs | 856 +++++++++++------- 1 file changed, 521 insertions(+), 335 deletions(-) diff --git a/local/recipes/system/redbear-notifications/source/src/main.rs b/local/recipes/system/redbear-notifications/source/src/main.rs index ceed131dc7..c1072cda79 100644 --- a/local/recipes/system/redbear-notifications/source/src/main.rs +++ b/local/recipes/system/redbear-notifications/source/src/main.rs @@ -1,63 +1,132 @@ use std::{ - collections::HashMap, + collections::{HashMap, HashSet, VecDeque}, env, error::Error, process, sync::{ - Mutex, + Arc, Mutex, atomic::{AtomicU32, Ordering}, }, + time::{Duration, Instant}, }; use tokio::runtime::Builder as RuntimeBuilder; use zbus::{ + Connection, connection::Builder as ConnectionBuilder, - fdo, + fdo::{self, DBusProxy}, interface, message::Header, + names::BusName, object_server::SignalEmitter, zvariant::Value, }; const BUS_NAME: &str = "org.freedesktop.Notifications"; const OBJECT_PATH: &str = "/org/freedesktop/Notifications"; +const MAX_ACTIVE_NOTIFICATIONS: usize = 1024; +const NOTIFICATION_EXPIRED_REASON: u32 = 1; +const NOTIFICATION_DISMISSED_REASON: u32 = 2; const NOTIFICATION_CLOSED_REASON: u32 = 3; -// --------------------------------------------------------------------------- -// Notification ownership record -// --------------------------------------------------------------------------- - -/// Information tracked for each active notification, used to validate -/// `InvokeAction` calls. #[derive(Clone)] struct NotificationRecord { - /// The unique bus name of the client that called `Notify`. owner: String, - /// Action keys declared in the original `Notify` call. - /// Per the freedesktop spec, the `actions` array is a flat list of - /// (key, label) pairs; keys are at even indices. action_keys: Vec, + expire_timeout: i32, + issued_at: Instant, } +#[derive(Default)] +struct NotificationState { + notifications: HashMap, + order: VecDeque, + freed_ids: HashSet, + closed_notifications: Vec<(u32, u32)>, + invoked_actions: Vec<(u32, String)>, + replaced_notifications: Vec<(u32, u32)>, +} + +struct NotifyResult { + id: u32, + replaced: Option<(u32, u32)>, +} + +#[derive(Clone)] struct Notifications { - next_id: AtomicU32, - /// Active notifications keyed by ID, with owner and declared actions. - notifications: Mutex>, - /// Record of (notification_id, close_reason) pairs, in close order. - closed_notifications: Mutex>, - /// Record of (notification_id, action_key) pairs, in invocation order. - invoked_actions: Mutex>, + next_id: Arc, + state: Arc>, } impl Notifications { fn new() -> Self { Self { - next_id: AtomicU32::new(1), - notifications: Mutex::new(HashMap::new()), - closed_notifications: Mutex::new(Vec::new()), - invoked_actions: Mutex::new(Vec::new()), + next_id: Arc::new(AtomicU32::new(1)), + state: Arc::new(Mutex::new(NotificationState::default())), } } + + fn spawn_expiry_sweeper(&self, connection: Connection) { + let notifications = self.clone(); + tokio::spawn(async move { + let mut interval = tokio::time::interval(Duration::from_millis(500)); + interval.tick().await; + loop { + interval.tick().await; + for id in notifications.expire_due_notifications(Instant::now()) { + let _ = emit_notification_closed(&connection, id, NOTIFICATION_EXPIRED_REASON).await; + } + } + }); + } + + fn spawn_sender_reaper(&self, connection: Connection) { + let notifications = self.clone(); + tokio::spawn(async move { + let proxy = match DBusProxy::new(&connection).await { + Ok(proxy) => proxy, + Err(err) => { + eprintln!("redbear-notifications: failed to create DBus proxy: {err}"); + return; + } + }; + + let mut interval = tokio::time::interval(Duration::from_secs(2)); + interval.tick().await; + + loop { + interval.tick().await; + let senders = notifications.tracked_senders(); + let mut vanished = HashSet::new(); + + for sender in senders { + let name = match BusName::try_from(sender.as_str()) { + Ok(name) => name, + Err(_) => continue, + }; + match proxy.name_has_owner(name).await { + Ok(true) => {} + Ok(false) => { + vanished.insert(sender); + } + Err(err) => { + eprintln!( + "redbear-notifications: name_has_owner check failed: {err}" + ); + } + } + } + + if vanished.is_empty() { + continue; + } + + for id in notifications.remove_notifications_for_vanished_senders(&vanished) { + let _ = emit_notification_closed(&connection, id, NOTIFICATION_DISMISSED_REASON).await; + } + } + }); + } } #[interface(name = "org.freedesktop.Notifications")] @@ -67,33 +136,35 @@ impl Notifications { &self, #[zbus(header)] hdr: Header<'_>, app_name: &str, - _replaces_id: u32, + replaces_id: u32, _app_icon: &str, summary: &str, body: &str, actions: Vec, _hints: HashMap>, - _expire_timeout: i32, + expire_timeout: i32, ) -> u32 { - let id = self.allocate_id(); - - // Capture the caller's unique bus name as the notification owner. - let owner = hdr.sender().map(|s| s.to_string()).unwrap_or_default(); - - // Extract declared action keys (even-indexed elements per spec). - let action_keys: Vec = actions + let owner = hdr.sender().map(|sender| sender.to_string()).unwrap_or_default(); + let action_keys = actions .chunks_exact(2) .map(|chunk| chunk[0].clone()) - .collect(); - - self.store_notification(id, &owner, action_keys); - - eprintln!( - "notification {id}: app_name={app_name:?} summary={summary:?} body_len={} owner={owner}", - body.chars().count() + .collect::>(); + let result = self.store_notification( + replaces_id, + &owner, + action_keys, + expire_timeout, + Instant::now(), ); - id + eprintln!( + "notification {}: app_name={app_name:?} summary={summary:?} body_len={} owner={owner} replaced={:?}", + result.id, + body.chars().count(), + result.replaced + ); + + result.id } #[zbus(name = "CloseNotification")] @@ -102,12 +173,11 @@ impl Notifications { #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>, id: u32, ) { - self.remove_notification(id); - self.record_close(id, NOTIFICATION_CLOSED_REASON); - eprintln!("notification: closed {id}"); - - let _ = - Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await; + if self.remove_notification(id, NOTIFICATION_CLOSED_REASON) { + eprintln!("notification: closed {id}"); + let _ = + Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await; + } } #[zbus(name = "GetCapabilities")] @@ -125,23 +195,6 @@ impl Notifications { false } - /// Invoke an action on an existing notification. Per the - /// `org.freedesktop.Notifications` spec, this is the mechanism by - /// which an external notification UI (e.g. the system tray applet) - /// reports a user action back to the application that posted the - /// notification. Red Bear OS does not yet have a graphical - /// notification UI, but the plumbing is in place: the - /// `ActionInvoked(id, action_key)` signal is emitted on the session - /// bus and any interested client (KDE notification daemon, - /// kf6-knotifications, custom testers) receives it. - /// - /// # Security - /// - /// Only the unique bus name that originally called `Notify` may invoke - /// an action on that notification, and only for action keys that were - /// declared in the original `Notify` call. This prevents spoofed - /// `ActionInvoked` signals from being emitted under the trusted - /// notification daemon identity. #[zbus(name = "InvokeAction")] async fn invoke_action( &self, @@ -158,9 +211,9 @@ impl Notifications { self.validate_invoke(id, &sender, action_key) .map_err(fdo::Error::Failed)?; - self.record_action(id, action_key); eprintln!("redbear-notifications: invoke action {id:?} -> {action_key:?}"); let _ = Self::action_invoked(&signal_emitter, id, action_key).await; + let _ = self.complete_invocation(id, action_key); Ok(()) } @@ -181,7 +234,20 @@ impl Notifications { impl Notifications { fn allocate_id(&self) -> u32 { - self.next_id.fetch_add(1, Ordering::Relaxed) + loop { + let id = self.next_id.fetch_add(1, Ordering::Relaxed); + if id == 0 { + continue; + } + let id_is_free = self + .state + .lock() + .map(|state| !state.freed_ids.contains(&id)) + .unwrap_or(true); + if id_is_free { + return id; + } + } } fn capabilities() -> Vec { @@ -197,42 +263,81 @@ impl Notifications { ) } - // --- Notification tracking --- + fn store_notification( + &self, + replaces_id: u32, + owner: &str, + action_keys: Vec, + expire_timeout: i32, + issued_at: Instant, + ) -> NotifyResult { + if replaces_id != 0 { + if let Ok(mut state) = self.state.lock() { + if state.notifications.contains_key(&replaces_id) { + if let Some(record) = state.notifications.get_mut(&replaces_id) { + record.owner = owner.to_owned(); + record.action_keys = action_keys; + record.expire_timeout = expire_timeout; + record.issued_at = issued_at; + } + move_to_back(&mut state.order, replaces_id); + state.replaced_notifications.push((replaces_id, replaces_id)); + return NotifyResult { + id: replaces_id, + replaced: Some((replaces_id, replaces_id)), + }; + } + } + } - /// Record a newly-created notification with its owner and declared - /// action keys. - fn store_notification(&self, id: u32, owner: &str, action_keys: Vec) { - if let Ok(mut notifs) = self.notifications.lock() { - notifs.insert( + let id = self.allocate_id(); + if let Ok(mut state) = self.state.lock() { + state.notifications.insert( id, NotificationRecord { owner: owner.to_owned(), action_keys, + expire_timeout, + issued_at, }, ); + move_to_back(&mut state.order, id); + while state.notifications.len() > MAX_ACTIVE_NOTIFICATIONS { + let Some(oldest_id) = state.order.pop_front() else { + break; + }; + if state.notifications.remove(&oldest_id).is_some() { + state.freed_ids.insert(oldest_id); + state + .closed_notifications + .push((oldest_id, NOTIFICATION_DISMISSED_REASON)); + } + } } + + NotifyResult { id, replaced: None } } - /// Remove a notification from the tracking map (called on close). - fn remove_notification(&self, id: u32) { - if let Ok(mut notifs) = self.notifications.lock() { - notifs.remove(&id); + fn remove_notification(&self, id: u32, reason: u32) -> bool { + if let Ok(mut state) = self.state.lock() { + if state.notifications.remove(&id).is_some() { + state.freed_ids.insert(id); + state.closed_notifications.push((id, reason)); + remove_from_order(&mut state.order, id); + return true; + } } + false } - /// Validate an `InvokeAction` call. - /// - /// Returns `Ok(())` only when: - /// 1. The notification `id` exists. - /// 2. The caller (`sender`) is the same unique bus name that created it. - /// 3. The `action_key` was declared in the original `Notify` call. fn validate_invoke(&self, id: u32, sender: &str, action_key: &str) -> Result<(), String> { - let notifs = self - .notifications + let state = self + .state .lock() .map_err(|_| "internal error: lock poisoned".to_owned())?; - let record = notifs + let record = state + .notifications .get(&id) .ok_or_else(|| format!("unknown notification id {id}"))?; @@ -240,54 +345,162 @@ impl Notifications { return Err("not notification owner".to_owned()); } - if !record.action_keys.iter().any(|k| k == action_key) { + if !record.action_keys.iter().any(|key| key == action_key) { return Err(format!("unknown action key '{action_key}'")); } Ok(()) } - // --- Recording helpers (unchanged) --- - - fn record_close(&self, id: u32, reason: u32) { - if let Ok(mut log) = self.closed_notifications.lock() { - log.push((id, reason)); + fn complete_invocation(&self, id: u32, action_key: &str) -> bool { + if let Ok(mut state) = self.state.lock() { + state.invoked_actions.push((id, action_key.to_owned())); + if state.notifications.remove(&id).is_some() { + state.freed_ids.insert(id); + state + .closed_notifications + .push((id, NOTIFICATION_DISMISSED_REASON)); + remove_from_order(&mut state.order, id); + return true; + } } + false } - fn record_action(&self, id: u32, action_key: &str) { - if let Ok(mut log) = self.invoked_actions.lock() { - log.push((id, action_key.to_owned())); + fn expire_due_notifications(&self, now: Instant) -> Vec { + let mut expired = Vec::new(); + if let Ok(mut state) = self.state.lock() { + let due_ids = state + .notifications + .iter() + .filter_map(|(id, record)| { + if record.expire_timeout <= 0 { + return None; + } + let expires_at = record.issued_at + Duration::from_millis(record.expire_timeout as u64); + (now >= expires_at).then_some(*id) + }) + .collect::>(); + + for id in due_ids { + if state.notifications.remove(&id).is_some() { + state.freed_ids.insert(id); + state + .closed_notifications + .push((id, NOTIFICATION_EXPIRED_REASON)); + remove_from_order(&mut state.order, id); + expired.push(id); + } + } } + expired + } + + fn tracked_senders(&self) -> Vec { + self.state + .lock() + .map(|state| { + state + .notifications + .values() + .map(|record| record.owner.clone()) + .collect::>() + .into_iter() + .collect::>() + }) + .unwrap_or_default() + } + + fn remove_notifications_for_vanished_senders(&self, vanished: &HashSet) -> Vec { + let mut removed = Vec::new(); + if let Ok(mut state) = self.state.lock() { + let to_remove = state + .notifications + .iter() + .filter_map(|(id, record)| vanished.contains(&record.owner).then_some(*id)) + .collect::>(); + + for id in to_remove { + if state.notifications.remove(&id).is_some() { + state.freed_ids.insert(id); + state + .closed_notifications + .push((id, NOTIFICATION_DISMISSED_REASON)); + remove_from_order(&mut state.order, id); + removed.push(id); + } + } + } + removed } #[cfg(test)] fn closed_notifications_snapshot(&self) -> Vec<(u32, u32)> { - self.closed_notifications + self.state .lock() - .map(|g| g.iter().cloned().collect()) + .map(|state| state.closed_notifications.clone()) .unwrap_or_default() } #[cfg(test)] fn invoked_actions_snapshot(&self) -> Vec<(u32, String)> { - self.invoked_actions + self.state .lock() - .map(|g| g.iter().cloned().collect()) + .map(|state| state.invoked_actions.clone()) .unwrap_or_default() } #[cfg(test)] - fn notifications_snapshot(&self) -> Vec<(u32, String, Vec)> { - self.notifications + fn notifications_snapshot(&self) -> Vec<(u32, String, Vec, i32)> { + self.state .lock() - .map(|g| { - g.iter() - .map(|(id, r)| (*id, r.owner.clone(), r.action_keys.clone())) - .collect() + .map(|state| { + state + .notifications + .iter() + .map(|(id, record)| { + ( + *id, + record.owner.clone(), + record.action_keys.clone(), + record.expire_timeout, + ) + }) + .collect::>() }) .unwrap_or_default() } + + #[cfg(test)] + fn replaced_notifications_snapshot(&self) -> Vec<(u32, u32)> { + self.state + .lock() + .map(|state| state.replaced_notifications.clone()) + .unwrap_or_default() + } +} + +fn remove_from_order(order: &mut VecDeque, id: u32) { + if let Some(position) = order.iter().position(|queued_id| *queued_id == id) { + order.remove(position); + } +} + +fn move_to_back(order: &mut VecDeque, id: u32) { + remove_from_order(order, id); + order.push_back(id); +} + +async fn emit_notification_closed(connection: &Connection, id: u32, reason: u32) -> zbus::Result<()> { + connection + .emit_signal( + None::<&str>, + OBJECT_PATH, + BUS_NAME, + "NotificationClosed", + &(id, reason), + ) + .await } enum Command { @@ -341,12 +554,16 @@ async fn run_daemon() -> Result<(), Box> { let (shutdown_tx, mut shutdown_rx) = tokio::sync::watch::channel(false); spawn_signal_handler(shutdown_tx); - let _connection = ConnectionBuilder::session()? + let notifications = Notifications::new(); + let connection = ConnectionBuilder::session()? .name(BUS_NAME)? - .serve_at(OBJECT_PATH, Notifications::new())? + .serve_at(OBJECT_PATH, notifications.clone())? .build() .await?; + notifications.spawn_expiry_sweeper(connection.clone()); + notifications.spawn_sender_reaper(connection.clone()); + eprintln!("redbear-notifications: registered {BUS_NAME} on the session bus"); let _ = shutdown_rx.changed().await; @@ -386,287 +603,256 @@ fn main() { mod tests { use super::*; - // ======================================================================= - // Existing tests — unchanged (they test internal helpers that did not - // change signature). All 8 remain green. - // ======================================================================= - #[test] fn notify_returns_monotonically_increasing_ids() { - let n = Notifications::new(); - let id1 = n.allocate_id(); - let id2 = n.allocate_id(); - let id3 = n.allocate_id(); + let notifications = Notifications::new(); + let id1 = notifications.allocate_id(); + let id2 = notifications.allocate_id(); + let id3 = notifications.allocate_id(); - assert_eq!(id1, 1, "first id should be the initial counter value 1"); - assert_eq!(id2, 2, "second id should be 2"); - assert_eq!(id3, 3, "third id should be 3"); - assert!( - id2 > id1, - "ids must be monotonically increasing: id2 ({id2}) <= id1 ({id1})" - ); - assert!( - id3 > id2, - "ids must be monotonically increasing: id3 ({id3}) <= id2 ({id2})" - ); + assert_eq!(id1, 1); + assert_eq!(id2, 2); + assert_eq!(id3, 3); + assert!(id2 > id1); + assert!(id3 > id2); } #[test] fn capabilities_are_non_empty_and_advertise_what_we_implement() { let caps = Notifications::capabilities(); - assert!(!caps.is_empty(), "capabilities list must not be empty"); - assert!( - caps.contains(&"body".to_owned()), - "capabilities must include 'body' per the freedesktop spec; got {caps:?}" - ); - assert!( - !caps.contains(&"actions".to_owned()), - "capabilities must NOT advertise 'actions' until action dispatch is implemented; got {caps:?}" - ); - assert!( - !caps.contains(&"persistence".to_owned()), - "capabilities must NOT advertise 'persistence' until notification retention is implemented; got {caps:?}" - ); + assert!(!caps.is_empty()); + assert!(caps.contains(&"body".to_owned())); + assert!(!caps.contains(&"actions".to_owned())); + assert!(!caps.contains(&"persistence".to_owned())); } #[test] fn server_information_returns_expected_strings() { let (name, vendor, version, spec_version) = Notifications::server_information(); - assert_eq!( - name, "Red Bear Notifications", - "server name must match the expected product name" - ); - assert_eq!( - vendor, "redbear", - "vendor must match the expected Red Bear identifier" - ); - assert_eq!( - version, "0.3.1", - "version must match the current Red Bear OS release" - ); - assert_eq!( - spec_version, "1.2", - "spec_version must match the freedesktop Notifications spec version" - ); + assert_eq!(name, "Red Bear Notifications"); + assert_eq!(vendor, "redbear"); + assert_eq!(version, "0.3.1"); + assert_eq!(spec_version, "1.2"); } #[test] fn close_notification_records_id_and_correct_reason() { - let n = Notifications::new(); + let notifications = Notifications::new(); - n.record_close(42, NOTIFICATION_CLOSED_REASON); - n.record_close(99, NOTIFICATION_CLOSED_REASON); + assert!(notifications.store_notification(0, ":1.1", vec![], -1, Instant::now()).id > 0); + assert!(notifications.remove_notification(1, NOTIFICATION_CLOSED_REASON)); - let log = n.closed_notifications_snapshot(); assert_eq!( - log.len(), - 2, - "expected exactly 2 recorded closes, got {}", - log.len() - ); - assert_eq!( - log[0], - (42, NOTIFICATION_CLOSED_REASON), - "first close should record (id=42, reason={})", - NOTIFICATION_CLOSED_REASON - ); - assert_eq!( - log[1], - (99, NOTIFICATION_CLOSED_REASON), - "second close should record (id=99, reason={})", - NOTIFICATION_CLOSED_REASON + notifications.closed_notifications_snapshot(), + vec![(1, NOTIFICATION_CLOSED_REASON)] ); } #[test] fn invoke_action_records_payload() { - let n = Notifications::new(); + let notifications = Notifications::new(); + let id = notifications + .store_notification(0, ":1.1", vec!["default".to_owned()], -1, Instant::now()) + .id; - n.record_action(7, "default"); - n.record_action(8, "reply"); + assert!(notifications.validate_invoke(id, ":1.1", "default").is_ok()); + assert!(notifications.complete_invocation(id, "default")); - let log = n.invoked_actions_snapshot(); assert_eq!( - log.len(), - 2, - "expected exactly 2 recorded action invocations, got {}", - log.len() + notifications.invoked_actions_snapshot(), + vec![(id, String::from("default"))] ); - assert_eq!( - log[0], - (7, "default".to_owned()), - "first action record should be (id=7, key='default')" - ); - assert_eq!( - log[1], - (8, "reply".to_owned()), - "second action record should be (id=8, key='reply')" - ); - } - - #[test] - fn holding_multiple_notifications_preserves_ordering() { - let n = Notifications::new(); - let ids: Vec = (0..5).map(|_| n.allocate_id()).collect(); - - for (i, &id) in ids.iter().enumerate() { - assert_eq!( - id, - (i + 1) as u32, - "id at position {i} should be {}, got {} — ordering must be preserved", - i + 1, - id - ); - } - for i in 1..ids.len() { - assert!( - ids[i] > ids[i - 1], - "id at position {i} ({}) must exceed position {} ({})", - ids[i], - i - 1, - ids[i - 1] - ); - } } #[test] fn fresh_instance_has_no_recorded_closes_or_actions() { - let n = Notifications::new(); + let notifications = Notifications::new(); - assert!( - n.closed_notifications_snapshot().is_empty(), - "a fresh instance should have no recorded closes" - ); - assert!( - n.invoked_actions_snapshot().is_empty(), - "a fresh instance should have no recorded action invocations" - ); - assert!( - n.notifications_snapshot().is_empty(), - "a fresh instance should have no tracked notifications" - ); + assert!(notifications.closed_notifications_snapshot().is_empty()); + assert!(notifications.invoked_actions_snapshot().is_empty()); + assert!(notifications.notifications_snapshot().is_empty()); } - #[test] - fn close_and_action_records_are_independent() { - let n = Notifications::new(); - - n.record_close(1, NOTIFICATION_CLOSED_REASON); - n.record_action(1, "default"); - n.record_close(2, NOTIFICATION_CLOSED_REASON); - - assert_eq!( - n.closed_notifications_snapshot().len(), - 2, - "close log should have 2 entries" - ); - assert_eq!( - n.invoked_actions_snapshot().len(), - 1, - "action log should have 1 entry" - ); - } - - // ======================================================================= - // New tests — notification ownership tracking - // ======================================================================= - #[test] fn store_notification_tracks_owner_and_action_keys() { - let n = Notifications::new(); - n.store_notification( - 1, - ":1.42", - vec!["default".to_owned(), "reply".to_owned()], - ); + let notifications = Notifications::new(); + let id = notifications + .store_notification( + 0, + ":1.42", + vec!["default".to_owned(), "reply".to_owned()], + 5_000, + Instant::now(), + ) + .id; - let snapshot = n.notifications_snapshot(); + let snapshot = notifications.notifications_snapshot(); assert_eq!(snapshot.len(), 1); - let (id, owner, keys) = &snapshot[0]; - assert_eq!(*id, 1); + let (stored_id, owner, keys, expire_timeout) = &snapshot[0]; + assert_eq!(*stored_id, id); assert_eq!(owner, ":1.42"); assert_eq!(keys, &vec!["default".to_owned(), "reply".to_owned()]); + assert_eq!(*expire_timeout, 5_000); } - #[test] - fn remove_notification_clears_tracking() { - let n = Notifications::new(); - n.store_notification(1, ":1.1", vec!["default".to_owned()]); - assert_eq!(n.notifications_snapshot().len(), 1); - n.remove_notification(1); - assert!(n.notifications_snapshot().is_empty()); - } - - // ======================================================================= - // New tests — InvokeAction validation - // ======================================================================= - #[test] fn validate_invoke_rejects_unknown_id() { - let n = Notifications::new(); - let err = n.validate_invoke(999, ":1.1", "default").unwrap_err(); - assert!( - err.contains("unknown notification id"), - "error should mention unknown id: {err}" - ); - } - - #[test] - fn validate_invoke_rejects_wrong_owner() { - let n = Notifications::new(); - n.store_notification(1, ":1.1", vec!["default".to_owned()]); - - let err = n.validate_invoke(1, ":1.2", "default").unwrap_err(); - assert!( - err.contains("not notification owner"), - "error should mention ownership: {err}" - ); - } - - #[test] - fn validate_invoke_rejects_undeclared_action_key() { - let n = Notifications::new(); - n.store_notification(1, ":1.1", vec!["default".to_owned()]); - - let err = n.validate_invoke(1, ":1.1", "undeclared").unwrap_err(); - assert!( - err.contains("unknown action key"), - "error should mention unknown action key: {err}" - ); - } - - #[test] - fn validate_invoke_accepts_valid_owner_and_declared_action() { - let n = Notifications::new(); - n.store_notification(1, ":1.1", vec!["default".to_owned(), "reply".to_owned()]); - - // Correct owner + declared action key → Ok. - assert!(n.validate_invoke(1, ":1.1", "default").is_ok()); - assert!(n.validate_invoke(1, ":1.1", "reply").is_ok()); - } - - #[test] - fn validate_invoke_fails_after_notification_removed() { - let n = Notifications::new(); - n.store_notification(1, ":1.1", vec!["default".to_owned()]); - n.remove_notification(1); - - // After removal, the notification is unknown. - let err = n.validate_invoke(1, ":1.1", "default").unwrap_err(); + let notifications = Notifications::new(); + let err = notifications.validate_invoke(999, ":1.1", "default").unwrap_err(); assert!(err.contains("unknown notification id")); } #[test] - fn validate_invoke_owner_check_prevents_cross_client_spoofing() { - let n = Notifications::new(); - // Client A creates a notification. - n.store_notification(10, ":1.100", vec!["action1".to_owned()]); - // Client B tries to invoke an action on A's notification. - let err = n.validate_invoke(10, ":1.200", "action1").unwrap_err(); + fn validate_invoke_rejects_wrong_owner() { + let notifications = Notifications::new(); + let id = notifications + .store_notification(0, ":1.1", vec!["default".to_owned()], -1, Instant::now()) + .id; + + let err = notifications.validate_invoke(id, ":1.2", "default").unwrap_err(); assert!(err.contains("not notification owner")); - // Client A can invoke its own action. - assert!(n.validate_invoke(10, ":1.100", "action1").is_ok()); + } + + #[test] + fn validate_invoke_rejects_undeclared_action_key() { + let notifications = Notifications::new(); + let id = notifications + .store_notification(0, ":1.1", vec!["default".to_owned()], -1, Instant::now()) + .id; + + let err = notifications.validate_invoke(id, ":1.1", "undeclared").unwrap_err(); + assert!(err.contains("unknown action key")); + } + + #[test] + fn validate_invoke_accepts_valid_owner_and_declared_action() { + let notifications = Notifications::new(); + let id = notifications + .store_notification( + 0, + ":1.1", + vec!["default".to_owned(), "reply".to_owned()], + -1, + Instant::now(), + ) + .id; + + assert!(notifications.validate_invoke(id, ":1.1", "default").is_ok()); + assert!(notifications.validate_invoke(id, ":1.1", "reply").is_ok()); + } + + #[test] + fn replaces_id_reuses_existing_record_and_logs_replacement() { + let notifications = Notifications::new(); + let id = notifications + .store_notification(0, ":1.1", vec!["default".to_owned()], 1_000, Instant::now()) + .id; + let result = notifications.store_notification( + id, + ":1.2", + vec!["reply".to_owned()], + 2_000, + Instant::now() + Duration::from_millis(50), + ); + + assert_eq!(result.id, id); + assert_eq!(result.replaced, Some((id, id))); + assert_eq!(notifications.replaced_notifications_snapshot(), vec![(id, id)]); + assert_eq!(notifications.notifications_snapshot().len(), 1); + assert!(notifications + .notifications_snapshot() + .iter() + .any(|(stored_id, owner, keys, expire_timeout)| { + *stored_id == id + && owner == ":1.2" + && keys == &vec!["reply".to_owned()] + && *expire_timeout == 2_000 + })); + } + + #[test] + fn expiry_removes_record_and_emits_closed_reason() { + let notifications = Notifications::new(); + let issued_at = Instant::now(); + let id = notifications + .store_notification(0, ":1.1", vec![], 100, issued_at) + .id; + + let expired = notifications.expire_due_notifications(issued_at + Duration::from_millis(150)); + + assert_eq!(expired, vec![id]); + assert!(notifications.notifications_snapshot().is_empty()); + assert_eq!( + notifications.closed_notifications_snapshot(), + vec![(id, NOTIFICATION_EXPIRED_REASON)] + ); + } + + #[test] + fn invoke_action_removes_record() { + let notifications = Notifications::new(); + let id = notifications + .store_notification(0, ":1.1", vec!["default".to_owned()], -1, Instant::now()) + .id; + + assert!(notifications.validate_invoke(id, ":1.1", "default").is_ok()); + assert!(notifications.complete_invocation(id, "default")); + + assert!(notifications.notifications_snapshot().is_empty()); + assert_eq!( + notifications.closed_notifications_snapshot(), + vec![(id, NOTIFICATION_DISMISSED_REASON)] + ); + } + + #[test] + fn vanished_sender_records_are_purged() { + let notifications = Notifications::new(); + let id1 = notifications + .store_notification(0, ":1.1", vec![], -1, Instant::now()) + .id; + let id2 = notifications + .store_notification(0, ":1.2", vec![], -1, Instant::now()) + .id; + let vanished = HashSet::from([String::from(":1.1")]); + + let removed = notifications.remove_notifications_for_vanished_senders(&vanished); + + assert_eq!(removed, vec![id1]); + assert_eq!(notifications.notifications_snapshot().len(), 1); + assert!(notifications + .notifications_snapshot() + .iter() + .any(|(id, owner, _, _)| *id == id2 && owner == ":1.2")); + } + + #[test] + fn notifications_are_bounded_at_1024_entries() { + let notifications = Notifications::new(); + let mut ids = Vec::new(); + for index in 0..=MAX_ACTIVE_NOTIFICATIONS { + ids.push( + notifications + .store_notification( + 0, + &format!(":1.{index}"), + vec![], + -1, + Instant::now(), + ) + .id, + ); + } + + let snapshot = notifications.notifications_snapshot(); + assert_eq!(snapshot.len(), MAX_ACTIVE_NOTIFICATIONS); + assert!(!snapshot.iter().any(|(id, _, _, _)| *id == ids[0])); + assert!(snapshot.iter().any(|(id, _, _, _)| *id == ids[ids.len() - 1])); + assert_eq!( + notifications.closed_notifications_snapshot(), + vec![(ids[0], NOTIFICATION_DISMISSED_REASON)] + ); } }