wip: cleaning up and adding commentary

[ci skip]

src/git/utils.ts

@@ -248,7 +248,7 @@ async function listObjectsAll({
   const objectSet: Set<string> = new Set();
   const objectDirs = await fs.promises.readdir(objectsDirPath);
   for (const objectDir of objectDirs) {
-    if (typeof objectDir !== 'string') never();
+    if (typeof objectDir !== 'string') utils.never();
     if (excludedDirs.includes(objectDir)) continue;
     const objectIds = await fs.promises.readdir(
       path.join(objectsDirPath, objectDir),

src/vaults/VaultInternal.ts

@@ -191,7 +191,7 @@ class VaultInternal {
         remoteVault: vaultsUtils.encodeVaultId(remoteVaultId),
       };
     } catch (e) {
-      // If the error flag set and we have the generalised SmartHttpError from
+      // If the error flag set, and we have the generalised SmartHttpError from
       // isomorphic git then we need to throw the polykey error
       if (e instanceof git.Errors.SmartHttpError && error) {
         throw error;
@@ -282,6 +282,10 @@ class VaultInternal {
     return await this.start_(fresh, tran, vaultName);
   }
 
+  /**
+   * We use a protected start method to avoid the `async-init` lifecycle deadlocking when doing the recursive call to
+   * create a DBTransaction.
+   */
   protected async start_(
     fresh: boolean,
     tran: DBTransaction,
@@ -292,7 +296,6 @@ class VaultInternal {
     );
     this.vaultMetadataDbPath = [...this.vaultsDbPath, this.vaultIdEncoded];
     this.vaultsNamesPath = [...this.vaultsDbPath, 'names'];
-    // Let's backup any metadata
     if (fresh) {
       await tran.clear(this.vaultMetadataDbPath);
       try {
@@ -305,9 +308,9 @@ class VaultInternal {
         }
       }
     }
-    await this.mkdirExists(this.vaultIdEncoded);
-    await this.mkdirExists(this.vaultDataDir);
-    await this.mkdirExists(this.vaultGitDir);
+    await vaultsUtils.mkdirExists(this.efs, this.vaultIdEncoded);
+    await vaultsUtils.mkdirExists(this.efs, this.vaultDataDir);
+    await vaultsUtils.mkdirExists(this.efs, this.vaultGitDir);
     await this.setupMeta({ vaultName, tran });
     await this.setupGit(tran);
     this.efsVault = await this.efs.chroot(this.vaultDataDir);
@@ -316,16 +319,6 @@ class VaultInternal {
     );
   }
 
-  protected async mkdirExists(directory: string) {
-    try {
-      await this.efs.mkdir(directory, { recursive: true });
-    } catch (e) {
-      if (e.code !== 'EEXIST') {
-        throw e;
-      }
-    }
-  }
-
   public async stop(): Promise<void> {
     this.logger.info(
       `Stopping ${this.constructor.name} - ${this.vaultIdEncoded}`,
@@ -342,6 +335,10 @@ class VaultInternal {
     return await this.destroy_(tran);
   }
 
+  /**
+   * We use a protected destroy method to avoid the `async-init` lifecycle deadlocking when doing the recursive call to
+   * create a DBTransaction.
+   */
   protected async destroy_(tran: DBTransaction) {
     this.logger.info(
       `Destroying ${this.constructor.name} - ${this.vaultIdEncoded}`,
@@ -518,7 +515,7 @@ class VaultInternal {
       );
       await tran.put([...vaultMetadataDbPath, VaultInternal.dirtyKey], true);
 
-      let result;
+      let result: TReturn;
       // Do what you need to do here, create the commit
       try {
         result = yield* g(efsVault);
@@ -564,7 +561,7 @@ class VaultInternal {
     // This error flag will contain the error returned by the cloning rpc stream
     let error;
     // Keeps track of whether the metadata needs changing to avoid unnecessary db ops
-    // 0 = no change, 1 = change with vault Id, 2 = change with vault name
+    // 0 = no change, 1 = change with vault ID, 2 = change with vault name
     let metaChange = 0;
     const remoteInfo = await tran.get<RemoteInfo>([
       ...this.vaultMetadataDbPath,
@@ -622,7 +619,7 @@ class VaultInternal {
         },
       );
     } catch (err) {
-      // If the error flag set and we have the generalised SmartHttpError from
+      // If the error flag set, and we have the generalised SmartHttpError from
       // isomorphic git then we need to throw the polykey error
       if (err instanceof git.Errors.SmartHttpError && error) {
         throw error;
@@ -650,7 +647,9 @@ class VaultInternal {
   }
 
   /**
-   * Setup the vault metadata
+   * Sets up the vault metadata.
+   * Creates a `dirty` boolean in the database to track dirty state of the vault.
+   * Also adds the vault's name to the database.
    */
   protected async setupMeta({
     vaultName,
@@ -659,14 +658,7 @@ class VaultInternal {
     vaultName?: VaultName;
     tran: DBTransaction;
   }): Promise<void> {
-    // Setup the vault metadata
-    // and you need to make certain preparations
-    // the meta gets created first
-    // if the SoT is the database
-    // are we supposed to check this?
-
-    // If this is not existing
-    // setup default vaults db
+    // Set up dirty key defaulting to false
     if (
       (await tran.get<boolean>([
         ...this.vaultMetadataDbPath,
@@ -693,11 +685,15 @@ class VaultInternal {
       );
     }
 
-    // Remote: [NodeId, VaultId] | undefined
-    // dirty: boolean
+    // Dirty: boolean
     // name: string | undefined
   }
 
+  /**
+   * Does an idempotent initialization of the git repository for the vault.
+   * If the vault is in a dirty state then we clean up the working directory
+   * or any history not part of the canonicalBranch.
+   */
   protected async setupGit(tran: DBTransaction): Promise<string> {
     // Initialization is idempotent
     // It works even with an existing git repository
@@ -818,15 +814,17 @@ class VaultInternal {
       action: vaultAction,
     });
     const result = vaultsGitInfoGetStream.meta?.result;
-    if (result == null || !utils.isObject(result)) utils.never();
-    if (!('vaultName' in result) || typeof result.vaultName != 'string') {
-      utils.never();
+    if (result == null || !utils.isObject(result)) {
+      utils.never('`result` must be a defined object');
+    }
+    if (!('vaultName' in result) || typeof result.vaultName !== 'string') {
+      utils.never('`vaultName` must be defined and a string');
     }
     if (
       !('vaultIdEncoded' in result) ||
-      typeof result.vaultIdEncoded != 'string'
+      typeof result.vaultIdEncoded !== 'string'
     ) {
-      utils.never();
+      utils.never('`vaultIdEncoded` must be defined and a string');
     }
     const vaultName = result.vaultName;
     const remoteVaultId = ids.parseVaultId(result.vaultIdEncoded);
@@ -1046,9 +1044,8 @@ class VaultInternal {
 
   /**
    * This will walk the current canonicalBranch history and delete any objects that are not a part of it.
-   * This is a dumb method since it will compare all objects to a walked path. There are better ways to do this.
+   * This is costly since it will compare the walked tree with all existing objects.
    */
-
   protected async garbageCollectGitObjectsGlobal() {
     const objectIdsAll = await gitUtils.listObjectsAll({
       fs: this.efs,
@@ -1062,7 +1059,7 @@ class VaultInternal {
       ref: vaultsUtils.canonicalBranch,
     });
     const reachableObjects = await gitUtils.listObjects({
-      fs: this.efs,
+      efs: this.efs,
       dir: this.vaultDataDir,
       gitDir: this.vaultGitDir,
       wants: [masterRef],
@@ -1092,7 +1089,7 @@ class VaultInternal {
     stopId: string,
   ) {
     const objects = await gitUtils.listObjects({
-      fs: this.efs,
+      efs: this.efs,
       dir: this.vaultDataDir,
       gitDir: this.vaultGitDir,
       wants: [startId],

src/vaults/utils.ts

@@ -105,6 +105,16 @@ async function deleteObject(fs: EncryptedFS, gitdir: string, ref: string) {
   }
 }
 
+async function mkdirExists(efs: EncryptedFS, directory: string) {
+  try {
+    await efs.mkdir(directory, { recursive: true });
+  } catch (e) {
+    if (e.code !== 'EEXIST') {
+      throw e;
+    }
+  }
+}
+
 export {
   tagLast,
   refs,
@@ -118,6 +128,7 @@ export {
   readDirRecursively,
   walkFs,
   deleteObject,
+  mkdirExists,
 };
 
 export { createVaultIdGenerator, encodeVaultId, decodeVaultId } from '../ids';