Skip to content

Commit b990b89

Browse files
xuyushun441-sysos-zhuangclaude
authored
fix(autonumber): single owner = persistent driver sequence (#1603) (#1608)
Autonumber values were generated twice: the SQL driver's persistent, atomic _objectstack_sequences table AND a non-persistent in-memory counter in the engine. The engine pre-filled before calling the driver, so the driver always saw a value and skipped — leaving the persistent sequence as dead code and risking duplicate numbers across instances / after a restart. - spec: DriverCapabilities.autonumber (optional) capability flag. - driver-sql: advertise supports.autonumber=true; bulkCreate now fills autonumber too (was create/upsert only); parse honors autonumberFormat OR format. - objectql: engine defers to the driver when it advertises native autonumber support (no more pre-fill / in-memory shadow); keeps the in-memory fallback for memory/mongodb; fallback also reads either format key; record-validation exempts autonumber from the required check (runtime-owned, assigned post-validation). Tests: driver bulkCreate + autonumberFormat + capability (driver-sql 140 pass); engine defer-vs-fallback + validator exemption (objectql 526 pass). Existing data respected via scanMaxNumericTail bootstrap. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 345e189 commit b990b89

8 files changed

Lines changed: 293 additions & 16 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/objectql': minor
4+
'@objectstack/driver-sql': minor
5+
---
6+
7+
fix(autonumber): one owner for autonumber generation — the persistent driver sequence (#1603)
8+
9+
Autonumber values were generated in TWO places: the SQL driver's persistent,
10+
atomic `_objectstack_sequences` table AND a non-persistent in-memory counter in
11+
the ObjectQL engine. Because the engine pre-filled the field BEFORE calling the
12+
driver, the driver always saw a value already set and skipped — so the
13+
persistent sequence was effectively dead code, and a multi-instance / post-restart
14+
deployment could mint duplicate numbers from the in-memory counter.
15+
16+
This makes generation single-owner:
17+
18+
- **`@objectstack/spec`** — `DriverCapabilities` gains an optional `autonumber`
19+
flag: "driver natively generates persistent autonumber/sequence values".
20+
21+
- **`@objectstack/driver-sql`** — advertises `supports.autonumber = true`.
22+
`bulkCreate()` now fills autonumber fields too (previously only `create()` /
23+
`upsert()` did), so bulk inserts also draw from the persistent sequence.
24+
Field parsing now honors either the spec-canonical `autonumberFormat` key OR
25+
the `format` shorthand (both appear in metadata).
26+
27+
- **`@objectstack/objectql`** — when the driver advertises native autonumber
28+
support, the engine NO LONGER pre-fills (it defers entirely to the persistent
29+
driver sequence as the single source of truth). For drivers without native
30+
support (memory, mongodb) the in-memory fallback is unchanged. The fallback
31+
also now reads either `autonumberFormat` or `format`. Record-validation
32+
exempts `autonumber` fields from the `required` check — the value is
33+
runtime-owned and assigned after validation, so a required record number is
34+
never rejected as "missing".
35+
36+
No metadata changes required. Existing data is respected: the driver bootstraps
37+
each sequence from the current max numeric tail on first use.
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi, beforeEach } from 'vitest';
4+
import { ObjectQL } from './engine';
5+
import { SchemaRegistry } from './registry';
6+
import type { IDataDriver } from '@objectstack/spec/contracts';
7+
8+
/**
9+
* #1603 — autonumber generation is owned by ONE layer.
10+
*
11+
* When the driver advertises `supports.autonumber === true` (the SQL driver,
12+
* which has a persistent `_objectstack_sequences` table), the engine must NOT
13+
* pre-fill `autonumber` fields with its in-memory counter — it defers to the
14+
* driver so the persistent sequence is the single source of truth. When the
15+
* driver does NOT advertise it (memory / mongodb), the engine keeps its
16+
* in-memory fallback so nothing regresses.
17+
*
18+
* A `required` autonumber must pass insert-validation in BOTH cases (the value
19+
* is runtime-owned, assigned after validation in the native-driver case).
20+
*/
21+
vi.mock('./registry', () => {
22+
const instance: any = {
23+
getObject: vi.fn(),
24+
resolveObject: vi.fn((n: string) => instance.getObject(n)),
25+
registerObject: vi.fn(),
26+
getObjectOwner: vi.fn(),
27+
registerNamespace: vi.fn(),
28+
registerKind: vi.fn(),
29+
registerItem: vi.fn(),
30+
registerApp: vi.fn(),
31+
installPackage: vi.fn(),
32+
reset: vi.fn(),
33+
metadata: { get: vi.fn(() => new Map()) },
34+
};
35+
function SchemaRegistry() {
36+
return instance;
37+
}
38+
Object.assign(SchemaRegistry, instance);
39+
return {
40+
SchemaRegistry,
41+
computeFQN: (_ns: string | undefined, name: string) => name,
42+
parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }),
43+
RESERVED_NAMESPACES: new Set(['base', 'system']),
44+
};
45+
});
46+
47+
function makeDriver(supportsAutonumber: boolean): IDataDriver & { created: any[] } {
48+
const created: any[] = [];
49+
const driver: any = {
50+
name: supportsAutonumber ? 'sql' : 'memory',
51+
supports: supportsAutonumber ? { autonumber: true } : {},
52+
connect: vi.fn().mockResolvedValue(undefined),
53+
disconnect: vi.fn().mockResolvedValue(undefined),
54+
// seedAutonumber (fallback path) reads existing rows; none exist yet.
55+
find: vi.fn().mockResolvedValue([]),
56+
findOne: vi.fn(),
57+
create: vi.fn(async (_obj: string, row: any) => {
58+
created.push(row);
59+
// Native driver assigns the autonumber itself (post-validation).
60+
return supportsAutonumber ? { id: 'r1', ...row, doc_no: 'D-0001' } : { id: 'r1', ...row };
61+
}),
62+
update: vi.fn(),
63+
delete: vi.fn(),
64+
count: vi.fn(),
65+
};
66+
driver.created = created;
67+
return driver as any;
68+
}
69+
70+
const DOC_SCHEMA = {
71+
name: 'doc',
72+
fields: {
73+
title: { type: 'text' },
74+
doc_no: { type: 'autonumber', required: true, format: 'D-{0000}' },
75+
},
76+
};
77+
78+
describe('ObjectQL autonumber ownership (#1603)', () => {
79+
let engine: ObjectQL;
80+
81+
beforeEach(() => {
82+
vi.clearAllMocks();
83+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(DOC_SCHEMA as any);
84+
engine = new ObjectQL();
85+
});
86+
87+
it('defers to a native-autonumber driver (does NOT pre-fill) and passes required-validation', async () => {
88+
const driver = makeDriver(true);
89+
engine.registerDriver(driver, true);
90+
await engine.init();
91+
92+
const result = await engine.insert('doc', { title: 'Spec' });
93+
94+
// Engine did NOT generate the value — the row handed to the driver has no doc_no.
95+
expect(driver.created).toHaveLength(1);
96+
expect(driver.created[0].doc_no).toBeUndefined();
97+
// Engine never scanned for a max via the fallback seed path.
98+
expect(driver.find).not.toHaveBeenCalled();
99+
// The driver's value comes back to the caller.
100+
expect(result.doc_no).toBe('D-0001');
101+
});
102+
103+
it('falls back to engine generation for a driver without native autonumber', async () => {
104+
const driver = makeDriver(false);
105+
engine.registerDriver(driver, true);
106+
await engine.init();
107+
108+
const result = await engine.insert('doc', { title: 'Spec' });
109+
110+
// Engine pre-filled the value before calling the driver.
111+
expect(driver.created).toHaveLength(1);
112+
expect(driver.created[0].doc_no).toBe('D-0001');
113+
expect(result.doc_no).toBe('D-0001');
114+
});
115+
});

‎packages/objectql/src/engine.ts‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -719,21 +719,27 @@ export class ObjectQL implements IDataEngine {
719719
}
720720

721721
/**
722-
* Generate values for empty `autonumber` fields on insert. Runs BEFORE
723-
* required-validation so a `required` autonumber (e.g. a record number) is
724-
* never rejected for "missing" — the runtime owns the value, not the client.
722+
* Generate values for empty `autonumber` fields on insert — ONLY for drivers
723+
* that do not generate them natively (memory, mongodb). For SQL-backed objects
724+
* the driver owns a persistent, atomic `_objectstack_sequences` table and
725+
* advertises `supports.autonumber === true`; the engine then defers entirely
726+
* and never pre-fills (so the persistent sequence is the single source of
727+
* truth — see #1603). Required-validation exempts `autonumber` either way, so
728+
* a `required` record number is never rejected for "missing" — the runtime
729+
* owns the value, not the client.
725730
*
726-
* The next value is `max(existing) + 1`, seeded once per `object.field` from
727-
* the store then incremented in memory (monotonic within the process,
728-
* resilient to deletions). `autonumberFormat` is honored, e.g.
729-
* `CASE-{0000}` → `CASE-0042`. NOTE: in-memory seeding is single-instance;
730-
* a persistent sequence store is a follow-up for multi-instance setups.
731+
* In the fallback path the next value is `max(existing) + 1`, seeded once per
732+
* `object.field` from the store then incremented in memory (monotonic within
733+
* the process, resilient to deletions). `autonumberFormat` is honored, e.g.
734+
* `CASE-{0000}` → `CASE-0042`. NOTE: this in-memory seeding is single-instance.
731735
*/
732736
private async applyAutonumbers(
733737
object: string,
734738
record: Record<string, unknown>,
735739
execCtx?: ExecutionContext,
740+
driverOwnsAutonumber?: boolean,
736741
): Promise<void> {
742+
if (driverOwnsAutonumber) return; // driver generates persistently in create()
737743
const fields = (this.getSchema(object) as any)?.fields;
738744
if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return;
739745
for (const [name, def] of Object.entries(fields)) {
@@ -745,7 +751,10 @@ export class ObjectQL implements IDataEngine {
745751
if (next == null) next = await this.seedAutonumber(object, name, execCtx);
746752
next += 1;
747753
this.autonumberCounters.set(key, next);
748-
record[name] = this.formatAutonumber((def as any).autonumberFormat, next);
754+
// Honor either the spec-canonical `autonumberFormat` or the shorthand
755+
// `format` (both appear in metadata; the driver reads both too) — #1603.
756+
const fmt = (def as any).autonumberFormat ?? (def as any).format;
757+
record[name] = this.formatAutonumber(fmt, next);
749758
}
750759
}
751760

@@ -1834,13 +1843,16 @@ export class ObjectQL implements IDataEngine {
18341843
let result;
18351844
const nowSnap = new Date();
18361845
const schemaForValidation = this._registry.getObject(object);
1846+
// When the driver generates autonumbers natively (persistent SQL
1847+
// sequence), the engine defers to it — see #1603.
1848+
const driverOwnsAutonumber = (driver as any)?.supports?.autonumber === true;
18371849
if (Array.isArray(hookContext.input.data)) {
18381850
// Bulk Create — apply defaults per row
18391851
const rows = (hookContext.input.data as any[]).map((row) =>
18401852
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
18411853
);
18421854
for (const r of rows) {
1843-
await this.applyAutonumbers(object, r as Record<string, unknown>, opCtx.context);
1855+
await this.applyAutonumbers(object, r as Record<string, unknown>, opCtx.context, driverOwnsAutonumber);
18441856
}
18451857
for (const r of rows) {
18461858
await this.encryptSecretFields(object, r, opCtx.context, hookContext.input.options);
@@ -1862,7 +1874,7 @@ export class ObjectQL implements IDataEngine {
18621874
opCtx.context,
18631875
nowSnap,
18641876
);
1865-
await this.applyAutonumbers(object, row, opCtx.context);
1877+
await this.applyAutonumbers(object, row, opCtx.context, driverOwnsAutonumber);
18661878
await this.encryptSecretFields(object, row, opCtx.context, hookContext.input.options);
18671879
validateRecord(schemaForValidation, row, 'insert');
18681880
evaluateValidationRules(schemaForValidation as any, row, 'insert', { logger: this.logger });
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { validateRecord } from './record-validator.js';
5+
6+
/**
7+
* Required-field validation, with the autonumber exemption (#1603).
8+
*
9+
* `autonumber` values are runtime-owned — the SQL driver assigns them from a
10+
* persistent sequence AFTER record-validation runs — so a missing value on an
11+
* insert must NOT be reported as a client "required" error.
12+
*/
13+
describe('validateRecord — required + autonumber exemption', () => {
14+
const schema = {
15+
fields: {
16+
title: { type: 'text', required: true },
17+
record_no: { type: 'autonumber', required: true, format: 'REC-{0000}' },
18+
},
19+
};
20+
21+
it('does NOT reject a missing required autonumber on insert', () => {
22+
// title supplied, record_no omitted → only the autonumber is missing.
23+
expect(() => validateRecord(schema, { title: 'Hello' }, 'insert')).not.toThrow();
24+
});
25+
26+
it('still rejects a missing required NON-autonumber field on insert', () => {
27+
expect(() => validateRecord(schema, { record_no: 'REC-0001' }, 'insert')).toThrow(/title/i);
28+
});
29+
30+
it('accepts an explicitly-provided autonumber value', () => {
31+
expect(() =>
32+
validateRecord(schema, { title: 'Hello', record_no: 'REC-0042' }, 'insert'),
33+
).not.toThrow();
34+
});
35+
});

‎packages/objectql/src/validation/record-validator.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ function optionValues(options: FieldDef['options']): string[] {
111111

112112
function validateOne(name: string, def: FieldDef, value: unknown): FieldValidationError | null {
113113
// ── required ────────────────────────────────────────────────────
114-
if (def.required && isMissing(value)) {
114+
// `autonumber` is runtime-owned: the value is generated by the engine /
115+
// driver (the SQL driver assigns it from a persistent sequence AFTER this
116+
// validation runs), so a missing value is never a client error — see #1603.
117+
if (def.required && isMissing(value) && def.type !== 'autonumber') {
115118
return { field: name, code: 'required', message: `${name} is required` };
116119
}
117120
if (isMissing(value)) return null; // nothing else to check

‎packages/plugins/driver-sql/src/sql-driver-autonumber.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,53 @@ describe('SqlDriver auto_number sequence', () => {
170170
expect(r.case_number).toBe('CASE-001');
171171
});
172172

173+
it('fills autonumber fields on bulkCreate (engine defers to the driver — #1603)', async () => {
174+
await driver.initObjects([
175+
{
176+
name: 'contract',
177+
fields: {
178+
organization_id: { type: 'string' },
179+
contract_number: { type: 'autonumber', format: 'CTR-{0000}' },
180+
name: { type: 'string' },
181+
},
182+
},
183+
]);
184+
185+
const rows = await driver.bulkCreate('contract', [
186+
{ organization_id: 'org_bulk', name: 'B1' },
187+
{ organization_id: 'org_bulk', name: 'B2' },
188+
{ organization_id: 'org_bulk', name: 'B3' },
189+
]);
190+
191+
const numbers = (rows as any[]).map((r) => r.contract_number).sort();
192+
expect(numbers).toEqual(['CTR-0001', 'CTR-0002', 'CTR-0003']);
193+
194+
// A subsequent single create continues the same persistent sequence.
195+
const next = await driver.create('contract', { organization_id: 'org_bulk', name: 'B4' });
196+
expect(next.contract_number).toBe('CTR-0004');
197+
});
198+
199+
it('advertises native autonumber support so the engine defers to it', () => {
200+
expect(driver.supports.autonumber).toBe(true);
201+
});
202+
203+
it('honors the spec-canonical `autonumberFormat` key as well as `format`', async () => {
204+
await driver.initObjects([
205+
{
206+
name: 'ticket',
207+
fields: {
208+
// spec-canonical key (field.zod.ts) rather than the `format` shorthand
209+
ticket_no: { type: 'autonumber', autonumberFormat: 'TK-{00000}' },
210+
},
211+
},
212+
]);
213+
214+
const r1 = await driver.create('ticket', {});
215+
const r2 = await driver.create('ticket', {});
216+
expect(r1.ticket_no).toBe('TK-00001');
217+
expect(r2.ticket_no).toBe('TK-00002');
218+
});
219+
173220
it('falls back to options.tenantId when the row has no tenant field value', async () => {
174221
await driver.initObjects([
175222
{

‎packages/plugins/driver-sql/src/sql-driver.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ export class SqlDriver implements IDataDriver {
130130
jsonFields: true,
131131
arrayFields: true,
132132
vectorSearch: false,
133+
// Persistent, atomic autonumber sequences via `_objectstack_sequences`
134+
// (see fillAutoNumberFields / getNextSequenceValue). The engine defers
135+
// autonumber generation to this driver — it is the single source of truth.
136+
autonumber: true,
133137

134138
// Schema Management
135139
schemaSync: true,
@@ -702,7 +706,12 @@ export class SqlDriver implements IDataDriver {
702706
async bulkCreate(object: string, data: any[], options?: DriverOptions): Promise<any> {
703707
this.auditMissingTenant(object, 'bulkCreate', options);
704708
for (const row of data) {
705-
if (row && typeof row === 'object') this.injectTenantOnInsert(object, row, options);
709+
if (row && typeof row === 'object') {
710+
this.injectTenantOnInsert(object, row, options);
711+
// Reserve a persistent sequence value for each row's autonumber
712+
// field(s) — the engine no longer pre-fills these (see #1603).
713+
await this.fillAutoNumberFields(object, row, options);
714+
}
706715
}
707716
const builder = this.getBuilder(object, options);
708717
return await builder.insert(data).returning('*');
@@ -1068,9 +1077,12 @@ export class SqlDriver implements IDataDriver {
10681077
(this.datetimeFields[tableName] ??= new Set()).add(name);
10691078
}
10701079
if (type === 'auto_number' || type === 'autonumber') {
1071-
const fmt = typeof field.format === 'string' && field.format
1072-
? field.format
1073-
: '{0000}';
1080+
// Honor either the spec-canonical `autonumberFormat` or the
1081+
// shorthand `format` (both appear in metadata) — see #1603.
1082+
const rawFmt = (typeof field.autonumberFormat === 'string' && field.autonumberFormat)
1083+
? field.autonumberFormat
1084+
: (typeof field.format === 'string' && field.format ? field.format : '');
1085+
const fmt = rawFmt || '{0000}';
10741086
const m = fmt.match(/\{(0+)\}/);
10751087
const padWidth = m ? m[1].length : 4;
10761088
const prefix = m ? fmt.slice(0, m.index ?? 0) : fmt;

‎packages/spec/src/data/driver.zod.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,22 @@ export const DriverCapabilitiesSchema = lazySchema(() => z.object({
227227
*/
228228
vectorSearch: z.boolean().default(false).describe('Supports vector embeddings and similarity search'),
229229

230+
/**
231+
* Whether the driver natively generates persistent autonumber / sequence
232+
* values inside `create()` / `bulkCreate()` / `upsert()` (e.g. a DB-backed
233+
* `_objectstack_sequences` table that survives restarts and is atomic across
234+
* concurrent writers / instances).
235+
*
236+
* When true, the ObjectQL engine MUST NOT pre-fill `autonumber` fields with
237+
* its own in-memory counter — the driver owns generation as the single,
238+
* persistent source of truth, and required-validation exempts the field
239+
* because the value is assigned after validation. When false/absent (memory,
240+
* mongodb), the engine falls back to its in-memory generator.
241+
*
242+
* Optional so existing driver capability objects need not be updated.
243+
*/
244+
autonumber: z.boolean().optional().describe('Driver natively generates persistent autonumber/sequence values'),
245+
230246
// ============================================================================
231247
// Schema Management
232248
// ============================================================================

0 commit comments

Comments
 (0)