feat(node): cleanup attributes and add size and permissions setters

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

__tests__/files/node.spec.ts

@@ -34,7 +34,7 @@ describe('Node testing', () => {
 })
 
 describe('FileId attribute', () => {
-	test('FileId null fallback', () => {
+	test('FileId undefined', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
 			mime: 'image/jpeg',
@@ -43,7 +43,7 @@ describe('FileId attribute', () => {
 		expect(file.fileid).toBeUndefined()
 	})
 
-	test('FileId null fallback', () => {
+	test('FileId definition', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
 			mime: 'image/jpeg',
@@ -54,7 +54,7 @@ describe('FileId attribute', () => {
 	})
 
 	// Mostly used when a node is unavailable
-	test('FileId negative fallback', () => {
+	test('FileId negative', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
 			mime: 'image/jpeg',
@@ -64,7 +64,7 @@ describe('FileId attribute', () => {
 		expect(file.fileid).toBe(-1234)
 	})
 
-	test('FileId attributes fallback', () => {
+	test('FileId attributes no fallback', () => {
 		const file = new Folder({
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma/Photos/',
 			mime: 'image/jpeg',
@@ -73,7 +73,79 @@ describe('FileId attribute', () => {
 				fileid: 1234,
 			},
 		})
-		expect(file.fileid).toBe(1234)
+		expect(file.fileid).toBeUndefined()
+	})
+})
+
+describe('Size attribute', () => {
+	test('Size definition', () => {
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			size: 1234,
+		})
+		expect(file.size).toBe(1234)
+	})
+
+	test('Size update', async () => {
+		const mtime = new Date()
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			size: 1234,
+			mtime,
+		})
+
+		expect(file.size).toBe(1234)
+		expect(file.mtime?.toISOString()).toBe(mtime?.toISOString())
+
+		// Wait for 10ms to ensure mtime is updated
+		await new Promise(resolve => setTimeout(resolve, 10))
+
+		// Update size
+		file.size = 5678
+
+		// Size and mtime are updated
+		expect(file.size).toBe(5678)
+		expect(file.mtime?.toISOString()).not.toBe(mtime?.toISOString())
+	})
+})
+
+describe('Permissions attribute', () => {
+	test('Permissions definition', () => {
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			permissions: Permission.READ | Permission.UPDATE | Permission.CREATE | Permission.DELETE,
+		})
+		expect(file.permissions).toBe(15)
+	})
+
+	test('Permissions update', async () => {
+		const mtime = new Date()
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			permissions: Permission.READ,
+			mtime,
+		})
+
+		expect(file.permissions).toBe(1)
+		expect(file.mtime?.toISOString()).toBe(mtime?.toISOString())
+
+		// Wait for 10ms to ensure mtime is updated
+		await new Promise(resolve => setTimeout(resolve, 10))
+
+		// Update permissions
+		file.permissions = Permission.ALL
+
+		// Permissions and mtime are updated
+		expect(file.permissions).toBe(31)
+		expect(file.mtime?.toISOString()).not.toBe(mtime?.toISOString())
 	})
 })
 
@@ -466,4 +538,26 @@ describe('Attributes update', () => {
 		expect(file.attributes?.['owner-display-name']).toBeUndefined()
 		expect(file.attributes?.['owner-id']).toBeUndefined()
 	})
+
+	test('Update attributes with protected getters', () => {
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/files/emma/Photos/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			attributes: {
+				etag: '1234',
+			},
+		})
+
+		file.update({
+			etag: '5678',
+			owner: 'admin',
+			fileid: 1234,
+		})
+
+		expect(file.attributes?.etag).toBe('5678')
+		expect(file.attributes?.owner).toBeUndefined()
+		expect(file.attributes?.fileid).toBeUndefined()
+	})
+
 })

lib/files/node.ts

@@ -25,6 +25,7 @@ import { encodePath } from '@nextcloud/paths'
 import { Permission } from '../permissions'
 import { FileType } from './fileType'
 import { Attribute, NodeData, isDavRessource, validateData } from './nodeData'
+import logger from '../utils/logger'
 
 export enum NodeStatus {
 	/** This is a new node and it doesn't exists on the filesystem yet */
@@ -68,7 +69,7 @@ export abstract class Node {
 
 		// Proxy the attributes to update the mtime on change
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any
-		this._attributes = new Proxy(data.attributes || {} as any, this.handler)
+		this._attributes = new Proxy(this.cleanAttributes(data.attributes || {}) as any, this.handler)
 		delete this._data.attributes
 
 		if (davService) {
@@ -78,6 +79,8 @@ export abstract class Node {
 
 	/**
 	 * Get the source url to this object
+	 * There is no setter as the source is not meant to be changed manually.
+	 * You can use the rename or move method to change the source.
 	 */
 	get source(): string {
 		// strip any ending slash
@@ -94,13 +97,17 @@ export abstract class Node {
 
 	/**
 	 * Get this object name
+	 * There is no setter as the source is not meant to be changed manually.
+	 * You can use the rename or move method to change the source.
 	 */
 	get basename(): string {
 		return basename(this.source)
 	}
 
 	/**
 	 * Get this object's extension
+	 * There is no setter as the source is not meant to be changed manually.
+	 * You can use the rename or move method to change the source.
 	 */
 	get extension(): string|null {
 		return extname(this.source)
@@ -109,6 +116,9 @@ export abstract class Node {
 	/**
 	 * Get the directory path leading to this object
 	 * Will use the relative path to root if available
+	 *
+	 * There is no setter as the source is not meant to be changed manually.
+	 * You can use the rename or move method to change the source.
 	 */
 	get dirname(): string {
 		if (this.root) {
@@ -137,20 +147,24 @@ export abstract class Node {
 
 	/**
 	 * Get the file mime
+	 * There is no setter as the mime is not meant to be changed
 	 */
 	get mime(): string|undefined {
 		return this._data.mime
 	}
 
 	/**
 	 * Get the file modification time
+	 * There is no setter as the modification time is not meant to be changed manually.
+	 * It will be automatically updated when the attributes are changed.
 	 */
 	get mtime(): Date|undefined {
 		return this._data.mtime
 	}
 
 	/**
 	 * Get the file creation time
+	 * There is no setter as the creation time is not meant to be changed
 	 */
 	get crtime(): Date|undefined {
 		return this._data.crtime
@@ -163,6 +177,14 @@ export abstract class Node {
 		return this._data.size
 	}
 
+	/**
+	 * Set the file size
+	 */
+	set size(size: number|undefined) {
+		this.updateMtime()
+		this._data.size = size
+	}
+
 	/**
 	 * Get the file attribute
 	 */
@@ -185,8 +207,17 @@ export abstract class Node {
 			: Permission.NONE
 	}
 
+	/**
+	 * Set the file permissions
+	 */
+	set permissions(permissions: Permission) {
+		this.updateMtime()
+		this._data.permissions = permissions
+	}
+
 	/**
 	 * Get the file owner
+	 * There is no setter as the owner is not meant to be changed
 	 */
 	get owner(): string|null {
 		// Remote ressources have no owner
@@ -205,6 +236,7 @@ export abstract class Node {
 
 	/**
 	 * Get the dav root of this object
+	 * There is no setter as the root is not meant to be changed
 	 */
 	get root(): string|null {
 		// If provided (recommended), use the root and strip away the ending slash
@@ -242,10 +274,10 @@ export abstract class Node {
 
 	/**
 	 * Get the node id if defined.
-	 * Will look for the fileid in attributes if undefined.
+	 * There is no setter as the fileid is not meant to be changed
 	 */
 	get fileid(): number|undefined {
-		return this._data?.id || this.attributes?.fileid
+		return this._data?.id
 	}
 
 	/**
@@ -304,7 +336,26 @@ export abstract class Node {
 	update(attributes: Attribute) {
 		// Proxy the attributes to update the mtime on change
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any
-		this._attributes = new Proxy(attributes || {} as any, this.handler)
+		this._attributes = new Proxy(this.cleanAttributes(attributes) as any, this.handler)
+	}
+
+	private cleanAttributes(attributes: Attribute): Attribute {
+		const getters = Object.entries(Object.getOwnPropertyDescriptors(Node.prototype))
+			.filter(e => typeof e[1].get === 'function' && e[0] !== '__proto__')
+			.map(e => e[0])
+
+		// Remove protected getters from the attributes
+		// you cannot update them
+		const clean: Attribute = {}
+		for (const key in attributes) {
+			if (getters.includes(key)) {
+				logger.debug(`Discarding protected attribute ${key}`, { node: this, attributes })
+				continue
+			}
+			clean[key] = attributes[key]
+		}
+
+		return clean
 	}
 
 }