From ad49f1c42f76d9122d41f1856160a80af4220091 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Fri, 13 Nov 2026 22:11:23 -0500 Subject: [PATCH 1/8] fix: cut long foreign-key index names on Postgres; verify refuses what generate refuses - migrate.foreignKeyIndexes: a __idx name over 63 bytes is cut to the 63 bytes Postgres stores (SQLite and D1 keep it whole), in migrate-ts, the C# EF context and the Kotlin Exposed tables, so a converged schema proposes nothing. The diff also keys a Postgres foreign key by its stored name, so a long
__fk is not proposed as a rename every run. - New shared corpus fixtures/foreign-key-index-conformance/, passed by the TypeScript, C# and Kotlin rules; two Postgres migration scenarios with a long table and column name. - mvn metaobjects:verify runs the generate goal's model refusals (output package clash, collection-name clash), with a test; Kotlin gets its own collection-name clash test. - docs/CONFORMANCE.md: the clash is per-port unit tests, not the segments corpus; the Drizzle identity start is string-tested only. - ADR-0060 records verify.handEdits as Node-CLI only by design. - The migration docs say what the code does with view grants (#574). --- AGENTS.md | 2 +- CHANGELOG.md | 14 ++ docs/CONFORMANCE.md | 34 +++-- docs/features/migrations-and-drift.md | 31 ++++- .../foreign-key-index-conformance/README.md | 39 ++++++ .../expected.postgres.json | 53 +++++++ .../expected.sqlite.json | 53 +++++++ .../input/meta.fk.json | 71 ++++++++++ fixtures/naming-conformance/README.md | 2 +- .../foreign-key-index-long-name-added.yaml | 23 ++++ ...converged-long-foreign-key-index-name.yaml | 17 +++ .../states/long-fk-index-names/meta.json | 25 ++++ .../ForeignKeyIndexConformanceTests.cs | 55 ++++++++ .../Generators/DbContextGenerator.cs | 85 +----------- .../Generators/ForeignKeyIndexes.cs | 129 ++++++++++++++++++ .../kotlin/KotlinExposedTableGenerator.kt | 49 +++++-- .../KotlinForeignKeyIndexConformanceTest.kt | 52 +++++++ .../generator/kotlin/KotlinRouteNamingTest.kt | 42 ++++++ .../mojo/MetaDataGeneratorMojo.java | 23 ++-- .../metaobjects/mojo/MetaDataVerifyMojo.java | 3 + .../mojo/MetaDataVerifyModelRefusalTest.java | 57 ++++++++ .../mojo/layout-collision/meta.Sales.json | 7 + .../mojo/layout-collision/meta.sales.json | 7 + .../packages/migrate-ts/src/diff/index.ts | 29 ++-- .../migrate-ts/src/expected-schema.ts | 16 ++- .../packages/migrate-ts/src/types.ts | 14 ++ .../foreign-key-index-conformance.test.ts | 59 ++++++++ .../ADR-0060-jvm-codegen-hash-manifest.md | 5 + 28 files changed, 859 insertions(+), 137 deletions(-) create mode 100644 fixtures/foreign-key-index-conformance/README.md create mode 100644 fixtures/foreign-key-index-conformance/expected.postgres.json create mode 100644 fixtures/foreign-key-index-conformance/expected.sqlite.json create mode 100644 fixtures/foreign-key-index-conformance/input/meta.fk.json create mode 100644 fixtures/persistence-conformance/migrations/foreign-key-index-long-name-added.yaml create mode 100644 fixtures/persistence-conformance/migrations/noop-converged-long-foreign-key-index-name.yaml create mode 100644 fixtures/persistence-conformance/migrations/states/long-fk-index-names/meta.json create mode 100644 server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs create mode 100644 server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs create mode 100644 server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt create mode 100644 server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java create mode 100644 server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json create mode 100644 server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json create mode 100644 server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts diff --git a/AGENTS.md b/AGENTS.md index 8808485aa..57020fe4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ PyPI has had no product change since `0.25.0` — nothing is broken. - **Kotlin** — `codegen-kotlin` (KotlinPoet on JVM): entity + Exposed table + Spring controller + payload + relations + filter allowlist + validator + stored-proc + output-parser generators. `integration-tests-kotlin` runs the persistence-conformance corpus through Exposed against Testcontainers Postgres. **Cross-port conformance corpora** (every port runs the shared corpus): -- Metamodel: `fixtures/conformance/` (510 fixtures; 39 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. +- Metamodel: `fixtures/conformance/` (510 fixtures; 40 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. - Render: `fixtures/render-conformance/`. TS / C# / Java / Kotlin / Python byte-identical. - Persistence: `fixtures/persistence-conformance/`. **Query** scenarios run on every port (TS / C# / Java / Kotlin / Python), each provisioning its test DB by executing the committed, TS-produced `canonical/schema.postgres.sql` (Postgres only — Derby dropped for the cross-port query corpus, ADR-0015; the FR-044 `report-*` scenarios also run on SQLite, MySQL and D1 where a port has the engine, see [docs/CONFORMANCE.md](docs/CONFORMANCE.md)). The **migration** scenarios are exercised by **TS only** (TS owns schema migrations). **The corpus now gates WRITES, not just reads (SP-H):** an `op: roundtrip` scenario type INSERTs through each port's runtime/ORM write codec (NOT raw SQL), reads the row back, and asserts the wire-normalized value. The `AllTypes` entity (`roundtrip-all-types.yaml`) carries one field of **every** persistable `field.*` subtype — string/int/long/double/float/decimal/boolean/date/time/timestamp(+tz)/currency/enum/uuid/object — plus an **array-of-VO** `field.object @isArray @storage:jsonb` column (`labels`, written as 2-element / empty-`[]` / single-element arrays across the three rows) — so every subtype write+read (incl. the array-of-value-object jsonb codec) round-trips through every port against Testcontainers PG. (`field.byte`/`field.short`/`field.class` were cut as non-functional registration-only stubs — the matrix tracks only genuinely-supported subtypes; see `fixtures/registry-conformance/README.md` → "Per-subtype write-round-trip matrix".) - API-contract: `fixtures/api-contract-conformance/`. TS / C# / Java / Kotlin / Python all green — each port runs **two lanes**: a hand-rolled reference server AND its **generated** API artifact booted over HTTP (the deployed controller/routes; TS+C# full-stack vs Testcontainers PG, Java/Kotlin/Python generated controller + in-memory repo behind the consumer seam). The generated fan-out found 10 real deployment bugs golden snapshots missed. Three sub-corpora run the **generated lane only, on all five ports** — `write-through/`, `projection/` (F22: a view-only `object.projection` serves GET list + GET by id and answers every write verb with `405 {"error": "method_not_allowed"}`) and `report/` (FR-044: a view-backed `object.report` serves GET list, answers `POST` with that same `405`, and mounts no `/{id}` route). That is deliberate, not a gap: what is under test is whether a port's GENERATOR emits those routes, and a hand-rolled reference server would answer every scenario by construction. The `m2m/` sub-corpus also gates **TPH x M:N together** (base-declared, subtype-declared, abstract-mid-declared, a non-subtype source onto a subtype TARGET, and the cross-subtype source id answering `200 []`) — the two corpora were originally built disjoint (`tph/` had no relationships, `m2m/` no discriminators), which is precisely how that defect class survived. diff --git a/CHANGELOG.md b/CHANGELOG.md index e6fdb9615..81f5748a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,6 +146,20 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi scalar field; gated by the `error-origin-aggregate-value-object` conformance fixture. Refusing, rather than summing the flattened amount column, keeps a sum from adding amounts in different currencies. `count` is unaffected. +- **A long foreign-key index or constraint name no longer reappears on every `meta migrate`.** + `migrate.foreignKeyIndexes` named an index `
__idx` with no length check, and + Postgres stores a name over 63 bytes cut to 63, so the diff (which compares names exactly) + proposed the index again on every run. On Postgres the name is now cut to the 63 bytes Postgres + stores (SQLite and D1 keep it whole), and the C# EF context and the Kotlin Exposed tables + declare the same cut name. The diff also matches a foreign-key constraint by its stored name, + so a derived `
__fk` over the limit is no longer proposed as a rename. A new + shared corpus, `fixtures/foreign-key-index-conformance/`, pins which foreign keys get an index + and its name, and the TypeScript, C# and Kotlin rules all pass it; two migration scenarios + prove a long name converges on Postgres. +- **`mvn metaobjects:verify` refuses the model `mvn metaobjects:generate` refuses.** Two metadata + packages that generate into one output directory (`ERR_NAME_COLLISION`) and a REST collection + claimed twice (`ERR_COLLECTION_NAME_COLLISION`) were checked by the generate goal only, so a + project could verify clean on a model generate would refuse. Java and Kotlin. ## [1.3.0] — 2026-11-11 diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index bf3f4b202..37bf7f71f 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -1,6 +1,6 @@ # Conformance coverage -The MetaObjects standard ships **39 shared conformance corpora** under +The MetaObjects standard ships **40 shared conformance corpora** under [`fixtures/`](../fixtures/). Every port runs every corpus that is *applicable to it* and asserts the same expected behaviour against the same fixtures. **This page is the inverse index**: fixture → feature doc + per-port pass status, and it is the @@ -35,7 +35,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/jinja-verify-conformance/`](../fixtures/jinja-verify-conformance/) (ADR-0061 — the payload check over a Jinja body) | 31 | ✓ | refuses Jinja | refuses, via Java | refuses Jinja | ✓ | | [`fixtures/extract-conformance/`](../fixtures/extract-conformance/) | 58 | ✓ | ✓ | inherits the shared JVM engine | ✓ | ✓ | | [`fixtures/output-prompt-conformance/`](../fixtures/output-prompt-conformance/) | 17 | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/persistence-conformance/`](../fixtures/persistence-conformance/) | 64 (55 query + 9 migration) | all 64; the 24 `report-*` and the `-15` decimal scenarios also on SQLite, MySQL and D1, `decimal-exact` also on MySQL (a scenario's `engines:` key is exact) | 55 query (migrations TS-only, ADR-0015); the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query (via Exposed); the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query; the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query (pg8000); the `report-*` and `engines:` scenarios also on SQLite (`sqlite3`) and MySQL (PyMySQL) | +| [`fixtures/persistence-conformance/`](../fixtures/persistence-conformance/) | 67 (55 query + 12 migration) | all 67; the 24 `report-*` and the `-15` decimal scenarios also on SQLite, MySQL and D1, `decimal-exact` also on MySQL (a scenario's `engines:` key is exact) | 55 query (migrations TS-only, ADR-0015); the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query (via Exposed); the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query; the `report-*` and `engines:` scenarios also on SQLite and MySQL | 55 query (pg8000); the `report-*` and `engines:` scenarios also on SQLite (`sqlite3`) and MySQL (PyMySQL) | | [`fixtures/api-contract-conformance/`](../fixtures/api-contract-conformance/) | 123 (31 core + 10 tph + 9 m2m + 5 jsonb + 2 write-through + 2 secret-fields + 2 enum-default + 12 projection + 37 report + 11 decimal + 2 duration) | ✓ (Fastify reference + generated lane); `report/`, `decimal/` and `duration/` also on SQLite, MySQL and D1 | ✓ (embedded HTTP + JDBC); `report/`, `decimal/` and `duration/` generated controllers over OMDB on Postgres, SQLite and MySQL | ✓ (embedded HTTP + Exposed); `report/`, `decimal/` and `duration/` also on SQLite and MySQL | ✓ (Kestrel + Npgsql); `report/`, `decimal/` and `duration/` also on SQLite and MySQL | ✓ (FastAPI + pg8000); `report/`, `decimal/` and `duration/` also on SQLite and MySQL | | [`fixtures/decimal-wire/`](../fixtures/decimal-wire/) | 1 vector file (format, parse, filter operand, 15-digit read) | ✓ (`runtime-ts` and the generated helper) | ✓ (`CanonicalDecimal`) | ✓ (the generated wire file) | ✓ (`DecimalWire`) | ✓ (`decimal_wire`) | | [`fixtures/duration-wire/`](../fixtures/duration-wire/) | 1 vector file: the `wire` vectors (parse, precision, refused) every port passes, and the `codecs` groups (Postgres text read and write, stored microseconds) each port passes only for the codecs it has | ✓ (`metadata` / `runtime-ts` and the generated helper) | ✓ | ✓ | ✓ | ✓ | @@ -61,6 +61,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/wire-conformance/`](../fixtures/wire-conformance/) (the `wire` setting, ADR-0019 Amendment 2) | 6 cases + 8 refusals | ✓ | ✓ | inherits via Java (one config reader) | ✓ | ✓ | | [`fixtures/enum-length-conformance/`](../fixtures/enum-length-conformance/) (a string-stored enum's column length: `@maxLength`, else `enums.maxLength`) | 5 cases + 1 overflow + 5 refusals; run by a port that sizes the column | ✓ | ✓ | inherits via Java (one config reader, one resolver) | ✓ | — (sizes no column: emits no DDL or column type; the config reader accepts and shape-checks the `enums` block) | | [`fixtures/check-constraint-conformance/`](../fixtures/check-constraint-conformance/) (the CHECK constraints `meta migrate` derives: name and expression text per dialect) | 1 model, 5 expected files (postgres, sqlite, d1, and both skip-redundant variants) | ✓ (reference, `migrate-ts`) | — (no CHECK in a generated model) | — (same) | ✓ (the EF context declares them; live-Postgres `EnsureCreated` lane) | — (same) | +| [`fixtures/foreign-key-index-conformance/`](../fixtures/foreign-key-index-conformance/) (the indexes `migrate.foreignKeyIndexes` adds: which foreign keys are uncovered, and each index's name per dialect, a name over 63 bytes cut on Postgres) | 1 model, 2 expected files (postgres, sqlite) | ✓ (reference, `migrate-ts`) | — (no index in a generated model) | ✓ (the Exposed tables declare them) | ✓ (the EF context declares them) | — (same as Java) | | [`fixtures/codegen-noop/`](../fixtures/codegen-noop/) (FR-044 — reporting vocabulary: what is lowered, what stays inert) | 1 model pair (`reporting/with` vs `reporting/without`) | ✓ (codegen + migrate) | ✓ | ✓ | ✓ | ✓ | | [`fixtures/cube-model/`](../fixtures/cube-model/) (FR-044 Plan 4 — the `cube-model` generator's golden Cube files, plus the canonical model's golden) | 46 cases (36 trees + 10 errors) + 1 canonical golden | ✓ (the exporter is TypeScript-only, spec R6; its `cube` lane also loads the files into a real Cube) | — | — | — | — | | [`fixtures/contract-conformance/`](../fixtures/contract-conformance/) (FR-022 — the `contract-schema` and `openapi` generators' golden trees and refusals) | 19 cases (14 trees + 5 errors) | ✓ (one neutral engine, ADR-0034 Amendment 10; the generated API lanes also hold real traffic to the documents) | — | — | — | — | @@ -119,12 +120,15 @@ existing database, the change sets `START WITH n` and moves the counter FORWARD back, because live rows already hold the ids it handed out. Tracked in [#544](https://github.com/metaobjectsdev/metaobjects/issues/544). -- *Honoured and pinned:* TypeScript (`meta migrate`). A fresh table is created `START WITH n`, and - the Drizzle schema template carries the start. A changed start writes `ALTER SEQUENCE … START - WITH n` plus an advance-only `setval`. The persistence migration scenarios - `sequence-start-change-moves-counter-forward` and `sequence-start-change-below-max-id` pin on - live Postgres that the next id is the start, or the highest id plus one when a row already - holds an id past the start. +- *Honoured and pinned:* TypeScript (`meta migrate`). A fresh table is created `START WITH n`. A + changed start writes `ALTER SEQUENCE … START WITH n` plus an advance-only `setval`. The + persistence migration scenarios `sequence-start-change-moves-counter-forward` and + `sequence-start-change-below-max-id` pin on live Postgres that the next id is the start, or the + highest id plus one when a row already holds an id past the start. +- *Honoured, string-tested only:* the Drizzle schema template (`entityFile()`). It writes the + start into the identity column's options (`startWith: n`), and a codegen unit test asserts that + text (`codegen-ts/test/templates/drizzle-schema.test.ts`). No test creates a table from the + Drizzle mapping and reads the first id, so the option's effect is not pinned. - *Honoured and pinned:* C#. The generated context carries the start on Npgsql, and the generated `DesignTimeServices` makes `dotnet ef migrations add` scaffold a start-only change as `SET START WITH n` plus the same advance-only move (Npgsql alone writes `RESTART WITH n`). @@ -423,9 +427,9 @@ All 37 fixtures → [features/migrations-and-drift.md](features/migrations-and-d may carry `"isArray": true`; with `fields` it is a list of objects, without it a list of scalars. The six `verify-positional-accessor-*` cases pin where `-last` is available. -### `fixtures/persistence-conformance/` (64 — 55 query + 9 migration) +### `fixtures/persistence-conformance/` (67 — 55 query + 12 migration) -- `migrations/*` (9) → [features/migrations-and-drift.md](features/migrations-and-drift.md) (schema migration section) +- `migrations/*` (12) → [features/migrations-and-drift.md](features/migrations-and-drift.md) (schema migration section) - `queries/*` (55) → [features/source-kinds.md](features/source-kinds.md) (query semantics against `source.rdb`) **Engines beyond Postgres (FR-044 reports, decimals).** The twenty-four `queries/report-*.yaml` @@ -615,7 +619,13 @@ only `KotlinSharedModelGeneratorTest`. **C# runs all 15**, in `SharedModelConfor `collection-segments.json` (23 cases) pins the REST collection segment from an object's `@collectionName` and the project's `naming.collections` pluralizer; every port runs it (see the -corpus README). The pluralize cases: +corpus README). It holds segment cases only. The collection-name clash refusal +(`ERR_COLLECTION_NAME_COLLISION`, two objects mounting one collection) is not in it: each port +gates the refusal with its own unit tests, which run the corpus's `collisionCases` pairs where +the README's runner table says so. Kotlin has no refusal of its own (the Maven goal runs the +shared JVM `RouteNaming` one before the Kotlin generators), and +`codegen-kotlin`'s `KotlinRouteNamingTest` checks it refuses a model the Kotlin facade loads, +for coinciding plurals and for a declared `@collectionName` that repeats another's segment. The pluralize cases: All 8 cases → the already-plural pluralize fix (CHANGELOG `## [Unreleased]`): an entity whose name is already a plural noun (`Stats`, `Settings`, `Series`) @@ -725,7 +735,7 @@ JSON and verify payload feeds it unchanged. ## Orphaned fixtures (tested but not yet documented) The fixtures in the nine corpora mapped above (metamodel 510 + yaml 16 + verify 45 -+ render 18 + persistence 64 + api-contract 123 + source-resolution 25 + scope 10 + ++ render 18 + persistence 67 + api-contract 123 + source-resolution 25 + scope 10 + dependency 47) each map to a feature doc, and so do the two requirement corpora, `shared-model-conformance`, the `cube-model` corpus and `explorer-conformance`, whose case lists live in their own READMEs. None are orphaned today. The remaining corpora in the totals table gate tooling contracts (registry manifests, provider diff --git a/docs/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 85ab3b758..61712c859 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -570,6 +570,17 @@ primary key. Two foreign keys over the same columns get one index, and a foreign longer one shares the longer one's index. A name that collides with another index is refused with `ERR_DUPLICATE_SQL_NAME`. +On Postgres a name over 63 bytes is cut to its first 63 bytes (on a character boundary), which is +the name Postgres stores for it, so a converged schema compares equal and the next run proposes +nothing. SQLite and D1 have no limit and keep the whole name. Two foreign keys on one table whose +names cut to the same 63 bytes are refused with `ERR_DUPLICATE_SQL_NAME`; cover one of them with +an `index.lookup` of your own naming. The diff also matches a foreign-key constraint by the name +Postgres stores, so a derived `
__fk` over the limit is not proposed as a rename on +every run. The shared corpus +[`fixtures/foreign-key-index-conformance/`](../../fixtures/foreign-key-index-conformance/README.md) +pins which foreign keys get an index and under what name, and the TypeScript, C# and Kotlin rules +all pass it. + It is separate from the `efcore` conventions, which index every foreign key too but also rename the primary-key and foreign-key constraints: a team can have the indexes without the renames. Under `efcore` the option adds nothing, since the preset's `IX_` indexes already cover every @@ -796,14 +807,23 @@ and a `CREATE VIEW`: A drop and a create of the same view needs no `--allow`. It has two costs a replace does not: -- **Privileges granted on the view are discarded.** `DROP VIEW` removes the view's grants with - it, and `meta migrate` writes no `GRANT`. A role that had `SELECT` on the view loses it, and - nothing reports that. In an offline plan every changed Postgres view takes this path. +- **Privileges granted on the view are carried over, but the view gets a new owner.** + `DROP VIEW` removes the view's grants with it, so on Postgres the migration reads them from the + catalog first (the view's own privileges and its column privileges, not the owner's) into a + session temporary table, and after the `CREATE` grants each one again to the same role, `WITH + GRANT OPTION` where it was held. A column privilege is granted again only when the new view + still has that column; one on a column the new definition dropped is gone. The apply log shows + a `NOTICE` with the count carried over. The down migration does the same when it drops and + creates a view. The recreated view is owned by whoever applies the migration, not by the old + view's owner. In an offline plan every changed Postgres view takes this path. These are the + only `GRANT` statements `meta migrate` writes: it declares no privileges of its own (#574 + tracks that). SQLite and D1 have no privileges. - If something depends on the view, Postgres refuses the `DROP VIEW` at apply and names the dependent. -Plan with `--from-db` to get the in-place replace when it is legal, which keeps both. Otherwise -grant the privileges again after the migration is applied. +Plan with `--from-db` to get the in-place replace when it is legal, which keeps the view's +owner and its dependents. Migrations are TypeScript-owned (ADR-0015): no other port writes a +migration, so this is the only port that carries privileges over. A view whose definition does **not** change is dropped and created too, when the migration retypes, drops or renames a column the view reads: Postgres refuses to retype or drop it @@ -900,6 +920,7 @@ The following conformance fixtures gate this feature's behavior across ports: - [`add-nullable-column.yaml`](../../fixtures/persistence-conformance/migrations/add-nullable-column.yaml) — incremental `ALTER TABLE … ADD COLUMN` for a new nullable field - [`drop-table-blocked-without-allow.yaml`](../../fixtures/persistence-conformance/migrations/drop-table-blocked-without-allow.yaml) — destructive operations require an explicit allow-flag - [`foreign-key-indexes-added.yaml`](../../fixtures/persistence-conformance/migrations/foreign-key-indexes-added.yaml) — `migrate.foreignKeyIndexes` turned on for an existing schema adds one `
__idx` per uncovered foreign key and nothing else +- [`foreign-key-index-long-name-added.yaml`](../../fixtures/persistence-conformance/migrations/foreign-key-index-long-name-added.yaml) and [`noop-converged-long-foreign-key-index-name.yaml`](../../fixtures/persistence-conformance/migrations/noop-converged-long-foreign-key-index-name.yaml) — a foreign-key index name over Postgres's 63-byte limit is created under its cut name and the next run proposes nothing - [`noop-converged-foreign-key-indexes.yaml`](../../fixtures/persistence-conformance/migrations/noop-converged-foreign-key-indexes.yaml) — with the option on, a converged schema emits zero SQL (the indexes introspect back exactly as emitted) - [`noop-converged-canonical.yaml`](../../fixtures/persistence-conformance/migrations/noop-converged-canonical.yaml) — **the idempotence gate**: apply the whole canonical schema, then diff the same metadata against the database it just produced. A converged schema must emit **zero** SQL. This is what catches any asymmetry between what `emit` writes, what `introspect` reads back, and what the expected schema models — the class of bug that makes `meta migrate` re-propose (and on SQLite, destructively rebuild) unchanged tables forever. diff --git a/fixtures/foreign-key-index-conformance/README.md b/fixtures/foreign-key-index-conformance/README.md new file mode 100644 index 000000000..ec3d95a26 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/README.md @@ -0,0 +1,39 @@ +# Foreign-key index conformance + +The indexes `migrate.foreignKeyIndexes` gives a model's foreign keys, pinned by name and column +list, per dialect. A port that declares these indexes in its own generated model must declare +exactly these, so the mapping and the database `meta migrate` builds agree. The rule for which +foreign keys are uncovered is written once per port (TypeScript, C#, Kotlin); this corpus is +what holds the three to one answer. + +## Layout + +- `input/meta.fk.json`: the model. + - `orders`: two foreign keys get an index; a third is covered by a declared `index.lookup` + that leads with its column. + - `order_tags`: a junction whose composite primary key covers its first foreign key. + - `shipments`: a `@unique` field covers a foreign key; two foreign keys over one column get one + index; a partial `index.lookup` (`@where`) covers a foreign key; a single-column foreign key + that leads a composite one shares the composite one's index. The composite reference carries + a `@constraintName`, because the default `
__fk` name would collide with + the single-column one. + - `customer_order_line_item_allocations_by_region`: a foreign key whose index name is over + 63 bytes. +- `expected..json`: `{ "
": [{ "name", "columns" }] }` for every table that gets + a foreign-key index, in the order `meta migrate` adds them. `postgres` cuts a name over 63 + bytes to its first 63 bytes (the name Postgres stores); `sqlite` keeps the whole name. + +## Runners + +- TypeScript: `server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts` + builds the expected schema and compares each file. It is the reference: + `META_WRITE_FK_INDEXES=1 bun test test/foreign-key-index-conformance.test.ts` (in + `migrate-ts`) rewrites the files after a deliberate change, which is then reviewed by hand. +- C#: `server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs` reads + `ForeignKeyIndexes.For`, the rule the generated EF context declares from. +- Kotlin: `server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt` + reads the Exposed table generator's `foreignKeyIndexes`. + +Java and Python declare no index in a generated model (schema comes from `meta migrate`, +ADR-0015), so they do not run it. The `efcore` conventions and MySQL add no index of this kind +in any port and are not in the corpus. diff --git a/fixtures/foreign-key-index-conformance/expected.postgres.json b/fixtures/foreign-key-index-conformance/expected.postgres.json new file mode 100644 index 000000000..aaf4f09b1 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/expected.postgres.json @@ -0,0 +1,53 @@ +{ + "customer_order_line_item_allocations_by_region": [ + { + "name": "customer_order_line_item_allocations_by_region_fulfilment_wareh", + "columns": [ + "fulfilment_warehouse_reference_id" + ] + }, + { + "name": "customer_order_line_item_allocations_by_region_order_id_idx", + "columns": [ + "order_id" + ] + } + ], + "order_tags": [ + { + "name": "order_tags_tag_id_idx", + "columns": [ + "tag_id" + ] + } + ], + "orders": [ + { + "name": "orders_customer_id_idx", + "columns": [ + "customer_id" + ] + }, + { + "name": "orders_customer_code_idx", + "columns": [ + "customer_code" + ] + } + ], + "shipments": [ + { + "name": "shipments_carrier_id_idx", + "columns": [ + "carrier_id" + ] + }, + { + "name": "shipments_country_region_code_idx", + "columns": [ + "country", + "region_code" + ] + } + ] +} diff --git a/fixtures/foreign-key-index-conformance/expected.sqlite.json b/fixtures/foreign-key-index-conformance/expected.sqlite.json new file mode 100644 index 000000000..b071bfbf9 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/expected.sqlite.json @@ -0,0 +1,53 @@ +{ + "customer_order_line_item_allocations_by_region": [ + { + "name": "customer_order_line_item_allocations_by_region_fulfilment_warehouse_reference_id_idx", + "columns": [ + "fulfilment_warehouse_reference_id" + ] + }, + { + "name": "customer_order_line_item_allocations_by_region_order_id_idx", + "columns": [ + "order_id" + ] + } + ], + "order_tags": [ + { + "name": "order_tags_tag_id_idx", + "columns": [ + "tag_id" + ] + } + ], + "orders": [ + { + "name": "orders_customer_id_idx", + "columns": [ + "customer_id" + ] + }, + { + "name": "orders_customer_code_idx", + "columns": [ + "customer_code" + ] + } + ], + "shipments": [ + { + "name": "shipments_carrier_id_idx", + "columns": [ + "carrier_id" + ] + }, + { + "name": "shipments_country_region_code_idx", + "columns": [ + "country", + "region_code" + ] + } + ] +} diff --git a/fixtures/foreign-key-index-conformance/input/meta.fk.json b/fixtures/foreign-key-index-conformance/input/meta.fk.json new file mode 100644 index 000000000..8303c18b6 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/input/meta.fk.json @@ -0,0 +1,71 @@ +{ + "metadata.root": { + "package": "acme", + "children": [ + { "object.entity": { "name": "Customer", "children": [ + { "source.rdb": { "@table": "customers" } }, + { "field.long": { "name": "id" } }, + { "field.string": { "name": "code" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } }, + { "identity.secondary": { "name": "customers_code_key", "@fields": ["code"] } } + ] } }, + { "object.entity": { "name": "Order", "children": [ + { "source.rdb": { "@table": "orders" } }, + { "field.long": { "name": "id" } }, + { "field.long": { "name": "customerId", "@column": "customer_id" } }, + { "field.long": { "name": "billToId", "@column": "bill_to_id" } }, + { "field.string": { "name": "customerCode", "@column": "customer_code" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } }, + { "identity.reference": { "name": "customer", "@fields": ["customerId"], "@references": "Customer" } }, + { "identity.reference": { "name": "billTo", "@fields": ["billToId"], "@references": "Customer" } }, + { "identity.reference": { "name": "byCode", "@fields": ["customerCode"], "@references": "Customer.code" } }, + { "index.lookup": { "name": "orders_bill_to", "@fields": ["billToId", "id"] } } + ] } }, + { "object.entity": { "name": "Tag", "children": [ + { "source.rdb": { "@table": "tags" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } } + ] } }, + { "object.entity": { "name": "OrderTag", "children": [ + { "source.rdb": { "@table": "order_tags" } }, + { "field.long": { "name": "orderId", "@column": "order_id" } }, + { "field.long": { "name": "tagId", "@column": "tag_id" } }, + { "identity.primary": { "name": "pk", "@fields": ["orderId", "tagId"] } }, + { "identity.reference": { "name": "order", "@fields": ["orderId"], "@references": "Order" } }, + { "identity.reference": { "name": "tag", "@fields": ["tagId"], "@references": "Tag" } } + ] } }, + { "object.entity": { "name": "Region", "children": [ + { "source.rdb": { "@table": "regions" } }, + { "field.string": { "name": "country" } }, + { "field.string": { "name": "code" } }, + { "identity.primary": { "name": "pk", "@fields": ["country", "code"] } } + ] } }, + { "object.entity": { "name": "Shipment", "children": [ + { "source.rdb": { "@table": "shipments" } }, + { "field.long": { "name": "id" } }, + { "field.long": { "name": "orderId", "@column": "order_id", "@unique": true } }, + { "field.long": { "name": "carrierId", "@column": "carrier_id" } }, + { "field.long": { "name": "returnCarrierId", "@column": "return_carrier_id" } }, + { "field.string": { "name": "country" } }, + { "field.string": { "name": "regionCode", "@column": "region_code" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } }, + { "identity.reference": { "name": "order", "@fields": ["orderId"], "@references": "Order" } }, + { "identity.reference": { "name": "carrier", "@fields": ["carrierId"], "@references": "Customer" } }, + { "identity.reference": { "name": "carrierAgain", "@fields": ["carrierId"], "@references": "Customer" } }, + { "identity.reference": { "name": "returnCarrier", "@fields": ["returnCarrierId"], "@references": "Customer" } }, + { "identity.reference": { "name": "countryOnly", "@fields": ["country"], "@references": "Customer.code" } }, + { "identity.reference": { "name": "region", "@fields": ["country", "regionCode"], "@references": "Region.country,code", "@constraintName": "shipments_region_fk" } }, + { "index.lookup": { "name": "shipments_open_returns", "@fields": ["returnCarrierId"], "@where": "return_carrier_id IS NOT NULL" } } + ] } }, + { "object.entity": { "name": "Allocation", "children": [ + { "source.rdb": { "@table": "customer_order_line_item_allocations_by_region" } }, + { "field.long": { "name": "id" } }, + { "field.long": { "name": "fulfilmentWarehouseReferenceId", "@column": "fulfilment_warehouse_reference_id" } }, + { "field.long": { "name": "orderId", "@column": "order_id" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } }, + { "identity.reference": { "name": "warehouse", "@fields": ["fulfilmentWarehouseReferenceId"], "@references": "Customer" } }, + { "identity.reference": { "name": "order", "@fields": ["orderId"], "@references": "Order" } } + ] } } + ] + } +} diff --git a/fixtures/naming-conformance/README.md b/fixtures/naming-conformance/README.md index 50ec72bcf..09144db3a 100644 --- a/fixtures/naming-conformance/README.md +++ b/fixtures/naming-conformance/README.md @@ -136,7 +136,7 @@ checks, and asserts each `shapeOnlyCases` pair is ACCEPTED: |---|---| | TypeScript | `server/typescript/packages/metadata/test/naming.test.ts` (pluralizers); `server/typescript/packages/codegen-ts/test/naming/collection-name-collision.test.ts` (refusal — `assertNoCollectionNameCollisions`, wired into `runGen` in `runner.ts`) | | C# | `server/csharp/MetaObjects.Codegen.Tests/CSharpNamingTests.cs` (pluralizers + refusal — `CSharpNaming.AssertNoCollectionNameCollisions`, wired into `CodegenRunner.Run`); `server/csharp/MetaObjects.Codegen.Tests/DbContextForeignKeyConfigTests.cs` (the historical Address/Addresses fixture, now an expected-error test at both the unit and `CodegenRunner.Run` integration level) | -| Java / Kotlin | `server/java/codegen-base/src/test/java/com/metaobjects/generator/util/RouteNamingTest.java` (pluralizers + refusal — `RouteNaming.assertNoCollectionNameCollisions`, wired into `MetaDataGeneratorMojo#executeGenerators`; Kotlin inherits both — it runs through the same Maven goal) | +| Java / Kotlin | `server/java/codegen-base/src/test/java/com/metaobjects/generator/util/RouteNamingTest.java` (pluralizers + refusal — `RouteNaming.assertNoCollectionNameCollisions`, wired into `MetaDataGeneratorMojo#executeGenerators` and the codegen mode of `MetaDataVerifyMojo`; Kotlin inherits both — it runs through the same Maven goals — and `server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRouteNamingTest.kt` checks the refusal on models the Kotlin facade loads) | | Python | `server/python/tests/codegen/test_route_path_naming.py` (pluralizers); `server/python/tests/codegen/test_collection_name_collision.py` (refusal — `assert_no_collection_name_collisions`, wired into `run_gen` in `runner.py`) | No HTTP server, no database — this corpus is intentionally a pure function diff --git a/fixtures/persistence-conformance/migrations/foreign-key-index-long-name-added.yaml b/fixtures/persistence-conformance/migrations/foreign-key-index-long-name-added.yaml new file mode 100644 index 000000000..bc81d6d94 --- /dev/null +++ b/fixtures/persistence-conformance/migrations/foreign-key-index-long-name-added.yaml @@ -0,0 +1,23 @@ +name: foreign-key-index-long-name-added +description: | + `migrate.foreignKeyIndexes` on a foreign key whose `
__idx` name is over + Postgres's 63-byte identifier limit. The name is cut to the first 63 bytes, which is the name + Postgres stores, so the index the up migration creates reads back under the name migrate + expects (noop-converged-long-foreign-key-index-name proves the next run proposes nothing). + +seed-metadata: ./states/long-fk-index-names/ +target-metadata: ./states/long-fk-index-names/ +schema-settings: + foreignKeyIndexes: true + +expect: + blocked: [] + up-contains: + - 'CREATE INDEX "customer_order_line_item_allocations_by_region_fulfilment_wareh" ON "customer_order_line_item_allocations_by_region" ("fulfilment_warehouse_reference_id");' + apply-up-then-query: + sql: | + SELECT indexname FROM pg_indexes + WHERE schemaname = 'public' AND tablename = 'customer_order_line_item_allocations_by_region' + AND indexname <> 'customer_order_line_item_allocations_by_region_pkey' + rows: + - { indexname: "customer_order_line_item_allocations_by_region_fulfilment_wareh" } diff --git a/fixtures/persistence-conformance/migrations/noop-converged-long-foreign-key-index-name.yaml b/fixtures/persistence-conformance/migrations/noop-converged-long-foreign-key-index-name.yaml new file mode 100644 index 000000000..1e6a098dc --- /dev/null +++ b/fixtures/persistence-conformance/migrations/noop-converged-long-foreign-key-index-name.yaml @@ -0,0 +1,17 @@ +name: noop-converged-long-foreign-key-index-name +description: | + IDEMPOTENCE GATE for a `migrate.foreignKeyIndexes` name over Postgres's 63-byte limit: the + schema built WITH the option, then diffed against the same metadata with the option still on, + must produce zero SQL. Postgres stores a longer name cut to 63 bytes and the diff compares + names exactly, so an uncut name would be proposed again on every migrate. + +seed-metadata: ./states/long-fk-index-names/ +seed-schema-settings: + foreignKeyIndexes: true +target-metadata: ./states/long-fk-index-names/ +schema-settings: + foreignKeyIndexes: true + +expect: + blocked: [] + up-empty: true diff --git a/fixtures/persistence-conformance/migrations/states/long-fk-index-names/meta.json b/fixtures/persistence-conformance/migrations/states/long-fk-index-names/meta.json new file mode 100644 index 000000000..f9dd5e1d9 --- /dev/null +++ b/fixtures/persistence-conformance/migrations/states/long-fk-index-names/meta.json @@ -0,0 +1,25 @@ +{ + "metadata.root": { + "package": "fulfilment", + "children": [ + { "object.entity": { + "name": "Warehouse", + "children": [ + { "source.rdb": { "@table": "warehouses" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "name": "pk", "@fields": "id", "@generation": "increment" } } + ] + }}, + { "object.entity": { + "name": "Allocation", + "children": [ + { "source.rdb": { "@table": "customer_order_line_item_allocations_by_region" } }, + { "field.long": { "name": "id" } }, + { "field.long": { "name": "fulfilmentWarehouseReferenceId", "@column": "fulfilment_warehouse_reference_id", "@required": true } }, + { "identity.primary": { "name": "pk", "@fields": "id", "@generation": "increment" } }, + { "identity.reference": { "name": "warehouse", "@fields": ["fulfilmentWarehouseReferenceId"], "@references": "Warehouse" } } + ] + }} + ] + } +} diff --git a/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs b/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs new file mode 100644 index 000000000..0a5c54c07 --- /dev/null +++ b/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs @@ -0,0 +1,55 @@ +// The shared foreign-key index corpus, fixtures/foreign-key-index-conformance/: which foreign +// keys `migrate.foreignKeyIndexes` indexes and under what name, per dialect. The TypeScript +// migrate suite writes and passes the same expected files, so the EF model declares exactly the +// indexes `meta migrate` creates. + +using System.Text.Json; +using MetaObjects.Codegen.Generators; +using MetaObjects.Loader; +using MetaObjects.Meta; +using Xunit; + +namespace MetaObjects.Codegen.Tests; + +public class ForeignKeyIndexConformanceTests +{ + private static string Corpus() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !Directory.Exists(Path.Combine(dir.FullName, "fixtures"))) + dir = dir.Parent; + Assert.NotNull(dir); + return Path.Combine(dir!.FullName, "fixtures", "foreign-key-index-conformance"); + } + + public static TheoryData Cases => new() + { + { "expected.postgres.json", "postgres" }, + { "expected.sqlite.json", "sqlite" }, + }; + + [Theory] + [MemberData(nameof(Cases))] + public void The_indexes_are_meta_migrates(string file, string dialect) + { + var load = MetaDataLoader.FromDirectory(Path.Combine(Corpus(), "input")); + Assert.Empty(load.Errors); + var config = new GenConfig { OutDir = "/unused", Namespace = "Acme", ForeignKeyIndexes = true, Dialect = dialect }; + var actual = load.Root.Objects() + .Where(o => o.IsEntity() && o.DbTable is not null) + .Select(o => (Table: CSharpNaming.Table(o), Indexes: ForeignKeyIndexes.For(o, TphPlanBuilder.For(o, load.Root), config))) + .Where(t => t.Indexes.Count > 0) + .ToDictionary(t => t.Table, t => t.Indexes.Select(i => $"{i.Name}({string.Join(",", i.Columns)})").ToList()); + + using var doc = JsonDocument.Parse(File.ReadAllText(Path.Combine(Corpus(), file))); + var expected = doc.RootElement.EnumerateObject().ToDictionary( + t => t.Name, + t => t.Value.EnumerateArray() + .Select(i => $"{i.GetProperty("name").GetString()}({string.Join(",", i.GetProperty("columns").EnumerateArray().Select(c => c.GetString()))})") + .ToList()); + + Assert.Equal(expected.Keys.OrderBy(k => k, StringComparer.Ordinal), actual.Keys.OrderBy(k => k, StringComparer.Ordinal)); + foreach (var (table, indexes) in expected) + Assert.Equal(indexes, actual[table]); + } +} diff --git a/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs index 6f0556357..83754cdd0 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs @@ -1372,94 +1372,19 @@ private void EmitIndexConfig(string owner, MetaObject entity, GenContext ctx, Li /// /// migrate.foreignKeyIndexes — the index meta migrate gives each enforced foreign /// key that nothing already leads with, so the model declares the index the database has, - /// under migrate's name (<table>_<column>[_<column>…]_idx). The - /// rule is migrate's own (uncoveredForeignKeyColumns in migrate-ts): the primary key, - /// a field's @unique, an identity.secondary and an index.lookup without - /// @expr cover a foreign key whose columns they lead with, in any order; two foreign - /// keys over one column set get one index; and a foreign key that leads a longer one shares - /// the longer one's index. Nothing under the efcore conventions, which already index - /// every foreign key with EF Core's own names, and nothing on MySQL, where InnoDB indexes each - /// foreign key itself. A TPH table is read whole: the base and every concrete subtype. + /// under migrate's name. The rule is . /// private void EmitForeignKeyIndexConfig(MetaObject entity, TphPlan? tph, GenContext ctx, List modelLines) { - var config = ctx.Config; - if (!config.ForeignKeyIndexes - || string.Equals(config.SchemaConventions, "efcore", StringComparison.Ordinal) - || string.Equals(config.Dialect, "mysql", StringComparison.OrdinalIgnoreCase)) - return; - var strategy = config.ColumnNamingStrategy; - var members = new List { entity }; - if (tph is not null) members.AddRange(tph.Subtypes.Select(st => st.Entity)); - - List? Columns(MetaObject owner, IEnumerable fieldNames) - { - var cols = new List(); - foreach (var name in fieldNames) - { - if (owner.FindField(name) is not { } f) return null; - cols.Add(CSharpNaming.Column(f, strategy)); - } - return cols; - } - - var existing = new List>(); - if (entity.PrimaryIdentity()?.Fields is { Count: > 0 } pk && Columns(entity, pk) is { } pkCols) existing.Add(pkCols); - foreach (var m in members) - { - // ADR-0039: resolving — @unique and index nodes may be inherited via extends. - foreach (var f in m.Fields().Where(f => f.Attr(FIELD_ATTR_UNIQUE) is true)) - existing.Add([CSharpNaming.Column(f, strategy)]); - foreach (var (node, fields) in m.SecondaryIdentities().Select(i => ((MetaData)i, i.Fields)) - .Concat(m.LookupIndexes().Select(i => ((MetaData)i, i.Fields)))) - { - if (node.Attr(DbConstants.IDENTITY_ATTR_EXPR) is string { Length: > 0 } || fields.Count == 0) continue; - if (Columns(m, fields) is { } cols) existing.Add(cols); - } - } - - // Each physical foreign key once, on the first class of the table that declares it. - var seen = new HashSet(StringComparer.Ordinal); - var candidates = new List<(MetaObject Owner, List Fields, List Columns)>(); - foreach (var m in members) + foreach (var index in ForeignKeyIndexes.For(entity, tph, ctx.Config)) { - foreach (var reference in m.ReferenceIdentities().Where(r => r.Enforce)) - { - if (!seen.Add(FkKey(reference))) continue; - var fields = new List(); - foreach (var name in ReferentialActions.ReadIdentityFields(reference)) - { - if (m.FindField(name) is not { } f) { fields.Clear(); break; } - fields.Add(f); - } - if (fields.Count == 0) continue; - var cols = fields.Select(f => CSharpNaming.Column(f, strategy)).ToList(); - if (existing.Any(e => LeadsWith(e, cols))) continue; - if (candidates.Any(c => c.Columns.Count == cols.Count && LeadsWith(c.Columns, cols))) continue; - candidates.Add((m, fields, cols)); - } - } - - var table = CSharpNaming.Table(entity); - foreach (var (owner, fields, cols) in candidates) - { - if (candidates.Any(o => o.Columns.Count > cols.Count && LeadsWith(o.Columns, cols))) continue; - var name = $"\"{CSharpNaming.CSharpStringLiteral($"{table}_{string.Join("_", cols)}_idx")}\""; - var props = fields.Select(PropertyName).ToList(); + var name = $"\"{CSharpNaming.CSharpStringLiteral(index.Name)}\""; + var props = index.Fields.Select(PropertyName).ToList(); var selector = props.Count == 1 ? $"x => x.{props[0]}" : $"x => new {{ {string.Join(", ", props.Select(p => "x." + p))} }}"; - modelLines.Add($" modelBuilder.Entity<{CSharpNaming.Pascal(owner.Name)}>().HasIndex({selector}, {name}).HasDatabaseName({name});"); + modelLines.Add($" modelBuilder.Entity<{CSharpNaming.Pascal(index.Owner.Name)}>().HasIndex({selector}, {name}).HasDatabaseName({name});"); } } - /// True when leads with exactly , - /// in any order (migrate's leadsWith). - private static bool LeadsWith(IReadOnlyList prefix, IReadOnlyList columns) - { - if (columns.Count == 0 || prefix.Count < columns.Count) return false; - var want = new HashSet(columns, StringComparer.Ordinal); - return prefix.Take(columns.Count).All(want.Contains); - } - // The SQL expressions migrate treats as an expression default (expected-schema.ts // EXPR_DEFAULT_PATTERNS): the rest of a string default is a literal. private static readonly System.Text.RegularExpressions.Regex ExprDefault = new( diff --git a/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs b/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs new file mode 100644 index 000000000..8d6d5959f --- /dev/null +++ b/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs @@ -0,0 +1,129 @@ +// `migrate.foreignKeyIndexes`: the index `meta migrate` gives each enforced foreign key that +// nothing already leads with, under migrate's name, so the generated EF model declares the index +// the database has. Ported from migrate-ts's expected-schema.ts (uncoveredForeignKeyColumns, +// foreignKeyIndexName); the shared corpus fixtures/foreign-key-index-conformance/ pins this port, +// TypeScript and Kotlin to the same answer. + +using System.Text; +using MetaObjects.Meta; +using MetaObjects.Persistence.Db; +using static MetaObjects.Core.Field.FieldConstants; + +namespace MetaObjects.Codegen.Generators; + +/// One foreign-key index: its name, the class that declares the foreign key, its fields +/// and their columns. +public sealed record ForeignKeyIndex(string Name, MetaObject Owner, IReadOnlyList Fields, IReadOnlyList Columns); + +/// The foreign-key indexes of an entity's table, as meta migrate adds them. +public static class ForeignKeyIndexes +{ + /// Postgres's identifier limit in bytes; a longer name is stored cut to it. + private const int PgIdentifierMaxBytes = 63; + + /// + /// The rule is migrate's own: the primary key, a field's @unique, an + /// identity.secondary and an index.lookup without @expr cover a foreign + /// key whose columns they lead with, in any order; two foreign keys over one column set get + /// one index; and a foreign key that leads a longer one shares the longer one's index. Empty + /// unless is set, under the efcore + /// conventions (which already index every foreign key with EF Core's own names) and on MySQL + /// (InnoDB indexes each foreign key itself). A TPH table is read whole: the base and every + /// concrete subtype. + /// + public static IReadOnlyList For(MetaObject entity, TphPlan? tph, GenConfig config) + { + if (!config.ForeignKeyIndexes + || string.Equals(config.SchemaConventions, "efcore", StringComparison.Ordinal) + || string.Equals(config.Dialect, "mysql", StringComparison.OrdinalIgnoreCase)) + return []; + var strategy = config.ColumnNamingStrategy; + var members = new List { entity }; + if (tph is not null) members.AddRange(tph.Subtypes.Select(st => st.Entity)); + + List? Columns(MetaObject owner, IEnumerable fieldNames) + { + var cols = new List(); + foreach (var name in fieldNames) + { + if (owner.FindField(name) is not { } f) return null; + cols.Add(CSharpNaming.Column(f, strategy)); + } + return cols; + } + + var existing = new List>(); + if (entity.PrimaryIdentity()?.Fields is { Count: > 0 } pk && Columns(entity, pk) is { } pkCols) existing.Add(pkCols); + foreach (var m in members) + { + // ADR-0039: resolving — @unique and index nodes may be inherited via extends. + foreach (var f in m.Fields().Where(f => f.Attr(FIELD_ATTR_UNIQUE) is true)) + existing.Add([CSharpNaming.Column(f, strategy)]); + foreach (var (node, fields) in m.SecondaryIdentities().Select(i => ((MetaData)i, i.Fields)) + .Concat(m.LookupIndexes().Select(i => ((MetaData)i, i.Fields)))) + { + if (node.Attr(DbConstants.IDENTITY_ATTR_EXPR) is string { Length: > 0 } || fields.Count == 0) continue; + if (Columns(m, fields) is { } cols) existing.Add(cols); + } + } + + // Each physical foreign key once, on the first class of the table that declares it. + var seen = new HashSet(StringComparer.Ordinal); + var candidates = new List<(MetaObject Owner, List Fields, List Columns)>(); + foreach (var m in members) + { + foreach (var reference in m.ReferenceIdentities().Where(r => r.Enforce)) + { + var fieldNames = ReferentialActions.ReadIdentityFields(reference); + if (!seen.Add(string.Join(",", fieldNames))) continue; + var fields = new List(); + foreach (var name in fieldNames) + { + if (m.FindField(name) is not { } f) { fields.Clear(); break; } + fields.Add(f); + } + if (fields.Count == 0) continue; + var cols = fields.Select(f => CSharpNaming.Column(f, strategy)).ToList(); + if (existing.Any(e => LeadsWith(e, cols))) continue; + if (candidates.Any(c => c.Columns.Count == cols.Count && LeadsWith(c.Columns, cols))) continue; + candidates.Add((m, fields, cols)); + } + } + + var table = CSharpNaming.Table(entity); + return candidates + .Where(c => !candidates.Any(o => o.Columns.Count > c.Columns.Count && LeadsWith(o.Columns, c.Columns))) + .Select(c => new ForeignKeyIndex(Name(table, c.Columns, config.Dialect), c.Owner, c.Fields, c.Columns)) + .ToList(); + } + + /// + /// <table>_<column>[_<column>…]_idx. Except on SQLite and D1, a name over 63 + /// bytes is cut to its first 63 bytes on a character boundary: the name Postgres stores, and + /// the one meta migrate expects. + /// + public static string Name(string table, IReadOnlyList columns, string? dialect) + { + var name = $"{table}_{string.Join("_", columns)}_idx"; + if (dialect is "sqlite" or "d1" || Encoding.UTF8.GetByteCount(name) <= PgIdentifierMaxBytes) return name; + var sb = new StringBuilder(); + var bytes = 0; + foreach (var rune in name.EnumerateRunes()) + { + var n = rune.Utf8SequenceLength; + if (bytes + n > PgIdentifierMaxBytes) break; + sb.Append(rune.ToString()); + bytes += n; + } + return sb.ToString(); + } + + /// True when leads with exactly , + /// in any order (migrate's leadsWith). + internal static bool LeadsWith(IReadOnlyList prefix, IReadOnlyList columns) + { + if (columns.Count == 0 || prefix.Count < columns.Count) return false; + var want = new HashSet(columns, StringComparer.Ordinal); + return prefix.Take(columns.Count).All(want.Contains); + } +} diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt index 795e1690b..a08bb6aeb 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt @@ -1221,9 +1221,9 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase_[_…]_idx`). The rule is + * nothing already leads with, so the table declares the index the database has under + * migrate's name (`
_[_…]_idx`, cut by [foreignKeyIndexName]). The rule is * migrate's own (`uncoveredForeignKeyColumns` in migrate-ts): the primary key, a field's * `@unique`, an `identity.secondary` and an `index.lookup` without `@expr` cover a foreign * key whose columns they lead with, in any order; two foreign keys over one column set get one * index; and one that leads a longer one shares the longer one's index. Empty unless the * project sets the option, and under the `efcore` conventions or on MySQL - * ([MigrateSettings.indexesForeignKeys]). A TPH table is read whole: base and subtypes. + * ([MigrateSettings.indexesForeignKeys]). A TPH table is read whole: base and subtypes. The + * shared corpus fixtures/foreign-key-index-conformance/ pins this to migrate's answer. */ - protected fun foreignKeyIndexes(entity: MetaObject, loader: MetaDataLoader, tableName: String): List>> { - if (!MigrateSettings.of(entity).indexesForeignKeys()) return emptyList() + protected fun foreignKeyIndexes(entity: MetaObject, loader: MetaDataLoader, tableName: String): List { + val settings = MigrateSettings.of(entity) + if (!settings.indexesForeignKeys()) return emptyList() val members = listOf(entity) + KotlinTphPlan.planFor(entity, loader)?.subtypes.orEmpty().map { it.entity } // ADR-0039: resolving — fields, identities, index nodes and @unique may be inherited. fun field(owner: MetaObject, name: String): MetaField<*>? = owner.metaFields.firstOrNull { it.name == name } @@ -1386,7 +1388,7 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase candidates.none { (o, _) -> o.size > cols.size && leadsWith(o, cols) } } - .map { (cols, fields) -> "${tableName}_${cols.joinToString("_")}_idx" to fields } + .map { (cols, fields) -> ForeignKeyIndex(foreignKeyIndexName(tableName, cols, settings.dialect), cols, fields) } } protected fun indexNameExpr(node: MetaData, shortName: String): String { @@ -2780,3 +2782,32 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase, val fields: List) + +/** Postgres's identifier limit in bytes; a longer name is stored cut to it. */ +private const val PG_IDENTIFIER_MAX_BYTES = 63 + +/** + * `
_[_…]_idx`, migrate's name for a foreign key's index. Except on SQLite + * and D1, a name over 63 bytes is cut to its first 63 bytes on a character boundary: the name + * Postgres stores, and the one `meta migrate` expects. + */ +fun foreignKeyIndexName(table: String, columns: List, dialect: String?): String { + val name = "${table}_${columns.joinToString("_")}_idx" + if (dialect == "sqlite" || dialect == "d1" || name.toByteArray(Charsets.UTF_8).size <= PG_IDENTIFIER_MAX_BYTES) return name + val out = StringBuilder() + var bytes = 0 + var i = 0 + while (i < name.length) { + val cp = name.codePointAt(i) + val ch = String(Character.toChars(cp)) + val n = ch.toByteArray(Charsets.UTF_8).size + if (bytes + n > PG_IDENTIFIER_MAX_BYTES) break + out.append(ch) + bytes += n + i += Character.charCount(cp) + } + return out.toString() +} diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt new file mode 100644 index 000000000..2bb223c1b --- /dev/null +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt @@ -0,0 +1,52 @@ +package com.metaobjects.generator.kotlin + +import com.fasterxml.jackson.databind.ObjectMapper +import com.metaobjects.config.MigrateSettings +import com.metaobjects.loader.MetaDataLoader +import com.metaobjects.metadata.ktx.loadString +import com.metaobjects.`object`.MetaObject +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * The shared foreign-key index corpus, `fixtures/foreign-key-index-conformance/`: which foreign + * keys `migrate.foreignKeyIndexes` indexes and under what name, per dialect. The TypeScript + * migrate suite writes and passes the same expected files, so the Exposed table declares exactly + * the indexes `meta migrate` creates. + */ +class KotlinForeignKeyIndexConformanceTest { + + private val corpus: Path = Path.of(System.getProperty("user.dir")) + .resolve("../../..").normalize().resolve("fixtures/foreign-key-index-conformance") + + /** Opens the generator's own rule to the test. */ + private class Probe : KotlinExposedTableGenerator() { + fun indexes(entity: MetaObject, loader: MetaDataLoader, table: String) = foreignKeyIndexes(entity, loader, table) + } + + private fun check(file: String, dialect: String) { + val input = corpus.resolve("input/meta.fk.json") + assertTrue(Files.exists(input), "shared fixture missing at $input") + val loader = loadString("fk-index-conformance", Files.readString(input)) + loader.root.setMigrateSettings(MigrateSettings(true, null, dialect)) + val probe = Probe() + val actual = sortedMapOf>() + for (entity in loader.metaObjects) { + val table = KotlinGenUtil.primaryRdbSource(entity)?.physicalName ?: continue + val indexes = probe.indexes(entity, loader, table) + if (indexes.isNotEmpty()) actual[table] = indexes.map { "${it.name}(${it.columns.joinToString(",")})" } + } + val expected = sortedMapOf>() + for ((table, list) in ObjectMapper().readTree(corpus.resolve(file).toFile()).fields()) { + expected[table] = list.map { ix -> "${ix["name"].asText()}(${ix["columns"].joinToString(",") { it.asText() }})" } + } + assertEquals(expected, actual) + } + + @Test fun `postgres - the indexes are meta migrate's`() = check("expected.postgres.json", "postgres") + + @Test fun `sqlite - the indexes are meta migrate's`() = check("expected.sqlite.json", "sqlite") +} diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRouteNamingTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRouteNamingTest.kt index 69e06b399..82e9bf9b8 100644 --- a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRouteNamingTest.kt +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRouteNamingTest.kt @@ -1,10 +1,14 @@ package com.metaobjects.generator.kotlin import com.metaobjects.database.PhysicalNaming +import com.metaobjects.generator.GeneratorException +import com.metaobjects.generator.util.GeneratorUtil +import com.metaobjects.generator.util.RouteNaming import com.metaobjects.metadata.ktx.loadString import java.nio.file.Files import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertTrue /** @@ -77,4 +81,42 @@ class KotlinRouteNamingTest { assertEquals("/api/author", routeFor("", "none")) assertEquals("/api/people", routeFor(declared, "none")) } + + private fun twoEntities(a: String, aExtra: String, b: String, bExtra: String) = """{ + "metadata.root": { "package": "acme::shop", "children": [ + { "object.entity": { "name": "$a", $aExtra "children": [ + { "field.long": { "name": "id" } }, + { "source.rdb": { "@table": "t_${a.lowercase()}" } }, + { "identity.primary": { "name": "pk", "@fields": ["id"] } } + ] } }, + { "object.entity": { "name": "$b", $bExtra "children": [ + { "field.long": { "name": "id" } }, + { "source.rdb": { "@table": "t_${b.lowercase()}" } }, + { "identity.primary": { "name": "pk", "@fields": ["id"] } } + ] } } + ] } + }""".trimIndent() + + /** + * The collection-name clash, on a model the Kotlin facade loads: two entities whose + * controllers would mount one REST collection are refused (ERR_COLLECTION_NAME_COLLISION), + * whether the plurals coincide or a declared `@collectionName` repeats another's segment. + * The refusal is the shared JVM one the Maven goal runs before the Kotlin generators. + */ + @Test + fun `two entities claiming one collection are refused`() { + for ((label, json) in listOf( + "plurals coincide" to twoEntities("Address", "", "Addresses", ""), + "declared name repeats a segment" to twoEntities("Person", "", "Member", "\"@collectionName\": \"Persons\","), + )) { + val loader = loadString("clash", json) + val own = GeneratorUtil.ownMetaObjects(loader) + val e = assertFailsWith(label) { RouteNaming.assertNoCollectionNameCollisions(own) } + assertTrue("ERR_COLLECTION_NAME_COLLISION" in e.message!!, "$label: ${e.message}") + val segments = own.map { KotlinNaming.controllerPath(it) }.toSet() + assertEquals(1, segments.size, "$label: the Kotlin controllers would share one path, saw $segments") + } + // Distinct segments load and pass. + RouteNaming.assertNoCollectionNameCollisions(GeneratorUtil.ownMetaObjects(loadString("ok", twoEntities("Person", "", "Member", "")))) + } } diff --git a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataGeneratorMojo.java b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataGeneratorMojo.java index 457900a49..4e343b630 100644 --- a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataGeneratorMojo.java +++ b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataGeneratorMojo.java @@ -41,6 +41,20 @@ public class MetaDataGeneratorMojo extends AbstractMetaDataMojo /** How this goal is named in a message telling the adopter what to run. */ static final String OWNER = "mvn metaobjects:generate"; + /** + * The refusals over the whole own object set that no single generator can make: two metadata + * packages generating into one output directory (#546, ERR_NAME_COLLISION) and two objects + * claiming one REST collection (ERR_COLLECTION_NAME_COLLISION). The generate goal and the + * verify goal both run them before any generator, so verify refuses the same model gen does. + */ + static void refuseModelCollisions(MetaDataLoader loader) { + // FR-023: an object a dependency owns gets no route here, so it cannot collide with one. + List own = + com.metaobjects.generator.util.GeneratorUtil.ownMetaObjects(loader); + com.metaobjects.generator.util.OutputPackageCollisions.assertDistinct(own); + RouteNaming.assertNoCollectionNameCollisions(own); + } + @Override protected void executeGenerators(MetaDataLoader loader, List generatorImpls) { @@ -57,14 +71,7 @@ protected void executeGenerators(MetaDataLoader loader, List generato // `loader` is null in SourceRootRegistrationTest's unit tests, which exercise // only the post-loop registerGeneratedSourceRoots() logic below with no real // metadata — nothing to collide-check in that case. - if (loader != null) { - // FR-023: an object a dependency owns gets no route here, so it cannot collide with one. - List own = - com.metaobjects.generator.util.GeneratorUtil.ownMetaObjects(loader); - // #546: two metadata packages that generate into one output directory. - com.metaobjects.generator.util.OutputPackageCollisions.assertDistinct(own); - RouteNaming.assertNoCollectionNameCollisions(own); - } + if (loader != null) refuseModelCollisions(loader); // Where this EXECUTION's codegen hash manifest lives: anchored on the module // basedir, so two executions in one module share it (merged — every entry names diff --git a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java index 23030af96..ceb448c16 100644 --- a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java +++ b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java @@ -435,6 +435,9 @@ private void verifyCodegen() throws MojoExecutionException, MojoFailureException ClassLoader projectClassLoader = createProjectClassLoader(); resolveProviders(projectClassLoader); MetaDataLoader loader = createLoader(projectClassLoader); + // The same model refusals the generate goal makes before any generator runs: a model + // gen refuses must not verify clean. + MetaDataGeneratorMojo.refuseModelCollisions(loader); runFieldLintAdvisory(loader); int requirementErrors = runRequirementGate(loader); diff --git a/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java new file mode 100644 index 000000000..97faa5b52 --- /dev/null +++ b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java @@ -0,0 +1,57 @@ +package com.metaobjects.mojo; + +import com.metaobjects.generator.kotlin.KotlinEntityGenerator; +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * The verify goal refuses the model the generate goal refuses: two metadata packages that would + * generate into one output directory (#546, ERR_NAME_COLLISION). Before, only generate ran the + * check, so a project whose committed tree predated the clash verified clean. + */ +public class MetaDataVerifyModelRefusalTest { + + private static void configure(AbstractMetaDataMojo mojo, Path outputDir) { + mojo.setLoader(LoaderParam.builder("layout-collision") + .withClassname("com.metaobjects.loader.MetaDataLoader") + .withSourceDir("./src/test/resources") + .withSources(List.of("mojo/layout-collision/meta.Sales.json", "mojo/layout-collision/meta.sales.json")) + .build()); + mojo.setGenerators(List.of(GeneratorParam.builder(KotlinEntityGenerator.class.getName()) + .withArg("outputDir", outputDir.toString()) + .build())); + mojo.setGlobals(Collections.emptyMap()); + } + + private static void assertRefused(AbstractMetaDataMojo mojo) { + try { + mojo.execute(); + fail("expected the run to be refused with ERR_NAME_COLLISION"); + } catch (Exception e) { + StringBuilder messages = new StringBuilder(); + for (Throwable t = e; t != null; t = t.getCause()) messages.append(t.getMessage()).append('\n'); + assertTrue(messages.toString(), messages.toString().contains("ERR_NAME_COLLISION")); + } + } + + @Test + public void generateRefusesTwoPackagesInOneDirectory() throws Exception { + MetaDataGeneratorMojo gen = new MetaDataGeneratorMojo(); + configure(gen, Files.createTempDirectory("layout-gen")); + assertRefused(gen); + } + + @Test + public void verifyRefusesTwoPackagesInOneDirectory() throws Exception { + MetaDataVerifyMojo verify = new MetaDataVerifyMojo(); + configure(verify, Files.createTempDirectory("layout-verify")); + assertRefused(verify); + } +} diff --git a/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json new file mode 100644 index 000000000..d950c37a7 --- /dev/null +++ b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json @@ -0,0 +1,7 @@ +{ "metadata.root": { "package": "acme::Sales", "children": [ + { "object.entity": { "name": "OrderSales", "children": [ + { "source.rdb": { "@table": "orders_sales_a" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } } + ] } } +] } } diff --git a/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json new file mode 100644 index 000000000..fdfc77eb2 --- /dev/null +++ b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json @@ -0,0 +1,7 @@ +{ "metadata.root": { "package": "acme::sales", "children": [ + { "object.entity": { "name": "Ordersales", "children": [ + { "source.rdb": { "@table": "orders_sales_b" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "name": "pk", "@fields": "id" } } + ] } } +] } } diff --git a/server/typescript/packages/migrate-ts/src/diff/index.ts b/server/typescript/packages/migrate-ts/src/diff/index.ts index 31db42927..6691c962e 100644 --- a/server/typescript/packages/migrate-ts/src/diff/index.ts +++ b/server/typescript/packages/migrate-ts/src/diff/index.ts @@ -6,7 +6,7 @@ import type { CheckDescriptor, ColumnDefault, NameChange, DeclaredRename, ExprCanonPath, SchemaAdvisory, } from "../types.js"; -import { PG_IDENTIFIER_MAX_BYTES } from "../types.js"; +import { PG_IDENTIFIER_MAX_BYTES, pgStoredIdentifier } from "../types.js"; import type { SqlType } from "../sql-type.js"; import { sqlTypeEquals } from "../sql-type.js"; import { applyStatus } from "./status.js"; @@ -767,16 +767,6 @@ async function carryThroughColumnRenames( } } -/** Postgres keeps the first 63 bytes of an identifier, so a longer name is compared as it is stored. */ -function pgStoredName(name: string): string { - if (Buffer.byteLength(name, "utf8") <= PG_IDENTIFIER_MAX_BYTES) return name; - let out = ""; - for (const ch of name) { - if (Buffer.byteLength(out + ch, "utf8") > PG_IDENTIFIER_MAX_BYTES) break; - out += ch; - } - return out; -} /** * The rename a live primary key needs to carry the name the schema conventions give it, or @@ -785,8 +775,8 @@ function pgStoredName(name: string): string { */ function primaryKeyRename(expected: TableDescriptor, actual: TableDescriptor, liveTable: string): NameChange | undefined { if (expected.primaryKey.length === 0 || actual.primaryKey.length === 0) return undefined; - const from = pgStoredName(actual.primaryKeyName ?? `${liveTable}_pkey`); - const to = pgStoredName(expected.primaryKeyName ?? `${expected.name}_pkey`); + const from = pgStoredIdentifier(actual.primaryKeyName ?? `${liveTable}_pkey`); + const to = pgStoredIdentifier(expected.primaryKeyName ?? `${expected.name}_pkey`); return from === to ? undefined : { from, to }; } @@ -1182,11 +1172,18 @@ function fkKeyingIsStructural( return distinct(expected) && distinct(actual); } +/** + * Keyed by column set when `structural`, else by name. On Postgres the name is the one Postgres + * stores (its first 63 bytes), so a derived name over the limit matches its live, cut form + * instead of being proposed as a rename on every run. + */ function fkMapFor( fks: readonly FkDescriptor[], structural: boolean, + dialect: Dialect | undefined, ): Map { - return new Map(fks.map((f) => [structural ? fkColsKey(f) : f.name, f] as const)); + const nameKey = (name: string): string => (dialect === "sqlite" || dialect === "d1" ? name : pgStoredIdentifier(name)); + return new Map(fks.map((f) => [structural ? fkColsKey(f) : nameKey(f.name), f] as const)); } function diffTableForeignKeys( @@ -1198,8 +1195,8 @@ function diffTableForeignKeys( const table = expected.name; const sx = schemaSpread(expected.schema); const structuralFk = fkKeyingIsStructural(expected.foreignKeys, actual.foreignKeys, dialect); - const expectedFk = fkMapFor(expected.foreignKeys, structuralFk); - const actualFk = fkMapFor(actual.foreignKeys, structuralFk); + const expectedFk = fkMapFor(expected.foreignKeys, structuralFk, dialect); + const actualFk = fkMapFor(actual.foreignKeys, structuralFk, dialect); // NOTE: the map key is the FK's identity (name on postgres; column set on // sqlite/d1 — see fkMapFor). Change records always carry the DESCRIPTOR's // constraint name, never the key. diff --git a/server/typescript/packages/migrate-ts/src/expected-schema.ts b/server/typescript/packages/migrate-ts/src/expected-schema.ts index a711be5bb..d83e6aa43 100644 --- a/server/typescript/packages/migrate-ts/src/expected-schema.ts +++ b/server/typescript/packages/migrate-ts/src/expected-schema.ts @@ -83,7 +83,7 @@ import type { import { qualifiedDbName } from "./qualified-name.js"; import { SQLITE_ISO_NOW } from "./column-default.js"; import { viewFingerprint } from "./view-fingerprint.js"; -import { DIALECT_SQLITE, PG_IDENTIFIER_MAX_BYTES, isSqliteFamily } from "./types.js"; +import { DIALECT_SQLITE, PG_IDENTIFIER_MAX_BYTES, isSqliteFamily, pgStoredIdentifier } from "./types.js"; import { resolveReferentialActions, findReferentialActionConflict, @@ -1433,7 +1433,7 @@ function applySchemaConventions( if (indexForeignKeys) { for (const columns of uncoveredForeignKeyColumns(t)) { push( - { name: foreignKeyIndexName(t.name, columns), columns }, + { name: foreignKeyIndexName(t.name, columns, dialect), columns }, `the foreign-key index on [${columns.join(", ")}] of table "${t.name}" (migrate.foreignKeyIndexes)`, ); } @@ -1443,9 +1443,15 @@ function applySchemaConventions( return added; } -/** The name `foreignKeyIndexes` gives a foreign key's index: `
_[_…]_idx`. */ -export function foreignKeyIndexName(table: string, columns: readonly string[]): string { - return `${table}_${columns.join("_")}_idx`; +/** + * The name `foreignKeyIndexes` gives a foreign key's index: `
_[_…]_idx`. + * Except on SQLite and D1, a name over 63 bytes is cut to the first 63 bytes, which is the name + * Postgres stores, so a converged schema compares equal and nothing is proposed again. Two names + * that cut to the same 63 bytes on one table are refused as duplicates. + */ +export function foreignKeyIndexName(table: string, columns: readonly string[], dialect?: Dialect): string { + const name = `${table}_${columns.join("_")}_idx`; + return isSqliteFamily(dialect) ? name : pgStoredIdentifier(name); } /** True when `prefix` leads with exactly `columns`, in any order (the advisory's rule, too). */ diff --git a/server/typescript/packages/migrate-ts/src/types.ts b/server/typescript/packages/migrate-ts/src/types.ts index 42da43abd..c8cc130a9 100644 --- a/server/typescript/packages/migrate-ts/src/types.ts +++ b/server/typescript/packages/migrate-ts/src/types.ts @@ -718,3 +718,17 @@ export function isSqliteFamily(dialect: Dialect | undefined): boolean { /** Postgres's identifier limit, in bytes (NAMEDATALEN - 1). The server truncates a longer name. */ export const PG_IDENTIFIER_MAX_BYTES = 63; + +/** + * The name Postgres stores for `name`: its first 63 bytes, cut on a character boundary. A name + * within the limit is returned unchanged. + */ +export function pgStoredIdentifier(name: string): string { + if (Buffer.byteLength(name, "utf8") <= PG_IDENTIFIER_MAX_BYTES) return name; + let out = ""; + for (const ch of name) { + if (Buffer.byteLength(out + ch, "utf8") > PG_IDENTIFIER_MAX_BYTES) break; + out += ch; + } + return out; +} diff --git a/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts b/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts new file mode 100644 index 000000000..1ca7cc89c --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts @@ -0,0 +1,59 @@ +// The shared foreign-key index corpus: `fixtures/foreign-key-index-conformance/` pins which +// foreign keys `migrate.foreignKeyIndexes` indexes and under what name, per dialect. The C# EF +// context generator and the Kotlin Exposed table generator write the same indexes into their +// models and pass the same expected files, so one rule decides "uncovered" in every port. +// +// Regenerate the expected files (after a deliberate change, reviewed): +// META_WRITE_FK_INDEXES=1 bun test test/foreign-key-index-conformance.test.ts + +import { describe, test, expect } from "bun:test"; +import { readFileSync, writeFileSync, readdirSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import { buildExpectedSchemaWithProvenance } from "../src/expected-schema.js"; +import { PG_IDENTIFIER_MAX_BYTES, type IndexDescriptor } from "../src/types.js"; +import { qualifiedDbName } from "../src/qualified-name.js"; + +const CORPUS = resolve(import.meta.dir, "../../../../../fixtures/foreign-key-index-conformance"); + +const CASES = [ + { file: "expected.postgres.json", dialect: "postgres" }, + { file: "expected.sqlite.json", dialect: "sqlite" }, +] as const; + +async function load() { + const dir = join(CORPUS, "input"); + const sources = readdirSync(dir).filter((f) => f.endsWith(".json")).sort() + .map((f) => new InMemoryStringSource(readFileSync(join(dir, f), "utf8"), { id: f, format: "json" })); + const { root, errors } = await new MetaDataLoader().load(sources); + expect(errors).toEqual([]); + return root; +} + +describe("foreign-key-index-conformance — the indexes migrate.foreignKeyIndexes adds", () => { + for (const c of CASES) { + test(c.file, async () => { + const built = buildExpectedSchemaWithProvenance(await load(), { dialect: c.dialect, foreignKeyIndexes: true }); + // Keyed by table name (the corpus has no @schema); tables with none are left out. + const added = built.foreignKeyIndexes ?? new Map(); + const actual = Object.fromEntries( + built.snapshot.tables + .map((t) => [t.name, added.get(qualifiedDbName(t)) ?? []] as const) + .filter(([, ixs]) => ixs.length > 0) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([table, ixs]) => [table, ixs.map((ix) => ({ name: ix.name, columns: [...ix.columns] }))]), + ); + const path = join(CORPUS, c.file); + if (process.env["META_WRITE_FK_INDEXES"] === "1") writeFileSync(path, JSON.stringify(actual, null, 2) + "\n"); + expect(actual).toEqual(JSON.parse(readFileSync(path, "utf8"))); + }); + } + + test("postgres: a name over 63 bytes is cut to the 63 bytes Postgres stores", async () => { + const built = buildExpectedSchemaWithProvenance(await load(), { dialect: "postgres", foreignKeyIndexes: true }); + const long = built.snapshot.tables.find((t) => t.name === "customer_order_line_item_allocations_by_region")!; + const ix = long.indexes.find((i) => i.columns[0] === "fulfilment_warehouse_reference_id")!; + expect(Buffer.byteLength(ix.name, "utf8")).toBe(PG_IDENTIFIER_MAX_BYTES); + expect(`${long.name}_fulfilment_warehouse_reference_id_idx`.startsWith(ix.name)).toBe(true); + }); +}); diff --git a/spec/decisions/ADR-0060-jvm-codegen-hash-manifest.md b/spec/decisions/ADR-0060-jvm-codegen-hash-manifest.md index 5616f7cc0..cafef7255 100644 --- a/spec/decisions/ADR-0060-jvm-codegen-hash-manifest.md +++ b/spec/decisions/ADR-0060-jvm-codegen-hash-manifest.md @@ -130,5 +130,10 @@ does not cover deletion, and deletion without evidence is not safe on any port. - The JVM matches the other four ports on the overwrite rule and the orphan rule. It differs only in item 6 (a refusal is a warning) and in having no `--generators` narrowing: an execution's `` list is always complete, so every run is a full run. +- (2026-11-13) `verify.handEdits`, which lets a hand-edited generated file pass `meta verify + --codegen` with a notice, is a Node CLI setting only, by design: TypeScript is the one port + whose `gen` merges a hand edit forward (three-way), so in Java, Kotlin, C# and Python the + drift gate compares bytes, any edit fails it, and the way to change generated output there is + to own the generator (`eject`). The language-differences page states the same split. Migration: [jvm-codegen-hash-manifest.md](../../docs/features/migrations/jvm-codegen-hash-manifest.md). From f2b0bf49cfc3b9a580cea807c925817500c44ed0 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Fri, 13 Nov 2026 22:16:07 -0500 Subject: [PATCH 2/8] feat(release): cut a patch from a release/. branch; gate a minor on the previous one being merged Forward-ports #578 from release/1.2: release.mjs and finish-release.mjs accept a maintenance branch for a patch on its own line, refuse a version that would move npm's latest backwards, and RELEASING documents the path. A minor or major cut is now refused while the previous release/. holds a commit that is not on main, and RELEASING Step 0 carries the same check. --- CHANGELOG.md | 8 ++ docs/RELEASING.md | 63 ++++++++++++- scripts/ci-local.sh | 2 +- scripts/eject-release-gate.test.ts | 8 +- scripts/finish-release.mjs | 28 +++--- scripts/finish-release.test.ts | 58 ++++++++++++ scripts/lib/release-branch.mjs | 138 +++++++++++++++++++++++++++++ scripts/release.mjs | 40 +++++++-- scripts/test-release-branch.mjs | 98 ++++++++++++++++++++ tsconfig.scripts.json | 1 + 10 files changed, 418 insertions(+), 26 deletions(-) create mode 100644 scripts/lib/release-branch.mjs create mode 100644 scripts/test-release-branch.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f5748a6..dc671b300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,14 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi lint gate), and a fresh npm install of the CLI drops from 245 MB to 151 MB of `node_modules`. `scripts/check-peer-ranges.ts` now fails a publishable package that requires `@biomejs/biome` through `dependencies` or a non-optional peer. +- **Release scripts: a patch can be cut from a `release/.` maintenance branch** + (brought from `release/1.2`, where 1.2.2 shipped this way). `scripts/release.mjs` and + `scripts/finish-release.mjs` accept that branch as well as `main`. The version must be on the + branch's own line, every push goes to that branch, and the sync check compares against it. + `release.mjs` also refuses, before anything is published, a version older than the newest + release and a release candidate of a version that already shipped, because either would move + npm's `latest` backwards. A minor or major cut is now refused while the previous maintenance + branch holds a commit that is not on `main`; `docs/RELEASING.md` Step 0 gains the same check. ### Deprecated diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 5006db9ed..7f824c579 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -49,6 +49,21 @@ Decide explicitly for every branch the scan returns: merge it into this cut, or Note that `no-mistakes/*` branches are usually gate replays of work already on `main` — confirm rather than assume. Do not skip past the list. +**The previous maintenance branch is on `main` (minor and major cuts).** Before a minor or +major is cut, every commit on the previous `release/.` maintenance branch must be +an ancestor of `main`. Otherwise a fix shipped as a patch from that branch is missing from the +new line. Check it by hand while writing the CHANGELOG: + +```bash +git fetch origin -q +git merge-base --is-ancestor origin/release/. origin/main && echo merged || git log --oneline origin/main..origin/release/. +``` + +`scripts/release.mjs` enforces it in its preflight: cutting `X.Y.0` (or an RC of it) is refused +while the highest `release/.` below `X.Y` on origin holds a commit `HEAD` does not. A patch +is not gated. To clear it, merge the maintenance branch into `main` by PR (see "Then bring the +branch to `main`" in the maintenance-branch section). + **The eject pending list (1.1.0).** 1.1.0 does not ship while `fixtures/generator-registry-conformance/registry.json` has an entry in `eject.pending` (ADR-0034 Amendment 8). The scripted paths enforce it: `scripts/release.mjs` (first check of @@ -186,7 +201,9 @@ Publish in tier order so a dependent never lands before its dependency. **`forge released. `v0.24.5` carries exactly that, and nothing caught it because no page displayed a coordinate yet. - So `release.mjs` pushes `main` and **stops**. After the ports are bumped, committed, + So `release.mjs` pushes the branch it runs on (`main`, or a `release/.` + maintenance branch — see the maintenance-branch section below) and **stops**. After the + ports are bumped, committed, pushed and tagged (`python-v*`, `csharp-v*`, `java-v.*` — `java-v8.*` at the 1.0 cut, NOT `java-v7.*`), run: @@ -618,6 +635,46 @@ default, so a dispatch meant to report cannot write. Then verify: `npm dist-tag ls @metaobjectsdev/` — authoritative, where `npm view` is CDN-cached and lags right after publish. +## Patch from a maintenance branch (`release/.`) + +**When.** `main` already holds work for the next minor (or major) that must not ship yet, and a +fix has to go out on the line already released. Cut the patch from a maintenance branch named +exactly `release/.` (for example `release/1.2`), branched from that line's last +release commit, with the fix merged into it by PR. If `main` has nothing unreleased beyond the +patch, cut from `main` as usual instead. + +**What the scripts do differently.** `scripts/release.mjs` and `scripts/finish-release.mjs` +accept `release/.` as well as `main` (the rule is +[`scripts/lib/release-branch.mjs`](../scripts/lib/release-branch.mjs), shared by both): + +- The version must be on the branch's own line: `release/1.2` cuts `1.2.x` (and `1.2.x-rc.N`) + and refuses anything else, such as `1.3.0`. +- Every push goes to that branch (`git push origin release/1.2`), never `main`. +- `finish-release`'s sync check compares HEAD with `origin/release/1.2`, not `origin/main`. + +Every other gate is unchanged. Bump the PyPI/NuGet/Maven manifests, run `bun run site:payload`, +commit and push on the maintenance branch, and push the `python-v*` / `csharp-v*` / `java-v*` +tags from it, exactly as on `main`. + +**The site follows the newest tag.** The website deploys from the newest `vN.N.N` release tag +(rule 7), wherever it was cut. A patch tag on a maintenance branch therefore becomes what +metaobjects.dev shows, including its payload and coordinates, until the next release is tagged. +The converse is enforced: `finish-release` refuses to tag a patch on a line older than a tag +that already exists (`v1.2.3` once `v1.3.0` exists), because the deploy would never resolve it. +So a maintenance patch is for a line that is still the newest release. The same holds on npm: +`release.mjs` publishes to `latest`, and a patch cut after a newer minor shipped would move +`latest` backwards — so `release.mjs` refuses, in its preflight and before anything is +published, any version older than the newest release tag, and any release candidate of a +version that has already shipped (npm sorts it below the full release; resolved the same +way the site deploy resolves its tag). It offers no non-`latest` dist-tag path. + +**Then bring the branch to `main`.** Re-land each fix on `main` by PR as soon as it ships. +Before the next minor or major is cut, also merge the maintenance branch itself into `main` by +PR, resolving its version bumps, site payload and CHANGELOG in `main`'s favour, so that every +commit on it is an ancestor of `main`. Step 0 checks this and `release.mjs` refuses the cut +until it holds; a fix that exists only on the maintenance branch would otherwise be missing +from the new line. + ## Isolated patch (one package) If only one package changed (e.g. a `cli` bugfix), bump just that package, `rm bun.lock && bun @@ -700,7 +757,9 @@ lock the repo/owner IDs against resurrection attacks.) ## Procedure -1. **Pick the publish commit on `main`** (a stable, merged tip — not a mid-refactor branch). Ensure +1. **Pick the publish commit on the branch the release is cut from** (`main`, or the + `release/.` maintenance branch for a patch — the workflow publishes the + COMMITTED ``, so the tag must name that branch's tree). Ensure the packaging config + `publish-csharp.yml` are on it. Set `` in `Directory.Build.props`. 2. **Validate locally** (catches immutable-version mistakes before they're permanent): ```bash diff --git a/scripts/ci-local.sh b/scripts/ci-local.sh index 073f695ed..1b56fb5ae 100755 --- a/scripts/ci-local.sh +++ b/scripts/ci-local.sh @@ -487,7 +487,7 @@ gate_site_reference() { bun_install && bun scripts/build-site-reference.ts --che # exercised) are the ones worth a test at all. See scripts/maven-coordinate.mjs. # And the publish workflows' tag check (scripts/check-release-tag.mjs), whose test also reads # each real manifest, so a layout change fails here rather than on a release tag. -gate_release_tag() { bun test scripts/finish-release.test.ts scripts/eject-release-gate.test.ts && node scripts/test-maven-coordinate.mjs && node scripts/test-prerelease-version.mjs && node scripts/test-check-release-tag.mjs && node scripts/test-changelog-promote.mjs && bash tools/prerelease/test-prerelease-link.sh; } +gate_release_tag() { bun test scripts/finish-release.test.ts scripts/eject-release-gate.test.ts && node scripts/test-maven-coordinate.mjs && node scripts/test-prerelease-version.mjs && node scripts/test-check-release-tag.mjs && node scripts/test-changelog-promote.mjs && node scripts/test-release-branch.mjs && bash tools/prerelease/test-prerelease-link.sh; } # ── repo-root scripts/ typechecks ───────────────────────────────────────────── # `bun test` transpiles per file and never typechecks, and `bun run --filter '*' diff --git a/scripts/eject-release-gate.test.ts b/scripts/eject-release-gate.test.ts index 98e743f50..90f45b093 100644 --- a/scripts/eject-release-gate.test.ts +++ b/scripts/eject-release-gate.test.ts @@ -102,9 +102,9 @@ describe("eject release gate — the command and its wiring", () => { // ADR-0034 Amendment 8 makes the gate the first preflight check of `release.mjs`. The // check after it is the branch check, so a fixture on another branch tells the two apart: - // a refused version stops before "must be on main", and an allowed one reaches it. + // a refused version stops before "not on main", and an allowed one reaches it. test("release.mjs asks the gate first in its preflight, and stops on a refusal", () => { - const RELEASE_SCRIPTS = ["release.mjs", "lib/changelog-promote.mjs", "publish-set.mjs", "maven-coordinate.mjs"]; + const RELEASE_SCRIPTS = ["release.mjs", "lib/changelog-promote.mjs", "lib/release-branch.mjs", "publish-set.mjs", "maven-coordinate.mjs"]; const onBranch = (pending: (typeof ENTRY)[]) => { const root = repoWith(JSON.stringify(manifest(pending)), RELEASE_SCRIPTS); const git = (...args: string[]) => spawnSync("git", ["-C", root, ...args], { encoding: "utf8" }); @@ -119,11 +119,11 @@ describe("eject release gate — the command and its wiring", () => { const refused = release(blocked, "1.1.0"); expect(refused.code).not.toBe(0); expect(refused.out).toContain("the eject contract does not hold yet"); - expect(refused.out).not.toContain("must be on main"); + expect(refused.out).not.toContain("not on main"); for (const allowed of [release(blocked, "1.1.0-rc.1"), release(onBranch([]), "1.1.0")]) { expect(allowed.out).not.toContain("the eject contract does not hold yet"); - expect(allowed.out).toContain("must be on main (you are on not-main)"); + expect(allowed.out).toContain("not on main or a release/. maintenance branch (you are on not-main)"); } }); diff --git a/scripts/finish-release.mjs b/scripts/finish-release.mjs index b3c790952..5fb4529d3 100644 --- a/scripts/finish-release.mjs +++ b/scripts/finish-release.mjs @@ -37,6 +37,7 @@ import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { mavenVersion } from "./maven-coordinate.mjs"; import { ejectReleaseRefusalAt } from "./eject-release-gate.mjs"; +import { releaseBranchRefusal, newestNpmReleaseTag } from "./lib/release-branch.mjs"; const REPO = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const CHECK_ONLY = process.argv.includes("--check"); @@ -80,8 +81,13 @@ ok("eject contract: the pending list is empty"); if (out("git status --porcelain") !== "") { die("working tree is dirty — commit or stash before tagging, or the tag names a tree you did not review"); } -if (out("git rev-parse --abbrev-ref HEAD") !== "main") die("not on main"); -// NOT `|| true`. A swallowed fetch leaves `origin/main` at a STALE remote-tracking ref, +// `main`, or a `release/.` maintenance branch tagging a version on its own +// line (scripts/lib/release-branch.mjs). The sync check below compares against THAT +// branch's remote, never origin/main. +const branch = out("git rev-parse --abbrev-ref HEAD"); +const branchRefusal = releaseBranchRefusal(branch, VERSION); +if (branchRefusal !== null) die(branchRefusal); +// NOT `|| true`. A swallowed fetch leaves `origin/` at a STALE remote-tracking ref, // and the very next comparison then prints "synced with origin" having verified nothing — // with git's own "Could not read from remote repository" scrolling past just above the // tick. The realistic case is the port-bump commit pushed from another checkout while @@ -93,10 +99,12 @@ try { die(`git fetch origin failed, so "synced with origin" cannot be verified: ` + `${e instanceof Error ? e.message : String(e)}`); } -if (out("git rev-parse HEAD") !== out("git rev-parse origin/main")) { - die("HEAD is not origin/main — push first; the deploy clones the REMOTE tag"); +// `|| true` here is not the swallow warned about above: a maintenance branch that was +// never pushed has no `origin/`, and that must read as "not synced", not crash. +if (out("git rev-parse HEAD") !== out(`git rev-parse --verify --quiet origin/${branch} || true`)) { + die(`HEAD is not origin/${branch} — push first; the deploy clones the REMOTE tag`); } -ok("on main, clean, synced with origin"); +ok(`on ${branch}, clean, synced with origin`); // ── 2. the tag is free ─────────────────────────────────────────────────────── // Never move a tag. A moved tag silently changes what the website deploys, and any @@ -378,13 +386,9 @@ ok("the llms mirrors state this release"); // newest tag of a frozen line and passes green forever. After the 1.0 cut it would have // resolved v0.25.0 at every future release, so the check could never fire again — the same // defect that kept the live site on 0.25.0 with every deploy step green. Nothing here needs -// maintaining at the next major. -const resolved = out(`git tag -l 'v*' | grep -E '^v[0-9]+\\.[0-9]+\\.[0-9]+$' | sort -rV || true`) - .split("\n").map((t) => t.trim()).filter(Boolean) - .find((t) => { - const pkg = out(`git show ${t}:server/typescript/packages/cli/package.json 2>/dev/null || true`); - try { return JSON.parse(pkg).version === t.slice(1); } catch { return false; } - }) ?? ""; +// maintaining at the next major. The selector itself is shared with `release.mjs` +// (scripts/lib/release-branch.mjs), so the two cannot disagree about the npm line. +const resolved = newestNpmReleaseTag(out); const wouldResolve = [resolved, tag].filter(Boolean).sort((a, b) => a.localeCompare(b, undefined, { numeric: true })).at(-1); if (wouldResolve !== tag) { diff --git a/scripts/finish-release.test.ts b/scripts/finish-release.test.ts index ea089fc87..713bdfe6e 100644 --- a/scripts/finish-release.test.ts +++ b/scripts/finish-release.test.ts @@ -36,6 +36,8 @@ const SCRIPT = readFileSync(resolve(REPO, "scripts/finish-release.mjs"), "utf8") const MAVEN_COORD = readFileSync(resolve(REPO, "scripts/maven-coordinate.mjs"), "utf8"); // Gate 0 asks the eject release gate, and the fixture ships the real one for the same reason. const EJECT_GATE = readFileSync(resolve(REPO, "scripts/eject-release-gate.mjs"), "utf8"); +// Gate 1's branch rule, shared with release.mjs — the real module, for the same reason. +const RELEASE_BRANCH = readFileSync(resolve(REPO, "scripts/lib/release-branch.mjs"), "utf8"); const EJECT_MANIFEST = "fixtures/generator-registry-conformance/registry.json"; /** The shared file as the release branch leaves it: nothing pending. */ const EJECT_CLEAR = `${JSON.stringify({ eject: { pending: [] } })}\n`; @@ -173,6 +175,7 @@ function makeRepo(spec: Spec = {}): Fixture { "scripts/finish-release.mjs": SCRIPT, "scripts/maven-coordinate.mjs": MAVEN_COORD, "scripts/eject-release-gate.mjs": EJECT_GATE, + "scripts/lib/release-branch.mjs": RELEASE_BRANCH, })) { const full = join(work, rel); mkdirSync(dirname(full), { recursive: true }); @@ -291,6 +294,46 @@ describe("finish-release gate 1: the tree that will be tagged", () => { expect(r.out).toContain("HEAD is not origin/main"); }); + // A maintenance branch `release/.` may tag a patch on its own line; the + // rule itself is unit-tested in scripts/test-release-branch.mjs. These cases prove the + // script wires it: the branch is accepted, a version off its line is refused, and the + // sync check compares against origin/, never origin/main. + const onMaintenance = (name: string, push = true) => { + const f = makeRepo(); + f.git(f.work, "checkout", "--quiet", "-b", name); + if (push) f.git(f.work, "push", "--quiet", "-u", "origin", name); + return f; + }; + + test("a maintenance branch on the release's own line passes gate 1", () => { + const r = onMaintenance("release/0.24").run(RELEASE, "--check"); + expect(r.out).toContain("on release/0.24, clean, synced with origin"); + expect(r.code).toBe(0); + }); + + test("a maintenance branch refuses a version off its line", () => { + const r = onMaintenance("release/0.23").run(RELEASE, "--check"); + expect(r.code).not.toBe(0); + expect(r.out).toContain("cuts only 0.23.x"); + expect(r.out).not.toContain("synced with origin"); + }); + + test("a maintenance branch syncs against its OWN remote, not origin/main", () => { + const f = onMaintenance("release/0.24"); + writeFileSync(join(f.work, "NOTES.md"), "unpushed\n"); + f.git(f.work, "add", "-A"); + f.git(f.work, "commit", "--quiet", "-m", "local only"); + const r = f.run(RELEASE, "--check"); + expect(r.code).not.toBe(0); + expect(r.out).toContain("HEAD is not origin/release/0.24"); + }); + + test("a maintenance branch never pushed is refused, not crashed on", () => { + const r = onMaintenance("release/0.24", false).run(RELEASE, "--check"); + expect(r.code).not.toBe(0); + expect(r.out).toContain("HEAD is not origin/release/0.24"); + }); + // The defect: the fetch was `|| true`, so a failure left `origin/main` at a stale // remote-tracking ref and the very next line printed "synced with origin" having // verified nothing. The realistic case is the port-bump commit pushed from another @@ -647,6 +690,21 @@ describe("finish-release gate 7: cutting the tag", () => { expect(f.git(f.origin, "tag", "-l")).toContain(`v${RELEASE}`); }); + test("tags and pushes from a maintenance branch, leaving origin/main where it was", () => { + const f = makeRepo(); + const mainBefore = f.git(f.origin, "rev-parse", "main"); + f.git(f.work, "checkout", "--quiet", "-b", "release/0.24"); + writeFileSync(join(f.work, "NOTES.md"), "the patch\n"); + f.git(f.work, "add", "-A"); + f.git(f.work, "commit", "--quiet", "-m", "patch on the maintenance line"); + f.git(f.work, "push", "--quiet", "-u", "origin", "release/0.24"); + const r = f.run(RELEASE); + expect(r.out).toContain(`cut and pushed v${RELEASE}`); + expect(r.code).toBe(0); + expect(f.git(f.origin, "rev-parse", `v${RELEASE}^{commit}`)).toBe(f.git(f.work, "rev-parse", "HEAD")); + expect(f.git(f.origin, "rev-parse", "main")).toBe(mainBefore); + }); + test("--check cuts nothing", () => { const f = makeRepo(); expect(f.run(RELEASE, "--check").code).toBe(0); diff --git a/scripts/lib/release-branch.mjs b/scripts/lib/release-branch.mjs new file mode 100644 index 000000000..b5c4030e2 --- /dev/null +++ b/scripts/lib/release-branch.mjs @@ -0,0 +1,138 @@ +// Which branch a release may be cut from — shared by `scripts/release.mjs` and +// `scripts/finish-release.mjs`, so the two cannot disagree about it. +// +// Two kinds of branch qualify: +// +// - `main`, for every release, as before. +// - a maintenance branch named `release/.`, for a PATCH on a line `main` +// has moved past. It may cut only versions on its own line: `release/1.2` cuts 1.2.x +// and nothing else, because a 1.3.0 cut from it would ship a minor without the work +// `main` holds for that minor. +// +// The caller pushes to, and compares HEAD against, `origin/` — never `origin/main` +// when it is on a maintenance branch. + +/** The pattern a maintenance branch name must match. */ +export const MAINTENANCE_BRANCH = /^release\/(\d+)\.(\d+)$/; + +/** + * Why `version` may not be released from `branch`, or `null` when it may. + * + * @param {string} branch the current branch, as `git rev-parse --abbrev-ref HEAD` prints it + * @param {string} version the version being cut (`1.2.2`, or `1.2.2-rc.1` for release.mjs) + * @returns {string | null} + */ +export function releaseBranchRefusal(branch, version) { + if (branch === "main") return null; + const m = MAINTENANCE_BRANCH.exec(branch); + if (m === null) { + return `not on main or a release/. maintenance branch (you are on ${branch})`; + } + const [, major, minor] = m; + const v = /^(\d+)\.(\d+)\./.exec(version); + if (v === null || v[1] !== major || v[2] !== minor) { + return `${branch} is the ${major}.${minor} maintenance branch and cuts only ${major}.${minor}.x — ` + + `${version} is not on that line (cut it from main, or from its own release/.)`; + } + return null; +} + +/** + * The newest tag on the npm release line, the way the site deploy resolves it: the newest + * `vN.N.N` whose `server/typescript/packages/cli/package.json` is versioned as the tag — + * the property that DEFINES the npm line, never a version prefix (the JVM `v7.20.12` ships + * cli `0.20.11` and must not count). `""` when no release tag exists. + * + * @param {(cmd: string) => string} out run a command and return its trimmed stdout, like + * each script's own `out` + * @returns {string} + */ +export function newestNpmReleaseTag(out) { + return out(`git tag -l 'v*' | grep -E '^v[0-9]+\\.[0-9]+\\.[0-9]+$' | sort -rV || true`) + .split("\n").map((t) => t.trim()).filter(Boolean) + .find((t) => { + const pkg = out(`git show ${t}:server/typescript/packages/cli/package.json 2>/dev/null || true`); + try { return JSON.parse(pkg).version === t.slice(1); } catch { return false; } + }) ?? ""; +} + +/** + * Why `version` may not be released while `newestTag` is the newest release tag, or `null` + * when it may. `bun publish` tags `latest`, so a version older than the newest release + * would move `latest` backwards — including an `-rc` cut on a line a newer release passed, + * and an `-rc` of the very version that already shipped (npm sorts `1.2.4-rc.1` below + * `1.2.4`). + * + * @param {string} version the version being cut (`1.2.4`, or `1.2.4-rc.1`) + * @param {string} newestTag the newest npm-line release tag (`v1.3.0`), or `""` if none + * @returns {string | null} + */ +export function latestRegressionRefusal(version, newestTag) { + if (newestTag === "") return null; + const a = /^(\d+)\.(\d+)\.(\d+)/.exec(version); + const b = /^v(\d+)\.(\d+)\.(\d+)$/.exec(newestTag); + if (a === null || b === null) return null; + for (let i = 1; i <= 3; i++) { + if (Number(a[i]) === Number(b[i])) continue; + if (Number(a[i]) > Number(b[i])) return null; + return `${version} is older than ${newestTag.slice(1)}, the newest release — npm publishes to ` + + `\`latest\`, so this would move \`latest\` backwards. A patch on a line a newer release ` + + `has passed needs a non-\`latest\` dist-tag path this script does not yet offer.`; + } + return /-rc\.\d+$/.test(version) + ? `${version} is a release candidate for ${newestTag.slice(1)}, which has already shipped — ` + + `npm sorts it below the full release, so publishing to \`latest\` would move \`latest\` ` + + `backwards.` + : null; +} + +/** + * The maintenance branch a minor or major cut of `version` must already contain: the + * `release/.` with the highest line below `version`'s own, among `branches`. + * `null` for a patch (a patch cuts from its own line, which the branch rule already + * guards) and when no lower maintenance branch exists. + * + * @param {string} version the version being cut (`1.3.0`, `2.0.0-rc.1`) + * @param {readonly string[]} branches maintenance branch names (`release/1.2`), as on origin + * @returns {string | null} + */ +export function previousMaintenanceBranch(version, branches) { + const v = /^(\d+)\.(\d+)\.(\d+)/.exec(version); + if (v === null || Number(v[3]) !== 0) return null; + const [major, minor] = [Number(v[1]), Number(v[2])]; + /** @type {string | null} */ + let best = null; + let bestMajor = -1; + let bestMinor = -1; + for (const b of branches) { + const m = MAINTENANCE_BRANCH.exec(b); + if (m === null) continue; + const bMajor = Number(m[1]); + const bMinor = Number(m[2]); + const below = bMajor < major || (bMajor === major && bMinor < minor); + const higher = bMajor > bestMajor || (bMajor === bestMajor && bMinor > bestMinor); + if (below && higher) [best, bestMajor, bestMinor] = [b, bMajor, bMinor]; + } + return best; +} + +/** + * Why a minor or major cut of `version` may not proceed because the previous maintenance + * branch holds commits `main` does not, or `null` when it may. A fix shipped as a patch from + * `release/1.2` and never brought to `main` would otherwise silently vanish from 1.3.0. + * + * @param {string} version the version being cut + * @param {readonly string[]} branches maintenance branch names on origin (`release/1.2`) + * @param {(branch: string) => number} unmergedCount how many commits `origin/` holds + * that the cut's HEAD does not (`git rev-list --count HEAD..origin/`) + * @returns {string | null} + */ +export function unmergedMaintenanceRefusal(version, branches, unmergedCount) { + const prev = previousMaintenanceBranch(version, branches); + if (prev === null) return null; + const n = unmergedCount(prev); + if (n === 0) return null; + return `origin/${prev} has ${n} commit(s) that are not on this branch — every commit on the ` + + `previous maintenance branch must be an ancestor of main before ${version} is cut. Merge ` + + `${prev} into main by PR (docs/RELEASING.md, "Patch from a maintenance branch").`; +} diff --git a/scripts/release.mjs b/scripts/release.mjs index 04ef11d35..4f82c5a30 100644 --- a/scripts/release.mjs +++ b/scripts/release.mjs @@ -33,6 +33,9 @@ import { createInterface } from "node:readline/promises"; import { publishSet } from "./publish-set.mjs"; import { mavenVersion } from "./maven-coordinate.mjs"; import { ejectReleaseRefusalAt } from "./eject-release-gate.mjs"; +import { + releaseBranchRefusal, newestNpmReleaseTag, latestRegressionRefusal, unmergedMaintenanceRefusal, +} from "./lib/release-branch.mjs"; const VERSION = process.argv[2]; const DRY = process.argv.includes("--dry-run"); @@ -70,10 +73,26 @@ console.log(`\n── Releasing ${VERSION}${DRY ? " (DRY RUN)" : ""} ──\n`) const ejectRefusal = ejectReleaseRefusalAt(VERSION, join(dirname(fileURLToPath(import.meta.url)), "..")); if (ejectRefusal !== null) die(ejectRefusal); +// `main`, or a `release/.` maintenance branch cutting a version on its own +// line (scripts/lib/release-branch.mjs). Everything below that pushes or compares against +// the remote uses THIS branch, so a maintenance cut never touches main. const branch = out("git rev-parse --abbrev-ref HEAD"); -if (branch !== "main") die(`must be on main (you are on ${branch})`); +const branchRefusal = releaseBranchRefusal(branch, VERSION); +if (branchRefusal !== null) die(branchRefusal); sh("git fetch origin", { quiet: true }); -if (out("git rev-list --count HEAD..origin/main") !== "0") die("main is behind origin/main — pull first"); +if (out(`git rev-parse --verify --quiet origin/${branch} || true`) === "") { + die(`${branch} has no origin/${branch} — push the branch first`); +} +if (out(`git rev-list --count HEAD..origin/${branch}`) !== "0") die(`${branch} is behind origin/${branch} — pull first`); + +// A minor or major cut must already hold every commit of the previous release/. +// maintenance branch: a fix shipped as a patch from it and never brought to main would +// otherwise be missing from the new line (docs/RELEASING.md, Step 0). +const maintenanceBranches = out("git branch -r --format='%(refname:short)'") + .split("\n").map((b) => b.trim().replace(/^origin\//, "")).filter(Boolean); +const unmergedRefusal = unmergedMaintenanceRefusal(VERSION, maintenanceBranches, + (b) => Number(out(`git rev-list --count HEAD..origin/${b}`))); +if (unmergedRefusal !== null) die(unmergedRefusal); // Clean tree, except the CHANGELOG (we promote its [Unreleased] section) and untracked noise. const dirty = out("git status --porcelain").split("\n") @@ -82,6 +101,13 @@ if (dirty.length) die(`uncommitted changes:\n${dirty.join("\n")}\n(commit or sta if (out(`git tag -l v${VERSION}`)) die(`tag v${VERSION} already exists`); +// `bun publish` tags `latest`, so a version older than the newest release would move +// `latest` backwards — finish-release gate 6 catches that only after the publish; this +// refuses it before anything irreversible. Same npm-line tag resolution the site deploy +// uses (scripts/lib/release-branch.mjs). +const latestRefusal = latestRegressionRefusal(VERSION, newestNpmReleaseTag(out)); +if (latestRefusal !== null) die(latestRefusal); + // Can we publish AT ALL? Checked here, before the version bump, because the bump is // committed and pushed long before the first `bun publish` — so discovering a dead // credential at the publish step strands the release mid-flight. The 0.24.0 cut found @@ -129,7 +155,7 @@ const taken = (await Promise.all(set.map(async (p) => { if (taken.length) die(`${VERSION} is already published for:\n ${taken.join("\n ")}\n` + `npm versions are permanent — pick the next free version.`); -ok(`preflight: on main, synced, ${VERSION} free on all ${set.length} packages`); +ok(`preflight: on ${branch}, synced, ${VERSION} free on all ${set.length} packages`); ok(`lockstep set @ ${current}: ${set.length} packages → ${VERSION}`); // The showcase corpus is what metaobjects.dev publishes as "real `meta gen` output", @@ -262,7 +288,7 @@ for (const pkg of ordered) { // --- PHASE 8: push (the tag is NOT cut here) ------------------------------- // This script publishes npm, which is one of FOUR registries. The PyPI/NuGet/Maven -// version bump is a separate, later commit — so at this point `main` states the new npm +// version bump is a separate, later commit — so at this point the branch states the new npm // coordinate and three stale ones, and the site payload committed above says exactly // that. // @@ -274,8 +300,8 @@ for (const pkg of ordered) { // // So the tag moves to the END of the coordinated cut: `scripts/finish-release.mjs`, // which refuses to tag until the payload's coordinates match what actually shipped. -sh("git push origin main", { quiet: true }); -ok("pushed main (tag deferred to finish-release)"); +sh(`git push origin ${branch}`, { quiet: true }); +ok(`pushed ${branch} (tag deferred to finish-release)`); // --- PHASE 11: post-publish smoke (the RC's safety net) ------------------- // NOT under tmpdir(). Node's resolution walks UP from the scratch dir, so a stale @@ -299,4 +325,4 @@ console.log(` 3. \`bun scripts/finish-release.mjs ${VERSION}\` — gates the c console.log(` The website pins to v${VERSION}, so tagging before step 1 publishes three stale versions.`); console.log(` Docs version refs are NOT auto-bumped (low-value churn for a patch); batch them when convenient.`); console.log(` The site payload IS auto-bumped and committed here — but only its npm coordinate.`); -console.log(` The PyPI/NuGet/Maven bump commit must run \`bun run site:payload\` or gates goes red on main.\n`); +console.log(` The PyPI/NuGet/Maven bump commit must run \`bun run site:payload\` or gates goes red on ${branch}.\n`); diff --git a/scripts/test-release-branch.mjs b/scripts/test-release-branch.mjs new file mode 100644 index 000000000..c867073a1 --- /dev/null +++ b/scripts/test-release-branch.mjs @@ -0,0 +1,98 @@ +#!/usr/bin/env node +// Tests for scripts/lib/release-branch.mjs — which branch a release may be cut from. +// +// node scripts/test-release-branch.mjs + +import { + releaseBranchRefusal, latestRegressionRefusal, newestNpmReleaseTag, + previousMaintenanceBranch, unmergedMaintenanceRefusal, +} from "./lib/release-branch.mjs"; + +let fails = 0; +const check = (cond, label) => (cond ? console.log(`ok: ${label}`) : (console.error(`FAIL: ${label}`), fails++)); +const refused = (branch, version) => releaseBranchRefusal(branch, version) !== null; + +// main is unchanged: any version, any line. +check(!refused("main", "1.2.2"), "main cuts a patch"); +check(!refused("main", "1.3.0"), "main cuts a minor"); +check(!refused("main", "2.0.0-rc.1"), "main cuts a release candidate"); + +// A maintenance branch cuts its own line only. +check(!refused("release/1.2", "1.2.2"), "release/1.2 cuts 1.2.2"); +check(!refused("release/1.2", "1.2.10"), "release/1.2 cuts 1.2.10"); +check(!refused("release/1.2", "1.2.3-rc.1"), "release/1.2 cuts a 1.2.x release candidate"); +check(refused("release/1.2", "1.3.0"), "release/1.2 refuses 1.3.0 (mismatched minor)"); +check(refused("release/1.2", "1.1.9"), "release/1.2 refuses 1.1.9 (an older minor)"); +check(refused("release/1.2", "2.2.0"), "release/1.2 refuses 2.2.0 (mismatched major)"); +check(refused("release/1.2", "1.20.0"), "release/1.2 refuses 1.20.0 (a prefix is not a match)"); +check(refused("release/1.20", "1.2.0"), "release/1.20 refuses 1.2.0"); +check(releaseBranchRefusal("release/1.2", "1.3.0")?.includes("cuts only 1.2.x") === true, + "the mismatch refusal names the line the branch cuts"); + +// Anything else is refused, as before. +check(refused("release-prep", "1.2.2"), "an ordinary branch is refused"); +check(refused("release/1", "1.2.2"), "release/ alone is not a maintenance branch"); +check(refused("release/1.2.x", "1.2.2"), "release/1.2.x is not a maintenance branch"); +check(refused("fix/release/1.2", "1.2.2"), "a nested name is not a maintenance branch"); +check(refused("HEAD", "1.2.2"), "a detached HEAD is refused"); +check(releaseBranchRefusal("release-prep", "1.2.2")?.includes("not on main") === true, + "the refusal for an ordinary branch still says it is not on main"); + +// `latest` never moves backwards: `bun publish` tags `latest`, so a version older than the +// newest release must be refused. The oracle is npm's own dist-tag semantics: `latest` +// resolves to whichever version was published last, so publishing an older one regresses it. +check(latestRegressionRefusal("1.2.4", "v1.3.0") !== null, + "a version older than the newest release is refused"); +check(latestRegressionRefusal("1.2.4-rc.1", "v1.3.0") !== null, + "an RC on a line a newer release passed is refused too"); +check(latestRegressionRefusal("1.2.4-rc.1", "v1.2.4") !== null, + "an RC of the version that already shipped is refused"); +check(latestRegressionRefusal("1.2.4", "v1.2.4") === null, + "the released version itself is not a regression"); +check(latestRegressionRefusal("1.3.0-rc.1", "v1.2.4") === null, + "an RC of a NEWER triple passes"); +check(latestRegressionRefusal("1.3.1", "v1.3.0") === null, + "a version newer than the newest release passes"); +check(latestRegressionRefusal("1.2.4", "v1.2.3") === null, + "a patch on the newest release line passes"); +check(latestRegressionRefusal("1.2.4", "") === null, + "no released tag yet passes"); +check(latestRegressionRefusal("1.2.4", "v1.3.0")?.includes("`latest` backwards") === true, + "the refusal names the `latest` constraint"); + +// The newest release is resolved by the property that defines the npm line (deploy.yml's +// selector): the newest `vN.N.N` whose cli/package.json is versioned as the tag. A version +// prefix would wrongly pick `v8.2.2` (no cli tree) or the JVM `v7.20.12` (cli 0.20.11). +const cliOf = new Map([["v1.3.0", "1.3.0"], ["v1.2.2", "1.2.2"], ["v7.20.12", "0.20.11"]]); +const fakeOut = (cmd) => { + if (cmd.startsWith("git tag -l")) return "v8.2.2\nv1.3.0\nv1.2.2\nv7.20.12"; + const t = /^git show (v[\d.]+):/.exec(cmd)?.[1]; + const v = t === undefined ? undefined : cliOf.get(t); + return v === undefined ? "" : `{"name":"@metaobjectsdev/cli","version":"${v}"}`; +}; +check(newestNpmReleaseTag(fakeOut) === "v1.3.0", + "the newest npm-line tag wins; a treeless tag and the JVM line are skipped"); + +// A minor or major cut must already hold every commit of the previous maintenance branch, +// or a fix shipped as a patch from it is silently missing from the new line. +const lines = ["release/1.1", "release/1.2", "release/0.25", "release/2.0", "release-prep"]; +check(previousMaintenanceBranch("1.3.0", lines) === "release/1.2", "1.3.0 checks release/1.2"); +check(previousMaintenanceBranch("1.3.0-rc.1", lines) === "release/1.2", "an RC of a minor checks it too"); +check(previousMaintenanceBranch("2.0.0", lines) === "release/1.2", "a major checks the newest line below it"); +check(previousMaintenanceBranch("1.10.0", ["release/1.9", "release/1.2"]) === "release/1.9", + "lines compare numerically, not as strings"); +check(previousMaintenanceBranch("1.3.1", lines) === null, "a patch checks nothing"); +check(previousMaintenanceBranch("1.2.0", ["release/1.2", "release/2.0"]) === null, + "no lower maintenance branch, nothing to check"); +const counts = (n) => () => n; +check(unmergedMaintenanceRefusal("1.3.0", lines, counts(0)) === null, "a fully merged branch passes"); +check(unmergedMaintenanceRefusal("1.3.0", lines, counts(2))?.includes("origin/release/1.2 has 2 commit(s)") === true, + "an unmerged commit refuses the cut and names the branch"); +check(unmergedMaintenanceRefusal("1.3.1", lines, counts(2)) === null, "a patch is not gated"); +check(unmergedMaintenanceRefusal("1.3.0", [], counts(2)) === null, "no maintenance branch, no gate"); + +if (fails > 0) { + console.error(`\n${fails} check(s) failed`); + process.exit(1); +} +console.log("\nall checks passed"); diff --git a/tsconfig.scripts.json b/tsconfig.scripts.json index 1d8f364c5..bde36e2df 100644 --- a/tsconfig.scripts.json +++ b/tsconfig.scripts.json @@ -149,6 +149,7 @@ "scripts/maven-coordinate.mjs", "scripts/eject-release-gate.mjs", "scripts/lib/changelog-promote.mjs", + "scripts/lib/release-branch.mjs", "scripts/check-release-tag.mjs" ] } From 3c4f51916ff3759e2a8a864bdd6c5427a6011bc8 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 06:13:34 -0500 Subject: [PATCH 3/8] docs: one owner for the page-size, unknown-field and validation-issue rules; site gate timeout; ts-poet names - docs/features/api-contract.md owns api.pagination, api.unknownFields and the validation issue list; language-differences, the five port pages and the CHANGELOG point to it instead of restating them. - The site-payload gate runs its tests with a 60 s per-test timeout, so a busy machine does not read as a red release gate. - CHANGELOG names def, literalOf, arrayOf, asOneline and Import.fromMaybeString among the ts-poet names the vendored printer does not carry. - site-payload.json counts the new shared foreign-key-index corpus. --- CHANGELOG.md | 165 +++++++------------------- docs/features/language-differences.md | 6 +- docs/ports/csharp.md | 12 +- docs/ports/java.md | 20 +--- docs/ports/kotlin.md | 17 +-- docs/ports/python.md | 15 +-- docs/ports/typescript.md | 15 +-- examples/showcase/site-payload.json | 2 +- scripts/ci-local.sh | 6 +- scripts/site/counts.ts | 2 +- 10 files changed, 80 insertions(+), 180 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d9f22cd..0843f61e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,94 +79,21 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi - **A page size for generated list routes: `api.pagination`, in every port.** An external review of 1.2.0 found that list endpoints had no hard cap: the TypeScript and C# routes returned the - whole table when `limit` was omitted, and `?limit=100000` was accepted in every port. Set - `"api": { "pagination": { "defaultLimit": 50, "maxLimit": 1000 } }` in - `.metaobjects/config.json` and a generated entity or projection list with no `limit` answers - the first `defaultLimit` rows, while an explicit `limit` above `maxLimit` answers - `400 {"error": "limit_too_large", "maxLimit": N}` (refused, not clamped); a negative `limit` - answers `400 pagination.invalid_value` in every port rather than reaching a store that reads - `LIMIT -1` as no limit, and the generated C# routes answer the same envelope for a `limit` or - `offset` that is not a whole number (`limit=abc`) rather than serving the default page. `meta init` writes - that block for a new project. **A project without the setting behaves exactly as before**, - because a cap added on upgrade would silently truncate a client that reads a whole list; - each port's verify advises (rule `unbounded-list-pages`, never a failure) when a routes - generator is wired and no page size is set (C# under `dotnet meta verify --codegen`). Reports keep their own row cap and opaque views their 1000. Every port's - generated list routes read it: the TypeScript Fastify and Hono routes (through a new - `pagination` option on the runtime mounts), the C# minimal-API routes, the Java and Kotlin - Spring controllers and the Python FastAPI routers. An owned (ejected) routes copy picks it up - when it is re-ejected. It is gated in the generated lane of every port by the new - `fixtures/api-contract-conformance/api-settings/` sub-corpus. Every port's config reader takes a - page size as a positive whole number however JSON spells it (`50.0` is `50`) and at most - 2147483647, and refuses a malformed block when the config is read; the shared cases, valid and - refused, are `api-settings/config-blocks.json`, run by every port. `limit=0` is a page of zero - rows in every port (the C# route served the default page; `api-settings/limit-zero`). An M:N - traversal route takes no `limit` and is not paged by the setting. A config key, not vocabulary: - `metamodelVersion` does not move. - -- **Report measures over Money and other value-object members (FR-044 R12).** A dimension's or - measure's `@of` may name `Entity.field.member`, a member of a value object stored with - `@storage: flattened` (`Payment.amount.amount`, `Payment.amount.currency`); the report reads - the member's own column. A `sum`, `avg`, `min` or `max` of a numeric member loads only when the - report lists a dimension over each non-numeric member of the same value object (its - currency), so amounts in different currencies are never combined; the rule follows the - measure into ratio and derived-measure operands. A member of a jsonb value object, an unknown - member and a deeper path are refused at load, and the Cube exporter refuses member paths. - Loader rules and report-shape derivation in TypeScript, C#, Java and Python. -- **`measure.derived`: arithmetic between measures (FR-044 R13).** `@expr` is FR-037 R5's - first arithmetic wave: `add`, `subtract`, `multiply` and `divide` over two operands, where an - operand is `{ "field": }` (a numeric `measure.aggregate` or `measure.ratio` of the - entity, or a numeric attribute dimension the report lists) or `{ "value": }`. It - expresses a ratio scaled to a percent and a measure over a grouped quantity (utilization = - billable hours × 100 ÷ (weekly capacity × 52 ÷ 12); budget burn = cost ÷ budget). The result - is a nullable `decimal`; `divide` reads null on a zero divisor. `meta migrate` lowers it on - Postgres, SQLite, MySQL and D1, every port reads and serves it, and the Cube exporter writes - one over measures as a `number` member (one reading a dimension is refused). Loader rule M11, - report rule R13 and a conformance fixture per refusal, in every loader port. - -- **`verify.handEdits`: a hand-edited generated file can fail `meta verify`.** `meta verify - --codegen` already listed each generated file carrying a hand edit, but always exited 0, so a - rule changed by hand in generated code (a `max(32)` made `max(64)`) passed the gate. Set - `"verify": { "handEdits": "fail" }` in `.metaobjects/config.json` and it exits 1; `"notice"` - keeps the report and exit 0. `--allow-hand-edits` relaxes `fail` for one run, and the existing - `--forbid-hand-edits` still forces a failure for one run (passing both is refused). The report - names each file, how to see the edit, how to drop it (or move a changed rule into the - metadata), and how to keep it. A project without the key behaves exactly as before. The C#, - Java, Kotlin and Python gates compare generated files byte for byte and already fail on any - edit; the docs now say so. -- **`migrate.foreignKeyIndexes`: an index on every foreign key, without the `efcore` renames.** - With it on, `meta migrate` gives each enforced foreign key that the primary key, a unique key or - an index does not already lead with a `
__idx` index. The Drizzle and Prisma - generators, the C# `AppDbContext` and the Kotlin Exposed tables declare the same indexes under - the same names; Java and Python map no indexes. It adds nothing under the `efcore` conventions - (which already index every foreign key) or on MySQL. A project without the key is unchanged and - keeps the unindexed-foreign-key advisory, which now names the option. Gated by two new - persistence-corpus migration scenarios (adding the indexes to an existing schema, and the - converged no-op). Java's config reader now carries the `migrate` block's - `foreignKeyIndexes`, `conventions` and `dialect` on the loaded root (`MigrateSettings`). - -- **`meta init --skills-dir ` and `meta agent-docs --skills-dir `: install the skills - for any coding assistant.** The `metaobjects-*` skills went only to `.claude/skills/`, which - Claude Code reads. `--skills-dir .agents/skills` (or any project-relative directory) puts them - where another assistant looks; the choice is recorded in `.metaobjects/.agent-context.json`, - so a later `--refresh-docs` keeps it, and moving the directory prunes the untouched copies at - the old one along with the skill directories that leaves empty (the old skills directory - itself stays, and so does any skill directory still holding a file of yours). The always-on file names the directory, so an assistant that does not load - skills opens `//SKILL.md` by path. A directory outside the project is refused. -- **[What differs by language](docs/features/language-differences.md).** One page lists what - the same model does differently per port: that schema migrations and database checks need - the Node `meta` tool in every language, the list default and cap, the validation error body, - filter extras, field names in code, generated data access and hand-edit handling. Every port - page now says up front that migrations and `verify --db` run in Node, and the README's - getting-started path says it for Java, Kotlin, C# and Python teams. -- **The shared render-drift case now holds the email helper too.** 1.3.0 made a template - variable that is not on the payload raise `ERR_VAR_NOT_ON_PAYLOAD` at render time, and - `fixtures/template-output-render-conformance/runtime-drift/` held it for the `@kind: document` - helper and the `template.prompt` helper (the TypeScript `promptRender()` handle and the Python - helper over a Mustache body among them). A re-audit of every generated render path in the five - ports found none that still renders an unknown variable as empty, but the `@kind: email` - helper was not in the case. It now is: `runtime-drift/email-subject/`, `email-html/` and - `email-text/` each drift one part of a `WelcomeEmail`, and every port's runner checks that - the subject, the html body and the text body each raise. No generated code changes. + whole table when `limit` was omitted, and `?limit=100000` was accepted in every port. The new + `api.pagination` block in `.metaobjects/config.json` gives a generated entity or projection list a + default page and refuses a larger `limit`; `meta init` writes it for a new project. **A project + without the setting behaves exactly as before**, because a cap added on upgrade would silently + truncate a client that reads a whole list; each port's verify advises (`unbounded-list-pages`, + never a failure). A negative `limit`, and in C# a `limit` or `offset` that is not a whole number, + now answers the malformed-bound 400 rather than reaching the store, and `limit=0` is a page of + zero rows in every port (the C# route served the default page). Every port's generated list + routes read it (TypeScript through a new `pagination` option on the runtime mounts); an owned + (ejected) routes copy picks it up when it is re-ejected. Gated in the generated lane of every port + by the new `fixtures/api-contract-conformance/api-settings/` sub-corpus, whose + `config-blocks.json` every port's config reader runs. The answers, limits and exceptions are in + [docs/features/api-contract.md](docs/features/api-contract.md#sort--pagination). A config key, not + vocabulary: `metamodelVersion` does not move. + ### Changed @@ -174,43 +101,37 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi Existing projects get `reject` when they regenerate.** Until now every port dropped an undeclared body key silently, so a client that misspelled a field (`emial`) got a 200 and lost the value without a word. A generated create, PUT or PATCH route now answers - `400 {"error": "unknown_fields", "fields": ["emial"]}` (the undeclared top-level keys, sorted) - before validation, and writes nothing. To keep the old behaviour set - `"api": { "unknownFields": "drop" }` in `.metaobjects/config.json` and regenerate. A declared - key the client may not set (a generated `id`, a `readOnly` or derived field, an `@autoSet` - stamp, a `writeOnce` field on update, the TPH discriminator, a sibling subtype's column) is - still dropped, not refused, under either value, so a client that sends back a row it read keeps - working. `meta init` writes `"unknownFields": "reject"` for a new project. Every port checks the - raw body before binding it, so Java and Kotlin answer the same whatever the host's Jackson - `FAIL_ON_UNKNOWN_PROPERTIES` says. TypeScript passes the declared field list to the runtime - mounts as a new `knownFields` option; a mount given none keeps dropping. Pinned by - `input-unknown-fields-rejected` and `tph/tph-unknown-fields-rejected` (both lanes of every - port) and `api-settings/unknown-fields-dropped` (`drop`, generated lane). Upgrade notes: + `400 unknown_fields` before validation and writes nothing. To keep the old behaviour set + `"api": { "unknownFields": "drop" }` in `.metaobjects/config.json` and regenerate. A declared key + the client may not set is still dropped, not refused, so a client that sends back a row it read + keeps working. `meta init` writes `reject` for a new project. TypeScript passes the declared + field list to the runtime mounts as a new `knownFields` option (a mount given none keeps + dropping); Java and Kotlin read the raw body, so the answer does not depend on Jackson's + `FAIL_ON_UNKNOWN_PROPERTIES`, and the Java value-object records nested in a write route's body now + carry `@JsonIgnoreProperties(ignoreUnknown = true)` (they answered `400 invalid_type` before). + Pinned by `input-unknown-fields-rejected`, `tph/tph-unknown-fields-rejected` and + `jsonb/jsonb-nested-undeclared-key-dropped` (both lanes of every port) and + `api-settings/unknown-fields-dropped` (`drop`, generated lane). The rule, which keys count as + declared and how each port matches them: + [docs/features/api-contract.md](docs/features/api-contract.md#unknown-request-fields-apiunknownfields). + Upgrade notes: [docs/features/migrations/unknown-fields-reject-by-default.md](docs/features/migrations/unknown-fields-reject-by-default.md). - Only the top level is judged: a member a value object does not declare is dropped, never - refused and never stored, on create and on PATCH, while the open JSON bag keeps every key - (`jsonb/jsonb-nested-undeclared-key-dropped`, both lanes of every port). The Java generated - value-object records nested in a write route's body now carry `@JsonIgnoreProperties(ignoreUnknown = true)` for this; they - answered `400 invalid_type` before. C# matches a key the way it binds the body, with the app's - JSON options: under the ASP.NET web default, which ignores case, `Title` is the declared - `title` and is not refused. A config key, not vocabulary: `metamodelVersion` does not move. - **A validation 400 lists every issue in every port.** The same model answered a failed create or update differently by language: TypeScript listed each problem, while C#, Java, Kotlin and Python returned a bare `{"error": "validation"}` (found by an external review of 1.2.0). Every port now - answers `400 {"error": "validation", "issues": [{"code", "path", "message"}]}` with every failure, - not the first. `path` is an array of wire names (value-object members, array indexes and map keys - as further elements), `code` uses Zod's names (`invalid_type`, `too_small`, `too_big`, - `invalid_format`, `invalid_value`, `custom`), and `message` is each port's own wording. C# ships a - new `ValidationIssues` runtime file (also copied by `dotnet meta eject routes`), and its create - route now reads the body itself so an unbindable body lists its issue rather than answering an - empty 400. Java ships `ValidationIssues` beside `PatchValidationException`, and a patch binds every - field before it throws. Kotlin emits `MetaValidationIssues.kt` with the controllers. Python maps - every Pydantic error in `constraint_errors`. The hand-rolled reference servers list issues too, - and two shared scenarios pin the list in both lanes (`validation-lists-issues`, - `tph/tph-validation-lists-issues`, with a new `issues` corpus assertion). A client that read only - `error` is unaffected. An owned (ejected) routes copy keeps the bare answer until it is re-ejected. + answers with an `issues` list holding every failure, in the shape + [docs/features/api-contract.md](docs/features/api-contract.md#a-validation-error-lists-every-issue) + defines. C# ships a new `ValidationIssues` runtime file (also copied by `dotnet meta eject routes`), + and its create route now reads the body itself so an unbindable body lists its issue rather than + answering an empty 400. Java ships `ValidationIssues` beside `PatchValidationException`, and a + patch binds every field before it throws. Kotlin emits `MetaValidationIssues.kt` with the + controllers. Python maps every Pydantic error in `constraint_errors`. The hand-rolled reference + servers list issues too, and two shared scenarios pin the list in both lanes + (`validation-lists-issues`, `tph/tph-validation-lists-issues`, with a new `issues` corpus + assertion). A client that read only `error` is unaffected. An owned (ejected) routes copy keeps + the bare answer until it is re-ejected. - **The always-on agent file works with any coding assistant, carries the REST conventions, and is written once.** `.metaobjects/CLAUDE.md`, an identical copy of `.metaobjects/AGENTS.md` @@ -287,8 +208,10 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi `codegen-ts-angular` drop their `ts-poet` dependency and import the printer from `@metaobjectsdev/codegen-ts/engine`, which an ejected template also uses. A bare `ts-poet` import in an older owned generator is still pointed at that entry by `meta gen`, which - carries `code`, `imp`, `joinCode`, `Import` and the `Code` type; ts-poet's `saveFiles`, - `conditionalOutput` and its `toString` import options are not carried over. A fresh npm + carries `code`, `imp`, `joinCode`, `Import` and the `Code` type; ts-poet's `def`, `literalOf`, + `arrayOf`, `asOneline`, `Import.fromMaybeString`, `saveFiles`, `conditionalOutput` and its + `toString` import options are not carried over, so an owned generator that imports one of them + no longer loads. A fresh npm install of the CLI drops from 151 MB (134,641,989 bytes) to 133 MB (115,302,944 bytes) of `node_modules`. `scripts/check-peer-ranges.ts` now also fails a publishable package that requires `dprint-node`, `ts-poet` or any `@types/*` package through `dependencies`, diff --git a/docs/features/language-differences.md b/docs/features/language-differences.md index b61818186..7449425db 100644 --- a/docs/features/language-differences.md +++ b/docs/features/language-differences.md @@ -34,11 +34,9 @@ project. | **Generated data access** | Drizzle tables and row types (`entityFile()`), CRUD queries (`queriesFile()`) | EF Core entities and `AppDbContext` | Spring DTOs and a repository interface | Exposed tables and a repository base | None: the generated router calls a repository `Protocol` you implement | | **Runtime metadata access** | `ObjectManager` on Kysely (`@metaobjectsdev/runtime-ts`) | None yet (roadmap) | OMDB (JDBC, Spring transactions) | Through Java's OMDB | `ObjectManager` on DB-API 2; not wired to the generated router | | **Generated REST routes** | Fastify (`routesFile()`; Hono and Drizzle mounts in `runtime-ts`) | ASP.NET minimal API | Spring `@RestController` | Spring `@RestController` | FastAPI `APIRouter` | -| **List with no `limit`** | `api.pagination.defaultLimit` rows when the project sets it; otherwise every matching row | Same; otherwise every matching row | Same; otherwise the first 50 | Same; otherwise the first 50 | Same; otherwise the first 50 | -| **Largest `limit` on an entity list** | `api.pagination.maxLimit`, then 400 `limit_too_large`, when the project sets it; otherwise no cap (an opaque raw-SQL view: 1,000) | Same; otherwise no cap | Same; otherwise no cap | Same; otherwise no cap | Same; otherwise no cap | +| **List with no `limit` and no `api.pagination`** | Every matching row | Every matching row | The first 50 | The first 50 | The first 50 | | **Largest report page** | 10,000 rows, then 400 `report_too_large` | Same | Same | Same | Same | -| **Validation error body** | 400 `{"error": "validation", "issues": [...]}`, every issue | Same (`message` is each port's own wording) | Same | Same | Same | -| **A write body key the entity does not declare** | 400 `{"error": "unknown_fields", "fields": [...]}` under `api.unknownFields: reject` (the default); ignored under `drop` | Same | Same | Same | Same | +| **`api.pagination`, `api.unknownFields`, the validation `issues` list** | The same in every port; defined once in [api-contract.md](api-contract.md) ([page size](api-contract.md#sort--pagination), [unknown fields](api-contract.md#unknown-request-fields-apiunknownfields), [validation issues](api-contract.md#a-validation-error-lists-every-issue)) | | | | | | **Filter and sort errors** | 400 with `field`, plus `op`, `expected` and the allowed fields | 400 with `field` | 400 with `field` | 400 with `field` | 400 with `field` | | **`?status=active` (a bare field, not `filter[status][eq]`)** | 400 `filter.bare_field`, naming the form that works | Ignored: every row comes back | Ignored | Ignored | Ignored | | **Filter extras beyond the nine operators** | `?search=`, `filter[or]` / `filter[and]`, leading-`%` refusal, filter-value format check | None | None | None | None | diff --git a/docs/ports/csharp.md b/docs/ports/csharp.md index c4e3e83b2..07bd1af32 100644 --- a/docs/ports/csharp.md +++ b/docs/ports/csharp.md @@ -1082,14 +1082,10 @@ directly into the generated list handler. A read-only projection on their list route, against an allowlist of their own fields. The `api` block of `.metaobjects/config.json` reaches the generated routes through the loaded root -(`MetaRoot.Api`). `api.pagination` (`defaultLimit`, `maxLimit`) gives every entity, TPH and projection -list route a default page and answers a larger `?limit=` with `400 { "error": "limit_too_large", -"maxLimit": N }`; a served report keeps its own row ceiling, and with no block the routes are unchanged. -`api.unknownFields` (`reject`, the default, or `drop`) makes a create, PUT or PATCH answer -`400 { "error": "unknown_fields", "fields": [...] }` for a body key no field of the entity (for a TPH -hierarchy, of the base or any subtype) declares; the check is `UnknownFieldCheck` in the helper runtime. -It matches a key the way the route binds the body, with the app's System.Text.Json options: under the -ASP.NET web default, which ignores case, `Title` is the declared `title`. `limit=0` is a page of zero rows. +(`MetaRoot.Api`): `api.pagination` pages every entity, TPH and projection list route, and +`api.unknownFields` is checked by `UnknownFieldCheck` in the helper runtime, matching a key the way +the route binds the body (the app's System.Text.Json options). What each setting answers is in +[api-contract](../features/api-contract.md) ([page size](../features/api-contract.md#sort--pagination), [unknown fields](../features/api-contract.md#unknown-request-fields-apiunknownfields)). `dotnet meta init` writes `.metaobjects/config.json` with the same new-project settings as `meta init`: `api.pagination` 50/1000, `api.unknownFields: reject`, `verify.handEdits: fail` and `migrate.foreignKeyIndexes: true`; an existing config is left alone. `dotnet meta verify --codegen` prints a diff --git a/docs/ports/java.md b/docs/ports/java.md index e86af6b9c..fdc6940fe 100644 --- a/docs/ports/java.md +++ b/docs/ports/java.md @@ -599,20 +599,12 @@ carries `naming` and `wire` (`NeutralConfig.getApiSettings()`, carried on the lo { "api": { "pagination": { "defaultLimit": 50, "maxLimit": 1000 }, "unknownFields": "reject" } } ``` -- `api.pagination`: a list route (entity, TPH base and per-subtype, projection) with no `limit` - returns `defaultLimit` rows, and a `limit` above `maxLimit` answers - `400 {"error": "limit_too_large", "maxLimit": N}`. Absent, a list route returns 50 rows when - `limit` is omitted and takes any `limit`, as before; `mvn metaobjects:verify` then warns - (`unbounded-list-pages`, advisory, never fails the build). A served report keeps its own row - ceiling, and an M:N traversal is not paged. -- `api.unknownFields` (`reject`, the default, or `drop`): a create, PUT or PATCH body key the - entity does not declare answers `400 {"error": "unknown_fields", "fields": [...]}` (keys in code - point order) and writes nothing. Declared keys the client may not set (the generated key, - `readOnly`, derived, `@autoSet`, `writeOnce` on update, the discriminator, a sibling - subtype's column) are not unknown: they are dropped as before. The keys are read off the raw - `JsonNode` (`UnknownBodyKeys`, shipped with the generated runtime) before the body is bound, so - the answer does not depend on the host `ObjectMapper`'s `FAIL_ON_UNKNOWN_PROPERTIES`. Under - `reject` the generated create handler takes `@RequestBody JsonNode body` rather than the DTO. +What each setting answers, and the default with no block, are in +[api-contract](../features/api-contract.md) ([page size](../features/api-contract.md#sort--pagination), [unknown fields](../features/api-contract.md#unknown-request-fields-apiunknownfields)). +Java-specific: with no `api.pagination`, `mvn metaobjects:verify` warns (`unbounded-list-pages`, +advisory, never fails the build). The body keys are read off the raw `JsonNode` +(`UnknownBodyKeys`, shipped with the generated runtime) before the body is bound, so under +`reject` the generated create handler takes `@RequestBody JsonNode body` rather than the DTO. ### Decimals and null members on the wire diff --git a/docs/ports/kotlin.md b/docs/ports/kotlin.md index 5952df51c..df6297ad1 100644 --- a/docs/ports/kotlin.md +++ b/docs/ports/kotlin.md @@ -653,18 +653,13 @@ controllers are byte-identical to today and `?format=` is ignored like any unkno `KotlinSpringControllerGenerator` reads the project's `api` block from the loaded root (`loader.root.apiSettings`, the same `.metaobjects/config.json` reader as `naming` and `wire`). -With `api.pagination` every generated list route (entity, TPH base and per-subtype, projection; -not a served report or an M:N traversal) uses `defaultLimit` when `limit` is omitted and answers a -`limit` above `maxLimit` with `400 {"error": "limit_too_large", "maxLimit": N}`. Without it the -routes keep their historic default of 50 and no cap. With `api.unknownFields` `reject` (the -default) create, PUT and PATCH read the raw `JsonNode` body, list any top-level key the entity does -not declare (for a TPH base the union of the base's and every subtype's fields) as -`400 {"error": "unknown_fields", "fields": [...]}` in code-point order, and write nothing; this -happens before binding, so the answer is the same whatever the host `ObjectMapper` does with -unknown properties. The check is generated into a per-package `MetaUnknownBodyKeys.kt` (no +What each setting answers, and the default with no block, are in +[api-contract](../features/api-contract.md) ([page size](../features/api-contract.md#sort--pagination), [unknown fields](../features/api-contract.md#unknown-request-fields-apiunknownfields)). +Kotlin-specific: under `reject` (the default) create, PUT and PATCH read the raw `JsonNode` body +and check its keys before binding (for a TPH base, against the union of the base's and every +subtype's fields). The check is generated into a per-package `MetaUnknownBodyKeys.kt` (no MetaObjects import; it ships with the `routes` eject). Because the create handler binds the raw -body, the controller now always takes the `ObjectMapper`. `drop` keeps today's behaviour and emits -no helper. +body, the controller now always takes the `ObjectMapper`. `drop` emits no helper. The Kotlin controllers import nothing from MetaObjects, so the request side (format parsing, the three 400 envelopes, the `Content-Disposition` and `Content-Type` values) is generated into the shared diff --git a/docs/ports/python.md b/docs/ports/python.md index b36d186c0..ce08e98f5 100644 --- a/docs/ports/python.md +++ b/docs/ports/python.md @@ -420,16 +420,11 @@ The routers read the `api` block of `.metaobjects/config.json` from the loaded r { "api": { "pagination": { "defaultLimit": 50, "maxLimit": 1000 }, "unknownFields": "reject" } } ``` -With `api.pagination`, an entity, projection or single-table list router answers `defaultLimit` -rows when `limit` is omitted, `400 {"error": "limit_too_large", "maxLimit": N}` above the cap and -`400 pagination.invalid_value` for a negative `limit`; a served report keeps its own ceiling. -Without it the routers keep their default of 50 and no cap, and `metaobjects verify` prints an -`unbounded-list-pages` advisory that never changes the exit code. With `api.unknownFields: -reject` (the default) a create, PUT or PATCH body carrying a key no field declares answers -`400 {"error": "unknown_fields", "fields": [...]}` (`unknown_body_keys` in -`metaobjects.codegen.runtime.constraint_errors`) before validation; `drop` ignores it. A declared -key the client may not set is dropped under either value. The contract is in -[api-contract](../features/api-contract.md#unknown-request-fields-apiunknownfields). +What each setting answers, and the default with no block, are in +[api-contract](../features/api-contract.md) ([page size](../features/api-contract.md#sort--pagination), [unknown fields](../features/api-contract.md#unknown-request-fields-apiunknownfields)). +Python-specific: with no `api.pagination`, `metaobjects verify` prints an `unbounded-list-pages` +advisory that never changes the exit code, and the unknown-key check is `unknown_body_keys` in +`metaobjects.codegen.runtime.constraint_errors`, run before validation. ### Decimals and null members on the wire diff --git a/docs/ports/typescript.md b/docs/ports/typescript.md index db8a7c25b..7841706cd 100644 --- a/docs/ports/typescript.md +++ b/docs/ports/typescript.md @@ -481,15 +481,12 @@ mounts (`mountCrudRoutes` in `@metaobjectsdev/runtime-ts/drizzle-fastify` and `/ { "api": { "pagination": { "defaultLimit": 50, "maxLimit": 1000 }, "unknownFields": "reject" } } ``` -`api.pagination` becomes the mount's `pagination` option: no `limit` answers `defaultLimit` rows -and a `limit` above `maxLimit` is `400 {"error": "limit_too_large", "maxLimit": N}`. Without it a -list returns every row, and `meta verify` advises (`unbounded-list-pages`). Under -`api.unknownFields: reject` (the default) the generated mount passes the entity's declared field -names as `knownFields`, and a create, PUT or PATCH body with any other key answers -`400 {"error": "unknown_fields", "fields": [...]}` before Zod runs; under `drop`, or on a -hand-written mount given no `knownFields`, the key is ignored. `meta init` writes both settings -for a new project. The contract is in -[api-contract](../features/api-contract.md#unknown-request-fields-apiunknownfields). +`api.pagination` becomes the mount's `pagination` option, and under `api.unknownFields: reject` the +generated mount passes the entity's declared field names as `knownFields`, checked before Zod runs; a +hand-written mount given no `knownFields` ignores an undeclared key. What each setting answers, and +the default with no block, are in [api-contract](../features/api-contract.md) ([page size](../features/api-contract.md#sort--pagination), +[unknown fields](../features/api-contract.md#unknown-request-fields-apiunknownfields)); the per-port defaults are in +[What differs by language](../features/language-differences.md). ### Decimals and null members on the wire diff --git a/examples/showcase/site-payload.json b/examples/showcase/site-payload.json index 3678dd161..77dc60265 100644 --- a/examples/showcase/site-payload.json +++ b/examples/showcase/site-payload.json @@ -8,7 +8,7 @@ }, "counts": { "fixtures": 510, - "corpora": 39, + "corpora": 40, "baseTypes": 17 }, "snippets": { diff --git a/scripts/ci-local.sh b/scripts/ci-local.sh index 7818d9919..17428d585 100755 --- a/scripts/ci-local.sh +++ b/scripts/ci-local.sh @@ -450,7 +450,11 @@ gate_no_api_prefix() { bun scripts/check-no-api-prefix.ts; } # documented pre-PR command. The other four ports are checked by `gate_showcase_all_ports` # in the heavy tier of the flagless run, and again by the release preflight # (`--check --all-ports`); both refuse to leave a port out. -gate_site_payload() { bun_install && bun test scripts/site && bun scripts/build-site-payload.ts --check; } +# +# `--timeout 60000`: several of these tests (registries, llms, reference) build real +# output and fit bun's default 5 s per test on an idle machine but not under heavy load, +# where a release gate read red with nothing wrong. A minute per test still fails a hang. +gate_site_payload() { bun_install && bun test --timeout 60000 scripts/site && bun scripts/build-site-payload.ts --check; } # ── the showcase is current in EVERY port, not just ts+sql ──────────────────── # `gate_site_payload` can only afford `--bun-only`, so a change to the python, csharp, diff --git a/scripts/site/counts.ts b/scripts/site/counts.ts index 07fe26594..8208d00ed 100644 --- a/scripts/site/counts.ts +++ b/scripts/site/counts.ts @@ -15,7 +15,7 @@ * * It lives under `scripts/site/` for two reasons: it feeds the payload, and this is the * only directory in `scripts/` whose bun tests a CI lane actually runs - * (`gate_site_payload` → `bun test scripts/site`). A counts test anywhere else in + * (`gate_site_payload` → `bun test --timeout 60000 scripts/site`). A counts test anywhere else in * `scripts/` would sit unrun, which is how a gate becomes decoration. */ import { readdirSync, readFileSync } from "node:fs"; From 8d59242004219fe7b17e742729110cef4308ab33 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 06:33:18 -0500 Subject: [PATCH 4/8] no-mistakes(review): Restore changelog bullets; cut long index names --- CHANGELOG.md | 68 ++++++++++++++++- .../migrate-ts/src/expected-schema.ts | 16 ++-- .../packages/migrate-ts/src/types.ts | 5 ++ .../test/long-declared-index-name.test.ts | 75 +++++++++++++++++++ 4 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 server/typescript/packages/migrate-ts/test/long-declared-index-name.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0843f61e3..e127efdf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,70 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi [docs/features/api-contract.md](docs/features/api-contract.md#sort--pagination). A config key, not vocabulary: `metamodelVersion` does not move. +- **Report measures over Money and other value-object members (FR-044 R12).** A dimension's or + measure's `@of` may name `Entity.field.member`, a member of a value object stored with + `@storage: flattened` (`Payment.amount.amount`, `Payment.amount.currency`); the report reads + the member's own column. A `sum`, `avg`, `min` or `max` of a numeric member loads only when the + report lists a dimension over each non-numeric member of the same value object (its + currency), so amounts in different currencies are never combined; the rule follows the + measure into ratio and derived-measure operands. A member of a jsonb value object, an unknown + member and a deeper path are refused at load, and the Cube exporter refuses member paths. + Loader rules and report-shape derivation in TypeScript, C#, Java and Python. +- **`measure.derived`: arithmetic between measures (FR-044 R13).** `@expr` is FR-037 R5's + first arithmetic wave: `add`, `subtract`, `multiply` and `divide` over two operands, where an + operand is `{ "field": }` (a numeric `measure.aggregate` or `measure.ratio` of the + entity, or a numeric attribute dimension the report lists) or `{ "value": }`. It + expresses a ratio scaled to a percent and a measure over a grouped quantity (utilization = + billable hours × 100 ÷ (weekly capacity × 52 ÷ 12); budget burn = cost ÷ budget). The result + is a nullable `decimal`; `divide` reads null on a zero divisor. `meta migrate` lowers it on + Postgres, SQLite, MySQL and D1, every port reads and serves it, and the Cube exporter writes + one over measures as a `number` member (one reading a dimension is refused). Loader rule M11, + report rule R13 and a conformance fixture per refusal, in every loader port. + +- **`verify.handEdits`: a hand-edited generated file can fail `meta verify`.** `meta verify + --codegen` already listed each generated file carrying a hand edit, but always exited 0, so a + rule changed by hand in generated code (a `max(32)` made `max(64)`) passed the gate. Set + `"verify": { "handEdits": "fail" }` in `.metaobjects/config.json` and it exits 1; `"notice"` + keeps the report and exit 0. `--allow-hand-edits` relaxes `fail` for one run, and the existing + `--forbid-hand-edits` still forces a failure for one run (passing both is refused). The report + names each file, how to see the edit, how to drop it (or move a changed rule into the + metadata), and how to keep it. A project without the key behaves exactly as before. The C#, + Java, Kotlin and Python gates compare generated files byte for byte and already fail on any + edit; the docs now say so. +- **`migrate.foreignKeyIndexes`: an index on every foreign key, without the `efcore` renames.** + With it on, `meta migrate` gives each enforced foreign key that the primary key, a unique key or + an index does not already lead with a `
__idx` index. The Drizzle and Prisma + generators, the C# `AppDbContext` and the Kotlin Exposed tables declare the same indexes under + the same names; Java and Python map no indexes. It adds nothing under the `efcore` conventions + (which already index every foreign key) or on MySQL. A project without the key is unchanged and + keeps the unindexed-foreign-key advisory, which now names the option. Gated by two new + persistence-corpus migration scenarios (adding the indexes to an existing schema, and the + converged no-op). Java's config reader now carries the `migrate` block's + `foreignKeyIndexes`, `conventions` and `dialect` on the loaded root (`MigrateSettings`). + +- **`meta init --skills-dir ` and `meta agent-docs --skills-dir `: install the skills + for any coding assistant.** The `metaobjects-*` skills went only to `.claude/skills/`, which + Claude Code reads. `--skills-dir .agents/skills` (or any project-relative directory) puts them + where another assistant looks; the choice is recorded in `.metaobjects/.agent-context.json`, + so a later `--refresh-docs` keeps it, and moving the directory prunes the untouched copies at + the old one along with the skill directories that leaves empty (the old skills directory + itself stays, and so does any skill directory still holding a file of yours). The always-on file names the directory, so an assistant that does not load + skills opens `//SKILL.md` by path. A directory outside the project is refused. +- **[What differs by language](docs/features/language-differences.md).** One page lists what + the same model does differently per port: that schema migrations and database checks need + the Node `meta` tool in every language, the list default and cap, the validation error body, + filter extras, field names in code, generated data access and hand-edit handling. Every port + page now says up front that migrations and `verify --db` run in Node, and the README's + getting-started path says it for Java, Kotlin, C# and Python teams. +- **The shared render-drift case now holds the email helper too.** 1.3.0 made a template + variable that is not on the payload raise `ERR_VAR_NOT_ON_PAYLOAD` at render time, and + `fixtures/template-output-render-conformance/runtime-drift/` held it for the `@kind: document` + helper and the `template.prompt` helper (the TypeScript `promptRender()` handle and the Python + helper over a Mustache body among them). A re-audit of every generated render path in the five + ports found none that still renders an unknown variable as empty, but the `@kind: email` + helper was not in the case. It now is: `runtime-drift/email-subject/`, `email-html/` and + `email-text/` each drift one part of a `WelcomeEmail`, and every port's runner checks that + the subject, the html body and the text body each raise. No generated code changes. ### Changed @@ -253,7 +317,9 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi so a derived `
__fk` over the limit is no longer proposed as a rename. A new shared corpus, `fixtures/foreign-key-index-conformance/`, pins which foreign keys get an index and its name, and the TypeScript, C# and Kotlin rules all pass it; two migration scenarios - prove a long name converges on Postgres. + prove a long name converges on Postgres. A declared `index.lookup` or `identity.secondary` name + over 63 bytes is compared under the name Postgres stores in the same way, so it no longer + reappears on every run either. - **`mvn metaobjects:verify` refuses the model `mvn metaobjects:generate` refuses.** Two metadata packages that generate into one output directory (`ERR_NAME_COLLISION`) and a REST collection claimed twice (`ERR_COLLECTION_NAME_COLLISION`) were checked by the generate goal only, so a diff --git a/server/typescript/packages/migrate-ts/src/expected-schema.ts b/server/typescript/packages/migrate-ts/src/expected-schema.ts index d83e6aa43..cf291013d 100644 --- a/server/typescript/packages/migrate-ts/src/expected-schema.ts +++ b/server/typescript/packages/migrate-ts/src/expected-schema.ts @@ -83,7 +83,7 @@ import type { import { qualifiedDbName } from "./qualified-name.js"; import { SQLITE_ISO_NOW } from "./column-default.js"; import { viewFingerprint } from "./view-fingerprint.js"; -import { DIALECT_SQLITE, PG_IDENTIFIER_MAX_BYTES, isSqliteFamily, pgStoredIdentifier } from "./types.js"; +import { DIALECT_SQLITE, PG_IDENTIFIER_MAX_BYTES, isSqliteFamily, storedIdentifier } from "./types.js"; import { resolveReferentialActions, findReferentialActionConflict, @@ -462,7 +462,7 @@ export function buildExpectedSchemaWithProvenance( // Name the DECLARATION an index came from, so a collision says where to rename it. for (const node of [...entity.secondaryIdentities(), ...entity.lookupIndexes()]) { indexOwnerLabels.set( - `${tableName}.${resolveIndexName(node)}`, + `${tableName}.${storedIdentifier(resolveIndexName(node), dialect)}`, `${node.type}.${node.subType} "${resolveIndexName(node)}" on ${entity.resolutionKey()} (table "${tableName}")`, ); } @@ -709,7 +709,7 @@ function buildTable( const descriptor: TableDescriptor = { name: tableName, columns, - indexes: buildSecondaryIndexes(entity, tableName, strategy), + indexes: buildSecondaryIndexes(entity, tableName, strategy, dialect), foreignKeys: buildForeignKeys(entity, tableName, resolveTargetTable, root, strategy, fkCtx, onConflict), checks: buildChecks(entity, tableName, strategy, dialect, skipRedundantChecks), primaryKey, @@ -731,7 +731,7 @@ function readDescription(node: { attr: (n: string) => unknown }): string | undef } function buildSecondaryIndexes( - entity: MetaObject, tableName: string, strategy: ColumnNamingStrategy, + entity: MetaObject, tableName: string, strategy: ColumnNamingStrategy, dialect: Dialect | undefined, ): IndexDescriptor[] { const indexes: IndexDescriptor[] = []; @@ -744,7 +744,7 @@ function buildSecondaryIndexes( if (field.attr(FIELD_ATTR_UNIQUE) !== true) continue; const colName = resolveColumnName(field, strategy); indexes.push({ - name: `${tableName}_${colName}_unique`, + name: storedIdentifier(`${tableName}_${colName}_unique`, dialect), columns: [colName], unique: true, }); @@ -774,7 +774,7 @@ function buildSecondaryIndexes( // function used to spell the same answer independently, which is how // `fdb4118f1` happened. One door, so the strip and the empty-name refusal // cannot apply to one caller and not the other. - name: resolveIndexName(identity), + name: storedIdentifier(resolveIndexName(identity), dialect), columns: expr ? [] : cols, unique: true, }; @@ -815,7 +815,7 @@ function buildSecondaryIndexes( }); const index: IndexDescriptor = { // See the identity.secondary site above — one resolver for both. - name: resolveIndexName(lookup), + name: storedIdentifier(resolveIndexName(lookup), dialect), columns: expr ? [] : cols, unique: false, }; @@ -1451,7 +1451,7 @@ function applySchemaConventions( */ export function foreignKeyIndexName(table: string, columns: readonly string[], dialect?: Dialect): string { const name = `${table}_${columns.join("_")}_idx`; - return isSqliteFamily(dialect) ? name : pgStoredIdentifier(name); + return storedIdentifier(name, dialect); } /** True when `prefix` leads with exactly `columns`, in any order (the advisory's rule, too). */ diff --git a/server/typescript/packages/migrate-ts/src/types.ts b/server/typescript/packages/migrate-ts/src/types.ts index c8cc130a9..e77928709 100644 --- a/server/typescript/packages/migrate-ts/src/types.ts +++ b/server/typescript/packages/migrate-ts/src/types.ts @@ -732,3 +732,8 @@ export function pgStoredIdentifier(name: string): string { } return out; } + +/** The name `dialect` stores for `name`: whole on SQLite and D1, cut to Postgres's 63 bytes elsewhere. */ +export function storedIdentifier(name: string, dialect: Dialect | undefined): string { + return isSqliteFamily(dialect) ? name : pgStoredIdentifier(name); +} diff --git a/server/typescript/packages/migrate-ts/test/long-declared-index-name.test.ts b/server/typescript/packages/migrate-ts/test/long-declared-index-name.test.ts new file mode 100644 index 000000000..2e9a55fa4 --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/long-declared-index-name.test.ts @@ -0,0 +1,75 @@ +// A declared index name over Postgres's 63-byte identifier limit is created under its first +// 63 bytes, and introspection reads that cut name back. The expected schema carries the name +// Postgres stores, so a converged database proposes nothing on the next run. SQLite and D1 +// store the name whole. +import { describe, test, expect } from "bun:test"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import type { MetaData } from "@metaobjectsdev/metadata"; +import { buildExpectedSchema } from "../src/expected-schema.js"; +import { diff } from "../src/diff/index.js"; + +const LONG_LOOKUP = `idx_events_${"a".repeat(59)}`; +const LONG_IDENTITY = `uq_events_${"b".repeat(60)}`; +const STORED_LOOKUP = LONG_LOOKUP.slice(0, 63); +const STORED_IDENTITY = LONG_IDENTITY.slice(0, 63); + +function model(lookupName: string, identityName: string): string { + return JSON.stringify({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "Event", + children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "userId" } }, + { "field.string": { name: "slug" } }, + { "source.rdb": { "@table": "events" } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + { "identity.secondary": { name: identityName, "@fields": ["slug"] } }, + { "index.lookup": { name: lookupName, "@fields": ["userId"] } }, + ], + }, + }, + ], + }, + }); +} + +async function load(json: string): Promise { + return (await new MetaDataLoader().load([new InMemoryStringSource(json)])).root; +} + +describe("a declared index name over 63 bytes on Postgres", () => { + test("the declared names are over the limit (the premise)", () => { + expect(LONG_LOOKUP.length).toBe(70); + expect(LONG_IDENTITY.length).toBe(70); + }); + + test("a database holding the names Postgres stored converges: nothing is proposed", async () => { + const root = await load(model(LONG_LOOKUP, LONG_IDENTITY)); + const expected = buildExpectedSchema(root, { dialect: "postgres" }); + const live = structuredClone(expected); + const events = live.tables.find((t) => t.name === "events")!; + // What introspection returns: the 63-byte names Postgres stored, written out literally. + for (const ix of events.indexes) { + if (ix.name.startsWith("idx_events_")) ix.name = STORED_LOOKUP; + if (ix.name.startsWith("uq_events_")) ix.name = STORED_IDENTITY; + } + expect(events.indexes.map((i) => i.name).sort()).toEqual([STORED_LOOKUP, STORED_IDENTITY].sort()); + expect((await diff({ expected, actual: live })).changes).toEqual([]); + }); + + test("two long names that cut to the same 63 bytes are refused as duplicates", async () => { + const root = await load(model(`${"c".repeat(63)}_x`, `${"c".repeat(63)}_y`)); + expect(() => buildExpectedSchema(root, { dialect: "postgres" })).toThrow(/ERR_DUPLICATE_SQL_NAME/); + }); + + test("SQLite keeps the declared name whole", async () => { + const root = await load(model(LONG_LOOKUP, LONG_IDENTITY)); + const expected = buildExpectedSchema(root, { dialect: "sqlite" }); + const names = expected.tables.find((t) => t.name === "events")!.indexes.map((i) => i.name); + expect(names.sort()).toEqual([LONG_LOOKUP, LONG_IDENTITY].sort()); + }); +}); From a5ef614ef098e9fc2b6c746bc432b8d3349a6d31 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 06:38:09 -0500 Subject: [PATCH 5/8] no-mistakes(review): Rename case-only layout-collision test resources --- .../com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java | 2 +- .../layout-collision/{meta.sales.json => meta.sales-lower.json} | 0 .../layout-collision/{meta.Sales.json => meta.sales-upper.json} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename server/java/maven-plugin/src/test/resources/mojo/layout-collision/{meta.sales.json => meta.sales-lower.json} (100%) rename server/java/maven-plugin/src/test/resources/mojo/layout-collision/{meta.Sales.json => meta.sales-upper.json} (100%) diff --git a/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java index 97faa5b52..b9e21f5b2 100644 --- a/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java +++ b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyModelRefusalTest.java @@ -22,7 +22,7 @@ private static void configure(AbstractMetaDataMojo mojo, Path outputDir) { mojo.setLoader(LoaderParam.builder("layout-collision") .withClassname("com.metaobjects.loader.MetaDataLoader") .withSourceDir("./src/test/resources") - .withSources(List.of("mojo/layout-collision/meta.Sales.json", "mojo/layout-collision/meta.sales.json")) + .withSources(List.of("mojo/layout-collision/meta.sales-upper.json", "mojo/layout-collision/meta.sales-lower.json")) .build()); mojo.setGenerators(List.of(GeneratorParam.builder(KotlinEntityGenerator.class.getName()) .withArg("outputDir", outputDir.toString()) diff --git a/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-lower.json similarity index 100% rename from server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales.json rename to server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-lower.json diff --git a/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-upper.json similarity index 100% rename from server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.Sales.json rename to server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-upper.json From 01600bb7fa3e9b61a4fcc11b34b7a9b07b6d1e8e Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 07:28:18 -0500 Subject: [PATCH 6/8] no-mistakes(document): docs: document the declared-index name cut --- CHANGELOG.md | 6 +++--- docs/features/migrations-and-drift.md | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e127efdf1..0c5ec2cb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -317,9 +317,9 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi so a derived `
__fk` over the limit is no longer proposed as a rename. A new shared corpus, `fixtures/foreign-key-index-conformance/`, pins which foreign keys get an index and its name, and the TypeScript, C# and Kotlin rules all pass it; two migration scenarios - prove a long name converges on Postgres. A declared `index.lookup` or `identity.secondary` name - over 63 bytes is compared under the name Postgres stores in the same way, so it no longer - reappears on every run either. + prove a long name converges on Postgres. A declared `index.lookup` or `identity.secondary` + name, and a `@unique` field's derived `
__unique`, is created under the name + Postgres stores in the same way, so it no longer reappears on every run either. - **`mvn metaobjects:verify` refuses the model `mvn metaobjects:generate` refuses.** Two metadata packages that generate into one output directory (`ERR_NAME_COLLISION`) and a REST collection claimed twice (`ERR_COLLECTION_NAME_COLLISION`) were checked by the generate goal only, so a diff --git a/docs/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 61712c859..6d875f6d6 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -521,6 +521,13 @@ program compares against (a unique-violation handler checking the constraint nam declared in metadata: `@constraintName` on an `identity.reference`, or the index node's own name. A declared name always wins over a convention. +On Postgres an index name over the 63-byte identifier limit is created cut to its first 63 +bytes — the name Postgres stores — whether the name is declared (`index.lookup`, +`identity.secondary`) or derived (a `@unique` field's `
__unique`, and the +[foreign-key indexes](#foreign-key-indexes-migrateforeignkeyindexes)); two names that cut to the +same 63 bytes are refused with `ERR_DUPLICATE_SQL_NAME`. A declared CHECK name is the exception, +refused rather than cut, so the CHECK always matches its declaration. + A project can supply its own conventions from `metaobjects.config.ts`, which wins over the preset name. Start from a preset so every member is present; an object missing one is refused by name: From d3fff181037890189b90414a934bb2aaf3842aa2 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 08:12:50 -0500 Subject: [PATCH 7/8] fix: generated tables declare a long declared index name as Postgres stores it meta migrate creates and compares a declared index.lookup / identity.secondary name over 63 bytes under the name Postgres stores (its first 63 bytes). The Drizzle tables, the EF Core context and the Exposed tables now declare that cut name on Postgres (SQLite and D1 keep the whole name; MySQL is exempt), so the mapping names the index the database holds. The names artifacts keep the declared, dialect-neutral name. The shared corpus fixtures/foreign-key-index-conformance/ gains a long index.lookup and identity.secondary and declared..json, written by migrate-ts and read by the TypeScript, C# and Kotlin generator tests. C# ForeignKeyIndexes joins the eject engine surface, as the schema contract CheckConstraints already is. --- CHANGELOG.md | 4 +- docs/CONFORMANCE.md | 2 +- docs/features/migrations-and-drift.md | 7 +- docs/ports/kotlin.md | 4 +- .../codegen/templates/drizzle-schema.ts | 15 +++- .../codegen/templates/drizzle-schema.ts | 15 +++- .../foreign-key-index-conformance/README.md | 14 +++- .../declared.postgres.json | 41 +++++++++++ .../declared.sqlite.json | 41 +++++++++++ .../input/meta.fk.json | 4 ++ .../registry.json | 2 + .../EjectedGeneratorsAloneTests.cs | 2 + .../ForeignKeyIndexConformanceTests.cs | 68 ++++++++++++++++++- .../Generators/DbContextGenerator.cs | 9 ++- .../Generators/ForeignKeyIndexes.cs | 28 ++++++-- .../kotlin/KotlinExposedTableGenerator.kt | 38 ++++++++--- .../KotlinForeignKeyIndexConformanceTest.kt | 41 ++++++++++- .../codegen-ts/src/foreign-key-indexes.ts | 11 +++ .../src/templates/drizzle-schema.ts | 15 +++- .../test/declared-index-name-cut.test.ts | 63 +++++++++++++++++ .../packages/migrate-ts/src/expected.ts | 1 + .../foreign-key-index-conformance.test.ts | 37 +++++++++- .../src/templates/drizzle-schema.ts | 15 +++- 23 files changed, 435 insertions(+), 42 deletions(-) create mode 100644 fixtures/foreign-key-index-conformance/declared.postgres.json create mode 100644 fixtures/foreign-key-index-conformance/declared.sqlite.json create mode 100644 server/typescript/packages/codegen-ts/test/declared-index-name-cut.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5ec2cb2..0714a58b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -319,7 +319,9 @@ loaded before stops loading. This vocabulary ships in 1.3.0 with `metamodelVersi and its name, and the TypeScript, C# and Kotlin rules all pass it; two migration scenarios prove a long name converges on Postgres. A declared `index.lookup` or `identity.secondary` name, and a `@unique` field's derived `
__unique`, is created under the name - Postgres stores in the same way, so it no longer reappears on every run either. + Postgres stores in the same way, so it no longer reappears on every run either. The Drizzle + tables, the EF context and the Exposed tables declare that cut name too; the shared corpus + pins it with a long `index.lookup` and `identity.secondary` (`declared..json`). - **`mvn metaobjects:verify` refuses the model `mvn metaobjects:generate` refuses.** Two metadata packages that generate into one output directory (`ERR_NAME_COLLISION`) and a REST collection claimed twice (`ERR_COLLECTION_NAME_COLLISION`) were checked by the generate goal only, so a diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index efc0e6df5..7cfecafe7 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -61,7 +61,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/wire-conformance/`](../fixtures/wire-conformance/) (the `wire` setting, ADR-0019 Amendment 2) | 6 cases + 8 refusals | ✓ | ✓ | inherits via Java (one config reader) | ✓ | ✓ | | [`fixtures/enum-length-conformance/`](../fixtures/enum-length-conformance/) (a string-stored enum's column length: `@maxLength`, else `enums.maxLength`) | 5 cases + 1 overflow + 5 refusals; run by a port that sizes the column | ✓ | ✓ | inherits via Java (one config reader, one resolver) | ✓ | — (sizes no column: emits no DDL or column type; the config reader accepts and shape-checks the `enums` block) | | [`fixtures/check-constraint-conformance/`](../fixtures/check-constraint-conformance/) (the CHECK constraints `meta migrate` derives: name and expression text per dialect) | 1 model, 5 expected files (postgres, sqlite, d1, and both skip-redundant variants) | ✓ (reference, `migrate-ts`) | — (no CHECK in a generated model) | — (same) | ✓ (the EF context declares them; live-Postgres `EnsureCreated` lane) | — (same) | -| [`fixtures/foreign-key-index-conformance/`](../fixtures/foreign-key-index-conformance/) (the indexes `migrate.foreignKeyIndexes` adds: which foreign keys are uncovered, and each index's name per dialect, a name over 63 bytes cut on Postgres) | 1 model, 2 expected files (postgres, sqlite) | ✓ (reference, `migrate-ts`) | — (no index in a generated model) | ✓ (the Exposed tables declare them) | ✓ (the EF context declares them) | — (same as Java) | +| [`fixtures/foreign-key-index-conformance/`](../fixtures/foreign-key-index-conformance/) (the indexes `migrate.foreignKeyIndexes` adds: which foreign keys are uncovered, and each index's name per dialect, a name over 63 bytes cut on Postgres; and the name each declared `identity.secondary` / `index.lookup` is created under) | 1 model, 4 expected files (FK and declared, postgres and sqlite) | ✓ (reference, `migrate-ts`; the Drizzle tables declare the declared names) | — (no index in a generated model) | ✓ (the Exposed tables declare them) | ✓ (the EF context declares them) | — (same as Java) | | [`fixtures/codegen-noop/`](../fixtures/codegen-noop/) (FR-044 — reporting vocabulary: what is lowered, what stays inert) | 1 model pair (`reporting/with` vs `reporting/without`) | ✓ (codegen + migrate) | ✓ | ✓ | ✓ | ✓ | | [`fixtures/cube-model/`](../fixtures/cube-model/) (FR-044 Plan 4 — the `cube-model` generator's golden Cube files, plus the canonical model's golden) | 46 cases (36 trees + 10 errors) + 1 canonical golden | ✓ (the exporter is TypeScript-only, spec R6; its `cube` lane also loads the files into a real Cube) | — | — | — | — | | [`fixtures/contract-conformance/`](../fixtures/contract-conformance/) (FR-022 — the `contract-schema` and `openapi` generators' golden trees and refusals) | 19 cases (14 trees + 5 errors) | ✓ (one neutral engine, ADR-0034 Amendment 10; the generated API lanes also hold real traffic to the documents) | — | — | — | — | diff --git a/docs/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 6d875f6d6..7ede1738b 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -526,7 +526,12 @@ bytes — the name Postgres stores — whether the name is declared (`index.look `identity.secondary`) or derived (a `@unique` field's `
__unique`, and the [foreign-key indexes](#foreign-key-indexes-migrateforeignkeyindexes)); two names that cut to the same 63 bytes are refused with `ERR_DUPLICATE_SQL_NAME`. A declared CHECK name is the exception, -refused rather than cut, so the CHECK always matches its declaration. +refused rather than cut, so the CHECK always matches its declaration. The generated Drizzle +tables, EF Core context and Exposed tables declare the cut name on Postgres as well, so the +mapping names the index the database holds; the generated names artifacts (`Names`, +`IDENTITY__INDEX`) keep the declared name, which is dialect-neutral. MySQL refuses an +over-long name rather than cutting it, and `meta migrate` writes no MySQL DDL, so nothing is cut +there. A project can supply its own conventions from `metaobjects.config.ts`, which wins over the preset name. Start from a preset so every member is present; an object missing one is refused diff --git a/docs/ports/kotlin.md b/docs/ports/kotlin.md index df6297ad1..23d4af8b6 100644 --- a/docs/ports/kotlin.md +++ b/docs/ports/kotlin.md @@ -758,7 +758,9 @@ names — has a member for each instead of one member between them. An `identity.secondary` or `index.lookup` also carries `IDENTITY__INDEX` / `INDEX__INDEX`, the database index name that the generated -`init { uniqueIndex(…) }` / `init { index(…) }` block references; +`init { uniqueIndex(…) }` / `init { index(…) }` block references (on Postgres a +name over 63 bytes is spelled in the table as the first 63 bytes, the name the +database stores, while the constant keeps the declared name); `identity.primary` deliberately carries none, because migrate names a primary key by a dialect-conditional formula this artifact must not restate. diff --git a/examples/advanced-modeling/codegen/templates/drizzle-schema.ts b/examples/advanced-modeling/codegen/templates/drizzle-schema.ts index e35ed5146..2a9c7ea95 100644 --- a/examples/advanced-modeling/codegen/templates/drizzle-schema.ts +++ b/examples/advanced-modeling/codegen/templates/drizzle-schema.ts @@ -26,7 +26,7 @@ import { dialectModule } from "@metaobjectsdev/codegen-ts/engine"; import { namesRef, physicalNameExpr, sourceSchemaExpr, indexNameExpr, columnExpr, } from "@metaobjectsdev/codegen-ts/engine"; -import { resolveTableSchema } from "@metaobjectsdev/metadata"; +import { resolveTableSchema, resolveIndexName } from "@metaobjectsdev/metadata"; import { renderEnumIntCustomType } from "./enum-int-codec.js"; import { renderDurationCustomType } from "./duration-wire-helper.js"; import { renderRelationsBlock } from "./relations-block.js"; @@ -35,7 +35,7 @@ import { collectTphSubtypeFields, tphConcreteSubtypes } from "./tph-discriminato import { tphStorageObject } from "@metaobjectsdev/codegen-ts/engine"; import { flattenedMembersOf } from "@metaobjectsdev/codegen-ts/engine"; import { effectivePackage } from "@metaobjectsdev/codegen-ts/engine"; -import { foreignKeyIndexesOf } from "@metaobjectsdev/codegen-ts/engine"; +import { foreignKeyIndexesOf, storedIndexName } from "@metaobjectsdev/codegen-ts/engine"; /** * Render the Drizzle table definition for one entity, including: @@ -384,7 +384,16 @@ export function renderDrizzleSchema( // `idx_
_` against an index the database actually held under // `identity.name` (fdb4118f1); and an empty `index.lookup` name — which the loader // accepts, unlike an identity — is refused here rather than emitting `index("")`. - const indexName = indexNameExpr(names, node); + // + // Except past Postgres's 63-byte identifier limit: migrate creates and compares such an + // index under the name Postgres stores (`storedIdentifier`, its first 63 bytes; SQLite + // keeps the whole name), so the mapping spells that cut name as a literal. The names + // artifact is dialect-neutral and keeps the declared name. + const declaredIndexName = resolveIndexName(node); + const storedName = storedIndexName(declaredIndexName, dialect); + const indexName = storedName === declaredIndexName + ? indexNameExpr(names, node) + : code`${JSON.stringify(storedName)}`; const head: Code = using !== undefined && using !== "btree" ? code`${indexSym}(${indexName}).using(${JSON.stringify(using)}, ${target})` diff --git a/examples/showcase/codegen/templates/drizzle-schema.ts b/examples/showcase/codegen/templates/drizzle-schema.ts index e35ed5146..2a9c7ea95 100644 --- a/examples/showcase/codegen/templates/drizzle-schema.ts +++ b/examples/showcase/codegen/templates/drizzle-schema.ts @@ -26,7 +26,7 @@ import { dialectModule } from "@metaobjectsdev/codegen-ts/engine"; import { namesRef, physicalNameExpr, sourceSchemaExpr, indexNameExpr, columnExpr, } from "@metaobjectsdev/codegen-ts/engine"; -import { resolveTableSchema } from "@metaobjectsdev/metadata"; +import { resolveTableSchema, resolveIndexName } from "@metaobjectsdev/metadata"; import { renderEnumIntCustomType } from "./enum-int-codec.js"; import { renderDurationCustomType } from "./duration-wire-helper.js"; import { renderRelationsBlock } from "./relations-block.js"; @@ -35,7 +35,7 @@ import { collectTphSubtypeFields, tphConcreteSubtypes } from "./tph-discriminato import { tphStorageObject } from "@metaobjectsdev/codegen-ts/engine"; import { flattenedMembersOf } from "@metaobjectsdev/codegen-ts/engine"; import { effectivePackage } from "@metaobjectsdev/codegen-ts/engine"; -import { foreignKeyIndexesOf } from "@metaobjectsdev/codegen-ts/engine"; +import { foreignKeyIndexesOf, storedIndexName } from "@metaobjectsdev/codegen-ts/engine"; /** * Render the Drizzle table definition for one entity, including: @@ -384,7 +384,16 @@ export function renderDrizzleSchema( // `idx_
_` against an index the database actually held under // `identity.name` (fdb4118f1); and an empty `index.lookup` name — which the loader // accepts, unlike an identity — is refused here rather than emitting `index("")`. - const indexName = indexNameExpr(names, node); + // + // Except past Postgres's 63-byte identifier limit: migrate creates and compares such an + // index under the name Postgres stores (`storedIdentifier`, its first 63 bytes; SQLite + // keeps the whole name), so the mapping spells that cut name as a literal. The names + // artifact is dialect-neutral and keeps the declared name. + const declaredIndexName = resolveIndexName(node); + const storedName = storedIndexName(declaredIndexName, dialect); + const indexName = storedName === declaredIndexName + ? indexNameExpr(names, node) + : code`${JSON.stringify(storedName)}`; const head: Code = using !== undefined && using !== "btree" ? code`${indexSym}(${indexName}).using(${JSON.stringify(using)}, ${target})` diff --git a/fixtures/foreign-key-index-conformance/README.md b/fixtures/foreign-key-index-conformance/README.md index ec3d95a26..1c584fc46 100644 --- a/fixtures/foreign-key-index-conformance/README.md +++ b/fixtures/foreign-key-index-conformance/README.md @@ -18,10 +18,15 @@ what holds the three to one answer. a `@constraintName`, because the default `
__fk` name would collide with the single-column one. - `customer_order_line_item_allocations_by_region`: a foreign key whose index name is over - 63 bytes. + 63 bytes, and a declared `identity.secondary` and `index.lookup` whose names are over 63 + bytes. - `expected..json`: `{ "
": [{ "name", "columns" }] }` for every table that gets a foreign-key index, in the order `meta migrate` adds them. `postgres` cuts a name over 63 bytes to its first 63 bytes (the name Postgres stores); `sqlite` keeps the whole name. +- `declared..json`: the same shape for the declared indexes (`identity.secondary`, + `index.lookup`): the name `meta migrate` creates each under, cut the same way. A generated + model must declare these names, or a long declared name maps to an index the database does not + hold. ## Runners @@ -29,10 +34,13 @@ what holds the three to one answer. builds the expected schema and compares each file. It is the reference: `META_WRITE_FK_INDEXES=1 bun test test/foreign-key-index-conformance.test.ts` (in `migrate-ts`) rewrites the files after a deliberate change, which is then reviewed by hand. + `server/typescript/packages/codegen-ts/test/declared-index-name-cut.test.ts` holds the + generated Drizzle tables to `declared..json`. - C#: `server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs` reads - `ForeignKeyIndexes.For`, the rule the generated EF context declares from. + `ForeignKeyIndexes.For`, the rule the generated EF context declares from, and + `ForeignKeyIndexes.DeclaredIndexName` against `declared..json`. - Kotlin: `server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt` - reads the Exposed table generator's `foreignKeyIndexes`. + reads the Exposed table generator's `foreignKeyIndexes` and `declaredIndexName`. Java and Python declare no index in a generated model (schema comes from `meta migrate`, ADR-0015), so they do not run it. The `efcore` conventions and MySQL add no index of this kind diff --git a/fixtures/foreign-key-index-conformance/declared.postgres.json b/fixtures/foreign-key-index-conformance/declared.postgres.json new file mode 100644 index 000000000..700087e47 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/declared.postgres.json @@ -0,0 +1,41 @@ +{ + "customer_order_line_item_allocations_by_region": [ + { + "name": "allocation_external_reference_key_for_customer_order_line_items", + "columns": [ + "external_reference" + ] + }, + { + "name": "customer_order_line_item_allocations_by_region_region_code_look", + "columns": [ + "region_code" + ] + } + ], + "customers": [ + { + "name": "customers_code_key", + "columns": [ + "code" + ] + } + ], + "orders": [ + { + "name": "orders_bill_to", + "columns": [ + "bill_to_id", + "id" + ] + } + ], + "shipments": [ + { + "name": "shipments_open_returns", + "columns": [ + "return_carrier_id" + ] + } + ] +} diff --git a/fixtures/foreign-key-index-conformance/declared.sqlite.json b/fixtures/foreign-key-index-conformance/declared.sqlite.json new file mode 100644 index 000000000..c36e3c2b4 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/declared.sqlite.json @@ -0,0 +1,41 @@ +{ + "customer_order_line_item_allocations_by_region": [ + { + "name": "allocation_external_reference_key_for_customer_order_line_items_by_region", + "columns": [ + "external_reference" + ] + }, + { + "name": "customer_order_line_item_allocations_by_region_region_code_lookup", + "columns": [ + "region_code" + ] + } + ], + "customers": [ + { + "name": "customers_code_key", + "columns": [ + "code" + ] + } + ], + "orders": [ + { + "name": "orders_bill_to", + "columns": [ + "bill_to_id", + "id" + ] + } + ], + "shipments": [ + { + "name": "shipments_open_returns", + "columns": [ + "return_carrier_id" + ] + } + ] +} diff --git a/fixtures/foreign-key-index-conformance/input/meta.fk.json b/fixtures/foreign-key-index-conformance/input/meta.fk.json index 8303c18b6..4e0b086fe 100644 --- a/fixtures/foreign-key-index-conformance/input/meta.fk.json +++ b/fixtures/foreign-key-index-conformance/input/meta.fk.json @@ -62,7 +62,11 @@ { "field.long": { "name": "id" } }, { "field.long": { "name": "fulfilmentWarehouseReferenceId", "@column": "fulfilment_warehouse_reference_id" } }, { "field.long": { "name": "orderId", "@column": "order_id" } }, + { "field.string": { "name": "externalReference", "@column": "external_reference" } }, + { "field.string": { "name": "regionCode", "@column": "region_code" } }, { "identity.primary": { "name": "pk", "@fields": "id" } }, + { "identity.secondary": { "name": "allocation_external_reference_key_for_customer_order_line_items_by_region", "@fields": ["externalReference"] } }, + { "index.lookup": { "name": "customer_order_line_item_allocations_by_region_region_code_lookup", "@fields": ["regionCode"] } }, { "identity.reference": { "name": "warehouse", "@fields": ["fulfilmentWarehouseReferenceId"], "@references": "Customer" } }, { "identity.reference": { "name": "order", "@fields": ["orderId"], "@references": "Order" } } ] } } diff --git a/fixtures/generator-registry-conformance/registry.json b/fixtures/generator-registry-conformance/registry.json index 3ae5b8e03..d536a236c 100644 --- a/fixtures/generator-registry-conformance/registry.json +++ b/fixtures/generator-registry-conformance/registry.json @@ -412,6 +412,8 @@ "MetaObjects.Codegen.Generators.CheckConstraint", "MetaObjects.Codegen.Generators.CheckConstraints", "MetaObjects.Codegen.Generators.FindInbound", + "MetaObjects.Codegen.Generators.ForeignKeyIndex", + "MetaObjects.Codegen.Generators.ForeignKeyIndexes", "MetaObjects.Codegen.Generators.InboundShape", "MetaObjects.Codegen.Generators.TphPlan", "MetaObjects.Codegen.Generators.TphPlanBuilder", diff --git a/server/csharp/MetaObjects.Codegen.Tests/EjectedGeneratorsAloneTests.cs b/server/csharp/MetaObjects.Codegen.Tests/EjectedGeneratorsAloneTests.cs index 02663ed1c..5bb22bf54 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/EjectedGeneratorsAloneTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/EjectedGeneratorsAloneTests.cs @@ -57,6 +57,8 @@ public class EjectedGeneratorsAloneTests ["MetaObjects.Codegen.CSharpNaming"] = "the naming and type-mapping tables", ["MetaObjects.Codegen.Generators.CheckConstraints"] = "the CHECKs meta migrate derives: a schema contract both delivery paths must agree on, not emitted code an owned copy should change", ["MetaObjects.Codegen.Generators.CheckConstraint"] = "one of those CHECKs, a name and an expression", + ["MetaObjects.Codegen.Generators.ForeignKeyIndexes"] = "the indexes and index names meta migrate creates (foreign-key indexes, and a declared name cut to what Postgres stores): a schema contract both delivery paths must agree on, held by fixtures/foreign-key-index-conformance", + ["MetaObjects.Codegen.Generators.ForeignKeyIndex"] = "one of those indexes, a name and its columns", ["MetaObjects.Codegen.ValueObjectNames"] = "the one answer to what a value object's type is called and where it lives", ["MetaObjects.Codegen.PackageBindingResolver"] = "package -> namespace binding", ["MetaObjects.Codegen.OutputPaths"] = "where an object's file lands under the output layout, and the claim that no two share a path: a path table, ruled engine", diff --git a/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs b/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs index 0a5c54c07..e6ac5fe19 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs @@ -1,7 +1,9 @@ // The shared foreign-key index corpus, fixtures/foreign-key-index-conformance/: which foreign // keys `migrate.foreignKeyIndexes` indexes and under what name, per dialect. The TypeScript // migrate suite writes and passes the same expected files, so the EF model declares exactly the -// indexes `meta migrate` creates. +// indexes `meta migrate` creates. `declared..json` pins the database name of every +// identity.secondary and index.lookup, so a declared name over 63 bytes is cut on Postgres the +// way migrate cuts it, in the EF model as in the database. using System.Text.Json; using MetaObjects.Codegen.Generators; @@ -52,4 +54,68 @@ public void The_indexes_are_meta_migrates(string file, string dialect) foreach (var (table, indexes) in expected) Assert.Equal(indexes, actual[table]); } + + public static TheoryData DeclaredCases => new() + { + { "declared.postgres.json", "postgres" }, + { "declared.sqlite.json", "sqlite" }, + }; + + [Theory] + [MemberData(nameof(DeclaredCases))] + public void Declared_index_names_are_meta_migrates(string file, string dialect) + { + var load = MetaDataLoader.FromDirectory(Path.Combine(Corpus(), "input")); + Assert.Empty(load.Errors); + var actual = new Dictionary>(); + foreach (var o in load.Root.Objects().Where(o => o.IsEntity() && o.DbTable is not null)) + { + var nodes = o.SecondaryIdentities().Select(i => ((MetaData)i, i.Fields)) + .Concat(o.LookupIndexes().Select(i => ((MetaData)i, i.Fields))); + var names = nodes + .Select(n => $"{ForeignKeyIndexes.DeclaredIndexName(n.Item1, dialect)}({string.Join(",", n.Fields.Select(f => CSharpNaming.Column(o.FindField(f)!, ColumnNamingStrategy.SnakeCase)))})") + .ToList(); + if (names.Count > 0) actual[CSharpNaming.Table(o)] = names; + } + + using var doc = JsonDocument.Parse(File.ReadAllText(Path.Combine(Corpus(), file))); + var expected = doc.RootElement.EnumerateObject().ToDictionary( + t => t.Name, + t => t.Value.EnumerateArray() + .Select(i => $"{i.GetProperty("name").GetString()}({string.Join(",", i.GetProperty("columns").EnumerateArray().Select(c => c.GetString()))})") + .ToList()); + + Assert.Equal(expected.Keys.OrderBy(k => k, StringComparer.Ordinal), actual.Keys.OrderBy(k => k, StringComparer.Ordinal)); + foreach (var (table, indexes) in expected) + Assert.Equal(indexes, actual[table]); + } + + [Theory] + [InlineData("postgres", "customer_order_line_item_allocations_by_region_region_code_look", "allocation_order_reference_lines_by_customer_and_region_order_r")] + [InlineData("sqlite", "customer_order_line_item_allocations_by_region_region_code_lookup", "allocation_order_reference_lines_by_customer_and_region_order_ref_unique")] + public void The_EF_model_declares_the_name_the_database_stores(string dialect, string lookup, string unique) + { + var load = MetaDataLoader.FromDirectory(Path.Combine(Corpus(), "input")); + Assert.Empty(load.Errors); + // A @unique field on a long table: its `
__unique` name is cut the same way. + var extra = new MetaDataLoader().Load([new InMemoryStringSource(""" + { "metadata.root": { "package": "acme", "children": [ + { "object.entity": { "name": "Long", "children": [ + { "source.rdb": { "@table": "allocation_order_reference_lines_by_customer_and_region" } }, + { "field.long": { "name": "id" } }, + { "field.string": { "name": "orderRef", "@column": "order_ref", "@unique": true } }, + { "identity.primary": { "name": "pk", "@fields": "id" } } + ] } } + ] } } + """, id: "long.json")]); + Assert.Empty(extra.Errors); + string Render(MetaObjects.Meta.MetaRoot root) => string.Join("\n", new DbContextGenerator().Generate(new GenContext + { + Entities = root.Objects(), Root = root, Warn = _ => { }, + Config = new GenConfig { OutDir = "/unused", Namespace = "Acme", Dialect = dialect }, + }).Select(f => f.Content)); + var context = Render(load.Root); + Assert.Contains($".HasDatabaseName(\"{lookup}\");", context); + Assert.Contains($".HasDatabaseName(\"{unique}\");", Render(extra.Root)); + } } diff --git a/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs index 83754cdd0..cd08e3653 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs @@ -1327,7 +1327,7 @@ private void EmitIndexConfig(string owner, MetaObject entity, GenContext ctx, Li if (f.Attr(FIELD_ATTR_UNIQUE) is not true || f.ResolvedIsArray()) continue; var column = CSharpNaming.Column(f, ctx.Config.ColumnNamingStrategy); modelLines.Add($" modelBuilder.Entity<{owner}>().HasIndex(x => x.{PropertyName(f)}).IsUnique()" + - $".HasDatabaseName(\"{CSharpNaming.CSharpStringLiteral($"{table}_{column}_unique")}\");"); + $".HasDatabaseName(\"{CSharpNaming.CSharpStringLiteral(ForeignKeyIndexes.StoredName($"{table}_{column}_unique", ctx.Config.Dialect))}\");"); } var nodes = entity.SecondaryIdentities().Select(i => ((MetaData)i, i.Fields, true)) .Concat(entity.LookupIndexes().Select(i => ((MetaData)i, i.Fields, false))) @@ -1343,7 +1343,12 @@ private void EmitIndexConfig(string owner, MetaObject entity, GenContext ctx, Li props.Add(PropertyName(mf)); } if (props.Count == 0) continue; - var indexName = CSharpNaming.IndexRef(entity, node, ctx.Config.ColumnNamingStrategy, ctx.Config.IncludeNames, ctx.Config); + // Past Postgres's 63-byte limit migrate creates the index under the name Postgres stores, + // so the model spells that cut name; the names artifact keeps the declared one. + var storedName = ForeignKeyIndexes.DeclaredIndexName(node, ctx.Config.Dialect); + var indexName = storedName == IndexNaming.ResolveIndexName(node) + ? CSharpNaming.IndexRef(entity, node, ctx.Config.ColumnNamingStrategy, ctx.Config.IncludeNames, ctx.Config) + : $"\"{CSharpNaming.CSharpStringLiteral(storedName)}\""; var selector = props.Count == 1 ? $"x => x.{props[0]}" : $"x => new {{ {string.Join(", ", props.Select(p => "x." + p))} }}"; // The NAMED overload: two indexes over one column list (a partial one beside a plain // one) are two EF indexes only when each carries its own name. diff --git a/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs b/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs index 8d6d5959f..3a1ce90ca 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs @@ -98,14 +98,22 @@ public static IReadOnlyList For(MetaObject entity, TphPlan? tph } /// - /// <table>_<column>[_<column>…]_idx. Except on SQLite and D1, a name over 63 - /// bytes is cut to its first 63 bytes on a character boundary: the name Postgres stores, and - /// the one meta migrate expects. + /// <table>_<column>[_<column>…]_idx, cut by : the + /// name meta migrate expects. /// - public static string Name(string table, IReadOnlyList columns, string? dialect) + public static string Name(string table, IReadOnlyList columns, string? dialect) => + StoredName($"{table}_{string.Join("_", columns)}_idx", dialect); + + /// + /// The name the database stores for , which is the name meta + /// migrate creates and compares (migrate-ts storedIdentifier): on Postgres a name + /// over 63 bytes is cut to its first 63 bytes on a character boundary. SQLite and D1 keep the + /// whole name, and so does MySQL, where migrate writes no DDL and an over-long name is refused + /// rather than cut. + /// + public static string StoredName(string name, string? dialect) { - var name = $"{table}_{string.Join("_", columns)}_idx"; - if (dialect is "sqlite" or "d1" || Encoding.UTF8.GetByteCount(name) <= PgIdentifierMaxBytes) return name; + if (dialect is "sqlite" or "d1" or "mysql" || Encoding.UTF8.GetByteCount(name) <= PgIdentifierMaxBytes) return name; var sb = new StringBuilder(); var bytes = 0; foreach (var rune in name.EnumerateRunes()) @@ -118,6 +126,14 @@ public static string Name(string table, IReadOnlyList columns, string? d return sb.ToString(); } + /// + /// The database name of an identity.secondary or index.lookup: its declared name + /// (), cut by . The shared + /// corpus fixtures/foreign-key-index-conformance/ (declared.<dialect>.json) pins it. + /// + public static string DeclaredIndexName(MetaData node, string? dialect) => + StoredName(IndexNaming.ResolveIndexName(node), dialect); + /// True when leads with exactly , /// in any order (migrate's leadsWith). internal static bool LeadsWith(IReadOnlyList prefix, IReadOnlyList columns) diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt index a08bb6aeb..6046f1a96 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt @@ -1214,12 +1214,12 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase ForeignKeyIndex(foreignKeyIndexName(tableName, cols, settings.dialect), cols, fields) } } - protected fun indexNameExpr(node: MetaData, shortName: String): String { + protected fun indexNameExpr(node: MetaData, shortName: String, dialect: String?): String { val name = IndexNaming.resolve(node) + // Past Postgres's 63-byte limit migrate creates the index under the name Postgres stores, + // so the table spells that cut name; the names artifact keeps the declared one. + val stored = declaredIndexName(node, dialect) + if (stored != name) return "\"${KotlinGenUtil.kotlinStringLiteral(stored)}\"" if (!useNames()) return "\"$name\"" val member = "${KotlinNaming.namesMember(node.type)}_${KotlinNaming.namesMember(name)}_INDEX" return "${KotlinNaming.namesObjectName(shortName)}.$member" @@ -2790,13 +2794,27 @@ data class ForeignKeyIndex(val name: String, val columns: List, val fiel private const val PG_IDENTIFIER_MAX_BYTES = 63 /** - * `
_[_…]_idx`, migrate's name for a foreign key's index. Except on SQLite - * and D1, a name over 63 bytes is cut to its first 63 bytes on a character boundary: the name - * Postgres stores, and the one `meta migrate` expects. + * `
_[_…]_idx`, migrate's name for a foreign key's index, cut by + * [storedIdentifier]. */ -fun foreignKeyIndexName(table: String, columns: List, dialect: String?): String { - val name = "${table}_${columns.joinToString("_")}_idx" - if (dialect == "sqlite" || dialect == "d1" || name.toByteArray(Charsets.UTF_8).size <= PG_IDENTIFIER_MAX_BYTES) return name +fun foreignKeyIndexName(table: String, columns: List, dialect: String?): String = + storedIdentifier("${table}_${columns.joinToString("_")}_idx", dialect) + +/** + * The database name of an `identity.secondary` or `index.lookup`: its declared name + * ([IndexNaming.resolve]), cut by [storedIdentifier]. The shared corpus + * fixtures/foreign-key-index-conformance/ (`declared..json`) pins it. + */ +fun declaredIndexName(node: MetaData, dialect: String?): String = storedIdentifier(IndexNaming.resolve(node), dialect) + +/** + * The name the database stores for [name], which is the name `meta migrate` creates and compares + * (migrate-ts `storedIdentifier`): on Postgres a name over 63 bytes is cut to its first 63 bytes on + * a character boundary. SQLite and D1 keep the whole name, and so does MySQL, where migrate writes + * no DDL and an over-long name is refused rather than cut. + */ +fun storedIdentifier(name: String, dialect: String?): String { + if (dialect == "sqlite" || dialect == "d1" || dialect == "mysql" || name.toByteArray(Charsets.UTF_8).size <= PG_IDENTIFIER_MAX_BYTES) return name val out = StringBuilder() var bytes = 0 var i = 0 diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt index 2bb223c1b..29f4143c9 100644 --- a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt @@ -3,6 +3,7 @@ package com.metaobjects.generator.kotlin import com.fasterxml.jackson.databind.ObjectMapper import com.metaobjects.config.MigrateSettings import com.metaobjects.loader.MetaDataLoader +import com.metaobjects.index.LookupIndex import com.metaobjects.metadata.ktx.loadString import com.metaobjects.`object`.MetaObject import java.nio.file.Files @@ -15,7 +16,9 @@ import kotlin.test.assertTrue * The shared foreign-key index corpus, `fixtures/foreign-key-index-conformance/`: which foreign * keys `migrate.foreignKeyIndexes` indexes and under what name, per dialect. The TypeScript * migrate suite writes and passes the same expected files, so the Exposed table declares exactly - * the indexes `meta migrate` creates. + * the indexes `meta migrate` creates. `declared..json` pins the database name of every + * `identity.secondary` and `index.lookup`, so a declared name over 63 bytes is cut on Postgres the + * way migrate cuts it, in the Exposed table as in the database. */ class KotlinForeignKeyIndexConformanceTest { @@ -49,4 +52,40 @@ class KotlinForeignKeyIndexConformanceTest { @Test fun `postgres - the indexes are meta migrate's`() = check("expected.postgres.json", "postgres") @Test fun `sqlite - the indexes are meta migrate's`() = check("expected.sqlite.json", "sqlite") + + private fun checkDeclared(file: String, dialect: String) { + val loader = loadString("fk-index-conformance", Files.readString(corpus.resolve("input/meta.fk.json"))) + val actual = sortedMapOf>() + for (entity in loader.metaObjects) { + val table = KotlinGenUtil.primaryRdbSource(entity)?.physicalName ?: continue + // ADR-0039: resolving — identities and index nodes may be inherited. + val nodes = entity.getIdentities(true).filter { it.isSecondary }.map { it to it.fields } + + entity.getChildren(LookupIndex::class.java, true).map { it to it.fields } + val names = nodes.map { (node, fields) -> + val cols = fields.map { f -> KotlinGenUtil.resolveColumnName(entity.metaFields.first { it.name == f }) } + "${declaredIndexName(node, dialect)}(${cols.joinToString(",")})" + } + if (names.isNotEmpty()) actual[table] = names + } + val expected = sortedMapOf>() + for ((table, list) in ObjectMapper().readTree(corpus.resolve(file).toFile()).fields()) { + expected[table] = list.map { ix -> "${ix["name"].asText()}(${ix["columns"].joinToString(",") { it.asText() }})" } + } + assertEquals(expected, actual) + } + + @Test fun `postgres - declared index names are meta migrate's`() = checkDeclared("declared.postgres.json", "postgres") + + @Test fun `sqlite - declared index names are meta migrate's`() = checkDeclared("declared.sqlite.json", "sqlite") + + @Test fun `the Exposed table declares the cut name on Postgres`() { + val loader = loadString("fk-index-conformance", Files.readString(corpus.resolve("input/meta.fk.json"))) + loader.root.setMigrateSettings(MigrateSettings(false, null, "postgres")) + val out = Files.createTempDirectory("kt-declared-index") + KotlinExposedTableGenerator().apply { setArgs(mapOf("outputDir" to out.toString())) }.execute(loader) + val source = Files.walk(out).use { s -> s.filter { it.toString().endsWith(".kt") }.map { Files.readString(it) }.toList() } + .joinToString("\n") + assertTrue("\"customer_order_line_item_allocations_by_region_region_code_look\"" in source, source) + assertTrue("\"allocation_external_reference_key_for_customer_order_line_items\"" in source, source) + } } diff --git a/server/typescript/packages/codegen-ts/src/foreign-key-indexes.ts b/server/typescript/packages/codegen-ts/src/foreign-key-indexes.ts index f9bf6e035..b344c8cb8 100644 --- a/server/typescript/packages/codegen-ts/src/foreign-key-indexes.ts +++ b/server/typescript/packages/codegen-ts/src/foreign-key-indexes.ts @@ -9,6 +9,7 @@ import type { MetaObject, MetaRoot } from "@metaobjectsdev/metadata"; import { buildExpectedSchemaWithProvenance, + storedIdentifier, type SchemaBuildSettings, } from "@metaobjectsdev/migrate-ts/expected"; import type { Dialect } from "./column-mapper.js"; @@ -59,3 +60,13 @@ export function foreignKeyIndexesOf( if (byFqn === undefined) byDialect.set(dialect, (byFqn = build(root, settings, dialect))); return byFqn.get(obj.resolutionKey()) ?? []; } + +/** + * The name the database stores for a declared index name, which is the name `meta migrate` + * creates and compares (`storedIdentifier`): on Postgres a name over 63 bytes is cut to its first + * 63 bytes on a character boundary; SQLite keeps the whole name. MySQL keeps it too: migrate writes + * no DDL there, and MySQL refuses an over-long name rather than cutting it. + */ +export function storedIndexName(name: string, dialect: Dialect): string { + return dialect === "mysql" ? name : storedIdentifier(name, dialect); +} diff --git a/server/typescript/packages/codegen-ts/src/templates/drizzle-schema.ts b/server/typescript/packages/codegen-ts/src/templates/drizzle-schema.ts index 8454b9a21..8d8d90116 100644 --- a/server/typescript/packages/codegen-ts/src/templates/drizzle-schema.ts +++ b/server/typescript/packages/codegen-ts/src/templates/drizzle-schema.ts @@ -26,7 +26,7 @@ import { dialectModule } from "../dialect-module.js"; import { namesRef, physicalNameExpr, sourceSchemaExpr, indexNameExpr, columnExpr, } from "../names.js"; -import { resolveTableSchema } from "@metaobjectsdev/metadata"; +import { resolveTableSchema, resolveIndexName } from "@metaobjectsdev/metadata"; import { renderEnumIntCustomType } from "./enum-int-codec.js"; import { renderDurationCustomType } from "./duration-wire-helper.js"; import { renderRelationsBlock } from "./relations-block.js"; @@ -35,7 +35,7 @@ import { collectTphSubtypeFields, tphConcreteSubtypes } from "./tph-discriminato import { tphStorageObject } from "../tph.js"; import { flattenedMembersOf } from "../flattened.js"; import { effectivePackage } from "../docs-paths.js"; -import { foreignKeyIndexesOf } from "../foreign-key-indexes.js"; +import { foreignKeyIndexesOf, storedIndexName } from "../foreign-key-indexes.js"; /** * Render the Drizzle table definition for one entity, including: @@ -384,7 +384,16 @@ export function renderDrizzleSchema( // `idx_
_` against an index the database actually held under // `identity.name` (fdb4118f1); and an empty `index.lookup` name — which the loader // accepts, unlike an identity — is refused here rather than emitting `index("")`. - const indexName = indexNameExpr(names, node); + // + // Except past Postgres's 63-byte identifier limit: migrate creates and compares such an + // index under the name Postgres stores (`storedIdentifier`, its first 63 bytes; SQLite + // keeps the whole name), so the mapping spells that cut name as a literal. The names + // artifact is dialect-neutral and keeps the declared name. + const declaredIndexName = resolveIndexName(node); + const storedName = storedIndexName(declaredIndexName, dialect); + const indexName = storedName === declaredIndexName + ? indexNameExpr(names, node) + : code`${JSON.stringify(storedName)}`; const head: Code = using !== undefined && using !== "btree" ? code`${indexSym}(${indexName}).using(${JSON.stringify(using)}, ${target})` diff --git a/server/typescript/packages/codegen-ts/test/declared-index-name-cut.test.ts b/server/typescript/packages/codegen-ts/test/declared-index-name-cut.test.ts new file mode 100644 index 000000000..7aa3a2eff --- /dev/null +++ b/server/typescript/packages/codegen-ts/test/declared-index-name-cut.test.ts @@ -0,0 +1,63 @@ +// A declared index name over Postgres's 63-byte identifier limit: `meta migrate` creates and +// compares it under the name Postgres stores (its first 63 bytes), so the generated Drizzle table +// must declare that cut name too, or the mapping and the database disagree on every such index. +// +// Held to the shared corpus `fixtures/foreign-key-index-conformance/`: `declared..json` +// is migrate's answer (written by migrate-ts's runner), and the C# EF context and the Kotlin +// Exposed table pass the same file. +import { describe, test, expect } from "bun:test"; +import { mkdtempSync, rmSync, readdirSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { MetaDataLoader } from "@metaobjectsdev/metadata"; +import { FileSource } from "@metaobjectsdev/metadata/core"; +import { runGen } from "../src/runner.js"; +import { entityFile } from "../src/generators/entity-file.js"; +import { defineConfig } from "../src/metaobjects-config.js"; + +const CORPUS = resolve(import.meta.dir, "../../../../../fixtures/foreign-key-index-conformance"); + +async function generated(dialect: "postgres" | "sqlite"): Promise { + const repo = mkdtempSync(join(tmpdir(), "declared-index-cut-")); + try { + const { root, errors } = await new MetaDataLoader().load([new FileSource(join(CORPUS, "input", "meta.fk.json"))]); + expect(errors).toEqual([]); + const outDir = join(repo, "out"); + await runGen({ + config: defineConfig({ outDir, dialect, extStyle: "js", generators: [entityFile()], dbImport: "@/db" }), + metadata: root, + projectRoot: repo, + }); + return readdirSync(outDir, { recursive: true, encoding: "utf8" }) + .filter((f) => typeof f === "string" && f.endsWith(".ts")) + .map((f) => readFileSync(join(outDir, f), "utf8")) + .join("\n"); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +} + +// The formatter may break a call over lines with a trailing comma: `index(\n "name",\n)`. +function indexNames(source: string): Set { + return new Set([...source.matchAll(/\b(?:uniqueIndex|index)\(\s*"([^"]+)",?\s*\)/g)].map((m) => m[1]!)); +} + +describe("the Drizzle table declares a declared index under the name migrate creates", () => { + for (const dialect of ["postgres", "sqlite"] as const) { + test(`${dialect}: every name in declared.${dialect}.json`, async () => { + const source = await generated(dialect); + const expected = JSON.parse(readFileSync(join(CORPUS, `declared.${dialect}.json`), "utf8")) as + Record; + const declared = indexNames(source); + for (const ix of Object.values(expected).flat()) expect(declared).toContain(ix.name); + }); + } + + test("postgres cuts the long name to 63 bytes; sqlite keeps it whole", async () => { + const long = "customer_order_line_item_allocations_by_region_region_code_lookup"; + const postgres = indexNames(await generated("postgres")); + expect(postgres).toContain(long.slice(0, 63)); + expect(postgres).not.toContain(long); + expect(indexNames(await generated("sqlite"))).toContain(long); + }); +}); diff --git a/server/typescript/packages/migrate-ts/src/expected.ts b/server/typescript/packages/migrate-ts/src/expected.ts index 7c446ae33..6afe6d055 100644 --- a/server/typescript/packages/migrate-ts/src/expected.ts +++ b/server/typescript/packages/migrate-ts/src/expected.ts @@ -10,6 +10,7 @@ export { buildExpectedSchema, buildExpectedSchemaWithProvenance, fieldSqlType, f export { pgType } from "./emit/postgres.js"; export type { BuildExpectedSchemaOptions, SchemaBuildSettings, ExpectedSchemaWithProvenance, SchemaProvenance } from "./expected-schema.js"; export { qualifiedDbName } from "./qualified-name.js"; +export { storedIdentifier } from "./types.js"; export type { SqlType } from "./sql-type.js"; export type { CheckDescriptor, diff --git a/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts b/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts index 1ca7cc89c..7b991dcb3 100644 --- a/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts +++ b/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts @@ -3,6 +3,11 @@ // context generator and the Kotlin Exposed table generator write the same indexes into their // models and pass the same expected files, so one rule decides "uncovered" in every port. // +// `declared..json` pins the other half of the agreement: the database name of every +// `identity.secondary` and `index.lookup`, which a declared name over 63 bytes cuts on Postgres +// exactly as a foreign-key index name is cut. The Drizzle, EF Core and Exposed mappings declare +// those names, so the C# and Kotlin runners read this file too. +// // Regenerate the expected files (after a deliberate change, reviewed): // META_WRITE_FK_INDEXES=1 bun test test/foreign-key-index-conformance.test.ts @@ -11,14 +16,14 @@ import { readFileSync, writeFileSync, readdirSync } from "node:fs"; import { join, resolve } from "node:path"; import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; import { buildExpectedSchemaWithProvenance } from "../src/expected-schema.js"; -import { PG_IDENTIFIER_MAX_BYTES, type IndexDescriptor } from "../src/types.js"; +import { PG_IDENTIFIER_MAX_BYTES, storedIdentifier, type IndexDescriptor } from "../src/types.js"; import { qualifiedDbName } from "../src/qualified-name.js"; const CORPUS = resolve(import.meta.dir, "../../../../../fixtures/foreign-key-index-conformance"); const CASES = [ - { file: "expected.postgres.json", dialect: "postgres" }, - { file: "expected.sqlite.json", dialect: "sqlite" }, + { file: "expected.postgres.json", declared: "declared.postgres.json", dialect: "postgres" }, + { file: "expected.sqlite.json", declared: "declared.sqlite.json", dialect: "sqlite" }, ] as const; async function load() { @@ -49,6 +54,32 @@ describe("foreign-key-index-conformance — the indexes migrate.foreignKeyIndexe }); } + for (const c of CASES) { + test(c.declared, async () => { + const root = await load(); + const built = buildExpectedSchemaWithProvenance(root, { dialect: c.dialect, foreignKeyIndexes: true }); + const added = built.foreignKeyIndexes ?? new Map(); + // Every index but the foreign-key ones and a `@unique` field's `
__unique`: + // the `identity.secondary` and `index.lookup` nodes, under the name migrate gives them. + const actual = Object.fromEntries( + built.snapshot.tables + .map((t) => { + const fk = new Set((added.get(qualifiedDbName(t)) ?? []).map((ix) => ix.name)); + const declared = t.indexes.filter((ix) => + !fk.has(ix.name) && !(ix.unique && ix.columns.length === 1 && + ix.name === storedIdentifier(`${t.name}_${ix.columns[0]}_unique`, c.dialect))); + return [t.name, declared] as const; + }) + .filter(([, ixs]) => ixs.length > 0) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([table, ixs]) => [table, ixs.map((ix) => ({ name: ix.name, columns: [...ix.columns] }))]), + ); + const path = join(CORPUS, c.declared); + if (process.env["META_WRITE_FK_INDEXES"] === "1") writeFileSync(path, JSON.stringify(actual, null, 2) + "\n"); + expect(actual).toEqual(JSON.parse(readFileSync(path, "utf8"))); + }); + } + test("postgres: a name over 63 bytes is cut to the 63 bytes Postgres stores", async () => { const built = buildExpectedSchemaWithProvenance(await load(), { dialect: "postgres", foreignKeyIndexes: true }); const long = built.snapshot.tables.find((t) => t.name === "customer_order_line_item_allocations_by_region")!; diff --git a/server/typescript/packages/test-generators/src/templates/drizzle-schema.ts b/server/typescript/packages/test-generators/src/templates/drizzle-schema.ts index e35ed5146..2a9c7ea95 100644 --- a/server/typescript/packages/test-generators/src/templates/drizzle-schema.ts +++ b/server/typescript/packages/test-generators/src/templates/drizzle-schema.ts @@ -26,7 +26,7 @@ import { dialectModule } from "@metaobjectsdev/codegen-ts/engine"; import { namesRef, physicalNameExpr, sourceSchemaExpr, indexNameExpr, columnExpr, } from "@metaobjectsdev/codegen-ts/engine"; -import { resolveTableSchema } from "@metaobjectsdev/metadata"; +import { resolveTableSchema, resolveIndexName } from "@metaobjectsdev/metadata"; import { renderEnumIntCustomType } from "./enum-int-codec.js"; import { renderDurationCustomType } from "./duration-wire-helper.js"; import { renderRelationsBlock } from "./relations-block.js"; @@ -35,7 +35,7 @@ import { collectTphSubtypeFields, tphConcreteSubtypes } from "./tph-discriminato import { tphStorageObject } from "@metaobjectsdev/codegen-ts/engine"; import { flattenedMembersOf } from "@metaobjectsdev/codegen-ts/engine"; import { effectivePackage } from "@metaobjectsdev/codegen-ts/engine"; -import { foreignKeyIndexesOf } from "@metaobjectsdev/codegen-ts/engine"; +import { foreignKeyIndexesOf, storedIndexName } from "@metaobjectsdev/codegen-ts/engine"; /** * Render the Drizzle table definition for one entity, including: @@ -384,7 +384,16 @@ export function renderDrizzleSchema( // `idx_
_` against an index the database actually held under // `identity.name` (fdb4118f1); and an empty `index.lookup` name — which the loader // accepts, unlike an identity — is refused here rather than emitting `index("")`. - const indexName = indexNameExpr(names, node); + // + // Except past Postgres's 63-byte identifier limit: migrate creates and compares such an + // index under the name Postgres stores (`storedIdentifier`, its first 63 bytes; SQLite + // keeps the whole name), so the mapping spells that cut name as a literal. The names + // artifact is dialect-neutral and keeps the declared name. + const declaredIndexName = resolveIndexName(node); + const storedName = storedIndexName(declaredIndexName, dialect); + const indexName = storedName === declaredIndexName + ? indexNameExpr(names, node) + : code`${JSON.stringify(storedName)}`; const head: Code = using !== undefined && using !== "btree" ? code`${indexSym}(${indexName}).using(${JSON.stringify(using)}, ${target})` From 925b5089f8f80672af18f6d49e82a7893f0d0eb0 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 14 Nov 2026 09:41:08 -0500 Subject: [PATCH 8/8] no-mistakes(document): docs: stored index names and verify-goal refusals --- docs/ports/csharp.md | 6 ++++-- docs/ports/java.md | 10 +++++++--- docs/ports/python.md | 2 +- docs/ports/typescript.md | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/ports/csharp.md b/docs/ports/csharp.md index 07bd1af32..6d65b1ea9 100644 --- a/docs/ports/csharp.md +++ b/docs/ports/csharp.md @@ -628,8 +628,10 @@ one. > `Names.Name` before regenerating. `ReadOnly` is not relocated but REMOVED: it was never > metadata, only a derivation over `@kind` — ask `SourcePrimaryKind` instead. -An identity or index carries an `Index` member — its DATABASE name — only where one -exists: `identity.secondary` and `index.lookup`. An `identity.primary` gets none, because +An identity or index carries an `Index` member — the declared index name; on Postgres +`meta migrate` creates a name over 63 bytes as its first 63 bytes, and the EF context +declares that cut name ([stored names](../features/migrations-and-drift.md#schema-conventions-names-for-objects-no-code-addresses)) — +only where one exists: `identity.secondary` and `index.lookup`. An `identity.primary` gets none, because no port's codegen names a primary key (Postgres migrations hardcode `
_pkey`, SQLite emits an unnamed PK), and carrying that would restate a migrate-only formula in the artifact built to stop exactly that. diff --git a/docs/ports/java.md b/docs/ports/java.md index fdc6940fe..90464aada 100644 --- a/docs/ports/java.md +++ b/docs/ports/java.md @@ -472,7 +472,9 @@ Every per-object file lands in the directory of its metadata package: `acme::blo Java package `acme.blog` and the file `acme/blog/Author.java`, so two objects named `Note` in two packages are two files. Two different metadata packages that would land in one directory (names that differ only in letter case, or `a.b` beside `a::b`) are refused before any generator runs, -with `ERR_NAME_COLLISION` naming both objects. Nothing is renamed to avoid a clash. +with `ERR_NAME_COLLISION` naming both objects. Nothing is renamed to avoid a clash. So is a REST +collection claimed twice (`ERR_COLLECTION_NAME_COLLISION`), and `mvn metaobjects:verify` runs both +refusals before it compares, so a model `metaobjects:generate` refuses cannot verify clean. A controller's REST collection segment is the object's `@collectionName` when it declares one (snake_cased: `CaseHistory` is `/case_history`), else the pluralizer `naming.collections` names in @@ -686,8 +688,10 @@ names — has a member for each instead of one member between them. deliberate: a field's subType does not change what its column denotes, while an object's decides table-vs-view and an identity's decides unique-vs-not (ADR-0040 put uniqueness in the type). An `identity.secondary` or `index.lookup` -also carries `IDENTITY__INDEX` / `INDEX__INDEX`, the database index -name; `identity.primary` deliberately carries none, because migrate names a +also carries `IDENTITY__INDEX` / `INDEX__INDEX`, the index name — +the declared name; on Postgres `meta migrate` creates a name over 63 bytes as +its first 63 bytes ([stored names](../features/migrations-and-drift.md#schema-conventions-names-for-objects-no-code-addresses)). +`identity.primary` deliberately carries none, because migrate names a primary key by a dialect-conditional formula this artifact must not restate. There is no `READ_ONLY`. It was never metadata — it is a derivation over diff --git a/docs/ports/python.md b/docs/ports/python.md index ce08e98f5..82dee379b 100644 --- a/docs/ports/python.md +++ b/docs/ports/python.md @@ -498,7 +498,7 @@ SUBSCRIBER_COLUMNS_BY_FIELD: Final[dict[str, str]] = { | `__FIELD` | the **logical** name — the field as your code and the wire call it | | `__COLUMN` | the **physical** column — `@column` if declared, else the name through the project's `naming.columns` rule | | `_IDENTITY__*` / `_INDEX__*` | one `identity.*` / `index.*` child, keyed by its metamodel name | -| `…_INDEX` | the database index name — on `identity.secondary` and `index.lookup` only | +| `…_INDEX` | the index name — on `identity.secondary` and `index.lookup` only. It holds the declared name; on Postgres `meta migrate` creates a name over 63 bytes as its first 63 bytes ([stored names](../features/migrations-and-drift.md#schema-conventions-names-for-objects-no-code-addresses)) | **Why the physical name is not just `_NAME`.** One member cannot hold a table, a view and a stored procedure and still mean something. In a single run diff --git a/docs/ports/typescript.md b/docs/ports/typescript.md index 7841706cd..98c0ec3b6 100644 --- a/docs/ports/typescript.md +++ b/docs/ports/typescript.md @@ -583,7 +583,7 @@ export const ProgramNames = { | `fields..name` | the **logical** name — the field as your code and the wire call it | | `fields..column` | the **physical** column — `@column` if declared, else `name` through the project's `naming.columns` rule (`.metaobjects/config.json`) | | `identities.` / `indexes.` | one `identity.*` / `index.*` child, keyed by its metamodel name | -| `…​.index` | the database index name — on `identity.secondary` and `index.lookup` only | +| `…​.index` | the index name — on `identity.secondary` and `index.lookup` only. It holds the declared name; on Postgres `meta migrate` creates a name over 63 bytes as its first 63 bytes, so the Drizzle table spells the cut form while this member keeps the declared name ([stored names](../features/migrations-and-drift.md#schema-conventions-names-for-objects-no-code-addresses)) | **Why the physical name is not just `name`.** One key cannot hold a table, a view and a stored procedure and still mean something. In a single run it used to: