refactor(securestoremanager): add platform service abstractions and constants

Implement deferred code review findings: - Add IDialogService/IClipboardService interfaces for testable platform operations - Create AvaloniaDialogService and AvaloniaClipboardService implementations - Extract dialog strings and file extensions to centralized Constants classes - Refactor ViewModels to use DI instead of event delegates - Update tests to use mock services
2026-01-19 16:54:35 -05:00
parent 1c546c111a
commit fbe58a81e4
33 changed files with 1790 additions and 298 deletions
@@ -1,5 +1,6 @@
 using System.Collections.ObjectModel;
 using System.Windows.Input;
+using JdeScoping.SecureStoreManager.Constants;
 using JdeScoping.SecureStoreManager.Services;

 namespace JdeScoping.SecureStoreManager.ViewModels;
@@ -10,33 +11,36 @@ namespace JdeScoping.SecureStoreManager.ViewModels;
 public class MainWindowViewModel : ViewModelBase
 {
    private readonly ISecureStoreManager _storeManager;
+    private readonly IDialogService _dialogService;
+    private readonly IClipboardService _clipboardService;
    private SecretItemViewModel? _selectedSecret;
    private string _statusMessage = "Ready";

-    public MainWindowViewModel() : this(new Services.SecureStoreManager())
+    public MainWindowViewModel(
+        ISecureStoreManager storeManager,
+        IDialogService dialogService,
+        IClipboardService clipboardService)
    {
-    }
-
-    public MainWindowViewModel(ISecureStoreManager storeManager)
-    {
-        _storeManager = storeManager;
+        _storeManager = storeManager ?? throw new ArgumentNullException(nameof(storeManager));
+        _dialogService = dialogService ?? throw new ArgumentNullException(nameof(dialogService));
+        _clipboardService = clipboardService ?? throw new ArgumentNullException(nameof(clipboardService));
        Secrets = new ObservableCollection<SecretItemViewModel>();

-        // File commands
-        NewStoreCommand = new RelayCommand(ExecuteNewStore);
-        OpenStoreCommand = new RelayCommand(ExecuteOpenStore);
-        SaveCommand = new RelayCommand(ExecuteSave, CanSave);
-        CloseStoreCommand = new RelayCommand(ExecuteCloseStore, () => _storeManager.IsStoreOpen);
-        ExitCommand = new RelayCommand(ExecuteExit);
+        // File commands (async)
+        NewStoreCommand = new AsyncRelayCommand(ExecuteNewStoreAsync);
+        OpenStoreCommand = new AsyncRelayCommand(ExecuteOpenStoreAsync);
+        SaveCommand = new AsyncRelayCommand(ExecuteSaveAsync, CanSave);
+        CloseStoreCommand = new AsyncRelayCommand(ExecuteCloseStoreAsync, () => _storeManager.IsStoreOpen);
+        ExitCommand = new AsyncRelayCommand(ExecuteExitAsync);

        // Secret commands
        AddSecretCommand = new RelayCommand(ExecuteAddSecret, () => _storeManager.IsStoreOpen);
        EditSecretCommand = new RelayCommand(ExecuteEditSecret, CanEditOrDeleteSecret);
-        DeleteSecretCommand = new RelayCommand(ExecuteDeleteSecret, CanEditOrDeleteSecret);
+        DeleteSecretCommand = new AsyncRelayCommand(ExecuteDeleteSecretAsync, CanEditOrDeleteSecret);

-        // Tools commands
-        GenerateKeyFileCommand = new RelayCommand(ExecuteGenerateKeyFile);
-        ExportKeyCommand = new RelayCommand(ExecuteExportKey, () => _storeManager.IsStoreOpen);
+        // Tools commands (async)
+        GenerateKeyFileCommand = new AsyncRelayCommand(ExecuteGenerateKeyFileAsync);
+        ExportKeyCommand = new AsyncRelayCommand(ExecuteExportKeyAsync, () => _storeManager.IsStoreOpen);
    }

    /// <summary>
@@ -50,7 +54,14 @@ public class MainWindowViewModel : ViewModelBase
    public SecretItemViewModel? SelectedSecret
    {
        get => _selectedSecret;
-        set => SetProperty(ref _selectedSecret, value);
+        set
+        {
+            if (SetProperty(ref _selectedSecret, value))
+            {
+                (EditSecretCommand as RelayCommand)?.RaiseCanExecuteChanged();
+                (DeleteSecretCommand as AsyncRelayCommand)?.RaiseCanExecuteChanged();
+            }
+        }
    }

    /// <summary>
@@ -134,7 +145,9 @@ public class MainWindowViewModel : ViewModelBase
        catch (Exception ex)
        {
            StatusMessage = $"Error creating store: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to create store:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToCreateStoreFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

@@ -166,7 +179,9 @@ public class MainWindowViewModel : ViewModelBase
        catch (Exception ex)
        {
            StatusMessage = $"Error opening store: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to open store:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToOpenStoreFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

@@ -185,7 +200,9 @@ public class MainWindowViewModel : ViewModelBase
        catch (Exception ex)
        {
            StatusMessage = $"Error saving secret: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to save secret:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToSaveSecretFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

@@ -198,10 +215,7 @@ public class MainWindowViewModel : ViewModelBase
        if (!_storeManager.HasUnsavedChanges)
            return true;

-        if (OnShowUnsavedChangesPrompt == null)
-            return true;
-
-        var result = await OnShowUnsavedChangesPrompt();
+        var result = await _dialogService.ShowUnsavedChangesPromptAsync();

        switch (result)
        {
@@ -216,7 +230,7 @@ public class MainWindowViewModel : ViewModelBase
        }
    }

-    private async void ExecuteNewStore()
+    private async Task ExecuteNewStoreAsync()
    {
        if (!await PromptForUnsavedChangesAsync())
            return;
@@ -225,7 +239,7 @@ public class MainWindowViewModel : ViewModelBase
        OnRequestNewStoreDialog?.Invoke();
    }

-    private async void ExecuteOpenStore()
+    private async Task ExecuteOpenStoreAsync()
    {
        if (!await PromptForUnsavedChangesAsync())
            return;
@@ -234,11 +248,6 @@ public class MainWindowViewModel : ViewModelBase
        OnRequestOpenStoreDialog?.Invoke();
    }

-    private async void ExecuteSave()
-    {
-        await ExecuteSaveAsync();
-    }
-
    private async Task ExecuteSaveAsync()
    {
        try
@@ -250,13 +259,15 @@ public class MainWindowViewModel : ViewModelBase
        catch (Exception ex)
        {
            StatusMessage = $"Error saving: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to save store:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToSaveStoreFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

    private bool CanSave() => _storeManager.IsStoreOpen && _storeManager.HasUnsavedChanges;

-    private async void ExecuteCloseStore()
+    private async Task ExecuteCloseStoreAsync()
    {
        if (!await PromptForUnsavedChangesAsync())
            return;
@@ -267,7 +278,7 @@ public class MainWindowViewModel : ViewModelBase
        StatusMessage = "Store closed";
    }

-    private async void ExecuteExit()
+    private async Task ExecuteExitAsync()
    {
        if (!await PromptForUnsavedChangesAsync())
            return;
@@ -290,15 +301,13 @@ public class MainWindowViewModel : ViewModelBase
        OnRequestEditSecretDialog?.Invoke(SelectedSecret.Key, SelectedSecret.ActualValue);
    }

-    private async void ExecuteDeleteSecret()
+    private async Task ExecuteDeleteSecretAsync()
    {
        if (SelectedSecret == null)
            return;

-        if (OnShowDeleteConfirmation == null)
-            return;
-
-        var confirmed = await OnShowDeleteConfirmation(SelectedSecret.Key);
+        var confirmMessage = string.Format(DialogStrings.ConfirmDeleteFormat, SelectedSecret.Key);
+        var confirmed = await _dialogService.ShowConfirmationAsync(confirmMessage, DialogStrings.ConfirmDeleteTitle);
        if (!confirmed)
            return;

@@ -313,18 +322,22 @@ public class MainWindowViewModel : ViewModelBase
        catch (Exception ex)
        {
            StatusMessage = $"Error deleting secret: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to delete secret:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToDeleteSecretFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

    private bool CanEditOrDeleteSecret() => _storeManager.IsStoreOpen && SelectedSecret != null;

-    private async void ExecuteGenerateKeyFile()
+    private async Task ExecuteGenerateKeyFileAsync()
    {
-        if (OnShowSaveFileDialog == null)
-            return;
+        var filePath = await _dialogService.ShowSaveFileDialogAsync(
+            DialogStrings.GenerateKeyFileTitle,
+            FileExtensions.KeyTypeName,
+            FileExtensions.KeyPattern,
+            FileExtensions.KeyExtension);

-        var filePath = await OnShowSaveFileDialog("Generate Key File", "Key Files", "*.key", ".key");
        if (string.IsNullOrEmpty(filePath))
            return;

@@ -332,21 +345,27 @@ public class MainWindowViewModel : ViewModelBase
        {
            _storeManager.GenerateKeyFile(filePath);
            StatusMessage = $"Generated key file: {filePath}";
-            await (OnShowInfo?.Invoke($"Key file generated successfully:\n\n{filePath}", "Key Generated") ?? Task.CompletedTask);
+            await _dialogService.ShowInfoAsync(
+                string.Format(DialogStrings.KeyFileGeneratedFormat, filePath),
+                DialogStrings.KeyGeneratedTitle);
        }
        catch (Exception ex)
        {
            StatusMessage = $"Error generating key: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to generate key file:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToGenerateKeyFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

-    private async void ExecuteExportKey()
+    private async Task ExecuteExportKeyAsync()
    {
-        if (OnShowSaveFileDialog == null)
-            return;
+        var filePath = await _dialogService.ShowSaveFileDialogAsync(
+            DialogStrings.ExportKeyTitle,
+            FileExtensions.KeyTypeName,
+            FileExtensions.KeyPattern,
+            FileExtensions.KeyExtension);

-        var filePath = await OnShowSaveFileDialog("Export Key", "Key Files", "*.key", ".key");
        if (string.IsNullOrEmpty(filePath))
            return;

@@ -354,12 +373,16 @@ public class MainWindowViewModel : ViewModelBase
        {
            _storeManager.ExportKey(filePath);
            StatusMessage = $"Exported key to: {filePath}";
-            await (OnShowInfo?.Invoke($"Key exported successfully:\n\n{filePath}", "Key Exported") ?? Task.CompletedTask);
+            await _dialogService.ShowInfoAsync(
+                string.Format(DialogStrings.KeyExportedFormat, filePath),
+                DialogStrings.KeyExportedTitle);
        }
        catch (Exception ex)
        {
            StatusMessage = $"Error exporting key: {ex.Message}";
-            await (OnShowError?.Invoke($"Failed to export key:\n\n{ex.Message}", "Error") ?? Task.CompletedTask);
+            await _dialogService.ShowErrorAsync(
+                string.Format(DialogStrings.FailedToExportKeyFormat, ex.Message),
+                DialogStrings.ErrorTitle);
        }
    }

@@ -372,7 +395,7 @@ public class MainWindowViewModel : ViewModelBase
        foreach (var key in _storeManager.GetKeys())
        {
            var value = _storeManager.GetSecret(key);
-            Secrets.Add(new SecretItemViewModel(key, value));
+            Secrets.Add(new SecretItemViewModel(key, value, _clipboardService));
        }
    }

@@ -383,35 +406,18 @@ public class MainWindowViewModel : ViewModelBase
        OnPropertyChanged(nameof(WindowTitle));

        // Manually raise CanExecuteChanged for all commands
-        (SaveCommand as RelayCommand)?.RaiseCanExecuteChanged();
-        (CloseStoreCommand as RelayCommand)?.RaiseCanExecuteChanged();
+        (SaveCommand as AsyncRelayCommand)?.RaiseCanExecuteChanged();
+        (CloseStoreCommand as AsyncRelayCommand)?.RaiseCanExecuteChanged();
        (AddSecretCommand as RelayCommand)?.RaiseCanExecuteChanged();
        (EditSecretCommand as RelayCommand)?.RaiseCanExecuteChanged();
-        (DeleteSecretCommand as RelayCommand)?.RaiseCanExecuteChanged();
-        (ExportKeyCommand as RelayCommand)?.RaiseCanExecuteChanged();
+        (DeleteSecretCommand as AsyncRelayCommand)?.RaiseCanExecuteChanged();
+        (ExportKeyCommand as AsyncRelayCommand)?.RaiseCanExecuteChanged();
    }

-    // Events for view to show dialogs (sync)
+    // Events for view to show dialogs (these require view-specific DataContext setup)
    public event Action? OnRequestNewStoreDialog;
    public event Action? OnRequestOpenStoreDialog;
    public event Action? OnRequestAddSecretDialog;
    public event Action<string, string>? OnRequestEditSecretDialog;
    public event Action? OnRequestClose;
-
-    // Events for view to show dialogs (async)
-    public event Func<string, string, Task>? OnShowError;
-    public event Func<string, string, Task>? OnShowInfo;
-    public event Func<Task<UnsavedChangesResult>>? OnShowUnsavedChangesPrompt;
-    public event Func<string, Task<bool>>? OnShowDeleteConfirmation;
-    public event Func<string, string, string, string, Task<string?>>? OnShowSaveFileDialog;
-}
-
-/// <summary>
-/// Result from unsaved changes prompt.
-/// </summary>
-public enum UnsavedChangesResult
-{
-    Save,
-    DontSave,
-    Cancel
 }