chore(files): adjust getContents to use AbortController

Signed-off-by: Ferdinand Thiessen <[email protected]>

Author: susnux

apps/files/src/services/Favorites.ts

@@ -1,45 +1,49 @@
-/**
+/*!
  * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+
 import type { ContentsWithRoot } from '@nextcloud/files'
 
 import { getCurrentUser } from '@nextcloud/auth'
 import { Folder, Permission } from '@nextcloud/files'
 import { getFavoriteNodes, getRemoteURL, getRootPath } from '@nextcloud/files/dav'
-import { CancelablePromise } from 'cancelable-promise'
+import logger from '../logger.ts'
 import { getContents as filesContents } from './Files.ts'
 import { client } from './WebdavClient.ts'
 
 /**
+ * Get the contents for the favorites view
  *
- * @param path
+ * @param path - The path to get the contents for
+ * @param options - Additional options
+ * @param options.signal - Optional AbortSignal to cancel the request
+ * @return A promise resolving to the contents with root folder
  */
-export function getContents(path = '/'): CancelablePromise<ContentsWithRoot> {
+export async function getContents(path = '/', { signal }: { signal?: AbortSignal } = {}): Promise<ContentsWithRoot> {
 	// We only filter root files for favorites, for subfolders we can simply reuse the files contents
-	if (path !== '/') {
+	if (path && path !== '/') {
 		return filesContents(path)
 	}
 
-	return new CancelablePromise((resolve, reject, cancel) => {
-		const promise = getFavoriteNodes(client)
-			.catch(reject)
-			.then((contents) => {
-				if (!contents) {
-					reject()
-					return
-				}
-				resolve({
-					contents,
-					folder: new Folder({
-						id: 0,
-						source: `${getRemoteURL()}${getRootPath()}`,
-						root: getRootPath(),
-						owner: getCurrentUser()?.uid || null,
-						permissions: Permission.READ,
-					}),
-				})
-			})
-		cancel(() => promise.cancel())
-	})
+	try {
+		const contents = await getFavoriteNodes({ client, signal })
+		return {
+			contents,
+			folder: new Folder({
+				id: 0,
+				source: `${getRemoteURL()}${getRootPath()}`,
+				root: getRootPath(),
+				owner: getCurrentUser()?.uid || null,
+				permissions: Permission.READ,
+			}),
+		}
+	} catch (error) {
+		if (signal?.aborted) {
+			logger.debug('Favorite nodes request was aborted')
+			throw new DOMException('Aborted', 'AbortError')
+		}
+		logger.error('Failed to load favorite nodes via WebDAV', { error })
+		throw error
+	}
 }

apps/files/src/services/Files.ts

@@ -6,7 +6,6 @@ import type { ContentsWithRoot, File, Folder } from '@nextcloud/files'
 import type { FileStat, ResponseDataDetailed } from 'webdav'
 
 import { getDefaultPropfind, getRootPath, resultToNode } from '@nextcloud/files/dav'
-import { CancelablePromise } from 'cancelable-promise'
 import { join } from 'path'
 import logger from '../logger.ts'
 import { useFilesStore } from '../store/files.ts'
@@ -20,66 +19,55 @@ import { searchNodes } from './WebDavSearch.ts'
  * This also allows to fetch local search results when the user is currently filtering.
  *
  * @param path - The path to query
+ * @param options - Options
+ * @param options.signal - Abort signal to cancel the request
  */
-export function getContents(path = '/'): CancelablePromise<ContentsWithRoot> {
-	const controller = new AbortController()
+export async function getContents(path = '/', options: { signal: AbortSignal }): Promise<ContentsWithRoot> {
 	const searchStore = useSearchStore(getPinia())
 
-	if (searchStore.query.length >= 3) {
-		return new CancelablePromise((resolve, reject, cancel) => {
-			cancel(() => controller.abort())
-			getLocalSearch(path, searchStore.query, controller.signal)
-				.then(resolve)
-				.catch(reject)
-		})
-	} else {
-		return defaultGetContents(path)
+	if (searchStore.query.length < 3) {
+		return await defaultGetContents(path, options)
 	}
+
+	return await getLocalSearch(path, searchStore.query, options.signal)
 }
 
 /**
  * Generic `getContents` implementation for the users files.
  *
  * @param path - The path to get the contents
+ * @param options - Options
+ * @param options.signal - Abort signal to cancel the request
  */
-export function defaultGetContents(path: string): CancelablePromise<ContentsWithRoot> {
+export async function defaultGetContents(path: string, options: { signal: AbortSignal }): Promise<ContentsWithRoot> {
 	path = join(getRootPath(), path)
-	const controller = new AbortController()
 	const propfindPayload = getDefaultPropfind()
 
-	return new CancelablePromise(async (resolve, reject, onCancel) => {
-		onCancel(() => controller.abort())
+	const contentsResponse = await client.getDirectoryContents(path, {
+		details: true,
+		data: propfindPayload,
+		includeSelf: true,
+		signal: options.signal,
+	}) as ResponseDataDetailed<FileStat[]>
 
-		try {
-			const contentsResponse = await client.getDirectoryContents(path, {
-				details: true,
-				data: propfindPayload,
-				includeSelf: true,
-				signal: controller.signal,
-			}) as ResponseDataDetailed<FileStat[]>
+	const root = contentsResponse.data[0]
+	const contents = contentsResponse.data.slice(1)
+	if (root?.filename !== path && `${root?.filename}/` !== path) {
+		logger.debug(`Exepected "${path}" but got filename "${root.filename}" instead.`)
+		throw new Error('Root node does not match requested path')
+	}
 
-			const root = contentsResponse.data[0]
-			const contents = contentsResponse.data.slice(1)
-			if (root?.filename !== path && `${root?.filename}/` !== path) {
-				logger.debug(`Exepected "${path}" but got filename "${root.filename}" instead.`)
-				throw new Error('Root node does not match requested path')
+	return {
+		folder: resultToNode(root) as Folder,
+		contents: contents.map((result) => {
+			try {
+				return resultToNode(result)
+			} catch (error) {
+				logger.error(`Invalid node detected '${result.basename}'`, { error })
+				return null
 			}
-
-			resolve({
-				folder: resultToNode(root) as Folder,
-				contents: contents.map((result) => {
-					try {
-						return resultToNode(result)
-					} catch (error) {
-						logger.error(`Invalid node detected '${result.basename}'`, { error })
-						return null
-					}
-				}).filter(Boolean) as File[],
-			})
-		} catch (error) {
-			reject(error)
-		}
-	})
+		}).filter(Boolean) as File[],
+	}
 }
 
 /**

apps/files/src/services/FolderTree.ts

@@ -4,7 +4,6 @@
  */
 
 import type { ContentsWithRoot } from '@nextcloud/files'
-import type { CancelablePromise } from 'cancelable-promise'
 
 import { getCurrentUser } from '@nextcloud/auth'
 import axios from '@nextcloud/axios'
@@ -47,10 +46,11 @@ const collator = Intl.Collator(
 const compareNodes = (a: TreeNodeData, b: TreeNodeData) => collator.compare(a.displayName ?? a.basename, b.displayName ?? b.basename)
 
 /**
+ * Get all tree nodes recursively
  *
- * @param tree
- * @param currentPath
- * @param nodes
+ * @param tree - The tree to process
+ * @param currentPath - The current path
+ * @param nodes - The nodes collected so far
  */
 function getTreeNodes(tree: Tree, currentPath: string = '/', nodes: TreeNode[] = []): TreeNode[] {
 	const sortedTree = tree.toSorted(compareNodes)
@@ -76,9 +76,10 @@ function getTreeNodes(tree: Tree, currentPath: string = '/', nodes: TreeNode[] =
 }
 
 /**
+ * Get folder tree nodes
  *
- * @param path
- * @param depth
+ * @param path - The path to get the tree from
+ * @param depth - The depth to fetch
  */
 export async function getFolderTreeNodes(path: string = '/', depth: number = 1): Promise<TreeNode[]> {
 	const { data: tree } = await axios.get<Tree>(generateOcsUrl('/apps/files/api/v1/folder-tree'), {
@@ -88,20 +89,22 @@ export async function getFolderTreeNodes(path: string = '/', depth: number = 1):
 	return nodes
 }
 
-export const getContents = (path: string): CancelablePromise<ContentsWithRoot> => getFiles(path)
+export const getContents = (path: string, options: { signal: AbortSignal }): Promise<ContentsWithRoot> => getFiles(path, options)
 
 /**
+ * Encode source URL
  *
- * @param source
+ * @param source - The source URL
  */
 export function encodeSource(source: string): string {
 	const { origin } = new URL(source)
 	return origin + encodePath(source.slice(origin.length))
 }
 
 /**
+ * Get parent source URL
  *
- * @param source
+ * @param source - The source URL
  */
 export function getSourceParent(source: string): string {
 	const parent = dirname(source)

apps/files/src/services/PersonalFiles.ts

@@ -4,7 +4,6 @@
  */
 
 import type { ContentsWithRoot, Node } from '@nextcloud/files'
-import type { CancelablePromise } from 'cancelable-promise'
 
 import { getCurrentUser } from '@nextcloud/auth'
 import { getContents as getFiles } from './Files.ts'
@@ -31,13 +30,17 @@ export function isPersonalFile(node: Node): boolean {
 }
 
 /**
+ * Get personal files from a given path
  *
- * @param path
+ * @param path - The path to get the personal files from
+ * @param options - Options
+ * @param options.signal - Abort signal to cancel the request
+ * @return A promise that resolves to the personal files
  */
-export function getContents(path: string = '/'): CancelablePromise<ContentsWithRoot> {
+export function getContents(path: string = '/', options: { signal: AbortSignal }): Promise<ContentsWithRoot> {
 	// get all the files from the current path as a cancellable promise
 	// then filter the files that the user does not own, or has shared / is a group folder
-	return getFiles(path)
+	return getFiles(path, options)
 		.then((content) => {
 			content.contents = content.contents.filter(isPersonalFile)
 			return content

apps/files/src/services/Recent.ts

@@ -8,7 +8,7 @@ import type { ResponseDataDetailed, SearchResult } from 'webdav'
 import { getCurrentUser } from '@nextcloud/auth'
 import { Folder, Permission } from '@nextcloud/files'
 import { getRecentSearch, getRemoteURL, getRootPath, resultToNode } from '@nextcloud/files/dav'
-import { CancelablePromise } from 'cancelable-promise'
+import logger from '../logger.ts'
 import { getPinia } from '../store/index.ts'
 import { useUserConfigStore } from '../store/userconfig.ts'
 import { client } from './WebdavClient.ts'
@@ -22,8 +22,10 @@ const lastTwoWeeksTimestamp = Math.round((Date.now() / 1000) - (60 * 60 * 24 * 1
  * If hidden files are not shown, then also recently changed files *in* hidden directories are filtered.
  *
  * @param path Path to search for recent changes
+ * @param options Options including abort signal
+ * @param options.signal Abort signal to cancel the request
  */
-export function getContents(path = '/'): CancelablePromise<ContentsWithRoot> {
+export async function getContents(path = '/', options: { signal: AbortSignal }): Promise<ContentsWithRoot> {
 	const store = useUserConfigStore(getPinia())
 
 	/**
@@ -35,10 +37,9 @@ export function getContents(path = '/'): CancelablePromise<ContentsWithRoot> {
 		|| store.userConfig.show_hidden // If configured to show hidden files we can early return
 		|| !node.dirname.split('/').some((dir) => dir.startsWith('.')) // otherwise only include the file if non of the parent directories is hidden
 
-	const controller = new AbortController()
-	const handler = async () => {
+	try {
 		const contentsResponse = await client.search('/', {
-			signal: controller.signal,
+			signal: options.signal,
 			details: true,
 			data: getRecentSearch(lastTwoWeeksTimestamp),
 		}) as ResponseDataDetailed<SearchResult>
@@ -61,10 +62,12 @@ export function getContents(path = '/'): CancelablePromise<ContentsWithRoot> {
 			}),
 			contents,
 		}
+	} catch (error) {
+		if (options.signal.aborted) {
+			logger.info('Fetching recent files aborted')
+			throw new DOMException('Aborted', 'AbortError')
+		}
+		logger.error('Failed to fetch recent files', { error })
+		throw error
 	}
-
-	return new CancelablePromise(async (resolve, reject, cancel) => {
-		cancel(() => controller.abort())
-		resolve(handler())
-	})
 }

apps/files/src/services/Search.spec.ts

@@ -57,8 +57,9 @@ describe('Search service', () => {
 			return []
 		})
 
-		const content = getContents()
-		content.cancel()
+		const controller = new AbortController()
+		const content = getContents('', { signal: controller.signal })
+		controller.abort()
 
 		// its cancelled thus the promise returns the event
 		const event = await promise

apps/files/src/services/Search.ts

@@ -8,37 +8,39 @@ import type { ContentsWithRoot } from '@nextcloud/files'
 import { getCurrentUser } from '@nextcloud/auth'
 import { Folder, Permission } from '@nextcloud/files'
 import { defaultRemoteURL, getRootPath } from '@nextcloud/files/dav'
-import { CancelablePromise } from 'cancelable-promise'
 import logger from '../logger.ts'
 import { getPinia } from '../store/index.ts'
 import { useSearchStore } from '../store/search.ts'
 import { searchNodes } from './WebDavSearch.ts'
 
 /**
  * Get the contents for a search view
+ *
+ * @param path - (not used)
+ * @param options - Options including abort signal
+ * @param options.signal - Abort signal to cancel the request
  */
-export function getContents(): CancelablePromise<ContentsWithRoot> {
-	const controller = new AbortController()
-
+export async function getContents(path, options: { signal: AbortSignal }): Promise<ContentsWithRoot> {
 	const searchStore = useSearchStore(getPinia())
 
-	return new CancelablePromise<ContentsWithRoot>(async (resolve, reject, cancel) => {
-		cancel(() => controller.abort())
-		try {
-			const contents = await searchNodes(searchStore.query, { signal: controller.signal })
-			resolve({
-				contents,
-				folder: new Folder({
-					id: 0,
-					source: `${defaultRemoteURL}${getRootPath()}}#search`,
-					owner: getCurrentUser()!.uid,
-					permissions: Permission.READ,
-					root: getRootPath(),
-				}),
-			})
-		} catch (error) {
-			logger.error('Failed to fetch search results', { error })
-			reject(error)
+	try {
+		const contents = await searchNodes(searchStore.query, { signal: options.signal })
+		return {
+			contents,
+			folder: new Folder({
+				id: 0,
+				source: `${defaultRemoteURL}${getRootPath()}}#search`,
+				owner: getCurrentUser()!.uid,
+				permissions: Permission.READ,
+				root: getRootPath(),
+			}),
+		}
+	} catch (error) {
+		if (options.signal.aborted) {
+			logger.info('Fetching search results aborted')
+			throw new DOMException('Aborted', 'AbortError')
 		}
-	})
+		logger.error('Failed to fetch search results', { error })
+		throw error
+	}
 }