From 56f70133145fff4b3c2270335d80e27b9d909cb3 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 23 Jun 2026 06:57:05 -0400 Subject: [PATCH] fix: put [JsonPropertyName("@type")] on each concrete JsonLdNode override System.Text.Json emitted both "@type" and a bare "Type" because the attribute was only on the abstract base member. Adds regression assertions for all node types. --- DeepDrftPublic.Client/Common/SeoJsonLd.cs | 8 +- DeepDrftTests/SeoModelTests.cs | 115 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/DeepDrftPublic.Client/Common/SeoJsonLd.cs b/DeepDrftPublic.Client/Common/SeoJsonLd.cs index 9c256fa..d9c5d61 100644 --- a/DeepDrftPublic.Client/Common/SeoJsonLd.cs +++ b/DeepDrftPublic.Client/Common/SeoJsonLd.cs @@ -63,7 +63,7 @@ public abstract record JsonLdNode /// The Deep DRFT collective entity — the home/about node. public sealed record MusicGroupNode : JsonLdNode { - public override string Type => "MusicGroup"; + [JsonPropertyName("@type")] public override string Type => "MusicGroup"; [JsonPropertyName("name")] public string Name { get; init; } = string.Empty; [JsonPropertyName("url")] public string? Url { get; init; } @@ -76,7 +76,7 @@ public sealed record MusicGroupNode : JsonLdNode /// A studio cut or a live session release. AlbumProductionType distinguishes them. public sealed record MusicAlbumNode : JsonLdNode { - public override string Type => "MusicAlbum"; + [JsonPropertyName("@type")] public override string Type => "MusicAlbum"; [JsonPropertyName("name")] public string Name { get; init; } = string.Empty; [JsonPropertyName("byArtist")] public ArtistRef? ByArtist { get; init; } @@ -96,7 +96,7 @@ public sealed record MusicAlbumNode : JsonLdNode /// A single recording — a mix release, or one track inside an album's track list. public sealed record MusicRecordingNode : JsonLdNode { - public override string Type => "MusicRecording"; + [JsonPropertyName("@type")] public override string Type => "MusicRecording"; [JsonPropertyName("name")] public string Name { get; init; } = string.Empty; [JsonPropertyName("byArtist")] public ArtistRef? ByArtist { get; init; } @@ -112,7 +112,7 @@ public sealed record MusicRecordingNode : JsonLdNode /// A browse/index surface listing releases (cuts/sessions/mixes/archive). public sealed record CollectionPageNode : JsonLdNode { - public override string Type => "CollectionPage"; + [JsonPropertyName("@type")] public override string Type => "CollectionPage"; [JsonPropertyName("name")] public string Name { get; init; } = string.Empty; [JsonPropertyName("description")] public string? Description { get; init; } diff --git a/DeepDrftTests/SeoModelTests.cs b/DeepDrftTests/SeoModelTests.cs index b6dc92a..41fcace 100644 --- a/DeepDrftTests/SeoModelTests.cs +++ b/DeepDrftTests/SeoModelTests.cs @@ -268,6 +268,121 @@ public class SeoModelTests Is.EqualTo("Aphex Twin")); } + // --- AC5 regression: no stray CLR `Type` property emitted alongside `@type` --- + // System.Text.Json previously emitted both `@type` (from the base [JsonPropertyName]) and `Type` + // (the raw CLR override name) on concrete derived nodes, failing the schema.org validator. + // The fix: repeat [JsonPropertyName("@type")] directly on each concrete override. + + [Test] + public void MusicAlbumNode_SerializesAtType_OnlyOnce_NoBareTypeProperty() + { + var tracks = new List { new() { TrackName = "T", TrackNumber = 1, DurationSeconds = 30 } }; + var node = Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Cut), tracks).JsonLd); + + Assert.Multiple(() => + { + Assert.That(node.GetProperty("@type").GetString(), Is.EqualTo("MusicAlbum"), + "MusicAlbumNode must emit @type"); + Assert.That(node.TryGetProperty("Type", out _), Is.False, + "MusicAlbumNode must NOT emit a bare Type property"); + }); + } + + [Test] + public void MusicRecordingNode_TopLevel_SerializesAtType_NoBareTypeProperty() + { + var tracks = new List { new() { TrackName = "The Mix", DurationSeconds = 3600 } }; + var node = Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Mix), tracks).JsonLd); + + Assert.Multiple(() => + { + Assert.That(node.GetProperty("@type").GetString(), Is.EqualTo("MusicRecording"), + "MusicRecordingNode must emit @type"); + Assert.That(node.TryGetProperty("Type", out _), Is.False, + "MusicRecordingNode must NOT emit a bare Type property"); + }); + } + + [Test] + public void MusicRecordingNode_NestedTrack_SerializesAtType_NoBareTypeProperty() + { + // The nested track[] MusicRecordingNode is a different code path from the top-level mix node. + var tracks = new List { new() { TrackName = "T", TrackNumber = 1, DurationSeconds = 30 } }; + var albumNode = Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Cut), tracks).JsonLd); + var trackNode = albumNode.GetProperty("track")[0]; + + Assert.Multiple(() => + { + Assert.That(trackNode.GetProperty("@type").GetString(), Is.EqualTo("MusicRecording"), + "nested MusicRecordingNode must emit @type"); + Assert.That(trackNode.TryGetProperty("Type", out _), Is.False, + "nested MusicRecordingNode must NOT emit a bare Type property"); + }); + } + + [Test] + public void MusicGroupNode_SerializesAtType_NoBareTypeProperty() + { + var node = Parse(SeoModel.ForHome(Options).JsonLd); + + Assert.Multiple(() => + { + Assert.That(node.GetProperty("@type").GetString(), Is.EqualTo("MusicGroup"), + "MusicGroupNode must emit @type"); + Assert.That(node.TryGetProperty("Type", out _), Is.False, + "MusicGroupNode must NOT emit a bare Type property"); + }); + } + + [Test] + public void CollectionPageNode_SerializesAtType_NoBareTypeProperty() + { + var node = Parse(SeoModel.ForBrowse(Options, ReleaseMedium.Cut, "/cuts").JsonLd); + + Assert.Multiple(() => + { + Assert.That(node.GetProperty("@type").GetString(), Is.EqualTo("CollectionPage"), + "CollectionPageNode must emit @type"); + Assert.That(node.TryGetProperty("Type", out _), Is.False, + "CollectionPageNode must NOT emit a bare Type property"); + }); + } + + [Test] + public void ArtistRef_NestedByArtist_SerializesAtType_NoBareTypeProperty() + { + // ArtistRef (byArtist) was already clean — this asserts it stays clean after the fix. + var node = Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Cut)).JsonLd); + var byArtist = node.GetProperty("byArtist"); + + Assert.Multiple(() => + { + Assert.That(byArtist.GetProperty("@type").GetString(), Is.EqualTo("MusicGroup"), + "ArtistRef must emit @type"); + Assert.That(byArtist.TryGetProperty("Type", out _), Is.False, + "ArtistRef must NOT emit a bare Type property"); + }); + } + + [Test] + public void AllNodes_ContextIsPresent_AndSchemaOrg() + { + // Belt-and-suspenders: @context must not regress alongside the @type fix. + var nodes = new[] + { + Parse(SeoModel.ForHome(Options).JsonLd), + Parse(SeoModel.ForAbout(Options).JsonLd), + Parse(SeoModel.ForBrowse(Options, ReleaseMedium.Cut, "/cuts").JsonLd), + Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Cut)).JsonLd), + Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Session)).JsonLd), + Parse(SeoModel.ForRelease(Options, Release(ReleaseMedium.Mix)).JsonLd), + }; + + foreach (var node in nodes) + Assert.That(node.GetProperty("@context").GetString(), Is.EqualTo("https://schema.org"), + "@context must remain present after the @type fix"); + } + // --- Critical: inline JSON-LD script-breakout escaping (XSS) ----------------- [Test]