From: Ferdinand Thiessen Date: Mon, 11 Nov 2024 15:51:43 +0000 (+0100) Subject: fix(files): Allow downloading multiple nodes not from same base X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fheads%2Fbackport%2F49203%2Fstable29;p=nextcloud-server.git 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 --- diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts index 8842bd09fb0..13b67363f0b 100644 --- a/apps/files/src/actions/downloadAction.spec.ts +++ b/apps/files/src/actions/downloadAction.spec.ts @@ -21,7 +21,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', @@ -121,7 +128,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) }) @@ -139,7 +148,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) }) @@ -156,7 +167,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) }) @@ -181,7 +196,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 03686cd4243..8fc5d66db9d 100644 --- a/apps/files/src/actions/downloadAction.ts +++ b/apps/files/src/actions/downloadAction.ts @@ -31,12 +31,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) } @@ -83,7 +128,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 } @@ -97,7 +142,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 3d32930c273..fc4eb35e665 100644 --- a/apps/files/src/actions/editLocallyAction.spec.ts +++ b/apps/files/src/actions/editLocallyAction.spec.ts @@ -137,7 +137,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') diff --git a/apps/files/src/actions/sidebarAction.ts b/apps/files/src/actions/sidebarAction.ts index 0db98546934..0e61d772e95 100644 --- a/apps/files/src/actions/sidebarAction.ts +++ b/apps/files/src/actions/sidebarAction.ts @@ -19,7 +19,7 @@ * along with this program. If not, see . * */ -import { Permission, type Node, View, FileAction, FileType } from '@nextcloud/files' +import { Permission, type Node, View, FileAction } from '@nextcloud/files' import { translate as t } from '@nextcloud/l10n' import InformationSvg from '@mdi/svg/svg/information-variant.svg?raw'