diff --git a/RadialActions.Tests/Settings/SettingsSerializationTests.cs b/RadialActions.Tests/Settings/SettingsSerializationTests.cs index bb552eb..29f73bb 100644 --- a/RadialActions.Tests/Settings/SettingsSerializationTests.cs +++ b/RadialActions.Tests/Settings/SettingsSerializationTests.cs @@ -109,4 +109,119 @@ public void DeserializeFromJson_MissingIsEnabled_DefaultsToTrue() Assert.Single(settings.Actions); Assert.True(settings.Actions[0].IsEnabled); } + + [Fact] + public void Load_MalformedPrimary_PreservesCorruptPrimaryBeforeDefaultsCanSave() + { + var directory = CreateTempDirectory(); + var settingsPath = Path.Combine(directory, "RadialActions.settings"); + File.WriteAllText(settingsPath, "{"); + + var settings = Settings.LoadAndInitializePersistenceState(settingsPath, out var canBeSaved); + + Assert.True(canBeSaved); + Assert.Equal(Settings.DefaultActivationHotkey, settings.ActivationHotkey); + Assert.Equal(Settings.DefaultSize, settings.Size); + Assert.Equal("{", File.ReadAllText(settingsPath)); + + var corruptPath = Assert.Single(Directory.GetFiles(directory, "RadialActions.settings.corrupt-*")); + Assert.Equal("{", File.ReadAllText(corruptPath)); + } + + [Fact] + public void Load_EmptyExistingFile_IsTreatedAsCorruptionNotFreshInstall() + { + var directory = CreateTempDirectory(); + var settingsPath = Path.Combine(directory, "RadialActions.settings"); + File.WriteAllText(settingsPath, string.Empty); + + var settings = Settings.LoadFromFileWithRecovery(settingsPath); + + Assert.Equal(Settings.DefaultActivationHotkey, settings.ActivationHotkey); + Assert.Equal(Settings.DefaultSize, settings.Size); + Assert.True(File.Exists(settingsPath)); + Assert.Single(Directory.GetFiles(directory, "RadialActions.settings.corrupt-*")); + } + + [Fact] + public void StartupWritableProbe_DoesNotRewriteSettingsFile() + { + var directory = CreateTempDirectory(); + var settingsPath = Path.Combine(directory, "RadialActions.settings"); + const string json = """ + { + "ActivationHotkey": "Ctrl+Alt+P", + "Size": 480 + } + """; + File.WriteAllText(settingsPath, json); + var lastWriteTime = new DateTime(2025, 1, 2, 3, 4, 5, DateTimeKind.Utc); + File.SetLastWriteTimeUtc(settingsPath, lastWriteTime); + + var settings = Settings.LoadAndInitializePersistenceState(settingsPath, out var canBeSaved); + + Assert.True(canBeSaved); + Assert.Equal("Ctrl+Alt+P", settings.ActivationHotkey); + Assert.Equal(json, File.ReadAllText(settingsPath)); + Assert.Equal(lastWriteTime, File.GetLastWriteTimeUtc(settingsPath)); + } + + [Fact] + public void Load_LockedUnreadablePrimary_DisablesSavingRecoveredDefaults() + { + var directory = CreateTempDirectory(); + var settingsPath = Path.Combine(directory, "RadialActions.settings"); + File.WriteAllText(settingsPath, "{"); + + using var lockedSettingsFile = new FileStream(settingsPath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + + var settings = Settings.LoadAndInitializePersistenceState(settingsPath, out var canBeSaved); + + Assert.False(canBeSaved); + Assert.Equal(Settings.DefaultActivationHotkey, settings.ActivationHotkey); + Assert.Empty(Directory.GetFiles(directory, "RadialActions.settings.corrupt-*")); + } + + [Fact] + public void Save_WritesAtomicallyAndLeavesNoTemporaryFileAfterSuccess() + { + var directory = CreateTempDirectory(); + var settingsPath = Path.Combine(directory, "RadialActions.settings"); + var settings = Settings.DeserializeFromJson("{}"); + settings.ActivationHotkey = "Ctrl+Shift+S"; + settings.Size = 640; + + var saved = settings.SaveToFile(settingsPath); + + Assert.True(saved); + Assert.Contains("Ctrl+Shift+S", File.ReadAllText(settingsPath)); + Assert.Empty(Directory.GetFiles(directory, "*.tmp")); + } + + [Fact] + public void Deserialize_PropertyLevelErrors_NormalizeWithoutDroppingUnrelatedValidSettings() + { + const string json = """ + { + "ActivationHotkey": "Ctrl+Alt+L", + "Size": 420, + "Actions": "not a collection", + "OpenMenuInScreenCenter": true + } + """; + + var settings = Settings.DeserializeFromJson(json); + + Assert.Equal("Ctrl+Alt+L", settings.ActivationHotkey); + Assert.Equal(420, settings.Size); + Assert.True(settings.OpenMenuInScreenCenter); + Assert.Equal(Settings.CreateDefaultActions().Count, settings.Actions.Count); + } + + private static string CreateTempDirectory() + { + var path = Path.Combine(Path.GetTempPath(), "RadialActions.Tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(path); + return path; + } } diff --git a/RadialActions/Properties/Settings.Engine.cs b/RadialActions/Properties/Settings.Engine.cs index a7b67ce..4a0c314 100644 --- a/RadialActions/Properties/Settings.Engine.cs +++ b/RadialActions/Properties/Settings.Engine.cs @@ -1,4 +1,5 @@ -using System.IO; +using System.Globalization; +using System.IO; using CommunityToolkit.Mvvm.ComponentModel; using Newtonsoft.Json; @@ -6,7 +7,16 @@ namespace RadialActions.Properties; public sealed partial class Settings : ObservableObject { - private static readonly Lazy _default = new(LoadAndAttemptSave); + private static readonly Lazy _default = new(LoadAndInitializePersistenceState); + + /// + /// Carries loaded settings plus whether future saves are allowed to write back to the same path. + /// + /// The settings object to use for the current app session. + /// + /// false when the primary settings file existed but could not be preserved before recovery. + /// + private sealed record SettingsLoadResult(Settings Settings, bool CanSaveSettings); private static readonly JsonSerializerSettings _jsonSerializerSettings = new() { @@ -60,9 +70,20 @@ private Settings() { } /// /// Saves to the default path in JSON format. /// + /// true when the default settings file was written successfully; otherwise, false. public bool Save() { - Log.Information($"Saving to {FilePath}"); + return SaveToFile(FilePath); + } + + /// + /// Saves this settings object to a specific path using an atomic file replacement. + /// + /// The primary settings file path to write. + /// true when the primary file was written successfully; otherwise, false. + internal bool SaveToFile(string filePath) + { + Log.Information("Saving to {FilePath}", filePath); try { @@ -73,18 +94,17 @@ public bool Save() { try { - File.WriteAllText(FilePath, json); + WriteAllTextAtomically(filePath, json); return true; } - catch + catch (Exception ex) { - // Wait before next attempt to read. - Log.Debug("Couldn't save file; Waiting a bit"); + Log.Debug(ex, "Couldn't save file; waiting a bit"); Thread.Sleep(250); } } } - catch (JsonSerializationException ex) + catch (JsonException ex) { Log.Error(ex, "Failed to save settings"); } @@ -92,11 +112,20 @@ public bool Save() return false; } + /// + /// Serializes this settings object to the app's JSON settings format. + /// + /// The formatted JSON settings document. public string SerializeToJson() { return JsonConvert.SerializeObject(this, _jsonSerializerSettings); } + /// + /// Deserializes a settings document and normalizes missing or invalid values. + /// + /// The JSON settings content to deserialize. + /// A normalized settings object. public static Settings DeserializeFromJson(string json) { var settings = new Settings(); @@ -110,46 +139,260 @@ public static Settings DeserializeFromJson(string json) } /// - /// Reads settings from the default path. + /// Reads settings from the specified path. /// - private static string ReadJsonFromFile() + /// The settings file path to read. + /// The complete JSON file content. + private static string ReadJsonFromFile(string filePath) { - using var fileStream = new FileStream(FilePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); + using var fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); using var streamReader = new StreamReader(fileStream); return streamReader.ReadToEnd(); } /// - /// Loads from the default path in JSON format. + /// Loads settings from the specified path with recovery behavior for unreadable primary files. + /// + /// The primary settings file path. + /// The settings object to use for the current app session. + internal static Settings LoadFromFileWithRecovery(string filePath) + => LoadFromFileWithRecoveryResult(filePath).Settings; + + /// + /// Loads settings and records whether it is safe to save back to the primary path later. /// - private static Settings LoadFromFile() + /// The primary settings file path. + /// + /// A load result containing settings plus a save gate that protects unreadable files that could not be preserved. + /// + private static SettingsLoadResult LoadFromFileWithRecoveryResult(string filePath) { + if (!File.Exists(filePath)) + { + Log.Information("Creating new settings because {FilePath} does not exist", filePath); + return new SettingsLoadResult(CreateDefaultSettings(), CanSaveSettings: true); + } + try { - var json = ReadJsonFromFile(); - return DeserializeFromJson(json); + var settings = LoadSettingsFileOrThrow(filePath); + return new SettingsLoadResult(settings, CanSaveSettings: true); } catch (Exception ex) { - Log.Error(ex, $"Failed to load {FilePath}"); - Log.Information("Creating new settings"); - var settings = new Settings(); - settings.NormalizeAfterLoad(); - return settings; + return RecoverFromPrimaryLoadFailure(filePath, ex); } } /// - /// Loads from the default path in JSON format then attempts to save in order to check if it can be done. + /// Loads from the default path in JSON format and probes whether the directory is writable. + /// + /// The singleton settings object for the default settings file path. + private static Settings LoadAndInitializePersistenceState() + { + var settings = LoadAndInitializePersistenceState(FilePath, out var canBeSaved); + CanBeSaved = canBeSaved; + Log.Debug("Settings can be saved: {CanBeSaved}", CanBeSaved); + + return settings; + } + + /// + /// Loads settings from a specific path and determines whether later saves should be enabled. + /// + /// The primary settings file path. + /// + /// Set to true only when recovery did not need to suppress saves and the directory is writable. + /// + /// The settings object to use for the current app session. + internal static Settings LoadAndInitializePersistenceState(string filePath, out bool canBeSaved) + { + var loadResult = LoadFromFileWithRecoveryResult(filePath); + canBeSaved = loadResult.CanSaveSettings && ProbeSettingsDirectoryWritable(filePath); + return loadResult.Settings; + } + + /// + /// Checks whether the settings directory can be written without modifying the real settings file. /// - private static Settings LoadAndAttemptSave() + /// The primary settings file path whose directory should be probed. + /// true when a throwaway probe file can be created and deleted; otherwise, false. + internal static bool ProbeSettingsDirectoryWritable(string filePath) { - var settings = LoadFromFile(); + var directoryPath = Path.GetDirectoryName(filePath); + if (string.IsNullOrWhiteSpace(directoryPath)) + { + directoryPath = "."; + } - CanBeSaved = settings.Save(); - Log.Debug($"Settings can be saved: {CanBeSaved}"); + var probePath = Path.Combine(directoryPath, $".settings-write-probe-{Guid.NewGuid():N}.tmp"); + try + { + Directory.CreateDirectory(directoryPath); + File.WriteAllText(probePath, string.Empty); + File.Delete(probePath); + return true; + } + catch (Exception ex) + { + Log.Warning(ex, "Settings directory is not writable"); + return false; + } + finally + { + TryDeleteFile(probePath); + } + } + + /// + /// Creates a default settings object and runs the normal post-load normalization pass. + /// + /// A normalized default settings object. + private static Settings CreateDefaultSettings() + { + var settings = new Settings(); + settings.NormalizeAfterLoad(); return settings; } + /// + /// Reads and deserializes one settings file, treating an empty existing file as corruption. + /// + /// The settings file path to load. + /// A normalized settings object. + /// Thrown when the file exists but contains no JSON content. + private static Settings LoadSettingsFileOrThrow(string filePath) + { + var json = ReadJsonFromFile(filePath); + if (string.IsNullOrWhiteSpace(json)) + { + throw new InvalidDataException($"Settings file is empty: {filePath}"); + } + + return DeserializeFromJson(json); + } + + /// + /// Recovers from a failed primary settings load without silently overwriting recoverable user data. + /// + /// The primary settings file path that failed to load. + /// The exception raised while loading the primary file. + /// + /// Recovered settings and a save gate. Saves are disabled when the unreadable primary file could not be copied aside. + /// + private static SettingsLoadResult RecoverFromPrimaryLoadFailure(string filePath, Exception loadException) + { + Log.Error(loadException, "Failed to load {FilePath}", filePath); + + var canSaveDefaults = PreserveCorruptSettingsFile(filePath); + if (!canSaveDefaults) + { + Log.Warning("Default settings will not be saved automatically because the unreadable primary file could not be preserved"); + } + + Log.Information("Creating new settings in memory after unreadable settings file"); + return new SettingsLoadResult(CreateDefaultSettings(), canSaveDefaults); + } + + /// + /// Copies an unreadable primary settings file to a timestamped corrupt snapshot path. + /// + /// The primary settings file path to preserve. + /// + /// true when no primary file exists or the copy succeeds; false when recovery should avoid saving. + /// + private static bool PreserveCorruptSettingsFile(string filePath) + { + if (!File.Exists(filePath)) + return true; + + var corruptPath = CreateCorruptFilePath(filePath); + + try + { + using var source = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + using var destination = new FileStream(corruptPath, FileMode.CreateNew, FileAccess.Write, FileShare.None); + source.CopyTo(destination); + Log.Warning("Preserved unreadable settings file at {CorruptPath}", corruptPath); + return true; + } + catch (Exception ex) + { + Log.Warning(ex, "Failed to preserve unreadable settings file {FilePath}", filePath); + return false; + } + } + + /// + /// Creates a unique timestamped corrupt snapshot path for a settings file. + /// + /// The primary settings file path. + /// A non-existing path using the `.corrupt-*` naming pattern. + private static string CreateCorruptFilePath(string filePath) + { + var timestamp = DateTimeOffset.UtcNow.ToString("yyyyMMddHHmmss", CultureInfo.InvariantCulture); + for (var attempt = 0; ; attempt++) + { + var suffix = attempt == 0 ? string.Empty : $"-{attempt}"; + var corruptPath = $"{filePath}.corrupt-{timestamp}{suffix}"; + if (!File.Exists(corruptPath)) + { + return corruptPath; + } + } + } + + /// + /// Writes text through a same-directory temporary file and atomically replaces the destination when possible. + /// + /// The destination file path. + /// The complete content to write. + private static void WriteAllTextAtomically(string filePath, string contents) + { + var directoryPath = Path.GetDirectoryName(filePath); + if (string.IsNullOrWhiteSpace(directoryPath)) + { + directoryPath = "."; + } + + Directory.CreateDirectory(directoryPath); + var tempFilePath = Path.Combine(directoryPath, $"{Path.GetFileName(filePath)}.{Guid.NewGuid():N}.tmp"); + + try + { + File.WriteAllText(tempFilePath, contents); + if (File.Exists(filePath)) + { + File.Replace(tempFilePath, filePath, null); + } + else + { + File.Move(tempFilePath, filePath); + } + } + finally + { + TryDeleteFile(tempFilePath); + } + } + + /// + /// Deletes a file when it exists, logging and ignoring cleanup failures. + /// + /// The file path to delete. + private static void TryDeleteFile(string filePath) + { + try + { + if (File.Exists(filePath)) + { + File.Delete(filePath); + } + } + catch (Exception ex) + { + Log.Debug(ex, "Failed to delete temporary file {FilePath}", filePath); + } + } }