Add required fields to Swagger for SchemaP #1536
Merged
Conversation
This file contains 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
There are two parts to this: 1) SchemaDoc is now an instance of a NearSemiRing type class, to support the existence of two different schema concatenation operations. The second operation `add` is used by the Alternative instance, and the main one `(<>)` continues to be used by the Applicative one. The Swagger implementation of `add` works the same as `(<>)`, but it intersects required fields instead of concatenating them. 2) Required fields are added to the Swagger doc by the `field` combinator. Single fields are always required, whereas trivial fields obtained with say `pure` are not. Since the `Alternative` instance intersects required fields, we automatically get the correct behaviour that fields with a default value are not required, as well as fields that use the `optional` combinator from `Control.Applicative`. Unfortunately, the NearSemiRing instance for Swagger is not exactly law-abiding, but this comes down to Swagger not really being designed to support schema alternatives. In special situations, one might need to tweak the required field of the resulting schema, but the current solution should cover most cases.
The Alternative instance used to work as well (and have the same implementation), but after the introduction of the NearSemiRing constraint for doc, the Monoid instance is the only one that can be used in this case.
jschaul
approved these changes
May 27, 2021
There was a problem hiding this comment.
Using https://www.npmjs.com/package/swagger-diff I get
swagger-diff swagger-staging-2021-05-27.json latest.json
swagger-diff
Errors (0)
Warnings (53)
edit-object-property-required definitions/UserProfile - Property qualified_id became required
edit-object-property-required definitions/UserProfile - Property name became required
edit-object-property-required definitions/UserProfile - Property accent_id became required
edit-object-property-required definitions/UserProfile - Property legalhold_status became required
edit-object-property-required definitions/Qualified UserId - Property id became required
edit-object-property-required definitions/Qualified UserId - Property domain became required
edit-object-property-required definitions/UserAsset - Property key became required
edit-object-property-required definitions/UserAsset - Property type became required
edit-object-property-required definitions/UserHandleInfo - Property qualified_user became required
edit-object-property-required definitions/Qualified Handle - Property handle became required
edit-object-property-required definitions/Qualified Handle - Property domain became required
edit-object-property-required definitions/ClientPrekey - Property client became required
edit-object-property-required definitions/ClientPrekey - Property prekey became required
edit-object-property-required definitions/PrekeyBundle - Property user became required
edit-object-property-required definitions/PrekeyBundle - Property clients became required
edit-object-property-required definitions/Conversation - Property id became required
edit-object-property-required definitions/Conversation - Property type became required
edit-object-property-required definitions/Conversation - Property creator became required
edit-object-property-required definitions/Conversation - Property access became required
edit-object-property-required definitions/Conversation - Property access_role became required
edit-object-property-required definitions/Conversation - Property members became required
edit-object-property-required definitions/Member - Property id became required
edit-object-property-required definitions/OtherMember - Property qualified_id became required
edit-object-property-required definitions/ConvMembers - Property self became required
edit-object-property-required definitions/ConvMembers - Property others became required
edit-object-property-required definitions/NewConv - Property users became required
edit-object-property-required definitions/ConvTeamInfo - Property teamid became required
edit-object-property-required definitions/Event - Property type became required
edit-object-property-required definitions/Event/properties/data - Property access became required
edit-object-property-required definitions/Event/properties/data - Property creator became required
edit-object-property-required definitions/Event/properties/data - Property status became required
edit-object-property-required definitions/Event/properties/data - Property access_role became required
edit-object-property-required definitions/Event/properties/data - Property text became required
edit-object-property-required definitions/Event/properties/data - Property sender became required
edit-object-property-required definitions/Event/properties/data - Property members became required
edit-object-property-required definitions/Event/properties/data - Property key became required
edit-object-property-required definitions/Event/properties/data - Property name became required
edit-object-property-required definitions/Event/properties/data - Property code became required
edit-object-property-required definitions/Event/properties/data - Property id became required
edit-object-property-required definitions/Event/properties/data - Property type became required
edit-object-property-required definitions/Event/properties/data - Property receipt_mode became required
edit-object-property-required definitions/Event/properties/data - Property user_ids became required
edit-object-property-required definitions/Event/properties/data - Property recipient became required
edit-object-property-required definitions/Event - Property data became required
edit-object-property-required definitions/Event - Property conversation became required
edit-object-property-required definitions/Event - Property from became required
edit-object-property-required definitions/Event - Property time became required
edit-object-property-required definitions/SimpleMember - Property id became required
edit-object-property-required definitions/SimpleMember - Property conversation_role became required
edit-object-property-required definitions/InviteQualified - Property qualified_users became required
edit-object-property-required definitions/UserLegalHoldStatusResponse/properties/last_prekey - Property key became required
edit-object-property-required definitions/UserLegalHoldStatusResponse/properties/last_prekey - Property id became required
edit-object-property-required definitions/Id - Property id became required
Glancing at that, e.g. definitions/InviteQualified - Property qualified_users became required looks right. If unset, we default to something using <|>, so RoleName remains not required.
And looking at the swagger I see e.g. this endpoint where the qualified userId is required but the conversation_role is not. So this looks like that's correct behaviour:
NearSemiRing implementation is magic, but if it works I'm happy with it :)
Closed
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.
This populates the
requiredfield of Swagger schemas generated usingSchemaP. See commit description for more information on the implementation.