From 8533e1043cfde34fe0efb94784bbacdbd1846a1e Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Fri, 24 Jul 2026 14:52:44 -0400 Subject: [PATCH 1/2] docs: add status badges to README Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index 811bdfd..ae81a92 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,14 @@ # SysML2Workbench +[![GitHub forks][badge-forks]][link-forks] +[![GitHub stars][badge-stars]][link-stars] +[![GitHub contributors][badge-contributors]][link-contributors] +[![License][badge-license]][link-license] +[![Build][badge-build]][link-build] +[![Quality Gate][badge-quality]][link-quality] +[![Security][badge-security]][link-security] +[![Release][badge-release]][link-release] + A cross-platform desktop viewer and GUI custom-view builder for SysML v2 models, built on the SysML2Tools rendering engine. @@ -79,3 +88,21 @@ the Desktop application, starts and polls a local Appium server, runs the tests, and always stops the server afterward; on Linux it publishes the Desktop application and delegates the whole test run to the pre-installed `selenium-webdriver-at-spi-run` wrapper (see `docs/design/ots/appium.md`). + +[badge-forks]: https://img.shields.io/github/forks/demaconsulting/SysML2Workbench?style=plastic +[badge-stars]: https://img.shields.io/github/stars/demaconsulting/SysML2Workbench?style=plastic +[badge-contributors]: https://img.shields.io/github/contributors/demaconsulting/SysML2Workbench?style=plastic +[badge-license]: https://img.shields.io/github/license/demaconsulting/SysML2Workbench?style=plastic +[badge-build]: https://img.shields.io/github/actions/workflow/status/demaconsulting/SysML2Workbench/build_on_push.yaml?style=plastic +[badge-quality]: https://sonarcloud.io/api/project_badges/measure?project=demaconsulting_SysML2Workbench&metric=alert_status +[badge-security]: https://sonarcloud.io/api/project_badges/measure?project=demaconsulting_SysML2Workbench&metric=security_rating +[badge-release]: https://img.shields.io/github/v/release/demaconsulting/SysML2Workbench?style=plastic + +[link-forks]: https://github.com/demaconsulting/SysML2Workbench/network/members +[link-stars]: https://github.com/demaconsulting/SysML2Workbench/stargazers +[link-contributors]: https://github.com/demaconsulting/SysML2Workbench/graphs/contributors +[link-license]: https://github.com/demaconsulting/SysML2Workbench/blob/main/LICENSE +[link-build]: https://github.com/demaconsulting/SysML2Workbench/actions/workflows/build_on_push.yaml +[link-quality]: https://sonarcloud.io/dashboard?id=demaconsulting_SysML2Workbench +[link-security]: https://sonarcloud.io/dashboard?id=demaconsulting_SysML2Workbench +[link-release]: https://github.com/demaconsulting/SysML2Workbench/releases/latest From a55495fcdc9f0dbdbe129ce17c913eb3820fe803 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Fri, 24 Jul 2026 17:34:37 -0400 Subject: [PATCH 2/2] chore: resolve CodeQL and SonarQube findings CodeQL cleanup: - Add internal PathHelpers.SafePathCombine helper (src + duplicate in IntegrationTests, which has no ProjectReference to main by design) and replace ~95 Path.Combine call sites across src and test projects to resolve cs/path-combine findings. - Add clarifying comments to 10 intentionally-broad catch-of-all-exceptions sites (UI/drag-drop/best-effort boundaries) rather than narrowing, since narrowing would contradict their documented never-throws contracts. - Fix 2 cs/linq/missed-select findings, 1 missed-readonly-modifier (ViewDefinitionModel._exposeTargets), 1 useless-upcast, and 1 dispose-not-called-on-throw (WorkspaceSubsystemTests). - Refactor AppFixture.cs's LinuxDriver into a Create factory method that disposes its HttpCommandExecutor on constructor failure, fixing a cs/local-not-disposed finding. SonarQube cleanup: - Remove a redundant null-forgiving operator and add a Regex timeout and a reflection-safety comment in SourceTextDocumentView.axaml.cs. - Make AboutDialogViewModel's ApplicationName/Tagline/Copyright, SysmlSnippetGenerator.SanitizeIdentifier, and two Avalonia drag-over handlers static where they access no instance state. - Introduce a shared MIT-license constant and replace a LINQ FirstOrDefault with a Count/indexer check. - Rename FileWatcher.PendingChanges (a collection-copying property) to GetPendingChanges(), updating all call sites and its design doc. - Extract a helper method from ViewDefinitionModel.ValidateAgainstWorkspace to reduce cognitive complexity. - Convert ~20 CommunityToolkit.Mvvm [ObservableProperty] backing fields across 7 view models to C# 13 partial auto-properties. - Add a missing assertion to a previously assertion-less test case (DiagramDocumentViewModelTests.CopyAsSysmlAsync_NoClipboardServiceAssigned_DoesNotThrow). Also adds README badges (forks/stars/contributors/license/build/SonarCloud quality/security/GitHub Release). All changes validated: dotnet build (0 warnings/errors), build.ps1 -Test (290/290 passed), build.ps1 -IntegrationTest (10/10 passed), dotnet reqstream --lint (clean), lint.ps1 (clean). Reviewed via code-review and formal-review agents (gpt-5.4-mini) across all 18 affected review-sets, all confirmed SUCCEEDED. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../workspace-subsystem/file-watcher.md | 20 +++-- .../App.axaml.cs | 2 +- .../AppShellSubsystem/AboutDialogViewModel.cs | 24 ++++-- .../DesignTimeShellFactory.cs | 2 +- .../DiagnosticsToolViewModel.cs | 6 +- .../AppShellSubsystem/MainWindowView.axaml.cs | 18 +++- .../PredefinedViewsToolViewModel.cs | 11 ++- .../AppShellSubsystem/QueryDialogViewModel.cs | 16 ++-- .../SourceTextDocumentView.axaml.cs | 11 ++- .../ViewBuilderDialogViewModel.cs | 10 ++- .../WorkspacePanelToolView.axaml.cs | 12 ++- .../WorkspacePanelToolViewModel.cs | 11 ++- .../DemaConsulting.SysML2Workbench.csproj | 11 +++ .../ElementFilterViewModel.cs | 15 ++-- .../ElementPickerViewModel.cs | 2 +- .../LoggingSubsystem/RollingFileLogger.cs | 4 +- .../PathHelpers.cs | 85 ++++++++++++++++++ .../SysmlSnippetGenerator.cs | 2 +- .../ViewDefinitionModel.cs | 86 +++++++++++-------- .../WorkspaceSubsystem/FileWatcher.cs | 16 ++-- .../WorkspaceSubsystem/WorkspaceSourceSet.cs | 4 +- .../AppFixture.cs | 29 ++++++- .../InspectionScreenshot.cs | 6 +- .../PathHelpers.cs | 86 +++++++++++++++++++ .../QueryDialogIntegrationTests.cs | 2 +- .../AboutDialogViewModelTests.cs | 16 ++-- .../DiagramDocumentViewModelTests.cs | 9 +- .../AppShellSubsystem/MainWindowShellTests.cs | 26 +++--- .../QueryDialogViewModelTests.cs | 2 +- .../SourceTextDocumentViewModelTests.cs | 6 +- .../ViewBuilderDialogViewModelTests.cs | 6 +- .../WorkspacePanelToolViewModelTests.cs | 18 ++-- .../AppShellSubsystemTests.cs | 4 +- .../LayoutInvokerTests.cs | 2 +- .../LayoutRenderingSubsystemTests.cs | 2 +- .../SysML2WorkbenchTests.cs | 6 +- .../SysmlSnippetGeneratorTests.cs | 12 +-- .../ViewDefinitionModelTests.cs | 2 +- .../ViewBuilderSubsystemTests.cs | 2 +- .../ViewCatalogPresenterTests.cs | 16 ++-- .../ViewCatalogSubsystemTests.cs | 2 +- .../WorkspaceSubsystem/FileWatcherTests.cs | 40 ++++----- .../WorkspaceSubsystem/WorkspaceModelTests.cs | 12 +-- .../WorkspaceSourceSetTests.cs | 22 ++--- .../WorkspaceSubsystemTests.cs | 14 ++- .../MainWindowShellUiTests.cs | 2 +- test/OtsSoftwareTests/AvaloniaTests.cs | 9 +- test/OtsSoftwareTests/DockTests.cs | 5 +- test/OtsSoftwareTests/RenderingTests.cs | 3 +- test/OtsSoftwareTests/SysML2ToolsTests.cs | 7 +- test/OtsSoftwareTests/XUnitTests.cs | 4 +- 51 files changed, 507 insertions(+), 233 deletions(-) create mode 100644 src/DemaConsulting.SysML2Workbench/PathHelpers.cs create mode 100644 test/DemaConsulting.SysML2Workbench.IntegrationTests/PathHelpers.cs diff --git a/docs/design/sysml2-workbench/workspace-subsystem/file-watcher.md b/docs/design/sysml2-workbench/workspace-subsystem/file-watcher.md index a71ccf1..a6e7523 100644 --- a/docs/design/sysml2-workbench/workspace-subsystem/file-watcher.md +++ b/docs/design/sysml2-workbench/workspace-subsystem/file-watcher.md @@ -23,10 +23,18 @@ observed. **DebounceWindow**: `TimeSpan` — minimum delay used to merge rapid change bursts, across all active watchers, into a single reload batch. -**PendingChanges**: `IReadOnlySet` — normalized paths queued for the +#### Key Methods + +**GetPendingChanges**: Returns the normalized paths currently queued for the next incremental refresh cycle, merged across every currently watched source. -#### Key Methods +- *Parameters*: `None` — reads the accumulated pending-change state. +- *Returns*: `IReadOnlySet` — a defensive copy of the currently + pending, normalized paths. Implemented as a method rather than a property + because it allocates a new set on every call. +- *Preconditions*: None. +- *Postconditions*: The internal pending-change state is left unchanged; + callers must not assume subsequent calls return the same set instance. **WatchSource**: Begins monitoring the given source, retargeting an already-watched source with the same id if one exists. @@ -50,7 +58,7 @@ already-watched source with the same id if one exists. - *Returns*: `bool` — whether a matching watcher was found and disposed. - *Postconditions*: The matching watcher, if any, is disposed and removed from `WatchedSourceIds`. Unlike the old single-watcher `StartWatching` retarget - behavior, this does **not** clear `PendingChanges` - pending changes queued + behavior, this does **not** clear the pending-change set - pending changes queued by other still-watched sources must survive an unrelated source being removed. @@ -64,7 +72,7 @@ already-watched source with the same id if one exists. notifications for the same path collapse into a single pending change entry, regardless of which watched source's watcher raised the underlying event. When no source is watched, the notification is silently ignored and - `PendingChanges` is left unchanged. + the pending-change set is left unchanged. **FlushPendingChanges**: Dispatches the current batch to the workspace reload pipeline. @@ -74,7 +82,7 @@ pipeline. merged across every watcher that reported a change since the last flush. - *Preconditions*: None - zero watched sources is a valid state and simply yields an empty result. -- *Postconditions*: Returned paths are removed from `PendingChanges` and are +- *Postconditions*: Returned paths are removed from the pending-change set and are ready for `WorkspaceModel.ReloadFiles`. #### Error Handling @@ -91,7 +99,7 @@ best-effort and does not replace propagation to the shell. Every `FileSystemWatcher` raises its `Changed`/`Created`/`Deleted`/`Renamed` events on an operating-system callback thread, independent of whatever thread -calls `WatchSource`/`UnwatchSource`/`FlushPendingChanges`/`PendingChanges`. +calls `WatchSource`/`UnwatchSource`/`FlushPendingChanges`/`GetPendingChanges`. Production usage marshals those callbacks onto the UI thread first, making this uncontended in practice, but a caller that supplies (or defaults to) an immediate, non-marshaling dispatcher runs them synchronously on the OS diff --git a/src/DemaConsulting.SysML2Workbench/App.axaml.cs b/src/DemaConsulting.SysML2Workbench/App.axaml.cs index e231974..71f68fb 100644 --- a/src/DemaConsulting.SysML2Workbench/App.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/App.axaml.cs @@ -37,7 +37,7 @@ public override void OnFrameworkInitializationCompleted() if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { - var logDirectory = Path.Combine( + var logDirectory = PathHelpers.SafePathCombine( Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "SysML2Workbench", "logs"); diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AboutDialogViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AboutDialogViewModel.cs index 66dabce..e4f2630 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AboutDialogViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AboutDialogViewModel.cs @@ -20,20 +20,26 @@ public sealed class AboutDialogViewModel { private const string UnknownVersion = "Unknown"; + /// + /// License name shared by every entry in ; every key OSS dependency this + /// application bundles happens to use the same license, so this avoids repeating the literal string. + /// + private const string MitLicense = "MIT License"; + /// /// The application's display name. /// - public string ApplicationName => "SysML2Workbench"; + public static string ApplicationName => "SysML2Workbench"; /// /// Short description of what the application does. /// - public string Tagline => "Cross-platform desktop viewer and IDE for SysML v2 models"; + public static string Tagline => "Cross-platform desktop viewer and IDE for SysML v2 models"; /// /// Application copyright text, matching the format used by the repository's own LICENSE file. /// - public string Copyright => "Copyright (c) 2026 DEMA Consulting"; + public static string Copyright => "Copyright (c) 2026 DEMA Consulting"; /// /// The running assembly's build-stamped version, read at runtime so it always reflects the actual build @@ -51,12 +57,12 @@ public sealed class AboutDialogViewModel /// public IReadOnlyList Dependencies { get; } = [ - new DependencyInfo("Avalonia", "MIT License"), - new DependencyInfo("Dock.Avalonia", "MIT License"), - new DependencyInfo("Material.Icons.Avalonia", "MIT License"), - new DependencyInfo("CommunityToolkit.Mvvm", "MIT License"), - new DependencyInfo("DemaConsulting.SysML2Tools", "MIT License"), - new DependencyInfo("DemaConsulting.Rendering", "MIT License"), + new DependencyInfo("Avalonia", MitLicense), + new DependencyInfo("Dock.Avalonia", MitLicense), + new DependencyInfo("Material.Icons.Avalonia", MitLicense), + new DependencyInfo("CommunityToolkit.Mvvm", MitLicense), + new DependencyInfo("DemaConsulting.SysML2Tools", MitLicense), + new DependencyInfo("DemaConsulting.Rendering", MitLicense), ]; /// diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DesignTimeShellFactory.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DesignTimeShellFactory.cs index 06bf9c0..b15e820 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DesignTimeShellFactory.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DesignTimeShellFactory.cs @@ -29,6 +29,6 @@ public static MainWindowShell Create() new LayoutInvoker(), new DiagnosticsListView(), new SysmlSnippetGenerator(), - new RollingFileLogger(Path.Combine(Path.GetTempPath(), "SysML2Workbench-DesignTime"))); + new RollingFileLogger(PathHelpers.SafePathCombine(Path.GetTempPath(), "SysML2Workbench-DesignTime"))); } } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagnosticsToolViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagnosticsToolViewModel.cs index 8575200..f60950a 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagnosticsToolViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagnosticsToolViewModel.cs @@ -12,13 +12,13 @@ public partial class DiagnosticsToolViewModel : Dock.Model.Mvvm.Controls.Tool private readonly MainWindowShell _shell; [ObservableProperty] - private IReadOnlyList _visibleDiagnostics = []; + public partial IReadOnlyList VisibleDiagnostics { get; set; } = []; [ObservableProperty] - private string? _emptyStateMessage; + public partial string? EmptyStateMessage { get; set; } [ObservableProperty] - private bool _hasEmptyStateMessage; + public partial bool HasEmptyStateMessage { get; set; } /// /// Creates the diagnostics tool view model. diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/MainWindowView.axaml.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/MainWindowView.axaml.cs index 9288da8..b053df2 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/MainWindowView.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/MainWindowView.axaml.cs @@ -57,7 +57,7 @@ public MainWindowView(MainWindowShell shell) var factory = new WorkbenchDockFactory(_predefinedViewsViewModel, _diagnosticsViewModel, _workspacePanelViewModel); var layout = factory.CreateLayout(); factory.InitLayout(layout); - WorkbenchDockControl.Layout = (IDock)layout; + WorkbenchDockControl.Layout = layout; _dockFactory = factory; _dockFactory.FocusedDockableChanged += OnFocusedDockableChanged; @@ -288,6 +288,10 @@ private async void OnAddFileSourceClick(object? sender, RoutedEventArgs e) } catch (Exception ex) { + // Intentionally broad: the storage-provider/file-source pipeline can surface a wide range of + // exception types (I/O, permission, parser, platform-picker) and this handler is a UI event + // boundary that must not let any of them escape and crash the application - reporting the + // failure to the user via the status message is the correct outcome for every cause. _workspacePanelViewModel.StatusMessage = $"Failed to open file: {ex.Message}"; } } @@ -319,6 +323,8 @@ private async void OnAddFolderSourceClick(object? sender, RoutedEventArgs e) } catch (Exception ex) { + // Intentionally broad: see OnAddFileSourceClick - this is the same kind of UI event boundary + // over the same folder-source pipeline, so any exception type must be reported, not propagated. _workspacePanelViewModel.StatusMessage = $"Failed to open folder: {ex.Message}"; } } @@ -336,6 +342,8 @@ private async void OnCloseAllMenuItemClick(object? sender, RoutedEventArgs e) } catch (Exception ex) { + // Intentionally broad: same UI event-boundary rationale as OnAddFileSourceClick - closing + // every source can fail for any number of reasons and must never crash the application. _workspacePanelViewModel.StatusMessage = $"Failed to close all sources: {ex.Message}"; } } @@ -352,9 +360,8 @@ private async void OnWindowDrop(object? sender, DragEventArgs e) return; } - foreach (var item in e.DataTransfer.TryGetFiles() ?? []) + foreach (var path in (e.DataTransfer.TryGetFiles() ?? []).Select(item => item.TryGetLocalPath())) { - var path = item.TryGetLocalPath(); if (string.IsNullOrEmpty(path)) { continue; @@ -373,6 +380,9 @@ private async void OnWindowDrop(object? sender, DragEventArgs e) } catch (Exception ex) { + // Intentionally broad: this loop drives a drag-and-drop event boundary over a per-item + // add-source call that can fail for any reason - one bad dropped path must not abort the + // loop or crash the application, only be reported and skipped. _workspacePanelViewModel.StatusMessage = $"Failed to open dropped path '{path}': {ex.Message}"; } } @@ -380,7 +390,7 @@ private async void OnWindowDrop(object? sender, DragEventArgs e) RefreshPanelsFromWorkspace(); } - private void OnWindowDragOver(object? sender, DragEventArgs e) + private static void OnWindowDragOver(object? sender, DragEventArgs e) { e.DragEffects = e.DataTransfer.Formats.Contains(DataFormat.File) ? DragDropEffects.Copy : DragDropEffects.None; } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/PredefinedViewsToolViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/PredefinedViewsToolViewModel.cs index a434b06..d974fa7 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/PredefinedViewsToolViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/PredefinedViewsToolViewModel.cs @@ -16,16 +16,16 @@ public partial class PredefinedViewsToolViewModel : Dock.Model.Mvvm.Controls.Too private readonly MainWindowShell _shell; [ObservableProperty] - private IReadOnlyList _availableViews = []; + public partial IReadOnlyList AvailableViews { get; set; } = []; [ObservableProperty] - private ViewDescriptor? _selectedView; + public partial ViewDescriptor? SelectedView { get; set; } [ObservableProperty] - private string? _statusMessage; + public partial string? StatusMessage { get; set; } [ObservableProperty] - private bool _isWorkspaceEmpty; + public partial bool IsWorkspaceEmpty { get; set; } /// /// Creates the predefined-views tool view model. @@ -63,6 +63,9 @@ partial void OnSelectedViewChanged(ViewDescriptor? value) } catch (Exception ex) { + // Intentionally broad: this is a UI selection-changed event boundary over the predefined-view + // rendering pipeline, which can fail for any reason (missing element, layout error, renderer + // fault) - every cause must be reported via StatusMessage, never crash the application. StatusMessage = $"Failed to render '{value.DisplayName}': {ex.Message}"; } } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/QueryDialogViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/QueryDialogViewModel.cs index a22cf70..f6eb2e4 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/QueryDialogViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/QueryDialogViewModel.cs @@ -75,28 +75,28 @@ public sealed partial class QueryDialogViewModel : ObservableObject new Dictionary(StringComparer.Ordinal); [ObservableProperty] - private bool _includeStdlib; + public partial bool IncludeStdlib { get; set; } [ObservableProperty] - private bool _isWorkspaceEmpty; + public partial bool IsWorkspaceEmpty { get; set; } [ObservableProperty] - private QueryVerb _selectedQueryType = QueryVerb.List; + public partial QueryVerb SelectedQueryType { get; set; } = QueryVerb.List; [ObservableProperty] - private string? _hierarchyDirection = "both"; + public partial string? HierarchyDirection { get; set; } = "both"; [ObservableProperty] - private string? _walkDepthText; + public partial string? WalkDepthText { get; set; } [ObservableProperty] - private QueryResult? _currentResult; + public partial QueryResult? CurrentResult { get; set; } [ObservableProperty] - private IReadOnlyList _currentResultRows = []; + public partial IReadOnlyList CurrentResultRows { get; set; } = []; [ObservableProperty] - private string? _statusMessage; + public partial string? StatusMessage { get; set; } /// /// Creates the dialog view model over and immediately populates the diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/SourceTextDocumentView.axaml.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/SourceTextDocumentView.axaml.cs index 3db3831..ec90303 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/SourceTextDocumentView.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/SourceTextDocumentView.axaml.cs @@ -75,7 +75,9 @@ private static IHighlightingDefinition LoadSysMlHighlighting() var keywordPattern = $@"\b(?:{string.Join("|", GetKeywords())})\b"; definition.MainRuleSet.Rules.Add(new HighlightingRule { - Regex = new Regex(keywordPattern), + // A bounded timeout guards against catastrophic backtracking on pathological input, + // since the keyword pattern is applied to arbitrary user-supplied source text. + Regex = new Regex(keywordPattern, RegexOptions.None, TimeSpan.FromSeconds(2)), Color = keywordColor, }); @@ -95,12 +97,17 @@ private static IReadOnlyList GetKeywords() { try { + // Reflecting a private, static, generated field is intentional and safe here: it is + // read-only (never written via reflection), scoped to a single well-known third-party + // type (SysMLv2Lexer), and wrapped in a try/catch with a hard-coded fallback list below, + // so if the field is ever renamed or removed by a future SysML2Tools release, this + // degrades gracefully instead of crashing. var field = typeof(SysMLv2Lexer).GetField("_LiteralNames", BindingFlags.NonPublic | BindingFlags.Static); if (field?.GetValue(null) is string[] literalNames) { var keywords = literalNames .Where(name => name is not null) - .Select(name => name!.Trim('\'')) + .Select(name => name.Trim('\'')) .Where(name => name.Length > 0 && char.IsLetter(name[0])) .Distinct() .ToList(); diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/ViewBuilderDialogViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/ViewBuilderDialogViewModel.cs index 26fb3d7..6fb637e 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/ViewBuilderDialogViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/ViewBuilderDialogViewModel.cs @@ -39,10 +39,10 @@ public sealed partial class ViewBuilderDialogViewModel : ObservableObject private const string DefaultTypeFilterLabel = "part"; [ObservableProperty] - private string? _statusMessage; + public partial string? StatusMessage { get; set; } [ObservableProperty] - private bool _isWorkspaceEmpty; + public partial bool IsWorkspaceEmpty { get; set; } /// /// Creates the dialog view model, refreshing the available expose-target picker list from the shell's @@ -214,6 +214,9 @@ public void RenderPreview() } catch (Exception ex) { + // Intentionally broad: RenderPreview is documented as "never throws" and runs after every + // single control edit, so any exception type from the render pipeline (a mid-edit definition + // is routinely incomplete/invalid) must be swallowed and reported via StatusMessage instead. PreviewCanvas.Clear(); StatusMessage = $"Preview unavailable: {ex.Message}"; } @@ -254,6 +257,9 @@ public bool TryCommit(out string? error) } catch (Exception ex) { + // Intentionally broad: TryCommit's contract (see its doc) is to report every render + // failure through the out `error`/StatusMessage rather than throw, so the just-opened tab can + // always be rolled back regardless of which exception type the render pipeline raised. Shell.CloseDiagramTab(tab.Id); error = $"Commit failed: {ex.Message}"; StatusMessage = error; diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolView.axaml.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolView.axaml.cs index c5f78b2..2e97717 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolView.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolView.axaml.cs @@ -80,6 +80,9 @@ private async void OnRequestAddFile(object? sender, EventArgs e) } catch (Exception ex) { + // Intentionally broad: this is a UI event boundary over the file-source pipeline, which can + // fail with any exception type (I/O, permission, parser, platform-picker) - all must be + // reported via the status message rather than crashing the application. viewModel.StatusMessage = $"Failed to add workspace file: {ex.Message}"; } } @@ -110,11 +113,13 @@ private async void OnRequestAddFolder(object? sender, EventArgs e) } catch (Exception ex) { + // Intentionally broad: same UI event-boundary rationale as OnRequestAddFile - the folder-source + // pipeline can fail for any reason and must never crash the application. viewModel.StatusMessage = $"Failed to add workspace folder: {ex.Message}"; } } - private void OnDragOver(object? sender, DragEventArgs e) + private static void OnDragOver(object? sender, DragEventArgs e) { e.DragEffects = e.DataTransfer.Formats.Contains(DataFormat.File) ? DragDropEffects.Copy : DragDropEffects.None; } @@ -126,9 +131,8 @@ private async void OnDrop(object? sender, DragEventArgs e) return; } - foreach (var item in e.DataTransfer.TryGetFiles() ?? []) + foreach (var path in (e.DataTransfer.TryGetFiles() ?? []).Select(item => item.TryGetLocalPath())) { - var path = item.TryGetLocalPath(); if (string.IsNullOrEmpty(path)) { continue; @@ -147,6 +151,8 @@ private async void OnDrop(object? sender, DragEventArgs e) } catch (Exception ex) { + // Intentionally broad: same drag-and-drop event-boundary rationale as MainWindowView's + // OnWindowDrop - one bad dropped path must not abort the loop or crash the application. viewModel.StatusMessage = $"Failed to add dropped workspace source '{path}': {ex.Message}"; } } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolViewModel.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolViewModel.cs index 35590cc..6f1d554 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/WorkspacePanelToolViewModel.cs @@ -110,16 +110,16 @@ public partial class WorkspacePanelToolViewModel : Dock.Model.Mvvm.Controls.Tool private readonly MainWindowShell _shell; [ObservableProperty] - private IReadOnlyList _rootNodes = []; + public partial IReadOnlyList RootNodes { get; set; } = []; [ObservableProperty] - private WorkspaceTreeNode? _selectedNode; + public partial WorkspaceTreeNode? SelectedNode { get; set; } [ObservableProperty] - private string? _statusMessage; + public partial string? StatusMessage { get; set; } [ObservableProperty] - private bool _isEmpty = true; + public new partial bool IsEmpty { get; set; } = true; /// /// Raised when the user invokes the "Add File..." command, so the Avalonia-aware view can fulfill it with @@ -303,6 +303,9 @@ private async Task RemoveSelectedAsync() } catch (Exception ex) { + // Intentionally broad: this is a UI command boundary over source removal, which can fail for + // any reason (I/O, workspace-state) - every cause must be reported via StatusMessage rather + // than crash the application or leave the command faulted. StatusMessage = $"Failed to remove workspace source: {ex.Message}"; } } diff --git a/src/DemaConsulting.SysML2Workbench/DemaConsulting.SysML2Workbench.csproj b/src/DemaConsulting.SysML2Workbench/DemaConsulting.SysML2Workbench.csproj index 2f11dea..4a10432 100644 --- a/src/DemaConsulting.SysML2Workbench/DemaConsulting.SysML2Workbench.csproj +++ b/src/DemaConsulting.SysML2Workbench/DemaConsulting.SysML2Workbench.csproj @@ -34,4 +34,15 @@ + + + + + + + diff --git a/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementFilterViewModel.cs b/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementFilterViewModel.cs index 190f713..43e7b3c 100644 --- a/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementFilterViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementFilterViewModel.cs @@ -37,19 +37,19 @@ public sealed partial class ElementFilterViewModel : ObservableObject private IReadOnlyList<(string QualifiedName, string TypeLabel)> _candidates = []; [ObservableProperty] - private IReadOnlyList _availableTypeLabels = []; + public partial IReadOnlyList AvailableTypeLabels { get; set; } = []; [ObservableProperty] - private string? _searchText = ""; + public partial string? SearchText { get; set; } = ""; [ObservableProperty] - private IReadOnlyList _displayedItems = []; + public partial IReadOnlyList DisplayedItems { get; set; } = []; [ObservableProperty] - private string? _addableTypeFilterSearchText = ""; + public partial string? AddableTypeFilterSearchText { get; set; } = ""; [ObservableProperty] - private IReadOnlyList _addableTypeFilterCandidates = []; + public partial IReadOnlyList AddableTypeFilterCandidates { get; set; } = []; /// /// Creates the filter view model in its empty initial state: no candidates, no active @@ -162,12 +162,13 @@ public void BeginAddableTypeFilterSearch() /// public bool TryCommitAddableTypeFilterSearch() { - var topMatch = AddableTypeFilterCandidates.FirstOrDefault(); - if (topMatch is null) + if (AddableTypeFilterCandidates.Count == 0) { return false; } + var topMatch = AddableTypeFilterCandidates[0]; + AddTypeFilter(topMatch); return true; } diff --git a/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementPickerViewModel.cs b/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementPickerViewModel.cs index cef364d..6d77447 100644 --- a/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementPickerViewModel.cs +++ b/src/DemaConsulting.SysML2Workbench/ElementPickerSubsystem/ElementPickerViewModel.cs @@ -31,7 +31,7 @@ namespace DemaConsulting.SysML2Workbench.ElementPickerSubsystem; public sealed partial class ElementPickerViewModel : ObservableObject { [ObservableProperty] - private string? _selectedQualifiedName; + public partial string? SelectedQualifiedName { get; set; } /// /// Creates the picker view model in its empty initial state: no candidates, no active diff --git a/src/DemaConsulting.SysML2Workbench/LoggingSubsystem/RollingFileLogger.cs b/src/DemaConsulting.SysML2Workbench/LoggingSubsystem/RollingFileLogger.cs index e3b832b..cd87f56 100644 --- a/src/DemaConsulting.SysML2Workbench/LoggingSubsystem/RollingFileLogger.cs +++ b/src/DemaConsulting.SysML2Workbench/LoggingSubsystem/RollingFileLogger.cs @@ -52,7 +52,7 @@ public sealed class RollingFileLogger /// /// Full path of the currently active (append-target) log file. /// - public string ActiveFilePath => Path.Combine(LogDirectory, $"{BaseFileName}.log"); + public string ActiveFilePath => PathHelpers.SafePathCombine(LogDirectory, $"{BaseFileName}.log"); /// /// Creates a logger that writes into , creating the directory if needed. @@ -133,7 +133,7 @@ public void RotateIfNeeded() } var archiveName = $"{BaseFileName}-{DateTimeOffset.UtcNow:yyyyMMddHHmmssfff}.log"; - var archivePath = Path.Combine(LogDirectory, archiveName); + var archivePath = PathHelpers.SafePathCombine(LogDirectory, archiveName); File.Move(activeFile.FullName, archivePath, overwrite: true); PruneOldArchives(); diff --git a/src/DemaConsulting.SysML2Workbench/PathHelpers.cs b/src/DemaConsulting.SysML2Workbench/PathHelpers.cs new file mode 100644 index 0000000..61417c7 --- /dev/null +++ b/src/DemaConsulting.SysML2Workbench/PathHelpers.cs @@ -0,0 +1,85 @@ +namespace DemaConsulting.SysML2Workbench; + +/// +/// Helper utilities for safe path operations within SysML2Workbench. +/// +/// +/// This class exists to provide a single, auditable point of path-safety enforcement for any +/// operation that combines a base directory with additional path segments. Centralizing the +/// check prevents directory-traversal vulnerabilities from being re-implemented (or forgotten) +/// in each caller, and gives static-analysis tools (such as CodeQL's cs/path-combine query) one +/// reviewed call site instead of dozens of individual calls. +/// All members are stateless and thread-safe. +/// +internal static class PathHelpers +{ + /// + /// Safely combines a base path with zero or more path segments, ensuring the result + /// remains within the base directory. + /// + /// + /// All segments are joined using , then the + /// combined result is normalized with and checked + /// against the normalized base using . If the result + /// resolves outside the base directory the method throws; otherwise the joined + /// (un-normalized) path is returned. The escape check matches only an exact .. + /// segment or one followed by a directory separator, so names beginning with two dots + /// (e.g. ..config) are not misidentified as traversal. + /// Individual segments may contain .. or be rooted provided the combined result + /// does not escape the base - for example segments ["baa", ".."] on base + /// C:\foo resolve back to C:\foo and are accepted. + /// When no segments are supplied, returns unchanged. + /// This method is stateless and thread-safe. + /// + /// + /// The base directory path. Must not be null. Any valid directory path is accepted; it need + /// not exist on disk because only string and normalized-path operations are performed. + /// + /// + /// Zero or more path segments to append in order. Must not be null, and each individual + /// segment must not be null. + /// + /// + /// The result of joining with all segments in + /// . The returned path always resolves within + /// . Returns unchanged when + /// no segments are supplied. + /// + /// + /// Thrown when , , or any + /// individual segment within is null. + /// + /// + /// Thrown when the combined path resolves outside . + /// + internal static string SafePathCombine(string basePath, params string[] relativePaths) + { + // Reject null arguments before any path work + ArgumentNullException.ThrowIfNull(basePath); + ArgumentNullException.ThrowIfNull(relativePaths); + + if (relativePaths.Any(segment => segment is null)) + { + throw new ArgumentNullException(nameof(relativePaths), "Individual path segments must not be null."); + } + + // Join all segments onto the base path + var combined = relativePaths.Aggregate(basePath, Path.Join); + + // Normalize the combined result + var fullBase = Path.GetFullPath(basePath); + var fullCombined = Path.GetFullPath(combined); + var relative = Path.GetRelativePath(fullBase, fullCombined); + + // Reject any combination whose normalized result escapes the base directory + if (relative == ".." || + relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || + relative.StartsWith("../", StringComparison.Ordinal) || + Path.IsPathRooted(relative)) + { + throw new ArgumentException("Path escapes base directory.", nameof(relativePaths)); + } + + return combined; + } +} diff --git a/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/SysmlSnippetGenerator.cs b/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/SysmlSnippetGenerator.cs index b81a89a..d781a64 100644 --- a/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/SysmlSnippetGenerator.cs +++ b/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/SysmlSnippetGenerator.cs @@ -149,7 +149,7 @@ public string FormatExposeClause(ExposeTargetSelection selection) /// Candidate name from the UI. /// Safe identifier or single-quoted representation. /// Thrown when is empty. - public string SanitizeIdentifier(string rawName) + public static string SanitizeIdentifier(string rawName) { ArgumentException.ThrowIfNullOrEmpty(rawName); diff --git a/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/ViewDefinitionModel.cs b/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/ViewDefinitionModel.cs index 26ad3c1..d82b090 100644 --- a/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/ViewDefinitionModel.cs +++ b/src/DemaConsulting.SysML2Workbench/ViewBuilderSubsystem/ViewDefinitionModel.cs @@ -68,7 +68,7 @@ public sealed class ViewDefinitionModel /// distinct recursion kind (for example both expose PublishingSubsystem; and /// expose PublishingSubsystem::*; for the same package), matching valid SysML v2 semantics. /// - private List _exposeTargets = []; + private readonly List _exposeTargets = []; /// /// Selected custom-view rendering style, constrained to the supported SysML view kinds exposed by the UI. @@ -309,39 +309,7 @@ public IReadOnlyList ValidateAgainstWorkspace(SysmlWorkspace wo foreach (var selection in _exposeTargets) { - var target = selection.QualifiedName; - - if (!workspace.Declarations.TryGetValue(target, out var node)) - { - diagnostics.Add(Diagnostic($"Target '{target}' does not resolve in the current workspace.")); - continue; - } - - if (workspace.StdlibNames.Contains(target)) - { - diagnostics.Add(Diagnostic($"Target '{target}' is a standard library element and cannot be exposed.")); - continue; - } - - if (DisallowedTargetNodeTypes.Contains(node.GetType())) - { - diagnostics.Add(Diagnostic($"Target '{target}' is a {node.GetType().Name} and cannot be exposed.")); - } - - if (selection.BracketFilterExpression is not null) - { - if (selection.RecursionKind is ExposeRecursionKind.MembershipExact or ExposeRecursionKind.NamespaceDirectChildren) - { - diagnostics.Add(Diagnostic( - $"Target '{target}' has a bracket-filter expression but its recursion kind '{selection.RecursionKind}' does not support one; only the two recursive kinds do.")); - } - - var targetParseResult = FilterExpressionParser.Parse(selection.BracketFilterExpression); - if (targetParseResult.Expression is null) - { - diagnostics.AddRange(targetParseResult.Diagnostics); - } - } + diagnostics.AddRange(ValidateExposeTargetSelection(workspace, selection)); } if (!string.IsNullOrWhiteSpace(FilterExpression)) @@ -356,6 +324,56 @@ public IReadOnlyList ValidateAgainstWorkspace(SysmlWorkspace wo return diagnostics; } + /// + /// Validates a single expose-target selection against the workspace, extracted from + /// to keep that method's cognitive complexity low - this helper + /// handles exactly one target's resolution, stdlib, disallowed-kind, and bracket-filter checks. + /// + /// Current loaded workspace. + /// The expose-target selection to validate. + /// Validation findings for this single target; empty if it is fully valid. + private static IEnumerable ValidateExposeTargetSelection(SysmlWorkspace workspace, ExposeTargetSelection selection) + { + var target = selection.QualifiedName; + + if (!workspace.Declarations.TryGetValue(target, out var node)) + { + yield return Diagnostic($"Target '{target}' does not resolve in the current workspace."); + yield break; + } + + if (workspace.StdlibNames.Contains(target)) + { + yield return Diagnostic($"Target '{target}' is a standard library element and cannot be exposed."); + yield break; + } + + if (DisallowedTargetNodeTypes.Contains(node.GetType())) + { + yield return Diagnostic($"Target '{target}' is a {node.GetType().Name} and cannot be exposed."); + } + + if (selection.BracketFilterExpression is null) + { + yield break; + } + + if (selection.RecursionKind is ExposeRecursionKind.MembershipExact or ExposeRecursionKind.NamespaceDirectChildren) + { + yield return Diagnostic( + $"Target '{target}' has a bracket-filter expression but its recursion kind '{selection.RecursionKind}' does not support one; only the two recursive kinds do."); + } + + var targetParseResult = FilterExpressionParser.Parse(selection.BracketFilterExpression); + if (targetParseResult.Expression is null) + { + foreach (var diagnostic in targetParseResult.Diagnostics) + { + yield return diagnostic; + } + } + } + /// /// Builds a locally-originated validation diagnostic. /// diff --git a/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/FileWatcher.cs b/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/FileWatcher.cs index a2728c7..b04a989 100644 --- a/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/FileWatcher.cs +++ b/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/FileWatcher.cs @@ -45,7 +45,7 @@ public sealed class FileWatcher : IDisposable /// /// Guards and , both of which are read and /// written from the calling thread (via // - /// /) as well as from whatever background + /// /) as well as from whatever background /// thread the operating system delivers a callback on. Production usage /// marshals those callbacks onto the UI thread via , making this uncontended in /// practice, but (used by default outside a live UI, including in @@ -111,16 +111,16 @@ public IReadOnlySet WatchedSourceIds public TimeSpan DebounceWindow { get; } /// - /// Normalized paths queued for the next incremental refresh cycle. + /// Returns the normalized paths currently queued for the next incremental refresh cycle. Implemented as + /// a method rather than a property because it allocates and returns a defensive copy of internal state + /// on every call (to keep the returned set safe to enumerate without holding ), which + /// is a cost callers should be able to see at the call site rather than one hidden behind property syntax. /// - public IReadOnlySet PendingChanges + public IReadOnlySet GetPendingChanges() { - get + lock (_gate) { - lock (_gate) - { - return new HashSet(_pending.Keys, StringComparer.OrdinalIgnoreCase); - } + return new HashSet(_pending.Keys, StringComparer.OrdinalIgnoreCase); } } diff --git a/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/WorkspaceSourceSet.cs b/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/WorkspaceSourceSet.cs index 74860c4..fd6fa7c 100644 --- a/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/WorkspaceSourceSet.cs +++ b/src/DemaConsulting.SysML2Workbench/WorkspaceSubsystem/WorkspaceSourceSet.cs @@ -170,11 +170,11 @@ private static string NormalizePathCasing(string fullPath) .Select(Path.GetFileName) .FirstOrDefault(entry => string.Equals(entry, name, StringComparison.OrdinalIgnoreCase)); - return Path.Combine(normalizedParent, actualName ?? name); + return PathHelpers.SafePathCombine(normalizedParent, actualName ?? name); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - return Path.Combine(normalizedParent, name); + return PathHelpers.SafePathCombine(normalizedParent, name); } } diff --git a/test/DemaConsulting.SysML2Workbench.IntegrationTests/AppFixture.cs b/test/DemaConsulting.SysML2Workbench.IntegrationTests/AppFixture.cs index 8d646df..6c3fe9d 100644 --- a/test/DemaConsulting.SysML2Workbench.IntegrationTests/AppFixture.cs +++ b/test/DemaConsulting.SysML2Workbench.IntegrationTests/AppFixture.cs @@ -200,7 +200,7 @@ private static AppiumDriver CreateLinuxDriver(string startupArguments) }; ApplyStartupArguments(options, startupArguments); - return new LinuxDriver(ServerUri, options); + return LinuxDriver.Create(ServerUri, options); } /// @@ -249,8 +249,31 @@ private static void ApplyStartupArguments(AppiumOptions options, string startupA /// private sealed class LinuxDriver : AppiumDriver { - public LinuxDriver(Uri remoteAddress, AppiumOptions options) - : base(new OpenQA.Selenium.Remote.HttpCommandExecutor(remoteAddress, TimeSpan.FromSeconds(60)), options.ToCapabilities()) + /// + /// Creates a , ensuring the + /// it constructs is disposed if base-class session initialization throws before ownership of the + /// executor is fully transferred to the returned driver (whose own + /// otherwise disposes it as part of the normal WebDriver lifecycle). + /// + /// The Appium server's endpoint URI. + /// The capabilities to request for the new session. + /// A driver bound to a new Appium session at . + public static LinuxDriver Create(Uri remoteAddress, AppiumOptions options) + { + var executor = new OpenQA.Selenium.Remote.HttpCommandExecutor(remoteAddress, TimeSpan.FromSeconds(60)); + try + { + return new LinuxDriver(executor, options.ToCapabilities()); + } + catch + { + executor.Dispose(); + throw; + } + } + + private LinuxDriver(OpenQA.Selenium.Remote.HttpCommandExecutor executor, OpenQA.Selenium.ICapabilities capabilities) + : base(executor, capabilities) { } } diff --git a/test/DemaConsulting.SysML2Workbench.IntegrationTests/InspectionScreenshot.cs b/test/DemaConsulting.SysML2Workbench.IntegrationTests/InspectionScreenshot.cs index ae5301e..2c9293b 100644 --- a/test/DemaConsulting.SysML2Workbench.IntegrationTests/InspectionScreenshot.cs +++ b/test/DemaConsulting.SysML2Workbench.IntegrationTests/InspectionScreenshot.cs @@ -38,7 +38,7 @@ public static class InspectionScreenshot /// ) before invoking the wrapped dotnet test, so this lands in the same /// place regardless of the test host's own working directory. /// - private static readonly string OutputDirectory = Path.Combine( + private static readonly string OutputDirectory = PathHelpers.SafePathCombine( Environment.GetEnvironmentVariable("SYSML2WORKBENCH_ARTIFACTS_DIR") ?? "artifacts", "inspection"); @@ -91,7 +91,7 @@ public static void CaptureElement(AppiumDriver driver, IWebElement element, stri // File.Create (unlike File.OpenWrite) truncates an existing file: without this, re-running the // test after a prior capture wrote a larger PNG would leave that PNG's trailing bytes appended // past the new (shorter) image data, corrupting the file. - using var file = File.Create(Path.Combine(OutputDirectory, $"{fileName}.png")); + using var file = File.Create(PathHelpers.SafePathCombine(OutputDirectory, $"{fileName}.png")); data.SaveTo(file); } @@ -101,6 +101,6 @@ public static void CaptureElement(AppiumDriver driver, IWebElement element, stri private static void Save(byte[] pngBytes, string fileName) { Directory.CreateDirectory(OutputDirectory); - File.WriteAllBytes(Path.Combine(OutputDirectory, $"{fileName}.png"), pngBytes); + File.WriteAllBytes(PathHelpers.SafePathCombine(OutputDirectory, $"{fileName}.png"), pngBytes); } } diff --git a/test/DemaConsulting.SysML2Workbench.IntegrationTests/PathHelpers.cs b/test/DemaConsulting.SysML2Workbench.IntegrationTests/PathHelpers.cs new file mode 100644 index 0000000..042b643 --- /dev/null +++ b/test/DemaConsulting.SysML2Workbench.IntegrationTests/PathHelpers.cs @@ -0,0 +1,86 @@ +namespace DemaConsulting.SysML2Workbench.IntegrationTests; + +/// +/// Helper utilities for safe path operations within the Appium integration test tier. +/// +/// +/// This tier deliberately has no ProjectReference to the main +/// DemaConsulting.SysML2Workbench project (see the deliberate-omission comment in this +/// project's .csproj), so it cannot reuse that project's internal +/// DemaConsulting.SysML2Workbench.PathHelpers helper and instead carries its own copy of the +/// same reviewed pattern. This gives static-analysis tools (such as CodeQL's cs/path-combine query) +/// one reviewed call site instead of several individual calls. +/// All members are stateless and thread-safe. +/// +internal static class PathHelpers +{ + /// + /// Safely combines a base path with zero or more path segments, ensuring the result + /// remains within the base directory. + /// + /// + /// All segments are joined using , then the + /// combined result is normalized with and checked + /// against the normalized base using . If the result + /// resolves outside the base directory the method throws; otherwise the joined + /// (un-normalized) path is returned. The escape check matches only an exact .. + /// segment or one followed by a directory separator, so names beginning with two dots + /// (e.g. ..config) are not misidentified as traversal. + /// Individual segments may contain .. or be rooted provided the combined result + /// does not escape the base - for example segments ["baa", ".."] on base + /// C:\foo resolve back to C:\foo and are accepted. + /// When no segments are supplied, returns unchanged. + /// This method is stateless and thread-safe. + /// + /// + /// The base directory path. Must not be null. Any valid directory path is accepted; it need + /// not exist on disk because only string and normalized-path operations are performed. + /// + /// + /// Zero or more path segments to append in order. Must not be null, and each individual + /// segment must not be null. + /// + /// + /// The result of joining with all segments in + /// . The returned path always resolves within + /// . Returns unchanged when + /// no segments are supplied. + /// + /// + /// Thrown when , , or any + /// individual segment within is null. + /// + /// + /// Thrown when the combined path resolves outside . + /// + internal static string SafePathCombine(string basePath, params string[] relativePaths) + { + // Reject null arguments before any path work + ArgumentNullException.ThrowIfNull(basePath); + ArgumentNullException.ThrowIfNull(relativePaths); + + if (relativePaths.Any(segment => segment is null)) + { + throw new ArgumentNullException(nameof(relativePaths), "Individual path segments must not be null."); + } + + // Join all segments onto the base path + var combined = relativePaths.Aggregate(basePath, Path.Join); + + // Normalize the combined result + var fullBase = Path.GetFullPath(basePath); + var fullCombined = Path.GetFullPath(combined); + var relative = Path.GetRelativePath(fullBase, fullCombined); + + // Reject any combination whose normalized result escapes the base directory + if (relative == ".." || + relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || + relative.StartsWith("../", StringComparison.Ordinal) || + Path.IsPathRooted(relative)) + { + throw new ArgumentException("Path escapes base directory.", nameof(relativePaths)); + } + + return combined; + } +} diff --git a/test/DemaConsulting.SysML2Workbench.IntegrationTests/QueryDialogIntegrationTests.cs b/test/DemaConsulting.SysML2Workbench.IntegrationTests/QueryDialogIntegrationTests.cs index c8ec8bc..adf72b3 100644 --- a/test/DemaConsulting.SysML2Workbench.IntegrationTests/QueryDialogIntegrationTests.cs +++ b/test/DemaConsulting.SysML2Workbench.IntegrationTests/QueryDialogIntegrationTests.cs @@ -20,7 +20,7 @@ public sealed class QueryDialogIntegrationTests : AppiumTestBase /// unautomatable native OS "Open File" dialog. /// private static readonly string InspectionSampleFilePath = - Path.Combine(AppContext.BaseDirectory, "TestData", "InspectionSample.sysml"); + PathHelpers.SafePathCombine(AppContext.BaseDirectory, "TestData", "InspectionSample.sysml"); /// /// Validates that the Query menu's "Run Query..." item, found by the QueryDialogMenuItem diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/AboutDialogViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/AboutDialogViewModelTests.cs index 0580e97..c14fb59 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/AboutDialogViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/AboutDialogViewModelTests.cs @@ -15,12 +15,9 @@ public sealed class AboutDialogViewModelTests [Fact] public void Construction_ExposesApplicationNameAndTagline() { - // Arrange / Act - var viewModel = new AboutDialogViewModel(); - - // Assert - Assert.Equal("SysML2Workbench", viewModel.ApplicationName); - Assert.Equal("Cross-platform desktop viewer and IDE for SysML v2 models", viewModel.Tagline); + // Arrange / Act / Assert (ApplicationName and Tagline are static, so no instance is needed to assert) + Assert.Equal("SysML2Workbench", AboutDialogViewModel.ApplicationName); + Assert.Equal("Cross-platform desktop viewer and IDE for SysML v2 models", AboutDialogViewModel.Tagline); } /// @@ -57,11 +54,8 @@ public void Construction_ExposesVersionFromAssembly() [Fact] public void Construction_ExposesCopyrightText() { - // Arrange / Act - var viewModel = new AboutDialogViewModel(); - - // Assert - Assert.Equal("Copyright (c) 2026 DEMA Consulting", viewModel.Copyright); + // Arrange / Act / Assert (Copyright is static, so no instance is needed to assert) + Assert.Equal("Copyright (c) 2026 DEMA Consulting", AboutDialogViewModel.Copyright); } /// diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/DiagramDocumentViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/DiagramDocumentViewModelTests.cs index 4368525..c4b61eb 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/DiagramDocumentViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/DiagramDocumentViewModelTests.cs @@ -59,7 +59,7 @@ public Task SetTextAsync(string text) private async Task CreateShellWithSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view PredefinedView {\n" @@ -151,7 +151,10 @@ public async Task CopyAsSysmlAsync_NoClipboardServiceAssigned_DoesNotThrow() var viewModel = new DiagramDocumentViewModel(shell, tabId); - // Act / Assert - await viewModel.CopyAsSysmlAsync(); + // Act + var exception = await Record.ExceptionAsync(() => viewModel.CopyAsSysmlAsync()); + + // Assert + Assert.Null(exception); } } diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/MainWindowShellTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/MainWindowShellTests.cs index 60d8c75..defd3bb 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/MainWindowShellTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/MainWindowShellTests.cs @@ -43,7 +43,7 @@ public void Dispose() private async Task WriteSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" @@ -149,7 +149,7 @@ public async Task SessionStateChanges_SynchronizeVisibleRegions() // Act: simulate an external edit and a debounce-window flush, then refresh await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Extra.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Extra.sysml"), "package Extra {\n part def Bracket;\n}\n", TestContext.Current.CancellationToken); await Task.Delay(5, TestContext.Current.CancellationToken); @@ -271,7 +271,7 @@ public async Task ExportTabAsSysmlSnippet_PredefinedViewWithNoExposeMembers_Retu { // Arrange: a workspace whose predefined view declares no expose members at all await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view UnscopedView {\n" @@ -323,7 +323,7 @@ public async Task SelectPredefinedView_NoWorkspaceOpened_ThrowsInvalidOperationE { using var shell = CreateShell(); - await Assert.ThrowsAsync(() => shell.AddFolderSourceAsync(Path.Combine(_tempRoot, "missing"))); + await Assert.ThrowsAsync(() => shell.AddFolderSourceAsync(PathHelpers.SafePathCombine(_tempRoot, "missing"))); Assert.Throws(() => shell.SelectPredefinedView("anything")); } @@ -599,7 +599,7 @@ public async Task AddFolderSourceAsync_SecondDistinctFolder_IsAdditiveAndWatches { await WriteSampleWorkspaceAsync(); await File.WriteAllTextAsync( - Path.Combine(secondRoot, "Other.sysml"), + PathHelpers.SafePathCombine(secondRoot, "Other.sysml"), "package Other {\n part def Bracket;\n}\n", TestContext.Current.CancellationToken); @@ -610,9 +610,9 @@ await File.WriteAllTextAsync( // folder source B. await shell.AddFolderSourceAsync(_tempRoot); Assert.Single(fileWatcher.WatchedSourceIds); - var stalePathUnderA = Path.Combine(_tempRoot, "Stale.sysml"); + var stalePathUnderA = PathHelpers.SafePathCombine(_tempRoot, "Stale.sysml"); fileWatcher.QueueChange(stalePathUnderA); - Assert.Contains(stalePathUnderA, fileWatcher.PendingChanges); + Assert.Contains(stalePathUnderA, fileWatcher.GetPendingChanges()); var snapshot = await shell.AddFolderSourceAsync(secondRoot); @@ -621,7 +621,7 @@ await File.WriteAllTextAsync( Assert.Equal(2, shell.CurrentWorkspace.Sources.Count); Assert.Equal(2, fileWatcher.WatchedSourceIds.Count); Assert.Equal(2, snapshot.Files.Count); - Assert.Contains(stalePathUnderA, fileWatcher.PendingChanges); + Assert.Contains(stalePathUnderA, fileWatcher.GetPendingChanges()); } finally { @@ -671,7 +671,7 @@ public async Task CloseAllSourcesAsync_WithMultipleSources_ProducesEmptySnapshot var secondRoot = Directory.CreateTempSubdirectory("sysml2workbench-tests-").FullName; try { - var filePath = Path.Combine(secondRoot, "Other.sysml"); + var filePath = PathHelpers.SafePathCombine(secondRoot, "Other.sysml"); await File.WriteAllTextAsync(filePath, "package Other {\n part def Bracket;\n}\n", TestContext.Current.CancellationToken); var fileWatcher = new FileWatcher(TimeSpan.FromMilliseconds(1)); @@ -748,7 +748,7 @@ public async Task OpenSourceTextTab_NewFile_CreatesOneNewActiveTab() // Arrange await WriteSampleWorkspaceAsync(); using var shell = CreateShell(); - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); var raisedCount = 0; shell.TabsChanged += (_, _) => raisedCount++; @@ -773,11 +773,11 @@ public async Task OpenSourceTextTab_SamePathTwice_RefocusesExistingTabWithoutDup // Arrange await WriteSampleWorkspaceAsync(); using var shell = CreateShell(); - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); var firstTab = shell.OpenSourceTextTab(filePath); // Open an unrelated tab so the source-text tab is no longer active, then re-open the same path. - var otherFilePath = Path.Combine(_tempRoot, "Other.sysml"); + var otherFilePath = PathHelpers.SafePathCombine(_tempRoot, "Other.sysml"); await File.WriteAllTextAsync(otherFilePath, "package Other {}\n", TestContext.Current.CancellationToken); shell.OpenSourceTextTab(otherFilePath); @@ -805,7 +805,7 @@ public async Task GetTabFilePath_ReturnsPathForSourceTextTab_AndNullOtherwise() await WriteSampleWorkspaceAsync(); using var shell = CreateShell(); await shell.AddFolderSourceAsync(_tempRoot); - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); var sourceTextTab = shell.OpenSourceTextTab(filePath); var view = shell.ViewCatalog.AvailableViews[0]; shell.SelectPredefinedView(view.QualifiedName); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/QueryDialogViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/QueryDialogViewModelTests.cs index e4544cb..d15aa56 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/QueryDialogViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/QueryDialogViewModelTests.cs @@ -45,7 +45,7 @@ public void Dispose() private async Task WriteSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part engineInstance : Engine;\n" diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/SourceTextDocumentViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/SourceTextDocumentViewModelTests.cs index 2f65b95..38c2cf3 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/SourceTextDocumentViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/SourceTextDocumentViewModelTests.cs @@ -61,7 +61,7 @@ private MainWindowShell CreateShell() public async Task Text_OpenedFile_MatchesFileContents() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); var contents = "package Sample {\n part def Engine;\n}\n"; await File.WriteAllTextAsync(filePath, contents, TestContext.Current.CancellationToken); using var shell = CreateShell(); @@ -81,7 +81,7 @@ public async Task Text_OpenedFile_MatchesFileContents() public async Task Title_OpenedFile_EqualsFileName() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await File.WriteAllTextAsync(filePath, "package Sample {}\n", TestContext.Current.CancellationToken); using var shell = CreateShell(); var tab = shell.OpenSourceTextTab(filePath); @@ -101,7 +101,7 @@ public async Task Title_OpenedFile_EqualsFileName() public async Task Text_DeletedFile_ProducesFriendlyErrorMessage_DoesNotThrow() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await File.WriteAllTextAsync(filePath, "package Sample {}\n", TestContext.Current.CancellationToken); using var shell = CreateShell(); var tab = shell.OpenSourceTextTab(filePath); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/ViewBuilderDialogViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/ViewBuilderDialogViewModelTests.cs index 3a1c367..9030f02 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/ViewBuilderDialogViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/ViewBuilderDialogViewModelTests.cs @@ -44,7 +44,7 @@ public void Dispose() private async Task WriteSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" @@ -60,7 +60,7 @@ await File.WriteAllTextAsync( private async Task WriteMultiKindSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "MultiKind.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "MultiKind.sysml"), "package MultiKind {\n" + " part def Engine;\n" + " part engineInstance : Engine;\n" @@ -77,7 +77,7 @@ await File.WriteAllTextAsync( private async Task WriteNoPartUsageSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "NoPartUsage.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "NoPartUsage.sysml"), "package NoPartUsage {\n" + " part def Engine;\n" + "}\n", diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/WorkspacePanelToolViewModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/WorkspacePanelToolViewModelTests.cs index 93f07c9..7ef318f 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/WorkspacePanelToolViewModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystem/WorkspacePanelToolViewModelTests.cs @@ -82,8 +82,8 @@ public void Construction_ZeroSources_ReportsEmptyTree() public async Task RebuildTree_FolderSource_ProducesSourceNodeWithFileChildren() { // Arrange - await WriteFileAsync(Path.Combine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); - await WriteFileAsync(Path.Combine(_tempRoot, "B.sysml"), "package B {\n part def Gadget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "B.sysml"), "package B {\n part def Gadget;\n}\n"); using var shell = CreateShell(); var viewModel = new WorkspacePanelToolViewModel(shell); @@ -108,10 +108,10 @@ public async Task RebuildTree_FolderSource_ProducesSourceNodeWithFileChildren() public async Task RebuildTree_FolderSourceWithSubfolders_PreservesOnDiskHierarchy() { // Arrange: A.sysml directly under the root, B.sysml one level down in "Sub", C.sysml two levels down. - Directory.CreateDirectory(Path.Combine(_tempRoot, "Sub", "Nested")); - await WriteFileAsync(Path.Combine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); - await WriteFileAsync(Path.Combine(_tempRoot, "Sub", "B.sysml"), "package B {\n part def Gadget;\n}\n"); - await WriteFileAsync(Path.Combine(_tempRoot, "Sub", "Nested", "C.sysml"), "package C {\n part def Doohickey;\n}\n"); + Directory.CreateDirectory(PathHelpers.SafePathCombine(_tempRoot, "Sub", "Nested")); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "Sub", "B.sysml"), "package B {\n part def Gadget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "Sub", "Nested", "C.sysml"), "package C {\n part def Doohickey;\n}\n"); using var shell = CreateShell(); var viewModel = new WorkspacePanelToolViewModel(shell); @@ -146,7 +146,7 @@ public async Task RebuildTree_FolderSourceWithSubfolders_PreservesOnDiskHierarch public async Task RebuildTree_FileSource_ProducesLeafSourceNodeWithNoChildren() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); using var shell = CreateShell(); var viewModel = new WorkspacePanelToolViewModel(shell); @@ -170,7 +170,7 @@ public async Task RebuildTree_FileSource_ProducesLeafSourceNodeWithNoChildren() public async Task RebuildTree_OverlappingFileAndFolder_DedupeReflectedInTreeShape() { // Arrange: the file source is registered first, then a folder source that also discovers that file - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); using var shell = CreateShell(); var viewModel = new WorkspacePanelToolViewModel(shell); @@ -294,7 +294,7 @@ public async Task RemoveSelectedCommand_WithSourceNodeSelected_RemovesOwningSour public async Task RemoveSelectedCommand_WithFileNodeSelected_RemovesOwningSource() { // Arrange - await WriteFileAsync(Path.Combine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n"); using var shell = CreateShell(); var viewModel = new WorkspacePanelToolViewModel(shell); await shell.AddFolderSourceAsync(_tempRoot); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystemTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystemTests.cs index eba4746..eafdab6 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystemTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/AppShellSubsystemTests.cs @@ -35,7 +35,7 @@ public void Dispose() private async Task WriteSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" @@ -96,7 +96,7 @@ public async Task SessionChanges_SynchronizeShellState() // Act: an external edit adds a second view await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Extra.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Extra.sysml"), "package Extra {\n part def Bracket;\n view ExtraView {\n expose Bracket;\n render asGridDiagram;\n }\n}\n", TestContext.Current.CancellationToken); await Task.Delay(5, TestContext.Current.CancellationToken); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystem/LayoutInvokerTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystem/LayoutInvokerTests.cs index 270daef..4679fee 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystem/LayoutInvokerTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystem/LayoutInvokerTests.cs @@ -31,7 +31,7 @@ public void Dispose() private async Task LoadSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" diff --git a/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystemTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystemTests.cs index 82730d2..6e3ed6e 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystemTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/LayoutRenderingSubsystemTests.cs @@ -25,7 +25,7 @@ public void Dispose() private async Task LoadSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" diff --git a/test/DemaConsulting.SysML2Workbench.Tests/SysML2WorkbenchTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/SysML2WorkbenchTests.cs index c9f9ac2..ccc1a0d 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/SysML2WorkbenchTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/SysML2WorkbenchTests.cs @@ -34,7 +34,7 @@ public void Dispose() private async Task WriteSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" @@ -77,7 +77,7 @@ public async Task OpenWorkspace_LoadsAndRefreshesWorkspace() // Act: an external change followed by a refresh await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Extra.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Extra.sysml"), "package Extra {\n part def Bracket;\n}\n", TestContext.Current.CancellationToken); await Task.Delay(5, TestContext.Current.CancellationToken); @@ -146,7 +146,7 @@ public async Task OpenWorkspace_ShowsWorkspaceDiagnostics() { // Arrange: a file with a deliberate syntax error await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Broken.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Broken.sysml"), "package Broken {\n part def Widget\n", TestContext.Current.CancellationToken); using var shell = CreateShell(); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/SysmlSnippetGeneratorTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/SysmlSnippetGeneratorTests.cs index b091e18..f127cc3 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/SysmlSnippetGeneratorTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/SysmlSnippetGeneratorTests.cs @@ -95,11 +95,8 @@ public void GenerateSnippet_NoExposeTargets_ThrowsInvalidOperationException() [Fact] public void SanitizeIdentifier_PlainIdentifier_ReturnsUnchanged() { - // Arrange - var generator = new SysmlSnippetGenerator(); - // Act - var result = generator.SanitizeIdentifier("EngineOverview"); + var result = SysmlSnippetGenerator.SanitizeIdentifier("EngineOverview"); // Assert Assert.Equal("EngineOverview", result); @@ -111,12 +108,9 @@ public void SanitizeIdentifier_PlainIdentifier_ReturnsUnchanged() [Fact] public void SanitizeIdentifier_ReservedWordOrInvalidCharacters_QuotesTheName() { - // Arrange - var generator = new SysmlSnippetGenerator(); - // Act - var quotedKeyword = generator.SanitizeIdentifier("view"); - var quotedSpaced = generator.SanitizeIdentifier("Engine Overview"); + var quotedKeyword = SysmlSnippetGenerator.SanitizeIdentifier("view"); + var quotedSpaced = SysmlSnippetGenerator.SanitizeIdentifier("Engine Overview"); // Assert Assert.Equal("'view'", quotedKeyword); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/ViewDefinitionModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/ViewDefinitionModelTests.cs index 3f25073..a8739f5 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/ViewDefinitionModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystem/ViewDefinitionModelTests.cs @@ -32,7 +32,7 @@ public void Dispose() private async Task LoadSampleWorkspaceAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" diff --git a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystemTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystemTests.cs index f3e9bfa..6f836c8 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystemTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/ViewBuilderSubsystemTests.cs @@ -31,7 +31,7 @@ public async Task EditDefinition_TracksCustomViewInputs() { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n part def Engine;\n part def Wheel;\n}\n", TestContext.Current.CancellationToken); var model = new WorkspaceModel(); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystem/ViewCatalogPresenterTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystem/ViewCatalogPresenterTests.cs index 7c7d1fa..4eecb8c 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystem/ViewCatalogPresenterTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystem/ViewCatalogPresenterTests.cs @@ -29,7 +29,7 @@ public void Dispose() /// private async Task LoadSingleViewWorkspaceAsync() { - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await File.WriteAllTextAsync( filePath, "package Sample {\n" @@ -134,9 +134,9 @@ public async Task RefreshCatalog_SelectionNoLongerPresent_ClearsSelection() presenter.RefreshCatalog(snapshot.Workspace, snapshot.RevisionId); presenter.SelectView("Sample::MyView"); - File.Delete(Path.Combine(_tempRoot, "Sample.sysml")); + File.Delete(PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml")); await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Empty.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Empty.sysml"), "package Empty {\n}\n", TestContext.Current.CancellationToken); var model = new WorkspaceModel(); @@ -159,7 +159,7 @@ await File.WriteAllTextAsync( /// private async Task LoadRichViewWorkspaceAsync() { - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await File.WriteAllTextAsync( filePath, "package Sample {\n" @@ -215,12 +215,15 @@ public async Task BuildViewDefinition_ResolvesKindExposeAndFilter() [Fact] public async Task BuildViewDefinition_UnknownViewId_ReturnsNull() { + // Arrange var snapshot = await LoadSingleViewWorkspaceAsync(); var presenter = new ViewCatalogPresenter(); presenter.RefreshCatalog(snapshot.Workspace, snapshot.RevisionId); + // Act var definition = presenter.BuildViewDefinition(snapshot.Workspace, "Sample::DoesNotExist"); + // Assert Assert.Null(definition); } @@ -231,7 +234,8 @@ public async Task BuildViewDefinition_UnknownViewId_ReturnsNull() [Fact] public async Task BuildViewDefinition_NoExposeMembers_ReturnsNull() { - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + // Arrange + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await File.WriteAllTextAsync( filePath, "package Sample {\n" @@ -250,8 +254,10 @@ await File.WriteAllTextAsync( var presenter = new ViewCatalogPresenter(); presenter.RefreshCatalog(snapshot.Workspace, snapshot.RevisionId); + // Act var definition = presenter.BuildViewDefinition(snapshot.Workspace, "Sample::MyView"); + // Assert Assert.Null(definition); } } diff --git a/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystemTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystemTests.cs index b8c0c3c..a374a4f 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystemTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/ViewCatalogSubsystemTests.cs @@ -24,7 +24,7 @@ public void Dispose() private async Task LoadWorkspaceWithTwoViewsAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Wheel;\n" diff --git a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/FileWatcherTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/FileWatcherTests.cs index 29c91e5..2fea0cf 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/FileWatcherTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/FileWatcherTests.cs @@ -42,17 +42,17 @@ public void ExternalFileChange_RaisesAffectedPathEvent() var now = DateTimeOffset.UtcNow; var watcher = new FileWatcher(TimeSpan.FromMilliseconds(50), () => now); watcher.WatchSource(FolderSource(_tempRoot)); - var changedPath = Path.Combine(_tempRoot, "Model.sysml"); + var changedPath = PathHelpers.SafePathCombine(_tempRoot, "Model.sysml"); // Act: simulate an external change notification, then advance the clock past the debounce window watcher.QueueChange(changedPath); - Assert.Contains(changedPath, watcher.PendingChanges); + Assert.Contains(changedPath, watcher.GetPendingChanges()); now = now.AddMilliseconds(100); var flushed = watcher.FlushPendingChanges(); // Assert: the changed path is surfaced by the flush and removed from the pending set Assert.Contains(changedPath, flushed); - Assert.DoesNotContain(changedPath, watcher.PendingChanges); + Assert.DoesNotContain(changedPath, watcher.GetPendingChanges()); } /// @@ -66,7 +66,7 @@ public void NotificationBurst_CoalescesIntoSingleReloadTrigger() var now = DateTimeOffset.UtcNow; var watcher = new FileWatcher(TimeSpan.FromMilliseconds(50), () => now); watcher.WatchSource(FolderSource(_tempRoot)); - var changedPath = Path.Combine(_tempRoot, "Model.sysml"); + var changedPath = PathHelpers.SafePathCombine(_tempRoot, "Model.sysml"); // Act: raise a burst of notifications for the same path in quick succession watcher.QueueChange(changedPath); @@ -95,7 +95,7 @@ public void FlushPendingChanges_WithinDebounceWindow_RetainsPathForLaterFlush() var now = DateTimeOffset.UtcNow; var watcher = new FileWatcher(TimeSpan.FromSeconds(1), () => now); watcher.WatchSource(FolderSource(_tempRoot)); - var changedPath = Path.Combine(_tempRoot, "Model.sysml"); + var changedPath = PathHelpers.SafePathCombine(_tempRoot, "Model.sysml"); watcher.QueueChange(changedPath); // Act: flush immediately, well before the debounce window elapses @@ -103,7 +103,7 @@ public void FlushPendingChanges_WithinDebounceWindow_RetainsPathForLaterFlush() // Assert: nothing is surfaced yet, and the path remains pending Assert.Empty(flushed); - Assert.Contains(changedPath, watcher.PendingChanges); + Assert.Contains(changedPath, watcher.GetPendingChanges()); } /// @@ -118,10 +118,10 @@ public void QueueChange_WithNoWatchedSources_IsIgnored() var watcher = new FileWatcher(TimeSpan.FromMilliseconds(50)); // Act: queuing does not throw - watcher.QueueChange(Path.Combine(_tempRoot, "Model.sysml")); + watcher.QueueChange(PathHelpers.SafePathCombine(_tempRoot, "Model.sysml")); // Assert: nothing was recorded, and flushing is likewise a harmless no-op - Assert.Empty(watcher.PendingChanges); + Assert.Empty(watcher.GetPendingChanges()); Assert.Empty(watcher.FlushPendingChanges()); } @@ -142,11 +142,11 @@ public void QueueChange_AfterSourceUnwatchedToZero_DoesNotThrow() watcher.UnwatchSource(source.Id); // Act: a stale change notification for the now-unwatched source arrives - var exception = Record.Exception(() => watcher.QueueChange(Path.Combine(_tempRoot, "Model.sysml"))); + var exception = Record.Exception(() => watcher.QueueChange(PathHelpers.SafePathCombine(_tempRoot, "Model.sysml"))); // Assert: no exception, and the stale notification was not recorded Assert.Null(exception); - Assert.Empty(watcher.PendingChanges); + Assert.Empty(watcher.GetPendingChanges()); Assert.Empty(watcher.FlushPendingChanges()); } @@ -200,8 +200,8 @@ public void UnwatchSource_RemovesOnlyThatWatcher_OthersContinueReporting() watcher.WatchSource(folderA); watcher.WatchSource(folderB); - var pathUnderA = Path.Combine(folderA.Path, "A.sysml"); - var pathUnderB = Path.Combine(folderB.Path, "B.sysml"); + var pathUnderA = PathHelpers.SafePathCombine(folderA.Path, "A.sysml"); + var pathUnderB = PathHelpers.SafePathCombine(folderB.Path, "B.sysml"); watcher.QueueChange(pathUnderA); watcher.QueueChange(pathUnderB); @@ -212,8 +212,8 @@ public void UnwatchSource_RemovesOnlyThatWatcher_OthersContinueReporting() Assert.True(removed); Assert.DoesNotContain(folderA.Id, watcher.WatchedSourceIds); Assert.Contains(folderB.Id, watcher.WatchedSourceIds); - Assert.Contains(pathUnderA, watcher.PendingChanges); - Assert.Contains(pathUnderB, watcher.PendingChanges); + Assert.Contains(pathUnderA, watcher.GetPendingChanges()); + Assert.Contains(pathUnderB, watcher.GetPendingChanges()); } finally { @@ -294,7 +294,7 @@ public async Task WatchSource_TwoFolders_ChangeUnderOneIsNotAttributedToTheOther watcher.WatchSource(FolderSource(rootB)); // Act: write a real file under root B only - var changedPathUnderB = Path.Combine(rootB, "Model.sysml"); + var changedPathUnderB = PathHelpers.SafePathCombine(rootB, "Model.sysml"); await File.WriteAllTextAsync(changedPathUnderB, "part def Vehicle;", TestContext.Current.CancellationToken); // Poll with a bounded timeout: real FileSystemWatcher notifications are delivered on an OS callback @@ -303,7 +303,7 @@ public async Task WatchSource_TwoFolders_ChangeUnderOneIsNotAttributedToTheOther var detected = false; while (DateTime.UtcNow < deadline && !detected) { - detected = watcher.PendingChanges.Any(path => + detected = watcher.GetPendingChanges().Any(path => string.Equals(path, changedPathUnderB, StringComparison.OrdinalIgnoreCase)); if (!detected) @@ -315,7 +315,7 @@ public async Task WatchSource_TwoFolders_ChangeUnderOneIsNotAttributedToTheOther // Assert: the real OS-level change under B was detected, and nothing under root A was ever written, // so no path under A can appear as a false-positive pending change. Assert.True(detected, $"Expected '{changedPathUnderB}' to be reported as a pending change under root B."); - Assert.DoesNotContain(watcher.PendingChanges, path => path.StartsWith(rootA, StringComparison.OrdinalIgnoreCase)); + Assert.DoesNotContain(watcher.GetPendingChanges(), path => path.StartsWith(rootA, StringComparison.OrdinalIgnoreCase)); } finally { @@ -327,7 +327,7 @@ public async Task WatchSource_TwoFolders_ChangeUnderOneIsNotAttributedToTheOther /// Regression test: reproduces many real, concurrently-arriving /// callbacks (via , which runs them synchronously on their own OS /// callback thread rather than marshaling onto a UI thread) racing repeated reads via - /// and from the + /// and from the /// test thread. Previously, concurrent unsynchronized access to the internal pending-changes dictionary /// could corrupt its state and crash the whole process rather than merely throwing a recoverable /// exception; this must now complete cleanly under sustained concurrent load. @@ -347,7 +347,7 @@ public async Task ConcurrentRealFileSystemChangesAndReads_DoesNotCorruptState() var counter = 0; while (!cts.IsCancellationRequested) { - var path = Path.Combine(_tempRoot, $"File{counter++ % 20}.sysml"); + var path = PathHelpers.SafePathCombine(_tempRoot, $"File{counter++ % 20}.sysml"); try { await File.WriteAllTextAsync(path, "part def Widget;", CancellationToken.None); @@ -363,7 +363,7 @@ public async Task ConcurrentRealFileSystemChangesAndReads_DoesNotCorruptState() { while (!cts.IsCancellationRequested) { - _ = watcher.PendingChanges.Count; + _ = watcher.GetPendingChanges().Count; _ = watcher.FlushPendingChanges(); await Task.Delay(5, CancellationToken.None); } diff --git a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceModelTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceModelTests.cs index a174b1b..59d0dd9 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceModelTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceModelTests.cs @@ -51,7 +51,7 @@ private static (IReadOnlyList Sources, WorkspaceSourceResolutio public async Task LoadWorkspaceAsync_BuildsTrackedFileTree() { // Arrange: a workspace folder containing a single valid SysML file - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var model = new WorkspaceModel(); var (sources, resolution) = ResolveFolder(_tempRoot); @@ -76,10 +76,10 @@ public async Task LoadWorkspaceAsync_FindsDiscoveredAndImportedFiles() { // Arrange: two files where the second imports a definition declared in the first await WriteFileAsync( - Path.Combine(_tempRoot, "A.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "A.sysml"), "package PackageA {\n part def Widget;\n}\n"); await WriteFileAsync( - Path.Combine(_tempRoot, "B.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "B.sysml"), "package PackageB {\n import PackageA::*;\n part myWidget : Widget;\n}\n"); var model = new WorkspaceModel(); var (sources, resolution) = ResolveFolder(_tempRoot); @@ -102,8 +102,8 @@ await WriteFileAsync( public async Task ReloadFile_UpdatesOnlyAffectedFileState() { // Arrange: load a workspace with one always-valid file and one file that will be broken later - var unaffectedPath = Path.Combine(_tempRoot, "Unaffected.sysml"); - var affectedPath = Path.Combine(_tempRoot, "Affected.sysml"); + var unaffectedPath = PathHelpers.SafePathCombine(_tempRoot, "Unaffected.sysml"); + var affectedPath = PathHelpers.SafePathCombine(_tempRoot, "Affected.sysml"); await WriteFileAsync(unaffectedPath, "package Unaffected {\n part def Widget;\n}\n"); await WriteFileAsync(affectedPath, "package Affected {\n part def Gadget;\n}\n"); var model = new WorkspaceModel(); @@ -154,7 +154,7 @@ public async Task ReloadFilesAsync_AfterResolutionBecomesEmpty_ProducesValidEmpt { // Arrange: load a non-empty workspace, then reset it to an empty resolution (mirrors what // MainWindowShell does after removing the last source). - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var model = new WorkspaceModel(); var (sources, resolution) = ResolveFolder(_tempRoot); diff --git a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceSourceSetTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceSourceSetTests.cs index a8920c5..6aafd35 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceSourceSetTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystem/WorkspaceSourceSetTests.cs @@ -54,7 +54,7 @@ public void Resolve_ZeroSources_ReturnsEmptyResolution() public async Task AddFile_SamePathTwice_ReturnsSameSourceAndDoesNotDuplicate() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var sourceSet = new WorkspaceSourceSet(); @@ -95,7 +95,7 @@ public void AddFolder_SamePathTwice_ReturnsSameSourceAndDoesNotDuplicate() public void AddFolder_MissingFolder_ThrowsDirectoryNotFoundException() { // Arrange - var missingPath = Path.Combine(_tempRoot, "does-not-exist"); + var missingPath = PathHelpers.SafePathCombine(_tempRoot, "does-not-exist"); var sourceSet = new WorkspaceSourceSet(); // Act / Assert @@ -132,7 +132,7 @@ public void RemoveSource_RegisteredThenUnknownId_ReturnsTrueThenFalse() public async Task ClearSources_WithRegisteredSources_RemovesAllAndResolveReturnsEmpty() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var sourceSet = new WorkspaceSourceSet(); sourceSet.AddFile(filePath); @@ -156,10 +156,10 @@ public async Task ClearSources_WithRegisteredSources_RemovesAllAndResolveReturns public async Task Resolve_FolderSource_DiscoversAllSysmlFilesRecursively() { // Arrange - var nestedDir = Path.Combine(_tempRoot, "nested"); + var nestedDir = PathHelpers.SafePathCombine(_tempRoot, "nested"); Directory.CreateDirectory(nestedDir); - await WriteFileAsync(Path.Combine(_tempRoot, "Top.sysml"), "package Top {\n part def Widget;\n}\n"); - await WriteFileAsync(Path.Combine(nestedDir, "Nested.sysml"), "package Nested {\n part def Gadget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(_tempRoot, "Top.sysml"), "package Top {\n part def Widget;\n}\n"); + await WriteFileAsync(PathHelpers.SafePathCombine(nestedDir, "Nested.sysml"), "package Nested {\n part def Gadget;\n}\n"); var sourceSet = new WorkspaceSourceSet(); var folderSource = sourceSet.AddFolder(_tempRoot); @@ -180,7 +180,7 @@ public async Task Resolve_FolderSource_DiscoversAllSysmlFilesRecursively() public async Task Resolve_FileOverlappingFolder_DedupesAndFirstRegisteredSourceWinsAttribution() { // Arrange: the file source is registered first, then a folder source that also discovers that file - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var sourceSet = new WorkspaceSourceSet(); var fileSource = sourceSet.AddFile(filePath); @@ -205,9 +205,9 @@ public async Task Resolve_NestedFolderOverlap_DedupesAndFirstRegisteredSourceWin { // Arrange: an outer folder registered first, and an inner (nested) folder registered second, both // discovering the same nested file. - var nestedDir = Path.Combine(_tempRoot, "nested"); + var nestedDir = PathHelpers.SafePathCombine(_tempRoot, "nested"); Directory.CreateDirectory(nestedDir); - var nestedFile = Path.Combine(nestedDir, "Nested.sysml"); + var nestedFile = PathHelpers.SafePathCombine(nestedDir, "Nested.sysml"); await WriteFileAsync(nestedFile, "package Nested {\n part def Gadget;\n}\n"); var sourceSet = new WorkspaceSourceSet(); var outerSource = sourceSet.AddFolder(_tempRoot); @@ -234,7 +234,7 @@ public void AddFolder_DifferentlyCasedPath_NormalizesToOnDiskCasingAndIsIdempote { // Arrange var actualName = Path.GetFileName(_tempRoot); - var upperCasedPath = Path.Combine(Path.GetDirectoryName(_tempRoot)!, actualName.ToUpperInvariant()); + var upperCasedPath = PathHelpers.SafePathCombine(Path.GetDirectoryName(_tempRoot)!, actualName.ToUpperInvariant()); if (!Directory.Exists(upperCasedPath)) { // The host filesystem is case-sensitive (for example Linux ext4), so the differently-cased path @@ -263,7 +263,7 @@ public void AddFolder_DifferentlyCasedPath_NormalizesToOnDiskCasingAndIsIdempote public async Task Sources_PreservesRegistrationOrder() { // Arrange - var filePath = Path.Combine(_tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"); await WriteFileAsync(filePath, "package Sample {\n part def Widget;\n}\n"); var otherDir = Directory.CreateTempSubdirectory("sysml2workbench-source-set-tests-other-").FullName; try diff --git a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystemTests.cs b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystemTests.cs index 654d562..28d337c 100644 --- a/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystemTests.cs +++ b/test/DemaConsulting.SysML2Workbench.Tests/WorkspaceSubsystemTests.cs @@ -29,7 +29,7 @@ public async Task OpenWorkspace_BuildsWorkspaceState() { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n part def Engine;\n}\n", TestContext.Current.CancellationToken); var model = new WorkspaceModel(); @@ -55,7 +55,7 @@ public async Task ExternalChange_RefreshesWorkspaceState() { // Arrange: an initially loaded, watched workspace await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n part def Engine;\n}\n", TestContext.Current.CancellationToken); var model = new WorkspaceModel(); @@ -64,11 +64,11 @@ await File.WriteAllTextAsync( await model.LoadWorkspaceAsync(sourceSet.Sources, sourceSet.Resolve()); var now = DateTimeOffset.UtcNow; - var watcher = new FileWatcher(TimeSpan.FromMilliseconds(1), () => now); + using var watcher = new FileWatcher(TimeSpan.FromMilliseconds(1), () => now); watcher.WatchSource(sourceSet.Sources[0]); // Act: an external process adds a new file, the watcher observes it, and the debounce window elapses - var newFilePath = Path.Combine(_tempRoot, "Extra.sysml"); + var newFilePath = PathHelpers.SafePathCombine(_tempRoot, "Extra.sysml"); await File.WriteAllTextAsync(newFilePath, "package Extra {\n part def Bracket;\n}\n", TestContext.Current.CancellationToken); watcher.QueueChange(newFilePath); now = now.AddSeconds(1); @@ -78,8 +78,6 @@ await File.WriteAllTextAsync( // Assert: the refreshed workspace state now includes the new file Assert.Equal(2, refreshed.Files.Count); Assert.True(refreshed.Workspace.Declarations.ContainsKey("Extra::Bracket")); - - watcher.Dispose(); } /// @@ -92,11 +90,11 @@ public async Task OpenWorkspace_ProducesUnifiedDiagnostics() // Arrange: two files, one containing a deliberate syntax error (unmatched brace) guaranteed to produce // a parser diagnostic regardless of semantic resolution specifics await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "A.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "A.sysml"), "package A {\n part def Widget;\n}\n", TestContext.Current.CancellationToken); await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "B.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "B.sysml"), "package B {\n part def Gadget\n", TestContext.Current.CancellationToken); var model = new WorkspaceModel(); diff --git a/test/DemaConsulting.SysML2Workbench.UiTests/AppShellSubsystem/MainWindowShellUiTests.cs b/test/DemaConsulting.SysML2Workbench.UiTests/AppShellSubsystem/MainWindowShellUiTests.cs index f61b798..82a9e63 100644 --- a/test/DemaConsulting.SysML2Workbench.UiTests/AppShellSubsystem/MainWindowShellUiTests.cs +++ b/test/DemaConsulting.SysML2Workbench.UiTests/AppShellSubsystem/MainWindowShellUiTests.cs @@ -103,7 +103,7 @@ public async Task MainWindowView_CloseAllMenuItem_Click_ClosesAllSources() var tempRoot = Directory.CreateTempSubdirectory("sysml2workbench-ui-tests-").FullName; try { - var filePath = Path.Combine(tempRoot, "Sample.sysml"); + var filePath = PathHelpers.SafePathCombine(tempRoot, "Sample.sysml"); await File.WriteAllTextAsync(filePath, "package Sample {\n part def Widget;\n}\n"); using var shell = CreateShell(); diff --git a/test/OtsSoftwareTests/AvaloniaTests.cs b/test/OtsSoftwareTests/AvaloniaTests.cs index af113ba..588b595 100644 --- a/test/OtsSoftwareTests/AvaloniaTests.cs +++ b/test/OtsSoftwareTests/AvaloniaTests.cs @@ -6,6 +6,7 @@ using Avalonia.Interactivity; using Avalonia.Threading; using Avalonia.VisualTree; +using DemaConsulting.SysML2Workbench; using DemaConsulting.SysML2Workbench.AppShellSubsystem; using DemaConsulting.SysML2Workbench.DiagnosticsPanelSubsystem; using DemaConsulting.SysML2Workbench.LayoutRenderingSubsystem; @@ -89,7 +90,7 @@ public async Task MainWindow_HostsDiagramAndDiagnosticsPanels() { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view PredefinedView {\n" @@ -138,7 +139,7 @@ public async Task DiagramContextMenu_CopyAsSysml_CopiesSnippetToClipboard() { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view PredefinedView {\n" @@ -198,7 +199,7 @@ public async Task DiagramContextMenu_CopyAsSysml_TwoCustomViewTabs_EachCopiesIts { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " part def Gearbox;\n" @@ -362,7 +363,7 @@ public async Task QueryDialog_SelectDescribeAndCopyAsMarkdown_PlacesRenderedMark { // Arrange: a real workspace with one part def, so Describe has something meaningful to say await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + "}\n"); diff --git a/test/OtsSoftwareTests/DockTests.cs b/test/OtsSoftwareTests/DockTests.cs index 9949479..81f6062 100644 --- a/test/OtsSoftwareTests/DockTests.cs +++ b/test/OtsSoftwareTests/DockTests.cs @@ -5,6 +5,7 @@ using Avalonia.Media; using Avalonia.Threading; using Avalonia.VisualTree; +using DemaConsulting.SysML2Workbench; using DemaConsulting.SysML2Workbench.AppShellSubsystem; using DemaConsulting.SysML2Workbench.DiagnosticsPanelSubsystem; using DemaConsulting.SysML2Workbench.LayoutRenderingSubsystem; @@ -416,11 +417,11 @@ public async Task MainWindowView_TabsChangedFromBackgroundThread_MarshalsDiagram try { await File.WriteAllTextAsync( - Path.Combine(firstRoot, "First.sysml"), + PathHelpers.SafePathCombine(firstRoot, "First.sysml"), "package First {\n part def Engine;\n}\n", TestContext.Current.CancellationToken); await File.WriteAllTextAsync( - Path.Combine(secondRoot, "Second.sysml"), + PathHelpers.SafePathCombine(secondRoot, "Second.sysml"), "package Second {\n part def Wheel;\n}\n", TestContext.Current.CancellationToken); diff --git a/test/OtsSoftwareTests/RenderingTests.cs b/test/OtsSoftwareTests/RenderingTests.cs index 73d0af3..59b08d8 100644 --- a/test/OtsSoftwareTests/RenderingTests.cs +++ b/test/OtsSoftwareTests/RenderingTests.cs @@ -5,6 +5,7 @@ using DemaConsulting.SysML2Tools.Rendering; using DemaConsulting.SysML2Tools.Semantic; using DemaConsulting.SysML2Tools.Stdlib; +using DemaConsulting.SysML2Workbench; namespace OtsSoftwareTests; @@ -29,7 +30,7 @@ public void Dispose() private async Task RenderSampleViewAsync() { await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view PredefinedView {\n" diff --git a/test/OtsSoftwareTests/SysML2ToolsTests.cs b/test/OtsSoftwareTests/SysML2ToolsTests.cs index 01db9cb..a7e446f 100644 --- a/test/OtsSoftwareTests/SysML2ToolsTests.cs +++ b/test/OtsSoftwareTests/SysML2ToolsTests.cs @@ -2,6 +2,7 @@ using DemaConsulting.SysML2Tools.Parser; using DemaConsulting.SysML2Tools.Semantic; using DemaConsulting.SysML2Tools.Stdlib; +using DemaConsulting.SysML2Workbench; namespace OtsSoftwareTests; @@ -33,11 +34,11 @@ public async Task LoadWorkspaceModel_ParsesAndResolvesImports() { // Arrange: two files where one imports a definition declared in the other await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Parts.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Parts.sysml"), "package Parts {\n part def Engine;\n}\n", TestContext.Current.CancellationToken); await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Vehicle.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Vehicle.sysml"), "package Vehicle {\n" + " private import Parts::*;\n" + " part def Car {\n" @@ -76,7 +77,7 @@ public async Task RenderView_GeneratesLayoutGraph() { // Arrange await File.WriteAllTextAsync( - Path.Combine(_tempRoot, "Sample.sysml"), + PathHelpers.SafePathCombine(_tempRoot, "Sample.sysml"), "package Sample {\n" + " part def Engine;\n" + " view PredefinedView {\n" diff --git a/test/OtsSoftwareTests/XUnitTests.cs b/test/OtsSoftwareTests/XUnitTests.cs index 82fd18d..13770e2 100644 --- a/test/OtsSoftwareTests/XUnitTests.cs +++ b/test/OtsSoftwareTests/XUnitTests.cs @@ -1,3 +1,5 @@ +using DemaConsulting.SysML2Workbench; + namespace OtsSoftwareTests; /// @@ -62,7 +64,7 @@ private static string FindRepositoryFile(string fileName) var directory = new DirectoryInfo(AppContext.BaseDirectory); while (directory is not null) { - var candidate = Path.Combine(directory.FullName, fileName); + var candidate = PathHelpers.SafePathCombine(directory.FullName, fileName); if (File.Exists(candidate)) { return candidate;