Skip to content

Commit

Permalink
refactor: id-based webdav requests for listings
Browse files Browse the repository at this point in the history
Introduce id-based webdav requests for space and folder listings. This is the recommended way to work with the webdav API according to oCIS because it delivers better performance, especially with slower storages.

Some of the webdav operations still need path handling though. Because of that, this change still "injects" full resources paths in the space loader. This can (hopefully) be removed once all operations work id-based or when we make the switch to Graph.
  • Loading branch information
Jannik Stehle committed Jul 12, 2024
1 parent bd10ab8 commit 32748a8
Show file tree
Hide file tree
Showing 45 changed files with 379 additions and 191 deletions.
4 changes: 2 additions & 2 deletions packages/web-app-external/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ export default defineWebApplication({
throw new Error(`An error has occurred: ${response.status}`)
}

const path = join(currentFolder.path, fileName) || ''
return clientService.webdav.getFileInfo(space, { path })
const fileId = response.data['file_id']
return clientService.webdav.getFileInfo(space, { fileId })
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export default defineComponent({
const sharedAncestor = computed(() => {
return Object.values(unref(ancestorMetaData)).find(
(a) =>
a.path !== unref(resource).path &&
a.id !== unref(resource).id &&
ShareTypes.containsAnyValue(ShareTypes.authenticated, a.shareTypes)
)
})
Expand Down
4 changes: 2 additions & 2 deletions packages/web-app-files/src/components/Spaces/SpaceHeader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ export default defineComponent({
}
const fileContentsResponse = await getFileContents(props.space, {
path: `.space/${props.space.spaceReadmeData.name}`
fileId: props.space.spaceReadmeData.id
})
const fileInfoResponse = await getFileInfo(props.space, {
path: `.space/${props.space.spaceReadmeData.name}`
fileId: props.space.spaceReadmeData.id
})
unobserveMarkdownContainerResize()
Expand Down
16 changes: 13 additions & 3 deletions packages/web-app-files/src/services/folder/loaderSpace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import {
isPersonalSpaceResource,
isPublicSpaceResource,
isShareSpaceResource,
SpaceResource
SpaceResource,
urlJoin
} from '@ownclouders/web-client'
import { unref } from 'vue'
import { FolderLoaderOptions } from './types'
import { authService } from 'web-runtime/src/services/auth'
import { useFileRouteReplace } from '@ownclouders/web-pkg'
import { IncomingShareResource } from '@ownclouders/web-client'
import { getIndicators } from '@ownclouders/web-pkg'
import { getIndicators, useResourcePath } from '@ownclouders/web-pkg'

export class FolderLoaderSpace implements FolderLoader {
public isEnabled(): boolean {
Expand All @@ -36,13 +37,14 @@ export class FolderLoaderSpace implements FolderLoader {
const { router, clientService, resourcesStore, userStore } = context
const { webdav } = clientService
const { replaceInvalidFileRoute } = useFileRouteReplace({ router })
const { getResourcePath } = useResourcePath({ router })

return useTask(function* (
signal1,
signal2,
space: SpaceResource,
path: string = null,
fileId: string | number = null,
fileId: string = null,
options: FolderLoaderOptions = {}
) {
try {
Expand Down Expand Up @@ -91,6 +93,14 @@ export class FolderLoaderSpace implements FolderLoader {
resources.forEach((r) => (r.remoteItemId = space.id))
}

/**
* Inject full paths into the resources since id-based requests return no path.
* This currently acts as a fall-back because some webdav operations still require
* to work with full paths. Ideally we remove this or only use it where needed.
**/
currentFolder.path = getResourcePath(space, currentFolder)
resources.forEach((r) => (r.path = urlJoin(currentFolder.path, r.name)))

resourcesStore.initResourceList({ currentFolder, resources })
} catch (error) {
resourcesStore.setCurrentFolder(null)
Expand Down
30 changes: 23 additions & 7 deletions packages/web-app-files/src/views/spaces/GenericSpace.vue
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ import {
} from 'vue'
import { RouteLocationNamedRaw } from 'vue-router'
import { useGettext } from 'vue3-gettext'
import { Resource } from '@ownclouders/web-client'
import { Resource, urlJoin } from '@ownclouders/web-client'
import {
isPersonalSpaceResource,
isProjectSpaceResource,
Expand Down Expand Up @@ -266,8 +266,13 @@ export default defineComponent({
const resourcesStore = useResourcesStore()
const { removeResources, resetSelection, updateResourceField } = resourcesStore
const { currentFolder, totalResourcesCount, totalResourcesSize, areHiddenFilesShown } =
storeToRefs(resourcesStore)
const {
currentFolder,
totalResourcesCount,
totalResourcesSize,
areHiddenFilesShown,
ancestorMetaData
} = storeToRefs(resourcesStore)
let loadResourcesEventToken: string
Expand All @@ -286,9 +291,15 @@ export default defineComponent({
const resourceTargetRouteCallback = ({
path,
fileId
fileId,
resource
}: CreateTargetRouteOptions): RouteLocationNamedRaw => {
const { params, query } = createFileRouteOptions(unref(space), { path, fileId })
const latest = Object.values(unref(ancestorMetaData))?.pop()
const { params, query } = createFileRouteOptions(unref(space), {
path: urlJoin(latest?.path || '', resource.name),
fileId
})
if (isPublicSpaceResource(unref(space))) {
return createLocationPublic('files-public-link', { params, query })
}
Expand Down Expand Up @@ -392,8 +403,12 @@ export default defineComponent({
return concatBreadcrumbs(
...rootBreadcrumbItems,
spaceBreadcrumbItem,
// FIXME: needs file ids for each parent folder path
...breadcrumbsFromPath({ route: unref(route), space, resourcePath: props.item })
...breadcrumbsFromPath({
route: unref(route),
space,
resourcePath: props.item,
ancestorMetaData
})
)
})
Expand Down Expand Up @@ -554,6 +569,7 @@ export default defineComponent({
const path = decodeURIComponent(fileTarget.path.slice(splitIndex, fileTarget.path.length))
try {
// we only have path info here because that comes from the breadcrumb
targetFolder = await clientService.webdav.getFileInfo(unref(space), { path })
} catch (e) {
console.error(e)
Expand Down
11 changes: 8 additions & 3 deletions packages/web-app-preview/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ import {
sortHelper,
useRoute,
useRouteQuery,
useRouter
useRouter,
useResourcePath
} from '@ownclouders/web-pkg'
import { createFileRouteOptions } from '@ownclouders/web-pkg'
import MediaControls from './components/MediaControls.vue'
Expand Down Expand Up @@ -127,6 +128,7 @@ export default defineComponent({
const router = useRouter()
const route = useRoute()
const appsStore = useAppsStore()
const { getResourcePath } = useResourcePath()
const contextRouteQuery = useRouteQuery('contextRouteQuery') as unknown as Ref<
Record<string, string>
>
Expand Down Expand Up @@ -281,7 +283,8 @@ export default defineComponent({
onPanZoomChanged,
preloadImageCount,
preview,
loading
loading,
getResourcePath
}
},
computed: {
Expand Down Expand Up @@ -343,9 +346,11 @@ export default defineComponent({
methods: {
setActiveFile(driveAliasAndItem: string) {
const space = unref(this.currentFileContext.space)
for (let i = 0; i < this.filteredFiles.length; i++) {
if (
unref(this.currentFileContext.space)?.getDriveAliasAndItem(this.filteredFiles[i]) ===
space?.getDriveAliasAndItem(this.getResourcePath(space, this.filteredFiles[i])) ===
driveAliasAndItem
) {
this.activeIndex = i
Expand Down
16 changes: 13 additions & 3 deletions packages/web-client/src/helpers/resource/functions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import path, { basename, dirname } from 'path'
import { urlJoin } from '../../utils'
import { DavPermission, DavProperty } from '../../webdav/constants'
import { Resource, ResourceIndicator, TrashResource, WebDavResponseResource } from './types'
import {
Resource,
ResourceIndicator,
SearchResource,
TrashResource,
WebDavResponseResource
} from './types'
import { camelCase } from 'lodash-es'

const fileExtensions = {
Expand All @@ -12,6 +18,10 @@ export const isTrashResource = (resource: Resource): resource is TrashResource =
return Object.hasOwn(resource, 'ddate')
}

export const isSearchResultResource = (resource: Resource): resource is SearchResource => {
return Object.hasOwn(resource, 'highlights')
}

export const extractDomSelector = (str: string): string => {
return str.replace(/[^A-Za-z0-9\-_]/g, '')
}
Expand Down Expand Up @@ -63,8 +73,8 @@ export const extractExtensionFromFile = (resource: Resource): string => {
return parts[parts.length - 1]
}

export const extractParentFolderName = (resource: Resource): string | null => {
return basename(dirname(resource.path)) || null
export const extractParentFolderName = (path: string): string | null => {
return basename(dirname(path)) || null
}

export const isShareRoot = (resource: Resource) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/web-client/src/helpers/space/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export function buildSpace(
},
canDeny: () => false,
getDomSelector: () => extractDomSelector(data.id),
getDriveAliasAndItem({ path }: Resource): string {
getDriveAliasAndItem(path: string): string {
return urlJoin(this.driveAlias, path, {
leadingSlash: false
})
Expand Down
2 changes: 1 addition & 1 deletion packages/web-client/src/helpers/space/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export interface SpaceResource extends Resource {

getWebDavUrl({ path }: { path: string }): string
getWebDavTrashUrl({ path }: { path: string }): string
getDriveAliasAndItem(resource: Resource): string
getDriveAliasAndItem(path: string): string

isViewer(user: User): boolean
isSecureViewer(user: User): boolean
Expand Down
11 changes: 8 additions & 3 deletions packages/web-client/src/webdav/getFileContents.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { urlJoin } from '../utils'
import { SpaceResource } from '../helpers'
import { extractNodeId, SpaceResource } from '../helpers'
import { WebDavOptions } from './types'
import { DAV, DAVRequestOptions } from './client'
import { HttpError } from '../errors'
Expand All @@ -14,7 +14,7 @@ export const GetFileContentsFactory = (dav: DAV, { axiosClient }: WebDavOptions)
return {
async getFileContents(
space: SpaceResource,
{ path }: { path?: string },
{ fileId, path }: { fileId?: string; path?: string },
{
responseType = 'text',
noCache = true,
Expand All @@ -25,8 +25,13 @@ export const GetFileContentsFactory = (dav: DAV, { axiosClient }: WebDavOptions)
noCache?: boolean
} & DAVRequestOptions = {}
): Promise<GetFileContentsResponse> {
let webDavPath = urlJoin(space.webDavPath, path)
if (fileId) {
webDavPath = `${space.webDavPath}!${extractNodeId(fileId)}`
}

try {
const response = await axiosClient.get(dav.getFileUrl(urlJoin(space.webDavPath, path)), {
const response = await axiosClient.get(dav.getFileUrl(webDavPath), {
responseType,
headers: {
...(noCache && { 'Cache-Control': 'no-cache' }),
Expand Down
2 changes: 1 addition & 1 deletion packages/web-client/src/webdav/getFileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const GetFileInfoFactory = (
return {
async getFileInfo(
space: SpaceResource,
resource?: { path?: string; fileId?: string | number },
resource?: { path?: string; fileId?: string },
options?: ListFilesOptions
): Promise<Resource> {
return (
Expand Down
4 changes: 1 addition & 3 deletions packages/web-client/src/webdav/getFileUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,14 @@ export const GetFileUrlFactory = (
} & DAVRequestOptions
): Promise<string> {
const inlineDisposition = disposition === 'inline'
const { path } = resource
let { downloadURL } = resource

let signed = true
if (!downloadURL && !inlineDisposition) {
// compute unsigned url
const webDavPath = space ? urlJoin(space.webDavPath, path) : resource.webDavPath
downloadURL = version
? dav.getFileUrl(urlJoin('meta', resource.fileId, 'v', version))
: dav.getFileUrl(webDavPath)
: dav.getFileUrl(resource.webDavPath)

if (username && doHeadRequest) {
await axiosClient.head(downloadURL)
Expand Down
6 changes: 5 additions & 1 deletion packages/web-client/src/webdav/listFiles.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
buildDeletedResource,
buildResource,
extractNodeId,
Resource,
WebDavResponseResource
} from '../helpers/resource'
Expand Down Expand Up @@ -30,7 +31,7 @@ export const ListFilesFactory = (
return {
async listFiles(
space: SpaceResource,
{ path, fileId }: { path?: string; fileId?: string | number } = {},
{ path, fileId }: { path?: string; fileId?: string } = {},
{ depth = 1, davProperties, isTrash = false, ...opts }: ListFilesOptions = {}
): Promise<ListFilesResult> {
let webDavResources: WebDavResponseResource[]
Expand Down Expand Up @@ -104,6 +105,9 @@ export const ListFilesFactory = (
if (isTrash) {
webDavPath = buildWebDavSpacesTrashPath(space.id.toString())
}
if (fileId) {
webDavPath = `${space.webDavPath}!${extractNodeId(fileId)}`
}

webDavResources = await dav.propfind(webDavPath, {
depth,
Expand Down
18 changes: 15 additions & 3 deletions packages/web-client/src/webdav/putFileContents.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { urlJoin } from '../utils'
import { SpaceResource } from '../helpers'
import { extractNodeId, SpaceResource } from '../helpers'
import { GetFileInfoFactory } from './getFileInfo'
import { WebDavOptions } from './types'
import { DAV, DAVRequestOptions } from './client'
Expand All @@ -14,28 +14,40 @@ export const PutFileContentsFactory = (
async putFileContents(
space: SpaceResource,
{
fileName,
fileId,
path,
content = '',
previousEntityTag = '',
overwrite,
onUploadProgress = null,
...opts
}: {
fileName?: string
fileId?: string
path?: string
content?: string | ArrayBuffer
previousEntityTag?: string
overwrite?: boolean
onUploadProgress?: ProgressEventCallback
} & DAVRequestOptions
) {
await dav.put(urlJoin(space.webDavPath, path), content, {
let webDavPath = urlJoin(space.webDavPath, path)
if (fileId) {
webDavPath = urlJoin(`${space.webDavPath}!${extractNodeId(fileId)}`, fileName)
}

const { result } = await dav.put(webDavPath, content, {
previousEntityTag,
overwrite,
onUploadProgress,
...opts
})

return getFileInfoFactory.getFileInfo(space, { path })
return getFileInfoFactory.getFileInfo(space, {
fileId: result.headers.get('Oc-Fileid'),
path
})
}
}
}
Loading

0 comments on commit 32748a8

Please sign in to comment.