Skip to content

Commit fe2dfa1

Browse files
os-zhuangclaude
andauthored
fix(objectql): the autonumber fallback reads the declared {0000} default, not the empty string (#7262) (#7433)
Execution half 2/3 -- the last one -- of the maintainer's route-3 ruling on #6555. `applyAutonumbers` resolved the format by hand (`autonumberFormat ?? format`, then `typeof fmt === 'string' ? fmt : ''`), so a format-LESS field parsed the EMPTY string and rendered `renderAutonumber`'s no-slot branch as a bare `1`, `2`, .... driver-sql substituted its own `'{0000}'` and issued `0001`. One metadata document, two number shapes, decided by which driver served it. It is now `resolveAutonumberFormat(def)` -- the resolver landed by #7265 and already read by driver-sql since #7263. Unlike the driver half this MOVES behaviour, two ways, both in the changeset: - a format-less field on the engine fallback path issues `0001` where it issued `1` (engine-fallback deployments only; stored driver-sql data is undisturbed, and #6468's counter continuity is unaffected -- `{0000}` renders prefix '' / suffix '', so the seeding scan stays on its unanchored legacy reading); - `??` -> truthiness means `autonumberFormat: ''` / `format: ''` resolve to the default instead of rendering bare, and an empty canonical key no longer masks a declared `format` shorthand. Tests. New `engine-autonumber-default-format.test.ts` covers the empty-string inputs no suite on either side declared before (the gap the drivers seat measured on this card), plus the format-less renders and the controls that must NOT move. `autonumber-seed-cross-side-parity.integration.test.ts` gains the bug report's own reproduction -- no format, stored 1/2/10, both sides `0011` -- which is what lets that file assert a shared RENDERING and not only a shared counter. Five pre-existing cases asserting bare `'1'`/`'2'`/`'8'`/`'11'` on format-less fields were re-pinned to the padded form, each with a #6555 comment. Closes #7262. Part of #6555. Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ad6317b commit fe2dfa1

8 files changed

Lines changed: 492 additions & 39 deletions
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql): the engine's autonumber fallback reads the declared `{0000}` default instead of parsing the empty string (#7262)
6+
7+
Execution half 2/3 — the last one — of the maintainer's route-3 ruling on #6555.
8+
`{0000}` became a declared contract default in `@objectstack/spec/data`
9+
(`DEFAULT_AUTONUMBER_FORMAT` / `resolveAutonumberFormat`); `driver-sql` stopped
10+
writing its own copy down, and the engine now stops too.
11+
12+
`applyAutonumbers` resolved the format by hand:
13+
14+
```ts
15+
const fmt = (def as any).autonumberFormat ?? (def as any).format;
16+
const tokens = parseAutonumberFormat(typeof fmt === 'string' ? fmt : '');
17+
```
18+
19+
An undeclared format therefore parsed the EMPTY string, whose empty token list
20+
`renderAutonumber` renders through its no-slot branch as a bare counter. It is
21+
now `resolveAutonumberFormat(def)` — one resolver, shared with the SQL driver.
22+
23+
**⚠ Unlike the driver half, this one MOVES behaviour — two ways.**
24+
25+
1. **A format-less field on the engine's fallback path issues `0001` where it
26+
issued `1`.** The path is taken whenever the driver does not advertise
27+
`supports.autonumber``driver-memory`, `driver-mongodb`, any driver without
28+
the capability. Per the ruling: *choosing {0000} keeps stored driver-sql data
29+
undisturbed; engine-fallback deployments flip from bare 1 to 0001 for newly
30+
issued numbers. Counter continuity itself is unaffected (#6468 pinned it).*
31+
The counter is genuinely untouched: `{0000}` renders an empty prefix and an
32+
empty suffix, so the seeding scan stays on its unanchored legacy reading and
33+
goes on reading already-stored bare values (`1`, `2`, `10` → next is 11,
34+
rendered `0011`). Only the width of newly issued numbers changes, and only on
35+
this path.
36+
37+
2. **An EMPTY declared format is now "undeclared".** The engine read the key with
38+
`??`, which respects an empty string, so `autonumberFormat: ''` reached
39+
`parseAutonumberFormat` as `''` and rendered bare. `resolveAutonumberFormat`
40+
counts anything that is not a non-empty string as undeclared — the SQL
41+
driver's long-standing truthiness rule, which is what makes the two sides
42+
agree — so `autonumberFormat: ''` and `format: ''` now resolve to `{0000}`
43+
too. One further consequence of the same rule: an empty canonical key no
44+
longer masks a declared shorthand, so
45+
`{ autonumberFormat: '', format: 'D-{0000}' }` renders `D-0001` where it used
46+
to render a bare `1`.
47+
48+
**To keep a bare, unpadded counter**, declare a format with no `{0..0}` slot —
49+
`autonumberFormat: 'PRE-'` renders `PRE-1`. `autonumberFormat: ''` is NOT that
50+
spelling. **To keep the `0001` shape** that SQL deployments already store, and
51+
that a format-less field now mints everywhere, change nothing.
52+
53+
With this, #6555 is closed: one metadata document mints one number shape,
54+
whichever driver serves it.

packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,13 @@ describe('SqlDriver autonumber seeding — the counter is located by the declare
164164
// byte-for-byte: `'10'` wins over `'2'` — a numeric max, never a
165165
// lexicographic one — so the counter continues at 11.
166166
//
167-
// The RENDERING of a format-less field is a separate, pre-existing matter
168-
// this fix does not touch: a format-less field resolves to the contract
169-
// default `{0000}` (`resolveAutonumberFormat`, #6555), so 11 renders
170-
// `0011` here — while the engine's fallback still emits the bare `11`
171-
// until #7262 lands the other half. That divergence is in the render
172-
// default, not in the seeding parse #6468 is about, so the cross-side
173-
// parity test uses explicitly-formatted fields.
167+
// The RENDERING of a format-less field is a separate matter this fix does
168+
// not touch: a format-less field resolves to the contract default
169+
// `{0000}` (`resolveAutonumberFormat`, #6555), so 11 renders `0011` here.
170+
// The engine's fallback rendered a bare `11` until #7262 landed the other
171+
// half of that ruling; both sides now read the declared default, and
172+
// `autonumber-seed-cross-side-parity.integration.test.ts` asserts these
173+
// very rows against each other.
174174
await initRec();
175175
await seedRows(['1', '2', '10']);
176176

Lines changed: 292 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #6555 (half 2/3, #7262) — a format-LESS autonumber field renders through the
5+
* contract default `{0000}`, not through the empty string.
6+
*
7+
* `applyAutonumbers` used to read the format by hand — `autonumberFormat ??
8+
* format`, then `typeof fmt === 'string' ? fmt : ''` — so a field declaring no
9+
* format handed `parseAutonumberFormat` the EMPTY string. An empty token list
10+
* renders through `renderAutonumber`'s no-slot branch as a bare counter: `1`,
11+
* `2`, …. `driver-sql` answered the same question with its own hardcoded
12+
* `|| '{0000}'` and issued `0001`, `0002`, …. One metadata document therefore
13+
* minted differently-shaped numbers depending on which driver served it, and a
14+
* suite asserting `'1'` against the memory driver did not hold in production on
15+
* SQL. The counter VALUE always agreed — #6468 pinned that — so the fork was
16+
* rendering width alone.
17+
*
18+
* The maintainer's route-3 ruling on #6555 (2026-08-08) moved the default into
19+
* the contract: `DEFAULT_AUTONUMBER_FORMAT` / `resolveAutonumberFormat` in
20+
* `@objectstack/spec/data` (#7265), read by `driver-sql` (#7263) and, here, by
21+
* the engine. This file is the engine-side pin for the two behaviour moves that
22+
* lands with.
23+
*
24+
* ## Why this file exists at all — a measured coverage gap
25+
*
26+
* The drivers seat measured, while landing #7263, that NOT ONE test on either
27+
* side declared an empty-string format: `git grep "format: ''\|autonumberFormat:
28+
* ''"` returned nothing across all 8 driver-sql autonumber suites and all 7
29+
* `engine-autonumber-*.test.ts` suites. `''` is precisely the input this half
30+
* moves (`??` respects an empty string, the resolver's truthiness rule does
31+
* not), so a green run of the pre-existing suites is not evidence about it in
32+
* EITHER direction. Every `''` case below was written for that gap.
33+
*
34+
* The counterpart pins live in
35+
* `packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts` (the SQL
36+
* arm, `0011` since #7263) and
37+
* `packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts`
38+
* (the two arms asserted against each other over one dataset).
39+
*
40+
* These tests drive a fake DRIVER (not a fake engine) whose `supports = {}`, so
41+
* the engine's own fallback owns the counter — the path the whole card is about.
42+
*/
43+
44+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
45+
import { ObjectQL } from './engine';
46+
import { SchemaRegistry } from './registry';
47+
import type { IDataDriver } from '@objectstack/spec/contracts';
48+
49+
vi.mock('./registry', () => {
50+
const instance: any = {
51+
getObject: vi.fn(),
52+
resolveObject: vi.fn((n: string) => instance.getObject(n)),
53+
registerObject: vi.fn(),
54+
getObjectOwner: vi.fn(),
55+
registerNamespace: vi.fn(),
56+
registerKind: vi.fn(),
57+
registerItem: vi.fn(),
58+
registerApp: vi.fn(),
59+
installPackage: vi.fn(),
60+
reset: vi.fn(),
61+
metadata: { get: vi.fn(() => new Map()) },
62+
};
63+
function SchemaRegistry() {
64+
return instance;
65+
}
66+
Object.assign(SchemaRegistry, instance);
67+
return {
68+
SchemaRegistry,
69+
computeFQN: (_ns: string | undefined, name: string) => name,
70+
parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }),
71+
RESERVED_NAMESPACES: new Set(['base', 'system']),
72+
};
73+
});
74+
75+
/** Date tokens render from the wall clock, so the clock is pinned. Only `Date`. */
76+
const FIXED_NOW = new Date('2026-06-15T09:00:00Z');
77+
78+
/**
79+
* Evaluate the operators the seeding walk actually emits. Anything else throws
80+
* rather than being tolerated: silently ignoring an unknown operator would let a
81+
* bad query pass as a good one.
82+
*/
83+
function matches(row: Record<string, unknown>, where: any): boolean {
84+
if (where == null) return true;
85+
for (const [key, cond] of Object.entries(where)) {
86+
if (key === '$and') {
87+
if (!(cond as any[]).every((w) => matches(row, w))) return false;
88+
continue;
89+
}
90+
if (key.startsWith('$')) throw new Error(`fake driver: unsupported logical operator ${key}`);
91+
const v = row[key];
92+
if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
93+
for (const [op, operand] of Object.entries(cond as Record<string, unknown>)) {
94+
if (op === '$startsWith') {
95+
if (typeof v !== 'string' || !v.startsWith(String(operand))) return false;
96+
} else if (op === '$gt') {
97+
if (!(String(v) > String(operand))) return false;
98+
} else if (op === '$eq') {
99+
if (v !== operand) return false;
100+
} else {
101+
throw new Error(`fake driver: unsupported operator ${op}`);
102+
}
103+
}
104+
} else if (v !== cond) {
105+
return false;
106+
}
107+
}
108+
return true;
109+
}
110+
111+
function makeDriver(rows: Array<Record<string, unknown>>): IDataDriver {
112+
const driver: any = {
113+
name: 'memory',
114+
version: '0.0.0',
115+
// No `autonumber` support — this is exactly the engine fallback path.
116+
supports: {},
117+
connect: vi.fn().mockResolvedValue(undefined),
118+
disconnect: vi.fn().mockResolvedValue(undefined),
119+
checkHealth: vi.fn().mockResolvedValue(true),
120+
execute: vi.fn(),
121+
find: vi.fn(async (_obj: string, ast: any) => {
122+
let out = rows.filter((r) => matches(r, ast?.where));
123+
const orderBy = ast?.orderBy;
124+
if (Array.isArray(orderBy) && orderBy.length > 0) {
125+
const { field, order } = orderBy[0];
126+
out = [...out].sort((a, b) => {
127+
const av = String(a[field] ?? '');
128+
const bv = String(b[field] ?? '');
129+
const cmp = av < bv ? -1 : av > bv ? 1 : 0;
130+
return order === 'desc' ? -cmp : cmp;
131+
});
132+
}
133+
if (typeof ast?.limit === 'number') out = out.slice(0, ast.limit);
134+
return out.map((r) => ({ ...r }));
135+
}),
136+
findOne: vi.fn(),
137+
create: vi.fn(async (_obj: string, row: any) => ({ id: 'new1', ...row })),
138+
update: vi.fn(),
139+
delete: vi.fn(),
140+
count: vi.fn(),
141+
};
142+
return driver as IDataDriver;
143+
}
144+
145+
const rowId = (n: number) => `r${String(n).padStart(6, '0')}`;
146+
147+
/**
148+
* A schema whose single autonumber field carries EXACTLY the given keys — the
149+
* point of most cases below is a key that is present and empty, which a
150+
* `format?: string` parameter cannot express.
151+
*/
152+
function schemaWith(declaration: Record<string, unknown>) {
153+
return {
154+
name: 'rec',
155+
fields: {
156+
title: { type: 'text' },
157+
rec_no: { type: 'autonumber', required: true, ...declaration },
158+
},
159+
};
160+
}
161+
162+
/** Stored rows carrying pre-existing record numbers, in insertion order. */
163+
const storedRows = (values: string[]) =>
164+
values.map((v, i) => ({ id: rowId(i + 1), rec_no: v }));
165+
166+
async function issueOne(schema: any, rows: Array<Record<string, unknown>> = []): Promise<string> {
167+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(schema as any);
168+
const engine = new ObjectQL();
169+
engine.registerDriver(makeDriver(rows) as any, true);
170+
await engine.init();
171+
const result: any = await engine.insert('rec', { title: 'next' });
172+
return result.rec_no;
173+
}
174+
175+
describe('ObjectQL applyAutonumbers — the contract default for a format-less field (#6555)', () => {
176+
beforeEach(() => {
177+
vi.clearAllMocks();
178+
vi.useFakeTimers({ toFake: ['Date'] });
179+
vi.setSystemTime(FIXED_NOW);
180+
});
181+
182+
afterEach(() => {
183+
vi.useRealTimers();
184+
});
185+
186+
// ----------------------------------------- (1) the primary behaviour move --
187+
188+
describe('an undeclared format renders `{0000}`, not the bare counter', () => {
189+
/** The bug report's own metadata: `{ rec_no: { type: 'autonumber' } }`. */
190+
it('issues `0001` on an empty store', async () => {
191+
expect(await issueOne(schemaWith({}))).toBe('0001');
192+
});
193+
194+
it('issues `0011` after stored `1` / `2` / `10` — the bug report verbatim', async () => {
195+
// The reproduction from #6555. Two facts in one assertion: the counter
196+
// still reads the stored BARE values (seeding is untouched — `{0000}`
197+
// renders prefix '' and suffix '', so the unanchored legacy reading still
198+
// applies and `'10'` beats `'2'` numerically), and the number it issues is
199+
// now RENDERED padded. `driver-sql` answers `0011` over the same rows.
200+
expect(await issueOne(schemaWith({}), storedRows(['1', '2', '10']))).toBe('0011');
201+
});
202+
203+
it('the counter continues across calls, each rendered padded', async () => {
204+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(schemaWith({}) as any);
205+
const engine = new ObjectQL();
206+
engine.registerDriver(makeDriver([]) as any, true);
207+
await engine.init();
208+
209+
const a: any = await engine.insert('rec', { title: 'a' });
210+
const b: any = await engine.insert('rec', { title: 'b' });
211+
212+
expect([a.rec_no, b.rec_no]).toEqual(['0001', '0002']);
213+
});
214+
});
215+
216+
// ------------------------------- (2) the second, smaller move: `''` inputs --
217+
218+
/**
219+
* The gap the drivers seat measured (#7262, comment 5237739551): no suite on
220+
* either side declared an empty-string format, and `''` is the one input whose
221+
* behaviour this half moves. The engine read the key with `??`, which respects
222+
* an empty string; `resolveAutonumberFormat` counts anything that is not a
223+
* NON-EMPTY string as undeclared — driver-sql's long-standing truthiness rule,
224+
* adopted deliberately so the two sides agree.
225+
*/
226+
describe('an EMPTY declared format is undeclared, and resolves to the default', () => {
227+
it("`autonumberFormat: ''` renders `0001`, not a bare `1`", async () => {
228+
expect(await issueOne(schemaWith({ autonumberFormat: '' }))).toBe('0001');
229+
});
230+
231+
it("`format: '' ` renders `0001`, not a bare `1`", async () => {
232+
expect(await issueOne(schemaWith({ format: '' }))).toBe('0001');
233+
});
234+
235+
it("an empty canonical key no longer MASKS a declared `format` shorthand", async () => {
236+
// The sharpest edge of `??` → truthiness, and the only case where the two
237+
// rules disagree on something other than the default: `'' ?? 'D-{0000}'`
238+
// is `''` (nullish coalescing does not fall through an empty string), so
239+
// the engine used to render bare and ignore the shorthand entirely. The
240+
// resolver falls through to it.
241+
expect(await issueOne(schemaWith({ autonumberFormat: '', format: 'D-{0000}' }))).toBe('D-0001');
242+
});
243+
244+
it('a key holding a non-string is undeclared too', async () => {
245+
// Unreachable through a parsed `FieldSchema`, reachable through the
246+
// unvalidated field documents both generators actually hold. The old code
247+
// fell to `''` here (`typeof fmt === 'string' ? fmt : ''`) and rendered
248+
// bare; the resolver answers the declared default, same as driver-sql.
249+
expect(await issueOne(schemaWith({ autonumberFormat: 42 }))).toBe('0001');
250+
expect(await issueOne(schemaWith({ format: null }))).toBe('0001');
251+
});
252+
});
253+
254+
// -------------------------------------------------- (3) controls — UNMOVED --
255+
256+
/**
257+
* Drift guards for the surface this change must NOT touch. Stated plainly:
258+
* these cannot go red when the fix is reverted, so they are not evidence for
259+
* the moving leg above — they exist to catch a future edit that overreaches.
260+
*/
261+
describe('a DECLARED format is honoured exactly as written', () => {
262+
it('`D-{0000}` is unchanged', async () => {
263+
expect(await issueOne(schemaWith({ format: 'D-{0000}' }), storedRows(['D-0001', 'D-0002']))).toBe('D-0003');
264+
});
265+
266+
it('the spec-canonical key still wins over the shorthand (#1603)', async () => {
267+
expect(await issueOne(schemaWith({ autonumberFormat: 'A-{000}', format: 'B-{000}' }))).toBe('A-001');
268+
});
269+
270+
it('a slot-less format still renders a BARE counter — the escape hatch', async () => {
271+
// The documented way to keep an unpadded number after this change: declare
272+
// a format with no `{0..0}` slot. `autonumberFormat: ''` is NOT that
273+
// spelling (see above), which is the whole reason the changeset spells
274+
// this out for anyone who was relying on the engine's bare rendering.
275+
expect(await issueOne(schemaWith({ format: 'PRE-' }))).toBe('PRE-1');
276+
});
277+
278+
it('a driver that owns autonumber is untouched — the engine fills nothing', async () => {
279+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(schemaWith({}) as any);
280+
const driver: any = makeDriver([]);
281+
driver.supports = { autonumber: true };
282+
const engine = new ObjectQL();
283+
engine.registerDriver(driver, true);
284+
await engine.init();
285+
286+
await engine.insert('rec', { title: 'next' });
287+
288+
// The driver's own sequence answers; the engine hands it an empty slot.
289+
expect(driver.create.mock.calls[0][1].rec_no).toBeUndefined();
290+
});
291+
});
292+
});

0 commit comments

Comments
 (0)