Skip to content

Commit 224d7a6

Browse files
committed
refactor(core): use async/await in unified search controller
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
1 parent a3c91f0 commit 224d7a6

2 files changed

Lines changed: 102 additions & 65 deletions

File tree

core/src/services/UnifiedSearchController.ts

Lines changed: 66 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
16
import { search as unifiedSearch } from './UnifiedSearchService.js'
27

38
type CategorySearchStatus = 'loading' | 'loaded' | 'failed' | 'blocked'
49

5-
export const REVEAL_INTERVAL = 1500 // milliseconds
6-
7-
type CategorySearchState = {
10+
interface CategorySearchState {
811
status: CategorySearchStatus
912
entries: unknown[]
1013
cursor: string | null
1114
hasMore: boolean
1215
loadMoreFailed: boolean
1316
}
1417

18+
export const REVEAL_INTERVAL_MS = 1500
19+
1520
/**
1621
* Runs a unified search across categories in priority order, blocking
1722
* lower-priority results until their predecessors arrive or a timer reveals them.
@@ -28,8 +33,9 @@ export class UnifiedSearchController {
2833
*
2934
* @param query the search term
3035
* @param categories category ids in priority order
36+
* @return resolves once every category has settled
3137
*/
32-
search(query: string, categories: string[]): void {
38+
async search(query: string, categories: string[]): Promise<void> {
3339
this.cancelPendingRequests()
3440
this.searchStates = {}
3541
this.searchGeneration++
@@ -38,52 +44,7 @@ export class UnifiedSearchController {
3844

3945
this.startRevealTimer()
4046

41-
categories.forEach((category) => {
42-
this.searchStates[category] = {
43-
status: 'loading',
44-
entries: [],
45-
cursor: null,
46-
hasMore: false,
47-
loadMoreFailed: false,
48-
}
49-
const { request, cancel } = unifiedSearch({
50-
type: category,
51-
query: this.query,
52-
cursor: null,
53-
})
54-
55-
this.pendingCancels.push(cancel)
56-
57-
request().then((response) => {
58-
if (this.searchGeneration !== generation) {
59-
// A new search has been started, ignore this result
60-
return
61-
}
62-
63-
const { entries, cursor, hasMore } = response.data.ocs.data
64-
this.searchStates[category] = {
65-
status: 'loaded',
66-
entries,
67-
cursor,
68-
hasMore,
69-
loadMoreFailed: false,
70-
}
71-
72-
this.reconcileCategoryStatuses(categories)
73-
}).catch(() => {
74-
if (this.searchGeneration !== generation) {
75-
return
76-
}
77-
this.searchStates[category] = {
78-
status: 'failed',
79-
entries: [],
80-
cursor: null,
81-
hasMore: false,
82-
loadMoreFailed: false,
83-
}
84-
this.reconcileCategoryStatuses(categories)
85-
})
86-
})
47+
await Promise.allSettled(categories.map((category) => this.searchCategory(category, generation, categories)))
8748
}
8849

8950
/**
@@ -93,7 +54,7 @@ export class UnifiedSearchController {
9354
*
9455
* @param category the category id to page
9556
*/
96-
loadMore(category: string): void {
57+
async loadMore(category: string): Promise<void> {
9758
const generation = this.searchGeneration
9859
const categoryState = this.searchStates[category]
9960
if (!categoryState || !categoryState.hasMore || categoryState.status !== 'loaded') {
@@ -110,22 +71,23 @@ export class UnifiedSearchController {
11071

11172
this.pendingCancels.push(cancel)
11273

113-
request().then((response) => {
74+
try {
75+
const response = await request()
11476
if (this.searchGeneration !== generation) {
11577
return
11678
}
11779
const { entries, cursor, hasMore } = response.data.ocs.data
11880
categoryState.entries.push(...entries)
11981
categoryState.cursor = cursor
12082
categoryState.hasMore = hasMore
121-
categoryState.status = 'loaded'
122-
}).catch(() => {
83+
} catch {
12384
if (this.searchGeneration !== generation) {
12485
return
12586
}
126-
categoryState.status = 'loaded'
12787
categoryState.loadMoreFailed = true
128-
})
88+
}
89+
90+
categoryState.status = 'loaded'
12991
}
13092

13193
/**
@@ -145,6 +107,53 @@ export class UnifiedSearchController {
145107
this.stopRevealTimer()
146108
}
147109

110+
private async searchCategory(category: string, generation: number, categories: string[]): Promise<void> {
111+
this.searchStates[category] = {
112+
status: 'loading',
113+
entries: [],
114+
cursor: null,
115+
hasMore: false,
116+
loadMoreFailed: false,
117+
}
118+
const { request, cancel } = unifiedSearch({
119+
type: category,
120+
query: this.query,
121+
cursor: null,
122+
})
123+
124+
this.pendingCancels.push(cancel)
125+
126+
try {
127+
const response = await request()
128+
if (this.searchGeneration !== generation) {
129+
// A new search has been started, ignore this result
130+
return
131+
}
132+
133+
const { entries, cursor, hasMore } = response.data.ocs.data
134+
this.searchStates[category] = {
135+
status: 'loaded',
136+
entries,
137+
cursor,
138+
hasMore,
139+
loadMoreFailed: false,
140+
}
141+
} catch {
142+
if (this.searchGeneration !== generation) {
143+
return
144+
}
145+
this.searchStates[category] = {
146+
status: 'failed',
147+
entries: [],
148+
cursor: null,
149+
hasMore: false,
150+
loadMoreFailed: false,
151+
}
152+
}
153+
154+
this.reconcileCategoryStatuses(categories)
155+
}
156+
148157
private reconcileCategoryStatuses(categories: string[]): void {
149158
categories.forEach((category) => {
150159
if (['loading', 'failed'].includes(this.searchStates[category].status)) {
@@ -163,7 +172,7 @@ export class UnifiedSearchController {
163172
if (hasPendingCategories) {
164173
this.startRevealTimer()
165174
}
166-
}, REVEAL_INTERVAL)
175+
}, REVEAL_INTERVAL_MS)
167176
}
168177

169178
private stopRevealTimer(): void {

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

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6-
import { REVEAL_INTERVAL, UnifiedSearchController } from '../../services/UnifiedSearchController.ts'
6+
import { REVEAL_INTERVAL_MS, UnifiedSearchController } from '../../services/UnifiedSearchController.ts'
77

88
const service = vi.hoisted(() => ({
99
search: vi.fn(),
@@ -215,6 +215,34 @@ describe('UnifiedSearchController', () => {
215215
loadMoreFailed: false,
216216
})
217217
})
218+
219+
it('ignores a stale response for a category the newer search dropped', async () => {
220+
const first = mockProviders(['files', 'talk'])
221+
222+
const searchController = new UnifiedSearchController()
223+
searchController.search('first', ['files', 'talk'])
224+
225+
// A newer search with a completely different category set supersedes it.
226+
const second = mockProviders(['deck'])
227+
searchController.search('second', ['deck'])
228+
229+
// The stale response is for 'files', which no longer exists in the
230+
// current search. Reconciling it must not throw on the missing category.
231+
first.files.resolve(['Stale files'])
232+
await vi.advanceTimersByTimeAsync(0)
233+
234+
expect(searchController.getSnapshot()).toEqual({
235+
deck: loading,
236+
})
237+
238+
// The live search still resolves normally.
239+
second.deck.resolve(['Live deck'])
240+
await vi.advanceTimersByTimeAsync(0)
241+
242+
expect(searchController.getSnapshot()).toEqual({
243+
deck: { status: 'loaded', entries: ['Live deck'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
244+
})
245+
})
218246
})
219247

220248
describe('resetting between searches', () => {
@@ -416,7 +444,7 @@ describe('UnifiedSearchController', () => {
416444
deck: { status: 'blocked', entries: ['Deck result'], cursor: undefined, hasMore: undefined, loadMoreFailed: false },
417445
})
418446

419-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
447+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
420448

421449
expect(searchController.getSnapshot()).toEqual({
422450
files: loading,
@@ -433,19 +461,19 @@ describe('UnifiedSearchController', () => {
433461

434462
// deck arrives out of order and is revealed by the first flush.
435463
providers.deck.resolve(['Deck result'])
436-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
464+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
437465
expect(searchController.getSnapshot().deck.status).toBe('loaded')
438466

439467
// A later flush passes with nothing blocked while files/talk keep loading.
440-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
468+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
441469

442470
// talk now arrives out of order (files still loading) and is blocked.
443471
providers.talk.resolve(['Talk result'])
444472
await vi.advanceTimersByTimeAsync(0)
445473
expect(searchController.getSnapshot().talk.status).toBe('blocked')
446474

447475
// The timer must still be running to flush talk on a later cycle.
448-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
476+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
449477
expect(searchController.getSnapshot().talk.status).toBe('loaded')
450478
})
451479

@@ -460,7 +488,7 @@ describe('UnifiedSearchController', () => {
460488
await vi.advanceTimersByTimeAsync(0)
461489

462490
// Nothing is loading or blocked, so the next flush should not re-arm.
463-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL)
491+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
464492
expect(vi.getTimerCount()).toBe(0)
465493
})
466494

@@ -472,7 +500,7 @@ describe('UnifiedSearchController', () => {
472500

473501
// First search: deck is blocked and its reveal timer is pending.
474502
first.deck.resolve(['First deck'])
475-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL - 500)
503+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS - 500)
476504
expect(searchController.getSnapshot().deck.status).toBe('blocked')
477505

478506
// A new search starts before the first timer fires. It must clear that
@@ -483,7 +511,7 @@ describe('UnifiedSearchController', () => {
483511
second.deck.resolve(['Second deck'])
484512
// Advance past when the first search's timer would have fired (500ms from
485513
// now) but before the second search's timer is due.
486-
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL - 500)
514+
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS - 500)
487515

488516
expect(searchController.getSnapshot().deck.status).toBe('blocked')
489517
})

0 commit comments

Comments
 (0)