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

Fix swagger spec #427

Merged
merged 1 commit into from
Nov 30, 2015
Merged

Fix swagger spec #427

merged 1 commit into from
Nov 30, 2015

Conversation

IvanGoncharov
Copy link
Contributor

I'm working on open catalog of REST API specifications.
And I really want to add your API into it, but it don't pass validation.
As first step, I fixed some obvious mistakes.

@d00rman
Copy link
Contributor

d00rman commented Nov 30, 2015

Thank you @IvanGoncharov for the contribution! And the catalogue project looks very very interesting! We'd love be part of it.

d00rman pushed a commit that referenced this pull request Nov 30, 2015
@d00rman d00rman merged commit cb2946b into wikimedia:master Nov 30, 2015
@Pchelolo
Copy link
Contributor

@IvanGoncharov Thank you for the patch.

BTW Which tool are you using for spec validation? This one?

@IvanGoncharov
Copy link
Contributor Author

@Pchelolo validator-badge is definitely step in right direction. But it validate only against official JSON Schema.
I use sway which has a lot of additional semantic validations.

@IvanGoncharov
Copy link
Contributor Author

@Pchelolo @d00rman Next problem that I see is optional path parameters. Like for example: /{module:transform}/html/to/wikitext{/title}{/revision}
Swagger spec(see this) doesn't support them:

If the parameter is in path, this property is required and its value MUST be true

Simplest solution would be to split function into few, like for example /{module:transform}/html/to/wikitext{/title}{/revision} will be spitted into:

  • /{module:transform}/html/to/wikitext
  • /{module:transform}/html/to/wikitext/{title}
  • /{module:transform}/html/to/wikitext/{title}/{revision}

Code duplication could be reduced by sharing parameters and responces.
If you agree with such solution I can submit a PR.

@gwicke
Copy link
Member

gwicke commented Dec 1, 2015

@IvanGoncharov: We proposed optional path & more generally RFC 6570 support in OAI/OpenAPI-Specification#93. There are quite a few other tasks requesting this, and there were some noises from the swagger folks that they'd add it in the next version of the spec.

I don't think that bloating our docs with a lot of overlapping routes would be desirable. Maybe we could find a way to automatically expand routes using some RFC 6570 features to a verbose Swagger 2.0 spec & then list that in your site?

@IvanGoncharov
Copy link
Contributor Author

@gwicke I think first step is to explicitly mark that you using such extension and it not done by mistake.
Swagger support specification extensions so I propose add something like x-hasOptionalPathParamets: true in top-level of specification.
I think separate NPM package which do expansion will be good alternative and it could be reused by others. I imagine it taking swagger finding all paths that have {/var} template in them checking that required is false for appropriate parameters and splitting such paths and finally remove x-hasOptionalPathParamets. It will also benefit you by allowing to validate your spec and also use it with unpatched libraries and tools.
In future we can added support for {+var} into the same lib.
What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants