diff --git a/AGENTS.md b/AGENTS.md index 81493b92e..7cbf2847c 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 69eb82938..0714a58b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,29 +79,20 @@ 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. + 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. - **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 @@ -174,43 +165,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` @@ -268,6 +253,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. - **`@metaobjectsdev/cli` no longer installs `dprint-node`.** The TypeScript generators printed code with `ts-poet`, which loads `dprint-node` on import, and dprint-node ships all seven of its platform binaries in one package (24 MB, of which a machine runs one). `codegen-ts` now @@ -279,8 +272,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`, @@ -313,6 +308,24 @@ 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. 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. 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 + 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 d838d6674..7cfecafe7 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/) | 139 (34 core + 14 tph + 9 m2m + 6 jsonb + 2 write-through + 2 secret-fields + 2 enum-default + 12 projection + 37 report + 11 decimal + 2 duration + 8 api-settings) | ✓ (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; 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) | — | — | — | — | @@ -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` @@ -621,7 +625,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`) @@ -731,7 +741,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 139 + source-resolution 25 + scope 10 + ++ render 18 + persistence 67 + api-contract 139 + 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/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/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/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 85ab3b758..7ede1738b 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -521,6 +521,18 @@ 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. 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 by name: @@ -570,6 +582,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 +819,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 +932,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/docs/ports/csharp.md b/docs/ports/csharp.md index c4e3e83b2..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. @@ -1082,14 +1084,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..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 @@ -599,20 +601,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 @@ -694,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/kotlin.md b/docs/ports/kotlin.md index 5952df51c..23d4af8b6 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 @@ -763,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/docs/ports/python.md b/docs/ports/python.md index b36d186c0..82dee379b 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 @@ -503,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 db8a7c25b..98c0ec3b6 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 @@ -586,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: 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/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/fixtures/foreign-key-index-conformance/README.md b/fixtures/foreign-key-index-conformance/README.md new file mode 100644 index 000000000..1c584fc46 --- /dev/null +++ b/fixtures/foreign-key-index-conformance/README.md @@ -0,0 +1,47 @@ +# 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, 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 + +- 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. + `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, 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` 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 +in any port and are not in the corpus. 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/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..4e0b086fe --- /dev/null +++ b/fixtures/foreign-key-index-conformance/input/meta.fk.json @@ -0,0 +1,75 @@ +{ + "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" } }, + { "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/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/scripts/ci-local.sh b/scripts/ci-local.sh index c8b84efb1..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, @@ -489,7 +493,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/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"; 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/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 new file mode 100644 index 000000000..e6ac5fe19 --- /dev/null +++ b/server/csharp/MetaObjects.Codegen.Tests/ForeignKeyIndexConformanceTests.cs @@ -0,0 +1,121 @@ +// 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. `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; +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]); + } + + 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 6f0556357..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. @@ -1372,94 +1377,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..3a1ce90ca --- /dev/null +++ b/server/csharp/MetaObjects.Codegen/Generators/ForeignKeyIndexes.cs @@ -0,0 +1,145 @@ +// `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, cut by : the + /// name meta migrate expects. + /// + 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) + { + 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()) + { + var n = rune.Utf8SequenceLength; + if (bytes + n > PgIdentifierMaxBytes) break; + sb.Append(rune.ToString()); + bytes += n; + } + 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) + { + 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..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,16 +1214,16 @@ 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,11 +1388,15 @@ 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 { + 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" @@ -2780,3 +2786,46 @@ 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, cut by + * [storedIdentifier]. + */ +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 + 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..29f4143c9 --- /dev/null +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinForeignKeyIndexConformanceTest.kt @@ -0,0 +1,91 @@ +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 +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. `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 { + + 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") + + 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/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 4973119c3..9ebaffeb3 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 @@ -464,6 +464,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..b9e21f5b2 --- /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-upper.json", "mojo/layout-collision/meta.sales-lower.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-lower.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-lower.json new file mode 100644 index 000000000..fdfc77eb2 --- /dev/null +++ b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-lower.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/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-upper.json b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-upper.json new file mode 100644 index 000000000..d950c37a7 --- /dev/null +++ b/server/java/maven-plugin/src/test/resources/mojo/layout-collision/meta.sales-upper.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/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/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..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 } 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, }; @@ -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 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/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/src/types.ts b/server/typescript/packages/migrate-ts/src/types.ts index 42da43abd..e77928709 100644 --- a/server/typescript/packages/migrate-ts/src/types.ts +++ b/server/typescript/packages/migrate-ts/src/types.ts @@ -718,3 +718,22 @@ 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; +} + +/** 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/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..7b991dcb3 --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/foreign-key-index-conformance.test.ts @@ -0,0 +1,90 @@ +// 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. +// +// `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 + +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, 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", declared: "declared.postgres.json", dialect: "postgres" }, + { file: "expected.sqlite.json", declared: "declared.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"))); + }); + } + + 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")!; + 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/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()); + }); +}); 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})` 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). 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" ] }