From 359bea39491475226faf1cc9cc8bf33717ac95b1 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Wed, 23 Sep 2026 22:24:14 -0700 Subject: [PATCH] Reject stale drawing packets before retiring Allocation gestures --- crates/oa-desktop/src/app.rs | 12 + .../src/app/allocation_drag_tests/group.rs | 2 + .../group/connection_lifetime.rs | 284 ++++++++++++++++++ 3 files changed, 298 insertions(+) create mode 100644 crates/oa-desktop/src/app/allocation_drag_tests/group/connection_lifetime.rs diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 646b0415..b3741927 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -1351,6 +1351,18 @@ impl App { let preview_read_only = self.process_preview_primary().is_some(); let selection_boundary = self.multiselect_boundary(); // Reject delayed gestures before they can retire newer input ownership. + if let Message::Connection( + crate::connection_edit::Event::Endpoint(captured, _) + | crate::connection_edit::Event::Restart(captured), + ) = &message + && (self.route_context() != Some(*captured) + || self + .connection + .as_ref() + .is_none_or(|session| session.context != *captured)) + { + return Task::none(); + } if let Message::DiagramSelection(event) | Message::StateDiagram(crate::behavior::state_edit::Event::Selection(event)) = &message && !self.multiselect_envelope(event) diff --git a/crates/oa-desktop/src/app/allocation_drag_tests/group.rs b/crates/oa-desktop/src/app/allocation_drag_tests/group.rs index eb894abb..944e8f65 100644 --- a/crates/oa-desktop/src/app/allocation_drag_tests/group.rs +++ b/crates/oa-desktop/src/app/allocation_drag_tests/group.rs @@ -2,6 +2,8 @@ use super::*; use crate::app::multiselect::{self, Choice}; use crate::diagram_usage::Target as SelectionTarget; +mod connection_lifetime; + struct Group { f: Fixture, y: Id, diff --git a/crates/oa-desktop/src/app/allocation_drag_tests/group/connection_lifetime.rs b/crates/oa-desktop/src/app/allocation_drag_tests/group/connection_lifetime.rs new file mode 100644 index 00000000..eda21144 --- /dev/null +++ b/crates/oa-desktop/src/app/allocation_drag_tests/group/connection_lifetime.rs @@ -0,0 +1,284 @@ +//! Delayed captured drawing packets must not retire a newer Allocation owner. +use super::*; +use crate::connection_edit::Event as ConnectionEvent; +use crate::connection_tool::Tool; + +fn drawing_tool(layer: Layer) -> Tool { + match layer { + Layer::Oa => Tool::Interaction, + Layer::Sa => Tool::FunctionalExchange, + } +} + +fn drawing_source(g: &mut Group, layer: Layer) -> crate::route_edit::Context { + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Choose(Some( + drawing_tool(layer), + )))); + let context = g.f.app.route_context().unwrap(); + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Endpoint( + context, g.f.work, + ))); + let session = g.f.app.connection.as_ref().unwrap(); + assert_eq!(session.context, context); + assert_eq!(session.source, Some(g.f.work)); + assert!(session.candidates.contains(&g.y)); + assert_eq!(g.f.app.route_context(), Some(context)); + context +} + +fn assert_captured_noop(app: &mut App, captured: crate::route_edit::Context, target: Id) { + for event in [ + ConnectionEvent::Endpoint(captured, target), + ConnectionEvent::Restart(captured), + ] { + assert_connection_noop(app, event); + } +} + +fn assert_connection_noop(app: &mut App, event: ConnectionEvent) { + let document = app.document.snapshot(); + let selected = selection(app); + let context = app.route_context(); + let camera = (app.pan, app.zoom, app.canvas_epoch, app.camera_epoch); + let ownership = (app.allocation_drag.epoch, app.allocation_drag.arrange_next); + let active = app + .allocation_drag + .active + .as_ref() + .map(|a| (a.press.clone(), a.crossed, a.cache.len())); + let preview = app.allocation_drag.preview.as_ref().map(|p| { + ( + p.token, + p.placement.diagram.clone(), + p.placement.delta, + p.project().clone(), + p.routed.clone(), + ) + }); + let feedback = app.allocation_drag.feedback.as_ref().map(|f| { + ( + f.press.token, + f.text.clone(), + f.refused, + f.targets.clone(), + f.inspection_targets.clone(), + ) + }); + let connection = |app: &App| { + app.connection + .as_ref() + .map(|s| (s.context, s.tool, s.source, s.candidates.clone())) + }; + let session = connection(app); + let interaction = app.source_symbols.interaction; + let status = app.status.clone(); + let _ = app.update(Message::Connection(event)); + assert_eq!(selection(app), selected); + assert_eq!(app.route_context(), context); + assert_eq!( + (app.pan, app.zoom, app.canvas_epoch, app.camera_epoch), + camera + ); + assert_eq!( + (app.allocation_drag.epoch, app.allocation_drag.arrange_next), + ownership, + "rejected drawing packet must not retire the newer input owner" + ); + match (active, app.allocation_drag.active.as_ref()) { + (Some((press, crossed, cached)), Some(current)) => { + assert!(Arc::ptr_eq(&press, ¤t.press)); + assert_eq!((current.crossed, current.cache.len()), (crossed, cached)); + } + (None, None) => {} + _ => panic!("rejected drawing packet changed the active Allocation token"), + } + match (preview, app.allocation_drag.preview.as_ref()) { + (Some((token, diagram, delta, project, routed)), Some(current)) => { + assert_eq!(current.token, token); + assert_eq!(current.placement.diagram, diagram); + assert_eq!(current.placement.delta, delta); + assert_eq!(current.project(), &project); + assert!(Arc::ptr_eq(¤t.routed, &routed)); + } + (None, None) => {} + _ => panic!("rejected drawing packet changed the Allocation preview"), + } + assert_eq!( + app.allocation_drag.feedback.as_ref().map(|f| ( + f.press.token, + f.text.clone(), + f.refused, + f.targets.clone(), + f.inspection_targets.clone(), + )), + feedback + ); + assert_eq!(connection(app), session); + assert_eq!(app.source_symbols.interaction, interaction); + assert_eq!(app.status, status); + assert!(Arc::ptr_eq(&document, &app.document.snapshot())); + assert_eq!(app.source_workspace.bundle(), document.bundle); +} + +#[test] +fn captured_connection_after_cancel_preserves_arrange_arm_and_live_group() { + for layer in [Layer::Oa, Layer::Sa] { + let mut g = group(layer); + let old = drawing_source(&mut g, layer); + let _ = g.f.app.update(Message::Escape); + assert!(g.f.app.connection.is_none()); + select(&mut g.f.app, &[g.an, g.yn]); + let _ = g.f.app.update(Message::WorkDrag(Event::ArrangeNext)); + assert!(g.f.app.allocation_drag.arrange_next); + let current = g.f.app.route_context().unwrap(); + assert_captured_noop(&mut g.f.app, old, g.y); + // Matching context with no session isolates the missing-owner guard. + assert_captured_noop(&mut g.f.app, current, g.y); + + let before = g.f.app.project().clone(); + let source = g.f.app.document.bundle().clone(); + let held = selection(&g.f.app); + let from = node_point(&g.f.app, g.an); + let to = from + Vector::new(400., 0.); + let mut state = CanvasState::default(); + let press = begin(&mut g.f.app, &mut state, from); + assert_eq!(press.mode, Mode::Arrange); + assert!(!g.f.app.allocation_drag.arrange_next); + move_pointer(&mut g.f.app, &mut state, to); + assert!(matches!( + g.f.app + .allocation_drag + .preview + .as_ref() + .map(|p| &p.candidate), + Some(Candidate::Arrange(_)) + )); + let current = g.f.app.route_context().unwrap(); + assert_captured_noop(&mut g.f.app, old, g.y); + assert_captured_noop(&mut g.f.app, current, g.y); + release(&mut g.f.app, &mut state, to); + let mut expected = before.clone(); + let diagram = expected.diagrams.get_mut(&g.f.diagram).unwrap(); + diagram.nodes.get_mut(&g.an).unwrap().position = xy(500., 100.); + diagram.nodes.get_mut(&g.f.node).unwrap().position = xy(540., 200.); + diagram.nodes.get_mut(&g.yn).unwrap().position = xy(540., 600.); + assert_eq!(selection(&g.f.app), held); + exact_history(&mut g.f.app, &before, &source, &expected); + } +} + +#[test] +fn captured_connection_context_mismatches_preserve_the_current_session() { + let mut g = group(Layer::Oa); + let current = drawing_source(&mut g, Layer::Oa); + // Session and current context agree: each old captured field is the sole + // invalid relationship, not hidden behind a missing session. + for captured in [ + crate::route_edit::Context { + document: Id::new_v4(), + ..current + }, + crate::route_edit::Context { + diagram: Id::new_v4(), + ..current + }, + crate::route_edit::Context { + revision: current.revision + 1, + ..current + }, + crate::route_edit::Context { + epoch: current.epoch + 1, + ..current + }, + ] { + assert_eq!(g.f.app.connection.as_ref().unwrap().context, current); + assert_captured_noop(&mut g.f.app, captured, g.y); + } + // Conversely, incoming/current contexts agree. Retain obsolete session + // metadata explicitly to isolate the session-context admission branch. + g.f.app.connection.as_mut().unwrap().context.epoch += 1; + assert_eq!(g.f.app.route_context(), Some(current)); + assert_captured_noop(&mut g.f.app, current, g.y); +} + +#[test] +fn captured_connection_current_restart_and_pair_commit_after_tool_boundary() { + for begin_tool in [false, true] { + let mut g = group(Layer::Oa); + select(&mut g.f.app, &[g.an, g.yn]); + let _ = g.f.app.update(Message::WorkDrag(Event::ArrangeNext)); + let from = node_point(&g.f.app, g.an); + let mut state = CanvasState::default(); + begin(&mut g.f.app, &mut state, from); + move_pointer(&mut g.f.app, &mut state, from + Vector::new(400., 0.)); + assert!(g.f.app.allocation_drag.preview.is_some()); + let before = g.f.app.project().clone(); + let source = g.f.app.document.bundle().clone(); + let epoch = g.f.app.allocation_drag.epoch; + let event = if begin_tool { + ConnectionEvent::Begin(Tool::Interaction, g.f.work) + } else { + ConnectionEvent::Choose(Some(Tool::Interaction)) + }; + let _ = g.f.app.update(Message::Connection(event)); + assert!(g.f.app.allocation_drag.epoch > epoch); + assert!(g.f.app.allocation_drag.active.is_none()); + assert!(g.f.app.allocation_drag.preview.is_none()); + assert!(!g.f.app.allocation_drag.arrange_next); + let context = g.f.app.route_context().unwrap(); + if !begin_tool { + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Endpoint( + context, g.f.work, + ))); + } + assert_eq!(g.f.app.connection.as_ref().unwrap().source, Some(g.f.work)); + assert!( + g.f.app + .connection + .as_ref() + .unwrap() + .candidates + .contains(&g.y) + ); + let document = g.f.app.document.snapshot(); + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Restart(context))); + let context = g.f.app.route_context().unwrap(); + let restarted = g.f.app.connection.as_ref().unwrap(); + assert_eq!(restarted.context, context); + assert_eq!(restarted.tool, Tool::Interaction); + assert_eq!(restarted.source, None); + assert!(restarted.candidates.contains(&g.f.work)); + assert!(Arc::ptr_eq(&document, &g.f.app.document.snapshot())); + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Endpoint( + context, g.f.work, + ))); + assert_eq!(g.f.app.connection.as_ref().unwrap().source, Some(g.f.work)); + let revision = g.f.app.document.revision(); + let _ = + g.f.app + .update(Message::Connection(ConnectionEvent::Endpoint(context, g.y))); + assert!(g.f.app.connection.is_none()); + assert_eq!(g.f.app.document.revision(), revision + 1); + let relation = &g.f.app.project().relations[&g.f.app.selected]; + assert_eq!(relation.kind(), RelationKind::Interaction); + assert_eq!((relation.source(), relation.target()), (g.f.work, g.y)); + let mut expected = before.clone(); + assert!( + expected + .relations + .insert(relation.id, relation.clone()) + .is_none() + ); + exact_history(&mut g.f.app, &before, &source, &expected); + } +}