From 87f834dd97cee3be6a109b082c981db94357541a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 23:31:34 +0000 Subject: [PATCH] Resolve a relative data directory once, for both the files and the lock [patch] JsonKeybindingRepository resolved its lock key with Path.GetFullPath at construction but kept the relative path for every file operation, so after the current directory changed a save went to another folder (and was lost on the next launch) while the lock still guarded the original one. The directory is now resolved once and that one path is used for both. Fixes ktsu-dev/Keybinding#217 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW26R7a4LaEL6NnFvBq --- Keybinding.Test/RelativeDataDirectoryTests.cs | 97 +++++++++++++++++++ .../Services/JsonKeybindingRepository.cs | 6 +- 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 Keybinding.Test/RelativeDataDirectoryTests.cs diff --git a/Keybinding.Test/RelativeDataDirectoryTests.cs b/Keybinding.Test/RelativeDataDirectoryTests.cs new file mode 100644 index 0000000..584c874 --- /dev/null +++ b/Keybinding.Test/RelativeDataDirectoryTests.cs @@ -0,0 +1,97 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Test; + +using ktsu.Keybinding.Core; +using ktsu.Keybinding.Core.Models; +using ktsu.Keybinding.Core.Services; + +/// +/// A relative data directory is resolved once, when the repository is built, so a later change of the +/// current directory (an OpenFileDialog without RestoreDirectory does this) moves neither the files nor +/// the lock (ktsu-dev/Keybinding#217). These tests change the process-wide current directory, so they +/// must not run alongside other tests. +/// +[TestClass] +[DoNotParallelize] +public class RelativeDataDirectoryTests +{ + private string _root = null!; + private string _originalDirectory = null!; + + [TestInitialize] + public void Setup() + { + _originalDirectory = Environment.CurrentDirectory; + _root = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(Path.Combine(_root, "a")); + Directory.CreateDirectory(Path.Combine(_root, "b")); + } + + [TestCleanup] + public void Cleanup() + { + Environment.CurrentDirectory = _originalDirectory; + if (Directory.Exists(_root)) + { + Directory.Delete(_root, recursive: true); + } + } + + [TestMethod] + public async Task Repository_SavesToTheDirectoryResolvedAtConstruction_AfterTheCurrentDirectoryChanges() + { + Environment.CurrentDirectory = Path.Combine(_root, "a"); + JsonKeybindingRepository repository = new("data"); + await repository.SaveProfileAsync(new Profile("first", "First")).ConfigureAwait(false); + + Environment.CurrentDirectory = Path.Combine(_root, "b"); + await repository.SaveProfileAsync(new Profile("second", "Second")).ConfigureAwait(false); + + Assert.IsFalse(Directory.Exists(Path.Combine(_root, "b", "data")), "No save may follow the current directory."); + JsonKeybindingRepository reader = new(Path.Combine(_root, "a", "data")); + IReadOnlyCollection profiles = await reader.LoadAllProfilesAsync().ConfigureAwait(false); + Assert.AreEqual("first,second", string.Join(',', profiles.Select(p => p.Id).Order(StringComparer.Ordinal))); + } + + [TestMethod] + public async Task Manager_RebindAfterTheCurrentDirectoryChanges_IsKeptInTheOriginalDirectory() + { + Environment.CurrentDirectory = Path.Combine(_root, "a"); + using (KeybindingManager manager = new("data")) + { + await manager.InitializeAsync().ConfigureAwait(false); + manager.Commands.RegisterCommand(new Command("copy", "Copy")); + manager.CreateDefaultProfile(); + manager.Keybindings.BindChord("copy", Chord.Parse("Ctrl+C")); + await manager.SaveAsync().ConfigureAwait(false); + + Environment.CurrentDirectory = Path.Combine(_root, "b"); + manager.Keybindings.BindChord("copy", Chord.Parse("Ctrl+Shift+C")); + await manager.SaveAsync().ConfigureAwait(false); + } + + Assert.IsFalse(Directory.Exists(Path.Combine(_root, "b", "data")), "No save may follow the current directory."); + Environment.CurrentDirectory = Path.Combine(_root, "a"); + using KeybindingManager restarted = new("data"); + await restarted.InitializeAsync().ConfigureAwait(false); + Assert.AreEqual(Chord.Parse("Ctrl+Shift+C"), restarted.Keybindings.GetChord("copy")); + } + + [TestMethod] + public async Task RepositoriesFromOneRelativePathUnderDifferentCurrentDirectories_WriteToTheirOwnDirectories() + { + Environment.CurrentDirectory = Path.Combine(_root, "a"); + JsonKeybindingRepository first = new("data"); + Environment.CurrentDirectory = Path.Combine(_root, "b"); + JsonKeybindingRepository second = new("data"); + + await Task.WhenAll(Enumerable.Range(0, 100).Select(i => + (i % 2 == 0 ? first : second).SaveProfileAsync(new Profile("p" + i, "P" + i)))).ConfigureAwait(false); + + IReadOnlyCollection inA = await new JsonKeybindingRepository(Path.Combine(_root, "a", "data")).LoadAllProfilesAsync().ConfigureAwait(false); + IReadOnlyCollection inB = await new JsonKeybindingRepository(Path.Combine(_root, "b", "data")).LoadAllProfilesAsync().ConfigureAwait(false); + Assert.AreEqual(50, inA.Count, "Every save through the first repository lands in a/data."); + Assert.AreEqual(50, inB.Count, "Every save through the second repository lands in b/data."); + } +} diff --git a/Keybinding/Services/JsonKeybindingRepository.cs b/Keybinding/Services/JsonKeybindingRepository.cs index 8da582d..e182071 100644 --- a/Keybinding/Services/JsonKeybindingRepository.cs +++ b/Keybinding/Services/JsonKeybindingRepository.cs @@ -51,8 +51,10 @@ public JsonKeybindingRepository(string dataDirectory) throw new ArgumentException(Constants.ErrorMessages.DataDirectoryNullOrWhitespace, nameof(dataDirectory)); } - _dataDirectory = dataDirectory.Trim(); - _lock = DirectoryLocks.GetOrAdd(Path.GetFullPath(_dataDirectory), _ => new SemaphoreSlim(1, 1)); + // Resolve a relative directory once, so a later change of the current directory moves neither + // the files this repository reads and writes nor the lock that guards them. + _dataDirectory = Path.GetFullPath(dataDirectory.Trim()); + _lock = DirectoryLocks.GetOrAdd(_dataDirectory, _ => new SemaphoreSlim(1, 1)); _jsonOptions = new JsonSerializerOptions { WriteIndented = true,