Skip to content

Commit d263b28

Browse files
Copilothotlong
Copilot
and
hotlong
committed
fix: remove legacy query key fallbacks (filters/sort/skip/top) from driver-sql
Strictly enforce IDataDriver / QueryAST protocol in SqlDriver: - find(): remove fallback to (query as any).filters/.sort/.skip/.top - updateMany(): remove fallback to (query as any).filters - deleteMany(): remove fallback to (query as any).filters - count(): remove fallback to (query as any).filters - applyFilters(): clean up skip-list of legacy keys Update all tests to use standard QueryAST keys (where/orderBy/offset/limit). Add tests verifying legacy keys are ignored. Closes #960 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/61332fa6-4cc7-491f-b72c-2c73c6357e02
1 parent fad2a3f commit d263b28

5 files changed

Lines changed: 99 additions & 53 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
schema (added `create`, `read`, `update`, `delete`, `bulkCreate`, `bulkUpdate`, `bulkDelete`,
2323
`savepoints`, `queryCTE`, `jsonQuery`, `geospatialQuery`, `streaming`, `schemaSync`, etc.).
2424

25+
### Removed
26+
- **`@objectstack/driver-sql` — Legacy query key fallbacks** — Removed support for deprecated
27+
query keys `filters` (use `where`), `sort` (use `orderBy`), `skip` (use `offset`), and `top`
28+
(use `limit`) from `find`, `updateMany`, `deleteMany`, and `count` methods. The SQL driver now
29+
strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for legacy key
30+
access have been eliminated. Tests updated to use only standard `QueryAST` keys.
31+
2532
### Deprecated
2633
- **`DriverInterface`** — Use `IDataDriver` from `@objectstack/spec/contracts` instead.
2734
`DriverInterface` remains as a type alias in both `@objectstack/spec/contracts` and

‎packages/plugins/driver-sql/src/sql-driver-advanced.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ describe('SqlDriver Advanced Operations (SQLite)', () => {
144144
});
145145

146146
it('should update many records', async () => {
147-
const result = await driver.updateMany('orders', { status: 'pending' } as any, { status: 'processing' });
147+
const result = await driver.updateMany('orders', { where: { status: 'pending' } } as any, { status: 'processing' });
148148

149149
expect(result).toBeGreaterThan(0);
150150

@@ -153,7 +153,7 @@ describe('SqlDriver Advanced Operations (SQLite)', () => {
153153
});
154154

155155
it('should delete many records', async () => {
156-
const result = await driver.deleteMany('orders', { status: 'cancelled' } as any);
156+
const result = await driver.deleteMany('orders', { where: { status: 'cancelled' } } as any);
157157

158158
expect(result).toBe(1);
159159

@@ -162,10 +162,10 @@ describe('SqlDriver Advanced Operations (SQLite)', () => {
162162
});
163163

164164
it('should handle empty bulk update and delete', async () => {
165-
const result = await driver.updateMany('orders', { status: 'nonexistent' } as any, { status: 'updated' });
165+
const result = await driver.updateMany('orders', { where: { status: 'nonexistent' } } as any, { status: 'updated' });
166166
expect(result).toBe(0);
167167

168-
const deleteResult = await driver.deleteMany('orders', { id: 'nonexistent' } as any);
168+
const deleteResult = await driver.deleteMany('orders', { where: { id: 'nonexistent' } } as any);
169169
expect(deleteResult).toBe(0);
170170
});
171171
});
@@ -365,7 +365,7 @@ describe('SqlDriver Advanced Operations (SQLite)', () => {
365365

366366
it('should handle count with complex filters', async () => {
367367
const count = await driver.count('orders', {
368-
$and: [{ status: 'completed' }, { amount: { $gt: 100 } }],
368+
where: { $and: [{ status: 'completed' }, { amount: { $gt: 100 } }] },
369369
} as any);
370370

371371
expect(count).toBe(2);

‎packages/plugins/driver-sql/src/sql-driver-queryast.test.ts‎

Lines changed: 72 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,22 +72,22 @@ describe('SqlDriver (QueryAST Format)', () => {
7272
});
7373

7474
describe('QueryAST Format Support', () => {
75-
it('should support QueryAST with "top" instead of "limit"', async () => {
75+
it('should support QueryAST with limit and orderBy', async () => {
7676
const results = await driver.find('products', {
7777
fields: ['name', 'price'],
78-
top: 2,
79-
sort: [{ field: 'price', order: 'asc' as const }],
78+
limit: 2,
79+
orderBy: [{ field: 'price', order: 'asc' as const }],
8080
} as any);
8181

8282
expect(results.length).toBe(2);
8383
expect(results[0].name).toBe('Mouse');
8484
expect(results[1].name).toBe('Chair');
8585
});
8686

87-
it('should support QueryAST sort format with object notation', async () => {
87+
it('should support QueryAST orderBy with object notation', async () => {
8888
const results = await driver.find('products', {
8989
fields: ['name'],
90-
sort: [
90+
orderBy: [
9191
{ field: 'category', order: 'asc' as const },
9292
{ field: 'price', order: 'desc' as const },
9393
],
@@ -98,12 +98,12 @@ describe('SqlDriver (QueryAST Format)', () => {
9898
expect(results[3].name).toBe('Desk');
9999
});
100100

101-
it('should support QueryAST with filters and pagination', async () => {
101+
it('should support QueryAST with where, offset, limit, and orderBy', async () => {
102102
const results = await driver.find('products', {
103-
filters: [['category', '=', 'Electronics']],
104-
skip: 1,
105-
top: 1,
106-
sort: [{ field: 'price', order: 'asc' as const }],
103+
where: [['category', '=', 'Electronics']],
104+
offset: 1,
105+
limit: 1,
106+
orderBy: [{ field: 'price', order: 'asc' as const }],
107107
} as any);
108108

109109
expect(results.length).toBe(1);
@@ -131,20 +131,20 @@ describe('SqlDriver (QueryAST Format)', () => {
131131
expect(furniture.total_price).toBe(550);
132132
});
133133

134-
it('should support count with QueryAST format', async () => {
134+
it('should support count with QueryAST where clause', async () => {
135135
const count = await driver.count('products', {
136-
filters: [['price', '>', 300]],
136+
where: [['price', '>', 300]],
137137
} as any);
138138
expect(count).toBe(3);
139139
});
140140
});
141141

142-
describe('Backward Compatibility', () => {
143-
it('should still support legacy UnifiedQuery format with "limit"', async () => {
142+
describe('Standard QueryAST Pagination', () => {
143+
it('should support limit with orderBy using standard keys', async () => {
144144
const results = await driver.find('products', {
145145
fields: ['name'],
146146
limit: 2,
147-
sort: [['price', 'asc']],
147+
orderBy: [['price', 'asc']],
148148
} as any);
149149

150150
expect(results.length).toBe(2);
@@ -161,14 +161,12 @@ describe('SqlDriver (QueryAST Format)', () => {
161161
const electronics = results.find((r: any) => r.category === 'Electronics');
162162
expect(electronics.avg_price).toBeCloseTo(541.67, 1);
163163
});
164-
});
165164

166-
describe('Mixed Format Support', () => {
167-
it('should handle query with both top and skip', async () => {
165+
it('should support offset and limit with orderBy', async () => {
168166
const results = await driver.find('products', {
169-
top: 3,
170-
skip: 2,
171-
sort: [{ field: 'name', order: 'asc' as const }],
167+
limit: 3,
168+
offset: 2,
169+
orderBy: [{ field: 'name', order: 'asc' as const }],
172170
} as any);
173171

174172
expect(results.length).toBe(3);
@@ -177,4 +175,57 @@ describe('SqlDriver (QueryAST Format)', () => {
177175
expect(results[2].name).toBe('Mouse');
178176
});
179177
});
178+
179+
describe('Legacy Keys Are Ignored', () => {
180+
it('should ignore legacy "filters" key — only "where" is recognized', async () => {
181+
const results = await driver.find('products', {
182+
filters: [['category', '=', 'Furniture']],
183+
} as any);
184+
185+
// "filters" is not recognized, so no WHERE clause is applied — returns all rows
186+
expect(results.length).toBe(5);
187+
});
188+
189+
it('should ignore legacy "sort" key — only "orderBy" is recognized', async () => {
190+
const results = await driver.find('products', {
191+
fields: ['name'],
192+
limit: 2,
193+
sort: [{ field: 'price', order: 'asc' as const }],
194+
} as any);
195+
196+
// "sort" is not recognized — no ORDER BY applied, so order is insertion order
197+
expect(results.length).toBe(2);
198+
// Without ORDER BY, results come in insertion order (Laptop, Mouse)
199+
expect(results[0].name).toBe('Laptop');
200+
});
201+
202+
it('should ignore legacy "skip" key — only "offset" is recognized', async () => {
203+
const results = await driver.find('products', {
204+
skip: 3,
205+
orderBy: [{ field: 'name', order: 'asc' as const }],
206+
} as any);
207+
208+
// "skip" is not recognized — no offset applied, returns all 5 rows
209+
expect(results.length).toBe(5);
210+
});
211+
212+
it('should ignore legacy "top" key — only "limit" is recognized', async () => {
213+
const results = await driver.find('products', {
214+
top: 2,
215+
orderBy: [{ field: 'name', order: 'asc' as const }],
216+
} as any);
217+
218+
// "top" is not recognized — no limit applied, returns all 5 rows
219+
expect(results.length).toBe(5);
220+
});
221+
222+
it('should ignore legacy "filters" in count — only "where" is recognized', async () => {
223+
const count = await driver.count('products', {
224+
filters: [['price', '>', 300]],
225+
} as any);
226+
227+
// "filters" is not recognized — counts all rows
228+
expect(count).toBe(5);
229+
});
230+
});
180231
});

‎packages/plugins/driver-sql/src/sql-driver.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('SqlDriver (SQLite Integration)', () => {
9393
});
9494

9595
it('should count objects', async () => {
96-
const count = await driver.count('users', { age: 17 } as any);
96+
const count = await driver.count('users', { where: { age: 17 } } as any);
9797
expect(count).toBe(2);
9898
});
9999

‎packages/plugins/driver-sql/src/sql-driver.ts‎

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,14 @@ export class SqlDriver implements IDataDriver {
177177
builder.select('*');
178178
}
179179

180-
// WHERE — support both `where` (standard) and `filters` (legacy)
181-
const filterCondition = query.where || (query as any).filters;
182-
if (filterCondition) {
183-
this.applyFilters(builder, filterCondition);
180+
// WHERE
181+
if (query.where) {
182+
this.applyFilters(builder, query.where);
184183
}
185184

186-
// ORDER BY — support both `orderBy` (standard) and `sort` (legacy)
187-
const sortArray = query.orderBy || (query as any).sort;
188-
if (sortArray && Array.isArray(sortArray)) {
189-
for (const item of sortArray) {
185+
// ORDER BY
186+
if (query.orderBy && Array.isArray(query.orderBy)) {
187+
for (const item of query.orderBy) {
190188
const field = item.field || item[0];
191189
const dir = item.order || item[1] || 'asc';
192190
if (field) {
@@ -195,12 +193,9 @@ export class SqlDriver implements IDataDriver {
195193
}
196194
}
197195

198-
// PAGINATION — support both offset/limit (standard) and skip/top (legacy)
199-
const offsetValue = query.offset ?? (query as any).skip;
200-
const limitValue = query.limit ?? (query as any).top;
201-
202-
if (offsetValue !== undefined) builder.offset(offsetValue);
203-
if (limitValue !== undefined) builder.limit(limitValue);
196+
// PAGINATION
197+
if (query.offset !== undefined) builder.offset(query.offset);
198+
if (query.limit !== undefined) builder.limit(query.limit);
204199

205200
let results: any[];
206201
try {
@@ -347,30 +342,23 @@ export class SqlDriver implements IDataDriver {
347342

348343
async updateMany(object: string, query: QueryAST, data: any, options?: DriverOptions): Promise<number> {
349344
const builder = this.getBuilder(object, options);
350-
const filters = query.where || (query as any).filters || query;
351-
if (filters) this.applyFilters(builder, filters);
345+
if (query.where) this.applyFilters(builder, query.where);
352346
const count = await builder.update(data);
353347
return count || 0;
354348
}
355349

356350
async deleteMany(object: string, query: QueryAST, options?: DriverOptions): Promise<number> {
357351
const builder = this.getBuilder(object, options);
358-
const filters = query.where || (query as any).filters || query;
359-
if (filters) this.applyFilters(builder, filters);
352+
if (query.where) this.applyFilters(builder, query.where);
360353
const count = await builder.delete();
361354
return count || 0;
362355
}
363356

364357
async count(object: string, query?: QueryAST, options?: DriverOptions): Promise<number> {
365358
const builder = this.getBuilder(object, options);
366359

367-
let actualFilters = query as any;
368-
if (query && (query.where || (query as any).filters)) {
369-
actualFilters = query.where || (query as any).filters;
370-
}
371-
372-
if (actualFilters) {
373-
this.applyFilters(builder, actualFilters);
360+
if (query?.where) {
361+
this.applyFilters(builder, query.where);
374362
}
375363

376364
const result = await builder.count<{ count: number }[]>('* as count');
@@ -742,7 +730,7 @@ export class SqlDriver implements IDataDriver {
742730
}
743731

744732
for (const [key, value] of Object.entries(filters)) {
745-
if (['filters', 'sort', 'limit', 'skip', 'offset', 'fields', 'orderBy'].includes(key)) continue;
733+
if (['limit', 'offset', 'fields', 'orderBy'].includes(key)) continue;
746734
builder.where(key, value as any);
747735
}
748736
return;

0 commit comments

Comments
 (0)