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.

Sign up for GitHub

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

Jump to bottom

Update spec to OpenAPI 3.0.1 #101

Merged
merged 1 commit into from
Apr 30, 2019
Merged

Update spec to OpenAPI 3.0.1 #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ language: node_js
node_js:
- "6"
- "10"
- "node"

sudo: false

Expand Down
137 changes: 72 additions & 65 deletions lib/filters/validator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const inMapping = {
path: 'params',
query: 'query',
header: 'headers',
formData: 'body',
body: 'body'
};

Expand Down Expand Up @@ -43,44 +42,47 @@ const supportedValidators = ['maximum',
* @constructor
*/
class Validator {
constructor(parameters, definitions) {
// Does not support validating parameters or requestBodies in components
constructor(parameters = [], requestBody = {}, schemas) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The name 'requestBody' is a little confusing. It's the req body spec, not the body itself right? Can we make it less confusing?

Copy link
Contributor

Choose a reason for hiding this comment

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

So, here we assume that only 'schemas' out of components will be used in parameter definitions (like we do now in RESTBase) for example. Starting to use parameters components seem nice, we have a lot of shared parameters like title or accept between endpoitns, so abstracting that out would be neat. However, these 2 PRs are already enormous, so we can followup to do that in the next iteration. For now, would you mind adding a comment that that is not supported?

Copy link
Member Author

@clarakosi clarakosi Apr 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

IDK requestBody is what is used in the swagger docs and spec so it seems appropriate to follow that name.

I did make a note that parameters and requestBodies are not currently supported in components.

this._ajv = constructAjv({ verbose: true });
if (definitions) {
Object.keys(definitions).forEach((schemaName) => {
if (schemas) {
Object.keys(schemas).forEach((schemaName) => {
this._ajv.addSchema(
definitions[schemaName],
`#/definitions/${schemaName}`
schemas[schemaName],
`#/components/schemas/${schemaName}`
);
});
}

this._paramCoercionFunc = this._createTypeCoercionFunc(parameters.filter((p) => {
return p.in !== 'formData' && p.in !== 'body' && p.type !== 'string';
return p.schema.type !== 'string';
}));
const bodyParams = parameters.filter((p) => p.in === 'body');
if (bodyParams.length > 1) {
throw new Error('Only a single body parameter allowed');
} else if (bodyParams.length) {
// Have a body parameter, special-case coercion to support formData and JSON
const bodyParam = bodyParams[0];
if (bodyParam.schema && bodyParam.schema.properties) {

if (requestBody.content) {
// Have a body parameter, special-case coercion to support form-data and JSON
const reqContent = requestBody.content;

// Only supports one possible content-type of the body
const bodyType = Object.keys(reqContent)[0];
clarakosi marked this conversation as resolved.
Show resolved Hide resolved

if (reqContent[bodyType].schema && reqContent[bodyType].schema.properties) {
this._bodyCoercionFunc = this._createTypeCoercionFunc(
Object.keys(bodyParam.schema.properties).map((prop) => {
return {
name: prop,
in: 'body',
type: bodyParam.schema.properties[prop].type,
required: bodyParam.schema.required &&
bodyParam.schema.required.indexOf(prop) !== -1
};
}));
Object.keys(reqContent[bodyType].schema.properties)
.map((prop) => {
return {
name: prop,
in: 'body',
schema: {
type: reqContent[bodyType].schema.properties[prop].type
},
required: reqContent[bodyType].schema.required &&
reqContent[bodyType].schema.required.indexOf(prop) !== -1
};
})
);
}
} else {
this._bodyCoercionFunc = this._createTypeCoercionFunc(parameters.filter((p) => {
return p.in === 'formData' && p.type !== 'string';
}));
}
this._validatorFunc = this._ajv.compile(this._convertToJsonSchema(parameters));
this._validatorFunc = this._ajv.compile(this._convertToJsonSchema(parameters, requestBody));
}

/**
Expand Down Expand Up @@ -124,53 +126,57 @@ class Validator {
* Converts a list of parameters from a swagger spec
* to JSON-schema for a request
* @param {Array} parameters list of params
* @param {Object} requestBody schema
* @return {Object} JSON schema
* @private
*/
Validator.prototype._convertToJsonSchema = function (parameters) {
Validator.prototype._convertToJsonSchema = function (parameters, requestBody) {
const schema = {
type: 'object',
properties: {}
};

parameters.forEach((param) => {
if (param.in !== 'body') {
if (!schema.properties[inMapping[param.in]]) {
schema.properties[inMapping[param.in]] = {
type: 'object',
properties: {}
};
// 'required' array must have at least one element according to json-schema spec,
// se we can't preinitialize it.
schema.required = schema.required || [];
schema.required.push(inMapping[param.in]);
}
if (!schema.properties[inMapping[param.in]]) {
schema.properties[inMapping[param.in]] = {
type: 'object',
properties: {}
};
// 'required' array must have at least one element according to json-schema spec,
// se we can't preinitialize it.
schema.required = schema.required || [];
schema.required.push(inMapping[param.in]);
}

const reqPartSchema = schema.properties[inMapping[param.in]];
const paramSchema = { type: param.type };
supportedValidators.forEach((validator) => {
paramSchema[validator] = param[validator];
});
reqPartSchema.properties[param.name] = paramSchema;
if (param.required) {
reqPartSchema.required = reqPartSchema.required || [];
reqPartSchema.required.push(param.name);
}
} else {
if (param.schema) {
schema.properties.body = param.schema;
} else {
schema.properties.body = {
type: 'object'
};
}
if (param.required) {
schema.required = schema.required || [];
schema.required.push('body');
}
const reqPartSchema = schema.properties[inMapping[param.in]];
const paramSchema = { type: param.schema.type };

supportedValidators.forEach((validator) => {
paramSchema[validator] = param.schema[validator];
});
reqPartSchema.properties[param.name] = paramSchema;
if (param.required) {
reqPartSchema.required = reqPartSchema.required || [];
reqPartSchema.required.push(param.name);
}
});

if (requestBody.content) {
const bodyType = Object.keys(requestBody.content)[0];

if (requestBody.content[bodyType].schema) {
schema.properties.body = requestBody.content[bodyType].schema;
} else {
schema.properties.body = {
type: 'object'
};
}
if (requestBody.required) {
schema.required = schema.required || [];
schema.required.push('body');
}
}

return schema;
};

Expand All @@ -187,7 +193,7 @@ Validator.prototype._createTypeCoercionFunc = function (parameters) {
const paramAccessor = `req.${inMapping[param.in]}["${param.name}"]`;
let paramCoercionCode = '';
let errorNotifier;
switch (param.type) {
switch (param.schema.type) {
case 'integer':
errorNotifier = `${'throw new HTTPError({status:400,body:{type:"bad_request",' +
' title:"Invalid parameters", detail: "data.'}${
Expand Down Expand Up @@ -249,14 +255,15 @@ Validator.prototype._createTypeCoercionFunc = function (parameters) {
const CACHE = new Map();

module.exports = (hyper, req, next, options, specInfo) => {
if (specInfo && specInfo.spec && specInfo.spec.parameters) {
if (specInfo && specInfo.spec && (specInfo.spec.parameters || specInfo.spec.requestBody)) {
const cachedValidator = CACHE.get(specInfo.spec);
if (cachedValidator) {
cachedValidator.validate(req);
} else {
const validator = new Validator(
specInfo.spec.parameters,
specInfo.specRoot && specInfo.specRoot.definitions
specInfo.spec.requestBody,
specInfo.specRoot.components && specInfo.specRoot.components.schemas
);
CACHE.set(specInfo.spec, validator);
validator.validate(req);
Expand Down
2 changes: 1 addition & 1 deletion lib/hyperswitch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ HyperSwitch.prototype.defaultListingHandler = function (match, hyper, req) {
status: 200,
body: Object.assign({}, match.value.specRoot, {
// Set the base path dynamically
basePath: getDocBasePath(req, match.value.specRoot)
clarakosi marked this conversation as resolved.
Show resolved Hide resolved
servers: [{ url: getDocBasePath(req, match.value.specRoot) }]
})
});
} else if (rq.path ||
Expand Down
19 changes: 9 additions & 10 deletions lib/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const fs = P.promisifyAll(require('fs'));
const handlerTemplate = require('./handlerTemplate');
const swaggerRouter = require('swagger-router');
const path = require('path');
const utils = require('./utils');

const Node = swaggerRouter.Node;
const Template = swaggerRouter.Template;
Expand Down Expand Up @@ -440,7 +441,7 @@ class Router {
subtree = new Node();
// Set up our specific value object
subtree.value = value;
value.path = childScope.specRoot.basePath + childScope.prefixPath;
value.path = childScope.specRoot.servers[0].url + childScope.prefixPath;
value.methods = {};
// XXX: Set ACLs and other value properties for path
// subtree.value.acls = ...;
Expand Down Expand Up @@ -474,7 +475,7 @@ class Router {
subtree.value = value;
// Copy over the remaining value properties.
Object.assign(subtree.value, origSubtree.value);
subtree.value.path = childScope.specRoot.basePath + childScope.prefixPath;
subtree.value.path = childScope.specRoot.servers[0].url + childScope.prefixPath;
specPromise = P.resolve();
}
branchNode.setChild(path[path.length - 1], subtree);
Expand Down Expand Up @@ -505,11 +506,10 @@ class Router {
scope.rootScope = scope;
}

// Merge in definitions & securityDefinitions from the spec.
// Merge in components from the spec.
// TODO: Do we need a clone here? Is it okay if those definitions are
// added to the higher level spec?
Object.assign(scope.specRoot.definitions, spec.definitions);
Object.assign(scope.specRoot.securityDefinitions, spec.securityDefinitions);
utils.mergeDeep(scope.specRoot.components, spec.components);
scope.specRoot.tags = scope.specRoot.tags.concat(spec.tags || [])
.filter((tag, index, self) => {
return index === self.findIndex((t) => {
Expand Down Expand Up @@ -637,22 +637,21 @@ class Router {
Router.prototype._createNewApiRoot = function (node, spec, scope) {
const specRoot = Object.assign({}, spec);
// Make sure the spec has the standard properties set up.
specRoot.swagger = spec.swagger || '2.0';
specRoot.definitions = spec.definitions || {};
specRoot.securityDefinitions = spec.securityDefinitions || {};
specRoot.openapi = spec.openapi || '3.0.1';
specRoot.components = spec.components || {};
specRoot['x-default-params'] = spec['x-default-params'] || {};
specRoot.tags = spec.tags || [];

delete specRoot['x-route-filters'];

// Reset paths. These are going to be built up during path setup.
specRoot.paths = {};
specRoot.basePath = scope.prefixPath;
specRoot.servers = [{ url: scope.prefixPath }];

node.setChild({ type: 'meta', name: 'apiRoot' }, new Node({
specRoot,
methods: {},
path: `${specRoot.basePath}/`,
path: `${specRoot.servers[0].url}/`,
globals: node.value && node.value.globals || scope.globals
}));

Expand Down
34 changes: 34 additions & 0 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,38 @@ class NullLogger {
}
utils.nullLogger = new NullLogger();

/**
* Deep merge two objects.
* @param {Object} target
* @param {Object} ...sources
* @return {Object}
*/
utils.mergeDeep = (target, ...sources) => {
const isObject = (item) => {
return (item && typeof item === 'object' && !Array.isArray(item));
};

if (!sources.length || !sources || !target || !isObject(target)) {
return target;
}

sources.forEach((source) => {
if (isObject(target) && isObject(source)) {
clarakosi marked this conversation as resolved.
Show resolved Hide resolved
Object.keys(source).forEach((key) => {
if (isObject(source[key])) {
if (!target[key]) {
target[key] = {};
}
utils.mergeDeep(target[key], source[key]);
} else if (isObject(target[key]) && !isObject(source[key])) {
throw new Error("Only objects of the same 'shape' are supported");
} else {
target[key] = source[key];
}
});
}
});
return target;
};

module.exports = utils;
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "hyperswitch",
"version": "0.11.1",
"version": "0.12.0",
clarakosi marked this conversation as resolved.
Show resolved Hide resolved
"description": "REST API creation framework",
"main": "index.js",
"scripts": {
Expand Down
Loading