-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-304055: IPA-106 Create: The request should be a Request suffixed object #473
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bc9e113
CLOUDP-304055: IPA-106 Create: The request should be a Request suffix…
yelizhenden-mdb a32c152
inline schemas
yelizhenden-mdb 4d22b78
fix
yelizhenden-mdb f72a013
test fix
yelizhenden-mdb b48e430
error message fix
yelizhenden-mdb 1c820e5
prettier fix
yelizhenden-mdb aedfe99
readme fix
yelizhenden-mdb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
221 changes: 221 additions & 0 deletions
221
tools/spectral/ipa/__tests__/createMethodRequestBodyIsRequestSuffixedObject.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,221 @@ | ||
| import testRule from './__helpers__/testRule'; | ||
| import { DiagnosticSeverity } from '@stoplight/types'; | ||
|
|
||
| const componentSchemas = { | ||
| schemas: { | ||
| SchemaRequest: { | ||
| type: 'object', | ||
| }, | ||
| Schema: { | ||
| type: 'object', | ||
| }, | ||
| }, | ||
| }; | ||
| testRule('xgen-IPA-106-create-method-request-body-is-request-suffixed-object', [ | ||
| { | ||
| name: 'valid methods', | ||
| document: { | ||
| components: componentSchemas, | ||
| paths: { | ||
| '/resource': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/SchemaRequest', | ||
| }, | ||
| }, | ||
| 'application/vnd.atlas.2024-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/SchemaRequest', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource/{id}': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/SchemaRequest', | ||
| }, | ||
| }, | ||
| 'application/vnd.atlas.2024-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/SchemaRequest', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource/{id}:customMethod': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| }, | ||
| 'application/vnd.atlas.2024-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/SchemaRequest', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| errors: [], | ||
| }, | ||
| { | ||
| name: 'invalid methods', | ||
| document: { | ||
| components: componentSchemas, | ||
| paths: { | ||
| '/resource': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource2': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| }, | ||
| 'application/vnd.atlas.2024-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource3': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| type: 'object', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| errors: [ | ||
| { | ||
| code: 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object', | ||
| message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', | ||
| path: ['paths', '/resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], | ||
| severity: DiagnosticSeverity.Warning, | ||
| }, | ||
| { | ||
| code: 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object', | ||
| message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', | ||
| path: ['paths', '/resource2', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], | ||
| severity: DiagnosticSeverity.Warning, | ||
| }, | ||
| { | ||
| code: 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object', | ||
| message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', | ||
| path: ['paths', '/resource2', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], | ||
| severity: DiagnosticSeverity.Warning, | ||
| }, | ||
| { | ||
| code: 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object', | ||
| message: 'The response body schema is defined inline and must reference a predefined schema. http://go/ipa/106', | ||
| path: ['paths', '/resource3', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], | ||
| severity: DiagnosticSeverity.Warning, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| name: 'invalid method with exception', | ||
| document: { | ||
| components: componentSchemas, | ||
| paths: { | ||
| '/resource': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| 'x-xgen-IPA-exception': { | ||
| 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object': 'reason', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource2': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| 'x-xgen-IPA-exception': { | ||
| 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object': 'reason', | ||
| }, | ||
| }, | ||
| 'application/vnd.atlas.2024-01-01+json': { | ||
| schema: { | ||
| $ref: '#/components/schemas/Schema', | ||
| }, | ||
| 'x-xgen-IPA-exception': { | ||
| 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object': 'reason', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| '/resource3': { | ||
| post: { | ||
| requestBody: { | ||
| content: { | ||
| 'application/vnd.atlas.2023-01-01+json': { | ||
| schema: { | ||
| type: 'object', | ||
| }, | ||
| 'x-xgen-IPA-exception': { | ||
| 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object': 'reason', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| errors: [], | ||
| }, | ||
| ]); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # IPA-104: Create | ||
| # http://go/ipa/106 | ||
|
|
||
| functions: | ||
| - createMethodRequestBodyIsRequestSuffixedObject | ||
|
|
||
| rules: | ||
| xgen-IPA-106-create-method-request-body-is-request-suffixed-object: | ||
| description: 'The Create method request should be a Request suffixed object. http://go/ipa/106' | ||
| message: '{{error}} http://go/ipa/106' | ||
| severity: warn | ||
| given: '$.paths[*].post.requestBody.content' | ||
| then: | ||
| field: '@key' | ||
| function: 'createMethodRequestBodyIsRequestSuffixedObject' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { hasException } from './utils/exceptions.js'; | ||
| import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; | ||
| import { isCustomMethod } from './utils/resourceEvaluation.js'; | ||
| import { resolveObject } from './utils/componentUtils.js'; | ||
|
|
||
| const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object'; | ||
| const ERROR_MESSAGE_SCHEMA_NAME = 'The response body schema must reference a schema with a Request suffix.'; | ||
| const ERROR_MESSAGE_SCHEMA_REF = 'The response body schema is defined inline and must reference a predefined schema.'; | ||
|
|
||
| export default (input, _, { path, documentInventory }) => { | ||
| const oas = documentInventory.unresolved; | ||
| const resourcePath = path[1]; | ||
|
|
||
| if (isCustomMethod(resourcePath)) { | ||
| return; | ||
| } | ||
|
|
||
| const contentPerMediaType = resolveObject(oas, path); | ||
|
|
||
| if (hasException(contentPerMediaType, RULE_NAME)) { | ||
| collectException(contentPerMediaType, RULE_NAME, path); | ||
| return; | ||
| } | ||
|
|
||
| if (contentPerMediaType.schema) { | ||
| const schema = contentPerMediaType.schema; | ||
| if (!schema.$ref) { | ||
| return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); | ||
| } | ||
|
|
||
| if (!schema.$ref.endsWith('Request')) { | ||
| return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); | ||
| } | ||
| } | ||
|
|
||
| collectAdoption(path, RULE_NAME); | ||
| }; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point me to where this is expected in the IPA 106? I disagree with enforcing this particular rule because it creates unnecessary redundancy in our codebase since developers are forced to duplicate views with only minor name changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I missed it. Thanks
I’m curious about the reasons behind prefixing view objects with “Request”. This naming convention concerns me because it might force us to create duplicate view definitions with different names, when we could potentially use
Schema.AccessModeto maintain a single view that adapts based on context. Am I missing something? Maybe I misunderstood the proposed change 🤔Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can think of
CreateRequestsuffixed schemas as an example for this guideline. View suffixed schemas are being used for responses as far as I knowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a somewhat recommend practice that helps isolate changes between payloads, some feel strongly it helps to make clear what's write only vs read only while like you some have concerns on duplication