diff --git a/DesktopClock.Tests/SettingsTests.cs b/DesktopClock.Tests/SettingsTests.cs
index 73e62a2..6167119 100644
--- a/DesktopClock.Tests/SettingsTests.cs
+++ b/DesktopClock.Tests/SettingsTests.cs
@@ -2,6 +2,7 @@
using System.IO;
using System.Reflection;
using System.Windows.Media;
+using System.Windows.Threading;
using DesktopClock.Properties;
namespace DesktopClock.Tests;
@@ -70,6 +71,132 @@ public void Save_ThenPopulate_ShouldRoundTripWpfAndTimeTypes()
Assert.Equal(DateTimeKind.Unspecified, loaded.CountdownTo.Kind);
}
+ [Fact]
+ public void Save_OverExistingFile_ShouldReplaceItWithoutLeavingTempFile()
+ {
+ using var _ = new TempSettingsFileScope();
+
+ var settings = CreateSettingsInstance();
+ settings.Format = "first";
+ Assert.True(settings.Save());
+
+ settings.Format = "second";
+ Assert.True(settings.Save());
+
+ var loaded = CreateSettingsInstance();
+ PopulateFromFile(loaded);
+
+ Assert.Equal("second", loaded.Format);
+ Assert.False(File.Exists(Settings.FilePath + ".tmp"));
+ }
+
+ [Fact]
+ public void ChangingASetting_ShouldSaveItShortlyAfterwards()
+ {
+ using var _ = new TempSettingsFileScope();
+
+ using var __ = new CanBeSavedScope();
+
+ var settings = CreateSettingsInstance();
+ settings.Format = "saved without exiting";
+
+ // The save runs on a short timer, so let the dispatcher run for a bit.
+ PumpDispatcher(TimeSpan.FromSeconds(2));
+
+ // Read the file directly; populating another instance here would queue its own save and leak into other tests.
+ Assert.Contains("saved without exiting", File.ReadAllText(Settings.FilePath));
+ }
+
+ [Fact]
+ public void EditingTheFile_ShouldCancelASaveStillWaitingFromAnEarlierChange()
+ {
+ using var _ = new TempSettingsFileScope();
+ using var __ = new CanBeSavedScope();
+
+ var settings = CreateSettingsInstance();
+ settings.Height = 99;
+
+ // Edit the file by hand before that change is saved, then let the watcher report it.
+ const string handEdit = "{ \"Format\": \"edited by hand\" }";
+ File.WriteAllText(Settings.FilePath, handEdit);
+ typeof(Settings).GetMethod("FileChanged", BindingFlags.NonPublic | BindingFlags.Instance)!.Invoke(settings, new object[] { null, null });
+ PumpDispatcher(TimeSpan.FromSeconds(2));
+
+ Assert.Equal("edited by hand", settings.Format);
+ Assert.Equal(handEdit, File.ReadAllText(Settings.FilePath));
+ }
+
+ [Fact]
+ public void Load_WithBrieflyLockedFile_ShouldWaitAndKeepSettings()
+ {
+ using var _ = new TempSettingsFileScope();
+
+ var original = CreateSettingsInstance();
+ original.Format = "kept through a lock";
+ Assert.True(original.Save());
+
+ // Hold the file like antivirus scanning it, then let go shortly after loading starts.
+ var lockStream = new FileStream(Settings.FilePath, FileMode.Open, FileAccess.Read, FileShare.None);
+ var releaser = new System.Threading.Thread(() =>
+ {
+ System.Threading.Thread.Sleep(150);
+ lockStream.Dispose();
+ });
+ releaser.Start();
+
+ var loaded = LoadAndAttemptSave();
+ releaser.Join();
+
+ Assert.Equal("kept through a lock", loaded.Format);
+ }
+
+ ///
+ /// Runs the app's startup load, restoring the static state it sets so other tests aren't affected.
+ ///
+ private static Settings LoadAndAttemptSave()
+ {
+ var canBeSaved = typeof(Settings).GetProperty(nameof(Settings.CanBeSaved), BindingFlags.Public | BindingFlags.Static)!.GetSetMethod(nonPublic: true)!;
+ var originalCanBeSaved = Settings.CanBeSaved;
+
+ try
+ {
+ canBeSaved.Invoke(null, new object[] { false });
+
+ var loadAndAttemptSave = typeof(Settings).GetMethod("LoadAndAttemptSave", BindingFlags.NonPublic | BindingFlags.Static)!;
+ return (Settings)loadAndAttemptSave.Invoke(null, null)!;
+ }
+ finally
+ {
+ canBeSaved.Invoke(null, new object[] { originalCanBeSaved });
+ }
+ }
+
+ private static void PumpDispatcher(TimeSpan duration)
+ {
+ var frame = new DispatcherFrame();
+ var stopTimer = new DispatcherTimer { Interval = duration };
+ stopTimer.Tick += (_, _) =>
+ {
+ stopTimer.Stop();
+ frame.Continue = false;
+ };
+ stopTimer.Start();
+ Dispatcher.PushFrame(frame);
+ }
+
+ ///
+ /// Lets settings save on their own during a test, as they do once the app has confirmed the file is writable.
+ ///
+ private sealed class CanBeSavedScope : IDisposable
+ {
+ private static readonly MethodInfo _setCanBeSaved = typeof(Settings).GetProperty(nameof(Settings.CanBeSaved), BindingFlags.Public | BindingFlags.Static)!.GetSetMethod(nonPublic: true)!;
+ private readonly bool _original = Settings.CanBeSaved;
+
+ public CanBeSavedScope() => _setCanBeSaved.Invoke(null, new object[] { true });
+
+ public void Dispose() => _setCanBeSaved.Invoke(null, new object[] { _original });
+ }
+
private static Settings CreateSettingsInstance() =>
(Settings)Activator.CreateInstance(typeof(Settings), nonPublic: true)!;
diff --git a/DesktopClock/MainWindow.xaml.cs b/DesktopClock/MainWindow.xaml.cs
index 0c54298..95386d1 100644
--- a/DesktopClock/MainWindow.xaml.cs
+++ b/DesktopClock/MainWindow.xaml.cs
@@ -311,6 +311,7 @@ private void Window_MouseDown(object sender, MouseButtonEventArgs e)
DragMove();
PixelShifter?.UpdateBasePosition(this);
+ Settings.Default.Placement = this.GetPlacement();
UpdateTimeString();
_systemClockTimer.Start();
@@ -452,6 +453,7 @@ private void NudgeWindow(KeyEventArgs e)
Top += nudge.Y;
PixelShifter?.UpdateBasePosition(this);
+ Settings.Default.Placement = this.GetPlacement();
e.Handled = true;
}
diff --git a/DesktopClock/Properties/Settings.cs b/DesktopClock/Properties/Settings.cs
index 2616065..a6576bc 100644
--- a/DesktopClock/Properties/Settings.cs
+++ b/DesktopClock/Properties/Settings.cs
@@ -1,7 +1,9 @@
-using System;
+using System;
using System.ComponentModel;
using System.IO;
+using System.Runtime.InteropServices;
using System.Windows.Media;
+using System.Windows.Threading;
using DesktopClock.Utilities;
using Newtonsoft.Json;
using WpfWindowPlacement;
@@ -11,6 +13,8 @@ namespace DesktopClock.Properties;
public sealed class Settings : INotifyPropertyChanged, IDisposable
{
private readonly FileSystemWatcher _watcher;
+ private readonly DispatcherTimer _saveTimer;
+ private bool _populatingFromFile;
private string _resolvedTimeZoneId;
private TimeZoneInfo _resolvedTimeZone;
private static readonly Lazy _default = new(LoadAndAttemptSave);
@@ -40,6 +44,23 @@ private Settings()
EnableRaisingEvents = true,
};
_watcher.Changed += FileChanged;
+
+ // Save shortly after a change instead of only on exit, so a crash, a forced close, or a shutdown that doesn't let the app exit normally only loses the last moment of changes. The short wait groups rapid changes, like dragging a slider, into one save.
+ _saveTimer = new DispatcherTimer { Interval = TimeSpan.FromSeconds(1) };
+ _saveTimer.Tick += (_, _) =>
+ {
+ _saveTimer.Stop();
+ Save();
+ };
+ PropertyChanged += (_, _) =>
+ {
+ // Values that were just read from the file are already saved.
+ if (!CanBeSaved || _populatingFromFile)
+ return;
+
+ _saveTimer.Stop();
+ _saveTimer.Start();
+ };
}
#pragma warning disable CS0067 // The event 'Settings.PropertyChanged' is never used. Handled by Fody.
@@ -441,7 +462,7 @@ public bool Save()
{
try
{
- File.WriteAllText(FilePath, json);
+ WriteAllTextAtomically(FilePath, json);
return true;
}
catch
@@ -463,6 +484,34 @@ public bool Save()
return false;
}
+ ///
+ /// Writes to a temporary file and then swaps it in, so the file is never left empty or half-written if the app is killed mid-save, such as during a Windows shutdown (#7).
+ ///
+ private static void WriteAllTextAtomically(string path, string contents)
+ {
+ var tempPath = path + ".tmp";
+
+ using (var stream = new FileStream(tempPath, FileMode.Create, FileAccess.Write))
+ using (var writer = new StreamWriter(stream))
+ {
+ writer.Write(contents);
+ writer.Flush();
+
+ // Make sure the new contents are on disk before they replace the old ones.
+ stream.Flush(flushToDisk: true);
+ }
+
+ // Swap it in with a single rename; File.Replace isn't safe here because it renames the old file away first, so being killed in between leaves no settings file at all.
+ if (!MoveFileEx(tempPath, path, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH))
+ throw new IOException($"Couldn't replace {path}.", Marshal.GetHRForLastWin32Error());
+ }
+
+ private const int MOVEFILE_REPLACE_EXISTING = 0x1;
+ private const int MOVEFILE_WRITE_THROUGH = 0x8;
+
+ [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
+ private static extern bool MoveFileEx(string existingFileName, string newFileName, int flags);
+
///
/// Populates the given settings with values from the default path.
///
@@ -480,15 +529,24 @@ private static void Populate(Settings settings)
///
private static Settings LoadFromFile()
{
- try
- {
- var settings = new Settings();
- Populate(settings);
- return settings;
- }
- catch
+ var settings = new Settings();
+
+ // The file can be locked for a moment, such as while antivirus scans it at sign-in, and falling back to defaults here would save them over every setting. Give it about as long as saving does before giving up.
+ for (var attempt = 1; ; attempt++)
{
- return new();
+ try
+ {
+ Populate(settings);
+ return settings;
+ }
+ catch (IOException) when (attempt < 4 && Exists)
+ {
+ System.Threading.Thread.Sleep(250);
+ }
+ catch
+ {
+ return new();
+ }
}
}
@@ -499,7 +557,7 @@ private static Settings LoadAndAttemptSave()
{
var settings = LoadFromFile();
- if (!File.Exists(FilePath))
+ if (!Exists)
{
settings.ApplySystemThemeDefaultsIfAvailable();
}
@@ -514,13 +572,24 @@ private static Settings LoadAndAttemptSave()
///
private void FileChanged(object sender, FileSystemEventArgs e)
{
- try
+ // Reload on the thread that saves and cancel any save still waiting from an earlier change, so it can't write a half-reloaded file or put old values back over the edit.
+ _saveTimer.Dispatcher.BeginInvoke(new Action(() =>
{
- Populate(this);
- }
- catch
- {
- }
+ _saveTimer.Stop();
+
+ try
+ {
+ _populatingFromFile = true;
+ Populate(this);
+ }
+ catch
+ {
+ }
+ finally
+ {
+ _populatingFromFile = false;
+ }
+ }));
}
private void ApplySystemThemeDefaultsIfAvailable()