Skip to content

Commit

Permalink
refactor: introduces MessageCarrier to simplify adding warnings and e…
Browse files Browse the repository at this point in the history
…rrors

- introduced MessageCarrier class to centralize the logic/checks for adding warnings and errors.
- introduced an instance of this class in each semantic validator to provide uniform warning and error reporting
- added unit tests for the MessageCarrier class
  • Loading branch information
Barrett Schonefeld committed Feb 21, 2020
1 parent 66f22bf commit 66c4ab7
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 168 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ The default values for each rule are described below.
| array_of_arrays | warning |
| property_case_convention | error, lower_snake_case |
| enum_case_convention | error, lower_snake_case |
| json_or_param_binary_string | warning |

###### walker
| Rule | Default |
Expand Down
3 changes: 1 addition & 2 deletions src/.defaultsForValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ const defaults = {
'description_mentions_json': 'warning',
'array_of_arrays': 'warning',
'property_case_convention': [ 'error', 'lower_snake_case'],
'enum_case_convention': [ 'error', 'lower_snake_case'],
'binary_string': 'warning'
'enum_case_convention': [ 'error', 'lower_snake_case']
},
'walker': {
'no_empty_descriptions': 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
}

// Assertation 3
const binaryStringStatus = configSchemas.binary_string;
const binaryStringStatus = configSchemas.json_or_param_binary_string;
if (binaryStringStatus !== 'off') {
for (const mimeType of requestBodyMimeTypes) {
if (mimeType === 'application/json') {
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/validation/oas3/semantic-validators/parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ module.exports.validate = function({ jsSpec }, config) {
const configSchemas = config.schemas;
config = config.parameters;

walk(resolvedSpec, [], function(obj, path) {
walk(jsSpec, [], function(obj, path) {
const isContentsOfParameterObject = isParameterObject(path, true); // 2nd arg is isOAS3
const isRef = !!obj.$ref;

// obj is a parameter object
if (isContentsOfParameterObject) {
if (isContentsOfParameterObject && !isRef) {
const allowedInValues = ['query', 'header', 'path', 'cookie'];
if (!obj.in) {
// bad because in is required
Expand Down
81 changes: 1 addition & 80 deletions test/plugins/validation/oas3/operations.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,10 @@ const expect = require('expect');
const {
validate
} = require('../../../../src/plugins/validation/oas3/semantic-validators/operations');
const config = require('../../../../src/.defaultsForValidator').defaults.oas3;

describe('validation plugin - semantic - operations - oas3', function() {
it('should complain about a request body not having a content field', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand All @@ -39,16 +30,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should warn about an operation with a non-form, array schema request body that does not set a name', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -83,16 +64,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not warn about an operation with a non-array json request body that does not set a name', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -120,16 +91,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not warn about an operation with a non-form request body that sets a name', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -158,16 +119,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not warn about an operation with a form request body that does not set a name', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -200,16 +151,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not warn about an operation with a referenced request body that does not set a name', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const resolvedSpec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -250,16 +191,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not crash in request body name check when path name contains a period', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/other.pets': {
Expand Down Expand Up @@ -294,16 +225,6 @@ describe('validation plugin - semantic - operations - oas3', function() {
});

it('should not crash when request body is behind a ref', function() {
const config = {
operations: {
no_request_body_content: 'error',
no_request_body_name: 'warning'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const jsSpec = {
paths: {
'/resource': {
Expand Down
90 changes: 8 additions & 82 deletions test/plugins/validation/oas3/parameters.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,11 @@
const expect = require('expect');

const {
validate
} = require('../../../../src/plugins/validation/oas3/semantic-validators/parameters');
const config = require('../../../../src/.defaultsForValidator').defaults.oas3;

describe('validation plugin - semantic - parameters - oas3', function() {
it('should not complain when parameter is valid', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -51,24 +39,12 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(0);
expect(res.warnings.length).toEqual(0);
});

it('should complain when `in` is missing', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -101,7 +77,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'paths',
Expand All @@ -117,18 +93,6 @@ describe('validation plugin - semantic - parameters - oas3', function() {
});

it('should complain when `in` is an invalid value', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -162,7 +126,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'paths',
Expand All @@ -179,18 +143,6 @@ describe('validation plugin - semantic - parameters - oas3', function() {
});

it('should complain when the parameter has an undescribed data type', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
paths: {
'/pets': {
Expand Down Expand Up @@ -221,7 +173,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'paths',
Expand All @@ -237,18 +189,6 @@ describe('validation plugin - semantic - parameters - oas3', function() {
});

it('should complain when a parameter describes data type with both `schema` and `content`', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
components: {
parameters: {
Expand All @@ -271,7 +211,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual([
'components',
Expand Down Expand Up @@ -475,7 +415,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const res = validate({ resolvedSpec: spec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(0);
expect(res.warnings.length).toEqual(0);
});
Expand Down Expand Up @@ -527,18 +467,6 @@ describe('validation plugin - semantic - parameters - oas3', function() {
});

it('should not complain about a schema property named `parameters`', function() {
const config = {
parameters: {
no_in_property: 'error',
invalid_in_property: 'error',
missing_schema_or_content: 'error',
has_schema_and_content: 'error'
},
schemas: {
json_or_param_binary_string: 'warning'
}
};

const spec = {
components: {
schemas: {
Expand All @@ -557,9 +485,7 @@ describe('validation plugin - semantic - parameters - oas3', function() {
}
};

const resolvedSpec = await resolver.dereference(spec);

const res = validate({ resolvedSpec: resolvedSpec }, config);
const res = validate({ jsSpec: spec }, config);
expect(res.errors.length).toEqual(0);
expect(res.warnings.length).toEqual(0);
});
Expand Down

0 comments on commit 66c4ab7

Please sign in to comment.