From e5660b89022343d7aef836bc45bf1d3b75f54982 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 02:35:28 +0300 Subject: [PATCH] Guard and preserve behavior forms across document lifecycle --- crates/oa-desktop/src/app.rs | 92 +++- .../src/app/behavior_lifecycle_tests.rs | 398 ++++++++++++++++++ crates/oa-desktop/src/behavior.rs | 15 +- 3 files changed, 495 insertions(+), 10 deletions(-) create mode 100644 crates/oa-desktop/src/app/behavior_lifecycle_tests.rs diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index cc86954d..dbb68d1c 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -12,6 +12,9 @@ use oa_core::{ use serde_json::{Value, json}; use std::{collections::BTreeSet, path::PathBuf, sync::Arc}; +#[cfg(test)] +mod behavior_lifecycle_tests; + #[derive(Debug, Clone)] pub enum LoadedSnapshot { Source(DiskProject), @@ -351,7 +354,10 @@ impl App { self.draft.as_ref().is_some_and(Draft::dirty) } pub fn dirty(&self) -> bool { - self.document.dirty() || self.draft_dirty() || self.source_dirty() + self.document.dirty() + || self.draft_dirty() + || self.source_dirty() + || self.behavior.has_pending() } fn sync_source(&mut self) { self.source_workspace.accept(self.document.bundle()); @@ -435,6 +441,11 @@ impl App { } } pub fn apply_source(&mut self) -> bool { + if self.behavior.has_pending() { + self.status = + "Apply or cancel the behavior form before applying source changes.".into(); + return false; + } if !self.source_dirty() { return true; } @@ -465,6 +476,10 @@ impl App { } } pub fn apply(&mut self) -> bool { + if self.behavior.has_pending() { + self.status = "Apply or cancel the behavior form before changing the model.".into(); + return false; + } if self.source_dirty() { self.status = "Apply all source drafts or revert them before changing the model.".into(); @@ -540,6 +555,30 @@ impl App { } } } + fn apply_behavior(&mut self) -> bool { + if !self.behavior.has_pending() { + return true; + } + if self.source_dirty() || self.draft_dirty() { + self.status = "Property, source and behavior drafts are retained. Revert the other drafts before applying the behavior form.".into(); + return false; + } + match self.behavior.prepare_pending(self.project()) { + Ok(Some(command)) => { + if self.execute(command) { + self.behavior.acknowledge_commit(); + true + } else { + false + } + } + Ok(None) => true, + Err(error) => { + self.status = error; + false + } + } + } fn reset(&mut self, project: Project, path: Option) { let window_size = self.window_size; let canvas_size = self.canvas_size; @@ -674,7 +713,7 @@ impl App { ) { self.model_menu = None; } - if self.source_dirty() + if (self.source_dirty() || self.behavior.has_pending()) && matches!( message, Message::Field(..) @@ -687,7 +726,8 @@ impl App { ) { self.status = - "Apply all source drafts or revert them before editing properties.".into(); + "Apply or cancel pending source and behavior drafts before editing properties." + .into(); return Task::none(); } if self.busy @@ -768,6 +808,35 @@ impl App { self.canvas_epoch = self.canvas_epoch.wrapping_add(1); self.selected_bend = None; } + if self.behavior.has_pending() + && matches!( + message, + Message::Page(page) if page != Page::Behavior + ) + { + self.status = "Apply or cancel the behavior form before leaving this view.".into(); + return Task::none(); + } + if self.behavior.has_pending() + && matches!( + message, + Message::BrowseGroup(_) + | Message::SourceEdit(_) + | Message::SourceCreateFile + | Message::SourceCloseFile(_) + | Message::SourceUndo + | Message::SourceRedo + | Message::SourceRecover + | Message::SourceLoadIncludes + | Message::Menu(_) + | Message::Connection(_) + | Message::PortAnchor(_) + | Message::Route(_) + ) + { + self.status = "Apply or cancel the behavior form before changing another view.".into(); + return Task::none(); + } let previous = self.selected; let previous_layer = self.layer; let entering_catalog = @@ -1297,14 +1366,22 @@ impl App { } Message::InspectorTab(tab) => self.inspector_tab = tab, Message::Behavior(event) => { - if event.is_submit() && !self.apply() { + if event.starts_form() && !self.behavior.has_pending() && !self.apply() { + return Task::none(); + } + if event.is_submit() && (self.source_dirty() || self.draft_dirty()) { + self.status = "Revert the other drafts before applying the behavior form. Your inputs are retained.".into(); return Task::none(); } let mut state = std::mem::take(&mut self.behavior); let result = state.update(self.project(), event); self.behavior = state; match result { - Ok(Some(command)) => return self.handle(Message::Command(command)), + Ok(Some(command)) => { + if self.execute(command) { + self.behavior.acknowledge_commit(); + } + } Ok(None) => {} Err(error) => self.status = error, } @@ -1440,6 +1517,9 @@ impl App { } } Message::ConfirmDelete => { + if !self.delete_pending || self.behavior.has_pending() { + return Task::none(); + } self.delete_pending = false; self.execute(Command::Delete(self.selected)); } @@ -1858,7 +1938,7 @@ impl App { return self.handle(Message::Save(false)); } Message::Save(save_as) => { - if !self.apply_source() || !self.apply() { + if !self.apply_behavior() || !self.apply_source() || !self.apply() { self.continue_after_save = false; self.pending = None; return Task::none(); diff --git a/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs b/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs new file mode 100644 index 00000000..c4cba873 --- /dev/null +++ b/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs @@ -0,0 +1,398 @@ +use super::*; +use crate::behavior::{self, states}; +use oa_core::source_project::ProjectLimits; + +struct Fixture { + app: App, + machine: Id, + source: Id, + target: Id, + work: Id, +} + +fn fixture() -> Fixture { + let mut project = Project::empty(); + let owner = project + .add_element(Kind::System, Layer::Sa, "System", None) + .unwrap(); + let machine = project + .add_element( + Kind::StateMachine, + Layer::Sa, + "Operating modes", + Some(owner), + ) + .unwrap(); + let region = project + .add_element(Kind::Region, Layer::Sa, "Operation", Some(machine)) + .unwrap(); + let source = project + .add_element(Kind::State, Layer::Sa, "Ready", Some(region)) + .unwrap(); + let target = project + .add_element(Kind::State, Layer::Sa, "Waiting", Some(region)) + .unwrap(); + let work = project + .add_element(Kind::Function, Layer::Sa, "Monitor connection", None) + .unwrap(); + let mut app = App::with_project(project); + send(&mut app, Message::Select(source)); + send(&mut app, Message::Page(Page::Behavior)); + Fixture { + app, + machine, + source, + target, + work, + } +} + +fn send(app: &mut App, message: Message) { + // Do not poll returned I/O tasks: these tests exercise the App transaction + // boundary without displaying a file dialog or writing to the filesystem. + let _ = app.update(message); +} + +fn state_event(app: &mut App, event: states::Event) { + send(app, Message::Behavior(behavior::Event::States(event))); +} + +fn choice(app: &App, id: Id) -> Choice { + Choice { + id, + label: app.project().named(id).into(), + } +} + +fn start_transition(fixture: &mut Fixture, complete: bool) { + let Fixture { + app, + machine, + source, + target, + work, + } = fixture; + state_event( + app, + states::Event::Start(*machine, states::Tool::Transition, *source), + ); + for event in [ + states::Event::Name("Wait for recovery".into()), + states::Event::Trigger(choice(app, *work)), + states::Event::Effect(choice(app, *work)), + states::Event::TriggerNotes("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_event(app, event); + } + if complete { + let target = choice(app, *target); + state_event(app, states::Event::Target(target)); + } + assert_eq!(app.behavior.pending_owner(), Some(*machine)); +} + +fn assert_typed_transition(app: &App, source: Id, target: Id, work: Id) { + assert_eq!(app.project().relations.len(), 1); + let relation = app.project().relations.values().next().unwrap(); + assert_eq!(relation.name, "Wait for recovery"); + let RelationBody::StateTransition { + source: actual_source, + target: actual_target, + triggers, + effects, + trigger, + guard, + effect, + .. + } = &relation.body + else { + panic!("Expected the prepared state transition"); + }; + assert_eq!((*actual_source, *actual_target), (source, target)); + assert_eq!(effects, &[work]); + assert_eq!(trigger, "A monitor result"); + assert_eq!(guard, "connection unavailable"); + assert_eq!(effect, "Record the timeout"); + assert_eq!(triggers.len(), 2); + assert_eq!(triggers[0], work); + assert!(matches!( + app.project().elements[&triggers[1]].shared(), + Some(SharedBody::Event { + body: EventBody::Time { kind: TimeEventKind::At, expression } + }) if expression == "12:30 UTC" + )); + assert_eq!( + app.project() + .elements + .values() + .filter(|e| e.kind() == Kind::Event) + .count(), + 1, + "Retry must not leave a time event from the rejected transaction" + ); + assert_eq!( + source_project::compile_bundle(app.document.bundle(), LIMITS) + .unwrap() + .decoded + .project, + *app.project() + ); +} + +#[test] +fn source_writeback_failure_keeps_typed_behavior_form_for_repair_and_one_undo() { + let mut fixture = fixture(); + let bundle = fixture.app.document.bundle().clone(); + let mut limits = ProjectLimits::default(); + limits.model.max_source_bytes = bundle.files.values().map(String::len).sum::() + 2000; + fixture.app.document = Document::from_bundle(bundle.clone(), limits).unwrap(); + fixture.app.sync(); + let before = fixture.app.document.snapshot(); + start_transition(&mut fixture, true); + let app = &mut fixture.app; + state_event(app, states::Event::Name("Transition".repeat(1024))); + let prepared = app + .behavior + .prepare_pending(app.project()) + .unwrap() + .unwrap(); + assert!(execute_batch(app.project(), &[prepared]).is_ok()); + + state_event(app, states::Event::Submit); + assert!(Arc::ptr_eq(&before, &app.document.snapshot())); + assert_eq!(app.source_workspace.bundle(), bundle); + assert_eq!(app.document.revision(), 0); + assert!(!app.document.can_undo()); + assert!(app.behavior.has_pending()); + assert!(app.dirty()); + assert!(!app.source_dirty()); + + state_event(app, states::Event::Name("Wait for recovery".into())); + state_event(app, states::Event::Submit); + assert!(!app.behavior.has_pending()); + assert_typed_transition(app, fixture.source, fixture.target, fixture.work); + assert_eq!(app.document.revision(), 1); + assert_eq!(app.source_workspace.bundle(), *app.document.bundle()); + let generated_files: Vec<_> = app + .document + .bundle() + .files + .iter() + .filter(|(path, _)| !bundle.files.contains_key(*path)) + .map(|(path, text)| (path.clone(), text.clone())) + .collect(); + assert!( + !generated_files.is_empty(), + "The time event creates a source file" + ); + send(app, Message::Undo); + assert_eq!(app.document.bundle(), &bundle); + assert_eq!(app.project(), &before.decoded.project); + assert_eq!( + source_workspace::reachable(&app.source_workspace.bundle()).unwrap(), + bundle + ); + assert_eq!( + app.source_workspace.reachable, + bundle.files.keys().cloned().collect::>() + ); + // Undo restores the accepted bundle exactly. Workspace retains clean buffers + // from removed includes so the user may inspect or explicitly close them. + for (path, text) in generated_files { + assert!(!app.source_workspace.reachable.contains(&path)); + let detached = &app.source_workspace.files[&path]; + assert_eq!(detached.text, text); + assert!(!detached.dirty()); + } + assert!(!app.source_workspace.detached_dirty()); + assert!(!app.document.can_undo()); + assert!(!app.dirty()); +} + +#[test] +fn incomplete_behavior_form_is_dirty_close_save_retains_it_and_cancel_allows_switching() { + let mut fixture = fixture(); + let before = fixture.app.document.snapshot(); + start_transition(&mut fixture, false); + let app = &mut fixture.app; + assert!(!app.document.dirty()); + assert!(app.dirty()); + assert!(app.title().contains(" *")); + let window = window::Id::unique(); + send(app, Message::Request(Pending::Close(window))); + assert!(matches!(app.pending, Some(Pending::Close(id)) if id == window)); + send(app, Message::SaveAndContinue); + assert!(app.behavior.has_pending()); + assert!(Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(app.pending.is_none()); + assert!(!app.continue_after_save); + assert!(!app.busy); + assert!(app.file_prompt.is_none()); + assert_eq!(app.page, Page::Behavior); + + state_event(app, states::Event::Cancel); + assert!(!app.behavior.has_pending()); + assert!(!app.dirty()); + send(app, Message::Page(Page::Text)); + assert_eq!(app.page, Page::Text); + send(app, Message::Select(fixture.target)); + assert_eq!(app.selected, fixture.target); +} + +#[test] +fn save_prepares_behavior_before_opening_destination_and_cancel_keeps_one_undo() { + let mut fixture = fixture(); + let before = fixture.app.document.snapshot(); + start_transition(&mut fixture, true); + let app = &mut fixture.app; + send(app, Message::Save(true)); + assert!(!app.behavior.has_pending()); + assert_typed_transition(app, fixture.source, fixture.target, fixture.work); + assert_eq!(app.document.revision(), 1); + assert!( + app.document.dirty(), + "Choosing a destination is not a successful disk save" + ); + assert!(app.file_prompt.is_some() || app.busy); + if app.file_prompt.is_some() { + send(app, Message::CancelFile); + } else { + send(app, Message::SavePath(None)); + } + assert!(!app.busy); + assert!(app.document.dirty()); + send(app, Message::Undo); + assert_eq!(app.document.bundle(), &before.bundle); + assert_eq!(app.project(), &before.decoded.project); + assert!(!app.document.can_undo()); + assert!(!app.dirty()); +} + +#[test] +fn pending_behavior_blocks_navigation_property_edits_and_model_history() { + let mut fixture = fixture(); + let before = fixture.app.document.snapshot(); + start_transition(&mut fixture, true); + let app = &mut fixture.app; + let properties = app.draft.as_ref().unwrap().materialize().unwrap(); + for message in [ + Message::Page(Page::Diagram), + Message::Page(Page::Catalog), + Message::Page(Page::Text), + Message::Page(Page::Guide), + Message::Layer(Layer::Oa), + Message::Select(fixture.target), + Message::Explore(Some(fixture.target)), + Message::BrowseGroup(crate::explorer::Group::Work), + Message::Field("/name".into(), "Overwrite properties".into()), + Message::Command(Command::Rename(fixture.source, "Overwrite model".into())), + Message::ConfirmDelete, + Message::Undo, + Message::Redo, + Message::ShortcutUndo(false), + Message::SourceForSelection, + Message::SourceNavigation(crate::source_navigation::Event::Open(false)), + ] { + send(app, message); + assert_eq!(app.page, Page::Behavior); + assert_eq!(app.selected, fixture.source); + assert_eq!(app.layer, Layer::Sa); + assert_eq!(app.behavior.pending_owner(), Some(fixture.machine)); + assert!(Arc::ptr_eq(&before, &app.document.snapshot())); + assert_eq!( + app.draft.as_ref().unwrap().materialize().unwrap(), + properties + ); + assert!(!app.source_dirty()); + } +} + +#[test] +fn passive_behavior_canvas_events_do_not_apply_property_drafts() { + let mut fixture = fixture(); + let app = &mut fixture.app; + let before = app.document.snapshot(); + send( + app, + Message::Field("/name".into(), "Unapplied state name".into()), + ); + assert!(app.draft_dirty()); + for event in [ + behavior::Event::States(states::Event::Viewport(iced::Size::new(900., 600.))), + behavior::Event::States(states::Event::Zoom(0.7)), + behavior::Event::States(states::Event::Fit), + behavior::Event::Topology(behavior::topology::Event::Viewport(iced::Size::new( + 900., 600., + ))), + behavior::Event::Topology(behavior::topology::Event::Zoom(0.7)), + behavior::Event::Topology(behavior::topology::Event::Fit), + ] { + send(app, Message::Behavior(event)); + assert!(Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(app.draft_dirty()); + assert!(!app.behavior.has_pending()); + assert_eq!(app.source_workspace.bundle(), before.bundle); + assert!(!app.document.can_undo()); + } +} + +#[test] +fn pending_behavior_blocks_source_creation_and_source_history_replay() { + for redo in [false, true] { + let mut fixture = fixture(); + let app = &mut fixture.app; + let baseline = app.source_workspace.bundle(); + let edited = format!( + "{}\n// Source draft that must stay in history.\n", + app.source_workspace.text() + ); + send(app, Message::SourceEdit(text_editor::Action::SelectAll)); + send( + app, + Message::SourceEdit(text_editor::Action::Edit(text_editor::Edit::Paste( + Arc::new(edited), + ))), + ); + send( + app, + if redo { + Message::SourceUndo + } else { + Message::RevertSource + }, + ); + assert!(!app.source_dirty()); + start_transition(&mut fixture, true); + let app = &mut fixture.app; + send( + app, + if redo { + Message::SourceRedo + } else { + Message::SourceUndo + }, + ); + assert_eq!( + app.source_workspace.bundle(), + baseline, + "Source history must not create a competing draft" + ); + send(app, Message::SourceNewPath("queued.arcadia".into())); + send(app, Message::SourceCreateFile); + assert_eq!( + app.source_workspace.bundle(), + baseline, + "A queued create-file action must not create a competing draft" + ); + assert!(!app.source_dirty()); + assert_eq!(app.behavior.pending_owner(), Some(fixture.machine)); + state_event(app, states::Event::Submit); + assert!(!app.behavior.has_pending()); + assert_typed_transition(app, fixture.source, fixture.target, fixture.work); + } +} diff --git a/crates/oa-desktop/src/behavior.rs b/crates/oa-desktop/src/behavior.rs index 4a6cb8a3..91d67d0b 100644 --- a/crates/oa-desktop/src/behavior.rs +++ b/crates/oa-desktop/src/behavior.rs @@ -14,10 +14,10 @@ use iced::{ }; use oa_core::*; use std::collections::BTreeMap; -mod scenario; +pub(crate) mod scenario; mod state_scene; -mod states; -mod topology; +pub(crate) mod states; +pub(crate) mod topology; #[derive(Debug, Clone)] pub enum Event { Topology(topology::Event), @@ -26,6 +26,13 @@ pub enum Event { Submit, } impl Event { + pub fn starts_form(&self) -> bool { + matches!(self, + Self::Topology(topology::Event::Start(..)) + | Self::Scenario(scenario::Event::Start(..)) + | Self::States(states::Event::Start(..) | states::Event::Edit(..)) + ) + } pub fn is_submit(&self) -> bool { matches!( self, @@ -123,7 +130,7 @@ pub fn view(app: &App) -> Element<'_, Message> { label: format!("{} ยท {}", e.kind(), e.name), }) .collect::>(); - let owner = selected_behavior(p, app.selected); + let owner = app.behavior.pending_owner().or_else(|| selected_behavior(p, app.selected)); let selected = owner .and_then(|id| choices.iter().find(|c| c.id == id)) .or(choices.first())