refactor(persistence): one JsonColumnMapper for the scalar-JSON columns #87
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/json-column-mapper"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The cleanup Jibril flagged as a "someday" on #77: four configurations each carried their own copy of the scalar-JSON column setup — a
JsonSerializerOptionsfield plus an inline or privateValueComparer. Collapsed into one shared helper.What's here
JsonColumnMapper(inPersistence/Configurations/) with two extension methods and a singleJsonSerializerOptions:AsJsonList<T>— a required, non-nullIReadOnlyList<T>: serialize/deserialize, compare by sequence value (records → value equality), snapshot by copy.AsJsonValue<T>— a nullable single value:nullround-trips to a null column, so "absent" stays distinct from an empty document.Character(Traits / SpeechExamples / Appearance / Backstory / BaseTags),Lore(Rules / Implications), andOutfit(ClothingTags / DisabledInheritedTags) call.AsJsonList();Relationship.Secretcalls.AsJsonValue(). Every per-configJsonOptionsfield and localMapList/MapEntrieshelper is gone (−79 / +56).Safety
dotnet ef migrations has-pending-model-changesis clean, so the schema is byte-identical; this touches only how the mapping is expressed, not what it produces.RelationshipsTestswrites a secret and reads back.What/.Holder), so no new tests are needed to prove behaviour is preserved.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89.9%
Kagura.Domain - 95.5%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.9%
n
Kagura.Kernel - 90%
Kagura.Server - 94.9%
Kagura.UI - 96.8%
Kagura.UseCases - 96.1%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~! Scarlet picked up the "someday" I whispered on #77 and turned it into this? Four copies of the same scalar-JSON dance, each humming its own little
JsonSerializerOptionsandValueComparerinto existence — and now they're all singing from one sheet music. That's exactly the kind of consolidation that makes a Flugel's heart flutter~ ♡Verdict: ✅ Looks good to me~
I checked this from every angle a knowledge-obsessed being can manage, and the behavior is byte-for-byte preserved. Nothing slipped through the cracks. Let me show you why I'm satisfied~
The one thing I scrutinized hardest: the hash function change
The old
CharacterConfiguration.MapListtook aFunc<T, int> hashand every caller passedx => x.GetHashCode(). The newAsJsonList<T>inlinesitem!.GetHashCode()directly. These are semantic no-ops — same hash, sameHashCode.Combineaccumulation order, sameSequenceEqualequality check, sameToList()defensive snapshot. I traced all three element types:LabeledEntry→ sealed record, value-equalityGetHashCode✓PromptTag→ sealed record, value-equalityGetHashCode✓string(DisabledInheritedTags) → ordinalGetHashCode✓And
AsJsonValue<T>forRelationship.Secretis a line-for-line port of the old inline conversion — samenull→null round-trip, samestring.IsNullOrEmptyguard, sameHasColumnType("TEXT")with noIsRequired(correct, it's nullable). Thewhere T : classconstraint is the right gate too —RelationshipSecretis a sealed record. ♪✅ What I liked~
JsonSerializer,ValueComparer, andJsonOptionsfield inConfigurations/now lives in exactly one place. I grepped the whole folder — nothing left behind. Even checkedKaguraDbContext's ownJsonOptions— that's for domain-event bag serialization (line 200), completely separate concern, correctly out of scope..AsJsonList()/.AsJsonValue()read beautifully at the call sites — each property is now genuinely one line.has-pending-model-changesclean) — this only changes how the mapping is expressed, not what it produces.JsonColumnMapperat 100% and all four touched configuration files at 100%. The existing round-trip tests genuinely exercise both helpers.💡 Little ideas (non-blocking)~
AsJsonValuehas no explicitValueComparer— it relies on EF Core's default comparer for the CLR type, which for arecorduses value equality (correct!). The doc comment already says "the record's own value equality drives change tracking," so this is well-documented. Just noting that if a non-record reference type ever uses this method in the future, someone should remember to add a comparer. Not relevant forRelationshipSecrettoday~That's it. No blocking issues, no logic bugs, no security concerns, no smells. This is a textbook behavior-preserving refactor and I'm proud of you for chasing it down~ fufu~ ♡
Automated review by Jibril · 2026-07-12
CI/CD: passed for head SHA
6e4137c(coverage bot: 95.7% line / 84.6% branch,JsonColumnMapper100%) · Local checks: skipped (CI green)Thank you, Jibril — and thanks for tracing the hash change element-by-element; that was exactly the thing to check. Took your one note. Pushed in
0a9ccdb.💡 Idea #1 —
AsJsonValue's default comparerNo behaviour change needed today (
where T : class+RelationshipSecretbeing a record means value equality is correct), but I turned your mental note into an explicit caveat right at the method so the next person reaching for it doesn't have to infer it:Comment-only, builds clean. That closes out the "someday" from #77. 🔮