CLOUDP-304053: IPA-106:Create - The resource must be the request body #492
Conversation
yelizhenden-mdb
changed the title
|
Any way we can putthose somewhere in the repository: Decision 2: Check for the same versions of create.requestBody and get.responses. If there is no equivalent version in the get response, ignore it. Could be manual notes in readme or part of the ipa docs we generate. |
| sparse-checkout: | | ||
| openapi/ | ||
| tools/spectral | ||
| - name: Setup Node |
There was a problem hiding this comment.
It could not find the new libraries I included for the rule, so added these steps
| !isEqual( | ||
| omitDeep(postMethodRequestContentPerMediaType.schema, 'readOnly', 'writeOnly'), | ||
| omitDeep(getMethodResponseContentPerMediaType.schema, 'readOnly', 'writeOnly') | ||
| ) |
There was a problem hiding this comment.
We need to be aware of circular dependencies: lodash/lodash#5764
There was a problem hiding this comment.
This will compare the resolved schemas. So I am expecting it will be all value comparison but not reference comparisons
There was a problem hiding this comment.
Yes. That was my point. I think we have >10 circular references in the models and that have been breaking postman for us: https://github.com/mongodb/openapi/blob/main/tools/postman/scripts/transform-for-api.sh#L49
Good to ensure that all current openapi violations in schema are valid. If not those can be extra unit tests.
There was a problem hiding this comment.
Okay, let me add extra unit tests for this case. Do you have any example in hand, I can imitate it as unit test?
There was a problem hiding this comment.
Looked into something simple but.. our schema is too complex so we can have simpler version for tests.
I think all unit tests should cover oneOf and then we can replace that across
There was a problem hiding this comment.
Included oneOf and circular dependency tests. Could you take another look if they match the cases in your mind?
tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js
Outdated
Show resolved
Hide resolved
| "apache-arrow": "^19.0.1", | ||
| "dotenv": "^16.4.7", | ||
| "eslint-plugin-jest": "^28.10.0", | ||
| "lodash": "^4.17.21", |
There was a problem hiding this comment.
FYI: lodash library is used for deep object comparison
There was a problem hiding this comment.
Good idea to use it for now.
In the past experience I have always started with it and then implemented custom versions to cover corner cases :)
There was a problem hiding this comment.
Fingers crossed hope we will not need custom one, but let's see :D
| "eslint-plugin-jest": "^28.10.0", | ||
| "lodash": "^4.17.21", | ||
| "markdown-table": "^3.0.4", | ||
| "omit-deep-lodash": "^1.1.7", |
There was a problem hiding this comment.
FYI: omit-deep-lodash library is used for recursive omitting
|
I might have missed it somewhere else, but should also the response of the CREATE match the request of the CREATE? I guess that's probably another validation? |
|
@marcosuma Yes, that's correct! There will be another validation that will validate |
4 participants
Proposed changes
Jira ticket: CLOUDP-304053
Check if the request body content of the
Createmethod and the response content of theGetmethod refer to the same resource.Important Note: The request body schemas can be inline or referenced. For both, check the field equivalence. (Don't check for readOnly/writeOnly properties)
Example: The create and get methods for the /groups/{groupId}/clusters resource follow this rule
Decision 1: If a resource provides a Create method but not a Get method (
post /resourceandget /resource/{id}should exist at the same time), ignore it.Decision 2: Check for the same versions of
create.requestBodyandget.responses. If there is no equivalent version in the get response, ignore it.Checklist
Changes to Spectral
Further comments