From 22775a77f4e7d835f56a43cfa8e784e42835de9d Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 14 May 2024 20:34:32 +1000 Subject: [PATCH] wip: cleaning up and adding commentary [ci skip] --- src/git/utils.ts | 2 +- src/vaults/VaultInternal.ts | 73 ++++++++++++++++++------------------- src/vaults/utils.ts | 11 ++++++ 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/git/utils.ts b/src/git/utils.ts index 24fa04c6b6..fef9d9b3ae 100644 --- a/src/git/utils.ts +++ b/src/git/utils.ts @@ -248,7 +248,7 @@ async function listObjectsAll({ const objectSet: Set = 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), diff --git a/src/vaults/VaultInternal.ts b/src/vaults/VaultInternal.ts index eea2fa0401..2a5f32b242 100644 --- a/src/vaults/VaultInternal.ts +++ b/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 { 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([ ...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 { - // 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([ ...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 { // 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], diff --git a/src/vaults/utils.ts b/src/vaults/utils.ts index c022763830..128edf1d98 100644 --- a/src/vaults/utils.ts +++ b/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';