search

civisanalytics / swagger-diff

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

NoMethodError 'rindex' for nil with array parameter #17

Closed davidvc closed 8 years ago

davidvc commented 8 years ago

Darn, found another one with the next Swagger output

NoMethodError:         NoMethodError: undefined method `rindex' for nil:NilClass
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:87:in `refs'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:196:in `block in request_params_inner'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:193:in `each'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:193:in `request_params_inner'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:18:in `block in request_params'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:17:in `each'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/specification.rb:17:in `request_params'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:92:in `block in incompatible_request_params_enumerator'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:138:in `each'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:138:in `each'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:138:in `each'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:138:in `none?'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:138:in `requests_compatible?'
            /Users/dvancouvering/Projects/Ventana/ventana/vendor/swagger-diff/lib/swagger/diff/diff.rb:10:in `compatible?'

I put in a puts statement and this is the parameter that's barfing:

#<Swagger::V2::Parameter description="object representing list of provider details which will be added to care team" in="body" name="body" required=true schema=#<Swagger::Schema items=#<Swagger::Schema $ref="#/definitions/CareTeamProvider"> type="array">>

I am attaching the full Swagger JSON I am trying to work with careteam.txt

jeffreyc commented 8 years ago

It looks like we don't handle arrays of parameters. Fortunately, this should be a trivial fix, as we already did all of the hard work in #15. I'll put together a PR and get it merged.

Once that's done, would be able to share the Swagger for your remaining services? Or would you mind testing master against your remaining specifications? I'd love to make sure 1.0.5 handles all of your remaining specifications before releasing it, and not just the ones we've tested so far.

Thanks again for your bug report, and for sharing your specification! It really helps to quickly track down the problematic code, and is making swagger-diff a better product.

davidvc commented 8 years ago

I definitely plan to test the remaining services. But I do get busy, and I don't want to hang up your release because you're waiting for me. So I have attached the JSON for the services that are key (these are not all of them, we have about ten more)

But I'll also keep testing and report issues.

On Thu, Nov 12, 2015 at 10:31 AM jeffreyc notifications@github.com wrote:

It looks like we don't handle arrays of parameters. Fortunately, this should be a trivial fix, as we already did all of the hard work in #15 https://github.com/civisanalytics/swagger-diff/pull/15. I'll put together a PR and get it merged.

Once that's done, would be able to share the Swagger for your remaining services? Or would you mind testing master against your remaining specifications? I'd love to make sure 1.0.5 handles all of your remaining specifications before releasing it, and not just the ones we've tested so far.

Thanks again for your bug report, and for sharing your specification! It really helps to quickly track down the problematic code, and is making swagger-diff a better product.

— Reply to this email directly or view it on GitHub https://github.com/civisanalytics/swagger-diff/issues/17#issuecomment-156193807 .

davidvc commented 8 years ago

The attachment didn't seem to make it in the email reply, and I can't attach a tar.gz through GitHub. Any suggestions?

jeffreyc commented 8 years ago

The attachment didn't seem to make it in the email reply, and I can't attach a tar.gz through GitHub. Any suggestions?

It looks like GitHub doesn't support non-text/non-image attachments. Could you share a Dropbox link, attach them as text, or create some gists?

davidvc commented 8 years ago

Do you still want them, even though I have verified they all work? I do see how it could have value, but I'm a bit hestitant to share a bunch of our internal API specs :(

On Thu, Nov 12, 2015 at 1:18 PM jeffreyc notifications@github.com wrote:

The attachment didn't seem to make it in the email reply, and I can't attach a tar.gz through GitHub. Any suggestions?

It looks like GitHub doesn't support non-text/non-image attachments. Could you share a Dropbox link, attach them as text, or create some gists?

— Reply to this email directly or view it on GitHub https://github.com/civisanalytics/swagger-diff/issues/17#issuecomment-156236836 .

davidvc commented 8 years ago

Also, one other question: swagger-rb gem didn't support "deprecated". I have a local fix and submitted a pull request that hasn't been accepted. It looks like you fixed that. If I got the latest swagger-rb gem would it contain that fix? I don't see a commit that looks like what we need...

On Thu, Nov 12, 2015 at 1:23 PM David Van Couvering david@vancouvering.com wrote:

Do you still want them, even though I have verified they all work? I do see how it could have value, but I'm a bit hestitant to share a bunch of our internal API specs :(

On Thu, Nov 12, 2015 at 1:18 PM jeffreyc notifications@github.com wrote:

The attachment didn't seem to make it in the email reply, and I can't attach a tar.gz through GitHub. Any suggestions?

It looks like GitHub doesn't support non-text/non-image attachments. Could you share a Dropbox link, attach them as text, or create some gists?

— Reply to this email directly or view it on GitHub https://github.com/civisanalytics/swagger-diff/issues/17#issuecomment-156236836 .

jeffreyc commented 8 years ago

Do you still want them, even though I have verified they all work? I do see how it could have value, but I'm a bit hestitant to share a bunch of our internal API specs :(

Not at all! I was merely offering to help, so you didn't have to track down any issues. If you've tested and verified things work, great! I will happily release 1.0.5, with confidence that you won't encounter further problems as you test the rest of your specifications :)

Also, please feel free to edit your existing comments to remove your API specifications, and to provide dummy examples for any future issues. Being able to recreate the issue is very helpful, but we definitely don't need any private specifications.

Also, one other question: swagger-rb gem didn't support "deprecated". I have a local fix and submitted a pull request that hasn't been accepted. It looks like you fixed that. If I got the latest swagger-rb gem would it contain that fix? I don't see a commit that looks like what we need...

Alas, we worked around it by monkey patching your PR into swagger-rb. I don't know if swagger-rb is being actively maintained, but, with one small change, it met our needs perfectly, and I'm unaware of any other Ruby Swagger parsers.