Skip to content

Serialize Note as {"Key":"CTRL"} so it round-trips through System.Text.Json - #137

Merged
matt-edmondson merged 2 commits into
mainfrom
fix/note-json-converter-122
Sep 27, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
fix/note-json-converter-122

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #122

What was wrong

NoteName is a SemanticString, so System.Text.Json treated it as a char collection: new Note("Ctrl") serialized to {"Key":["C","T","R","L"]}, and neither that nor {"Key":"CTRL"} could be deserialized. Note advertised direct serialization through [JsonConstructor] anyway.

Change

  • New internal NoteJsonConverter, attached with [JsonConverter] on Note.
    • Writes {"Key":"CTRL"}, and applies the options' PropertyNamingPolicy to the property name.
    • Reads the Key property (matched case-insensitively, other properties skipped) through the normalizing Note(string) constructor, so ctrl and Control both come back as CTRL.
    • A missing, non-string or blank key, or a non-object token, throws JsonException.
  • Because Chord holds Notes, a Chord's notes now serialize as strings too. Chord and Phrase still cannot be deserialized directly, since neither has a [JsonConstructor]. The issue doesn't ask for that, so it isn't in this PR.

Tests

NoteJsonSerializationTests has 12 tests: serialization, naming policy, round-trip, lowercase and alias input, camelCase property name, unknown properties, null, notes inside a list, and the error cases. With the attribute removed, 9 of them fail. Full suite: 114/114 pass on net10.0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk


Generated by Claude Code

matt-edmondson and others added 2 commits September 27, 2026 10:25
…t.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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
…AreSequenceEqual

Addresses SonarCloud S3776 (cognitive complexity 16 > 15) and MSTEST0068.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 7cf7346 into main Sep 27, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the fix/note-json-converter-122 branch September 27, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Note is marked [JsonConstructor] but System.Text.Json serializes its key as a char array and can't deserialize it

1 participant