release: Chatter.Rest.Hal 2.0.0

CHANGELOG.md

@@ -7,4 +7,76 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-[Unreleased]: https://github.com/brenpike/Chatter.Rest.Hal/compare/hal/v1.1.0...HEAD
+## [2.0.0] - 2026-08-24
+
+A hardening and correctness release: a full-repository review produced 22 verified fixes across
+converters, domain types, builders, and extensions, followed by a clause-by-clause conformance
+audit against draft-kelly-json-hal-11. All changes were coordinated under the design decisions
+recorded in [#86](https://github.com/brenpike/Chatter.Rest.Hal/issues/86): HAL-spec conformance is
+a hard rule, malformed input always fails as `JsonException`, and duplicate link relations are
+rejected.
+
+### Breaking changes
+
+- **Reserved names are literal and case-sensitive.** Converters no longer force
+  case-insensitive parsing: `_links`/`_embedded` match ordinally per the HAL spec, and the caller's
+  `JsonSerializerOptions.PropertyNameCaseInsensitive` is honored for ordinary properties (last
+  case-variant wins, matching duplicate-key normalization). Case-variant state properties such as
+  `{"Name":..,"name":..}` no longer collide. ([#93](https://github.com/brenpike/Chatter.Rest.Hal/issues/93))
+- **Malformed input throws `JsonException`.** Structurally invalid HAL (non-object resources,
+  non-string hrefs, invalid `_links`/`_embedded` shapes) fails at the deserialization call with
+  `JsonException` — never `InvalidOperationException`/`ArgumentException`, and never deferred to
+  property access. (Duplicate JSON keys are not an error: they normalize last-wins, see below.) ([#94](https://github.com/brenpike/Chatter.Rest.Hal/issues/94),
+  [#95](https://github.com/brenpike/Chatter.Rest.Hal/issues/95))
+- **Duplicate link relations are rejected.** `LinkCollection.Add` (and
+  `EmbeddedResourceCollection.Add` for duplicate names) throws `ArgumentException` on a duplicate
+  key; the fluent builders instead merge repeated `AddLink(rel)`/`AddSelf()`/`AddCuries()` into the
+  existing relation. Duplicate keys in *input JSON* are normalized last-wins.
+  ([#99](https://github.com/brenpike/Chatter.Rest.Hal/issues/99),
+  [#104](https://github.com/brenpike/Chatter.Rest.Hal/issues/104))
+- **`curies` serializes as an array by default**, per HAL §8.3, on every builder path — a single
+  definition emits `[{...}]`. `AsArray()` is now a no-op.
+  ([#119](https://github.com/brenpike/Chatter.Rest.Hal/issues/119))
+- **Single-element embedded collections keep their array shape** on round-trip, matching how links
+  preserve single-vs-array form. ([#97](https://github.com/brenpike/Chatter.Rest.Hal/issues/97))
+- **Equality is HAL-content equality.** `Resource`, `Link`, `LinkObject`, and the collections
+  compare by the HAL document they produce: reading properties never changes a hash code, the state
+  key derives from the actual writer (single source of truth), and shape flags count only where
+  they affect serialization. ([#100](https://github.com/brenpike/Chatter.Rest.Hal/issues/100))
+- **`State<T>()` returns detached projections.** Each call materializes a fresh snapshot from the
+  state of record; mutating the result no longer affects serialization or equality (a state
+  supplied to the constructor directly as `T` remains by-reference). `As<T>()` no longer caches for
+  in-memory resources, so post-call mutations are reflected.
+  ([#101](https://github.com/brenpike/Chatter.Rest.Hal/issues/101))
+- **`GetLinkObjectOrDefault` returns the first matching link object** instead of throwing when a
+  relation carries several. ([#105](https://github.com/brenpike/Chatter.Rest.Hal/issues/105))
+
+### Fixed
+
+- Builder crashes and misplacement: `AddLink`/`AddSelf`/`AddCuries` after `AddResources(...)` no
+  longer throw `NullReferenceException`; `AddEmbedded` after configuring a link attaches to the
+  correct resource; `FindParent` no longer self-matches.
+  ([#102](https://github.com/brenpike/Chatter.Rest.Hal/issues/102),
+  [#103](https://github.com/brenpike/Chatter.Rest.Hal/issues/103))
+- `ResourceConverter.Write` no longer drops state fields whose CLR names are `Links`/`Embedded`;
+  a state property literally named `_links`/`_embedded` is suppressed only when it would duplicate
+  the resource's own emitted collection. ([#96](https://github.com/brenpike/Chatter.Rest.Hal/issues/96))
+- CURIE expansion percent-encodes per RFC 6570, rejects empty suffixes, and searches all curie
+  definitions; eager null/whitespace validation in builders.
+  ([#105](https://github.com/brenpike/Chatter.Rest.Hal/issues/105))
+- Primitive or array resource state fails serialization with a descriptive `JsonException` instead
+  of producing invalid HAL; `AddHalConverters` registers correctly when a subset of converters was
+  already present. ([#98](https://github.com/brenpike/Chatter.Rest.Hal/issues/98))
+- Deeply nested `_embedded` chains parse in O(size) (no per-ancestor re-serialization), the
+  duplicate-key pre-scan and rebuild are iterative (no stack overflow at raised `MaxDepth`), and
+  options-registered custom converters take precedence over the built-in fast paths.
+
+### Notes
+
+- Full HAL spec-conformance audit: 28/30 normative requirements conform; the two deviations found
+  were fixed ([#119](https://github.com/brenpike/Chatter.Rest.Hal/issues/119)) or documented
+  ([#120](https://github.com/brenpike/Chatter.Rest.Hal/issues/120), open: empty-string href
+  tolerance).
+
+[Unreleased]: https://github.com/brenpike/Chatter.Rest.Hal/compare/hal/v2.0.0...HEAD
+[2.0.0]: https://github.com/brenpike/Chatter.Rest.Hal/compare/hal/v1.1.0...hal/v2.0.0

CLAUDE.md

@@ -80,7 +80,7 @@ Observed conventions:
 
 | Package | Version |
 |---|---|
-| `Chatter.Rest.Hal` | `1.1.0` |
+| `Chatter.Rest.Hal` | `2.0.0` |
 | `Chatter.Rest.Hal.CodeGenerators` | `0.4.0` |
 
 External dependency: `Chatter.Rest.UriTemplates` v0.1.0.

docs/HAL_TEST_PLAN.md

@@ -205,7 +205,8 @@ CURIEs are established via the `curies` reserved link relation — an array of n
 - ✅ `LinkCollectionExtensionsTests.ExpandCurieRelation_Should_Return_Full_Uri_When_Curie_Exists`
 
 ### 5.4 CURIE round-trip serialization
-- ✅ `HalCuriesAndTemplatedTests.Curie_Definition_Serializes_As_Array_Of_LinkObjects` — serializes and verifies CURIE structure in JSON output
+- ✅ `HalCuriesAndTemplatedTests.Curie_Definition_Serializes_As_Array_Of_LinkObjects` — serializes and verifies CURIE structure in JSON output (two definitions)
+- ✅ `BuilderTests.Builder_Constructs_Valid_CURIE_Structure` — a SINGLE curie definition also serializes as a one-element array (HAL §8.3 array form is the builder default since 2.0.0, #119)
 
 ### 5.5 Undefined CURIE prefix is handled gracefully
 - ✅ `HalCuriesAndTemplatedTests.Curie_Expansion_Returns_Original_When_No_Matching_Prefix` — verifies undefined CURIE prefix returns original relation

docs/api.md

@@ -134,6 +134,7 @@ public interface IResourceCuriesLinkCreationStage
     IResourceLinkObjectPropertiesSelectionStage AddLinkObject(string href, string name);
 
     // Force the "curies" relation to serialize as a JSON array.
+    // No-op since 2.0.0: curies is array-form by default per HAL §8.3.
     IResourceCuriesLinkCreationStage AsArray();
 }
 ```
@@ -285,8 +286,11 @@ public sealed record Resource : IHalPart
     // Lazily initialized; never null after first access.
     public EmbeddedResourceCollection Embedded { get; set; }
 
-    // Deserialize the resource's state properties into T.
-    // Result is cached after the first successful call.
+    // Deserialize the resource's state properties into T. Each call
+    // materializes a detached projection from the original state, so
+    // mutating the result affects neither serialization nor equality —
+    // except when the state was supplied to the constructor directly as
+    // a T instance, which is returned by reference.
     // Returns null if the state is absent or deserialization fails.
     public T? State<T>() where T : class;
 

docs/architecture.md

@@ -26,8 +26,8 @@ public sealed record Resource : IHalPart
 }
 ```
 
-- `State<T>()` — returns the resource state as `T`. If the underlying state is a `JsonElement`, it deserializes it on first call and caches the result. If state is a `JsonObject` (from the deserialization path), it calls the lazy `_stateCreator` delegate. Returns `null` on failure rather than throwing.
-- `As<T>()` — serializes the entire `Resource` to a `JsonNode` (if not already cached in `_resourceNode`) then deserializes that node to `T`. Use this to round-trip a HAL response into a typed DTO that includes `_links`/`_embedded` properties.
+- `State<T>()` — returns the resource state as `T`, materialized as a detached projection from the state of record (the constructor-supplied `JsonElement`, or the lazy `_stateCreator` delegate on the deserialization path) on every call. Projections never become the serialization source, so mutating a returned object affects neither serialization nor equality; a state supplied directly as `T` is the one exception and is returned by reference. Returns `null` on failure rather than throwing.
+- `As<T>()` — converts the full `Resource` (including `_links`/`_embedded`) to `T`. A parsed resource converts from the retained source node; an in-memory resource is re-serialized on every call, so links, embedded resources and state added after an earlier `As<T>()` call are reflected in the result. Use this to round-trip a HAL response into a typed DTO that declares `Links`/`Embedded` properties.
 - **Lazy init via `Func<T>` delegates** — the internal deserialization constructor (`internal Resource(JsonNode?, Func<JsonObject?>, Func<LinkCollection?>, Func<EmbeddedResourceCollection?>)`) stores the three factory delegates. `Links` and `Embedded` property getters invoke these delegates on first access and cache the result, making deserialization allocation-lazy.
 - `StateObject` — internal property that drives `ResourceConverter.Write`. Its getter calls `State<object>()`.
 

docs/serialization.md

@@ -267,8 +267,9 @@ Lazily deserializes the resource's state portion into a strongly typed object:
 public T? State<T>() where T : class
 ```
 
-- If the internal state is a `JsonElement`, deserializes it to `T` and caches the result.
-- If the internal state is null, invokes the `_stateCreator` delegate (which returns the JSON minus `_links`/`_embedded`) and deserializes that.
+- If the internal state is a `JsonElement`, deserializes it to `T` on every call — the returned object is a detached projection, so mutating it does not change what the resource serializes or how it compares.
+- If the internal state is null, invokes the `_stateCreator` delegate (which returns the JSON minus `_links`/`_embedded`) and deserializes that, likewise detached on every call.
+- A state supplied to the constructor directly as `T` is returned by reference (the one non-detached case).
 - Returns `null` on any exception.
 - **Special guard:** When `T == typeof(Link)`, requires the JSON object to have exactly one property before deserializing. This prevents a multi-property state DTO from being misidentified as a HAL link.
 
@@ -280,10 +281,9 @@ Casts the entire `Resource` (including `_links` and `_embedded`) to a strongly t
 public T? As<T>() where T : class
 ```
 
-- Serializes the `Resource` to a `JsonNode` via `JsonSerializer.SerializeToNode(this)` if not already cached in `_resourceNode`.
-- Deserializes that node to `T`.
-- Returns `null` on any exception.
-- The `_resourceNode` is cached after the first call — subsequent calls to `As<T>()` skip re-serialization.
+- A parsed resource converts from the `JsonNode` it was parsed from, preserving the original document shape.
+- An in-memory resource is serialized to a `JsonNode` via `JsonSerializer.SerializeToNode(this)` on every call — there is no node cache, so mutations made after an earlier `As<T>()` call are reflected in the result (at the cost of re-serialization per call).
+- Deserializes that node to `T`; returns `null` on any exception.
 - Because the full resource (including `_links` and `_embedded`) is included in the serialized node, use this method for DTOs decorated with `[HalResponse]` (from the source generator) that declare `Links` and `Embedded` properties.
 
 ### 5.6 `EmbeddedResourceCollectionConverter.Read`

src/Chatter.Rest.Hal/Builders/HalBuilder.cs

@@ -22,24 +22,33 @@ public abstract class HalBuilder<THalPart> : IBuildResource, IBuildHalPart<THalP
 	public IBuildHalPart<IHalPart>? Parent { get; }
 
 	/// <summary>
-	/// Finds the nearest parent builder that produces the specified HAL type.
+	/// Finds the nearest ancestor builder that produces the specified HAL type.
 	/// </summary>
 	/// <typeparam name="TParent">The type of HAL domain object to find.</typeparam>
-	/// <returns>The parent builder, or null if not found.</returns>
+	/// <returns>The ancestor builder, or null if not found.</returns>
+	/// <remarks>
+	/// The search starts at <see cref="Parent"/>: this builder is never its own parent, so a
+	/// builder that happens to satisfy <see cref="IBuildHalPart{THalPart}"/> for
+	/// <typeparamref name="TParent"/> can no longer resolve to itself. Builders that declare
+	/// <typeparamref name="TParent"/> unbuildable via <see cref="IDeclareUnbuildableHalParts"/>
+	/// are skipped so a lookup never resolves to a builder whose BuildPart() throws.
+	/// </remarks>
 	public IBuildHalPart<TParent>? FindParent<TParent>() where TParent : class, IHalPart
 	{
-		if (this is IBuildHalPart<TParent> rootBuilder)
-			return rootBuilder;
-
-		if (IsRoot())
-			return null;
-
-		if (Parent is IBuildHalPart<TParent> parentBuilder)
-			return parentBuilder;
+		for (var candidate = Parent; candidate is not null; candidate = candidate.Parent)
+		{
+			if (candidate is IBuildHalPart<TParent> match && !DeclaresUnbuildable<TParent>(candidate))
+			{
+				return match;
+			}
+		}
 
-		return Parent!.FindParent<TParent>();
+		return null;
 	}
 
+	private static bool DeclaresUnbuildable<TParent>(IBuildHalPart<IHalPart> candidate) where TParent : class, IHalPart
+		=> candidate is IDeclareUnbuildableHalParts declaring && declaring.CannotBuild(typeof(TParent));
+
 	/// <summary>
 	/// Finds the root builder in the builder hierarchy.
 	/// </summary>

src/Chatter.Rest.Hal/Builders/IDeclareUnbuildableHalParts.cs

@@ -0,0 +1,21 @@
+using System;
+
+namespace Chatter.Rest.Hal.Builders;
+
+/// <summary>
+/// Implemented by builders that satisfy <see cref="IBuildHalPart{THalPart}"/> for a HAL part
+/// they cannot actually build, usually because a staged interface forces the implementation.
+/// </summary>
+/// <remarks>
+/// <see cref="HalBuilder{THalPart}.FindParent{TParent}"/> skips such builders so an ancestor
+/// lookup never resolves to a builder whose BuildPart() throws.
+/// </remarks>
+internal interface IDeclareUnbuildableHalParts
+{
+	/// <summary>
+	/// Determines whether this builder is unable to build the specified HAL part type.
+	/// </summary>
+	/// <param name="halPartType">The HAL part type being looked up.</param>
+	/// <returns>true if this builder cannot build <paramref name="halPartType"/>; otherwise, false.</returns>
+	bool CannotBuild(Type halPartType);
+}

src/Chatter.Rest.Hal/Builders/LinkBuilder.cs

@@ -27,6 +27,9 @@ private LinkBuilder(IBuildHalPart<LinkCollection> parent, string rel) : base(par
 	{
 		_rel = rel;
 		_linkObjects = LinkObjectCollectionBuilder.New(this);
+		// The reserved "curies" relation is array-form by default per HAL section 8.3, whichever
+		// builder path creates it — AddCuries(), AddLink("curies"), or a merge into either.
+		_isArray = rel == CuriesLink;
 	}
 
 	/// <summary>
@@ -47,6 +50,11 @@ private LinkBuilder(IBuildHalPart<LinkCollection> parent, string rel) : base(par
 	/// <summary>
 	/// Creates a new link builder for a "curies" link.
 	/// </summary>
+	/// <remarks>
+	/// The link is array-form by default: HAL (draft-kelly-json-hal §8.3) establishes CURIEs via an
+	/// array of Link Objects, and common HAL clients index <c>_links.curies</c> as an array, so a
+	/// single definition must still serialize as <c>[{…}]</c>. <c>AsArray()</c> remains a no-op.
+	/// </remarks>
 	/// <param name="parent">The parent link collection builder.</param>
 	/// <returns>A new link builder.</returns>
 	public static LinkBuilder Curies(IBuildHalPart<LinkCollection> parent) => new(parent, CuriesLink);

src/Chatter.Rest.Hal/Builders/LinkCollectionBuilder.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using Chatter.Rest.Hal.Builders.Stages;
 
 namespace Chatter.Rest.Hal.Builders;
@@ -11,39 +12,58 @@ public sealed class LinkCollectionBuilder : HalBuilder<LinkCollection>, IAddLink
 	private LinkCollectionBuilder(IBuildHalPart<Resource> parent) : base(parent) { }
 	internal static LinkCollectionBuilder New(IBuildHalPart<Resource> parent) => new(parent);
 
-	private readonly IList<IBuildHalPart<Link>> _linkBuilders = new List<IBuildHalPart<Link>>();
+	private readonly IList<LinkBuilder> _linkBuilders = new List<LinkBuilder>();
+	private readonly IDictionary<string, LinkBuilder> _linkBuildersByRel = new Dictionary<string, LinkBuilder>(StringComparer.Ordinal);
 
 	/// <summary>
 	/// Adds a link with the specified relation to the collection.
 	/// </summary>
-	/// <param name="rel">The link relation.</param>
+	/// <param name="rel">The link relation. Must not be null or whitespace.</param>
 	/// <returns>A link creation stage.</returns>
+	/// <exception cref="ArgumentException">Thrown when <paramref name="rel"/> is null or whitespace.</exception>
+	/// <remarks>
+	/// Repeating a relation returns the builder already registered for it, so its link objects
+	/// merge into the single link for that relation. HAL's "_links" is a JSON object keyed by
+	/// relation, so two links sharing a relation could never serialize into spec-valid output.
+	/// </remarks>
 	public ILinkCreationStage AddLink(string rel)
 	{
-		var link = LinkBuilder.WithRel(this, rel);
-		_linkBuilders.Add(link);
-		return link;
+		// Validated here rather than in Build() so the exception points at the faulting call.
+		if (string.IsNullOrWhiteSpace(rel))
+		{
+			throw new ArgumentException("Value cannot be null or whitespace.", nameof(rel));
+		}
+
+		return GetOrAddLink(rel, r => LinkBuilder.WithRel(this, r));
 	}
 
 	/// <summary>
 	/// Adds a "self" link to the collection.
 	/// </summary>
 	/// <returns>A link creation stage.</returns>
-	public ILinkCreationStage AddSelf()
-	{
-		var link = LinkBuilder.Self(this);
-		_linkBuilders.Add(link);
-		return link;
-	}
+	/// <remarks>Repeated calls merge into the single "self" link, as described on <see cref="AddLink"/>.</remarks>
+	public ILinkCreationStage AddSelf() => GetOrAddLink(LinkBuilder.SelfLink, _ => LinkBuilder.Self(this));
 
 	/// <summary>
 	/// Adds a "curies" link to the collection for defining compact URI relations.
 	/// </summary>
 	/// <returns>A curies link creation stage.</returns>
-	public ICuriesLinkCreationStage AddCuries()
+	/// <remarks>
+	/// Repeated calls merge into the single "curies" link, so each additional CURIE definition
+	/// extends that link's array rather than emitting a second "curies" entry.
+	/// </remarks>
+	public ICuriesLinkCreationStage AddCuries() => GetOrAddLink(LinkBuilder.CuriesLink, _ => LinkBuilder.Curies(this));
+
+	private LinkBuilder GetOrAddLink(string rel, Func<string, LinkBuilder> create)
 	{
-		var link = LinkBuilder.Curies(this);
+		if (_linkBuildersByRel.TryGetValue(rel, out var existing))
+		{
+			return existing;
+		}
+
+		var link = create(rel);
 		_linkBuilders.Add(link);
+		_linkBuildersByRel.Add(rel, link);
 		return link;
 	}