Skip to content
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

Swagger fixes #1625

Merged
merged 9 commits into from
Jun 25, 2021
Merged

Swagger fixes #1625

merged 9 commits into from
Jun 25, 2021

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Jun 23, 2021

This fixes issues in our Swagger schema reported by @ffflorian, plus other validation errors reported by https://editor.swagger.io/.

Fixes

  • Removed spaces and parentheses from schema names. I simply used dots instead of spaces and avoided parentheses.
  • Fixed dangling reference by explicitly declaring Action in a manually defined Swagger.Schema.
  • Made the schema produced by optField optional (this should have been part of Add optField combinator and related #1621, but I forgot).
  • Fixed the schema for FormRedirect, which made absolutely no sense. It was generated by the generic mechanism, but now it's written by hand. See the corresponding commit (or comment) for more information on why it was broken.
  • Removed duplicated SecurityRequirement entries. This is a bit strange, since it is a list of maps, instead of simply a map, so it concatenates in the wrong way. For now I've simply fixed it by removing duplicates at the point where we generate the final Swagger.
  • Removed duplicated required properties. Again, this should be a set instead of a list, but for now I've also fixed it by running a simple-minded sanitisation function where we generate the final Swagger.

Checklist

  • Title of this PR explains the impact of the change.
  • The description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • The CHANGELOG.md file in the Unreleased section has been updated to explain the change which will be included in the release notes.

@pcapriotti pcapriotti changed the title Pcapriotti/swagger fixes Swagger fixes Jun 23, 2021
@ffflorian
Copy link
Contributor

Awesome, thanks!

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good (I didn't check the swagger validation though, but it sounds like the changes make sense!)

But there's one failing test:

wire-api           >     Wrapped "some_user" User:                                                                       FAIL
wire-api           >       *** Failed! Falsified (after 1 test):
wire-api           >       Wrapped {unwrap = User {userId = 00000000-0000-0001-0000-000000000001, userQualifiedId = Qualified {qUnqualified = 00000001-0000-0001-0000-000000000001, qDomain = Domain {_domainText = "ajwq35b.a2u5bq4.8lp-3.8-1.6zg5348y.xxrns"}}, userIdentity = Just (SSOIdentity (UserSSOId "" "") Nothing (Just (Phone {fromPhone = "+278953481"}))), userDisplayName = Name {fromName = "x7R\20804GQ4`\US\1000239.\USJ\13298\"J\NUL-\194823h\NAK\39098\41837\\\CANmwx\1060529LZ\991509\fAUG\1009402\CAN\1086277\1027925\10427\186475i\20672\EM\144756\183997\49059}V\985687l\1053834\180076\2594l9\134281\157781$\143450\110982,\NUL0l\1040542\STX<=\27654lF\EMCv\\\NUL$[a\177053n\1077255ztFK=p\1014114\7469I\SYN2a\994430\ACKe\DC2\nB"}, userPict = Pict {fromPict = []}, userAssets = [], userAccentId = ColourId {fromColourId = -1}, userDeleted = True, userLocale = gd-CC, userService = Just (ServiceRef {_serviceRefId = 00000000-0000-0000-0000-000000000001, _serviceRefProvider = 00000001-0000-0001-0000-000000000000}), userHandle = Just (Handle {fromHandle = "sl2x_v6v9tu.qc-yc_v9nopktq4mfx5el04i_1cr4lgaw3whj7xu6f6f25mlxapuhjd6u4l5774lnvgt.gic0t166bgv.7ciov2awghemyor731xvoqw8z9xf2ys9l4yiyxeme7sfj7kld27u246oh87yp"}), userExpire = Just (1864-05-09T22:53:27.762Z), userTeam = Nothing, userManagedBy = ManagedByWire}}
wire-api           >       Validation against the schema fails:
wire-api           >         * unknown schema "Handle"
wire-api           >         * unknown schema "ServiceRef"
wire-api           >         * unknown schema "Locale"
wire-api           >         * unknown schema "ManagedBy"
wire-api           >         * unknown schema "Qualified.UserId"
wire-api           >         * unknown schema "Pict"
wire-api           >         * unknown schema "UTCTimeMillis"
wire-api           >         * unknown schema "UserSSOId"
wire-api           >         * unknown schema "UUID"
wire-api           >                      
wire-api           >       JSON value:    
wire-api           >       {              
wire-api           >           "some_user": {
wire-api           >               "phone": "+278953481",
wire-api           >               "handle": "sl2x_v6v9tu.qc-yc_v9nopktq4mfx5el04i_1cr4lgaw3whj7xu6f6f25mlxapuhjd6u4l5774lnvgt.gic0t166bgv.7ciov2awghemyor731xvoqw8z9xf2ys9l4yiyxeme7sfj7kld27u246oh87yp",
wire-api           >               "service": {
wire-api           >                   "id": "00000000-0000-0000-0000-000000000001",
wire-api           >                   "provider": "00000001-0000-0001-0000-000000000000"
wire-api           >               },     
wire-api           >               "locale": "gd-CC",
wire-api           >               "managed_by": "wire",
wire-api           >               "qualified_id": {
wire-api           >                   "domain": "ajwq35b.a2u5bq4.8lp-3.8-1.6zg5348y.xxrns",
wire-api           >                   "id": "00000001-0000-0001-0000-000000000001"
wire-api           >               },     
wire-api           >               "accent_id": -1,
wire-api           >               "picture": [],
wire-api           >               "name": "x7R兄GQ4`\u001f󴌯.\u001fJ㏲\"J\u0000-洴h\u0015颺ꍭ\\\u0018mwx􂺱LZ󲄕\u000cAUG󶛺\u0018􉍅󺽕⢻𭡫i僀\u0019𣕴𬺽뾣}V󰩗l􁒊𫽬ਢl9𠲉𦡕$𣁚𛆆,\u00000l󾂞\u0002<=氆lF\u0019Cv\\\u0000$[a𫎝n􇀇ztFK=p󷥢ᴭI\u00162a󲱾\u0006e\u0012\nB",
wire-api           >               "expires_at": "1864-05-09T22:53:27.762Z",
wire-api           >               "sso_id": {
wire-api           >                   "subject": "",
wire-api           >                   "tenant": ""
wire-api           >               },     
wire-api           >               "id": "00000000-0000-0001-0000-000000000001",
wire-api           >               "deleted": true,
wire-api           >               "assets": []
wire-api           >           }          
wire-api           >       }              
wire-api           >                      
wire-api           >       Swagger Schema:
wire-api           >       {              
wire-api           >           "type": "object",
wire-api           >           "properties": {
wire-api           >               "some_user": {
wire-api           >                   "required": [
wire-api           >                       "id",
wire-api           >                       "qualified_id",
wire-api           >                       "name",
wire-api           >                       "picture",
wire-api           >                       "assets",
wire-api           >                       "accent_id",
wire-api           >                       "locale",
wire-api           >                       "managed_by"
wire-api           >                   ], 
wire-api           >                   "type": "object",
wire-api           >                   "properties": {
wire-api           >                       "email": {
wire-api           >                           "$ref": "#/definitions/Email"
wire-api           >                       },
wire-api           >                       "phone": {
wire-api           >                           "type": "string"
wire-api           >                       },
wire-api           >                       "handle": {
wire-api           >                           "$ref": "#/definitions/Handle"
wire-api           >                       },
wire-api           >                       "service": {
wire-api           >                           "$ref": "#/definitions/ServiceRef"
wire-api           >                       },
wire-api           >                       "locale": {
wire-api           >                           "$ref": "#/definitions/Locale"
wire-api           >                       },
wire-api           >                       "managed_by": {
wire-api           >                           "$ref": "#/definitions/ManagedBy"
wire-api           >                       },
wire-api           >                       "qualified_id": {
wire-api           >                           "$ref": "#/definitions/Qualified.UserId"
wire-api           >                       },
wire-api           >                       "accent_id": {
wire-api           >                           "maximum": 2147483647,
wire-api           >                           "format": "int32",
wire-api           >                           "minimum": -2147483648,
wire-api           >                           "type": "integer"
wire-api           >                       },
wire-api           >                       "picture": {
wire-api           >                           "$ref": "#/definitions/Pict"
wire-api           >                       },
wire-api           >                       "name": {
wire-api           >                           "type": "string"
wire-api           >                       },
wire-api           >                       "expires_at": {
wire-api           >                           "$ref": "#/definitions/UTCTimeMillis"
wire-api           >                       },
wire-api           >                       "team": {
wire-api           >                           "$ref": "#/definitions/UUID"
wire-api           >                       },
wire-api           >                       "sso_id": {
wire-api           >                           "$ref": "#/definitions/UserSSOId"
wire-api           >                       },
wire-api           >                       "id": {
wire-api           >                           "$ref": "#/definitions/UUID"
wire-api           >                       },
wire-api           >                       "deleted": {
wire-api           >                           "type": "boolean"
wire-api           >                       },
wire-api           >                       "assets": {
wire-api           >                           "items": {
wire-api           >                               "$ref": "#/definitions/UserAsset"
wire-api           >                           },
wire-api           >                           "type": "array"
wire-api           >                       }
wire-api           >                   }  
wire-api           >               }      
wire-api           >           }          
wire-api           >       }              
wire-api           >                      
wire-api           >       Swagger Description Context:
wire-api           >       {}             
wire-api           >                      
wire-api           >       Use --quickcheck-replay=230370 to reproduce.

@pcapriotti pcapriotti force-pushed the pcapriotti/swagger-fixes branch from 629ecd7 to 6aef92c Compare June 24, 2021 06:49
The reason it was broken is that genericDeclareNamedSchema tries to define the schema for this type as a heterogeneous array (i.e. tuple) with Swagger types String and AuthnRequest. However, Swagger does not support heterogeneous arrays, and this results in an array whose underlying type which is at the same time marked as a string, and referring to the schema for AuthnRequest, which is of course invalid.
@pcapriotti
Copy link
Contributor Author

The test is fixed now (I had accidentally created a dangling reference).

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it out on the swagger validation tools, looks good!

@pcapriotti pcapriotti merged commit 60872f3 into develop Jun 25, 2021
@pcapriotti pcapriotti deleted the pcapriotti/swagger-fixes branch June 25, 2021 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants