Skip to content
Closed
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
@@ -0,0 +1,92 @@
import { outdent } from 'outdent';
import { lintDocument } from '../../../lint';
import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils';
import { BaseResolver } from '../../../resolve';

describe('Oas2 response-contains-property', () => {
it('oas2-response-contains-property: should report on response object when not contains properties', async () => {
const document = parseYamlToDocument(
outdent`
swagger: '2.0'
schemes:
- https
basePath: /v2
paths:
'/accounts/{accountId}':
get:
description: Retrieve a sub account under the master account.
operationId: account
responses:
'200':
description: Account object returned
schema:
type: object
properties:
created_at:
description: Account creation date/time
format: date-time
type: string
owner_email:
description: Account Owner email
type: string
'404':
description: User not found
`,
);

const results = await lintDocument({
externalRefResolver: new BaseResolver(),
document,
config: makeConfig({
'response-contains-property': {
severity: 'error',
mustExist: ['id'],
},
}),
});
expect(results).toMatchInlineSnapshot(`
Array [
Object {
"location": Array [
Object {
"pointer": "#/paths/~1accounts~1{accountId}/get/responses/200/schema/properties",
"reportOnKey": false,
"source": Source {
"absoluteRef": "",
"body": "swagger: '2.0'
schemes:
- https
basePath: /v2
paths:
'/accounts/{accountId}':
get:
description: Retrieve a sub account under the master account.
operationId: account
responses:
'200':
description: Account object returned
schema:
type: object
properties:
created_at:
description: Account creation date/time
format: date-time
type: string
owner_email:
description: Account Owner email
type: string
'404':
description: User not found",
"mimeType": undefined,
},
},
],
"message": "Response object must have a top-level \\"id\\" property.",
"ruleId": "response-contains-property",
"severity": "error",
"suggest": Array [],
},
]
`);
});
});
2 changes: 2 additions & 0 deletions packages/core/src/rules/oas2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { PathExcludesPatterns } from '../common/path-excludes-patterns';
import { RequestMimeType } from './request-mime-type';
import { ResponseMimeType } from './response-mime-type';
import { PathSegmentPlural } from '../common/path-segment-plural';
import { ResponseContainsProperty } from './response-contains-property';

export const rules = {
spec: OasSpec as Oas2Rule,
Expand Down Expand Up @@ -76,6 +77,7 @@ export const rules = {
'request-mime-type': RequestMimeType as Oas2Rule,
'response-mime-type': ResponseMimeType as Oas2Rule,
'path-segment-plural': PathSegmentPlural as Oas2Rule,
'response-contains-property': ResponseContainsProperty as Oas2Rule,
};

export const preprocessors = {};
23 changes: 23 additions & 0 deletions packages/core/src/rules/oas2/response-contains-property.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Oas2Rule } from '../../visitors';

export const ResponseContainsProperty: Oas2Rule = (options) => {
const mustExist = options.mustExist || [];
return {
Response: {
skip: (_response, key) => {
return !['200', '201', '202'].includes(key.toString());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What do these numbers mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tatomyr
HTTP status codes, we omit all statuses except those listed above, because those requests have no response content.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, they may have response content but may be errors, and that's why they aren't applicable either.

204 - no content
4XX - bad requests
5XX - server errors

It's common that things like 4XX have different properties.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as #537 (comment)

But here also different mime types are in play, so I'm not sure what to suggest yet.

@adamaltman do you have some ideas?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I think the 2XX, 4XX configuration options is a good idea.

},
Schema (schema, { report, location }) {
if (schema.type !== 'object') return;
for (let element of mustExist) {
if (!schema.properties?.[element]) {
report({
message: `Response object must have a top-level "${element}" property.`,
location: location.child('properties'),
});
}
}
}
}
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { outdent } from 'outdent';
import { lintDocument } from '../../../lint';
import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils';
import { BaseResolver } from '../../../resolve';

describe('Oas3 response-contains-property', () => {
it('oas3-response-contains-property: should report on response object when not contains properties', async () => {
const document = parseYamlToDocument(
outdent`
openapi: 3.0.3
info:
version: 3.0.0
paths:
/store/subscribe:
post:
responses:
'201':
content:
application/json:
schema:
type: object
properties:
subscriptionId:
type: string
example: AAA-123-BBB-456
`,
);

const results = await lintDocument({
externalRefResolver: new BaseResolver(),
document,
config: makeConfig({
'response-contains-property': {
severity: 'error',
mustExist: ['id'],
},
}),
});

expect(results).toMatchInlineSnapshot(`
Array [
Object {
"location": Array [
Object {
"pointer": "#/paths/~1store~1subscribe/post/responses/201/content/application~1json/schema/properties",
"reportOnKey": false,
"source": Source {
"absoluteRef": "",
"body": "openapi: 3.0.3
info:
version: 3.0.0
paths:
/store/subscribe:
post:
responses:
'201':
content:
application/json:
schema:
type: object
properties:
subscriptionId:
type: string
example: AAA-123-BBB-456",
"mimeType": undefined,
},
},
],
"message": "Response object must have a top-level \\"id\\" property.",
"ruleId": "response-contains-property",
"severity": "error",
"suggest": Array [],
},
]
`);
});
});
2 changes: 2 additions & 0 deletions packages/core/src/rules/oas3/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { PathSegmentPlural } from '../common/path-segment-plural';
import { PathExcludesPatterns } from '../common/path-excludes-patterns';
import { NoInvalidSchemaExamples } from '../common/no-invalid-schema-examples';
import { NoInvalidParameterExamples } from '../common/no-invalid-parameter-examples';
import { ResponseContainsProperty } from './response-contains-property';

export const rules = {
spec: OasSpec,
Expand Down Expand Up @@ -92,6 +93,7 @@ export const rules = {
'path-segment-plural': PathSegmentPlural,
'no-invalid-schema-examples': NoInvalidSchemaExamples,
'no-invalid-parameter-examples': NoInvalidParameterExamples,
'response-contains-property': ResponseContainsProperty,
} as Oas3RuleSet;

export const preprocessors = {};
25 changes: 25 additions & 0 deletions packages/core/src/rules/oas3/response-contains-property.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Oas3Rule } from '../../visitors';

export const ResponseContainsProperty: Oas3Rule = (options) => {
const mustExist = options.mustExist || [];
return {
Response: {
skip: (_response, key) => {
return !['200', '201', '202'].includes(key.toString());
},
MediaType: {
Schema (schema, { report, location }) {
if (schema.type !== 'object') return;
for (let element of mustExist) {
if (!schema.properties?.[element]) {
report({
message: `Response object must have a top-level "${element}" property.`,
location: location.child('properties'),
});
}
}
}
}
}
}
};