Skip to content

Commit

Permalink
fix(files): do not rely on unique fileid
Browse files Browse the repository at this point in the history
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
  • Loading branch information
skjnldsv committed May 10, 2024
1 parent 1772508 commit 963ee41
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 51 deletions.
17 changes: 9 additions & 8 deletions apps/files/src/components/FileEntry/FileEntryCheckbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
import { useKeyboardStore } from '../../store/keyboard.ts'
import { useSelectionStore } from '../../store/selection.ts'
import logger from '../../logger.js'
import type { FileSource } from '../../types.ts'

export default defineComponent({
name: 'FileEntryCheckbox',
Expand Down Expand Up @@ -83,10 +84,10 @@ export default defineComponent({
return this.selectionStore.selected
},
isSelected() {
return this.selectedFiles.includes(this.fileid)
return this.selectedFiles.includes(this.source.source)
},
index() {
return this.nodes.findIndex((node: Node) => node.fileid === this.fileid)
return this.nodes.findIndex((node: Node) => node.source === this.source.source)
},
isFile() {
return this.source.type === FileType.File
Expand All @@ -105,20 +106,20 @@ export default defineComponent({

// Get the last selected and select all files in between
if (this.keyboardStore?.shiftKey && lastSelectedIndex !== null) {
const isAlreadySelected = this.selectedFiles.includes(this.fileid)
const isAlreadySelected = this.selectedFiles.includes(this.source.source)

const start = Math.min(newSelectedIndex, lastSelectedIndex)
const end = Math.max(lastSelectedIndex, newSelectedIndex)

const lastSelection = this.selectionStore.lastSelection
const filesToSelect = this.nodes
.map(file => file.fileid)
.map(file => file.source)
.slice(start, end + 1)
.filter(Boolean) as number[]
.filter(Boolean) as FileSource[]

// If already selected, update the new selection _without_ the current file
const selection = [...lastSelection, ...filesToSelect]
.filter(fileid => !isAlreadySelected || fileid !== this.fileid)
.filter(source => !isAlreadySelected || source !== this.source.source)

logger.debug('Shift key pressed, selecting all files in between', { start, end, filesToSelect, isAlreadySelected })
// Keep previous lastSelectedIndex to be use for further shift selections
Expand All @@ -127,8 +128,8 @@ export default defineComponent({
}

const selection = selected
? [...this.selectedFiles, this.fileid]
: this.selectedFiles.filter(fileid => fileid !== this.fileid)
? [...this.selectedFiles, this.source.source]
: this.selectedFiles.filter(source => source !== this.source.source)

logger.debug('Updating selection', { selection })
this.selectionStore.set(selection)
Expand Down
7 changes: 4 additions & 3 deletions apps/files/src/components/FileEntryMixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { hashCode } from '../utils/hashUtils.ts'
import { dataTransferToFileTree, onDropExternalFiles, onDropInternalFiles } from '../services/DropService.ts'
import logger from '../logger.js'
import { showError } from '@nextcloud/dialogs'
import type { FileSource } from '../types.ts'

Vue.directive('onClickOutside', vOnClickOutside)

Expand Down Expand Up @@ -104,10 +105,10 @@ export default defineComponent({
return this.draggingStore.dragging
},
selectedFiles() {
return this.selectionStore.selected
return this.selectionStore.selected as FileSource[]
},
isSelected() {
return this.fileid && this.selectedFiles.includes(this.fileid)
return this.selectedFiles.includes(this.source.source)
},

isRenaming() {
Expand All @@ -132,7 +133,7 @@ export default defineComponent({

// If we're dragging a selection, we need to check all files
if (this.selectedFiles.length > 0) {
const nodes = this.selectedFiles.map(fileid => this.filesStore.getNode(fileid)) as Node[]
const nodes = this.selectedFiles.map(source => this.filesStore.getNode(source)) as Node[]
return nodes.every(canDrag)
}
return canDrag(this.source)
Expand Down
3 changes: 2 additions & 1 deletion apps/files/src/components/FilesListTableHeader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import FilesListTableHeaderButton from './FilesListTableHeaderButton.vue'
import filesSortingMixin from '../mixins/filesSorting.ts'
import logger from '../logger.js'
import type { Node } from '@nextcloud/files'
import type { FileSource } from '../types.ts'

export default defineComponent({
name: 'FilesListTableHeader',
Expand Down Expand Up @@ -186,7 +187,7 @@ export default defineComponent({

onToggleAll(selected) {
if (selected) {
const selection = this.nodes.map(node => node.fileid).filter(Boolean) as number[]
const selection = this.nodes.map(node => node.source).filter(Boolean) as FileSource[]
logger.debug('Added all nodes to selection', { selection })
this.selectionStore.setLastIndex(null)
this.selectionStore.set(selection)
Expand Down
12 changes: 6 additions & 6 deletions apps/files/src/components/FilesListTableHeaderActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { useFilesStore } from '../store/files.ts'
import { useSelectionStore } from '../store/selection.ts'
import filesListWidthMixin from '../mixins/filesListWidth.ts'
import logger from '../logger.js'
import type { FileId } from '../types'
import type { FileSource } from '../types'

// The registered actions list
const actions = getFileActions()
Expand All @@ -81,7 +81,7 @@ export default defineComponent({
required: true,
},
selectedNodes: {
type: Array as PropType<FileId[]>,
type: Array as PropType<FileSource[]>,
default: () => ([]),
},
},
Expand Down Expand Up @@ -161,7 +161,7 @@ export default defineComponent({

async onActionClick(action) {
const displayName = action.displayName(this.nodes, this.currentView)
const selectionIds = this.selectedNodes
const selectionSources = this.selectedNodes
try {
// Set loading markers
this.loading = action.id
Expand All @@ -182,9 +182,9 @@ export default defineComponent({
// Handle potential failures
if (results.some(result => result === false)) {
// Remove the failed ids from the selection
const failedIds = selectionIds
.filter((fileid, index) => results[index] === false)
this.selectionStore.set(failedIds)
const failedSources = selectionSources
.filter((source, index) => results[index] === false)
this.selectionStore.set(failedSources)

if (results.some(result => result === null)) {
// If some actions returned null, we assume that the dev
Expand Down
25 changes: 14 additions & 11 deletions apps/files/src/store/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@
*
*/
import type { Folder, Node } from '@nextcloud/files'
import type { FilesStore, RootsStore, RootOptions, Service, FilesState, FileId } from '../types'
import type { FilesStore, RootsStore, RootOptions, Service, FilesState, FileSource } from '../types'

import { defineStore } from 'pinia'
import { subscribe } from '@nextcloud/event-bus'
import logger from '../logger'
import Vue from 'vue'
import { hashCode } from '../utils/hashUtils'

export const useFilesStore = function(...args) {
const store = defineStore('files', {
Expand All @@ -36,19 +37,20 @@ export const useFilesStore = function(...args) {

getters: {
/**
* Get a file or folder by id
* Get a file or folder by its source
*/
getNode: (state) => (id: FileId): Node|undefined => state.files[id],
getNode: (state) => (source: FileSource): Node|undefined => state.files[hashCode(source)],

/**
* Get a list of files or folders by their IDs
* Does not return undefined values
* Note: does not return undefined values
*/
getNodes: (state) => (ids: FileId[]): Node[] => ids
.map(id => state.files[id])
getNodes: (state) => (sources: FileSource[]): Node[] => sources
.map(source => state.files[hashCode(source)])
.filter(Boolean),

/**
* Get a file or folder by id
* Get the root folder of a service
*/
getRoot: (state) => (service: Service): Folder|undefined => state.roots[service],
},
Expand All @@ -58,10 +60,11 @@ export const useFilesStore = function(...args) {
// 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)
logger.error('Trying to update/set a node without fileid', { node })
return acc
}
acc[node.fileid] = node

acc[hashCode(node.source)] = node
return acc
}, {} as FilesStore)

Expand All @@ -70,8 +73,8 @@ export const useFilesStore = function(...args) {

deleteNodes(nodes: Node[]) {
nodes.forEach(node => {
if (node.fileid) {
Vue.delete(this.files, node.fileid)
if (node.source) {
Vue.delete(this.files, hashCode(node.source))
}
})
},
Expand Down
18 changes: 9 additions & 9 deletions apps/files/src/store/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
import type { FileId, PathsStore, PathOptions, ServicesState } from '../types'
import type { FileSource, PathsStore, PathOptions, ServicesState } from '../types'
import { defineStore } from 'pinia'
import { FileType, Folder, Node, getNavigation } from '@nextcloud/files'
import { subscribe } from '@nextcloud/event-bus'
Expand All @@ -38,7 +38,7 @@ export const usePathsStore = function(...args) {

getters: {
getPath: (state) => {
return (service: string, path: string): FileId|undefined => {
return (service: string, path: string): FileSource|undefined => {
if (!state.paths[service]) {
return undefined
}
Expand All @@ -55,7 +55,7 @@ export const usePathsStore = function(...args) {
}

// Now we can set the provided path
Vue.set(this.paths[payload.service], payload.path, payload.fileid)
Vue.set(this.paths[payload.service], payload.path, payload.source)
},

onCreatedNode(node: Node) {
Expand All @@ -70,7 +70,7 @@ export const usePathsStore = function(...args) {
this.addPath({
service,
path: node.path,
fileid: node.fileid,
source: node.source,
})
}

Expand All @@ -81,26 +81,26 @@ export const usePathsStore = function(...args) {
if (!root._children) {
Vue.set(root, '_children', [])
}
root._children.push(node.fileid)
root._children.push(node.source)
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 parentId = this.paths[service][node.dirname]
const parentFolder = files.getNode(parentId) as Folder
const parentSource = this.paths[service][node.dirname]
const parentFolder = files.getNode(parentSource) as Folder
logger.debug('Path already exists, updating children', { parentFolder, node })

if (!parentFolder) {
logger.error('Parent folder not found', { parentId })
logger.error('Parent folder not found', { parentSource })
return
}

if (!parentFolder._children) {
Vue.set(parentFolder, '_children', [])
}
parentFolder._children.push(node.fileid)
parentFolder._children.push(node.source)
return
}

Expand Down
6 changes: 3 additions & 3 deletions apps/files/src/store/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
import type { FileId, SelectionStore } from '../types'
import type { FileSource, SelectionStore } from '../types'
import { defineStore } from 'pinia'
import Vue from 'vue'

Expand All @@ -34,14 +34,14 @@ export const useSelectionStore = defineStore('selection', {
/**
* Set the selection of fileIds
*/
set(selection = [] as FileId[]) {
set(selection = [] as FileSource[]) {
Vue.set(this, 'selected', [...new Set(selection)])
},

/**
* Set the last selected index
*/
setLastIndex(lastSelectedIndex = null as FileId | null) {
setLastIndex(lastSelectedIndex = null as number | null) {
// Update the last selection if we provided a new selection starting point
Vue.set(this, 'lastSelection', lastSelectedIndex ? this.selected : [])
Vue.set(this, 'lastSelectedIndex', lastSelectedIndex)
Expand Down
11 changes: 6 additions & 5 deletions apps/files/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ import type { Upload } from '@nextcloud/upload'
// Global definitions
export type Service = string
export type FileId = number
export type FileSource = string
export type ViewId = string

// Files store
export type FilesStore = {
[fileid: FileId]: Node
[source: FileSource]: Node
}

export type RootsStore = {
Expand All @@ -48,7 +49,7 @@ export interface RootOptions {

// Paths store
export type PathConfig = {
[path: string]: number
[path: string]: FileSource
}

export type ServicesState = {
Expand All @@ -62,7 +63,7 @@ export type PathsStore = {
export interface PathOptions {
service: Service
path: string
fileid: FileId
source: FileSource
}

// User config store
Expand All @@ -74,8 +75,8 @@ export interface UserConfigStore {
}

export interface SelectionStore {
selected: FileId[]
lastSelection: FileId[]
selected: FileSource[]
lastSelection: FileSource[]
lastSelectedIndex: number | null
}

Expand Down
9 changes: 5 additions & 4 deletions apps/files/src/utils/hashUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
*/

export const hashCode = function(str: string): number {
return str.split('').reduce(function(a, b) {
a = ((a << 5) - a) + b.charCodeAt(0)
return a & a
}, 0)
let hash = 0
for (let i = 0; i < str.length; i++) {
hash = ((hash << 5) - hash + str.charCodeAt(i)) | 0
}
return (hash >>> 0)
}
2 changes: 1 addition & 1 deletion apps/files/src/views/FilesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ export default defineComponent({

// Define current directory children
// TODO: make it more official
this.$set(folder, '_children', contents.map(node => node.fileid))
this.$set(folder, '_children', contents.map(node => node.source))

// If we're in the root dir, define the root
if (dir === '/') {
Expand Down

0 comments on commit 963ee41

Please sign in to comment.