perf(security,protocol): stop asking the same question twice within one request

.changeset/count-opt-out-and-permission-set-memo.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/metadata-protocol": minor
+"@objectstack/plugin-security": patch
+---
+
+Stop issuing two DB queries for questions already answered earlier in the same
+request (#10757). One authenticated `GET /data/:object?$top=1` measured **24 DB
+queries before, 23 after** — **22** when the caller opts out of the count.
+Measured with `X-OS-Debug-Timing: json` on `pnpm dev:crm`, whose `Server-Timing`
+carries `db;dur=…;desc="N queries"`.
+
+**`$count=false` now skips the COUNT query** (`@objectstack/metadata-protocol`).
+The parameter has been declared (`ODataQuerySchema.$count`), aliased on the wire
+(`$count` → `count`), reserved out of the implicit-field-filter bucket,
+arity-checked and boolean-coerced for a long time — and then deleted unread, so
+every paginated list ran `engine.count()` whether or not the caller wanted a
+total. It is honoured now:
+
+```
+GET /data/task?$top=25 → { records, total, hasMore } (unchanged)
+GET /data/task?$top=25&$count=true → { records, total, hasMore } (unchanged)
+GET /data/task?$top=25&$count=false → { records, hasMore } (no COUNT query)
+```
+
+Read the shape of that carefully before adopting it:
+
+- **Only an explicit `false` opts out.** An ABSENT `$count` still counts and
+ still reports `total`. OData reads absent as "omit the count", and taking that
+ reading here would silently strip `total` from every existing caller — none of
+ them send the parameter, all of them read the number. The asymmetry is
+ deliberate and pinned by tests.
+- **`total` is OMITTED, never estimated.** `FindDataResponse.total` is declared
+ optional ("if requested"), so absent is the declared shape for "not
+ requested". A caller that opted out and then reads `total` gets `undefined`,
+ not a plausible-looking guess — guard the read (`total ?? undefined`) or do
+ not send `$count=false`.
+- **`hasMore` is still answered**, from the page alone: a full page means there
+ may be more. Same page-local rule the `$search` path already uses.
+
+**A find and its COUNT resolve permission sets once, not twice**
+(`@objectstack/plugin-security`). `findData` answers a paginated list with two
+engine operations, and the security middleware runs on both; each pass re-read
+`sys_permission_set` for the same context with identical bindings. The
+resolution is now memoized per execution context — a `WeakMap` keyed on the
+context object, which is built once per request and collected with it, so
+nothing outlives the caller it was resolved for — and **retired by any write**:
+a process-wide epoch is bumped on every `insert`/`update`/`delete` the engine
+middleware sees, ahead of the `isSystem` bypass so a seeder, a package publish
+or an auto-org-admin grant invalidates too. A context whose grants are rewritten
+in place re-resolves as well (the memo key covers `positions`, `permissions`,
+`principalKind` and the presence of `userId`). No authorization answer is reused
+across a write, across a context, or across a request.
+
+Not a fix for the whole cost: the remaining ~22 queries per authenticated
+request are session resolution, grant resolution, localization and metadata
+reads that repeat on every request. Removing those needs cross-request caching
+with an invalidation design, which is deliberately not in this change.

packages/metadata-protocol/src/protocol.count-opt-out.test.ts

@@ -0,0 +1,168 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #10757 — `$count=false` skips the COUNT query.
+ *
+ * ## What was wrong
+ *
+ * `$count` has been a fully-plumbed parameter for a long time: declared in the
+ * spec (`ODataQuerySchema.$count`, `packages/spec/src/api/odata.zod.ts`),
+ * aliased on the wire (`$count` → `count`, `WIRE_DOLLAR_ALIASES`), reserved out
+ * of the implicit-field-filter bucket (`RESERVED_LIST_QUERY_PARAMS`),
+ * arity-checked (`protocol.query-param-arity.test.ts`) and boolean-coerced —
+ * and then DELETED unread by the protocol-key strip in `findData`. So every
+ * paginated list issued `engine.count()` whether or not the caller wanted a
+ * `total`, which on a remote database is a whole round trip per request. The
+ * measured trace on a real stack put it at query 24 of 24 for one
+ * `GET /data/:object?$top=1`.
+ *
+ * ## The two directions this suite pins, and why both are needed
+ *
+ * 1. **The OPT-OUT works** — an explicit `false` (either spelling) means no
+ * `engine.count()` call and no `total` key. `expect(count).not.toHaveBeenCalled()`
+ * is the load-bearing assertion; asserting only the absent `total` would
+ * stay green if a future edit ran the query and merely dropped the number,
+ * which is the whole cost with none of the saving.
+ *
+ * 2. **Nothing else changed** — absent `$count`, and explicit `$count=true`,
+ * both still count and still report `total`. This is the direction that
+ * makes the opt-out safe to ship: OData reads an ABSENT `$count` as "omit
+ * the count", and taking that reading here would silently strip `total`
+ * from every existing caller (none of them send the parameter, all of them
+ * read the number). The asymmetry is deliberate, so it is pinned rather
+ * than left to be "tidied up" later.
+ *
+ * `total` is OMITTED rather than estimated — `FindDataResponseSchema` declares
+ * it optional ("if requested"), and a page-local guess handed back to a caller
+ * who declined the real number is how an estimate ends up rendered as a record
+ * count. `hasMore` is still answered from the page alone.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+const SCHEMA = {
+ name: 'invoice',
+ nameField: 'name',
+ fields: {
+ name: { name: 'name', type: 'text' },
+ status: { name: 'status', type: 'text' },
+ },
+};
+
+function makeProtocol(pageSize: number) {
+ const find = vi.fn(async () => Array.from({ length: pageSize }, (_, i) => ({ id: `r${i}` })));
+ const count = vi.fn(async () => 3125);
+ const engine = {
+ registry: { getObject: (n: string) => (n === 'invoice' ? SCHEMA : undefined) },
+ find,
+ count,
+ aggregate: vi.fn(async () => [] as unknown[]),
+ };
+ return { p: new ObjectStackProtocolImplementation(engine as any), find, count };
+}
+
+describe('[#10757] findData honours $count=false', () => {
+ describe('opt-out — the COUNT query is not issued', () => {
+ // Both wire spellings reach the same normalized `count` slot; a fix that
+ // read only one of them would leave the other paying for the query.
+ for (const spelling of ['$count', 'count'] as const) {
+ it(`?${spelling}=false skips engine.count() and omits total`, async () => {
+ const { p, count } = makeProtocol(1);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $top: 1, [spelling]: 'false' },
+ } as never);
+
+ expect(count).not.toHaveBeenCalled();
+ expect('total' in (result as object)).toBe(false);
+ });
+ }
+
+ it('accepts the already-boolean form a POST body carries', async () => {
+ const { p, count } = makeProtocol(1);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $top: 1, count: false },
+ } as never);
+
+ expect(count).not.toHaveBeenCalled();
+ expect('total' in (result as object)).toBe(false);
+ });
+
+ it('still answers hasMore from the page: a FULL page means there may be more', async () => {
+ const { p } = makeProtocol(10);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $top: 10, $count: 'false' },
+ } as never);
+
+ expect(result.hasMore).toBe(true);
+ });
+
+ it('…and a SHORT page means there are not', async () => {
+ const { p } = makeProtocol(3);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $top: 10, $count: 'false' },
+ } as never);
+
+ expect(result.hasMore).toBe(false);
+ });
+
+ it('leaves `count` off the engine option bag (it is a protocol-layer flag)', async () => {
+ const { p, find } = makeProtocol(1);
+
+ await p.findData({ object: 'invoice', query: { $top: 1, $count: 'false' } } as never);
+
+ const bag = (find.mock.calls[0] as unknown[])[1] as Record<string, unknown>;
+ expect('count' in bag).toBe(false);
+ expect('$count' in bag).toBe(false);
+ });
+ });
+
+ describe('unchanged for every caller that does not opt out', () => {
+ it('an ABSENT $count still counts and still reports total', async () => {
+ const { p, count } = makeProtocol(1);
+
+ const result = await p.findData({ object: 'invoice', query: { $top: 1 } } as never);
+
+ expect(count).toHaveBeenCalledTimes(1);
+ expect(result.total).toBe(3125);
+ expect(result.hasMore).toBe(true);
+ });
+
+ it('an explicit $count=true still counts and still reports total', async () => {
+ const { p, count } = makeProtocol(1);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $top: 1, $count: 'true' },
+ } as never);
+
+ expect(count).toHaveBeenCalledTimes(1);
+ expect(result.total).toBe(3125);
+ });
+
+ it('$count=false without a limit is a no-op — the full set is already the total', async () => {
+ // No `limit` ⇒ the whole result set came back, so `records.length` IS
+ // the total and `engine.count()` was never called even before #10757.
+ // Pinned so the opt-out cannot accidentally start suppressing a total
+ // that costs nothing.
+ const { p, count } = makeProtocol(4);
+
+ const result = await p.findData({
+ object: 'invoice',
+ query: { $count: 'false' },
+ } as never);
+
+ expect(count).not.toHaveBeenCalled();
+ expect(result.total).toBe(4);
+ expect(result.hasMore).toBe(false);
+ });
+ });
+});

packages/metadata-protocol/src/protocol.ts

@@ -8881,7 +8881,8 @@ export class ObjectStackProtocolImplementation implements
  // ride the spread into the engine AST and OVERRIDE the resolved
  // object, splitting `ast.object` from the table actually queried —
  // a mismatch is refused, never resolved by picking a winner.
- // - `count`: a response-shape flag this method consumed above.
+ // - `count`: a response-shape flag this method consumed above (and,
+ // since #10757, one it actually HONOURS — see `countOptOut` below).
  // - QueryAST tombstones (`cursor`/`joins`/`windowFunctions`/
  // `distinct`): reserved at the wire gate so they are not read as
  // field filters; on the wire they stay ignored-with-tombstone-docs
@@ -8899,6 +8900,22 @@ export class ObjectStackProtocolImplementation implements
  err.code = 'QUERY_OBJECT_MISMATCH';
  throw err;
  }
+ // [#10757] `$count=false` — read the flag BEFORE the strip below deletes
+ // it, because the strip is what kept it from ever being honoured: the
+ // parameter has been declared (`ODataQuerySchema.$count`,
+ // `packages/spec/src/api/odata.zod.ts`), aliased (`$count` → `count`,
+ // {@link WIRE_DOLLAR_ALIASES}), reserved from the implicit-field-filter
+ // bucket ({@link RESERVED_LIST_QUERY_PARAMS}), arity-checked and boolean-
+ // coerced — and then deleted unread, so every list request paid for the
+ // COUNT query below whether or not the caller wanted a `total`.
+ //
+ // Only an EXPLICIT `false` opts out. An absent `$count` keeps today's
+ // behaviour (count runs, `total` is reported) rather than taking OData's
+ // "absent means omit" reading: every existing caller sends nothing and
+ // reads `total`, so the OData default would silently break all of them.
+ // The parameter is therefore an opt-OUT here, and that asymmetry is
+ // deliberate — see the changeset for the wording that ships to consumers.
+ const countOptOut = options.count === false;
  for (const k of ['object', 'count', 'joins', 'windowFunctions', 'cursor', 'distinct', 'having']) {
  delete options[k];
  }
@@ -8915,26 +8932,55 @@ export class ObjectStackProtocolImplementation implements
  // reporting a wrong total.
  const pageLimit = typeof options.limit === 'number' && options.limit > 0 ? options.limit : undefined;
  const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0;
- let total = records.length;
+ let total: number | undefined = records.length;
  let hasMore = false;
  if (pageLimit !== undefined) {
  // `distinct` used to suppress the count here too — #4286 finding 2:
  // the flag's ONLY observable effect platform-wide, on a capability
  // that never deduplicated a row. Removed with `query.distinct`
  // (tombstoned in spec 17); `total`/`hasMore` are truthful again.
  const countable = options.search == null;
- if (countable) {
+ if (countOptOut) {
+ // [#10757] The caller said it does not need `total`, so the
+ // COUNT query is not issued at all — that is the whole point of
+ // the parameter, and on a remote database it is a full round
+ // trip saved per list request.
+ //
+ // `total` is OMITTED rather than estimated. `FindDataResponse`
+ // declares it optional ("Total number of records matching the
+ // filter (IF REQUESTED)",
+ // `packages/spec/src/api/protocol.zod.ts`), so absent is the
+ // declared shape for "not requested" — and it is the only
+ // honest one: the `search` branch below reports an estimate
+ // because it has no better number to give, while here a real
+ // number was available and the caller declined it. Handing back
+ // a plausible-looking guess under those circumstances is how a
+ // page-local estimate ends up rendered as a record count.
+ //
+ // `hasMore` is still answered, from the page alone: a FULL page
+ // means there may be more. Same page-local rule the search
+ // branch uses, and it never over-reports the data — it can only
+ // say "maybe more" on an exactly-full last page.
+ hasMore = records.length === pageLimit;
+ total = undefined;
+ } else if (countable) {
+ // [#10757] `counted` is a separate, always-assigned local so
+ // `hasMore` below compares against a `number`: `total` became
+ // optional when `$count=false` gained the right to omit it, and
+ // TypeScript cannot narrow it back across the try/catch.
+ let counted: number;
  try {
-total = await this.engine.count(request.object, {
+counted = await this.engine.count(request.object, {
  where: options.where,
  context: options.context,
  } as any);
  } catch {
  // engine.count() has its own find().length fallback; if it still
  // throws, degrade to a page-local total rather than failing the list.
-total = pageOffset + records.length;
+counted = pageOffset + records.length;
  }
- hasMore = pageOffset + records.length < total;
+ total = counted;
+ hasMore = pageOffset + records.length < counted;
  } else {
  hasMore = records.length === pageLimit;
  total = pageOffset + records.length + (hasMore ? 1 : 0);
@@ -8943,7 +8989,11 @@ export class ObjectStackProtocolImplementation implements
  return {
  object: request.object,
  records,
- total,
+ // [#10757] Omitted, not `undefined`-valued: a JSON body carrying
+ // `"total": null` (or a key some serializers keep) reads as "the
+ // total is nothing", which is a different claim from "no total was
+ // requested". The key is simply absent.
+ ...(total === undefined ? {} : { total }),
  hasMore,
  };
  }