Skip to content

Commit 211425e

Browse files
baozhoutaoos-zhuangclaude
authored
fix(objectql): findData 返回真实 total/hasMore (#2212) (#2222)
* fix(objectql): return real total/hasMore from findData (#2212) 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). * 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 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6d3bf54 commit 211425e

3 files changed

Lines changed: 119 additions & 7 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): return the real `total`/`hasMore` from `findData` (#2212)
6+
7+
`ObjectStackProtocolImplementation.findData` previously returned placeholder
8+
pagination metadata: `total` was always the **page** length and `hasMore` was
9+
always `false`. Front-end tables therefore believed every result set was a
10+
single page and never requested records past the first batch (e.g. row 51+ was
11+
unreachable).
12+
13+
For a normal limited query it now runs `engine.count()` over the same `where` to
14+
get the true match total and derives `hasMore` from `offset + page length < total`.
15+
`engine.count()` only honors `where`, so `search`/`distinct` queries skip the
16+
count and fall back to a page-local estimate (a full page implies there may be
17+
more) instead of reporting a wrong total. Unlimited queries return the full set,
18+
whose length already is the total. The aggregate/group branch now reports the
19+
full group count as `total` with `hasMore` reflecting whether the client-side
20+
slice dropped any groups.

packages/objectql/src/protocol-data.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
1616
mockEngine = {
1717
find: vi.fn().mockResolvedValue([]),
1818
findOne: vi.fn().mockResolvedValue(null),
19+
count: vi.fn().mockResolvedValue(0),
1920
};
2021
protocol = new ObjectStackProtocolImplementation(mockEngine);
2122
});
@@ -154,6 +155,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
154155
}),
155156
);
156157
});
158+
159+
// ───────────────────────────────────────────────────────────
160+
// Pagination metadata (issue #2212): with a `limit`, `total` must be
161+
// the match total (via engine.count), not the page size; `hasMore`
162+
// must reflect whether more pages remain.
163+
// ───────────────────────────────────────────────────────────
164+
165+
it('returns the real match total (not the page size) when a limit is present', async () => {
166+
mockEngine.find.mockResolvedValue(Array.from({ length: 100 }, (_, i) => ({ id: `r${i}` })));
167+
mockEngine.count.mockResolvedValue(3125);
168+
169+
const result = await protocol.findData({ object: 'task', query: { $top: 100, $skip: 0 } });
170+
171+
expect(mockEngine.count).toHaveBeenCalledWith('task', expect.objectContaining({ where: undefined }));
172+
expect(result.total).toBe(3125);
173+
expect(result.hasMore).toBe(true);
174+
});
175+
176+
it('forwards the same where filter to engine.count', async () => {
177+
mockEngine.find.mockResolvedValue([{ id: 'r1' }]);
178+
mockEngine.count.mockResolvedValue(42);
179+
180+
await protocol.findData({ object: 'task', query: { $top: 10, filter: { status: 'open' } } });
181+
182+
expect(mockEngine.count).toHaveBeenCalledWith('task', expect.objectContaining({ where: { status: 'open' } }));
183+
});
184+
185+
it('reports hasMore=false on the last page', async () => {
186+
// offset 3120, 5 returned, total 3125 → 3120 + 5 === 3125 → no more.
187+
mockEngine.find.mockResolvedValue(Array.from({ length: 5 }, (_, i) => ({ id: `r${i}` })));
188+
mockEngine.count.mockResolvedValue(3125);
189+
190+
const result = await protocol.findData({ object: 'task', query: { $top: 100, $skip: 3120 } });
191+
192+
expect(result.total).toBe(3125);
193+
expect(result.hasMore).toBe(false);
194+
});
195+
196+
it('does NOT call engine.count when no limit is given (full result set)', async () => {
197+
mockEngine.find.mockResolvedValue([{ id: 't1' }, { id: 't2' }]);
198+
199+
const result = await protocol.findData({ object: 'task', query: {} });
200+
201+
expect(mockEngine.count).not.toHaveBeenCalled();
202+
expect(result.total).toBe(2);
203+
expect(result.hasMore).toBe(false);
204+
});
205+
206+
it('skips count for search queries and estimates hasMore from a full page', async () => {
207+
// engine.count() can't reproduce a $search, so we must not call it; a
208+
// full page (length === limit) implies there may be more.
209+
mockEngine.find.mockResolvedValue(Array.from({ length: 10 }, (_, i) => ({ id: `r${i}` })));
210+
211+
const result = await protocol.findData({ object: 'task', query: { $top: 10, $search: 'foo' } });
212+
213+
expect(mockEngine.count).not.toHaveBeenCalled();
214+
expect(result.hasMore).toBe(true);
215+
});
157216
});
158217

159218
// ═══════════════════════════════════════════════════════════════

packages/objectql/src/protocol.ts

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,26 +2164,59 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
21642164
aggregations: options.aggregations,
21652165
context: options.context,
21662166
} as any);
2167-
// Apply limit client-side (EngineAggregateOptions doesn't carry limit)
2167+
// Apply limit client-side (EngineAggregateOptions doesn't carry limit).
2168+
// `records` is the full grouped set, so its length IS the real total
2169+
// and `hasMore` follows from whether the slice dropped any groups.
21682170
const limited = typeof options.limit === 'number' && options.limit > 0
21692171
? records.slice(0, options.limit)
21702172
: records;
21712173
return {
21722174
object: request.object,
21732175
records: limited,
2174-
total: limited.length,
2175-
hasMore: false,
2176+
total: records.length,
2177+
hasMore: limited.length < records.length,
21762178
};
21772179
}
21782180

21792181
const records = await this.engine.find(request.object, options);
2180-
// Spec: FindDataResponseSchema — only `records` is returned.
2181-
// OData `value` adaptation (if needed) is handled in the HTTP dispatch layer.
2182+
// Pagination metadata. When a `limit` is present the response is a single
2183+
// page, so `records.length` is the page size — NOT the match total. Run a
2184+
// count over the same `where` so the client can render total pages and know
2185+
// whether more pages remain (true server-side pagination). Without a limit
2186+
// the full result set is returned, so its length already IS the total.
2187+
//
2188+
// engine.count() only honors `where`; a `search`/`distinct` query can't be
2189+
// reproduced by it, so for those we skip the count and fall back to a
2190+
// page-local estimate (a full page implies there may be more) rather than
2191+
// reporting a wrong total.
2192+
const pageLimit = typeof options.limit === 'number' && options.limit > 0 ? options.limit : undefined;
2193+
const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0;
2194+
let total = records.length;
2195+
let hasMore = false;
2196+
if (pageLimit !== undefined) {
2197+
const countable = options.search == null && options.distinct == null;
2198+
if (countable) {
2199+
try {
2200+
total = await this.engine.count(request.object, {
2201+
where: options.where,
2202+
context: options.context,
2203+
} as any);
2204+
} catch {
2205+
// engine.count() has its own find().length fallback; if it still
2206+
// throws, degrade to a page-local total rather than failing the list.
2207+
total = pageOffset + records.length;
2208+
}
2209+
hasMore = pageOffset + records.length < total;
2210+
} else {
2211+
hasMore = records.length === pageLimit;
2212+
total = pageOffset + records.length + (hasMore ? 1 : 0);
2213+
}
2214+
}
21822215
return {
21832216
object: request.object,
21842217
records,
2185-
total: records.length,
2186-
hasMore: false
2218+
total,
2219+
hasMore,
21872220
};
21882221
}
21892222

0 commit comments

Comments
 (0)