Skip to content

Commit

Permalink
feat: list valid types and formats in error messages (#145)
Browse files Browse the repository at this point in the history 
Changes the way problems with type/format values are handled so that messages are more descriptive about what is actually wrong
  • Loading branch information
@barrett-schonefeld
barrett-schonefeld authored Feb 25, 2020
1 parent b33875a commit 600bf8e
Show file tree
Hide file tree
Showing 4 changed files with 385 additions and 169 deletions.
57 changes: 0 additions & 57 deletions src/plugins/validation/2and3/semantic-validators/parameters-ibm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// Header parameters must not define a content-type or an accept-type.
// http://watson-developer-cloud.github.io/api-guidelines/swagger-coding-style#do-not-explicitly-define-a-content-type-header-parameter

const pick = require('lodash/pick');
const includes = require('lodash/includes');
const { checkCase, isParameterObject, walk } = require('../../../utils');
const MessageCarrier = require('../../../utils/messageCarrier');

Expand Down Expand Up @@ -107,18 +105,6 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
}
}

const checkStatus = config.invalid_type_format_pair;
if (checkStatus !== 'off') {
const valid = formatValid(obj, isOAS3);
if (!valid) {
messages.addMessage(
path,
'Parameter type+format is not well-defined.',
checkStatus
);
}
}

const isParameterRequired = obj.required;
let isDefaultDefined;
if (isOAS3) {
Expand All @@ -139,46 +125,3 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {

return messages;
};

function formatValid(obj, isOAS3) {
// References will be checked when the parameters / definitions / components are scanned.
if (obj.$ref || (obj.schema && obj.schema.$ref)) {
return true;
}
const schema = obj.schema || pick(obj, ['type', 'format', 'items']);
if (!schema.type) {
return false;
}
switch (schema.type) {
case 'integer':
return (
!schema.format ||
includes(['int32', 'int64'], schema.format.toLowerCase())
);
case 'number':
return (
!schema.format ||
includes(['float', 'double'], schema.format.toLowerCase())
);
case 'string':
return (
!schema.format ||
includes(
['byte', 'date', 'date-time', 'password'],
schema.format.toLowerCase()
)
);
case 'boolean':
return schema.format === undefined; // No valid formats for boolean -- should be omitted
case 'array':
if (!schema.items) {
return false;
}
return formatValid(schema.items);
case 'object':
return true; // TODO: validate nested schemas
case 'file':
return !isOAS3 && obj.in === 'formData';
}
return false;
}
162 changes: 131 additions & 31 deletions src/plugins/validation/2and3/semantic-validators/schema-ibm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
This includes responses, request bodies, parameters (with content rather than schema),
both at the operation level and within the top-level "components" object
*/

const modelLocations = ['definitions', 'schemas', 'properties'];
const modelLocations = [
'definitions',
'schemas',
'properties',
'parameters'
];
if (
current === 'schema' ||
current === 'items' ||
Expand Down Expand Up @@ -158,60 +162,156 @@ function generateFormatErrors(schema, contextPath, config, isOAS3, messages) {
}

checkStatus = config.invalid_type_format_pair;
if (checkStatus !== 'off' && !formatValid(schema, contextPath, isOAS3)) {
if (checkStatus !== 'off') {
typeFormatErrors(schema, contextPath, isOAS3, messages, checkStatus);
}
}

function typeFormatErrors(obj, path, isOAS3, messages, checkStatus) {
// error if format defined but not type
if (!obj.type && obj.format) {
messages.addMessage(
contextPath.concat(['type']),
'Property type+format is not well-defined.',
path.concat(['format']),
'Format defined without a type.',
checkStatus
);
}
}

function formatValid(property, path, isOAS3) {
if (property.$ref) {
return true;
// we will check ref in defintions section
// only proceed if type defined
if (obj.$ref || !obj.type) {
return;
}
// if type is not present, skip validation of format
if (!property.type) {
return true;

const validIntegerFormats = ['int32', 'int64'];
const validNumberFormats = ['float', 'double'];
const validStringFormats = [
'byte',
'binary',
'date',
'date-time',
'password'
];
const validTypes = [
'integer',
'number',
'string',
'boolean',
'object',
'array'
];
if (!isOAS3) {
validTypes.push('file');
}
let valid = true;
switch (property.type) {
switch (obj.type) {
case 'integer':
valid =
!property.format ||
includes(['int32', 'int64'], property.format.toLowerCase());
if (
obj.format &&
!includes(validIntegerFormats, obj.format.toLowerCase())
) {
messages.addMessage(
path.concat(['type']),
`Schema of type integer should use one of the following formats: ${validIntegerFormats.join(
', '
)}.`,
checkStatus
);
}
break;
case 'number':
valid =
!property.format ||
includes(['float', 'double'], property.format.toLowerCase());
if (
obj.format &&
!includes(validNumberFormats, obj.format.toLowerCase())
) {
messages.addMessage(
path.concat(['type']),
`Schema of type number should use one of the following formats: ${validNumberFormats.join(
', '
)}.`,
checkStatus
);
}
break;
case 'string':
valid =
!property.format ||
includes(
['byte', 'binary', 'date', 'date-time', 'password'],
property.format.toLowerCase()
if (
obj.format &&
!includes(validStringFormats, obj.format.toLowerCase())
) {
messages.addMessage(
path.concat(['type']),
`Schema of type string should use one of the following formats: ${validStringFormats.join(
', '
)}.`,
checkStatus
);
}
break;
case 'boolean':
valid = property.format === undefined; // No valid formats for boolean -- should be omitted
// No valid formats for boolean, format should be undefined
if (obj.format !== undefined) {
messages.addMessage(
path.concat(['type']),
`Schema of type boolean should not have a format.`,
checkStatus
);
}
break;
case 'object':
// No valid format pairings for object, format should be undefined
if (obj.format !== undefined) {
messages.addMessage(
path.concat(['type']),
'Schema of type object should not have a format.',
checkStatus
);
}
break;
case 'array':
valid = true;
// No valid format pairings for array, format should be undefined
if (obj.format !== undefined) {
messages.addMessage(
path.concat(['type']),
'Schema of type array should not have a format.',
checkStatus
);
}
break;
case 'file':
// schemas of type file are allowed in swagger2 for responses and parameters
// of type 'formData' - the violating parameters are caught by parameters-ibm
// of type 'formData'
// note: type file is only allowed for root schemas (not properties, etc.)
valid = !isOAS3 && isRootSchema(path);
if (isOAS3 || (!obj.in && !isRootSchema(path))) {
messages.addMessage(
path.concat(['type']),
'File type only valid for swagger2 and must be used as root schema.',
checkStatus
);
} else if (obj.in && obj.in !== 'formData') {
messages.addMessage(
path.concat(['type']),
'File type parameter must use in: formData.',
checkStatus
);
}
// Format should not be defined for schema of type file.
// Error reported in addition to potential error above.
// Only check for swagger2 because type file should not be used in OAS3.
if (!isOAS3 && obj.format !== undefined) {
messages.addMessage(
path.concat(['type']),
'Schema of type file should not have a format.',
checkStatus
);
}
break;
default:
valid = false;
// invalid type
messages.addMessage(
path.concat(['type']),
`Invalid type. Valid types are: ${validTypes.join(', ')}.`,
checkStatus
);
}
return valid;
}

// http://watson-developer-cloud.github.io/api-guidelines/swagger-coding-style#models
Expand Down
70 changes: 0 additions & 70 deletions test/plugins/validation/2and3/parameters-ibm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,40 +119,6 @@ describe('validation plugin - semantic - parameters-ibm', () => {
expect(res.warnings.length).toEqual(0);
});

it('should return an error when type does not match format', () => {
const spec = {
paths: {
'/pets': {
get: {
parameters: [
{
name: 'good_name',
in: 'query',
description: 'This is a good description.',
type: 'number',
format: 'int32'
}
]
}
}
}
};

const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'paths',
'/pets',
'get',
'parameters',
'0'
]);
expect(res.errors[0].message).toEqual(
'Parameter type+format is not well-defined.'
);
expect(res.warnings.length).toEqual(0);
});

it('should not validate within extensions', () => {
const spec = {
paths: {
Expand Down Expand Up @@ -486,42 +452,6 @@ describe('validation plugin - semantic - parameters-ibm', () => {
);
});

it('should return an error when parameter type+format is not well-defined', () => {
const spec = {
paths: {
'/pets': {
get: {
parameters: [
{
name: 'good_name',
in: 'query',
description: 'This is a good description.',
schema: {
type: 'number',
format: 'int32'
}
}
]
}
}
}
};

const res = validate({ jsSpec: spec, isOAS3: true }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'paths',
'/pets',
'get',
'parameters',
'0'
]);
expect(res.errors[0].message).toEqual(
'Parameter type+format is not well-defined.'
);
expect(res.warnings.length).toEqual(0);
});

it('should flag a required parameter that specifies a default value', () => {
const spec = {
paths: {
Expand Down
Loading

0 comments on commit 600bf8e

Please sign in to comment.