From ddc512ef9c42570a4bd39d3c611f10bb9f8db79f Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Wed, 27 Nov 2024 18:22:55 +1100 Subject: [PATCH] chore: updated `VaultsSecretsRemove` to use new resource aquisition --- src/client/handlers/VaultsSecretsRemove.ts | 180 +++++------------ src/client/types.ts | 24 ++- tests/client/handlers/vaults.test.ts | 217 ++++++++++++++------- 3 files changed, 208 insertions(+), 213 deletions(-) diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index 1115f62e2..247571012 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -1,5 +1,4 @@ import type { DB } from '@matrixai/db'; -import { ResourceAcquire, withG } from '@matrixai/resources'; import type { ClientRPCRequestParams, ClientRPCResponseResult, @@ -8,12 +7,12 @@ import type { SuccessOrErrorMessage, } from '../types'; import type VaultManager from '../../vaults/VaultManager'; +import { ResourceAcquire, withG } from '@matrixai/resources'; import { DuplexHandler } from '@matrixai/rpc'; +import { FileSystemWritable } from '../../vaults/types'; import * as vaultsUtils from '../../vaults/utils'; import * as vaultsErrors from '../../vaults/errors'; import * as clientErrors from '../errors'; -import { FileSystemWritable } from '../../vaults/types'; -import * as utils from '../../utils'; class VaultsSecretsRemove extends DuplexHandler< { @@ -37,23 +36,16 @@ class VaultsSecretsRemove extends DuplexHandler< const vaultAcquires: Array> = []; // Extracts the header message from the iterator const headerMessage = await (async () => { - let header: SecretsRemoveHeaderMessage | undefined; - for await (const value of input) { - // TS cannot properly narrow down this type as it is too deeply wrapped. - // The as keyword is used to help it with type narrowing. - const message = value as - | SecretIdentifierMessageTagged - | SecretsRemoveHeaderMessage; - if (message.type === 'VaultNamesHeaderMesage') header = message; - break; + const iterator = input[Symbol.asyncIterator](); + const header = (await iterator.next()).value; + if (header.type === 'VaultNamesHeaderMessage') { + if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); + return header; } - // The header message is mandatory - if (header == null) throw new clientErrors.ErrorClientInvalidHeader(); - return header; })(); // Create an array of write acquires await db.withTransactionF(async (tran) => { - for (const vaultName of headerMessage!.vaultNames) { + for (const vaultName of headerMessage.vaultNames) { const vaultIdFromName = await vaultManager.getVaultId(vaultName, tran); const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); if (vaultId == null) { @@ -61,9 +53,11 @@ class VaultsSecretsRemove extends DuplexHandler< `Vault ${vaultName} does not exist`, ); } - await vaultManager.withVaults([vaultId], async (vault) => { - vaultAcquires.push(vault.acquireWrite()); - }); + const acquire = await vaultManager.withVaults( + [vaultId], + async (vault) => vault.acquireWrite(), + ); + vaultAcquires.push(acquire); } }); // Acquire all locks in parallel and perform all operations at once @@ -75,130 +69,48 @@ class VaultsSecretsRemove extends DuplexHandler< for (let i = 0; i < efses.length; i++) { vaultMap.set(headerMessage!.vaultNames[i], efses[i]); } - for await (const value of input) { - // TS cannot properly narrow down this type as it is too deeply wrapped. - // The as keyword is used to help it with type narrowing. - const message = value as - | SecretIdentifierMessageTagged - | SecretsRemoveHeaderMessage; + for await (const message of input) { // Ignoring any header messages - if (message.type === 'SecretIdentifierMessage') { - const efs = vaultMap.get(message.nameOrId); - if (efs == null) { - throw new vaultsErrors.ErrorVaultsVaultUndefined( - `Vault ${message.nameOrId} was not present in the header message`, - ); + if (message.type !== 'SecretIdentifierMessage') continue; + const efs = vaultMap.get(message.nameOrId); + if (efs == null) { + throw new vaultsErrors.ErrorVaultsVaultUndefined( + `Vault ${message.nameOrId} was not present in the header message`, + ); + } + try { + const stat = await efs.stat(message.secretName); + if (stat.isDirectory()) { + await efs.rmdir(message.secretName, { + recursive: headerMessage.recursive, + }); + } else { + await efs.unlink(message.secretName); } - try { - const stat = await efs.stat(message.secretName); - if (stat.isDirectory()) { - await efs.rmdir(message.secretName, { - recursive: headerMessage.recursive, - }); - } else { - await efs.unlink(message.secretName); - } + yield { + type: 'success', + success: true, + }; + } catch (e) { + if ( + e.code === 'ENOENT' || + e.code === 'ENOTEMPTY' || + e.code === 'EINVAL' + ) { + // INVAL can be triggered if removing the root of the + // vault is attempted. yield { - type: 'success', - success: true, + type: 'error', + code: e.code, + reason: message.secretName, }; - } catch (e) { - if ( - e.code === 'ENOENT' || - e.code === 'ENOTEMPTY' || - e.code === 'EINVAL' - ) { - // INVAL can be triggered if removing the root of the - // vault is attempted. - yield { - type: 'error', - code: e.code, - reason: message.secretName, - }; - } else { - throw e; - } + } else { + throw e; } } } }, ); - - // Create a record of secrets to be removed, grouped by vault names - // const vaultGroups: Record> = {}; - // const secretNames: Array<[string, string]> = []; - // let metadata: any = undefined; - // for await (const secretRemoveMessage of input) { - // if (metadata == null) metadata = secretRemoveMessage.metadata ?? {}; - // secretNames.push([ - // secretRemoveMessage.nameOrId, - // secretRemoveMessage.secretName, - // ]); - // } - // secretNames.forEach(([vaultName, secretName]) => { - // if (vaultGroups[vaultName] == null) { - // vaultGroups[vaultName] = []; - // } - // vaultGroups[vaultName].push(secretName); - // }); - // Now, all the paths will be removed for a vault within a single commit - // yield* db.withTransactionG( - // async function* (tran): AsyncGenerator { - // for (const [vaultName, secretNames] of Object.entries(vaultGroups)) { - // const vaultIdFromName = await vaultManager.getVaultId( - // vaultName, - // tran, - // ); - // const vaultId = - // vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName); - // if (vaultId == null) { - // throw new vaultsErrors.ErrorVaultsVaultUndefined(); - // } - // yield* vaultManager.withVaultsG( - // [vaultId], - // async function* (vault): AsyncGenerator { - // yield* vault.writeG( - // async function* (efs): AsyncGenerator { - // for (const secretName of secretNames) { - // try { - // const stat = await efs.stat(secretName); - // if (stat.isDirectory()) { - // await efs.rmdir(secretName, { - // recursive: metadata?.options?.recursive, - // }); - // } else { - // await efs.unlink(secretName); - // } - // yield { - // type: 'success', - // success: true, - // }; - // } catch (e) { - // if ( - // e.code === 'ENOENT' || - // e.code === 'ENOTEMPTY' || - // e.code === 'EINVAL' - // ) { - // // INVAL can be triggered if removing the root of the - // // vault is attempted. - // yield { - // type: 'error', - // code: e.code, - // reason: secretName, - // }; - // } else { - // throw e; - // } - // } - // } - // }, - // ); - // }, - // tran, - // ); - // } - // }, - // ); }; } diff --git a/src/client/types.ts b/src/client/types.ts index de48d9c83..a6a653f98 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -1,7 +1,8 @@ import type { ClientManifest, JSONObject, - JSONRPCResponseResult, + JSONRPCResponseMetadata, + // JSONRPCResponseResult, RPCClient, } from '@matrixai/rpc'; import type { @@ -25,6 +26,17 @@ import type { } from '../nodes/types'; import type { AuditEventsGetTypeOverride } from './callers/auditEventsGet'; +// TEMP: For testing and debugging. This will go into js-rpc or something. +type JSONRPCResponseResult< + T extends JSONObject = JSONObject, + M extends JSONObject = JSONObject, +> = T & { + metadata?: JSONRPCResponseMetadata & + M & + (T extends { metadata: infer U } ? U : {}) & + JSONObject; +}; + type ClientRPCRequestParams = JSONRPCResponseResult< T, @@ -362,14 +374,14 @@ type SecretStatMessage = { type SecretIdentifierMessageTagged = SecretIdentifierMessage & { type: 'SecretIdentifierMessage'; -} +}; -type VaultNamesHeaderMesage = { - type: 'VaultNamesHeaderMesage'; +type VaultNamesHeaderMessage = { + type: 'VaultNamesHeaderMessage'; vaultNames: Array; }; -type SecretsRemoveHeaderMessage = VaultNamesHeaderMesage & { +type SecretsRemoveHeaderMessage = VaultNamesHeaderMessage & { recursive?: boolean; }; @@ -449,7 +461,7 @@ export type { SecretFilesMessage, SecretStatMessage, SecretIdentifierMessageTagged, - VaultNamesHeaderMesage, + VaultNamesHeaderMessage, SecretsRemoveHeaderMessage, SignatureMessage, OverrideRPClientType, diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index 9889cfc2f..139e23918 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -3,13 +3,10 @@ import type { FileSystem } from '@/types'; import type { VaultId } from '@/ids'; import type NodeManager from '@/nodes/NodeManager'; import type { - ClientRPCRequestParams, ContentSuccessMessage, ErrorMessage, LogEntryMessage, SecretContentMessage, - SecretIdentifierMessage, - SecretsRemoveHeaderMessage, VaultListMessage, VaultPermissionMessage, } from '@/client/types'; @@ -76,7 +73,9 @@ import * as nodesUtils from '@/nodes/utils'; import * as vaultsUtils from '@/vaults/utils'; import * as vaultsErrors from '@/vaults/errors'; import * as networkUtils from '@/network/utils'; +import * as utils from '@/utils'; import * as testsUtils from '../../utils'; +import { error } from 'console'; describe('vaultsClone', () => { const logger = new Logger('vaultsClone test', LogLevel.WARN, [ @@ -1486,11 +1485,9 @@ describe('vaultsSecretsMkdir', () => { await writer.close(); for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(dirPath); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(dirPath); } await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -1556,11 +1553,8 @@ describe('vaultsSecretsMkdir', () => { // Check if the operation concluded as expected for await (const data of response.readable) { if (data.type === 'error') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(dirPath3); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(dirPath3); } } await vaultManager.withVaults( @@ -1594,11 +1588,9 @@ describe('vaultsSecretsMkdir', () => { // Check if the operation concluded as expected for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - expect(error.code).toEqual('EEXIST'); - expect(error.reason).toEqual(dirPath); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('EEXIST'); + expect(data.reason).toEqual(dirPath); } await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -1739,10 +1731,8 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - expect(message.secretContent).toEqual(secretContent); + if (data.type !== 'success') utils.never() + expect(data.secretContent).toEqual(secretContent); } }); test('fails to read invalid secret', async () => { @@ -1760,11 +1750,9 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(secretName); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(secretName); } }); test('fails to read a directory', async () => { @@ -1788,11 +1776,9 @@ describe('vaultsSecretsCat', () => { // Read response for await (const data of response.readable) { expect(data.type).toEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('EISDIR'); - expect(error.reason).toEqual(secretName); + if (data.type !== 'error') utils.never(); + expect(data.code).toEqual('EISDIR'); + expect(data.reason).toEqual(secretName); } }); test('reads multiple secrets in order', async () => { @@ -1820,10 +1806,8 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + if (data.type !== 'success') utils.never(); + totalContent += data.secretContent; } expect(totalContent).toEqual(`${secretContent1}${secretContent2}`); }); @@ -1864,10 +1848,8 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { expect(data.type).toEqual('success'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + if (data.type !== 'success') utils.never(); + totalContent += data.secretContent; } expect(totalContent).toEqual( `${secretContent1}${secretContent2}${secretContent3}`, @@ -1913,16 +1895,10 @@ describe('vaultsSecretsCat', () => { let totalContent = ''; for await (const data of response.readable) { if (data.type === 'success') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const message = data as ContentSuccessMessage; - totalContent += message.secretContent; + totalContent += data.secretContent; } else { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'success'. - const error = data as ErrorMessage; - expect(error.code).toEqual('ENOENT'); - expect(error.reason).toEqual(invalidName); + expect(data.code).toEqual('ENOENT'); + expect(data.reason).toEqual(invalidName); } } expect(totalContent).toEqual( @@ -2374,11 +2350,9 @@ describe('vaultsSecretsRemove', () => { // Write paths const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - let variable: ClientRPCRequestParams = {type: 'VaultNamesHeaderMesage', vaultNames: ['invalid']}; - console.log(variable); // Header message await writer.write({ - type: 'VaultNamesHeaderMesage', + type: 'VaultNamesHeaderMessage', vaultNames: ['invalid'], }); // Content messages @@ -2410,17 +2384,27 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: '/' }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: '/', + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('error'); - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; - // The error code should be an invalid operation - expect(error.code).toStrictEqual('EINVAL'); + if (data.type !== 'error') utils.never(); + expect(data.code).toStrictEqual('EINVAL'); } // Check + expect(loopRun).toBeTruthy(); await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { expect(await efs.exists(secretName)).toBeTruthy(); @@ -2442,12 +2426,29 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); } + expect(loopRun).toBeTruthy(); // Check each secret was deleted await vaultManager.withVaults([vaultId], async (vault) => { await vault.readF(async (efs) => { @@ -2472,18 +2473,33 @@ describe('vaultsSecretsRemove', () => { // Delete secrets const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: invalidName }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: invalidName, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); let errorCount = 0; for await (const data of response.readable) { if (data.type === 'error') { - // TS cannot properly evaluate a type as nested as this, so we use the - // as keyword to help it. Inside this block, the type of data is 'error'. - const error = data as ErrorMessage; // No other file name should raise this error - expect(error.reason).toStrictEqual(invalidName); + expect(data.reason).toStrictEqual(invalidName); errorCount++; continue; } @@ -2514,19 +2530,39 @@ describe('vaultsSecretsRemove', () => { // Get log size let logLength = 0; await vaultManager.withVaults([vaultId], async (vault) => { + console.log(await vault.log()) logLength = (await vault.log()).length; }); // Delete secret const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded, secretName: secretName2 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded, + secretName: secretName2, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); + console.log(data.type); } + expect(loopRun).toBeTruthy(); // Ensure single log message for deleting the secrets await vaultManager.withVaults([vaultId], async (vault) => { + console.log(await vault.log()) expect((await vault.log()).length).toEqual(logLength + 1); }); }); @@ -2555,14 +2591,35 @@ describe('vaultsSecretsRemove', () => { // Delete secret const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); - await writer.write({ nameOrId: vaultIdEncoded1, secretName: secretName1 }); - await writer.write({ nameOrId: vaultIdEncoded2, secretName: secretName2 }); - await writer.write({ nameOrId: vaultIdEncoded1, secretName: secretName3 }); + // Header message + await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded1, vaultIdEncoded2], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded1, + secretName: secretName1, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded2, + secretName: secretName2, + }); + await writer.write({ + type: 'SecretIdentifierMessage', + nameOrId: vaultIdEncoded1, + secretName: secretName3, + }); await writer.close(); + let loopRun = false; for await (const data of response.readable) { + loopRun = true; expect(data.type).toStrictEqual('success'); } // Ensure single log message for deleting the secrets + expect(loopRun).toBeTruthy(); await vaultManager.withVaults( [vaultId1, vaultId2], async (vault1, vault2) => { @@ -2595,10 +2652,17 @@ describe('vaultsSecretsRemove', () => { // Deleting directory with recursive set should not fail const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); + // Header message await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + recursive: true, + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', nameOrId: vaultIdEncoded, secretName: dirName, - metadata: { options: { recursive: true } }, }); await writer.close(); for await (const data of response.readable) { @@ -2632,7 +2696,14 @@ describe('vaultsSecretsRemove', () => { // Deleting directory with recursive set should not fail const response = await rpcClient.methods.vaultsSecretsRemove(); const writer = response.writable.getWriter(); + // Header message await writer.write({ + type: 'VaultNamesHeaderMessage', + vaultNames: [vaultIdEncoded], + }); + // Content messages + await writer.write({ + type: 'SecretIdentifierMessage', nameOrId: vaultIdEncoded, secretName: dirName, });