Skip to content

[1/4] Serializer canonicalization - #19

Open
alextomas955 wants to merge 10 commits into
extension-sdk-basefrom
v1.1-1-serializer
Open

[1/4] Serializer canonicalization#19
alextomas955 wants to merge 10 commits into
extension-sdk-basefrom
v1.1-1-serializer

Conversation

@alextomas955

Copy link
Copy Markdown
Owner

Stack 1 of 4 (base: extension-sdk-base). Foundation for the contract-first pipeline — must land first.

Unifies System.Text.Json onto one canonical, source-generated configuration across controllers, SignalR, minimal APIs, and the extension endpoint path. Enums serialize as camelCase strings everywhere (previously integers leaked on the minimal-API/extension paths); reads remain tolerant of both integer and string. Special-type wire policy (DateTime/Guid/byte[]) documented and round-trip tested.

19 files, +766/−108. Wire format for controllers/SignalR is unchanged; the extension endpoint path changes integer→string (the intended fix). Backend tests green.

Review this first; PRs 2–4 stack on top.

- Add CoveJsonContext source-gen JsonSerializerContext (camelCase, string enums)
- Upgrade CoveJson.Default to a frozen, combined source-gen + reflection resolver
  with a global camelCase string-enum converter for the reflection-fallback path
- Relocate CriterionModifierJsonConverter into Cove.Core as a public type-specific
  converter, registered ahead of the global enum converter so it wins for CriterionModifier
- Document the polymorphic-DTO wire convention for future use
- Pin externally-visible / SignalR-contract enum wire names with
  [JsonStringEnumMemberName]: GenderEnum, CircumcisedEnum, EventType,
  JobStatus, JobUnitOutcome; note FilterMode's integer DB persistence is
  independent of its string wire form
- Add EnumWireFormatTests: all 27 core enums emit camelCase strings, read
  from int-or-string, an unregistered enum proves the reflection-fallback
  path emits strings, and CriterionModifier parity/lenient-read
- Add SpecialTypeRoundTripTests: DateTime ISO-8601, Guid 36-char D, byte[]
  base64; document empty decimal WriteAsString opt-in list
- Register the CriterionModifier converter ahead of the global enum factory
  so the type-specific converter wins (first-match-wins converter ordering)
…enum tests

- Assert MVC, SignalR, and Http.Json options serialize an enum to the same camelCase string
- Assert the Http.Json path serializes an unregistered DTO enum as a camelCase string (reflection fallback)
- Guard existing controller enum wire output stays a camelCase string
…anonical JSON

- Add ConfigureHttpJsonOptions so host minimal-API and extension MapEndpoints
  responses serialize enums as camelCase strings instead of integers
- Introduce a single ApplyCanonicalJson helper that copies the resolver, naming,
  and every converter from CoveJson.Default onto MVC, SignalR, and Http.Json options
- Point the integration read harness at CoveJson.Default so tests assert the real wire contract
- Target the recognized IntegrationTest environment in the new parity tests
…ical

- ConfigService and ScraperService derive their options from CoveJson.Default,
  keeping WriteIndented / WhenWritingNull flags
- DynamicGroups reads filter and query JSON through CoveJson.Default and drops its
  private CriterionModifier converter, now supplied by the shared canonical converter
…canonical

- FieldProvenanceService, ScrapeAttemptService, and UserService (UI prefs) use
  CoveJson.Default for their DB-blob (de)serialization
- Add UserUiPreferences round-trip, no-drift, and pinned camelCase fixture tests
  proving the persisted blob shape is byte-compatible after the switch
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR unifies all serialization boundaries (MVC controllers, SignalR, minimal-API / extension endpoints, and manual call sites) onto a single frozen CoveJson.Default instance backed by a source-generated context plus a reflection fallback, fixing an integer-enum leak on the extension endpoint path and ensuring consistent camelCase-string wire output everywhere.

  • CoveJson.cs / CoveJsonContext.cs introduce the canonical options combining source-gen metadata for four registered DTO roots with a DefaultJsonTypeInfoResolver for extension/ALC types, and a two-converter list that preserves CriterionModifier's lenient read while applying the global camelCase enum policy to everything else.
  • ApplyCanonicalJson in Program.cs copies the canonical policy in-place onto MVC, SignalR, and HTTP.Json options; [JsonStringEnumMemberName] pins are added to EventType, JobStatus, JobUnitOutcome, GenderEnum, and CircumcisedEnum to lock wire names against future C# renames.
  • A comprehensive test suite covers every core enum member, pinned literals, the reflection-fallback path for unregistered types, source-gen/reflection parity, special-type round-trips, and a byte-compatible drift check against the replaced bespoke options.

Confidence Score: 4/5

Safe to merge. Wire output for controllers and SignalR is unchanged; the only intentional wire change (integer to string) is on the extension endpoint path and is the stated goal.

The core refactor is structurally sound and well-tested. The one design concern is that ApplyCanonicalJson appends converters additively rather than clearing first — the ordering guarantee holds only because all three target options objects happen to start with empty converter lists today.

src/Cove.Api/Program.cs (ApplyCanonicalJson converter ordering assumption) and src/Cove.Tests/EnumWireFormatTests.cs (hardcoded enum count 27).

Important Files Changed

Filename Overview
src/Cove.Core/Common/CoveJson.cs New canonical frozen JsonSerializerOptions combining source-gen context + reflection fallback; enums via ordered converters (CriterionModifier-specific first, global camelCase second).
src/Cove.Core/Common/CoveJsonContext.cs New source-gen serializer context for four host DTO roots; UseStringEnumConverter = true covers registered types, reflection fallback handles the rest.
src/Cove.Core/Common/CriterionModifierJsonConverter.cs Extracted verbatim from DynamicGroups.cs; lenient read (separator-insensitive forms + integer), canonical camelCase write.
src/Cove.Api/Program.cs Introduces ApplyCanonicalJson helper; converter list is appended without clearing, which is correct today but the ordering guarantee is implicit.
src/Cove.Api/Services/ConfigService.cs Migrates _jsonOpts to new(CoveJson.Default) copy with WriteIndented + WhenWritingNull override.
src/Cove.Api/Services/DynamicGroups.cs Removes local CriterionModifierJsonConverter class and local JsonOptions; deserialization now routes through CoveJson.Default.
src/Cove.Core/Enums/Enums.cs Adds [JsonStringEnumMemberName] pins to GenderEnum and CircumcisedEnum to lock wire names against future C# renames.
src/Cove.Core/Events/Events.cs Adds [JsonStringEnumMemberName] pins to all EventType members to lock the SignalR wire contract.
src/Cove.Core/Interfaces/IJobService.cs Adds [JsonStringEnumMemberName] pins to JobStatus and JobUnitOutcome.
src/Cove.Tests/EnumWireFormatTests.cs Comprehensive enum contract tests; hardcoded count 27 in AllCoreEnums_CountIs27 will produce an ambiguous failure when a new enum is added without updating the list.
src/Cove.Tests/Integration/SerializerWireParityTests.cs Integration tests verifying MVC/SignalR/HTTP.Json parity and unregistered DTO enum serialization as strings.
src/Cove.Tests/SpecialTypeRoundTripTests.cs Locks DateTime/Guid/byte[] wire policy and asserts no decimal fields in Cove.Core.DTOs.
src/Cove.Tests/UserUiPreferencesRoundTripTests.cs Round-trip and pre-consolidation drift tests for UserUiPreferencesDto.
src/Cove.Tests/SourceGenEnumParityTests.cs Guards against source-gen vs reflection split-brain for unpinned multi-word enums on registered DTOs.
src/Cove.Data/Auth/UserRoleServices.cs Replaces bespoke UiPreferencesJsonOptions with CoveJson.Default; drift validated by UserUiPreferencesRoundTripTests.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Serialization Request] --> B{Which boundary?}
    B -->|MVC Controller| C[MVC JsonSerializerOptions]
    B -->|SignalR Hub| D[SignalR PayloadSerializerOptions]
    B -->|Minimal-API / Extension Endpoints| E[HTTP.Json SerializerOptions]
    B -->|Manual call site| F[CoveJson.Default directly]
    C & D & E -->|ApplyCanonicalJson| G[CoveJson.Default properties copied]
    F --> G
    G --> H{TypeInfoResolver}
    H -->|Registered DTO| I[CoveJsonContext source-gen fast path]
    H -->|Unregistered type| J[DefaultJsonTypeInfoResolver reflection fallback]
    I --> K[Converters list - first-match-wins]
    J --> K
    K -->|CriterionModifier| L[CriterionModifierJsonConverter - lenient read]
    K -->|Any other enum| M[JsonStringEnumConverter CamelCase]
    K -->|Non-enum| N[Framework default]
    L & M & N --> O[Wire: camelCase props + camelCase string enums]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Serialization Request] --> B{Which boundary?}
    B -->|MVC Controller| C[MVC JsonSerializerOptions]
    B -->|SignalR Hub| D[SignalR PayloadSerializerOptions]
    B -->|Minimal-API / Extension Endpoints| E[HTTP.Json SerializerOptions]
    B -->|Manual call site| F[CoveJson.Default directly]
    C & D & E -->|ApplyCanonicalJson| G[CoveJson.Default properties copied]
    F --> G
    G --> H{TypeInfoResolver}
    H -->|Registered DTO| I[CoveJsonContext source-gen fast path]
    H -->|Unregistered type| J[DefaultJsonTypeInfoResolver reflection fallback]
    I --> K[Converters list - first-match-wins]
    J --> K
    K -->|CriterionModifier| L[CriterionModifierJsonConverter - lenient read]
    K -->|Any other enum| M[JsonStringEnumConverter CamelCase]
    K -->|Non-enum| N[Framework default]
    L & M & N --> O[Wire: camelCase props + camelCase string enums]
Loading

Comments Outside Diff (2)

  1. src/Cove.Api/Program.cs, line 25-32 (link)

    P2 ApplyCanonicalJson appends converters without clearing the target list

    options.Converters.Add(converter) is additive. The ordering contract that CriterionModifierJsonConverter must be registered before JsonStringEnumConverter (first-match-wins) holds only because the three target options objects (MvcJsonOptions, PayloadSerializerOptions, Http.Json SerializerOptions) all start with an empty Converters list at the call sites today. If any upstream service registration or future middleware pre-populates converters on those options before ApplyCanonicalJson runs, a JsonStringEnumConverter registered first would intercept CriterionModifier reads before the lenient converter gets a chance, silently dropping the separator-insensitive forms. A comment on the method (or a options.Converters.Clear() before the loop) would make the assumption explicit and prevent a hard-to-diagnose regression.

  2. src/Cove.Tests/EnumWireFormatTests.cs, line 639-643 (link)

    P2 Hardcoded count assertion produces ambiguous failure on new enums

    When a new enum is added to Cove.Core without updating AllCoreEnums, this test fails with Assert.Equal(27, 28) — nothing in the failure message indicates which enum is missing. A more diagnostic alternative is to assert that AllCoreEnums contains every enum type defined in the Cove.Core assembly, letting the diff between the two sets identify the gap immediately.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "refactor: drop unused serialization impo..." | Re-trigger Greptile

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.

1 participant