diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index ec8a79580..6103ab5eb 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -142,7 +142,8 @@ jobs: cd ../integration-tests && bun test test/validation-conformance.test.ts # migrate-ts — the sole cross-port schema-migration engine (ADR-0015). # Its unit + integrity suites need no DB; the PG integration tests - # (gated on MIGRATE_TS_PG_URL) self-skip here and run in integration-tests.yml. + # (gated on MIGRATE_TS_PG_URL) self-skip here and run in local-ci's ts-slow + # lane (every push to main) + integration-tests.yml (the v* tag backstop). cd ../migrate-ts && bun test # Doc-template + CLI suites: byte-identity template gate, embedded-template # gate, neutrality / collision guards, and the docs golden corpus + diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 36f7f1983..f5010eaf9 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -116,8 +116,14 @@ jobs: # migrate-ts PG integration tests — the apply / lifecycle / rollback + # introspection suites that exercise REAL Postgres behavior (advisory locks, # multi-tenant ledger, down-migrations) that pg-mem cannot fake. They - # `describe.skip` unless MIGRATE_TS_PG_URL is set, so no workflow ran them - # until now. A `services: postgres` container supplies the URL. + # `describe.skip` unless MIGRATE_TS_PG_URL is set; a `services: postgres` + # container supplies the URL. + # + # The PRIMARY gate for these suites is local-ci.yml's ts-slow lane, on every push + # to main. This job is the cold-environment RELEASE BACKSTOP on the v* tag. Tags + # are pushed AFTER publish (docs/RELEASING.md), so red HERE means a broken release + # is already live on four immutable registries — treat it as an incident, never as + # noise. That inversion is exactly how this lane sat red for eight releases. migrate-ts-pg: runs-on: ubuntu-latest services: @@ -150,4 +156,7 @@ jobs: - name: Run migrate-ts suite against real Postgres env: MIGRATE_TS_PG_URL: postgres://migrate:migrate@localhost:5432/migrate_test + # Arms the in-suite sentinel: if the URL above ever stops being set, the + # suite FAILS instead of silently skipping and reporting a green release gate. + MIGRATE_TS_PG_EXPECT: '1' run: cd server/typescript/packages/migrate-ts && bun test diff --git a/.github/workflows/local-ci.yml b/.github/workflows/local-ci.yml index 5e612f2a3..c334489f2 100644 --- a/.github/workflows/local-ci.yml +++ b/.github/workflows/local-ci.yml @@ -140,6 +140,16 @@ jobs: persist-credentials: false - env: METAOBJECTS_TEST_PG_URL: postgres://metaobjects:metaobjects@localhost:${{ job.services.postgres.ports['5432'] }}/metaobjects_test + # Arms migrate-ts's real-Postgres suites (apply / lifecycle / rollback / + # introspection) on every push to main. They previously ran ONLY on the v* tag + # push — strictly AFTER the immutable four-registry publish — and so sat red for + # eight straight releases (v0.20.11 … v0.21.1) with nobody looking. Reuses this + # job's existing sidecar; the suite is proven to coexist in one database in a + # serial run (the hosted tag job runs it against a single migrate_test DB). + MIGRATE_TS_PG_URL: postgres://metaobjects:metaobjects@localhost:${{ job.services.postgres.ports['5432'] }}/metaobjects_test + # Makes the in-suite sentinel FAIL if the URL above ever rots away (renamed + # variable, dropped sidecar) rather than describe.skip-ing in silence. + MIGRATE_TS_PG_EXPECT: '1' run: scripts/ci-local.sh --only ts-slow --strict-toolchains # Java FAST lane — java + kotlin conformance only (the quick correctness signal). diff --git a/CHANGELOG.md b/CHANGELOG.md index 01d2e7d3c..30b7247af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,43 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixed — a POJO-bound temporal value in a jsonb column was written locale- and timezone-dependently (Java/Maven only) + +Closes the last carry-forward item from the #275 batch: the OMDB jsonb temporal path — the +motivating blast radius for that whole fix — had **no test at any level**, and adding one surfaced +a live defect on the adjacent branch. + +`MetaObjectGsonInitializer` registers the metadata-driven serializers against each `MetaObject`'s +declared `@object` class. A value object bound to a hand-written POJO through `ObjectClassRegistry` +is therefore serialized by Gson's **default reflection**, so its `java.util.Date` properties never +reached `TemporalWireFormat` and took Gson's built-in adapter instead. A `@storage: jsonb` column +holding such an object stored e.g. `"Jun 3, 2026, 10:30:00 AM"` — rendered in the JVM's **local +zone rather than UTC**, varying with the default **locale**, silently dropping **milliseconds** (so +even a Java-only round-trip did not return the original instant), and **unreadable by the other +four ports**, which expect the ISO form in `fixtures/persistence-conformance/normalization.md`. +Same defect class as #275, on the one path #275 did not reach. + +A new `TemporalGsonAdapter` is registered for `java.util.Date` on that builder: it writes +`TemporalWireFormat.formatInstant` (the canonical `…Z` instant) and reads tolerantly — +`TemporalWireFormat.parse` first, falling back to Gson's former localized default so rows already +written in the legacy format still load. The metadata-driven path is provably unaffected: +`MetaObjectSerializer`'s `DATE` branch formats and calls `addProperty` itself rather than +delegating to `context.serialize`, so that output is byte-identical. + +**Bounded narrowing, deliberate:** without an owning `MetaField` there is no way to know whether a +POJO property is a `field.date` (date-only) or a `@localTime` timestamp (no `Z`), so a POJO-bound +temporal is written as a full instant. That is lossless and portable where the previous behavior +was neither; a value object needing the exact per-field shape should stay on the metadata-driven +path, which consults its `MetaField`. + +Gated by three new end-to-end tests in `JsonbFieldDBTest` (metadata-driven single, array, and +POJO-bound), each asserting the **stored column text**, not just instant equality — Gson's default +format round-trips within Java while being unportable, so an equality-only assertion would have +passed against the bug. The `jsonbtest` fixture gained a `Moment` value object carrying all three +temporal shapes; before this it held only `string` and `int`, which is why no temporal value had +ever crossed this codec. Verified non-vacuous by mutation (dropping the `Z`, and dropping the +millisecond fraction, each fail the new tests). + ## [0.21.2] — npm `0.21.2` · PyPI `0.21.2` · NuGet `0.21.2` · Maven `7.21.2` Coordinated PATCH. **Changed product code: npm only** (`codegen-ts`, plus a comment-only note in diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 2d0b0f66d..ef347b248 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -198,6 +198,17 @@ A red run here has caught real cross-port divergence (view-DDL identifier quotin strategy mismatches) that the unit suites missed. ### 3. Promote to `latest` + +**Before `bun publish`: confirm the `local-ci` run for the release commit is green.** +Its `ts-slow` lane now carries the real-Postgres migrate gate. Publishing is irreversible on +all four registries, and the `v*` tag is pushed *after* `bun publish` — so the tag-triggered +`integration-tests` run can never be the pre-publish gate. This is the last gate that can +precede the irreversible step. + +```bash +gh run list --workflow local-ci.yml --limit 1 --json headSha,conclusion +``` + ```bash # bump the candidate set to the final rm bun.lock && bun install diff --git a/scripts/ci-local.sh b/scripts/ci-local.sh index a3ded168a..f28180f54 100755 --- a/scripts/ci-local.sh +++ b/scripts/ci-local.sh @@ -146,6 +146,14 @@ gate_ts_build_typecheck() { bun_install && bun run --filter '*' build && bun run # integration tests, but the typecheck already runs in ts-fast, so don't repeat it. gate_ts_build() { bun_install && bun run --filter '*' build; } +# migrate-ts real-Postgres suites — apply / lifecycle / rollback / introspection against a +# real engine. ADR-0015 makes migrate-ts the project's ONLY migrate engine, so this is the +# ONLY real-engine gate on migration correctness. The CI ts-slow job supplies +# MIGRATE_TS_PG_URL (its Postgres sidecar) plus MIGRATE_TS_PG_EXPECT=1, which arms an +# in-suite sentinel that fails loudly if that URL ever stops being set. Without those env +# vars (a local run with no Postgres) the PG describes self-skip exactly as before. +gate_migrate_ts_pg() { bun_install && ( cd server/typescript/packages/migrate-ts && bun test ); } + # ── conformance.yml — per-port conformance corpora (exact CI commands) ──────── gate_conf_ts() { bun_install || return 1 @@ -345,6 +353,9 @@ else # runs (umbrella `ts` / local full), its build already produced it — only build # here when ts-slow runs in isolation (the CI ts-slow job). if want_any ts ts-slow && ! want_any ts ts-fast; then step_if bun "ts build (for integration)" gate_ts_build; fi + # Ordered BEFORE the docker integration step so a container-readiness flake there + # can never prevent the migrate verdict from being produced. + want_any ts ts-slow && step_if bun "migrate-ts real-PG suite" gate_migrate_ts_pg want_any ts ts-slow && run_integration_for ts ts want_any java java-slow && run_integration_for java java kotlin want python && run_integration_for python python diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/json/TemporalWireFormat.java b/server/java/metadata/src/main/java/com/metaobjects/io/json/TemporalWireFormat.java index ab9732187..ab014dbc4 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/io/json/TemporalWireFormat.java +++ b/server/java/metadata/src/main/java/com/metaobjects/io/json/TemporalWireFormat.java @@ -76,6 +76,28 @@ public static String format(MetaField mf, Date d) { return localTime ? base : base + "Z"; } + /** + * Write-side without field context: the tz-aware instant form + * ({@code "YYYY-MM-DDTHH:MM:SS[.fff]Z"}), identical to what {@link #format} produces for a + * default {@code field.timestamp}. + * + *

Used where a {@link Date} is serialized with no owning {@link MetaField} to consult — + * notably a {@code java.util.Date} property on a registry-bound POJO, which Gson serializes + * by plain reflection and so never reaches {@link #format}. Absent field context the + * {@code field.date} (date-only) and {@code @localTime} (no {@code Z}) shapes are + * unknowable, so this always emits the fully-qualified instant — the one form that is + * lossless, locale- and timezone-independent, and parseable by every port (and by + * {@link #parse}). + * + * @param d the value to format, or {@code null} + * @return the wire string, or {@code null} if {@code d} is {@code null} + */ + public static String formatInstant(Date d) { + if (d == null) return null; + LocalDateTime wallClock = Instant.ofEpochMilli(d.getTime()).atZone(ZoneOffset.UTC).toLocalDateTime(); + return wallClock.format(TIMESTAMP_FMT) + fractionalSuffix(wallClock.getNano()) + "Z"; + } + /** * Read-side: tolerant parse, tried in order: {@link Instant#parse} (the {@code Z} form), * {@link LocalDateTime#parse} anchored at UTC (the no-{@code Z} form), then diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectGsonInitializer.java b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectGsonInitializer.java index f12d06b3a..5bb3e0718 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectGsonInitializer.java +++ b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectGsonInitializer.java @@ -31,6 +31,15 @@ public final static GsonBuilder addAdaptersToBuilder(MetaDataLoader loader, Gson private final static GsonBuilder addAdaptersToBuilder(MetaDataLoader loader, GsonBuilder builder, boolean addSerializer, boolean addDeserializer) { + // Canonical java.util.Date wire form for every REFLECTIVELY-serialized Date — chiefly a + // temporal property on a registry-bound POJO, which never reaches MetaObjectSerializer + // and so used to take Gson's locale-, timezone-dependent and millisecond-lossy default. + // The metadata-driven path is unaffected (MetaObjectSerializer's DATE branch formats and + // addProperty's directly rather than delegating to context.serialize). + TemporalGsonAdapter temporal = new TemporalGsonAdapter(); + if (addSerializer) builder.registerTypeAdapter(java.util.Date.class, (com.google.gson.JsonSerializer) temporal); + if (addDeserializer) builder.registerTypeAdapter(java.util.Date.class, (com.google.gson.JsonDeserializer) temporal); + List classList = new ArrayList<>(); Map nameClassMap = getMetaObjectToClassMap(loader); diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/TemporalGsonAdapter.java b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/TemporalGsonAdapter.java new file mode 100644 index 000000000..d09073d27 --- /dev/null +++ b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/TemporalGsonAdapter.java @@ -0,0 +1,78 @@ +package com.metaobjects.io.object.gson; + +import com.metaobjects.io.json.TemporalWireFormat; +import com.google.gson.*; + +import java.lang.reflect.Type; +import java.text.DateFormat; +import java.text.ParseException; +import java.util.Date; + +/** + * Canonical {@code java.util.Date} wire form for every Gson path that does NOT go through + * {@link MetaObjectSerializer} — i.e. any {@code Date} Gson reaches by plain reflection. + * + *

The gap this closes. {@link MetaObjectGsonInitializer} registers + * {@link MetaObjectSerializer}/{@link MetaObjectDeserializer} against each {@link + * com.metaobjects.object.MetaObject}'s declared {@code @object} class. A value object bound to a + * hand-written POJO through {@link com.metaobjects.registry.ObjectClassRegistry} is therefore + * serialized by Gson's DEFAULT reflection instead, and its {@code Date} properties took Gson's + * built-in adapter. That emitted a localized {@code DateFormat.DEFAULT} string — e.g. + * {@code "Jun 3, 2026, 10:30:00 AM"} — which is: + *

+ * The visible blast radius is an OMDB {@code @storage:jsonb} column holding a POJO-bound value + * object with a temporal field (see {@code GenericSQLDriver#serializeJsonb}). This is the same + * defect class as #275, on the one path #275 did not reach. + * + *

Write: always {@link TemporalWireFormat#formatInstant} (the {@code ...Z} instant). + * Field context is unavailable here, so the {@code field.date} and {@code @localTime} shapes + * cannot be reproduced — a POJO-bound temporal property is written as a full instant. That is a + * deliberate, documented narrowing: it is lossless and portable, where the previous behavior was + * neither. A value object that needs the exact per-field shape should stay on the + * metadata-driven path (no POJO binding), which consults its {@link + * com.metaobjects.field.MetaField} and calls {@link TemporalWireFormat#format}. + * + *

Read: {@link TemporalWireFormat#parse} first (all three canonical shapes), then a + * fallback through Gson's own former default ({@link DateFormat#getDateTimeInstance()}) so rows + * already written in the legacy localized format still load. Write canonical, read tolerant. + * + *

This adapter never sees a {@code Date} on the metadata-driven path: {@link + * MetaObjectSerializer}'s {@code DATE} branch formats and calls {@code addProperty} itself rather + * than delegating to {@code context.serialize}, so that output is unaffected. + */ +public final class TemporalGsonAdapter implements JsonSerializer, JsonDeserializer { + + @Override + public JsonElement serialize(Date src, Type type, JsonSerializationContext context) { + if (src == null) return JsonNull.INSTANCE; + return new JsonPrimitive(TemporalWireFormat.formatInstant(src)); + } + + @Override + public Date deserialize(JsonElement json, Type type, JsonDeserializationContext context) { + if (json == null || json.isJsonNull()) return null; + String s = json.getAsString(); + try { + return TemporalWireFormat.parse(s); + } catch (IllegalArgumentException canonicalMiss) { + // Legacy: a value written before this adapter existed, in Gson's default + // localized form. Best-effort so old rows still read; millisecond precision + // was already lost when it was written and cannot be recovered here. + try { + return DateFormat.getDateTimeInstance().parse(s); + } catch (ParseException legacyMiss) { + throw new JsonParseException( + "Cannot parse temporal value [" + s + "] as either the canonical wire form " + + "or the legacy localized form", canonicalMiss); + } + } + } +} diff --git a/server/java/omdb/src/test/java/com/metaobjects/manager/db/JsonbFieldDBTest.java b/server/java/omdb/src/test/java/com/metaobjects/manager/db/JsonbFieldDBTest.java index 9a3f1d1a8..bb3dca32d 100644 --- a/server/java/omdb/src/test/java/com/metaobjects/manager/db/JsonbFieldDBTest.java +++ b/server/java/omdb/src/test/java/com/metaobjects/manager/db/JsonbFieldDBTest.java @@ -95,6 +95,32 @@ public Label(String key, int weight) { public void setWeight(int weight) { this.weight = weight; } } + /** + * Public mutable POJO matching jsonbtest::Moment — the temporal value-object (#275). + * `at` is a default tz-aware field.timestamp, `atLocal` a field.timestamp @localTime, + * and `on` a field.date; together they cover all three TemporalWireFormat shapes. + */ + public static class Moment { + private java.util.Date at; + private java.util.Date atLocal; + private java.util.Date on; + + public Moment() {} + + public Moment(java.util.Date at, java.util.Date atLocal, java.util.Date on) { + this.at = at; + this.atLocal = atLocal; + this.on = on; + } + + public java.util.Date getAt() { return at; } + public void setAt(java.util.Date at) { this.at = at; } + public java.util.Date getAtLocal() { return atLocal; } + public void setAtLocal(java.util.Date atLocal) { this.atLocal = atLocal; } + public java.util.Date getOn() { return on; } + public void setOn(java.util.Date on) { this.on = on; } + } + // --------------------------------------------------------------------------- // Static test infrastructure // --------------------------------------------------------------------------- @@ -147,7 +173,9 @@ public static void setupDB() throws Exception { "CREATE TABLE JSONB_ITEM (\n" + " id BIGINT GENERATED ALWAYS AS IDENTITY CONSTRAINT JSONB_ITEM_id_PK PRIMARY KEY,\n" + " prefs VARCHAR(4000),\n" - + " labels VARCHAR(4000)\n" + + " labels VARCHAR(4000),\n" + + " moment VARCHAR(4000),\n" + + " moments VARCHAR(4000)\n" + ")"); } } @@ -367,4 +395,169 @@ public void testJsonbEmptyAndSingleArrayRoundTrip() throws Exception { assertEquals("solo", only.getKey()); assertEquals(99, only.getWeight()); } + + // --------------------------------------------------------------------------- + // Test: TEMPORAL value-object through the jsonb codec (#275) + // + // #275 replaced three divergent DATE branches (Gson serializer, Gson + // deserializer, streaming JsonObjectReader) with one TemporalWireFormat. The + // motivating blast-radius claim for that fix was the OMDB jsonb path — a + // field.timestamp / field.date living inside a @storage:jsonb value object — + // and it shipped with NO test at any level. Before this, jsonbtest::Prefs and + // ::Label carried only string + int, so no temporal value had ever crossed + // the jsonb codec here. + // + // The instant-equality assertion alone is too weak: Gson's own default Date + // format round-trips within Java while emitting a locale-dependent, + // millisecond-lossy string that no other port can read. So these pin the + // COLUMN TEXT as well — the cross-port wire form from + // fixtures/persistence-conformance/normalization.md, which is what makes the + // jsonb value readable by the TS/Python/C# ports. + // --------------------------------------------------------------------------- + + /** 2026-06-03T14:30:00.123Z — deliberately non-zero millis, to pin the fraction rule. */ + private static java.util.Date fixedInstant() { + return java.util.Date.from(java.time.Instant.parse("2026-06-03T14:30:00.123Z")); + } + + /** Read a column's raw stored text on the SAME connection the write used. */ + private String rawColumn(String column) throws Exception { + Connection c = (Connection) ((ObjectConnectionDB) oc).getDatastoreConnection(); + try (Statement s = c.createStatement(); + ResultSet rs = s.executeQuery("SELECT " + column + " FROM JSONB_ITEM")) { + assertTrue("expected a row to read " + column + " from", rs.next()); + return rs.getString(1); + } + } + + @Test + public void testJsonbTemporalWireFormIsCrossPortCanonical() throws Exception { + // The metadata-driven path (no POJO binding) — this is the one #275 fixed, + // where MetaObjectSerializer delegates to TemporalWireFormat. + ObjectClassRegistry.resetGlobal(); + + MetaObject itemMo = registry.findMetaObjectByName("jsonbtest::Item"); + MetaObject momentMo = registry.findMetaObjectByName("jsonbtest::Moment"); + + java.util.Date d = fixedInstant(); + ValueObject moment = (ValueObject) momentMo.newInstance(); + moment.setObject("at", d); + moment.setObject("atLocal", d); + moment.setObject("on", d); + + ValueObject item = (ValueObject) itemMo.newInstance(); + item.setObject("moment", moment); + omdb.createObject(oc, item); + + String json = rawColumn("moment"); + assertNotNull("moment column must hold JSON", json); + // field.timestamp (default, tz-aware) — UTC instant with a Z. + assertTrue("tz-aware timestamp must be the ISO instant form, got: " + json, + json.contains("\"2026-06-03T14:30:00.123Z\"")); + // field.timestamp @localTime — naive wall clock at UTC, no Z. + assertTrue("@localTime timestamp must be the naive form (no Z), got: " + json, + json.contains("\"2026-06-03T14:30:00.123\"")); + // field.date — calendar date of the instant at UTC. + assertTrue("field.date must be the date-only form, got: " + json, + json.contains("\"2026-06-03\"")); + // Gson's default Date format ("Jun 3, 2026, ...") is locale-dependent and + // drops millis — its presence would mean the wire form regressed to it. + assertFalse("must not fall back to Gson's default Date format, got: " + json, + json.contains("Jun 3, 2026")); + + // And the instant survives the full round-trip, to the millisecond. + Collection items = omdb.getObjects(oc, itemMo); + assertFalse("Expected the temporal row", items.isEmpty()); + ValueObject loadedMoment = + (ValueObject) ((ValueObject) items.iterator().next()).getObject("moment"); + assertNotNull("moment must round-trip", loadedMoment); + assertEquals("tz-aware timestamp instant must survive", + d, loadedMoment.getObject("at")); + assertEquals("@localTime timestamp instant must survive", + d, loadedMoment.getObject("atLocal")); + // field.date is date-only on the wire by contract, so it reads back + // anchored at midnight UTC — the documented TemporalWireFormat truncation. + assertEquals("field.date must read back at midnight UTC", + java.util.Date.from(java.time.Instant.parse("2026-06-03T00:00:00Z")), + loadedMoment.getObject("on")); + } + + @Test + public void testJsonbTemporalRoundTripThroughABoundPojo() throws Exception { + // The POJO-binding branch of the jsonb codec: with a jsonbtest::Moment -> + // Moment.class binding registered, serializeJsonb/deserializeJsonb use Gson's + // plain REFLECTION, so MetaObjectSerializer (and TemporalWireFormat with it) is + // never consulted. Until TemporalGsonAdapter this wrote Gson's default localized + // Date — "Jun 3, 2026, 10:30:00 AM": rendered in the JVM's local zone rather than + // UTC, varying with the default locale, silently dropping the .123 millis, and + // unreadable by the other four ports. + ObjectClassRegistry reg = new ObjectClassRegistry(); + reg.register(new ObjectClassBindingProvider() { + @Override + public Map> bindings() { + Map> m = new LinkedHashMap<>(); + m.put("jsonbtest::Moment", Moment.class); + return m; + } + }); + ObjectClassRegistry.setGlobal(reg); + + MetaObject itemMo = registry.findMetaObjectByName("jsonbtest::Item"); + java.util.Date d = fixedInstant(); + + ValueObject item = (ValueObject) itemMo.newInstance(); + item.setObject("moment", new Moment(d, d, d)); + omdb.createObject(oc, item); + + String json = rawColumn("moment"); + assertNotNull("moment column must hold JSON", json); + assertFalse("must not emit Gson's locale/timezone-dependent default Date, got: " + json, + json.contains("Jun 3, 2026")); + assertTrue("a POJO-bound Date must serialize as the canonical UTC instant, got: " + json, + json.contains("\"2026-06-03T14:30:00.123Z\"")); + + Collection items = omdb.getObjects(oc, itemMo); + assertFalse("Expected the POJO-bound temporal row", items.isEmpty()); + Object loaded = ((ValueObject) items.iterator().next()).getObject("moment"); + assertTrue("must read back as the bound POJO, got: " + + (loaded == null ? "null" : loaded.getClass().getName()), loaded instanceof Moment); + // The millisecond that Gson's default format used to discard. + assertEquals("instant must survive to the millisecond", d, ((Moment) loaded).getAt()); + } + + @Test + public void testJsonbTemporalArrayRoundTrip() throws Exception { + // The array-of-VO codec is a separate branch from the single-VO one + // (jsonbTargetType -> List); a temporal element must survive it too. + ObjectClassRegistry.resetGlobal(); + + MetaObject itemMo = registry.findMetaObjectByName("jsonbtest::Item"); + MetaObject momentMo = registry.findMetaObjectByName("jsonbtest::Moment"); + + java.util.Date d = fixedInstant(); + ValueObject moment = (ValueObject) momentMo.newInstance(); + moment.setObject("at", d); + + List moments = new ArrayList<>(); + moments.add(moment); + + ValueObject item = (ValueObject) itemMo.newInstance(); + item.setObjectArray("moments", moments); + omdb.createObject(oc, item); + + String json = rawColumn("moments"); + assertNotNull("moments column must hold JSON", json); + assertTrue("array element timestamp must use the ISO instant form, got: " + json, + json.contains("\"2026-06-03T14:30:00.123Z\"")); + + Collection items = omdb.getObjects(oc, itemMo); + assertFalse("Expected the temporal-array row", items.isEmpty()); + Object loadedArr = ((ValueObject) items.iterator().next()).getObject("moments"); + assertTrue("moments must read back as a List, got: " + + (loadedArr == null ? "null" : loadedArr.getClass().getName()), loadedArr instanceof List); + List loaded = (List) loadedArr; + assertEquals("array size", 1, loaded.size()); + assertEquals("array element instant must survive", + d, ((ValueObject) loaded.get(0)).getObject("at")); + } } diff --git a/server/java/omdb/src/test/resources/meta.jsonb.json b/server/java/omdb/src/test/resources/meta.jsonb.json index 3af3e414b..3069684c3 100644 --- a/server/java/omdb/src/test/resources/meta.jsonb.json +++ b/server/java/omdb/src/test/resources/meta.jsonb.json @@ -35,6 +35,23 @@ "isArray": true } }, + { + "field.object": { + "name": "moment", + "@column": "moment", + "@storage": "jsonb", + "@objectRef": "jsonbtest::Moment" + } + }, + { + "field.object": { + "name": "moments", + "@column": "moments", + "@storage": "jsonb", + "@objectRef": "jsonbtest::Moment", + "isArray": true + } + }, { "identity.primary": { "name": "primary", @@ -65,6 +82,33 @@ ] } }, + { + "object.value": { + "name": "Moment", + "@object": "com.metaobjects.object.value.ValueObject", + "children": [ + { + "field.timestamp": { + "name": "at", + "@column": "at" + } + }, + { + "field.timestamp": { + "name": "atLocal", + "@column": "atLocal", + "@localTime": true + } + }, + { + "field.date": { + "name": "on", + "@column": "on" + } + } + ] + } + }, { "object.value": { "name": "Label", diff --git a/server/typescript/packages/codegen-ts/src/metaobjects-config.ts b/server/typescript/packages/codegen-ts/src/metaobjects-config.ts index 6d67e7365..81d61f95d 100644 --- a/server/typescript/packages/codegen-ts/src/metaobjects-config.ts +++ b/server/typescript/packages/codegen-ts/src/metaobjects-config.ts @@ -121,13 +121,13 @@ export interface MetaobjectsGenConfig extends Omit][gte]=...`) is not yet supported — - * `runtime-ts`'s filter parser keeps the qs value a string, which throws at - * request time against a Date-mode Drizzle column. Known limitation; not - * threaded through at runtime, but `runGen` DOES warn (once per run, naming - * every offending entity+field) when this mode meets a `@filterable` - * `field.timestamp` — see `runner.ts`'s Important-4 check. Track before - * recommending date mode for a filterable timestamp field. + * Date-mode filtering (`?filter[][gte]=...`) IS supported: a + * `@filterable` `field.timestamp` generated under this mode carries + * `dateValues: true` in its `FilterAllowlist` rule, and `runtime-ts`'s filter + * parser coerces the query-string value with `new Date(...)` rather than binding + * a string against a Date-typed column (a malformed value is rejected as + * `filter.invalid_value`). `field.date` / `field.time` are unaffected — Drizzle + * types both as strings under every dialect. */ timestampMode?: "date" | "string"; /** Path prefix applied to generated route registrations + hook fetch URLs. Defaults to "". */ diff --git a/server/typescript/packages/codegen-ts/src/reference/entity.ts b/server/typescript/packages/codegen-ts/src/reference/entity.ts index 2326aa085..b4fe96434 100644 --- a/server/typescript/packages/codegen-ts/src/reference/entity.ts +++ b/server/typescript/packages/codegen-ts/src/reference/entity.ts @@ -100,7 +100,7 @@ function renderEntity(entity: MetaObject, ctx: RenderContext, opts?: RenderEntit ...(enumAliases !== null ? [enumAliases] : []), renderZodValidators(entity, ctx), renderEntityConstants(entity, ctx.apiPrefix), - ...(allowlists ? [renderFilterAllowlist(entity), renderSortAllowlist(entity)] : []), + ...(allowlists ? [renderFilterAllowlist(entity, undefined, ctx), renderSortAllowlist(entity)] : []), renderFilterType(entity), ...(tphBlock !== null ? [tphBlock] : []), ]; diff --git a/server/typescript/packages/codegen-ts/src/runner.ts b/server/typescript/packages/codegen-ts/src/runner.ts index 9ff5c0b35..e7e970cbd 100644 --- a/server/typescript/packages/codegen-ts/src/runner.ts +++ b/server/typescript/packages/codegen-ts/src/runner.ts @@ -180,36 +180,11 @@ export async function runGen(opts: RunGenOpts): Promise { // 2. Resolve targets + entity-module target. const config = normalizeConfig(opts.config); - // Important-4 (post-#281 pre-publish review) — a @filterable timestamp field - // under timestampMode:"date" throws at REQUEST time in runtime-ts's filter - // parser (documented limitation; see drizzle-fastify/filter-parser.ts's - // "datetime" coerce case). Detect-and-WARN at generation time instead of - // leaving it a silent build-time no-signal (repo precedent: #226/#258 - // detect-and-refuse an un-appliable migration at gen time rather than fail - // at apply). Reading config.timestampMode AFTER normalizeConfig means this is - // naturally silent on sqlite/D1 (normalized to "string" there) and in the - // default "string" mode — no dialect/mode branching needed here. One warning - // for the whole run, naming every offending entity+field, not one per field. - if (config.timestampMode === "date") { - const offenders = safeEntities - .filter((e) => !e.isAbstract) - .map((e) => ({ - entity: e.name, - fields: e.fields() - .filter((f) => f.subType === FIELD_SUBTYPE_TIMESTAMP && f.attr(FIELD_ATTR_FILTERABLE) === true) - .map((f) => f.name), - })) - .filter((o) => o.fields.length > 0); - if (offenders.length > 0) { - const named = offenders.map((o) => `${o.entity}.${o.fields.join(",")}`).join("; "); - warnings.push( - `timestampMode: "date" — @filterable timestamp field(s) [${named}] will throw at ` + - `request time when filtered (e.g. ?filter[field][gte]=...) — runtime-ts's filter ` + - `parser does not yet thread the Date-mode column type through. Not enforced; ` + - `remove @filterable from these fields or avoid filtering them until this is fixed.`, - ); - } - } + // (Historical: a warning stood here for a @filterable timestamp under + // timestampMode:"date", which used to throw at REQUEST time in runtime-ts's + // filter parser. That limitation is fixed — the generated allowlist now carries + // `dateValues: true` for a Date-mode timestamp column and the parser coerces with + // `new Date(...)` — so the warning was removed rather than left to cry wolf.) const targets = config.targets; const targetOf = (g: Generator): ResolvedTarget => { diff --git a/server/typescript/packages/codegen-ts/src/templates/entity-file.ts b/server/typescript/packages/codegen-ts/src/templates/entity-file.ts index 5a34b8fdc..2da8911e5 100644 --- a/server/typescript/packages/codegen-ts/src/templates/entity-file.ts +++ b/server/typescript/packages/codegen-ts/src/templates/entity-file.ts @@ -165,7 +165,7 @@ ${docsPrefix}export type ${entity.name} = ${z}.infer([ FIELD_SUBTYPE_INT, @@ -52,8 +53,15 @@ function filterableFields(entity: MetaObject, exclude?: string): MetaField[] { * `exclude` (FR-017): drop a field from the allowlist. Used by per-subtype TPH * allowlists to omit the discriminator — it's pinned by the per-subtype route * path, so a client must not filter on it. + * + * `ctx` supplies `timestampMode`: under `"date"` a `field.timestamp` column binds a JS + * `Date`, so its rule carries `dateValues: true` and runtime-ts's filter parser coerces + * the query-string value with `new Date(...)` rather than binding a string against a + * Date-typed column (which threw at request time). `ctx.timestampMode` is already + * normalized to `"string"` for sqlite/D1 at both config choke points, so no dialect + * branching is needed here. Absent `ctx`, or in the default mode, output is unchanged. */ -export function renderFilterAllowlist(entity: MetaObject, exclude?: string): Code { +export function renderFilterAllowlist(entity: MetaObject, exclude?: string, ctx?: RenderContext): Code { const fields = filterableFields(entity, exclude); if (fields.length === 0) { return code` @@ -66,7 +74,12 @@ export const ${entity.name}FilterAllowlist = {} as const satisfies FilterAllowli .map((f) => { const ops = opsForSubType(f.subType).map((o) => JSON.stringify(o)).join(", "); const sub = filterSubTypeFor(f.subType); - return ` ${f.name}: { ops: [${ops}] as const, subType: ${JSON.stringify(sub)} as const, leadingWildcard: false }`; + // Only field.timestamp is governed by timestampMode — Drizzle types + // field.date / field.time as strings under every dialect. + const dateValues = ctx?.timestampMode === "date" && f.subType === FIELD_SUBTYPE_TIMESTAMP + ? ", dateValues: true as const" + : ""; + return ` ${f.name}: { ops: [${ops}] as const, subType: ${JSON.stringify(sub)} as const, leadingWildcard: false${dateValues} }`; }) .join(",\n"); return code` diff --git a/server/typescript/packages/codegen-ts/src/templates/projection-decl.ts b/server/typescript/packages/codegen-ts/src/templates/projection-decl.ts index 86c331264..37f6d7afe 100644 --- a/server/typescript/packages/codegen-ts/src/templates/projection-decl.ts +++ b/server/typescript/packages/codegen-ts/src/templates/projection-decl.ts @@ -176,7 +176,7 @@ ${constFieldLines.join("\n")} } as const; `, ...(allowlists - ? [renderFilterAllowlist(projection), renderSortAllowlist(projection)] + ? [renderFilterAllowlist(projection, undefined, ctx), renderSortAllowlist(projection)] : []), renderFilterType(projection), ]; diff --git a/server/typescript/packages/codegen-ts/src/templates/value-object-file.ts b/server/typescript/packages/codegen-ts/src/templates/value-object-file.ts index 1eb417a4e..c36b29cdf 100644 --- a/server/typescript/packages/codegen-ts/src/templates/value-object-file.ts +++ b/server/typescript/packages/codegen-ts/src/templates/value-object-file.ts @@ -40,7 +40,7 @@ export function renderValueObjectFile(obj: MetaObject, apiPrefix = "", ctx?: Ren // not filter on it). Included fields are the subtype's own + inherited base // filterable fields. Drives the per-subtype REST routes' filter layer. const discField = tphSubtype ? tphDiscriminatorPin(obj)?.fieldName : undefined; - const tphFilterAllowlist = tphSubtype ? renderFilterAllowlist(obj, discField) : null; + const tphFilterAllowlist = tphSubtype ? renderFilterAllowlist(obj, discField, ctx) : null; const tphSortAllowlist = tphSubtype ? renderSortAllowlist(obj, discField) : null; // FR-017 Tier 3: the per-subtype CLIENT filter type, discriminator-excluded — // kept in lockstep with the per-subtype allowlist above so a typed diff --git a/server/typescript/packages/codegen-ts/test/templates/filter-allowlist.test.ts b/server/typescript/packages/codegen-ts/test/templates/filter-allowlist.test.ts index dcf37f733..a93c177a6 100644 --- a/server/typescript/packages/codegen-ts/test/templates/filter-allowlist.test.ts +++ b/server/typescript/packages/codegen-ts/test/templates/filter-allowlist.test.ts @@ -3,6 +3,9 @@ import { renderFilterAllowlist, renderSortAllowlist } from "../../src/templates/ import { resolve } from "node:path"; import { MetaDataLoader } from "@metaobjectsdev/metadata"; import { FileSource } from "@metaobjectsdev/metadata/core"; +import { makeRenderContext } from "../../src/render-context.js"; +import { buildPkMap } from "../../src/pk-resolver.js"; +import { buildRelationMap } from "../../src/relation-resolver.js"; const FIXTURE = resolve(import.meta.dir, "..", "fixtures", "filter-fixture.json"); @@ -73,3 +76,54 @@ describe("renderSortAllowlist", () => { expect(out).toMatch(/price:\s*\{/); }); }); + +describe("renderFilterAllowlist — timestampMode date-mode marking", () => { + // A date-mode timestamp column binds a JS Date, so the rule must carry + // `dateValues: true` for runtime-ts's parser to coerce with `new Date(...)` + // instead of binding a string (which threw at request time). The behavioral + // half of this contract is pinned in runtime-ts's filter-parser-date-mode test. + async function allowlistFor(mode?: "date" | "string") { + const { root } = await new MetaDataLoader().load([new FileSource(FIXTURE)]); + const entity = root.objects().find((c) => c.name === "Subscriber")!; + const ctx = makeRenderContext({ + dialect: "postgres", + ...(mode !== undefined && { timestampMode: mode }), + loadedRoot: root, + outDir: "/x", + dbImport: "~/db", + pkMap: buildPkMap(root), + relationMap: buildRelationMap(root), + }); + return renderFilterAllowlist(entity, undefined, ctx).toString(); + } + + test('timestampMode:"date" marks the timestamp field and ONLY the timestamp field', async () => { + const out = await allowlistFor("date"); + expect(out).toMatch(/createdAt:\s*\{[^}]*dateValues: true/); + // Non-timestamp fields must not be marked — the flag is meaningless for them + // and would make the parser try to Date-coerce a string or boolean. + expect(out).not.toMatch(/email:\s*\{[^}]*dateValues/); + expect(out).not.toMatch(/subscribed:\s*\{[^}]*dateValues/); + }); + + test('the default "string" mode emits no dateValues at all', async () => { + expect(await allowlistFor()).not.toContain("dateValues"); + expect(await allowlistFor("string")).not.toContain("dateValues"); + }); + + test('dialect:"sqlite" never marks — timestampMode normalizes to "string" there', async () => { + const { root } = await new MetaDataLoader().load([new FileSource(FIXTURE)]); + const entity = root.objects().find((c) => c.name === "Subscriber")!; + const ctx = makeRenderContext({ + dialect: "sqlite", timestampMode: "date", loadedRoot: root, + outDir: "/x", dbImport: "~/db", + pkMap: buildPkMap(root), relationMap: buildRelationMap(root), + }); + expect(renderFilterAllowlist(entity, undefined, ctx).toString()).not.toContain("dateValues"); + }); + + test("no ctx (bare call) is unchanged", async () => { + const entity = await loadEntity("Subscriber"); + expect(renderFilterAllowlist(entity).toString()).not.toContain("dateValues"); + }); +}); diff --git a/server/typescript/packages/codegen-ts/test/timestamp-mode-execution.test.ts b/server/typescript/packages/codegen-ts/test/timestamp-mode-execution.test.ts index 1818163a7..6b04ec667 100644 --- a/server/typescript/packages/codegen-ts/test/timestamp-mode-execution.test.ts +++ b/server/typescript/packages/codegen-ts/test/timestamp-mode-execution.test.ts @@ -9,11 +9,14 @@ // dynamically `import()` it, then call the real `safeParse`/`parse` on the // real Zod object it exports — not a string match. // -// Covers the three Criticals + Important 5 found by that review, plus a -// codegen-time WARNING for Important 4 (added on top after the initial fix -// was reviewed clean — a cheap generation-time detect for the one runtime gap -// left documented-not-fixed: filtering a date-mode timestamp throws at -// request time; see runGen's warning + filter-parser.ts's limitation note): +// Covers the three Criticals + Important 5 found by that review. Important 4 +// (filtering a date-mode timestamp threw at request time) was originally shipped +// as a generation-time WARNING because the runtime fix was deferred; that fix has +// since landed — the generated allowlist carries `dateValues` and runtime-ts's +// parser coerces with `new Date(...)` — so the warning was removed and the last +// describe block below now pins its ABSENCE. The behavioral pins for the fix +// itself live in runtime-ts's filter-parser-date-mode test and in +// templates/filter-allowlist.test.ts. // CRITICAL 1 — z.date() rejects every JSON wire value; fix is z.coerce.date(). // CRITICAL 2 — sqlite/D1 + date mode used to emit non-compiling code (a // regression #281 itself introduced); fix normalizes the mode @@ -269,27 +272,19 @@ describe('IMPORTANT 4: runGen warns (once) when timestampMode: "date" meets a @f } } - test('warns exactly once, naming every offending entity+field, when dialect:"postgres" + timestampMode:"date"', async () => { - const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "postgres", "date"); - const hits = warnings.filter((w) => w.includes('timestampMode: "date"')); - expect(hits.length).toBe(1); // once per run, not once per field/entity - expect(hits[0]).toContain("Post.updatedAt"); - expect(hits[0]).toContain("Comment.postedAt"); - expect(hits[0]).not.toContain("archivedAt"); // non-filterable — not named - }); - - test('silent in the default "string" mode (timestampMode omitted)', async () => { - const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "postgres"); - expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]); - }); - - test('silent when no field is both @filterable and a timestamp, even in date mode', async () => { - const { warnings } = await runWithMetadata(NO_FILTERABLE_TIMESTAMP, "postgres", "date"); - expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]); - }); - - test('silent under dialect:"sqlite" — timestampMode normalizes to "string" (Critical 2), so Important 4 never fires', async () => { - const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "sqlite", "date"); - expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]); + test("no warning is emitted in any mode — the limitation it announced is fixed", async () => { + // The warning was correct while date-mode filtering threw at request time. Now + // that the allowlist carries `dateValues` and the parser coerces, it would be + // crying wolf, so it is gone. Pinned in all four combinations so a revert of + // the fix cannot quietly re-land the warning instead of the behavior. + for (const [json, dialect, mode] of [ + [TWO_FILTERABLE_TIMESTAMPS, "postgres", "date"], + [TWO_FILTERABLE_TIMESTAMPS, "postgres", undefined], + [NO_FILTERABLE_TIMESTAMP, "postgres", "date"], + [TWO_FILTERABLE_TIMESTAMPS, "sqlite", "date"], + ] as const) { + const { warnings } = await runWithMetadata(json, dialect, mode); + expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]); + } }); }); diff --git a/server/typescript/packages/codegen-ts/test/zod-validators-execution.test.ts b/server/typescript/packages/codegen-ts/test/zod-validators-execution.test.ts new file mode 100644 index 000000000..3e90c2d38 --- /dev/null +++ b/server/typescript/packages/codegen-ts/test/zod-validators-execution.test.ts @@ -0,0 +1,353 @@ +// `renderZodValidators` — EXECUTION pins for the field-subtype + validator matrix. +// +// WHY THIS FILE EXISTS. `test/templates/zod-validators.test.ts` covers this same +// emitter with ~390 lines of `toContain` assertions. Those prove the generator +// emitted the string the test author expected; they cannot prove the emitted +// schema COMPILES or BEHAVES. This repo has three named instances of exactly +// that gap: +// +// • 0.20.6 — `z.string().ip()` for `field.inet`. Removed in Zod 4, so every +// generated inet validator threw `.ip is not a function`. Text-green. +// • 0.21.2 — `timestampMode: "date"` emitted `Date`-typed columns against +// `string`-producing validators. Did not compile. Text-green. +// • 0.21.2 again — the FIX for the above repaired Postgres and introduced the +// same class of bug on SQLite. Text-green, because the goldens were +// regenerated to match the new (wrong) output. +// +// That third case is the important one: a text-asserting suite is WEAKEST +// exactly when a fix lands, because the expected strings get updated to match +// whatever the new emitter produces. So these tests never assert on source +// text. They render the real output, write it to a temp `.ts` file INSIDE this +// package (so bare `"zod"` resolves through the workspace's node_modules), +// dynamically `import()` it, and call the real `safeParse` on the real Zod +// object — asserting on ACCEPT/REJECT of concrete values. +// +// Scope: the semantic pins that a string match structurally cannot check — +// FR-036's full-match regex + non-empty-required + strictest-wins rules, the +// Zod-version-fragile format validators (`field.inet`, `field.uri`, +// `@stringFormat`), and the enum/numeric/array bound chains. +// +// Sibling: `timestamp-mode-execution.test.ts` (same technique, timestampMode). + +import { describe, test, expect, afterEach } from "bun:test"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { + TypeId, TYPE_IDENTITY, TYPE_VALIDATOR, + IDENTITY_SUBTYPE_PRIMARY, OBJECT_SUBTYPE_ENTITY, + FIELD_SUBTYPE_LONG, FIELD_SUBTYPE_INT, FIELD_SUBTYPE_STRING, + FIELD_SUBTYPE_ENUM, FIELD_SUBTYPE_URI, FIELD_SUBTYPE_INET, + FIELD_ATTR_REQUIRED, FIELD_ATTR_MAX_LENGTH, FIELD_ATTR_VALUES, + FIELD_ATTR_LENIENT, FIELD_ATTR_STRING_FORMAT, + STRING_FORMAT_EMAIL, STRING_FORMAT_HOSTNAME, + VALIDATOR_SUBTYPE_LENGTH, VALIDATOR_SUBTYPE_REGEX, + VALIDATOR_SUBTYPE_NUMERIC, VALIDATOR_SUBTYPE_ARRAY, + VALIDATOR_ATTR_MIN, VALIDATOR_ATTR_MAX, VALIDATOR_ATTR_PATTERN, +} from "@metaobjectsdev/metadata"; +import type { AttrValue, MetaField, MetaObject } from "@metaobjectsdev/metadata"; +import { meta, metaObject, metaField } from "./_meta-build.js"; +import { renderZodValidators } from "../src/templates/zod-validators.js"; + +// biome-ignore lint/suspicious/noExplicitAny: dynamically imported generated module — no static shape +type GeneratedModule = Record; + +const tmpDirs: string[] = []; +afterEach(() => { + while (tmpDirs.length > 0) { + rmSync(tmpDirs.pop()!, { recursive: true, force: true }); + } +}); + +/** + * Write a rendered file (imports included — ts-poet's `Code.toString()` hoists + * them) to a temp `.ts` INSIDE this package so bare specifiers resolve through + * the workspace node_modules, then dynamically import it. Real execution of the + * generated module, not a text match. + */ +async function executeGenerated(source: string): Promise { + const dir = mkdtempSync(join(import.meta.dir, "tmp-zod-exec-")); + tmpDirs.push(dir); + const file = join(dir, "schema.ts"); + writeFileSync(file, source); + return import(pathToFileURL(file).href); +} + +/** An entity with an auto-increment PK (excluded from Insert/Update) plus `fields`. */ +function entityWith(name: string, ...fields: MetaField[]): MetaObject { + const obj = metaObject(OBJECT_SUBTYPE_ENTITY, name); + obj.addChild(metaField(FIELD_SUBTYPE_LONG, "id")); + for (const f of fields) obj.addChild(f); + const primary = meta(new TypeId(TYPE_IDENTITY, IDENTITY_SUBTYPE_PRIMARY), "primary"); + primary.setAttr("fields", ["id"]); + primary.setAttr("generation", "increment"); + obj.addChild(primary); + return obj; +} + +function validator(subType: string, name: string, attrs: Record) { + const v = meta(new TypeId(TYPE_VALIDATOR, subType), name); + for (const [k, val] of Object.entries(attrs)) v.setAttr(k, val); + return v; +} + +/** Render `entity` and return its executed `InsertSchema`. */ +async function insertSchemaOf(entity: MetaObject): Promise { + const mod = await executeGenerated(renderZodValidators(entity).toString()); + return mod[`${entity.name}InsertSchema`]; +} + +const accepts = (schema: GeneratedModule[string], value: unknown) => + schema.safeParse(value).success; + +describe("FR-036 Pin 2 — validator.regex @pattern is FULL-MATCH, not a search", () => { + // The emitter wraps an authored pattern as `^(?:…)$`. The non-capturing group + // is load-bearing and a text assertion cannot show why: JS alternation binds + // loosely, so the "obvious" `^cat|dog$` parses as `(^cat)|(dog$)` and happily + // accepts "cathouse" and "hotdog". Only executing the emitted RegExp against + // those values proves the grouping is right. + test("an alternation pattern rejects values that merely CONTAIN a match", async () => { + const kind = metaField(FIELD_SUBTYPE_STRING, "kind"); + kind.setAttr(FIELD_ATTR_REQUIRED, true); + kind.addChild(validator(VALIDATOR_SUBTYPE_REGEX, "kindFormat", { + [VALIDATOR_ATTR_PATTERN]: "cat|dog", + })); + const schema = await insertSchemaOf(entityWith("Pet", kind)); + + expect(accepts(schema, { kind: "cat" })).toBe(true); + expect(accepts(schema, { kind: "dog" })).toBe(true); + // Would ACCEPT under a naive `^cat|dog$` anchoring — the actual regression. + expect(accepts(schema, { kind: "cathouse" })).toBe(false); + expect(accepts(schema, { kind: "hotdog" })).toBe(false); + expect(accepts(schema, { kind: "xcatx" })).toBe(false); + }); + + test("an already-anchored authored pattern still behaves as full-match (redundant wrap is safe)", async () => { + const slug = metaField(FIELD_SUBTYPE_STRING, "slug"); + slug.setAttr(FIELD_ATTR_REQUIRED, true); + slug.addChild(validator(VALIDATOR_SUBTYPE_REGEX, "slugFormat", { + [VALIDATOR_ATTR_PATTERN]: "^[a-z0-9-]+$", + })); + const schema = await insertSchemaOf(entityWith("Post", slug)); + + expect(accepts(schema, { slug: "my-post-1" })).toBe(true); + expect(accepts(schema, { slug: "My Post" })).toBe(false); + expect(accepts(schema, { slug: "" })).toBe(false); + }); +}); + +describe("FR-036 Pin 1 — @required string is NON-EMPTY; an explicit @min is authoritative (#224)", () => { + test('default: a @required string rejects "" and null but ACCEPTS whitespace', async () => { + const title = metaField(FIELD_SUBTYPE_STRING, "title"); + title.setAttr(FIELD_ATTR_REQUIRED, true); + const schema = await insertSchemaOf(entityWith("Post", title)); + + expect(accepts(schema, { title: "hello" })).toBe(true); + expect(accepts(schema, { title: "" })).toBe(false); + expect(accepts(schema, { title: null })).toBe(false); + expect(accepts(schema, {})).toBe(false); + // The contract is non-EMPTY, deliberately not non-BLANK: whitespace passes. + expect(accepts(schema, { title: " " })).toBe(true); + }); + + test('@min:0 restores "must be provided, may be empty" — "" passes, absent still fails', async () => { + const title = metaField(FIELD_SUBTYPE_STRING, "title"); + title.setAttr(FIELD_ATTR_REQUIRED, true); + title.addChild(validator(VALIDATOR_SUBTYPE_LENGTH, "titleLen", { [VALIDATOR_ATTR_MIN]: 0 })); + const schema = await insertSchemaOf(entityWith("Post", title)); + + expect(accepts(schema, { title: "" })).toBe(true); + // Still REQUIRED — the opt-out relaxes emptiness, never presence. A text + // test for "no .min(1)" cannot distinguish this from dropping required. + expect(accepts(schema, {})).toBe(false); + expect(accepts(schema, { title: null })).toBe(false); + }); + + test("an explicit @min above the floor is enforced", async () => { + const code = metaField(FIELD_SUBTYPE_STRING, "code"); + code.setAttr(FIELD_ATTR_REQUIRED, true); + code.addChild(validator(VALIDATOR_SUBTYPE_LENGTH, "codeLen", { [VALIDATOR_ATTR_MIN]: 3 })); + const schema = await insertSchemaOf(entityWith("Item", code)); + + expect(accepts(schema, { code: "abc" })).toBe(true); + expect(accepts(schema, { code: "ab" })).toBe(false); + }); +}); + +describe("FR-036 A3 — @maxLength × validator.length @max is strictest-wins", () => { + test("the tighter validator @max wins over a looser field @maxLength", async () => { + const title = metaField(FIELD_SUBTYPE_STRING, "title"); + title.setAttr(FIELD_ATTR_REQUIRED, true); + title.setAttr(FIELD_ATTR_MAX_LENGTH, 200); + title.addChild(validator(VALIDATOR_SUBTYPE_LENGTH, "titleLen", { [VALIDATOR_ATTR_MAX]: 50 })); + const schema = await insertSchemaOf(entityWith("Post", title)); + + expect(accepts(schema, { title: "x".repeat(50) })).toBe(true); + expect(accepts(schema, { title: "x".repeat(51) })).toBe(false); + }); + + test("the tighter field @maxLength wins over a looser validator @max", async () => { + const title = metaField(FIELD_SUBTYPE_STRING, "title"); + title.setAttr(FIELD_ATTR_REQUIRED, true); + title.setAttr(FIELD_ATTR_MAX_LENGTH, 10); + title.addChild(validator(VALIDATOR_SUBTYPE_LENGTH, "titleLen", { [VALIDATOR_ATTR_MAX]: 500 })); + const schema = await insertSchemaOf(entityWith("Post", title)); + + expect(accepts(schema, { title: "x".repeat(10) })).toBe(true); + expect(accepts(schema, { title: "x".repeat(11) })).toBe(false); + }); +}); + +describe("field.inet — the 0.20.6 regression class, executed", () => { + // `z.string().ip()` was removed in Zod 4; the emitter now builds an explicit + // regex union (net-regex.ts). Nothing but running it against real literals + // proves the union still works on the resolved Zod. + test("accepts IPv4/IPv6 literals and rejects non-literals", async () => { + const addr = metaField(FIELD_SUBTYPE_INET, "addr"); + addr.setAttr(FIELD_ATTR_REQUIRED, true); + const schema = await insertSchemaOf(entityWith("Host", addr)); + + for (const ok of ["192.168.1.1", "0.0.0.0", "255.255.255.255", "::1", "2001:db8::1"]) { + expect(accepts(schema, { addr: ok })).toBe(true); + } + // H1 (#234 review): a leading-zero octet is octal/decimal-ambiguous — reject, + // matching Python `ipaddress` and the hand-parsers the other ports emit. + expect(accepts(schema, { addr: "010.0.0.1" })).toBe(false); + expect(accepts(schema, { addr: "192.168.01.1" })).toBe(false); + // Not an address at all / out of range / wrong arity. + for (const bad of ["256.1.1.1", "1.2.3", "1.2.3.4.5", "", "not-an-ip"]) { + expect(accepts(schema, { addr: bad })).toBe(false); + } + // A literal only — never CIDR, never a hostname (a hostname would imply a + // DNS lookup on the request path, the bug #234 fixed on the JVM ports). + expect(accepts(schema, { addr: "192.168.1.1/24" })).toBe(false); + expect(accepts(schema, { addr: "example.com" })).toBe(false); + }); + + test("H2: IPv4-mapped IPv6 is accepted (cross-port parity with the native address libs)", async () => { + const addr = metaField(FIELD_SUBTYPE_INET, "addr"); + addr.setAttr(FIELD_ATTR_REQUIRED, true); + const schema = await insertSchemaOf(entityWith("Host", addr)); + + expect(accepts(schema, { addr: "::ffff:192.168.1.1" })).toBe(true); + expect(accepts(schema, { addr: "2001:db8::1.2.3.4" })).toBe(true); + }); + + test("@lenient binds a plain string — strictness is genuinely opted out, not merely relabelled", async () => { + const addr = metaField(FIELD_SUBTYPE_INET, "addr"); + addr.setAttr(FIELD_ATTR_REQUIRED, true); + addr.setAttr(FIELD_ATTR_LENIENT, true); + const schema = await insertSchemaOf(entityWith("Host", addr)); + + expect(accepts(schema, { addr: "example.com" })).toBe(true); + expect(accepts(schema, { addr: "192.168.1.1/24" })).toBe(true); + }); +}); + +describe("field.uri — absolute, scheme-bearing only", () => { + test("accepts absolute URIs (incl. urn:/mailto:) and rejects relative ones", async () => { + const link = metaField(FIELD_SUBTYPE_URI, "link"); + link.setAttr(FIELD_ATTR_REQUIRED, true); + const schema = await insertSchemaOf(entityWith("Doc", link)); + + for (const ok of ["https://example.com/a?b=c", "http://x.io", "ftp://h/f", + "urn:isbn:0451450523", "mailto:a@b.com"]) { + expect(accepts(schema, { link: ok })).toBe(true); + } + // ADR-0036/0037: a URI must be absolute and scheme-bearing. + for (const bad of ["/relative/path", "example.com", "not a url", ""]) { + expect(accepts(schema, { link: bad })).toBe(false); + } + }); + + test("@lenient binds a plain string, so a relative reference passes", async () => { + const link = metaField(FIELD_SUBTYPE_URI, "link"); + link.setAttr(FIELD_ATTR_REQUIRED, true); + link.setAttr(FIELD_ATTR_LENIENT, true); + const schema = await insertSchemaOf(entityWith("Doc", link)); + + expect(accepts(schema, { link: "/relative/path" })).toBe(true); + }); +}); + +describe("@stringFormat — the canonical matcher is codegen-owned", () => { + test("email accepts a well-formed address and rejects malformed ones", async () => { + const email = metaField(FIELD_SUBTYPE_STRING, "email"); + email.setAttr(FIELD_ATTR_REQUIRED, true); + email.setAttr(FIELD_ATTR_STRING_FORMAT, STRING_FORMAT_EMAIL); + const schema = await insertSchemaOf(entityWith("User", email)); + + expect(accepts(schema, { email: "a@b.com" })).toBe(true); + for (const bad of ["a@b", "a b@c.com", "a@@b.com", "plain", ""]) { + expect(accepts(schema, { email: bad })).toBe(false); + } + }); + + test("hostname accepts a DNS name and rejects a URL or a spaced string", async () => { + const host = metaField(FIELD_SUBTYPE_STRING, "host"); + host.setAttr(FIELD_ATTR_REQUIRED, true); + host.setAttr(FIELD_ATTR_STRING_FORMAT, STRING_FORMAT_HOSTNAME); + const schema = await insertSchemaOf(entityWith("Node", host)); + + expect(accepts(schema, { host: "example.com" })).toBe(true); + expect(accepts(schema, { host: "sub.example.co.uk" })).toBe(true); + for (const bad of ["https://example.com", "not a host", ""]) { + expect(accepts(schema, { host: bad })).toBe(false); + } + }); +}); + +describe("field.enum — membership is enforced, not merely typed", () => { + test("accepts a declared member and rejects a non-member (case-sensitively)", async () => { + const status = metaField(FIELD_SUBTYPE_ENUM, "status"); + status.setAttr(FIELD_ATTR_REQUIRED, true); + status.setAttr(FIELD_ATTR_VALUES, ["DRAFT", "PUBLISHED"]); + const schema = await insertSchemaOf(entityWith("Order", status)); + + expect(accepts(schema, { status: "DRAFT" })).toBe(true); + expect(accepts(schema, { status: "PUBLISHED" })).toBe(true); + for (const bad of ["ARCHIVED", "draft", "", null]) { + expect(accepts(schema, { status: bad })).toBe(false); + } + }); +}); + +describe("numeric and array bound chains", () => { + test("validator.numeric @min/@max bounds an int, and the int-ness itself holds", async () => { + const qty = metaField(FIELD_SUBTYPE_INT, "qty"); + qty.setAttr(FIELD_ATTR_REQUIRED, true); + qty.addChild(validator(VALIDATOR_SUBTYPE_NUMERIC, "qtyRange", { + [VALIDATOR_ATTR_MIN]: 1, [VALIDATOR_ATTR_MAX]: 10, + })); + const schema = await insertSchemaOf(entityWith("Line", qty)); + + expect(accepts(schema, { qty: 1 })).toBe(true); + expect(accepts(schema, { qty: 10 })).toBe(true); + expect(accepts(schema, { qty: 0 })).toBe(false); + expect(accepts(schema, { qty: 11 })).toBe(false); + // z.number().int() — a fractional value and a numeric STRING are both rejected. + expect(accepts(schema, { qty: 5.5 })).toBe(false); + expect(accepts(schema, { qty: "5" })).toBe(false); + }); + + test("validator.array @min/@max bounds ELEMENT COUNT, not string length", async () => { + const tags = metaField(FIELD_SUBTYPE_STRING, "tags"); + tags.setAttr(FIELD_ATTR_REQUIRED, true); + // `isArray` is a NATIVE boolean property, not an attr — setAttr("isArray") + // would be an ERR_RESERVED_ATTR-shaped mistake and silently leave the field + // scalar. (Caught by this very test failing on the first run.) + tags.setIsArray(true); + tags.addChild(validator(VALIDATOR_SUBTYPE_ARRAY, "tagCount", { + [VALIDATOR_ATTR_MIN]: 1, [VALIDATOR_ATTR_MAX]: 3, + })); + const schema = await insertSchemaOf(entityWith("Post", tags)); + + expect(accepts(schema, { tags: ["a"] })).toBe(true); + expect(accepts(schema, { tags: ["a", "b", "c"] })).toBe(true); + expect(accepts(schema, { tags: [] })).toBe(false); + expect(accepts(schema, { tags: ["a", "b", "c", "d"] })).toBe(false); + // The bounds are on the array, so a long single element is fine — this is + // the distinction a `.min(1).max(3)` text match cannot make. + expect(accepts(schema, { tags: ["x".repeat(500)] })).toBe(true); + }); +}); diff --git a/server/typescript/packages/migrate-ts/test/integration/pg-gate-sentinel.test.ts b/server/typescript/packages/migrate-ts/test/integration/pg-gate-sentinel.test.ts new file mode 100644 index 000000000..f539adbfa --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/integration/pg-gate-sentinel.test.ts @@ -0,0 +1,30 @@ +/** + * Loud-skip guard for the real-Postgres suites in this package. + * + * Every one of them (`apply-pg`, `lifecycle-pg`, `postgres-roundtrip`, + * `postgres-lenient-inet`, `pg-adopt-view-239`, …) `describe.skip`s when + * `MIGRATE_TS_PG_URL` is unset. That is correct for a contributor with no local + * Postgres — but in CI the same silence let the lane rot RED for eight consecutive + * releases (v0.20.11 … v0.21.1) while every other lane stayed green, because the only + * workflow that set the URL ran on the `v*` tag push, i.e. strictly AFTER the + * irreversible four-registry publish. + * + * A lane that INTENDS to run these suites sets `MIGRATE_TS_PG_EXPECT=1` alongside the + * URL. This test then fails loudly if the URL plumbing ever rots — a renamed variable, + * a dropped sidecar, an unpublished port — instead of the suites quietly skipping and + * the lane reporting success over zero real-engine coverage. + * + * A workflow-level `test -n "$MIGRATE_TS_PG_URL"` step cannot do this job: it checks the + * workflow's environment, not what the test process actually reads, so a rename inside + * the tests is exactly the drift it would miss. The URL-set-but-Postgres-broken arm needs + * no sentinel — those suites already fail loudly on connect. + */ +import { describe, expect, test } from "bun:test"; + +describe("real-PG gate sentinel", () => { + test("MIGRATE_TS_PG_URL is set when the lane declares MIGRATE_TS_PG_EXPECT=1", () => { + if (process.env["MIGRATE_TS_PG_EXPECT"] === "1") { + expect(process.env["MIGRATE_TS_PG_URL"]).toBeTruthy(); + } + }); +}); diff --git a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-allowlist.ts b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-allowlist.ts index 412c7b35a..072982c33 100644 --- a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-allowlist.ts +++ b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-allowlist.ts @@ -20,6 +20,18 @@ export interface FilterFieldRule { readonly ops: readonly FilterOp[]; readonly subType: FilterSubType; readonly leadingWildcard: boolean; + /** + * `subType: "datetime"` only. Set by codegen when the column was emitted under + * `timestampMode: "date"`, i.e. the Drizzle column binds a JS `Date` rather than an + * ISO string. The filter parser then coerces the query-string value with `new Date(...)` + * instead of passing the raw string through — binding a string to a Date-typed column + * throws `value.toISOString is not a function` at request time. + * + * Optional and defaulting to false, so an allowlist generated before this existed keeps + * its exact previous behavior. `field.date` / `field.time` never set it: Drizzle types + * both as strings under every dialect, so they are not governed by `timestampMode`. + */ + readonly dateValues?: boolean; } export type FilterAllowlist = Readonly>; diff --git a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts index c4a2f852e..9e92f03c3 100644 --- a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts +++ b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts @@ -150,14 +150,14 @@ function compileOp( throw new FilterParseError("filter.unsupported_op", `Op "${op}" not supported for field "${field}".`, { field, op, allowed: rule.ops }); } switch (op as FilterOp) { - case "eq": return eq(col as any, coerce(value, rule.subType, field, op)); - case "ne": return ne(col as any, coerce(value, rule.subType, field, op)); - case "gt": return gt(col as any, coerce(value, rule.subType, field, op)); - case "gte": return gte(col as any, coerce(value, rule.subType, field, op)); - case "lt": return lt(col as any, coerce(value, rule.subType, field, op)); - case "lte": return lte(col as any, coerce(value, rule.subType, field, op)); + case "eq": return eq(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); + case "ne": return ne(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); + case "gt": return gt(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); + case "gte": return gte(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); + case "lt": return lt(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); + case "lte": return lte(col as any, coerce(value, rule.subType, field, op, rule.dateValues)); case "in": { - const list = String(value).split(",").map((v) => coerce(v.trim(), rule.subType, field, op)); + const list = String(value).split(",").map((v) => coerce(v.trim(), rule.subType, field, op, rule.dateValues)); if (list.length > maxInList) { throw new FilterParseError("filter.in_too_large", `In-list size ${list.length} exceeds limit ${maxInList}.`, { field, limit: maxInList }); } @@ -180,7 +180,7 @@ function compileOp( } } -function coerce(value: unknown, subType: string, field: string, op: string): unknown { +function coerce(value: unknown, subType: string, field: string, op: string, dateValues?: boolean): unknown { if (value === null || value === undefined) return null; const s = typeof value === "string" ? value : String(value); switch (subType) { @@ -197,21 +197,25 @@ function coerce(value: unknown, subType: string, field: string, op: string): unk } return n; } - // KNOWN LIMITATION (codegen-ts's `timestampMode: "date"` config option, - // Important-4 assessment): this keeps the qs value a STRING unconditionally. - // Under codegen's Postgres-only "date" mode the Drizzle column is Date-typed - // and calls `value.toISOString()` on any bound value — comparing it against a - // string here throws `TypeError: value.toISOString is not a function` for - // every op except isNull (eq/ne/gt/gte/lt/lte/in all bind through the same - // typed column). Not reachable in the default "string" mode (the only mode - // this parser was originally designed against). A correct fix needs the - // entity's `timestampMode` threaded from codegen into the generated - // allowlist or mount options so this function can `new Date(s)` — deferred - // as a separate, more invasive change; do not filter a "date"-mode timestamp - // field until that lands. `codegen-ts`'s `runGen` warns (once per run) at - // generation time when this combination is reachable — see the Important-4 - // check in runner.ts — but does not refuse the build. - case "datetime": return s; + // Under codegen's Postgres-only `timestampMode: "date"` the Drizzle column is + // Date-typed and calls `value.toISOString()` on any bound value, so passing the + // raw qs string through threw `TypeError: value.toISOString is not a function` + // for every op except isNull. The generated allowlist now carries `dateValues` + // for exactly those columns (see FilterFieldRule), so the value is bound as a + // real Date. In the default "string" mode — and for field.date / field.time, + // which Drizzle types as strings under every dialect — the flag is absent and + // the value stays a string, unchanged. + case "datetime": { + if (dateValues !== true) return s; + const d = new Date(s); + // `new Date("garbage")` yields an Invalid Date rather than throwing; binding + // one would emit `NaN`-shaped SQL. Reject at the boundary, matching how the + // number and boolean cases report a malformed value. + if (Number.isNaN(d.getTime())) { + throw new FilterParseError("filter.invalid_value", `Field "${field}" op "${op}" requires a date, got "${s}".`, { field, op, expected: "date" }); + } + return d; + } default: return s; } } diff --git a/server/typescript/packages/runtime-ts/test/filter-parser-date-mode.test.ts b/server/typescript/packages/runtime-ts/test/filter-parser-date-mode.test.ts new file mode 100644 index 000000000..35697689c --- /dev/null +++ b/server/typescript/packages/runtime-ts/test/filter-parser-date-mode.test.ts @@ -0,0 +1,142 @@ +// Date-mode timestamp filtering — behavioral pins. +// +// Under codegen-ts's `timestampMode: "date"` a Drizzle pg `timestamp({ mode: "date" })` +// column binds a JS `Date` and calls `.toISOString()` on whatever it is given. The filter +// parser used to hand it the raw query-string value, so EVERY op except `isNull` threw +// `TypeError: value.toISOString is not a function` at request time — a documented +// limitation that `meta gen` could only warn about. +// +// The generated allowlist now carries `dateValues: true` for exactly those columns and the +// parser coerces with `new Date(...)`. These tests drive the REAL Drizzle column through +// `parseFilterParams` and let Drizzle actually serialize the bound parameter, rather than +// asserting on the shape of the returned expression tree — the failure being fixed lived +// inside Drizzle's own binding step, so anything short of that would not have caught it. + +import { describe, test, expect } from "bun:test"; +import { pgTable, bigserial, timestamp, PgDialect } from "drizzle-orm/pg-core"; +import { gte as drizzleGte } from "drizzle-orm"; +import type { FilterAllowlist } from "../src/drizzle-fastify/filter-allowlist.js"; +import { parseFilterParams, FilterParseError } from "../src/drizzle-fastify/filter-parser.js"; + +// `mode: "date"` is exactly what codegen emits under timestampMode: "date". +const dateModeTable = pgTable("events", { + id: bigserial("id", { mode: "number" }).primaryKey(), + occurredAt: timestamp("occurred_at", { mode: "date", withTimezone: true }), +}); + +// The default mode: a string-typed column, unchanged behavior. +const stringModeTable = pgTable("events_s", { + id: bigserial("id", { mode: "number" }).primaryKey(), + occurredAt: timestamp("occurred_at", { mode: "string", withTimezone: true }), +}); + +const TS_OPS = ["eq", "ne", "gt", "gte", "lt", "lte", "in", "isNull"] as const; + +const dateModeAllowlist: FilterAllowlist = { + occurredAt: { ops: TS_OPS, subType: "datetime", leadingWildcard: false, dateValues: true }, +}; + +const stringModeAllowlist: FilterAllowlist = { + occurredAt: { ops: TS_OPS, subType: "datetime", leadingWildcard: false }, +}; + +const pg = new PgDialect(); + +/** + * Build the WHERE expression and run Drizzle's REAL parameter serialization over it — + * `sqlToQuery` applies each column's `mapToDriverValue`, which is precisely where a + * string bound to a date-mode column blew up. Returns the driver-bound params. + */ +function boundParams( + table: typeof dateModeTable | typeof stringModeTable, + allowlist: FilterAllowlist, + query: Record, +): unknown[] { + const { where } = parseFilterParams({ query, table, allowlist, sortAllowlist: {}, dialect: "postgres" }); + if (where === undefined) throw new Error("expected a WHERE expression"); + return pg.sqlToQuery(where.getSQL()).params; +} + +describe('timestampMode: "date" — filtering no longer throws at request time', () => { + test("CONTROL: binding a raw string to a date-mode column really does throw", () => { + // Proves these tests are not vacuous — this is the exact reported failure, + // reproduced directly against Drizzle, and it is what the parser used to hand over. + expect(() => + pg.sqlToQuery( + drizzleGte(dateModeTable.occurredAt, "2026-06-03T14:30:00.123Z" as never).getSQL(), + ), + ).toThrow(/toISOString is not a function/); + }); + + test("gte binds an ISO wire value cleanly against a date-mode column", () => { + const params = boundParams(dateModeTable, dateModeAllowlist, { + filter: { occurredAt: { gte: "2026-06-03T14:30:00.123Z" } }, + }); + expect(params.length).toBe(1); + // The driver value for a date-mode pg timestamp is the ISO string it derived + // from the Date we bound — i.e. mapToDriverValue ran instead of throwing. + expect(params[0]).toBe("2026-06-03T14:30:00.123Z"); + }); + + test("every comparison op survives the bind, not just gte", () => { + for (const op of ["eq", "ne", "gt", "gte", "lt", "lte"] as const) { + const params = boundParams(dateModeTable, dateModeAllowlist, { + filter: { occurredAt: { [op]: "2026-06-03T14:30:00.000Z" } }, + }); + expect(`${op}:${params[0]}`).toBe(`${op}:2026-06-03T14:30:00.000Z`); + } + }); + + test("in coerces every member of the list", () => { + const params = boundParams(dateModeTable, dateModeAllowlist, { + filter: { occurredAt: { in: "2026-06-03T00:00:00.000Z,2026-06-04T00:00:00.000Z" } }, + }); + expect(params).toEqual(["2026-06-03T00:00:00.000Z", "2026-06-04T00:00:00.000Z"]); + }); + + test("a date-only value is accepted (midnight UTC)", () => { + const params = boundParams(dateModeTable, dateModeAllowlist, { + filter: { occurredAt: { gte: "2026-06-03" } }, + }); + expect(params[0]).toBe("2026-06-03T00:00:00.000Z"); + }); + + test("a malformed value is rejected at the boundary, not bound as an Invalid Date", () => { + // `new Date("garbage")` yields an Invalid Date rather than throwing; binding one + // would emit NaN-shaped SQL instead of a 400. + expect(() => + boundParams(dateModeTable, dateModeAllowlist, { + filter: { occurredAt: { gte: "not-a-date" } }, + }), + ).toThrow(FilterParseError); + }); + + test("isNull still coerces as a boolean, unaffected by dateValues", () => { + const { where } = parseFilterParams({ + query: { filter: { occurredAt: { isNull: "true" } } }, + table: dateModeTable, + allowlist: dateModeAllowlist, + sortAllowlist: {}, + dialect: "postgres", + }); + expect(where).toBeDefined(); + }); +}); + +describe('default "string" mode is unchanged', () => { + test("without dateValues the value stays a string", () => { + const params = boundParams(stringModeTable, stringModeAllowlist, { + filter: { occurredAt: { gte: "2026-06-03T14:30:00.123Z" } }, + }); + expect(params).toEqual(["2026-06-03T14:30:00.123Z"]); + }); + + test("a value string mode would accept is not newly rejected", () => { + // String mode does no well-formedness check — that stays true, so an allowlist + // generated before `dateValues` existed behaves exactly as it did. + const params = boundParams(stringModeTable, stringModeAllowlist, { + filter: { occurredAt: { eq: "whatever-the-db-wants" } }, + }); + expect(params).toEqual(["whatever-the-db-wants"]); + }); +});