Feature: Extension loader #25

oscar60310 · 2022-07-28T09:17:29Z

Description

This PR implements the design of Unite extension loader: Create a central extension loader to import internal/external extensions and manage their config.

Design

Super classes of extensions

We are going to create a super class for each extension type, these classes located at packages/<package-name>/src/models/extensions.
```
📦packages
 ┣ 📂build
 ┃ ┣ 📂src
 ┃ ┃ ┣ 📂models
 ┃ ┃ ┃ ┣ 📂extensions
 ┃ ┃ ┃ ┃ ┣ 📜schemaReader.ts
 ┃ ┃ ┃ ┃ ┗ 📜specGenerator.ts
 ┣ 📂core
 ┃ ┣ 📂src
 ┃ ┃ ┣ 📂models
 ┃ ┃ ┃ ┣ 📂extensions
 ┃ ┃ ┃ ┃ ┣ 📜codeLoader.ts
 ┃ ┃ ┃ ┃ ┣ 📜dataSource.ts
 ┃ ┃ ┃ ┃ ┣ 📜filterBuilder.ts
 ┃ ┃ ┃ ┃ ┣ 📜filterRunner.ts
 ┃ ┃ ┃ ┃ ┣ 📜inputValidator.ts
 ┃ ┃ ┃ ┃ ┣ 📜persistentStore.ts
 ┃ ┃ ┃ ┃ ┣ 📜serializer.ts
 ┃ ┃ ┃ ┃ ┣ 📜tagBuilder.ts
 ┃ ┃ ┃ ┃ ┣ 📜tagRunner.ts
 ┃ ┃ ┃ ┃ ┣ 📜templateEngine.ts
 ┃ ┃ ┃ ┃ ┗ 📜templateProvider.ts
 ┣ 📂serve
 ┃ ┣ 📂src
 ┃ ┃ ┣ 📂models
 ┃ ┃ ┃ ┣ 📂extensions
 ┃ ┃ ┃ ┃ ┣ 📜responseFormatter.ts
 ┃ ┃ ┃ ┃ ┗ 📜routeMiddleware.ts
```
These extension types have been created.
- Template Provider: Provide the content of .sql files.
- TagBuilder: Parse template, create custom tags {% someTag %}, traverse AST, generate metadata…
- TagRunner: Runtime code of custom tags.
- FilterBuilder: Create custom filter {{ [1,2,3] | customFIlter }}, taverse AST, generate metadata…
- FilterRunner: Runtime code of custom filter.
- CompileTimeExtension: Traverse AST, generate metadata…
- RuntimeExtension
- SchemaReader: Provide the content of .yaml files.
- PersistentStore: Persistent store of Vulcan artifacts.
- Serializer: Vulcan artifacts serializer.
- SpecGenerator: Vulcan schema → Spec (e.g. OAS)
- CodeLoader: Load compiled code and provide js functions while needed.
- Validator: Validate input parameters.
- ResponseFormatter: Serialize response (e.g. CSV, JSON …)
- Server Koa Middleware
- DataSource: Warehouse drivers.

Every superclass must be decorated by VulcanExtension decorator which indicates the identifier of this type of extension.

@VulcanExtension(TYPES.Extension_PersistentStore)
export abstract class PersistentStore<C = any> extends ExtensionBase<C> {
  abstract save(data: Buffer): Promise<void>;
  abstract load(): Promise<Buffer>;
}

Internal extension must be decorated by @VulcanInternalExtension decorator which indicates the module name of this extension. We can use getConfig() to retrieve module-based config, if you don't need module-based config, you can leave the module name empty.

@VulcanInternalExtension('module-name')
export class JSONSerializer<T> extends Serializer<T> {
  public serialize(data: T): Buffer {
    return Buffer.from(JSON.stringify(data), 'utf-8');
  }

  public deserialize(raw: Buffer): T {
    return JSON.parse(raw.toString('utf-8')) as T;
  }
}

Vulcan Extension Id

In some cases, we need a name switch among extensions, e.g. chose the serializer for artifact builder:

export class BinaraySerializer<T> extends Serializer<T> {
   xxx
}

artifact:
   serializer: 'bin' # json -> bin

This name often needs to be obtained before it is initialized, so I use a decorator @VulcanExtensionId to record it, we can call getExtensionId() or get the metadata with key EXTENSION_IDENTIFIER_METADATA_KEY to get the name. I replace all name properties with this decorator including formaters, validators ... etc.

Use same container with core package

We used another container and use core container as its parent, it might cause some issues because we'll load external extensions at core package but inject dependencies from build or serve package. So I decided to use the same container in this PR.

# before
const coreContainer = new CoreContainer();
await coreContainer.load(options);
this.inversifyContainer.parent = coreContainer.getInversifyContainer();

# after
this.coreContainer = new CoreContainer();
await this.coreContainer.load(options);
this.inversifyContainer = this.coreContainer.getInversifyContainer();

Integration testing

For testing across packages, we create a package with the name integration-testing, it uses pg-mem as a mock data warehouse

- Add extension loader and its module. - Update import path from ‘@vulcan-sql/core/containers’ to '@vulcan-sql/core/types' to prevent improper dependencies. - Update extension config from string array to object to receive module aliases. - Update validatorLoader to receive imported classes. - Migrate all core extensions to new style.

- Use same container with core package instead of creating a new one (for external extensions). - Add two types of extensions: route middleware and formatter. - Bind Vulcan application to container. - Using centralized extension loader, remove the old loader in serve package. - Use new super class for all extensions. Co-authored-by: kokokuo <v6610688@gmail.com>

- Add @VulcanExtensionId decorator - Expose templateProvider, serializer, and persistenStore interfaces. - Load dynamic named extensions. - Change provider configuration from enum to string to support external extensions.

- expose spec generator and schema parser. - add project level option - add document generator

…f name property

…e property

kokokuo

Extremely powerful code changed 👍

Besides some comment suggestions, and a few more questions, all part is LGTM 💯

kokokuo · 2022-08-11T03:17:59Z

packages/core/src/containers/modules/executor.ts

+    bind(TYPES.ExecutorOptions).to(ExecutorOptions);
+
+    // Data source
+    bind(TYPES.Factory_DataSource).toAutoNamedFactory(


Just curious, why here not use generic typing annotation for `

bind<interfaces.AutoNamedFactory<DataSource>>(TYPES.Factory_DataSource).toAutoNamedFactory(TYPES.Extension_DataSource);

like PersistentStore in artifactBuilderModule ?

bind<interfaces.AutoNamedFactory<PersistentStore>>( TYPES.Factory_PersistentStore ).toAutoNamedFactory<PersistentStore>(TYPES.Extension_PersistentStore);

The generic types here only take effect while binding, we have to specify these interfaces again while resolving: containter.get<SomeInterface>(TYPES.XXXX) , so I didn't define them while binding. But for keeping consistency, I'll add them back~

Thanks for @oscar60310 solving my question :D

kokokuo · 2022-08-11T03:38:13Z

packages/core/src/containers/modules/templateEngine.ts

-    ).toAutoNamedFactory<TemplateProvider>(TYPES.TemplateProvider);
+    ).toAutoNamedFactory<TemplateProvider>(TYPES.Extension_TemplateProvider);
+
+    if (options.provider) {


Seems the provider does not get, according to template engine source code, it will throw an error when calling compile, right?

If so, maybe you could add a simple comment to let others know the reason why the template provider not need to bind to prevent guessing the reason or tracing to much code deeply

Sure, I'll add some comments at container module.

Thanks for @oscar60310 !

kokokuo · 2022-08-11T03:39:03Z

packages/core/src/containers/modules/templateEngine.ts

@@ -60,8 +61,19 @@ export const templateEngineModule = (
      .whenTargetNamed('compileTime');

    // Loader
-    bind<ICodeLoader>(TYPES.CompilerLoader)
-      .to(InMemoryCodeLoader)
+    bind(TYPES.Factory_CompilerLoader).toAutoNamedFactory(


Same as above DataSource auto factory question.

I've add types definitions for all binding~

Thanks for @oscar60310 !

kokokuo · 2022-08-11T04:04:39Z

packages/core/src/models/executorOptions.ts

@@ -0,0 +1,3 @@
+export interface IExecutorOptions {
+  type?: string;


What do the type used for? the data source type used for creating different data sources?
If so maybe you could add a comment or name it, because type is a universal word.

Yes, it indicated the driver to use while sending queries, single driver design might be changed in the future. The property name here affects the user config, that is, "executor.type" v.s. "executor.executorType". So I think it would be better to use the name "type" and add some comments to the interface.

Thanks for @oscar60310 !

kokokuo · 2022-08-11T04:07:16Z

packages/core/src/models/artifactBuilderOptions.ts

 export interface IArtifactBuilderOptions {
-  provider: PersistentStoreType;
-  serializer: SerializerType;
+  provider: string;


Suggest adding a comment on the provider to notice it;s PersistentStoreType, because we could only know it isstring type,and provider could have mutiple meaning, also could give some sample for hint.

I'll add comments for them.

Thanks alot !

kokokuo · 2022-08-12T03:48:32Z

packages/build/src/models/documentGeneratorOptions.ts

@@ -0,0 +1,4 @@
+export interface IDocumentGeneratorOptions {
+  specs?: string[];


I think you could add some comment to expose what the data provide to spec and give some sample, e.g: oas3

I've set the built-in enum as a union type, it might help us to know more about the property.

export enum DocumentGeneratorSpec { oas3 = 'oas3', } export interface IDocumentGeneratorOptions { specs?: (string | DocumentGeneratorSpec)[]; folderPath: string; }

BTW, it takes no effect when using the interface:

kokokuo · 2022-08-12T03:53:00Z

packages/core/src/models/artifactBuilderOptions.ts

-  provider: PersistentStoreType;
-  serializer: SerializerType;
+  provider: string;
+  serializer: string;


Same as provider, the serializer is string type and there is no sample for expose what value we should provide, so we may could add some sample or some comment, e.g: file format type for serializer ... so on

kokokuo · 2022-08-12T03:53:56Z

packages/serve/src/containers/modules/routeGenerator.ts

@@ -27,7 +27,7 @@ export const routeGeneratorModule = () =>
      .to(PaginationTransformer)
      .inSingletonScope();

-    // Roue Generator
+    // Route Generator
    bind<RouteGenerator>(TYPES.RouteGenerator)


Thanks for modifying my typo !!

kokokuo · 2022-08-12T03:53:59Z

packages/serve/src/containers/modules/routeGenerator.ts

@@ -17,7 +17,7 @@ export const routeGeneratorModule = () =>
      .to(RequestTransformer)
      .inSingletonScope();

-    // Request Transformer
+    // Request Validator


Thanks for modifying my typo !!

kokokuo · 2022-08-12T04:57:49Z

packages/serve/src/containers/modules/application.ts

+
+export const applicationModule = () =>
+  new AsyncContainerModule(async (bind) => {
+    bind(TYPES.VulcanApplication).to(VulcanApplication);


Just curious for bind, seems the interface/class no need to indicate explicitly e.g: bind<VulcanApplication>(TYPES.VulcanApplication).to(VulcanApplication); ?

In the beginning, I think the only interface should define explicitly in bind, but seems classes could also indicate class type in bind, e.g schemaPaser module in build package:

bind<SchemaParser>(TYPES.SchemaParser).to(SchemaParser).inSingletonScope();

but sometimes you would define indicate class type, e.g: documentGenerator module in build package

bind(TYPES.DocumentGenerator).to(DocumentGenerator);

And also sometimes, even the interface, won't indicate directly when using bind, but sometimes use it:

// schemaPaser module in build package bind<ISchemaParserOptions>(TYPES.SchemaParserInputOptions).toConstantValue( options || ({} as any) ); // executor module bind(TYPES.ExecutorInputOptions).toConstantValue(options); bind(TYPES.ExecutorOptions).to(ExecutorOptions);

Although I know its just to indicate the used class/interface for generic function/method for typescript feature, and also could let vscode could IntelliSense know, but still curious does it exist any reason inversify.js needed to indicate interface /class type explicitly that I missed ?

Although I know its just to indicate the used class/interface for generic function/method for typescript feature, and also could let vscode could IntelliSense know, but still curious does it exist any reason inversify.js needed to indicate interface /class type explicitly that I missed ?

No, Inversify doesn't need the generics, it only cares about the identifiers. The generics here just make sure we bind the correct target on the identifier.

I've added generics for all bindings with interfaces/abstract classes when possible, but in some cases (like VulcanApplication) we need to use the class directly as the generics type.

Thanks for solving my multiple similar questions again and again!

oscar60310 · 2022-08-17T05:45:17Z

Hi @kokokuo , all issues have been fixed vai new commits, thanks a lot!

kokokuo · 2022-08-17T06:45:14Z

Thanks for fixing the suggestion and relying my questions !

oscar60310 force-pushed the feature/extension-loader branch 8 times, most recently from 35df3a6 to 4ebc102 Compare August 3, 2022 06:42

oscar60310 force-pushed the feature/extension-loader branch 7 times, most recently from 90910fe to b4138de Compare August 4, 2022 07:21

oscar60310 and others added 6 commits August 4, 2022 16:30

fix(build): update to fit new validator loaders

5c6061b

feat(core): add module name to extension

73e5ab9

chore(dbt, ext-utils): update to new extension style

628fc8d

fix(core): let template engine option be optional

ad5c7d0

test(serve): add test for server

4b8f34b

oscar60310 force-pushed the feature/extension-loader branch 3 times, most recently from c18bfe5 to 91d34ea Compare August 5, 2022 04:19

feat(core): support dynamic named extensions

80df5d9

- Add @VulcanExtensionId decorator - Expose templateProvider, serializer, and persistenStore interfaces. - Load dynamic named extensions. - Change provider configuration from enum to string to support external extensions.

oscar60310 force-pushed the feature/extension-loader branch 4 times, most recently from 8fc9055 to 3bc3d81 Compare August 5, 2022 07:44

oscar60310 force-pushed the feature/extension-loader branch from da6d02c to b65d1dd Compare August 5, 2022 08:55

oscar60310 added 8 commits August 8, 2022 15:06

feat(core, build): expose spec generator and schema parser

77a8ac7

- expose spec generator and schema parser. - add project level option - add document generator

refactor(core): let input validators use @VulcanExtensionId instead o…

50c420b

…f name property

refactor(serve): let formatters use @VulcanExtensionId instead of nam…

da7f20d

…e property

feat(core): expose codeLoader to extension list

1605c74

refactor(core, build): move factory logic to container module

528bc59

feat(int-test): add integration testing package

4da91ce

refactor(core): exporse data source extensions

b36c62f

test(serve, int-test): test query from pg-mem

b2cfd97

oscar60310 force-pushed the feature/extension-loader branch from 0fab9b5 to b2cfd97 Compare August 8, 2022 07:46

oscar60310 changed the title ~~[WIP] Feature: Extension loader~~ Feature: Extension loader Aug 8, 2022

oscar60310 requested a review from kokokuo August 8, 2022 07:46

oscar60310 marked this pull request as ready for review August 8, 2022 07:46

kokokuo approved these changes Aug 12, 2022

View reviewed changes

fix(core,serve): fix improper imports

9bad6f1

oscar60310 force-pushed the feature/extension-loader branch from 58c8aa4 to 9bad6f1 Compare August 12, 2022 07:35

kokokuo mentioned this pull request Aug 16, 2022

Feature: SQL Injection Prevention (Flow Simulation) #23

Merged

oscar60310 added 3 commits August 17, 2022 10:44

refactor(build,core): add missing generic for containers binding

a8c80a6

refactor(build,core): add missing enums for providers, serializers

4b905db

refactor(build,core): add missing comments for option interfaces

1ef7c5d

kokokuo mentioned this pull request Aug 17, 2022

Feature: Add Auth middleware and Authenticator. #26

Merged

kokokuo merged commit 66e58aa into develop Aug 17, 2022

kokokuo deleted the feature/extension-loader branch August 17, 2022 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Extension loader #25

Feature: Extension loader #25

oscar60310 commented Jul 28, 2022 •

edited

Loading

kokokuo left a comment

kokokuo Aug 11, 2022

oscar60310 Aug 17, 2022

kokokuo Aug 17, 2022

kokokuo Aug 11, 2022

oscar60310 Aug 17, 2022

kokokuo Aug 17, 2022

kokokuo Aug 11, 2022

oscar60310 Aug 17, 2022

kokokuo Aug 17, 2022

kokokuo Aug 11, 2022

oscar60310 Aug 17, 2022

kokokuo Aug 17, 2022

kokokuo Aug 11, 2022

oscar60310 Aug 17, 2022

kokokuo Aug 17, 2022

kokokuo Aug 12, 2022

oscar60310 Aug 17, 2022 •

edited

Loading

kokokuo Aug 12, 2022

kokokuo Aug 12, 2022

kokokuo Aug 12, 2022

kokokuo Aug 12, 2022

oscar60310 Aug 17, 2022 •

edited

Loading

kokokuo Aug 17, 2022

oscar60310 commented Aug 17, 2022

kokokuo commented Aug 17, 2022

		@@ -0,0 +1,3 @@
		export interface IExecutorOptions {
		type?: string;

		@@ -0,0 +1,4 @@
		export interface IDocumentGeneratorOptions {
		specs?: string[];

Feature: Extension loader #25

Feature: Extension loader #25

Conversation

oscar60310 commented Jul 28, 2022 • edited Loading

Description

Design

Super classes of extensions

Vulcan Extension Id

Use same container with core package

Integration testing

kokokuo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar60310 commented Aug 17, 2022

kokokuo commented Aug 17, 2022

oscar60310 commented Jul 28, 2022 •

edited

Loading

oscar60310 Aug 17, 2022 •

edited

Loading

oscar60310 Aug 17, 2022 •

edited

Loading