diff --git a/CHANGELOG.md b/CHANGELOG.md index f007545789..381d14d2a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ * [#2387](https://github.com/ruby-grape/grape/pull/2387): Fix rubygems version within workflows - [@ericproulx](https://github.com/ericproulx). * [#2405](https://github.com/ruby-grape/grape/pull/2405): Fix edge workflow - [@ericproulx](https://github.com/ericproulx). * [#2414](https://github.com/ruby-grape/grape/pull/2414): Fix Rack::Lint missing content-type - [@ericproulx](https://github.com/ericproulx). +* [#2378](https://github.com/ruby-grape/grape/pull/2378): Do not overwrite `route_param` with a regular one if they share same name - [@arg](https://github.com/arg). * Your contribution here. ### 2.0.0 (2023/11/11) diff --git a/README.md b/README.md index f6c8ddf227..c15c5f1340 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ - [Include Parent Namespaces](#include-parent-namespaces) - [Include Missing](#include-missing) - [Evaluate Given](#evaluate-given) + - [Parameter Precedence](#parameter-precedence) - [Parameter Validation and Coercion](#parameter-validation-and-coercion) - [Supported Parameter Types](#supported-parameter-types) - [Integer/Fixnum and Coercions](#integerfixnum-and-coercions) @@ -1198,6 +1199,35 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/child -d '{"chil } ```` +### Parameter Precedence + +Using `route_param` takes higher precedence over a regular parameter defined with same name: + +```ruby +params do + requires :foo, type: String +end +route_param :foo do + get do + { value: params[:foo] } + end +end +``` + +**Request** + +```bash +curl -X POST -H "Content-Type: application/json" localhost:9292/bar -d '{"foo": "baz"}' +``` + +**Response** + +```json +{ + "value": "bar" +} +``` + ## Parameter Validation and Coercion You can define validations and coercion options for your parameters using a `params` block. diff --git a/UPGRADING.md b/UPGRADING.md index 900d0a340a..d7ef9248b0 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -9,6 +9,59 @@ The `rack_response` method has been deprecated and the `error_response` method h See [#2414](https://github.com/ruby-grape/grape/pull/2414) for more information. +#### Change in parameters precedence + +When using together with `Grape::Extensions::Hash::ParamBuilder`, `route_param` takes higher precedence over a regular parameter defined with same name, which now matches the default param builder behavior. + +This was a regression introduced by [#2326](https://github.com/ruby-grape/grape/pull/2326) in Grape v1.8.0. + +```ruby +grape.configure do |config| + config.param_builder = Grape::Extensions::Hash::ParamBuilder +end + +params do + requires :foo, type: String +end +route_param :foo do + get do + { value: params[:foo] } + end +end +``` + +Request: + +```bash +curl -X POST -H "Content-Type: application/json" localhost:9292/bar -d '{"foo": "baz"}' +``` + +Response prior to v1.8.0: + +```json +{ + "value": "bar" +} +``` + +v1.8.0..v2.0.0: + +```json +{ + "value": "baz" +} +``` + +v2.1.0+: + +```json +{ + "value": "bar" +} +``` + +See [#2378](https://github.com/ruby-grape/grape/pull/2378) for details. + #### Grape::Router::Route.route_xxx methods have been removed - `route_method` is accessible through `request_method` diff --git a/lib/grape/extensions/hash.rb b/lib/grape/extensions/hash.rb index 3ad07b210e..cb8bd4b061 100644 --- a/lib/grape/extensions/hash.rb +++ b/lib/grape/extensions/hash.rb @@ -12,8 +12,12 @@ module ParamBuilder def build_params rack_params.deep_dup.tap do |params| - params.deep_merge!(grape_routing_args) if env.key?(Grape::Env::GRAPE_ROUTING_ARGS) params.deep_symbolize_keys! + + if env.key?(Grape::Env::GRAPE_ROUTING_ARGS) + grape_routing_args.deep_symbolize_keys! + params.deep_merge!(grape_routing_args) + end end end end diff --git a/spec/grape/extensions/param_builders/hash_spec.rb b/spec/grape/extensions/param_builders/hash_spec.rb index e35096bf78..872330bb45 100644 --- a/spec/grape/extensions/param_builders/hash_spec.rb +++ b/spec/grape/extensions/param_builders/hash_spec.rb @@ -79,5 +79,34 @@ def app expect(last_response.status).to eq(200) expect(last_response.body).to eq('["bar", nil]') end + + it 'does not overwrite route_param with a regular param if they have same name' do + subject.namespace :route_param do + route_param :foo do + get { params.to_json } + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('{"foo":"bar"}') + end + + it 'does not overwrite route_param with a defined regular param if they have same name' do + subject.namespace :route_param do + params do + requires :foo, type: String + end + route_param :foo do + get do + params[:foo] + end + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('bar') + end end end diff --git a/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb b/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb index 992a07789c..0f741193b2 100644 --- a/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb +++ b/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb @@ -101,5 +101,34 @@ def app expect(last_response.body).to eq('["bar", "bar"]') end end + + it 'does not overwrite route_param with a regular param if they have same name' do + subject.namespace :route_param do + route_param :foo do + get { params.to_json } + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('{"foo":"bar"}') + end + + it 'does not overwrite route_param with a defined regular param if they have same name' do + subject.namespace :route_param do + params do + requires :foo, type: String + end + route_param :foo do + get do + [params[:foo], params['foo']] + end + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", "bar"]') + end end end diff --git a/spec/grape/extensions/param_builders/hashie/mash_spec.rb b/spec/grape/extensions/param_builders/hashie/mash_spec.rb index 54a1879829..7b58c1d3f8 100644 --- a/spec/grape/extensions/param_builders/hashie/mash_spec.rb +++ b/spec/grape/extensions/param_builders/hashie/mash_spec.rb @@ -75,5 +75,35 @@ def app expect(last_response.status).to eq(200) expect(last_response.body).to eq('["bar", "bar"]') end + + it 'does not overwrite route_param with a regular param if they have same name' do + subject.namespace :route_param do + route_param :foo do + get { params.to_json } + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('{"foo":"bar"}') + end + + it 'does not overwrite route_param with a defined regular param if they have same name' do + subject.namespace :route_param do + params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder # rubocop:disable RSpec/DescribedClass + requires :foo, type: String + end + route_param :foo do + get do + [params[:foo], params['foo']] + end + end + end + + get '/route_param/bar', foo: 'baz' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", "bar"]') + end end end