Skip to content

Commit 4d536b9

Browse files
committed
no-mistakes(test): Verified C# field.map nullable-property fix resolves NULL-column 500s
1 parent 85f87c1 commit 4d536b9

4 files changed

Lines changed: 423 additions & 22 deletions

File tree

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public class MapFieldCodegenTests
2828
{ "field.string": { "name": "name", "@required": true } },
2929
{ "field.map": { "name": "labels", "@valueType": "string" } },
3030
{ "field.map": { "name": "addresses", "@objectRef": "Address" } },
31+
{ "field.map": { "name": "channels", "@valueType": "string", "@required": true } },
3132
{ "identity.primary": { "@fields": "id" } }
3233
]}}
3334
]}}
@@ -55,9 +56,17 @@ public void Scalar_valued_map_emits_a_string_value_dictionary()
5556
var files = new EntityGenerator().Generate(Ctx(Load())).ToList();
5657
var customer = files.Single(f => f.Path == "Customer.g.cs").Content;
5758

58-
// @valueType:string → Dictionary<string, string>, [Column]-mapped, never null.
59+
// @valueType:string → Dictionary<string, string>, [Column]-mapped. NULLABILITY
60+
// FOLLOWS THE COLUMN (ObjectNavProperty's rule): the migration creates a map
61+
// column NULLABLE by default, and a non-nullable property over it made EF Core 8
62+
// skip the shaper's IsDBNull check — a 500 on every read of a NULL-map row (the
63+
// MapNullColumnGeneratedServerTest lane drives that end-to-end). So a
64+
// non-required map is NULLABLE with NO initializer (absent stays null, round-trips
65+
// as SQL NULL — never silently rewritten to {}), while a @required map keeps the
66+
// non-null empty-dictionary initializer.
5967
Assert.Contains("[Column(CustomerNames.LabelsColumn)]", customer); // §A6 (task 4)
60-
Assert.Contains("public Dictionary<string, string> Labels { get; set; } = new();", customer);
68+
Assert.Contains("public Dictionary<string, string>? Labels { get; set; }", customer);
69+
Assert.Contains("public Dictionary<string, string> Channels { get; set; } = new();", customer);
6170
}
6271

6372
[Fact]
@@ -66,9 +75,10 @@ public void Object_valued_map_emits_a_value_object_value_dictionary()
6675
var files = new EntityGenerator().Generate(Ctx(Load())).ToList();
6776
var customer = files.Single(f => f.Path == "Customer.g.cs").Content;
6877

69-
// @objectRef:Address → Dictionary<string, Address>; the VO is emitted as a POCO.
78+
// @objectRef:Address → Dictionary<string, Address> (nullable — see the scalar test
79+
// for the column-following rule); the VO is emitted as a POCO.
7080
Assert.Contains("[Column(CustomerNames.AddressesColumn)]", customer); // §A6 (task 4)
71-
Assert.Contains("public Dictionary<string, Address> Addresses { get; set; } = new();", customer);
81+
Assert.Contains("public Dictionary<string, Address>? Addresses { get; set; }", customer);
7282
Assert.Contains("public class Address", files.Single(f => f.Path == "Address.g.cs").Content);
7383
}
7484

@@ -84,6 +94,13 @@ public void Scalar_valued_map_gets_a_jsonb_storage_mapping_in_the_DbContext()
8494
"modelBuilder.Entity<Customer>().Property(x => x.Labels).HasColumnType(\"jsonb\")"
8595
+ ".HasConversion(MapJsonb.Converter<string>(), MapJsonb.Comparer<string>());",
8696
dbCtx);
97+
// The @required map's property is non-null, so its converter is the non-null
98+
// factory — EF Core 8's nullability-aware HasConversion checks the converter's
99+
// model type against the property's own annotation (a mismatch is CS8620).
100+
Assert.Contains(
101+
"modelBuilder.Entity<Customer>().Property(x => x.Channels).HasColumnType(\"jsonb\")"
102+
+ ".HasConversion(MapJsonb.RequiredConverter<string>(), MapJsonb.Comparer<string>());",
103+
dbCtx);
87104
}
88105

89106
[Fact]
@@ -108,8 +125,10 @@ public void Map_jsonb_helper_is_emitted_only_when_a_map_is_present()
108125
// in-place `entity.Labels["k"] = v` is never detected and the UPDATE never fires --
109126
// the same silent non-persistence this mapping exists to fix.
110127
Assert.Contains("private static class MapJsonb", withMap);
111-
Assert.Contains("Dictionary<string, TValue>, string> Converter<TValue>()", withMap);
112-
Assert.Contains("Dictionary<string, TValue>> Comparer<TValue>()", withMap);
128+
// The nullable type arguments match the nullable map property (EF Core 8's
129+
// nullability-aware HasConversion expects them; see EmitMapJsonbHelper remarks).
130+
Assert.Contains("Dictionary<string, TValue>?, string> Converter<TValue>()", withMap);
131+
Assert.Contains("Dictionary<string, TValue>?> Comparer<TValue>()", withMap);
113132

114133
// Equality must be ENTRY-WISE, not a comparison of serialized JSON. JSON string
115134
// equality is key-ORDER sensitive, so a dictionary rebuilt in a different order would
@@ -156,9 +175,10 @@ public void A_read_only_projection_map_column_also_gets_its_jsonb_mapping()
156175
""";
157176
var ctx = Ctx(Load(model, "proj.json"));
158177

159-
// The property is emitted...
178+
// The property is emitted... (nullable, no initializer — the view's map column is
179+
// nullable like any other, and a NULL cell must read as null, not 500.)
160180
var entity = Assert.Single(new EntityGenerator().Generate(ctx)).Content;
161-
Assert.Contains("public Dictionary<string, int> Tallies { get; set; } = new();", entity);
181+
Assert.Contains("public Dictionary<string, int>? Tallies { get; set; }", entity);
162182

163183
// ...so the storage mapping must be too.
164184
var dbCtx = Assert.Single(new DbContextGenerator().Generate(ctx)).Content;

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

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public virtual IEnumerable<EmittedFile> Generate(GenContext ctx)
115115
// explicit mapping makes EF agree with the jsonb column the TS-owned migration
116116
// creates (ADR-0015).
117117
foreach (var f in p.Fields().Where(f => f.SubType == FIELD_SUBTYPE_MAP))
118-
modelLines.Add(MapJsonbConfig(name, f, ctx));
118+
modelLines.Add(MapJsonbConfig(name, p, f, ctx));
119119
}
120120
foreach (var e in objects.Where(o => o.IsEntity() && !o.IsReadOnlyProjection()))
121121
{
@@ -641,9 +641,9 @@ private static void EmitUnmappedEnumHelper(StringBuilder sb)
641641
/// (<see cref="EmitFieldTypeConfig"/>) and the read-only-projection path, which both emit
642642
/// a <c>Dictionary&lt;string, V&gt;</c> property and so both need the mapping.
643643
/// </summary>
644-
private static string MapJsonbConfig(string className, MetaField f, GenContext ctx) =>
644+
private static string MapJsonbConfig(string className, MetaObject owner, MetaField f, GenContext ctx) =>
645645
$" modelBuilder.Entity<{className}>().Property(x => x.{CSharpNaming.Pascal(f.Name)})"
646-
+ MapJsonbSuffix(f, ctx) + ";";
646+
+ MapJsonbSuffix(owner, f, ctx) + ";";
647647

648648
/// <summary>
649649
/// The column-type + converter/comparer suffix every <c>field.map</c> mapping ends in, whatever
@@ -652,11 +652,18 @@ private static string MapJsonbConfig(string className, MetaField f, GenContext c
652652
/// a converter change fixed in one tier only is exactly the failure the flattened tier's old
653653
/// known-gap entry described.
654654
/// </summary>
655-
private static string MapJsonbSuffix(MetaField f, GenContext ctx)
655+
private static string MapJsonbSuffix(MetaObject owner, MetaField f, GenContext ctx)
656656
{
657657
var valueType = MapValueTypeRef(f, ctx);
658+
// The SAME requiredness predicate the property emission uses (MapProperty), so the
659+
// converter's nullability annotation always matches the property it converts: EF Core
660+
// 8's nullability-aware HasConversion signature checks the converter's model type
661+
// against the property's own annotation, and a mismatch is a CS8620 warning.
662+
var converter = CSharpNaming.IsRequired(owner, f)
663+
? $"{MapJsonbHelperName}.RequiredConverter<{valueType}>()"
664+
: $"{MapJsonbHelperName}.Converter<{valueType}>()";
658665
return $".HasColumnType(\"jsonb\").HasConversion("
659-
+ $"{MapJsonbHelperName}.Converter<{valueType}>(), {MapJsonbHelperName}.Comparer<{valueType}>())";
666+
+ $"{converter}, {MapJsonbHelperName}.Comparer<{valueType}>())";
660667
}
661668

662669
/// <summary>Name of the generated jsonb (de)serialization helper the field.map configs use.</summary>
@@ -710,6 +717,12 @@ private static string MapValueTypeRef(MetaField f, GenContext ctx)
710717
/// <para>Fully qualified throughout: the generated file's usings are a fixed set
711718
/// (<see cref="EmitUsings"/>), and widening it would change byte-identical output for
712719
/// every model.</para>
720+
/// <para>The converter/comparer type arguments are the NULLABLE dictionary: a
721+
/// non-required map's property is nullable (nullability follows the column), and EF
722+
/// Core 8's nullability-aware <c>HasConversion</c> signature expects the converter's
723+
/// model type to match. Annotation-only — EF passes null through a value converter
724+
/// untranslated and never invokes the expressions on it, so the lambdas' bodies are
725+
/// unchanged.</para>
713726
/// </remarks>
714727
private static void EmitMapJsonbHelper(StringBuilder sb)
715728
{
@@ -727,14 +740,24 @@ private static void EmitMapJsonbHelper(StringBuilder sb)
727740
sb.AppendLine(" };");
728741
sb.AppendLine();
729742
sb.AppendLine(" internal static Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<");
730-
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>, string> Converter<TValue>() => new(");
743+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>, string> RequiredConverter<TValue>() => new(");
744+
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Serialize(v, Options),");
745+
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Deserialize<");
746+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>>(v, Options)!);");
747+
sb.AppendLine();
748+
sb.AppendLine(" /// <summary>Same conversion as RequiredConverter, over the NULLABLE dictionary a");
749+
sb.AppendLine(" /// non-required map's property carries — keep the two bodies identical. EF Core's");
750+
sb.AppendLine(" /// nullability-aware HasConversion checks the converter's model type against the");
751+
sb.AppendLine(" /// property's own annotation, so one factory cannot serve both arms.</summary>");
752+
sb.AppendLine(" internal static Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<");
753+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>?, string> Converter<TValue>() => new(");
731754
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Serialize(v, Options),");
732755
sb.AppendLine(" v => System.Text.Json.JsonSerializer.Deserialize<");
733756
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>>(v, Options)!);");
734757
sb.AppendLine();
735758
sb.AppendLine(" internal static Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer<");
736-
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>> Comparer<TValue>() => new(");
737-
sb.AppendLine(" (a, b) => Eq(a, b), v => Hash(v), v => Snap(v)!);");
759+
sb.AppendLine(" System.Collections.Generic.Dictionary<string, TValue>?> Comparer<TValue>() => new(");
760+
sb.AppendLine(" (a, b) => Eq(a, b), v => Hash(v), v => Snap(v));");
738761
sb.AppendLine();
739762
sb.AppendLine(" /// <summary>Order-INDEPENDENT equality: a dictionary rebuilt in a different key");
740763
sb.AppendLine(" /// order holds the same value, and comparing serialized JSON would call it changed");
@@ -908,7 +931,7 @@ private void EmitFieldTypeConfig(
908931
// the generated code cannot make on a consumer's behalf. The COMPARER is not optional --
909932
// see EmitMapJsonbHelper.
910933
foreach (var f in fieldList.Where(f => f.SubType == FIELD_SUBTYPE_MAP))
911-
modelLines.Add(MapJsonbConfig(className, f, ctx));
934+
modelLines.Add(MapJsonbConfig(className, entity, f, ctx));
912935

913936
foreach (var f in fieldList.Where(f => f.SubType == FIELD_SUBTYPE_ENUM))
914937
{
@@ -1114,7 +1137,7 @@ private void EmitFieldTypeConfig(
11141137
if (nf.SubType == FIELD_SUBTYPE_MAP)
11151138
{
11161139
sb.AppendLine($" b.Property(p => p.{CSharpNaming.Pascal(nf.Name)})"
1117-
+ $".HasColumnName(\"{nestedColAny}\")" + MapJsonbSuffix(nf, ctx) + ";");
1140+
+ $".HasColumnName(\"{nestedColAny}\")" + MapJsonbSuffix(vo, nf, ctx) + ";");
11181141
continue;
11191142
}
11201143

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,18 +1351,30 @@ protected static string EscapeAttrString(string s) =>
13511351
// Dictionary<string, V> property stored in one json column — the map analog of
13521352
// ObjectNavProperty's single-jsonb-column object field. Keys are always strings;
13531353
// V is a value-object (@objectRef) or a scalar (@valueType, defaulting to string).
1354-
// isArray does not apply (a map is never wrapped in a collection-of-maps), so the
1355-
// property is a non-null dictionary with an empty-dictionary initializer (matching
1356-
// the array/enum-array convention — the dictionary itself is never null in C#).
1354+
// isArray does not apply (a map is never wrapped in a collection-of-maps).
1355+
// Nullability follows the COLUMN, exactly as ObjectNavProperty's single/array arms
1356+
// do: the TS-owned migration creates the map column NULLABLE by default (ADR-0015),
1357+
// and a NULL cell is ordinary cross-port state (a row written by another port or
1358+
// raw SQL). A NON-nullable property over that column makes EF Core 8 skip the
1359+
// shaper's IsDBNull check, so every read arm 500s on a single NULL-map row — the
1360+
// initializer never runs, and MapJsonb's null-guarded comparer sits after the crash
1361+
// point. So a @required map is a non-null dictionary with an empty-dictionary
1362+
// initializer, and a non-required map is a NULLABLE dictionary with NO initializer:
1363+
// an absent value stays null, reads back as null, and round-trips as SQL NULL
1364+
// rather than being silently rewritten to an empty object (a present {} stays a
1365+
// distinct empty map).
13571366
// When @objectRef names a value object, the resolved VO becomes a referenced POCO
13581367
// via the normal ReferencedValueObjects walk. withAttributes adds the EF [Column]
13591368
// mapping; value-object POCOs (no EF mapping) pass false.
13601369
protected string MapProperty(MetaObject owner, MetaField field, GenContext ctx, bool withAttributes = true)
13611370
{
13621371
var valueType = CSharpNaming.MapValueType(field);
13631372
var propName = PropertyName(field);
1373+
var required = CSharpNaming.IsRequired(owner, field);
13641374
var colAttr = withAttributes ? $" [Column({CSharpNaming.ColumnRef(owner, field, ctx.Config.ColumnNamingStrategy, ctx.Config.IncludeNames)})]\n" : "";
1365-
return $"{colAttr} public Dictionary<string, {valueType}> {propName} {{ get; set; }} = new();";
1375+
return required
1376+
? $"{colAttr} public Dictionary<string, {valueType}> {propName} {{ get; set; }} = new();"
1377+
: $"{colAttr} public Dictionary<string, {valueType}>? {propName} {{ get; set; }}";
13661378
}
13671379

13681380
// The CLR scalar type for a projection field whose value is a MIN/MAX aggregate:

0 commit comments

Comments
 (0)