From 20830454edc541f1715675a92d6c6e2d60e7893e Mon Sep 17 00:00:00 2001 From: matt-edmondson Date: Thu, 8 Oct 2026 13:26:34 +0000 Subject: [PATCH] Round-trip Chord, Phrase, Command and Profile through System.Text.Json [patch] Chord and Phrase had two public constructors and no [JsonConstructor], so deserializing their own output threw NotSupportedException. Each now has a private [JsonConstructor] taking its property's own type, which is what the serializer binds against. Command's semantic strings serialized as char arrays. A SemanticStringJsonConverter writes them as plain strings and reads them back through Create, and the semantic-string constructor is marked [JsonConstructor]. Profile.Chords is get-only, so a deserialized profile silently came back with no bindings. A private [JsonConstructor] takes the chords and binds them through SetChord. Populate handling was not an option: System.Text.Json does not support it on a type with a parameterized constructor. Fixes #141 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01T4yQx7YuowTGorXJRkhe8x --- .../ModelJsonSerializationTests.cs | 80 +++++++++++++++++++ Keybinding/Models/Command.cs | 7 ++ Keybinding/Models/MusicalTypes.cs | 16 ++++ Keybinding/Models/Profile.cs | 16 ++++ .../Models/SemanticStringJsonConverter.cs | 45 +++++++++++ 5 files changed, 164 insertions(+) create mode 100644 Keybinding.Test/ModelJsonSerializationTests.cs create mode 100644 Keybinding/Models/SemanticStringJsonConverter.cs diff --git a/Keybinding.Test/ModelJsonSerializationTests.cs b/Keybinding.Test/ModelJsonSerializationTests.cs new file mode 100644 index 0000000..a158c3d --- /dev/null +++ b/Keybinding.Test/ModelJsonSerializationTests.cs @@ -0,0 +1,80 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Test; + +using System.Text.Json; +using ktsu.Keybinding.Core.Models; + +[TestClass] +public class ModelJsonSerializationTests +{ + private static T? RoundTrip(T value) => JsonSerializer.Deserialize(JsonSerializer.Serialize(value)); + + [TestMethod] + public void Chord_RoundTrip_ReturnsEqualChord() + { + Chord original = Chord.Parse("Ctrl+Shift+S"); + Assert.AreEqual(original, RoundTrip(original)); + } + + [TestMethod] + public void Phrase_RoundTrip_ReturnsEqualPhrase() + { + Phrase original = Phrase.Parse("Ctrl+K, Ctrl+C"); + Assert.AreEqual(original, RoundTrip(original)); + } + + [TestMethod] + public void Command_SerializesSemanticStringsAsStrings() + { + Command command = new("file.save", "Save", "Saves the file", "File"); + using JsonDocument document = JsonDocument.Parse(JsonSerializer.Serialize(command)); + JsonElement root = document.RootElement; + + Assert.AreEqual("file.save", root.GetProperty(nameof(Command.Id)).GetString()); + Assert.AreEqual("Save", root.GetProperty(nameof(Command.Name)).GetString()); + Assert.AreEqual("Saves the file", root.GetProperty(nameof(Command.Description)).GetString()); + Assert.AreEqual("File", root.GetProperty(nameof(Command.Category)).GetString()); + } + + [TestMethod] + public void Command_RoundTrip_PreservesEveryField() + { + Command original = new("file.save", "Save", "Saves the file", "File"); + Command? roundTripped = RoundTrip(original); + + Assert.IsNotNull(roundTripped); + Assert.AreEqual(original, roundTripped); + Assert.AreEqual(original.Name, roundTripped.Name); + Assert.AreEqual(original.Description, roundTripped.Description); + Assert.AreEqual(original.Category, roundTripped.Category); + } + + [TestMethod] + public void Command_RoundTrip_KeepsMissingDescriptionAndCategoryNull() + { + Command? roundTripped = RoundTrip(new Command("file.save", "Save")); + + Assert.IsNotNull(roundTripped); + Assert.IsNull(roundTripped.Description); + Assert.IsNull(roundTripped.Category); + } + + [TestMethod] + public void Profile_RoundTrip_KeepsBoundChords() + { + Profile original = new("default", "Default", "The default profile"); + original.SetChord("file.save", Chord.Parse("Ctrl+S")); + original.SetChord("edit.copy", Chord.Parse("Ctrl+C")); + + Profile? roundTripped = RoundTrip(original); + + Assert.IsNotNull(roundTripped); + Assert.AreEqual(original, roundTripped); + Assert.AreEqual(original.Name, roundTripped.Name); + Assert.AreEqual(original.Description, roundTripped.Description); + Assert.AreEqual(2, roundTripped.ChordCount); + Assert.AreEqual(Chord.Parse("Ctrl+S"), roundTripped.GetChord("file.save")); + Assert.AreEqual(Chord.Parse("Ctrl+C"), roundTripped.GetChord("edit.copy")); + } +} diff --git a/Keybinding/Models/Command.cs b/Keybinding/Models/Command.cs index 8a9d851..6d3dc04 100644 --- a/Keybinding/Models/Command.cs +++ b/Keybinding/Models/Command.cs @@ -2,6 +2,8 @@ namespace ktsu.Keybinding.Core.Models; +using System.Text.Json.Serialization; + /// /// Represents a command that can be executed via keybindings /// @@ -15,6 +17,7 @@ public sealed class Command : IEquatable /// Optional description of what the command does /// Optional category for grouping commands /// Thrown when id or name is null or whitespace + [JsonConstructor] public Command(CommandId id, CommandName name, CommandDescription? description = null, CommandCategory? category = null) { Ensure.NotNull(id); @@ -64,21 +67,25 @@ public Command(string id, string name, string? description = null, string? categ /// /// Gets the unique identifier for the command /// + [JsonConverter(typeof(SemanticStringJsonConverter))] public CommandId Id { get; } /// /// Gets the display name of the command /// + [JsonConverter(typeof(SemanticStringJsonConverter))] public CommandName Name { get; } /// /// Gets the description of what the command does /// + [JsonConverter(typeof(SemanticStringJsonConverter))] public CommandDescription? Description { get; } /// /// Gets the category for grouping commands /// + [JsonConverter(typeof(SemanticStringJsonConverter))] public CommandCategory? Category { get; } /// diff --git a/Keybinding/Models/MusicalTypes.cs b/Keybinding/Models/MusicalTypes.cs index 0ceac48..27cfe72 100644 --- a/Keybinding/Models/MusicalTypes.cs +++ b/Keybinding/Models/MusicalTypes.cs @@ -169,6 +169,14 @@ public Chord(Note note) _notes = [note]; } + // The serializer binds constructor parameters by name and exact type, so it needs a constructor that + // takes the Notes property's own type. + [JsonConstructor] + [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Called by System.Text.Json through [JsonConstructor].")] + private Chord(IReadOnlyList notes) : this((IEnumerable)notes) + { + } + /// /// Gets all notes in this chord /// @@ -367,6 +375,14 @@ public Phrase(Chord chord) : this([chord]) { } + // The serializer binds constructor parameters by name and exact type, so it needs a constructor that + // takes the Sequence property's own type. + [JsonConstructor] + [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Called by System.Text.Json through [JsonConstructor].")] + private Phrase(IReadOnlyList sequence) : this((IEnumerable)sequence) + { + } + /// /// Gets the sequence of chords in this phrase /// diff --git a/Keybinding/Models/Profile.cs b/Keybinding/Models/Profile.cs index 2e33422..3093aa8 100644 --- a/Keybinding/Models/Profile.cs +++ b/Keybinding/Models/Profile.cs @@ -3,6 +3,7 @@ namespace ktsu.Keybinding.Core.Models; using System.Collections.Generic; +using System.Text.Json.Serialization; /// /// Represents a keybinding profile with command to chord mappings @@ -33,6 +34,21 @@ public Profile(string id, string name, string? description = null) Description = description?.Trim(); } + // The Chords property is get-only, so the serializer can only restore the bindings through a + // constructor parameter of the same name and type. Without it a round-tripped profile comes back empty. + [JsonConstructor] + [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Called by System.Text.Json through [JsonConstructor].")] + private Profile(string id, string name, string? description, Dictionary? chords) : this(id, name, description) + { + if (chords is not null) + { + foreach (KeyValuePair binding in chords) + { + SetChord(binding.Key, binding.Value); + } + } + } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0032:Use auto property", Justification = "The only property over this field is obsolete, and the field is what the lock guards.")] private readonly Dictionary _chords = []; diff --git a/Keybinding/Models/SemanticStringJsonConverter.cs b/Keybinding/Models/SemanticStringJsonConverter.cs new file mode 100644 index 0000000..e1f4e0f --- /dev/null +++ b/Keybinding/Models/SemanticStringJsonConverter.cs @@ -0,0 +1,45 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Core.Models; + +using System.Text.Json; +using System.Text.Json.Serialization; +using ktsu.Semantics.Strings; + +/// +/// Serializes a semantic string as a plain JSON string. Without it System.Text.Json treats the +/// semantic string as a collection of chars, writing an array it cannot read back. Reading goes +/// through , so the type's validation still applies. +/// +/// The semantic string type +internal sealed class SemanticStringJsonConverter : JsonConverter + where T : SemanticString +{ + /// + public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.String) + { + throw new JsonException($"Expected a string for {typeof(T).Name} but found {reader.TokenType}."); + } + + string value = reader.GetString()!; + try + { + return SemanticString.Create(value); + } + catch (ArgumentException ex) + { + throw new JsonException($"'{value}' is not a valid {typeof(T).Name}.", ex); + } + } + + /// + public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options) + { + Ensure.NotNull(writer); + Ensure.NotNull(value); + + writer.WriteStringValue(value.ToString()); + } +}