fix(files): Allow downloading multiple nodes not from same base

When downloading files in e.g. the *favorites* or *recent* view,
then the nodes are not always share the same parent folder
and we can not use the current directory as it is probably just a
virtual one.

So we calculate the longest common base and use that as the directory
for the download endpoint.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts
index 59e533441be6f104abc141625ebc883f3d5521d1..5c63acefebc41424cedc42fa406a71aabfc2dac7 100644 (file)
--- a/apps/files/src/actions/downloadAction.spec.ts
+++ b/apps/files/src/actions/downloadAction.spec.ts
@@ -4,7 +4,14 @@
  */
 import { action } from './downloadAction'
 import { expect } from '@jest/globals'
-import { File, Folder, Permission, View, FileAction, DefaultType } from '@nextcloud/files'
+import {
+       File,
+       Folder,
+       Permission,
+       View,
+       FileAction,
+       DefaultType,
+} from '@nextcloud/files'
 
 const view = {
        id: 'files',
@@ -104,7 +111,9 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toBe(null)
                expect(link.download).toEqual('')
-               expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+               expect(link.href).toEqual(
+                       'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
+               )
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -122,7 +131,9 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toStrictEqual([null])
                expect(link.download).toEqual('')
-               expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+               expect(link.href).toEqual(
+                       'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
+               )
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -139,7 +150,11 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toBe(null)
                expect(link.download).toEqual('')
-               expect(link.href.startsWith('/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22FooBar%22%5D&downloadStartSecret=')).toBe(true)
+               expect(
+                       link.href.startsWith(
+                               '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22FooBar%22%5D&downloadStartSecret=',
+                       ),
+               ).toBe(true)
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -164,7 +179,76 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toStrictEqual([null, null])
                expect(link.download).toEqual('')
-               expect(link.href.startsWith('/index.php/apps/files/ajax/download.php?dir=%2FDir&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D&downloadStartSecret=')).toBe(true)
                expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D&downloadStartSecret=',
+               )
+       })
+
+       test('Download multiple nodes from different sources', async () => {
+               const files = [
+                       new File({
+                               id: 1,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1/foo.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new File({
+                               id: 2,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 2/bar.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new File({
+                               id: 3,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 2/baz.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+               ]
+
+               const exec = await action.execBatch!(files, view, '/Dir')
+
+               // Silent action
+               expect(exec).toStrictEqual([null, null, null])
+               expect(link.download).toEqual('')
+               expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22bar.txt%22%2C%22baz.txt%22%5D&downloadStartSecret=',
+               )
+       })
+
+       test('Download node and parent folder', async () => {
+               const files = [
+                       new File({
+                               id: 1,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1/foo.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new Folder({
+                               id: 2,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1',
+                               owner: 'admin',
+                               permissions: Permission.READ,
+                       }),
+               ]
+
+               const exec = await action.execBatch!(files, view, '/Dir')
+
+               // Silent action
+               expect(exec).toStrictEqual([null, null])
+               expect(link.download).toEqual('')
+               expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22Folder%201%22%5D&downloadStartSecret=',
+               )
        })
 })

diff --git a/apps/files/src/actions/downloadAction.ts b/apps/files/src/actions/downloadAction.ts
index cce1e48579a8a0ecc863647d2e96276d2a4641da..f1860c7cd3fbec1370e69f48e0fcd6c3bed96a1b 100644 (file)
--- a/apps/files/src/actions/downloadAction.ts
+++ b/apps/files/src/actions/downloadAction.ts
@@ -17,12 +17,57 @@ const triggerDownload = function(url: string) {
        hiddenElement.click()
 }
 
-const downloadNodes = function(dir: string, nodes: Node[]) {
+/**
+ * Find the longest common path prefix of both input paths
+ * @param first The first path
+ * @param second The second path
+ */
+function longestCommonPath(first: string, second: string): string {
+       const firstSegments = first.split('/').filter(Boolean)
+       const secondSegments = second.split('/').filter(Boolean)
+       let base = '/'
+       for (const [index, segment] of firstSegments.entries()) {
+               if (index >= second.length) {
+                       break
+               }
+               if (segment !== secondSegments[index]) {
+                       break
+               }
+               const sep = base === '/' ? '' : '/'
+               base = `${base}${sep}${segment}`
+       }
+       return base
+}
+
+/**
+ * Handle downloading multiple nodes
+ * @param nodes The nodes to download
+ */
+function downloadNodes(nodes: Node[]): void {
+       // Remove nodes that are already included in parent folders
+       // Example: Download A/foo.txt and A will only return A as A/foo.txt is already included
+       const filteredNodes = nodes.filter((node) => {
+               const parent = nodes.find((other) => (
+                       other.type === FileType.Folder
+                               && node.path.startsWith(`${other.path}/`)
+               ))
+               return parent === undefined
+       })
+
+       let base = filteredNodes[0].dirname
+       for (const node of filteredNodes.slice(1)) {
+               base = longestCommonPath(base, node.dirname)
+       }
+       base = base || '/'
+
+       // Remove the common prefix
+       const filenames = filteredNodes.map((node) => node.path.slice(base === '/' ? 1 : (base.length + 1)))
+
        const secret = Math.random().toString(36).substring(2)
-       const url = generateUrl('/apps/files/ajax/download.php?dir={dir}&files={files}&downloadStartSecret={secret}', {
-               dir,
+       const url = generateUrl('/apps/files/ajax/download.php?dir={base}&files={files}&downloadStartSecret={secret}', {
+               base,
                secret,
-               files: JSON.stringify(nodes.map(node => node.basename)),
+               files: JSON.stringify(filenames),
        })
        triggerDownload(url)
 }
@@ -69,7 +114,7 @@ export const action = new FileAction({
 
        async exec(node: Node, view: View, dir: string) {
                if (node.type === FileType.Folder) {
-                       downloadNodes(dir, [node])
+                       downloadNodes([node])
                        return null
                }
 
@@ -83,7 +128,7 @@ export const action = new FileAction({
                        return [null]
                }
 
-               downloadNodes(dir, nodes)
+               downloadNodes(nodes)
                return new Array(nodes.length).fill(null)
        },
 

diff --git a/apps/files/src/actions/editLocallyAction.spec.ts b/apps/files/src/actions/editLocallyAction.spec.ts
index ca2e1d05f962c3f6a690f1447b3edd661ec9102f..7eaf24dd3a5e14fbb49ad0fd51d52f02ae41674b 100644 (file)
--- a/apps/files/src/actions/editLocallyAction.spec.ts
+++ b/apps/files/src/actions/editLocallyAction.spec.ts
@@ -120,7 +120,7 @@ describe('Edit locally action enabled tests', () => {
 describe('Edit locally action execute tests', () => {
        test('Edit locally opens proper URL', async () => {
                jest.spyOn(axios, 'post').mockImplementation(async () => ({
-                       data: { ocs: { data: { token: 'foobar' } } }
+                       data: { ocs: { data: { token: 'foobar' } } },
                }))
                const mockedShowError = jest.mocked(showError)
                const spyDialogBuilder = jest.spyOn(dialogBuilder, 'build')