diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs index fa4a788..d3024ec 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs @@ -1,6 +1,7 @@ using Avalonia; using Avalonia.Controls; using Avalonia.Input.Platform; +using Avalonia.Threading; using DemaConsulting.SysML2Workbench.LoggingSubsystem; namespace DemaConsulting.SysML2Workbench.AppShellSubsystem; @@ -37,17 +38,30 @@ public AvaloniaClipboardService(Visual anchor, RollingFileLogger? logger = null) } /// + /// + /// The actual clipboard write is always marshaled onto via + /// , even when this method + /// is itself already called from the UI thread: the native OS clipboard (Win32 OpenClipboard in + /// particular) can only be accessed from the single UI/STA thread, and a caller further up the awaited + /// chain may have resumed on a thread-pool thread (for example after a ConfigureAwait(false) + /// elsewhere in the call chain) without this method's own caller being aware of it. Marshaling + /// unconditionally here is the only way this unit can guarantee correctness regardless of the calling + /// thread. + /// public async Task SetTextAsync(string text) { ArgumentNullException.ThrowIfNull(text); - var clipboard = TopLevel.GetTopLevel(_anchor)?.Clipboard; - if (clipboard is null) + await Dispatcher.UIThread.InvokeAsync(async () => { - _logger?.Log(LogLevel.Error, "Unable to copy to the clipboard: no Avalonia TopLevel/clipboard is available."); - return; - } + var clipboard = TopLevel.GetTopLevel(_anchor)?.Clipboard; + if (clipboard is null) + { + _logger?.Log(LogLevel.Error, "Unable to copy to the clipboard: no Avalonia TopLevel/clipboard is available."); + return; + } - await clipboard.SetTextAsync(text); + await clipboard.SetTextAsync(text); + }); } } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs index 8246e47..8b7b690 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs @@ -55,7 +55,13 @@ private void OnDataContextChanged(object? sender, EventArgs e) if (_viewModel is not null) { _viewModel.DiagramChanged += OnDiagramChanged; - _viewModel.ClipboardService ??= new AvaloniaClipboardService(this); + + // Always rebind to this view instance rather than only the first ever attached (`??=`): Dock can + // recreate/reattach a tab's DiagramDocumentView while its DiagramDocumentViewModel persists (for + // example when two custom-preview tabs are created in quick succession), which would otherwise leave + // the clipboard service anchored to a now-detached, stale view - causing TopLevel.GetTopLevel to + // resolve null and the clipboard write to silently no-op. + _viewModel.ClipboardService = new AvaloniaClipboardService(this); LoadCurrentDiagram(); } } diff --git a/test/OtsSoftwareTests/AvaloniaTests.cs b/test/OtsSoftwareTests/AvaloniaTests.cs index 6c87b1e..35e7fd8 100644 --- a/test/OtsSoftwareTests/AvaloniaTests.cs +++ b/test/OtsSoftwareTests/AvaloniaTests.cs @@ -1,3 +1,4 @@ +using System.Reflection; using Avalonia; using Avalonia.Controls; using Avalonia.Headless.XUnit; @@ -181,6 +182,133 @@ await File.WriteAllTextAsync( window.Close(); } + /// + /// Regression test for the "Copy as SysML always returns the last-created custom view" bug: opening the + /// View Builder's real "OK" commit sequence twice ( + /// immediately followed by , exactly as + /// ViewBuilderDialogViewModel.TryCommit does) creates two distinct, real + /// tabs hosted by the real Dock-composed . + /// Selecting the first tab and copying must yield the first view's snippet, not the second (previously + /// failing because DiagramDocumentView.OnDataContextChanged bound the clipboard service only once + /// per view model via ??=, so re-selecting a tab whose view had been detached and later + /// re-attached left its clipboard write silently targeting a stale, detached anchor). + /// + [AvaloniaFact] + public async Task DiagramContextMenu_CopyAsSysml_TwoCustomViewTabs_EachCopiesItsOwnSnippet() + { + // Arrange + await File.WriteAllTextAsync( + Path.Combine(_tempRoot, "Sample.sysml"), + "package Sample {\n" + + " part def Engine;\n" + + " part def Gearbox;\n" + + "}\n"); + using var shell = CreateShell(); + var window = new MainWindowView(shell); + window.Show(); + Dispatcher.UIThread.RunJobs(); + + await shell.AddFolderSourceAsync(_tempRoot); + Dispatcher.UIThread.RunJobs(); + + // Act: commit the first custom view (General diagram over Engine), mirroring ViewBuilderDialog's real + // OK-button sequence. + var firstDefinition = new ViewDefinitionModel(); + firstDefinition.SetViewKind(ViewKind.General); + firstDefinition.AddExposeTarget("Sample::Engine"); + shell.OpenNewCustomPreviewTab(); + shell.PreviewCustomView(firstDefinition); + Dispatcher.UIThread.RunJobs(); + var firstTabId = shell.ActiveTabId!; + var expectedFirstSnippet = shell.ExportTabAsSysmlSnippet(firstTabId); + Assert.NotNull(expectedFirstSnippet); + + // Act: commit a second, distinct custom view (Interconnection diagram over Gearbox), which opens a + // second real tab/DiagramDocumentView rather than re-rendering the first in place. + var secondDefinition = new ViewDefinitionModel(); + secondDefinition.SetViewKind(ViewKind.Interconnection); + secondDefinition.AddExposeTarget("Sample::Gearbox"); + shell.OpenNewCustomPreviewTab(); + shell.PreviewCustomView(secondDefinition); + Dispatcher.UIThread.RunJobs(); + var secondTabId = shell.ActiveTabId!; + var expectedSecondSnippet = shell.ExportTabAsSysmlSnippet(secondTabId); + Assert.NotNull(expectedSecondSnippet); + Assert.NotEqual(firstTabId, secondTabId); + Assert.NotEqual(expectedFirstSnippet, expectedSecondSnippet); + + // Act: switch back to the first tab and copy - this is the step that previously returned the second + // (last-created) tab's snippet regardless of which tab was actually active. Uses the real Dock factory's + // SetActiveDockable/SetFocusedDockable, exactly as Dock's own tab-header click handler does, so this + // faithfully reproduces a user clicking back to the first tab rather than merely poking shell state. + SelectDiagramTab(window, firstTabId); + Dispatcher.UIThread.RunJobs(); + var firstCopiedSnippet = await CopyActiveTabAsSysmlAsync(window); + + // Assert: the first tab's own snippet was copied, not the second tab's. + Assert.Equal(expectedFirstSnippet, firstCopiedSnippet); + + // Act: switch to the second tab and copy too, confirming it still copies its own snippet correctly. + SelectDiagramTab(window, secondTabId); + Dispatcher.UIThread.RunJobs(); + var secondCopiedSnippet = await CopyActiveTabAsSysmlAsync(window); + + // Assert + Assert.Equal(expectedSecondSnippet, secondCopiedSnippet); + + window.Close(); + } + + /// + /// Selects the diagram document tab with the given tab id via the real Dock factory's + /// SetActiveDockable/SetFocusedDockable - exactly what Dock's own tab-header click handler + /// invokes - so the resulting notification (raised + /// through 's FocusedDockableChanged forwarding) faithfully reflects a + /// real user tab click rather than only updating shell-side bookkeeping. Reached via reflection since + /// 's Dock factory and per-tab view-model tracking are private implementation + /// details not otherwise exposed to tests. + /// + private static void SelectDiagramTab(MainWindowView window, string tabId) + { + var windowType = typeof(MainWindowView); + var dockFactoryField = windowType.GetField("_dockFactory", BindingFlags.NonPublic | BindingFlags.Instance); + var tabViewModelsField = windowType.GetField("_tabViewModelsByTabId", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(dockFactoryField); + Assert.NotNull(tabViewModelsField); + + var dockFactory = (WorkbenchDockFactory)dockFactoryField!.GetValue(window)!; + var tabViewModels = (Dictionary)tabViewModelsField!.GetValue(window)!; + var tabViewModel = tabViewModels[tabId]; + + dockFactory.SetActiveDockable(tabViewModel); + dockFactory.SetFocusedDockable(dockFactory.DiagramDock, tabViewModel); + } + + /// + /// Drives the real "Copy as SysML" context-menu MenuItem for whichever + /// is currently hosted by , and returns whatever text ends up on the headless + /// platform's real clipboard afterward. + /// + private static async Task CopyActiveTabAsSysmlAsync(MainWindowView window) + { + var diagramBorder = FindByName(window, "DiagramBorder"); + Assert.NotNull(diagramBorder); + var contextMenu = diagramBorder!.ContextMenu; + Assert.NotNull(contextMenu); + contextMenu!.Open(diagramBorder); + Dispatcher.UIThread.RunJobs(); + + var copyMenuItem = FindByName(window, "CopyAsSysmlMenuItem"); + Assert.NotNull(copyMenuItem); + Assert.True(copyMenuItem!.IsEnabled); + copyMenuItem.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + Dispatcher.UIThread.RunJobs(); + + var clipboard = TopLevel.GetTopLevel(window)?.Clipboard; + Assert.NotNull(clipboard); + return await clipboard!.TryGetTextAsync(); + } + /// /// Finds a named control anywhere in 's visual tree, regardless of which /// Dock-hosted UserControl's own compiled NameScope it belongs to (unlike a control's own