diff --git a/docs/adr/0004-native-document-windows-and-private-recents.md b/docs/adr/0004-native-document-windows-and-private-recents.md index d4c29fb..06c6577 100644 --- a/docs/adr/0004-native-document-windows-and-private-recents.md +++ b/docs/adr/0004-native-document-windows-and-private-recents.md @@ -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 diff --git a/docs/adr/0006-settings-file-protocol.md b/docs/adr/0006-settings-file-protocol.md new file mode 100644 index 0000000..bb76851 --- /dev/null +++ b/docs/adr/0006-settings-file-protocol.md @@ -0,0 +1,138 @@ +# ADR-0006: Settings file protocol + +- **Status:** Accepted +- **Date:** 2026-11-10 + +## Context + +One `settings.json` per edition is shared by every reader window and by every +process that uses the same data folder: an installed copy and a portable copy, or +two builds started from different folders, are separate processes over one file. + +AppKit's `SettingsStore` returns defaults whenever the file cannot be read and writes +a whole snapshot through one fixed temporary name with no lock. With real files and +real processes this lost data in two ways: a load during a sharing violation returned +defaults that the next save wrote back, and two processes saving in turn erased each +other's changes. + +The files beside `settings.json` are now a contract between application versions that +may run at the same time, so the protocol is recorded here. + +## Decision + +`AppSettingsStore` in the core library owns the file. AppKit still defines the shell +preferences (`ShellSettings`); only persistence moved. + +**Change, not snapshot.** `Update` takes a function from the stored settings to the +new settings. It runs under the lock against what is on disk at that moment, so a +caller cannot write back a stale copy of a value it did not change. + +**Lock.** `settings.json.lock` is opened with no sharing for the duration of an +update. It is never deleted: deleting a lock file leaves a window in which a second +opener is refused with access denied. The operating system releases the handle if +the process dies. A lock that stays held for every attempt refuses the update. + +**Read for an update.** The file is opened for reading while denying writers, and +that handle is held until the rename. A program rewriting the file in place therefore +reads as busy, not as invalid content, and none can start between the read and the +rename. An empty file is waited on for the same attempts, because a program that +clears a file before rewriting it leaves it empty for a moment. + +**Read for display.** The same open is tried first. If another handle has write +access, the file is read beside it instead of waiting, since such a handle is usually +idle. Content read that way which does not parse is retried and, if it never parses, +reported as unreadable rather than corrupt: it cannot be judged while it can still be +written. An empty file reads as defaults at once. + +**What a read can find.** + +| Found | Used for display | On update | +| --- | --- | --- | +| No file | Defaults | The file is created | +| Empty file | Defaults | Replaced after the wait; nothing kept aside | +| Valid settings | The stored values | Replaced | +| Some invalid properties | Valid properties kept, invalid ones as defaults | Replaced; a copy of the original is kept aside | +| Not settings at all | Defaults | Replaced; a copy of the original is kept aside, unless it is over 16 MiB | +| Unreadable or being written | Defaults | Refused; nothing is written | + +A trailing comma, a comment and a quoted number are valid: they are what a hand edit +commonly leaves behind. Text the JSON reader cannot decode, such as an unpaired +surrogate escape, makes its property invalid. Repair works per top-level property: +one bad entry in a list costs that list. A file over 16 MiB is not loaded. + +**Write.** The new content is written to `settings.json.<32 hex digits>.tmp` and +flushed to disk. That file is then renamed over `settings.json` in one step with the +Windows rename that replaces an open file (`FileRenameInfoEx` with POSIX semantics). +At no point is there no `settings.json`: a reader sees the old file or the new one, +and a process killed at any moment leaves one of them. Handles that were open on the +old file and allow deletion go on reading it. `File.Replace` is not used because it +is two renames with the file missing between them, and `File.Move` with overwrite is +not used because it refuses while any handle is open on the destination. + +The rename is given the path in the form Windows takes as written, so a data folder +whose path is longer than 260 characters, or whose name is not well-formed UTF-16, +behaves like any other. + +On a volume that does not implement that rename, the store closes its own read +handle and falls back to `File.Move` with overwrite. If the file did not exist when +it was read, the temporary file is renamed into place without overwriting. A failed +update leaves the stored file as it was and removes its temporary file. + +**Kept-aside copies.** Before invalid content is replaced it is copied to a temporary +file. Once the save has succeeded the copy is renamed to `settings.json.corrupt`, or +to the first free of `settings.json.corrupt-2` to `settings.json.corrupt-9`; the last +name is overwritten once all exist. A save that fails removes its temporary copy and +leaves every earlier copy as it was, and the invalid file is still in place. The copy +is a courtesy for recovering a hand edit, with these limits: + +- a file over 16 MiB is replaced without a copy, since it was never this + application's settings and copying it would stall the window; +- a copy that cannot be written, for example on a full disk, or cannot be given its + name, is removed and reported, and the save goes ahead, because refusing would + block every later change too; +- a process killed between the save and the rename of the copy loses that copy. + +Nothing reads the copies back. They contain whatever the file contained, including +recent-document paths, so clearing recent files deletes them. + +**Abandoned temporary files.** A process killed between its write and its rename +leaves a temporary file, and possibly a temporary copy. The next update, holding the +lock, removes files that match this store's own name pattern and no others. + +**Waiting.** Only a sharing violation is waited out: at most 20 attempts, 50 ms +apart, for the lock, the read and the rename. Access denied is treated as permanent +and reported at once, so a read-only profile does not stall a window. The fallback +move is the exception: it answers access denied while any handle is open on the +file, a reader's included, so there access denied is waited out as well, unless the +file carries the read-only attribute. + +**Reporting.** Faults are reported by kind with the exception type and HRESULT. No +path and no operating-system message text reaches the log. + +## Consequences + +- An update is a synchronous call on the window thread. It normally takes a few + milliseconds and at most about one second per contended step. +- The JSON written is byte-compatible with earlier releases, so a downgrade reads it. +- The rename needs Windows 10 version 1607 or later and a file system that supports + it; NTFS does. Other volumes use the fallback, which is not safe against a kill + between closing the file and the move completing. +- An earlier release running against the same data folder still has its own defect: + it takes no lock, and when a read fails it falls back to defaults and may save + them. This store cannot prevent that. It also makes that release's saves fail more + often while both run, because the earlier release cannot replace a file this store + has open. Run one version per data folder. +- The store keeps no in-memory copy. Windows hold their own view of the settings and + merge only the fields they changed (ADR-0004). + +## Options considered + +- **A named mutex instead of a lock file.** Rejected: a session-local name does not + cover two sessions of one user on the same profile, and a global name can be + created first by another account on the machine. +- **Renaming the invalid file away instead of copying it.** Rejected: between that + rename and the new file arriving there is no settings file, and a kill there loses + the profile. +- **Fixing AppKit's store instead.** Still worthwhile for other applications, but the + read-modify-write API is a different shape, and this application needed the fix + without waiting for a package release. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 551daa2..fdcf6f2 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -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 are renamed into place in one step, so the file is never absent; a file that is unreadable or being written by another program is never overwritten, a copy of invalid content is kept aside under a bounded set of names and removed when recent files are cleared, a file over 16 MiB is not loaded, and faults are logged by kind, exception type and HRESULT only (ADR-0006) | | 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 | diff --git a/docs/user-guide.md b/docs/user-guide.md index 1b4c4bf..7e47870 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -73,6 +73,8 @@ 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 using `settings.json`, or the file or folder is read-only. Nothing is overwritten while the file cannot be read; try the change again. +- **Settings were edited by hand:** a trailing comma, a comment or a quoted number is accepted; the app rewrites the file in its own format on the next save, so comments are not kept. A syntax error such as a missing comma or quote makes the whole file unreadable: every setting starts from its default, and the next save keeps a copy of your file as described below. A setting whose value is not valid is ignored and the other settings are kept. When either happens, the next save keeps a copy of the file as it was, named `settings.json.corrupt` (then `.corrupt-2` up to `.corrupt-9`). Those copies contain everything the file contained; **File → Recent files → Clear recent files** deletes them. 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). diff --git a/src/MarkdownViewr.Windows.Core/AppIdentity.cs b/src/MarkdownViewr.Windows.Core/AppIdentity.cs index c284366..9ef5b27 100644 --- a/src/MarkdownViewr.Windows.Core/AppIdentity.cs +++ b/src/MarkdownViewr.Windows.Core/AppIdentity.cs @@ -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 /// Loads and saves at . - public static SettingsStore SettingsStore { get; } = new(Paths.SettingsFile, AppSettingsContext.Default.AppSettings); + public static AppSettingsStore SettingsStore { get; } = new(Paths.SettingsFile); diff --git a/src/MarkdownViewr.Windows.Core/AppSettings.cs b/src/MarkdownViewr.Windows.Core/AppSettings.cs index 2e0b17b..89ce5a0 100644 --- a/src/MarkdownViewr.Windows.Core/AppSettings.cs +++ b/src/MarkdownViewr.Windows.Core/AppSettings.cs @@ -202,8 +202,9 @@ public sealed class AppSettings : ShellSettings _ = Themes.ReaderTheme.ParseCompatible(json); return true; } - catch(JsonException) + catch(Exception exception) when(exception is JsonException or InvalidOperationException) { + // A string the JSON reader cannot decode, such as an unpaired surrogate escape, is invalid content too. return false; } } diff --git a/src/MarkdownViewr.Windows.Core/AppSettingsContext.cs b/src/MarkdownViewr.Windows.Core/AppSettingsContext.cs index 0ad1c6f..d881bbb 100644 --- a/src/MarkdownViewr.Windows.Core/AppSettingsContext.cs +++ b/src/MarkdownViewr.Windows.Core/AppSettingsContext.cs @@ -3,13 +3,17 @@ * SPDX-License-Identifier: MIT */ +using System.Text.Json; using System.Text.Json.Serialization; namespace MarkdownViewr.Windows.Core; -/// Source-generated JSON serializer for , so trimmed builds need no reflection. -[JsonSourceGenerationOptions(WriteIndented = true)] +/// +/// Source-generated JSON serializer for , so trimmed builds need no reflection. +/// Reading tolerates what a hand edit commonly leaves behind: a trailing comma, a comment, a quoted number. +/// +[JsonSourceGenerationOptions(WriteIndented = true, AllowTrailingCommas = true, ReadCommentHandling = JsonCommentHandling.Skip, NumberHandling = JsonNumberHandling.AllowReadingFromString)] [JsonSerializable(typeof(AppSettings))] public sealed partial class AppSettingsContext : JsonSerializerContext { diff --git a/src/MarkdownViewr.Windows.Core/AppSettingsStore.cs b/src/MarkdownViewr.Windows.Core/AppSettingsStore.cs new file mode 100644 index 0000000..feeaf90 --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/AppSettingsStore.cs @@ -0,0 +1,534 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using System.Globalization; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; + + +namespace MarkdownViewr.Windows.Core; + +/// +/// 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 renamed into place in +/// one step, so two instances cannot erase each other's change and the file is never absent. +/// A file that cannot be read, or that another program is writing, is never treated as empty; +/// content that is invalid while nobody is writing it is repaired where possible and a copy of +/// the original is kept aside. ADR-0006 describes the files involved. +/// +public sealed class AppSettingsStore +{ + /// Attempts for the lock, a read, and the final rename before the operation gives up. + public const int MaximumAttempts = 20; + + /// A larger file is not this application's settings and is never loaded into memory. + public const int MaximumBytes = 16 * 1024 * 1024; + + /// Copies of invalid files kept aside before the last name is reused. + public const int MaximumKeptAside = 9; + + /// The pause between attempts while another handle is in the way. + public static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(50); + + private const int SharingViolation = unchecked((int)0x80070020); + private static readonly JsonDocumentOptions s_lenient = new() { AllowTrailingCommas = true, CommentHandling = JsonCommentHandling.Skip }; + private readonly Action _report; + private readonly Action _wait; + private readonly Func _renameOver; + + + + /// Creates the store; faults go to , and tests supply to count attempts instead of sleeping. + public AppSettingsStore(string path, Action? report = null, Action? wait = null) + : this(path, report, wait, FileSwap.TryRenameOver) + { + } + + + + /// Creates the store over a given rename, so tests can stand in for a volume that cannot rename over an open file. + internal AppSettingsStore(string path, Action? report, Action? wait, Func renameOver) + { + ArgumentException.ThrowIfNullOrWhiteSpace(path); + Path = System.IO.Path.GetFullPath(path); + _report = report ?? ((_, _) => { }); + _wait = wait ?? Thread.Sleep; + _renameOver = renameOver; + } + + + + /// The settings file, as an absolute path. + public string Path { get; } + + + + /// + /// Reads the stored settings for display. A missing, empty, invalid or unreadable file yields + /// defaults and is left untouched. A handle that another program merely holds with write + /// access does not delay the read. + /// + public AppSettings Load() + { + _ = TryRead(forUpdate: false, out AppSettings settings, out _); + return settings; + } + + + + /// + /// 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 was being written by another program, or the write failed. The stored file is + /// unchanged in every null case. + /// + public AppSettings? Update(Func change) + { + ArgumentNullException.ThrowIfNull(change); + FileStream? stored = null; + try + { + Directory.CreateDirectory(System.IO.Path.GetDirectoryName(Path)!); + using FileStream? gate = Acquire(); + if(gate is null) + { + return null; + } + // The read handle denies writers until the rename, so the content judged here is the content replaced. + SettingsRead read = TryRead(forUpdate: true, out AppSettings current, out stored); + if(read == SettingsRead.Unreadable) + { + return null; + } + AppSettings edited = change(current); + edited.Sanitize(); + RemoveAbandonedTemporaryFiles(); + string? copy = read is SettingsRead.Corrupt or SettingsRead.Repaired ? CopyAside(stored!) : null; + Replace(JsonSerializer.SerializeToUtf8Bytes(edited, AppSettingsContext.Default.AppSettings), stored, copy); + if(copy is not null) + { + KeepAside(copy); + } + return edited; + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + _report(SettingsFault.WriteFailed, exception); + return null; + } + finally + { + stored?.Dispose(); + } + } + + + + /// + /// Removes the copies of invalid files that saves have kept aside. They hold whatever the + /// file held, including the recent-document history, so clearing that history removes them. + /// + public void DeleteKeptAside() + { + for(int slot = 1; slot <= MaximumKeptAside; slot++) + { + TryDelete(KeptAsideName(slot)); + } + } + + + + /// + /// Holds an exclusive handle beside the settings file; the operating system releases it if the + /// process dies. A lock that stays held, or cannot be opened for another I/O reason, is busy. + /// + private FileStream? Acquire() + { + try + { + FileStream? gate = null; + Retry(() => gate = new FileStream(Path + ".lock", FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None)); + return gate; + } + catch(IOException exception) + { + _report(SettingsFault.Busy, exception); + return null; + } + } + + + + /// + /// Reads the file, waiting out other handles a bounded number of times, and reports what it + /// found. For an update the open handle is returned so writers stay out until the rename. + /// + private SettingsRead TryRead(bool forUpdate, out AppSettings settings, out FileStream? stored) + { + settings = new AppSettings(); + stored = null; + SettingsRead? read = null; + for(int attempt = 1; read is null; attempt++) + { + if(attempt > 1) + { + _wait(RetryDelay); + } + read = Attempt(forUpdate, attempt == MaximumAttempts, ref settings, ref stored); + } + if(read is SettingsRead.Corrupt or SettingsRead.Repaired) + { + _report(read == SettingsRead.Corrupt ? SettingsFault.Corrupt : SettingsFault.Repaired, null); + } + settings.Sanitize(); + return read.Value; + } + + + + /// + /// One read. Returns null to try again: another handle is in the way, the file is empty while + /// a save is waiting for whoever cleared it, or the content read beside a writer did not parse. + /// On the last attempt it always answers. + /// + private SettingsRead? Attempt(bool forUpdate, bool last, ref AppSettings settings, ref FileStream? stored) + { + FileStream? stream = null; + bool keep = false; + try + { + stream = Open(forUpdate, out bool writersExcluded); + SettingsRead read = Parse(stream, ref settings); + if(read == SettingsRead.Empty && forUpdate && !last) + { + // A program that clears the file before rewriting it is about to write. + return null; + } + if(read == SettingsRead.Corrupt && !writersExcluded) + { + // Content read while another program can write may simply be half written. + return last ? Unreadable(null) : null; + } + keep = forUpdate; + stored = keep ? stream : null; + return read; + } + catch(Exception exception) when(exception is FileNotFoundException or DirectoryNotFoundException) + { + return SettingsRead.Missing; + } + catch(IOException exception) when(IsTransient(exception) && !last) + { + return null; + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + return Unreadable(exception); + } + finally + { + if(!keep) + { + stream?.Dispose(); + } + } + } + + + + /// Reports a file whose content could not be established and answers that it must not be replaced. + private SettingsRead Unreadable(Exception? exception) + { + _report(SettingsFault.Unreadable, exception); + return SettingsRead.Unreadable; + } + + + + /// + /// Opens the file denying writers, so a program rewriting it in place reads as busy rather + /// than as invalid content. A read for display falls back to reading beside a handle that has + /// write access, because such a handle is usually idle; the caller then knows the content may + /// be half written. + /// + private FileStream Open(bool forUpdate, out bool writersExcluded) + { + writersExcluded = true; + try + { + return new FileStream(Path, FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete); + } + catch(IOException exception) when(IsTransient(exception) && !forUpdate) + { + writersExcluded = false; + return new FileStream(Path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + } + } + + + + /// Reads at most and classifies the content; an invalid property costs that property, never the whole profile. + private static SettingsRead Parse(FileStream stream, ref AppSettings settings) + { + if(stream.Length == 0) + { + return SettingsRead.Empty; + } + if(stream.Length > MaximumBytes) + { + return SettingsRead.Corrupt; + } + using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 4096, leaveOpen: true); + string json = reader.ReadToEnd(); + try + { + if(JsonSerializer.Deserialize(json, AppSettingsContext.Default.AppSettings) is { } loaded) + { + settings = loaded; + return SettingsRead.Loaded; + } + } + catch(Exception exception) when(IsInvalidContent(exception)) + { + if(Salvage(json) is { } repaired) + { + settings = repaired; + return SettingsRead.Repaired; + } + } + return SettingsRead.Corrupt; + } + + + + /// Keeps every stored property that is valid on its own and drops the ones that are not; null when the file is not a JSON object at all. + private static AppSettings? Salvage(string json) + { + try + { + if(JsonNode.Parse(json, documentOptions: s_lenient) is not JsonObject stored) + { + return null; + } + foreach(string name in stored.Select(property => property.Key).ToArray()) + { + try + { + _ = new JsonObject { [name] = stored[name]?.DeepClone() }.Deserialize(AppSettingsContext.Default.AppSettings); + } + catch(Exception exception) when(IsInvalidContent(exception)) + { + stored.Remove(name); + } + } + return stored.Deserialize(AppSettingsContext.Default.AppSettings); + } + catch(Exception exception) when(IsInvalidContent(exception) || exception is ArgumentException) + { + // Not JSON, or an object that repeats a property name. + return null; + } + } + + + + /// + /// What the JSON reader throws for content that is not valid settings: a syntax or type error, + /// or text it cannot decode, such as an unpaired surrogate escape. + /// + private static bool IsInvalidContent(Exception exception) => exception is JsonException or InvalidOperationException; + + + + /// + /// Copies an invalid file to a temporary name before it is replaced and returns that name. + /// The copy is a courtesy, so the save goes ahead without one, with the reason reported and + /// null returned, when the file is past the size bound (it was never this application's + /// settings) or the copy cannot be written. A copy left behind by a save that never finished + /// is removed with the other abandoned temporary files. + /// + private string? CopyAside(FileStream stored) + { + if(stored.Length > MaximumBytes) + { + _report(SettingsFault.NotKeptAside, null); + return null; + } + string temporary = TemporaryName(); + try + { + using(var copy = new FileStream(temporary, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + { + stored.Position = 0; + stored.CopyTo(copy); + copy.Flush(flushToDisk: true); + } + return temporary; + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + TryDelete(temporary); + _report(SettingsFault.NotKeptAside, exception); + return null; + } + } + + + + /// + /// Gives the copy of a replaced invalid file its lasting name: the first free kept-aside name, + /// or the last one once all are taken. This happens after the save, so a save that fails + /// never disturbs an earlier copy. The copy is a courtesy: one that cannot be placed is + /// removed and reported, and the save stands. + /// + private void KeepAside(string copy) + { + int slot = 1; + while(slot < MaximumKeptAside && (File.Exists(KeptAsideName(slot)) || Directory.Exists(KeptAsideName(slot)))) + { + slot++; + } + try + { + File.Move(copy, KeptAsideName(slot), overwrite: true); + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + TryDelete(copy); + _report(SettingsFault.NotKeptAside, exception); + } + } + + + + /// The kept-aside names: .corrupt, then .corrupt-2 to .corrupt-9. + private string KeptAsideName(int slot) => slot == 1 ? Path + ".corrupt" : Path + ".corrupt-" + slot.ToString(CultureInfo.InvariantCulture); + + + + /// A name beside the settings file that only this store uses, so an abandoned one can be recognized and removed. + private string TemporaryName() => Path + "." + Guid.NewGuid().ToString("N") + ".tmp"; + + + + /// + /// Writes beside the file and flushes to disk, then renames over it in one step. A failure + /// leaves the stored file as it was, and neither the temporary file nor the copy of the + /// invalid original behind. + /// + private void Replace(byte[] content, FileStream? stored, string? copy) + { + string temporary = TemporaryName(); + bool replaced = false; + try + { + using(var stream = new FileStream(temporary, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + { + stream.Write(content); + stream.Flush(flushToDisk: true); + } + if(stored is null) + { + // No overwrite: a file that appeared since the read is not this instance's to replace. + File.Move(temporary, Path); + } + else + { + bool renamed = false; + Retry(() => renamed = _renameOver(temporary, Path)); + if(!renamed) + { + // This volume cannot rename over an open file, so the file is closed first. This + // move is refused while any handle is open on the file, a reader's included. + stored.Dispose(); + Retry(() => File.Move(temporary, Path, overwrite: true), IsBlockedByAHandle); + } + } + replaced = true; + } + finally + { + if(!replaced) + { + TryDelete(temporary); + if(copy is not null) + { + TryDelete(copy); + } + } + } + } + + + + /// A writer that died before its rename left a temporary file with this store's own name pattern; under the lock every such file is abandoned. + private void RemoveAbandonedTemporaryFiles() + { + string prefix = System.IO.Path.GetFileName(Path) + "."; + foreach(string file in Directory.EnumerateFiles(System.IO.Path.GetDirectoryName(Path)!, prefix + "*.tmp")) + { + string name = System.IO.Path.GetFileName(file); + if(name.Length == prefix.Length + 36 && Guid.TryParseExact(name.AsSpan(prefix.Length, 32), "N", out _)) + { + TryDelete(file); + } + } + } + + + + /// + /// Repeats an operation only while another handle is in the way, a bounded number of times; + /// the last failure is thrown. says which failures mean that: + /// a sharing violation unless the operation reports an open handle differently. + /// + private void Retry(Action operation, Func? inTheWay = null) + { + inTheWay ??= IsTransient; + for(int attempt = 1; ; attempt++) + { + try + { + operation(); + return; + } + catch(Exception exception) when(attempt < MaximumAttempts && inTheWay(exception)) + { + _wait(RetryDelay); + } + } + } + + + + /// A sharing violation: another handle on the file that is expected to go away. Access denied is not transient. + private static bool IsTransient(Exception exception) => exception is IOException && exception.HResult == SharingViolation; + + + + /// + /// What a move over the settings file fails with while another handle is open on it. Unlike + /// an open or a rename, that move answers access denied, which therefore has to be waited + /// out. A read-only file gives the same answer for good, so it is told apart by its attribute. + /// + private bool IsBlockedByAHandle(Exception exception) + => IsTransient(exception) || (exception is UnauthorizedAccessException && !new FileInfo(Path).IsReadOnly); + + + + /// Removes a file if it is there and can be removed; a folder with that name, or a file in use, is left. + private static void TryDelete(string path) + { + try + { + File.Delete(path); + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + // Cleanup only: the operation this was part of has already succeeded or failed. + } + } +} diff --git a/src/MarkdownViewr.Windows.Core/FileSwap.cs b/src/MarkdownViewr.Windows.Core/FileSwap.cs new file mode 100644 index 0000000..f2c9eae --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/FileSwap.cs @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using System.ComponentModel; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + + +namespace MarkdownViewr.Windows.Core; + +/// +/// Replaces a file by renaming another over it in a single step. The destination is never +/// absent, not even for an instant, and the rename works while the destination is open through +/// handles that allow deletion; those handles go on reading the content they opened. +/// refuses when any handle is open on the +/// destination, and is two renames with the +/// destination missing between them, so neither can do this. +/// +internal static partial class FileSwap +{ + private const uint DeleteAccess = 0x00010000; + private const uint ShareAll = 0x00000007; + private const uint OpenExisting = 3; + private const int FileRenameInfoEx = 22; + private const uint ReplaceIfExists = 0x1; + private const uint PosixSemantics = 0x2; + private const int AccessDenied = 5; + + + + /// + /// Renames over . Returns false when the + /// volume cannot rename over an open file, as file systems other than NTFS may not. Throws + /// with the Windows error as its HRESULT when the rename is refused, + /// for example by a handle on the destination that does not allow deletion, and + /// when access is denied. + /// + public static bool TryRenameOver(string source, string destination) + { + using SafeFileHandle handle = CreateFile(Verbatim(source), DeleteAccess, ShareAll, 0, OpenExisting, 0, 0); + if(handle.IsInvalid) + { + throw Failure(Marshal.GetLastPInvokeError()); + } + // FILE_RENAME_INFO: flags, root directory handle, name length in bytes, then the name itself. + // The name is copied as stored: a file name may hold text that is not well-formed UTF-16. + byte[] name = MemoryMarshal.AsBytes(Verbatim(destination).AsSpan()).ToArray(); + int lengthOffset = 2 * IntPtr.Size; + int nameOffset = lengthOffset + sizeof(int); + byte[] rename = new byte[nameOffset + name.Length + sizeof(char)]; + BitConverter.TryWriteBytes(rename, ReplaceIfExists | PosixSemantics); + BitConverter.TryWriteBytes(rename.AsSpan(lengthOffset), name.Length); + name.CopyTo(rename, nameOffset); + if(SetFileInformationByHandle(handle, FileRenameInfoEx, rename, rename.Length)) + { + return true; + } + int error = Marshal.GetLastPInvokeError(); + return IsUnsupported(error) ? false : throw Failure(error); + } + + + + /// + /// The form of a full path that Windows takes as written, without the 260-character limit the + /// plain form has in a process that has not opted in to long paths. The managed file calls add + /// it themselves; a direct Windows call has to. + /// + internal static string Verbatim(string fullPath) + { + if(fullPath.StartsWith(@"\\?\", StringComparison.Ordinal) || fullPath.StartsWith(@"\\.\", StringComparison.Ordinal)) + { + return fullPath; + } + return fullPath.StartsWith(@"\\", StringComparison.Ordinal) ? string.Concat(@"\\?\UNC\", fullPath.AsSpan(2)) : @"\\?\" + fullPath; + } + + + + /// The errors a volume answers with when it does not implement this kind of rename: invalid function, not supported, invalid parameter. + internal static bool IsUnsupported(int error) => error is 1 or 50 or 87; + + + + /// The exception a managed file operation would throw for the same Windows error. + private static Exception Failure(int error) + { + string message = new Win32Exception(error).Message; + return error == AccessDenied ? new UnauthorizedAccessException(message) : new IOException(message, unchecked((int)0x80070000) | error); + } + + + + /// Opens the file to be renamed with delete access, which a managed file stream cannot request. + [LibraryImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + private static partial SafeFileHandle CreateFile(string path, uint access, uint share, nint security, uint disposition, uint flags, nint template); + + + + /// Applies the rename to the open file. + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool SetFileInformationByHandle(SafeFileHandle file, int informationClass, byte[] information, int size); +} diff --git a/src/MarkdownViewr.Windows.Core/MarkdownViewr.Windows.Core.csproj b/src/MarkdownViewr.Windows.Core/MarkdownViewr.Windows.Core.csproj index 317c5d4..e4eab67 100644 --- a/src/MarkdownViewr.Windows.Core/MarkdownViewr.Windows.Core.csproj +++ b/src/MarkdownViewr.Windows.Core/MarkdownViewr.Windows.Core.csproj @@ -8,6 +8,8 @@ win-x86;win-x64;win-arm64 true + + true diff --git a/src/MarkdownViewr.Windows.Core/SettingsFault.cs b/src/MarkdownViewr.Windows.Core/SettingsFault.cs new file mode 100644 index 0000000..5f321c0 --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/SettingsFault.cs @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + + +namespace MarkdownViewr.Windows.Core; + +/// Why the settings file could not be used as stored; reported without paths or operating-system message text. +public enum SettingsFault +{ + /// The file does not hold settings at all; defaults are used and the next save keeps a copy of the file aside. + Corrupt, + /// Some stored properties were invalid and read as their defaults; the next save keeps a copy of the original aside. + Repaired, + /// The file could not be read, or another program was writing it; it is left untouched and nothing is written over it. + Unreadable, + /// Another instance kept the settings lock for every attempt; the change was not saved. + Busy, + /// The change could not be written; the stored file is unchanged. + WriteFailed, + /// The invalid original could not be copied aside; the change was saved without the copy. + NotKeptAside, +} diff --git a/src/MarkdownViewr.Windows.Core/SettingsRead.cs b/src/MarkdownViewr.Windows.Core/SettingsRead.cs new file mode 100644 index 0000000..b741223 --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/SettingsRead.cs @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + + +namespace MarkdownViewr.Windows.Core; + +/// What reading the settings file found; only forbids writing over it. +internal enum SettingsRead +{ + /// Valid stored settings. + Loaded, + /// No file yet; defaults apply and the first save creates it. + Missing, + /// A file with no content, as a program that clears a file before rewriting it leaves it; defaults apply and a save replaces it. + Empty, + /// Valid properties were kept and invalid ones dropped; a save keeps a copy of the original aside. + Repaired, + /// Not settings at all; defaults apply and a save keeps a copy of the file aside. + Corrupt, + /// The content is unknown, so it must not be replaced. + Unreadable, +} diff --git a/src/MarkdownViewr.Windows/App.Documents.cs b/src/MarkdownViewr.Windows/App.Documents.cs index 2c47571..4ac93a8 100644 --- a/src/MarkdownViewr.Windows/App.Documents.cs +++ b/src/MarkdownViewr.Windows/App.Documents.cs @@ -107,12 +107,14 @@ public partial class App - /// Records a successfully committed path through the latest store snapshot, then refreshes every recent menu. + /// Records a successfully committed path in the latest stored history under the settings lock, then refreshes every recent menu. 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,16 @@ public partial class App /// Clears only the local history, leaving open documents, reading preferences and files intact. 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; + if(saved) + { + // Copies of an invalid settings file kept beside it would still list the history. + AppIdentity.SettingsStore.DeleteKeptAside(); + } RefreshRecentMenus(); return saved; } diff --git a/src/MarkdownViewr.Windows/AppIdentity.cs b/src/MarkdownViewr.Windows/AppIdentity.cs index 4f472f8..0aff32b 100644 --- a/src/MarkdownViewr.Windows/AppIdentity.cs +++ b/src/MarkdownViewr.Windows/AppIdentity.cs @@ -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 - /// Each edition owns its own persisted preferences and recent document list. - public static SettingsStore SettingsStore { get; } = new(Paths.SettingsFile, AppSettingsContext.Default.AppSettings); + /// Each edition owns its own persisted preferences and recent document list; faults are logged by kind, never with a path or message text. + public static Core.AppSettingsStore SettingsStore { get; } = new(Paths.SettingsFile, ReportSettingsFault); @@ -56,6 +55,12 @@ internal static class AppIdentity + /// A settings fault is an operational event: the kind, exception type and HRESULT identify it without disclosing the profile. + 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}.")); + + + /// Daily activity files use the edition's own path and identity prefix. public static IActivityLog CreateLog() => FileActivityLog.ForApp(Info, Paths); } diff --git a/src/MarkdownViewr.Windows/MainWindow.Editor.cs b/src/MarkdownViewr.Windows/MainWindow.Editor.cs index aeee634..c8752c0 100644 --- a/src/MarkdownViewr.Windows/MainWindow.Editor.cs +++ b/src/MarkdownViewr.Windows/MainWindow.Editor.cs @@ -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; diff --git a/src/MarkdownViewr.Windows/MainWindow.PrintSettings.cs b/src/MarkdownViewr.Windows/MainWindow.PrintSettings.cs index 0f10fa9..f7d6bde 100644 --- a/src/MarkdownViewr.Windows/MainWindow.PrintSettings.cs +++ b/src/MarkdownViewr.Windows/MainWindow.PrintSettings.cs @@ -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; } diff --git a/src/MarkdownViewr.Windows/MainWindow.Settings.cs b/src/MarkdownViewr.Windows/MainWindow.Settings.cs index 97dea65..7596d26 100644 --- a/src/MarkdownViewr.Windows/MainWindow.Settings.cs +++ b/src/MarkdownViewr.Windows/MainWindow.Settings.cs @@ -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; } diff --git a/src/MarkdownViewr.Windows/MainWindow.ThemeManagement.cs b/src/MarkdownViewr.Windows/MainWindow.ThemeManagement.cs index db224e7..b48d327 100644 --- a/src/MarkdownViewr.Windows/MainWindow.ThemeManagement.cs +++ b/src/MarkdownViewr.Windows/MainWindow.ThemeManagement.cs @@ -56,8 +56,8 @@ public sealed partial class MainWindow /// Failed persistence does not apply an unsaved theme draft; sibling fields untouched by this edit retain their latest values. private async Task 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; diff --git a/src/MarkdownViewr.Windows/MainWindow.xaml.cs b/src/MarkdownViewr.Windows/MainWindow.xaml.cs index e26f58f..c8af6c1 100644 --- a/src/MarkdownViewr.Windows/MainWindow.xaml.cs +++ b/src/MarkdownViewr.Windows/MainWindow.xaml.cs @@ -522,8 +522,7 @@ public sealed partial class MainWindow : Window, IDisposable /// Saves the settings and reports a failed save in the status line. 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); } diff --git a/tests/MarkdownViewr.Windows.Core.Tests/AppIdentityTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/AppIdentityTests.cs index fe5ba41..5feb21f 100644 --- a/tests/MarkdownViewr.Windows.Core.Tests/AppIdentityTests.cs +++ b/tests/MarkdownViewr.Windows.Core.Tests/AppIdentityTests.cs @@ -90,8 +90,8 @@ public class AppSettingsTests public void SaveAndLoad_KeepsTheTheme() { using var temp = new TempDirectory(); - var store = new KofTwentyTwo.AppKit.Settings.SettingsStore(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); diff --git a/tests/MarkdownViewr.Windows.Core.Tests/AppSettingsStoreTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/AppSettingsStoreTests.cs new file mode 100644 index 0000000..86d831e --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/AppSettingsStoreTests.cs @@ -0,0 +1,1440 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using System.Text; +using System.Text.Json.Nodes; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// +/// The settings file is shared by windows and processes: these tests use real files, real +/// handles held the way other programs hold them, and concurrent store instances. They count +/// attempts instead of measuring time. +/// +public class AppSettingsStoreTests +{ + private const string KeptPath = @"C:\docs\keep-me.md"; + + + + /// A first run has no file: defaults apply and reading creates nothing. + [Fact] + public void Load_MissingFile_ReturnsDefaultsWithoutCreatingAnything() + { + using var temp = new TempDirectory(); + var faults = new List(); + 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"))); + } + + + + /// The first save creates the folder and file, and returns the sanitized settings that were written. + [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"))); + } + + + + /// The store addresses one file whatever the working folder becomes later. + [Fact] + public void Constructor_RelativePath_IsMadeAbsolute() + { + var store = new AppSettingsStore(Path.Combine("relative-profile", "settings.json")); + Assert.True(Path.IsPathFullyQualified(store.Path)); + Assert.Equal(Path.GetFullPath(Path.Combine("relative-profile", "settings.json")), store.Path); + Assert.Throws(() => new AppSettingsStore(" ")); + } + + + + /// A change is applied to what is stored now, not to what this instance read earlier. + [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); + } + + + + /// Callers that merge return a new object: that object is what is saved and what comes back. + [Fact] + public void Update_ChangeReturnsANewInstance_SavesAndReturnsThatInstance() + { + using var temp = new TempDirectory(); + Seed(temp); + var store = new AppSettingsStore(temp.File("settings.json")); + var replacement = new AppSettings { DocumentTheme = "Dracula", OutlineWidth = 9000 }; + AppSettings? saved = store.Update(_ => replacement); + Assert.Same(replacement, saved); + Assert.Equal(500, replacement.OutlineWidth); + AppSettings loaded = store.Load(); + Assert.Equal("Dracula", loaded.DocumentTheme); + Assert.Equal("System", loaded.Theme); + Assert.Empty(loaded.RecentFiles); + } + + + + /// Stored values outside their range are repaired on every read, not only on save. + [Fact] + public void Load_StoredValuesOutOfRange_AreSanitized() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), """{"OutlineDepth":99,"Theme":"Purple","OutlineWidth":-4}"""); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load(); + Assert.Equal(6, loaded.OutlineDepth); + Assert.Equal("System", loaded.Theme); + Assert.Equal(140, loaded.OutlineWidth); + } + + + + /// A file saved by hand with a byte order mark, in UTF-8 or UTF-16, is still the user's settings. + [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(); + 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); + } + + + + /// Without a byte order mark the file is UTF-8, so text outside ASCII reads as written. + [Fact] + public void Load_Utf8WithoutByteOrderMark_ReadsTextOutsideAscii() + { + using var temp = new TempDirectory(); + File.WriteAllBytes(temp.File("settings.json"), new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes("""{"GlobalCss":"/* café ☕ */"}""")); + Assert.Equal("/* café ☕ */", new AppSettingsStore(temp.File("settings.json")).Load().GlobalCss); + } + + + + /// What a hand edit commonly leaves behind is still valid settings: a trailing comma, a comment, a quoted number. + [Theory] + [InlineData("""{"Theme":"Dark","OutlineDepth":4,}""")] + [InlineData("""{ /* my settings */ "Theme":"Dark", "OutlineDepth":4 }""")] + [InlineData("""{"Theme":"Dark","OutlineDepth":"4"}""")] + public void Load_HandEditedSyntax_ReadsWithoutAFault(string content) + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), content); + var faults = new List(); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load(); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal(4, loaded.OutlineDepth); + Assert.Empty(faults); + } + + + + /// The file on disk stays what earlier versions wrote and can read: indented UTF-8 JSON with no byte order mark. + [Fact] + public void Update_WrittenFile_IsIndentedUtf8WithoutByteOrderMarkAndReadableByThePreviousStore() + { + using var temp = new TempDirectory(); + Seed(temp); + byte[] bytes = File.ReadAllBytes(temp.File("settings.json")); + Assert.Equal((byte)'{', bytes[0]); + Assert.Contains(Environment.NewLine + " \"Theme\": \"Dark\"", Encoding.UTF8.GetString(bytes), StringComparison.Ordinal); + var previous = new KofTwentyTwo.AppKit.Settings.SettingsStore(temp.File("settings.json"), AppSettingsContext.Default.AppSettings); + AppSettings read = previous.Load(); + Assert.Equal("Nord", read.DocumentTheme); + Assert.Equal("body { color: red }", read.GlobalCss); + Assert.Equal([KeptPath], read.RecentFiles); + read.Theme = "Light"; + Assert.True(previous.Save(read)); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load(); + Assert.Equal("Light", loaded.Theme); + Assert.Equal("Nord", loaded.DocumentTheme); + } + + + + /// A change that throws leaves the stored file as it was and open to every other program at once. + [Fact] + public void Update_ChangeThrows_ReleasesTheStoredFile() + { + using var temp = new TempDirectory(); + Seed(temp); + byte[] before = File.ReadAllBytes(temp.File("settings.json")); + var store = new AppSettingsStore(temp.File("settings.json")); + Assert.Throws(() => store.Update(_ => throw new InvalidOperationException("change failed"))); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + } + Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// Another handle holding the file for a moment (a scanner, a backup) is waited out, not read as an empty profile. + [Fact] + public void Load_FileLockedBriefly_RetriesAndReadsTheStoredValues() + { + using var temp = new TempDirectory(); + Seed(temp); + using var exclusive = new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None); + var delays = new List(); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), delay => + { + delays.Add(delay); + if(delays.Count == 3) + { + exclusive.Dispose(); + } + }); + AppSettings loaded = store.Load(); + Assert.Equal([AppSettingsStore.RetryDelay, AppSettingsStore.RetryDelay, AppSettingsStore.RetryDelay], delays); + Assert.Equal(TimeSpan.FromMilliseconds(50), AppSettingsStore.RetryDelay); + Assert.Equal("Nord", loaded.DocumentTheme); + Assert.Equal([KeptPath], loaded.RecentFiles); + Assert.Empty(faults); + } + + + + /// A file that stays unreadable yields defaults for display and is reported; the attempts are bounded. + [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(exception); + } + + + + /// + /// 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. + /// + [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(); + 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); + } + + + + /// + /// Access denied is permanent, not a moment to wait out: it is reported at once, defaults are + /// shown, and nothing is written. A folder where the file should be answers exactly that way. + /// + [Fact] + public void LoadAndUpdate_AccessDenied_ReportUnreadableWithoutWaitingOrWriting() + { + using var temp = new TempDirectory(); + Directory.CreateDirectory(temp.File("settings.json")); + 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++); + Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme); + bool changed = false; + Assert.Null(store.Update(settings => + { + changed = true; + return settings; + })); + Assert.False(changed); + Assert.Equal(0, waits); + Assert.Equal([SettingsFault.Unreadable, SettingsFault.Unreadable], faults.Select(entry => entry.Fault)); + Assert.All(faults, entry => Assert.IsType(entry.Exception)); + Assert.True(Directory.Exists(temp.File("settings.json"))); + Assert.Equal(["settings.json.lock"], FileNames(temp.Path)); + } + + + + /// + /// Another program rewriting the file in place has truncated it and not finished. That is a + /// busy file, not invalid content: it is not set aside, and once the writer finishes the + /// change is applied to what the writer wrote. + /// + [Fact] + public void Update_FileBeingRewrittenInPlace_WaitsForTheWriterAndKeepsItsContent() + { + using var temp = new TempDirectory(); + Seed(temp); + using var writer = new FileStream(temp.File("settings.json"), FileMode.Create, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => + { + if(++waits == 2) + { + writer.Write("""{"Theme":"Light","DocumentTheme":"Dracula","GlobalCss":"p { margin: 0 }"}"""u8); + writer.Dispose(); + } + }); + AppSettings? saved = store.Update(settings => + { + settings.OutlineDepth = 5; + return settings; + }); + Assert.NotNull(saved); + Assert.Equal(2, waits); + Assert.Empty(faults); + AppSettings loaded = store.Load(); + Assert.Equal("Light", loaded.Theme); + Assert.Equal("Dracula", loaded.DocumentTheme); + Assert.Equal("p { margin: 0 }", loaded.GlobalCss); + Assert.Equal(5, loaded.OutlineDepth); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A writer that never finishes leaves the save refused and the file exactly as the writer has it. + [Fact] + public void Update_FileStillBeingRewritten_IsRefusedAndNothingIsSetAside() + { + using var temp = new TempDirectory(); + Seed(temp); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { }); + using(new FileStream(temp.File("settings.json"), FileMode.Create, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete)) + { + Assert.Null(store.Update(settings => settings)); + Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme); + } + Assert.Equal([SettingsFault.Unreadable], faults); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + Assert.Equal(0, new FileInfo(temp.File("settings.json")).Length); + } + + + + /// Content that is not settings at all yields defaults, is reported, and stays where it is until something is saved. + [Theory] + [InlineData("{")] + [InlineData("null")] + [InlineData("[1]")] + [InlineData("\"text\"")] + [InlineData("not json")] + public void Load_InvalidContent_ReturnsDefaultsAndLeavesTheFile(string content) + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), content); + var faults = new List(); + 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)); + } + + + + /// An invalid value in a file that also has hand-edited syntax still costs only that value. + [Fact] + public void Load_InvalidValueBesideHandEditedSyntax_KeepsEveryOtherValue() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), """{ /* mine */ "Theme":"Dark", "OutlineDepth":"deep", "DocumentTheme":"Nord", }"""); + var faults = new List(); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load(); + Assert.Equal([SettingsFault.Repaired], faults); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal("Nord", loaded.DocumentTheme); + Assert.Equal(3, loaded.OutlineDepth); + } + + + + /// An invalid property costs only that property: every other stored preference, before and after it, is kept. + [Fact] + public void Load_InvalidProperties_KeepsEveryOtherProperty() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), """{"Theme":"Dark","OutlineDepth":5.5,"DocumentTheme":"Nord","RecentFiles":["C:\\docs\\a.md",5],"GlobalCss":"body { color: red }","FutureSetting":{"a":1},"WrapOutlineEntries":true,"ContentWidthPx":1234,"Frontmatter":"Metadata","RenderHighlight":false,"ShowSplashScreen":false,"OutlineWidth":300,"Print":{"Theme":"Nord","Borders":true}}"""); + var faults = new List(); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load(); + Assert.Equal([SettingsFault.Repaired], faults); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal("Nord", loaded.DocumentTheme); + Assert.Equal("body { color: red }", loaded.GlobalCss); + Assert.True(loaded.WrapOutlineEntries); + Assert.Equal(1234, loaded.ContentWidthPx); + Assert.Equal("Metadata", loaded.Frontmatter); + Assert.False(loaded.RenderHighlight); + Assert.False(loaded.ShowSplashScreen); + Assert.Equal(300, loaded.OutlineWidth); + Assert.Equal("Nord", loaded.Print.Theme); + Assert.True(loaded.Print.Borders); + Assert.Equal(3, loaded.OutlineDepth); + Assert.Empty(loaded.RecentFiles); + } + + + + /// + /// Text the JSON reader cannot decode, an unpaired surrogate escape, is an invalid property + /// like any other: it is dropped, the rest load, and nothing is thrown into startup. + /// + [Fact] + public void LoadAndUpdate_UndecodableText_DropsThatPropertyAndSavesTheRest() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), "{\"DocumentTheme\":\"Nord\",\"GlobalCss\":\"a\\uD800b\",\"Theme\":\"Dark\"}"); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)); + AppSettings loaded = store.Load(); + Assert.Equal("Nord", loaded.DocumentTheme); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal("", loaded.GlobalCss); + Assert.NotNull(store.Update(settings => settings)); + Assert.Equal([SettingsFault.Repaired, SettingsFault.Repaired], faults); + Assert.Equal("Dark", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.corrupt", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A file with no content is not invalid settings: it reads as defaults, without a fault and without a wait. + [Fact] + public void Load_EmptyFile_ReturnsDefaultsWithoutAFaultOrAWait() + { + using var temp = new TempDirectory(); + File.WriteAllBytes(temp.File("settings.json"), []); + int waits = 0; + var faults = new List(); + Assert.Equal("Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++).Load().DocumentTheme); + Assert.Empty(faults); + Assert.Equal(0, waits); + } + + + + /// + /// A program that clears the file, closes it, and then writes it again leaves it empty for a + /// moment. A save waits for the content instead of filling the gap with defaults. + /// + [Fact] + public void Update_FileClearedThenRewritten_WaitsForTheContent() + { + using var temp = new TempDirectory(); + Seed(temp); + File.WriteAllBytes(temp.File("settings.json"), []); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => + { + if(++waits == 2) + { + File.WriteAllText(temp.File("settings.json"), """{"Theme":"Light","DocumentTheme":"Dracula"}"""); + } + }); + AppSettings? saved = store.Update(settings => + { + settings.OutlineDepth = 5; + return settings; + }); + Assert.NotNull(saved); + Assert.Equal(2, waits); + Assert.Empty(faults); + Assert.Equal("Dracula", store.Load().DocumentTheme); + Assert.Equal("Light", store.Load().Theme); + Assert.Equal(5, store.Load().OutlineDepth); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A file that stays empty is replaced after the wait, and nothing is kept aside: there is nothing to keep. + [Fact] + public void Update_FileStaysEmpty_SavesAfterTheWaitWithoutKeepingAnythingAside() + { + using var temp = new TempDirectory(); + File.WriteAllBytes(temp.File("settings.json"), []); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Dark"; + return settings; + })); + Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits); + Assert.Empty(faults); + Assert.Equal("Dark", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// + /// A handle another program merely holds with write access does not delay a read for display: + /// the stored values are read beside it. + /// + [Fact] + public void Load_FileHeldWithWriteAccess_ReadsTheStoredValuesWithoutWaiting() + { + using var temp = new TempDirectory(); + Seed(temp); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Delete)) + { + Assert.Equal("Nord", store.Load().DocumentTheme); + } + Assert.Equal(0, waits); + Assert.Empty(faults); + } + + + + /// + /// Content read beside a writer that does not parse may simply be half written. The read waits + /// for the writer; if the content never becomes valid it is reported as unreadable, not as + /// corrupt, because it cannot be judged while someone is writing it. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Load_HalfWrittenContentBesideAWriter_WaitsAndNeverCallsItCorrupt(bool writerFinishes) + { + using var temp = new TempDirectory(); + using var writer = new FileStream(temp.File("settings.json"), FileMode.Create, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete); + writer.Write("""{"DocumentTheme":"Dra"""u8); + writer.Flush(); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => + { + if(++waits == 2 && writerFinishes) + { + writer.Write("""cula"}"""u8); + writer.Flush(); + } + }); + AppSettings loaded = store.Load(); + Assert.Equal(writerFinishes ? "Dracula" : "Catppuccin Mocha", loaded.DocumentTheme); + Assert.Equal(writerFinishes ? 2 : AppSettingsStore.MaximumAttempts - 1, waits); + Assert.Equal(writerFinishes ? [] : [SettingsFault.Unreadable], faults); + } + + + + /// A repeated property beside an invalid value is still read without throwing into startup. + [Fact] + public void Load_RepeatedPropertyAndInvalidValue_DoesNotThrow() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), """{"Theme":"Dark","Theme":"Light","OutlineDepth":"deep"}"""); + var faults = new List(); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load(); + Assert.Contains(Assert.Single(faults), new[] { SettingsFault.Corrupt, SettingsFault.Repaired }); + Assert.Equal(3, loaded.OutlineDepth); + } + + + + /// A saved theme whose text the JSON reader cannot decode is discarded like any other invalid theme; it cannot stop the application starting. + [Fact] + public void Load_SavedThemeWithUnpairedSurrogate_DiscardsThatThemeOnly() + { + using var temp = new TempDirectory(); + string valid = Themes.ThemeCatalog.Find("Nord").Json; + string undecodable = valid.Replace("\"Nord\"", "\"\\uD800\"", StringComparison.Ordinal); + Assert.NotEqual(valid, undecodable, StringComparer.Ordinal); + var stored = new JsonObject { ["Theme"] = "Dark", ["UserThemes"] = new JsonArray(undecodable, valid) }; + File.WriteAllText(temp.File("settings.json"), stored.ToJsonString()); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load(); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal([valid], loaded.UserThemes); + } + + + + /// Saving over invalid content keeps that content under a free name; an earlier kept file is not overwritten. + [Fact] + public void Update_InvalidContent_KeepsItAsideUnderAFreeNameAndSavesTheChange() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), "{ \"Theme\": \"Dark\", "); + File.WriteAllText(temp.File("settings.json.corrupt"), "an earlier invalid file"); + var store = new AppSettingsStore(temp.File("settings.json")); + Assert.NotNull(store.Update(settings => + { + settings.DocumentTheme = "Nord"; + return settings; + })); + Assert.Equal("an earlier invalid file", File.ReadAllText(temp.File("settings.json.corrupt"))); + Assert.Equal("{ \"Theme\": \"Dark\", ", File.ReadAllText(temp.File("settings.json.corrupt-2"))); + Assert.Equal("Nord", store.Load().DocumentTheme); + Assert.Equal("System", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.corrupt", "settings.json.corrupt-2", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A repaired file is saved with its valid values and the change, and the original is kept for the value that was dropped. + [Fact] + public void Update_RepairedContent_SavesTheValidValuesAndKeepsTheOriginalAside() + { + using var temp = new TempDirectory(); + const string Original = """{"Theme":"Dark","OutlineDepth":"deep","DocumentTheme":"Nord"}"""; + File.WriteAllText(temp.File("settings.json"), Original); + var store = new AppSettingsStore(temp.File("settings.json")); + Assert.NotNull(store.Update(settings => + { + settings.GlobalCss = "p { margin: 0 }"; + return settings; + })); + Assert.Equal(Original, File.ReadAllText(temp.File("settings.json.corrupt"))); + var faults = new List(); + AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load(); + Assert.Empty(faults); + Assert.Equal("Dark", loaded.Theme); + Assert.Equal("Nord", loaded.DocumentTheme); + Assert.Equal("p { margin: 0 }", loaded.GlobalCss); + } + + + + /// Kept files are bounded: once every slot is used the newest slot is reused and the older ones stay. + [Fact] + public void Update_EveryKeptSlotUsed_ReusesTheNewestSlot() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json.corrupt"), "kept 1"); + for(int slot = 2; slot <= AppSettingsStore.MaximumKeptAside; slot++) + { + string number = slot.ToString(System.Globalization.CultureInfo.InvariantCulture); + File.WriteAllText(temp.File("settings.json.corrupt-" + number), "kept " + number); + } + File.WriteAllText(temp.File("settings.json"), "newest invalid"); + Assert.NotNull(new AppSettingsStore(temp.File("settings.json")).Update(settings => settings)); + Assert.Equal("kept 1", File.ReadAllText(temp.File("settings.json.corrupt"))); + Assert.Equal("kept 8", File.ReadAllText(temp.File("settings.json.corrupt-8"))); + Assert.Equal("newest invalid", File.ReadAllText(temp.File("settings.json.corrupt-9"))); + Assert.False(File.Exists(temp.File("settings.json.corrupt-10"))); + Assert.Equal(9, AppSettingsStore.MaximumKeptAside); + } + + + + /// A folder where a kept-aside copy would go is skipped; the next name is used. + [Fact] + public void Update_KeptAsideNameIsAFolder_UsesTheNextName() + { + using var temp = new TempDirectory(); + Directory.CreateDirectory(temp.File("settings.json.corrupt")); + File.WriteAllText(temp.File("settings.json"), "not json"); + var faults = new List(); + Assert.NotNull(new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Update(settings => settings)); + Assert.Equal("not json", File.ReadAllText(temp.File("settings.json.corrupt-2"))); + Assert.Equal([SettingsFault.Corrupt], faults); + } + + + + /// When no copy can be made the save still goes through, and the missing copy is reported. + [Fact] + public void Update_CopyCannotBeKeptAside_SavesAndReportsIt() + { + using var temp = new TempDirectory(); + for(int slot = 2; slot <= AppSettingsStore.MaximumKeptAside; slot++) + { + Directory.CreateDirectory(temp.File("settings.json.corrupt-" + slot.ToString(System.Globalization.CultureInfo.InvariantCulture))); + } + Directory.CreateDirectory(temp.File("settings.json.corrupt")); + File.WriteAllText(temp.File("settings.json"), "not json"); + var faults = new List<(SettingsFault Fault, Exception? Exception)>(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception))); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Dark"; + return settings; + })); + Assert.Equal([SettingsFault.Corrupt, SettingsFault.NotKeptAside], faults.Select(entry => entry.Fault)); + Assert.IsType(faults[1].Exception); + Assert.Equal("Dark", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// Kept-aside copies hold whatever the file held, so they can be removed on request; folders with those names are left. + [Fact] + public void DeleteKeptAside_CopiesAndAFolder_RemovesOnlyTheCopies() + { + using var temp = new TempDirectory(); + Seed(temp); + File.WriteAllText(temp.File("settings.json.corrupt"), "copy 1"); + File.WriteAllText(temp.File("settings.json.corrupt-2"), "copy 2"); + File.WriteAllText(temp.File("settings.json.corrupt-9"), "copy 9"); + File.WriteAllText(temp.File("settings.json.corrupt-10"), "not a kept-aside name"); + Directory.CreateDirectory(temp.File("settings.json.corrupt-3")); + new AppSettingsStore(temp.File("settings.json")).DeleteKeptAside(); + Assert.Equal(["settings.json", "settings.json.corrupt-10", "settings.json.lock"], FileNames(temp.Path)); + Assert.True(Directory.Exists(temp.File("settings.json.corrupt-3"))); + } + + + + /// + /// The stored file is in place at the moment the new one is renamed over it and immediately + /// after: there is no step at which the folder has no settings file. + /// + [Fact] + public void Update_AtTheRename_TheStoredFileIsStillInPlace() + { + using var temp = new TempDirectory(); + Seed(temp); + string path = temp.File("settings.json"); + var observed = new List(); + var store = new AppSettingsStore(path, null, null, (source, destination) => + { + observed.Add(File.Exists(destination) ? "stored file present before" : "stored file missing before"); + observed.Add(File.Exists(source) ? "new file written" : "new file missing"); + bool renamed = FileSwap.TryRenameOver(source, destination); + observed.Add(File.Exists(destination) ? "stored file present after" : "stored file missing after"); + return renamed; + }); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Light"; + return settings; + })); + Assert.Equal(["stored file present before", "new file written", "stored file present after"], observed); + Assert.Equal("Light", store.Load().Theme); + } + + + + /// On a volume that cannot rename over an open file the store closes its own handle and replaces the file the older way. + [Fact] + public void Update_VolumeCannotRenameOverAnOpenFile_ReplacesAfterClosingItsHandle() + { + using var temp = new TempDirectory(); + Seed(temp); + int attempts = 0; + var store = new AppSettingsStore(temp.File("settings.json"), null, _ => { }, (_, _) => + { + attempts++; + return false; + }); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Light"; + return settings; + })); + Assert.Equal(1, attempts); + Assert.Equal("Light", store.Load().Theme); + Assert.Equal("Nord", store.Load().DocumentTheme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// Only a sharing violation is waited out at the rename; any other I/O failure fails the save at once. + [Theory] + [InlineData(unchecked((int)0x80070020), 3, true)] + [InlineData(unchecked((int)0x80070021), 0, false)] + [InlineData(unchecked((int)0x8007001F), 0, false)] + [InlineData(0x00000020, 0, false)] + public void Update_RenameFails_WaitsOnlyForASharingViolation(int result, int expectedWaits, bool saved) + { + using var temp = new TempDirectory(); + Seed(temp); + int waits = 0; + int attempts = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++, (source, destination) => + ++attempts <= 3 ? throw new IOException("refused", result) : FileSwap.TryRenameOver(source, destination)); + Assert.Equal(saved, store.Update(settings => settings) is not null); + Assert.Equal(expectedWaits, waits); + Assert.Equal(saved ? [] : [SettingsFault.WriteFailed], faults); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// When the save over invalid content fails, the invalid file is still in place and the copy made for this save is removed again. + [Fact] + public void Update_InvalidContentAndFailedSave_LeavesTheInvalidFileInPlace() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), "not json"); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { }); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + Assert.Null(store.Update(settings => settings)); + } + Assert.Equal([SettingsFault.Corrupt, SettingsFault.WriteFailed], faults); + Assert.Equal("not json", File.ReadAllText(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A failed save over invalid content leaves every earlier kept-aside copy as it was, the last one included. + [Fact] + public void Update_InvalidContentAndFailedSaveWithEveryNameUsed_KeepsTheEarlierCopies() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json.corrupt"), "kept 1"); + for(int slot = 2; slot <= AppSettingsStore.MaximumKeptAside; slot++) + { + string number = slot.ToString(System.Globalization.CultureInfo.InvariantCulture); + File.WriteAllText(temp.File("settings.json.corrupt-" + number), "kept " + number); + } + File.WriteAllText(temp.File("settings.json"), "newest invalid"); + string[] before = FileNames(temp.Path); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { }); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + Assert.Null(store.Update(settings => settings)); + } + Assert.Equal([SettingsFault.Corrupt, SettingsFault.WriteFailed], faults); + Assert.Equal("kept 9", File.ReadAllText(temp.File("settings.json.corrupt-9"))); + Assert.Equal("kept 1", File.ReadAllText(temp.File("settings.json.corrupt"))); + Assert.Equal("newest invalid", File.ReadAllText(temp.File("settings.json"))); + Assert.Equal([.. before, "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// When the invalid file cannot be copied, here because another program has locked its bytes, the save still goes through, the missing copy is reported and no partial copy is left. + [Fact] + [System.Runtime.Versioning.SupportedOSPlatform("windows")] + public void Update_InvalidFileCannotBeRead_SavesWithoutACopyAndReportsIt() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), "not json"); + var faults = new List<(SettingsFault Fault, Exception? Exception)>(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception))); + FileStream? locker = null; + try + { + Assert.NotNull(store.Update(settings => + { + locker = new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + locker.Lock(0, locker.Length); + settings.Theme = "Dark"; + return settings; + })); + } + finally + { + locker?.Dispose(); + } + Assert.Equal([SettingsFault.Corrupt, SettingsFault.NotKeptAside], faults.Select(entry => entry.Fault)); + Assert.IsType(faults[1].Exception); + Assert.Equal("Dark", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A file past the size bound is replaced without being copied: copying it would stall the window and fill the disk with something that was never settings. + [Fact] + public void Update_OversizedFile_IsReplacedWithoutACopy() + { + using var temp = new TempDirectory(); + using(var oversized = new FileStream(temp.File("settings.json"), FileMode.CreateNew)) + { + oversized.SetLength(AppSettingsStore.MaximumBytes + 1); + } + var faults = new List<(SettingsFault Fault, Exception? Exception)>(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception))); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Dark"; + return settings; + })); + Assert.Equal([SettingsFault.Corrupt, SettingsFault.NotKeptAside], faults.Select(entry => entry.Fault)); + Assert.Null(faults[1].Exception); + Assert.Equal("Dark", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// + /// On a volume that cannot rename over an open file, the move that replaces it is refused with + /// access denied while a reader has the file open. That is waited out like any other handle, + /// each pause being the store's retry delay. + /// + [Fact] + public void Update_FallbackMoveBlockedByAReader_WaitsForIt() + { + using var temp = new TempDirectory(); + Seed(temp); + FileStream? reader = new(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete); + var delays = new List(); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), delay => + { + delays.Add(delay); + if(delays.Count == 2) + { + reader.Dispose(); + } + }, (_, _) => false); + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Light"; + return settings; + })); + Assert.Equal([AppSettingsStore.RetryDelay, AppSettingsStore.RetryDelay], delays); + Assert.Empty(faults); + Assert.Equal("Light", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// The same move on a read-only file is refused for good, which the store sees from the attribute: it gives up at once and leaves the file and the folder as they were. + [Fact] + public void Update_FallbackMoveOnAReadOnlyFile_FailsWithoutWaiting() + { + using var temp = new TempDirectory(); + Seed(temp); + byte[] before = File.ReadAllBytes(temp.File("settings.json")); + File.SetAttributes(temp.File("settings.json"), FileAttributes.ReadOnly); + try + { + 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++, (_, _) => false); + Assert.Null(store.Update(settings => settings)); + Assert.Equal(0, waits); + Assert.Equal(SettingsFault.WriteFailed, Assert.Single(faults).Fault); + Assert.IsType(faults[0].Exception); + Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + finally + { + File.SetAttributes(temp.File("settings.json"), FileAttributes.Normal); + } + } + + + + /// + /// The data folder can be anywhere the person points it. A path past the classic 260-character + /// limit, and a folder whose name is not well-formed UTF-16 (which the file system allows), + /// are saved to again and again like any other. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Update_UnusualFolderPath_SavesRepeatedly(bool longPath) + { + using var temp = new TempDirectory(); + string folder = longPath + ? Path.Combine(temp.Path, new string('a', 120), new string('b', 120), new string('c', 60)) + : Path.Combine(temp.Path, "profile \uD800 name"); + var faults = new List(); + var store = new AppSettingsStore(Path.Combine(folder, "settings.json"), (fault, _) => faults.Add(fault)); + Assert.Equal(longPath, store.Path.Length > 260); + foreach(string theme in new[] { "Dark", "Light", "Dark" }) + { + Assert.NotNull(store.Update(settings => + { + settings.Theme = theme; + return settings; + })); + Assert.Equal(theme, store.Load().Theme); + } + Assert.Empty(faults); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(folder)); + } + + + + /// A change that throws leaves the file, the lock and the folder as they were; the next save works. + [Fact] + public void Update_ChangeThrows_LeavesTheFileAndReleasesTheLock() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("settings.json"), "not json"); + var store = new AppSettingsStore(temp.File("settings.json")); + Assert.Throws(() => store.Update(_ => throw new InvalidOperationException("caller bug"))); + Assert.Equal("not json", File.ReadAllText(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + Assert.Throws(() => store.Update(null!)); + Assert.NotNull(store.Update(settings => settings)); + } + + + + /// Reading for display holds nothing afterwards: another program can replace or rewrite the file at once. + [Fact] + public void Load_AfterReading_ReleasesTheFile() + { + using var temp = new TempDirectory(); + Seed(temp); + var store = new AppSettingsStore(temp.File("settings.json")); + Assert.Equal("Nord", store.Load().DocumentTheme); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + Assert.Equal("Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), wait: _ => { }).Load().DocumentTheme); + } + Assert.Equal("Nord", store.Load().DocumentTheme); + } + + + + /// + /// Between judging the stored content and swapping in the new file, no other program can + /// rewrite it in place: the content that was read is the content that is replaced. + /// + [Fact] + public void Update_WhileTheChangeRuns_TheStoredFileCannotBeRewrittenInPlace() + { + using var temp = new TempDirectory(); + Seed(temp); + var store = new AppSettingsStore(temp.File("settings.json")); + bool refused = false; + Assert.NotNull(store.Update(settings => + { + refused = Record.Exception(() => new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete).Dispose()) is IOException; + settings.Theme = "Light"; + return settings; + })); + Assert.True(refused); + Assert.Equal("Light", store.Load().Theme); + Assert.Equal("Nord", store.Load().DocumentTheme); + } + + + + /// A file that appears after this instance found none was written by someone else and is not overwritten. + [Fact] + public void Update_FileAppearsAfterTheRead_IsNotOverwritten() + { + using var temp = new TempDirectory(); + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)); + Assert.Null(store.Update(settings => + { + File.WriteAllText(temp.File("settings.json"), """{"DocumentTheme":"Dracula"}"""); + settings.Theme = "Light"; + return settings; + })); + Assert.Equal([SettingsFault.WriteFailed], faults); + Assert.Equal("""{"DocumentTheme":"Dracula"}""", File.ReadAllText(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + AppSettings? saved = store.Update(settings => + { + settings.Theme = "Light"; + return settings; + }); + Assert.NotNull(saved); + Assert.Equal("Dracula", saved.DocumentTheme); + } + + + + /// A file is read up to the size bound and not loaded past it, even when it would parse. + [Theory] + [InlineData(0, true)] + [InlineData(1, false)] + public void Load_FileAtAndPastTheSizeBound_IsBoundedExactly(int extraBytes, bool loaded) + { + using var temp = new TempDirectory(); + byte[] padded = new byte[AppSettingsStore.MaximumBytes + extraBytes]; + Array.Fill(padded, (byte)' '); + """{"DocumentTheme":"Nord"}"""u8.CopyTo(padded); + File.WriteAllBytes(temp.File("settings.json"), padded); + var faults = new List(); + Assert.Equal(loaded ? "Nord" : "Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load().DocumentTheme); + Assert.Equal(loaded ? [] : [SettingsFault.Corrupt], faults); + } + + + + /// While another instance holds the lock for every attempt, the change is refused and the file is unchanged. + [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(); + 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"))); + } + + + + /// + /// Mutual exclusion, shown without threads: while one instance is inside its change, a second + /// instance's save is refused for every attempt, and the first instance's change is what ends + /// up stored. + /// + [Fact] + public void Update_WhileAnotherInstanceIsInsideItsChange_IsRefusedAndTheFirstChangeIsSaved() + { + using var temp = new TempDirectory(); + Seed(temp); + int waits = 0; + var faults = new List(); + var first = new AppSettingsStore(temp.File("settings.json")); + var second = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++); + bool secondChanged = false; + AppSettings? saved = first.Update(settings => + { + Assert.Null(second.Update(other => + { + secondChanged = true; + other.Theme = "Light"; + return other; + })); + settings.DocumentTheme = "Dracula"; + return settings; + }); + Assert.NotNull(saved); + Assert.False(secondChanged); + Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits); + Assert.Equal([SettingsFault.Busy], faults); + AppSettings loaded = second.Load(); + Assert.Equal("Dracula", loaded.DocumentTheme); + Assert.Equal("Dark", loaded.Theme); + Assert.NotNull(second.Update(other => other)); + } + + + + /// A lock released while this instance waits lets the change through. + [Fact] + public void Update_LockReleasedDuringWait_SavesTheChange() + { + using var temp = new TempDirectory(); + Seed(temp); + using var other = new FileStream(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); + } + + + + /// A reader that allows the file to be replaced, as this store's own reads do, does not stop a save. + [Fact] + public void Update_FileOpenByAReaderThatAllowsReplacement_StillSaves() + { + using var temp = new TempDirectory(); + Seed(temp); + int waits = 0; + var store = new AppSettingsStore(temp.File("settings.json"), wait: _ => waits++); + using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete)) + { + Assert.NotNull(store.Update(settings => + { + settings.Theme = "Light"; + return settings; + })); + } + Assert.Equal(0, waits); + Assert.Equal("Light", store.Load().Theme); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A handle that forbids replacing the file fails the save cleanly: the stored file stays and no temporary file is left. + [Fact] + public void Update_ReplaceBlocked_LeavesTheStoredFileAndNoTemporaryFile() + { + using var temp = new TempDirectory(); + Seed(temp); + byte[] before = File.ReadAllBytes(temp.File("settings.json")); + int waits = 0; + var faults = new List(); + 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)); + } + + + + /// A read-only settings file fails the save at once instead of stalling the window for every attempt. + [Fact] + public void Update_ReadOnlyFile_FailsWithoutWaiting() + { + using var temp = new TempDirectory(); + Seed(temp); + byte[] before = File.ReadAllBytes(temp.File("settings.json")); + 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++); + File.SetAttributes(temp.File("settings.json"), FileAttributes.ReadOnly); + try + { + Assert.Null(store.Update(settings => + { + settings.Theme = "Light"; + return settings; + })); + Assert.Equal("Nord", store.Load().DocumentTheme); + } + finally + { + File.SetAttributes(temp.File("settings.json"), FileAttributes.Normal); + } + Assert.Equal(0, waits); + (SettingsFault fault, Exception? exception) = Assert.Single(faults); + Assert.Equal(SettingsFault.WriteFailed, fault); + Assert.IsType(exception); + Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json"))); + Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path)); + } + + + + /// A lock that can never be taken, here a folder with the lock's name, is a failed write reported at once, not a busy lock. + [Fact] + public void Update_LockCannotBeOpened_FailsWithoutWaiting() + { + using var temp = new TempDirectory(); + Directory.CreateDirectory(temp.File("settings.json.lock")); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++); + Assert.Null(store.Update(settings => settings)); + Assert.Equal(0, waits); + Assert.Equal([SettingsFault.WriteFailed], faults); + Assert.Empty(FileNames(temp.Path)); + } + + + + /// A location that cannot hold a settings folder reports a failed write instead of throwing into the caller. + [Fact] + public void Update_FolderCannotBeCreated_ReportsWriteFailed() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("blocker"), "a file where the folder should be"); + int waits = 0; + var faults = new List(); + var store = new AppSettingsStore(Path.Combine(temp.File("blocker"), "settings.json"), (fault, _) => faults.Add(fault), _ => waits++); + Assert.Null(store.Update(settings => settings)); + Assert.Equal([SettingsFault.WriteFailed], faults); + Assert.Equal(0, waits); + Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme); + } + + + + /// + /// Only this store's own abandoned temporary files are removed by the next save. One still in + /// use, the fixed-name temporary file an older release writes, and anything else ending in + /// .tmp are left alone. + /// + [Fact] + public void Update_AbandonedTemporaryFiles_RemovesOnlyItsOwn() + { + using var temp = new TempDirectory(); + Seed(temp); + string own = "settings.json." + Guid.NewGuid().ToString("N") + ".tmp"; + string inUse = "settings.json." + Guid.NewGuid().ToString("N") + ".tmp"; + string notHex = "settings.json." + new string('z', 32) + ".tmp"; + foreach(string name in new[] { own, inUse, notHex, "settings.json.tmp", "settings.json.bak.tmp", "settings.json.0123.tmp", "notes.tmp" }) + { + File.WriteAllText(temp.File(name), "left behind"); + } + var store = new AppSettingsStore(temp.File("settings.json")); + using(new FileStream(temp.File(inUse), FileMode.Open, FileAccess.Read, FileShare.None)) + { + Assert.NotNull(store.Update(settings => settings)); + } + string[] expected = [inUse, notHex, "notes.tmp", "settings.json", "settings.json.0123.tmp", "settings.json.bak.tmp", "settings.json.lock", "settings.json.tmp"]; + Assert.Equal(expected.Order(StringComparer.Ordinal), FileNames(temp.Path), StringComparer.Ordinal); + } + + + + /// + /// Instances that each record their own documents at the same time keep every one. A refused + /// save is retried a bounded number of times, as a user would; a save that reported success + /// must never be erased by another instance. + /// + [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 failures = new System.Collections.Concurrent.ConcurrentQueue(); + var threads = new List(); + for(int instance = 0; instance < Instances; instance++) + { + int id = instance; + var thread = new Thread(() => RecordDocuments(temp.File("settings.json"), id, Each, start, failures)); + thread.Start(); + threads.Add(thread); + } + start.Set(); + threads.ForEach(thread => thread.Join()); + Assert.Empty(failures); + 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)); + } + + + + /// One instance's work: a failure is recorded for the test to assert, never thrown on a bare thread. + private static void RecordDocuments(string file, int id, int count, ManualResetEventSlim start, System.Collections.Concurrent.ConcurrentQueue failures) + { + try + { + var store = new AppSettingsStore(file); + start.Wait(); + for(int index = 0; index < count; index++) + { + string path = $@"C:\docs\instance{id}-{index}.md"; + int refusals = 0; + while(store.Update(settings => + { + settings.RecentFiles = Documents.RecentDocuments.Record(settings.RecentFiles, path); + return settings; + }) is null) + { + if(++refusals == 50) + { + failures.Enqueue(path + " was refused 50 times"); + return; + } + } + } + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException or InvalidOperationException) + { + failures.Enqueue(exception.ToString()); + } + } + + + + /// A profile with non-default values in several unrelated fields, so a wipe is visible. + 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; + })); + } + + + + /// The folder's file names in ordinal order. + private static string[] FileNames(string folder) + => Directory.GetFiles(folder).Select(file => Path.GetFileName(file)!).Order(StringComparer.Ordinal).ToArray(); +} diff --git a/tests/MarkdownViewr.Windows.Core.Tests/EditorConfigurationTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/EditorConfigurationTests.cs index 0319464..2e09c1d 100644 --- a/tests/MarkdownViewr.Windows.Core.Tests/EditorConfigurationTests.cs +++ b/tests/MarkdownViewr.Windows.Core.Tests/EditorConfigurationTests.cs @@ -76,21 +76,24 @@ public sealed class EditorConfigurationTests - /// Atomic AppKit persistence retains named IDs, order and folder mode using the trimmed serializer. + /// Atomic persistence retains named IDs, order and folder mode using the trimmed serializer. [Fact] public void SaveAndLoad_EditorList_PreservesIdentityOrderAndMode() { using var temporary = new TempDirectory(); - var store = new KofTwentyTwo.AppKit.Settings.SettingsStore(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); } diff --git a/tests/MarkdownViewr.Windows.Core.Tests/FileSwapTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/FileSwapTests.cs new file mode 100644 index 0000000..4868c21 --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/FileSwapTests.cs @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// Renaming one real file over another, with other real handles open on the destination. +public sealed class FileSwapTests +{ + /// A full path is given the form Windows takes as written: a drive path, a network path, and paths that already have it. + [Theory] + [InlineData(@"C:\data\settings.json", @"\\?\C:\data\settings.json")] + [InlineData(@"\\server\share\settings.json", @"\\?\UNC\server\share\settings.json")] + [InlineData(@"\\?\C:\data\settings.json", @"\\?\C:\data\settings.json")] + [InlineData(@"\\?\UNC\server\share\settings.json", @"\\?\UNC\server\share\settings.json")] + [InlineData(@"\\.\C:\data\settings.json", @"\\.\C:\data\settings.json")] + public void Verbatim_FullPath_IsPrefixedOnce(string path, string expected) + { + Assert.Equal(expected, FileSwap.Verbatim(path)); + } + + + + /// The rename replaces the destination and removes the source name. + [Fact] + public void TryRenameOver_ExistingDestination_ReplacesIt() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("stored.json"), "old"); + File.WriteAllText(temp.File("new.tmp"), "new"); + Assert.True(FileSwap.TryRenameOver(temp.File("new.tmp"), temp.File("stored.json"))); + Assert.Equal("new", File.ReadAllText(temp.File("stored.json"))); + Assert.False(File.Exists(temp.File("new.tmp"))); + } + + + + /// A reader that allows deletion does not stop the rename, and goes on reading the content it opened. + [Fact] + public void TryRenameOver_DestinationOpenByAReaderThatAllowsDeletion_ReplacesItUnderTheReader() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("stored.json"), "old content"); + File.WriteAllText(temp.File("new.tmp"), "new"); + using(var reader = new StreamReader(new FileStream(temp.File("stored.json"), FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete))) + { + Assert.True(FileSwap.TryRenameOver(temp.File("new.tmp"), temp.File("stored.json"))); + Assert.Equal("new", File.ReadAllText(temp.File("stored.json"))); + Assert.Equal("old content", reader.ReadToEnd()); + } + Assert.Equal(["stored.json"], Directory.GetFiles(temp.Path).Select(file => Path.GetFileName(file)!), StringComparer.Ordinal); + } + + + + /// A handle that does not allow deletion refuses the rename with a sharing violation, and both files stay as they were. + [Fact] + public void TryRenameOver_DestinationOpenWithoutDeleteSharing_IsASharingViolation() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("stored.json"), "old"); + File.WriteAllText(temp.File("new.tmp"), "new"); + using(new FileStream(temp.File("stored.json"), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + IOException refused = Assert.Throws(() => FileSwap.TryRenameOver(temp.File("new.tmp"), temp.File("stored.json"))); + Assert.Equal(unchecked((int)0x80070020), refused.HResult); + } + Assert.Equal("old", File.ReadAllText(temp.File("stored.json"))); + Assert.Equal("new", File.ReadAllText(temp.File("new.tmp"))); + } + + + + /// A read-only destination is access denied, as it is for a managed move. + [Fact] + public void TryRenameOver_ReadOnlyDestination_IsAccessDenied() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("stored.json"), "old"); + File.WriteAllText(temp.File("new.tmp"), "new"); + File.SetAttributes(temp.File("stored.json"), FileAttributes.ReadOnly); + try + { + Assert.Throws(() => FileSwap.TryRenameOver(temp.File("new.tmp"), temp.File("stored.json"))); + } + finally + { + File.SetAttributes(temp.File("stored.json"), FileAttributes.Normal); + } + Assert.Equal("old", File.ReadAllText(temp.File("stored.json"))); + } + + + + /// A source that does not exist is reported with the Windows error for a missing file. + [Fact] + public void TryRenameOver_MissingSource_IsAnIoFailure() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("stored.json"), "old"); + IOException missing = Assert.Throws(() => FileSwap.TryRenameOver(temp.File("absent.tmp"), temp.File("stored.json"))); + Assert.Equal(unchecked((int)0x80070002), missing.HResult); + Assert.Equal("old", File.ReadAllText(temp.File("stored.json"))); + } + + + + /// A destination that does not exist yet is simply created by the rename. + [Fact] + public void TryRenameOver_NoDestinationYet_RenamesTheSource() + { + using var temp = new TempDirectory(); + File.WriteAllText(temp.File("new.tmp"), "new"); + Assert.True(FileSwap.TryRenameOver(temp.File("new.tmp"), temp.File("stored.json"))); + Assert.Equal("new", File.ReadAllText(temp.File("stored.json"))); + } + + + + /// Only the errors a volume gives for an unimplemented rename mean "use another way"; every other error is a failure. + [Theory] + [InlineData(1, true)] + [InlineData(50, true)] + [InlineData(87, true)] + [InlineData(0, false)] + [InlineData(2, false)] + [InlineData(5, false)] + [InlineData(32, false)] + [InlineData(17, false)] + public void IsUnsupported_WindowsError_SeparatesAnUnimplementedRenameFromAFailure(int error, bool unsupported) + => Assert.Equal(unsupported, FileSwap.IsUnsupported(error)); +}