fix(Node): Do not drop readonly attributes but only forbid updating them

Attributes that are readonly should still be listed in the `attributes` getter,
otherwise this is a hard breaking change as a lot of apps already depend on `attributes`.
So instead only ensure that readonly attributes are skipped by the proxy handler.

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

__tests__/files/node.spec.ts

@@ -532,6 +532,8 @@ describe('Attributes update', () => {
 
 		file.update({
 			etag: '5678',
+			'owner-display-name': undefined,
+			'owner-id': undefined,
 		})
 
 		expect(file.attributes?.etag).toBe('5678')

lib/files/node.ts

@@ -44,22 +44,30 @@ export abstract class Node {
 	private _attributes: Attribute
 	private _knownDavService = /(remote|public)\.php\/(web)?dav/i
 
+	private readonlyAttributes = Object.entries(Object.getOwnPropertyDescriptors(Node.prototype))
+		.filter(e => typeof e[1].get === 'function' && e[0] !== '__proto__')
+		.map(e => e[0])
+
 	private handler = {
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any
-		set: (target: Attribute, prop: string, value: any): any => {
+		set: (target: Attribute, prop: string, value: unknown): boolean => {
+			if (this.readonlyAttributes.includes(prop)) {
+				return false
+			}
 			// Edit modification time
 			this.updateMtime()
 			// Apply original changes
 			return Reflect.set(target, prop, value)
 		},
-		deleteProperty: (target: Attribute, prop: string) => {
+		deleteProperty: (target: Attribute, prop: string): boolean => {
+			if (this.readonlyAttributes.includes(prop)) {
+				return false
+			}
 			// Edit modification time
 			this.updateMtime()
 			// Apply original changes
 			return Reflect.deleteProperty(target, prop)
 		},
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any
-	} as ProxyHandler<any>
+	} as ProxyHandler<Attribute>
 
 	constructor(data: NodeData, davService?: RegExp) {
 		// Validate data
@@ -68,8 +76,7 @@ export abstract class Node {
 		this._data = data
 
 		// Proxy the attributes to update the mtime on change
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any
-		this._attributes = new Proxy(this.cleanAttributes(data.attributes || {}) as any, this.handler)
+		this._attributes = new Proxy(data.attributes || {}, this.handler)
 		delete this._data.attributes
 
 		if (davService) {
@@ -334,28 +341,23 @@ export abstract class Node {
 	 * @param attributes The new attributes to update
 	 */
 	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(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
+		for (const [name, value] of Object.entries(attributes)) {
+			try {
+				if (value === undefined) {
+					delete this.attributes[name]
+				} else {
+					this.attributes[name] = value
+				}
+			} catch (e) {
+				// Ignore but warn on readonly attributes
+				if (e instanceof TypeError) {
+					logger.warn(`Cannot update readonly attribute ${name} on Node`)
+				} else {
+					// Throw all other exceptions
+					throw e
+				}
 			}
-			clean[key] = attributes[key]
 		}
-
-		return clean
 	}
 
 }