Merge pull request #58611 from nextcloud/fix/files-snowflake

fix(files): correctly handle nodes with snowflake ids

apps/files/src/store/files.ts

@@ -3,203 +3,259 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { Folder, Node } from '@nextcloud/files'
-import type { FileSource, FilesState, FilesStore, RootOptions, RootsStore, Service } from '../types.ts'
+import type { IFolder, INode } from '@nextcloud/files'
+import type { FileSource, FilesStore, RootOptions, RootsStore, Service } from '../types.ts'
 
 import { subscribe } from '@nextcloud/event-bus'
 import { defineStore } from 'pinia'
-import Vue from 'vue'
+import Vue, { ref } from 'vue'
 import logger from '../logger.ts'
 import { fetchNode } from '../services/WebdavClient.ts'
 import { usePathsStore } from './paths.ts'
 
 /**
- *
- * @param args
+ * Store for files and folders in the files app.
  */
-export function useFilesStore(...args) {
-	const store = defineStore('files', {
-		state: (): FilesState => ({
-			files: {} as FilesStore,
-			roots: {} as RootsStore,
-		}),
+export const useFilesStore = defineStore('files', () => {
+	const files = ref<FilesStore>({})
+	const roots = ref<RootsStore>({})
 
-		getters: {
-			/**
-			 * Get a file or folder by its source
-			 *
-			 * @param state
-			 */
-			getNode: (state) => (source: FileSource): Node | undefined => state.files[source],
+	// initialize the store once its used first time
+	initalizeStore()
 
-			/**
-			 * Get a list of files or folders by their IDs
-			 * Note: does not return undefined values
-			 *
-			 * @param state
-			 */
-			getNodes: (state) => (sources: FileSource[]): Node[] => sources
-				.map((source) => state.files[source])
-				.filter(Boolean),
-
-			/**
-			 * Get files or folders by their file ID
-			 * Multiple nodes can have the same file ID but different sources
-			 * (e.g. in a shared context)
-			 *
-			 * @param state
-			 */
-			getNodesById: (state) => (fileId: number): Node[] => Object.values(state.files).filter((node) => node.fileid === fileId),
-
-			/**
-			 * Get the root folder of a service
-			 *
-			 * @param state
-			 */
-			getRoot: (state) => (service: Service): Folder | undefined => state.roots[service],
-		},
-
-		actions: {
-			/**
-			 * Get cached directory matching a given path
-			 *
-			 * @param service - The service (files view)
-			 * @param path - The path relative within the service
-			 * @return The folder if found
-			 */
-			getDirectoryByPath(service: string, path?: string): Folder | undefined {
-				const pathsStore = usePathsStore()
-				let folder: Folder | undefined
-
-				// Get the containing folder from path store
-				if (!path || path === '/') {
-					folder = this.getRoot(service)
-				} else {
-					const source = pathsStore.getPath(service, path)
-					if (source) {
-						folder = this.getNode(source) as Folder | undefined
-					}
-				}
-
-				return folder
-			},
-
-			/**
-			 * Get cached child nodes within a given path
-			 *
-			 * @param service - The service (files view)
-			 * @param path - The path relative within the service
-			 * @return Array of cached nodes within the path
-			 */
-			getNodesByPath(service: string, path?: string): Node[] {
-				const folder = this.getDirectoryByPath(service, path)
-
-				// If we found a cache entry and the cache entry was already loaded (has children) then use it
-				return (folder?._children ?? [])
-					.map((source: string) => this.getNode(source))
-					.filter(Boolean)
-			},
-
-			updateNodes(nodes: Node[]) {
-				// Update the store all at once
-				const files = nodes.reduce((acc, node) => {
-					if (!node.fileid) {
-						logger.error('Trying to update/set a node without fileid', { node })
-						return acc
-					}
-
-					acc[node.source] = node
-					return acc
-				}, {} as FilesStore)
-
-				Vue.set(this, 'files', { ...this.files, ...files })
-			},
-
-			deleteNodes(nodes: Node[]) {
-				nodes.forEach((node) => {
-					if (node.source) {
-						Vue.delete(this.files, node.source)
-					}
-				})
-			},
-
-			setRoot({ service, root }: RootOptions) {
-				Vue.set(this.roots, service, root)
-			},
-
-			onDeletedNode(node: Node) {
-				this.deleteNodes([node])
-			},
-
-			onCreatedNode(node: Node) {
-				this.updateNodes([node])
-			},
-
-			onMovedNode({ node, oldSource }: { node: Node, oldSource: string }) {
-				if (!node.fileid) {
-					logger.error('Trying to update/set a node without fileid', { node })
-					return
-				}
-
-				// Update the path of the node
-				Vue.delete(this.files, oldSource)
-				this.updateNodes([node])
-			},
-
-			async onUpdatedNode(node: Node) {
-				if (!node.fileid) {
-					logger.error('Trying to update/set a node without fileid', { node })
-					return
-				}
-
-				// If we have multiple nodes with the same file ID, we need to update all of them
-				const nodes = this.getNodesById(node.fileid)
-				if (nodes.length > 1) {
-					await Promise.all(nodes.map((node) => fetchNode(node.path))).then(this.updateNodes)
-					logger.debug(nodes.length + ' nodes updated in store', { fileid: node.fileid })
-					return
-				}
-
-				// If we have only one node with the file ID, we can update it directly
-				if (nodes.length === 1 && node.source === nodes[0].source) {
-					this.updateNodes([node])
-					return
-				}
-
-				// Otherwise, it means we receive an event for a node that is not in the store
-				fetchNode(node.path).then((n) => this.updateNodes([n]))
-			},
-
-			// Handlers for legacy sidebar (no real nodes support)
-			onAddFavorite(node: Node) {
-				const ourNode = this.getNode(node.source)
-				if (ourNode) {
-					Vue.set(ourNode.attributes, 'favorite', 1)
-				}
-			},
-
-			onRemoveFavorite(node: Node) {
-				const ourNode = this.getNode(node.source)
-				if (ourNode) {
-					Vue.set(ourNode.attributes, 'favorite', 0)
-				}
-			},
-		},
-	})
-
-	const fileStore = store(...args)
-	// Make sure we only register the listeners once
-	if (!fileStore._initialized) {
-		subscribe('files:node:created', fileStore.onCreatedNode)
-		subscribe('files:node:deleted', fileStore.onDeletedNode)
-		subscribe('files:node:updated', fileStore.onUpdatedNode)
-		subscribe('files:node:moved', fileStore.onMovedNode)
-		// legacy sidebar
-		subscribe('files:favorites:added', fileStore.onAddFavorite)
-		subscribe('files:favorites:removed', fileStore.onRemoveFavorite)
-
-		fileStore._initialized = true
+	/**
+	 * Get a file or folder by its source
+	 *
+	 * @param source - The file source
+	 */
+	function getNode(source: FileSource): INode | undefined {
+		return files.value[source]
 	}
 
-	return fileStore
-}
+	/**
+	 * Get a list of files or folders by their IDs
+	 * Note: does not return undefined values
+	 *
+	 * @param sources - The file sources
+	 */
+	function getNodes(sources: FileSource[]): INode[] {
+		return sources
+			.map((source) => files.value[source])
+			.filter(Boolean) as INode[]
+	}
+
+	/**
+	 * Get files or folders by their ID
+	 * Multiple nodes can have the same ID but different sources
+	 * (e.g. in a shared context)
+	 *
+	 * @param id - The file ID
+		*/
+	function getNodesById(id: string): INode[] {
+		return Object.values(files.value)
+			.filter((node) => node.id === id)
+	}
+
+	/**
+	 * Get the root folder of a service
+	 *
+	 * @param service - The service (files view)
+	 * @return The root folder if set
+	 */
+	function getRoot(service: Service): IFolder | undefined {
+		return roots.value[service]
+	}
+
+	/**
+	 * Get cached directory matching a given path
+	 *
+	 * @param service - The service (files view)
+	 * @param path - The path relative within the service
+	 * @return The folder if found
+	 */
+	function getDirectoryByPath(service: string, path?: string): IFolder | undefined {
+		const pathsStore = usePathsStore()
+		let folder: IFolder | undefined
+
+		// Get the containing folder from path store
+		if (!path || path === '/') {
+			folder = getRoot(service)
+		} else {
+			const source = pathsStore.getPath(service, path)
+			if (source) {
+				folder = getNode(source) as IFolder | undefined
+			}
+		}
+
+		return folder
+	}
+
+	/**
+	 * Get cached child nodes within a given path
+	 *
+	 * @param service - The service (files view)
+	 * @param path - The path relative within the service
+	 * @return Array of cached nodes within the path
+	 */
+	function getNodesByPath(service: string, path?: string): INode[] {
+		const folder = getDirectoryByPath(service, path)
+
+		// If we found a cache entry and the cache entry was already loaded (has children) then use it
+		return ((folder as { _children?: string[] })?._children ?? [])
+			.map((source: string) => getNode(source))
+			.filter(Boolean) as INode[]
+	}
+
+	/**
+	 * Update or set nodes in the store
+	 *
+	 * @param nodes - The nodes to update or set
+	 */
+	function updateNodes(nodes: INode[]) {
+		// Update the store all at once
+		const newNodes = nodes.reduce((acc, node) => {
+			if (files.value[node.source]?.id && !node.id) {
+				logger.error('Trying to update/set a node without id', { node })
+				return acc
+			}
+
+			acc[node.source] = node
+			return acc
+		}, {} as FilesStore)
+
+		files.value = { ...files.value, ...newNodes }
+	}
+
+	/**
+	 * Delete nodes from the store
+	 *
+	 * @param nodes - The nodes to delete
+	 */
+	function deleteNodes(nodes: INode[]) {
+		const entries = Object.entries(files.value)
+			.filter(([, node]) => !nodes.some((n) => n.source === node.source))
+		files.value = Object.fromEntries(entries)
+	}
+
+	/**
+	 * Set the root folder for a service
+	 *
+	 * @param options - The options for setting the root
+	 * @param options.service - The service (files view)
+	 * @param options.root - The root folder
+	 */
+	function setRoot({ service, root }: RootOptions) {
+		roots.value = { ...roots.value, [service]: root }
+	}
+
+	return {
+		files,
+		roots,
+
+		deleteNodes,
+		getDirectoryByPath,
+		getNode,
+		getNodes,
+		getNodesById,
+		getNodesByPath,
+		getRoot,
+		setRoot,
+		updateNodes,
+	}
+
+	// Internal helper functions
+
+	/**
+	 * Initialize the store by subscribing to events
+	 */
+	function initalizeStore() {
+		subscribe('files:node:created', onCreatedNode)
+		subscribe('files:node:deleted', onDeletedNode)
+		subscribe('files:node:updated', onUpdatedNode)
+		subscribe('files:node:moved', onMovedNode)
+		// legacy sidebar
+		subscribe('files:favorites:added', onAddFavorite)
+		subscribe('files:favorites:removed', onRemoveFavorite)
+	}
+
+	/**
+	 * Called when a node is deleted, removes the node from the store
+	 *
+	 * @param node - The deleted node
+	 */
+	function onDeletedNode(node: INode) {
+		deleteNodes([node])
+	}
+
+	/**
+	 * Handler for when a node is created
+	 *
+	 * @param node - The created node
+	 */
+	function onCreatedNode(node: INode) {
+		updateNodes([node])
+	}
+
+	/**
+	 * Handler for when a node is moved, updates the path of the node in the store
+	 *
+	 * @param context - The context of the moved node
+	 * @param context.node - The moved node
+	 * @param context.oldSource - The old source of the node before it was moved
+	 */
+	function onMovedNode({ node, oldSource }: { node: INode, oldSource: string }) {
+		// Update the path of the node
+		delete files.value[oldSource]
+		updateNodes([node])
+	}
+
+	/**
+	 * Handler for when a node is updated, updates the node in the store
+	 *
+	 * @param node - The updated node
+	 */
+	async function onUpdatedNode(node: INode) {
+		// If we have multiple nodes with the same file ID, we need to update all of them
+		const nodes = node.id
+			? getNodesById(node.id)
+			: getNodes([node.source])
+		if (nodes.length > 1) {
+			await Promise.all(nodes.map((node) => fetchNode(node.path))).then(updateNodes)
+			logger.debug(nodes.length + ' nodes updated in store', { fileid: node.id, source: node.source })
+			return
+		}
+
+		// If we have only one node with the file ID, we can update it directly
+		if (nodes.length === 1 && node.source === nodes[0]!.source) {
+			updateNodes([node])
+			return
+		}
+
+		// Otherwise, it means we receive an event for a node that is not in the store
+		fetchNode(node.path).then((n) => updateNodes([n]))
+	}
+
+	/**
+	 * Handlers for legacy sidebar (no real nodes support)
+	 *
+	 * @param node - The node that was added to favorites
+	 */
+	function onAddFavorite(node: INode) {
+		const ourNode = getNode(node.source)
+		if (ourNode) {
+			Vue.set(ourNode.attributes, 'favorite', 1)
+		}
+	}
+
+	/**
+	 * Handler for when a node is removed from favorites
+	 *
+	 * @param node - The removed favorite
+	 */
+	function onRemoveFavorite(node: INode) {
+		const ourNode = getNode(node.source)
+		if (ourNode) {
+			Vue.set(ourNode.attributes, 'favorite', 0)
+		}
+	}
+})

cypress/e2e/files_external/files-external-failed.cy.ts

@@ -11,14 +11,12 @@ import { AuthBackend, createStorageWithConfig, StorageBackend } from './StorageU
 describe('Files user credentials', { testIsolation: true }, () => {
 	let currentUser: User
 
-	beforeEach(() => {
-	})
-
 	before(() => {
 		cy.runOccCommand('app:enable files_external')
 		cy.createRandomUser().then((user) => {
 			currentUser = user
 		})
+		cy.runCommand('php ./cron.php')
 	})
 
 	afterEach(() => {
@@ -42,6 +40,11 @@ describe('Files user credentials', { testIsolation: true }, () => {
 		cy.login(currentUser)
 		cy.visit('/apps/files')
 
+		// TODO: Why does the first PROPFIND does not return it?
+		getRowForFile('Storage1')
+			.if('not.exist')
+			.reload()
+
 		// Ensure the row is visible and marked as unavailable
 		getRowForFile('Storage1').as('row').should('be.visible')
 		cy.get('@row').find('[data-cy-files-list-row-name-link]')