From 92d779fa94bbede5a795e0e1635ec9441a931b2e Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 27 Jun 2026 21:11:45 -0400 Subject: [PATCH] fix(codegen-kotlin): projection `extends` inherits @dbColumnType + gets a self-contained enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An `object.projection` field bound to a base-entity field via `extends:` silently dropped two pieces of the base field's shaping in the Exposed-table generator: 1. @dbColumnType was read OWN-ONLY (`KotlinTypeMapper.dbColumnType` → `stringAttr` hardcoded `includeParent=false`) while @maxLength was read inherited (`stringMaxLength` → `true`). So an extends-bound projection field inherited its length but fell back to varchar/datetime for uuid / text_array / timestamp_with_tz columns — forcing callers to re-declare @dbColumnType on every view column. Fix: read @dbColumnType through the `extends` super-field chain (own value still wins). `stringAttr` gains an `includeParent` param (default false). 2. `enumTypeName` collapsed ANY extends-bound enum onto the super field's bare short name (`status` → root-package `Status`), colliding across every entity with a `status`. Only a package-level ABSTRACT enum super (FR-019 shared enum, no declaring object) should collapse onto the shared type. A super bound to a CONCRETE entity now falls through to per-object naming, so a projection enum field gets its OWN `` enum in its OWN package — self-contained (no cross-package reference), with values inherited via `extends` (readEnumValues is already inheritance-aware). The metadata model is untouched — both reads resolve existing `extends` links at codegen time (no load-time mutation). Adds KotlinProjectionExtendsInheritanceTest; FR-019 shared-enum conformance + full codegen-kotlin suite (248) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_018mjBXyYKk6tbQymzhgMmdB --- .../generator/kotlin/KotlinTypeMapper.kt | 28 +++-- .../KotlinProjectionExtendsInheritanceTest.kt | 116 ++++++++++++++++++ 2 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionExtendsInheritanceTest.kt diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt index 1765f81f6..391531d03 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt @@ -154,8 +154,17 @@ object KotlinTypeMapper { // top-most super (`Priority`) so the generated type is shared (and deduped by FQN at // emission). Walk the super chain to the root. When there is no super, fall back to the // per-entity `` naming. Mirrors the C#/TS/Python ports. + // Collapse onto a SHARED enum type only for a package-level ABSTRACT enum super (FR-019, + // e.g. two fields `extends: Priority`) — identified by having NO declaring object. A super + // bound to a CONCRETE entity field (e.g. a read-model projection `extends`-ing + // `ActiveNpc.status`) does NOT collapse: it falls through to the per-object naming below, + // so the projection gets its OWN `` enum in its OWN package + // (self-contained — no cross-package reference), populated with the values it inherits via + // `extends` ([KotlinEnumEmitter.readEnumValues] is inheritance-aware). Without this guard + // such a field named the enum from the super's bare short name (`status`), which `splitFqn` + // collapsed to a root-package `Status` — colliding across every entity that has a `status`. val superRoot = resolveSuperRoot(field) - if (superRoot != null) { + if (superRoot != null && runCatching { superRoot.declaringObject }.getOrNull() == null) { val (superPkg, superShort) = PackageMapping.splitFqn(superRoot.name) return ClassName(superPkg, superShort.replaceFirstChar { it.uppercase() }) } @@ -410,11 +419,15 @@ object KotlinTypeMapper { } /** - * Read the `@dbColumnType` attribute (own-only, case-folded) for column-type overrides. + * Read the `@dbColumnType` attribute (case-folded) for column-type overrides. Resolved + * THROUGH the `extends` super-field chain (`includeParent = true`), so an `object.projection` + * field that binds a base-entity column via `extends:` inherits that column's physical type + * (uuid / text_array / timestamp_with_tz) — exactly as it already inherits `@maxLength` (see + * [stringMaxLength]). Own value still wins (checked first by [MetaData.hasMetaAttr]). * Returns null when absent. See [ATTR_DB_COLUMN_TYPE] for recognised values. */ private fun dbColumnType(field: MetaField<*>): String? = - stringAttr(field, ATTR_DB_COLUMN_TYPE)?.lowercase() + stringAttr(field, ATTR_DB_COLUMN_TYPE, includeParent = true)?.lowercase() /** * True iff [field] carries `@dbColumnType=timestamp_with_tz` (case-insensitive). @@ -434,15 +447,16 @@ object KotlinTypeMapper { field is TimestampField && timestampWithTzOptIn(field) /** - * Best-effort read of a named string attribute (own-only) on [field]. Returns null when + * Best-effort read of a named string attribute on [field] (own-only by default; + * [includeParent] = true also walks the `extends` super-field chain). Returns null when * the attribute is absent, throws during lookup, or isn't a [com.metaobjects.attr.MetaAttribute]. * Used for non-typed dispatch keys (e.g. `@kind`) that aren't part of the registered * StringField attribute schema. */ - private fun stringAttr(field: MetaField<*>, name: String): String? { - if (!field.hasMetaAttr(name, false)) return null + private fun stringAttr(field: MetaField<*>, name: String, includeParent: Boolean = false): String? { + if (!field.hasMetaAttr(name, includeParent)) return null val attr = runCatching { - field.getMetaAttr(name, false) + field.getMetaAttr(name, includeParent) }.getOrNull() as? com.metaobjects.attr.MetaAttribute<*> ?: return null return attr.valueAsString } diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionExtendsInheritanceTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionExtendsInheritanceTest.kt new file mode 100644 index 000000000..072c05213 --- /dev/null +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionExtendsInheritanceTest.kt @@ -0,0 +1,116 @@ +package com.metaobjects.generator.kotlin + +import com.metaobjects.metadata.ktx.loadString +import java.nio.file.Files +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Regression guard for projection `extends:` PHYSICAL-TYPE inheritance in the Kotlin + * Exposed-table codegen. An `object.projection` field that binds to a base-entity field + * via `extends:` must inherit that base field's *physical* shaping — the same way it + * already inherits `@maxLength` — for: + * + * - `@dbColumnType=uuid` → `uuid("col")` (NOT `varchar(col, 36)`) + * - `@dbColumnType=text_array` → `array(...)` (NOT `varchar`) + * - `@dbColumnType=timestamp_with_tz` → `instantWithTimeZone(...)` (NOT `datetime`) + * - a `field.enum` super → the projection's OWN `Status` enum + * (`ProgramViewStatus`, self-contained, values inherited via `extends`), NOT a + * root-package `Status` (the cross-entity-collision bug). + * + * Root causes (both in this module's generator, NOT the metadata model): + * 1. `KotlinTypeMapper.dbColumnType()` read the attribute OWN-ONLY while `stringMaxLength()` + * read it INHERITED — so an `extends`-bound projection field inherited `@maxLength` + * but silently dropped `@dbColumnType`, falling back to varchar/datetime. + * 2. `KotlinTypeMapper.enumTypeName()` collapsed ANY `extends`-bound enum onto the super's + * short name (`status` → root `Status`). Only a package-level ABSTRACT enum super should + * collapse; a CONCRETE entity super must fall through to per-projection naming so the + * projection gets its own `Status` (in its own package), avoiding a cross-package + * reference AND the root-`Status` collision. + * + * The shared-abstract-enum (FR-019 `Priority`) path is intentionally left untouched: an + * abstract package-level enum has NO declaring object, so it keeps its package-qualified + * shared naming — see [KotlinFr019SharedProvidedConformanceTest]. + */ +class KotlinProjectionExtendsInheritanceTest { + + // Program (writable base) carries uuid / text_array / timestamp_with_tz / enum fields. + // ProgramView projects them via `extends:` with NO own physical shaping — every + // physical type below MUST be inherited from the base field. + private val fixture = """{ + "metadata.root": { "package": "acme::commerce", "children": [ + { "object.entity": { "name": "Program", "children": [ + { "field.string": { "name": "id", "@required": true, "@dbColumnType": "uuid" } }, + { "field.string": { "name": "ownerId", "@dbColumnType": "uuid" } }, + { "field.enum": { "name": "status", "@required": true, + "@values": ["DRAFT", "LIVE", "ARCHIVED"] } }, + { "field.string": { "name": "tags", "@dbColumnType": "text_array" } }, + { "field.timestamp": { "name": "publishedAt", "@dbColumnType": "timestamp_with_tz" } }, + { "source.rdb": { "@table": "programs" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } + ] } }, + { "object.projection": { "name": "ProgramView", "children": [ + { "source.rdb": { "@kind": "view", "@view": "v_program" } }, + { "field.string": { "name": "id", "extends": "acme::commerce::Program.id", "@required": true } }, + { "field.string": { "name": "ownerId", "extends": "acme::commerce::Program.ownerId" } }, + { "field.enum": { "name": "status", "extends": "acme::commerce::Program.status", "@required": true } }, + { "field.string": { "name": "tags", "extends": "acme::commerce::Program.tags" } }, + { "field.timestamp": { "name": "publishedAt", "extends": "acme::commerce::Program.publishedAt" } }, + { "identity.primary": { "name": "id", "extends": "acme::commerce::Program.id" } } + ] } } + ] } + }""".trimIndent() + + @Test fun `projection extends inherits dbColumnType and base-entity enum`() { + val outDir = Files.createTempDirectory("kproj-extends-") + try { + val loader = loadString("projection-extends", fixture) + for (gen in listOf(KotlinEntityGenerator(), KotlinExposedTableGenerator())) { + gen.setArgs(mapOf("outputDir" to outDir.toString())) + gen.execute(loader) + } + + val tableKt = outDir.resolve("acme/commerce/ProgramViewTable.kt") + assertTrue(Files.exists(tableKt), + "expected projection Exposed table $tableKt; files=${Files.walk(outDir).toList()}") + val table = Files.readString(tableKt) + + // --- @dbColumnType=uuid inherited via extends --- + assertTrue("val id = uuid(\"id\")" in table, + "uuid PK must inherit @dbColumnType=uuid (uuid(\"id\")); saw:\n$table") + assertTrue("val ownerId = uuid(\"owner_id\")" in table, + "ownerId must inherit @dbColumnType=uuid; saw:\n$table") + assertFalse("varchar(\"id\"" in table || "varchar(\"owner_id\"" in table, + "uuid columns must NOT fall back to varchar; saw:\n$table") + + // --- @dbColumnType=text_array inherited --- + assertTrue("val tags = array(\"tags\"" in table, + "tags must inherit @dbColumnType=text_array (array); saw:\n$table") + + // --- @dbColumnType=timestamp_with_tz inherited --- + assertTrue("val publishedAt = instantWithTimeZone(\"published_at\")" in table, + "publishedAt must inherit @dbColumnType=timestamp_with_tz (instantWithTimeZone); saw:\n$table") + + // --- enum gets the projection's OWN self-contained type, not root Status --- + assertTrue("enumerationByName(\"status\", " in table && "ProgramViewStatus::class" in table, + "status must reference the projection's own ProgramViewStatus enum; saw:\n$table") + assertFalse(", Status::class)" in table, + "status must NOT collapse to a root-package `Status` enum; saw:\n$table") + + // The projection's own enum class is materialized in the projection's package, + // populated with the values INHERITED via `extends` (no stray root `Status.kt`). + val viewEnumKt = outDir.resolve("acme/commerce/ProgramViewStatus.kt") + assertTrue(Files.exists(viewEnumKt), + "projection enum ProgramViewStatus.kt must be materialized (values inherited via extends)") + val viewEnumSrc = Files.readString(viewEnumKt) + for (m in listOf("DRAFT", "LIVE", "ARCHIVED")) + assertTrue(m in viewEnumSrc, + "inherited enum member $m must be present in ProgramViewStatus; saw:\n$viewEnumSrc") + assertFalse(Files.exists(outDir.resolve("Status.kt")), + "no root-package Status.kt may be emitted for the extends-bound projection enum") + } finally { + outDir.toFile().deleteRecursively() + } + } +}