[stable29] fix(files): Ensure children are removed from folder and not duplicated

apps/files/src/eventbus.d.ts

@@ -3,9 +3,12 @@ import type { Node } from '@nextcloud/files'
 declare module '@nextcloud/event-bus' {
 	export interface NextcloudEvents {
 		// mapping of 'event name' => 'event type'
-		'files:favorites:removed': Node
 		'files:favorites:added': Node
+		'files:favorites:removed': Node
+		'files:node:created': Node
+		'files:node:deleted': Node
 		'files:node:renamed': Node
+		'files:node:updated': Node
 		'nextcloud:unified-search.search': { query: string }
 	}
 }

apps/files/src/store/files.ts

@@ -44,9 +44,10 @@ const fetchNode = async (node: Node): Promise<Node> => {
 
 export const useFilesStore = function(...args) {
 	const store = defineStore('files', {
-		state: (): FilesState => ({
+		state: () => ({
 			files: {} as FilesStore,
 			roots: {} as RootsStore,
+			_initialized: false,
 		}),
 
 		getters: {

apps/files/src/store/paths.spec.ts

@@ -0,0 +1,130 @@
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { describe, beforeEach, test, expect } from '@jest/globals'
+import { setActivePinia, createPinia } from 'pinia'
+import { usePathsStore } from './paths.ts'
+import { emit } from '@nextcloud/event-bus'
+import { File, Folder } from '@nextcloud/files'
+import { useFilesStore } from './files.ts'
+
+describe('Path store', () => {
+
+	let store: ReturnType<typeof usePathsStore>
+	let files: ReturnType<typeof useFilesStore>
+	let root: Folder & { _children?: string[] }
+
+	beforeEach(() => {
+		setActivePinia(createPinia())
+
+		root = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/', id: 1 })
+		files = useFilesStore()
+		files.setRoot({ service: 'files', root })
+
+		store = usePathsStore()
+	})
+
+	test('Folder is created', () => {
+		// no defined paths
+		expect(store.paths).toEqual({})
+
+		// create the folder
+		const node = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/folder', id: 2 })
+		emit('files:node:created', node)
+
+		// see that the path is added
+		expect(store.paths).toEqual({ files: { [node.path]: node.source } })
+
+		// see that the node is added
+		expect(root._children).toEqual([node.source])
+	})
+
+	test('File is created', () => {
+		// no defined paths
+		expect(store.paths).toEqual({})
+
+		// create the file
+		const node = new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', id: 2, mime: 'text/plain' })
+		emit('files:node:created', node)
+
+		// see that there are still no paths
+		expect(store.paths).toEqual({})
+
+		// see that the node is added
+		expect(root._children).toEqual([node.source])
+	})
+
+	test('Existing file is created', () => {
+		// no defined paths
+		expect(store.paths).toEqual({})
+
+		// create the file
+		const node1 = new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', id: 2, mime: 'text/plain' })
+		emit('files:node:created', node1)
+
+		// see that there are still no paths
+		expect(store.paths).toEqual({})
+
+		// see that the node is added
+		expect(root._children).toEqual([node1.source])
+
+		// create the same named file again
+		const node2 = new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', id: 2, mime: 'text/plain' })
+		emit('files:node:created', node2)
+
+		// see that there are still no paths and the children are not duplicated
+		expect(store.paths).toEqual({})
+		expect(root._children).toEqual([node1.source])
+
+	})
+
+	test('Existing folder is created', () => {
+		// no defined paths
+		expect(store.paths).toEqual({})
+
+		// create the file
+		const node1 = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/folder', id: 2 })
+		emit('files:node:created', node1)
+
+		// see the path is added
+		expect(store.paths).toEqual({ files: { [node1.path]: node1.source } })
+
+		// see that the node is added
+		expect(root._children).toEqual([node1.source])
+
+		// create the same named file again
+		const node2 = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/folder', id: 2 })
+		emit('files:node:created', node2)
+
+		// see that there is still only one paths and the children are not duplicated
+		expect(store.paths).toEqual({ files: { [node1.path]: node1.source } })
+		expect(root._children).toEqual([node1.source])
+	})
+
+	test('Folder is deleted', () => {
+		const node = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/folder', id: 2 })
+		emit('files:node:created', node)
+		// see that the path is added and the children are set-up
+		expect(store.paths).toEqual({ files: { [node.path]: node.source } })
+		expect(root._children).toEqual([node.source])
+
+		emit('files:node:deleted', node)
+		// See the path is removed
+		expect(store.paths).toEqual({ files: {} })
+		// See the child is removed
+		expect(root._children).toEqual([])
+	})
+
+	test('File is deleted', () => {
+		const node = new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', id: 2, mime: 'text/plain' })
+		emit('files:node:created', node)
+		// see that the children are set-up
+		expect(root._children).toEqual([node.source])
+
+		emit('files:node:deleted', node)
+		// See the child is removed
+		expect(root._children).toEqual([])
+	})
+})

apps/files/src/store/paths.ts

@@ -19,7 +19,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import type { FileSource, PathsStore, PathOptions, ServicesState } from '../types'
+import type { FileSource, PathsStore, PathOptions, ServicesState, Service } from '../types'
 import { defineStore } from 'pinia'
 import { FileType, Folder, Node, getNavigation } from '@nextcloud/files'
 import { subscribe } from '@nextcloud/event-bus'
@@ -34,7 +34,8 @@ export const usePathsStore = function(...args) {
 	const store = defineStore('paths', {
 		state: () => ({
 			paths: {} as ServicesState,
-		} as PathsStore),
+			_initialized: false,
+		}),
 
 		getters: {
 			getPath: (state) => {
@@ -58,6 +59,57 @@ export const usePathsStore = function(...args) {
 				Vue.set(this.paths[payload.service], payload.path, payload.source)
 			},
 
+			deletePath(service: Service, path: string) {
+				// skip if service does not exist
+				if (!this.paths[service]) {
+					return
+				}
+
+				Vue.delete(this.paths[service], path)
+			},
+
+			onDeletedNode(node: Node) {
+				const service = getNavigation()?.active?.id || 'files'
+
+				if (node.type === FileType.Folder) {
+					// Delete the path
+					this.deletePath(
+						service,
+						node.path,
+					)
+				}
+
+				// Remove node from children
+				if (node.dirname === '/') {
+					const root = files.getRoot(service) as Folder & { _children?: string[] }
+					// ensure sources are unique
+					const children = new Set(root._children ?? [])
+					children.delete(node.source)
+					Vue.set(root, '_children', [...children.values()])
+					return
+				}
+
+				if (this.paths[service][node.dirname]) {
+					const parentSource = this.paths[service][node.dirname]
+					const parentFolder = files.getNode(parentSource) as Folder & { _children?: string[] }
+
+					if (!parentFolder) {
+						logger.error('Parent folder not found', { parentSource })
+						return
+					}
+
+					logger.debug('Path exists, removing from children', { parentFolder, node })
+
+					// ensure sources are unique
+					const children = new Set(parentFolder._children ?? [])
+					children.delete(node.source)
+					Vue.set(parentFolder, '_children', [...children.values()])
+					return
+				}
+
+				logger.debug('Parent path does not exists, skipping children update', { node })
+			},
+
 			onCreatedNode(node: Node) {
 				const service = getNavigation()?.active?.id || 'files'
 				if (!node.fileid) {
@@ -77,30 +129,30 @@ export const usePathsStore = function(...args) {
 				// Update parent folder children if exists
 				// If the folder is the root, get it and update it
 				if (node.dirname === '/') {
-					const root = files.getRoot(service)
-					if (!root._children) {
-						Vue.set(root, '_children', [])
-					}
-					root._children.push(node.source)
+					const root = files.getRoot(service) as Folder & { _children?: string[] }
+					// ensure sources are unique
+					const children = new Set(root._children ?? [])
+					children.add(node.source)
+					Vue.set(root, '_children', [...children.values()])
 					return
 				}
 
 				// If the folder doesn't exists yet, it will be
 				// fetched later and its children updated anyway.
 				if (this.paths[service][node.dirname]) {
 					const parentSource = this.paths[service][node.dirname]
-					const parentFolder = files.getNode(parentSource) as Folder
+					const parentFolder = files.getNode(parentSource) as Folder & { _children?: string[] }
 					logger.debug('Path already exists, updating children', { parentFolder, node })
 
 					if (!parentFolder) {
 						logger.error('Parent folder not found', { parentSource })
 						return
 					}
 
-					if (!parentFolder._children) {
-						Vue.set(parentFolder, '_children', [])
-					}
-					parentFolder._children.push(node.source)
+					// ensure sources are unique
+					const children = new Set(parentFolder._children ?? [])
+					children.add(node.source)
+					Vue.set(parentFolder, '_children', [...children.values()])
 					return
 				}
 
@@ -114,7 +166,7 @@ export const usePathsStore = function(...args) {
 	if (!pathsStore._initialized) {
 		// TODO: watch folders to update paths?
 		subscribe('files:node:created', pathsStore.onCreatedNode)
-		// subscribe('files:node:deleted', pathsStore.onDeletedNode)
+		subscribe('files:node:deleted', pathsStore.onDeletedNode)
 		// subscribe('files:node:moved', pathsStore.onMovedNode)
 
 		pathsStore._initialized = true

apps/files/src/views/FilesList.vue

@@ -652,6 +652,7 @@ export default defineComponent({
 			}
 			sidebarAction.exec(this.currentFolder, this.currentView!, this.currentFolder.path)
 		},
+
 		toggleGridView() {
 			this.userConfigStore.update('grid_view', !this.userConfig.grid_view)
 		},

cypress/e2e/files/FilesUtils.ts

@@ -45,6 +45,21 @@ export const triggerInlineActionForFile = (filename: string, actionId: string) =
 	getActionsForFile(filename).get(`button[data-cy-files-list-row-action="${CSS.escape(actionId)}"]`).should('exist').click()
 }
 
+export const createFolder = (folderName: string) => {
+	cy.intercept('MKCOL', /\/remote.php\/dav\/files\//).as('createFolder')
+
+	// TODO: replace by proper data-cy selectors
+	cy.get('[data-cy-upload-picker] .action-item__menutoggle').first().click()
+	cy.contains('.upload-picker__menu-entry button', 'New folder').click()
+	cy.get('[data-cy-files-new-node-dialog]').should('be.visible')
+	cy.get('[data-cy-files-new-node-dialog-input]').type(`{selectall}${folderName}`)
+	cy.get('[data-cy-files-new-node-dialog-submit]').click()
+
+	cy.wait('@createFolder')
+
+	getRowForFile(folderName).should('be.visible')
+}
+
 export const moveFile = (fileName: string, dirPath: string) => {
 	getRowForFile(fileName).should('be.visible')
 	triggerActionForFile(fileName, 'move-copy')

cypress/e2e/files/duplicated-node-regression.cy.ts

@@ -0,0 +1,33 @@
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { createFolder, getRowForFile, triggerActionForFile } from './FilesUtils.ts'
+
+before(() => {
+	cy.createRandomUser()
+		.then((user) => {
+			cy.mkdir(user, '/only once')
+			cy.login(user)
+			cy.visit('/apps/files')
+		})
+})
+
+/**
+ * Regression test for https://github.com/nextcloud/server/issues/47904
+ */
+it('Ensure nodes are not duplicated in the file list', () => {
+	// See the folder
+	getRowForFile('only once').should('be.visible')
+	// Delete the folder
+	cy.intercept('DELETE', '**/remote.php/dav/**').as('deleteFolder')
+	triggerActionForFile('only once', 'delete')
+	cy.wait('@deleteFolder')
+	getRowForFile('only once').should('not.exist')
+	// Create the folder again
+	createFolder('only once')
+	// See folder exists only once
+	getRowForFile('only once')
+		.should('have.length', 1)
+})