diff --git a/.changeset/driver-owned-query-methods-driver-query.md b/.changeset/driver-owned-query-methods-driver-query.md new file mode 100644 index 0000000000..92ce51ee1f --- /dev/null +++ b/.changeset/driver-owned-query-methods-driver-query.md @@ -0,0 +1,47 @@ +--- +"@objectstack/driver-memory": major +"@objectstack/driver-mongodb": major +--- + +refactor(drivers)!: memory / mongodb 的 `aggregate` / `distinct` 也收进 `DriverQuery`，契约没覆盖的方法不再要求把对象名写两遍 (#6212 批 C) + +#6210 的 changeset 结尾专门留了一句：`aggregate` / `distinct` **不在**那次范围内，因为它们不是 `IDataDriver` 收窄的那六个方法。#6212 记下了这笔账，本次结清 memory 与 mongodb 这两个包的部分。 + +这批方法的第一个实参**已经是对象名**，query 里却仍旧要求再写一遍： + +| 位置 | 收窄前 | 收窄后 | +|:--|:--|:--| +| `MongoDBDriver.aggregate` | `query: QueryAST` | `query: DriverQuery` | +| `InMemoryDriver.distinct` | `query?: QueryInput` | `query?: DriverQuery` | +| `InMemoryDriver.aggregate` | `Record[] \| QueryAST` | `Record[] \| DriverQuery` | +| `InMemoryDriver.performAggregation`（私有） | `Omit` | `DriverQuery` | + +因为 `QueryAST` / `QueryInput` 都把 `object` 声明成**必填**，一个手上只有 `where` 的调用方根本叫不出这个类型的名字，于是伸手去拿 `as any` —— 连 `where` / `orderBy` / `limit` 的检查一起关掉。这正是 #5181 记过账的那笔代价（cloud#1053 实测 20 处，cloud#1030 的 `$like` 就是从这个口子活到运行时的）。收窄之后调用方可以直接写字面量： + +```ts +// 收窄前：object 是必填，这句编译不过，于是 ... as any +// 收窄后：直接过，且 where / orderBy / aggregations 逐个受检 +await driver.aggregate('order', { + groupBy: ['region'], + aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }], +}); +``` + +同一次改动收回了 4 处已经多余的 `as any`（memory 2、mongodb 2），`check:query-options-erasure` 的测试面因此从 267 降到 263，baseline 已按门禁要求同 PR `--update`。 + +**`InMemoryDriver.aggregate` 的联合刻意保留。** 两条分支都有活体生产者：mongo 管线数组那支由 `memory-analytics.ts` 喂，AST 那支由 objectql 引擎与 `@objectstack/verify` 的日期分桶探针喂。退役任何一支都会打断其中一条。 + +**顺带把 `#6212` 正文的一处归因证伪了**：正文说 `performAggregation` 当初选 `Omit` 是被 `groupBy` 的元素类型差异逼的。实测 `QueryInput` 与 `QueryAST` 在 `groupBy` 上**逐字相同**，差异只在 `search` / `orderBy` / `expand`；直接换 `DriverQuery` 零报错。所以那不是被迫的选择，契约优先取 `DriverQuery`，不再引入第二个查询类型家族。 + +**零运行时改动。** 非测试改动 100% 是类型注解，无逻辑、无行为、无 emit 差异（`as` 断言在编译期即被抹除）。测试全绿：memory 532、mongodb 206（另 137 条需真实 mongod，按既有 opt-in 规则跳过）。这也是 #5499 冻结面上被允许的处置口径 —— 与 #6210 在同一批驱动上走的是同一条。 + +**迁移面：删掉调用字面量里的 `object:` 键**，与 #5181 / #6210 同一句话，现在覆盖到 `aggregate` / `distinct`。编译器会逐处指出来： + +``` +error TS2353: Object literal may only specify known properties, + and 'object' does not exist in type 'DriverQuery'. +``` + +本仓实测只有一处需要改（`memory-driver.test.ts` 的 `distinct` 用例），且它写的值与第一实参逐字相等，纯冗余。 + +标 major 的依据与 #5181 / #6210 一致：**源码级破坏性**（调用点内联字面量），运行时行为零变化。`check:api-surface` 只记录导出的存在与否、不记录签名，因此这条说明同样是该变更唯一的下游载体。 diff --git a/packages/drivers/driver-memory/src/memory-driver-query-narrowing.test.ts b/packages/drivers/driver-memory/src/memory-driver-query-narrowing.test.ts new file mode 100644 index 0000000000..7d17b7668a --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-driver-query-narrowing.test.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#6212 batch C — `InMemoryDriver`'s three driver-owned query + * methods take `DriverQuery`, not a type that repeats the object name. + * + * `distinct` / `aggregate` / `performAggregation` are NOT declared on + * `IDataDriver`, so #5181's narrowing and #6075's follow-through never reached + * them: their first argument was already the object name while the query type + * (`QueryInput` / `QueryAST`) still *required* `object`, so a caller holding + * only a `where` could not name a type for it and reached for `as any` — + * losing `where`, `orderBy` and `limit` checking in the same stroke + * (cloud#1053, and `$like` surviving to runtime in cloud#1030). + * + * **Why the pins in this file are real.** They are resolved by `tsc`, not by + * vitest: reverting a signature makes the `@ts-expect-error` directives unused, + * and an unused directive is itself an error, so + * `pnpm --filter @objectstack/driver-memory typecheck` goes red. That works + * here only because this package's `tsconfig.json` does NOT exclude the + * test-file glob — it has no `TEST_DEBT` entry in + * `scripts/check-type-check-coverage.mjs` and reports zero errors, which is the + * measurable baseline these pins move away from. The sibling `driver-mongodb` + * package DOES exclude its tests, so the identical pin written there would be + * the phantom check AGENTS.md's `PINS_CHECKED` invariant warns about — it is + * deliberately not written; mongodb's narrowing is held by `tsc` over its + * source plus the repo-wide `check:type-check-debt` re-measure. + * + * The `expect()` calls only give the assertions a home vitest will run. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import type { DriverQuery } from '@objectstack/spec/contracts'; +import { InMemoryDriver } from './memory-driver.js'; + +/** `'dropped'` when `T` does not carry `object` at all; `never` when it does. */ +type DropsObject = 'object' extends keyof T ? never : 'dropped'; + +describe('InMemoryDriver — driver-owned query methods take DriverQuery (#6212 batch C)', () => { + let driver: InMemoryDriver; + const tbl = 'narrowing_probe'; + + beforeEach(async () => { + driver = new InMemoryDriver({ persistence: false }); + await driver.connect(); + }); + + describe('signatures', () => { + it('reads `object` off neither `distinct` nor the AST arm of `aggregate`', () => { + // Read off the METHODS rather than off the `DriverQuery` alias. A revert + // that puts `QueryInput` / `QueryAST` back on one signature while leaving + // the alias imported would sail past any alias-scoped assertion; here that + // slot resolves to `never` and the line goes red, naming which method. + type DistinctQuery = NonNullable[2]>; + // The AST arm is the non-array member of `aggregate`'s union. + type AggregateArg = Parameters[1]; + type AggregateAstArm = Extract; + + const perMethod: [DropsObject, DropsObject] = [ + 'dropped', + 'dropped', + ]; + expect(perMethod).toHaveLength(2); + }); + + it('keeps the mongo-pipeline arm of `aggregate` — BOTH arms have live producers', () => { + // ⛔ Neither arm may be retired. The pipeline arm is fed by + // `memory-analytics.ts` (`this.driver.aggregate(tableName, pipeline)`); + // the AST arm by objectql's engine and `@objectstack/verify`'s + // date-bucket parity probe. This pin fails if the union collapses. + type AggregateArg = Parameters[1]; + type PipelineArmKept = Record[] extends AggregateArg ? 'kept' : never; + const kept: PipelineArmKept = 'kept'; + expect(kept).toBe('kept'); + }); + }); + + describe('what the narrowing gives back', () => { + it('lets `distinct` take a bare `where` — the literal that forced the casts', async () => { + await driver.create(tbl, { id: '1', role: 'admin', active: true }); + await driver.create(tbl, { id: '2', role: 'user', active: false }); + await driver.create(tbl, { id: '3', role: 'user', active: true }); + + // No cast. Before the narrowing `object` was REQUIRED on `QueryInput`, so + // this literal did not compile at all. + const roles = await driver.distinct(tbl, 'role', { where: { active: true } }); + expect(roles.sort()).toEqual(['admin', 'user']); + }); + + it('lets `aggregate` take a bare AST literal, with `where`/`groupBy` still checked', async () => { + await driver.create(tbl, { id: '1', category: 'travel', amount: 100 }); + await driver.create(tbl, { id: '2', category: 'travel', amount: 50 }); + await driver.create(tbl, { id: '3', category: 'meals', amount: 30 }); + + const rows = await driver.aggregate(tbl, { + groupBy: ['category'], + aggregations: [{ function: 'sum', field: 'amount', alias: 'amount' }], + }); + const byCat = Object.fromEntries(rows.map((r: any) => [r.category, r.amount])); + expect(byCat).toEqual({ travel: 150, meals: 30 }); + }); + + it('still runs a real MongoDB pipeline array through Mingo, unchanged', async () => { + await driver.create(tbl, { id: '1', category: 'travel', amount: 100 }); + await driver.create(tbl, { id: '2', category: 'meals', amount: 30 }); + + const rows = await driver.aggregate(tbl, [ + { $match: { category: 'travel' } }, + { $group: { _id: null, total: { $sum: '$amount' } } }, + ]); + expect((rows[0] as any).total).toBe(100); + }); + }); + + // Every pin below sits on a real CALL to the narrowed method, never on a + // `const x: DriverQuery = …` literal. An alias-scoped pin would stay green + // through a revert of the signature — `DriverQuery` lacks `object` whatever + // `distinct` declares — which is the dead-pin shape #5018/#4984 paid for. + describe('what the narrowing now rejects', () => { + it('refuses the redundant `object` key on a `distinct` call-site literal', async () => { + // A caller can pass a typed variable through untouched… + const q: DriverQuery = { where: { active: true } }; + expect(await driver.distinct(tbl, 'role', q)).toEqual([]); + + // …but may no longer state the object name a second time. + await driver.distinct(tbl, 'role', { + // @ts-expect-error - 'object' does not exist in type 'DriverQuery' + object: tbl, + where: { active: true }, + }); + }); + + it('refuses the redundant `object` key on an `aggregate` call-site literal', async () => { + await driver.aggregate(tbl, { + // @ts-expect-error - 'object' does not exist in type 'DriverQuery' + object: tbl, + aggregations: [{ function: 'count', alias: 'n' }], + }); + }); + + it('restores the `orderBy` check a blanket cast switched off (#4674)', async () => { + // `orderBy` is `SortNode[]` (`{ field, order }`), closed since #4721. The + // `direction` spelling is `IReportService`'s vocabulary and sorted the + // wrong way in silence. Before this batch the only way to hand `aggregate` + // a bare AST was `as any`, which switched this check off with it. + await driver.aggregate(tbl, { + aggregations: [{ function: 'count', alias: 'n' }], + // @ts-expect-error - spell the direction `order`, never `direction` + orderBy: [{ field: 'amount', direction: 'desc' }], + }); + }); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-driver.test.ts b/packages/drivers/driver-memory/src/memory-driver.test.ts index a1d8af5a34..dc3eefe86e 100644 --- a/packages/drivers/driver-memory/src/memory-driver.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver.test.ts @@ -308,7 +308,6 @@ describe('InMemoryDriver', () => { await driver.create(testTable, { id: '3', role: 'user', active: true }); const roles = await driver.distinct(testTable, 'role', { - object: testTable, where: { active: true }, }); expect(roles).toHaveLength(2); @@ -742,7 +741,7 @@ describe('InMemoryDriver', () => { const rows = await driver.aggregate(tbl, { groupBy: ['category'], aggregations: [{ function: 'sum', field: 'amount', alias: 'amount' }], - } as any); + }); const byCat = Object.fromEntries(rows.map((r: any) => [r.category, r.amount])); expect(byCat).toEqual({ travel: 150, meals: 30 }); }); @@ -751,7 +750,7 @@ describe('InMemoryDriver', () => { const rows = await driver.aggregate(tbl, { where: { category: 'travel' }, aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }, { function: 'count', field: '*', alias: 'count' }], - } as any); + }); expect(rows).toEqual([{ total: 150, count: 2 }]); }); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 258200ef73..71c97c6f67 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import type { QueryAST, QueryInput, DriverOptions } from '@objectstack/spec/data'; +import type { DriverOptions } from '@objectstack/spec/data'; import { canonicalAstOperator } from '@objectstack/spec/data'; import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts'; import { Logger, createLogger, nextUtcCalendarDay } from '@objectstack/core'; @@ -609,7 +609,7 @@ export class InMemoryDriver implements IDataDriver { /** * Get distinct values for a field, optionally filtered. */ - async distinct(object: string, field: string, query?: QueryInput): Promise { + async distinct(object: string, field: string, query?: DriverQuery): Promise { let records = this.getTable(object); if (query?.where) { const mongoQuery = this.convertToMongoQuery(query.where, object); @@ -650,16 +650,21 @@ export class InMemoryDriver implements IDataDriver { * { $group: { _id: null, avgPrice: { $avg: '$price' } } } * ]); */ - async aggregate(object: string, pipeline: Record[] | QueryAST, options?: DriverOptions): Promise { + async aggregate(object: string, pipeline: Record[] | DriverQuery, options?: DriverOptions): Promise { // ObjectQL's engine calls driver.aggregate(object, AST) with the SAME - // QueryAST shape find() consumes ({ where, groupBy, aggregations }) — not a - // MongoDB pipeline. Passing that object into Mingo's Aggregator crashed + // DriverQuery shape find() consumes ({ where, groupBy, aggregations }) — not + // a MongoDB pipeline. Passing that object into Mingo's Aggregator crashed // with "this[#pipeline].map is not a function" (the analytics fallback path // on in-memory environments). Detect the AST shape and serve it through the // SAME filtering + performAggregation path find() uses; a real pipeline // array keeps the Mingo behavior unchanged. + // + // BOTH arms of the union have live producers, so neither may be retired: + // the pipeline arm is fed by `memory-analytics.ts` (`this.driver.aggregate( + // tableName, pipeline)`), the AST arm by objectql's engine and + // `@objectstack/verify`'s date-bucket parity probe. if (!Array.isArray(pipeline)) { - const query = pipeline as QueryAST; + const query = pipeline; this.logger.debug('Aggregate operation (QueryAST)', { object, groupBy: (query as any).groupBy, @@ -1035,7 +1040,7 @@ export class InMemoryDriver implements IDataDriver { // Aggregation Logic // =================================== - private performAggregation(records: any[], query: Omit): any[] { + private performAggregation(records: any[], query: DriverQuery): any[] { const { groupBy, aggregations } = query; const groups: Map = new Map(); diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts index 487f56bf6a..efd0f76b86 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts @@ -315,7 +315,7 @@ describe.skipIf(!sharedMongod)('MongoDBDriver', () => { it('should count all records', async () => { const results = await driver.aggregate('order', { aggregations: [{ function: 'count', alias: 'total' }], - } as any); + }); expect(results[0].total).toBe(4); }); @@ -323,7 +323,7 @@ describe.skipIf(!sharedMongod)('MongoDBDriver', () => { const results = await driver.aggregate('order', { aggregations: [{ function: 'sum', field: 'amount', alias: 'total_amount' }], groupBy: ['region'], - } as any); + }); expect(results.length).toBe(2); const us = results.find((r) => r.region === 'US'); diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.ts index 062982468b..387b4bbac4 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.ts @@ -8,7 +8,7 @@ * ObjectStack's query protocol, aggregations, transactions, and streaming. */ -import type { QueryAST, DriverOptions } from '@objectstack/spec/data'; +import type { DriverOptions } from '@objectstack/spec/data'; import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts'; import { MongoClient, @@ -465,7 +465,7 @@ export class MongoDBDriver implements IDataDriver { // Aggregation // =========================================================================== - async aggregate(object: string, query: QueryAST, options?: DriverOptions): Promise[]> { + async aggregate(object: string, query: DriverQuery, options?: DriverOptions): Promise[]> { const collection = this.getCollection(object); const session = this.getSession(options); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 3d4cbc531d..966afd1d34 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -519,12 +519,19 @@ const TEST_DEBT = { + 'tighten via the ℹ hint immediately after landing (#5278 option A).', }, '@objectstack/driver-mongodb': { - errors: 43, - note: 'TS2345 x33, TS1309 x7, TS2550 x3. Re-measured 43 at 5ab08428, DOWN from 44 -- but the ' - + 'composition changed completely: the TS2591 x15 the old note pinned on a missing `types:["node"]` ' - + 'are all gone, and TS1309 (await in a non-async context) has appeared. A -1 delta over a ledger ' - + 'entry that turned over two thirds of its content is exactly why counts alone cannot be trusted ' - + 'to describe debt (#5278).', + errors: 10, + note: 'TS1309 x7, TS2550 x3. Was 43 (TS2345 x33 + these 10), measured at 5ab08428 and still exactly ' + + '43 at d367f03d6^ -- the commit immediately before PR #6210. That PR (#6075) narrowed this ' + + "driver's six IDataDriver query methods to `DriverQuery`, which is what retired all 33 TS2345: " + + "they were this package's OWN test literals failing `Property 'object' is missing in type` " + + 'against a `QueryAST` that still required it. The ledger was never ratcheted down, so 33 errors ' + + 'of slack sat here. #6212 batch C lowers it to the measured 10 because that slack made the batch ' + + "OWN change unpinnable: this package's tsconfig excludes `*.test.ts`, so `pnpm typecheck` cannot " + + "see `aggregate`'s narrowing at all, and its only consumers are those excluded tests. Reverting " + + '`aggregate(object, query: DriverQuery)` back to `QueryAST` measures 12 here -- which the old ' + + '43-ceiling would have swallowed in silence. At 10 it goes red, which is the whole point of a ' + + 'ratchet. Re-measured 10 at 2bc187641, and the pristine tree at that commit reports the same 10, ' + + "so none of the -33 is this PR's doing.", }, '@objectstack/lint': { errors: 42, diff --git a/scripts/query-options-erasure-baseline.json b/scripts/query-options-erasure-baseline.json index 75661a7673..8f3544d598 100644 --- a/scripts/query-options-erasure-baseline.json +++ b/scripts/query-options-erasure-baseline.json @@ -52,6 +52,6 @@ "packages/services/service-settings/src/settings-service.ts": 2 }, "testSurface": { - "sites": 267 + "sites": 263 } }