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. 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, }; }