Skip to content

Commit

Permalink
fix(FilePicker): Cleanup DAV handling and properly handle `currentFol…
Browse files Browse the repository at this point in the history
…der`

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 <opensource@fthiessen.de>
  • Loading branch information
susnux committed Jul 2, 2024
1 parent 00e605d commit b3c0fc6
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 97 deletions.
12 changes: 9 additions & 3 deletions lib/components/FilePicker/FilePicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = []
})
/**
Expand Down
27 changes: 5 additions & 22 deletions lib/composables/dav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down Expand Up @@ -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 } })),
Expand All @@ -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('/')
Expand All @@ -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)
Expand Down
92 changes: 20 additions & 72 deletions lib/composables/dav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,48 +27,6 @@ export const useDAVFiles = function(
*/
const client = davGetClient()

const resultToNode = (result: FileStat) => davResultToNode(result)

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.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.getDirectoryContents(join(davRootPath, 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
nodes = nodes.filter((file) => file.path !== currentPath.value)
resolve(nodes)
} catch (error) {
reject(error)
}
})
}

/**
* All files in current view and path
*/
Expand All @@ -77,49 +35,33 @@ export const useDAVFiles = function(
/**
* The current folder
*/
const folder = shallowRef<Folder>()
watch([currentPath], async () => {
folder.value = (files.value.find(({ path }) => path === currentPath.value) ?? await getFile(currentPath.value)) as Folder
}, { immediate: true })
const folder = shallowRef<Folder|null>(null)

/**
* Loading state of the files
*/
const isLoading = ref(true)

/**
* The cancelable promise
* The cancelable promise used internally to cancel on fast navigation
*/
const promise = ref<null | CancelablePromise<unknown>>(null)
const promise = ref<null | CancelablePromise<Node[] | ContentsWithRoot>>(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<Folder>} The created directory
*/
async function createDirectory(name: string): Promise<Folder> {
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<FileStat>
return resultToNode(data)
}

/**
* Force reload files using the DAV client
*/
Expand All @@ -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
Expand All @@ -157,7 +106,6 @@ export const useDAVFiles = function(
files,
folder,
loadFiles: loadDAVFiles,
getFile,
createDirectory,
}
}
34 changes: 34 additions & 0 deletions lib/utils/dav.spec.ts
Original file line number Diff line number Diff line change
@@ -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` })
})
})
76 changes: 76 additions & 0 deletions lib/utils/dav.ts
Original file line number Diff line number Diff line change
@@ -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<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.search('/', {
signal: controller.signal,
details: true,
data: davGetRecentSearch(lastTwoWeek),
}) as ResponseDataDetailed<SearchResult>
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<ContentsWithRoot> {
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<FileStat[]>
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<FileStat>
return davResultToNode(data)
}

0 comments on commit b3c0fc6

Please sign in to comment.