Allow using nicknames for body definitions

[magni-]

We have an API defined roughly as follows:

resource :things do
  desc "Batch update things" do
    nickname "put-things"
  end
  params do
    # a bunch of params
  end
  put "/" do
    # ...
  end

  segment "/:id" do
    desc "Update thing" do
      nickname "put-thing"
    end
    params do
      # a bunch of other params
    end
    put "/" do
      # ...
    end
  end
end

We noticed the docs for our API were incorrect. Both paths.things.put.parameters.0.schema.$ref and paths.things{id}.put.parameters.1.schema.$ref (parameters.0 is for the path :id parameter) are pointing to the same thing: "#/definitions/putThings. It turns out the put-thing body parameter definition is overriding the previous put-things body parameter definition, rather than defining its own thing. It's been a while since I've dug into the grape-swagger code, so I went with the (IMO) easy fix: using route nicknames if they're available to name the body parameter definitions.

Edit: Based on the changes I had to make to get specs to pass, this change would also have solved our issue without using route nicknames, since the references become putTests and putTestsId.

Related issue: ruby-grape/grape#2035

[magni-]

I'm not quite sure why the build is erroring on Ruby@head, seems unrelated to the changes in this PR?

NoMethodError:
  undefined method `<<' for {:constraint=>"[^/\\?#.]"}:Hash

# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:59:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:112:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:39:in `block in parse'
# <internal:kernel>:90:in `tap'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:39:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:71:in `node'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-grape-1.0.2/lib/mustermann/grape.rb:20:in `block in <class:Grape>'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:91:in `read'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:59:in `block in parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:58:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/node.rb:171:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:71:in `node'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:59:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/parser.rb:17:in `parse'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/pattern.rb:91:in `block in to_ast'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/equality_map.rb:43:in `fetch'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/pattern.rb:90:in `to_ast'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/ast/pattern.rb:81:in `compile'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/regexp_based.rb:19:in `initialize'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/pattern.rb:59:in `new'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/pattern.rb:59:in `block in new'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/equality_map.rb:43:in `fetch'
# ./vendor/bundle/ruby/3.2.0+1/gems/mustermann-1.1.1/lib/mustermann/pattern.rb:59:in `new'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/router/pattern.rb:23:in `initialize'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/router/route.rb:53:in `new'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/router/route.rb:53:in `initialize'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:168:in `new'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:168:in `block in to_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:205:in `block (2 levels) in map_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:205:in `map'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:205:in `block in map_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:205:in `map'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:205:in `map_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:165:in `to_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:138:in `routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:138:in `collect'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/endpoint.rb:138:in `routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api/instance.rb:97:in `map'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api/instance.rb:97:in `prepare_routes'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/dsl/routing.rb:190:in `routes'
# ./lib/grape-swagger.rb:27:in `combine_routes'
# ./lib/grape-swagger.rb:131:in `add_swagger_documentation'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api.rb:160:in `replay_step_on'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api.rb:151:in `block in add_setup'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api.rb:150:in `each'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api.rb:150:in `add_setup'
# ./vendor/bundle/ruby/3.2.0+1/gems/grape-1.6.2/lib/grape/api.rb:55:in `block (2 levels) in override_all_methods!'
# ./spec/lib/oapi_tasks_spec.rb:22:in `<class:Base>'
# ./spec/lib/oapi_tasks_spec.rb:19:in `<module:Api>'
# ./spec/lib/oapi_tasks_spec.rb:6:in `block in <top (required)>'
# ./spec/lib/oapi_tasks_spec.rb:5:in `<top (required)>'

[magni-]

I'm still seeing the same failure on mustermann@2.0.1. It looks like it would work if we added nil before constraint here: https://github.com/ruby-grape/mustermann-grape/blob/v1.0.2/lib/mustermann/grape.rb#L20 ? But the proper fix seems to be an update to mustermann itself, I started a PR here: sinatra/mustermann#134

Either way I don't think this should be a blocker for this PR 🙇🏼

[magni-]

@LeFnord anything else I should add to this PR, or does this look good to go? 🙇🏼 Thanks!

[magni-]

I'm still seeing the same failure on mustermann@2.0.1. It looks like it would work if we added nil before constraint here: https://github.com/ruby-grape/mustermann-grape/blob/v1.0.2/lib/mustermann/grape.rb#L20 ? But the proper fix seems to be an update to mustermann itself, I started a PR here: sinatra/mustermann#134

Either way I don't think this should be a blocker for this PR 🙇🏼

mustermann v2.0.2 was released and the build is now fully green 🎉

[LeFnord]

thanks @magni- … it seems it changes something so maybe add an entry to UPGRADING.md

and one question: should we add gem musterman, '~> 2.0.2' to ensure it works correct?

[magni-]

it seems it changes something so maybe add an entry to UPGRADING.md

👍🏼

and one question: should we add gem musterman, '~> 2.0.2' to ensure it works correct?

Good question. The build works fine on all released versions of Ruby, it's just the upcoming 3.2 (which won't be released for another 5 months) which had issues. If we add an explicit requirement, might as well make it to ~> 3 which was just released (the only change from 2.0.2 is dropping support for Ruby 2.5 and below. grape-swagger already requires Ruby 2.7+).

[magni-]

@LeFnord I've updated UPGRADING. For the version number, this feels more like a 1.5.0 than a 1.4.3 ?

[LeFnord]

@magni- … yeah, definitely will it be 1.5.0 …
what is with mustermann, should it be aded to the Gemfile?

besides that it looks good … great job, tahnk you

[magni-]

@magni- … yeah, definitely will it be 1.5.0 … what is with mustermann, should it be aded to the Gemfile?

I think it makes more sense to update the requirement in mustermann-grape instead? https://github.com/ruby-grape/mustermann-grape/blob/master/mustermann-grape.gemspec#L20

[LeFnord]

ok makes sense :)

[magni-]

@LeFnord could you release this to RubyGems? Thank you! 🙇🏼

[LeFnord]

done