search

civisanalytics / swagger-diff

Utility for comparing two Swagger specifications.
BSD 3-Clause "New" or "Revised" License
265 stars 32 forks source link

return type with no $ref causing failure #12

Closed davidvc closed 8 years ago

davidvc commented 8 years ago

I am getting this error when processing a Swagger JSON:

NoMethodError:         NoMethodError: undefined method `rindex' for nil:NilClass
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:88:in `refs'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:186:in `block in response_attributes_inner'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:176:in `each'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:176:in `response_attributes_inner'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:28:in `block in response_attributes'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:27:in `each'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:27:in `response_attributes'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:117:in `block in incompatible_response_attributes_enumerator'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:108:in `each'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:108:in `each'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:108:in `each'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:108:in `incompatible_response_attributes'
/Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:16:in `incompatibilities'

I am attaching the JSON that causes the error

davidvc commented 8 years ago

I believe it is this snippet that is causing the problem. Notice that in the 200 response the schema has a "type" field but no "$ref" field. I believe, looking at the definition of a JSON schema, that's OK. It looks like your code expects every response object to have a "$ref" field in its schema object either at the top level or if type is "array" as a field in an item object in the "items" array.

"/features": {
  "get": {
    "responses": {
      "200": {
        "schema": {
          "items": {
            "type": "string"
          },
          "type": "array"
        },
        "description": "successful operation"
      }
    },
    "parameters": [],
    "produces": [
      "application/json"
    ],
    "operationId": "getFeatures",
    "description": "",
    "summary": "The list of features that may be enabled/disabled",
    "tags": [
      "features"
    ]
  }
},
jeffreyc commented 8 years ago

Thanks for the report, and for the example! Though we've been using swagger-diff for over a month, the Swagger specification is pretty large, and I'm not surprised if there are some corners that are untested. It looks like we expect all responses to contain references, but my read of the Swagger specification is that your Swagger is perfectly valid, and we need to handle embedded schemas as well as references. I'll look into refactoring the response parsing to handle this.

davidvc commented 8 years ago

I wanted to let you know that with the latest code from your master branch, I got swagger-diff to work with the two services that were failing! Now I have about 12 other services to add to the mix, we'll see how it goes :)

Thanks for all your work and great responsiveness! This tool has great promise and should be invaluable in keeping ourselves honest. I am meeting with a Big Customer today who wants to know how we will prevent this from happening in the future, and swagger-diff is one of the tools I am going to pull out and share as a key component of the strategy.

All the best!

David

On Tue, Nov 10, 2015 at 4:10 PM jeffreyc notifications@github.com wrote:

Closed #12 https://github.com/civisanalytics/swagger-diff/issues/12 via

15 https://github.com/civisanalytics/swagger-diff/pull/15.

— Reply to this email directly or view it on GitHub https://github.com/civisanalytics/swagger-diff/issues/12#event-460766918 .

jeffreyc commented 8 years ago

That's great; I'm glad to hear that things are working for you! I just released 1.0.4, so you don't need to pull master. Hopefully things will just work for your other services, but if not, we welcome more issues or pull requests.

It's also great to hear that you're getting value from swagger-diff! We hope it's as useful to you as it has been to us :)