Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: refactor to IoC (inverse of control) pattern in serve package #18

Merged
merged 11 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jest.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* Add reflect0metadata to collect inversify IoC decorator information when run jest test.
*/
import 'reflect-metadata';
23 changes: 21 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,33 @@
"name": "vulcan",
"version": "0.0.0",
"license": "MIT",
"scripts": {},
"scripts": {
"test": "jest"
},
"private": true,
"dependencies": {
"class-validator": "^0.13.2",
"@koa/cors": "^3.3.0",
"glob": "^8.0.1",
"inversify": "^6.0.1",
"dayjs": "^1.11.2",
"joi": "^17.6.0",
"js-yaml": "^4.1.0",
"koa": "^2.13.4",
"koa-bodyparser": "^4.3.0",
"koa-compose": "^4.1.0",
"koa-router": "^10.1.1",
"koa2-ratelimit": "^1.1.1",
"lodash": "^4.17.21",
"nunjucks": "^3.2.3",
"openapi3-ts": "^2.0.2",
"reflect-metadata": "^0.1.13",
"tslib": "^2.3.0"
"tslib": "^2.3.0",
"tslog": "^3.3.3",
"uuid": "^8.3.2"
},
"devDependencies": {
"@faker-js/faker": "^6.3.1",
"@nrwl/cli": "14.0.3",
"@nrwl/eslint-plugin-nx": "14.0.3",
"@nrwl/jest": "14.0.3",
Expand All @@ -26,9 +38,15 @@
"@types/glob": "^7.2.0",
"@types/jest": "27.4.1",
"@types/js-yaml": "^4.0.5",
"@types/koa": "^2.13.4",
"@types/koa-compose": "^3.2.5",
"@types/koa-router": "^7.4.4",
"@types/koa2-ratelimit": "^0.9.3",
"@types/koa__cors": "^3.3.0",
"@types/lodash": "^4.14.182",
"@types/node": "16.11.7",
"@types/supertest": "^2.0.12",
"@types/uuid": "^8.3.4",
"@typescript-eslint/eslint-plugin": "~5.18.0",
"@typescript-eslint/parser": "~5.18.0",
"cz-conventional-changelog": "3.3.0",
Expand All @@ -37,6 +55,7 @@
"jest": "27.5.1",
"nx": "14.0.3",
"prettier": "^2.5.1",
"supertest": "^6.2.3",
"ts-essentials": "^9.1.2",
"ts-jest": "27.1.4",
"ts-node": "9.1.1",
Expand Down
1 change: 1 addition & 0 deletions packages/build/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ module.exports = {
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/packages/build',
setupFilesAfterEnv: ['../../jest.setup.ts'],
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { SchemaParserMiddleware } from './middleware';
import { chain } from 'lodash';
import { APISchema, ValidatorLoader } from '@vulcan/core';
import { APISchema, IValidatorLoader } from '@vulcan/core';

export const checkValidator =
(loader: ValidatorLoader): SchemaParserMiddleware =>
(loader: IValidatorLoader): SchemaParserMiddleware =>
async (schemas, next) => {
await next();
const transformedSchemas = schemas as APISchema;
Expand All @@ -16,11 +16,9 @@ export const checkValidator =
throw new Error('Validator name is required');
}

const validator = loader.getLoader(validatorRequest.name);
const validator = await loader.load(validatorRequest.name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I'll call getLocal multiple times, it appeared to me that we have imported the code of validators to memory, we just initialized them here. But with your code, we seem to import the code via .load function, will it cause performance issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @oscar60310 for reviewing and suggesting, after discussing in the morning, this part will keep and do it after a meeting discussion.


// TODO: indicate the detail of error
if (!validator.validateSchema(validatorRequest.args)) {
throw new Error(`Validator ${validatorRequest.name} schema invalid`);
}
validator.validateSchema(validatorRequest.args);
}
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
APISchema,
FieldDataType,
RequestParameter,
RequestSchema as RequestParameter,
ResponseProperty,
ValidatorDefinition,
} from '@vulcan/core';
Expand Down
18 changes: 11 additions & 7 deletions packages/build/src/lib/schema-parser/middleware/setConstraints.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
import { APISchema, ValidatorLoader } from '@vulcan/core';
import { APISchema, IValidatorLoader } from '@vulcan/core';
import { chain } from 'lodash';
import { SchemaParserMiddleware } from './middleware';

export const setConstraints =
(loader: ValidatorLoader): SchemaParserMiddleware =>
(loader: IValidatorLoader): SchemaParserMiddleware =>
async (rawSchema, next) => {
await next();
const schema = rawSchema as APISchema;

for (const request of schema.request || []) {
request.constraints = chain(request.validators || [])
.map((validator) => ({
validator: loader.getLoader(validator.name),
// load validator and keep args
const validatorsWithArgs = await Promise.all(
(request.validators || []).map(async (validator) => ({
validator: await loader.load(validator.name),
args: validator.args,
}))
);
// set constraint by validator and args
request.constraints = chain(validatorsWithArgs)
.filter(({ validator }) => !!validator.getConstraints)
.flatMap(({ validator, args }) =>
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
validator.getConstraints!(args)
)
// Group by constraint class (RequiredConstraint, MinValueConstraint ....)
) // Group by constraint class (RequiredConstraint, MinValueConstraint ....)
.groupBy((constraint) => constraint.constructor.name)
.values()
.map((constraints) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/build/src/lib/schema-parser/schemaParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
APISchema,
TemplateMetadata,
TYPES as CORE_TYPES,
ValidatorLoader,
IValidatorLoader,
} from '@vulcan/core';
import { SchemaData, SchemaFormat, SchemaReader } from './schema-reader';
import * as yaml from 'js-yaml';
Expand Down Expand Up @@ -41,7 +41,7 @@ export class SchemaParser {
@inject(TYPES.Factory_SchemaReader)
schemaReaderFactory: interfaces.AutoNamedFactory<SchemaReader>,
@inject(TYPES.SchemaParserOptions) schemaParserOptions: SchemaParserOptions,
@inject(CORE_TYPES.ValidatorLoader) validatorLoader: ValidatorLoader
@inject(CORE_TYPES.IValidatorLoader) validatorLoader: IValidatorLoader
) {
this.schemaReader = schemaReaderFactory(schemaParserOptions.reader);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
MinLengthConstraint,
MinValueConstraint,
RegexConstraint,
RequestParameter,
RequestSchema as RequestParameter,
RequiredConstraint,
ResponseProperty,
} from '@vulcan/core';
Expand Down
1 change: 1 addition & 0 deletions packages/build/test/builder/builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ it('Builder.build should work', async () => {
provider: TemplateProviderType.LocalFile,
folderPath: path.resolve(__dirname, 'source'),
},
extensions: [],
};

// Act, Assert
Expand Down
30 changes: 16 additions & 14 deletions packages/build/test/schema-parser/middleware/checkValidator.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { RawAPISchema } from '@vulcan/build/schema-parser';
import { checkValidator } from '@vulcan/build/schema-parser/middleware/checkValidator';
import { ValidatorLoader } from '@vulcan/core';
import { IValidatorLoader } from '@vulcan/core';
import * as sinon from 'ts-sinon';

it('Should pass if there is no error', async () => {
Expand All @@ -13,11 +13,11 @@ it('Should pass if there is no error', async () => {
},
],
};
const stubValidatorLoader = sinon.stubInterface<ValidatorLoader>();
stubValidatorLoader.getLoader.returns({
const stubValidatorLoader = sinon.stubInterface<IValidatorLoader>();
stubValidatorLoader.load.resolves({
name: 'validator1',
validateSchema: () => true,
validateData: () => true,
validateSchema: () => null,
validateData: () => null,
});

// Act Assert
Expand All @@ -36,11 +36,11 @@ it('Should throw if some validators have no name', async () => {
},
],
};
const stubValidatorLoader = sinon.stubInterface<ValidatorLoader>();
stubValidatorLoader.getLoader.returns({
const stubValidatorLoader = sinon.stubInterface<IValidatorLoader>();
stubValidatorLoader.load.resolves({
name: 'validator1',
validateSchema: () => true,
validateData: () => true,
validateSchema: () => null,
validateData: () => null,
});

// Act Assert
Expand All @@ -59,15 +59,17 @@ it('Should throw if the arguments of a validator is invalid', async () => {
},
],
};
const stubValidatorLoader = sinon.stubInterface<ValidatorLoader>();
stubValidatorLoader.getLoader.returns({
const stubValidatorLoader = sinon.stubInterface<IValidatorLoader>();
stubValidatorLoader.load.resolves({
name: 'validator1',
validateSchema: () => false,
validateData: () => true,
validateSchema: () => {
throw new Error();
},
validateData: () => null,
});

// Act Assert
await expect(
checkValidator(stubValidatorLoader)(schema, async () => Promise.resolve())
).rejects.toThrow('Validator validator1 schema invalid');
).rejects.toThrow();
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RawAPISchema } from '@vulcan/build/schema-parser/.';
import { RawAPISchema } from '@vulcan/build/schema-parser';
import { generatePathParameters } from '@vulcan/build/schema-parser/middleware';
import { FieldDataType, FieldInType } from '@vulcan/core';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RawAPISchema } from '@vulcan/build/schema-parser/.';
import { RawAPISchema } from '@vulcan/build/schema-parser';
import { generateTemplateSource } from '@vulcan/build/schema-parser/middleware/generateTemplateSource';

it('Should keep templateSource in schema', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RawAPISchema } from '@vulcan/build/schema-parser/.';
import { RawAPISchema } from '@vulcan/build/schema-parser';
import { generateUrl } from '@vulcan/build/schema-parser/middleware/generateUrl';

it('Should keep url in schema', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import {
Constraint,
MinValueConstraint,
RequiredConstraint,
ValidatorLoader,
IValidatorLoader,
} from '@vulcan/core';
import * as sinon from 'ts-sinon';

it('Should set and compose constraints', async () => {
// Arrange
const stubValidatorLoader = sinon.stubInterface<ValidatorLoader>();
stubValidatorLoader.getLoader.callsFake((name) => ({
const stubValidatorLoader = sinon.stubInterface<IValidatorLoader>();
stubValidatorLoader.load.callsFake(async (name) => ({
name,
validateData: () => true,
validateSchema: () => true,
validateData: () => null,
validateSchema: () => null,
getConstraints: (args) => {
if (name === 'required') return [Constraint.Required()];
return [Constraint.MinValue(args.value)];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { RawAPISchema } from '@vulcan/build/schema-parser/.';
import 'reflect-metadata';
import { RawAPISchema } from '@vulcan/build/schema-parser';
import { transformValidator } from '@vulcan/build/schema-parser/middleware/transformValidator';

it('Should convert string validator to proper format', async () => {
Expand Down
14 changes: 7 additions & 7 deletions packages/build/test/schema-parser/schemaParser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import {
SchemaParser,
SchemaReader,
} from '@vulcan/build/schema-parser';
import { ValidatorLoader, TYPES as CORE_TYPES } from '@vulcan/core';
import { IValidatorLoader, TYPES as CORE_TYPES } from '@vulcan/core';
import { Container } from 'inversify';
import * as sinon from 'ts-sinon';

let container: Container;
let stubSchemaReader: sinon.StubbedInstance<SchemaReader>;
let stubValidatorLoader: sinon.StubbedInstance<ValidatorLoader>;
let stubValidatorLoader: sinon.StubbedInstance<IValidatorLoader>;

beforeEach(() => {
container = new Container();
stubSchemaReader = sinon.stubInterface<SchemaReader>();
stubValidatorLoader = sinon.stubInterface<ValidatorLoader>();
stubValidatorLoader = sinon.stubInterface<IValidatorLoader>();

container
.bind(TYPES.Factory_SchemaReader)
Expand All @@ -33,7 +33,7 @@ beforeEach(() => {
.to(SchemaParserOptions)
.inSingletonScope();
container
.bind(CORE_TYPES.ValidatorLoader)
.bind(CORE_TYPES.IValidatorLoader)
.toConstantValue(stubValidatorLoader);
container.bind(TYPES.SchemaParser).to(SchemaParser).inSingletonScope();
});
Expand All @@ -59,10 +59,10 @@ request:
};
};
stubSchemaReader.readSchema.returns(generator());
stubValidatorLoader.getLoader.returns({
stubValidatorLoader.load.resolves({
name: 'validator1',
validateSchema: () => true,
validateData: () => true,
validateSchema: () => null,
validateData: () => null,
});
const schemaParser = container.get<SchemaParser>(TYPES.SchemaParser);

Expand Down
2 changes: 1 addition & 1 deletion packages/build/test/spec-generator/oas3.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { OAS3SpecGenerator } from '@vulcan/build/spec-generator';
import { getSchemas, getConfig } from './schema';
import * as jsYaml from 'js-yaml';
import * as fs from 'fs/promises';
import { promises as fs } from 'fs';
import * as path from 'path';
import { get } from 'lodash';

Expand Down
Loading