From 8af880ed831493f38cedea827396a154a8fbce37 Mon Sep 17 00:00:00 2001 From: Vangie Du Date: Fri, 21 Apr 2023 03:47:10 +0800 Subject: [PATCH] fix: ensure metadata is updated correctly so that `watchFile` is correctly triggered (#891) * fix: some metadata changes not follow common unix filesystem lead to node.watchFile not trigger 1. nlink default value is 2 2. add subfolder nlink++ 3. atime updated 4. ctime updated after uid/gid/atime/mtime/perm/nlink updated https://github.com/streamich/memfs/issues/889 * fix: ignore . and .. from readdir refactor: add the appropriate modifiers to properties & getters & setters. Co-authored-by: Gareth Jones --------- Co-authored-by: Gareth Jones --- src/__tests__/node.test.ts | 46 ++++++++++++++- src/__tests__/volume.test.ts | 9 +++ src/node.ts | 108 ++++++++++++++++++++++++++++++----- src/volume.ts | 24 +++++--- 4 files changed, 163 insertions(+), 24 deletions(-) diff --git a/src/__tests__/node.test.ts b/src/__tests__/node.test.ts index c88798028..d5a8694ab 100644 --- a/src/__tests__/node.test.ts +++ b/src/__tests__/node.test.ts @@ -1,4 +1,4 @@ -import { Node } from '../node'; +import { Node, Link } from '../node'; import { constants } from '../constants'; describe('node.ts', () => { @@ -60,6 +60,16 @@ describe('node.ts', () => { node.read(buf, 0, 1, 1); expect(buf.equals(Buffer.from([2]))).toBe(true); }); + it('updates the atime and ctime', () => { + const node = new Node(1); + const oldAtime = node.atime; + const oldCtime = node.ctime; + node.read(Buffer.alloc(0)); + const newAtime = node.atime; + const newCtime = node.ctime; + expect(newAtime).not.toBe(oldAtime); + expect(newCtime).not.toBe(oldCtime); + }); }); describe('.chmod(perm)', () => { const node = new Node(1); @@ -69,5 +79,39 @@ describe('node.ts', () => { expect(node.perm).toBe(0o600); expect(node.isFile()).toBe(true); }); + describe.each(['uid', 'gid', 'atime', 'mtime', 'perm', 'nlink'])('when %s changes', field => { + it('updates the property and the ctime', () => { + const node = new Node(1); + const oldCtime = node.ctime; + node[field] = 1; + const newCtime = node.ctime; + expect(newCtime).not.toBe(oldCtime); + expect(node[field]).toBe(1); + }); + }); + describe('.getString(encoding?)', () => { + it('updates the atime and ctime', () => { + const node = new Node(1); + const oldAtime = node.atime; + const oldCtime = node.ctime; + node.getString(); + const newAtime = node.atime; + const newCtime = node.ctime; + expect(newAtime).not.toBe(oldAtime); + expect(newCtime).not.toBe(oldCtime); + }); + }); + describe('.getBuffer()', () => { + it('updates the atime and ctime', () => { + const node = new Node(1); + const oldAtime = node.atime; + const oldCtime = node.ctime; + node.getBuffer(); + const newAtime = node.atime; + const newCtime = node.ctime; + expect(newAtime).not.toBe(oldAtime); + expect(newCtime).not.toBe(oldCtime); + }); + }); }); }); diff --git a/src/__tests__/volume.test.ts b/src/__tests__/volume.test.ts index fe7b134cd..9aea5c26c 100644 --- a/src/__tests__/volume.test.ts +++ b/src/__tests__/volume.test.ts @@ -332,10 +332,13 @@ describe('volume', () => { describe('.openSync(path, flags[, mode])', () => { const vol = new Volume(); it('Create new file at root (/test.txt)', () => { + const oldMtime = vol.root.getNode().mtime; const fd = vol.openSync('/test.txt', 'w'); + const newMtime = vol.root.getNode().mtime; expect(vol.root.getChild('test.txt')).toBeInstanceOf(Link); expect(typeof fd).toBe('number'); expect(fd).toBeGreaterThan(0); + expect(oldMtime).not.toBe(newMtime); }); it('Error on file not found', () => { try { @@ -989,10 +992,16 @@ describe('volume', () => { describe('.mkdirSync(path[, options])', () => { it('Create dir at root', () => { const vol = new Volume(); + const oldMtime = vol.root.getNode().mtime; + const oldNlink = vol.root.getNode().nlink; vol.mkdirSync('/test'); + const newMtime = vol.root.getNode().mtime; + const newNlink = vol.root.getNode().nlink; const child = tryGetChild(vol.root, 'test'); expect(child).toBeInstanceOf(Link); expect(child.getNode().isDirectory()).toBe(true); + expect(oldMtime).not.toBe(newMtime); + expect(newNlink).toBe(oldNlink + 1); }); it('Create 2 levels deep folders', () => { const vol = new Volume(); diff --git a/src/node.ts b/src/node.ts index 30faafaa3..9ac8e7563 100644 --- a/src/node.ts +++ b/src/node.ts @@ -19,34 +19,97 @@ export class Node extends EventEmitter { ino: number; // User ID and group ID. - uid: number = getuid(); - gid: number = getgid(); + private _uid: number = getuid(); + private _gid: number = getgid(); - atime = new Date(); - mtime = new Date(); - ctime = new Date(); + private _atime = new Date(); + private _mtime = new Date(); + private _ctime = new Date(); // data: string = ''; buf: Buffer; - perm = 0o666; // Permissions `chmod`, `fchmod` + private _perm = 0o666; // Permissions `chmod`, `fchmod` mode = S_IFREG; // S_IFDIR, S_IFREG, etc.. (file by default?) // Number of hard links pointing at this Node. - nlink = 1; + private _nlink = 1; // Steps to another node, if this node is a symlink. symlink: string[]; constructor(ino: number, perm: number = 0o666) { super(); - this.perm = perm; + this._perm = perm; this.mode |= perm; this.ino = ino; } + public set ctime(ctime: Date) { + this._ctime = ctime; + } + + public get ctime(): Date { + return this._ctime; + } + + public set uid(uid: number) { + this._uid = uid; + this.ctime = new Date(); + } + + public get uid(): number { + return this._uid; + } + + public set gid(gid: number) { + this._gid = gid; + this.ctime = new Date(); + } + + public get gid(): number { + return this._gid; + } + + public set atime(atime: Date) { + this._atime = atime; + this.ctime = new Date(); + } + + public get atime(): Date { + return this._atime; + } + + public set mtime(mtime: Date) { + this._mtime = mtime; + this.ctime = new Date(); + } + + public get mtime(): Date { + return this._mtime; + } + + public set perm(perm: number) { + this._perm = perm; + this.ctime = new Date(); + } + + public get perm(): number { + return this._perm; + } + + public set nlink(nlink: number) { + this._nlink = nlink; + this.ctime = new Date(); + } + + public get nlink(): number { + return this._nlink; + } + getString(encoding = 'utf8'): string { + this.atime = new Date(); return this.getBuffer().toString(encoding); } @@ -57,6 +120,7 @@ export class Node extends EventEmitter { } getBuffer(): Buffer { + this.atime = new Date(); if (!this.buf) this.setBuffer(bufferAllocUnsafe(0)); return bufferFrom(this.buf); // Return a copy. } @@ -122,6 +186,7 @@ export class Node extends EventEmitter { // Returns the number of bytes read. read(buf: Buffer | Uint8Array, off: number = 0, len: number = buf.byteLength, pos: number = 0): number { + this.atime = new Date(); if (!this.buf) this.buf = bufferAllocUnsafe(0); let actualLen = len; @@ -262,8 +327,11 @@ export class Link extends EventEmitter { // Recursively sync children steps, e.g. in case of dir rename set steps(val) { this._steps = val; - for (const child of Object.values(this.children)) { - child?.syncSteps(); + for (const [child, link] of Object.entries(this.children)) { + if (child === '.' || child === '..') { + continue; + } + link?.syncSteps(); } } @@ -289,10 +357,8 @@ export class Link extends EventEmitter { link.setNode(node); if (node.isDirectory()) { - // link.setChild('.', link); - // link.getNode().nlink++; - // link.setChild('..', this); - // this.getNode().nlink++; + link.children['.'] = link; + link.getNode().nlink++; } this.setChild(name, link); @@ -305,19 +371,33 @@ export class Link extends EventEmitter { link.parent = this; this.length++; + const node = link.getNode(); + if (node.isDirectory()) { + link.children['..'] = this; + this.getNode().nlink++; + } + + this.getNode().mtime = new Date(); this.emit('child:add', link, this); return link; } deleteChild(link: Link) { + const node = link.getNode(); + if (node.isDirectory()) { + delete link.children['..']; + this.getNode().nlink--; + } delete this.children[link.getName()]; this.length--; + this.getNode().mtime = new Date(); this.emit('child:delete', link, this); } getChild(name: string): Link | undefined { + this.getNode().mtime = new Date(); if (Object.hasOwnProperty.call(this.children, name)) { return this.children[name]; } diff --git a/src/volume.ts b/src/volume.ts index 2c1520065..9282003a1 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -666,11 +666,11 @@ export class Volume { } }; - // root.setChild('.', root); - // root.getNode().nlink++; + root.setChild('.', root); + root.getNode().nlink++; - // root.setChild('..', root); - // root.getNode().nlink++; + root.setChild('..', root); + root.getNode().nlink++; this.root = root; } @@ -879,6 +879,9 @@ export class Volume { } for (const name in children) { + if (name === '.' || name === '..') { + continue; + } isEmpty = false; const child = link.getChild(name); @@ -1741,7 +1744,7 @@ export class Volume { for (const name in link.children) { const child = link.getChild(name); - if (!child) { + if (!child || name === '.' || name === '..') { continue; } @@ -1758,6 +1761,9 @@ export class Volume { const list: TDataOut[] = []; for (const name in link.children) { + if (name === '.' || name === '..') { + continue; + } list.push(strToEncoding(name, options.encoding)); } @@ -2798,8 +2804,8 @@ export class FSWatcher extends EventEmitter { }; // children nodes changed - Object.values(link.children).forEach(childLink => { - if (childLink) { + Object.entries(link.children).forEach(([name, childLink]) => { + if (childLink && name !== '.' && name !== '..') { watchLinkNodeChanged(childLink); } }); @@ -2814,8 +2820,8 @@ export class FSWatcher extends EventEmitter { }); if (recursive) { - Object.values(link.children).forEach(childLink => { - if (childLink) { + Object.entries(link.children).forEach(([name, childLink]) => { + if (childLink && name !== '.' && name !== '..') { watchLinkChildrenChanged(childLink); } });