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

Fails to load due to null http operations in spec file #268

Closed
csuich2 opened this issue Mar 4, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@csuich2
Copy link

commented Mar 4, 2015

If a spec contains null http operations, Swagger UI fails to load due to missing null checks in two places:

Uncaught TypeError: Cannot read property 'parameters' of null (swagger-client.js:332)
Uncaught TypeError: Cannot read property 'tags' of null (swagger-client.js:686)

I came across this issue as my API is build & served with Jersey/JAX-RS and our serialization strategy includes serializing nulls.

I have identified a fix and will submit a PR shortly.

...
paths: {
  "/myapi": {
    "get": { ... },
    "put": { ... },
    "post": null,
    "delete": null,
    ...
  }
}
@webron

This comment has been minimized.

Copy link
Member

commented Mar 4, 2015

Technically, that's not a valid spec.

@fehguy

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2015

Yes, we should try, though, to not fail when we see null. @whitlockjc this is probably an easy fix.

@csuich2

This comment has been minimized.

Copy link
Author

commented Mar 4, 2015

I read through https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md and didn't se anything that would specify whether a null value for an operation was valid or invalid. Did I miss something in the spec that defines this?

@webron

This comment has been minimized.

Copy link
Member

commented Mar 4, 2015

@csuich2 - what @fehguy mentioned is true, we shouldn't fail on such things. Generally, null values are not supported across the spec. If you feel it should be clarified, please open an issue on swagger-spec and I'll take care of it. Thanks.

@whitlockjc

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2015

@webron is right. If you look at the JSON Schema, it allows a pathItemObject of which null is not a valid structure. If you do not want to support those HTTP verbs for that path, just omit them. If some other tool is generating this invalid Swagger document, file a bug.

@fehguy is right as well. swagger-js should be resilient to these types of things and I will look into it soon.

@csuich2

This comment has been minimized.

Copy link
Author

commented Mar 4, 2015

The reason the HTTP verbs are null is because of our APIs serialization. We explicitly serialize null values using GSON for all of our API responses which may not be a common practice, but I'm not sure I would label it as uncommon either.

@whitlockjc

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2015

Common or not, it is not valid. In the end, Swagger is a documentation format and it has rules in place. These rules are documented in the Swagger Specification and they are enforced structurally via the Swagger JSON Schema. Your explicit serialization is violating one of the rules, it's as simple as that. In the end, I will update swagger-js so that it will not fail when it encounters these but if you start using other tooling, like tools that validate your Swagger documents, you will run into this again I'm sure.

@csuich2

This comment has been minimized.

Copy link
Author

commented Mar 4, 2015

Thanks for talking this through, everyone! I've opened an issue for some clarification on the spec project.

Thanks!

@csuich2 csuich2 closed this Mar 4, 2015

schloerke added a commit to trestletech/plumber that referenced this issue Dec 19, 2018

schloerke added a commit to trestletech/plumber that referenced this issue Jan 11, 2019

Revamp Swagger Spec to OpenAPI v3 (#365)
* Migrate to new swagger dist files. As of now, these are all local. Eventually will be delivered via CDN.

* Update parse-globals defaultGlobals to reflect openapi 3.0.2

* Add comments on swagger-ui html

* ifelse to switch

* remove unused top level items in swagger and add a blank paths list

* udpate rbuildignore

* add download script for swagger-ui

* add swagger-ui@3.20.2

* commit working code before gutting v2 spec

* comment unused code

* use new swagger wrapper function

* use relative swagger.json url

* relative url back one dir

* try basic server info for cloud dev

* use dynamic relative url given window.location information

* schemes should not be in globals

* use swagger pkg

* clean up swagger creation

* answer TODO about trailing slashes

* use swagger::swagger_spec

* have swagger params be done with a list and not a data.frame

helps to know how it will be formatted exactly

* do not allow asJSON to be passed into pr$swaggerFile()

not necessary if we are switching to using lists and not data.frames

* swagger endpoints will use list information and not clone r6 objects to then extract information

* recursively remove NA and NULL values from swagger output

Fixes #322
Fixes #323

See swagger-api/swagger-js#268

Thank you @Hong-Revo for a recursive blueprint in #323

* is.na(list()) returns NA. it should return FALSE

* move param type to schema

* add test to check swagger against a cli validator

* add notes on how to install node.js pkg

* remove ... from swaggerFile/openAPIFile

* use swagger master v3.20.3.9999

* add a manual testing file for manual testing before release

* add an index route to the static only plumber example

* update example ot use addGlobalProcessor instead of registerHook

* use `spec` and not `sf`for param name to swagger file fn

* add expect_silent (or comment) test for tests that have no tests

* run validation on all inst/examples for plumber

* make sure basic http protocol is supplied to rstudio swaggerCallback

* add news item and split to look like shiny's


Co-authored-by: James Blair <james.m.blair09@gmail.com>
Co-authored-by: Barret Schloerke <schloerke@gmail.com>"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.