Skip to content

Commit 8040ee8

Browse files
dmealingclaude
andcommitted
simplify: ask each question once, and in the place that already owns the answer
Cleanup pass over the map work. Nothing here changes what a correct model generates, with one deliberate exception noted below. **Sort allowlist → the canonical band.** Both sort allowlists now ask `FilterOps.supportsFiltering(subType)` — the cross-port answer the filter side already uses and the loader itself checks in `validateSortableHasSupportedSubtype`. This closes a hole the hand-listed version had left: the TPH base allowlist still skipped only `ObjectField`, so a map was unsortable on a vanilla entity and sortable on a TPH base. Hand-maintained exclusion lists are how that happened; a shared band cannot drift from the loader. **One predicate for "which value object does this map carry."** `SpringTypeMapper` was resolving `@objectRef` itself and accepting ANY target, while its sibling requires `object.value` — so `@objectRef` naming an entity typed a component as that entity while nothing emitted a record for it. It now calls the sibling. The FQN rule gets a name (`fqJavaTypeName`), and the array-wrap rule gets one (`wrapsAsList`) so its two call sites state it once instead of twice. **One emitter for element-wise PATCH validation.** The map branch was the array branch copy-pasted with a renamed loop variable, leaving the 400 envelope in three places. **C#: gate the helper on what was emitted, not a second derivation of it.** `NeedsMapJsonbHelper` hand-mirrored three emission sites and was wrong about two — it undercounted them and did not model `jsonbObjectsOnly` suppressing the flattened arm. Replaced by a scan of `modelLines`, which is complete at that point: exact by construction. `MapJsonbSuffix` likewise keeps the top-level and flattened tiers from drifting. **The one behavior change — the emitted comparer.** It compared serialized JSON, which is key-ORDER sensitive: a dictionary rebuilt in a different order read as changed and would issue an UPDATE for a row nothing touched. It also serialized both dictionaries on every equality check and deep-copied on every snapshot. Now entry-wise, with scalars settling on `EqualityComparer<T>.Default` and never serializing; only a value-object value falls through to JSON, because the generated POCO compares by reference. The snapshot deep-copies only when the value type needs it. Pinned by test. **Two KNOWN_GAPS files that had become false.** C# G9 ("a field.map member of a flattened value object gets no EF column mapping") described the gap this work closed, prescribed "do not use field.map inside a flattened value object", and pinned itself to a test renamed two commits ago — while the generator's surviving warning still says "See KNOWN_GAPS.md". Marked closed with what replaced it. G7's contract clause called a map an owned navigation, which the converter path contradicts. The Java entry said field.map was "still staged out, needs a persistence-conformance roundtrip column first" — the codegen rung did not depend on that gate, but the gate is still open and still right for the RUNTIME tier, which is now what it says. **Tests.** A drift gate pins the map's `@valueType` table against `javaTypeName`'s own arms, so adding a scalar subtype to one and not the other fails instead of shipping a divergent map. The hand-rolled javac block became `SpringTestFixtures.compileGenerated` — the 17th copy in the package, and now the last one that needs writing. Dropped a test wholly subsumed by the table test and a compile-fixture field covered by two other receivers. Java 246 green on `clean test`; C# 442 + 1024 + 291 + 77 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013sjn9TaHoZFdDPF2dLcppv
1 parent cf29990 commit 8040ee8

12 files changed

Lines changed: 308 additions & 224 deletions

File tree

‎server/csharp/MetaObjects.Codegen.Tests/DbContextCompileTests.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
// array string "tags" (isArray) → .PrimitiveCollection()
2323
// field.object homeAddress @storage flattened → OwnsOne(...) per-property column names
2424
// field.object config (default storage) → OwnsOne(...).ToJson(...)
25-
// field.map labels/scores (@valueType) → .HasColumnType("jsonb").HasConversion(
25+
// field.map labels (@valueType) → .HasColumnType("jsonb").HasConversion(
2626
// MapJsonb.Converter<T>(), MapJsonb.Comparer<T>())
2727
// field.map sites (@objectRef) → the same, typed by the value object.
2828
// These prove the EF API surface actually RESOLVES: the two-arg
@@ -85,7 +85,6 @@ public class DbContextCompileTests
8585
{ "field.object": { "name": "homeAddress", "@objectRef": "Address", "@storage": "flattened" } },
8686
{ "field.object": { "name": "config", "@objectRef": "Address" } },
8787
{ "field.map": { "name": "labels", "@valueType": "string" } },
88-
{ "field.map": { "name": "scores", "@valueType": "int" } },
8988
{ "field.map": { "name": "sites", "@objectRef": "Address" } },
9089
{ "identity.primary": { "@fields": "id" } }
9190
]}},

‎server/csharp/MetaObjects.Codegen.Tests/MapFieldCodegenTests.cs‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ public class MapFieldCodegenTests
3131
]}}
3232
""";
3333

34-
private static MetaRoot Load()
34+
private static MetaRoot Load(string model = Model, string id = "map.json")
3535
{
36-
var r = new MetaDataLoader().Load([new InMemoryStringSource(Model, id: "map.json")]);
36+
var r = new MetaDataLoader().Load([new InMemoryStringSource(model, id: id)]);
3737
Assert.Empty(r.Errors);
3838
return r.Root;
3939
}
@@ -75,11 +75,9 @@ public void Scalar_valued_map_gets_a_jsonb_storage_mapping_in_the_DbContext()
7575
{
7676
var dbCtx = Assert.Single(new DbContextGenerator().Generate(Ctx(Load()))).Content;
7777

78-
// Without an explicit mapping EF does not persist a Dictionary the way the
79-
// TS-owned schema DDL declares it: on Npgsql a Dictionary<string, string> binds to
80-
// HSTORE by default, and a Dictionary<string, int> binds to nothing at all. The
81-
// column the migration creates is jsonb, so the context must say jsonb and supply
82-
// the (de)serializer -- the C# analog of Kotlin's jsonb(col, encoder, decoder).
78+
// WHY an explicit mapping is needed at all is stated once, at the emission site
79+
// (the map loop in DbContextGenerator.EmitFieldTypeConfig): an unmapped Dictionary
80+
// does not land on the jsonb column the migration creates.
8381
Assert.Contains(
8482
"modelBuilder.Entity<Customer>().Property(x => x.Labels).HasColumnType(\"jsonb\")"
8583
+ ".HasConversion(MapJsonb.Converter<string>(), MapJsonb.Comparer<string>());",
@@ -111,6 +109,14 @@ public void Map_jsonb_helper_is_emitted_only_when_a_map_is_present()
111109
Assert.Contains("Dictionary<string, TValue>, string> Converter<TValue>()", withMap);
112110
Assert.Contains("Dictionary<string, TValue>> Comparer<TValue>()", withMap);
113111

112+
// Equality must be ENTRY-WISE, not a comparison of serialized JSON. JSON string
113+
// equality is key-ORDER sensitive, so a dictionary rebuilt in a different order would
114+
// read as changed and issue an UPDATE for a row nothing touched — and it would
115+
// serialize both dictionaries on every check. Scalars take the default comparer and
116+
// never serialize; only a value-object value falls through to JSON.
117+
Assert.Contains("if (!b.TryGetValue(kv.Key, out var other)) return false;", withMap);
118+
Assert.Contains("EqualityComparer<TValue>.Default.Equals(kv.Value, other)) continue;", withMap);
119+
114120
// A model with no field.map must stay byte-identical -- the helper is gated, exactly
115121
// as the UnmappedEnumValue helper is.
116122
const string noMap = """
@@ -123,13 +129,8 @@ public void Map_jsonb_helper_is_emitted_only_when_a_map_is_present()
123129
]}}
124130
]}}
125131
""";
126-
var r = new MetaDataLoader().Load([new InMemoryStringSource(noMap, id: "nomap.json")]);
127-
Assert.Empty(r.Errors);
128-
var without = Assert.Single(new DbContextGenerator().Generate(new GenContext
129-
{
130-
Entities = r.Root.Objects(), Root = r.Root,
131-
Config = new GenConfig { OutDir = "/tmp", Namespace = "Acme.Generated" },
132-
})).Content;
132+
var without = Assert.Single(
133+
new DbContextGenerator().Generate(Ctx(Load(noMap, "nomap.json")))).Content;
133134
Assert.DoesNotContain("MapJsonb", without);
134135
}
135136

@@ -150,13 +151,7 @@ public void A_read_only_projection_map_column_also_gets_its_jsonb_mapping()
150151
]}}
151152
]}}
152153
""";
153-
var r = new MetaDataLoader().Load([new InMemoryStringSource(model, id: "proj.json")]);
154-
Assert.Empty(r.Errors);
155-
var ctx = new GenContext
156-
{
157-
Entities = r.Root.Objects(), Root = r.Root,
158-
Config = new GenConfig { OutDir = "/tmp", Namespace = "Acme.Generated" },
159-
};
154+
var ctx = Ctx(Load(model, "proj.json"));
160155

161156
// The property is emitted...
162157
var entity = Assert.Single(new EntityGenerator().Generate(ctx)).Content;

‎server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs‎

Lines changed: 75 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,13 @@ public virtual IEnumerable<EmittedFile> Generate(GenContext ctx)
231231
EmitDbSetDeclarations(sb, objects, ctx);
232232
EmitOnModelCreatingBody(sb, modelLines, ctx);
233233
if (NeedsUnmappedEnumHelper(objects, ctx.Root)) EmitUnmappedEnumHelper(sb);
234-
if (NeedsMapJsonbHelper(objects, ctx.Root)) EmitMapJsonbHelper(sb);
234+
// Gated on the EMITTED text, not on a second derivation of it. Every site that uses the
235+
// helper appends to modelLines (entity, read-only projection, and the flattened-VO member
236+
// block, which lands here as one OwnedTypeConfig string), and modelLines is complete by
237+
// now — so this is exact by construction. The predicate it replaces had to hand-mirror
238+
// three emission sites, and had already been wrong twice: it undercounted them, and it
239+
// did not model jsonbObjectsOnly suppressing the flattened arm on a read model.
240+
if (modelLines.Any(l => l.Contains(MapJsonbHelperName))) EmitMapJsonbHelper(sb);
235241
sb.AppendLine("}");
236242
return [new EmittedFile("AppDbContext.g.cs", sb.ToString())];
237243
}
@@ -635,12 +641,22 @@ private static void EmitUnmappedEnumHelper(StringBuilder sb)
635641
/// (<see cref="EmitFieldTypeConfig"/>) and the read-only-projection path, which both emit
636642
/// a <c>Dictionary&lt;string, V&gt;</c> property and so both need the mapping.
637643
/// </summary>
638-
private static string MapJsonbConfig(string className, MetaField f, GenContext ctx)
644+
private static string MapJsonbConfig(string className, MetaField f, GenContext ctx) =>
645+
$" modelBuilder.Entity<{className}>().Property(x => x.{CSharpNaming.Pascal(f.Name)})"
646+
+ MapJsonbSuffix(f, ctx) + ";";
647+
648+
/// <summary>
649+
/// The column-type + converter/comparer suffix every <c>field.map</c> mapping ends in, whatever
650+
/// the receiver. Written once so the two tiers — a top-level property and a FLATTENED value
651+
/// object's member, which differ only in the receiver and a <c>HasColumnName</c> — cannot drift:
652+
/// a converter change fixed in one tier only is exactly the failure the flattened tier's old
653+
/// known-gap entry described.
654+
/// </summary>
655+
private static string MapJsonbSuffix(MetaField f, GenContext ctx)
639656
{
640657
var valueType = MapValueTypeRef(f, ctx);
641-
return $" modelBuilder.Entity<{className}>().Property(x => x.{CSharpNaming.Pascal(f.Name)})"
642-
+ $".HasColumnType(\"jsonb\").HasConversion("
643-
+ $"{MapJsonbHelperName}.Converter<{valueType}>(), {MapJsonbHelperName}.Comparer<{valueType}>());";
658+
return $".HasColumnType(\"jsonb\").HasConversion("
659+
+ $"{MapJsonbHelperName}.Converter<{valueType}>(), {MapJsonbHelperName}.Comparer<{valueType}>())";
644660
}
645661

646662
/// <summary>Name of the generated jsonb (de)serialization helper the field.map configs use.</summary>
@@ -664,8 +680,9 @@ private static string MapJsonbConfig(string className, MetaField f, GenContext c
664680
private static string MapValueTypeRef(MetaField f, GenContext ctx)
665681
{
666682
if (f.ObjectRef is not { } oref || oref.Length == 0) return CSharpNaming.MapValueType(f);
667-
var bare = CSharpNaming.Pascal(CSharpNaming.StripPkg(oref));
668-
var vo = ctx.Root.FindObject(CSharpNaming.StripPkg(oref));
683+
var shortName = CSharpNaming.StripPkg(oref);
684+
var bare = CSharpNaming.Pascal(shortName);
685+
var vo = ctx.Root.FindObject(shortName);
669686
if (vo is null) return bare;
670687
var ns = PackageBindingResolver.Resolve(
671688
ctx.Config, PackageBindingResolver.EffectivePackage(vo), vo.Name);
@@ -682,10 +699,14 @@ private static string MapValueTypeRef(MetaField f, GenContext ctx)
682699
/// <para>The COMPARER is the load-bearing half, not decoration. EF snapshots a
683700
/// value-converted property by REFERENCE unless told otherwise, so with a converter alone
684701
/// an in-place <c>entity.Labels["k"] = v</c> is never detected as a change and the UPDATE
685-
/// never fires — the same silent non-persistence this whole mapping exists to fix. The
686-
/// snapshot therefore deep-copies (serialize + deserialize), and equality compares the
687-
/// serialized JSON: two dictionaries are equal exactly when they would write the same
688-
/// jsonb, which is the right notion for a value object as well as a scalar.</para>
702+
/// never fires — the same silent non-persistence this whole mapping exists to fix.</para>
703+
/// <para>It compares ENTRY-WISE rather than by serialized JSON, which matters in both
704+
/// directions: JSON string equality is key-ORDER sensitive, so a dictionary rebuilt in a
705+
/// different order would read as changed and issue an UPDATE for a row nothing touched;
706+
/// and it made every equality check serialize both dictionaries. Scalars now settle on
707+
/// <c>EqualityComparer&lt;T&gt;.Default</c> with no serialization at all, while a value-object
708+
/// value still falls through to a JSON compare because the generated POCO is a class and
709+
/// compares by reference. The snapshot deep-copies only when the value type needs it.</para>
689710
/// <para>Fully qualified throughout: the generated file's usings are a fixed set
690711
/// (<see cref="EmitUsings"/>), and widening it would change byte-identical output for
691712
/// every model.</para>
@@ -710,37 +731,52 @@ private static void EmitMapJsonbHelper(StringBuilder sb)
710731
sb.AppendLine();
711732
sb.AppendLine(" internal static Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer<");
712733
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>> Comparer<TValue>() => new(");
713-
sb.AppendLine(" (a, b) => System.Text.Json.JsonSerializer.Serialize(a, Options)");
714-
sb.AppendLine(" == System.Text.Json.JsonSerializer.Serialize(b, Options),");
715-
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Serialize(v, Options).GetHashCode(),");
716-
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Deserialize<");
734+
sb.AppendLine(" (a, b) => Eq(a, b), v => Hash(v), v => Snap(v));");
735+
sb.AppendLine();
736+
sb.AppendLine(" /// <summary>Order-INDEPENDENT equality: a dictionary rebuilt in a different key");
737+
sb.AppendLine(" /// order holds the same value, and comparing serialized JSON would call it changed");
738+
sb.AppendLine(" /// and issue an UPDATE for a row nothing touched. Scalars settle on the default");
739+
sb.AppendLine(" /// comparer (no serialization at all); a value-object value falls through to a JSON");
740+
sb.AppendLine(" /// compare, since the generated POCO is a class and compares by reference.</summary>");
741+
sb.AppendLine(" private static bool Eq<TValue>(");
742+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>? a,");
743+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>? b)");
744+
sb.AppendLine(" {");
745+
sb.AppendLine(" if (a is null || b is null) return ReferenceEquals(a, b);");
746+
sb.AppendLine(" if (a.Count != b.Count) return false;");
747+
sb.AppendLine(" foreach (var kv in a)");
748+
sb.AppendLine(" {");
749+
sb.AppendLine(" if (!b.TryGetValue(kv.Key, out var other)) return false;");
750+
sb.AppendLine(" if (System.Collections.Generic.EqualityComparer<TValue>.Default.Equals(kv.Value, other)) continue;");
751+
sb.AppendLine(" if (System.Text.Json.JsonSerializer.Serialize(kv.Value, Options)");
752+
sb.AppendLine(" != System.Text.Json.JsonSerializer.Serialize(other, Options)) return false;");
753+
sb.AppendLine(" }");
754+
sb.AppendLine(" return true;");
755+
sb.AppendLine(" }");
756+
sb.AppendLine();
757+
sb.AppendLine(" /// <summary>Keys and count only — order-independent (XOR), allocation-free, and");
758+
sb.AppendLine(" /// stable for a value-object value whose GetHashCode is reference-based. Equal");
759+
sb.AppendLine(" /// dictionaries always hash equal, which is all EF requires.</summary>");
760+
sb.AppendLine(" private static int Hash<TValue>(System.Collections.Generic.Dictionary<string, TValue> v)");
761+
sb.AppendLine(" {");
762+
sb.AppendLine(" var h = v.Count;");
763+
sb.AppendLine(" foreach (var k in v.Keys) h ^= k.GetHashCode();");
764+
sb.AppendLine(" return h;");
765+
sb.AppendLine(" }");
766+
sb.AppendLine();
767+
sb.AppendLine(" /// <summary>The change-tracking snapshot. A value-object value needs a DEEP copy or");
768+
sb.AppendLine(" /// an in-place edit to a nested object mutates the snapshot too and goes undetected;");
769+
sb.AppendLine(" /// an immutable scalar only needs the dictionary copied.</summary>");
770+
sb.AppendLine(" private static System.Collections.Generic.Dictionary<string, TValue> Snap<TValue>(");
771+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue> v) =>");
772+
sb.AppendLine(" typeof(TValue).IsValueType || typeof(TValue) == typeof(string)");
773+
sb.AppendLine(" ? new System.Collections.Generic.Dictionary<string, TValue>(v)");
774+
sb.AppendLine(" : System.Text.Json.JsonSerializer.Deserialize<");
717775
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>>(");
718-
sb.AppendLine(" System.Text.Json.JsonSerializer.Serialize(v, Options), Options)!);");
776+
sb.AppendLine(" System.Text.Json.JsonSerializer.Serialize(v, Options), Options)!;");
719777
sb.AppendLine(" }");
720778
}
721779

722-
/// <summary>
723-
/// True when any emitted config will reference <see cref="MapJsonbHelperName"/>. Mirrors
724-
/// BOTH sites that emit one: the map loop in <see cref="EmitFieldTypeConfig"/> (an emitted
725-
/// object's own <c>field.map</c>) AND the flattened-member arm of
726-
/// <see cref="OwnedTypeConfig"/> (a map member of a FLATTENED value object, which gets its
727-
/// own jsonb column at the prefixed name). A value object is not in
728-
/// <paramref name="objects"/>, so scanning only their own fields would emit a config
729-
/// naming a helper this file never declares — the same trap
730-
/// <see cref="NeedsUnmappedEnumHelper"/> documents.
731-
/// </summary>
732-
/// <remarks>
733-
/// A NON-flattened value object's map member is deliberately absent: it lives inside the
734-
/// owning column's JSON document, where System.Text.Json serializes the dictionary and no
735-
/// EF column mapping of its own exists.
736-
/// </remarks>
737-
private static bool NeedsMapJsonbHelper(IEnumerable<MetaObject> objects, MetaRoot root) =>
738-
objects.Any(o => o.Fields().Any(f =>
739-
f.SubType == FIELD_SUBTYPE_MAP
740-
|| (f.SubType == FIELD_SUBTYPE_OBJECT && f.Storage == STORAGE_FLATTENED && !f.ResolvedIsArray()
741-
&& f.ObjectRef is { } oref && root.FindObject(CSharpNaming.StripPkg(oref)) is { } vo
742-
&& vo.Fields().Any(nf => nf.SubType == FIELD_SUBTYPE_MAP))));
743-
744780
/// <summary>
745781
/// True when any emitted converter will reference <see cref="UnmappedEnumHelperName"/>
746782
/// — i.e. the model carries at least one int-backed <c>field.enum</c>. Mirrors the
@@ -1067,11 +1103,8 @@ private void EmitFieldTypeConfig(
10671103
// default: a column the migration never creates (42703 at the engine).
10681104
if (nf.SubType == FIELD_SUBTYPE_MAP)
10691105
{
1070-
var mapValue = MapValueTypeRef(nf, ctx);
10711106
sb.AppendLine($" b.Property(p => p.{CSharpNaming.Pascal(nf.Name)})"
1072-
+ $".HasColumnName(\"{nestedColAny}\").HasColumnType(\"jsonb\").HasConversion("
1073-
+ $"{MapJsonbHelperName}.Converter<{mapValue}>(), "
1074-
+ $"{MapJsonbHelperName}.Comparer<{mapValue}>());");
1107+
+ $".HasColumnName(\"{nestedColAny}\")" + MapJsonbSuffix(nf, ctx) + ";");
10751108
continue;
10761109
}
10771110

0 commit comments

Comments
 (0)