Merge pull request #49344 from nextcloud/backport/49203/stable30

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

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=',
+		)
 	})
 })

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)
 }
@@ -67,9 +112,9 @@ export const action = new FileAction({
 		return nodes.every(isDownloadable)
 	},
 
-	async exec(node: Node, view: View, dir: string) {
+	async exec(node: Node) {
 		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)
 	},
 

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')

apps/files/src/store/files.ts

@@ -70,7 +70,7 @@ export const useFilesStore = function(...args) {
 			 *
 			 * @param service The service (files view)
 			 * @param path The path relative within the service
-			 * @returns Array of cached nodes within the path
+			 * @return Array of cached nodes within the path
 			 */
 			getNodesByPath(service: string, path?: string): Node[] {
 				const pathsStore = usePathsStore()

apps/files/src/views/FilesList.vue

@@ -422,7 +422,7 @@ export default defineComponent({
 
 		showCustomEmptyView() {
 			return !this.loading && this.isEmptyDir && this.currentView?.emptyView !== undefined
-		}
+		},
 	},
 
 	watch: {

apps/files_external/src/actions/enterCredentialsAction.ts

@@ -23,6 +23,12 @@ type CredentialResponse = {
 	password?: string,
 }
 
+/**
+ *
+ * @param node
+ * @param login
+ * @param password
+ */
 async function setCredentials(node: Node, login: string, password: string): Promise<null|true> {
 	const configResponse = await axios.put(generateUrl('apps/files_external/userglobalstorages/{id}', node.attributes), {
 		backendOptions: { user: login, password },

apps/files_sharing/src/models/Share.ts

@@ -314,7 +314,7 @@ export default class Share {
 
 	/**
 	 * Get the shared item id
-		 */
+	 */
 	get fileSource(): number {
 		return this._share.file_source
 	}

apps/files_sharing/src/utils/GeneratePassword.ts

@@ -16,6 +16,7 @@ const passwordSet = 'abcdefgijkmnopqrstwxyzABCDEFGHJKLMNPQRSTWXYZ23456789'
  * Generate a valid policy password or
  * request a valid password if password_policy
  * is enabled
+ * @param verbose
  */
 export default async function(verbose = false): Promise<string> {
 	// password policy is enabled, let's request a pass

apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue

@@ -343,16 +343,16 @@ export default {
 				.sort((a, b) => a.name.localeCompare(b.name))
 		},
 	},
-	mounted() {
-		if (this.app.groups.length > 0) {
-			this.groupCheckedAppsData = true
-		}
-	},
 	watch: {
 		'app.id'() {
 			this.removeData = false
 		},
 	},
+	mounted() {
+		if (this.app.groups.length > 0) {
+			this.groupCheckedAppsData = true
+		}
+	},
 	methods: {
 		toggleRemoveData() {
 			this.removeData = !this.removeData