From 46a0a7dff1b7a87832027add95573531ea8fbf97 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 03:57:41 +0000 Subject: [PATCH] fix(objectql,metadata-protocol): refuse aggregate queries that answered a wrong table, and an empty aggregate list engine.aggregate refuses a dateGranularity over a field declared neither a date nor a datetime (INVALID_FIELD, path groupBy.N.dateGranularity), and an aggregated row that would carry two columns under one name (INVALID_QUERY, path at the second position). findData refuses an empty aggregations or groupBy list with no non-empty axis beside it (INVALID_QUERY), which used to reach engine.find and come back as 404 OBJECT_NOT_FOUND. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018QKYDyfAfcAWGKY4UCv3e3 --- .../25818-aggregate-query-shape-refusals.md | 14 ++ .../src/protocol.empty-aggregate-axis.test.ts | 75 +++++++ packages/metadata-protocol/src/protocol.ts | 31 +++ .../src/aggregate-query-shape-door.ts | 177 ++++++++++++++++ .../engine-aggregate-query-shape-door.test.ts | 194 ++++++++++++++++++ packages/objectql/src/engine.ts | 8 + 6 files changed, 499 insertions(+) create mode 100644 .changeset/25818-aggregate-query-shape-refusals.md create mode 100644 packages/metadata-protocol/src/protocol.empty-aggregate-axis.test.ts create mode 100644 packages/objectql/src/aggregate-query-shape-door.ts create mode 100644 packages/objectql/src/engine-aggregate-query-shape-door.test.ts diff --git a/.changeset/25818-aggregate-query-shape-refusals.md b/.changeset/25818-aggregate-query-shape-refusals.md new file mode 100644 index 00000000000..8a1170aaa72 --- /dev/null +++ b/.changeset/25818-aggregate-query-shape-refusals.md @@ -0,0 +1,14 @@ +--- +'@objectstack/objectql': patch +'@objectstack/metadata-protocol': patch +--- + +fix(objectql,metadata-protocol): an aggregate query that would answer a wrong table is refused 400, and an empty aggregate list no longer answers "not registered" + +Clause-②: no +Metadata-protocol: none + +- **Date bucket on a non-date field.** `engine.aggregate` refuses a `groupBy` entry whose `dateGranularity` buckets a field the object declares neither a `date` nor a `datetime`: `INVALID_FIELD` / 400, `path: 'groupBy.N.dateGranularity'`. Before, every row fell into one `null` bucket (a `number` field into a `1970-01` bucket) under 200. A `formula` or undeclared field is not judged. +- **Two columns under one name.** `engine.aggregate` refuses a query whose aggregated row would carry two values under one column: an aggregation `alias` used twice, an alias equal to a group column (the field name, or the entry's own `alias`), or two group entries projecting one column. `INVALID_QUERY` / 400, `path` at the second position (`aggregations.1.alias`, `groupBy.1.field`). Before, the last value written won under 200: a count overwritten by a sum, or the group keys replaced by counts. Give the entry its own `alias`. +- Both refusals run at the engine's `aggregate` entry, before any driver is asked, so the REST query door, its dispatcher twin, the analytics ObjectQL strategy and the MCP data bridge answer alike. +- **Empty aggregate list.** `POST /data/:object/query` with `aggregations: []` (or `groupBy: []`, or both) and no non-empty axis beside it answers `400 INVALID_QUERY` naming `query.aggregations` (or `query.groupBy`) as empty. Before, it answered `404 OBJECT_NOT_FOUND`, "Object '…' is not registered", for a registered object. An empty list beside a non-empty axis is served as before. diff --git a/packages/metadata-protocol/src/protocol.empty-aggregate-axis.test.ts b/packages/metadata-protocol/src/protocol.empty-aggregate-axis.test.ts new file mode 100644 index 00000000000..d8755b18969 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.empty-aggregate-axis.test.ts @@ -0,0 +1,75 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25819] An aggregate axis sent as an EMPTY list, with no non-empty axis + * beside it, is refused `INVALID_QUERY` / 400 by `findData`, naming the empty + * key, before the engine is asked. + * + * Measured on `origin/main` `cb0a2fcf4f` through `findData` over the real + * engine: `{ aggregations: [] }` (with or without a `where`), `{ groupBy: [] }` + * and both together routed to `engine.find`, which refuses an option it does + * not execute with an uncoded error ("find('acct') does not recognise option + * 'aggregations'…"); the REST door's unknown-object heuristic reads the quoted + * object name in it as `404 OBJECT_NOT_FOUND`, "Object 'acct' is not + * registered". The wire half is pinned over the real engine in + * `@objectstack/objectql`'s `engine-aggregate-query-shape-door.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +const SCHEMA = { + name: 'acct', + fields: { + name: { name: 'name', type: 'text' }, + status: { name: 'status', type: 'text' }, + }, +}; + +function makeProtocol() { + const find = vi.fn(async () => [] as unknown[]); + const aggregate = vi.fn(async () => [] as unknown[]); + const engine = { + registry: { getObject: (n: string) => (n === 'acct' ? SCHEMA : undefined) }, + find, + aggregate, + count: vi.fn(async () => 0), + }; + return { p: new ObjectStackProtocolImplementation(engine as any), find, aggregate }; +} + +describe('[#25819] findData — an empty aggregate axis is refused for what it is', () => { + const REFUSED: ReadonlyArray, string, string]> = [ + [{ aggregations: [] }, 'aggregations', "'query.aggregations' is an empty list"], + [{ aggregations: [], where: { status: 'open' } }, 'aggregations', "'query.aggregations' is an empty list"], + [{ groupBy: [] }, 'groupBy', "'query.groupBy' is an empty list"], + [{ groupBy: [], aggregations: [] }, 'aggregations', "'query.groupBy' and 'query.aggregations' are empty lists"], + ]; + + for (const [query, param, words] of REFUSED) { + it(`refuses ${JSON.stringify(query)} with INVALID_QUERY / 400 — the engine is never asked`, async () => { + const { p, find, aggregate } = makeProtocol(); + const err: any = await p.findData({ object: 'acct', query } as never).then(() => null, (e) => e); + expect({ code: err?.code, status: err?.status, param: err?.param, object: err?.object }) + .toEqual({ code: 'INVALID_QUERY', status: 400, param, object: 'acct' }); + expect(err.message).toContain(words); + expect(find).not.toHaveBeenCalled(); + expect(aggregate).not.toHaveBeenCalled(); + }); + } + + it('CONTROL an empty list beside a non-empty axis is served as before, through engine.aggregate', async () => { + const { p, find, aggregate } = makeProtocol(); + await p.findData({ object: 'acct', query: { groupBy: ['status'], aggregations: [] } } as never); + await p.findData({ object: 'acct', query: { groupBy: [], aggregations: [{ function: 'count', alias: 'n' }] } } as never); + expect(aggregate).toHaveBeenCalledTimes(2); + expect(find).not.toHaveBeenCalled(); + }); + + it('CONTROL a query with neither axis is the plain row read', async () => { + const { p, find, aggregate } = makeProtocol(); + await p.findData({ object: 'acct', query: { where: { status: 'open' } } } as never); + expect(find).toHaveBeenCalledTimes(1); + expect(aggregate).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 684c73ccc80..c4410970ae5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -13433,6 +13433,37 @@ export class ObjectStackProtocolImplementation implements // rows returned ungrouped, looking exactly like a served query. this.assertGroupByFieldsExist(request.object, options.groupBy); this.assertAggregationFieldsExist(request.object, options.aggregations); + // [#25819] An aggregate axis sent as an EMPTY list, with no non-empty + // axis beside it, asks for an aggregate of nothing. The routing below + // reads it as no aggregate and handed the key to `engine.find`, which + // refuses an option it does not execute with an uncoded error whose + // words carry the quoted object name — read at the REST door as the + // unknown-object heuristic: `404 OBJECT_NOT_FOUND`, "Object '…' is not + // registered", for a registered object. No shipped caller sends `[]` + // meaning the plain rows, so it is refused for what it is. An empty + // list beside a non-empty one is served as before (`groupBy` with + // `aggregations: []` lists the groups). + { + const emptyAxes = (['groupBy', 'aggregations'] as const) + .filter((k) => Array.isArray(options[k]) && options[k].length === 0); + const servedAxis = (['groupBy', 'aggregations'] as const) + .some((k) => Array.isArray(options[k]) && options[k].length > 0); + if (emptyAxes.length > 0 && !servedAxis) { + const named = emptyAxes.map((k) => `'query.${k}'`).join(' and '); + const err: any = new Error( + `${named} ${emptyAxes.length > 1 ? 'are empty lists' : 'is an empty list'} on object ` + + `'${request.object}': an aggregate query names at least one aggregation, or a non-empty ` + + `'groupBy' to list its groups. The query was NOT run. Send e.g. { "aggregations": ` + + `[{ "function": "count", "alias": "n" }] }, or omit ${emptyAxes.map((k) => `'${k}'`).join(' and ')} ` + + 'for the plain rows.', + ); + err.status = 400; + err.code = 'INVALID_QUERY'; + err.param = emptyAxes[emptyAxes.length - 1]; + err.object = request.object; + throw err; + } + } // [#21086] A stored metadata body is served only as its type's read // projection (see `redactStoredMetadataRows` below), and a GROUP KEY // cannot be projected — so grouping by the body column of a diff --git a/packages/objectql/src/aggregate-query-shape-door.ts b/packages/objectql/src/aggregate-query-shape-door.ts new file mode 100644 index 00000000000..d1955d43f51 --- /dev/null +++ b/packages/objectql/src/aggregate-query-shape-door.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25818] Two shapes of an aggregate query that the engine's `aggregate` + * answered with a plausible table under 200, now refused before any driver is + * asked: + * + * 1. a `groupBy` entry that date-buckets (`{ field, dateGranularity }`) a field + * the object does not declare a date or datetime — + * {@link assertDateBucketTargetsAreTemporal}; + * 2. two columns of the aggregated row under one name: an aggregation alias + * used twice, an aggregation alias equal to a group column, or two group + * entries projecting one column — {@link assertAggregatedColumnsAreDistinct}. + * + * ## What ran before this door, measured on `origin/main` `cb0a2fcf4f` + * + * Through `findData` (the `POST /api/v1/data/:object/query` door) over three + * rows, `status` holding `open`, `open`, `won`: + * + * | query | answer | + * |:--|:--| + * | `groupBy: [{ field: 'name', dateGranularity: 'month' }]` (a `text` field) | `200 [{ name: null, n: 3 }]`: every row in one `null` bucket | + * | `groupBy: [{ field: 'amount', dateGranularity: 'month' }]` (a `number` field) | `200 [{ amount: '1970-01', n: 3 }]`: the numbers read as epoch milliseconds | + * | `groupBy: [{ field: 'at_time', dateGranularity: 'day' }]` (a `time` field) | `200 [{ at_time: null, n: 3 }]` | + * | `aggregations: [{ count, alias: 'n' }, { sum amount, alias: 'n' }]` | `200 [{ n: 60 }]`: the count overwritten by the sum | + * | `groupBy: ['status'], aggregations: [{ count, alias: 'status' }]` | `200 [{ status: 2 }, { status: 1 }]`: the group keys replaced by counts | + * | `groupBy: [{ field: 'opened_on', dateGranularity: 'month' }, { field: 'opened_on', dateGranularity: 'year' }]` | `200 [{ opened_on: '2026', n: 1 }, { opened_on: '2026', n: 2 }]`: the month key overwritten by the year | + * + * The ingress door already refuses an unknown `groupBy` field, because an + * unknown column "puts every row in ONE bucket keyed null"; these are that + * hazard, and its overwrite twin, reached by a declared field. + * + * ## Where it stands + * + * At the entry of `engine.aggregate`, beside the structured-JSON group door and + * the aggregate × field-type door, so it holds for every caller that reaches + * the engine: the REST query door and its dispatcher twin (both through + * `findData`), the analytics ObjectQL strategy, the MCP data bridge, a flow or + * a hook. After those two doors, so a `json` field under a `dateGranularity` + * keeps the structured-JSON refusal's words. + * + * ## What "a date field" is + * + * The declared type's class, read by `declaredFieldClasses` (having-filter.ts) + * — the reading `in-memory-aggregation.ts` buckets a `date` field's calendar + * day by. A `date` (`CALENDAR_DATE_TYPES`) and a `datetime` (`INSTANT_TYPES`) + * pass. **Not judged:** a field the declaration cannot type — undeclared (the + * ingress door answers an unknown one first; a system column the map does not + * declare is bucketed as before), a `formula`, or a registry-less host. + * + * ## The column names + * + * A group column is the entry's name, or `alias ?? field` for the object form + * — the name `applyInMemoryAggregation` and `driver-sql` project it under. An + * aggregation column is its `alias`. Entries that name nothing are left to the + * shape gates that judge them. + * + * `INVALID_FIELD` for the date bucket (a verdict about the named field's type, + * as the structured-JSON group door gives) and `INVALID_QUERY` for a column + * collision (the query names one column twice; no field is wrong). Each + * refusal carries `path`, the position it judged (`groupBy.0.dateGranularity`, + * `aggregations.1.alias`). + * + * @see https://github.com/objectstack-ai/objectstack/issues/25818 + */ + +import { StandardErrorCode } from '@objectstack/spec/api'; +import { declaredFieldClasses } from './having-filter.js'; + +type ShapeRefusal = Error & { + code?: string; status?: number; httpStatus?: number; + field?: string; fields?: string[]; object?: string; param?: string; path?: string; +}; + +function refusal( + code: 'INVALID_FIELD' | 'INVALID_QUERY', + message: string, + extra: Partial, +): ShapeRefusal { + const err = new Error(message) as ShapeRefusal; + err.code = StandardErrorCode.enum[code]; + err.status = 400; + // …and `httpStatus`, the same number under ADR-0112 D5's spelling — what a + // consumer holding the THROWN error reads; `status` stays for the HTTP doors. + err.httpStatus = 400; + Object.assign(err, extra); + return err; +} + +/** + * Refuse a `groupBy` entry whose `dateGranularity` buckets a field the object + * declares neither a date nor a datetime — `INVALID_FIELD` / 400, naming the + * position, the field and its declared type. See the module header. + */ +export function assertDateBucketTargetsAreTemporal( + object: string, + schema: unknown, + groupBy: unknown, +): void { + if (!Array.isArray(groupBy) || groupBy.length === 0) return; + const fields = (schema as { fields?: unknown } | undefined)?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return; + let classes: ReturnType | undefined; + for (const [i, entry] of groupBy.entries()) { + if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) continue; + const { field, dateGranularity } = entry as { field?: unknown; dateGranularity?: unknown }; + if (dateGranularity == null || typeof field !== 'string') continue; + classes ??= declaredFieldClasses(fields); + const cls = classes.get(field); + if (cls === undefined || cls === 'date' || cls === 'datetime') continue; + const type = String((fields as Record)[field]?.type); + const path = `groupBy.${i}.dateGranularity`; + throw refusal( + 'INVALID_FIELD', + `aggregate('${object}'): ${path} buckets '${field}' by '${String(dateGranularity)}', but '${field}' ` + + `is a declared ${type} field — only a date or datetime field can be date-bucketed. The query was ` + + `NOT run. Group by '${field}' without 'dateGranularity', or bucket a date or datetime field. A ` + + 'value that is not a date has no calendar position: every row fell into one null bucket (or, ' + + 'for a number, an epoch bucket) while the answer looked served.', + { field, fields: [field], object, param: 'groupBy', path }, + ); + } +} + +/** The column a `groupBy` entry projects: its name, or `alias ?? field`. */ +function groupColumnOf(entry: unknown): string | undefined { + if (typeof entry === 'string') return entry; + if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) return undefined; + const { alias, field } = entry as { alias?: unknown; field?: unknown }; + const name = alias ?? field; + return typeof name === 'string' && name.length > 0 ? name : undefined; +} + +/** `groupBy.N` for a name, `groupBy.N.alias` / `groupBy.N.field` for the object form. */ +function groupColumnPath(entry: unknown, i: number): string { + if (typeof entry === 'string') return `groupBy.${i}`; + return (entry as { alias?: unknown }).alias != null ? `groupBy.${i}.alias` : `groupBy.${i}.field`; +} + +/** + * Refuse a query whose aggregated row would carry two columns under one name — + * `INVALID_QUERY` / 400, naming both positions. See the module header. + */ +export function assertAggregatedColumnsAreDistinct( + object: string, + groupBy: unknown, + aggregations: unknown, +): void { + const seen = new Map(); + const claim = (column: string, path: string, param: 'groupBy' | 'aggregations', what: string) => { + const first = seen.get(column); + if (first === undefined) { + seen.set(column, path); + return; + } + throw refusal( + 'INVALID_QUERY', + `aggregate('${object}'): ${path} names the column '${column}', which ${first} already names — ` + + `one row cannot carry two values under one column. The query was NOT run. ${what} A repeated ` + + 'column name kept only the last value written under it: a count overwritten by a sum, or the ' + + 'group keys replaced by the numbers, while the answer looked served.', + { object, param, path, fields: [column] }, + ); + }; + for (const [i, entry] of (Array.isArray(groupBy) ? groupBy : []).entries()) { + const column = groupColumnOf(entry); + if (column === undefined) continue; + claim(column, groupColumnPath(entry, i), 'groupBy', + "Give one of the group entries its own 'alias'."); + } + for (const [i, entry] of (Array.isArray(aggregations) ? aggregations : []).entries()) { + const alias = (entry as { alias?: unknown } | null)?.alias; + if (typeof alias !== 'string' || alias.length === 0) continue; + claim(alias, `aggregations.${i}.alias`, 'aggregations', + "Give each aggregation an 'alias' of its own, distinct from every group column."); + } +} diff --git a/packages/objectql/src/engine-aggregate-query-shape-door.test.ts b/packages/objectql/src/engine-aggregate-query-shape-door.test.ts new file mode 100644 index 00000000000..399609a3c19 --- /dev/null +++ b/packages/objectql/src/engine-aggregate-query-shape-door.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25818] `engine.aggregate` refuses, before any driver is asked: + * + * - a `dateGranularity` over a field declared neither a date nor a datetime + * (`INVALID_FIELD` / 400, `path: groupBy.N.dateGranularity`); + * - two columns of the aggregated row under one name — a repeated aggregation + * alias, an alias equal to a group column, two group entries projecting one + * column (`INVALID_QUERY` / 400, `path` at the second position). + * + * The measured base answers are in `aggregate-query-shape-door.ts`'s header. + * The recording driver holds three rows and has no `aggregate`, so a served + * query runs the engine's in-memory aggregation over them: the controls below + * read real buckets, and a refusal is proven to precede every read. + * + * [#25819] The REST door's empty-list refusal is the protocol's own + * (`protocol.empty-aggregate-axis.test.ts`); the wire half below runs it over + * the real engine, which is what turned it into `404 OBJECT_NOT_FOUND`. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { mapDataError } from '@objectstack/types'; +import type { EngineAggregateOptions } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; + +const OBJECT = 'agg_shape_probe'; + +const PROBE = { + name: OBJECT, + label: 'Aggregate shape probe', + fields: { + name: { name: 'name', type: 'text' }, + status: { name: 'status', type: 'select', options: [{ label: 'Open', value: 'open' }, { label: 'Won', value: 'won' }] }, + amount: { name: 'amount', type: 'number' }, + active: { name: 'active', type: 'boolean' }, + at_time: { name: 'at_time', type: 'time' }, + opened_on: { name: 'opened_on', type: 'date' }, + seen_at: { name: 'seen_at', type: 'datetime' }, + doubled: { name: 'doubled', type: 'formula', expression: 'amount * 2' }, + }, +}; + +const ROWS = [ + { id: '1', name: 'a', status: 'open', amount: 10, opened_on: '2026-01-05', seen_at: '2026-01-05T10:00:00Z' }, + { id: '2', name: 'b', status: 'open', amount: 20, opened_on: '2026-02-05', seen_at: '2026-02-05T10:00:00Z' }, + { id: '3', name: 'c', status: 'won', amount: 30, opened_on: '2026-02-07', seen_at: '2026-02-07T10:00:00Z' }, +]; + +const COUNT = { function: 'count', alias: 'n' } as const; + +/** A recording driver with no `aggregate`: the engine aggregates its `find` rows in memory. */ +function makeRecordingDriver() { + const reads: unknown[] = []; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(_o: string, ast: unknown) { reads.push(ast); return ROWS.map((r) => ({ ...r })); }, + async findOne(_o: string, ast: unknown) { reads.push(ast); return { ...ROWS[0] }; }, + async count(_o: string, ast: unknown) { reads.push(ast); return ROWS.length; }, + async create(_o: string, data: Record) { return data; }, + async update(_o: string, _id: string, data: Record) { return data; }, + async delete() { return true; }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, reads }; +} + +type Thrown = (Error & { + code?: string; status?: number; httpStatus?: number; + field?: string; fields?: string[]; object?: string; param?: string; path?: string; +}) | null; + +const refusalOf = async (p: Promise): Promise => p.then(() => null, (e: any) => e); +const envelopeOf = (err: Thrown) => ({ code: err?.code, status: err?.status, httpStatus: err?.httpStatus }); + +describe('[#25818] the aggregate query shapes that answered a plausible wrong table', () => { + let engine: ObjectQL; + let reads: unknown[]; + + beforeEach(async () => { + const rec = makeRecordingDriver(); + reads = rec.reads; + engine = new ObjectQL(); + engine.registerDriver(rec.driver, true); + await engine.init(); + engine.registry.registerObject(PROBE as any, 'test'); + reads.length = 0; + }); + + const aggregate = (q: Record) => engine.aggregate(OBJECT, q as EngineAggregateOptions); + + it('refuses a dateGranularity over a text, select, number, boolean or time field: INVALID_FIELD / 400 at groupBy.N.dateGranularity — no read', async () => { + for (const [field, type] of [['name', 'text'], ['status', 'select'], ['amount', 'number'], ['active', 'boolean'], ['at_time', 'time']]) { + const err = await refusalOf(aggregate({ + groupBy: ['status', { field, dateGranularity: 'month' }], + aggregations: [COUNT], + })); + expect(envelopeOf(err), field).toEqual({ code: 'INVALID_FIELD', status: 400, httpStatus: 400 }); + expect({ path: err?.path, param: err?.param, field: err?.field, object: err?.object }, field) + .toEqual({ path: 'groupBy.1.dateGranularity', param: 'groupBy', field, object: OBJECT }); + expect(err!.message, field).toContain(`groupBy.1.dateGranularity buckets '${field}' by 'month', but '${field}' is a declared ${type} field`); + } + expect(reads, 'every refusal precedes the driver').toHaveLength(0); + }); + + it('refuses a repeated aggregation alias: INVALID_QUERY / 400 at the second aggregations.N.alias — no read', async () => { + const err = await refusalOf(aggregate({ + aggregations: [COUNT, { function: 'sum', field: 'amount', alias: 'n' }], + })); + expect(envelopeOf(err)).toEqual({ code: 'INVALID_QUERY', status: 400, httpStatus: 400 }); + expect({ path: err?.path, param: err?.param, fields: err?.fields, object: err?.object }) + .toEqual({ path: 'aggregations.1.alias', param: 'aggregations', fields: ['n'], object: OBJECT }); + expect(err!.message).toContain("aggregations.1.alias names the column 'n', which aggregations.0.alias already names"); + expect(reads).toHaveLength(0); + }); + + it('refuses an aggregation alias equal to a group column — the field name, or the entry\'s own alias — no read', async () => { + const cases: ReadonlyArray = [ + [['status'], 'status', 'groupBy.0'], + [[{ field: 'status' }], 'status', 'groupBy.0.field'], + [[{ field: 'status', alias: 's' }], 's', 'groupBy.0.alias'], + [['name', { field: 'opened_on', dateGranularity: 'month' }], 'opened_on', 'groupBy.1.field'], + ]; + for (const [groupBy, alias, first] of cases) { + const label = JSON.stringify(groupBy); + const err = await refusalOf(aggregate({ groupBy, aggregations: [{ function: 'count', alias }] })); + expect(envelopeOf(err), label).toEqual({ code: 'INVALID_QUERY', status: 400, httpStatus: 400 }); + expect({ path: err?.path, param: err?.param }, label).toEqual({ path: 'aggregations.0.alias', param: 'aggregations' }); + expect(err!.message, label).toContain(`aggregations.0.alias names the column '${alias}', which ${first} already names`); + } + expect(reads).toHaveLength(0); + }); + + it('refuses two group entries projecting one column, at the second entry — no read', async () => { + const err = await refusalOf(aggregate({ + groupBy: [{ field: 'opened_on', dateGranularity: 'month' }, { field: 'opened_on', dateGranularity: 'year' }], + aggregations: [COUNT], + })); + expect(envelopeOf(err)).toEqual({ code: 'INVALID_QUERY', status: 400, httpStatus: 400 }); + expect({ path: err?.path, param: err?.param }).toEqual({ path: 'groupBy.1.field', param: 'groupBy' }); + expect(reads).toHaveLength(0); + }); + + it('CONTROL a date or datetime bucket, a formula or undeclared bucket, and distinct columns are served', async () => { + await expect(aggregate({ groupBy: [{ field: 'opened_on', dateGranularity: 'month' }], aggregations: [COUNT] })) + .resolves.toEqual([{ opened_on: '2026-01', n: 1 }, { opened_on: '2026-02', n: 2 }]); + await expect(aggregate({ groupBy: [{ field: 'seen_at', dateGranularity: 'month' }], aggregations: [COUNT] })) + .resolves.toEqual([{ seen_at: '2026-01', n: 1 }, { seen_at: '2026-02', n: 2 }]); + // The declaration cannot type a formula or an undeclared column: no verdict. + await expect(aggregate({ groupBy: [{ field: 'doubled', dateGranularity: 'month' }], aggregations: [COUNT] })).resolves.toBeDefined(); + await expect(aggregate({ groupBy: [{ field: 'created_at', dateGranularity: 'month' }], aggregations: [COUNT] })).resolves.toBeDefined(); + // An alias moves the group column, so the field's own name is free. + await expect(aggregate({ groupBy: [{ field: 'status', alias: 's' }], aggregations: [{ function: 'count', alias: 'status' }] })) + .resolves.toEqual([{ s: 'open', status: 2 }, { s: 'won', status: 1 }]); + // Two buckets of one field, each under its own alias. + await expect(aggregate({ + groupBy: [{ field: 'opened_on', dateGranularity: 'year', alias: 'y' }, { field: 'opened_on', dateGranularity: 'month', alias: 'm' }], + aggregations: [COUNT, { function: 'sum', field: 'amount', alias: 'total' }], + })).resolves.toEqual([{ y: '2026', m: '2026-01', n: 1, total: 10 }, { y: '2026', m: '2026-02', n: 2, total: 50 }]); + expect(reads, "each served query read once").toHaveLength(6); + }); + + it('the REST door answers the engine\'s refusal, not a 200 — through findData and the wire mapping', async () => { + const protocol = new ObjectStackProtocolImplementation(engine); + const bodies: ReadonlyArray, string, string]> = [ + [{ groupBy: [{ field: 'name', dateGranularity: 'month' }], aggregations: [COUNT] }, 'INVALID_FIELD', 'groupBy.0.dateGranularity'], + [{ aggregations: [COUNT, { function: 'sum', field: 'amount', alias: 'n' }] }, 'INVALID_QUERY', 'aggregations.1.alias'], + [{ groupBy: ['status'], aggregations: [{ function: 'count', alias: 'status' }] }, 'INVALID_QUERY', 'aggregations.0.alias'], + ]; + for (const [query, code, path] of bodies) { + const label = JSON.stringify(query); + const err = await refusalOf(protocol.findData({ object: OBJECT, query } as any)); + expect({ code: err?.code, status: err?.status, path: err?.path }, label).toEqual({ code, status: 400, path }); + const wire = mapDataError(err, OBJECT); + expect({ status: wire.status, code: wire.body.code }, label).toEqual({ status: 400, code }); + expect(String(wire.body.error), label).toContain(path); + } + expect(reads).toHaveLength(0); + }); + + it('[#25819] an empty aggregations list answers 400 INVALID_QUERY on the wire, never 404 OBJECT_NOT_FOUND for a registered object', async () => { + const protocol = new ObjectStackProtocolImplementation(engine); + for (const query of [{ aggregations: [] }, { aggregations: [], where: { status: 'open' } }]) { + const err = await refusalOf(protocol.findData({ object: OBJECT, query } as any)); + const wire = mapDataError(err, OBJECT); + expect({ status: wire.status, code: wire.body.code }, JSON.stringify(query)).toEqual({ status: 400, code: 'INVALID_QUERY' }); + expect(String(wire.body.error)).toContain("'query.aggregations' is an empty list"); + } + expect(reads).toHaveLength(0); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index d778d28509c..f7a9f810daa 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -276,6 +276,7 @@ import { } from './secret-fields.js'; import { assertGroupByNamesNoJsonStoredField } from './group-by-structured-json-door.js'; import { assertAggregationFieldTypesAccepted } from './aggregate-field-type-door.js'; +import { assertAggregatedColumnsAreDistinct, assertDateBucketTargetsAreTemporal } from './aggregate-query-shape-door.js'; import { applyProtection, pluralToSingular, ExternalWriteForbiddenError } from '@objectstack/spec/shared'; import { SchemaRegistry, computeFQN, type ArtifactInstallScope } from './registry.js'; import { expandSearchToFilter, normalizeSearch, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES } from './search-filter.js'; @@ -21954,6 +21955,13 @@ export class ObjectQL implements IObjectQLEngine { // string on SQLite and a 500 on PostgreSQL; `sum` over one answered `0`, // `0` and a 500. Every row of the table is asked (see the door's header). assertAggregationFieldTypesAccepted(object, this._registry.getObject(object), query.aggregations); + // [#25818] …and the two shapes that answered a plausible wrong table: a + // `dateGranularity` over a field declared neither a date nor a datetime + // (every row in one null bucket), and two columns of the aggregated row + // under one name (the last value written won). After the two doors + // above, so a JSON field under a date bucket keeps their words. + assertDateBucketTargetsAreTemporal(object, this._registry.getObject(object), query.groupBy); + assertAggregatedColumnsAreDistinct(object, query.groupBy, query.aggregations); // [#10576] The per-aggregation `filter` (`AggregationNodeSchema.filter`, // the contract half of #10413) is a second filter position on this verb, // so it walks through the same refusal doors `where` does at this seam: