fix(metadata-protocol)!: the Data API refuses an empty projection and a count flag that is neither true nor false

.changeset/26644-empty-projection-count-flag.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/metadata-protocol': patch
+---
+
+fix(metadata-protocol)!: the Data API refuses an empty projection and a count flag that is neither true nor false, instead of reading them as absent (#26644)
+
+Clause-②: no (narrowing) — `GET /data/:object`, `POST /data/:object/query` (both transports: `RestServer` and the runtime dispatcher) and `GET /data/:object/:id` refuse a projection that names no field, and the list doors refuse a `count` / `$count` other than true or false, where they answered 200 before. Nothing is widened and no export changes.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: two query-parameter values the list normalizer read as the absent default are now refused 400 VALIDATION_ERROR naming the parameter. A caller drops the parameter or sends a well-formed value; there is nothing for a migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing at the shared list normalizer (`findData`) and the by-id read (`getData`), served by every Data API door that uses them.
+
+- **Before.** A blank or comma-only `$select` / `select` / `fields` (`?$select=`, `?$select=,`), and an empty array in a JSON body (`"fields": []`), became an empty projection, which every driver serves as no projection: all columns, under a 200. A `$count` / `count` other than `true` or `false` (`?$count=maybe`, `?$count=1`, a blank `?count=`) was left unlowered, so the `count=false` opt-out never matched and the count ran: a `total` was reported as if `$count=true` had been sent.
+- **Now.** Both are refused `400 VALIDATION_ERROR` before any read, with a message naming the parameter the caller wrote and the value (`Query parameter '$select' names no field; got ""`, `Query parameter '$count' must be true or false; got "maybe"`), as a blank `$top` already is. The accepted count values are the spec's `QueryTransportParamsSchema.count`: the boolean, or the strings `true` and `false`. `GET /data/:object/:id?select=` answers with the same code; that route's catch already reports every projection refusal (an unknown field included) as `404`, and the empty projection joins that answer.
+- **Unchanged.** An absent parameter keeps its default: every column, and the count runs. A named projection (`$select=a,b`, trailing commas and spaces trimmed as before), `$count=true`, `$count=false` and a JSON `null` answer exactly as before. `POST /data/:object/query` already refused a non-boolean `count` at its body schema (`400 VALIDATION_FAILED`), and still does.
+- **Who sees it.** A caller that builds `$select` from a list that can be empty, or sends a mistyped `$count`. It now learns that from a 400 instead of an answer that looked normal.
+
+**The remedy.** Leave the parameter out to read every column or to keep the count, or send a projection naming at least one field and a count flag of `true` or `false`.

packages/metadata-protocol/src/protocol.empty-projection-unknown-count.test.ts

@@ -0,0 +1,205 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #26644 — the list normalizer read two malformed values as the absent default.
+ *
+ *  - **Projection.** A string `fields` (or its folded `select` / `$select`
+ *    spellings) was `split(',').map(trim).filter(Boolean)`, so a blank or
+ *    comma-only value became `[]` — and `assertProjectionFieldsExist` has
+ *    nothing to refuse in an empty list, so an empty projection was served as
+ *    NO projection: every column. An explicit `fields: []` took the same road.
+ *    Absent means "every column"; an empty projection asks for none, so reading
+ *    it as absent is the silent widening `pageWindowValue` already refuses on
+ *    the row axis (a blank `$top`).
+ *  - **Count.** The boolean step lowered only `'true'` / `'false'`; any other
+ *    value stayed as it was, `countOptOut = options.count === false` was false,
+ *    and the count ran — `?$count=maybe` reported a `total` like `?$count=true`.
+ *
+ * Both are now refused `400 VALIDATION_ERROR` naming the parameter the caller
+ * wrote and the value, before the engine is asked. The accepted `count` values
+ * are the spec's (`QueryTransportParamsSchema.count`): the boolean, or the two
+ * strings a querystring carries it as.
+ *
+ * `getData` (`GET /data/:object/:id`) reads `select` against the same field
+ * map, and its own comment promised "gated exactly as on the list path", so its
+ * projection takes the same refusal: one rule for both routes.
+ *
+ * ## Two assertion classes
+ *
+ *  1. **REFUSAL** — `code`, `status` and `param` on the throw, and the engine
+ *     never reached. The unfixed normalizer never threw for these, so a bare
+ *     `toThrow()` would be half a pin.
+ *  2. **PRESERVATION** — a well-formed projection reaches `engine.find` as the
+ *     same list it always did, and `count` absent / `true` / `false` (either
+ *     spelling, `null` as absent) counts or skips exactly as before.
+ *
+ * The door-level half (both REST list routes and the by-id route over a real
+ * `SqlDriver`) is `packages/rest/src/data-empty-projection-count-door.test.ts`;
+ * the runtime dispatcher's is
+ * `packages/runtime/src/domains/data-empty-projection-count.test.ts`.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+const SCHEMA = {
+    name: 'invoice',
+    nameField: 'name',
+    fields: {
+        name: { name: 'name', type: 'text' },
+        status: { name: 'status', type: 'text' },
+    },
+};
+
+function makeProtocol() {
+    const find = vi.fn(async () => [{ id: 'r1', name: 'a', status: 'open' }] as unknown[]);
+    const findOne = vi.fn(async (object: string, query?: EngineFindOneQueryInput) => {
+        assertEngineFindOnePredicate(object, query);
+        return { id: 'r1', name: 'a', status: 'open' };
+    });
+    const count = vi.fn(async () => 7);
+    const engine = {
+        registry: { getObject: (n: string) => (n === 'invoice' ? SCHEMA : undefined) },
+        find,
+        findOne,
+        count,
+        aggregate: vi.fn(async () => [] as unknown[]),
+    };
+    return { p: new ObjectStackProtocolImplementation(engine as any), find, findOne, count };
+}
+
+type Refusal = { message: string; status?: number; code?: string; param?: string };
+
+async function refusal(run: (p: any) => Promise<unknown>, reached: (m: ReturnType<typeof makeProtocol>) => unknown[]): Promise<Refusal> {
+    const m = makeProtocol();
+    let answered: unknown;
+    try {
+        answered = await run(m.p);
+    } catch (e) {
+        const err = e as Error & { status?: number; code?: string; param?: string };
+        for (const spy of reached(m)) expect(spy, 'the engine was reached before the refusal').not.toHaveBeenCalled();
+        return { message: err.message, status: err.status, code: err.code, param: err.param };
+    }
+    throw new Error(`ACCEPTED (answered ${JSON.stringify(answered)}) instead of refused`);
+}
+
+const listRefusal = (query: Record<string, unknown>) =>
+    refusal((p) => p.findData({ object: 'invoice', query }), (m) => [m.find, m.count]);
+const getRefusal = (select: unknown) =>
+    refusal((p) => p.getData({ object: 'invoice', id: 'r1', select }), (m) => [m.findOne]);
+
+// ---------------------------------------------------------------------------
+// 1. REFUSAL
+// ---------------------------------------------------------------------------
+
+describe('#26644 — an empty projection is refused, naming the spelling the caller wrote', () => {
+    it.each<[string, Record<string, unknown>]>([
+        ['$select', { $select: '' }],
+        ['$select', { $select: '   ' }],
+        ['$select', { $select: ',' }],
+        ['$select', { $select: ' , ,' }],
+        ['select', { select: '' }],
+        ['fields', { fields: '' }],
+        ['fields', { fields: [] }],
+        ['select', { select: [] }],
+    ])('list: %s in %j answers 400 VALIDATION_ERROR (was: every column)', async (param, query) => {
+        const err = await listRefusal(query);
+
+        expect(err.code).toBe('VALIDATION_ERROR');
+        expect(err.status).toBe(400);
+        expect(err.param).toBe(param);
+        expect(err.message).toContain(`Query parameter '${param}' names no field`);
+    });
+
+    it.each<[unknown]>([[''], ['  '], [','], [[]]])('by id: select %j answers 400 VALIDATION_ERROR (was: every column)', async (select) => {
+        const err = await getRefusal(select);
+
+        expect(err.code).toBe('VALIDATION_ERROR');
+        expect(err.status).toBe(400);
+        expect(err.param).toBe('select');
+        expect(err.message).toContain("Query parameter 'select' names no field");
+    });
+
+    it('quotes the value it refused', async () => {
+        const err = await listRefusal({ $select: ' , ' });
+
+        expect(err.message).toContain('got " , "');
+    });
+});
+
+describe('#26644 — a count flag that is not true or false is refused', () => {
+    it.each<[string, Record<string, unknown>]>([
+        ['$count', { $count: 'maybe' }],
+        ['$count', { $count: '' }],
+        ['$count', { $count: '1' }],
+        ['$count', { $count: 'TRUE' }],
+        ['count', { count: 'yes' }],
+        ['count', { count: 0 }],
+        ['count', { count: 1 }],
+    ])('%s in %j answers 400 VALIDATION_ERROR (was: counted)', async (param, query) => {
+        const err = await listRefusal(query);
+
+        expect(err.code).toBe('VALIDATION_ERROR');
+        expect(err.status).toBe(400);
+        expect(err.param).toBe(param);
+        expect(err.message).toContain(`Query parameter '${param}' must be true or false`);
+    });
+
+    it('quotes the value it refused, bounded', async () => {
+        const err = await listRefusal({ $count: 'x'.repeat(500) });
+
+        expect(err.message).toContain('got "xxxx');
+        expect(err.message.length).toBeLessThan(400);
+    });
+});
+
+// ---------------------------------------------------------------------------
+// 2. PRESERVATION
+// ---------------------------------------------------------------------------
+
+describe('#26644 — a well-formed projection and count flag answer exactly as before', () => {
+    it.each<[Record<string, unknown>, unknown]>([
+        [{ $select: 'name,status' }, ['name', 'status']],
+        [{ $select: ' name , status ,' }, ['name', 'status']],
+        [{ select: 'name' }, ['name']],
+        [{ fields: ['name'] }, ['name']],
+        [{}, undefined],
+        [{ fields: null }, undefined],
+    ])('list: %j → fields %j', async (query, fields) => {
+        const { p, find } = makeProtocol();
+        await p.findData({ object: 'invoice', query } as never);
+
+        expect(find).toHaveBeenCalledTimes(1);
+        expect(((find.mock.calls[0] as unknown[])[1] as Record<string, unknown>).fields).toEqual(fields);
+    });
+
+    it.each<[unknown, unknown]>([
+        ['name,status', ['name', 'status']],
+        [['name'], ['name']],
+        [undefined, undefined],
+        [null, undefined],
+    ])('by id: select %j → fields %j', async (select, fields) => {
+        const { p, findOne } = makeProtocol();
+        await p.getData({ object: 'invoice', id: 'r1', select } as never);
+
+        expect(findOne).toHaveBeenCalledTimes(1);
+        expect(((findOne.mock.calls[0] as unknown[])[1] as Record<string, unknown>).fields).toEqual(fields);
+    });
+
+    it.each<[Record<string, unknown>, boolean]>([
+        [{}, true],
+        [{ $count: 'true' }, true],
+        [{ count: true }, true],
+        [{ count: null }, true],
+        [{ $count: 'false' }, false],
+        [{ count: false }, false],
+    ])('%j with a window → counted: %s', async (query, counted) => {
+        const { p, count } = makeProtocol();
+        // The count runs only for a paged request: unpaged, the page IS the total.
+        const res: any = await p.findData({ object: 'invoice', query: { $top: '1', ...query } } as never);
+
+        expect(count).toHaveBeenCalledTimes(counted ? 1 : 0);
+        expect('total' in res).toBe(counted);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -156,6 +156,7 @@ import {
     foldQueryAliasSlots,
     QUERY_TRANSPORT_ALIAS_SLOTS, QUERY_TRANSPORT_DOLLAR_ALIASES, QUERY_TRANSPORT_DOLLAR_PARAMS,
     QueryPageWindowParamSchema,
+    QueryTransportParamsSchema,
     type QueryAliasConflict,
     type DroppedFieldsEvent, type QueryAST, type EngineQueryOptionsParsed,
     type ValidationAdvisoryEvent,
@@ -4619,6 +4620,73 @@ function pageWindowValue(value: unknown, param: string): number {
     throw err;
 }
 
+/**
+ * A refused query value as the refusal quotes it: JSON, so a blank or
+ * whitespace-only value is visible, and bounded, so a long one cannot bury
+ * the sentence around it.
+ */
+function quotedQueryValue(value: unknown): string {
+    // `String` for a number: `JSON.stringify(Infinity)` is `null`.
+    const shown = typeof value === 'number' ? String(value) : JSON.stringify(value) ?? String(value);
+    return shown.length > 64 ? `${shown.slice(0, 64)}…` : shown;
+}
+
+/**
+ * [#26644] A projection (`fields`, spelled `$select` / `select` on the wire) as
+ * the list of field names the engine reads, or a refusal naming the parameter
+ * the caller wrote.
+ *
+ * A comma string is split and trimmed as it always was; what is refused is a
+ * value that names NO field — a blank or comma-only string, or an empty array.
+ * It used to become `[]`, which `assertProjectionFieldsExist` has nothing to
+ * check in and every driver serves as no projection: every column, under a
+ * 200. Absent means "every column"; an empty projection asks for none, so
+ * reading it as absent is the widening {@link pageWindowValue} refuses on the
+ * row axis (a blank `$top`), here on the column axis.
+ *
+ * One rule for both routes that read a projection: the list's `findData` and
+ * the by-id `getData`.
+ */
+function projectionFieldsValue(value: string | readonly unknown[], param: string): unknown[] {
+    const fields = typeof value === 'string'
+        ? value.split(',').map((s) => s.trim()).filter(Boolean)
+        : [...value];
+    if (fields.length > 0) return fields;
+    const err: any = new Error(
+        `Query parameter '${param}' names no field; got ${quotedQueryValue(value)}. The query was not run: `
+        + 'an empty projection was read as no projection, which answers every column. Name at least one '
+        + 'field, or leave the parameter out to read every column.',
+    );
+    err.status = 400;
+    err.code = 'VALIDATION_ERROR';
+    err.param = param;
+    throw err;
+}
+
+/**
+ * [#26644] The response's total-count flag (`count`, spelled `$count` on the
+ * wire) as the boolean `findData` reads its opt-out from, or a refusal naming
+ * the parameter the caller wrote.
+ *
+ * The accept set is the spec's `QueryTransportParamsSchema.count`, parsed here
+ * rather than restated: the boolean, or the two strings a querystring carries
+ * it as (`null` is absent). Any other value used to stay as it was, so the
+ * `options.count === false` opt-out read it as "count anyway" — `?$count=maybe`
+ * reported a `total` exactly like `?$count=true`.
+ */
+function countFlagValue(value: unknown, param: string): boolean | null | undefined {
+    const flag = QueryTransportParamsSchema.shape.count.safeParse(value);
+    if (flag.success) return typeof flag.data === 'string' ? flag.data === 'true' : flag.data;
+    const err: any = new Error(
+        `Query parameter '${param}' must be true or false; got ${quotedQueryValue(value)}. The query was not `
+        + 'run: any other value was read as absent, which counts and reports a total.',
+    );
+    err.status = 400;
+    err.code = 'VALIDATION_ERROR';
+    err.param = param;
+    throw err;
+}
+
 /**
  * [#7321] Refuse a repeated occurrence of any list-query parameter this
  * normalizer reads as single-valued, and normalise the benign one-element array
@@ -12961,10 +13029,14 @@ export class ObjectStackProtocolImplementation implements
         // querystring param arrives as a bare string — `?fields=name` or the
         // folded `?select=` / `$select` spellings — which drivers' `.map()`
         // calls over `query.fields` would otherwise throw on.
+        //
+        // [#26644] …and a value naming no field is refused rather than becoming
+        // `[]`, which every driver serves as no projection at all (see
+        // {@link projectionFieldsValue}).
         const projectionKey = slotParam('fields');
-        if (typeof options.fields === 'string') {
-            options.fields = options.fields.split(',').map((s: string) => s.trim()).filter(Boolean);
-        } else if (options.fields !== undefined && !Array.isArray(options.fields)) {
+        if (typeof options.fields === 'string' || Array.isArray(options.fields)) {
+            options.fields = projectionFieldsValue(options.fields, projectionKey);
+        } else if (options.fields !== undefined) {
             delete options.fields;
         }
         // [#4226] Unknown projection columns are refused rather than dropped —
@@ -13138,11 +13210,12 @@ export class ObjectStackProtocolImplementation implements
         );
         if (narrowedSearchFields) options.searchFields = narrowedSearchFields;
 
-        // Boolean fields
-        for (const key of ['distinct', 'count']) {
-            if (options[key] === 'true') options[key] = true;
-            else if (options[key] === 'false') options[key] = false;
-        }
+        // Boolean fields. [#26644] `count` is CHECKED, not merely lowered: a
+        // value that is neither true nor false is refused rather than left to
+        // read as "count anyway" below (see {@link countFlagValue}).
+        if (options.distinct === 'true') options.distinct = true;
+        else if (options.distinct === 'false') options.distinct = false;
+        if (options.count != null) options.count = countFlagValue(options.count, slotParam('count'));
 
         // [#2926 ⑩] Every supported OData-style `$` alias has been consumed and
         // deleted above ($top/$skip/$orderby/$select/$count/$search/
@@ -13536,9 +13609,13 @@ export class ObjectStackProtocolImplementation implements
         // widened projection would recreate, one route over, the very split
         // ("two routes, opposite answers for one input") this issue was filed
         // about.
-        if (request.select) {
-            queryOptions.fields = typeof request.select === 'string'
-                ? request.select.split(',').map((s: string) => s.trim()).filter(Boolean)
+        //
+        // [#26644] …including the empty projection: a blank `?select=` was
+        // falsy and skipped, a comma-only one became `[]` — both answered every
+        // column. Asked on any value present, as the list path asks it.
+        if (request.select != null) {
+            queryOptions.fields = typeof request.select === 'string' || Array.isArray(request.select)
+                ? projectionFieldsValue(request.select, 'select')
                 : request.select;
             this.assertProjectionFieldsExist(request.object, queryOptions.fields, 'select');
         }