WIP - #305 - fixes and expanding tests

Author: tegefaulkes

src/vaults/VaultInternal.ts

@@ -188,15 +188,15 @@ class VaultInternal {
   protected vaultsNamesDomain: DBDomain;
   protected efs: EncryptedFS;
   protected efsVault: EncryptedFS;
-  protected _lock: RWLock = new RWLock();
+  protected lock: RWLock = new RWLock();
 
   public readLock: ResourceAcquire = async () => {
-    const release = await this._lock.acquireRead();
+    const release = await this.lock.acquireRead();
     return [async () => release()];
   };
 
   public writeLock: ResourceAcquire = async () => {
-    const release = await this._lock.acquireWrite();
+    const release = await this.lock.acquireWrite();
     return [async () => release()];
   };
 
@@ -301,9 +301,14 @@ class VaultInternal {
     );
     const vaultDb = await this.db.level(this.vaultIdEncoded, this.vaultsDb);
     await vaultDb.clear();
-    await this.efs.rmdir(this.vaultIdEncoded, {
-      recursive: true,
-    });
+    try {
+      await this.efs.rmdir(this.vaultIdEncoded, {
+        recursive: true,
+      });
+    } catch (e) {
+      if (e.code !== 'ENOENT') throw e;
+      // Otherwise ignore
+    }
     this.logger.info(
       `Destroyed ${this.constructor.name} - ${this.vaultIdEncoded}`,
     );
@@ -415,7 +420,7 @@ class VaultInternal {
       )) != null
     ) {
       // Mirrored vaults are immutable
-      throw new vaultsErrors.ErrorVaultImmutable();
+      throw new vaultsErrors.ErrorVaultRemoteDefined();
     }
     return withF([this.writeLock], async () => {
       await this.db.put(
@@ -592,7 +597,7 @@ class VaultInternal {
     return withG([this.writeLock], async function* () {
       if ((await db.get(vaultDbDomain, VaultInternal.remoteKey)) != null) {
         // Mirrored vaults are immutable
-        throw new vaultsErrors.ErrorVaultImmutable();
+        throw new vaultsErrors.ErrorVaultRemoteDefined();
       }
       await db.put(vaultDbDomain, VaultInternal.dirtyKey, true);
       const result = yield* g(efsVault);
@@ -607,6 +612,7 @@ class VaultInternal {
     });
   }
 
+  // TODO: this needs to respect the write lock since we are writing to the EFS
   @ready(new vaultsErrors.ErrorVaultNotRunning())
   public async pullVault({
     nodeConnectionManager,
@@ -626,7 +632,7 @@ class VaultInternal {
       this.vaultMetadataDbDomain,
       VaultInternal.remoteKey,
     );
-    if (remoteInfo == null) throw Error('Vault has no remote to pull from');
+    if (remoteInfo == null) throw new vaultsErrors.ErrorVaultRemoteUndefined();
 
     if (pullNodeId == null) {
       pullNodeId = nodesUtils.decodeNodeId(remoteInfo.remoteNode)!;
@@ -660,17 +666,19 @@ class VaultInternal {
             pullVaultNameOrId!,
             'pull',
           );
-          await git.pull({
-            fs: this.efs,
-            http: { request },
-            dir: this.vaultDataDir,
-            gitdir: this.vaultGitDir,
-            url: `http://`,
-            ref: 'HEAD',
-            singleBranch: true,
-            author: {
-              name: nodesUtils.encodeNodeId(pullNodeId!),
-            },
+          await withF([this.writeLock], async () => {
+            await git.pull({
+              fs: this.efs,
+              http: { request },
+              dir: this.vaultDataDir,
+              gitdir: this.vaultGitDir,
+              url: `http://`,
+              ref: 'HEAD',
+              singleBranch: true,
+              author: {
+                name: nodesUtils.encodeNodeId(pullNodeId!),
+              },
+            });
           });
           return remoteVaultId;
         },

src/vaults/VaultManager.ts

@@ -125,6 +125,7 @@ class VaultManager {
   protected vaultsDb: DBLevel;
   protected vaultsNamesDbDomain: DBDomain = [...this.vaultsDbDomain, 'names'];
   protected vaultsNamesDb: DBLevel;
+  protected vaultsNamesLock: RWLock = new RWLock();
   // VaultId -> VaultMetadata
   protected vaultMap: VaultMap = new Map();
   protected vaultKey: Buffer;
@@ -307,22 +308,29 @@ class VaultManager {
 
   @ready(new vaultsErrors.ErrorVaultManagerNotRunning())
   public async createVault(vaultName: VaultName): Promise<VaultId> {
-    // Check if the vault name already exists;
-    if ((await this.getVaultId(vaultName)) != null) {
-      throw new vaultsErrors.ErrorVaultsVaultDefined();
-    }
+    // Adding vault to name map
     const vaultId = await this.generateVaultId();
-    const lock = new RWLock();
-    const vaultIdString = vaultId.toString() as VaultIdString;
-    this.vaultMap.set(vaultIdString, { lock });
-    return await withF([this.getWriteLock(vaultId)], async () => {
-      // Adding vault to name map
+    await this.vaultsNamesLock.withWrite(async () => {
+      const vaultIdBuffer = await this.db.get(
+        this.vaultsNamesDbDomain,
+        vaultName,
+        true,
+      );
+      // Check if the vault name already exists;
+      if (vaultIdBuffer != null) {
+        throw new vaultsErrors.ErrorVaultsVaultDefined();
+      }
       await this.db.put(
         this.vaultsNamesDbDomain,
         vaultName,
         vaultId.toBuffer(),
         true,
       );
+    });
+    const lock = new RWLock();
+    const vaultIdString = vaultId.toString() as VaultIdString;
+    this.vaultMap.set(vaultIdString, { lock });
+    return await withF([this.getWriteLock(vaultId)], async () => {
       // Creating vault
       const vault = await VaultInternal.createVaultInternal({
         vaultId,
@@ -394,7 +402,9 @@ class VaultManager {
       // Removing from map
       this.vaultMap.delete(vaultIdString);
       // Removing name->id mapping
-      await this.db.del(this.vaultsNamesDbDomain, vaultName);
+      await this.vaultsNamesLock.withWrite(async () => {
+        await this.db.del(this.vaultsNamesDbDomain, vaultName);
+      });
     });
     this.logger.info(`Destroyed Vault ${vaultsUtils.encodeVaultId(vaultId)}`);
   }
@@ -425,12 +435,7 @@ class VaultManager {
     // Stream of vaultName VaultId key value pairs
     for await (const vaultNameBuffer of this.vaultsNamesDb.createKeyStream()) {
       const vaultName = vaultNameBuffer.toString() as VaultName;
-      const vaultIdBuffer = await this.db.get(
-        this.vaultsNamesDbDomain,
-        vaultNameBuffer,
-        true,
-      );
-      const vaultId = IdInternal.fromBuffer<VaultId>(vaultIdBuffer!);
+      const vaultId = (await this.getVaultId(vaultName))!;
       vaults.set(vaultName, vaultId);
     }
     return vaults;
@@ -463,13 +468,15 @@ class VaultManager {
       ];
       await this.db.put(vaultDbDomain, VaultInternal.nameKey, newVaultName);
       // Updating name->id map
-      await this.db.del(this.vaultsNamesDbDomain, oldVaultName);
-      await this.db.put(
-        this.vaultsNamesDbDomain,
-        newVaultName,
-        vaultId.toBuffer(),
-        true,
-      );
+      await this.vaultsNamesLock.withWrite(async () => {
+        await this.db.del(this.vaultsNamesDbDomain, oldVaultName);
+        await this.db.put(
+          this.vaultsNamesDbDomain,
+          newVaultName,
+          vaultId.toBuffer(),
+          true,
+        );
+      });
     });
   }
 
@@ -478,13 +485,15 @@ class VaultManager {
    */
   @ready(new vaultsErrors.ErrorVaultManagerNotRunning())
   public async getVaultId(vaultName: VaultName): Promise<VaultId | undefined> {
-    const vaultIdBuffer = await this.db.get(
-      this.vaultsNamesDbDomain,
-      vaultName,
-      true,
-    );
-    if (vaultIdBuffer == null) return;
-    return IdInternal.fromBuffer<VaultId>(vaultIdBuffer);
+    return await this.vaultsNamesLock.withWrite(async () => {
+      const vaultIdBuffer = await this.db.get(
+        this.vaultsNamesDbDomain,
+        vaultName,
+        true,
+      );
+      if (vaultIdBuffer == null) return;
+      return IdInternal.fromBuffer<VaultId>(vaultIdBuffer);
+    });
   }
 
   /**
@@ -769,7 +778,6 @@ class VaultManager {
   }
 
   @ready(new vaultsErrors.ErrorVaultManagerNotRunning())
-  // TODO: write a test for this, check if it actually handles conflicts
   protected async generateVaultId(): Promise<VaultId> {
     let vaultId = vaultsUtils.generateVaultId();
     let i = 0;

src/vaults/errors.ts

@@ -54,11 +54,13 @@ class ErrorVaultReferenceMissing extends ErrorVault {
   exitCode = sysexits.USAGE;
 }
 
-// Yes it is immutable
-// But this is because you don't own the vault right now
+class ErrorVaultRemoteDefined extends ErrorVaults {
+  description = 'Vault is a clone of a remote vault and can not be mutated';
+  exitCode = sysexits.USAGE;
+}
 
-class ErrorVaultImmutable extends ErrorVaults {
-  description = 'Vault cannot be mutated';
+class ErrorVaultRemoteUndefined extends ErrorVaults {
+  description = 'Vault has no remote set and can not be pulled';
   exitCode = sysexits.USAGE;
 }
 
@@ -117,7 +119,8 @@ export {
   ErrorVaultDestroyed,
   ErrorVaultReferenceInvalid,
   ErrorVaultReferenceMissing,
-  ErrorVaultImmutable,
+  ErrorVaultRemoteDefined,
+  ErrorVaultRemoteUndefined,
   ErrorVaultsVaultUndefined,
   ErrorVaultsVaultDefined,
   ErrorVaultsRecursive,