From 24595810a1d9d1866c099e1dafb976fbec09e46f Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 02:18:14 +0300 Subject: [PATCH] Preserve behavior drafts until document commit succeeds --- crates/oa-desktop/src/behavior.rs | 261 ++++++++++++++++++++- crates/oa-desktop/src/behavior/scenario.rs | 31 ++- crates/oa-desktop/src/behavior/states.rs | 36 ++- crates/oa-desktop/src/behavior/topology.rs | 42 ++-- 4 files changed, 336 insertions(+), 34 deletions(-) diff --git a/crates/oa-desktop/src/behavior.rs b/crates/oa-desktop/src/behavior.rs index 0867bb7d..4a6cb8a3 100644 --- a/crates/oa-desktop/src/behavior.rs +++ b/crates/oa-desktop/src/behavior.rs @@ -42,13 +42,70 @@ pub struct State { scenario: scenario::State, states: states::State, } +const PENDING_DRAFT: &str = "Apply or cancel the current behavior form before starting another."; impl State { + pub fn has_pending(&self) -> bool { + self.pending_owner().is_some() + } + + pub fn pending_owner(&self) -> Option { + self.topology + .pending_owner() + .or_else(|| self.scenario.pending_owner()) + .or_else(|| self.states.pending_owner()) + } + + /// Prepare the active form without discarding inputs. The caller must execute + /// this exact command and acknowledge only a successful document transaction. + pub fn prepare_pending(&self, p: &Project) -> Result, String> { + if self.topology.pending_owner().is_some() { + self.topology.prepare_pending(p).map(Some) + } else if self.scenario.pending_owner().is_some() { + self.scenario.prepare_pending(p).map(Some) + } else if self.states.pending_owner().is_some() { + self.states.prepare_pending(p).map(Some) + } else { + Ok(None) + } + } + + /// Clear the active form after its prepared command commits successfully. + /// Rejected commands must leave the form available for repair and retry. + pub fn acknowledge_commit(&mut self) { + self.topology.acknowledge_commit(); + self.scenario.acknowledge_commit(); + self.states.acknowledge_commit(); + } + pub fn update(&mut self, p: &Project, event: Event) -> Result, String> { + if self.has_pending() + && matches!( + &event, + Event::Topology(topology::Event::Start(..)) + | Event::Scenario(scenario::Event::Start(..)) + | Event::States(states::Event::Start(..) | states::Event::Edit(..)) + ) + { + return Err(PENDING_DRAFT.into()); + } + let cancels_pending = match &event { + Event::Topology(topology::Event::Cancel) => self.topology.pending_owner().is_some(), + Event::Scenario(scenario::Event::Cancel) => self.scenario.pending_owner().is_some(), + Event::States(states::Event::Cancel) => self.states.pending_owner().is_some(), + _ => false, + }; + if cancels_pending { + self.acknowledge_commit(); + return Ok(None); + } match event { Event::Topology(event) => self.topology.update(p, event), Event::Scenario(event) => self.scenario.update(p, event), Event::States(event) => self.states.update(p, event), - Event::Submit => self.topology.update(p, topology::Event::Submit), + Event::Submit => self + .prepare_pending(p)? + .map(Some) + .ok_or_else(|| "Choose a behavior modeling tool.".into()), } } } @@ -698,6 +755,208 @@ impl canvas::Program for Scene { #[cfg(test)] mod tests { use super::*; + use crate::document::{Document, DocumentError}; + use oa_core::source_project::{self, ProjectLimits}; + + #[test] + fn one_behavior_form_survives_start_edit_and_unrelated_cancel_until_commit() { + let p = Project::demo(); + let owner = |kind| p.elements.values().find(|e| e.kind() == kind).unwrap().id; + let process = owner(Kind::Process); + let scenario = owner(Kind::Scenario); + let machine = owner(Kind::StateMachine); + let transition = p + .relations + .values() + .find(|r| r.kind() == RelationKind::StateTransition) + .unwrap() + .id; + let starts = [ + Event::Topology(topology::Event::Start( + process, + topology::Tool::Control, + None, + )), + Event::Scenario(scenario::Event::Start(scenario, scenario::Tool::Fragment)), + Event::States(states::Event::Start(machine, states::Tool::Region, machine)), + ]; + let names = [ + Event::Topology(topology::Event::Name("Keep this draft".into())), + Event::Scenario(scenario::Event::Name("Keep this draft".into())), + Event::States(states::Event::Name("Keep this draft".into())), + ]; + let cancels = [ + Event::Topology(topology::Event::Cancel), + Event::Scenario(scenario::Event::Cancel), + Event::States(states::Event::Cancel), + ]; + for (index, expected_owner) in [process, scenario, machine].into_iter().enumerate() { + let mut state = State::default(); + assert!(!state.has_pending()); + assert!(state.prepare_pending(&p).unwrap().is_none()); + state.update(&p, starts[index].clone()).unwrap(); + state.update(&p, names[index].clone()).unwrap(); + if index == 1 { + state + .update( + &p, + Event::Scenario(scenario::Event::Expression("approved".into())), + ) + .unwrap(); + } + assert_eq!(state.pending_owner(), Some(expected_owner)); + for replacement in starts + .iter() + .cloned() + .chain([Event::States(states::Event::Edit(transition))]) + { + assert_eq!(state.update(&p, replacement).unwrap_err(), PENDING_DRAFT); + assert_eq!(state.pending_owner(), Some(expected_owner)); + } + state.update(&p, cancels[(index + 1) % 3].clone()).unwrap(); + assert!(state.has_pending(), "Another editor's cancel is stale"); + let command = state.update(&p, Event::Submit).unwrap().unwrap(); + assert!(state.has_pending(), "Preparation must retain the form"); + let expected = execute_batch(&p, std::slice::from_ref(&command)).unwrap(); + assert!(expected.elements.values().any(|e| { + !p.elements.contains_key(&e.id) + && e.name == "Keep this draft" + && e.parent == Some(expected_owner) + })); + let mut document = Document::from_project(p.clone(), ProjectLimits::default()).unwrap(); + document.execute(command).unwrap(); + assert_eq!(document.project(), &expected); + state.acknowledge_commit(); + assert!(!state.has_pending()); + assert!(state.prepare_pending(document.project()).unwrap().is_none()); + assert_eq!(document.revision(), 1); + assert!(document.undo()); + assert_eq!(document.project(), &p); + assert!(!document.can_undo()); + + state.update(&p, starts[index].clone()).unwrap(); + state.update(&p, cancels[index].clone()).unwrap(); + assert!(!state.has_pending(), "Explicit cancel discards this form"); + state.update(&p, starts[(index + 1) % 3].clone()).unwrap(); + } + } + + #[test] + fn failed_document_commit_retains_typed_transition_and_time_event_for_repair() { + let mut p = Project::empty(); + let owner = p + .add_element(Kind::System, Layer::Sa, "System", None) + .unwrap(); + let machine = p + .add_element(Kind::StateMachine, Layer::Sa, "Modes", Some(owner)) + .unwrap(); + let region = p + .add_element(Kind::Region, Layer::Sa, "Region", Some(machine)) + .unwrap(); + let source = p + .add_element(Kind::State, Layer::Sa, "Ready", Some(region)) + .unwrap(); + let target = p + .add_element(Kind::State, Layer::Sa, "Waiting", Some(region)) + .unwrap(); + let work = p + .add_element(Kind::Function, Layer::Sa, "Monitor", None) + .unwrap(); + let choice = |id| Choice { + id, + label: p.named(id).into(), + }; + let mut state = State::default(); + for event in [ + states::Event::Start(machine, states::Tool::Transition, source), + states::Event::Name("Transition".repeat(1024)), + states::Event::Trigger(choice(work)), + states::Event::Effect(choice(work)), + states::Event::TriggerNotes("On a monitor result".into()), + states::Event::Guard("connection unavailable".into()), + states::Event::EffectNotes("Record the timeout".into()), + states::Event::Timed(true), + states::Event::TimeKind("At".into()), + states::Event::Expression("12:30 UTC".into()), + ] { + state.update(&p, Event::States(event)).unwrap(); + } + assert!( + state.prepare_pending(&p).is_err(), + "A target is still missing" + ); + assert_eq!(state.pending_owner(), Some(machine)); + state + .update(&p, Event::States(states::Event::Target(choice(target)))) + .unwrap(); + let command = state.prepare_pending(&p).unwrap().unwrap(); + let bundle = source_project::format_bundle(&p); + let mut limits = ProjectLimits::default(); + limits.model.max_source_bytes = + bundle.files.values().map(String::len).sum::() + 2000; + let mut document = Document::from_bundle(bundle.clone(), limits).unwrap(); + assert!(matches!( + document.execute(command), + Err(DocumentError::Source(_)) + )); + assert_eq!(document.project(), &p); + assert_eq!(document.bundle(), &bundle); + assert_eq!(document.revision(), 0); + assert!(!document.can_undo()); + assert!(state.has_pending()); + + state + .update( + &p, + Event::States(states::Event::Name("Wait for recovery".into())), + ) + .unwrap(); + let command = state.prepare_pending(&p).unwrap().unwrap(); + let Command::Batch(commands) = &command else { + panic!("Expected one atomic transition batch") + }; + assert_eq!(commands.len(), 2); + let Command::CreateElement(event) = &commands[0] else { + panic!("Expected a typed time event") + }; + assert!( + matches!(event.shared(), Some(SharedBody::Event { body: EventBody::Time { kind: TimeEventKind::At, expression } }) if expression == "12:30 UTC") + ); + let Command::CreateRelation(relation) = &commands[1] else { + panic!("Expected the transition") + }; + let RelationBody::StateTransition { + triggers, + effects, + trigger, + guard, + effect, + .. + } = &relation.body + else { + panic!() + }; + assert_eq!(triggers, &[work, event.id]); + assert_eq!(effects, &[work]); + assert_eq!(trigger, "On a monitor result"); + assert_eq!(guard, "connection unavailable"); + assert_eq!(effect, "Record the timeout"); + let expected = execute_batch(&p, std::slice::from_ref(&command)).unwrap(); + document.execute(command).unwrap(); + assert_eq!( + document.project(), + &expected, + "Commit the exact prepared identities" + ); + state.acknowledge_commit(); + assert!(!state.has_pending()); + assert_eq!(document.revision(), 1); + assert!(document.undo()); + assert_eq!(document.bundle(), &bundle); + assert_eq!(document.project(), &p); + assert!(!document.can_undo()); + } + #[test] fn legacy_process_and_chain_keep_their_ordered_exchange_scene() { let p = Project::demo(); diff --git a/crates/oa-desktop/src/behavior/scenario.rs b/crates/oa-desktop/src/behavior/scenario.rs index 800fd881..d4e42800 100644 --- a/crates/oa-desktop/src/behavior/scenario.rs +++ b/crates/oa-desktop/src/behavior/scenario.rs @@ -103,7 +103,28 @@ struct Draft { values: BTreeMap, } impl State { + pub(super) fn pending_owner(&self) -> Option { + self.draft.as_ref().map(|d| d.owner) + } + + pub(super) fn prepare_pending(&self, p: &Project) -> Result { + let command = self + .draft + .as_ref() + .ok_or("Choose a scenario tool first.")? + .command(p)?; + execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; + Ok(command) + } + + pub(super) fn acknowledge_commit(&mut self) { + self.draft = None; + } + pub fn update(&mut self, p: &Project, event: Event) -> Result, String> { + if self.draft.is_some() && matches!(&event, Event::Start(..)) { + return Err(PENDING_DRAFT.into()); + } match event { Event::Start(owner, tool) => { if p.elements.get(&owner).map(oa_core::Element::kind) != Some(Kind::Scenario) { @@ -127,14 +148,8 @@ impl State { values: BTreeMap::new(), }); } - Event::Cancel => self.draft = None, - Event::Submit => { - let d = self.draft.as_ref().ok_or("Choose a scenario tool first.")?; - let command = d.command(p)?; - execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; - self.draft = None; - return Ok(Some(command)); - } + Event::Cancel => self.acknowledge_commit(), + Event::Submit => return self.prepare_pending(p).map(Some), other => { if let Some(d) = &mut self.draft { match other { diff --git a/crates/oa-desktop/src/behavior/states.rs b/crates/oa-desktop/src/behavior/states.rs index 4a459425..675c9a43 100644 --- a/crates/oa-desktop/src/behavior/states.rs +++ b/crates/oa-desktop/src/behavior/states.rs @@ -189,7 +189,28 @@ impl Draft { } } impl State { + pub(super) fn pending_owner(&self) -> Option { + self.draft.as_ref().map(|d| d.machine) + } + + pub(super) fn prepare_pending(&self, p: &Project) -> Result { + let command = self + .draft + .as_ref() + .ok_or("Choose a state modeling tool.")? + .command(p)?; + execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; + Ok(command) + } + + pub(super) fn acknowledge_commit(&mut self) { + self.draft = None; + } + pub fn update(&mut self, p: &Project, event: Event) -> Result, String> { + if self.draft.is_some() && matches!(&event, Event::Start(..) | Event::Edit(..)) { + return Err(PENDING_DRAFT.into()); + } match event { Event::Start(machine, tool, selected) => { let mut d = Draft::new(machine, tool); @@ -249,17 +270,8 @@ impl State { d.effect = effect.clone(); self.draft = Some(d); } - Event::Cancel => self.draft = None, - Event::Submit => { - let command = self - .draft - .as_ref() - .ok_or("Choose a state modeling tool.")? - .command(p)?; - execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; - self.draft = None; - return Ok(Some(command)); - } + Event::Cancel => self.acknowledge_commit(), + Event::Submit => return self.prepare_pending(p).map(Some), Event::Viewport(size) => self.viewport = Some(size), Event::Zoom(zoom) => self.zoom = Some(zoom.clamp(0.1, 2.)), Event::Fit => self.zoom = None, @@ -810,6 +822,7 @@ mod tests { } let mut result = execute_batch(&p, &[state.update(&p, Event::Submit).unwrap().unwrap()]).unwrap(); + state.acknowledge_commit(); let r = result.relations.values_mut().next().unwrap(); r.description = "Retain configuration".into(); let (id, alias) = (r.id, r.alias.clone()); @@ -846,6 +859,7 @@ mod tests { .unwrap(); let result = execute_batch(&p, &[state.update(&p, Event::Submit).unwrap().unwrap()]).unwrap(); + state.acknowledge_commit(); let region = result .children(ready) .find(|e| e.kind() == Kind::Region) diff --git a/crates/oa-desktop/src/behavior/topology.rs b/crates/oa-desktop/src/behavior/topology.rs index 409660a1..d3de8c6c 100644 --- a/crates/oa-desktop/src/behavior/topology.rs +++ b/crates/oa-desktop/src/behavior/topology.rs @@ -76,7 +76,28 @@ struct Draft { control: ControlKind, } impl State { + pub(super) fn pending_owner(&self) -> Option { + self.draft.as_ref().map(|d| d.owner) + } + + pub(super) fn prepare_pending(&self, p: &Project) -> Result { + let command = self + .draft + .as_ref() + .ok_or("Choose a contribution tool first.")? + .command(p)?; + execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; + Ok(command) + } + + pub(super) fn acknowledge_commit(&mut self) { + self.draft = None; + } + pub fn update(&mut self, p: &Project, event: Event) -> Result, String> { + if self.draft.is_some() && matches!(&event, Event::Start(..)) { + return Err(PENDING_DRAFT.into()); + } match event { Event::Viewport(size) => self.viewport = Some(size), Event::Zoom(zoom) => self.zoom = Some(zoom.clamp(0.1, 2.)), @@ -118,19 +139,8 @@ impl State { control: ControlKind::Choice, }); } - Event::Cancel => self.draft = None, - Event::Submit => { - let d = self - .draft - .as_ref() - .ok_or("Choose a contribution tool first.")?; - let command = d.command(p)?; - // Validate before clearing the user's inputs. The ordinary command - // handler remains responsible for the single undoable transaction. - execute_batch(p, std::slice::from_ref(&command)).map_err(|e| e.to_string())?; - self.draft = None; - return Ok(Some(command)); - } + Event::Cancel => self.acknowledge_commit(), + Event::Submit => return self.prepare_pending(p).map(Some), other => { if let Some(d) = &mut self.draft { match other { @@ -1772,9 +1782,13 @@ mod tests { assert!(state.draft.is_some()); state.update(&p, Event::Guard("ready".into())).unwrap(); let command = state.update(&p, Event::Submit).unwrap().unwrap(); - assert!(state.draft.is_none()); + assert!(state.draft.is_some()); assert!(matches!(command, Command::Batch(_))); assert_eq!(p.elements[&owner].steps().unwrap().len(), 2); + let result = execute_batch(&p, &[command]).unwrap(); + state.acknowledge_commit(); + assert!(state.draft.is_none()); + assert_eq!(result.elements[&owner].steps().unwrap().len(), 6); } #[test] fn cycle_projection_terminates_and_retains_actual_links() {