feat(core): cancel and dispose unified search requests

Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>

Author: pringelmann

core/src/services/UnifiedSearchController.ts

@@ -11,12 +11,24 @@ type CategorySearchItem = {
 	hasMore: boolean
 }
 
+/**
+ * Runs a unified search across categories in priority order, blocking
+ * lower-priority results until their predecessors arrive or a timer reveals them.
+ */
 export class UnifiedSearchController {
 	private searchItems: Record<string, CategorySearchItem> = {}
 	private requestId: number = 0
 	private revealTimer: ReturnType<typeof setTimeout> | null = null
-
+	private searchAbortHandlers: (() => void)[] = []
+
+	/**
+	 * Start a search. Cancels and replaces any search already in flight.
+	 *
+	 * @param query the search term
+	 * @param categories category ids in priority order
+	 */
 	search(query: string, categories: string[]): void {
+		this.cancelPendingRequests()
 		this.searchItems = {}
 		this.requestId++
 		const dispatchId = this.requestId
@@ -30,12 +42,14 @@ export class UnifiedSearchController {
 				cursor: null,
 				hasMore: false,
 			}
-			const { request } = unifiedSearch({
+			const { request, cancel } = unifiedSearch({
 				type: category,
 				query,
 				cursor: null,
 			})
 
+			this.searchAbortHandlers.push(cancel)
+
 			request().then((response) => {
 				if (this.requestId !== dispatchId) {
 					// A new search has been started, ignore this result
@@ -66,7 +80,24 @@ export class UnifiedSearchController {
 		})
 	}
 
-	reconcileCategoryStatuses(categories: string[]): void {
+	/**
+	 * A shallow copy of the current per-category state, safe to read for rendering.
+	 *
+	 * @return the current search items keyed by category id
+	 */
+	getSnapshot(): Record<string, CategorySearchItem> {
+		return { ...this.searchItems }
+	}
+
+	/**
+	 * Tear down on unmount: cancels in-flight requests and stops the reveal timer.
+	 */
+	dispose(): void {
+		this.cancelPendingRequests()
+		this.stopRevealTimer()
+	}
+
+	private reconcileCategoryStatuses(categories: string[]): void {
 		categories.forEach((category) => {
 			if (['loading', 'failed'].includes(this.searchItems[category].status)) {
 				return
@@ -75,15 +106,39 @@ export class UnifiedSearchController {
 		})
 	}
 
-	unblockAllCategories(categories: string[]): void {
+	private startRevealTimer(): void {
+		this.stopRevealTimer()
+		this.revealTimer = setTimeout(() => {
+			const categories = Object.keys(this.searchItems)
+			const hasPendingCategories = categories.some((category) => ['loading', 'blocked'].includes(this.searchItems[category].status))
+			this.unblockAllCategories(categories)
+			if (hasPendingCategories) {
+				this.startRevealTimer()
+			}
+		}, REVEAL_INTERVAL)
+	}
+
+	private stopRevealTimer(): void {
+		if (this.revealTimer) {
+			clearTimeout(this.revealTimer)
+			this.revealTimer = null
+		}
+	}
+
+	private cancelPendingRequests(): void {
+		this.searchAbortHandlers.forEach((cancel) => cancel())
+		this.searchAbortHandlers = []
+	}
+
+	private unblockAllCategories(categories: string[]): void {
 		categories.forEach((category) => {
 			if (this.searchItems[category].status === 'blocked') {
 				this.searchItems[category].status = 'loaded'
 			}
 		})
 	}
 
-	categoryShouldBlock(category: string, categories: string[]): boolean {
+	private categoryShouldBlock(category: string, categories: string[]): boolean {
 		const categoryItem = this.searchItems[category]
 		if (!categoryItem) {
 			return false
@@ -94,22 +149,4 @@ export class UnifiedSearchController {
 			return item && ['loading', 'blocked'].includes(item.status)
 		})
 	}
-
-	startRevealTimer(): void {
-		if (this.revealTimer) {
-			clearTimeout(this.revealTimer)
-		}
-		this.revealTimer = setTimeout(() => {
-			const categories = Object.keys(this.searchItems)
-			const hasPendingCategories = categories.some((category) => ['loading', 'blocked'].includes(this.searchItems[category].status))
-			this.unblockAllCategories(categories)
-			if (hasPendingCategories) {
-				this.startRevealTimer()
-			}
-		}, REVEAL_INTERVAL)
-	}
-
-	getSnapshot(): Record<string, CategorySearchItem> {
-		return { ...this.searchItems }
-	}
 }

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

@@ -214,6 +214,50 @@ describe('UnifiedSearchController', () => {
 		})
 	})
 
+	describe('cancellation', () => {
+		it('cancels the previous search\'s in-flight requests when a new search starts', () => {
+			const first = mockProviders(['files', 'talk'])
+
+			const searchController = new UnifiedSearchController()
+			searchController.search('first', ['files', 'talk'])
+
+			// A new search supersedes the first while its requests are in flight.
+			mockProviders(['files', 'talk'])
+			searchController.search('second', ['files', 'talk'])
+
+			expect(first.files.cancel).toHaveBeenCalledOnce()
+			expect(first.talk.cancel).toHaveBeenCalledOnce()
+		})
+	})
+
+	describe('dispose', () => {
+		it('cancels in-flight requests when disposed', () => {
+			const providers = mockProviders(['files', 'talk'])
+
+			const searchController = new UnifiedSearchController()
+			searchController.search('query', ['files', 'talk'])
+
+			searchController.dispose()
+
+			expect(providers.files.cancel).toHaveBeenCalledOnce()
+			expect(providers.talk.cancel).toHaveBeenCalledOnce()
+		})
+
+		it('stops the reveal timer when disposed', () => {
+			mockProviders(['files', 'talk'])
+
+			const searchController = new UnifiedSearchController()
+			searchController.search('query', ['files', 'talk'])
+
+			// A search arms the reveal timer.
+			expect(vi.getTimerCount()).toBe(1)
+
+			searchController.dispose()
+
+			expect(vi.getTimerCount()).toBe(0)
+		})
+	})
+
 	describe('reveal timer', () => {
 		it('marks blocked categories as loaded after a certain amount of time has elapsed', async () => {
 			const providers = mockProviders(['files', 'talk', 'deck'])