Skip to content

Commit 0fb7774

Browse files
dmealingclaude
andcommitted
feat(fr-035): present-key PATCH tristate — Kotlin port (completes all 5 ports)
Green the cross-port `update-explicit-null-clears` gate on Kotlin (both lanes), completing FR-035 Part B across TS / Python / C# / Java / Kotlin. The generated controller bound `@Valid @RequestBody dto: <Entity>` (a non-null-ctor data class), so a partial PATCH omitting a @required field 400'd at Jackson binding and an explicit null was a silent no-op. Rebind the update handler off the typed data class onto the raw JSON tree — minimal inline, NOT the repo-delegation refactor: - codegen-kotlin: the non-TPH `<Entity>Controller` constructor-injects an `ObjectMapper` (only when it has settable scalar fields — a PK+object/jsonb-only entity keeps a no-arg controller, so no unused-param `-Werror` break) and the update handler binds `@RequestBody body: JsonNode`. Per body key: a non-object body → 400; an explicit null on a @required field → 400; a present value binds through the configured `ObjectMapper` (`treeToValue` with a `TypeReference` so boxing / generics / inline enums survive) and per-field-dispatches into the inline Exposed `update {}` (required → typed local; nullable → `if (n.isNull) it[Table.col]=null else …`); an absent key is untouched; an effectively-empty patch is a no-op read-back. Columns are qualified `it[Table.col]` (a field named like the `body` param would otherwise shadow the column). A value whose JSON shape can't bind → 400 (JsonMappingException), matching create, not a 500. - reference server + KNOWN_GAPS + `PatchTristateEdgeCasesTest` (no-settable-field controller emits no ObjectMapper/TypeReference; `PATCH {"name":{…}}` → 400). Known gaps (KNOWN_GAPS.md, cross-port PATCH-4 follow-ups): (1) present-VALUE constraint validation (@Size/@Pattern/@notblank) is not run on PATCH values (only null-on-@required → 400) — matches C#/Java; TS/Python enforce it. (2) a `field.string @dbColumnType:jsonb` open bag is a create-only column (kotlinx JsonElement can't bind on the raw-JsonNode PATCH path without tripping the #179 open-bag guard) — PATCH leaves it untouched. Gate green both Kotlin lanes (generated 23/0; reference 23/0 — the reference lane occasionally flakes on PG-testcontainer readiness under load, a known environmental issue, not a code failure); codegen-kotlin 48 reports / 0 failures; jsonb 1/0 both, m2m 3/0 both, tph generated 5/0, enum/uuid run-tests green. Generated Kotlin compiles under allWarningsAsErrors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGQ7oSuNcjhsMHWwZzhBwr
1 parent 25bc2d2 commit 0fb7774

11 files changed

Lines changed: 302 additions & 46 deletions

File tree

server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KNOWN_GAPS.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,25 @@ byte-identical to the Java port by construction.
4646
`metaobjects-om` (which transitively brings `render` + `metadata`) and
4747
`com.metaobjects.loader.MetaDataLoader`. Consumers wanting nested extraction must have
4848
`metaobjects-om` on the classpath.
49+
50+
## FR-035 partial-PATCH (present-key tristate)
51+
52+
The generated `@RestController`'s PATCH/PUT handler binds the raw `JsonNode` and
53+
per-field-binds present values via the Spring `ObjectMapper` (absent → untouched;
54+
explicit null → clears a nullable column or 400 on a `@required` field; present
55+
value → set). Two deliberate gaps:
56+
57+
- **Present-value constraint validation** is NOT run on PATCH. Dropping `@Valid`
58+
means `@Size` / `@Pattern` / `@NotBlank` / `@Min` / `@Max` (emitted by
59+
`validationAnnotations`) no longer fire on a PATCH body's present values — only a
60+
present-null-on-`@required` → 400. This matches the C#/Java partial-PATCH state; TS
61+
and Python DO validate present values on PATCH. A cross-port PATCH-4 follow-up.
62+
- **`field.string @dbColumnType=jsonb` open-bag** is a **create-only** column on the
63+
generated CRUD. The generated `create` writes it (bound from the `@Valid` DTO's
64+
kotlinx `JsonElement` property — exercised by the `jsonb-open-bag-roundtrip` corpus),
65+
but the generated `update` does NOT: the raw-JsonNode patch path cannot bind a kotlinx
66+
`JsonElement` via Jackson `treeToValue`, and the type name must not surface
67+
un-imported in the controller (the #179 filter/sort guard). So a `PATCH` leaves an
68+
open-bag column untouched. A consumer needing to PATCH an open-bag column overrides
69+
the generated update handler. Typed value-object jsonb (`field.object`) is
70+
object-typed and separately out of the CRUD DTO scope.

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

Lines changed: 82 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,19 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
151151
val pkFieldName = primary?.fields?.firstOrNull() ?: DEFAULT_PK_FIELD
152152
val pkParamType = primaryKeyParamType(entity, pkFieldName)
153153

154+
// FR-035: the PATCH-settable columns = scalar fields minus the PK. ObjectField / MapField
155+
// (jsonb value-objects) and the `field.string @dbColumnType=jsonb` open bag are excluded:
156+
// their in-process type is a kotlinx JsonElement / VO with no clean Jackson treeToValue
157+
// bind on the raw-JsonNode patch path. (create DOES write the open bag from the DTO;
158+
// an open bag is thus a create-only CRUD column — see KNOWN_GAPS.)
159+
// When this is EMPTY (a PK + only object/map/jsonb columns) the controller emits no
160+
// ObjectMapper ctor param / TypeReference import (they'd be unused → allWarningsAsErrors).
161+
val patchSettableFields = entity.metaFields.filter {
162+
it !is ObjectField && it !is MapField && it.name != pkFieldName &&
163+
!KotlinTypeMapper.isJsonbOpenBag(it)
164+
}
165+
val hasPatchFields = patchSettableFields.isNotEmpty()
166+
154167
// Sort allowlist: every scalar field is sortable. Skip ObjectField (no SQL column
155168
// surface on the Exposed Table; @storage controls a separate column shape) and the
156169
// `field.string @dbColumnType=jsonb` open bag — a JSONB value is not a scalar sort
@@ -190,6 +203,14 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
190203
append("import org.jetbrains.exposed.sql.selectAll\n")
191204
append("import org.jetbrains.exposed.sql.update\n")
192205
append("import org.jetbrains.exposed.sql.transactions.transaction\n")
206+
append("import com.fasterxml.jackson.databind.JsonNode\n")
207+
// ObjectMapper + TypeReference are used ONLY by the per-field patch bind, which is
208+
// emitted only when there ARE settable fields — omit them otherwise (unused import /
209+
// ctor param would fail the module's allWarningsAsErrors compile).
210+
if (hasPatchFields) {
211+
append("import com.fasterxml.jackson.core.type.TypeReference\n")
212+
append("import com.fasterxml.jackson.databind.ObjectMapper\n")
213+
}
193214
append("import jakarta.validation.Valid\n")
194215
append("import org.springframework.http.HttpStatus\n")
195216
append("import org.springframework.http.ResponseEntity\n")
@@ -284,7 +305,15 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
284305
append("/** GENERATED — REST controller for ${shortName} entity. Implements the cross-port API contract. */\n")
285306
append("@RestController\n")
286307
append("@RequestMapping(\"$routeBase\")\n")
287-
append("class ${shortName}Controller {\n\n")
308+
// FR-035 present-key PATCH: the update handler binds the RAW JsonNode (not the
309+
// @Valid data class, which cannot see absent-vs-null) and per-field-binds present
310+
// values via the Spring-configured ObjectMapper — injected only when there ARE
311+
// settable fields (a PK + only object/jsonb columns needs no mapper).
312+
if (hasPatchFields) {
313+
append("class ${shortName}Controller(private val objectMapper: ObjectMapper) {\n\n")
314+
} else {
315+
append("class ${shortName}Controller {\n\n")
316+
}
288317

289318
// List handler — pagination + sort + withCount + FR-009 filter operators.
290319
//
@@ -341,6 +370,11 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
341370
// If the entity has no auto-incrementing PK the consumer can override the
342371
// generated handler; this is the 95% case.
343372
if (field.name == pkFieldName) continue
373+
// NOTE: a `field.string @dbColumnType=jsonb` open bag IS written here (create
374+
// binds it from the @Valid DTO's kotlinx JsonElement property, gated by the
375+
// jsonb-open-bag-roundtrip corpus). PATCH cannot (the raw-JsonNode path can't bind
376+
// a JsonElement without surfacing the un-imported type the #179 guard forbids), so
377+
// an open bag is a create-only column on the generated CRUD — see KNOWN_GAPS.
344378
append(" it[${field.name}] = dto.${field.name}\n")
345379
}
346380
append(" }[${tableObjectName}.${pkFieldName}]\n")
@@ -353,31 +387,57 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase<MetaOb
353387
// Stacking @PatchMapping + @PutMapping on the same method does NOT register both
354388
// in Spring MVC — only one composed @RequestMapping per method is honored, so the
355389
// other verb 405s. (Surfaced by the SP-F generated-controller HTTP lane.)
390+
// FR-035 present-key tristate: bind the RAW JsonNode so absent-vs-null is visible.
391+
// A non-object body, or an explicit null on a @required field, is a 400; an explicit
392+
// null on a nullable field CLEARS it; an omitted field is untouched; a present value
393+
// binds through the configured ObjectMapper (same codecs as create). An empty
394+
// effective patch is a no-op read-back (avoids Exposed's empty-SET error).
395+
// Settable columns are `patchSettableFields` (computed up top): scalar fields minus
396+
// the PK, EXCLUDING object/map/jsonb-open-bag (an open bag is a create-only CRUD
397+
// column — PATCH can't bind a kotlinx JsonElement; see KNOWN_GAPS). When empty, no
398+
// update{} block is emitted (an empty Exposed SET throws) — a no-op read-back.
356399
append(" @RequestMapping(value = [\"/{id}\"], method = [RequestMethod.PATCH, RequestMethod.PUT])\n")
357-
append(" fun update(@PathVariable id: $pkParamType, @Valid @RequestBody dto: ${shortName}): ResponseEntity<Any> = transaction {\n")
358-
append(" val updated = ${tableObjectName}.update({ ${tableObjectName}.${pkFieldName} eq id }) {\n")
359-
for (field in entity.metaFields) {
360-
if (field is ObjectField || field is MapField) continue
361-
if (field.name == pkFieldName) continue
362-
// Partial merge: an OPTIONAL field omitted from a PATCH must not clobber the
363-
// stored value. Optional DTO props are nullable + default to null, so absent
364-
// reads as null here — null-guard them (the same merge the TPH per-subtype
365-
// handler uses). Required props are non-null (Jackson rejects a body missing
366-
// them with a 400 before this runs), so they are always present → written
367-
// unconditionally. Guarding a non-null prop would be an always-true warning
368-
// and break -Werror consumers. (Explicit-null-clears is the deferred tristate.)
400+
append(" fun update(@PathVariable id: $pkParamType, @RequestBody body: JsonNode): ResponseEntity<Any> = transaction {\n")
401+
append(" if (!body.isObject) return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to \"validation\") as Any)\n")
402+
for (field in patchSettableFields) {
369403
if (KotlinGenUtil.isRequiredField(field)) {
370-
append(" it[${field.name}] = dto.${field.name}\n")
371-
} else {
372-
append(" if (dto.${field.name} != null) it[${field.name}] = dto.${field.name}\n")
404+
append(" if (body.has(\"${field.name}\") && body.get(\"${field.name}\").isNull) return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to \"validation\") as Any)\n")
373405
}
374406
}
375-
append(" }\n")
376-
append(" if (updated == 0) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf(\"error\" to \"not_found\") as Any)\n")
377-
append(" else {\n")
378-
append(" val row = ${tableObjectName}.selectAll().where { ${tableObjectName}.${pkFieldName} eq id }.single()\n")
379-
append(" ResponseEntity.ok(rowTo${shortName}(row) as Any)\n")
380-
append(" }\n")
407+
if (hasPatchFields) {
408+
val settableNamesList = patchSettableFields.joinToString(", ") { "\"${it.name}\"" }
409+
append(" if (listOf($settableNamesList).any { body.has(it) }) {\n")
410+
// A present value that cannot bind to its column's Kotlin type (e.g. a JSON object
411+
// for a String column) throws from treeToValue INSIDE the statement build — map
412+
// that to 400 (the create @Valid path 400s the same value), not a 500. Catch the
413+
// specific Jackson JsonMappingException so no unrelated exception is swallowed.
414+
append(" try {\n")
415+
append(" ${tableObjectName}.update({ ${tableObjectName}.${pkFieldName} eq id }) {\n")
416+
for (field in patchSettableFields) {
417+
// The DATA-CLASS element type (a field.enum's materialized enum class, not the
418+
// wire String) so the bound value matches the Exposed Column<E>. Columns are
419+
// qualified (`Table.col`) so a field named like the `body` param can't shadow them.
420+
val elem = KotlinTypeMapper.enumTypeName(field, entity) ?: KotlinTypeMapper.kotlinTypeName(field)
421+
val ktType = if (field.isArrayType) "kotlin.collections.List<$elem>" else "$elem"
422+
val col = "$tableObjectName.${field.name}"
423+
// A typed local coerces the ObjectMapper's platform return type to the exact
424+
// column type (matching the data-class property the old handler assigned) so
425+
// Exposed's set() overload resolves for enum / EntityID-FK columns too.
426+
if (KotlinGenUtil.isRequiredField(field)) {
427+
append(" if (body.has(\"${field.name}\")) { val v: $ktType = objectMapper.treeToValue(body.get(\"${field.name}\"), object : TypeReference<$ktType>() {}); it[$col] = v }\n")
428+
} else {
429+
append(" if (body.has(\"${field.name}\")) { val n = body.get(\"${field.name}\"); if (n.isNull) it[$col] = null else { val v: $ktType = objectMapper.treeToValue(n, object : TypeReference<$ktType>() {}); it[$col] = v } }\n")
430+
}
431+
}
432+
append(" }\n")
433+
append(" } catch (e: com.fasterxml.jackson.databind.JsonMappingException) {\n")
434+
append(" return@transaction ResponseEntity.badRequest().body(mapOf(\"error\" to \"validation\") as Any)\n")
435+
append(" }\n")
436+
append(" }\n")
437+
}
438+
append(" val row = ${tableObjectName}.selectAll().where { ${tableObjectName}.${pkFieldName} eq id }.singleOrNull()\n")
439+
append(" if (row == null) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf(\"error\" to \"not_found\") as Any)\n")
440+
append(" else ResponseEntity.ok(rowTo${shortName}(row) as Any)\n")
381441
append(" }\n\n")
382442

383443
// DELETE — Exposed's `deleteWhere` is an extension fn on Table; the import

server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-controller/acme/blog/AuthorController.kt

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import org.jetbrains.exposed.sql.insert
1010
import org.jetbrains.exposed.sql.selectAll
1111
import org.jetbrains.exposed.sql.update
1212
import org.jetbrains.exposed.sql.transactions.transaction
13+
import com.fasterxml.jackson.databind.JsonNode
14+
import com.fasterxml.jackson.core.type.TypeReference
15+
import com.fasterxml.jackson.databind.ObjectMapper
1316
import jakarta.validation.Valid
1417
import org.springframework.http.HttpStatus
1518
import org.springframework.http.ResponseEntity
@@ -232,7 +235,7 @@ private fun rowToAuthor(row: ResultRow): Author = Author(
232235
/** GENERATED — REST controller for Author entity. Implements the cross-port API contract. */
233236
@RestController
234237
@RequestMapping("/api/authors")
235-
class AuthorController {
238+
class AuthorController(private val objectMapper: ObjectMapper) {
236239

237240
@GetMapping
238241
fun list(
@@ -280,15 +283,21 @@ class AuthorController {
280283
}
281284

282285
@RequestMapping(value = ["/{id}"], method = [RequestMethod.PATCH, RequestMethod.PUT])
283-
fun update(@PathVariable id: Long, @Valid @RequestBody dto: Author): ResponseEntity<Any> = transaction {
284-
val updated = AuthorTable.update({ AuthorTable.id eq id }) {
285-
it[name] = dto.name
286-
}
287-
if (updated == 0) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf("error" to "not_found") as Any)
288-
else {
289-
val row = AuthorTable.selectAll().where { AuthorTable.id eq id }.single()
290-
ResponseEntity.ok(rowToAuthor(row) as Any)
286+
fun update(@PathVariable id: Long, @RequestBody body: JsonNode): ResponseEntity<Any> = transaction {
287+
if (!body.isObject) return@transaction ResponseEntity.badRequest().body(mapOf("error" to "validation") as Any)
288+
if (body.has("name") && body.get("name").isNull) return@transaction ResponseEntity.badRequest().body(mapOf("error" to "validation") as Any)
289+
if (listOf("name").any { body.has(it) }) {
290+
try {
291+
AuthorTable.update({ AuthorTable.id eq id }) {
292+
if (body.has("name")) { val v: kotlin.String = objectMapper.treeToValue(body.get("name"), object : TypeReference<kotlin.String>() {}); it[AuthorTable.name] = v }
293+
}
294+
} catch (e: com.fasterxml.jackson.databind.JsonMappingException) {
295+
return@transaction ResponseEntity.badRequest().body(mapOf("error" to "validation") as Any)
296+
}
291297
}
298+
val row = AuthorTable.selectAll().where { AuthorTable.id eq id }.singleOrNull()
299+
if (row == null) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf("error" to "not_found") as Any)
300+
else ResponseEntity.ok(rowToAuthor(row) as Any)
292301
}
293302

294303
@DeleteMapping("/{id}")

server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthLineController.kt

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import org.jetbrains.exposed.sql.insert
1010
import org.jetbrains.exposed.sql.selectAll
1111
import org.jetbrains.exposed.sql.update
1212
import org.jetbrains.exposed.sql.transactions.transaction
13+
import com.fasterxml.jackson.databind.JsonNode
14+
import com.fasterxml.jackson.core.type.TypeReference
15+
import com.fasterxml.jackson.databind.ObjectMapper
1316
import jakarta.validation.Valid
1417
import org.springframework.http.HttpStatus
1518
import org.springframework.http.ResponseEntity
@@ -232,7 +235,7 @@ private fun rowToAuthLine(row: ResultRow): AuthLine = AuthLine(
232235
/** GENERATED — REST controller for AuthLine entity. Implements the cross-port API contract. */
233236
@RestController
234237
@RequestMapping("/api/authlines")
235-
class AuthLineController {
238+
class AuthLineController(private val objectMapper: ObjectMapper) {
236239

237240
@GetMapping
238241
fun list(
@@ -280,15 +283,20 @@ class AuthLineController {
280283
}
281284

282285
@RequestMapping(value = ["/{id}"], method = [RequestMethod.PATCH, RequestMethod.PUT])
283-
fun update(@PathVariable id: Long, @Valid @RequestBody dto: AuthLine): ResponseEntity<Any> = transaction {
284-
val updated = AuthLineTable.update({ AuthLineTable.id eq id }) {
285-
if (dto.label != null) it[label] = dto.label
286-
}
287-
if (updated == 0) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf("error" to "not_found") as Any)
288-
else {
289-
val row = AuthLineTable.selectAll().where { AuthLineTable.id eq id }.single()
290-
ResponseEntity.ok(rowToAuthLine(row) as Any)
286+
fun update(@PathVariable id: Long, @RequestBody body: JsonNode): ResponseEntity<Any> = transaction {
287+
if (!body.isObject) return@transaction ResponseEntity.badRequest().body(mapOf("error" to "validation") as Any)
288+
if (listOf("label").any { body.has(it) }) {
289+
try {
290+
AuthLineTable.update({ AuthLineTable.id eq id }) {
291+
if (body.has("label")) { val n = body.get("label"); if (n.isNull) it[AuthLineTable.label] = null else { val v: kotlin.String = objectMapper.treeToValue(n, object : TypeReference<kotlin.String>() {}); it[AuthLineTable.label] = v } }
292+
}
293+
} catch (e: com.fasterxml.jackson.databind.JsonMappingException) {
294+
return@transaction ResponseEntity.badRequest().body(mapOf("error" to "validation") as Any)
295+
}
291296
}
297+
val row = AuthLineTable.selectAll().where { AuthLineTable.id eq id }.singleOrNull()
298+
if (row == null) ResponseEntity.status(HttpStatus.NOT_FOUND).body(mapOf("error" to "not_found") as Any)
299+
else ResponseEntity.ok(rowToAuthLine(row) as Any)
292300
}
293301

294302
@DeleteMapping("/{id}")

server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/AuthorApiServer.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,12 @@ class AuthorApiServer(private val pg: PostgresContainer) : AutoCloseable {
197197
@Suppress("UNCHECKED_CAST")
198198
val body = readJsonBody(exchange) as? Map<String, Any?>
199199
?: return sendJson(exchange, 400, mapOf("error" to "validation"))
200+
// FR-035 PATCH-2: an explicit null on a @required field (name / createdAt, per
201+
// meta.json) is a 400 — a present null on the nullable bio clears it, and an
202+
// omitted required field is untouched (never a 400).
203+
for (f in listOf("name", "createdAt")) {
204+
if (body.containsKey(f) && body[f] == null) return sendJson(exchange, 400, mapOf("error" to "validation"))
205+
}
200206
val updated = transaction(db) {
201207
AuthorTable.update({ AuthorTable.id eq id }) {
202208
(body["name"] as? String)?.let { v -> it[name] = v }

server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/generated/EnumFilterControllerRunTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class EnumFilterControllerRunTest {
8989
"jdbc:h2:mem:enum_filter;DB_CLOSE_DELAY=-1;MODE=PostgreSQL", driver = "org.h2.Driver")
9090
transaction(db) { SchemaUtils.create(widgetTable) }
9191

92-
val controller = controllerClass.getDeclaredConstructor().newInstance()
92+
val controller = controllerClass.getDeclaredConstructor(ObjectMapper::class.java).newInstance(mapper)
9393
val converter = MappingJackson2HttpMessageConverter().apply { objectMapper = mapper }
9494
val mvc = MockMvcBuilders.standaloneSetup(controller).setMessageConverters(converter).build()
9595

0 commit comments

Comments
 (0)