fs/provider: keep memory fs's parent_uid consistent

XiaochenCui · XiaochenCui · commit cd93021b6938 · 2025-08-09T11:04:55.000-07:00
diff --git a/src/backend/src/modules/puterfs/MountpointService.js b/src/backend/src/modules/puterfs/MountpointService.js
@@ -19,7 +19,7 @@
  */
 // const Mountpoint = o => ({ ...o });
 
-const { RootNodeSelector, NodeUIDSelector, NodeChildSelector, NodePathSelector, NodeInternalIDSelector } = require("../../filesystem/node/selectors");
+const { RootNodeSelector, NodeUIDSelector, NodeChildSelector, NodePathSelector, NodeInternalIDSelector, NodeSelector } = require("../../filesystem/node/selectors");
 const BaseService = require("../../services/BaseService");
 
 /**
@@ -89,12 +89,7 @@ class MountpointService extends BaseService {
     
     async get_provider (selector) {
         // type check
-        if ( ! (selector instanceof RootNodeSelector) &&
-            ! (selector instanceof NodeUIDSelector) &&
-            ! (selector instanceof NodeChildSelector) &&
-            ! (selector instanceof NodePathSelector) &&
-            ! (selector instanceof NodeInternalIDSelector)
-        ) {
+        if ( ! (selector instanceof NodeSelector) ) {
             throw new Error('Invalid selector type');
         }
 
@@ -104,14 +99,16 @@ class MountpointService extends BaseService {
 
         if ( selector instanceof NodeUIDSelector ) {
             for ( const [path, { provider }] of Object.entries(this.mountpoints_) ) {
-                const result = await provider._stat_by_uid({
+                const result = await provider.stat_by_uid({
                     uid: selector.value,
                 });
                 if ( result ) {
                     return provider;
                 }
             }
-            console.log('no provider found');
+
+            // No provider found, but we shouldn't throw an error here
+            // because it's a valid case for a node that doesn't exist.
         }
 
         if ( selector instanceof NodeChildSelector ) {
diff --git a/src/backend/src/modules/puterfs/customfs/MemoryFSProvider.js b/src/backend/src/modules/puterfs/customfs/MemoryFSProvider.js
@@ -33,12 +33,14 @@ class MemoryFile {
      * @param {Object} param
      * @param {string} param.full_path
      * @param {boolean} param.is_dir
-     * @param {Buffer} param.content - The content of the file, `null` if the file is a directory.
+     * @param {Buffer|null} param.content - The content of the file, `null` if the file is a directory.
+     * @param {string|null} [param.parent_uid] - UID of parent directory; null for root.
      */
     constructor({
         full_path,
         is_dir,
         content,
+        parent_uid = null,
     }) {
         this.uuid = uuidv4();
 
@@ -49,8 +51,8 @@ class MemoryFile {
 
         this.content = content;
 
-        // TODO (xiaochen): return consistent parent_uid
-        this.parent_uid = uuidv4();
+        // parent_uid should reflect the actual parent's uid; null for root
+        this.parent_uid = parent_uid;
 
         // TODO (xiaochen): return sensible values for "user_id", currently
         // it must be 2 (admin) to pass the test.
@@ -89,6 +91,7 @@ class MemoryFSProvider {
             full_path: '/',
             is_dir: true,
             content: null,
+            parent_uid: null,
         });
         this.entriesByPath.set('/', root.uuid);
         this.entriesByUUID.set(root.uuid, root);
@@ -132,40 +135,62 @@ class MemoryFSProvider {
     }
 
     /**
-     * Check the integrity of the whole memory filesystem and the input. Throws error if any violation is found.
+     * Check the integrity of the whole memory filesystem. Throws error if any violation is found.
      * 
-     * @param {MemoryFile|FSNodeContext} entry - The entry to check.
      * @returns {Promise<void>}
      */
-    _integrity_check (entry) {
+    _integrity_check () {
         if ( config.env !== 'dev' ) {
             // only check in debug mode since it's expensive
             return;
         }
 
-        if ( entry ) {
-            // check all directories along the path are valid
-            const path_to_check = 'entry' in entry ? entry.entry?.path : entry.path;
-            const inner_path = this._inner_path(path_to_check);
-            const path_components = inner_path.split('/');
-            for ( let i = 2; i < path_components.length; i++ ) {
-                const path_component = path_components.slice(0, i).join('/');
-                const entry_uuid = this.entriesByPath.get(path_component);
-                if ( ! entry_uuid ) {
-                    throw new Error(`Directory ${path_component} does not exist`);
-                }
-            }
-        }
-
         // check the 2 maps are consistent
         if ( this.entriesByPath.size !== this.entriesByUUID.size ) {
             throw new Error('Path map and UUID map have different sizes');
         }
+
         for ( const [inner_path, uuid] of this.entriesByPath ) {
             const entry = this.entriesByUUID.get(uuid);
-            if ( ! entry || this._inner_path(entry.path) !== inner_path ) {
+
+            // entry should exist
+            if ( ! entry ) {
+                throw new Error(`Entry ${uuid} does not exist`);
+            }
+
+            // path should match
+            if ( this._inner_path(entry.path) !== inner_path ) {
                 throw new Error(`Path ${inner_path} does not match entry ${uuid}`);
             }
+
+            // uuid should match
+            if ( entry.uuid !== uuid ) {
+                throw new Error(`UUID ${uuid} does not match entry ${entry.uuid}`);
+            }
+
+            // parent should exist
+            if ( entry.parent_uid ) {
+                const parent_entry = this.entriesByUUID.get(entry.parent_uid);
+                if ( ! parent_entry ) {
+                    throw new Error(`Parent ${entry.parent_uid} does not exist`);
+                }
+            }
+
+            // parent's path should be a prefix of the entry's path
+            if ( entry.parent_uid ) {
+                const parent_entry = this.entriesByUUID.get(entry.parent_uid);
+                if ( ! entry.path.startsWith(parent_entry.path) ) {
+                    throw new Error(`Parent ${entry.parent_uid} path ${parent_entry.path} is not a prefix of entry ${entry.path}`);
+                }
+            }
+
+            // parent should be a directory
+            if ( entry.parent_uid ) {
+                const parent_entry = this.entriesByUUID.get(entry.parent_uid);
+                if ( ! parent_entry.is_dir ) {
+                    throw new Error(`Parent ${entry.parent_uid} is not a directory`);
+                }
+            }
         }
     }
 
@@ -219,7 +244,7 @@ class MemoryFSProvider {
      * @param {string} param.uid 
      * @returns {Promise<Object|null>} - The result of the stat operation, or `null` if the node doesn't exist.
      */
-    async _stat_by_uid ({ uid }) {
+    async stat_by_uid ({ uid }) {
         const entry = this.entriesByUUID.get(uid);
         if ( ! entry ) {
             return null;
@@ -271,6 +296,7 @@ class MemoryFSProvider {
             full_path: full_path,
             is_dir: true,
             content: null,
+            parent_uid: parent.uid,
         });
         this.entriesByPath.set(inner_path, entry.uuid);
         this.entriesByUUID.set(entry.uuid, entry);
@@ -279,7 +305,7 @@ class MemoryFSProvider {
         const fs = context.get('services').get('filesystem');
         const node = await fs.node(entry.path);
 
-        this._integrity_check(node);
+        this._integrity_check();
 
         return node;
     }
@@ -330,7 +356,7 @@ class MemoryFSProvider {
             this.entriesByUUID.delete(node.uid);
         }
 
-        this._integrity_check(node);
+        this._integrity_check();
     }
 
     /**
@@ -359,15 +385,14 @@ class MemoryFSProvider {
      * @returns {Promise<MemoryFile>}
      */
     async move({ context, node, new_parent, new_name, metadata }) {
-        this._integrity_check(null);
-
         // create the new entry
         const new_full_path = _path.join(new_parent.path, new_name);
         const new_inner_path = this._inner_path(new_full_path);
         const entry = new MemoryFile({
             full_path: new_full_path,
             is_dir: node.entry.is_dir,
             content: node.entry.content,
+            parent_uid: new_parent.uid,
         });
         entry.uuid = node.entry.uuid;
         this.entriesByPath.set(new_inner_path, entry.uuid);
@@ -378,7 +403,7 @@ class MemoryFSProvider {
         this.entriesByPath.delete(inner_path);
         // should not delete the entry by uuid because it's the same
 
-        this._integrity_check(entry);
+        this._integrity_check();
 
         return entry;
     }
@@ -456,6 +481,7 @@ class MemoryFSProvider {
             full_path: full_path,
             is_dir: false,
             content: file.stream.read(),
+            parent_uid: parent.uid,
         });
         this.entriesByPath.set(inner_path, entry.uuid);
         this.entriesByUUID.set(entry.uuid, entry);
@@ -495,13 +521,12 @@ class MemoryFSProvider {
             this.entriesByUUID.set(node.uid, original_entry);
         }
 
-        this._integrity_check(node);
-
-        // return node;
         const fs = context.get('services').get('filesystem');
         node = await fs.node(original_entry.path);
         await node.fetchEntry();
 
+        this._integrity_check();
+
         return node;
     }
 }
diff --git a/src/backend/src/modules/puterfs/lib/PuterFSProvider.js b/src/backend/src/modules/puterfs/lib/PuterFSProvider.js
@@ -861,7 +861,7 @@ class PuterFSProvider extends putility.AdvancedBase {
      * @param {string} param.uid 
      * @returns {Promise<Object|null>} - The result of the stat operation, or `null` if the node doesn't exist.
      */
-    async _stat_by_uid ({ uid }) {
+    async stat_by_uid ({ uid }) {
         const svc = Context.get('services');
         const svc_fsEntry = svc.get('fsEntryService');
         const entry = await svc_fsEntry.get(uid);
diff --git a/src/backend/src/modules/web/lib/eggspress.js b/src/backend/src/modules/web/lib/eggspress.js
@@ -181,7 +181,7 @@ module.exports = function eggspress (route, settings, handler) {
       } else await handler(req, res, next);
     } catch (e) {
         if (! (e instanceof APIError)) {
-          // Any non-APIError indicates an unhandled error from the backend.
+          // Any non-APIError indicates an unhandled error (i.e. a bug) from the backend.
           // We add a dedicated branch to facilitate debugging.
           console.error(e);
           api_error_handler(e, req, res, next);
