Handle nullables #268
Merged
Handle nullables #268
Conversation
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
The thing description is invalid if we have fields with `allow_none` enabled. That's because JSONSchema permits `type` to be a list, which is the sensible way to represent a field that may be `null` or a value. I've modified our marshmallow_jsonschema library to expand this into one duplicated sub-schema for each type in the list, combined using `oneOf`. This is allowed by the thing description (and, as far as I know, is valid JSON Schema as well).
Using the `field2property` method of the `FieldConverterMixin` from `apispec.ext.marshmallow` is not reliable, as some methods (specifically those dealing with nullable fields) depend on being linked to an `APISpec` object. I've changed our code so we now use the initialised converter object from our current spec - it adds an argument, but makes everything more robust.
|
@jtc42 I'm going to merge this assuming everything comes up green - it's a minor change and is only likely to be noticed because it fixes some errors. I'd love to find a better solution than embedding |
Codecov Report
@@ Coverage Diff @@
## master #268 +/- ##
==========================================
+ Coverage 91.19% 91.23% +0.03%
==========================================
Files 41 41
Lines 1851 1859 +8
Branches 292 294 +2
==========================================
+ Hits 1688 1696 +8
Misses 105 105
Partials 58 58
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
W3C Thing Description does not permit the
typeof aDataSchemaobject to be a list. This is valid JSON Schema, but is not valid Thing Description. This most commonly occurs when fields are declared withallow_none=True.This PR fixes two things:
APISpecobject and I was trying to fudge my way around it.marshmallow_jsonschemaand doesn't enforce the Thing Description restriction ontype.I've fixed it in two places:
labthings.openapi.utilities.ensure_schemato make sure nullable fields are serialised correctlymarshmallow_jsonschemaso that it expands list types, as described inlabthings.json.marshmallow_jsonschema.base.convert_list_type_to_oneofThis closes #267