Skip to content

Commit 84cde02

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): projection read-type mirrors view-column nullability + cross-port compile guards
A non-required (nullable) projection field generated a nullable Drizzle view column but a NON-null Zod read type, so the generated projection query returned `T | null` into a non-null `<Name>` field and failed to compile under strict TS (the likely source of the "null is not assignable to number" friction seen building real apps). Fix: the projection Zod read schema now appends `.nullable()` whenever the view column is not `.notNull()`, so the read type matches what `db.select().from(view)` actually yields (and is more correct — a nullable derived field is typed nullable). Closes the cross-port test gap that let the original PR #80 projection bug ship: only C# compiled generated projection code. Adds a real compile/structure guard to every port that lacked one — TS (tsc), Python (exec), Java (in-process javac of the DTO), Kotlin (kotlin-compile-testing of the data class) — each exercising a projection with a required id + a non-required derived field. Test-only for Java/Kotlin/Python (those ports were already correct); the TS change is the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LuZWKnWzYGVnESijL7uuky
1 parent d1c40de commit 84cde02

5 files changed

Lines changed: 602 additions & 4 deletions

File tree

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
package com.metaobjects.generator.kotlin
2+
3+
import com.metaobjects.metadata.ktx.loadString
4+
import com.tschuchort.compiletesting.KotlinCompilation
5+
import com.tschuchort.compiletesting.SourceFile
6+
import java.nio.file.Files
7+
import kotlin.io.path.isRegularFile
8+
import kotlin.io.path.readText
9+
import kotlin.test.Test
10+
import kotlin.test.assertEquals
11+
import kotlin.test.assertFalse
12+
import kotlin.test.assertTrue
13+
14+
/**
15+
* Cross-port guard for the projection-codegen bug class fixed in TypeScript PR #80
16+
* (`835d6be4` — "fix projection/driver-type codegen"). That PR's root symptom was a
17+
* read/write inconsistency in a projection's generated artifacts: the read-only
18+
* projection emitted output that referenced a write-shaped member that the read-only
19+
* surface never declared, so the build broke and the agent reverted to hand-rolling.
20+
* Its robust complement (`server/typescript/.../projection/compile.test.ts`) actually
21+
* COMPILES the generated projection — so any read-only/writable inconsistency that
22+
* produces non-compiling output is caught, not just the one symptom we already knew.
23+
*
24+
* The C# port compiles its generated projection code; TS and Python now do too. This
25+
* is the Kotlin equivalent: it runs an `object.projection` carrying a REQUIRED
26+
* pass-through id AND a NON-required derived aggregate field — the exact shape that
27+
* exposed the nullable-view-column vs non-null-read-type mismatch in TS — through
28+
* `KotlinEntityGenerator` (immutable `data class`) + `KotlinExposedTableGenerator`
29+
* (read-only Exposed `Table`), then COMPILES the emitted Kotlin with the real Kotlin
30+
* compiler (kotlin-compile-testing, as [KotlinOutputCompilesTest] does). A nullable
31+
* column whose data-class read type was non-null (or vice-versa), or any other
32+
* read/write inconsistency, fails to compile here.
33+
*
34+
* It also asserts the read-only structural contract directly — the entity is an
35+
* immutable `data class` (no `var`), the projection's Exposed table carries the
36+
* READ-ONLY VIEW guard with no `.autoIncrement()`/`.references(...)`, and the
37+
* write-surface generators (controller / validator / filter-allowlist) SKIP the
38+
* projection entirely (they only emit for `object.entity`).
39+
*
40+
* Scope of the true compile: this module's test classpath carries kotlinx.serialization
41+
* (so `@Serializable` data classes resolve) but NOT jetbrains-exposed, so the emitted
42+
* `*Table.kt` files cannot be compiled here (the existing [KotlinOutputCompilesTest]
43+
* likewise only compiles data-class / payload output, never an Exposed table). We
44+
* therefore COMPILE the read-side projection data class — which is exactly the surface
45+
* the PR-#80 nullable-vs-non-null read-type mismatch lives on — and assert the Exposed
46+
* table's read-only-ness + nullable-column/read-type consistency structurally.
47+
*/
48+
@OptIn(org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi::class)
49+
class KotlinProjectionCompileTest {
50+
51+
// Program (writable base) + Week (the to-many target) + a ProgramSummary
52+
// projection over a read-only view. `id` is a REQUIRED pass-through pk;
53+
// `weekCount` is a NON-required derived aggregate → its view column is
54+
// nullable, so the data-class read type must be nullable too. This is the
55+
// PR-#80 nullable-column-vs-non-null-read-type case.
56+
private val projectionFixture = """{
57+
"metadata.root": { "package": "acme::commerce", "children": [
58+
{ "object.entity": { "name": "Program", "children": [
59+
{ "field.long": { "name": "id" } },
60+
{ "field.string": { "name": "title", "@required": true } },
61+
{ "source.rdb": { "@table": "programs" } },
62+
{ "identity.primary": { "name": "id", "@fields": "id", "@generation": "increment" } },
63+
{ "relationship.association": { "name": "weeks", "@objectRef": "Week", "@cardinality": "many" } }
64+
] } },
65+
{ "object.entity": { "name": "Week", "children": [
66+
{ "field.long": { "name": "id" } },
67+
{ "field.long": { "name": "programId" } },
68+
{ "source.rdb": { "@table": "weeks" } },
69+
{ "identity.primary": { "name": "id", "@fields": "id", "@generation": "increment" } },
70+
{ "identity.reference": { "name": "ref_program", "@fields": "programId", "@references": "Program" } }
71+
] } },
72+
{ "object.projection": { "name": "ProgramSummary", "children": [
73+
{ "source.rdb": { "@kind": "view", "@view": "v_program_summary" } },
74+
{ "field.long": { "name": "id", "extends": "acme::commerce::Program.id", "@required": true } },
75+
{ "field.string": { "name": "title", "children": [
76+
{ "origin.passthrough": { "@from": "acme::commerce::Program.title" } }
77+
] } },
78+
{ "field.int": { "name": "weekCount", "children": [
79+
{ "origin.aggregate": { "@agg": "count", "@of": "Week.id", "@via": "Program.weeks" } }
80+
] } },
81+
{ "identity.primary": { "name": "id", "extends": "acme::commerce::Program.id" } }
82+
] } }
83+
] }
84+
}""".trimIndent()
85+
86+
@Test fun `projection entity data class plus Exposed table compile and are read-only`() {
87+
val outDir = Files.createTempDirectory("kproj-compile-")
88+
try {
89+
val loader = loadString("projection-compile", projectionFixture)
90+
91+
// Run BOTH the entity (data class) and the Exposed table generator into one tree.
92+
for (gen in listOf(KotlinEntityGenerator(), KotlinExposedTableGenerator())) {
93+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
94+
gen.execute(loader)
95+
}
96+
97+
val summaryKt = outDir.resolve("acme/commerce/ProgramSummary.kt")
98+
val summaryTableKt = outDir.resolve("acme/commerce/ProgramSummaryTable.kt")
99+
assertTrue(Files.exists(summaryKt),
100+
"expected projection data class $summaryKt; files=${Files.walk(outDir).toList()}")
101+
assertTrue(Files.exists(summaryTableKt),
102+
"expected projection Exposed table $summaryTableKt; files=${Files.walk(outDir).toList()}")
103+
104+
val summarySrc = Files.readString(summaryKt)
105+
val tableSrc = Files.readString(summaryTableKt)
106+
107+
// --- Read-only structural contract on the data class ---
108+
assertTrue("data class ProgramSummary" in summarySrc,
109+
"projection must be an immutable data class; saw:\n$summarySrc")
110+
// Immutable: no mutable `var` properties anywhere in the read-only projection.
111+
assertFalse(Regex("""\bvar\s+\w""").containsMatchIn(summarySrc),
112+
"projection data class must be immutable (no `var`); saw:\n$summarySrc")
113+
// The NON-required derived aggregate must read as a nullable type — the
114+
// PR-#80 mismatch is exactly a non-null read type over a nullable view column.
115+
assertTrue("val weekCount: Int? = null" in summarySrc,
116+
"non-required derived `weekCount` must be nullable (`Int? = null`); saw:\n$summarySrc")
117+
118+
// --- Read-only structural contract on the Exposed table ---
119+
assertTrue("READ-ONLY VIEW" in tableSrc,
120+
"projection table must carry the READ-ONLY VIEW guard; saw:\n$tableSrc")
121+
assertTrue("object ProgramSummaryTable : Table(\"v_program_summary\")" in tableSrc,
122+
"projection table must bind to the @view physical name; saw:\n$tableSrc")
123+
// Views are read-only: no auto-increment, no FK constraints on the view body.
124+
assertFalse(".autoIncrement()" in tableSrc,
125+
"projection (view) columns must NOT use .autoIncrement(); saw:\n$tableSrc")
126+
assertFalse(".references(" in tableSrc,
127+
"projection (view) must NOT emit FK .references(...); saw:\n$tableSrc")
128+
// The nullable view column for the non-required aggregate matches the nullable
129+
// read type above (the two-sided consistency PR #80 was about).
130+
assertTrue("val weekCount = integer(\"week_count\").nullable()" in tableSrc,
131+
"weekCount view column must be nullable, matching its nullable read type; saw:\n$tableSrc")
132+
133+
// --- TRUE Kotlin compile of the generated projection DATA CLASSES ---
134+
// Exposed is not on this module's test classpath, so we compile the read-side
135+
// data classes (the `*.kt` entity/projection files, NOT the `*Table.kt` files).
136+
// The projection data class is precisely the surface the PR-#80 nullable read-type
137+
// mismatch lives on: a non-null read type over a nullable view column fails here.
138+
val dataClassPaths = Files.walk(outDir).filter { it.isRegularFile() }
139+
.filter { !it.fileName.toString().endsWith("Table.kt") }
140+
.toList()
141+
assertTrue(dataClassPaths.any { it.fileName.toString() == "ProgramSummary.kt" },
142+
"expected the projection data class to be among the compiled sources")
143+
val dataClassSources = dataClassPaths.map { path ->
144+
SourceFile.kotlin(
145+
path.parent.relativize(path).toString().replace('/', '_'),
146+
path.readText(),
147+
)
148+
}
149+
150+
val result = KotlinCompilation().apply {
151+
this.sources = dataClassSources
152+
inheritClassPath = true // brings kotlinx.serialization onto the classpath
153+
messageOutputStream = System.out
154+
}.compile()
155+
156+
assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode,
157+
"generated projection Kotlin data classes failed to compile:\n${result.messages}")
158+
} finally {
159+
outDir.toFile().deleteRecursively()
160+
}
161+
}
162+
163+
@Test fun `write-surface generators skip the projection`() {
164+
val outDir = Files.createTempDirectory("kproj-skip-")
165+
try {
166+
val loader = loadString("projection-skip", projectionFixture)
167+
168+
// Controller / validator / filter-allowlist generators only emit for
169+
// object.entity — they must NOT produce any artifact for the projection.
170+
for (gen in listOf(
171+
KotlinSpringControllerGenerator(),
172+
KotlinValidatorGenerator(),
173+
KotlinFilterAllowlistGenerator(),
174+
)) {
175+
// packageName is required by the validator generator (it emits a
176+
// package-level startup-validator stub regardless of entity count).
177+
gen.setArgs(mapOf(
178+
"outputDir" to outDir.toString(),
179+
"packageName" to "acme.commerce",
180+
))
181+
gen.execute(loader)
182+
}
183+
184+
val emitted = if (Files.exists(outDir)) {
185+
Files.walk(outDir).filter { it.isRegularFile() }
186+
.map { it.fileName.toString() }.toList()
187+
} else emptyList()
188+
189+
// No projection-named write-surface artifact may exist.
190+
assertFalse(emitted.any { it.startsWith("ProgramSummary") },
191+
"write-surface generators must SKIP the projection; emitted=$emitted")
192+
// Specifically: no controller / validator / filter-allowlist for the projection.
193+
for (suffix in listOf("Controller.kt", "Validator.kt", "FilterAllowlist.kt")) {
194+
assertFalse(Files.exists(outDir.resolve("acme/commerce/ProgramSummary$suffix")),
195+
"must NOT emit ProgramSummary$suffix for a read-only projection; emitted=$emitted")
196+
}
197+
} finally {
198+
outDir.toFile().deleteRecursively()
199+
}
200+
}
201+
}

0 commit comments

Comments
 (0)