diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 25cc95ec..de1787f5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -58,16 +58,18 @@ SettingsStore atomic file replacement ↓ Current.ReplaceWith ↓ -one synchronous Changed event +one synchronous Changed(origin) event ``` A failed mutation or failed write does not replace committed settings and does not publish a settings change. `SettingsCoordinator.Current` exposes `IReadOnlySightAdaptSettings` and returns a defensive snapshot. Mutable `SightAdaptSettings` is confined to explicit transaction callbacks and normalization/persistence boundaries. `SettingsUseCaseContext` centralizes snapshot, change-event and commit plumbing for application use-case façades without sharing their domain commands. `Commit` and `PersistCurrent` use one persistence execution path; only `Commit` publishes the synchronous `Changed` event after a successful write. +Every published `SettingsChangedEventArgs` carries the explicit change origin supplied by the transaction boundary. A use-case context marks its own commits with a private origin token; local presentation code performs its one explicit post-commit follow-up and ignores only that origin, while external changes still refresh it. `RuntimeCoordinator` uses itself as the origin for runtime commands and ignores only its own generic callback because those commands already perform their runtime follow-up. Nested commits retain independent origins, so reentrant event delivery does not rely on component Boolean suppression flags. + ## Authorities | Concern | Authority | |---|---| -| Settings transaction and published read-only snapshots | `SettingsCoordinator` | +| Settings transaction, change origin and published read-only snapshots | `SettingsCoordinator` | | Shared use-case snapshot/event/commit plumbing | `SettingsUseCaseContext` | | Settings JSON persistence and atomic replacement | `SettingsStore` | | Persisted JSON DTOs and legacy-field migration | `PersistedSettingsMapper` | diff --git a/src/SightAdapt/ConfigurationForm.cs b/src/SightAdapt/ConfigurationForm.cs index ed519155..fbf514d4 100644 --- a/src/SightAdapt/ConfigurationForm.cs +++ b/src/SightAdapt/ConfigurationForm.cs @@ -16,7 +16,6 @@ private readonly Action private readonly ApplicationAssignmentsGrid _assignmentsGrid; private readonly ModernButton _editVisualProfileButton; private bool _refreshing; - private bool _committingGridValue; public ConfigurationForm( SettingsCoordinator settingsCoordinator, @@ -445,7 +444,11 @@ private void AutomaticModeCheckedChanged(object? sender, EventArgs eventArgs) { ShowCommitError(result.ErrorMessage); RefreshProfiles(); + return; } + + UpdateAutomaticModeState( + _automaticModeSwitch.Checked); } private void UpdateAutomaticModeState(bool automaticMode) @@ -474,17 +477,7 @@ private void AssignmentChanged( var displayedRow = ApplicationAssignmentRowMapper.Map( displayedAssignment); - SettingsCommitResult result; - - _committingGridValue = true; - try - { - result = _useCases.Apply(change); - } - finally - { - _committingGridValue = false; - } + var result = _useCases.Apply(change); if (!result.Succeeded) { @@ -557,7 +550,10 @@ private void EditSelectedVisualProfile() { ShowCommitError(result.ErrorMessage); RefreshProfiles(); + return; } + + RefreshProfiles(); } internal void ManageVisualProfiles() @@ -638,6 +634,8 @@ private void AddOrUpdateProfile(ApplicationIdentity identity) return; } + RefreshProfiles(); + MessageBox.Show( this, result.Value @@ -671,12 +669,17 @@ private void RemoveSelectedProfile() if (!result.Succeeded) { ShowCommitError(result.ErrorMessage); + return; } + + RefreshProfiles(); } - private void SettingsChanged(object? sender, EventArgs eventArgs) + private void SettingsChanged( + object? sender, + SettingsChangedEventArgs eventArgs) { - if (_committingGridValue) + if (_useCases.IsLocalChange(eventArgs)) { return; } diff --git a/src/SightAdapt/ConfigurationUseCases.cs b/src/SightAdapt/ConfigurationUseCases.cs index b8de5bdf..7a3e7c97 100644 --- a/src/SightAdapt/ConfigurationUseCases.cs +++ b/src/SightAdapt/ConfigurationUseCases.cs @@ -26,12 +26,16 @@ internal ConfigurationUseCases( public string SettingsPath => _settings.SettingsPath; - public event EventHandler? Changed + public event EventHandler? Changed { add => _settings.Changed += value; remove => _settings.Changed -= value; } + public bool IsLocalChange( + SettingsChangedEventArgs eventArgs) => + _settings.IsLocalChange(eventArgs); + public SettingsCommitResult SetAutomaticMode(bool enabled) { return _settings.Commit(settings => diff --git a/src/SightAdapt/RuntimeCoordinator.cs b/src/SightAdapt/RuntimeCoordinator.cs index 9b047414..dec4b1fc 100644 --- a/src/SightAdapt/RuntimeCoordinator.cs +++ b/src/SightAdapt/RuntimeCoordinator.cs @@ -14,7 +14,6 @@ internal sealed class RuntimeCoordinator private readonly RuntimeOverlayActivator _overlayActivator; private readonly AutomaticActivationService _automaticActivation; private readonly Func _readSettings; - private bool _committingSettings; internal RuntimeCoordinator( SettingsCoordinator settingsCoordinator, @@ -187,14 +186,19 @@ public void HandleForegroundWindowChanged( public void HandleSettingsChanged() { - HandleSettingsChanged(ReadSettings()); + HandleSettingsChanged( + ReadSettings(), + eventArgs: null); } internal void HandleSettingsChanged( - IReadOnlySightAdaptSettings settings) + IReadOnlySightAdaptSettings settings, + SettingsChangedEventArgs? eventArgs) { ArgumentNullException.ThrowIfNull(settings); - if (_committingSettings) + if (ReferenceEquals( + eventArgs?.Origin, + this)) { return; } @@ -248,15 +252,9 @@ private SettingsCommitResult CommitSettings( { ArgumentNullException.ThrowIfNull(mutation); - _committingSettings = true; - try - { - return _settingsCoordinator.Commit(mutation); - } - finally - { - _committingSettings = false; - } + return _settingsCoordinator.Commit( + mutation, + origin: this); } private void ShowCommitError(string? message) diff --git a/src/SightAdapt/SettingsCoordinator.cs b/src/SightAdapt/SettingsCoordinator.cs index 741b2ff7..58b07658 100644 --- a/src/SightAdapt/SettingsCoordinator.cs +++ b/src/SightAdapt/SettingsCoordinator.cs @@ -12,6 +12,12 @@ public static SettingsCommitResult Failure(string message) => new(false, message); } +internal sealed class SettingsChangedEventArgs( + object? origin) : EventArgs +{ + public object? Origin { get; } = origin; +} + internal sealed class SettingsCommitResult { private readonly T _value; @@ -74,18 +80,21 @@ public SettingsCoordinator( public bool SettingsWereMigrated => _store.SettingsWereMigrated; - public event EventHandler? Changed; + public event EventHandler? Changed; public SettingsCommitResult Commit( - Action mutation) + Action mutation, + object? origin = null) { ArgumentNullException.ThrowIfNull(mutation); - var result = Commit(settings => - { - mutation(settings); - return null; - }); + var result = Commit( + settings => + { + mutation(settings); + return null; + }, + origin); return result.Succeeded ? SettingsCommitResult.Success() @@ -95,19 +104,22 @@ public SettingsCommitResult Commit( } public SettingsCommitResult Commit( - Func mutation) + Func mutation, + object? origin = null) { ArgumentNullException.ThrowIfNull(mutation); return ExecuteTransaction( mutation, - publishChanged: true); + publishChanged: true, + origin); } public SettingsCommitResult PersistCurrent() { var result = ExecuteTransaction( _ => null, - publishChanged: false); + publishChanged: false, + origin: null); return result.Succeeded ? SettingsCommitResult.Success() @@ -118,7 +130,8 @@ public SettingsCommitResult PersistCurrent() private SettingsCommitResult ExecuteTransaction( Func operation, - bool publishChanged) + bool publishChanged, + object? origin) { ArgumentNullException.ThrowIfNull(operation); @@ -145,7 +158,9 @@ private SettingsCommitResult ExecuteTransaction( _current.ReplaceWith(candidate); if (publishChanged) { - Changed?.Invoke(this, EventArgs.Empty); + Changed?.Invoke( + this, + new SettingsChangedEventArgs(origin)); } return SettingsCommitResult.Success(value); diff --git a/src/SightAdapt/SettingsUseCaseContext.cs b/src/SightAdapt/SettingsUseCaseContext.cs index 3a414a2c..6f13edd1 100644 --- a/src/SightAdapt/SettingsUseCaseContext.cs +++ b/src/SightAdapt/SettingsUseCaseContext.cs @@ -3,6 +3,7 @@ namespace SightAdapt; internal sealed class SettingsUseCaseContext { private readonly SettingsCoordinator _coordinator; + private readonly object _changeOrigin = new(); public SettingsUseCaseContext( SettingsCoordinator coordinator) @@ -17,17 +18,30 @@ public SettingsUseCaseContext( public string SettingsPath => _coordinator.SettingsPath; - public event EventHandler? Changed + public event EventHandler? Changed { add => _coordinator.Changed += value; remove => _coordinator.Changed -= value; } + public bool IsLocalChange( + SettingsChangedEventArgs eventArgs) + { + ArgumentNullException.ThrowIfNull(eventArgs); + return ReferenceEquals( + eventArgs.Origin, + _changeOrigin); + } + public SettingsCommitResult Commit( Action mutation) => - _coordinator.Commit(mutation); + _coordinator.Commit( + mutation, + _changeOrigin); public SettingsCommitResult Commit( Func mutation) => - _coordinator.Commit(mutation); + _coordinator.Commit( + mutation, + _changeOrigin); } diff --git a/src/SightAdapt/SightAdaptContext.cs b/src/SightAdapt/SightAdaptContext.cs index 54a429a7..7893eb60 100644 --- a/src/SightAdapt/SightAdaptContext.cs +++ b/src/SightAdapt/SightAdaptContext.cs @@ -157,14 +157,16 @@ private void ForegroundWindowChanged( private void SettingsChanged( object? sender, - EventArgs eventArgs) + SettingsChangedEventArgs eventArgs) { var settings = _settingsCoordinator.Current; _tray.SetAutomaticMode(settings.AutomaticMode); ApplyApplicationState( _stateController.Current, settings); - _runtimeCoordinator.HandleSettingsChanged(settings); + _runtimeCoordinator.HandleSettingsChanged( + settings, + eventArgs); } private void ShowConfiguration() diff --git a/src/SightAdapt/VisualProfileManagerForm.cs b/src/SightAdapt/VisualProfileManagerForm.cs index 6840ea0f..2929220a 100644 --- a/src/SightAdapt/VisualProfileManagerForm.cs +++ b/src/SightAdapt/VisualProfileManagerForm.cs @@ -9,7 +9,6 @@ internal sealed class VisualProfileManagerForm : Form private readonly ModernButton _renameButton; private readonly ModernButton _editButton; private readonly ModernButton _deleteButton; - private bool _committingLocalChange; internal VisualProfileManagerForm( SettingsCoordinator settingsCoordinator) @@ -404,16 +403,7 @@ internal void Commit( { ArgumentNullException.ThrowIfNull(command); - SettingsCommitResult result; - _committingLocalChange = true; - try - { - result = command(_useCases); - } - finally - { - _committingLocalChange = false; - } + var result = command(_useCases); if (result.Succeeded) { @@ -430,12 +420,16 @@ internal void Commit( RefreshProfiles(); } - private void SettingsChanged(object? sender, EventArgs eventArgs) + private void SettingsChanged( + object? sender, + SettingsChangedEventArgs eventArgs) { - if (!_committingLocalChange) + if (_useCases.IsLocalChange(eventArgs)) { - RefreshProfiles(); + return; } + + RefreshProfiles(); } } diff --git a/src/SightAdapt/VisualProfileUseCases.cs b/src/SightAdapt/VisualProfileUseCases.cs index da1dba52..80a9811e 100644 --- a/src/SightAdapt/VisualProfileUseCases.cs +++ b/src/SightAdapt/VisualProfileUseCases.cs @@ -23,12 +23,16 @@ internal VisualProfileUseCases( public IReadOnlySightAdaptSettings Snapshot => _settings.Snapshot; - public event EventHandler? Changed + public event EventHandler? Changed { add => _settings.Changed += value; remove => _settings.Changed -= value; } + public bool IsLocalChange( + SettingsChangedEventArgs eventArgs) => + _settings.IsLocalChange(eventArgs); + public string CreateAvailableName(string baseName) { var snapshot = Snapshot; diff --git a/tests/SightAdapt.Tests/RuntimeCoordinatorTests.cs b/tests/SightAdapt.Tests/RuntimeCoordinatorTests.cs index 43451c8e..a3a64833 100644 --- a/tests/SightAdapt.Tests/RuntimeCoordinatorTests.cs +++ b/tests/SightAdapt.Tests/RuntimeCoordinatorTests.cs @@ -254,8 +254,10 @@ public void DisableAutomaticMode() public void WireSettingsChanged() { - Settings.Changed += (_, _) => - Coordinator.HandleSettingsChanged(); + Settings.Changed += (_, eventArgs) => + Coordinator.HandleSettingsChanged( + Settings.Current, + eventArgs); } public void Dispose() diff --git a/tests/SightAdapt.Tests/SettingsCoordinatorTests.cs b/tests/SightAdapt.Tests/SettingsCoordinatorTests.cs index 49ff5b62..b7bbeb37 100644 --- a/tests/SightAdapt.Tests/SettingsCoordinatorTests.cs +++ b/tests/SightAdapt.Tests/SettingsCoordinatorTests.cs @@ -109,6 +109,52 @@ public void PersistCurrentDoesNotPublishChangedEvent() Assert.AreEqual(0, changedEvents); } + + [TestMethod] + public void ChangedEventPublishesOriginAndNestedCommitIndependently() + { + using var temporaryDirectory = + new TestWorkspace(); + var coordinator = + new SettingsCoordinator( + new SettingsStore(Path.Combine( + temporaryDirectory.Path, + "settings.json"))); + var outerOrigin = new object(); + var nestedOrigin = new object(); + var origins = new List(); + var nestedCommitted = false; + + coordinator.Changed += (_, eventArgs) => + { + origins.Add(eventArgs.Origin); + if (nestedCommitted) + { + return; + } + + nestedCommitted = true; + var nested = coordinator.Commit( + settings => + AutomaticModeManagementService + .Enable(settings), + nestedOrigin); + Assert.IsTrue(nested.Succeeded); + }; + + var outer = coordinator.Commit( + settings => + AutomaticModeManagementService + .Disable(settings), + outerOrigin); + + Assert.IsTrue(outer.Succeeded); + Assert.AreEqual(2, origins.Count); + Assert.AreSame(outerOrigin, origins[0]); + Assert.AreSame(nestedOrigin, origins[1]); + Assert.IsTrue(coordinator.Current.AutomaticMode); + } + [TestMethod] public void FailedPersistenceDoesNotPublishCandidateState() {