Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/finddata-pagination-total.md
Original file line numberDiff line numberDiff line change
@@ -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.
59 changes: 59 additions & 0 deletions packages/objectql/src/protocol-data.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
});
Expand DownExpand Up@@ -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);
});
});

// ═══════════════════════════════════════════════════════════════
Expand Down
47 changes: 40 additions & 7 deletions packages/objectql/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
};
}

Expand Down