Give each CreateManager(dataDirectory) its own profiles and commands [patch]

Keybinding.Test/KeybindingManagerFactoryTests.cs

@@ -0,0 +1,77 @@
+// 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 }.Where(Directory.Exists))
+		{
+			Directory.Delete(dir, recursive: true);
+		}
+	}
+
+	private static async Task<string> 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<IKeybindingManagerFactory>();
+
+		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<IKeybindingManagerFactory>();
+
+		using KeybindingManager manager = factory.CreateManager(_dirB);
+
+		Assert.AreNotSame(provider.GetRequiredService<IProfileManager>(), manager.Profiles);
+		Assert.AreNotSame(provider.GetRequiredService<ICommandRegistry>(), manager.Commands);
+	}
+}

Keybinding/Contracts/IKeybindingManagerFactory.cs

@@ -8,15 +8,18 @@ namespace ktsu.Keybinding.Core.Contracts;
 public interface IKeybindingManagerFactory
 {
 	/// <summary>
-	/// Creates a new KeybindingManager instance
+	/// Gets a KeybindingManager for the default data directory
 	/// </summary>
-	/// <returns>A new KeybindingManager instance</returns>
+	/// <returns>
+	/// The KeybindingManager registered with dependency injection when there is one, which is shared rather than new;
+	/// otherwise a new instance
+	/// </returns>
 	public KeybindingManager CreateManager();
 
 	/// <summary>
 	/// Creates a new KeybindingManager instance with specified data directory
 	/// </summary>
 	/// <param name="dataDirectory">Directory to store keybinding data</param>
-	/// <returns>A new KeybindingManager instance</returns>
+	/// <returns>A new KeybindingManager instance whose commands and profiles are not shared with any other manager</returns>
 	public KeybindingManager CreateManager(string dataDirectory);
 }

Keybinding/Services/KeybindingManagerFactory.cs

@@ -50,20 +50,10 @@ public KeybindingManager CreateManager()
 	}
 
 	/// <inheritdoc/>
-	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);
-	}
+	/// <remarks>
+	/// 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.
+	/// </remarks>
+	public KeybindingManager CreateManager(string dataDirectory) => new(dataDirectory);
 }