From c0510f4b7683612c3475dde713c1e0acd0f99248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Mon, 22 Jun 2026 23:36:11 +0800 Subject: [PATCH 1/2] fix(objectql): return real total/hasMore from findData (#2212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findData previously returned stub pagination metadata — `total` equal to the current page's record count and `hasMore` hard-coded to false. The frontend grid therefore believed every result set fit on a single page and never requested records beyond the first batch (e.g. row #51+ unreachable). When a `limit` is present the response is one page, so `records.length` is the page size, not the match total. Run engine.count() over the same `where` to report the true total and derive `hasMore` from offset + page vs total. engine.count() only honors `where`, so for `search`/`distinct` queries (which it can't reproduce) fall back to a page-local estimate instead of a wrong total. Without a limit the full set is returned, so its length is the total. The aggregation/grouped branch had the same bug: it sliced to `limit` but reported the sliced length as the total with hasMore=false. It now reports the full grouped count as total and hasMore from whether the slice dropped any groups. Adds 5 pagination tests to protocol-data.test.ts (39/39 pass). --- packages/objectql/src/protocol-data.test.ts | 59 +++++++++++++++++++++ packages/objectql/src/protocol.ts | 47 +++++++++++++--- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/packages/objectql/src/protocol-data.test.ts b/packages/objectql/src/protocol-data.test.ts index 9ac4d64366..fc460a369d 100644 --- a/packages/objectql/src/protocol-data.test.ts +++ b/packages/objectql/src/protocol-data.test.ts @@ -16,6 +16,7 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { mockEngine = { find: vi.fn().mockResolvedValue([]), findOne: vi.fn().mockResolvedValue(null), + count: vi.fn().mockResolvedValue(0), }; protocol = new ObjectStackProtocolImplementation(mockEngine); }); @@ -154,6 +155,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { }), ); }); + + // ─────────────────────────────────────────────────────────── + // Pagination metadata (issue #2212): with a `limit`, `total` must be + // the match total (via engine.count), not the page size; `hasMore` + // must reflect whether more pages remain. + // ─────────────────────────────────────────────────────────── + + it('returns the real match total (not the page size) when a limit is present', async () => { + mockEngine.find.mockResolvedValue(Array.from({ length: 100 }, (_, i) => ({ id: `r${i}` }))); + mockEngine.count.mockResolvedValue(3125); + + const result = await protocol.findData({ object: 'task', query: { $top: 100, $skip: 0 } }); + + expect(mockEngine.count).toHaveBeenCalledWith('task', expect.objectContaining({ where: undefined })); + expect(result.total).toBe(3125); + expect(result.hasMore).toBe(true); + }); + + it('forwards the same where filter to engine.count', async () => { + mockEngine.find.mockResolvedValue([{ id: 'r1' }]); + mockEngine.count.mockResolvedValue(42); + + await protocol.findData({ object: 'task', query: { $top: 10, filter: { status: 'open' } } }); + + expect(mockEngine.count).toHaveBeenCalledWith('task', expect.objectContaining({ where: { status: 'open' } })); + }); + + it('reports hasMore=false on the last page', async () => { + // offset 3120, 5 returned, total 3125 → 3120 + 5 === 3125 → no more. + mockEngine.find.mockResolvedValue(Array.from({ length: 5 }, (_, i) => ({ id: `r${i}` }))); + mockEngine.count.mockResolvedValue(3125); + + const result = await protocol.findData({ object: 'task', query: { $top: 100, $skip: 3120 } }); + + expect(result.total).toBe(3125); + expect(result.hasMore).toBe(false); + }); + + it('does NOT call engine.count when no limit is given (full result set)', async () => { + mockEngine.find.mockResolvedValue([{ id: 't1' }, { id: 't2' }]); + + const result = await protocol.findData({ object: 'task', query: {} }); + + expect(mockEngine.count).not.toHaveBeenCalled(); + expect(result.total).toBe(2); + expect(result.hasMore).toBe(false); + }); + + it('skips count for search queries and estimates hasMore from a full page', async () => { + // engine.count() can't reproduce a $search, so we must not call it; a + // full page (length === limit) implies there may be more. + mockEngine.find.mockResolvedValue(Array.from({ length: 10 }, (_, i) => ({ id: `r${i}` }))); + + const result = await protocol.findData({ object: 'task', query: { $top: 10, $search: 'foo' } }); + + expect(mockEngine.count).not.toHaveBeenCalled(); + expect(result.hasMore).toBe(true); + }); }); // ═══════════════════════════════════════════════════════════════ diff --git a/packages/objectql/src/protocol.ts b/packages/objectql/src/protocol.ts index c083866bfe..f7a981b4e7 100644 --- a/packages/objectql/src/protocol.ts +++ b/packages/objectql/src/protocol.ts @@ -2164,26 +2164,59 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { aggregations: options.aggregations, context: options.context, } as any); - // Apply limit client-side (EngineAggregateOptions doesn't carry limit) + // Apply limit client-side (EngineAggregateOptions doesn't carry limit). + // `records` is the full grouped set, so its length IS the real total + // and `hasMore` follows from whether the slice dropped any groups. const limited = typeof options.limit === 'number' && options.limit > 0 ? records.slice(0, options.limit) : records; return { object: request.object, records: limited, - total: limited.length, - hasMore: false, + total: records.length, + hasMore: limited.length < records.length, }; } const records = await this.engine.find(request.object, options); - // Spec: FindDataResponseSchema — only `records` is returned. - // OData `value` adaptation (if needed) is handled in the HTTP dispatch layer. + // Pagination metadata. When a `limit` is present the response is a single + // page, so `records.length` is the page size — NOT the match total. Run a + // count over the same `where` so the client can render total pages and know + // whether more pages remain (true server-side pagination). Without a limit + // the full result set is returned, so its length already IS the total. + // + // engine.count() only honors `where`; a `search`/`distinct` query can't be + // reproduced by it, so for those we skip the count and fall back to a + // page-local estimate (a full page implies there may be more) rather than + // 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 hasMore = false; + if (pageLimit !== undefined) { + const countable = options.search == null && options.distinct == null; + if (countable) { + try { + total = 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; + } + hasMore = pageOffset + records.length < total; + } else { + hasMore = records.length === pageLimit; + total = pageOffset + records.length + (hasMore ? 1 : 0); + } + } return { object: request.object, records, - total: records.length, - hasMore: false + total, + hasMore, }; } From 68b2de7c81f9cf954092b0248fffdd2f9c8d3181 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 23 Jun 2026 09:42:49 +0800 Subject: [PATCH 2/2] chore(changeset): add patch changeset for findData pagination fix (#2212) Closes the missing-changeset CI gate; @objectstack/objectql user-facing fix. Co-Authored-By: Claude Opus 4.8 --- .changeset/finddata-pagination-total.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .changeset/finddata-pagination-total.md diff --git a/.changeset/finddata-pagination-total.md b/.changeset/finddata-pagination-total.md new file mode 100644 index 0000000000..0fe41f6793 --- /dev/null +++ b/.changeset/finddata-pagination-total.md @@ -0,0 +1,20 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): return the real `total`/`hasMore` from `findData` (#2212) + +`ObjectStackProtocolImplementation.findData` previously returned placeholder +pagination metadata: `total` was always the **page** length and `hasMore` was +always `false`. Front-end tables therefore believed every result set was a +single page and never requested records past the first batch (e.g. row 51+ was +unreachable). + +For a normal limited query it now runs `engine.count()` over the same `where` to +get the true match total and derives `hasMore` from `offset + page length < total`. +`engine.count()` only honors `where`, so `search`/`distinct` queries skip the +count and fall back to a page-local estimate (a full page implies there may be +more) instead of reporting a wrong total. Unlimited queries return the full set, +whose length already is the total. The aggregate/group branch now reports the +full group count as `total` with `hasMore` reflecting whether the client-side +slice dropped any groups.