search

civisanalytics / swagger-diff

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

Support for extensions #20

Closed davidvc closed 8 years ago

davidvc commented 8 years ago

A colleague added an extension to our Swagger (using the @Extension annotation for Jersey) that looks like this:

extensions = { @Extension(properties = { @ExtensionProperty(name = "x-cached", value = "true") }) })

This causes swagger-diff to fail:

   Hashie::CoercionError: Cannot coerce property "get" from Hash to Swagger::V2::Operation: The property 'x-cached' is not defined for Swagger::V2::Operation.
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/extensions/coercion.rb:46:in `rescue in set_value_with_coercion'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/extensions/coercion.rb:34:in `set_value_with_coercion'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/extensions/indifferent_access.rb:107:in `indifferent_writer'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/dash.rb:168:in `block in initialize_attributes'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/dash.rb:167:in `each_pair'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/dash.rb:167:in `initialize_attributes'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/hashie-3.3.2/lib/hashie/dash.rb:99:in `initialize'
    /home/jenkins/.rvm/gems/ruby-2.1.2/gems/swagger-core-0.2.3/lib/swagger/swagger_object.rb:16:in `initialize'

Is there any chance we could fix this?

Thanks,

David

jeffreyc commented 8 years ago

Thank you for your feedback! It looks like this is an issue with swagger-rb, the underlying library swagger-diff uses to parse Swagger specifications. Unfortunately, we’re juggling a few different priorities at this time, and probably won’t get to this quickly. If you’re interested, we’d welcome a PR addressing this issue. It looks like there are a few possible ways to approach this:

  1. File an issue with swagger-rb and wait for them to fix it. However, swagger-rb appears to be inactive, or possibly abandoned, so I don’t anticipate any issue being addressed quickly.
  2. Add another monkey patch to patch.rb and a new spec to specification_spec.rb. This is a stopgap, but should be relatively easy.
  3. Replace the Swagger parser. ruby_swagger looks to be a viable candidate that's under active development, though it's also pretty young, so may not be feature-complete yet. swagger_parser is another candidate. It's less active, though also has no outstanding issues/PRs, so may just be stable. This is better than a monkey patch, but would be a lot of work.

If you opt for 2 or 3, you may also want to file an issue with swagger-rb so they’re aware and can address the underlying problem if and when they have time.

Either way, we’d be happy to review a PR, and appreciate the contribution!

davidvc commented 8 years ago

Thanks! I'll consider these possibilities. We, like you, are juggling priorities :)

On Thu, Jan 7, 2016 at 8:50 AM jeffreyc notifications@github.com wrote:

Thank you for your feedback! It looks like this is an issue with swagger-rb https://github.com/swagger-rb/swagger-rb, the underlying library swagger-diff uses to parse Swagger specifications. Unfortunately, we’re juggling a few different priorities at this time, and probably won’t get to this quickly. If you’re interested, we’d welcome a PR addressing this issue. It looks like there are a few possible ways to approach this:

1.

File an issue https://github.com/swagger-rb/swagger-rb/issues with swagger-rb and wait for them to fix it. However, swagger-rb appears to be inactive, or possibly abandoned, so I don’t anticipate any issue being addressed quickly. 2.

Add another monkey patch to patch.rb https://github.com/civisanalytics/swagger-diff/blob/master/lib/swagger/diff/patch.rb and a new spec to specification_spec.rb https://github.com/civisanalytics/swagger-diff/blob/master/spec/swagger/diff/specification_spec.rb#L181. This is a stopgap, but should be relatively easy. 3.

Replace the Swagger parser. ruby_swagger https://rubygems.org/gems/ruby-swagger looks to be a viable candidate that's under active development, though it's also pretty young, so may not be feature-complete yet. swagger_parser https://rubygems.org/gems/swagger_parser is another candidate. It's less active, though also has no outstanding issues/PRs, so may just be stable. This is better than a monkey patch, but would be a lot of work.

If you opt for 2 or 3, you may also want to file an issue with swagger-rb so they’re aware and can address the underlying problem if and when they have time.

Either way, we’d be happy to review a PR, and appreciate the contribution!

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

jeffreyc commented 8 years ago

I just released Swagger::Diff 1.1.0. Among other improvements, it replaces the underlying parser, and should now support your Swagger specification. If you encounter any other problems, please let us know.

davidvc commented 8 years ago

That's great, thanks! I'll give it a shot as soon as I can.

On Fri, May 20, 2016 at 2:25 PM jeffreyc notifications@github.com wrote:

I just released Swagger::Diff 1.1.0. Among other improvements, it replaces the underlying parser, and should now support your Swagger specification. If you encounter any other problems, please let us know.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/civisanalytics/swagger-diff/issues/20#issuecomment-220722304