diff --git a/crates/lanspread-peer/src/call_to_play.rs b/crates/lanspread-peer/src/call_to_play.rs index 7de9da1..461880f 100644 --- a/crates/lanspread-peer/src/call_to_play.rs +++ b/crates/lanspread-peer/src/call_to_play.rs @@ -1,7 +1,8 @@ //! Replicated event history for Call to Play coordination. use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeSet, HashMap, HashSet}, + fmt, time::{SystemTime, UNIX_EPOCH}, }; @@ -26,7 +27,39 @@ const EXPIRED_RETENTION_MS: i64 = 5 * 60_000; #[derive(Debug, Default)] pub(crate) struct CallToPlayStore { events: Vec, - event_ids: HashSet, +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct BatchMerge { + pub(crate) applied: Vec, + pub(crate) duplicates: usize, + pub(crate) obsolete: usize, + pub(crate) missing_call_ids: Vec, +} + +impl BatchMerge { + pub(crate) fn needs_history(&self) -> bool { + !self.missing_call_ids.is_empty() + } +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum MergeError { + Invalid(&'static str), + ConflictingEvent(String), + HistoryFull, +} + +impl fmt::Display for MergeError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Invalid(reason) => formatter.write_str(reason), + Self::ConflictingEvent(event_id) => { + write!(formatter, "conflicting Call to Play event ID {event_id}") + } + Self::HistoryFull => formatter.write_str("Call to Play event history is full"), + } + } } impl CallToPlayStore { @@ -35,121 +68,309 @@ impl CallToPlayStore { } fn snapshot_at(&mut self, now: i64) -> Vec { - self.compact_inactive_calls(now); + compact_history(&mut self.events, now); self.events.clone() } - pub(crate) fn insert(&mut self, event: CallToPlayEvent) -> Result { - validate_event(&event)?; - if self.event_ids.contains(&event.id) { - return Ok(false); - } - - let event_id = event.id.clone(); - self.event_ids.insert(event_id.clone()); - self.events.push(event); - self.compact_inactive_calls(now_ms()); - if self.events.len() > MAX_EVENTS { - self.events.retain(|event| event.id != event_id); - self.event_ids.remove(&event_id); - return Err("Call to Play event history is full"); - } - Ok(true) + pub(crate) fn merge_batch( + &mut self, + incoming: Vec, + ) -> Result { + self.merge_batch_at(incoming, now_ms()) } - pub(crate) fn insert_all(&mut self, events: Vec) -> Vec { - let mut accepted = Vec::new(); - for event in events { - match self.insert(event.clone()) { - Ok(true) => accepted.push(event), - Ok(false) => {} - Err(err) => log::warn!("Ignoring invalid Call to Play event {}: {err}", event.id), + fn merge_batch_at( + &mut self, + incoming: Vec, + now: i64, + ) -> Result { + for event in &incoming { + validate_event(event).map_err(MergeError::Invalid)?; + } + + let (incoming, mut duplicates) = deduplicate_batch(incoming)?; + + // Expiry is evaluated before accepting new actions. An action cannot keep + // an already-retired root alive by itself; the sender must provide the + // complete history in the same batch. + let mut retained = self.events.clone(); + compact_history(&mut retained, now); + + let retained_by_id = retained + .iter() + .map(|event| (event.id.as_str(), event)) + .collect::>(); + let mut new_events = Vec::with_capacity(incoming.len()); + for event in incoming { + if let Some(existing) = retained_by_id.get(event.id.as_str()) { + if *existing != &event { + return Err(MergeError::ConflictingEvent(event.id)); + } + duplicates += 1; + } else { + new_events.push(event); } } - accepted + drop(retained_by_id); + + let rooted_calls = retained + .iter() + .chain(&new_events) + .filter(|event| matches!(event.action, CallToPlayAction::Create { .. })) + .map(|event| event.call_id.clone()) + .collect::>(); + let retained_tombstones = terminal_tombstone_call_ids(&retained); + let mut applicable = Vec::with_capacity(new_events.len()); + let mut missing_call_ids = BTreeSet::new(); + let mut obsolete = 0; + + for event in new_events { + if retained_tombstones.contains(event.call_id.as_str()) { + obsolete += 1; + continue; + } + if !matches!(event.action, CallToPlayAction::Create { .. }) + && !rooted_calls.contains(event.call_id.as_str()) + { + missing_call_ids.insert(event.call_id); + continue; + } + applicable.push(event); + } + + let applicable_ids = applicable + .iter() + .map(|event| event.id.clone()) + .collect::>(); + retained.extend(applicable); + compact_history(&mut retained, now); + + if unresolved_event_count(&retained) > MAX_EVENTS { + return Err(MergeError::HistoryFull); + } + + let retained_ids = retained + .iter() + .map(|event| event.id.clone()) + .collect::>(); + let applied = retained + .iter() + .filter(|event| applicable_ids.contains(&event.id)) + .cloned() + .collect::>(); + obsolete += applicable_ids + .iter() + .filter(|event_id| !retained_ids.contains(*event_id)) + .count(); + + self.events = retained; + Ok(BatchMerge { + applied, + duplicates, + obsolete, + missing_call_ids: missing_call_ids.into_iter().collect(), + }) + } +} + +fn deduplicate_batch( + incoming: Vec, +) -> Result<(Vec, usize), MergeError> { + let mut unique = Vec::::with_capacity(incoming.len()); + let mut indexes = HashMap::::with_capacity(incoming.len()); + let mut duplicates = 0; + + for event in incoming { + if let Some(index) = indexes.get(&event.id).copied() { + if unique[index] != event { + return Err(MergeError::ConflictingEvent(event.id)); + } + duplicates += 1; + } else { + indexes.insert(event.id.clone(), unique.len()); + unique.push(event); + } } - fn compact_inactive_calls(&mut self, now: i64) { - let mut creators = HashMap::::new(); - for event in &self.events { + Ok((unique, duplicates)) +} + +#[derive(Debug)] +struct HistoryIndex { + creators: HashMap, + terminal_events: HashMap, + extensions: HashMap, +} + +#[derive(Clone, Debug)] +struct CreateRecord { + at: i64, + event_id: String, + actor_id: String, + deadline: i64, +} + +impl CreateRecord { + fn order_key(&self) -> (i64, &str) { + (self.at, &self.event_id) + } +} + +#[derive(Clone, Debug)] +struct EventRecord { + at: i64, + event_id: String, +} + +impl EventRecord { + fn order_key(&self) -> (i64, &str) { + (self.at, &self.event_id) + } +} + +#[derive(Clone, Debug)] +struct ExtensionRecord { + event: EventRecord, + deadline: i64, +} + +impl HistoryIndex { + fn build(events: &[CallToPlayEvent]) -> Self { + let mut creators = HashMap::::new(); + for event in events { let CallToPlayAction::Create { deadline, .. } = event.action else { continue; }; - let candidate = (event.at, event.id.clone(), event.actor_id.clone(), deadline); + let candidate = CreateRecord { + at: event.at, + event_id: event.id.clone(), + actor_id: event.actor_id.clone(), + deadline, + }; creators .entry(event.call_id.clone()) .and_modify(|current| { - if (candidate.0, &candidate.1) < (current.0, ¤t.1) { + if candidate.order_key() < current.order_key() { current.clone_from(&candidate); } }) .or_insert(candidate); } - let mut terminal_events = HashMap::::new(); - for event in &self.events { - let Some((created_at, create_id, creator_id, _)) = creators.get(&event.call_id) else { + let mut terminal_events = HashMap::::new(); + let mut extensions = HashMap::::new(); + for event in events { + let Some(creator) = creators.get(&event.call_id) else { continue; }; - if !matches!( - event.action, - CallToPlayAction::Cancel | CallToPlayAction::Start - ) || event.actor_id != *creator_id - || (event.at, &event.id) <= (*created_at, create_id) + if event.actor_id != creator.actor_id + || (event.at, event.id.as_str()) <= creator.order_key() { continue; } - let candidate = (event.at, event.id.clone()); - terminal_events - .entry(event.call_id.clone()) - .and_modify(|current| { - if (candidate.0, &candidate.1) < (current.0, ¤t.1) { - current.clone_from(&candidate); - } - }) - .or_insert(candidate); + + if matches!( + event.action, + CallToPlayAction::Cancel | CallToPlayAction::Start + ) { + let candidate = EventRecord { + at: event.at, + event_id: event.id.clone(), + }; + terminal_events + .entry(event.call_id.clone()) + .and_modify(|current| { + if candidate.order_key() < current.order_key() { + current.clone_from(&candidate); + } + }) + .or_insert(candidate); + } else if let CallToPlayAction::AddTime { deadline } = event.action { + let candidate = ExtensionRecord { + event: EventRecord { + at: event.at, + event_id: event.id.clone(), + }, + deadline, + }; + extensions + .entry(event.call_id.clone()) + .and_modify(|current| { + if candidate.event.order_key() > current.event.order_key() { + current.clone_from(&candidate); + } + }) + .or_insert(candidate); + } } - let mut extensions = HashMap::::new(); - for event in &self.events { - let CallToPlayAction::AddTime { deadline } = event.action else { - continue; - }; - let Some((created_at, create_id, creator_id, _)) = creators.get(&event.call_id) else { - continue; - }; - if event.actor_id != *creator_id || (event.at, &event.id) <= (*created_at, create_id) { - continue; - } - let candidate = (event.at, event.id.clone(), deadline); - extensions - .entry(event.call_id.clone()) - .and_modify(|current| { - if (candidate.0, &candidate.1) > (current.0, ¤t.1) { - current.clone_from(&candidate); - } - }) - .or_insert(candidate); + Self { + creators, + terminal_events, + extensions, } - let expired_calls = creators - .iter() - .filter_map(|(call_id, (_, _, _, original_deadline))| { - let deadline = extensions - .get(call_id) - .map_or(*original_deadline, |(_, _, deadline)| *deadline); - (now - deadline > EXPIRED_RETENTION_MS).then(|| call_id.clone()) - }) - .collect::>(); - - self.events.retain(|event| { - if expired_calls.contains(&event.call_id) { - return false; - } - terminal_events - .get(&event.call_id) - .is_none_or(|(_, terminal_id)| event.id == *terminal_id) - }); } + + fn expired_call_ids(&self, now: i64) -> HashSet<&str> { + self.creators + .iter() + .filter_map(|(call_id, creator)| { + if self.terminal_events.contains_key(call_id) { + return None; + } + let deadline = self + .extensions + .get(call_id) + .map_or(creator.deadline, |extension| extension.deadline); + (now - deadline > EXPIRED_RETENTION_MS).then_some(call_id.as_str()) + }) + .collect() + } +} + +fn compact_history(events: &mut Vec, now: i64) { + let index = HistoryIndex::build(events); + let expired_calls = index.expired_call_ids(now); + + events.retain(|event| { + if expired_calls.contains(event.call_id.as_str()) { + return false; + } + index + .terminal_events + .get(&event.call_id) + .is_none_or(|terminal| event.id == terminal.event_id) + }); +} + +fn terminal_tombstone_call_ids(events: &[CallToPlayEvent]) -> HashSet { + let rooted_calls = events + .iter() + .filter(|event| matches!(event.action, CallToPlayAction::Create { .. })) + .map(|event| event.call_id.clone()) + .collect::>(); + + events + .iter() + .filter(|event| { + matches!( + event.action, + CallToPlayAction::Cancel | CallToPlayAction::Start + ) && !rooted_calls.contains(event.call_id.as_str()) + }) + .map(|event| event.call_id.clone()) + .collect() +} + +fn unresolved_event_count(events: &[CallToPlayEvent]) -> usize { + let index = HistoryIndex::build(events); + events + .iter() + .filter(|event| { + index.creators.contains_key(&event.call_id) + && !index.terminal_events.contains_key(&event.call_id) + }) + .count() } fn now_ms() -> i64 { @@ -166,21 +387,25 @@ pub(crate) async fn publish( ctx: &Ctx, tx_notify_ui: &UnboundedSender, mut event: CallToPlayEvent, -) -> Result<(), &'static str> { +) -> Result<(), String> { event.actor_id.clone_from(ctx.peer_id.as_ref()); - match ctx.call_to_play.write().await.insert(event.clone()) { - Ok(false) => return Ok(()), - Err(err) => { - log::warn!("Rejecting local Call to Play event {}: {err}", event.id); - return Err(err); + let merged = ctx + .call_to_play + .write() + .await + .merge_batch(vec![event.clone()]) + .map_err(|err| err.to_string())?; + if merged.needs_history() { + return Err("Call to Play history is missing".to_string()); + } + if merged.applied.is_empty() { + if merged.obsolete > 0 { + return Err("Call to Play event is obsolete".to_string()); } - Ok(true) => {} + return Ok(()); } - events::send( - tx_notify_ui, - PeerEvent::CallToPlayEvents(vec![event.clone()]), - ); + events::send(tx_notify_ui, PeerEvent::CallToPlayEvents(merged.applied)); let peer_addresses = ctx.peer_game_db.read().await.get_peer_addresses(); let peer_id = ctx.peer_id.clone(); @@ -275,14 +500,18 @@ fn validate_nonempty( mod tests { use lanspread_proto::{CallToPlayAction, CallToPlayEvent}; - use super::{CallToPlayStore, MAX_EVENTS}; + use super::{CallToPlayStore, MAX_EVENTS, MergeError}; const TEST_NOW: i64 = 8_000_000_000_000; fn create_event(id: &str) -> CallToPlayEvent { + create_event_for("call-1", id) + } + + fn create_event_for(call_id: &str, id: &str) -> CallToPlayEvent { CallToPlayEvent { id: id.to_string(), - call_id: "call-1".to_string(), + call_id: call_id.to_string(), actor_id: "peer-alice".to_string(), actor_name: "Alice".to_string(), at: TEST_NOW, @@ -307,138 +536,253 @@ mod tests { } #[test] - fn deduplicates_events_without_reordering_history() { + fn deduplicates_events_without_reordering_new_history() { let mut store = CallToPlayStore::default(); - assert!( - store - .insert(create_event("event-1")) - .expect("valid event should be inserted") - ); - assert!( - !store - .insert(create_event("event-1")) - .expect("duplicate valid event should be accepted") - ); - assert!( - store - .insert(create_event("event-2")) - .expect("valid event should be inserted") - ); + let first = create_event("event-1"); + let second = create_event("event-2"); + let merged = store + .merge_batch_at(vec![first.clone(), first.clone(), second.clone()], TEST_NOW) + .expect("valid batch should merge"); - let ids = store - .snapshot() - .into_iter() - .map(|event| event.id) - .collect::>(); + assert_eq!(merged.applied, [first, second]); + assert_eq!(merged.duplicates, 1); + assert_eq!(merged.obsolete, 0); + assert!(!merged.needs_history()); + + let duplicate = store + .merge_batch_at(vec![create_event("event-1")], TEST_NOW) + .expect("stored duplicate should be harmless"); + assert!(duplicate.applied.is_empty()); + assert_eq!(duplicate.duplicates, 1); + + let ids = event_ids(store.snapshot_at(TEST_NOW)); assert_eq!(ids, ["event-1", "event-2"]); } #[test] - fn rejects_malformed_events() { + fn invalid_batch_leaves_store_unchanged() { let mut store = CallToPlayStore::default(); - let mut event = create_event("event-1"); - event.action = CallToPlayAction::SendMessage { + store + .merge_batch_at(vec![create_event("create")], TEST_NOW) + .expect("create should fit"); + let before = store.snapshot_at(TEST_NOW); + + let mut invalid = action_event( + "invalid", + "call-1", + CallToPlayAction::SendMessage { + message_id: "message-1".to_string(), + text: "valid before corruption".to_string(), + }, + ); + invalid.action = CallToPlayAction::SendMessage { message_id: "message-1".to_string(), text: " ".to_string(), }; - assert_eq!(store.insert(event), Err("invalid message")); - assert!(store.snapshot().is_empty()); - } - - #[test] - fn merge_returns_only_new_valid_events() { - let mut store = CallToPlayStore::default(); - store - .insert(create_event("event-1")) - .expect("valid event should be inserted"); - let mut invalid = create_event("invalid"); - invalid.actor_name.clear(); - - let accepted = store.insert_all(vec![ - create_event("event-1"), - invalid, - create_event("event-2"), - ]); - - assert_eq!(accepted, [create_event("event-2")]); - } - - #[test] - fn terminal_action_evicts_the_whole_call_even_at_capacity() { - let mut store = CallToPlayStore::default(); - store - .insert(create_event("create")) - .expect("create should fit"); - for index in 1..MAX_EVENTS { - store - .insert(action_event( - &format!("event-{index}"), - "call-1", - CallToPlayAction::Rsvp, - )) - .expect("active history should fit through the cap"); - } - - assert!( - store - .insert(action_event("start", "call-1", CallToPlayAction::Start)) - .expect("terminal action should compact the full call") + assert_eq!( + store.merge_batch_at( + vec![ + action_event("rsvp", "call-1", CallToPlayAction::Rsvp), + invalid, + ], + TEST_NOW, + ), + Err(MergeError::Invalid("invalid message")) ); - let snapshot = store.snapshot(); - assert_eq!(snapshot.len(), 1); - assert_eq!(snapshot[0].id, "start"); + assert_eq!(store.snapshot_at(TEST_NOW), before); } #[test] - fn terminal_action_keeps_unrelated_active_calls() { + fn conflicting_event_id_leaves_store_unchanged() { let mut store = CallToPlayStore::default(); store - .insert(create_event("call-1-create")) - .expect("first call should fit"); - let mut other_create = create_event("call-2-create"); - other_create.call_id = "call-2".to_string(); - store.insert(other_create).expect("second call should fit"); - store - .insert(action_event("cancel", "call-1", CallToPlayAction::Cancel)) - .expect("cancel should compact the first call"); + .merge_batch_at(vec![create_event("create")], TEST_NOW) + .expect("create should fit"); + let before = store.snapshot_at(TEST_NOW); + let mut conflicting = create_event("create"); + conflicting.actor_name = "Mallory".to_string(); - let snapshot = store.snapshot(); - assert_eq!(snapshot.len(), 2); - assert!(snapshot.iter().any(|event| event.id == "cancel")); - assert!(snapshot.iter().any(|event| event.call_id == "call-2")); + assert_eq!( + store.merge_batch_at(vec![conflicting], TEST_NOW), + Err(MergeError::ConflictingEvent("create".to_string())) + ); + assert_eq!(store.snapshot_at(TEST_NOW), before); + } + + #[test] + fn create_and_extension_revive_expired_history_in_any_batch_order() { + let extended_deadline = TEST_NOW + 20 * 60_000; + let extension = action_event( + "extend", + "call-1", + CallToPlayAction::AddTime { + deadline: extended_deadline, + }, + ); + let after_recovery_window = TEST_NOW + 6 * 60_000 + 1; + + for history in [ + vec![create_event("create"), extension.clone()], + vec![extension.clone(), create_event("create")], + ] { + let mut store = CallToPlayStore::default(); + store + .merge_batch_at(vec![create_event("create")], TEST_NOW) + .expect("initial create should fit"); + + let missing = store + .merge_batch_at(vec![extension.clone()], after_recovery_window) + .expect("missing history is an acknowledged merge outcome"); + assert!(missing.applied.is_empty()); + assert_eq!(missing.missing_call_ids, ["call-1"]); + assert!(store.snapshot_at(after_recovery_window).is_empty()); + + let revived = store + .merge_batch_at(history, after_recovery_window) + .expect("complete history should revive the call atomically"); + assert_eq!(revived.applied.len(), 2); + assert_eq!( + event_ids(store.snapshot_at(after_recovery_window)), + event_ids(revived.applied) + ); + } + } + + #[test] + fn immediately_obsolete_event_is_not_applied() { + let mut store = CallToPlayStore::default(); + let after_recovery_window = TEST_NOW + 6 * 60_000 + 1; + let merged = store + .merge_batch_at(vec![create_event("create")], after_recovery_window) + .expect("obsolete is an acknowledged merge outcome"); + + assert!(merged.applied.is_empty()); + assert_eq!(merged.obsolete, 1); + assert!(store.snapshot_at(after_recovery_window).is_empty()); + } + + #[test] + fn terminal_tombstone_prevents_stale_history_resurrection() { + let mut store = CallToPlayStore::default(); + let start = action_event("start", "call-1", CallToPlayAction::Start); + store + .merge_batch_at(vec![create_event("create"), start.clone()], TEST_NOW) + .expect("terminal history should merge"); + + let stale = store + .merge_batch_at( + vec![ + create_event("create"), + action_event( + "extend", + "call-1", + CallToPlayAction::AddTime { + deadline: TEST_NOW + 20 * 60_000, + }, + ), + ], + TEST_NOW, + ) + .expect("stale history is an obsolete merge outcome"); + + assert!(stale.applied.is_empty()); + assert_eq!(stale.obsolete, 2); + assert_eq!(store.snapshot_at(TEST_NOW), [start]); + } + + #[test] + fn terminal_actions_succeed_at_the_active_history_cap() { + for terminal_action in [CallToPlayAction::Start, CallToPlayAction::Cancel] { + let mut store = full_active_store(); + let terminal = action_event("terminal", "call-1", terminal_action); + + let merged = store + .merge_batch_at(vec![terminal.clone()], TEST_NOW) + .expect("terminal action should compact the full call"); + assert_eq!(merged.applied.as_slice(), std::slice::from_ref(&terminal)); + assert_eq!(store.snapshot_at(TEST_NOW), [terminal]); + } + } + + #[test] + fn terminal_tombstones_do_not_consume_active_history_capacity() { + let mut store = full_active_store(); + store + .merge_batch_at( + vec![action_event( + "call-1-start", + "call-1", + CallToPlayAction::Start, + )], + TEST_NOW, + ) + .expect("start should compact active history"); + + let created = create_event_for("call-2", "call-2-create"); + let merged = store + .merge_batch_at(vec![created.clone()], TEST_NOW) + .expect("terminal tombstone must not block a new active call"); + assert_eq!(merged.applied, [created]); + assert_eq!(store.snapshot_at(TEST_NOW).len(), 2); } #[test] fn full_active_history_returns_an_error() { - let mut store = CallToPlayStore::default(); - store - .insert(create_event("create")) - .expect("create should fit"); - for index in 1..MAX_EVENTS { - store - .insert(action_event( - &format!("event-{index}"), - "call-1", - CallToPlayAction::Rsvp, - )) - .expect("active history should fit through the cap"); - } + let mut store = full_active_store(); + let before = store.snapshot_at(TEST_NOW); assert_eq!( - store.insert(action_event("overflow", "call-1", CallToPlayAction::Rsvp)), - Err("Call to Play event history is full") + store.merge_batch_at( + vec![action_event("overflow", "call-1", CallToPlayAction::Rsvp,)], + TEST_NOW, + ), + Err(MergeError::HistoryFull) ); - assert_eq!(store.snapshot().len(), MAX_EVENTS); + assert_eq!(store.snapshot_at(TEST_NOW), before); } #[test] - fn expired_call_history_is_evicted_as_a_unit() { + fn visible_call_retains_complete_chat_history() { + let mut store = CallToPlayStore::default(); + let message = action_event( + "message-event", + "call-1", + CallToPlayAction::SendMessage { + message_id: "message-1".to_string(), + text: "Ready when you are".to_string(), + }, + ); + let merged = store + .merge_batch_at( + vec![ + create_event("create"), + action_event("rsvp", "call-1", CallToPlayAction::Rsvp), + message.clone(), + ], + TEST_NOW, + ) + .expect("visible history should merge"); + + assert_eq!(merged.applied.len(), 3); + assert!(store.snapshot_at(TEST_NOW).contains(&message)); + } + + fn full_active_store() -> CallToPlayStore { + let mut history = Vec::with_capacity(MAX_EVENTS); + history.push(create_event("create")); + history.extend((1..MAX_EVENTS).map(|index| { + action_event(&format!("event-{index}"), "call-1", CallToPlayAction::Rsvp) + })); + let mut store = CallToPlayStore::default(); store - .insert(create_event("create")) - .expect("active call should fit"); + .merge_batch_at(history, TEST_NOW) + .expect("active history should fit through the cap"); + store + } - assert!(store.snapshot_at(TEST_NOW + 5 * 60_000 + 60_001).is_empty()); + fn event_ids(events: Vec) -> Vec { + events.into_iter().map(|event| event.id).collect() } } diff --git a/crates/lanspread-peer/src/lib.rs b/crates/lanspread-peer/src/lib.rs index 7ea223b..663a438 100644 --- a/crates/lanspread-peer/src/lib.rs +++ b/crates/lanspread-peer/src/lib.rs @@ -504,9 +504,7 @@ async fn handle_peer_commands( handle_connect_peer_command(ctx, tx_notify_ui, addr).await; } PeerCommand::PublishCallToPlay { event, reply } => { - let result = call_to_play::publish(ctx, tx_notify_ui, event) - .await - .map_err(str::to_owned); + let result = call_to_play::publish(ctx, tx_notify_ui, event).await; let _ = reply.send(result); } PeerCommand::GetCallToPlayEvents { reply } => { diff --git a/crates/lanspread-peer/src/services/handshake.rs b/crates/lanspread-peer/src/services/handshake.rs index 498de67..914d014 100644 --- a/crates/lanspread-peer/src/services/handshake.rs +++ b/crates/lanspread-peer/src/services/handshake.rs @@ -195,9 +195,21 @@ async fn merge_call_to_play_events( tx_notify_ui: &UnboundedSender, incoming: Vec, ) { - let accepted = store.write().await.insert_all(incoming); - if !accepted.is_empty() { - events::send(tx_notify_ui, PeerEvent::CallToPlayEvents(accepted)); + match store.write().await.merge_batch(incoming) { + Ok(merged) => { + if merged.needs_history() { + log::warn!( + "Call to Play handshake omitted roots for calls: {}", + merged.missing_call_ids.join(", ") + ); + } + if !merged.applied.is_empty() { + events::send(tx_notify_ui, PeerEvent::CallToPlayEvents(merged.applied)); + } + } + Err(err) => { + log::warn!("Rejecting Call to Play handshake history: {err}"); + } } } @@ -371,8 +383,8 @@ mod tests { ctx.call_to_play .write() .await - .insert(call_to_play_event()) - .expect("valid event should be inserted"); + .merge_batch(vec![call_to_play_event()]) + .expect("valid event should be merged"); let hello = build_hello_from_state(&ctx) .await diff --git a/crates/lanspread-peer/src/services/stream.rs b/crates/lanspread-peer/src/services/stream.rs index 7dac869..5c2d73d 100644 --- a/crates/lanspread-peer/src/services/stream.rs +++ b/crates/lanspread-peer/src/services/stream.rs @@ -146,12 +146,24 @@ async fn handle_call_to_play_events( return; } - let accepted = ctx.call_to_play.write().await.insert_all(incoming); - if !accepted.is_empty() { - events::send( - &ctx.tx_notify_ui, - crate::PeerEvent::CallToPlayEvents(accepted), - ); + match ctx.call_to_play.write().await.merge_batch(incoming) { + Ok(merged) => { + if merged.needs_history() { + log::warn!( + "Ignoring Call to Play actions without history from {peer_id}: {}", + merged.missing_call_ids.join(", ") + ); + } + if !merged.applied.is_empty() { + events::send( + &ctx.tx_notify_ui, + crate::PeerEvent::CallToPlayEvents(merged.applied), + ); + } + } + Err(err) => { + log::warn!("Rejecting Call to Play events from {peer_id}: {err}"); + } } }