Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix incorrect directory contents when navigating quickly #1299

Merged
merged 5 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 43 additions & 7 deletions lib/composables/dav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => ({
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
})
})
74 changes: 56 additions & 18 deletions lib/composables/dav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
17 changes: 12 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 4 additions & 2 deletions vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
],
},
},
},
Expand Down