diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index 6843f176c2..dfba2b68e8 100644 --- a/src/vaults/VaultInternal.ts +++ b/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; }, diff --git a/src/vaults/VaultManager.ts b/src/vaults/VaultManager.ts index 4b06e33ad6..382ea70cd1 100644 --- a/src/vaults/VaultManager.ts +++ b/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 { - // 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(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 { - const vaultIdBuffer = await this.db.get( - this.vaultsNamesDbDomain, - vaultName, - true, - ); - if (vaultIdBuffer == null) return; - return IdInternal.fromBuffer(vaultIdBuffer); + return await this.vaultsNamesLock.withWrite(async () => { + const vaultIdBuffer = await this.db.get( + this.vaultsNamesDbDomain, + vaultName, + true, + ); + if (vaultIdBuffer == null) return; + return IdInternal.fromBuffer(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 { let vaultId = vaultsUtils.generateVaultId(); let i = 0; diff --git a/src/vaults/errors.ts b/src/vaults/errors.ts index 5d8a7d51d5..8dcf750e15 100644 --- a/src/vaults/errors.ts +++ b/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, diff --git a/tests/vaults/VaultInternal.test.ts b/tests/vaults/VaultInternal.test.ts index ead6501350..79d1aab9a6 100644 --- a/tests/vaults/VaultInternal.test.ts +++ b/tests/vaults/VaultInternal.test.ts @@ -44,8 +44,6 @@ describe('VaultInternal', () => { const secret1 = { name: 'secret-1', content: 'secret-content-1' }; const secret2 = { name: 'secret-2', content: 'secret-content-2' }; - beforeAll(async () => {}); - beforeEach(async () => { dataDir = await fs.promises.mkdtemp( path.join(os.tmpdir(), 'polykey-test-'), @@ -294,22 +292,6 @@ describe('VaultInternal', () => { const log = await vault.log(); expect(log.length).toEqual(4); }); - test('write locks read', async () => { - await vault.writeF(async (efs) => { - await efs.writeFile('secret-1', 'secret-content'); - }); - - await Promise.all([ - vault.writeF(async (efs) => { - await efs.writeFile('secret-1', 'SUPER-DUPER-SECRET-CONTENT'); - }), - vault.readF(async (efs) => { - expect((await efs.readFile('secret-1')).toString()).toEqual( - 'SUPER-DUPER-SECRET-CONTENT', - ); - }), - ]); - }); test('commit added if mutation in write', async () => { const commit = (await vault.log())[0].commitId; await vault.writeF(async (efs) => { @@ -409,65 +391,6 @@ describe('VaultInternal', () => { // Has a new commit. expect(await vault.log()).toHaveLength(2); }); - test('locking occurs when making a commit.', async () => { - // We want to check if the locking is happening. so we need a way to see if an operation is being blocked. - - let resolveDelay; - const delayPromise = new Promise((resolve, _reject) => { - resolveDelay = resolve; - }); - let firstCommitResolved = false; - let firstCommitResolveTime; - - // @ts-ignore - expect(vault.lock.isLocked()).toBeFalsy(); - - const commit1 = vault.writeF(async (efs) => { - await efs.writeFile(secret1.name, secret1.content); - await delayPromise; // Hold the lock hostage. - firstCommitResolved = true; - firstCommitResolveTime = Date.now(); - }); - - // Now that we are holding the lock hostage, - // @ts-ignore - expect(vault.lock.isLocked()).toBeTruthy(); - // We want to check if any action resolves before the lock is released. - - let secondCommitResolved = false; - let secondCommitResolveTime; - const commit2 = vault.writeF(async (efs) => { - await efs.writeFile(secret2.name, secret2.content); - secondCommitResolved = true; - await sleep(2); - secondCommitResolveTime = Date.now(); - }); - - // Give plenty of time for a commit to resolve. - await sleep(200); - - // Now we want to check for the expected conditions. - // 1. Both commist have not completed. - // commit 1 is holding the lock. - expect(firstCommitResolved).toBeFalsy(); - expect(secondCommitResolved).toBeFalsy(); - - // 2. We release the hostage so both should resolve. - await sleep(200); - resolveDelay(); - await commit1; - await commit2; - expect(firstCommitResolved).toBeTruthy(); - expect(secondCommitResolved).toBeTruthy(); - expect(secondCommitResolveTime).toBeGreaterThan(firstCommitResolveTime); - // @ts-ignore - expect(vault.lock.isLocked()).toBeFalsy(); - - // Commit order should be commit2 -> commit1 -> init - const log = await vault.log(); - expect(log[0].message).toContain(secret2.name); - expect(log[1].message).toContain(secret1.name); - }); test('read operation allowed', async () => { await vault.writeF(async (efs) => { await efs.writeFile(secret1.name, secret1.content); @@ -514,6 +437,86 @@ describe('VaultInternal', () => { }), ]); }); + test('no commit after read', async () => { + await vault.writeF(async (efs) => { + await efs.writeFile(secret1.name, secret1.content); + await efs.writeFile(secret2.name, secret2.content); + }); + const commit = (await vault.log())[0].commitId; + await vault.readF(async (efs) => { + expect((await efs.readFile(secret1.name)).toString()).toEqual( + secret1.content, + ); + }); + const log = await vault.log(); + expect(log).toHaveLength(2); + expect(log[0].commitId).toStrictEqual(commit); + }); + test('only exposes limited commands of VaultInternal', async () => { + // Converting a vault to the interface + const vaultInterface = vault as Vault; + + // Using the avaliable functions. + await vaultInterface.writeF(async (efs) => { + await efs.writeFile('test', 'testContent'); + }); + + await vaultInterface.readF(async (efs) => { + const content = (await efs.readFile('test')).toString(); + expect(content).toStrictEqual('testContent'); + }); + + expect(vaultInterface.vaultDataDir).toBeTruthy(); + expect(vaultInterface.vaultGitDir).toBeTruthy(); + expect(vaultInterface.vaultId).toBeTruthy(); + expect(vaultInterface.writeF).toBeTruthy(); + expect(vaultInterface.writeG).toBeTruthy(); + expect(vaultInterface.readF).toBeTruthy(); + expect(vaultInterface.readG).toBeTruthy(); + expect(vaultInterface.log).toBeTruthy(); + expect(vaultInterface.version).toBeTruthy(); + + // Can we convert back? + const vaultNormal = vaultInterface as VaultInternal; + expect(vaultNormal.destroy).toBeTruthy(); // This exists again. + }); + test('cannot commit when the remote field is set', async () => { + // Write remote metadata + await db.put( + [...vaultsDbDomain, vaultsUtils.encodeVaultId(vaultId)], + VaultInternal.remoteKey, + { remoteNode: '', remoteVault: '' }, + ); + const commit = (await vault.log(undefined, 1))[0]; + await vault.version(commit.commitId); + const files = await vault.readF(async (efs) => { + return await efs.readdir('.'); + }); + expect(files).toEqual([]); + await expect( + vault.writeF(async (efs) => { + await efs.writeFile('test', 'testdata'); + }), + ).rejects.toThrow(vaultsErrors.ErrorVaultRemoteDefined); + }); + // Old locking tests + // TODO: review and remove? + test('write locks read', async () => { + await vault.writeF(async (efs) => { + await efs.writeFile('secret-1', 'secret-content'); + }); + + await Promise.all([ + vault.writeF(async (efs) => { + await efs.writeFile('secret-1', 'SUPER-DUPER-SECRET-CONTENT'); + }), + vault.readF(async (efs) => { + expect((await efs.readFile('secret-1')).toString()).toEqual( + 'SUPER-DUPER-SECRET-CONTENT', + ); + }), + ]); + }); test('read locks write', async () => { await vault.writeF(async (efs) => { await efs.writeFile(secret1.name, secret1.content); @@ -535,20 +538,64 @@ describe('VaultInternal', () => { }), ]); }); - test('no commit after read', async () => { - await vault.writeF(async (efs) => { + test('locking occurs when making a commit.', async () => { + // We want to check if the locking is happening. so we need a way to see if an operation is being blocked. + + let resolveDelay; + const delayPromise = new Promise((resolve, _reject) => { + resolveDelay = resolve; + }); + let firstCommitResolved = false; + let firstCommitResolveTime; + + // @ts-ignore + expect(vault.lock.isLocked()).toBeFalsy(); + + const commit1 = vault.writeF(async (efs) => { await efs.writeFile(secret1.name, secret1.content); - await efs.writeFile(secret2.name, secret2.content); + await delayPromise; // Hold the lock hostage. + firstCommitResolved = true; + firstCommitResolveTime = Date.now(); }); - const commit = (await vault.log())[0].commitId; - await vault.readF(async (efs) => { - expect((await efs.readFile(secret1.name)).toString()).toEqual( - secret1.content, - ); + + // Now that we are holding the lock hostage, + // @ts-ignore + expect(vault.lock.isLocked()).toBeTruthy(); + // We want to check if any action resolves before the lock is released. + + let secondCommitResolved = false; + let secondCommitResolveTime; + const commit2 = vault.writeF(async (efs) => { + await efs.writeFile(secret2.name, secret2.content); + secondCommitResolved = true; + await sleep(2); + secondCommitResolveTime = Date.now(); }); + + // Give plenty of time for a commit to resolve. + await sleep(200); + + // Now we want to check for the expected conditions. + // 1. Both commist have not completed. + // commit 1 is holding the lock. + expect(firstCommitResolved).toBeFalsy(); + expect(secondCommitResolved).toBeFalsy(); + + // 2. We release the hostage so both should resolve. + await sleep(200); + resolveDelay(); + await commit1; + await commit2; + expect(firstCommitResolved).toBeTruthy(); + expect(secondCommitResolved).toBeTruthy(); + expect(secondCommitResolveTime).toBeGreaterThan(firstCommitResolveTime); + // @ts-ignore + expect(vault.lock.isLocked()).toBeFalsy(); + + // Commit order should be commit2 -> commit1 -> init const log = await vault.log(); - expect(log).toHaveLength(2); - expect(log[0].commitId).toStrictEqual(commit); + expect(log[0].message).toContain(secret2.name); + expect(log[1].message).toContain(secret1.name); }); test('locking occurs when making an access.', async () => { await vault.writeF(async (efs) => { @@ -607,51 +654,223 @@ describe('VaultInternal', () => { // @ts-ignore expect(vault.lock.isLocked()).toBeFalsy(); }); - test('only exposes limited commands of VaultInternal', async () => { - // Converting a vault to the interface - const vaultInterface = vault as Vault; - - // Using the avaliable functions. - await vaultInterface.writeF(async (efs) => { - await efs.writeFile('test', 'testContent'); - }); - - await vaultInterface.readF(async (efs) => { - const content = (await efs.readFile('test')).toString(); - expect(content).toStrictEqual('testContent'); - }); - - expect(vaultInterface.vaultDataDir).toBeTruthy(); - expect(vaultInterface.vaultGitDir).toBeTruthy(); - expect(vaultInterface.vaultId).toBeTruthy(); - expect(vaultInterface.writeF).toBeTruthy(); - expect(vaultInterface.writeG).toBeTruthy(); - expect(vaultInterface.readF).toBeTruthy(); - expect(vaultInterface.readG).toBeTruthy(); - expect(vaultInterface.log).toBeTruthy(); - expect(vaultInterface.version).toBeTruthy(); + // Locking tests + const waitDelay = 200; + const runGen = async (gen) => { + for await (const _ of gen) { + // Do nothing + } + }; + test('writeF respects read and write locking', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseWrite = await lock.acquireWrite(); + + let finished = false; + const writeP = vault.writeF(async () => { + finished = true; + }); + await sleep(waitDelay); + expect(finished).toBe(false); + releaseWrite(); + await writeP; + expect(finished).toBe(true); + + const releaseRead = await lock.acquireRead(); + finished = false; + const writeP2 = vault.writeF(async () => { + finished = true; + }); + await sleep(waitDelay); + releaseRead(); + await writeP2; + expect(finished).toBe(true); + }); + test('writeG respects read and write locking', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseWrite = await lock.acquireWrite(); + + let finished = false; + const writeGen = vault.writeG(async function* () { + yield; + finished = true; + yield; + }); + const runP = runGen(writeGen); + await sleep(waitDelay); + expect(finished).toBe(false); + releaseWrite(); + await runP; + expect(finished).toBe(true); + + const releaseRead = await lock.acquireRead(); + finished = false; + const writeGen2 = vault.writeG(async function* () { + yield; + finished = true; + yield; + }); + const runP2 = runGen(writeGen2); + await sleep(waitDelay); + releaseRead(); + await runP2; + expect(finished).toBe(true); + }); + test('readF respects write locking', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseWrite = await lock.acquireWrite(); + + let finished = false; + const writeP = vault.readF(async () => { + finished = true; + }); + await sleep(waitDelay); + expect(finished).toBe(false); + releaseWrite(); + await writeP; + expect(finished).toBe(true); + }); + test('readG respects write locking', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseWrite = await lock.acquireWrite(); + let finished = false; + const writeGen = vault.readG(async function* () { + yield; + finished = true; + yield; + }); + const runP = runGen(writeGen); + await sleep(waitDelay); + expect(finished).toBe(false); + releaseWrite(); + await runP; + expect(finished).toBe(true); + }); + test('readF allows concurrent reads', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseRead = await lock.acquireRead(); + const finished: boolean[] = []; + const doThing = async () => { + finished.push(true); + }; + await Promise.all([ + vault.readF(doThing), + vault.readF(doThing), + vault.readF(doThing), + vault.readF(doThing), + ]); + expect(finished.length).toBe(4); + releaseRead(); + }); + test('readG allows concurrent reads', async () => { + // @ts-ignore: kidnap lock + const lock = vault.lock; + // Hold a write lock + const releaseRead = await lock.acquireRead(); + const finished: boolean[] = []; + const doThing = async function* () { + yield; + finished.push(true); + yield; + }; + await Promise.all([ + runGen(vault.readG(doThing)), + runGen(vault.readG(doThing)), + runGen(vault.readG(doThing)), + runGen(vault.readG(doThing)), + ]); + expect(finished.length).toBe(4); + releaseRead(); + }); + test.todo('pullVault respects write locking'); + // Life- cycle + test('can create with CreateVaultInternal', async () => { + let vault1: VaultInternal | undefined; + try { + const vaultId1 = vaultsUtils.generateVaultId(); + vault1 = await VaultInternal.createVaultInternal({ + db, + efs, + keyManager: fakeKeyManager, + vaultId: vaultId1, + vaultsDb, + vaultsDbDomain, + logger, + }); + // Data exists for vault now. + expect(await efs.readdir('.')).toContain( + vaultsUtils.encodeVaultId(vaultId1), + ); + } finally { + await vault1?.stop(); + await vault1?.destroy(); + } + }); + test('can create an existing vault with CreateVaultInternal', async () => { + let vault1: VaultInternal | undefined; + let vault2: VaultInternal | undefined; + try { + const vaultId1 = vaultsUtils.generateVaultId(); + vault1 = await VaultInternal.createVaultInternal({ + db, + efs, + keyManager: fakeKeyManager, + vaultId: vaultId1, + vaultsDb, + vaultsDbDomain, + logger, + }); + // Data exists for vault now. + expect(await efs.readdir('.')).toContain( + vaultsUtils.encodeVaultId(vaultId1), + ); + await vault1.stop(); + // Data persists + expect(await efs.readdir('.')).toContain( + vaultsUtils.encodeVaultId(vaultId1), + ); - // Can we convert back? - const vaultNormal = vaultInterface as VaultInternal; - expect(vaultNormal.destroy).toBeTruthy(); // This exists again. + // Re-opening the vault + vault2 = await VaultInternal.createVaultInternal({ + db, + efs, + keyManager: fakeKeyManager, + vaultId: vaultId1, + vaultsDb, + vaultsDbDomain, + logger, + }); + + // Data still exists and no new data was created + expect(await efs.readdir('.')).toContain( + vaultsUtils.encodeVaultId(vaultId1), + ); + expect(await efs.readdir('.')).toHaveLength(2); + } finally { + await vault1?.stop(); + await vault1?.destroy(); + await vault2?.stop(); + await vault2?.destroy(); + } }); - test('cannot commit when the remote field is set', async () => { - // Write remote metadata - await db.put( - [...vaultsDbDomain, vaultsUtils.encodeVaultId(vaultId)], - VaultInternal.remoteKey, - { remoteNode: '', remoteVault: '' }, - ); - const commit = (await vault.log(undefined, 1))[0]; - await vault.version(commit.commitId); - const files = await vault.readF(async (efs) => { - return await efs.readdir('.'); - }); - expect(files).toEqual([]); - await expect( - vault.writeF(async (efs) => { - await efs.writeFile('test', 'testdata'); - }), - ).rejects.toThrow(vaultsErrors.ErrorVaultImmutable); + test.todo('can create with CloneVaultInternal'); + test('stop is idempotent', async () => { + // Should complete with no errors + await vault.stop(); + await vault.stop(); + }); + test('destroy is idempotent', async () => { + await vault.stop(); + await vault.destroy(); + await vault.destroy(); }); }); diff --git a/tests/vaults/VaultManager.test.ts b/tests/vaults/VaultManager.test.ts index ab68668d60..48445867ab 100644 --- a/tests/vaults/VaultManager.test.ts +++ b/tests/vaults/VaultManager.test.ts @@ -14,7 +14,7 @@ import path from 'path'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { IdInternal } from '@matrixai/id'; import { DB } from '@matrixai/db'; -import { destroyed, status } from '@matrixai/async-init'; +import { destroyed } from '@matrixai/async-init'; import { running } from '@matrixai/async-init/dist/utils'; import ACL from '@/acl/ACL'; import GestaltGraph from '@/gestalts/GestaltGraph'; @@ -29,7 +29,7 @@ import * as nodesUtils from '@/nodes/utils'; import ForwardProxy from '@/network/ForwardProxy'; import * as vaultsUtils from '@/vaults/utils'; import * as keysUtils from '@/keys/utils'; -import { RWLock, sleep } from '@/utils'; +import { sleep } from '@/utils'; import * as testsUtils from '../utils'; const mockedGenerateDeterministicKeyPair = jest @@ -63,7 +63,6 @@ describe('VaultManager', () => { // We only ever use this to get NodeId, No need to create a whole one const nodeId = testsUtils.generateRandomNodeId(); - const nodeIdEncoded = nodesUtils.encodeNodeId(nodeId); const dummyKeyManager = { getNodeId: () => nodeId, } as KeyManager; @@ -1027,7 +1026,7 @@ describe('VaultManager', () => { await vaultManager.withVaults([clonedVaultId], async (clonedVault) => { await expect( vaultOps.renameSecret(clonedVault, secretNames[0], secretNames[2]), - ).rejects.toThrow(vaultsErrors.ErrorVaultImmutable); + ).rejects.toThrow(vaultsErrors.ErrorVaultRemoteDefined); }); } finally { await vaultManager?.stop(); @@ -1250,53 +1249,6 @@ describe('VaultManager', () => { } }); // Locking tests - // FIXME: no real locking while creating a vault right now. - // This needs to be fixed and this test converted into a - // concurrent create vault test. - test('creating a vault respects locks', async () => { - // We need to override generateVaultId - const vaultId = vaultsUtils.generateVaultId(); - const vaultIdString = vaultId.toString() as VaultIdString; - // Force generation of our known vaultId - const generateVaultIdSpy = jest - .spyOn(vaultsUtils, 'generateVaultId') - .mockReturnValue(vaultId); - const vaultManager = await VaultManager.createVaultManager({ - vaultsPath, - keyManager: dummyKeyManager, - gestaltGraph: {} as GestaltGraph, - nodeConnectionManager: {} as NodeConnectionManager, - acl: {} as ACL, - notificationsManager: {} as NotificationsManager, - db, - logger: logger.getChild(VaultManager.name), - }); - try { - // @ts-ignore: kidnapping the map - const vaultMap = vaultManager.vaultMap; - // Add a lock to the map and lock it - const lock = new RWLock(); - vaultMap.set(vaultIdString, { lock }); - const release = await lock.acquireWrite(); - // Try to create vault - console.log(vaultMap); - const createP = vaultManager.createVault('vaultName'); - await createP; - // Vault shouldn't be created - await sleep(1000); - expect((await vaultMap.get(vaultIdString))?.vault).toBeUndefined(); - // Release lock - release(); - // Vault creation should resolve - await createP; - // Vault should exist - expect((await vaultMap.get(vaultIdString))?.vault).toBeDefined(); - } finally { - generateVaultIdSpy.mockRestore(); - await vaultManager?.stop(); - await vaultManager?.destroy(); - } - }); test('stopping respects locks', async () => { const vaultManager = await VaultManager.createVaultManager({ vaultsPath, @@ -1394,7 +1346,6 @@ describe('VaultManager', () => { // Getting and holding the lock const vaultAndLock = vaultMap.get(vaultId.toString() as VaultIdString)!; const lock = vaultAndLock.lock; - const vault = vaultAndLock.vault!; const release = await lock.acquireWrite(); // Try to use vault let finished = false; @@ -1426,7 +1377,33 @@ describe('VaultManager', () => { logger: logger.getChild(VaultManager.name), }); try { - const vaultId = await vaultManager.createVault(vaultName); + await vaultManager.createVault(vaultName); + // @ts-ignore: kidnapping the map + const vaultMap = vaultManager.vaultMap; + expect(vaultMap.size).toBe(1); + } finally { + await vaultManager?.stop(); + await vaultManager?.destroy(); + } + }); + test('Concurrently creating vault with same name only creates 1 vault', async () => { + const vaultManager = await VaultManager.createVaultManager({ + vaultsPath, + keyManager: dummyKeyManager, + gestaltGraph: {} as GestaltGraph, + nodeConnectionManager: {} as NodeConnectionManager, + acl: {} as ACL, + notificationsManager: {} as NotificationsManager, + db, + logger: logger.getChild(VaultManager.name), + }); + try { + await expect( + Promise.all([ + vaultManager.createVault(vaultName), + vaultManager.createVault(vaultName), + ]), + ).rejects.toThrow(vaultsErrors.ErrorVaultsVaultDefined); // @ts-ignore: kidnapping the map const vaultMap = vaultManager.vaultMap; expect(vaultMap.size).toBe(1); @@ -1588,4 +1565,6 @@ describe('VaultManager', () => { await vaultManager?.destroy(); } }); + test.todo('generateVaultId handles vault conflicts'); + test.todo('pullVault respects locking'); });