Skip to content

Commit da00178

Browse files
dmealingclaude
dmealing
and
claude
committed
feat(codegen): FR-017 TPH — FR-009 filtering on the polymorphic + per-subtype lists (Java + Kotlin)
Follow-up to the TPH api-contract lanes (review finding): the reference Python/TS TPH routers wire the FR-009 filter pipeline into both the polymorphic and per-subtype list handlers, but the Java + Kotlin generators emitted sort-only TPH controllers — a real cross-port capability gap. Both now filter, matching the reference (and retiring the previously-dead Java AuthFilterAllowlist). - Allowlist (Java + Kotlin) unions the base + subtype-only filterable columns for a TPH base, but EXCLUDES the discriminator (addressable via the per-subtype route segment) and decimal columns (decimal wire formatting differs per port → no portable operand; also dodges the generic pipeline's enum/BigDecimal coercion limits). Java + Kotlin filter the same surface. - Java: TPH repository gains list/count/listByType filter params; the controller parses filter[field][op]=v against the union allowlist and threads predicates (polymorphic + per-subtype). - Kotlin: the self-contained controller emits the FR-009 parse+Op-dispatch pipeline over the union columns and AND's it with the per-subtype discriminator scope. Also from the review: - Java union DTO drops now-inert bean-validation (the controller omits @Valid) → an all-nullable partial-PATCH-friendly wire body, matching the Kotlin union data class. - Kotlin emitTph fails loud at generation if a @discriminatorValue is not a valid Kotlin enum-constant identifier (the controller references it as <Enum>.<Value>). Tested by new local filter smokes (the shared corpus is sort-only): Java TPH 5/0, Kotlin TPH 5/0. No regression: codegen-spring 144/0, codegen-kotlin green, Java + Kotlin vanilla Author/m2m lanes green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 09ae8d5 commit da00178

9 files changed

Lines changed: 270 additions & 32 deletions

File tree

‎server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinFilterAllowlistGenerator.kt‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,26 @@ open class KotlinFilterAllowlistGenerator : MultiFileDirectGeneratorBase<MetaObj
5959
// table-only). View / materializedView are read-only; storedProc has its
6060
// own dispatch; tableFunction has no controller surface today.
6161
if (sourceRdb.effectiveKind != MetaSource.KIND_TABLE) continue
62-
emit(entity, outRoot)
62+
emit(entity, outRoot, loader)
6363
}
6464
}
6565

66-
protected open fun emit(entity: MetaObject, outRoot: Path) {
66+
protected open fun emit(entity: MetaObject, outRoot: Path, loader: MetaDataLoader) {
6767
val (pkg, shortName) = PackageMapping.splitFqn(entity.name)
6868
val className = "${shortName}FilterAllowlist"
6969

70-
val opsByField = computeFilterableOps(entity)
70+
// FR-017 TPH: a discriminator base's allowlist unions its subtype-only filterable columns,
71+
// but EXCLUDES (a) the discriminator — addressable via the per-subtype route segment, and
72+
// (b) decimal columns — outside the cross-port HTTP filter/value contract. Keeps the Java +
73+
// Kotlin filter surface identical (see the Java SpringFilterAllowlistGenerator).
74+
val opsByField = if (KotlinTphPlan.isTphBase(entity, loader)) {
75+
val disc = KotlinTphPlan.planFor(entity, loader)!!.discriminatorField
76+
val union = (entity.metaFields + KotlinTphPlan.collectSubtypeFields(entity, loader))
77+
.filter { it.name != disc && it !is com.metaobjects.field.DecimalField }
78+
computeFilterableOps(union)
79+
} else {
80+
computeFilterableOps(entity)
81+
}
7182

7283
val src = buildString {
7384
if (pkg.isNotEmpty()) {
@@ -134,14 +145,22 @@ open class KotlinFilterAllowlistGenerator : MultiFileDirectGeneratorBase<MetaObj
134145
* collapse to the empty op-set (defensive — the allowlist becomes an effective
135146
* "field is unknown" gate).
136147
*/
137-
fun computeFilterableOps(entity: MetaObject): Map<String, Set<String>> {
148+
fun computeFilterableOps(entity: MetaObject): Map<String, Set<String>> =
149+
computeFilterableOps(entity.metaFields)
150+
151+
/**
152+
* [computeFilterableOps] over an explicit field list — used for a TPH base, whose allowlist
153+
* is the UNION of the base's own + every subtype-only filterable column (so the polymorphic
154+
* + per-subtype lists can filter on subtype columns, matching the Java/Python/TS lanes).
155+
*/
156+
fun computeFilterableOps(fields: Iterable<MetaField<*>>): Map<String, Set<String>> {
138157
val out = linkedMapOf<String, Set<String>>()
139-
for (field in entity.metaFields) {
158+
for (field in fields) {
140159
if (field is ObjectField) continue
141160
if (!isFilterable(field)) continue
142161
val ops = opsForSubtype(field.subType)
143162
if (ops.isEmpty()) continue
144-
out[field.name] = ops
163+
out.putIfAbsent(field.name, ops) // dedup base/subtype column names
145164
}
146165
return out
147166
}

‎server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,37 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
392392
val discField = base.metaFields.first { it.name == plan.discriminatorField }
393393
val discEnum = KotlinTypeMapper.enumTypeName(discField, base)?.simpleName
394394
?: error("TPH base ${base.name}: discriminator field '${plan.discriminatorField}' is not an enum")
395+
// The controller scopes/injects the discriminator as `<Enum>.<Value>` (an enum constant), so
396+
// every @discriminatorValue MUST be a valid Kotlin identifier — fail loud at generation with a
397+
// clear message rather than emit code that won't compile (the Java lane uses string literals
398+
// and has no such constraint).
399+
for (st in plan.subtypes) {
400+
require(st.value.matches(Regex("[A-Za-z_][A-Za-z0-9_]*"))) {
401+
"FR-017 TPH base ${base.name}: @discriminatorValue '${st.value}' on ${st.entity.name} is not a valid " +
402+
"Kotlin enum-constant identifier; the generated $discEnum.${st.value} reference would not compile."
403+
}
404+
}
395405

396406
// Union scalar fields (base own + subtype-only), in the data class / table order.
397407
val scalarFields = (base.metaFields.filterNot { it is ObjectField } +
398408
KotlinTphPlan.collectSubtypeFields(base, plan).filterNot { it is ObjectField })
399409
val sortFields = base.metaFields.filterNot { it is ObjectField }.map { it.name }
400410
val baseFieldNames = base.metaFields.map { it.name }.toSet()
411+
val allowlistName = "${shortName}FilterAllowlist"
412+
// Union filter-dispatch specs (base + subtype columns) for the FR-009 pipeline — EXCLUDING the
413+
// discriminator (route-addressable; enum column) and decimal columns (outside the cross-port
414+
// HTTP filter contract). Mirrors the allowlist's exclusions, so the dispatch never references
415+
// an enum/BigDecimal cast the generic pipeline can't coerce.
416+
val filterSpecs = scalarFields
417+
.filter { it.name != plan.discriminatorField && it !is com.metaobjects.field.DecimalField }
418+
.map { ScalarFieldSpec(it.name, it.subType, columnElementType(it)) }
401419
// Non-discriminator, non-PK columns the create/update handlers write from the body.
402420
val writableFields = scalarFields.map { it.name }
403421
.filter { it != plan.discriminatorField && it != "id" }
404422

405423
val src = buildString {
406424
if (pkg.isNotEmpty()) append("package $pkg\n\n")
425+
append("import org.jetbrains.exposed.sql.Op\n")
407426
append("import org.jetbrains.exposed.sql.ResultRow\n")
408427
append("import org.jetbrains.exposed.sql.SortOrder\n")
409428
append("import org.jetbrains.exposed.sql.SqlExpressionBuilder\n")
@@ -423,7 +442,15 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
423442
append("import org.springframework.web.bind.annotation.RequestMapping\n")
424443
append("import org.springframework.web.bind.annotation.RequestMethod\n")
425444
append("import org.springframework.web.bind.annotation.RequestParam\n")
426-
append("import org.springframework.web.bind.annotation.RestController\n\n")
445+
append("import org.springframework.web.bind.annotation.RestController\n")
446+
// FR-009 filter pipeline support (mirrors the vanilla controller's import set).
447+
append("import java.net.URLDecoder\n")
448+
append("import java.nio.charset.StandardCharsets\n")
449+
append("import java.sql.Timestamp\n")
450+
append("import java.time.LocalDate\n")
451+
append("import java.time.LocalDateTime\n")
452+
append("import java.time.LocalTime\n")
453+
append("import java.time.format.DateTimeFormatter\n\n")
427454

428455
// sort allowlist (base scalar columns — the polymorphic sort surface)
429456
append("/** GENERATED — sort allowlist for $shortName (cross-port API contract). */\n")
@@ -442,6 +469,10 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
442469
append(" return field to dir\n")
443470
append("}\n\n")
444471

472+
// FR-009 filter pipeline over the UNION columns (parse + per-field Exposed Op dispatch),
473+
// shared by the polymorphic + per-subtype list handlers — matches the Python/TS TPH lanes.
474+
emitFilterPipeline(this, shortName, table, allowlistName, filterSpecs)
475+
445476
// rowTo<Base>: union ResultRow → data class
446477
append("/** GENERATED — map an Exposed ResultRow to the union $shortName data class. */\n")
447478
append("private fun rowTo${shortName}(row: ResultRow): $shortName = $shortName(\n")
@@ -460,8 +491,12 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
460491
append(" @RequestParam(required = false) offset: Int?,\n")
461492
append(" @RequestParam(required = false) sort: String?,\n")
462493
append(" @RequestParam(required = false, name = \"withCount\") withCount: Int?,\n")
494+
append(" @RequestParam allParams: Map<String, String>,\n")
463495
append(" ): ResponseEntity<Any> = transaction {\n")
464-
append(" var q = $table.selectAll()\n")
496+
append(" val filterResult = parse${shortName}Filter(allParams)\n")
497+
append(" if (filterResult.error != null) return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to filterResult.error) as Any)\n")
498+
append(" val whereOp = ${shortName}WhereOp(filterResult.predicates)\n")
499+
append(" var q = if (whereOp != null) $table.selectAll().where { whereOp } else $table.selectAll()\n")
465500
append(" if (sort != null) {\n")
466501
append(" val parsed = parse${shortName}Sort(sort)\n")
467502
append(" ?: return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to \"invalid_sort\") as Any)\n")
@@ -489,14 +524,21 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
489524

490525
append(" // --- subtype ${st.value} (segment /$seg) ---\n")
491526

492-
// per-subtype list
527+
// per-subtype list (discriminator scope AND'd with the FR-009 filter)
493528
append(" @GetMapping(\"/$seg\")\n")
494529
append(" fun list$sfx(\n")
495530
append(" @RequestParam(required = false) limit: Int?,\n")
496531
append(" @RequestParam(required = false) offset: Int?,\n")
497532
append(" @RequestParam(required = false) sort: String?,\n")
533+
append(" @RequestParam allParams: Map<String, String>,\n")
498534
append(" ): ResponseEntity<Any> = transaction {\n")
499-
append(" var q = $table.selectAll().where { $table.${plan.discriminatorField} eq $disc }\n")
535+
append(" val filterResult = parse${shortName}Filter(allParams)\n")
536+
append(" if (filterResult.error != null) return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to filterResult.error) as Any)\n")
537+
append(" val whereOp = ${shortName}WhereOp(filterResult.predicates)\n")
538+
append(" var q = $table.selectAll().where {\n")
539+
append(" val d = $table.${plan.discriminatorField} eq $disc\n")
540+
append(" if (whereOp != null) d and whereOp else d\n")
541+
append(" }\n")
500542
append(" if (sort != null) {\n")
501543
append(" val parsed = parse${shortName}Sort(sort)\n")
502544
append(" ?: return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to \"invalid_sort\") as Any)\n")

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ protected void emitTph(MetaObject base, TphPlan.Plan plan, Path outRoot) {
351351
String repoName = SpringNaming.repositoryName(shortName);
352352
String controllerName = SpringNaming.controllerName(shortName);
353353
String routeBase = SpringNaming.controllerPath(shortName);
354+
String allowlistName = SpringNaming.filterAllowlistName(shortName);
354355

355356
// Sort allowlist: the base's own scalar columns (the polymorphic sort surface). Subtype
356357
// columns are not sortable across the polymorphic collection.
@@ -373,6 +374,10 @@ protected void emitTph(MetaObject base, TphPlan.Plan plan, Path outRoot) {
373374
src.append("import org.springframework.web.bind.annotation.RequestMethod;\n");
374375
src.append("import org.springframework.web.bind.annotation.RequestParam;\n");
375376
src.append("import org.springframework.web.bind.annotation.RestController;\n");
377+
src.append("import com.metaobjects.generator.spring.runtime.FilterParseResult;\n");
378+
src.append("import com.metaobjects.generator.spring.runtime.FilterParser;\n");
379+
src.append("import com.metaobjects.generator.spring.runtime.FilterPredicate;\n");
380+
src.append("import jakarta.servlet.http.HttpServletRequest;\n");
376381
src.append("import java.util.List;\n");
377382
src.append("import java.util.Map;\n");
378383
src.append("import java.util.Set;\n\n");
@@ -402,17 +407,22 @@ protected void emitTph(MetaObject base, TphPlan.Plan plan, Path outRoot) {
402407
src.append(" @RequestParam(required = false) Integer limit,\n");
403408
src.append(" @RequestParam(required = false) Integer offset,\n");
404409
src.append(" @RequestParam(required = false) String sort,\n");
405-
src.append(" @RequestParam(required = false, name = \"withCount\") Integer withCount) {\n");
410+
src.append(" @RequestParam(required = false, name = \"withCount\") Integer withCount,\n");
411+
src.append(" HttpServletRequest request) {\n");
406412
src.append(" int actualLimit = limit != null ? limit : 50;\n");
407413
src.append(" int actualOffset = offset != null ? offset : 0;\n");
408414
src.append(" ").append(repoName).append(".SortClause sortClause = null;\n");
409415
src.append(" if (sort != null) {\n");
410416
src.append(" sortClause = parseSort(sort);\n");
411417
src.append(" if (sortClause == null) return ResponseEntity.badRequest().body(Map.of(\"error\", \"invalid_sort\"));\n");
412418
src.append(" }\n");
413-
src.append(" List<").append(dtoName).append("> rows = repository.list(actualLimit, actualOffset, sortClause);\n");
419+
src.append(" FilterParseResult filter = FilterParser.parse(request.getQueryString(), ")
420+
.append(allowlistName).append(".FIELDS, ").append(allowlistName).append(".OPS_BY_FIELD);\n");
421+
src.append(" if (filter.error() != null) return ResponseEntity.badRequest().body(Map.of(\"error\", filter.error()));\n");
422+
src.append(" List<FilterPredicate> filters = filter.predicates();\n");
423+
src.append(" List<").append(dtoName).append("> rows = repository.list(actualLimit, actualOffset, sortClause, filters);\n");
414424
src.append(" if (withCount != null && withCount == 1) {\n");
415-
src.append(" return ResponseEntity.ok(Map.of(\"rows\", rows, \"total\", repository.count()));\n");
425+
src.append(" return ResponseEntity.ok(Map.of(\"rows\", rows, \"total\", repository.count(filters)));\n");
416426
src.append(" }\n");
417427
src.append(" return ResponseEntity.ok(rows);\n");
418428
src.append(" }\n\n");
@@ -438,16 +448,20 @@ protected void emitTph(MetaObject base, TphPlan.Plan plan, Path outRoot) {
438448
src.append(" public ResponseEntity<?> list").append(suffix).append("(\n");
439449
src.append(" @RequestParam(required = false) Integer limit,\n");
440450
src.append(" @RequestParam(required = false) Integer offset,\n");
441-
src.append(" @RequestParam(required = false) String sort) {\n");
451+
src.append(" @RequestParam(required = false) String sort,\n");
452+
src.append(" HttpServletRequest request) {\n");
442453
src.append(" int actualLimit = limit != null ? limit : 50;\n");
443454
src.append(" int actualOffset = offset != null ? offset : 0;\n");
444455
src.append(" ").append(repoName).append(".SortClause sortClause = null;\n");
445456
src.append(" if (sort != null) {\n");
446457
src.append(" sortClause = parseSort(sort);\n");
447458
src.append(" if (sortClause == null) return ResponseEntity.badRequest().body(Map.of(\"error\", \"invalid_sort\"));\n");
448459
src.append(" }\n");
460+
src.append(" FilterParseResult filter = FilterParser.parse(request.getQueryString(), ")
461+
.append(allowlistName).append(".FIELDS, ").append(allowlistName).append(".OPS_BY_FIELD);\n");
462+
src.append(" if (filter.error() != null) return ResponseEntity.badRequest().body(Map.of(\"error\", filter.error()));\n");
449463
src.append(" return ResponseEntity.ok(repository.listByType(\"").append(disc)
450-
.append("\", actualLimit, actualOffset, sortClause));\n");
464+
.append("\", actualLimit, actualOffset, sortClause, filter.predicates()));\n");
451465
src.append(" }\n\n");
452466

453467
// per-subtype get (404 cross-subtype)

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,17 @@ protected void emit(MetaObject entity, Path outRoot) {
107107
*/
108108
protected void emitTphUnion(MetaObject base, MetaDataLoader loader, Path outRoot) {
109109
List<MetaField> fields = new ArrayList<>(scalarFields(base));
110-
List<String> annotationsPerField = new ArrayList<>(fields.size());
111-
for (MetaField field : fields) annotationsPerField.add(validationAnnotations(field));
112110
for (MetaField field : TphPlan.collectSubtypeFields(base, loader)) {
113111
if (field instanceof ObjectField) continue;
114112
fields.add(field);
115-
annotationsPerField.add(""); // nullable union column — no validation
116113
}
114+
// The TPH union DTO is a partial-PATCH-friendly wire body — every component is a nullable
115+
// wrapper and carries NO bean-validation (a per-subtype POST/PATCH supplies only its own
116+
// columns; the single table's column nullability is the real constraint). The controller
117+
// intentionally omits @Valid, so keeping @NotNull here would be inert + misleading. Matches
118+
// the Kotlin lane's all-nullable union data class.
119+
List<String> annotationsPerField = new ArrayList<>(fields.size());
120+
for (int i = 0; i < fields.size(); i++) annotationsPerField.add("");
117121
emitRecord(base, outRoot, fields, annotationsPerField);
118122
}
119123

0 commit comments

Comments
 (0)