Merge pull request #804 from objectstack-ai/copilot/unify-filter-parameter-naming

Author: hotlong

ROADMAP.md

@@ -110,6 +110,10 @@ This strategy ensures rapid iteration while maintaining a clear path to producti
 | `defineStack()` strict by default | ✅ | `strict: true` default since v3.0.2, validates schemas + cross-references |
 | `z.any()` elimination in UI protocol | ✅ | All `filter` fields → `FilterConditionSchema` or `ViewFilterRuleSchema`, all `value` fields → typed unions |
 | Filter format unification | ✅ | MongoDB-style filters use `FilterConditionSchema`, declarative view/tab filters use `ViewFilterRuleSchema` — `z.array(z.unknown())` eliminated |
+| Filter parameter naming (`filter` vs `filters`) | ✅ | Canonical HTTP param: `filter` (singular). `filters` accepted for backward compat. `HttpFindQueryParamsSchema` added. Client SDK + protocol.ts unified. |
+| `isFilterAST()` structural validation | ✅ | Exported from `data/filter.zod.ts` — validates AST shape (comparison/logical/legacy) instead of naïve `Array.isArray`. `VALID_AST_OPERATORS` constant. |
+| GET by ID parameter pollution prevention | ✅ | `GetDataRequestSchema` allowlists `select`/`expand`. Dispatcher whitelists only safe params. |
+| Dispatcher response field mapping | ✅ | `handleApiEndpoint` uses spec-correct `result.records`/`result.total` instead of `result.data`/`result.count` |
 | Seed data → object cross-reference | ✅ | `validateCrossReferences` detects seed data referencing undefined objects |
 | Navigation → object/dashboard/page/report cross-reference | ✅ | App navigation items validated against defined metadata (recursive group support) |
 | Negative validation tests (dashboard, page, report, view) | ✅ | Missing required fields, invalid enums, type violations, cross-reference errors all covered |

content/docs/guides/api-reference.mdx

@@ -139,11 +139,15 @@ Query records with filtering, sorting, selection, and pagination.
 | Parameter | Location | Description |
 |:----------|:---------|:------------|
 | `object` | path | Object name |
-| `$select` | query | Comma-separated field names |
-| `$filter` | query | Filter expression (OData-style or JSON) |
-| `$orderby` | query | Sort expression (e.g. `-created_at`) |
-| `$top` | query | Limit (default: 20) |
-| `$skip` | query | Offset |
+| `select` | query | Comma-separated field names |
+| `filter` | query | Filter expression (JSON). `filters` also accepted for backward compatibility. |
+| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`) |
+| `top` | query | Limit (default: 20) |
+| `skip` | query | Offset |
+| `expand` | query | Comma-separated list of relations to eager-load |
+| `search` | query | Full-text search query |
+
+> **Note:** OData-style `$`-prefixed parameters (`$filter`, `$select`, `$orderby`, `$top`, `$skip`) are supported via the [OData endpoint](/docs/references/api/odata). The standard REST API uses unprefixed parameter names.
 
 **Response**:
 ```json
@@ -157,7 +161,14 @@ Query records with filtering, sorting, selection, and pagination.
 
 ### `GET /data/:object/:id`
 
-Get a single record by ID.
+Get a single record by ID. Only `select` and `expand` query parameters are allowed; all other parameters are discarded.
+
+| Parameter | Location | Description |
+|:----------|:---------|:------------|
+| `object` | path | Object name |
+| `id` | path | Record ID |
+| `select` | query | Comma-separated field names to include |
+| `expand` | query | Comma-separated list of relations to eager-load |
 
 **Response**: `{ object: "account", id: "1", record: { ... } }`
 

packages/client/src/index.ts

@@ -1,6 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { QueryAST, SortNode, AggregationNode } from '@objectstack/spec/data';
+import { QueryAST, SortNode, AggregationNode, isFilterAST } from '@objectstack/spec/data';
 import { 
   BatchUpdateRequest, 
   BatchUpdateResponse, 
@@ -115,7 +115,10 @@ export type DiscoveryResult = GetDiscoveryResponse;
 
 export interface QueryOptions {
   select?: string[]; // Simplified Selection
-  filters?: Record<string, any>; // Map or AST
+  /** @canonical Preferred filter parameter (singular). */
+  filter?: Record<string, any> | unknown[]; // Map or AST
+  /** @deprecated Use `filter` (singular). Kept for backward compatibility. */
+  filters?: Record<string, any> | unknown[]; // Map or AST
   sort?: string | string[] | SortNode[]; // 'name' or ['-created_at'] or AST
   top?: number;
   skip?: number;
@@ -1445,14 +1448,19 @@ export class ObjectStackClient {
         }
 
         // 4. Handle Filters (Simple vs AST)
-        if (options.filters) {
+        // Canonical HTTP param name: `filter` (singular). `filters` (plural) is accepted
+        // for backward compatibility but `filter` is the standard going forward.
+        const filterValue = options.filter ?? options.filters;
+        if (filterValue) {
              // Detect AST filter format vs simple key-value map. AST filters use an array structure
-             // with [field, operator, value] or [logicOp, ...nodes] shape (see isFilterAST).
+             // with [field, operator, value] or [logicOp, ...nodes] shape (see isFilterAST from spec).
              // For complex filter expressions, use .query() which builds a proper QueryAST.
-             if (this.isFilterAST(options.filters)) {
-                 queryParams.set('filters', JSON.stringify(options.filters));
-             } else {
-                 Object.entries(options.filters).forEach(([k, v]) => {
+             if (this.isFilterAST(filterValue) || Array.isArray(filterValue)) {
+                 // AST or any array → serialize as JSON in `filter` param
+                 queryParams.set('filter', JSON.stringify(filterValue));
+             } else if (typeof filterValue === 'object' && filterValue !== null) {
+                 // Plain key-value map → append each as individual query params
+                 Object.entries(filterValue as Record<string, unknown>).forEach(([k, v]) => {
                      if (v !== undefined && v !== null) {
                         queryParams.append(k, String(v));
                      }
@@ -1571,10 +1579,9 @@ export class ObjectStackClient {
    */
 
   private isFilterAST(filter: any): boolean {
-    // Basic check: if array, it's [field, op, val] or [logic, node, node]
-    // If object but not basic KV map... harder to tell without schema
-    // For now, assume if it passes Array.isArray it's an AST root
-    return Array.isArray(filter);
+    // Delegate to the spec-exported structural validator instead of naive Array.isArray.
+    // This checks for valid AST shapes: [field, op, val], [logic, ...nodes], or [[cond], ...].
+    return isFilterAST(filter);
   }
 
   /**

packages/objectql/src/protocol.ts

@@ -10,7 +10,7 @@ import type {
 } from '@objectstack/spec/api';
 import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
 import type { IFeedService } from '@objectstack/spec/contracts';
-import { parseFilterAST } from '@objectstack/spec/data';
+import { parseFilterAST, isFilterAST } from '@objectstack/spec/data';
 
 // We import SchemaRegistry directly since this class lives in the same package
 import { SchemaRegistry } from './registry.js';
@@ -304,17 +304,21 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
             delete options.orderBy;
         }
 
+        // Filter: normalize `filter`/`filters` (plural) → `filter` (singular, canonical)
+        // Accept both `filter` and `filters` for backward compatibility.
+        if (options.filters !== undefined && options.filter === undefined) {
+            options.filter = options.filters;
+        }
+        delete options.filters;
+
         // Filter: JSON string → object
         if (typeof options.filter === 'string') {
             try { options.filter = JSON.parse(options.filter); } catch { /* keep as-is */ }
         }
-        if (typeof options.filters === 'string') {
-            try { options.filter = JSON.parse(options.filters); delete options.filters; } catch { /* keep as-is */ }
-        }
 
         // Filter AST array → FilterCondition object
         // Converts ["and", ["field", "=", "val"], ...] to { $and: [{ field: "val" }, ...] }
-        if (Array.isArray(options.filter)) {
+        if (isFilterAST(options.filter)) {
             options.filter = parseFilterAST(options.filter);
         }
 

packages/runtime/src/http-dispatcher.ts

@@ -377,8 +377,14 @@ export class HttpDispatcher {
             // GET /data/:object/:id
             if (parts.length === 2 && m === 'GET') {
                 const id = parts[1];
+                // Spec: Only select/expand are allowlisted query params for GET by ID.
+                // All other query parameters are discarded to prevent parameter pollution.
+                const { select, expand } = query || {};
+                const allowedParams: Record<string, unknown> = {};
+                if (select != null) allowedParams.select = select;
+                if (expand != null) allowedParams.expand = expand;
                 // Spec: broker returns GetDataResponse = { object, id, record }
-                const result = await broker.call('data.get', { object: objectName, id, ...query }, { request: context.request });
+                const result = await broker.call('data.get', { object: objectName, id, ...allowedParams }, { request: context.request });
                 return { handled: true, response: this.success(result) };
             }
 
@@ -942,7 +948,8 @@ export class HttpDispatcher {
                         // Map standard CRUD operations
                         if (operation === 'find') {
                              const result = await broker.call('data.query', { object, query }, { request: context.request });
-                             return { handled: true, response: this.success(result.data, { count: result.count }) };
+                             // Spec: FindDataResponse = { object, records, total?, hasMore? }
+                             return { handled: true, response: this.success(result.records, { total: result.total }) };
                         }
                         if (operation === 'get' && query.id) {
                              const result = await broker.call('data.get', { object, id: query.id }, { request: context.request });

packages/spec/src/api/protocol.test.ts

@@ -5,6 +5,7 @@ import {
   GetDataResponseSchema,
   FindDataRequestSchema,
   FindDataResponseSchema,
+  HttpFindQueryParamsSchema,
   CreateDataRequestSchema,
   CreateDataResponseSchema,
   UpdateDataRequestSchema,
@@ -367,3 +368,102 @@ describe('GetDiscoveryResponseSchema (capabilities)', () => {
     expect(result.success).toBe(false);
   });
 });
+
+// ==========================================
+// GetDataRequestSchema — select/expand params
+// ==========================================
+
+describe('GetDataRequestSchema (select/expand)', () => {
+  it('should accept request with select and expand', () => {
+    const result = GetDataRequestSchema.safeParse({
+      object: 'contact',
+      id: 'c1',
+      select: ['name', 'email'],
+      expand: ['account', 'owner'],
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.select).toEqual(['name', 'email']);
+      expect(result.data.expand).toEqual(['account', 'owner']);
+    }
+  });
+
+  it('should accept request without select/expand (optional)', () => {
+    const result = GetDataRequestSchema.safeParse({
+      object: 'contact',
+      id: 'c1',
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.select).toBeUndefined();
+      expect(result.data.expand).toBeUndefined();
+    }
+  });
+
+  it('should strip unknown query parameters via strict parsing', () => {
+    const result = GetDataRequestSchema.strict().safeParse({
+      object: 'contact',
+      id: 'c1',
+      select: ['name'],
+      // Unknown params that should be rejected by strict mode
+      unknownParam: 'should-be-stripped',
+    });
+    expect(result.success).toBe(false);
+  });
+});
+
+// ==========================================
+// HttpFindQueryParamsSchema — HTTP parameter naming
+// ==========================================
+
+describe('HttpFindQueryParamsSchema', () => {
+  it('should accept canonical filter (singular) parameter', () => {
+    const result = HttpFindQueryParamsSchema.safeParse({
+      filter: '{"status":"active"}',
+      select: 'name,email',
+      top: 10,
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.filter).toBe('{"status":"active"}');
+      expect(result.data.top).toBe(10);
+    }
+  });
+
+  it('should accept deprecated filters (plural) parameter for backward compat', () => {
+    const result = HttpFindQueryParamsSchema.safeParse({
+      filters: '{"status":"active"}',
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.filters).toBe('{"status":"active"}');
+    }
+  });
+
+  it('should coerce string numbers for top/skip', () => {
+    const result = HttpFindQueryParamsSchema.safeParse({
+      top: '25',
+      skip: '50',
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.top).toBe(25);
+      expect(result.data.skip).toBe(50);
+    }
+  });
+
+  it('should accept sort, orderBy, expand, search, distinct, count', () => {
+    const result = HttpFindQueryParamsSchema.safeParse({
+      sort: 'name asc,created_at desc',
+      expand: 'owner,account',
+      search: 'John',
+      distinct: true,
+      count: true,
+    });
+    expect(result.success).toBe(true);
+  });
+
+  it('should accept empty object (all params optional)', () => {
+    expect(HttpFindQueryParamsSchema.safeParse({}).success).toBe(true);
+  });
+});

packages/spec/src/api/protocol.zod.ts

@@ -240,9 +240,9 @@ export const GetUiViewResponseSchema = ViewSchema;
  * {
  *   "object": "customers",
  *   "query": {
- *     "filters": [["status", "=", "active"], ["revenue", ">", 10000]],
- *     "sort": "name desc",
- *     "top": 10
+ *     "where": { "status": "active", "revenue": { "$gt": 10000 } },
+ *     "orderBy": [{ "field": "name", "order": "desc" }],
+ *     "limit": 10
  *   }
  * }
  */
@@ -263,19 +263,55 @@ export const FindDataResponseSchema = z.object({
   hasMore: z.boolean().optional().describe('True if there are more records available (pagination).'),
 });
 
+/**
+ * HTTP Find Query Parameters
+ * 
+ * Canonical HTTP query parameter names for GET /data/:object list endpoints.
+ * The canonical filter parameter is `filter` (singular). The plural `filters` is
+ * accepted for backward compatibility but `filter` is the standard going forward.
+ * 
+ * This schema defines the allowlisted query parameters for HTTP GET list requests.
+ * Server-side parsers (protocol.ts) should accept both `filter` and `filters` and
+ * normalize to `filter` (singular) internally.
+ * 
+ * @example
+ * GET /api/v1/data/contacts?filter={"status":"active"}&select=name,email&top=10&sort=name
+ */
+export const HttpFindQueryParamsSchema = z.object({
+  /** @canonical Singular form — the standard going forward. JSON string of filter expression or AST. */
+  filter: z.string().optional().describe('JSON-encoded filter expression (canonical, singular).'),
+  /** @deprecated Use `filter` (singular). Accepted for backward compatibility. */
+  filters: z.string().optional().describe('JSON-encoded filter expression (deprecated plural alias).'),
+  select: z.string().optional().describe('Comma-separated list of fields to retrieve.'),
+  sort: z.string().optional().describe('Sort expression (e.g. "name asc,created_at desc" or "-created_at").'),
+  orderBy: z.string().optional().describe('Alias for sort (OData compatibility).'),
+  top: z.coerce.number().optional().describe('Max records to return (limit).'),
+  skip: z.coerce.number().optional().describe('Records to skip (offset).'),
+  expand: z.string().optional().describe('Comma-separated list of relations to eager-load.'),
+  search: z.string().optional().describe('Full-text search query.'),
+  distinct: z.coerce.boolean().optional().describe('SELECT DISTINCT flag.'),
+  count: z.coerce.boolean().optional().describe('Include total count in response.'),
+});
+
 /**
  * Get Data Request
  * Retrieval of a single record by its unique identifier.
+ * Only `select` and `expand` are permitted as additional query parameters.
+ * All other query parameters should be discarded to prevent parameter pollution.
  * 
  * @example
  * {
  *   "object": "contracts",
- *   "id": "cnt_123456"
+ *   "id": "cnt_123456",
+ *   "select": ["name", "status", "amount"],
+ *   "expand": ["owner", "account"]
  * }
  */
 export const GetDataRequestSchema = z.object({
   object: z.string().describe('The object name.'),
   id: z.string().describe('The unique record identifier (primary key).'),
+  select: z.array(z.string()).optional().describe('Fields to include in the response (allowlisted query param).'),
+  expand: z.array(z.string()).optional().describe('Relations to eager-load (allowlisted query param).'),
 });
 
 /**