From c8dff666d8da833ac0c69d415410eede83f0127a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 02:25:37 +0000 Subject: [PATCH 1/2] Give each CreateManager(dataDirectory) its own profiles and commands [patch] With a service provider, CreateManager(dataDirectory) reused the singleton ICommandRegistry and IProfileManager, so managers for different directories shared profiles and saving one wrote another directory's profiles into it. It now always builds a fresh KeybindingManager for the directory. The parameterless CreateManager() doc now says it returns the DI-registered manager when there is one, rather than claiming a new instance. Fixes ktsu-dev/Keybinding#114 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D --- .../KeybindingManagerFactoryTests.cs | 80 +++++++++++++++++++ .../Contracts/IKeybindingManagerFactory.cs | 9 ++- .../Services/KeybindingManagerFactory.cs | 22 ++--- 3 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 Keybinding.Test/KeybindingManagerFactoryTests.cs diff --git a/Keybinding.Test/KeybindingManagerFactoryTests.cs b/Keybinding.Test/KeybindingManagerFactoryTests.cs new file mode 100644 index 0000000..d6ed4fb --- /dev/null +++ b/Keybinding.Test/KeybindingManagerFactoryTests.cs @@ -0,0 +1,80 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Test; + +using Microsoft.Extensions.DependencyInjection; +using ktsu.Keybinding.Core; +using ktsu.Keybinding.Core.Contracts; +using ktsu.Keybinding.Core.Extensions; + +[TestClass] +public class KeybindingManagerFactoryTests +{ + private string _dirA = null!; + private string _dirB = null!; + + [TestInitialize] + public void Setup() + { + _dirA = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + _dirB = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(_dirA); + Directory.CreateDirectory(_dirB); + } + + [TestCleanup] + public void Cleanup() + { + foreach (string dir in new[] { _dirA, _dirB }) + { + if (Directory.Exists(dir)) + { + Directory.Delete(dir, recursive: true); + } + } + } + + private static async Task LoadProfileIdsAsync(string dataDirectory) + { + using KeybindingManager manager = new(dataDirectory); + await manager.InitializeAsync().ConfigureAwait(false); + return string.Join(',', manager.Profiles.GetAllProfiles().Select(p => p.Id).Order(StringComparer.Ordinal)); + } + + [TestMethod] + public async Task CreateManager_WithServiceProvider_KeepsDirectoriesSeparate() + { + ServiceCollection services = new(); + services.AddKeybinding(_dirA); + using ServiceProvider provider = services.BuildServiceProvider(); + IKeybindingManagerFactory factory = provider.GetRequiredService(); + + using KeybindingManager a = factory.CreateManager(_dirA); + using KeybindingManager b = factory.CreateManager(_dirB); + + await a.InitializeAsync().ConfigureAwait(false); + a.Profiles.CreateProfile("user-a", "User A"); + await a.SaveAsync().ConfigureAwait(false); + + await b.InitializeAsync().ConfigureAwait(false); + await b.SaveAsync().ConfigureAwait(false); + + Assert.IsFalse(b.Profiles.ProfileExists("user-a")); + Assert.AreEqual("user-a", await LoadProfileIdsAsync(_dirA).ConfigureAwait(false)); + Assert.AreEqual(string.Empty, await LoadProfileIdsAsync(_dirB).ConfigureAwait(false)); + } + + [TestMethod] + public void CreateManager_WithServiceProvider_DoesNotReuseSingletonState() + { + ServiceCollection services = new(); + services.AddKeybinding(_dirA); + using ServiceProvider provider = services.BuildServiceProvider(); + IKeybindingManagerFactory factory = provider.GetRequiredService(); + + using KeybindingManager manager = factory.CreateManager(_dirB); + + Assert.AreNotSame(provider.GetRequiredService(), manager.Profiles); + Assert.AreNotSame(provider.GetRequiredService(), manager.Commands); + } +} diff --git a/Keybinding/Contracts/IKeybindingManagerFactory.cs b/Keybinding/Contracts/IKeybindingManagerFactory.cs index 6402b41..f674f91 100644 --- a/Keybinding/Contracts/IKeybindingManagerFactory.cs +++ b/Keybinding/Contracts/IKeybindingManagerFactory.cs @@ -8,15 +8,18 @@ namespace ktsu.Keybinding.Core.Contracts; public interface IKeybindingManagerFactory { /// - /// Creates a new KeybindingManager instance + /// Gets a KeybindingManager for the default data directory /// - /// A new KeybindingManager instance + /// + /// The KeybindingManager registered with dependency injection when there is one, which is shared rather than new; + /// otherwise a new instance + /// public KeybindingManager CreateManager(); /// /// Creates a new KeybindingManager instance with specified data directory /// /// Directory to store keybinding data - /// A new KeybindingManager instance + /// A new KeybindingManager instance whose commands and profiles are not shared with any other manager public KeybindingManager CreateManager(string dataDirectory); } diff --git a/Keybinding/Services/KeybindingManagerFactory.cs b/Keybinding/Services/KeybindingManagerFactory.cs index 661b20e..d85ffee 100644 --- a/Keybinding/Services/KeybindingManagerFactory.cs +++ b/Keybinding/Services/KeybindingManagerFactory.cs @@ -50,20 +50,10 @@ public KeybindingManager CreateManager() } /// - public KeybindingManager CreateManager(string dataDirectory) - { - if (_serviceProvider != null) - { - // If we have a service provider, try to resolve services but use custom directory - if (_serviceProvider.GetService(typeof(ICommandRegistry)) is ICommandRegistry commandRegistry && - _serviceProvider.GetService(typeof(IProfileManager)) is IProfileManager profileManager) - { - JsonKeybindingRepository repository = new(dataDirectory); - return new KeybindingManager(commandRegistry, profileManager, repository); - } - } - - // Fallback to standard constructor - return new KeybindingManager(dataDirectory); - } + /// + /// The manager always gets its own command registry and profile manager. Reusing the singletons registered in + /// the service provider would let managers for different directories share profiles, so saving one directory + /// would write another directory's profiles into it. + /// + public KeybindingManager CreateManager(string dataDirectory) => new(dataDirectory); } From 44c85a38eb44260e60641ec49b268afc714cf7e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 02:28:17 +0000 Subject: [PATCH 2/2] Filter the cleanup directories with Where Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D --- Keybinding.Test/KeybindingManagerFactoryTests.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Keybinding.Test/KeybindingManagerFactoryTests.cs b/Keybinding.Test/KeybindingManagerFactoryTests.cs index d6ed4fb..2c5c9de 100644 --- a/Keybinding.Test/KeybindingManagerFactoryTests.cs +++ b/Keybinding.Test/KeybindingManagerFactoryTests.cs @@ -25,12 +25,9 @@ public void Setup() [TestCleanup] public void Cleanup() { - foreach (string dir in new[] { _dirA, _dirB }) + foreach (string dir in new[] { _dirA, _dirB }.Where(Directory.Exists)) { - if (Directory.Exists(dir)) - { - Directory.Delete(dir, recursive: true); - } + Directory.Delete(dir, recursive: true); } }