Skip to content

Commit 224efb1

Browse files
committed
fix(driver-sql): declare the indexes key initObjects / registerObjectMetadata already read
Both entry points took `Array<{ name; fields?; tenancy? }>` with no `indexes`, while `registerManagedObjectMetadata` read the key out of those very objects through an `(obj as any)` cast and filled `managedObjectIndexes` from it — the map `syncDeclaredIndexes` renders every declared UNIQUE from. The sibling `detectManagedDrift` on the same class already declared `indexes?: any[]`, so the two halves of one class disagreed about the shape of the same input. This is the shape #4311 fixed for `tenancy`, one key over. Nothing tripped over it because TypeScript's excess-property check fires on a fresh object literal and not on one bound to a variable first, and every caller in the package happened to bind first — a green that held for a reason unrelated to correctness. - Add `indexes?: any[]` to `registerObjectMetadata`, `initObjects` and the shared `registerManagedObjectMetadata` helper, spelled as `detectManagedDrift` spells it. - Delete the `as any` at the `managedObjectIndexes.set` read site: the cast was the evidence that the declaration and the read disagreed. - Pin the fresh-object-literal form, which is the only form that can go red on this defect; a variable-bound call measures nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
1 parent 2e6a2ea commit 224efb1

3 files changed

Lines changed: 172 additions & 5 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
`SqlDriver.initObjects()` and `SqlDriver.registerObjectMetadata()` now declare the `indexes` key they have always read.
6+
7+
Both entry points took `Array<{ name; fields?; tenancy? }>`, with no `indexes` in the type. The key was read out of those very objects one call deep anyway, through an `as any`, in `registerManagedObjectMetadata` — and the map it fills, `managedObjectIndexes`, is what `syncDeclaredIndexes` renders every declared UNIQUE from. So the driver's whole index-sync path was driven by a key its own signature said did not exist, while the sibling `detectManagedDrift` on the same class had always declared `indexes?: any[]`: the two halves of one class disagreed about the shape of the same input.
8+
9+
That is the shape #4311 already fixed for `tenancy`, one key over, and the comment #4311 left above `initObjects` described `indexes` word for word.
10+
11+
**Why nothing tripped over it.** TypeScript's excess-property check fires on a fresh object literal and not on one bound to a variable first, so the same object was accepted or rejected by nothing but where it was spelled — `await driver.initObjects([{ ...bare, indexes: [] }])` was rejected with TS2353, `const o = { ...bare, indexes: [] }; await driver.initObjects([o])` was accepted, and the index was synced either way. Every caller happened to bind first, so the package typechecked green for a reason unrelated to correctness.
12+
13+
**Why this matters beyond a compile error.** The loud symptom was a rejected correct call. The quiet one is the reachable branch: an author — or an AI — reading the signature concludes `indexes` is not accepted and drops the key, and a declared UNIQUE is then never synced, with no error at authoring time and no error at boot. The schema says those rows cannot collide; they can.
14+
15+
What changed, all inside `SqlDriver`:
16+
17+
- `registerObjectMetadata(objects)`, `initObjects(objects)` and the shared `registerManagedObjectMetadata(obj)` helper each gained `indexes?: any[]`, spelled exactly as `detectManagedDrift` already spells it.
18+
- The `(obj as any)` cast at the `managedObjectIndexes.set` read site is gone. The cast was the evidence that the declaration and the read disagreed; leaving it would have fixed the signature while keeping the "the type does not admit me but I read it anyway" path alive.
19+
20+
This relaxes a driver-local narrowing back toward the contract it implements — `IDataDriver.registerObjectMetadata?(schemas: unknown[])` in `@objectstack/spec` accepts `unknown[]`, and `SqlDriver` narrowed it on its own — so it is not a widening of the protocol. No call that compiles today stops compiling: the parameter type only gained an optional key.
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16570] `initObjects` and `registerObjectMetadata` accept `indexes` — the
5+
* key they have always READ — spelled as a **fresh object literal**.
6+
*
7+
* ## The defect this pins
8+
*
9+
* Both entry points declared `Array<{ name; fields?; tenancy? }>`, with no
10+
* `indexes`. The key was read one call deep anyway, through an `as any`, in
11+
* `registerManagedObjectMetadata`:
12+
*
13+
* ```ts
14+
* this.managedObjectIndexes.set(tableName, (obj as any).indexes);
15+
* ```
16+
*
17+
* and `managedObjectIndexes` is what `syncDeclaredIndexes` renders every
18+
* declared UNIQUE from — so the driver's whole index-sync path was driven by a
19+
* key its own signature said did not exist. `detectManagedDrift`, on the same
20+
* class, had always declared `indexes?: any[]`: the two halves of one class
21+
* disagreed about the shape of the same input. That is the shape #4311 already
22+
* fixed for `tenancy`, and the comment it left above `initObjects` described
23+
* `indexes` word for word.
24+
*
25+
* ## Why the FORM of this pin is the whole point
26+
*
27+
* TypeScript's excess-property check fires on a **fresh object literal** and
28+
* not on one bound to a variable first, so the same object was accepted or
29+
* rejected by where it was spelled:
30+
*
31+
* ```ts
32+
* await driver.initObjects([{ ...bare, indexes: [] }]); // TS2353
33+
* const withoutIndex = { ...bare, indexes: [] };
34+
* await driver.initObjects([withoutIndex]); // accepted
35+
* ```
36+
*
37+
* Every existing caller in this package happened to bind first — one of them
38+
* (`sql-driver-11794-richtext-text-family.test.ts`) even wrote the workaround
39+
* down: *"Hoisted (not an inline literal) … `indexes` rides through
40+
* `initObjects` beyond its narrow parameter type"*. So the package typechecked
41+
* green for a reason unrelated to correctness, and a **variable-bound pin
42+
* cannot go red on this defect** — it measures nothing. Every call below is
43+
* therefore an inline literal in argument position, which is what makes
44+
* `tsc --noEmit` (this package's `typecheck` script) the instrument that
45+
* measures it: revert either signature and these lines stop compiling with
46+
*
47+
* TS2353: Object literal may only specify known properties, and 'indexes'
48+
* does not exist in type '{ name: string; fields?: Record<string, any>
49+
* | undefined; tenancy?: any; }'.
50+
*
51+
* The runtime assertions are the other half: they prove the key is not merely
52+
* *admitted* by the type but still *read* — recorded in `managedObjectIndexes`
53+
* (§1), rendered into a physical UNIQUE (§2), and cleared when withdrawn (§3).
54+
* A signature relaxation that quietly stopped reading the key would pass the
55+
* compile leg alone.
56+
*
57+
* Runs on the always-available in-memory SQLite cell: the defect is in a
58+
* parameter type and in the registry it feeds, neither of which is
59+
* dialect-specific.
60+
*/
61+
62+
import { describe, it, expect, afterEach } from 'vitest';
63+
import { SqlDriver } from './sql-driver.js';
64+
import { dialectCell } from './live-dialect-matrix.testkit.js';
65+
66+
const SQLITE = dialectCell('sqlite');
67+
68+
/**
69+
* The un-indexed base object, deliberately WITHOUT `indexes` — every call site
70+
* below spreads it and writes `indexes` inline, so the literal being checked is
71+
* fresh in argument position. `tenancy: { enabled: false }` keeps the declared
72+
* UNIQUE on the plain (non-tenant-scoped) path, and the bounded `maxLength`
73+
* keeps `v` a keyable varchar rather than an unbounded TEXT.
74+
*/
75+
const bareObject = (name: string) => ({
76+
name,
77+
tenancy: { enabled: false },
78+
fields: { v: { type: 'text', maxLength: 64 } },
79+
});
80+
81+
/** What the driver recorded for `table`, read off the protected registry. */
82+
const recordedIndexes = (driver: SqlDriver, table: string): unknown =>
83+
(driver as unknown as { managedObjectIndexes: Map<string, unknown> }).managedObjectIndexes.get(table);
84+
85+
describe('initObjects / registerObjectMetadata accept `indexes` as a fresh object literal (#16570)', () => {
86+
let driver: SqlDriver | undefined;
87+
afterEach(async () => {
88+
await driver?.disconnect().catch(() => {});
89+
driver = undefined;
90+
});
91+
92+
it('§1 registerObjectMetadata: the inline literal compiles AND the key is recorded', async () => {
93+
const T = 'os16570_register';
94+
driver = new SqlDriver(SQLITE.config());
95+
const declared = [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }];
96+
97+
// Fresh literal in argument position — not hoisted to a variable first.
98+
driver.registerObjectMetadata([{ ...bareObject(T), indexes: declared }]);
99+
100+
expect(recordedIndexes(driver, T)).toEqual(declared);
101+
});
102+
103+
it('§2 initObjects: the inline literal compiles AND the declared UNIQUE is physically synced', async () => {
104+
const T = 'os16570_init';
105+
driver = new SqlDriver(SQLITE.config());
106+
107+
// Fresh literal in argument position.
108+
await driver.initObjects([
109+
{ ...bareObject(T), indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }] },
110+
]);
111+
112+
const knex = (driver as unknown as { knex: (t: string) => any }).knex;
113+
await knex(T).insert({ id: 'a', v: 'same' });
114+
// If `indexes` had been dropped at authoring time — the silent failure mode
115+
// this card is about — this second row would be accepted.
116+
await expect(knex(T).insert({ id: 'b', v: 'same' })).rejects.toThrow();
117+
});
118+
119+
it('§3 initObjects: the exact `{ ...bare, indexes: [] }` spelling from the card compiles, and withdraws the entry', async () => {
120+
const T = 'os16570_withdraw';
121+
driver = new SqlDriver(SQLITE.config());
122+
const bare = bareObject(T);
123+
124+
await driver.initObjects([{ ...bare, indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }] }]);
125+
expect(recordedIndexes(driver, T)).toHaveLength(1);
126+
127+
// The spelling named in the card, verbatim: an empty array must CLEAR the
128+
// entry, not leave the previous one standing.
129+
await driver.initObjects([{ ...bare, indexes: [] }]);
130+
expect(recordedIndexes(driver, T)).toEqual([]);
131+
});
132+
});

packages/drivers/driver-sql/src/sql-driver.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9599,7 +9599,7 @@ export class SqlDriver implements IDataDriver {
95999599
* which `initObjects` goes on to use for its DDL.
96009600
*/
96019601
protected registerManagedObjectMetadata(
9602-
obj: { name: string; fields?: Record<string, any>; tenancy?: any },
9602+
obj: { name: string; fields?: Record<string, any>; tenancy?: any; indexes?: any[] },
96039603
): { tableName: string; tenantField: string | null } {
96049604
const tableName = StorageNameMapping.resolveTableName(obj);
96059605
// #2186: remember the authoritative metadata field set for this table so
@@ -9608,8 +9608,8 @@ export class SqlDriver implements IDataDriver {
96089608
// Always overwrite — a metadata change that REMOVES `indexes` must clear
96099609
// the previous entry, or drift detection keeps expecting an index nobody
96109610
// declares any more (and never reports it as orphaned).
9611-
if (Array.isArray((obj as any).indexes)) {
9612-
this.managedObjectIndexes.set(tableName, (obj as any).indexes);
9611+
if (Array.isArray(obj.indexes)) {
9612+
this.managedObjectIndexes.set(tableName, obj.indexes);
96139613
} else {
96149614
this.managedObjectIndexes.delete(tableName);
96159615
}
@@ -9729,7 +9729,7 @@ export class SqlDriver implements IDataDriver {
97299729
* Idempotent: pure metadata assignment, safe to re-drive on every reload.
97309730
*/
97319731
registerObjectMetadata(
9732-
objects: Array<{ name: string; fields?: Record<string, any>; tenancy?: any }>,
9732+
objects: Array<{ name: string; fields?: Record<string, any>; tenancy?: any; indexes?: any[] }>,
97339733
): void {
97349734
for (const obj of objects) this.registerManagedObjectMetadata(obj);
97359735
}
@@ -9740,7 +9740,22 @@ export class SqlDriver implements IDataDriver {
97409740
// undeclared here until #4311 (`registerExternalObject` and
97419741
// `computeAndRecordTenantField` both had it), so a caller spelling the key
97429742
// correctly was rejected by the type while the driver read it regardless.
9743-
async initObjects(objects: Array<{ name: string; fields?: Record<string, any>; tenancy?: any }>): Promise<void> {
9743+
//
9744+
// `indexes` is the same story, one key over, and it went undeclared here for
9745+
// longer: `registerManagedObjectMetadata` fills `managedObjectIndexes` from
9746+
// it, and that map is what `syncDeclaredIndexes` renders every declared
9747+
// UNIQUE from — so the whole index-sync path was driven by a key this
9748+
// signature said did not exist, reached through an `as any`. The sibling
9749+
// `detectManagedDrift` on this class had always declared it, so the two
9750+
// halves disagreed about the shape of the same input. Nothing tripped over
9751+
// it because TypeScript's excess-property check fires on a FRESH object
9752+
// literal and not on one bound to a variable first, and every caller here
9753+
// happened to bind first — a green that held for a reason unrelated to
9754+
// correctness. `src/sql-driver-16570-init-objects-indexes-param.test.ts`
9755+
// pins the fresh-literal form so it cannot silently go back.
9756+
async initObjects(
9757+
objects: Array<{ name: string; fields?: Record<string, any>; tenancy?: any; indexes?: any[] }>,
9758+
): Promise<void> {
97449759
// In-memory registration FIRST, and deliberately ahead of the DDL gate
97459760
// below: being refused permission to alter a schema is not a reason to stay
97469761
// ignorant of the objects we were just told about. On a datasource we are a

0 commit comments

Comments
 (0)