Skip to content

Commit cf29990

Browse files
dmealingclaude
andcommitted
review: close the six surfaces a map newly REACHES now that codegen no longer throws
Every finding is a consequence of the same thing: while `SpringTypeMapper` threw on a `field.map`, no downstream surface could ever see one. Removing the throw made six paths reachable that had never been exercised, and each had been written as "ObjectField or nothing". 1. PAYLOAD (medium-high). `SpringPayloadGenerator.resolveFieldType` routed a map past the nested-payload arm into the plain type mapper, typing the component as the SOURCE value object's FQN — a type the payload path never emits. `nestedTargetOf` matched only `ObjectField`, so the target never entered the emission closure either. A `@payloadRef` VO carrying a map-of-VO therefore emitted a `<Name>Payload` naming a record nothing generated. Now routed through `resolveMapFieldType`, which emits the nested payload and returns `Map<String, <Target>Payload>`; `nestedTargetOf` agrees, as its javadoc requires. 2. C# PROJECTION (medium). The jsonb map config lived only in `EmitFieldTypeConfig`, which never runs for a read-only `object.projection` — but `EntityGenerator` emits the `Dictionary<string, V>` property for one anyway. A view exposing a map got a property with no mapping, which is the exact failure the mapping exists to prevent. The config is now a shared `MapJsonbConfig` called from both sites. 3. SORT ALLOWLIST (medium). Every map name landed in `SORT_ALLOWLIST`, so `?sort=labels:asc` passed and lowered to `ORDER BY` over a jsonb column — meaningless ordering on Postgres, 400 elsewhere, api-contract divergence. The filter side was already safe, but only because no operator band matches a map. 4. UNRESOLVED REF (low). `mapValueJavaType` promised null for an unresolvable `@objectRef` while `MetaDataUtil.getObjectRef` throws — and its own sibling `mapValueObjectRefOf` catches. One dangling ref gave a not-found from the mapper and a silent skip from the validation path. Now caught, as documented. 5. VALUE-OBJECT `@Valid` (low). The entity DTO's cascade was widened to span maps but the VO generator's was not, so nested constraints went unenforced exactly one level down. 6. PATCH (low). POST cascades into a map's value objects via `@Valid`; PATCH validates per-field with `validateValue`, which does not cascade. PATCH could persist a nested value object POST rejects. It now validates each map value. Each fix carries a test that fails without it, including two the old assertions could not have caught: the sort test also asserts a plain string STAYS sortable (so it cannot pass on an empty allowlist), and the projection test asserts the property is emitted before asserting the mapping is. The projection arm is compile-proven by adding a map to `DbContextCompileTests`' projection, since it is a different emitter from the entity one. Java 246 green on `clean test`; C# 442 codegen + 1024 conformance + 291 render + 77 cli green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013sjn9TaHoZFdDPF2dLcppv
1 parent 5a8c97e commit cf29990

8 files changed

Lines changed: 279 additions & 17 deletions

File tree

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
// HasConversion(ValueConverter, ValueComparer) overload, and a generic helper
3030
// returning ValueConverter<Dictionary<string,T>,string>. A string-contains test
3131
// cannot tell a real overload from a plausible-looking one.
32-
// - object.projection ProgramSummary (view-kind source, keyless) → .ToView(...).HasNoKey()
32+
// - object.projection ProgramSummary (view-kind source, keyless) → .ToView(...).HasNoKey(),
33+
// carrying a field.map so the PROJECTION arm of the map config compiles too — its emitter
34+
// is a separate call site from the entity one, and the property is emitted either way
3335
// - object.entity Invoice — a #214 WRITE-THROUGH entity (table invoices + replica view
3436
// v_invoice_with_client + a derived origin.passthrough clientName): the derived-free
3537
// write entity, the view-mapped InvoiceView read model (.ToView), the InvoiceView DbSet,
@@ -90,7 +92,8 @@ public class DbContextCompileTests
9092
{ "object.projection": { "name": "ProgramSummary", "children": [
9193
{ "source.rdb": { "@kind": "view", "@table": "v_program_summary" } },
9294
{ "field.long": { "name": "id" } },
93-
{ "field.int": { "name": "weekCount" } }
95+
{ "field.int": { "name": "weekCount" } },
96+
{ "field.map": { "name": "tallies", "@valueType": "int" } }
9497
]}},
9598
{ "object.entity": { "name": "Client", "children": [
9699
{ "source.rdb": { "@kind": "table", "@table": "clients" } },

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,45 @@ public void Map_jsonb_helper_is_emitted_only_when_a_map_is_present()
133133
Assert.DoesNotContain("MapJsonb", without);
134134
}
135135

136+
[Fact]
137+
public void A_read_only_projection_map_column_also_gets_its_jsonb_mapping()
138+
{
139+
// EntityGenerator emits the Dictionary property for a PROJECTION too, but the
140+
// DbContext's projection loop emits only ToView + enum conversions — so a view
141+
// exposing a field.map got a Dictionary property with no mapping at all. That is
142+
// exactly the failure this whole mapping exists to prevent: a Dictionary<string,int>
143+
// binds to nothing on Npgsql, so AppDbContext fails model building.
144+
const string model = """
145+
{ "metadata.root": { "package": "acme", "children": [
146+
{ "object.projection": { "name": "CustomerSummary", "children": [
147+
{ "source.rdb": { "@kind": "view", "@table": "v_customer_summary" } },
148+
{ "field.long": { "name": "id" } },
149+
{ "field.map": { "name": "tallies", "@valueType": "int" } }
150+
]}}
151+
]}}
152+
""";
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+
};
160+
161+
// The property is emitted...
162+
var entity = Assert.Single(new EntityGenerator().Generate(ctx)).Content;
163+
Assert.Contains("public Dictionary<string, int> Tallies { get; set; } = new();", entity);
164+
165+
// ...so the storage mapping must be too.
166+
var dbCtx = Assert.Single(new DbContextGenerator().Generate(ctx)).Content;
167+
Assert.Contains(
168+
"modelBuilder.Entity<CustomerSummary>().Property(x => x.Tallies).HasColumnType(\"jsonb\")"
169+
+ ".HasConversion(MapJsonb.Converter<int>(), MapJsonb.Comparer<int>());",
170+
dbCtx);
171+
// ...and the helper it names must be declared, or the generated file will not compile.
172+
Assert.Contains("private static class MapJsonb", dbCtx);
173+
}
174+
136175
[Fact]
137176
public void Generated_entities_and_value_objects_compile_together()
138177
{

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ public virtual IEnumerable<EmittedFile> Generate(GenContext ctx)
109109
// a string would fail materialization exactly as the ordinal default does here.
110110
foreach (var f in p.Fields().Where(f => f.SubType == FIELD_SUBTYPE_ENUM && !f.ResolvedIsArray()))
111111
modelLines.Add($" modelBuilder.Entity<{name}>().Property(x => x.{CSharpNaming.Pascal(f.Name)}).{EnumConversionCall(name, p, f, ctx.Config)};");
112+
// A field.map column on a view needs its jsonb mapping for the SAME reason:
113+
// EntityGenerator emits the Dictionary property for a projection too, and an
114+
// unmapped Dictionary<string,int> binds to nothing on Npgsql, so the model fails
115+
// to build. This loop and EmitFieldTypeConfig's are the two call sites
116+
// NeedsMapJsonbHelper has to cover.
117+
foreach (var f in p.Fields().Where(f => f.SubType == FIELD_SUBTYPE_MAP))
118+
modelLines.Add(MapJsonbConfig(name, f, ctx));
112119
}
113120
foreach (var e in objects.Where(o => o.IsEntity() && !o.IsReadOnlyProjection()))
114121
{
@@ -622,6 +629,20 @@ private static void EmitUnmappedEnumHelper(StringBuilder sb)
622629
sb.AppendLine(" \"@intValueMap — the database holds a value the model does not describe.\");");
623630
}
624631

632+
/// <summary>
633+
/// The EF config line pinning one <c>field.map</c> to its jsonb column: the column type
634+
/// plus the shared converter/comparer pair. Shared by the entity/write-through path
635+
/// (<see cref="EmitFieldTypeConfig"/>) and the read-only-projection path, which both emit
636+
/// a <c>Dictionary&lt;string, V&gt;</c> property and so both need the mapping.
637+
/// </summary>
638+
private static string MapJsonbConfig(string className, MetaField f, GenContext ctx)
639+
{
640+
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}>());";
644+
}
645+
625646
/// <summary>Name of the generated jsonb (de)serialization helper the field.map configs use.</summary>
626647
private const string MapJsonbHelperName = "MapJsonb";
627648

@@ -841,14 +862,7 @@ private void EmitFieldTypeConfig(
841862
// no reliance on Npgsql's dynamic-JSON opt-in the generated code cannot make for a
842863
// consumer. The COMPARER is not optional -- see EmitMapJsonbHelper.
843864
foreach (var f in fieldList.Where(f => f.SubType == FIELD_SUBTYPE_MAP))
844-
{
845-
var prop = CSharpNaming.Pascal(f.Name);
846-
var valueType = MapValueTypeRef(f, ctx);
847-
modelLines.Add(
848-
$" modelBuilder.Entity<{className}>().Property(x => x.{prop})"
849-
+ $".HasColumnType(\"jsonb\").HasConversion("
850-
+ $"{MapJsonbHelperName}.Converter<{valueType}>(), {MapJsonbHelperName}.Comparer<{valueType}>());");
851-
}
865+
modelLines.Add(MapJsonbConfig(className, f, ctx));
852866

853867
foreach (var f in fieldList.Where(f => f.SubType == FIELD_SUBTYPE_ENUM))
854868
{
@@ -1053,10 +1067,11 @@ private void EmitFieldTypeConfig(
10531067
// default: a column the migration never creates (42703 at the engine).
10541068
if (nf.SubType == FIELD_SUBTYPE_MAP)
10551069
{
1070+
var mapValue = MapValueTypeRef(nf, ctx);
10561071
sb.AppendLine($" b.Property(p => p.{CSharpNaming.Pascal(nf.Name)})"
10571072
+ $".HasColumnName(\"{nestedColAny}\").HasColumnType(\"jsonb\").HasConversion("
1058-
+ $"{MapJsonbHelperName}.Converter<{MapValueTypeRef(nf, ctx)}>(), "
1059-
+ $"{MapJsonbHelperName}.Comparer<{MapValueTypeRef(nf, ctx)}>());");
1073+
+ $"{MapJsonbHelperName}.Converter<{mapValue}>(), "
1074+
+ $"{MapJsonbHelperName}.Comparer<{mapValue}>());");
10601075
continue;
10611076
}
10621077

‎server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringControllerGenerator.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,14 @@ protected void emit(MetaObject entity, Path outRoot) {
168168
String pkType = SpringTypeMapper.primaryKeyJavaType(entity);
169169

170170
// Sort allowlist: every scalar field is sortable. Skip ObjectField (no SQL column
171-
// surface today; @storage controls a separate column shape).
171+
// surface today; @storage controls a separate column shape) and MapField: a map is one
172+
// jsonb column holding a JSON object, so ORDER BY over it sorts by Postgres' jsonb
173+
// collation — meaningless as an ordering, and a 400 on the ports that reject it, which
174+
// makes the api-contract diverge. The filter side already excludes maps by having no
175+
// operator band for them.
172176
List<String> sortFields = new ArrayList<>();
173177
for (MetaField field : entity.getMetaFields()) {
174-
if (field instanceof ObjectField) continue;
178+
if (field instanceof ObjectField || field instanceof com.metaobjects.field.MapField) continue;
175179
sortFields.add(field.getName());
176180
}
177181

@@ -426,6 +430,24 @@ private static void appendValueObjectValidation(StringBuilder src, MetaObject en
426430
src.append(" }\n");
427431
}
428432
}
433+
// A field.map @objectRef carries the same nested value objects, and the POST path now
434+
// cascades into them via @Valid on the DTO component. PATCH validates field-by-field
435+
// with validateValue, which does NOT cascade — so without this the two write paths
436+
// disagree for maps where they agree for field.object, and a PATCH could write a
437+
// nested VO that POST rejects. Validate each map VALUE (keys are plain strings).
438+
for (MetaField vf : entity.getMetaFields()) {
439+
if (SpringDtoGenerator.mapValueObjectRefOf(vf) == null) continue;
440+
String name = vf.getName();
441+
String cap = SpringNaming.capitalize(name);
442+
src.append(" if (patch.has").append(cap).append("() && patch.")
443+
.append(name).append("() != null) {\n");
444+
src.append(" for (var __e : patch.").append(name).append("().values()) {\n");
445+
src.append(" if (__e != null && !validator.validate(__e).isEmpty()) {\n");
446+
src.append(" return ResponseEntity.badRequest().body(Map.of(\"error\", \"validation\"));\n");
447+
src.append(" }\n");
448+
src.append(" }\n");
449+
src.append(" }\n");
450+
}
429451
}
430452

431453
/**

‎server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringPayloadGenerator.java‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,10 @@ public static MetaObject nestedTargetOf(MetaField<?> field, MetaDataLoader loade
267267
if (target == null || !MetaObject.SUBTYPE_VALUE.equals(target.getSubType())) return null;
268268
return target;
269269
}
270-
return null;
270+
// A field.map @objectRef carries a nested payload target exactly as field.object does
271+
// (see resolveMapFieldType). This walk and the emission walk MUST agree on the target
272+
// set — a target the closure misses is a record the name map never names.
273+
return SpringDtoGenerator.mapValueObjectRefOf(field);
271274
}
272275

273276
/**
@@ -477,6 +480,13 @@ public String resolveFieldType(MetaField<?> field,
477480
if (field instanceof ObjectField of) {
478481
return resolveObjectFieldType(of, loader, nestedPkg, outRoot, emittedNestedFqns, nameMap);
479482
}
483+
// A field.map @objectRef must take the SAME nested-payload path a field.object does.
484+
// Falling through to javaTypeName would type the component as the SOURCE value
485+
// object's FQN — a type the payload path never emits — so the generated
486+
// <Name>Payload would name a record that does not exist.
487+
if (field instanceof com.metaobjects.field.MapField mf) {
488+
return resolveMapFieldType(mf, loader, nestedPkg, outRoot, emittedNestedFqns, nameMap);
489+
}
480490
// Scalar array (`isArray: true` on a non-object, non-enum field): the declared
481491
// contract is field.<subType> + @isArray, so wrap the element type as
482492
// java.util.List<ElementType> — matching Kotlin's scalar-array arm and Python's
@@ -517,6 +527,32 @@ protected String resolveObjectFieldType(ObjectField field,
517527
return emitNestedAndReturnType(target, loader, nestedPkg, outRoot, emittedNestedFqns, field.isArrayType(), nameMap);
518528
}
519529

530+
/**
531+
* Declared {@code field.map @objectRef}: recursively emit
532+
* {@code <CapitalizedTargetShortName>Payload} for the referenced value object and return
533+
* {@code java.util.Map<String, TargetPayload>}. The map analog of
534+
* {@link #resolveObjectFieldType} — a payload component must name a PAYLOAD record, never
535+
* the source value object, and the target has to enter the emission closure or nothing
536+
* generates it.
537+
*
538+
* <p>A scalar-valued map ({@code @valueType}) references no value object, so it falls
539+
* through to the plain type mapper ({@code Map<String, Scalar>}). {@code isArray} does not
540+
* apply to a map, so there is no list-wrapping arm here.</p>
541+
*/
542+
protected String resolveMapFieldType(com.metaobjects.field.MapField field,
543+
MetaDataLoader loader,
544+
String nestedPkg,
545+
Path outRoot,
546+
Set<String> emittedNestedFqns,
547+
Map<String, String> nameMap) {
548+
MetaObject target = SpringDtoGenerator.mapValueObjectRefOf(field);
549+
if (target == null) return SpringTypeMapper.javaTypeName(field);
550+
// asList:false — the bare nested record name, which we wrap as the map VALUE.
551+
String nested = emitNestedAndReturnType(
552+
target, loader, nestedPkg, outRoot, emittedNestedFqns, false, nameMap);
553+
return "java.util.Map<String, " + nested + ">";
554+
}
555+
520556
/**
521557
* Nested-payload emit path used by {@link #resolveObjectFieldType} (single or
522558
* list, depending on {@code asList}). Emits the record at most once per run via

‎server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringTypeMapper.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,18 @@ private static String mapValueJavaType(MapField field) {
181181
// ADR-0039: @objectRef / @valueType are EFFECTIVE properties -- hasMetaAttr/getMetaAttr
182182
// (the one-arg, RESOLVING forms) so a value inherited via `extends` is not dropped.
183183
if (field.hasMetaAttr(MapField.ATTR_OBJECTREF)) {
184-
MetaObject ref = field.getObjectRef();
184+
MetaObject ref;
185+
try {
186+
ref = field.getObjectRef();
187+
} catch (RuntimeException unresolved) {
188+
// MetaDataUtil.getObjectRef THROWS MetaDataNotFoundException on a dangling ref
189+
// rather than returning null. Caught so this method actually keeps the contract
190+
// its javadoc states, and so it agrees with its sibling
191+
// SpringDtoGenerator.mapValueObjectRefOf — otherwise one dangling ref produces
192+
// a not-found from the type mapper and a silent skip from the @Valid /
193+
// value-object-reachability path.
194+
return null;
195+
}
185196
if (ref == null) return null;
186197
String[] split = SpringNaming.splitFqn(ref.getName());
187198
return split[0].isEmpty() ? split[1] : split[0] + "." + split[1];

‎server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringValueObjectGenerator.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ private void emit(MetaObject vo, Path outRoot) {
104104
boolean usesValid = false;
105105
for (MetaField field : fields) {
106106
String a = SpringDtoGenerator.validationAnnotations(field);
107-
if (SpringDtoGenerator.isValueObjectJsonbField(field)) {
107+
// valueObjectRefOf, not isValueObjectJsonbField: it spans BOTH shapes that carry a
108+
// value object (field.object @objectRef and field.map @objectRef). The entity DTO
109+
// cascades into a map of value objects; a VO nesting the same shape must too, or
110+
// the nested constraints go unenforced one level down.
111+
if (SpringDtoGenerator.valueObjectRefOf(field) != null) {
108112
a = a.isEmpty() ? "@Valid" : "@Valid " + a;
109113
usesValid = true;
110114
}

0 commit comments

Comments
 (0)