-
Notifications
You must be signed in to change notification settings - Fork 10
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
Allow oneOf to be parsed. RE: #5 #81
Conversation
5d9fb57
to
f2e9ae0
Compare
src/parser/open-api/oneOfResolver.js
Outdated
* @param {object} obj | ||
* @return {array} | ||
*/ | ||
function getOneOfPaths (obj) { |
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.
Nice!
If you want, you could rename k
to key
. It would be more descriptive.
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.
Fixed.
src/parser/open-api/schemaParser.js
Outdated
const resolvedJsonSchema = resolveAllOf(jsonSchema) | ||
const outputSchema = getPropertiesNode(resolvedJsonSchema.properties, resolvedJsonSchema.required) | ||
let resolved = resolveAllOf(jsonSchema) | ||
resolved = resolveOneOf(jsonSchema) |
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.
It looks like the result of resolveAllOf
is discarded.
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.
Nice find!
"required": ["foo"] | ||
} | ||
] | ||
} |
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.
I would reorder the elements in this file for better readability. Currently the order is as follows:
- string - bar
- integer - foo
- integer - bar
- string - foo
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.
Fixed
return obj | ||
} | ||
|
||
const states = getStates(paths, obj) |
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.
What do you think about permutations
, as opposed to states
?
src/parser/open-api/oneOfResolver.js
Outdated
* If no `oneOf` keys were found, an empty array is returned. | ||
* | ||
* @param {object} obj | ||
* @return {array} |
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.
This should be {string[]} paths
to be consistent with the docs on getStates
function.
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.
Fixed.
Allow oneOf to be parsed. RE: temando#5
Allow oneOf to be parsed. RE: temando#5
This MR updates
schemaParser
to be able to consumeoneOf
definitions (using a helper,resolveOneOf
).It works by creating full permutations of the object by applying each of the possible
oneOf
states.I expect performance will degrade on large schemas, but given we only run the schema parsing at a property level, I think this should be sufficient for the short term.
I have added three test cases:
oneOf
, which is identical to our scenario for authentication with the Temando Platform APItype: [ array, of, types]
Other:
getTestsFromFixtures
that helps load a set of inputs/outputs from a directoryschemaParser
. Most objects in this logic are created as the result of aJSON.parse
, so the checks tohasOwnProperty
are redundantNote this MR does introduce visualising of the
oneOf
schema, that'll be handled in #32.