Merge pull request #1299 from nextcloud-libraries/fix/fetch-folder-content

fix: Fix incorrect directory contents when navigating quickly

Author: Pytal

lib/composables/dav.spec.ts

@@ -20,9 +20,10 @@
  *
  */
 
+import type { Ref } from 'vue'
 import { describe, it, expect, vi, afterEach } from 'vitest'
 import { shallowMount } from '@vue/test-utils'
-import { defineComponent, ref, toRef } from 'vue'
+import { defineComponent, ref, toRef, nextTick } from 'vue'
 import { useDAVFiles } from './dav'
 
 const nextcloudFiles = vi.hoisted(() => ({
@@ -44,6 +45,14 @@ const waitLoaded = (vue: ReturnType<typeof shallowMount>) => new Promise((resolv
 	w()
 })
 
+const waitRefLoaded = (isLoading: Ref<boolean>) => new Promise((resolve) => {
+	const w = () => {
+		if (isLoading.value) window.setTimeout(w, 50)
+		else resolve(true)
+	}
+	w()
+})
+
 const TestComponent = defineComponent({
 	props: ['currentView', 'currentPath', 'isPublic'],
 	setup(props) {
@@ -209,16 +218,43 @@ describe('dav composable', () => {
 		expect(isLoading.value).toBe(true)
 		await loadFiles()
 		expect(isLoading.value).toBe(false)
-		expect(client.getDirectoryContents).toBeCalledWith(`${nextcloudFiles.davRootPath}/`, { details: true })
+		expect(client.getDirectoryContents).toBeCalledWith(`${nextcloudFiles.davRootPath}/`, expect.objectContaining({ details: true }))
 
 		view.value = 'recent'
-		await loadFiles()
-		expect(isLoading.value).toBe(false)
-		expect(client.search).toBeCalled()
+		await waitRefLoaded(isLoading)
+		expect(client.search).toBeCalledWith('/', expect.objectContaining({ details: true }))
 
 		view.value = 'favorites'
-		await loadFiles()
-		expect(isLoading.value).toBe(false)
+		await waitRefLoaded(isLoading)
 		expect(nextcloudFiles.getFavoriteNodes).toBeCalled()
 	})
+
+	it('request cancelation works', async () => {
+		const client = {
+			stat: vi.fn((v) => ({ data: { path: v } })),
+			getDirectoryContents: vi.fn((p, o) => ({ data: [] })),
+			search: vi.fn((p, o) => ({ data: { results: [], truncated: false } })),
+		}
+		nextcloudFiles.davGetClient.mockImplementationOnce(() => client)
+		nextcloudFiles.davResultToNode.mockImplementationOnce((v) => v)
+
+		const view = ref<'files' | 'recent' | 'favorites'>('files')
+		const path = ref('/')
+		const { loadFiles, isLoading } = useDAVFiles(view, path, ref(false))
+
+		const abort = vi.spyOn(AbortController.prototype, 'abort')
+
+		loadFiles()
+		view.value = 'recent'
+		await waitRefLoaded(isLoading)
+		expect(abort).toBeCalledTimes(1)
+
+		view.value = 'files'
+		await nextTick()
+		view.value = 'recent'
+		await nextTick()
+		view.value = 'favorites'
+		await waitRefLoaded(isLoading)
+		expect(abort).toBeCalledTimes(2)
+	})
 })

lib/composables/dav.ts

@@ -27,6 +27,7 @@ import { davGetClient, davGetDefaultPropfind, davGetRecentSearch, davRemoteURL,
 import { generateRemoteUrl } from '@nextcloud/router'
 import { join } from 'path'
 import { computed, onMounted, ref, watch } from 'vue'
+import { CancelablePromise } from 'cancelable-promise'
 
 /**
  * Handle file loading using WebDAV
@@ -68,6 +69,48 @@ export const useDAVFiles = function(
 
 	const resultToNode = (result: FileStat) => davResultToNode(result, defaultRootPath.value, defaultRemoteUrl.value)
 
+	const getRecentNodes = (): CancelablePromise<Node[]> => {
+		const controller = new AbortController()
+		// unix timestamp in seconds, two weeks ago
+		const lastTwoWeek = Math.round(Date.now() / 1000) - (60 * 60 * 24 * 14)
+		return new CancelablePromise(async (resolve, reject, onCancel) => {
+			onCancel(() => controller.abort())
+			try {
+				const { data } = await client.value.search('/', {
+					signal: controller.signal,
+					details: true,
+					data: davGetRecentSearch(lastTwoWeek),
+				}) as ResponseDataDetailed<SearchResult>
+				const nodes = data.results.map(resultToNode)
+				resolve(nodes)
+			} catch (error) {
+				reject(error)
+			}
+		})
+	}
+
+	const getNodes = (): CancelablePromise<Node[]> => {
+		const controller = new AbortController()
+		return new CancelablePromise(async (resolve, reject, onCancel) => {
+			onCancel(() => controller.abort())
+			try {
+				const results = await client.value.getDirectoryContents(`${defaultRootPath.value}${currentPath.value}`, {
+					signal: controller.signal,
+					details: true,
+					data: davGetDefaultPropfind(),
+				}) as ResponseDataDetailed<FileStat[]>
+				let nodes = results.data.map(resultToNode)
+				// Hack for the public endpoint which always returns folder itself
+				if (isPublicEndpoint.value) {
+					nodes = nodes.filter((file) => file.path !== currentPath.value)
+				}
+				resolve(nodes)
+			} catch (error) {
+				reject(error)
+			}
+		})
+	}
+
 	/**
 	 * All files in current view and path
 	 */
@@ -78,6 +121,11 @@ export const useDAVFiles = function(
 	 */
 	const isLoading = ref(true)
 
+	/**
+	 * The cancelable promise
+	 */
+	const promise = ref<null | CancelablePromise<unknown>>(null)
+
 	/**
 	 * Create a new directory in the current path
 	 * @param name Name of the new directory
@@ -112,31 +160,21 @@ export const useDAVFiles = function(
 	 * Force reload files using the DAV client
 	 */
 	async function loadDAVFiles() {
+		if (promise.value) {
+			promise.value.cancel()
+		}
 		isLoading.value = true
 
 		if (currentView.value === 'favorites') {
-			files.value = await getFavoriteNodes(client.value, currentPath.value, defaultRootPath.value)
+			promise.value = getFavoriteNodes(client.value, currentPath.value, defaultRootPath.value)
 		} else if (currentView.value === 'recent') {
-			// unix timestamp in seconds, two weeks ago
-			const lastTwoWeek = Math.round(Date.now() / 1000) - (60 * 60 * 24 * 14)
-			const { data } = await client.value.search('/', {
-				details: true,
-				data: davGetRecentSearch(lastTwoWeek),
-			}) as ResponseDataDetailed<SearchResult>
-			files.value = data.results.map(resultToNode)
+			promise.value = getRecentNodes()
 		} else {
-			const results = await client.value.getDirectoryContents(`${defaultRootPath.value}${currentPath.value}`, {
-				details: true,
-				data: davGetDefaultPropfind(),
-			}) as ResponseDataDetailed<FileStat[]>
-			files.value = results.data.map(resultToNode)
-
-			// Hack for the public endpoint which always returns folder itself
-			if (isPublicEndpoint.value) {
-				files.value = files.value.filter((file) => file.path !== currentPath.value)
-			}
+			promise.value = getNodes()
 		}
+		files.value = await promise.value as Node[]
 
+		promise.value = null
 		isLoading.value = false
 	}
 

package-lock.json

@@ -58,13 +58,14 @@
     "@nextcloud/auth": "^2.2.1",
     "@nextcloud/axios": "^2.4.0",
     "@nextcloud/event-bus": "^3.1.0",
-    "@nextcloud/files": "^3.1.1",
+    "@nextcloud/files": "^3.2.0",
     "@nextcloud/initial-state": "^2.1.0",
     "@nextcloud/l10n": "^2.2.0",
     "@nextcloud/router": "^3.0.0",
     "@nextcloud/typings": "^1.8.0",
     "@types/toastify-js": "^1.12.3",
     "@vueuse/core": "^10.9.0",
+    "cancelable-promise": "^4.3.1",
     "toastify-js": "^1.12.0",
     "vue-frag": "^1.4.3",
     "webdav": "^5.5.0"

package.json

@@ -41,10 +41,12 @@ export default defineConfig((env) => {
 						classNameStrategy: 'non-scoped',
 					},
 				},
-				// Fix unresolvable .css extension for ssr
 				server: {
 					deps: {
-						inline: [/@nextcloud\/vue/],
+						inline: [
+							/@nextcloud\/vue/, // Fix unresolvable .css extension for ssr
+							/@nextcloud\/files/, // Fix CommonJS cancelable-promise not supporting named exports
+						],
 					},
 				},
 			},