Merge pull request #974 from nextcloud-libraries/fix/mtime-auto

__tests__/files/file.spec.ts

@@ -188,7 +188,7 @@ describe('File data change', () => {
 	})
 })
 
-describe('Altering attributes updates mtime', () => {
+describe('Altering attributes does NOT updates mtime', () => {
 	test('mtime is updated on existing attribute', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
@@ -202,12 +202,12 @@ describe('Altering attributes updates mtime', () => {
 		expect(file.attributes.test).toBe(true)
 		file.attributes.test = false
 
-		// Check that mtime has been updated
-		expect(file.mtime?.getDate()).toBe(new Date().getDate())
+		// Check that mtime has NOT been updated
+		expect(file.mtime?.getDate()).toBe(1)
 		expect(file.attributes.test).toBe(false)
 	})
 
-	test('mtime is updated on new attribute', () => {
+	test('mtime is NOT updated on new attribute', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
 			mime: 'image/jpeg',
@@ -217,12 +217,12 @@ describe('Altering attributes updates mtime', () => {
 		expect(file.attributes.test).toBeFalsy()
 		file.attributes.test = true
 
-		// Check that mtime has been updated
-		expect(file.mtime?.getDate()).toBe(new Date().getDate())
+		// Check that mtime has NOT been updated
+		expect(file.mtime?.getDate()).toBe(1)
 		expect(file.attributes.test).toBe(true)
 	})
 
-	test('mtime is updated on deleted attribute', () => {
+	test('mtime is NOT updated on deleted attribute', () => {
 		const file = new File({
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
 			mime: 'image/jpeg',
@@ -235,8 +235,8 @@ describe('Altering attributes updates mtime', () => {
 		expect(file.attributes.test).toBe(true)
 		delete file.attributes.test
 
-		// Check that mtime has been updated
-		expect(file.mtime?.getDate()).toBe(new Date().getDate())
+		// Check that mtime has NOT been updated
+		expect(file.mtime?.getDate()).toBe(1)
 		expect(file.attributes.test).toBeUndefined()
 	})
 
@@ -245,15 +245,52 @@ describe('Altering attributes updates mtime', () => {
 			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
 			mime: 'image/jpeg',
 			owner: 'emma',
-			attributes: {
-				test: true,
-			},
+			permissions: Permission.READ,
 		})
-		expect(file.attributes.test).toBe(true)
-		delete file.attributes.test
+
+		expect(file.permissions).toBe(Permission.READ)
+		file.permissions = Permission.ALL
 
 		// Check that mtime has been updated
 		expect(file.mtime).toBeUndefined()
-		expect(file.attributes.test).toBeUndefined()
+		expect(file.permissions).toBe(Permission.ALL)
+	})
+
+})
+
+describe('Altering top-level properties updates mtime', () => {
+	test('mtime is updated on permissions change', () => {
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			mtime: new Date(Date.UTC(1990, 0, 1, 0, 0, 0)),
+			permissions: Permission.READ,
+		})
+
+		expect(file.permissions).toBe(Permission.READ)
+		file.permissions = Permission.ALL
+
+		// Check that mtime has been updated
+		expect(file.mtime?.getDate()).toBe(new Date().getDate())
+		expect(file.permissions).toBe(Permission.ALL)
 	})
+
+	test('mtime is updated on size change', () => {
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/files/emma',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			mtime: new Date(Date.UTC(1990, 0, 1, 0, 0, 0)),
+			size: 100,
+		})
+
+		expect(file.size).toBe(100)
+		file.size = 200
+
+		// Check that mtime has been updated
+		expect(file.mtime?.getDate()).toBe(new Date().getDate())
+		expect(file.size).toBe(200)
+	})
+
 })

__tests__/files/node.spec.ts

@@ -77,6 +77,61 @@ describe('FileId attribute', () => {
 	})
 })
 
+describe('Mtime attribute', () => {
+	test('Mtime definition', () => {
+		const mtime = new Date()
+		const file = new File({
+			source: 'https://cloud.domain.com/remote.php/dav/picture.jpg',
+			mime: 'image/jpeg',
+			owner: 'emma',
+			mtime,
+		})
+		expect(file.mtime?.toISOString()).toBe(mtime.toISOString())
+	})
+
+	test('Mtime manual 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',
+			mtime,
+		})
+
+		expect(file.mtime?.toISOString()).toBe(mtime.toISOString())
+
+		// Wait for 10ms to ensure mtime is updated
+		await new Promise(resolve => setTimeout(resolve, 10))
+
+		// Update mtime
+		file.mtime = new Date()
+
+		// Mtime is updated
+		expect(file.mtime?.toISOString()).not.toBe(mtime.toISOString())
+	})
+
+	test('Mtime method 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',
+			mtime,
+		})
+
+		expect(file.mtime?.toISOString()).toBe(mtime.toISOString())
+
+		// Wait for 10ms to ensure mtime is updated
+		await new Promise(resolve => setTimeout(resolve, 10))
+
+		// Update mtime
+		file.updateMtime()
+
+		// Mtime is updated
+		expect(file.mtime?.toISOString()).not.toBe(mtime.toISOString())
+	})
+})
+
 describe('Size attribute', () => {
 	test('Size definition', () => {
 		const file = new File({

lib/files/node.ts

@@ -53,17 +53,15 @@ export abstract class Node {
 			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): boolean => {
 			if (this.readonlyAttributes.includes(prop)) {
 				return false
 			}
-			// Edit modification time
-			this.updateMtime()
+
 			// Apply original changes
 			return Reflect.deleteProperty(target, prop)
 		},
@@ -89,9 +87,6 @@ export abstract class Node {
 		// Update attributes, this sanitizes the attributes to only contain valid attributes
 		this.update(data.attributes ?? {})
 
-		// Reset the mtime if changed while updating the attributes
-		this._data.mtime = data.mtime
-
 		if (davService) {
 			this._knownDavService = davService
 		}
@@ -175,13 +170,18 @@ export abstract class Node {
 
 	/**
 	 * 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
 	}
 
+	/**
+	 * Set the file modification time
+	 */
+	set mtime(mtime: Date|undefined) {
+		this._data.mtime = mtime
+	}
+
 	/**
 	 * Get the file creation time
 	 * There is no setter as the creation time is not meant to be changed
@@ -341,16 +341,17 @@ export abstract class Node {
 	}
 
 	/**
-	 * Update the mtime if exists.
+	 * Update the mtime if exists
 	 */
-	private updateMtime() {
+	updateMtime() {
 		if (this._data.mtime) {
 			this._data.mtime = new Date()
 		}
 	}
 
 	/**
 	 * Update the attributes of the node
+	 * Warning, updating attributes will NOT automatically update the mtime.
 	 *
 	 * @param attributes The new attributes to update on the Node attributes
 	 */