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: Syntax sugars for parameter validators #108

Merged
merged 12 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
VulcanInternalExtension,
DocumentSpec,
ConfigurationError,
TypeConstraint,
} from '@vulcan-sql/core';
import { isEmpty } from 'lodash';

Expand Down Expand Up @@ -121,8 +122,10 @@ export class OAS3SpecGenerator extends SpecGenerator<oas3.OpenAPIObject> {
for (const constraint of parameter.constraints) {
if (constraint instanceof MinValueConstraint) {
schema.minimum = constraint.getMinValue();
schema.exclusiveMinimum = constraint.isExclusive();
} else if (constraint instanceof MaxValueConstraint) {
schema.maximum = constraint.getMaxValue();
schema.exclusiveMaximum = constraint.isExclusive();
} else if (constraint instanceof MinLengthConstraint) {
schema.minLength = constraint.getMinLength();
} else if (constraint instanceof MaxLengthConstraint) {
Expand All @@ -131,6 +134,8 @@ export class OAS3SpecGenerator extends SpecGenerator<oas3.OpenAPIObject> {
schema.pattern = constraint.getRegex();
} else if (constraint instanceof EnumConstraint) {
schema.enum = constraint.getList();
} else if (constraint instanceof TypeConstraint) {
schema.type = constraint.getType();
}
}

Expand Down
21 changes: 12 additions & 9 deletions packages/build/src/lib/schema-parser/middleware/addParameter.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { FieldInType } from '@vulcan-sql/core';
import { FieldInType, ValidatorDefinition } from '@vulcan-sql/core';
import { RawAPISchema, SchemaParserMiddleware } from './middleware';

interface Parameter {
name: string;
lineNo: number;
columnNo: number;
locations: { lineNo: number; columnNo: number }[];
validators: ValidatorDefinition[];
}

export class AddParameter extends SchemaParserMiddleware {
Expand All @@ -20,15 +20,18 @@ export class AddParameter extends SchemaParserMiddleware {
parameters.forEach((parameter) => {
// We only check the first value of nested parameters
const name = parameter.name.split('.')[0];
if (
!schemas.request!.some(
(paramInSchema) => paramInSchema.fieldName === name
)
) {
const existedParam = schemas.request!.find(
(paramInSchema) => paramInSchema.fieldName === name
);

if (existedParam) {
if (!existedParam.validators) existedParam.validators = [];
existedParam.validators.push(...(parameter.validators || []));
Copy link
Contributor

Choose a reason for hiding this comment

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

If there are the same name validators in metadata and schema requests, will we exclude one of them?

Because we may face some cases which the args are different in schema request and SQL syntax validation filter:

  • schema request has integer validator with args min=10, and max=20
  • metadata exists in the validator with args min=5, and max=90 which parsed from SQL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed all validators to the list because I didn't know how to compose all of the validators, e.g.
string(format="\d+")
string(format="[1-5]+")

So I prefer to execute all of these validators.

P.S. Our constraints have composed logic so that we won't duplicate them.

Copy link
Contributor

@kokokuo kokokuo Oct 12, 2022

Choose a reason for hiding this comment

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

Thanks so much

} else {
schemas.request!.push({
fieldName: name,
fieldIn: FieldInType.QUERY,
validators: [],
validators: parameter.validators || [],
});
}
});
Expand Down
38 changes: 38 additions & 0 deletions packages/build/test/schema-parser/middleware/addParameter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ it(`Should add query parameter automatically when some parameters haven't be def
fieldName: 'param1',
fieldIn: FieldInType.PATH,
description: 'origin',
validators: [],
});
expect(schema.request![1]).toEqual({
fieldName: 'param2',
Expand Down Expand Up @@ -131,3 +132,40 @@ it('Should tolerate empty request and add fallback value for it', async () => {
// Assert
expect(schema.request).toEqual([]);
});

it(`Should merge validators when some parameters are defined in both schema and metadata`, async () => {
// Arrange
const schema: RawAPISchema = {
sourceName: 'some-name',
templateSource: 'some-name',
request: [
{
fieldName: 'param1',
fieldIn: FieldInType.PATH,
description: 'origin',
validators: [{ name: 'required' }],
},
],
metadata: {
'parameter.vulcan.com': [
{
name: 'param1',
locations: [],
validators: [{ name: 'integer', args: { min: 3 } }],
},
],
errors: [],
},
};
const addParameter = new AddParameter();
// Act
await addParameter.handle(schema, async () => Promise.resolve());

// Assert
expect(schema.request![0]).toEqual({
fieldName: 'param1',
fieldIn: FieldInType.PATH,
description: 'origin',
validators: [{ name: 'required' }, { name: 'integer', args: { min: 3 } }],
});
});
26 changes: 15 additions & 11 deletions packages/core/src/containers/modules/templateEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import {
NunjucksCompiler,
Compiler,
TemplateEngine,
BaseCompilerEnvironment,
RuntimeCompilerEnvironment,
BuildTimeCompilerEnvironment,
} from '@vulcan-sql/core/template-engine';
import { AsyncContainerModule, interfaces } from 'inversify';
import { TemplateEngineOptions } from '../../options';
import * as nunjucks from 'nunjucks';

export const templateEngineModule = (options: ITemplateEngineOptions = {}) =>
new AsyncContainerModule(async (bind) => {
Expand Down Expand Up @@ -40,22 +42,24 @@ export const templateEngineModule = (options: ITemplateEngineOptions = {}) =>
.inSingletonScope();
}

// Compiler environment
bind<nunjucks.Environment>(TYPES.CompilerEnvironment)
// Compiler environment, we need to initialize them manually because they extends some old js libraries.
bind<BaseCompilerEnvironment>(TYPES.CompilerEnvironment)
.toDynamicValue((context) => {
// We only need loader in runtime
const codeLoader = context.container.get<CodeLoader>(
TYPES.CompilerLoader
return new RuntimeCompilerEnvironment(
context.container.get(TYPES.CompilerLoader),
context.container.getAll(TYPES.Extension_TemplateEngine),
context.container.get(TYPES.ValidatorLoader)
);

return new nunjucks.Environment(codeLoader);
})
.inSingletonScope()
.whenTargetNamed('runtime');

bind<nunjucks.Environment>(TYPES.CompilerEnvironment)
.toDynamicValue(() => {
return new nunjucks.Environment();
bind<BaseCompilerEnvironment>(TYPES.CompilerEnvironment)
.toDynamicValue((context) => {
return new BuildTimeCompilerEnvironment(
context.container.getAll(TYPES.Extension_TemplateEngine),
context.container.get(TYPES.ValidatorLoader)
);
})
.inSingletonScope()
.whenTargetNamed('compileTime');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,28 @@
import { TYPES } from '@vulcan-sql/core/types';
import { inject, named } from 'inversify';
import { FILTER_METADATA_NAME } from './constants';
import * as nunjucks from 'nunjucks';
import {
CompileTimeExtension,
VulcanInternalExtension,
} from '@vulcan-sql/core/models';
import { BaseCompilerEnvironment } from '../../compiler-environment';

@VulcanInternalExtension()
export class FilterChecker extends CompileTimeExtension {
public override metadataName = FILTER_METADATA_NAME;
private env: nunjucks.Environment;
private filters = new Set<string>();

constructor(
@inject(TYPES.ExtensionConfig)
config: any,
@inject(TYPES.ExtensionName)
name: string,
@inject(TYPES.CompilerEnvironment)
@named('compileTime')
compileTimeEnv: nunjucks.Environment
public override onVisit(
node: nunjucks.nodes.Node,
env: BaseCompilerEnvironment
) {
super(config, name);
this.env = compileTimeEnv;
}

public override onVisit(node: nunjucks.nodes.Node) {
if (node instanceof nunjucks.nodes.Filter) {
if (
node.name instanceof nunjucks.nodes.Symbol ||
node.name instanceof nunjucks.nodes.Literal
) {
// If the node is a filter and has a expected name node, we check whether the filter is loaded.
// If the filter is not loaded, getFilter function will throw an error.
this.env.getFilter(node.name.value);
env.getFilter(node.name.value);
this.filters.add(node.name.value);
}
}
Expand Down
Loading