Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable30] fix(files): properly update paths and folder children on node move #49610

Merged
merged 5 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions __tests__/FixJSDOMEnvironment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import JSDOMEnvironment from 'jest-environment-jsdom'

// https://github.com/facebook/jest/blob/v29.4.3/website/versioned_docs/version-29.4/Configuration.md#testenvironment-string
export default class FixJSDOMEnvironment extends JSDOMEnvironment {

constructor(...args: ConstructorParameters<typeof JSDOMEnvironment>) {
super(...args)

// https://github.com/jsdom/jsdom/issues/3363
// 31 ad above switched to vitest and don't have that issue
this.global.structuredClone = structuredClone
}

}
6 changes: 3 additions & 3 deletions apps/files/src/eventbus.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ declare module '@nextcloud/event-bus' {
// mapping of 'event name' => 'event type'
'files:config:updated': { key: string, value: unknown }

'files:favorites:removed': Node
'files:favorites:added': Node
'files:favorites:removed': Node

'files:node:created': Node
'files:node:deleted': Node
'files:node:updated': Node
'files:node:renamed': Node
'files:node:moved': { node: Node, oldSource: string }
'files:node:renamed': Node
'files:node:updated': Node

'files:filter:added': IFileListFilter
'files:filter:removed': string
Expand Down
4 changes: 2 additions & 2 deletions apps/files/src/services/Files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { ContentsWithRoot, File, Folder } from '@nextcloud/files'
import type { ContentsWithRoot, File, Folder, Node } from '@nextcloud/files'
import type { FileStat, ResponseDataDetailed } from 'webdav'

import { CancelablePromise } from 'cancelable-promise'
Expand All @@ -14,7 +14,7 @@ import logger from '../logger.ts'
* Slim wrapper over `@nextcloud/files` `davResultToNode` to allow using the function with `Array.map`
* @param node The node returned by the webdav library
*/
export const resultToNode = (node: FileStat): File | Folder => davResultToNode(node)
export const resultToNode = (node: FileStat): Node => davResultToNode(node)

export const getContents = (path = '/'): CancelablePromise<ContentsWithRoot> => {
const controller = new AbortController()
Expand Down
12 changes: 12 additions & 0 deletions apps/files/src/store/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ export const useFilesStore = function(...args) {
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 })
Expand Down Expand Up @@ -160,6 +171,7 @@ export const useFilesStore = function(...args) {
subscribe('files:node:created', fileStore.onCreatedNode)
subscribe('files:node:deleted', fileStore.onDeletedNode)
subscribe('files:node:updated', fileStore.onUpdatedNode)
subscribe('files:node:moved', fileStore.onMovedNode)

fileStore._initialized = true
}
Expand Down
36 changes: 36 additions & 0 deletions apps/files/src/store/paths.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,40 @@ describe('Path store', () => {
// See the child is removed
expect(root._children).toEqual([])
})

test('Folder is moved', () => {
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])

const renamedNode = node.clone()
renamedNode.rename('new-folder')

expect(renamedNode.path).toBe('/new-folder')
expect(renamedNode.source).toBe('http://example.com/remote.php/dav/files/test/new-folder')

emit('files:node:moved', { node: renamedNode, oldSource: node.source })
// See the path is updated
expect(store.paths).toEqual({ files: { [renamedNode.path]: renamedNode.source } })
// See the child is updated
expect(root._children).toEqual([renamedNode.source])
})

test('File is moved', () => {
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])
expect(store.paths).toEqual({})

const renamedNode = node.clone()
renamedNode.rename('new-file.txt')

emit('files:node:moved', { node: renamedNode, oldSource: node.source })
// See the child is updated
expect(root._children).toEqual([renamedNode.source])
expect(store.paths).toEqual({})
})
})
121 changes: 64 additions & 57 deletions apps/files/src/store/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
*/
import type { FileSource, PathOptions, ServicesState, Service } from '../types'
import { defineStore } from 'pinia'
import { FileType, Folder, Node, getNavigation } from '@nextcloud/files'
import { dirname } from '@nextcloud/paths'
import { type Node, File, FileType, Folder, getNavigation } from '@nextcloud/files'
import { subscribe } from '@nextcloud/event-bus'
import Vue from 'vue'
import logger from '../logger'
Expand Down Expand Up @@ -51,6 +52,27 @@ export const usePathsStore = function(...args) {
Vue.delete(this.paths[service], path)
},

onCreatedNode(node: Node) {
const service = getNavigation()?.active?.id || 'files'
if (!node.fileid) {
logger.error('Node has no fileid', { node })
return
}

// Only add path if it's a folder
if (node.type === FileType.Folder) {
this.addPath({
service,
path: node.path,
source: node.source,
})
}

// Update parent folder children if exists
// If the folder is the root, get it and update it
this.addNodeToParentChildren(node)
},

onDeletedNode(node: Node) {
const service = getNavigation()?.active?.id || 'files'

Expand All @@ -62,95 +84,80 @@ export const usePathsStore = function(...args) {
)
}

// 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 })
this.deleteNodeFromParentChildren(node)
},

onCreatedNode(node: Node) {
onMovedNode({ node, oldSource }: { node: Node, oldSource: string }) {
const service = getNavigation()?.active?.id || 'files'
if (!node.fileid) {
logger.error('Node has no fileid', { node })
return
}

// Only add path if it's a folder
// Update the path of the node
if (node.type === FileType.Folder) {
// Delete the old path if it exists
const oldPath = Object.entries(this.paths[service]).find(([, source]) => source === oldSource)
if (oldPath?.[0]) {
this.deletePath(service, oldPath[0])
}

// Add the new path
this.addPath({
service,
path: node.path,
source: node.source,
})
}

// 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) as Folder & { _children?: string[] }
// Dummy simple clone of the renamed node from a previous state
const oldNode = new File({ source: oldSource, owner: node.owner, mime: node.mime })

this.deleteNodeFromParentChildren(oldNode)
this.addNodeToParentChildren(node)
},

deleteNodeFromParentChildren(node: Node) {
const service = getNavigation()?.active?.id || 'files'

// Update children of a root folder
const parentSource = dirname(node.source)
const folder = (node.dirname === '/' ? files.getRoot(service) : files.getNode(parentSource)) as Folder & { _children?: string[] }
if (folder) {
// ensure sources are unique
const children = new Set(root._children ?? [])
children.add(node.source)
Vue.set(root, '_children', [...children.values()])
const children = new Set(folder._children ?? [])
children.delete(node.source)
Vue.set(folder, '_children', [...children.values()])
logger.debug('Children updated', { parent: folder, node, children: folder._children })
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 & { _children?: string[] }
logger.debug('Path already exists, updating children', { parentFolder, node })
logger.debug('Parent path does not exists, skipping children update', { node })
},

if (!parentFolder) {
logger.error('Parent folder not found', { parentSource })
return
}
addNodeToParentChildren(node: Node) {
const service = getNavigation()?.active?.id || 'files'

// Update children of a root folder
const parentSource = dirname(node.source)
const folder = (node.dirname === '/' ? files.getRoot(service) : files.getNode(parentSource)) as Folder & { _children?: string[] }
if (folder) {
// ensure sources are unique
const children = new Set(parentFolder._children ?? [])
const children = new Set(folder._children ?? [])
children.add(node.source)
Vue.set(parentFolder, '_children', [...children.values()])
Vue.set(folder, '_children', [...children.values()])
logger.debug('Children updated', { parent: folder, node, children: folder._children })
return
}

logger.debug('Parent path does not exists, skipping children update', { node })
},

},
})

const pathsStore = store(...args)
// Make sure we only register the listeners once
if (!pathsStore._initialized) {
// TODO: watch folders to update paths?
subscribe('files:node:created', pathsStore.onCreatedNode)
subscribe('files:node:deleted', pathsStore.onDeletedNode)
// subscribe('files:node:moved', pathsStore.onMovedNode)
subscribe('files:node:moved', pathsStore.onMovedNode)

pathsStore._initialized = true
}
Expand Down
6 changes: 1 addition & 5 deletions dist/5828-5828.js.license
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ SPDX-FileCopyrightText: Sindre Sorhus
SPDX-FileCopyrightText: Roman Shtylman <shtylman@gmail.com>
SPDX-FileCopyrightText: Roeland Jago Douma
SPDX-FileCopyrightText: Raynos <raynos2@gmail.com>
SPDX-FileCopyrightText: Perry Mitchell <perry@perrymitchell.net>
SPDX-FileCopyrightText: Nextcloud GmbH and Nextcloud contributors
SPDX-FileCopyrightText: Matt Zabriskie
SPDX-FileCopyrightText: Joyent
Expand Down Expand Up @@ -63,7 +62,7 @@ This file is generated from multiple sources. Included packages:
- version: 3.3.1
- license: GPL-3.0-or-later
- @nextcloud/files
- version: 3.9.2
- version: 3.10.0
- license: AGPL-3.0-or-later
- @nextcloud/initial-state
- version: 2.2.0
Expand Down Expand Up @@ -254,9 +253,6 @@ This file is generated from multiple sources. Included packages:
- vue
- version: 2.7.16
- license: MIT
- webdav
- version: 5.7.1
- license: MIT
- which-typed-array
- version: 1.1.15
- license: MIT
4 changes: 2 additions & 2 deletions dist/6127-6127.js

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions dist/6127-6127.js.license
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ SPDX-FileCopyrightText: Roeland Jago Douma
SPDX-FileCopyrightText: Richie Bendall
SPDX-FileCopyrightText: Raynos <raynos2@gmail.com>
SPDX-FileCopyrightText: Philipp Kewisch
SPDX-FileCopyrightText: Perry Mitchell <perry@perrymitchell.net>
SPDX-FileCopyrightText: Paul Vorbach <paul@vorba.ch> (http://paul.vorba.ch)
SPDX-FileCopyrightText: Paul Vorbach <paul@vorb.de> (http://vorb.de)
SPDX-FileCopyrightText: OpenJS Foundation and other contributors
Expand Down Expand Up @@ -131,7 +130,7 @@ This file is generated from multiple sources. Included packages:
- version: 3.3.1
- license: GPL-3.0-or-later
- @nextcloud/files
- version: 3.9.2
- version: 3.10.0
- license: AGPL-3.0-or-later
- @nextcloud/initial-state
- version: 2.2.0
Expand Down Expand Up @@ -592,9 +591,6 @@ This file is generated from multiple sources. Included packages:
- web-namespaces
- version: 2.0.1
- license: MIT
- webdav
- version: 5.7.1
- license: MIT
- which-typed-array
- version: 1.1.15
- license: MIT
Expand Down
2 changes: 1 addition & 1 deletion dist/6127-6127.js.map

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions dist/6473-6473.js.license
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ SPDX-FileCopyrightText: Sindre Sorhus
SPDX-FileCopyrightText: Roman Shtylman <shtylman@gmail.com>
SPDX-FileCopyrightText: Roeland Jago Douma
SPDX-FileCopyrightText: Raynos <raynos2@gmail.com>
SPDX-FileCopyrightText: Perry Mitchell <perry@perrymitchell.net>
SPDX-FileCopyrightText: Nextcloud GmbH and Nextcloud contributors
SPDX-FileCopyrightText: Matt Zabriskie
SPDX-FileCopyrightText: Joyent
Expand Down Expand Up @@ -63,7 +62,7 @@ This file is generated from multiple sources. Included packages:
- version: 3.3.1
- license: GPL-3.0-or-later
- @nextcloud/files
- version: 3.9.2
- version: 3.10.0
- license: AGPL-3.0-or-later
- @nextcloud/initial-state
- version: 2.2.0
Expand Down Expand Up @@ -254,9 +253,6 @@ This file is generated from multiple sources. Included packages:
- vue
- version: 2.7.16
- license: MIT
- webdav
- version: 5.7.1
- license: MIT
- which-typed-array
- version: 1.1.15
- license: MIT
4 changes: 2 additions & 2 deletions dist/7358-7358.js

Large diffs are not rendered by default.

Loading
Loading