fix: Add deprecation warning when accessing toplevel attributes throu…

…gh Node.attributes

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

__tests__/files/node.spec.ts

@@ -1,4 +1,4 @@
-import { describe, expect, test } from 'vitest'
+import { describe, expect, test, vi } from 'vitest'
 
 import { File } from '../../lib/files/file'
 import { Folder } from '../../lib/files/folder'
@@ -546,6 +546,7 @@ describe('Attributes update', () => {
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma/Photos/picture.jpg',
 			mime: 'image/jpeg',
 			owner: 'emma',
+			size: 9999,
 			attributes: {
 				etag: '1234',
 				size: 9999,
@@ -561,8 +562,25 @@ describe('Attributes update', () => {
 
 		expect(file.attributes?.etag).toBe('5678')
 		expect(file.attributes?.size).toBe(9999)
-		expect(file.attributes?.owner).toBeUndefined()
+		expect(file.attributes?.owner).toBe('emma')
 		expect(file.attributes?.fileid).toBeUndefined()
 	})
 
+	test('Deprecated access to toplevel attributes', () => {
+		const spy = vi.spyOn(window.console, 'warn')
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/files/emma/Photos/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			size: 9999,
+			attributes: {
+				etag: '1234',
+				size: 9999,
+			},
+		})
+
+		expect(file.attributes.size).toBe(9999)
+		expect(spy).toBeCalledTimes(1)
+	})
+
 })

lib/files/node.ts

@@ -67,21 +67,33 @@ export abstract class Node {
 			// Apply original changes
 			return Reflect.deleteProperty(target, prop)
 		},
+		// TODO: This is deprecated and only needed for files v3
+		get: (target: Attribute, prop: string, receiver) => {
+			if (this.readonlyAttributes.includes(prop)) {
+				logger.warn(`Accessing "Node.attributes.${prop}" is deprecated, access it directly on the Node instance.`)
+				return Reflect.get(this, prop)
+			}
+			return Reflect.get(target, prop, receiver)
+		},
 	} as ProxyHandler<Attribute>
 
 	constructor(data: NodeData, davService?: RegExp) {
 		// Validate data
 		validateData(data, davService || this._knownDavService)
 
-		this._data = data
+		this._data = { ...data, attributes: {} }
 
 		// Proxy the attributes to update the mtime on change
-		this._attributes = new Proxy(data.attributes || {}, this.handler)
-		delete this._data.attributes
+		this._attributes = new Proxy(this._data.attributes!, this.handler)
 
 		if (davService) {
 			this._knownDavService = davService
 		}
+
+		// Update attributes, this sanitizes the attributes to only contain valid attributes
+		this.update(data.attributes ?? {})
+
+		this._data.mtime = data.mtime
 	}
 
 	/**
@@ -194,6 +206,7 @@ export abstract class Node {
 
 	/**
 	 * Get the file attribute
+	 * This contains all additional attributes not provided by the Node class
 	 */
 	get attributes(): Attribute {
 		return this._attributes
@@ -338,7 +351,7 @@ export abstract class Node {
 	/**
 	 * Update the attributes of the node
 	 *
-	 * @param attributes The new attributes to update
+	 * @param attributes The new attributes to update on the Node attributes
 	 */
 	update(attributes: Attribute) {
 		for (const [name, value] of Object.entries(attributes)) {
@@ -349,13 +362,12 @@ export abstract class Node {
 					this.attributes[name] = value
 				}
 			} catch (e) {
-				// Ignore but warn on readonly attributes
+				// Ignore readonly attributes
 				if (e instanceof TypeError) {
-					logger.warn(`Cannot update readonly attribute ${name} on Node`)
-				} else {
-					// Throw all other exceptions
-					throw e
+					continue
 				}
+				// Throw all other exceptions
+				throw e
 			}
 		}
 	}