From bd9a3b65a205a956fe6bcaa072db4fab577f55bb Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Sun, 27 Sep 2026 10:25:15 +0000 Subject: [PATCH 1/2] Serialize Note as {"Key":"CTRL"} so it round-trips through System.Text.Json [patch] NoteName is a SemanticString, which System.Text.Json treats as a collection of chars, so Note serialized as {"Key":["C","T","R","L"]} and could not be read back. A JsonConverter on Note now writes the key as a string and reads it through the normalizing Note(string) constructor, so lowercase and alias input deserializes to the canonical note. Fixes #122 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk --- Keybinding.Test/NoteJsonSerializationTests.cs | 89 +++++++++++++++++++ Keybinding/Models/MusicalTypes.cs | 1 + Keybinding/Models/NoteJsonConverter.cs | 78 ++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 Keybinding.Test/NoteJsonSerializationTests.cs create mode 100644 Keybinding/Models/NoteJsonConverter.cs diff --git a/Keybinding.Test/NoteJsonSerializationTests.cs b/Keybinding.Test/NoteJsonSerializationTests.cs new file mode 100644 index 0000000..82d5efb --- /dev/null +++ b/Keybinding.Test/NoteJsonSerializationTests.cs @@ -0,0 +1,89 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Test; + +using System.Text.Json; +using ktsu.Keybinding.Core.Models; + +[TestClass] +public class NoteJsonSerializationTests +{ + private static readonly JsonSerializerOptions CamelCaseOptions = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + [TestMethod] + public void Serialize_WritesKeyAsString() + { + Assert.AreEqual("{\"Key\":\"CTRL\"}", JsonSerializer.Serialize(new Note("Ctrl"))); + } + + [TestMethod] + public void Serialize_HonorsPropertyNamingPolicy() + { + Assert.AreEqual("{\"key\":\"CTRL\"}", JsonSerializer.Serialize(new Note("Ctrl"), CamelCaseOptions)); + } + + [TestMethod] + public void RoundTrip_ReturnsEqualNote() + { + Note original = new("F5"); + Note? roundTripped = JsonSerializer.Deserialize(JsonSerializer.Serialize(original)); + Assert.AreEqual(original, roundTripped); + } + + [TestMethod] + public void Deserialize_LowercaseKey_ReturnsCanonicalNote() + { + Assert.AreEqual(new Note("Ctrl"), JsonSerializer.Deserialize("{\"Key\":\"ctrl\"}")); + } + + [TestMethod] + public void Deserialize_AliasKey_ReturnsCanonicalNote() + { + Note? note = JsonSerializer.Deserialize("{\"Key\":\"Control\"}"); + Assert.AreEqual("CTRL", note?.ToString()); + } + + [TestMethod] + public void Deserialize_CamelCasePropertyName_ReturnsNote() + { + Assert.AreEqual(new Note("Alt"), JsonSerializer.Deserialize("{\"key\":\"alt\"}")); + } + + [TestMethod] + public void Deserialize_IgnoresUnknownProperties() + { + Assert.AreEqual(new Note("S"), JsonSerializer.Deserialize("{\"Other\":[1,{\"a\":2}],\"Key\":\"s\"}")); + } + + [TestMethod] + public void Deserialize_Null_ReturnsNull() + { + Assert.IsNull(JsonSerializer.Deserialize("null")); + } + + [TestMethod] + public void Deserialize_InsideCollection_RoundTrips() + { + List notes = [new("Ctrl"), new("Shift"), new("S")]; + List? roundTripped = JsonSerializer.Deserialize>(JsonSerializer.Serialize(notes)); + CollectionAssert.AreEqual(notes, roundTripped); + } + + [TestMethod] + public void Deserialize_MissingKey_ThrowsJsonException() + { + Assert.ThrowsExactly(() => JsonSerializer.Deserialize("{}")); + } + + [TestMethod] + public void Deserialize_BlankKey_ThrowsJsonException() + { + Assert.ThrowsExactly(() => JsonSerializer.Deserialize("{\"Key\":\" \"}")); + } + + [TestMethod] + public void Deserialize_NonObject_ThrowsJsonException() + { + Assert.ThrowsExactly(() => JsonSerializer.Deserialize("[\"CTRL\"]")); + } +} diff --git a/Keybinding/Models/MusicalTypes.cs b/Keybinding/Models/MusicalTypes.cs index 83dbdce..a1abbb2 100644 --- a/Keybinding/Models/MusicalTypes.cs +++ b/Keybinding/Models/MusicalTypes.cs @@ -7,6 +7,7 @@ namespace ktsu.Keybinding.Core.Models; /// /// Represents a musical note - a single key /// +[JsonConverter(typeof(NoteJsonConverter))] public sealed class Note : IEquatable { /// diff --git a/Keybinding/Models/NoteJsonConverter.cs b/Keybinding/Models/NoteJsonConverter.cs new file mode 100644 index 0000000..7475161 --- /dev/null +++ b/Keybinding/Models/NoteJsonConverter.cs @@ -0,0 +1,78 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Core.Models; + +using System.Text.Json; +using System.Text.Json.Serialization; + +/// +/// Serializes a as {"Key":"CTRL"}. Without it System.Text.Json treats the +/// key as a collection of chars, writing an array it cannot read back. Reading goes +/// through , so lowercase and alias input comes back as the canonical note. +/// +internal sealed class NoteJsonConverter : JsonConverter +{ + private const string KeyPropertyName = nameof(Note.Key); + + /// + public override Note Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.StartObject) + { + throw new JsonException($"Expected an object for {nameof(Note)} but found {reader.TokenType}."); + } + + string? key = null; + bool foundKey = false; + while (reader.Read()) + { + if (reader.TokenType == JsonTokenType.EndObject) + { + if (!foundKey) + { + throw new JsonException($"{nameof(Note)} is missing its {KeyPropertyName} property."); + } + + try + { + return new Note(key!); + } + catch (ArgumentException ex) + { + throw new JsonException($"'{key}' is not a valid {nameof(Note)} key.", ex); + } + } + + string propertyName = reader.GetString()!; + reader.Read(); + if (string.Equals(propertyName, KeyPropertyName, StringComparison.OrdinalIgnoreCase)) + { + if (reader.TokenType != JsonTokenType.String) + { + throw new JsonException($"Expected a string for {nameof(Note)}.{KeyPropertyName} but found {reader.TokenType}."); + } + + key = reader.GetString(); + foundKey = true; + } + else + { + reader.Skip(); + } + } + + throw new JsonException($"Unexpected end of JSON while reading a {nameof(Note)}."); + } + + /// + public override void Write(Utf8JsonWriter writer, Note value, JsonSerializerOptions options) + { + Ensure.NotNull(writer); + Ensure.NotNull(value); + Ensure.NotNull(options); + + writer.WriteStartObject(); + writer.WriteString(options.PropertyNamingPolicy?.ConvertName(KeyPropertyName) ?? KeyPropertyName, value.Key.ToString()); + writer.WriteEndObject(); + } +} From fb19029156d0a97d895f762983731cd7563b52b4 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Sun, 27 Sep 2026 10:32:40 +0000 Subject: [PATCH 2/2] Split note construction out of NoteJsonConverter.Read and use Assert.AreSequenceEqual Addresses SonarCloud S3776 (cognitive complexity 16 > 15) and MSTEST0068. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk --- Keybinding.Test/NoteJsonSerializationTests.cs | 2 +- Keybinding/Models/NoteJsonConverter.cs | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Keybinding.Test/NoteJsonSerializationTests.cs b/Keybinding.Test/NoteJsonSerializationTests.cs index 82d5efb..ff7059f 100644 --- a/Keybinding.Test/NoteJsonSerializationTests.cs +++ b/Keybinding.Test/NoteJsonSerializationTests.cs @@ -66,7 +66,7 @@ public void Deserialize_InsideCollection_RoundTrips() { List notes = [new("Ctrl"), new("Shift"), new("S")]; List? roundTripped = JsonSerializer.Deserialize>(JsonSerializer.Serialize(notes)); - CollectionAssert.AreEqual(notes, roundTripped); + Assert.AreSequenceEqual(notes, roundTripped); } [TestMethod] diff --git a/Keybinding/Models/NoteJsonConverter.cs b/Keybinding/Models/NoteJsonConverter.cs index 7475161..6ef7771 100644 --- a/Keybinding/Models/NoteJsonConverter.cs +++ b/Keybinding/Models/NoteJsonConverter.cs @@ -28,19 +28,9 @@ public override Note Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSer { if (reader.TokenType == JsonTokenType.EndObject) { - if (!foundKey) - { - throw new JsonException($"{nameof(Note)} is missing its {KeyPropertyName} property."); - } - - try - { - return new Note(key!); - } - catch (ArgumentException ex) - { - throw new JsonException($"'{key}' is not a valid {nameof(Note)} key.", ex); - } + return foundKey + ? CreateNote(key!) + : throw new JsonException($"{nameof(Note)} is missing its {KeyPropertyName} property."); } string propertyName = reader.GetString()!; @@ -64,6 +54,18 @@ public override Note Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSer throw new JsonException($"Unexpected end of JSON while reading a {nameof(Note)}."); } + private static Note CreateNote(string key) + { + try + { + return new Note(key); + } + catch (ArgumentException ex) + { + throw new JsonException($"'{key}' is not a valid {nameof(Note)} key.", ex); + } + } + /// public override void Write(Utf8JsonWriter writer, Note value, JsonSerializerOptions options) {