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

chore: improve OAS functions #1362

Merged
merged 2 commits into from
Nov 30, 2020
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
32 changes: 0 additions & 32 deletions src/rulesets/oas/functions/__tests__/oasOpIdUnique.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,6 @@ describe('oasOpIdUnique', () => {
});

expect(results).toEqual([
{
code: 'operation-operationId-unique',
message: 'Every operation must have a unique `operationId`.',
path: ['paths', '/path1', 'get', 'operationId'],
range: {
end: {
character: 28,
line: 4,
},
start: {
character: 23,
line: 4,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: 'operation-operationId-unique',
message: 'Every operation must have a unique `operationId`.',
Expand Down Expand Up @@ -103,22 +87,6 @@ describe('oasOpIdUnique', () => {
});

expect(results).toEqual([
{
code: 'operation-operationId-unique',
message: 'Every operation must have a unique `operationId`.',
path: ['paths', '/path1', 'get', 'operationId'],
range: {
end: {
character: 28,
line: 4,
},
start: {
character: 23,
line: 4,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: 'operation-operationId-unique',
message: 'Every operation must have a unique `operationId`.',
Expand Down
12 changes: 4 additions & 8 deletions src/rulesets/oas/functions/oasExample.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import type { IFunction, IFunctionContext, IFunctionResult } from '../../../types';
import type { Dictionary } from '@stoplight/types';
import type { IFunction, IFunctionContext, IFunctionResult, JSONSchema } from '../../../types';
import type { ISchemaOptions } from '../../../functions/schema';

function isObject(value: unknown): value is Dictionary<any> {
return value !== null && typeof value === 'object';
}
import { isObject } from './utils/isObject';

interface IOasExampleOptions {
oasVersion: 2 | 3;
Expand Down Expand Up @@ -69,7 +65,7 @@ export const oasExample: IFunction<IOasExampleOptions> = function (
}

const schemaOpts: ISchemaOptions = {
schema: opts.schemaField === '$' ? targetVal : targetVal[opts.schemaField],
schema: opts.schemaField === '$' ? targetVal : (targetVal[opts.schemaField] as JSONSchema),
oasVersion: opts.oasVersion,
};

Expand Down Expand Up @@ -104,7 +100,7 @@ export const oasExample: IFunction<IOasExampleOptions> = function (

const result = this.functions.schema.call(
this,
keyed ? exampleValue.value : exampleValue,
keyed && isObject(exampleValue) ? exampleValue.value : exampleValue,
schemaOpts,
{
given: paths.given,
Expand Down
26 changes: 15 additions & 11 deletions src/rulesets/oas/functions/oasOpFormDataConsumeCheck.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
import type { IFunction, IFunctionResult } from '../../../types';
import type { IFunction } from '../../../types';

const validConsumeValue = /(application\/x-www-form-urlencoded|multipart\/form-data)/;

export const oasOpFormDataConsumeCheck: IFunction = targetVal => {
const results: IFunctionResult[] = [];
const parameters: unknown = targetVal.parameters;
const consumes: unknown = targetVal.consumes;

const parameters = targetVal.parameters;
const consumes = targetVal.consumes || [];
if (!Array.isArray(parameters) || !Array.isArray(consumes)) {
return;
}

if (parameters?.find((p: any) => p.in === 'formData')) {
if (!consumes.join(',').match(/(application\/x-www-form-urlencoded|multipart\/form-data)/)) {
results.push({
message: 'consumes must include urlencoded, multipart, or formdata media type when using formData parameter',
});
}
if (parameters.some(p => p?.in === 'formData') && !validConsumeValue.test(consumes?.join(','))) {
return [
{
message: 'Consumes must include urlencoded, multipart, or form-data media type when using formData parameter.',
},
];
}

return results;
return;
};

export default oasOpFormDataConsumeCheck;
38 changes: 15 additions & 23 deletions src/rulesets/oas/functions/oasOpIdUnique.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,29 @@
import type { IFunction, IFunctionResult } from '../../../types';
import { getAllOperations } from './utils/getAllOperations';

export const oasOpIdUnique: IFunction = (targetVal, _options, functionPaths) => {
export const oasOpIdUnique: IFunction = targetVal => {
const results: IFunctionResult[] = [];

const { paths = {} } = targetVal;
const { paths } = targetVal;

const ids: any[] = [];
const seenIds: unknown[] = [];

for (const path in paths) {
if (Object.keys(paths[path]).length > 0) {
for (const operation in paths[path]) {
if (operation !== 'parameters') {
const { operationId } = paths[path][operation];

if (operationId) {
ids.push({
path: ['paths', path, operation, 'operationId'],
operationId,
});
}
}
}
for (const { path, operation } of getAllOperations(paths)) {
if (!('operationId' in paths[path][operation])) {
continue;
}
}

ids.forEach(operationId => {
if (ids.filter(id => id.operationId === operationId.operationId).length > 1) {
const { operationId } = paths[path][operation];

if (seenIds.includes(operationId)) {
results.push({
message: 'operationId must be unique',
path: operationId.path || functionPaths.given,
message: 'operationId must be unique.',
path: ['paths', path, operation, 'operationId'],
});
} else {
seenIds.push(operationId);
}
});
}

return results;
};
Expand Down
64 changes: 39 additions & 25 deletions src/rulesets/oas/functions/oasOpSecurityDefined.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
import type { JsonPath } from '@stoplight/types';

const _get = require('lodash/get');
import type { IFunction, IFunctionResult } from '../../../types';
import { getAllOperations } from './utils/getAllOperations';
import { isObject } from './utils/isObject';

import { IFunction, IFunctionResult } from '../../../types';
function _get(value: unknown, path: JsonPath): unknown {
for (const segment of path) {
if (!isObject(value)) {
break;
}

value = value[segment];
}

return value;
}

export const oasOpSecurityDefined: IFunction<{
schemesPath: JsonPath;
Expand All @@ -11,30 +23,32 @@ export const oasOpSecurityDefined: IFunction<{

const { schemesPath } = options;

const { paths = {} } = targetVal;
const schemes = _get(targetVal, schemesPath) || {};
const allDefs = Object.keys(schemes);

for (const path in paths) {
if (Object.keys(paths[path]).length > 0)
for (const operation in paths[path]) {
if (operation !== 'parameters') {
const { security = [] } = paths[path][operation];

for (const index in security) {
if (security[index]) {
const securityKeys = Object.keys(security[index]);

if (securityKeys.length > 0 && !allDefs.includes(securityKeys[0])) {
results.push({
message: 'operation referencing undefined security scheme',
path: ['paths', path, operation, 'security', index],
});
}
}
}
}
const { paths } = targetVal;
const schemes = _get(targetVal, schemesPath);

const allDefs = isObject(schemes) ? Object.keys(schemes) : [];

for (const { path, operation } of getAllOperations(paths)) {
const { security } = paths[path][operation];

if (!Array.isArray(security)) {
continue;
}

for (const [index, value] of security.entries()) {
if (!isObject(value)) {
continue;
}

const securityKeys = Object.keys(value);

if (securityKeys.length > 0 && !allDefs.includes(securityKeys[0])) {
results.push({
message: 'Operation referencing undefined security scheme.',
path: ['paths', path, operation, 'security', index],
});
}
}
}

return results;
Expand Down
22 changes: 13 additions & 9 deletions src/rulesets/oas/functions/oasOpSuccessResponse.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import type { IFunction, IFunctionResult } from '../../../types';
import type { IFunction } from '../../../types';
import { isObject } from './utils/isObject';

export const oasOpSuccessResponse: IFunction = targetVal => {
if (!targetVal) {
if (!isObject(targetVal)) {
return;
}

const results: IFunctionResult[] = [];
const responses = Object.keys(targetVal);
if (responses.filter(response => Number(response) >= 200 && Number(response) < 400).length === 0) {
results.push({
message: 'operations must define at least one 2xx or 3xx response',
});
for (const response of Object.keys(targetVal)) {
if (Number(response) >= 200 && Number(response) < 400) {
return;
}
}
return results;

return [
{
message: 'operations must define at least one 2xx or 3xx response',
},
];
};

export default oasOpSuccessResponse;
47 changes: 27 additions & 20 deletions src/rulesets/oas/functions/oasTagDefined.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,37 @@
// an operation is also present in the global tags array.

import type { IFunction, IFunctionResult } from '../../../types';
import { getAllOperations } from './utils/getAllOperations';
import { isObject } from './utils/isObject';

export const oasTagDefined: IFunction = targetVal => {
const results: IFunctionResult[] = [];

const globalTags = (targetVal.tags || []).map(({ name }: { name: string }) => name);

const { paths = {} } = targetVal;

const validOperationKeys = ['get', 'head', 'post', 'put', 'patch', 'delete', 'options', 'trace'];

for (const path in paths) {
if (Object.keys(paths[path]).length > 0) {
for (const operation in paths[path]) {
if (validOperationKeys.includes(operation)) {
const { tags = [] } = paths[path][operation];
tags.forEach((tag: string, index: number) => {
if (globalTags.indexOf(tag) === -1) {
results.push({
message: 'Operation tags should be defined in global tags.',
path: ['paths', path, operation, 'tags', index],
});
}
});
}
const globalTags: string[] = [];

if (Array.isArray(targetVal.tags)) {
for (const tag of targetVal.tags) {
if (isObject(tag) && typeof tag.name === 'string') {
globalTags.push(tag.name);
}
}
}

const { paths } = targetVal;

for (const { path, operation } of getAllOperations(paths)) {
const { tags } = paths[path][operation];

if (!Array.isArray(tags)) {
continue;
}

for (const [i, tag] of tags.entries()) {
if (!globalTags.includes(tag)) {
results.push({
message: 'Operation tags should be defined in global tags.',
path: ['paths', path, operation, 'tags', i],
});
}
}
}
Expand Down
33 changes: 33 additions & 0 deletions src/rulesets/oas/functions/utils/getAllOperations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { isObject } from './isObject';

const validOperationKeys = ['get', 'head', 'post', 'put', 'patch', 'delete', 'options', 'trace'];

export function* getAllOperations(paths: unknown): IterableIterator<{ path: string; operation: string }> {
if (!isObject(paths)) {
return;
}

const item = {
path: '',
operation: '',
};

for (const path of Object.keys(paths)) {
const operations = paths[path];
if (!isObject(operations)) {
continue;
}

item.path = path;

for (const operation of Object.keys(operations)) {
if (!isObject(operations[operation]) || !validOperationKeys.includes(operation)) {
continue;
}

item.operation = operation;

yield item;
}
}
}
5 changes: 5 additions & 0 deletions src/rulesets/oas/functions/utils/isObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { Dictionary } from '@stoplight/types';

export function isObject(value: unknown): value is Dictionary<unknown> {
return value !== null && typeof value === 'object';
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,10 @@ OpenAPI 3.x detected
2:6 warning info-description OpenAPI object info `description` must be present and non-empty string. info
7:9 warning operation-description Operation `description` must be present and non-empty string. paths./test.get
7:9 warning operation-tags Operation should have non-empty `tags` array. paths./test.get
8:20 error operation-operationId-unique Every operation must have a unique `operationId`. paths./test.get.operationId
10:9 error parser Mapping key must be a string scalar rather than number paths./test.get.responses[200]
12:10 warning operation-description Operation `description` must be present and non-empty string. paths./test.post
12:10 warning operation-tags Operation should have non-empty `tags` array. paths./test.post
13:20 error operation-operationId-unique Every operation must have a unique `operationId`. paths./test.post.operationId
15:9 error parser Mapping key must be a string scalar rather than number paths./test.post.responses[200]

11 problems (4 errors, 7 warnings, 0 infos, 0 hints)
10 problems (3 errors, 7 warnings, 0 infos, 0 hints)