Skip invalid stored profiles and chords instead of failing the load

Keybinding.Test/InvalidStoredProfileEntryTests.cs

@@ -0,0 +1,108 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Test;
+
+using ktsu.Keybinding.Core;
+using ktsu.Keybinding.Core.Models;
+
+[TestClass]
+public class InvalidStoredProfileEntryTests
+{
+	private string _testDataDirectory = null!;
+
+	[TestInitialize]
+	public void Setup()
+	{
+		_testDataDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+		Directory.CreateDirectory(_testDataDirectory);
+	}
+
+	[TestCleanup]
+	public void Cleanup()
+	{
+		if (Directory.Exists(_testDataDirectory))
+		{
+			Directory.Delete(_testDataDirectory, recursive: true);
+		}
+	}
+
+	private async Task WriteProfilesAsync(string json) =>
+		await File.WriteAllTextAsync(Path.Combine(_testDataDirectory, "profiles.json"), json).ConfigureAwait(false);
+
+	[TestMethod]
+	public async Task InitializeAsync_CorruptProfilesFile_LoadsNoProfiles()
+	{
+		await WriteProfilesAsync("{not json").ConfigureAwait(false);
+
+		using KeybindingManager manager = new(_testDataDirectory);
+		await manager.InitializeAsync().ConfigureAwait(false);
+
+		Assert.IsEmpty(manager.Profiles.GetAllProfiles());
+	}
+
+	[TestMethod]
+	public async Task InitializeAsync_ChordWithNoNotes_SkipsOnlyThatBinding()
+	{
+		await WriteProfilesAsync("""
+			[{"id":"default","name":"Default","chords":{"file.save":{"notes":[]},"file.open":{"notes":["CTRL","O"]}}}]
+			""").ConfigureAwait(false);
+
+		using KeybindingManager manager = new(_testDataDirectory);
+		await manager.InitializeAsync().ConfigureAwait(false);
+
+		Profile? profile = manager.Profiles.GetProfile("default");
+		Assert.IsNotNull(profile);
+		Assert.IsNull(profile.GetChord("file.save"));
+		Assert.IsNotNull(profile.GetChord("file.open"));
+	}
+
+	[TestMethod]
+	public async Task InitializeAsync_BlankNote_SkipsOnlyThatBinding()
+	{
+		await WriteProfilesAsync("""
+			[{"id":"default","name":"Default","chords":{"file.save":{"notes":["CTRL"," "]},"file.open":{"notes":["CTRL","O"]}}}]
+			""").ConfigureAwait(false);
+
+		using KeybindingManager manager = new(_testDataDirectory);
+		await manager.InitializeAsync().ConfigureAwait(false);
+
+		Profile? profile = manager.Profiles.GetProfile("default");
+		Assert.IsNotNull(profile);
+		Assert.IsNull(profile.GetChord("file.save"));
+		Assert.IsNotNull(profile.GetChord("file.open"));
+	}
+
+	[TestMethod]
+	public async Task InitializeAsync_ProfileWithBlankIdOrName_SkipsOnlyThatProfile()
+	{
+		await WriteProfilesAsync("""
+			[{"id":"","name":"No Id"},{"id":"noname","name":" "},{"id":"default","name":"Default"}]
+			""").ConfigureAwait(false);
+
+		using KeybindingManager manager = new(_testDataDirectory);
+		await manager.InitializeAsync().ConfigureAwait(false);
+
+		Assert.AreEqual("default", string.Join(',', manager.Profiles.GetAllProfiles().Select(p => p.Id)));
+	}
+
+	[TestMethod]
+	public async Task SaveAsync_AfterSkippingInvalidEntries_KeepsTheValidOnes()
+	{
+		await WriteProfilesAsync("""
+			[{"id":"default","name":"Default","chords":{"file.save":{"notes":[]},"file.open":{"notes":["CTRL","O"]}}}]
+			""").ConfigureAwait(false);
+
+		{
+			using KeybindingManager manager = new(_testDataDirectory);
+			await manager.InitializeAsync().ConfigureAwait(false);
+			manager.Profiles.CreateProfile("vim", "Vim");
+			await manager.SaveAsync().ConfigureAwait(false);
+		}
+
+		using KeybindingManager reloaded = new(_testDataDirectory);
+		await reloaded.InitializeAsync().ConfigureAwait(false);
+
+		Assert.AreEqual("default,vim", string.Join(',', reloaded.Profiles.GetAllProfiles().Select(p => p.Id).Order(StringComparer.Ordinal)));
+		Assert.IsNotNull(reloaded.Profiles.GetProfile("default")?.GetChord("file.open"));
+	}
+}

Keybinding/Services/JsonKeybindingRepository.cs

@@ -101,17 +101,17 @@ private async Task<List<Profile>> LoadAllProfilesInternalAsync()
 			string json = await File.ReadAllTextAsync(profilesPath).ConfigureAwait(false);
 			List<ProfileDto> profileDtos = JsonSerializer.Deserialize<List<ProfileDto>>(json, _jsonOptions) ?? [];
 
-			return [.. profileDtos.Select(dto =>
+			List<Profile> profiles = [];
+			foreach (ProfileDto? dto in profileDtos)
 			{
-				Profile profile = new(dto.Id, dto.Name, dto.Description);
-				foreach (KeyValuePair<string, ChordDto> kvp in dto.Chords ?? [])
+				Profile? profile = ToProfile(dto);
+				if (profile is not null)
 				{
-					Chord chord = new(kvp.Value.Notes.Select(noteString => new Note(noteString)));
-					profile.SetChord(kvp.Key, chord);
+					profiles.Add(profile);
 				}
+			}
 
-				return profile;
-			})];
+			return profiles;
 		}
 		catch (JsonException)
 		{
@@ -120,6 +120,45 @@ private async Task<List<Profile>> LoadAllProfilesInternalAsync()
 		}
 	}
 
+	/// <summary>
+	/// Converts a stored profile, skipping it when its id or name is invalid and skipping any chord
+	/// whose notes are invalid, so one bad entry does not stop the rest of the file from loading.
+	/// A skipped entry is not written back by a later save.
+	/// </summary>
+	private static Profile? ToProfile(ProfileDto? dto)
+	{
+		if (dto is null)
+		{
+			return null;
+		}
+
+		Profile profile;
+		try
+		{
+			profile = new(dto.Id, dto.Name, dto.Description);
+		}
+		catch (ArgumentException ex)
+		{
+			System.Diagnostics.Debug.WriteLine($"Skipping stored profile '{dto.Id}': {ex.Message}");
+			return null;
+		}
+
+		foreach (KeyValuePair<string, ChordDto> kvp in dto.Chords ?? [])
+		{
+			try
+			{
+				Chord chord = new((kvp.Value?.Notes ?? []).Select(noteString => new Note(noteString)));
+				profile.SetChord(kvp.Key, chord);
+			}
+			catch (ArgumentException ex)
+			{
+				System.Diagnostics.Debug.WriteLine($"Skipping chord for '{kvp.Key}' in profile '{profile.Id}': {ex.Message}");
+			}
+		}
+
+		return profile;
+	}
+
 	/// <inheritdoc/>
 	public async Task DeleteProfileAsync(string profileId)
 	{