fix(settings): keep stored preferences when the file is shared or unreadable
A load that hit a sharing violation returned defaults, and the next save wrote them back over the stored themes, CSS, editors and recent files. Two processes over one data folder also erased each other's changes: each did load, modify, save with no lock and one shared temporary file name. AppSettingsStore applies one change to the latest stored settings under a lock file beside settings.json and replaces the file by rename. A file that cannot be read after bounded retries is never overwritten; only invalid content counts as corruption and is kept as settings.json.corrupt. A file over 16 MiB is not loaded, abandoned temporary files are removed, and faults are logged by kind, exception type and HRESULT only. Every save in the app now goes through Update. Tests use real exclusive handles and concurrent instances and count attempts instead of time. Closes #46 Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: James Maes <james@kof22.com>
This commit is contained in:
1 parent
8763ff3206
commit
6919d3ce4a
17 files changed
+763
-45
No files matched your search
@@ -30,7 +30,7 @@ and isolated boundary evidence in ADR-0005. Explorer and installed lifecycle
|
||||
acceptance remain required; literal process arguments alone are not association
|
||||
proof.
|
||||
|
||||
Store at most20 successful absolute paths in the private AppKit settings file,
|
||||
Store at most20 successful absolute paths in the private settings file,
|
||||
deduplicated with Windows path comparison. Update only after a parsed snapshot
|
||||
commits. Clear history removes entries without deleting files or closing readers.
|
||||
Keep paths out of activity logs/telemetry. Each preference save merges only fields
|
||||
|
||||
@@ -34,7 +34,7 @@ physical directory. It does not authorize native commands or remote resource acc
|
||||
| Remote resources | Denied by CSP and resource interception |
|
||||
| External links | HTTP(S) only, actual browser user gesture, external Windows handler; no document navigation outside current page |
|
||||
| Heading navigation | Labels extracted after sanitation; authored identity attributes stripped; generated numeric targets only. Fixed host operations reveal/jump/collapse on the current intercepted page; authored scripts, messages and host objects remain disabled |
|
||||
| Settings / logs | AppKit sanitation; per-user data root; log operational events without document content or paths |
|
||||
| Settings / logs | AppKit sanitation; per-user data root; log operational events without document content or paths. Settings changes are read-modify-write under a lock file beside `settings.json` and replace it by rename; an unreadable file is never overwritten, invalid content is kept as `settings.json.corrupt`, a file over 16 MiB is not loaded, and faults are logged by kind, exception type and HRESULT only |
|
||||
| Updates | Repository HTTPS feed, Velopack integrity checks and production signing preflight |
|
||||
| Repository / CI | Protected main and release tags; producer-bound checks; no untrusted PR checkout with privileged review token |
|
||||
|
||||
|
||||
@@ -73,6 +73,7 @@ Documents are rendered locally. Authored scripts and remote images are blocked;
|
||||
- **Open in editor is disabled:** hover for the reason, then configure an installed editor in Settings.
|
||||
- **The viewer is not a default option:** use Setup rather than a portable copy, then reopen Windows Default Apps.
|
||||
- **The app does not start:** confirm Microsoft Edge WebView2 Runtime is installed and inspect the activity logs if available.
|
||||
- **A setting could not be saved:** another program is holding `settings.json`, or the folder is read-only. Nothing is overwritten while the file cannot be read; try the change again. If the file was edited by hand and is no longer valid, the app starts with defaults and keeps the invalid file as `settings.json.corrupt` on the next save.
|
||||
|
||||
For help, use [Discussions](https://github.com/KofTwentyTwo/MarkdownViewr.Windows/discussions). Report reproducible problems through [Issues](https://github.com/KofTwentyTwo/MarkdownViewr.Windows/issues/new/choose), including **Help → About** version, Windows version, and relevant sanitized logs. Report vulnerabilities privately using [SECURITY.md](../SECURITY.md).
|
||||
|
||||
|
||||
@@ -5,7 +5,6 @@
|
||||
|
||||
using KofTwentyTwo.AppKit;
|
||||
using KofTwentyTwo.AppKit.Logging;
|
||||
using KofTwentyTwo.AppKit.Settings;
|
||||
|
||||
|
||||
namespace MarkdownViewr.Windows.Core;
|
||||
@@ -73,7 +72,7 @@ public static class AppIdentity
|
||||
|
||||
|
||||
/// <summary>Loads and saves <see cref="AppSettings"/> at <see cref="AppPaths.SettingsFile"/>.</summary>
|
||||
public static SettingsStore<AppSettings> SettingsStore { get; } = new(Paths.SettingsFile, AppSettingsContext.Default.AppSettings);
|
||||
public static AppSettingsStore SettingsStore { get; } = new(Paths.SettingsFile);
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,250 @@
|
||||
/*
|
||||
* Copyright (c) 2026 James Maes
|
||||
* SPDX-License-Identifier: MIT
|
||||
*/
|
||||
|
||||
using System.Text;
|
||||
using System.Text.Json;
|
||||
|
||||
|
||||
namespace MarkdownViewr.Windows.Core;
|
||||
|
||||
/// <summary>
|
||||
/// The settings file shared by every window and every process of one edition. A change is
|
||||
/// applied to the latest stored settings under a cross-process lock and replaces the file
|
||||
/// atomically, so two instances cannot erase each other's change. A file that cannot be
|
||||
/// read is never treated as empty: only invalid content is replaced, and it is kept aside.
|
||||
/// </summary>
|
||||
public sealed class AppSettingsStore
|
||||
{
|
||||
/// <summary>Attempts for the lock, a read, and the final replace before the operation gives up.</summary>
|
||||
public const int MaximumAttempts = 20;
|
||||
|
||||
/// <summary>A larger file is not this application's settings and is never loaded into memory.</summary>
|
||||
public const int MaximumBytes = 16 * 1024 * 1024;
|
||||
|
||||
private static readonly TimeSpan s_retryDelay = TimeSpan.FromMilliseconds(50);
|
||||
private readonly Action<SettingsFault, Exception?> _report;
|
||||
private readonly Action<TimeSpan> _wait;
|
||||
|
||||
|
||||
|
||||
/// <summary>Creates the store; faults go to <paramref name="report"/>, and tests supply <paramref name="wait"/> to count attempts instead of sleeping.</summary>
|
||||
public AppSettingsStore(string path, Action<SettingsFault, Exception?>? report = null, Action<TimeSpan>? wait = null)
|
||||
{
|
||||
ArgumentException.ThrowIfNullOrWhiteSpace(path);
|
||||
Path = System.IO.Path.GetFullPath(path);
|
||||
_report = report ?? ((_, _) => { });
|
||||
_wait = wait ?? Thread.Sleep;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>The settings file.</summary>
|
||||
public string Path { get; }
|
||||
|
||||
|
||||
|
||||
/// <summary>Reads the stored settings for display. A missing, invalid, or unreadable file yields defaults and is left untouched.</summary>
|
||||
public AppSettings Load()
|
||||
{
|
||||
_ = TryRead(out AppSettings settings);
|
||||
return settings;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Applies one change to the latest stored settings and saves the result. Returns the saved
|
||||
/// settings, or null when nothing was written: the lock stayed busy, the file could not be
|
||||
/// read, or the write failed. The previous file is intact in every null case.
|
||||
/// </summary>
|
||||
public AppSettings? Update(Func<AppSettings, AppSettings> change)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(change);
|
||||
try
|
||||
{
|
||||
Directory.CreateDirectory(System.IO.Path.GetDirectoryName(Path)!);
|
||||
using FileStream? gate = Acquire();
|
||||
if(gate is null)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
SettingsRead read = TryRead(out AppSettings current);
|
||||
if(read == SettingsRead.Unreadable)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
if(read == SettingsRead.Corrupt)
|
||||
{
|
||||
Retry(() => File.Move(Path, Path + ".corrupt", overwrite: true));
|
||||
}
|
||||
AppSettings edited = change(current);
|
||||
edited.Sanitize();
|
||||
RemoveAbandonedTemporaryFiles();
|
||||
Replace(JsonSerializer.SerializeToUtf8Bytes(edited, AppSettingsContext.Default.AppSettings));
|
||||
return edited;
|
||||
}
|
||||
catch(Exception exception) when(exception is IOException or UnauthorizedAccessException)
|
||||
{
|
||||
_report(SettingsFault.WriteFailed, exception);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Holds an exclusive handle beside the settings file; the operating system releases it if the process dies.</summary>
|
||||
private FileStream? Acquire()
|
||||
{
|
||||
try
|
||||
{
|
||||
FileStream? gate = null;
|
||||
Retry(() => gate = new FileStream(Path + ".lock", FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None));
|
||||
return gate;
|
||||
}
|
||||
catch(IOException exception) when(IsTransient(exception))
|
||||
{
|
||||
_report(SettingsFault.Busy, exception);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Distinguishes a missing file, invalid content, and a file that could not be read; only the last keeps the caller from writing.</summary>
|
||||
private SettingsRead TryRead(out AppSettings settings)
|
||||
{
|
||||
settings = new AppSettings();
|
||||
SettingsRead result;
|
||||
try
|
||||
{
|
||||
string? json = null;
|
||||
Retry(() => json = ReadText());
|
||||
AppSettings? loaded = json is null ? null : JsonSerializer.Deserialize(json, AppSettingsContext.Default.AppSettings);
|
||||
if(loaded is null)
|
||||
{
|
||||
_report(SettingsFault.Corrupt, null);
|
||||
result = SettingsRead.Corrupt;
|
||||
}
|
||||
else
|
||||
{
|
||||
settings = loaded;
|
||||
result = SettingsRead.Loaded;
|
||||
}
|
||||
}
|
||||
catch(Exception exception) when(exception is FileNotFoundException or DirectoryNotFoundException)
|
||||
{
|
||||
result = SettingsRead.Missing;
|
||||
}
|
||||
catch(JsonException exception)
|
||||
{
|
||||
_report(SettingsFault.Corrupt, exception);
|
||||
result = SettingsRead.Corrupt;
|
||||
}
|
||||
catch(Exception exception) when(exception is IOException or UnauthorizedAccessException)
|
||||
{
|
||||
_report(SettingsFault.Unreadable, exception);
|
||||
result = SettingsRead.Unreadable;
|
||||
}
|
||||
settings.Sanitize();
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Reads without blocking a concurrent replace; an oversized file reads as invalid content instead of exhausting memory.</summary>
|
||||
private string? ReadText()
|
||||
{
|
||||
using var stream = new FileStream(Path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
|
||||
if(stream.Length > MaximumBytes)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true);
|
||||
return reader.ReadToEnd();
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Writes beside the file, flushes to disk, then renames over it; a failure leaves the previous file and no temporary file.</summary>
|
||||
private void Replace(byte[] content)
|
||||
{
|
||||
string temporary = Path + "." + Guid.NewGuid().ToString("N") + ".tmp";
|
||||
bool replaced = false;
|
||||
try
|
||||
{
|
||||
using(var stream = new FileStream(temporary, FileMode.CreateNew, FileAccess.Write, FileShare.None))
|
||||
{
|
||||
stream.Write(content);
|
||||
stream.Flush(flushToDisk: true);
|
||||
}
|
||||
Retry(() => File.Move(temporary, Path, overwrite: true));
|
||||
replaced = true;
|
||||
}
|
||||
finally
|
||||
{
|
||||
if(!replaced)
|
||||
{
|
||||
TryDelete(temporary);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A writer that died before its rename left a temporary file; the lock holder is the only writer, so every one found is abandoned.</summary>
|
||||
private void RemoveAbandonedTemporaryFiles()
|
||||
{
|
||||
foreach(string file in Directory.EnumerateFiles(System.IO.Path.GetDirectoryName(Path)!, System.IO.Path.GetFileName(Path) + "*.tmp"))
|
||||
{
|
||||
TryDelete(file);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Repeats an operation only for a sharing violation or a pending delete, a bounded number of times; the last failure is thrown.</summary>
|
||||
private void Retry(Action operation)
|
||||
{
|
||||
for(int attempt = 1; ; attempt++)
|
||||
{
|
||||
try
|
||||
{
|
||||
operation();
|
||||
return;
|
||||
}
|
||||
catch(Exception exception) when(attempt < MaximumAttempts && IsTransient(exception))
|
||||
{
|
||||
_wait(s_retryDelay);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Another handle on the file (an editor, a scanner, a second instance) or a delete that has not completed; a missing file is not transient.</summary>
|
||||
private static bool IsTransient(Exception exception)
|
||||
{
|
||||
if(exception is UnauthorizedAccessException)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
return exception is IOException and not (FileNotFoundException or DirectoryNotFoundException) && (exception.HResult & 0xFFFF) is 32 or 33;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Cleanup only: a temporary file that cannot be removed now is removed by the next successful save.</summary>
|
||||
private static void TryDelete(string path)
|
||||
{
|
||||
try
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
catch(Exception exception) when(exception is IOException or UnauthorizedAccessException)
|
||||
{
|
||||
// The outcome of the save is already decided.
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
/*
|
||||
* Copyright (c) 2026 James Maes
|
||||
* SPDX-License-Identifier: MIT
|
||||
*/
|
||||
|
||||
|
||||
namespace MarkdownViewr.Windows.Core;
|
||||
|
||||
/// <summary>Why the settings file could not be used as stored; reported without paths or operating-system message text.</summary>
|
||||
public enum SettingsFault
|
||||
{
|
||||
/// <summary>The file exists but does not hold valid settings; defaults are used and the next save keeps the invalid file aside.</summary>
|
||||
Corrupt,
|
||||
|
||||
/// <summary>The file could not be read after bounded retries; it is left untouched and nothing is written over it.</summary>
|
||||
Unreadable,
|
||||
|
||||
/// <summary>Another process kept the settings lock for every attempt; the change was not saved.</summary>
|
||||
Busy,
|
||||
|
||||
/// <summary>The change could not be written; the previous file remains in place.</summary>
|
||||
WriteFailed,
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
/*
|
||||
* Copyright (c) 2026 James Maes
|
||||
* SPDX-License-Identifier: MIT
|
||||
*/
|
||||
|
||||
|
||||
namespace MarkdownViewr.Windows.Core;
|
||||
|
||||
/// <summary>What reading the settings file found; only <see cref="Unreadable"/> forbids writing over it.</summary>
|
||||
internal enum SettingsRead
|
||||
{
|
||||
/// <summary>Valid stored settings.</summary>
|
||||
Loaded,
|
||||
|
||||
/// <summary>No file yet; defaults apply and the first save creates it.</summary>
|
||||
Missing,
|
||||
|
||||
/// <summary>Invalid content; defaults apply and a save keeps the invalid file aside.</summary>
|
||||
Corrupt,
|
||||
|
||||
/// <summary>The content is unknown, so it must not be replaced.</summary>
|
||||
Unreadable,
|
||||
}
|
||||
@@ -107,12 +107,14 @@ public partial class App
|
||||
|
||||
|
||||
|
||||
/// <summary>Records a successfully committed path through the latest store snapshot, then refreshes every recent menu.</summary>
|
||||
/// <summary>Records a successfully committed path in the latest stored history under the settings lock, then refreshes every recent menu.</summary>
|
||||
internal bool RecordRecentDocument(string path)
|
||||
{
|
||||
Core.AppSettings settings = AppIdentity.SettingsStore.Load();
|
||||
settings.RecentFiles = Core.Documents.RecentDocuments.Record(settings.RecentFiles, path);
|
||||
bool saved = AppIdentity.SettingsStore.Save(settings);
|
||||
bool saved = AppIdentity.SettingsStore.Update(settings =>
|
||||
{
|
||||
settings.RecentFiles = Core.Documents.RecentDocuments.Record(settings.RecentFiles, path);
|
||||
return settings;
|
||||
}) is not null;
|
||||
RefreshRecentMenus();
|
||||
return saved;
|
||||
}
|
||||
@@ -122,9 +124,11 @@ public partial class App
|
||||
/// <summary>Clears only the local history, leaving open documents, reading preferences and files intact.</summary>
|
||||
internal bool ClearRecentDocuments()
|
||||
{
|
||||
Core.AppSettings settings = AppIdentity.SettingsStore.Load();
|
||||
settings.RecentFiles = [];
|
||||
bool saved = AppIdentity.SettingsStore.Save(settings);
|
||||
bool saved = AppIdentity.SettingsStore.Update(settings =>
|
||||
{
|
||||
settings.RecentFiles = [];
|
||||
return settings;
|
||||
}) is not null;
|
||||
RefreshRecentMenus();
|
||||
return saved;
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@
|
||||
|
||||
using KofTwentyTwo.AppKit;
|
||||
using KofTwentyTwo.AppKit.Logging;
|
||||
using KofTwentyTwo.AppKit.Settings;
|
||||
using Velopack.Locators;
|
||||
|
||||
|
||||
@@ -31,8 +30,8 @@ internal static class AppIdentity
|
||||
|
||||
|
||||
|
||||
/// <summary>Each edition owns its own persisted preferences and recent document list.</summary>
|
||||
public static SettingsStore<AppSettings> SettingsStore { get; } = new(Paths.SettingsFile, AppSettingsContext.Default.AppSettings);
|
||||
/// <summary>Each edition owns its own persisted preferences and recent document list; faults are logged by kind, never with a path or message text.</summary>
|
||||
public static Core.AppSettingsStore SettingsStore { get; } = new(Paths.SettingsFile, ReportSettingsFault);
|
||||
|
||||
|
||||
|
||||
@@ -56,6 +55,12 @@ internal static class AppIdentity
|
||||
|
||||
|
||||
|
||||
/// <summary>A settings fault is an operational event: the kind, exception type and HRESULT identify it without disclosing the profile.</summary>
|
||||
private static void ReportSettingsFault(Core.SettingsFault fault, Exception? exception)
|
||||
=> App.Log.Warning(FormattableString.Invariant($"Settings file fault: {fault}; exception={exception?.GetType().Name ?? "none"}; HRESULT=0x{exception?.HResult ?? 0:X8}."));
|
||||
|
||||
|
||||
|
||||
/// <summary>Daily activity files use the edition's own path and identity prefix.</summary>
|
||||
public static IActivityLog CreateLog() => FileActivityLog.ForApp(Info, Paths);
|
||||
}
|
||||
@@ -48,8 +48,8 @@ public sealed partial class MainWindow
|
||||
Core.AppSettings draft = Core.SettingsChanges.Copy(before);
|
||||
draft.ConfiguredEditors = choices;
|
||||
draft.ExternalEditor = choices.FirstOrDefault()?.Executable ?? "";
|
||||
Core.AppSettings merged = Core.SettingsChanges.Merge(AppIdentity.SettingsStore.Load(), before, draft);
|
||||
if(!AppIdentity.SettingsStore.Save(merged))
|
||||
Core.AppSettings? merged = AppIdentity.SettingsStore.Update(current => Core.SettingsChanges.Merge(current, before, draft));
|
||||
if(merged is null)
|
||||
{
|
||||
StatusText.Text = "Could not save editor configuration.";
|
||||
return;
|
||||
|
||||
@@ -19,9 +19,12 @@ public sealed partial class MainWindow
|
||||
{
|
||||
return false;
|
||||
}
|
||||
Core.AppSettings settings = AppIdentity.SettingsStore.Load();
|
||||
settings.Print = options.Sanitize();
|
||||
if(!AppIdentity.SettingsStore.Save(settings))
|
||||
Core.AppSettings? settings = AppIdentity.SettingsStore.Update(current =>
|
||||
{
|
||||
current.Print = options.Sanitize();
|
||||
return current;
|
||||
});
|
||||
if(settings is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -61,8 +61,8 @@ public sealed partial class MainWindow
|
||||
draft.ContentWidthEnabled = appearance.LimitWidth;
|
||||
draft.ContentWidthPx = appearance.WidthPixels;
|
||||
draft.GlobalCss = appearance.GlobalCss;
|
||||
Core.AppSettings merged = Core.SettingsChanges.Merge(AppIdentity.SettingsStore.Load(), before, draft);
|
||||
if(!AppIdentity.SettingsStore.Save(merged))
|
||||
Core.AppSettings? merged = AppIdentity.SettingsStore.Update(current => Core.SettingsChanges.Merge(current, before, draft));
|
||||
if(merged is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
@@ -79,14 +79,16 @@ public sealed partial class MainWindow
|
||||
{
|
||||
return false;
|
||||
}
|
||||
Core.AppSettings settings = AppIdentity.SettingsStore.Load();
|
||||
settings.ShowSplashScreen = showSplash;
|
||||
settings.SplashMilliseconds = milliseconds;
|
||||
settings.CheckForUpdatesOnStartup = checkUpdates;
|
||||
settings.ShowOutlineByDefault = showOutline;
|
||||
settings.ShowSourceByDefault = showSource;
|
||||
settings.Sanitize();
|
||||
if(!AppIdentity.SettingsStore.Save(settings))
|
||||
Core.AppSettings? settings = AppIdentity.SettingsStore.Update(current =>
|
||||
{
|
||||
current.ShowSplashScreen = showSplash;
|
||||
current.SplashMilliseconds = milliseconds;
|
||||
current.CheckForUpdatesOnStartup = checkUpdates;
|
||||
current.ShowOutlineByDefault = showOutline;
|
||||
current.ShowSourceByDefault = showSource;
|
||||
return current;
|
||||
});
|
||||
if(settings is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
@@ -120,10 +122,13 @@ public sealed partial class MainWindow
|
||||
{
|
||||
return false;
|
||||
}
|
||||
Core.AppSettings settings = AppIdentity.SettingsStore.Load();
|
||||
settings.WrapOutlineEntries = wrap;
|
||||
settings.ShowOutlineDepthMarkers = depthMarkers;
|
||||
if(!AppIdentity.SettingsStore.Save(settings))
|
||||
Core.AppSettings? settings = AppIdentity.SettingsStore.Update(current =>
|
||||
{
|
||||
current.WrapOutlineEntries = wrap;
|
||||
current.ShowOutlineDepthMarkers = depthMarkers;
|
||||
return current;
|
||||
});
|
||||
if(settings is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -56,8 +56,8 @@ public sealed partial class MainWindow
|
||||
/// <summary>Failed persistence does not apply an unsaved theme draft; sibling fields untouched by this edit retain their latest values.</summary>
|
||||
private async Task<bool> CommitThemeDraftAsync(Core.AppSettings before, Core.AppSettings edited)
|
||||
{
|
||||
Core.AppSettings merged = Core.SettingsChanges.Merge(AppIdentity.SettingsStore.Load(), before, edited);
|
||||
if(!AppIdentity.SettingsStore.Save(merged))
|
||||
Core.AppSettings? merged = AppIdentity.SettingsStore.Update(current => Core.SettingsChanges.Merge(current, before, edited));
|
||||
if(merged is null)
|
||||
{
|
||||
StatusText.Text = "Could not save document themes.";
|
||||
return false;
|
||||
|
||||
@@ -522,8 +522,7 @@ public sealed partial class MainWindow : Window, IDisposable
|
||||
/// <summary>Saves the settings and reports a failed save in the status line.</summary>
|
||||
private void Save()
|
||||
{
|
||||
Core.AppSettings merged = Core.SettingsChanges.Merge(AppIdentity.SettingsStore.Load(), _lastSavedSettings, _settings);
|
||||
if(AppIdentity.SettingsStore.Save(merged))
|
||||
if(AppIdentity.SettingsStore.Update(current => Core.SettingsChanges.Merge(current, _lastSavedSettings, _settings)) is not null)
|
||||
{
|
||||
_lastSavedSettings = Core.SettingsChanges.Copy(_settings);
|
||||
}
|
||||
|
||||
@@ -90,8 +90,8 @@ public class AppSettingsTests
|
||||
public void SaveAndLoad_KeepsTheTheme()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
var store = new KofTwentyTwo.AppKit.Settings.SettingsStore<AppSettings>(temp.File("settings.json"), AppSettingsContext.Default.AppSettings);
|
||||
Assert.True(store.Save(new AppSettings { Theme = "Dark", DocumentTheme = "GitHub Light", Frontmatter = "Metadata", UserThemes = [Themes.ThemeCatalog.Find("GitHub Light").Json] }));
|
||||
var store = new AppSettingsStore(temp.File("settings.json"));
|
||||
Assert.NotNull(store.Update(_ => new AppSettings { Theme = "Dark", DocumentTheme = "GitHub Light", Frontmatter = "Metadata", UserThemes = [Themes.ThemeCatalog.Find("GitHub Light").Json] }));
|
||||
Assert.Equal("Dark", store.Load().Theme);
|
||||
Assert.Equal("GitHub Light", store.Load().DocumentTheme);
|
||||
Assert.Equal(Documents.FrontmatterMode.Metadata, store.Load().FrontmatterKind);
|
||||
|
||||
@@ -0,0 +1,403 @@
|
||||
/*
|
||||
* Copyright (c) 2026 James Maes
|
||||
* SPDX-License-Identifier: MIT
|
||||
*/
|
||||
|
||||
using System.Text;
|
||||
|
||||
|
||||
namespace MarkdownViewr.Windows.Core.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// The settings file is shared by windows and processes: these tests use real files, real
|
||||
/// exclusive handles and concurrent store instances, and count attempts instead of time.
|
||||
/// </summary>
|
||||
public class AppSettingsStoreTests
|
||||
{
|
||||
private const string KeptPath = @"C:\docs\keep-me.md";
|
||||
|
||||
|
||||
|
||||
/// <summary>A first run has no file: defaults apply and reading creates nothing.</summary>
|
||||
[Fact]
|
||||
public void Load_MissingFile_ReturnsDefaultsWithoutCreatingAnything()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(Path.Combine(temp.Path, "profile", "settings.json"), (fault, _) => faults.Add(fault));
|
||||
Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
|
||||
Assert.Empty(faults);
|
||||
Assert.False(Directory.Exists(Path.Combine(temp.Path, "profile")));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>The first save creates the folder and file, and returns the sanitized settings that were written.</summary>
|
||||
[Fact]
|
||||
public void Update_MissingFolder_CreatesTheFileAndReturnsSanitizedSettings()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
var store = new AppSettingsStore(Path.Combine(temp.Path, "profile", "settings.json"));
|
||||
AppSettings? saved = store.Update(settings =>
|
||||
{
|
||||
settings.Theme = "Dark";
|
||||
settings.OutlineDepth = 99;
|
||||
return settings;
|
||||
});
|
||||
Assert.NotNull(saved);
|
||||
Assert.Equal(6, saved.OutlineDepth);
|
||||
AppSettings loaded = store.Load();
|
||||
Assert.Equal("Dark", loaded.Theme);
|
||||
Assert.Equal(6, loaded.OutlineDepth);
|
||||
Assert.Equal(["settings.json", "settings.json.lock"], FileNames(Path.Combine(temp.Path, "profile")));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A change is applied to what is stored now, not to what this instance read earlier.</summary>
|
||||
[Fact]
|
||||
public void Update_AnotherInstanceSavedFirst_ChangesTheLatestSettings()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
var first = new AppSettingsStore(temp.File("settings.json"));
|
||||
var second = new AppSettingsStore(temp.File("settings.json"));
|
||||
_ = first.Load();
|
||||
Assert.NotNull(second.Update(settings =>
|
||||
{
|
||||
settings.GlobalCss = "body { color: red }";
|
||||
return settings;
|
||||
}));
|
||||
Assert.NotNull(first.Update(settings =>
|
||||
{
|
||||
settings.Theme = "Light";
|
||||
return settings;
|
||||
}));
|
||||
AppSettings loaded = second.Load();
|
||||
Assert.Equal("body { color: red }", loaded.GlobalCss);
|
||||
Assert.Equal("Light", loaded.Theme);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A file saved by hand with a byte order mark, in UTF-8 or UTF-16, is still the user's settings.</summary>
|
||||
[Theory]
|
||||
[InlineData(false)]
|
||||
[InlineData(true)]
|
||||
public void Load_ByteOrderMark_ReadsTheStoredValues(bool utf16)
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Encoding marked = utf16 ? Encoding.Unicode : new UTF8Encoding(encoderShouldEmitUTF8Identifier: true);
|
||||
File.WriteAllText(temp.File("settings.json"), """{"Theme":"Dark","DocumentTheme":"Nord"}""", marked);
|
||||
var faults = new List<SettingsFault>();
|
||||
AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
|
||||
Assert.Equal("Dark", loaded.Theme);
|
||||
Assert.Equal("Nord", loaded.DocumentTheme);
|
||||
Assert.Empty(faults);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Another handle holding the file for a moment (a scanner, a backup) is waited out, not read as an empty profile.</summary>
|
||||
[Fact]
|
||||
public void Load_FileLockedBriefly_RetriesAndReadsTheStoredValues()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
FileStream? exclusive = new(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None);
|
||||
int waits = 0;
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ =>
|
||||
{
|
||||
if(++waits == 3)
|
||||
{
|
||||
exclusive.Dispose();
|
||||
}
|
||||
});
|
||||
AppSettings loaded = store.Load();
|
||||
Assert.Equal(3, waits);
|
||||
Assert.Equal("Nord", loaded.DocumentTheme);
|
||||
Assert.Equal([KeptPath], loaded.RecentFiles);
|
||||
Assert.Empty(faults);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A file that stays unreadable yields defaults for display and is reported; the attempts are bounded.</summary>
|
||||
[Fact]
|
||||
public void Load_FileLockedThroughout_ReturnsDefaultsAndReportsUnreadable()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
int waits = 0;
|
||||
var faults = new List<(SettingsFault Fault, Exception? Exception)>();
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception)), _ => waits++);
|
||||
using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None))
|
||||
{
|
||||
Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
|
||||
}
|
||||
Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits);
|
||||
(SettingsFault fault, Exception? exception) = Assert.Single(faults);
|
||||
Assert.Equal(SettingsFault.Unreadable, fault);
|
||||
Assert.IsType<IOException>(exception);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// The data-loss regression: a read failure is not an empty profile. Nothing is written over
|
||||
/// a file whose content is unknown, so the stored theme, CSS and history survive.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Update_FileLockedThroughout_WritesNothingOverIt()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
byte[] before = File.ReadAllBytes(temp.File("settings.json"));
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { });
|
||||
bool changed = false;
|
||||
using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None))
|
||||
{
|
||||
Assert.Null(store.Update(settings =>
|
||||
{
|
||||
changed = true;
|
||||
return settings;
|
||||
}));
|
||||
}
|
||||
Assert.False(changed);
|
||||
Assert.Equal([SettingsFault.Unreadable], faults);
|
||||
Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json")));
|
||||
Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
|
||||
AppSettings loaded = store.Load();
|
||||
Assert.Equal("Nord", loaded.DocumentTheme);
|
||||
Assert.Equal("body { color: red }", loaded.GlobalCss);
|
||||
Assert.Equal([KeptPath], loaded.RecentFiles);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Only invalid content is corruption: reading it yields defaults, reports it, and leaves the file where it is.</summary>
|
||||
[Theory]
|
||||
[InlineData("")]
|
||||
[InlineData("{")]
|
||||
[InlineData("null")]
|
||||
[InlineData("[1]")]
|
||||
[InlineData("{\"OutlineDepth\":\"deep\"}")]
|
||||
public void Load_InvalidContent_ReturnsDefaultsAndLeavesTheFile(string content)
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
File.WriteAllText(temp.File("settings.json"), content);
|
||||
var faults = new List<SettingsFault>();
|
||||
AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
|
||||
Assert.Equal("Catppuccin Mocha", loaded.DocumentTheme);
|
||||
Assert.Equal([SettingsFault.Corrupt], faults);
|
||||
Assert.Equal(content, File.ReadAllText(temp.File("settings.json")));
|
||||
Assert.Equal(["settings.json"], FileNames(temp.Path));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Saving over invalid content keeps that content aside, so a hand edit gone wrong can still be recovered.</summary>
|
||||
[Fact]
|
||||
public void Update_InvalidContent_KeepsItAsideAndSavesTheChange()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
File.WriteAllText(temp.File("settings.json"), "{ \"Theme\": \"Dark\", ");
|
||||
File.WriteAllText(temp.File("settings.json.corrupt"), "an older invalid file");
|
||||
var store = new AppSettingsStore(temp.File("settings.json"));
|
||||
Assert.NotNull(store.Update(settings =>
|
||||
{
|
||||
settings.DocumentTheme = "Nord";
|
||||
return settings;
|
||||
}));
|
||||
Assert.Equal("{ \"Theme\": \"Dark\", ", File.ReadAllText(temp.File("settings.json.corrupt")));
|
||||
Assert.Equal("Nord", store.Load().DocumentTheme);
|
||||
Assert.Equal("System", store.Load().Theme);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A file far larger than any settings is not loaded into memory, even when it would parse; it reads as invalid content.</summary>
|
||||
[Fact]
|
||||
public void Load_OversizedFile_ReadsAsInvalidContent()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
byte[] valid = Encoding.UTF8.GetBytes("""{"DocumentTheme":"Nord"}""");
|
||||
byte[] padded = new byte[AppSettingsStore.MaximumBytes + 1];
|
||||
Array.Fill(padded, (byte)' ');
|
||||
valid.CopyTo(padded, 0);
|
||||
File.WriteAllBytes(temp.File("settings.json"), padded);
|
||||
var faults = new List<SettingsFault>();
|
||||
Assert.Equal("Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load().DocumentTheme);
|
||||
Assert.Equal([SettingsFault.Corrupt], faults);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>While another instance holds the lock for every attempt, the change is refused and the file is unchanged.</summary>
|
||||
[Fact]
|
||||
public void Update_LockHeldByAnotherInstance_ReportsBusyAndWritesNothing()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
byte[] before = File.ReadAllBytes(temp.File("settings.json"));
|
||||
int waits = 0;
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++);
|
||||
using(new FileStream(temp.File("settings.json.lock"), FileMode.Open, FileAccess.ReadWrite, FileShare.None))
|
||||
{
|
||||
Assert.Null(store.Update(settings => settings));
|
||||
}
|
||||
Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits);
|
||||
Assert.Equal([SettingsFault.Busy], faults);
|
||||
Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json")));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A lock released while this instance waits lets the change through.</summary>
|
||||
[Fact]
|
||||
public void Update_LockReleasedDuringWait_SavesTheChange()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
FileStream other = new(temp.File("settings.json.lock"), FileMode.Open, FileAccess.ReadWrite, FileShare.None);
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), wait: _ => other.Dispose());
|
||||
AppSettings? saved = store.Update(settings =>
|
||||
{
|
||||
settings.Theme = "Light";
|
||||
return settings;
|
||||
});
|
||||
Assert.NotNull(saved);
|
||||
Assert.Equal("Light", store.Load().Theme);
|
||||
Assert.Equal([KeptPath], store.Load().RecentFiles);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A handle that forbids replacing the file fails the save cleanly: the previous file stays and no temporary file is left.</summary>
|
||||
[Fact]
|
||||
public void Update_ReplaceBlocked_LeavesPreviousFileAndNoTemporaryFile()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
byte[] before = File.ReadAllBytes(temp.File("settings.json"));
|
||||
int waits = 0;
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++);
|
||||
using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read))
|
||||
{
|
||||
Assert.Null(store.Update(settings =>
|
||||
{
|
||||
settings.Theme = "Light";
|
||||
return settings;
|
||||
}));
|
||||
}
|
||||
Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits);
|
||||
Assert.Equal([SettingsFault.WriteFailed], faults);
|
||||
Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json")));
|
||||
Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A location that cannot hold a settings folder reports a failed write instead of throwing into the caller.</summary>
|
||||
[Fact]
|
||||
public void Update_FolderCannotBeCreated_ReportsWriteFailed()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
File.WriteAllText(temp.File("blocker"), "a file where the folder should be");
|
||||
var faults = new List<SettingsFault>();
|
||||
var store = new AppSettingsStore(Path.Combine(temp.File("blocker"), "settings.json"), (fault, _) => faults.Add(fault), _ => { });
|
||||
Assert.Null(store.Update(settings => settings));
|
||||
Assert.Equal([SettingsFault.WriteFailed], faults);
|
||||
Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>Temporary files left by a writer that died before its rename are removed by the next save; one still in use is left alone.</summary>
|
||||
[Fact]
|
||||
public void Update_AbandonedTemporaryFiles_AreRemovedUnlessInUse()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
Seed(temp);
|
||||
File.WriteAllText(temp.File("settings.json.tmp"), "left by the previous store");
|
||||
File.WriteAllText(temp.File("settings.json.0123.tmp"), "left by a writer that died");
|
||||
File.WriteAllText(temp.File("settings.json.busy.tmp"), "still open elsewhere");
|
||||
File.WriteAllText(temp.File("notes.tmp"), "not ours");
|
||||
var store = new AppSettingsStore(temp.File("settings.json"));
|
||||
using(new FileStream(temp.File("settings.json.busy.tmp"), FileMode.Open, FileAccess.Read, FileShare.None))
|
||||
{
|
||||
Assert.NotNull(store.Update(settings => settings));
|
||||
}
|
||||
Assert.Equal(["notes.tmp", "settings.json", "settings.json.busy.tmp", "settings.json.lock"], FileNames(temp.Path));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// The lost-update regression: instances that each record their own documents at the same
|
||||
/// time keep every one. A refused save is retried, as a user would; a save that reported
|
||||
/// success must never be erased by another instance.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Update_ConcurrentInstances_KeepEveryChange()
|
||||
{
|
||||
using var temp = new TempDirectory();
|
||||
const int Instances = 4;
|
||||
const int Each = 5;
|
||||
using var start = new ManualResetEventSlim();
|
||||
var threads = new List<Thread>();
|
||||
for(int instance = 0; instance < Instances; instance++)
|
||||
{
|
||||
int id = instance;
|
||||
var thread = new Thread(() =>
|
||||
{
|
||||
var store = new AppSettingsStore(temp.File("settings.json"));
|
||||
start.Wait();
|
||||
for(int index = 0; index < Each; index++)
|
||||
{
|
||||
string path = $@"C:\docs\instance{id}-{index}.md";
|
||||
while(store.Update(settings =>
|
||||
{
|
||||
settings.RecentFiles = Documents.RecentDocuments.Record(settings.RecentFiles, path);
|
||||
return settings;
|
||||
}) is null)
|
||||
{
|
||||
Thread.Yield();
|
||||
}
|
||||
}
|
||||
});
|
||||
thread.Start();
|
||||
threads.Add(thread);
|
||||
}
|
||||
start.Set();
|
||||
threads.ForEach(thread => thread.Join());
|
||||
string[] recorded = new AppSettingsStore(temp.File("settings.json")).Load().RecentFiles;
|
||||
Assert.Equal(Instances * Each, recorded.Length);
|
||||
Assert.Equal(Instances * Each, recorded.Distinct(StringComparer.Ordinal).Count());
|
||||
Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>A profile with non-default values in several unrelated fields, so a wipe is visible.</summary>
|
||||
private static void Seed(TempDirectory temp)
|
||||
{
|
||||
Assert.NotNull(new AppSettingsStore(temp.File("settings.json")).Update(settings =>
|
||||
{
|
||||
settings.Theme = "Dark";
|
||||
settings.DocumentTheme = "Nord";
|
||||
settings.GlobalCss = "body { color: red }";
|
||||
settings.RecentFiles = [KeptPath];
|
||||
return settings;
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>The folder's file names in ordinal order.</summary>
|
||||
private static string[] FileNames(string folder)
|
||||
=> Directory.GetFiles(folder).Select(file => Path.GetFileName(file)!).Order(StringComparer.Ordinal).ToArray();
|
||||
}
|
||||
@@ -76,21 +76,24 @@ public sealed class EditorConfigurationTests
|
||||
|
||||
|
||||
|
||||
/// <summary>Atomic AppKit persistence retains named IDs, order and folder mode using the trimmed serializer.</summary>
|
||||
/// <summary>Atomic persistence retains named IDs, order and folder mode using the trimmed serializer.</summary>
|
||||
[Fact]
|
||||
public void SaveAndLoad_EditorList_PreservesIdentityOrderAndMode()
|
||||
{
|
||||
using var temporary = new TempDirectory();
|
||||
var store = new KofTwentyTwo.AppKit.Settings.SettingsStore<AppSettings>(temporary.File("settings.json"), AppSettingsContext.Default.AppSettings);
|
||||
var store = new AppSettingsStore(temporary.File("settings.json"));
|
||||
var first = new EditorConfiguration(Guid.NewGuid(), "Folder editor", @"C:\Editor\folder.exe", true);
|
||||
var second = new EditorConfiguration(Guid.NewGuid(), "File editor", @"C:\Editor\file.exe");
|
||||
Assert.True(store.Save(new AppSettings { ConfiguredEditors = [first, second], ExternalEditor = @"C:\Other\old.exe" }));
|
||||
Assert.NotNull(store.Update(_ => new AppSettings { ConfiguredEditors = [first, second], ExternalEditor = @"C:\Other\old.exe" }));
|
||||
AppSettings loaded = store.Load();
|
||||
Assert.Equal(new[] { first, second }, loaded.ConfiguredEditors);
|
||||
Assert.Equal(first.Executable, loaded.ExternalEditor);
|
||||
loaded.ConfiguredEditors = [];
|
||||
loaded.ExternalEditor = "";
|
||||
Assert.True(store.Save(loaded));
|
||||
Assert.NotNull(store.Update(settings =>
|
||||
{
|
||||
settings.ConfiguredEditors = [];
|
||||
settings.ExternalEditor = "";
|
||||
return settings;
|
||||
}));
|
||||
Assert.Empty(store.Load().ConfiguredEditors);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user