From fd35627d2c7b88cac4bcecdf5261f05e59816d20 Mon Sep 17 00:00:00 2001 From: Gimmy Date: Wed, 6 Dec 2023 11:22:58 +0200 Subject: [PATCH 1/2] validate and consolidate config --- __tests__/directory/v3/index.test.ts | 283 ++++++++++++++++++++++++++- lib/directory/errors.ts | 1 + lib/directory/v3/index.ts | 150 +++++++------- lib/directory/v3/null.ts | 69 +++++++ 4 files changed, 421 insertions(+), 82 deletions(-) create mode 100644 lib/directory/v3/null.ts diff --git a/__tests__/directory/v3/index.test.ts b/__tests__/directory/v3/index.test.ts index 9a2dcbc..13fe76f 100644 --- a/__tests__/directory/v3/index.test.ts +++ b/__tests__/directory/v3/index.test.ts @@ -29,6 +29,14 @@ import { createAsyncIterable } from "@connectrpc/connect/protocol"; import * as connectNode from "@connectrpc/connect-node"; import { + nullExporterProxy, + nullImporterProxy, + nullModelProxy, + nullReaderProxy, + nullWriterProxy, +} from "../../../lib/directory/v3/null"; +import { + ClientNotConfiguredError, DirectoryServiceV3, DirectoryV3, EtagMismatchError, @@ -193,6 +201,251 @@ describe("DirectoryV3", () => { expect(directory.ModelClient).toBeDefined(); }); + describe("Reader", () => { + it("inherits base config", () => { + const mockTransport = jest.spyOn(connectNode, "createGrpcTransport"); + const mockFs = jest + .spyOn(fs, "readFileSync") + .mockImplementation((path: fs.PathOrFileDescriptor) => { + return path as string; + }); + + const directory = DirectoryServiceV3({ + tenantId: "tenantId", + apiKey: "apiKey", + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ReaderClient).toBeDefined(); + + mockFs.mockReset(); + mockTransport.mockReset(); + }); + + it("allows to be configured individually", () => { + const mockTransport = jest.spyOn(connectNode, "createGrpcTransport"); + const mockFs = jest + .spyOn(fs, "readFileSync") + .mockImplementation((path: fs.PathOrFileDescriptor) => { + return path as string; + }); + + const directory = DirectoryServiceV3({ + reader: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ReaderClient).toBeDefined(); + expect(mockTransport.mock.calls).toEqual([ + [ + expect.objectContaining({ + baseUrl: "https://directory.prod.aserto.com:8443", + httpVersion: "2", + nodeOptions: { rejectUnauthorized: true }, + }), + ], + ]); + mockFs.mockReset(); + mockTransport.mockReset(); + }); + + it("overwrites base config", () => { + const mockTransport = jest.spyOn(connectNode, "createGrpcTransport"); + const mockFs = jest + .spyOn(fs, "readFileSync") + .mockImplementation((path: fs.PathOrFileDescriptor) => { + return path as string; + }); + + const directory = DirectoryServiceV3({ + reader: { + url: "readerurl", + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ReaderClient).toBeDefined(); + expect(mockTransport.mock.calls).toEqual([ + [ + expect.objectContaining({ + baseUrl: "https://readerurl", + httpVersion: "2", + nodeOptions: { rejectUnauthorized: true }, + }), + ], + ]); + mockFs.mockReset(); + }); + + describe("when config is missing", () => { + const directory = DirectoryServiceV3({ + writer: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + it("is defined as a proxy", () => { + expect(directory.ReaderClient).toBe(nullReaderProxy); + }); + + it("throws ClientNotConfigured Error when called", async () => { + await expect(directory.objects({ objectType: "" })).rejects.toThrow( + ClientNotConfiguredError + ); + + await expect(directory.objects({ objectType: "" })).rejects.toThrow( + `Cannot call 'getObjects', 'Reader' is not configured.` + ); + }); + }); + }); + + describe("Writer", () => { + it("inherits base config", () => { + const directory = DirectoryServiceV3({ + tenantId: "tenantId", + apiKey: "apiKey", + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.WriterClient).toBeDefined(); + }); + + describe("when config is missing", () => { + const directory = DirectoryServiceV3({ + reader: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + it("is defined as a proxy", () => { + expect(directory.WriterClient).toBe(nullWriterProxy); + }); + + it("throws ClientNotConfigured Error when called", async () => { + await expect(directory.setObject({})).rejects.toThrow( + ClientNotConfiguredError + ); + + await expect(directory.setObject({})).rejects.toThrow( + `Cannot call 'setObject', 'Writer' is not configured.` + ); + }); + }); + }); + + describe("Importer", () => { + it("inherits base config", () => { + const directory = DirectoryServiceV3({ + tenantId: "tenantId", + apiKey: "apiKey", + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ImporterClient).toBeDefined(); + }); + + describe("when config is missing", () => { + const directory = DirectoryServiceV3({ + reader: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + it("is defined as a proxy", () => { + expect(directory.ImporterClient).toBe(nullImporterProxy); + }); + + it("throws ClientNotConfigured Error when called", async () => { + await expect(directory.import(createAsyncIterable([]))).rejects.toThrow( + ClientNotConfiguredError + ); + + await expect(directory.import(createAsyncIterable([]))).rejects.toThrow( + `Cannot call 'import', 'Importer' is not configured.` + ); + }); + }); + }); + + describe("Exporter", () => { + it("inherits base config", () => { + const directory = DirectoryServiceV3({ + tenantId: "tenantId", + apiKey: "apiKey", + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ExporterClient).toBeDefined(); + }); + + describe("when config is missing", () => { + const directory = DirectoryServiceV3({ + reader: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + it("is defined as a proxy", () => { + expect(directory.ExporterClient).toBe(nullExporterProxy); + }); + + it("throws ClientNotConfigured Error when called", async () => { + await expect(directory.export({ options: "all" })).rejects.toThrow( + ClientNotConfiguredError + ); + + await expect(directory.export({ options: "all" })).rejects.toThrow( + `Cannot call 'export', 'Exporter' is not configured.` + ); + }); + }); + }); + + describe("Model", () => { + it("inherits base config", () => { + const directory = DirectoryServiceV3({ + tenantId: "tenantId", + apiKey: "apiKey", + }); + + expect(directory).toBeInstanceOf(DirectoryV3); + expect(directory.ModelClient).toBeDefined(); + }); + + describe("when config is missing", () => { + const directory = DirectoryServiceV3({ + reader: { + tenantId: "tenantId", + apiKey: "apiKey", + }, + }); + + it("is defined as a proxy", () => { + expect(directory.ModelClient).toBe(nullModelProxy); + }); + + it("throws ClientNotConfigured Error when called", async () => { + await expect(directory.deleteManifest()).rejects.toThrow( + ClientNotConfiguredError + ); + + await expect(directory.deleteManifest()).rejects.toThrow( + `Cannot call 'deleteManifest', 'Model' is not configured.` + ); + }); + }); + }); + describe("checkPermission", () => { it("calls checkPermission with valid params", async () => { const mockCheckPermission = jest @@ -505,7 +758,15 @@ describe("DirectoryV3", () => { .spyOn(directory.WriterClient, "setObject") .mockRejectedValue(new Error("Directory service error")); - const params = { objectType: "user" }; + const params = { + object: { + type: "user", + id: "test-user", + properties: { + displayName: "test user", + }, + }, + }; await expect(directory.setObject(params)).rejects.toThrow( "Directory service error" ); @@ -520,7 +781,15 @@ describe("DirectoryV3", () => { new ConnectError("Invalid argument", Code.InvalidArgument) ); - const params = {}; + const params = { + object: { + type: "user", + id: "test-user", + properties: { + displayName: "test user", + }, + }, + }; // error class await expect(directory.setObject(params)).rejects.toThrow( @@ -541,7 +810,15 @@ describe("DirectoryV3", () => { new ConnectError("Invalid argument", Code.FailedPrecondition) ); - const params = {}; + const params = { + object: { + type: "user", + id: "test-user", + properties: { + displayName: "test user", + }, + }, + }; // error class await expect(directory.setObject(params)).rejects.toThrow( diff --git a/lib/directory/errors.ts b/lib/directory/errors.ts index 4db6e66..decb1f9 100644 --- a/lib/directory/errors.ts +++ b/lib/directory/errors.ts @@ -6,6 +6,7 @@ * @extends Error */ class DirectoryServiceError extends Error {} +export class ClientNotConfiguredError extends DirectoryServiceError {} /** * Object or Relation is not found. * Extends the DirectoryServiceError class. diff --git a/lib/directory/v3/index.ts b/lib/directory/v3/index.ts index bde9991..444ef78 100644 --- a/lib/directory/v3/index.ts +++ b/lib/directory/v3/index.ts @@ -1,5 +1,4 @@ import { readFileSync } from "fs"; -import { ClientSessionOptions, SecureClientSessionOptions } from "http2"; import { PaginationRequest } from "@aserto/node-directory/src/gen/cjs/aserto/directory/common/v3/common_pb"; import { Exporter } from "@aserto/node-directory/src/gen/cjs/aserto/directory/exporter/v3/exporter_connect"; import { ExportRequest } from "@aserto/node-directory/src/gen/cjs/aserto/directory/exporter/v3/exporter_pb"; @@ -40,6 +39,13 @@ import { NotFoundError, UnauthenticatedError, } from "../errors"; +import { + nullExporterProxy, + nullImporterProxy, + nullModelProxy, + nullReaderProxy, + nullWriterProxy, +} from "./null"; import { CheckPermissionRequest, CheckRelationRequest, @@ -111,22 +117,41 @@ export class DirectoryV3 { return headers; }; + const validConfig = ( + config: ServiceConfig | undefined, + fallback: ServiceConfig | undefined + ) => { + return ( + !!config?.url || + !!fallback?.url || + ((!!config?.apiKey || !!fallback?.apiKey) && + (config?.tenantId || !!fallback?.tenantId)) + ); + }; + const createTransport = ( - serviceUrl: string, - apikey?: string, - tenantId?: string, - nodeOptions?: ClientSessionOptions | SecureClientSessionOptions + config: ServiceConfig | undefined, + fallback: ServiceConfig | undefined ) => { + if (!validConfig(config, fallback)) { + return; + } + + const serviceUrl = config?.url || fallback?.url; + const apiKey = config?.apiKey || fallback?.apiKey; + const tenantId = config?.tenantId || fallback?.tenantId; + const nodeOptions = createNodeOptions(config); + if ( serviceUrl !== baseServiceUrl || - apikey !== baseApiKey || + apiKey !== baseApiKey || tenantId !== baseTenantId || nodeOptions !== baseNodeOptions ) { return createGrpcTransport({ httpVersion: "2", - baseUrl: `https://${serviceUrl}`, - interceptors: [createHeadersInterceptor(apikey, tenantId)], + baseUrl: `https://${serviceUrl || "directory.prod.aserto.com:8443"}`, + interceptors: [createHeadersInterceptor(apiKey, tenantId)], nodeOptions: nodeOptions, }); } @@ -145,84 +170,48 @@ export class DirectoryV3 { rejectUnauthorized = config.rejectUnauthorized; } - const baseServiceUrl = config.url ?? "directory.prod.aserto.com:8443"; + const baseServiceUrl = config.url; const baseApiKey = config.apiKey; const baseTenantId = config.tenantId; const baseCaFile = !!config.caFile ? readFileSync(config.caFile) : undefined; - const readerServiceUrl = config.reader?.url || baseServiceUrl; - const readerApiKey = config.reader?.apiKey || baseApiKey; - const readerTenantId = config.reader?.tenantId || baseTenantId; - const readerNodeOptions = createNodeOptions(config.reader); - - const writerServiceUrl = config.writer?.url || baseServiceUrl; - const writerApiKey = config.writer?.apiKey || baseApiKey; - const writerTenantId = config.writer?.tenantId || baseTenantId; - const writerNodeOptions = createNodeOptions(config.writer); - - const importerServiceUrl = config.importer?.url || baseServiceUrl; - const importerApiKey = config.importer?.apiKey || baseApiKey; - const importerTenantId = config.importer?.tenantId || baseTenantId; - const importerNodeOptions = createNodeOptions(config.importer); - - const exporterServiceUrl = config.exporter?.url || baseServiceUrl; - const exporterApiKey = config.exporter?.apiKey || baseApiKey; - const exporterTenantId = config.exporter?.tenantId || baseTenantId; - const exporterNodeOptions = createNodeOptions(config.exporter); - - const modelServiceUrl = config.model?.url || baseServiceUrl; - const modelApiKey = config.model?.apiKey || baseApiKey; - const modelTenantId = config.model?.tenantId || baseTenantId; - const modelNodeOptions = createNodeOptions(config.model); - const baseNodeOptions = { rejectUnauthorized, ca: baseCaFile }; - const baseGrpcTransport = createGrpcTransport({ - httpVersion: "2", - baseUrl: `https://${baseServiceUrl}`, - interceptors: [baseServiceHeaders], - nodeOptions: baseNodeOptions, - }); - - const readerGrpcTransport = createTransport( - readerServiceUrl, - readerApiKey, - readerTenantId, - readerNodeOptions - ); - const writerGrpcTransport = createTransport( - writerServiceUrl, - writerApiKey, - writerTenantId, - writerNodeOptions - ); - const importerGrpcTransport = createTransport( - importerServiceUrl, - importerApiKey, - importerTenantId, - importerNodeOptions - ); - const exporterGrpcTransport = createTransport( - exporterServiceUrl, - exporterApiKey, - exporterTenantId, - exporterNodeOptions - ); - - const modelGrpcTransport = createTransport( - modelServiceUrl, - modelApiKey, - modelTenantId, - modelNodeOptions - ); - - this.ReaderClient = createPromiseClient(Reader, readerGrpcTransport); - this.WriterClient = createPromiseClient(Writer, writerGrpcTransport); - this.ImporterClient = createPromiseClient(Importer, importerGrpcTransport); - this.ExporterClient = createPromiseClient(Exporter, exporterGrpcTransport); - this.ModelClient = createPromiseClient(Model, modelGrpcTransport); + const baseGrpcTransport = + !!config.url || (!!config.apiKey && !!config.tenantId) + ? createGrpcTransport({ + httpVersion: "2", + baseUrl: `https://${baseServiceUrl}`, + interceptors: [baseServiceHeaders], + nodeOptions: baseNodeOptions, + }) + : undefined; + + const readerGrpcTransport = createTransport(config.reader, config); + const writerGrpcTransport = createTransport(config.writer, config); + const importerGrpcTransport = createTransport(config.importer, config); + const exporterGrpcTransport = createTransport(config.exporter, config); + + const modelGrpcTransport = createTransport(config.model, config); + + this.ReaderClient = !!readerGrpcTransport + ? createPromiseClient(Reader, readerGrpcTransport) + : (nullReaderProxy as unknown as PromiseClient); + this.WriterClient = !!writerGrpcTransport + ? createPromiseClient(Writer, writerGrpcTransport) + : (nullWriterProxy as unknown as PromiseClient); + this.ImporterClient = !!importerGrpcTransport + ? createPromiseClient(Importer, importerGrpcTransport) + : (nullImporterProxy as unknown as PromiseClient); + this.ExporterClient = !!exporterGrpcTransport + ? createPromiseClient(Exporter, exporterGrpcTransport) + : (nullExporterProxy as unknown as PromiseClient); + + this.ModelClient = !!modelGrpcTransport + ? createPromiseClient(Model, modelGrpcTransport) + : (nullModelProxy as unknown as PromiseClient); } async checkPermission(params: CheckPermissionRequest) { @@ -299,7 +288,7 @@ export class DirectoryV3 { const response = await this.WriterClient.setObject(newParams); - return response.result; + return response?.result; } catch (error) { handleError(error, "setObject"); } @@ -390,6 +379,9 @@ export class DirectoryV3 { async getManifest(params?: PlainMessage) { try { const response = this.ModelClient.getManifest(params!); + if (!response) { + return; + } const data = (await readAsyncIterable(response)) .map((el) => el.msg) diff --git a/lib/directory/v3/null.ts b/lib/directory/v3/null.ts new file mode 100644 index 0000000..43f8804 --- /dev/null +++ b/lib/directory/v3/null.ts @@ -0,0 +1,69 @@ +// import { GetManifestResponse } from "@aserto/node-directory/src/gen/cjs/aserto/directory/model/v3/model_pb"; + +import { Exporter } from "@aserto/node-directory/src/gen/cjs/aserto/directory/exporter/v3/exporter_connect"; +import { Importer } from "@aserto/node-directory/src/gen/cjs/aserto/directory/importer/v3/importer_connect"; +import { Model } from "@aserto/node-directory/src/gen/cjs/aserto/directory/model/v3/model_connect"; +import { Reader } from "@aserto/node-directory/src/gen/cjs/aserto/directory/reader/v3/reader_connect"; +import { Writer } from "@aserto/node-directory/src/gen/cjs/aserto/directory/writer/v3/writer_connect"; + +import { ClientNotConfiguredError } from "../errors"; + +type Client = + | typeof Reader + | typeof Writer + | typeof Importer + | typeof Exporter + | typeof Model; + +const interceptCall = (target: Client, prop: string, receiver: unknown) => { + const fn = Reflect.get(target.methods, prop, receiver); + if (typeof fn !== "function") { + throw new ClientNotConfiguredError( + `Cannot call '${prop}', '${target?.typeName + ?.split(".") + ?.slice(-1)}' is not configured.` + ); + } + + throw new TypeError( + `'${prop}' is not defined on '${target?.typeName?.split(".")?.slice(-1)}'` + ); +}; + +const nullReaderProxy = new Proxy(Reader, { + get(target, prop, receiver) { + return interceptCall(target, prop as string, receiver); + }, +}); + +const nullWriterProxy = new Proxy(Writer, { + get(target, prop, receiver) { + return interceptCall(target, prop as string, receiver); + }, +}); + +const nullImporterProxy = new Proxy(Importer, { + get(target, prop, receiver) { + return interceptCall(target, prop as string, receiver); + }, +}); + +const nullExporterProxy = new Proxy(Exporter, { + get(target, prop, receiver) { + return interceptCall(target, prop as string, receiver); + }, +}); + +const nullModelProxy = new Proxy(Model, { + get(target, prop, receiver) { + return interceptCall(target, prop as string, receiver); + }, +}); + +export { + nullReaderProxy, + nullWriterProxy, + nullImporterProxy, + nullExporterProxy, + nullModelProxy, +}; From 4a64d6904734fb41c10694e2a35171b3c077b26a Mon Sep 17 00:00:00 2001 From: Gimmy Date: Thu, 7 Dec 2023 09:52:40 +0200 Subject: [PATCH 2/2] throw ConfigError --- __tests__/directory/v3/index.test.ts | 16 ++++++---------- lib/directory/errors.ts | 2 +- lib/directory/v3/null.ts | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/__tests__/directory/v3/index.test.ts b/__tests__/directory/v3/index.test.ts index 13fe76f..b4acadd 100644 --- a/__tests__/directory/v3/index.test.ts +++ b/__tests__/directory/v3/index.test.ts @@ -36,7 +36,7 @@ import { nullWriterProxy, } from "../../../lib/directory/v3/null"; import { - ClientNotConfiguredError, + ConfigError, DirectoryServiceV3, DirectoryV3, EtagMismatchError, @@ -296,7 +296,7 @@ describe("DirectoryV3", () => { it("throws ClientNotConfigured Error when called", async () => { await expect(directory.objects({ objectType: "" })).rejects.toThrow( - ClientNotConfiguredError + ConfigError ); await expect(directory.objects({ objectType: "" })).rejects.toThrow( @@ -330,9 +330,7 @@ describe("DirectoryV3", () => { }); it("throws ClientNotConfigured Error when called", async () => { - await expect(directory.setObject({})).rejects.toThrow( - ClientNotConfiguredError - ); + await expect(directory.setObject({})).rejects.toThrow(ConfigError); await expect(directory.setObject({})).rejects.toThrow( `Cannot call 'setObject', 'Writer' is not configured.` @@ -366,7 +364,7 @@ describe("DirectoryV3", () => { it("throws ClientNotConfigured Error when called", async () => { await expect(directory.import(createAsyncIterable([]))).rejects.toThrow( - ClientNotConfiguredError + ConfigError ); await expect(directory.import(createAsyncIterable([]))).rejects.toThrow( @@ -401,7 +399,7 @@ describe("DirectoryV3", () => { it("throws ClientNotConfigured Error when called", async () => { await expect(directory.export({ options: "all" })).rejects.toThrow( - ClientNotConfiguredError + ConfigError ); await expect(directory.export({ options: "all" })).rejects.toThrow( @@ -435,9 +433,7 @@ describe("DirectoryV3", () => { }); it("throws ClientNotConfigured Error when called", async () => { - await expect(directory.deleteManifest()).rejects.toThrow( - ClientNotConfiguredError - ); + await expect(directory.deleteManifest()).rejects.toThrow(ConfigError); await expect(directory.deleteManifest()).rejects.toThrow( `Cannot call 'deleteManifest', 'Model' is not configured.` diff --git a/lib/directory/errors.ts b/lib/directory/errors.ts index decb1f9..9ab9ae7 100644 --- a/lib/directory/errors.ts +++ b/lib/directory/errors.ts @@ -6,7 +6,7 @@ * @extends Error */ class DirectoryServiceError extends Error {} -export class ClientNotConfiguredError extends DirectoryServiceError {} +export class ConfigError extends DirectoryServiceError {} /** * Object or Relation is not found. * Extends the DirectoryServiceError class. diff --git a/lib/directory/v3/null.ts b/lib/directory/v3/null.ts index 43f8804..1b1e376 100644 --- a/lib/directory/v3/null.ts +++ b/lib/directory/v3/null.ts @@ -6,7 +6,7 @@ import { Model } from "@aserto/node-directory/src/gen/cjs/aserto/directory/model import { Reader } from "@aserto/node-directory/src/gen/cjs/aserto/directory/reader/v3/reader_connect"; import { Writer } from "@aserto/node-directory/src/gen/cjs/aserto/directory/writer/v3/writer_connect"; -import { ClientNotConfiguredError } from "../errors"; +import { ConfigError } from "../errors"; type Client = | typeof Reader @@ -18,7 +18,7 @@ type Client = const interceptCall = (target: Client, prop: string, receiver: unknown) => { const fn = Reflect.get(target.methods, prop, receiver); if (typeof fn !== "function") { - throw new ClientNotConfiguredError( + throw new ConfigError( `Cannot call '${prop}', '${target?.typeName ?.split(".") ?.slice(-1)}' is not configured.`