diff --git a/.changeset/data-driver-query-omit-object.md b/.changeset/data-driver-query-omit-object.md new file mode 100644 index 0000000000..5556b45409 --- /dev/null +++ b/.changeset/data-driver-query-omit-object.md @@ -0,0 +1,29 @@ +--- +"@objectstack/spec": major +--- + +refactor(spec)!: `IDataDriver` 的 query 参数改为 `DriverQuery`（`Omit`），对象名只写一遍 (#5181) + +`IDataDriver.find/findOne/count/updateMany/deleteMany/explain` 的第一个实参已经是对象名，而它们要求的 `QueryAST` 又把 `object` 列为必填 —— 同一个事实被要求写两遍，并因此有了两处互相矛盾的余地。上层为这份歧义已经付过账：objectql 引擎刻意把键序写成 `{ ...query, object }`，好让一个夹带的 `query.object` 覆盖不掉已解析的名字；wire 层则用一条具名 400（`QUERY_OBJECT_MISMATCH`）拒绝不一致。 + +驱动这一侧付的账是**成片的 cast**：一个手上只有 `where` 的直接调用方叫不出这个类型的名字，于是 `as any`，连带把 `where`/`orderBy`/`fields` 的类型检查一起关掉（cloud#1053 实测 20 处；cloud#1030 的 `$like` 就是从这个口子活到运行时的）。 + +**FROM → TO** + +```ts +// FROM —— 对象名写两遍 +await driver.find('account', { object: 'account', where: { status: 'open' } }); +// TO —— 第一个实参就是对象名 +await driver.find('account', { where: { status: 'open' } }); +``` + +一行修复：**删掉驱动调用字面量里的 `object:` 键**。编译器会把每一处指出来（TS2353 `'object' does not exist in type 'DriverQuery'`）。 + +**两个方向的兼容性，都不强迫任何一侧动** + +- **调用方**：手上是一个 `QueryAST` **值**的，原样传即可 —— 它具备 `DriverQuery` 要求的全部属性，多出来的那个在非新鲜字面量上 TypeScript 一律接受。新被拒绝的**恰好只是冗余本身**：写在调用点上、拼出 `object` 的内联字面量。本仓的迁移面因此实测只有 1 个文件 6 处（`@objectstack/metadata` 的 history-cleanup），已在同一 PR 里删除；引擎的 `driver.find(object, ast, …)` 一个字都不用改。 +- **驱动实现**：仍旧声明 `query: QueryAST` 的实现继续编译 —— 方法参数按双变比较。它们不再可以做的是**读 `query.object`**：调用方现在有权省略，声明会对一个运行时为 `undefined` 的值说谎。本仓五个驱动（memory / mongodb / sql / sqlite-wasm / turso）实测没有一个读它，因此本次不动驱动代码；把驱动签名一并迁到 `DriverQuery` 是后续的机械收尾。 + +`QueryAST` 的 zod 形状（`data/query.zod.ts` 的 `BaseQuerySchema`）**没有动**：`object` 在引擎与 hook 那一层是被读的，改的只是驱动契约的参数类型。`expand` 条目里的 `object` 同样保留 —— 那里它命名的是**关联对象**，没有任何实参携带这个事实，不是冗余。 + +标 major 是因为这是**源码级破坏性**变更（调用点字面量），运行时行为零变化。注意 `check:api-surface` 只看得见新增的 `DriverQuery` 导出、看不见参数类型的收窄（它记录导出存在与否，不记录签名），所以这条迁移说明是该变更唯一的下游载体。 diff --git a/packages/metadata/src/utils/history-cleanup.ts b/packages/metadata/src/utils/history-cleanup.ts index a9125b2820..3a9981195d 100644 --- a/packages/metadata/src/utils/history-cleanup.ts +++ b/packages/metadata/src/utils/history-cleanup.ts @@ -126,7 +126,6 @@ export class HistoryCleanupManager { if (organizationId) baseWhere.organization_id = organizationId; const metaItems = await driver.find(historyTableName, { - object: historyTableName, where: baseWhere, fields: ['type', 'name'], }); @@ -148,7 +147,6 @@ export class HistoryCleanupManager { try { // Fetch only the IDs of records beyond the retention limit (oldest first) const historyRecords = await driver.find(historyTableName, { - object: historyTableName, where: filter, orderBy: [{ field: 'version', order: 'desc' as const }], fields: ['id'], @@ -192,7 +190,7 @@ export class HistoryCleanupManager { } // Fallback: fetch IDs then delete - const records = await driver.find(table, { object: table, where: filter, fields: ['id'] }); + const records = await driver.find(table, { where: filter, fields: ['id'] }); const ids = records.map((r: Record) => r.id as string).filter(Boolean); return this.bulkDeleteByIds(driver, table, ids); } @@ -270,7 +268,6 @@ export class HistoryCleanupManager { } recordsByAge = await driver.count(historyTableName, { - object: historyTableName, where: filter, }); } @@ -278,7 +275,6 @@ export class HistoryCleanupManager { // Count records that would be deleted by version limit if (this.policy.maxVersions) { const metaItems = await driver.find(historyTableName, { - object: historyTableName, where: baseWhere, fields: ['type', 'name'], }); @@ -297,7 +293,6 @@ export class HistoryCleanupManager { const filter: Record = { type, name, ...baseWhere }; const count = await driver.count(historyTableName, { - object: historyTableName, where: filter, }); diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index aa8a981dfc..e272d6037e 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -66,6 +66,7 @@ "DelegableAdminScope (interface)", "DelegableScope (interface)", "DeployExecutionResult (interface)", + "DriverQuery (type)", "EMBEDDER_SERVICE (const)", "EmailAddress (type)", "EmailAttachment (interface)", diff --git a/packages/spec/src/contracts/data-driver.test.ts b/packages/spec/src/contracts/data-driver.test.ts index 3514aca37e..89b0bbebb3 100644 --- a/packages/spec/src/contracts/data-driver.test.ts +++ b/packages/spec/src/contracts/data-driver.test.ts @@ -1,5 +1,7 @@ import { describe, it, expect } from 'vitest'; -import type { IDataDriver } from './data-driver'; +import type { DriverQuery, IDataDriver } from './data-driver'; +import type { QueryAST } from '../data/query.zod'; +import type { DriverOptions } from '../data/driver.zod'; describe('IDataDriver', () => { it('should allow creating a conforming mock implementation', () => { @@ -143,4 +145,122 @@ describe('IDataDriver', () => { expect('findStream' in legacyShaped).toBe(true); }); }); + + // =========================================================================== + // DriverQuery — the AST no longer repeats the object name (#5181) + // =========================================================================== + // + // Every pin below is resolved by tsc, not by vitest: reverting the change + // (`query: DriverQuery` back to `query: QueryAST`) makes the `@ts-expect-error` + // directives unused, and an unused directive is itself an error, so + // `pnpm --filter @objectstack/spec typecheck` goes red. This file carries no + // entry in `test-typecheck-debt.json`, which is what makes "zero errors" the + // measurable baseline these pins move away from. The `expect()` calls only + // give the assertions a home vitest will run. + + describe('DriverQuery', () => { + it('does not carry `object` at all — argument one is the only spelling', () => { + type ObjectDropped = 'object' extends keyof DriverQuery ? never : 'dropped'; + const dropped: ObjectDropped = 'dropped'; + // Everything else survives: this is a subtraction of one key, not a new dialect. + type WhereKept = 'where' extends keyof DriverQuery ? 'kept' : never; + const kept: WhereKept = 'kept'; + expect([dropped, kept]).toEqual(['dropped', 'kept']); + }); + + it('is what all six query-taking methods actually declare', () => { + // This pin reads the parameter off the CONTRACT rather than off the alias, + // and that is the point: a revert that puts `QueryAST` back on one + // signature while leaving `DriverQuery` defined would sail past every + // alias-scoped assertion in this block. Here that slot resolves to `never` + // and the line goes red — per method, so the message names which one. + type DropsObject = 'object' extends keyof T ? never : 'dropped'; + const perMethod: [ + DropsObject[1]>, + DropsObject[1]>, + DropsObject[1]>>, + DropsObject>[1]>, + DropsObject>[1]>, + DropsObject>[1]>, + ] = ['dropped', 'dropped', 'dropped', 'dropped', 'dropped', 'dropped']; + expect(perMethod).toHaveLength(6); + }); + + it('lets a caller pass only the query, which is what forced the casts', () => { + // Before #5181 this literal did not compile (`object` was required), so a + // caller holding just a `where` reached for `as any` — and lost the type + // checking on everything else in the same stroke (cloud#1053, 20 sites). + const q: DriverQuery = { where: { status: 'open' }, limit: 10 }; + expect(q.limit).toBe(10); + }); + + it('rejects the redundant object key in a call-site literal', () => { + // The excess-property check is the whole enforcement: writing the object + // name twice is now a compile error rather than a convention nobody could + // enforce. It is also what stops the two spellings from disagreeing — + // the hazard the engine spends a key order on (`{ ...query, object }`) + // and the wire layer spends a 400 on (`QUERY_OBJECT_MISMATCH`). + // @ts-expect-error - 'object' does not exist in type 'DriverQuery' + const redundant: DriverQuery = { object: 'account', where: { status: 'open' } }; + expect(redundant).toBeTruthy(); + }); + + it('still accepts a whole QueryAST value, so existing callers do not move', () => { + // A `QueryAST` variable has every property `DriverQuery` requires and one + // more; TypeScript admits the extra on any value that is not a fresh + // literal. This is why the engine's `driver.find(object, ast, …)` needed + // no edit — only literals written at the call site are re-judged. + const ast: QueryAST = { object: 'account', where: { status: 'open' } }; + const asDriverQuery: DriverQuery = ast; + expect(asDriverQuery.where).toEqual({ status: 'open' }); + }); + + it('keeps `object` inside an expand entry, where it is not redundant', () => { + // The nested value names the RELATED object — a fact no argument carries. + const q: DriverQuery = { + fields: ['title'], + expand: { owner: { object: 'user', fields: ['name'] } }, + }; + expect(q.expand?.owner?.object).toBe('user'); + + // @ts-expect-error - a nested expand entry still requires its own `object` + const missing: DriverQuery = { expand: { owner: { fields: ['name'] } } }; + expect(missing).toBeTruthy(); + }); + + it('keeps an implementation that still declares the full QueryAST', () => { + // Method parameters are compared bivariantly, so a driver written against + // the old signature needs no edit to keep satisfying the contract. What it + // may no longer do is READ `query.object` — callers are free to omit it — + // and no driver in this repository does. + const legacyImplementation: Pick = { + async find(_object: string, _query: QueryAST, _options?: DriverOptions) { + return []; + }, + async count(_object: string, _query?: QueryAST, _options?: DriverOptions) { + return 0; + }, + }; + expect(legacyImplementation.count).toBeDefined(); + }); + + it('recovers the checks a blanket cast switched off — but not all of them', () => { + // What the cast hid and this change gives back: the typed slots. + // `orderBy` is `SortNode[]`, closed since #4721, so the `direction` + // spelling that silently sorted the wrong way is a compile error again. + // @ts-expect-error - spell the direction `order`, never `direction` + const wrongSortKey: DriverQuery = { orderBy: [{ field: 'created_at', direction: 'desc' }] }; + expect(wrongSortKey).toBeTruthy(); + + // What it does NOT give back, stated here so nobody reads more into the + // fix than it delivers: `where` is `FilterCondition`, whose index + // signature is `[key: string]: any` because ANY field name is a legal key. + // An operator the dialect does not have is therefore still not a type + // error — `$like` (cloud#1030) reaches the runtime filter compiler and is + // rejected there, not here. Removing the cast does not close that door; + // only a closed operator vocabulary would, which is a separate change. + const unknownOperator: DriverQuery = { where: { name: { $like: 'acme%' } } }; + expect(unknownOperator.where).toBeTruthy(); + }); + }); }); diff --git a/packages/spec/src/contracts/data-driver.ts b/packages/spec/src/contracts/data-driver.ts index 0600eca20f..e30d88155a 100644 --- a/packages/spec/src/contracts/data-driver.ts +++ b/packages/spec/src/contracts/data-driver.ts @@ -3,6 +3,42 @@ import type { DriverOptions, DriverCapabilities } from '../data/driver.zod.js'; import type { QueryAST } from '../data/query.zod.js'; +/** + * DriverQuery — the query AST as a **driver** receives it: {@link QueryAST} + * minus its top-level `object`. + * + * Every {@link IDataDriver} method that takes a query already takes the object + * name as its FIRST argument, so requiring the AST to carry it again asked the + * caller to state one fact twice — and gave that fact two places to disagree. + * Both layers above the driver already pay for the ambiguity: the engine orders + * its keys deliberately (`{ ...query, object }`, objectql `engine.ts`) so a + * stray `query.object` cannot overwrite the resolved name, and the wire layer + * refuses a mismatch with a named 400 (`QUERY_OBJECT_MISMATCH`, + * metadata-protocol `protocol.ts`). Below the driver boundary the redundancy + * was paid for in blanket casts instead: a direct caller holding only a `where` + * could not name a type for it, reached for `as any`, and lost `where`'s type + * checking along with the object name — which is how an operator the filter + * dialect does not have (`$like`) survived compilation and reached the runtime + * (objectstack#5181, cloud#1053, cloud#1030). + * + * What this deliberately does NOT drop is the `object` inside an `expand` + * entry: those values stay full `QueryAST`, and there the key names the + * RELATED object — a fact no argument carries, so it is not redundant. + * + * Compatibility runs in both directions, and neither side is forced to move: + * - **Callers** may still hand over a whole `QueryAST` value. It carries every + * property this type requires, and TypeScript admits the extra one on any + * value that is not a fresh literal. What is newly rejected is precisely the + * redundancy: a literal written inline at the call site that spells `object`. + * - **Implementations** that still declare `query: QueryAST` keep compiling, + * because method parameters are compared bivariantly. What they may no + * longer do is READ `query.object` — a caller is now free to omit it, so the + * declaration would be lying about a value that is `undefined` at runtime. + * No driver in this repository reads it; the object name arrives as argument + * one, which is the whole point. + */ +export type DriverQuery = Omit; + /** * IDataDriver - Comprehensive Database Driver Interface * @@ -102,7 +138,7 @@ export interface IDataDriver { * imposing an order there would change plan selection for the majority of * reads to buy nothing (objectstack#4363). */ - find(object: string, query: QueryAST, options?: DriverOptions): Promise[]>; + find(object: string, query: DriverQuery, options?: DriverOptions): Promise[]>; // `findStream` was removed in 17.0.0 (#4484, ADR-0049 enforce-or-remove). It was a // REQUIRED method promising reads "optimized for large datasets to avoid memory @@ -128,7 +164,7 @@ export interface IDataDriver { * a deterministic single-row read should be handed an `orderBy`, which is a * thing the caller can express (objectstack#4363). */ - findOne(object: string, query: QueryAST, options?: DriverOptions): Promise | null>; + findOne(object: string, query: DriverQuery, options?: DriverOptions): Promise | null>; /** * Create a new record. @@ -156,7 +192,7 @@ export interface IDataDriver { /** * Count records matching a query. */ - count(object: string, query?: QueryAST, options?: DriverOptions): Promise; + count(object: string, query?: DriverQuery, options?: DriverOptions): Promise; // =========================================================================== // Bulk Operations @@ -172,10 +208,10 @@ export interface IDataDriver { bulkDelete(object: string, ids: Array, options?: DriverOptions): Promise; /** Update multiple records matching a query (optional) */ - updateMany?(object: string, query: QueryAST, data: Record, options?: DriverOptions): Promise; + updateMany?(object: string, query: DriverQuery, data: Record, options?: DriverOptions): Promise; /** Delete multiple records matching a query (optional) */ - deleteMany?(object: string, query: QueryAST, options?: DriverOptions): Promise; + deleteMany?(object: string, query: DriverQuery, options?: DriverOptions): Promise; // =========================================================================== // Temporal Storage Convention (ADR-0053 D-A1/D-A2, #3912) @@ -306,5 +342,5 @@ export interface IDataDriver { * Analyze query performance. * Returns execution plan without executing the query (optional). */ - explain?(object: string, query: QueryAST, options?: DriverOptions): Promise; + explain?(object: string, query: DriverQuery, options?: DriverOptions): Promise; }