WIP - #340 fixing comments

src/client/service/gestaltsActionsGetByIdentity.ts

@@ -44,7 +44,7 @@ function gestaltsActionsGetByIdentity({
         identityId,
       );
       if (result == null) {
-        // Node doesn't exist, so no permissions. might throw error instead TBD.
+        // Node doesn't exist, so no permissions
         response.setActionList([]);
       } else {
         // Contains permission

src/client/service/gestaltsActionsGetByNode.ts

@@ -36,7 +36,7 @@ function gestaltsActionsGetByNode({
       );
       const result = await gestaltGraph.getGestaltActionsByNode(nodeId);
       if (result == null) {
-        // Node doesn't exist, so no permissions. might throw error instead TBD.
+        // Node doesn't exist, so no permissions
         response.setActionList([]);
       } else {
         // Contains permission

src/client/service/vaultsLog.ts

@@ -21,7 +21,7 @@ function vaultsLog({
     try {
       const metadata = await authenticate(call.metadata);
       call.sendMetadata(metadata);
-      // Getting the vault.
+      // Getting the vault
       const vaultsLogMessage = call.request;
       const vaultMessage = vaultsLogMessage.getVault();
       if (vaultMessage == null) {

src/client/service/vaultsPermissionGet.ts

@@ -37,7 +37,7 @@ function vaultsPermissionGet({
       // Getting permissions
       const rawPermissions = await acl.getVaultPerm(vaultId);
       const permissionList: Record<NodeIdEncoded, VaultActions> = {};
-      // Getting the relevant information.
+      // Getting the relevant information
       for (const nodeId in rawPermissions) {
         permissionList[nodeId] = rawPermissions[nodeId].vaults[vaultId];
       }
@@ -46,7 +46,7 @@ function vaultsPermissionGet({
       vaultPermissionsMessage.setVault(vaultMessage);
       const nodeMessage = new nodesPB.Node();
 
-      // Constructing the message.
+      // Constructing the message
       for (const nodeIdString in permissionList) {
         const nodeId = IdInternal.fromString<NodeId>(nodeIdString);
         nodeMessage.setNodeId(nodesUtils.encodeNodeId(nodeId));

src/client/service/vaultsPermissionUnset.ts

@@ -54,7 +54,7 @@ function vaultsPermissionUnset({
       for (const action of actions) {
         await acl.unsetVaultAction(vaultId, nodeId, action);
       }
-      // We need to check if there are still shared vaults.
+      // We need to check if there are still shared vaults
       const nodePermissions = await acl.getNodePerm(nodeId);
       // Remove scan permissions if no more shared vaults
       if (nodePermissions != null) {

src/client/service/vaultsVersion.ts

@@ -43,7 +43,7 @@ function vaultsVersion({
           return [latestOid, currentVersionId];
         },
       );
-      // Checking if latest version ID.
+      // Checking if latest version ID
       const isLatestVersion = latestOid === currentVersionId;
       // Creating message
       response.setIsLatestVersion(isLatestVersion);

src/vaults/VaultInternal.ts

@@ -253,7 +253,7 @@ class VaultInternal {
       this.vaultIdEncoded,
       this.vaultsDb,
     );
-    // Let's backup any metadata.
+    // Let's backup any metadata
 
     if (fresh) {
       await this.vaultMetadataDb.clear();
@@ -798,7 +798,7 @@ class VaultInternal {
    * Creates a commit while moving the canonicalBranch reference
    */
   protected async createCommit() {
-    // Checking if commit is appending or branching.
+    // Checking if commit is appending or branching
     const headRef = await git.resolveRef({
       fs: this.efs,
       dir: this.vaultDataDir,
@@ -980,7 +980,7 @@ class VaultInternal {
   }
 
   /**
-   * Deletes any git objects that can't be reached from the canonicalBranch.
+   * Deletes any git objects that can't be reached from the canonicalBranch
    */
   protected async garbageCollectGitObjects() {
     // To garbage collect the git objects,

src/vaults/VaultManager.ts

@@ -493,7 +493,7 @@ class VaultManager {
   }
 
   /**
-   * Returns a dictionary of VaultActions for each node.
+   * Returns a dictionary of VaultActions for each node
    * @param vaultId
    */
   @ready(new vaultsErrors.ErrorVaultManagerNotRunning())
@@ -502,7 +502,7 @@ class VaultManager {
   ): Promise<Record<NodeId, VaultActions>> {
     const rawPermissions = await this.acl.getVaultPerm(vaultId);
     const permissions: Record<NodeId, VaultActions> = {};
-    // Getting the relevant information.
+    // Getting the relevant information
     for (const nodeId in rawPermissions) {
       permissions[nodeId] = rawPermissions[nodeId].vaults[vaultId];
     }
@@ -738,7 +738,7 @@ class VaultManager {
   }
 
   /**
-   * Returns all the shared vaults for a NodeId.
+   * Returns all the shared vaults for a NodeId
    */
   public async *handleScanVaults(
     nodeId: NodeId,
@@ -874,33 +874,33 @@ class VaultManager {
   // THIS can also be replaced with generic withF and withG
 
   /**
-   * Takes a function and runs it with the listed vaults. locking is handled automatically.
-   * @param vaultIds List of vault ID for vaults you wish to use.
-   * @param f Function you wish to run with the provided vaults.
+   * Takes a function and runs it with the listed vaults. locking is handled automatically
+   * @param vaultIds List of vault ID for vaults you wish to use
+   * @param f Function you wish to run with the provided vaults
    */
   @ready(new vaultsErrors.ErrorVaultManagerNotRunning())
   public async withVaults<T>(
     vaultIds: VaultId[],
     f: (...args: Vault[]) => Promise<T>,
   ): Promise<T> {
     // Stages:
-    // 1. Obtain vaults,
-    // 2. Call function with vaults while locking the vaults.
-    // 3. Catch any problems and preform clean up in finally.
-    // 4. return result.
+    // 1. Obtain vaults
+    // 2. Call function with vaults while locking the vaults
+    // 3. Catch any problems and preform clean up in finally
+    // 4. return result
 
     const vaults = await Promise.all(
       vaultIds.map(async (vaultId) => {
         return await this.getVault(vaultId);
       }),
     );
 
-    // Obtaining locks.
+    // Obtaining locks
     const vaultLocks = vaultIds.map((vaultId) => {
       return this.getReadLock(vaultId);
     });
 
-    // Running the function with locking.
+    // Running the function with locking
     return await withF(vaultLocks, () => {
       return f(...vaults);
     });

src/vaults/VaultOps.ts

@@ -15,8 +15,8 @@ type FileOptions = {
 // - add succeeded
 // - secret exists
 // - secret with directory
-// Might just drop the return type.
-// I don't see a case where it would be false without an error.
+// Might just drop the return type
+// I don't see a case where it would be false without an error
 // - Add locking?
 async function addSecret(
   vault: Vault,
@@ -135,7 +135,7 @@ async function statSecret(vault: Vault, secretName: string) {
 // TODO: tests
 // - delete a secret
 // - Secret doesn't exist
-// - delete a full and empty directory with and without recursive.
+// - delete a full and empty directory with and without recursive
 async function deleteSecret(
   vault: Vault,
   secretName: string,
@@ -190,7 +190,7 @@ async function mkdir(
 // TODO: tests
 // - adding existing directory
 // - adding non-existent directory
-// - adding a file.
+// - adding a file
 async function addSecretDirectory(
   vault: Vault,
   secretDirectory: string,
@@ -244,7 +244,7 @@ async function addSecretDirectory(
  * Retrieves a list of the secrets in a vault
  */
 // TODO: tests
-// - read secrets.
+// - read secrets
 // - no secrets
 async function listSecrets(vault: Vault): Promise<string[]> {
   return await vault.readF(async (efs) => {

tests/vaults/VaultInternal.test.ts

@@ -376,7 +376,7 @@ describe('VaultInternal', () => {
       await efs.rename(secret1.name, `${secret1.name}-new`);
       await efs.unlink(secret2.name);
     });
-    // Checking changes.
+    // Checking changes
     await vault.readF(async (efs) => {
       expect(await efs.exists(secret1.name)).toBeFalsy();
       expect(await efs.exists(`${secret1.name}-new`)).toBeTruthy();
@@ -397,27 +397,27 @@ describe('VaultInternal', () => {
       }),
     ).rejects.toThrow();
 
-    // Make sure secret1 wasn't written when the above commit failed.
+    // Make sure secret1 wasn't written when the above commit failed
     await vault.readF(async (efs) => {
       expect(await efs.readdir('.')).not.toContain(secret1.name);
     });
 
-    // No new commit.
+    // No new commit
     expect(await vault.log()).toHaveLength(1);
 
-    // Succeeding commit operation.
+    // Succeeding commit operation
     await vault.writeF(async (efs) => {
       await efs.writeFile(secret2.name, secret2.content);
     });
 
-    // Secret 1 shouldn't exist while secret2 exists.
+    // Secret 1 shouldn't exist while secret2 exists
     await vault.readF(async (efs) => {
       const directory = await efs.readdir('.');
       expect(directory).not.toContain(secret1.name); //
       expect(directory).toContain(secret2.name);
     });
 
-    // Has a new commit.
+    // Has a new commit
     expect(await vault.log()).toHaveLength(2);
   });
   test('concurrent read operations allowed', async () => {
@@ -474,7 +474,7 @@ describe('VaultInternal', () => {
     // Converting a vault to the interface
     const vaultInterface = vault as Vault;
 
-    // Using the avaliable functions.
+    // Using the avaliable functions
     await vaultInterface.writeF(async (efs) => {
       await efs.writeFile('test', 'testContent');
     });
@@ -496,7 +496,7 @@ describe('VaultInternal', () => {
 
     // Can we convert back?
     const vaultNormal = vaultInterface as VaultInternal;
-    expect(vaultNormal.destroy).toBeTruthy(); // This exists again.
+    expect(vaultNormal.destroy).toBeTruthy(); // This exists again
   });
   test('cannot commit when the remote field is set', async () => {
     // Write remote metadata
@@ -548,7 +548,7 @@ describe('VaultInternal', () => {
     await vault.writeF(async (efs) => {
       await efs.writeFile('secret-2', 'secret-content');
     });
-    // Write files to the working directory.
+    // Write files to the working directory
     // @ts-ignore: kidnap vault EFS
     const vaultEFS = vault.efsVault;
     await vaultEFS.writeFile('dirty', 'dirtyData');
@@ -583,7 +583,7 @@ describe('VaultInternal', () => {
     await vault.writeF(async (efs) => {
       await efs.writeFile('secret-2', 'secret-content');
     });
-    // Creating out of history commits.
+    // Creating out of history commits
     // @ts-ignore: kidnap vault EFS
     const vaultEFS = vault.efs;
     const log = await vault.log();
@@ -659,7 +659,7 @@ describe('VaultInternal', () => {
     // Failing commit operation
     await expect(() => runGen(gen)).rejects.toThrow();
 
-    // Make sure secret1 wasn't written when the above commit failed.
+    // Make sure secret1 wasn't written when the above commit failed
     await vault.readF(async (efs) => {
       expect(await efs.readdir('.')).not.toContain(secret1.name);
     });
@@ -882,7 +882,7 @@ describe('VaultInternal', () => {
         vaultsDbDomain,
         logger,
       });
-      // Data exists for vault now.
+      // Data exists for vault now
       expect(await efs.readdir('.')).toContain(
         vaultsUtils.encodeVaultId(vaultId1),
       );
@@ -905,7 +905,7 @@ describe('VaultInternal', () => {
         vaultsDbDomain,
         logger,
       });
-      // Data exists for vault now.
+      // Data exists for vault now
       expect(await efs.readdir('.')).toContain(
         vaultsUtils.encodeVaultId(vaultId1),
       );

tests/vaults/VaultManager.test.ts

@@ -226,7 +226,7 @@ describe('VaultManager', () => {
       // @ts-ignore: protected method
       const vault = await vaultManager.getVault(secondVaultId);
       await vaultManager.destroyVault(secondVaultId);
-      // The mapping should be gone.
+      // The mapping should be gone
       expect((await vaultManager.listVaults()).size).toBe(0);
       // The vault should be destroyed
       expect(vault[destroyed]).toBe(true);
@@ -474,7 +474,7 @@ describe('VaultManager', () => {
     let localNodeIdEncoded: NodeIdEncoded;
 
     beforeAll(async () => {
-      // Creating agents.
+      // Creating agents
       allDataDir = await fs.promises.mkdtemp(
         path.join(os.tmpdir(), 'polykey-test-'),
       );
@@ -494,7 +494,7 @@ describe('VaultManager', () => {
       remoteKeynode2Id = remoteKeynode2.keyManager.getNodeId();
       remoteKeynode2IdEncoded = nodesUtils.encodeNodeId(remoteKeynode2Id);
 
-      // Adding details to each agent.
+      // Adding details to each agent
       await remoteKeynode1.nodeGraph.setNode(remoteKeynode2Id, {
         host: remoteKeynode2.revProxy.getIngressHost(),
         port: remoteKeynode2.revProxy.getIngressPort(),
@@ -1343,7 +1343,7 @@ describe('VaultManager', () => {
     });
   });
   test('handleScanVaults should list all vaults with permissions', async () => {
-    // 1. we need to set up state.
+    // 1. we need to set up state
     const acl = await ACL.createACL({
       db,
       logger,
@@ -1364,7 +1364,7 @@ describe('VaultManager', () => {
       logger: logger.getChild(VaultManager.name),
     });
     try {
-      // Setting up state.
+      // Setting up state
       const nodeId1 = testsUtils.generateRandomNodeId();
       const nodeId2 = testsUtils.generateRandomNodeId();
       await gestaltGraph.setNode({
@@ -1420,7 +1420,7 @@ describe('VaultManager', () => {
     }
   });
   test('ScanVaults should get all vaults with permissions from remote node', async () => {
-    // 1. we need to set up state.
+    // 1. we need to set up state
     const remoteAgent = await PolykeyAgent.createPolykeyAgent({
       password: 'password',
       nodePath: path.join(dataDir, 'remoteNode'),
@@ -1475,7 +1475,7 @@ describe('VaultManager', () => {
       logger: logger.getChild(VaultManager.name),
     });
     try {
-      // Setting up state.
+      // Setting up state
       const targetNodeId = remoteAgent.keyManager.getNodeId();
       const nodeId1 = keyManager.getNodeId();