[full-ci] Fix several file handling issues for apps (#6456)

* Switch around condition in getUrlForResource to make it analogous to getFileContents

* Sanitize paths when building webdav paths

* Use webDavPath in getUrlForResource

* Fix parsing of nested objects in dot syntax in router.parseQuery

* mediaviewer: fix preview images when no etag is available

* Fix path handling in mock sdk

* Add changelog item

__mocks__/sdk.js

@@ -9,27 +9,21 @@ import fixturePublicFiles from '../__fixtures__/publicFiles'
 import fixtureRecipients from '../__fixtures__/recipients'
 import { DateTime } from 'luxon'
 
+const mockPath = (path) => {
+  if (path.startsWith('/files/')) {
+    path = '/' + path.split('/').splice(3).join('/')
+  }
+  return path
+}
+
 export default {
   files: {
     list: (path = '/') => {
-      if (path.startsWith('/files/')) {
-        path = path.split('/').splice(3).join('/')
-        if (path === '') {
-          path = '/'
-        }
-      }
-
-      return fixtureFiles[path]
+      return fixtureFiles[mockPath(path)]
     },
     getFavoriteFiles: () => fixtureFavoriteFiles,
     fileInfo: (path) => {
-      if (path.startsWith('/files/')) {
-        path = path.split('/').splice(3).join('/')
-        if (path === '') {
-          path = '/'
-        }
-      }
-      return fixtureFiles[path][0]
+      return fixtureFiles[mockPath(path)][0]
     }
   },
   users: {

changelog/unreleased/bugfix-app-file-handling

@@ -0,0 +1,6 @@
+Bugfix: File handling in apps
+
+We fixed loading and saving files in apps in all contexts. It's now possible to open files in apps in personal files, favorites, share views and spaces.
+
+https://github.com/owncloud/web/pull/6456
+

packages/web-app-files/src/helpers/resource/resource.ts

@@ -3,5 +3,6 @@
 export interface Resource {
   id: number | string
   path: string
+  webDavPath: string
   downloadURL?: string
 }

packages/web-app-files/src/helpers/resources.js

@@ -177,11 +177,11 @@ export function buildSpace(space) {
 }
 
 export function buildWebDavFilesPath(userId, path) {
-  return `/files/${userId}/${path}`
+  return '/' + `files/${userId}/${path}`.split('/').filter(Boolean).join('/')
 }
 
 export function buildWebDavSpacesPath(spaceId, path) {
-  return `/spaces/${spaceId}/${path}`
+  return '/' + `spaces/${spaceId}/${path}`.split('/').filter(Boolean).join('/')
 }
 
 export function attachIndicators(resource, sharesTree) {

packages/web-app-files/tests/unit/helpers/resource/sameResource.spec.ts

@@ -4,20 +4,30 @@ describe('isSameResource', () => {
   test('evaluates to false if one of the resources is nullish', () => {
     expect(isSameResource(null, null)).toBe(false)
     expect(isSameResource(undefined, undefined)).toBe(false)
-    expect(isSameResource(null, { id: 1, path: '' })).toBe(false)
-    expect(isSameResource(undefined, { id: 1, path: '' })).toBe(false)
-    expect(isSameResource({ id: 1, path: '' }, null)).toBe(false)
-    expect(isSameResource({ id: 1, path: '' }, undefined)).toBe(false)
+    expect(isSameResource(null, { id: 1, path: '', webDavPath: '' })).toBe(false)
+    expect(isSameResource(undefined, { id: 1, path: '', webDavPath: '' })).toBe(false)
+    expect(isSameResource({ id: 1, path: '', webDavPath: '' }, null)).toBe(false)
+    expect(isSameResource({ id: 1, path: '', webDavPath: '' }, undefined)).toBe(false)
   })
   test('evaluates to false if ids are of different types', () => {
-    expect(isSameResource({ id: 1, path: '' }, { id: '1', path: '' })).toBe(false)
+    expect(
+      isSameResource({ id: 1, path: '', webDavPath: '' }, { id: '1', path: '', webDavPath: '' })
+    ).toBe(false)
   })
   test('evaluates to false if ids are different values', () => {
-    expect(isSameResource({ id: 1, path: '' }, { id: 2, path: '' })).toBe(false)
-    expect(isSameResource({ id: '1', path: '' }, { id: '2', path: '' })).toBe(false)
+    expect(
+      isSameResource({ id: 1, path: '', webDavPath: '' }, { id: 2, path: '', webDavPath: '' })
+    ).toBe(false)
+    expect(
+      isSameResource({ id: '1', path: '', webDavPath: '' }, { id: '2', path: '', webDavPath: '' })
+    ).toBe(false)
   })
   test('evaluates to true if ids are the same', () => {
-    expect(isSameResource({ id: 1, path: '' }, { id: 1, path: '' })).toBe(true)
-    expect(isSameResource({ id: '1', path: '' }, { id: '1', path: '' })).toBe(true)
+    expect(
+      isSameResource({ id: 1, path: '', webDavPath: '' }, { id: 1, path: '', webDavPath: '' })
+    ).toBe(true)
+    expect(
+      isSameResource({ id: '1', path: '', webDavPath: '' }, { id: '1', path: '', webDavPath: '' })
+    ).toBe(true)
   })
 })

packages/web-app-media-viewer/src/App.vue

@@ -185,7 +185,8 @@ export default {
         x: this.thumbDimensions,
         y: this.thumbDimensions,
         // strip double quotes from etag
-        c: this.activeMediaFile.etag.substr(1, this.activeMediaFile.etag.length - 2),
+        // we have no etag, e.g. on shared with others page
+        c: this.activeMediaFile.etag?.substr(1, this.activeMediaFile.etag.length - 2),
         scalingup: 0,
         preview: 1,
         a: 1
@@ -256,7 +257,7 @@ export default {
 
     // update route and url
     updateLocalHistory() {
-      this.$route.params.filePath = this.activeMediaFile.path
+      this.$route.params.filePath = this.activeMediaFile.webDavPath
       history.pushState({}, document.title, this.$router.resolve(this.$route).href)
     },
 

packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts

@@ -29,30 +29,31 @@ export function useAppFileHandling(options: AppFileHandlingOptions): AppFileHand
   const isPublicLinkContext = options.isPublicLinkContext
   const publicLinkPassword = options.publicLinkPassword
 
-  const getUrlForResource = ({ path, downloadURL }: Resource, query: QueryParameters = null) => {
-    const queryStr = !query ? '' : qs.stringify(query)
-    if (!unref(isPublicLinkContext)) {
-      const urlPath = ['..', 'dav', 'files', store.getters.user.id, path.replace(/^\//, '')].join(
-        '/'
-      )
-      return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?')
-    }
+  const getUrlForResource = (
+    { webDavPath, downloadURL }: Resource,
+    query: QueryParameters = null
+  ) => {
+    const queryStr = qs.stringify(query)
+    if (unref(isPublicLinkContext)) {
+      // If the resource does not contain the downloadURL we fallback to the normal
+      // public files path.
+      if (!downloadURL) {
+        // TODO: check whether we can fix the resource to always contain public-files in the webDavPath
+        const urlPath = ['public-files', webDavPath].join('/')
+        return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?')
+      }
 
-    // If the resource does not contain the downloadURL we fallback to the normal
-    // public files path.
-    if (!downloadURL) {
-      const urlPath = ['..', 'dav', 'public-files', path].join('/')
-      return [client.files.getFileUrl(urlPath), queryStr].filter(Boolean).join('?')
-    }
+      // In a public context, i.e. public shares, the downloadURL contains a pre-signed url to
+      // download the file.
+      const [url, signedQuery] = downloadURL.split('?')
 
-    // In a public context, i.e. public shares, the downloadURL contains a pre-signed url to
-    // download the file.
-    const [url, signedQuery] = downloadURL.split('?')
+      // Since the pre-signed url contains query parameters and the caller of this method
+      // can also provide query parameters we have to combine them.
+      const combinedQuery = [queryStr, signedQuery].filter(Boolean).join('&')
+      return [url, combinedQuery].filter(Boolean).join('?')
+    }
 
-    // Since the pre-signed url contains query parameters and the caller of this method
-    // can also provide query parameters we have to combine them.
-    const combinedQuery = [queryStr, signedQuery].filter(Boolean).join('&')
-    return [url, combinedQuery].filter(Boolean).join('?')
+    return [client.files.getFileUrl(webDavPath), queryStr].filter(Boolean).join('?')
   }
 
   const getFileContents = async (filePath: string) => {

packages/web-runtime/src/router/index.js

@@ -50,7 +50,9 @@ const base = document.querySelector('base')
 export const router = patchRouter(
   new Router({
     parseQuery(query) {
-      return qs.parse(query)
+      return qs.parse(query, {
+        allowDots: true
+      })
     },
     stringifyQuery(obj) {
       return qs.stringify(obj, {