Skip to content

Commit 2913589

Browse files
committed
feat(core): paginate unified search results per category
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
1 parent 4626e70 commit 2913589

2 files changed

Lines changed: 202 additions & 13 deletions

File tree

core/src/services/UnifiedSearchController.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ type CategorySearchItem = {
99
entries: unknown[]
1010
cursor: string | null
1111
hasMore: boolean
12+
loadMoreFailed: boolean
1213
}
1314

1415
/**
1516
* Runs a unified search across categories in priority order, blocking
1617
* lower-priority results until their predecessors arrive or a timer reveals them.
1718
*/
1819
export class UnifiedSearchController {
20+
private query: string = ''
1921
private searchItems: Record<string, CategorySearchItem> = {}
2022
private requestId: number = 0
2123
private revealTimer: ReturnType<typeof setTimeout> | null = null
@@ -32,6 +34,7 @@ export class UnifiedSearchController {
3234
this.searchItems = {}
3335
this.requestId++
3436
const dispatchId = this.requestId
37+
this.query = query
3538

3639
this.startRevealTimer()
3740

@@ -41,10 +44,11 @@ export class UnifiedSearchController {
4144
entries: [],
4245
cursor: null,
4346
hasMore: false,
47+
loadMoreFailed: false,
4448
}
4549
const { request, cancel } = unifiedSearch({
4650
type: category,
47-
query,
51+
query: this.query,
4852
cursor: null,
4953
})
5054

@@ -62,6 +66,7 @@ export class UnifiedSearchController {
6266
entries,
6367
cursor,
6468
hasMore,
69+
loadMoreFailed: false,
6570
}
6671

6772
this.reconcileCategoryStatuses(categories)
@@ -74,12 +79,55 @@ export class UnifiedSearchController {
7479
entries: [],
7580
cursor: null,
7681
hasMore: false,
82+
loadMoreFailed: false,
7783
}
7884
this.reconcileCategoryStatuses(categories)
7985
})
8086
})
8187
}
8288

89+
/**
90+
* Fetch the next page for one category and append it. A no-op unless the
91+
* category is loaded with more pages. On failure the existing results stay
92+
* and `loadMoreFailed` is raised, so calling again retries.
93+
*
94+
* @param category the category id to page
95+
*/
96+
loadMore(category: string): void {
97+
const dispatchId = this.requestId
98+
const categoryItem = this.searchItems[category]
99+
if (!categoryItem || !categoryItem.hasMore || categoryItem.status !== 'loaded') {
100+
return
101+
}
102+
categoryItem.status = 'loading'
103+
categoryItem.loadMoreFailed = false
104+
105+
const { request, cancel } = unifiedSearch({
106+
type: category,
107+
query: this.query,
108+
cursor: categoryItem.cursor,
109+
})
110+
111+
this.searchAbortHandlers.push(cancel)
112+
113+
request().then((response) => {
114+
if (this.requestId !== dispatchId) {
115+
return
116+
}
117+
const { entries, cursor, hasMore } = response.data.ocs.data
118+
categoryItem.entries.push(...entries)
119+
categoryItem.cursor = cursor
120+
categoryItem.hasMore = hasMore
121+
categoryItem.status = 'loaded'
122+
}).catch(() => {
123+
if (this.requestId !== dispatchId) {
124+
return
125+
}
126+
categoryItem.status = 'loaded'
127+
categoryItem.loadMoreFailed = true
128+
})
129+
}
130+
83131
/**
84132
* A shallow copy of the current per-category state, safe to read for rendering.
85133
*

core/src/tests/services/UnifiedSearchController.spec.ts

Lines changed: 153 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,26 @@ function deferredProvider() {
2929
}
3030
}
3131

32+
/**
33+
* Deferred stand-in that serves successive pages. Each `request()` call takes
34+
* the next page; a test resolves page N on demand with `resolvePage(n, data)`,
35+
* where `data` is the full `{ entries, cursor, hasMore }` payload.
36+
*/
37+
function pagedProvider() {
38+
const pages: ReturnType<typeof Promise.withResolvers<{ entries: unknown[], cursor: string | null, hasMore: boolean }>>[] = []
39+
const pageAt = (index: number) => (pages[index] ??= Promise.withResolvers())
40+
let call = 0
41+
return {
42+
cancel: vi.fn(),
43+
request: async () => {
44+
const data = await pageAt(call++).promise
45+
return { data: { ocs: { data } } }
46+
},
47+
resolvePage: (index: number, data: { entries: unknown[], cursor: string | null, hasMore: boolean }) => pageAt(index).resolve(data),
48+
rejectPage: (index: number, reason?: unknown) => pageAt(index).reject(reason),
49+
}
50+
}
51+
3252
/**
3353
* Register one deferred provider per category type on the mocked service.
3454
* Returns the map so a test can resolve/reject a specific category on demand,
@@ -44,7 +64,7 @@ function mockProviders(types: string[]) {
4464
* The initial per-category state before any provider has resolved. Identical
4565
* for every pending category, so tests assert against this shared shape.
4666
*/
47-
const loading = { status: 'loading', entries: [], cursor: null, hasMore: false }
67+
const loading = { status: 'loading', entries: [], cursor: null, hasMore: false, loadMoreFailed: false }
4868

4969
beforeEach(() => {
5070
vi.useFakeTimers()
@@ -81,7 +101,7 @@ describe('UnifiedSearchController', () => {
81101
await vi.advanceTimersByTimeAsync(0)
82102

83103
expect(searchController.getSnapshot()).toEqual({
84-
files: { status: 'loaded', entries: ['Some result'], cursor: undefined, hasMore: undefined },
104+
files: { status: 'loaded', entries: ['Some result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
85105
})
86106
})
87107
})
@@ -100,7 +120,7 @@ describe('UnifiedSearchController', () => {
100120
expect(searchController.getSnapshot()).toEqual({
101121
files: loading,
102122
talk: loading,
103-
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined },
123+
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
104124
})
105125
})
106126

@@ -117,7 +137,7 @@ describe('UnifiedSearchController', () => {
117137
expect(searchController.getSnapshot()).toEqual({
118138
files: loading,
119139
talk: loading,
120-
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined },
140+
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
121141
})
122142

123143
providers.files.resolve(['Files result'])
@@ -126,9 +146,9 @@ describe('UnifiedSearchController', () => {
126146
await vi.advanceTimersByTimeAsync(0)
127147

128148
expect(searchController.getSnapshot()).toEqual({
129-
files: { status: 'loaded', entries: ['Files result'], cursor: undefined, hasMore: undefined },
130-
talk: { status: 'loaded', entries: ['Talk result'], cursor: undefined, hasMore: undefined },
131-
deck: { status: 'loaded', entries: ['Deck result'], cursor: undefined, hasMore: undefined },
149+
files: { status: 'loaded', entries: ['Files result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
150+
talk: { status: 'loaded', entries: ['Talk result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
151+
deck: { status: 'loaded', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
132152
})
133153
})
134154

@@ -143,7 +163,7 @@ describe('UnifiedSearchController', () => {
143163

144164
expect(searchController.getSnapshot()).toEqual({
145165
files: loading,
146-
talk: { status: 'blocked', entries: ['Talk result'], cursor: undefined, hasMore: undefined },
166+
talk: { status: 'blocked', entries: ['Talk result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
147167
deck: loading,
148168
})
149169

@@ -154,8 +174,8 @@ describe('UnifiedSearchController', () => {
154174
await vi.advanceTimersByTimeAsync(0)
155175

156176
expect(searchController.getSnapshot()).toEqual({
157-
files: { status: 'failed', entries: [], cursor: null, hasMore: false },
158-
talk: { status: 'loaded', entries: ['Talk result'], cursor: undefined, hasMore: undefined },
177+
files: { status: 'failed', entries: [], cursor: null, hasMore: false, loadMoreFailed: false },
178+
talk: { status: 'loaded', entries: ['Talk result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
159179
deck: loading,
160180
})
161181
})
@@ -192,6 +212,7 @@ describe('UnifiedSearchController', () => {
192212
entries: ['Live files'],
193213
cursor: undefined,
194214
hasMore: undefined,
215+
loadMoreFailed: false,
195216
})
196217
})
197218
})
@@ -258,6 +279,126 @@ describe('UnifiedSearchController', () => {
258279
})
259280
})
260281

282+
describe('pagination', () => {
283+
it('appends the next page of results when loadMore is called', async () => {
284+
const files = pagedProvider()
285+
service.search.mockReturnValue(files)
286+
287+
const searchController = new UnifiedSearchController()
288+
searchController.search('query', ['files'])
289+
290+
files.resolvePage(0, { entries: ['a'], cursor: 'cursor-1', hasMore: true })
291+
await vi.advanceTimersByTimeAsync(0)
292+
293+
expect(searchController.getSnapshot().files).toEqual({
294+
status: 'loaded',
295+
entries: ['a'],
296+
cursor: 'cursor-1',
297+
hasMore: true,
298+
loadMoreFailed: false,
299+
})
300+
301+
searchController.loadMore('files')
302+
303+
files.resolvePage(1, { entries: ['b'], cursor: 'cursor-2', hasMore: false })
304+
await vi.advanceTimersByTimeAsync(0)
305+
306+
expect(searchController.getSnapshot().files).toEqual({
307+
status: 'loaded',
308+
entries: ['a', 'b'],
309+
cursor: 'cursor-2',
310+
hasMore: false,
311+
loadMoreFailed: false,
312+
})
313+
})
314+
315+
it('re-dispatches with the stored cursor', async () => {
316+
const files = pagedProvider()
317+
service.search.mockReturnValue(files)
318+
319+
const searchController = new UnifiedSearchController()
320+
searchController.search('query', ['files'])
321+
322+
files.resolvePage(0, { entries: ['a'], cursor: 'cursor-1', hasMore: true })
323+
await vi.advanceTimersByTimeAsync(0)
324+
325+
searchController.loadMore('files')
326+
327+
expect(service.search).toHaveBeenLastCalledWith(expect.objectContaining({ type: 'files', query: 'query', cursor: 'cursor-1' }))
328+
})
329+
330+
it('flags a page-load failure without dropping the results already loaded', async () => {
331+
const files = pagedProvider()
332+
service.search.mockReturnValue(files)
333+
334+
const searchController = new UnifiedSearchController()
335+
searchController.search('query', ['files'])
336+
337+
files.resolvePage(0, { entries: ['a'], cursor: 'cursor-1', hasMore: true })
338+
await vi.advanceTimersByTimeAsync(0)
339+
340+
searchController.loadMore('files')
341+
files.rejectPage(1, new Error('network'))
342+
await vi.advanceTimersByTimeAsync(0)
343+
344+
// Results stay put, the category is still loaded, and hasMore stays true
345+
// so the next loadMore retries. The failure is surfaced on its own flag.
346+
expect(searchController.getSnapshot().files).toEqual({
347+
status: 'loaded',
348+
entries: ['a'],
349+
cursor: 'cursor-1',
350+
hasMore: true,
351+
loadMoreFailed: true,
352+
})
353+
})
354+
355+
it('clears the failure flag when a later page loads successfully', async () => {
356+
const files = pagedProvider()
357+
service.search.mockReturnValue(files)
358+
359+
const searchController = new UnifiedSearchController()
360+
searchController.search('query', ['files'])
361+
362+
files.resolvePage(0, { entries: ['a'], cursor: 'cursor-1', hasMore: true })
363+
await vi.advanceTimersByTimeAsync(0)
364+
365+
// A first loadMore fails and raises the flag.
366+
searchController.loadMore('files')
367+
files.rejectPage(1, new Error('network'))
368+
await vi.advanceTimersByTimeAsync(0)
369+
expect(searchController.getSnapshot().files.loadMoreFailed).toBe(true)
370+
371+
// Retrying succeeds and must clear the stale flag.
372+
searchController.loadMore('files')
373+
files.resolvePage(2, { entries: ['b'], cursor: 'cursor-2', hasMore: false })
374+
await vi.advanceTimersByTimeAsync(0)
375+
376+
expect(searchController.getSnapshot().files).toEqual({
377+
status: 'loaded',
378+
entries: ['a', 'b'],
379+
cursor: 'cursor-2',
380+
hasMore: false,
381+
loadMoreFailed: false,
382+
})
383+
})
384+
385+
it('does nothing when the category has no more pages', async () => {
386+
const files = pagedProvider()
387+
service.search.mockReturnValue(files)
388+
389+
const searchController = new UnifiedSearchController()
390+
searchController.search('query', ['files'])
391+
392+
files.resolvePage(0, { entries: ['a'], cursor: 'cursor-1', hasMore: false })
393+
await vi.advanceTimersByTimeAsync(0)
394+
395+
searchController.loadMore('files')
396+
397+
// The initial search is the only dispatch; loadMore must not fire another.
398+
expect(service.search).toHaveBeenCalledTimes(1)
399+
})
400+
})
401+
261402
describe('reveal timer', () => {
262403
it('marks blocked categories as loaded after a certain amount of time has elapsed', async () => {
263404
const providers = mockProviders(['files', 'talk', 'deck'])
@@ -272,15 +413,15 @@ describe('UnifiedSearchController', () => {
272413
expect(searchController.getSnapshot()).toEqual({
273414
files: loading,
274415
talk: loading,
275-
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined },
416+
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
276417
})
277418

278419
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
279420

280421
expect(searchController.getSnapshot()).toEqual({
281422
files: loading,
282423
talk: loading,
283-
deck: { status: 'loaded', entries: ['Deck result'], cursor: undefined, hasMore: undefined },
424+
deck: { status: 'loaded', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
284425
})
285426
})
286427

0 commit comments

Comments
 (0)