Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/data-driver-query-omit-object.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"@objectstack/spec": major
---

refactor(spec)!: `IDataDriver` 的 query 参数改为 `DriverQuery`(`Omit<QueryAST, 'object'>`),对象名只写一遍 (#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` 导出、看不见参数类型的收窄(它记录导出存在与否,不记录签名),所以这条迁移说明是该变更唯一的下游载体。
7 changes: 1 addition & 6 deletions packages/metadata/src/utils/history-cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
});
Expand All @@ -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'],
Expand Down Expand Up @@ -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<string, unknown>) => r.id as string).filter(Boolean);
return this.bulkDeleteByIds(driver, table, ids);
}
Expand Down Expand Up @@ -270,15 +268,13 @@ export class HistoryCleanupManager {
}

recordsByAge = await driver.count(historyTableName, {
object: historyTableName,
where: filter,
});
}

// 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'],
});
Expand All @@ -297,7 +293,6 @@ export class HistoryCleanupManager {
const filter: Record<string, unknown> = { type, name, ...baseWhere };

const count = await driver.count(historyTableName, {
object: historyTableName,
where: filter,
});

Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface/contracts.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"DelegableAdminScope (interface)",
"DelegableScope (interface)",
"DeployExecutionResult (interface)",
"DriverQuery (type)",
"EMBEDDER_SERVICE (const)",
"EmailAddress (type)",
"EmailAttachment (interface)",
Expand Down
122 changes: 121 additions & 1 deletion packages/spec/src/contracts/data-driver.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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<T> = 'object' extends keyof T ? never : 'dropped';
const perMethod: [
DropsObject<Parameters<IDataDriver['find']>[1]>,
DropsObject<Parameters<IDataDriver['findOne']>[1]>,
DropsObject<NonNullable<Parameters<IDataDriver['count']>[1]>>,
DropsObject<Parameters<NonNullable<IDataDriver['updateMany']>>[1]>,
DropsObject<Parameters<NonNullable<IDataDriver['deleteMany']>>[1]>,
DropsObject<Parameters<NonNullable<IDataDriver['explain']>>[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<IDataDriver, 'find' | 'count'> = {
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();
});
});
});
48 changes: 42 additions & 6 deletions packages/spec/src/contracts/data-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryAST, 'object'>;

/**
* IDataDriver - Comprehensive Database Driver Interface
*
Expand Down Expand Up @@ -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<Record<string, unknown>[]>;
find(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>;

// `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
Expand All @@ -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<Record<string, unknown> | null>;
findOne(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown> | null>;

/**
* Create a new record.
Expand Down Expand Up @@ -156,7 +192,7 @@ export interface IDataDriver {
/**
* Count records matching a query.
*/
count(object: string, query?: QueryAST, options?: DriverOptions): Promise<number>;
count(object: string, query?: DriverQuery, options?: DriverOptions): Promise<number>;

// ===========================================================================
// Bulk Operations
Expand All @@ -172,10 +208,10 @@ export interface IDataDriver {
bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void>;

/** Update multiple records matching a query (optional) */
updateMany?(object: string, query: QueryAST, data: Record<string, unknown>, options?: DriverOptions): Promise<number>;
updateMany?(object: string, query: DriverQuery, data: Record<string, unknown>, options?: DriverOptions): Promise<number>;

/** Delete multiple records matching a query (optional) */
deleteMany?(object: string, query: QueryAST, options?: DriverOptions): Promise<number>;
deleteMany?(object: string, query: DriverQuery, options?: DriverOptions): Promise<number>;

// ===========================================================================
// Temporal Storage Convention (ADR-0053 D-A1/D-A2, #3912)
Expand Down Expand Up @@ -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<unknown>;
explain?(object: string, query: DriverQuery, options?: DriverOptions): Promise<unknown>;
}
Loading