Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Make DriverInterface a deprecated type alias for IDataDriver in spec/contracts - Remove standalone DriverInterface from core (re-exports from spec) - Add IDataDriver to core re-exports for direct import - Update InMemoryDriver to implement IDataDriver with full DriverCapabilities - Update SqlDriver to implement IDataDriver with missing methods: findStream, upsert, bulkUpdate, bulkDelete, commit, rollback, dropTable, explain - Align supports objects with complete DriverCapabilities schema - Update tests to match new contract (updateMany/deleteMany return number) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/01412f6e-dbc1-4384-a4d7-5b6bd4a575b0
…tion Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/01412f6e-dbc1-4384-a4d7-5b6bd4a575b0
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/01412f6e-dbc1-4384-a4d7-5b6bd4a575b0
|
@copilot 评估是否已经完成了任务中要求的所有开发并修以下问题，11:16:02.844 @objectstack/spec:build: ✓ data/TenantDatabaseLifecycle.json |
… return types)
- driver-memory: updateMany/deleteMany use QueryAST, return number (not { count })
- driver-sql: all query methods use QueryAST, count query is optional
- Both drivers build successfully with strict type checking
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/b04909b4-c504-4ebb-b035-9774568c5bf0
Fixed in c9c1c51. The build errors were caused by two issues:
Both |
|
@copilot 11:49:13.271 @objectstack/spec:build: ✓ data/TenantDatabaseLifecycle.json |
…nown> IDataDriver.create/update/updateMany require Record<string, unknown> but HookContext.input is Record<string, unknown> making .data typed as unknown. Added explicit casts at the 3 call sites in engine.ts (lines 901, 952, 955). Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/f6a26f6f-89b7-44f6-a67a-52714bd9c1a6
Fixed in edddf61. The |
There was a problem hiding this comment.
Pull request overview
Unifies the codebase around a single authoritative data driver contract (IDataDriver in @objectstack/spec/contracts) by deprecating DriverInterface as an alias and aligning driver implementations/tests/docs to the expanded contract.
Changes:
- Deprecates
DriverInterfaceand re-exports the unified contract from core/spec for backward-compatible imports. - Updates
driver-sqlanddriver-memoryto implementIDataDriver(methods +supportscapability descriptor) and aligns return types (updateMany/deleteMany→number,delete→boolean). - Updates tests/docs/changelog to reflect the unified contract and updated method shapes.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/spec/src/contracts/data-engine.ts | Replaces the old DriverInterface interface with a deprecated alias to IDataDriver. |
| packages/spec/src/contracts/data-engine.test.ts | Updates contract tests to validate DriverInterface as an alias of IDataDriver and exercises more of the unified surface. |
| packages/spec/llms.txt | Documentation updates to position IDataDriver as the authoritative driver contract. |
| packages/plugins/driver-sql/src/sql-driver.ts | Switches to IDataDriver, adds missing methods/capabilities, and aligns return types. |
| packages/plugins/driver-sql/src/sql-driver-advanced.test.ts | Updates assertions for the new updateMany/deleteMany return types. |
| packages/plugins/driver-memory/src/memory-driver.ts | Switches to IDataDriver and expands supports to the full capability set; aligns return types. |
| packages/objectql/src/engine.ts | Tightens typings for driver create/update calls to use Record<string, unknown>. |
| packages/objectql/src/engine.test.ts | Updates test typing to IDataDriver (but still uses legacy mock shape/casts). |
| packages/core/src/index.ts | Re-exports IDataDriver alongside deprecated DriverInterface. |
| packages/core/src/contracts/data-engine.ts | Removes the duplicate core driver contract and re-exports the spec contract type. |
| CHANGELOG.md | Documents the unified contract and the breaking return-type changes. |
|
|
||
| async bulkCreate(object: string, data: any[], options?: DriverOptions): Promise<any> { | ||
| const builder = this.getBuilder(object, options); | ||
| return await builder.insert(data).returning('*'); |
There was a problem hiding this comment.
bulkCreate currently inserts raw data and returns the database response without applying the same ID normalization and SQLite JSON/boolean serialization used by create() (formatInput/formatOutput). Under the IDataDriver contract, bulkCreate should return records with id set and without leaking driver-specific formats; consider generating missing IDs per row and formatting input/output consistently (or delegating to create() / a shared helper).
| return await builder.insert(data).returning('*'); | |
| // Normalize IDs and apply driver-specific input formatting per row. | |
| const ids: Array<string | number> = []; | |
| const rowsToInsert = data.map((row) => { | |
| const { _id, ...rest } = row || {}; | |
| const toInsert: Record<string, any> = { ...rest }; | |
| if (_id !== undefined && toInsert.id === undefined) { | |
| toInsert.id = _id; | |
| } else if (toInsert.id === undefined) { | |
| toInsert.id = nanoid(DEFAULT_ID_LENGTH); | |
| } | |
| ids.push(toInsert.id); | |
| const formatted = this.formatInput(object, toInsert); | |
| if (this.tablesWithTimestamps.has(object)) { | |
| if (this.isSqlite) { | |
| const now = new Date().toISOString().replace('T', ' ').replace('Z', ''); | |
| formatted.created_at = now; | |
| formatted.updated_at = now; | |
| } else { | |
| const nowFn = this.knex.fn.now(); | |
| formatted.created_at = nowFn; | |
| formatted.updated_at = nowFn; | |
| } | |
| } | |
| return formatted; | |
| }); | |
| let inserted: any[]; | |
| if (this.isSqlite) { | |
| // SQLite does not reliably support .returning('*') on INSERT. | |
| await builder.insert(rowsToInsert); | |
| inserted = await this.getBuilder(object, options).whereIn('id', ids).select('*'); | |
| } else { | |
| inserted = await builder.insert(rowsToInsert).returning('*'); | |
| } | |
| return inserted.map((row) => this.formatOutput(object, row) || row); |
| const builder = this.getBuilder(object, options); | ||
| const filters = query.where || (query as any).filters || query; | ||
| if (filters) this.applyFilters(builder, filters); | ||
| const count = await builder.update(data); |
There was a problem hiding this comment.
updateMany updates with the raw data object, but update() applies formatInput() (SQLite JSON serialization) and timestamp handling. For consistency and correctness (especially on SQLite JSON fields), updateMany should format/sanitize the update payload the same way as update() before calling builder.update().
| const count = await builder.update(data); | |
| const formattedData = this.formatInput(object, data, options); | |
| const count = await builder.update(formattedData); |
| mockDriver = { | ||
| name: 'default-driver', | ||
| connect: vi.fn().mockResolvedValue(undefined), | ||
| disconnect: vi.fn().mockResolvedValue(undefined), | ||
| find: vi.fn().mockResolvedValue([{ id: '1', name: 'Test Record' }]), | ||
| findOne: vi.fn(), | ||
| create: vi.fn().mockResolvedValue({ id: '1', success: true }), | ||
| update: vi.fn(), | ||
| delete: vi.fn(), | ||
| count: vi.fn(), | ||
| capabilities: {} as any // Simplified | ||
| } as unknown as DriverInterface; | ||
| } as unknown as IDataDriver; |
There was a problem hiding this comment.
The engine test mocks are still using a capabilities property and casting as unknown as IDataDriver, which bypasses the unified contract this PR is trying to enforce. Update these mocks to provide the required supports field (and any required methods you rely on) and avoid the double-cast so the test suite actually catches interface drift.
| @@ -67,7 +67,7 @@ | |||
| delete: vi.fn(), | |||
| count: vi.fn(), | |||
| capabilities: {} as any | |||
| } as unknown as DriverInterface; | |||
| } as unknown as IDataDriver; | |||
There was a problem hiding this comment.
Same issue for the second driver mock: it uses capabilities and relies on as unknown as IDataDriver, so the test won’t detect missing required IDataDriver members like supports, checkHealth, etc. Prefer a properly shaped mock (or a minimal factory) that conforms to IDataDriver without unsafe casts.
DriverInterface(core, 13 methods) andIDataDriver(spec, 28 methods) were two competing driver contracts. All plugins implemented the minimal one, leaving protocol gaps undetectable by CI due toas anycasts.Contract unification
spec/contracts/data-engine.ts:DriverInterfaceis nowtype DriverInterface = IDataDriver(deprecated alias)core/contracts/data-engine.ts: Removed duplicate definition; re-exports from speccore/index.ts: AddedIDataDriverexport alongside deprecatedDriverInterfaceDriver implementations aligned to
IDataDriverdriver-sql: AddedfindStream,upsert,bulkUpdate,bulkDelete,commit,rollback,dropTable,explain. FullDriverCapabilitiesobject.driver-memory: FullDriverCapabilitiesalignment (18 missing capability flags added)Breaking return type changes
updateMany/deleteMany→ returnnumber(count), not{ modifiedCount }/{ deletedCount }delete→ returnsboolean, notanyUsage
Backward compatibility
DriverInterfacepreserved as deprecated type alias — existing imports compilecommitTransaction/rollbackTransactionon SqlDriver preserved as deprecated wrappersDriverInterfaceSchema(Zod) indata/driver.zod.tsunchangedOriginal prompt
⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.