From b3c0fc6c4e8c326e7ce6323a4ac69632505560c3 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 2 Jul 2024 17:10:55 +0200 Subject: [PATCH] fix(FilePicker): Cleanup DAV handling and properly handle `currentFolder` Instead of fetching the directory in a second request we simply do it like the file app and always request the content together with its root. This also allows to remove some functions by refactoring. Signed-off-by: Ferdinand Thiessen --- lib/components/FilePicker/FilePicker.vue | 12 +++- lib/composables/dav.spec.ts | 27 ++----- lib/composables/dav.ts | 92 ++++++------------------ lib/utils/dav.spec.ts | 34 +++++++++ lib/utils/dav.ts | 76 ++++++++++++++++++++ 5 files changed, 144 insertions(+), 97 deletions(-) create mode 100644 lib/utils/dav.spec.ts create mode 100644 lib/utils/dav.ts diff --git a/lib/components/FilePicker/FilePicker.vue b/lib/components/FilePicker/FilePicker.vue index 5f3ab0da..2f6867a8 100644 --- a/lib/components/FilePicker/FilePicker.vue +++ b/lib/components/FilePicker/FilePicker.vue @@ -146,13 +146,19 @@ const isOpen = ref(true) * Map buttons to Dialog buttons by wrapping the callback function to pass the selected files */ const dialogButtons = computed(() => { - const nodes = selectedFiles.value.length === 0 && props.allowPickDirectory && currentFolder.value ? [currentFolder.value] : selectedFiles.value + const nodes = selectedFiles.value.length === 0 + && props.allowPickDirectory + && currentFolder.value + ? [currentFolder.value] + : selectedFiles.value + const buttons = typeof props.buttons === 'function' ? props.buttons(nodes, currentPath.value, currentView.value) : props.buttons return buttons.map((button) => ({ ...button, + disabled: button.disabled || isLoading.value, callback: () => { // lock default close handling isHandlingCallback = true @@ -203,9 +209,9 @@ const navigatedPath = ref('') watch([navigatedPath], () => { if (props.path === undefined && navigatedPath.value) { window.sessionStorage.setItem('NC.FilePicker.LastPath', navigatedPath.value) - // Reset selected files - selectedFiles.value = [] } + // Reset selected files + selectedFiles.value = [] }) /** diff --git a/lib/composables/dav.spec.ts b/lib/composables/dav.spec.ts index 6e24e482..1d0ccc57 100644 --- a/lib/composables/dav.spec.ts +++ b/lib/composables/dav.spec.ts @@ -71,7 +71,6 @@ describe('dav composable', () => { expect(Array.isArray(vue.vm.files)).toBe(true) expect(vue.vm.files.length).toBe(0) // functions - expect(typeof vue.vm.getFile === 'function').toBe(true) expect(typeof vue.vm.loadFiles === 'function').toBe(true) }) @@ -153,23 +152,6 @@ describe('dav composable', () => { expect(client.search).toBeCalledTimes(1) }) - it('getFile works', async () => { - const client = { - stat: vi.fn((v) => ({ data: { path: v } })), - getDirectoryContents: vi.fn(() => ({ data: [] })), - } - nextcloudFiles.davGetClient.mockImplementation(() => client) - nextcloudFiles.davResultToNode.mockImplementation((v) => v) - - const { getFile } = useDAVFiles(ref('files'), ref('/')) - - const node = await getFile('/some/path/file.ext') - expect(node).toEqual({ path: `${nextcloudFiles.davRootPath}/some/path/file.ext` }) - // Check mock usage - expect(client.stat).toBeCalledWith(`${nextcloudFiles.davRootPath}/some/path/file.ext`, { details: true }) - expect(nextcloudFiles.davResultToNode).toBeCalledWith({ path: `${nextcloudFiles.davRootPath}/some/path/file.ext` }) - }) - it('createDirectory works', async () => { const client = { stat: vi.fn((v) => ({ data: { path: v } })), @@ -189,11 +171,12 @@ describe('dav composable', () => { it('loadFiles work', 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 } })), + getDirectoryContents: vi.fn((_p, _o) => ({ data: [] })), + search: vi.fn((_p, _o) => ({ data: { results: [], truncated: false } })), } nextcloudFiles.davGetClient.mockImplementationOnce(() => client) nextcloudFiles.davResultToNode.mockImplementationOnce((v) => v) + nextcloudFiles.getFavoriteNodes.mockImplementationOnce(() => Promise.resolve([])) const view = ref<'files' | 'recent' | 'favorites'>('files') const path = ref('/') @@ -216,8 +199,8 @@ describe('dav composable', () => { 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 } })), + getDirectoryContents: vi.fn((_p, _o) => ({ data: [] })), + search: vi.fn((_p, _o) => ({ data: { results: [], truncated: false } })), } nextcloudFiles.davGetClient.mockImplementationOnce(() => client) nextcloudFiles.davResultToNode.mockImplementationOnce((v) => v) diff --git a/lib/composables/dav.ts b/lib/composables/dav.ts index 159c20f5..6467c302 100644 --- a/lib/composables/dav.ts +++ b/lib/composables/dav.ts @@ -2,14 +2,14 @@ * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { Folder, Node } from '@nextcloud/files' +import type { ContentsWithRoot, Folder, Node } from '@nextcloud/files' import type { ComputedRef, Ref } from 'vue' -import type { FileStat, ResponseDataDetailed, SearchResult } from 'webdav' -import { davGetClient, davGetDefaultPropfind, davGetRecentSearch, davResultToNode, davRootPath, getFavoriteNodes } from '@nextcloud/files' -import { join } from 'path' -import { onMounted, ref, shallowRef, watch } from 'vue' +import { davGetClient, davRootPath, getFavoriteNodes } from '@nextcloud/files' import { CancelablePromise } from 'cancelable-promise' +import { join } from 'node:path' +import { onMounted, ref, shallowRef, watch } from 'vue' +import { getFile, getNodes, getRecentNodes } from '../utils/dav' /** * Handle file loading using WebDAV @@ -27,48 +27,6 @@ export const useDAVFiles = function( */ const client = davGetClient() - const resultToNode = (result: FileStat) => davResultToNode(result) - - const getRecentNodes = (): CancelablePromise => { - 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.search('/', { - signal: controller.signal, - details: true, - data: davGetRecentSearch(lastTwoWeek), - }) as ResponseDataDetailed - const nodes = data.results.map(resultToNode) - resolve(nodes) - } catch (error) { - reject(error) - } - }) - } - - const getNodes = (): CancelablePromise => { - const controller = new AbortController() - return new CancelablePromise(async (resolve, reject, onCancel) => { - onCancel(() => controller.abort()) - try { - const results = await client.getDirectoryContents(join(davRootPath, currentPath.value), { - signal: controller.signal, - details: true, - data: davGetDefaultPropfind(), - }) as ResponseDataDetailed - let nodes = results.data.map(resultToNode) - // Hack for the public endpoint which always returns folder itself - nodes = nodes.filter((file) => file.path !== currentPath.value) - resolve(nodes) - } catch (error) { - reject(error) - } - }) - } - /** * All files in current view and path */ @@ -77,10 +35,7 @@ export const useDAVFiles = function( /** * The current folder */ - const folder = shallowRef() - watch([currentPath], async () => { - folder.value = (files.value.find(({ path }) => path === currentPath.value) ?? await getFile(currentPath.value)) as Folder - }, { immediate: true }) + const folder = shallowRef(null) /** * Loading state of the files @@ -88,12 +43,13 @@ export const useDAVFiles = function( const isLoading = ref(true) /** - * The cancelable promise + * The cancelable promise used internally to cancel on fast navigation */ - const promise = ref>(null) + const promise = ref>(null) /** * Create a new directory in the current path + * The directory will be added to the current file list * @param name Name of the new directory * @return {Promise} The created directory */ @@ -101,25 +57,11 @@ export const useDAVFiles = function( const path = join(currentPath.value, name) await client.createDirectory(join(davRootPath, path)) - const directory = await getFile(path) as Folder + const directory = await getFile(client, path) as Folder files.value = [...files.value, directory] return directory } - /** - * Get information for one file - * - * @param path The path of the file or folder - * @param rootPath DAV root path, defaults to '/files/USERID' - */ - async function getFile(path: string, rootPath: string = davRootPath) { - const { data } = await client.stat(join(rootPath, path), { - details: true, - data: davGetDefaultPropfind(), - }) as ResponseDataDetailed - return resultToNode(data) - } - /** * Force reload files using the DAV client */ @@ -132,11 +74,18 @@ export const useDAVFiles = function( if (currentView.value === 'favorites') { promise.value = getFavoriteNodes(client, currentPath.value) } else if (currentView.value === 'recent') { - promise.value = getRecentNodes() + promise.value = getRecentNodes(client) + } else { + promise.value = getNodes(client, currentPath.value) + } + const content = await promise.value + if ('folder' in content) { + folder.value = content.folder + files.value = content.contents } else { - promise.value = getNodes() + folder.value = null + files.value = content } - files.value = await promise.value as Node[] promise.value = null isLoading.value = false @@ -157,7 +106,6 @@ export const useDAVFiles = function( files, folder, loadFiles: loadDAVFiles, - getFile, createDirectory, } } diff --git a/lib/utils/dav.spec.ts b/lib/utils/dav.spec.ts new file mode 100644 index 00000000..a0cfff79 --- /dev/null +++ b/lib/utils/dav.spec.ts @@ -0,0 +1,34 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const nextcloudFiles = vi.hoisted(() => ({ + davResultToNode: vi.fn((v) => v), + davGetDefaultPropfind: vi.fn(() => 'propfind content'), + davRootPath: '/root/path', +})) +vi.mock('@nextcloud/files', () => nextcloudFiles) + +describe('DAV utils', () => { + beforeEach(() => { + vi.resetModules() + }) + + it('getFile works', async () => { + const client = { + stat: vi.fn((v) => Promise.resolve({ data: { path: v } })), + getDirectoryContents: vi.fn(() => ({ data: [] })), + } + + const { getFile } = await import('./dav') + + const node = await getFile(client, '/some/path/file.ext') + expect(node).toEqual({ path: `${nextcloudFiles.davRootPath}/some/path/file.ext` }) + // Check mock usage + expect(client.stat).toBeCalledWith(`${nextcloudFiles.davRootPath}/some/path/file.ext`, { details: true, data: 'propfind content' }) + expect(nextcloudFiles.davResultToNode).toBeCalledWith({ path: `${nextcloudFiles.davRootPath}/some/path/file.ext` }) + }) +}) \ No newline at end of file diff --git a/lib/utils/dav.ts b/lib/utils/dav.ts new file mode 100644 index 00000000..7384afea --- /dev/null +++ b/lib/utils/dav.ts @@ -0,0 +1,76 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { ContentsWithRoot, Node } from '@nextcloud/files' +import type { FileStat, ResponseDataDetailed, SearchResult, WebDAVClient } from 'webdav' + +import { davGetDefaultPropfind, davGetRecentSearch, davResultToNode, davRootPath } from '@nextcloud/files' +import { CancelablePromise } from 'cancelable-promise' +import { join } from 'node:path' + +/** + * Get the recently changed nodes from the last two weeks + * @param client The WebDAV client + */ +export function getRecentNodes(client: WebDAVClient): CancelablePromise { + 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.search('/', { + signal: controller.signal, + details: true, + data: davGetRecentSearch(lastTwoWeek), + }) as ResponseDataDetailed + const nodes = data.results.map((result: FileStat) => davResultToNode(result)) + resolve(nodes) + } catch (error) { + reject(error) + } + }) +} + +/** + * Get the directory content + * @param client The WebDAV client + * @param directoryPath The path to fetch + */ +export function getNodes(client: WebDAVClient, directoryPath: string): CancelablePromise { + const controller = new AbortController() + return new CancelablePromise(async (resolve, reject, onCancel) => { + onCancel(() => controller.abort()) + try { + const results = await client.getDirectoryContents(join(davRootPath, directoryPath), { + signal: controller.signal, + details: true, + includeSelf: true, + data: davGetDefaultPropfind(), + }) as ResponseDataDetailed + const nodes = results.data.map((result: FileStat) => davResultToNode(result)) + resolve({ + contents: nodes.filter(({ path }) => path !== directoryPath), + folder: nodes.find(({ path }) => path === directoryPath), + }) + } catch (error) { + reject(error) + } + }) +} + +/** + * Get information for one file + * + * @param client The WebDAV client + * @param path The path of the file or folder + */ +export async function getFile(client: WebDAVClient, path: string) { + const { data } = await client.stat(join(davRootPath, path), { + details: true, + data: davGetDefaultPropfind(), + }) as ResponseDataDetailed + return davResultToNode(data) +}