search

ruby-grape / grape-swagger

Add OAPI/swagger v2.0 compliant documentation to your grape API
MIT License
1.09k stars 471 forks source link

Grape 2.2.0 compatibility #940

Closed padde closed 2 weeks ago

padde commented 2 weeks ago

Running on grape 2.0.2 resulted in the following exceptions:

NameError:
  uninitialized constant Grape::ContentTypes::CONTENT_TYPES
# ./lib/grape-swagger/endpoint.rb:15:in `content_types_for'

NoMethodError:
  undefined method `formatters' for module Grape::Formatter
# ./lib/grape-swagger/endpoint.rb:14:in `content_types_for'

This is due to internal refactorings in grape as of v2.2.0, specifically https://github.com/ruby-grape/grape/commit/fb67ea99

Fixes #939

padde commented 2 weeks ago

Instead of relying on a Grape constant that's not supposed to be part of the public interface, just copy it into grape-swagger? Expand CI to use multiple versions of Grape to make sure our compatibility is tested well.

@dblock sure, we could do that and it would vastly simplify the implementation, as we wouldn't have to massage the internal constants as much to make it work for our purposes here.

padde commented 2 weeks ago

@dblock looking further into the code that is used from grape here, it seems that there always has been a flaw in the way we retrieve content types in grape-swagger. Grape allows adding user-defined content types. However, if I get this right, grape-swagger never even considered the ones coming from the settings, just the default values from the internal constants that are now causing the issue. Let's continue discussion in https://github.com/ruby-grape/grape-swagger/issues/939#issuecomment-2355343643

padde commented 2 weeks ago

Can you try and please turn this into a matrix with two entries: a ruby version and a grape version?

@dblock sure. Honestly, I just expanded on the existing approach to avoid this discussion but I am happily reducing the duplication!

We don't need to test all possible permutations either.

That is a bit vague - which permutations should we test then, if not all? I will for now follow the "wasteful" approach with all permutations, feel free to tell me which ones to remove, if you like.