Skip to content

Commit c499d50

Browse files
dmealingclaude
andcommitted
feat(codegen-ts,runtime-ts): support filtering a date-mode timestamp
Filtering a @filterable field.timestamp under timestampMode: "date" threw at request time. The Drizzle pg column binds a JS Date and calls .toISOString() on whatever it is given, while the filter parser passed the raw query-string value through — so eq/ne/gt/gte/lt/lte/in all died with `TypeError: value.toISOString is not a function`. Only isNull survived, since it coerces as a boolean regardless of subtype. The generated FilterAllowlist rule now carries `dateValues: true` for exactly those columns, and the parser coerces with `new Date(...)`. A malformed value is rejected as filter.invalid_value rather than bound as an Invalid Date, which would have emitted NaN-shaped SQL instead of a 400. `dateValues` is optional, so an allowlist generated before this existed keeps its exact previous behavior. Only field.timestamp is marked — Drizzle types field.date and field.time as strings under every dialect, so neither is governed by timestampMode — and ctx.timestampMode is already normalized to "string" for sqlite/D1 at both config choke points, so no dialect branching is needed in the emitter. The gen-time warning that announced this limitation is removed rather than left to cry wolf; its test now pins the warning's ABSENCE in all four mode/dialect combinations, so reverting the fix cannot quietly re-land the warning in place of the behavior. Tests drive Drizzle's real parameter serialization (PgDialect.sqlToQuery, which applies mapToDriverValue) rather than asserting on the expression tree — the failure lived inside that binding step. Includes a control test that binds a raw string to a date-mode column and asserts it still throws the original TypeError, so the suite is provably non-vacuous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
1 parent 81c8adf commit c499d50

12 files changed

Lines changed: 288 additions & 93 deletions

File tree

‎server/typescript/packages/codegen-ts/src/metaobjects-config.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,13 @@ export interface MetaobjectsGenConfig extends Omit<ResolvedGenConfig, "dbImport"
121121
* porting docs). This keeps the option a safe no-op on sqlite/D1 instead of
122122
* emitting a non-compiling column + a Zod schema disagreeing with it.
123123
*
124-
* Date-mode filtering (`?filter[<timestamp>][gte]=...`) is not yet supported —
125-
* `runtime-ts`'s filter parser keeps the qs value a string, which throws at
126-
* request time against a Date-mode Drizzle column. Known limitation; not
127-
* threaded through at runtime, but `runGen` DOES warn (once per run, naming
128-
* every offending entity+field) when this mode meets a `@filterable`
129-
* `field.timestamp` — see `runner.ts`'s Important-4 check. Track before
130-
* recommending date mode for a filterable timestamp field.
124+
* Date-mode filtering (`?filter[<timestamp>][gte]=...`) IS supported: a
125+
* `@filterable` `field.timestamp` generated under this mode carries
126+
* `dateValues: true` in its `FilterAllowlist` rule, and `runtime-ts`'s filter
127+
* parser coerces the query-string value with `new Date(...)` rather than binding
128+
* a string against a Date-typed column (a malformed value is rejected as
129+
* `filter.invalid_value`). `field.date` / `field.time` are unaffected — Drizzle
130+
* types both as strings under every dialect.
131131
*/
132132
timestampMode?: "date" | "string";
133133
/** Path prefix applied to generated route registrations + hook fetch URLs. Defaults to "". */

‎server/typescript/packages/codegen-ts/src/reference/entity.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ function renderEntity(entity: MetaObject, ctx: RenderContext, opts?: RenderEntit
100100
...(enumAliases !== null ? [enumAliases] : []),
101101
renderZodValidators(entity, ctx),
102102
renderEntityConstants(entity, ctx.apiPrefix),
103-
...(allowlists ? [renderFilterAllowlist(entity), renderSortAllowlist(entity)] : []),
103+
...(allowlists ? [renderFilterAllowlist(entity, undefined, ctx), renderSortAllowlist(entity)] : []),
104104
renderFilterType(entity),
105105
...(tphBlock !== null ? [tphBlock] : []),
106106
];

‎server/typescript/packages/codegen-ts/src/runner.ts‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -180,36 +180,11 @@ export async function runGen(opts: RunGenOpts): Promise<RunGenResult> {
180180
// 2. Resolve targets + entity-module target.
181181
const config = normalizeConfig(opts.config);
182182

183-
// Important-4 (post-#281 pre-publish review) — a @filterable timestamp field
184-
// under timestampMode:"date" throws at REQUEST time in runtime-ts's filter
185-
// parser (documented limitation; see drizzle-fastify/filter-parser.ts's
186-
// "datetime" coerce case). Detect-and-WARN at generation time instead of
187-
// leaving it a silent build-time no-signal (repo precedent: #226/#258
188-
// detect-and-refuse an un-appliable migration at gen time rather than fail
189-
// at apply). Reading config.timestampMode AFTER normalizeConfig means this is
190-
// naturally silent on sqlite/D1 (normalized to "string" there) and in the
191-
// default "string" mode — no dialect/mode branching needed here. One warning
192-
// for the whole run, naming every offending entity+field, not one per field.
193-
if (config.timestampMode === "date") {
194-
const offenders = safeEntities
195-
.filter((e) => !e.isAbstract)
196-
.map((e) => ({
197-
entity: e.name,
198-
fields: e.fields()
199-
.filter((f) => f.subType === FIELD_SUBTYPE_TIMESTAMP && f.attr(FIELD_ATTR_FILTERABLE) === true)
200-
.map((f) => f.name),
201-
}))
202-
.filter((o) => o.fields.length > 0);
203-
if (offenders.length > 0) {
204-
const named = offenders.map((o) => `${o.entity}.${o.fields.join(",")}`).join("; ");
205-
warnings.push(
206-
`timestampMode: "date" — @filterable timestamp field(s) [${named}] will throw at ` +
207-
`request time when filtered (e.g. ?filter[field][gte]=...) — runtime-ts's filter ` +
208-
`parser does not yet thread the Date-mode column type through. Not enforced; ` +
209-
`remove @filterable from these fields or avoid filtering them until this is fixed.`,
210-
);
211-
}
212-
}
183+
// (Historical: a warning stood here for a @filterable timestamp under
184+
// timestampMode:"date", which used to throw at REQUEST time in runtime-ts's
185+
// filter parser. That limitation is fixed — the generated allowlist now carries
186+
// `dateValues: true` for a Date-mode timestamp column and the parser coerces with
187+
// `new Date(...)` — so the warning was removed rather than left to cry wolf.)
213188

214189
const targets = config.targets;
215190
const targetOf = (g: Generator): ResolvedTarget => {

‎server/typescript/packages/codegen-ts/src/templates/entity-file.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ ${docsPrefix}export type ${entity.name} = ${z}.infer<typeof ${entity.name}Schema
165165
...(enumAliases !== null ? [enumAliases] : []),
166166
renderZodValidators(entity, ctx),
167167
renderEntityConstants(entity, ctx.apiPrefix),
168-
...(allowlists ? [renderFilterAllowlist(entity), renderSortAllowlist(entity)] : []),
168+
...(allowlists ? [renderFilterAllowlist(entity, undefined, ctx), renderSortAllowlist(entity)] : []),
169169
renderFilterType(entity),
170170
...(tphBlock !== null ? [tphBlock] : []),
171171
];

‎server/typescript/packages/codegen-ts/src/templates/filter-allowlist.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
opsForSubType,
1717
} from "@metaobjectsdev/metadata";
1818
import { sortableFields } from "./filter-shared.js";
19+
import type { RenderContext } from "../render-context.js";
1920

2021
const NUMBER_SUBTYPES = new Set<string>([
2122
FIELD_SUBTYPE_INT,
@@ -52,8 +53,15 @@ function filterableFields(entity: MetaObject, exclude?: string): MetaField[] {
5253
* `exclude` (FR-017): drop a field from the allowlist. Used by per-subtype TPH
5354
* allowlists to omit the discriminator — it's pinned by the per-subtype route
5455
* path, so a client must not filter on it.
56+
*
57+
* `ctx` supplies `timestampMode`: under `"date"` a `field.timestamp` column binds a JS
58+
* `Date`, so its rule carries `dateValues: true` and runtime-ts's filter parser coerces
59+
* the query-string value with `new Date(...)` rather than binding a string against a
60+
* Date-typed column (which threw at request time). `ctx.timestampMode` is already
61+
* normalized to `"string"` for sqlite/D1 at both config choke points, so no dialect
62+
* branching is needed here. Absent `ctx`, or in the default mode, output is unchanged.
5563
*/
56-
export function renderFilterAllowlist(entity: MetaObject, exclude?: string): Code {
64+
export function renderFilterAllowlist(entity: MetaObject, exclude?: string, ctx?: RenderContext): Code {
5765
const fields = filterableFields(entity, exclude);
5866
if (fields.length === 0) {
5967
return code`
@@ -66,7 +74,12 @@ export const ${entity.name}FilterAllowlist = {} as const satisfies FilterAllowli
6674
.map((f) => {
6775
const ops = opsForSubType(f.subType).map((o) => JSON.stringify(o)).join(", ");
6876
const sub = filterSubTypeFor(f.subType);
69-
return ` ${f.name}: { ops: [${ops}] as const, subType: ${JSON.stringify(sub)} as const, leadingWildcard: false }`;
77+
// Only field.timestamp is governed by timestampMode — Drizzle types
78+
// field.date / field.time as strings under every dialect.
79+
const dateValues = ctx?.timestampMode === "date" && f.subType === FIELD_SUBTYPE_TIMESTAMP
80+
? ", dateValues: true as const"
81+
: "";
82+
return ` ${f.name}: { ops: [${ops}] as const, subType: ${JSON.stringify(sub)} as const, leadingWildcard: false${dateValues} }`;
7083
})
7184
.join(",\n");
7285
return code`

‎server/typescript/packages/codegen-ts/src/templates/projection-decl.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ ${constFieldLines.join("\n")}
176176
} as const;
177177
`,
178178
...(allowlists
179-
? [renderFilterAllowlist(projection), renderSortAllowlist(projection)]
179+
? [renderFilterAllowlist(projection, undefined, ctx), renderSortAllowlist(projection)]
180180
: []),
181181
renderFilterType(projection),
182182
];

‎server/typescript/packages/codegen-ts/src/templates/value-object-file.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function renderValueObjectFile(obj: MetaObject, apiPrefix = "", ctx?: Ren
4040
// not filter on it). Included fields are the subtype's own + inherited base
4141
// filterable fields. Drives the per-subtype REST routes' filter layer.
4242
const discField = tphSubtype ? tphDiscriminatorPin(obj)?.fieldName : undefined;
43-
const tphFilterAllowlist = tphSubtype ? renderFilterAllowlist(obj, discField) : null;
43+
const tphFilterAllowlist = tphSubtype ? renderFilterAllowlist(obj, discField, ctx) : null;
4444
const tphSortAllowlist = tphSubtype ? renderSortAllowlist(obj, discField) : null;
4545
// FR-017 Tier 3: the per-subtype CLIENT filter type, discriminator-excluded —
4646
// kept in lockstep with the per-subtype allowlist above so a typed

‎server/typescript/packages/codegen-ts/test/templates/filter-allowlist.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import { renderFilterAllowlist, renderSortAllowlist } from "../../src/templates/
33
import { resolve } from "node:path";
44
import { MetaDataLoader } from "@metaobjectsdev/metadata";
55
import { FileSource } from "@metaobjectsdev/metadata/core";
6+
import { makeRenderContext } from "../../src/render-context.js";
7+
import { buildPkMap } from "../../src/pk-resolver.js";
8+
import { buildRelationMap } from "../../src/relation-resolver.js";
69

710
const FIXTURE = resolve(import.meta.dir, "..", "fixtures", "filter-fixture.json");
811

@@ -73,3 +76,54 @@ describe("renderSortAllowlist", () => {
7376
expect(out).toMatch(/price:\s*\{/);
7477
});
7578
});
79+
80+
describe("renderFilterAllowlist — timestampMode date-mode marking", () => {
81+
// A date-mode timestamp column binds a JS Date, so the rule must carry
82+
// `dateValues: true` for runtime-ts's parser to coerce with `new Date(...)`
83+
// instead of binding a string (which threw at request time). The behavioral
84+
// half of this contract is pinned in runtime-ts's filter-parser-date-mode test.
85+
async function allowlistFor(mode?: "date" | "string") {
86+
const { root } = await new MetaDataLoader().load([new FileSource(FIXTURE)]);
87+
const entity = root.objects().find((c) => c.name === "Subscriber")!;
88+
const ctx = makeRenderContext({
89+
dialect: "postgres",
90+
...(mode !== undefined && { timestampMode: mode }),
91+
loadedRoot: root,
92+
outDir: "/x",
93+
dbImport: "~/db",
94+
pkMap: buildPkMap(root),
95+
relationMap: buildRelationMap(root),
96+
});
97+
return renderFilterAllowlist(entity, undefined, ctx).toString();
98+
}
99+
100+
test('timestampMode:"date" marks the timestamp field and ONLY the timestamp field', async () => {
101+
const out = await allowlistFor("date");
102+
expect(out).toMatch(/createdAt:\s*\{[^}]*dateValues: true/);
103+
// Non-timestamp fields must not be marked — the flag is meaningless for them
104+
// and would make the parser try to Date-coerce a string or boolean.
105+
expect(out).not.toMatch(/email:\s*\{[^}]*dateValues/);
106+
expect(out).not.toMatch(/subscribed:\s*\{[^}]*dateValues/);
107+
});
108+
109+
test('the default "string" mode emits no dateValues at all', async () => {
110+
expect(await allowlistFor()).not.toContain("dateValues");
111+
expect(await allowlistFor("string")).not.toContain("dateValues");
112+
});
113+
114+
test('dialect:"sqlite" never marks — timestampMode normalizes to "string" there', async () => {
115+
const { root } = await new MetaDataLoader().load([new FileSource(FIXTURE)]);
116+
const entity = root.objects().find((c) => c.name === "Subscriber")!;
117+
const ctx = makeRenderContext({
118+
dialect: "sqlite", timestampMode: "date", loadedRoot: root,
119+
outDir: "/x", dbImport: "~/db",
120+
pkMap: buildPkMap(root), relationMap: buildRelationMap(root),
121+
});
122+
expect(renderFilterAllowlist(entity, undefined, ctx).toString()).not.toContain("dateValues");
123+
});
124+
125+
test("no ctx (bare call) is unchanged", async () => {
126+
const entity = await loadEntity("Subscriber");
127+
expect(renderFilterAllowlist(entity).toString()).not.toContain("dateValues");
128+
});
129+
});

‎server/typescript/packages/codegen-ts/test/timestamp-mode-execution.test.ts‎

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@
99
// dynamically `import()` it, then call the real `safeParse`/`parse` on the
1010
// real Zod object it exports — not a string match.
1111
//
12-
// Covers the three Criticals + Important 5 found by that review, plus a
13-
// codegen-time WARNING for Important 4 (added on top after the initial fix
14-
// was reviewed clean — a cheap generation-time detect for the one runtime gap
15-
// left documented-not-fixed: filtering a date-mode timestamp throws at
16-
// request time; see runGen's warning + filter-parser.ts's limitation note):
12+
// Covers the three Criticals + Important 5 found by that review. Important 4
13+
// (filtering a date-mode timestamp threw at request time) was originally shipped
14+
// as a generation-time WARNING because the runtime fix was deferred; that fix has
15+
// since landed — the generated allowlist carries `dateValues` and runtime-ts's
16+
// parser coerces with `new Date(...)` — so the warning was removed and the last
17+
// describe block below now pins its ABSENCE. The behavioral pins for the fix
18+
// itself live in runtime-ts's filter-parser-date-mode test and in
19+
// templates/filter-allowlist.test.ts.
1720
// CRITICAL 1 — z.date() rejects every JSON wire value; fix is z.coerce.date().
1821
// CRITICAL 2 — sqlite/D1 + date mode used to emit non-compiling code (a
1922
// regression #281 itself introduced); fix normalizes the mode
@@ -269,27 +272,19 @@ describe('IMPORTANT 4: runGen warns (once) when timestampMode: "date" meets a @f
269272
}
270273
}
271274

272-
test('warns exactly once, naming every offending entity+field, when dialect:"postgres" + timestampMode:"date"', async () => {
273-
const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "postgres", "date");
274-
const hits = warnings.filter((w) => w.includes('timestampMode: "date"'));
275-
expect(hits.length).toBe(1); // once per run, not once per field/entity
276-
expect(hits[0]).toContain("Post.updatedAt");
277-
expect(hits[0]).toContain("Comment.postedAt");
278-
expect(hits[0]).not.toContain("archivedAt"); // non-filterable — not named
279-
});
280-
281-
test('silent in the default "string" mode (timestampMode omitted)', async () => {
282-
const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "postgres");
283-
expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]);
284-
});
285-
286-
test('silent when no field is both @filterable and a timestamp, even in date mode', async () => {
287-
const { warnings } = await runWithMetadata(NO_FILTERABLE_TIMESTAMP, "postgres", "date");
288-
expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]);
289-
});
290-
291-
test('silent under dialect:"sqlite" — timestampMode normalizes to "string" (Critical 2), so Important 4 never fires', async () => {
292-
const { warnings } = await runWithMetadata(TWO_FILTERABLE_TIMESTAMPS, "sqlite", "date");
293-
expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]);
275+
test("no warning is emitted in any mode — the limitation it announced is fixed", async () => {
276+
// The warning was correct while date-mode filtering threw at request time. Now
277+
// that the allowlist carries `dateValues` and the parser coerces, it would be
278+
// crying wolf, so it is gone. Pinned in all four combinations so a revert of
279+
// the fix cannot quietly re-land the warning instead of the behavior.
280+
for (const [json, dialect, mode] of [
281+
[TWO_FILTERABLE_TIMESTAMPS, "postgres", "date"],
282+
[TWO_FILTERABLE_TIMESTAMPS, "postgres", undefined],
283+
[NO_FILTERABLE_TIMESTAMP, "postgres", "date"],
284+
[TWO_FILTERABLE_TIMESTAMPS, "sqlite", "date"],
285+
] as const) {
286+
const { warnings } = await runWithMetadata(json, dialect, mode);
287+
expect(warnings.filter((w) => w.includes('timestampMode: "date"'))).toEqual([]);
288+
}
294289
});
295290
});

‎server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-allowlist.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ export interface FilterFieldRule {
2020
readonly ops: readonly FilterOp[];
2121
readonly subType: FilterSubType;
2222
readonly leadingWildcard: boolean;
23+
/**
24+
* `subType: "datetime"` only. Set by codegen when the column was emitted under
25+
* `timestampMode: "date"`, i.e. the Drizzle column binds a JS `Date` rather than an
26+
* ISO string. The filter parser then coerces the query-string value with `new Date(...)`
27+
* instead of passing the raw string through — binding a string to a Date-typed column
28+
* throws `value.toISOString is not a function` at request time.
29+
*
30+
* Optional and defaulting to false, so an allowlist generated before this existed keeps
31+
* its exact previous behavior. `field.date` / `field.time` never set it: Drizzle types
32+
* both as strings under every dialect, so they are not governed by `timestampMode`.
33+
*/
34+
readonly dateValues?: boolean;
2335
}
2436

2537
export type FilterAllowlist = Readonly<Record<string, FilterFieldRule>>;

0 commit comments

Comments
 (0)