Skip to content

Commit 45abe0d

Browse files
ericproulxericproulx
authored
Deprecates Grape's Extensions for ParamsBuilder in favor of build_with (#2540)
* Deprecates Grape's Extensions for ParamsBuilder in favor of build_with Adds params_builder.rb registration process Update README.md Update UPGRADING.md Add exceptions query_parsing.rb and unknown_params_builder.rb * Fix hashie specs * Fix cops * Remove query_parsing part * Fix has has * Revert Grape.config.param_builder * Add CHANGELOG.md * Fixes
1 parent 0f57e01 commit 45abe0d

26 files changed

+496
-260
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* [#2535](https://github.com/ruby-grape/grape/pull/2535): Delegates calls to inner objects - [@ericproulx](https://github.com/ericproulx).
77
* [#2537](https://github.com/ruby-grape/grape/pull/2537): Use activesupport `try` pattern - [@ericproulx](https://github.com/ericproulx).
88
* [#2536](https://github.com/ruby-grape/grape/pull/2536): Update normalize_path like Rails - [@ericproulx](https://github.com/ericproulx).
9+
* [#2540](https://github.com/ruby-grape/grape/pull/2540): Introduce Params builder with symbolized short name - [@ericproulx](https://github.com/ericproulx).
910
* Your contribution here.
1011

1112
#### Fixes

‎README.md

+8-6
Original file line numberDiff line numberDiff line change
@@ -719,10 +719,13 @@ For example, for the `param_builder`, the following code could run in an initial
719719

720720
```ruby
721721
Grape.configure do |config|
722-
config.param_builder = Grape::Extensions::Hashie::Mash::ParamBuilder
722+
config.param_builder = :hashie_mash
723723
end
724724
```
725725

726+
Available parameter builders are `:hash`, `:hash_with_indifferent_access`, and `:hashie_mash`.
727+
See [params_builder](lib/grape/params_builder).
728+
726729
You can also configure a single API:
727730

728731
```ruby
@@ -789,7 +792,7 @@ By default parameters are available as `ActiveSupport::HashWithIndifferentAccess
789792

790793
```ruby
791794
class API < Grape::API
792-
include Grape::Extensions::Hashie::Mash::ParamBuilder
795+
build_with :hashie_mash
793796

794797
params do
795798
optional :color, type: String
@@ -803,16 +806,15 @@ The class can also be overridden on individual parameter blocks using `build_wit
803806

804807
```ruby
805808
params do
806-
build_with Grape::Extensions::Hash::ParamBuilder
809+
build_with :hash
807810
optional :color, type: String
808811
end
809812
```
810813

811-
Or globally with the [Configuration](#configuration) `Grape.configure.param_builder`.
812-
813814
In the example above, `params["color"]` will return `nil` since `params` is a plain `Hash`.
814815

815-
Available parameter builders are `Grape::Extensions::Hash::ParamBuilder`, `Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder` and `Grape::Extensions::Hashie::Mash::ParamBuilder`.
816+
Available parameter builders are `:hash`, `:hash_with_indifferent_access`, and `:hashie_mash`.
817+
See [params_builder](lib/grape/params_builder).
816818

817819
### Declared
818820

‎UPGRADING.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
Upgrading Grape
22
===============
33

4+
#### Params Builder
5+
6+
- Passing a class to `build_with` or `Grape.config.param_builder` has been deprecated in favor of a symbolized short_name. See `SHORTNAME_LOOKUP` in [params_builder](lib/grape/params_builder.rb).
7+
- Including Grape's extensions like `Grape::Extensions::Hashie::Mash::ParamBuilder` has been deprecated in favor of using `build_with` at the route level.
8+
49
### Upgrading to >= 2.4.0
510

611
#### Custom Validators

‎lib/grape.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def self.deprecator
6464
end
6565

6666
configure do |config|
67-
config.param_builder = Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder
67+
config.param_builder = :hash_with_indifferent_access
6868
config.compile_methods!
6969
end
7070
end

‎lib/grape/dsl/parameters.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ module Parameters
2323
# class API < Grape::API
2424
# desc "Get collection"
2525
# params do
26-
# build_with Grape::Extensions::Hashie::Mash::ParamBuilder
26+
# build_with :hashie_mash
2727
# requires :user_id, type: Integer
2828
# end
2929
# get do
3030
# params['user_id']
3131
# end
3232
# end
33-
def build_with(build_with = nil)
33+
def build_with(build_with)
3434
@api.namespace_inheritable(:build_params_with, build_with)
3535
end
3636

‎lib/grape/dsl/routing.rb

+4
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ def scope(_name = nil, &block)
6767
end
6868
end
6969

70+
def build_with(build_with)
71+
namespace_inheritable(:build_params_with, build_with)
72+
end
73+
7074
# Do not route HEAD requests to GET requests automatically.
7175
def do_not_route_head!
7276
namespace_inheritable(:do_not_route_head, true)

‎lib/grape/exceptions/unknown_params_builder.rb

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module Exceptions
5+
class UnknownParamsBuilder < Base
6+
def initialize(params_builder_type)
7+
super(message: compose_message(:unknown_params_builder, params_builder_type: params_builder_type))
8+
end
9+
end
10+
end
11+
end

‎lib/grape/extensions/active_support/hash_with_indifferent_access.rb

+2-5
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,8 @@ module ParamBuilder
88
extend ::ActiveSupport::Concern
99

1010
included do
11-
namespace_inheritable(:build_params_with, Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder)
12-
end
13-
14-
def params_builder
15-
Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder
11+
Grape.deprecator.warn 'This concern has been deprecated. Use `build_with` with one of the following short_name (:hash, :hash_with_indifferent_access, :hashie_mash) instead.'
12+
namespace_inheritable(:build_params_with, :hash_with_indifferent_access)
1613
end
1714

1815
def build_params

‎lib/grape/extensions/hash.rb

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ module ParamBuilder
77
extend ::ActiveSupport::Concern
88

99
included do
10-
namespace_inheritable(:build_params_with, Grape::Extensions::Hash::ParamBuilder)
10+
Grape.deprecator.warn 'This concern has been deprecated. Use `build_with` with one of the following short_name (:hash, :hash_with_indifferent_access, :hashie_mash) instead.'
11+
namespace_inheritable(:build_params_with, :hash)
1112
end
1213

1314
def build_params

‎lib/grape/extensions/hashie/mash.rb

+3-5
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ module Hashie
66
module Mash
77
module ParamBuilder
88
extend ::ActiveSupport::Concern
9-
included do
10-
namespace_inheritable(:build_params_with, Grape::Extensions::Hashie::Mash::ParamBuilder)
11-
end
129

13-
def params_builder
14-
Grape::Extensions::Hashie::Mash::ParamBuilder
10+
included do
11+
Grape.deprecator.warn 'This concern has been deprecated. Use `build_with` with one of the following short_name (:hash, :hash_with_indifferent_access, :hashie_mash) instead.'
12+
namespace_inheritable(:build_params_with, :hashie_mash)
1513
end
1614

1715
def build_params

‎lib/grape/locale/en.yml

+2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ en:
3333
problem: 'unknown :using for versioner: %{strategy}'
3434
resolution: 'available strategy for :using is :path, :header, :accept_version_header, :param'
3535
unknown_validator: 'unknown validator: %{validator_type}'
36+
unknown_params_builder: 'unknown params_builder: %{params_builder_type}'
3637
unknown_options: 'unknown options: %{options}'
3738
unknown_parameter: 'unknown parameter: %{param}'
3839
incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}'
@@ -57,3 +58,4 @@ en:
5758
problem: 'invalid version header'
5859
resolution: '%{message}'
5960
invalid_response: 'Invalid response'
61+
query_parsing: 'query params are not parsable'

‎lib/grape/params_builder.rb

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module ParamsBuilder
5+
extend Grape::Util::Registry
6+
7+
SHORT_NAME_LOOKUP = {
8+
'Grape::Extensions::Hash::ParamBuilder' => :hash,
9+
'Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder' => :hash_with_indifferent_access,
10+
'Grape::Extensions::Hashie::Mash::ParamBuilder' => :hashie_mash
11+
}.freeze
12+
13+
module_function
14+
15+
def params_builder_for(short_name)
16+
verified_short_name = verify_short_name!(short_name)
17+
18+
raise Grape::Exceptions::UnknownParamsBuilder, verified_short_name unless registry.key?(verified_short_name)
19+
20+
registry[verified_short_name]
21+
end
22+
23+
def verify_short_name!(short_name)
24+
return short_name if short_name.is_a?(Symbol)
25+
26+
class_name = short_name.name
27+
SHORT_NAME_LOOKUP[class_name].tap do |real_short_name|
28+
Grape.deprecator.warn "#{class_name} has been deprecated. Use short name :#{real_short_name} instead."
29+
end
30+
end
31+
end
32+
end

‎lib/grape/params_builder/base.rb

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module ParamsBuilder
5+
class Base
6+
class << self
7+
def call(_params)
8+
raise NotImplementedError
9+
end
10+
11+
def inherited(klass)
12+
super
13+
ParamsBuilder.register(klass)
14+
end
15+
end
16+
end
17+
end
18+
end

‎lib/grape/params_builder/hash.rb

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module ParamsBuilder
5+
class Hash < Base
6+
def self.call(params)
7+
params.deep_symbolize_keys
8+
end
9+
end
10+
end
11+
end

‎lib/grape/params_builder/hash_with_indifferent_access.rb

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module ParamsBuilder
5+
class HashWithIndifferentAccess < Base
6+
def self.call(params)
7+
params.with_indifferent_access
8+
end
9+
end
10+
end
11+
end

‎lib/grape/params_builder/hashie_mash.rb

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module ParamsBuilder
5+
class HashieMash < Base
6+
def self.call(params)
7+
::Hashie::Mash.new(params)
8+
end
9+
end
10+
end
11+
end

‎lib/grape/request.rb

+15-9
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,38 @@
22

33
module Grape
44
class Request < Rack::Request
5+
DEFAULT_PARAMS_BUILDER = :hash_with_indifferent_access
56
HTTP_PREFIX = 'HTTP_'
67

78
alias rack_params params
89

910
def initialize(env, build_params_with: nil)
10-
extend build_params_with || Grape.config.param_builder
1111
super(env)
12+
@params_builder = Grape::ParamsBuilder.params_builder_for(build_params_with || Grape.config.param_builder)
1213
end
1314

1415
def params
15-
@params ||= build_params
16-
rescue EOFError
17-
raise Grape::Exceptions::EmptyMessageBody.new(content_type)
18-
rescue Rack::Multipart::MultipartPartLimitError
19-
raise Grape::Exceptions::TooManyMultipartFiles.new(Rack::Utils.multipart_part_limit)
16+
@params ||= make_params
2017
end
2118

2219
def headers
2320
@headers ||= build_headers
2421
end
2522

26-
private
27-
23+
# needs to be public until extensions param_builder are removed
2824
def grape_routing_args
2925
# preserve version from query string parameters
30-
env[Grape::Env::GRAPE_ROUTING_ARGS].except(:version, :route_info)
26+
env[Grape::Env::GRAPE_ROUTING_ARGS]&.except(:version, :route_info) || {}
27+
end
28+
29+
private
30+
31+
def make_params
32+
@params_builder.call(rack_params).deep_merge!(grape_routing_args)
33+
rescue EOFError
34+
raise Grape::Exceptions::EmptyMessageBody.new(content_type)
35+
rescue Rack::Multipart::MultipartPartLimitError
36+
raise Grape::Exceptions::TooManyMultipartFiles.new(Rack::Utils.multipart_part_limit)
3137
end
3238

3339
def build_headers

‎spec/grape/api_spec.rb

+21
Original file line numberDiff line numberDiff line change
@@ -4694,4 +4694,25 @@ def uniqe_id_route
46944694

46954695
it { is_expected.to be_bad_request }
46964696
end
4697+
4698+
describe '.build_with' do
4699+
let(:app) do
4700+
Class.new(described_class) do
4701+
build_with :unknown
4702+
params do
4703+
requires :a_param, type: Integer
4704+
end
4705+
get
4706+
end
4707+
end
4708+
4709+
before do
4710+
get '/'
4711+
end
4712+
4713+
it 'raises an UnknownParamsBuilder error' do
4714+
expect(last_response).to be_server_error
4715+
expect(last_response.body).to eq('unknown params_builder: unknown')
4716+
end
4717+
end
46974718
end

‎spec/grape/endpoint/declared_spec.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
context 'when params are not built with default class' do
4747
it 'returns an object that corresponds with the params class - hash with indifferent access' do
4848
subject.params do
49-
build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder
49+
build_with :hash_with_indifferent_access
5050
end
5151
subject.get '/declared' do
5252
d = declared(params, include_missing: true)
@@ -59,7 +59,7 @@
5959

6060
it 'returns an object that corresponds with the params class - hash' do
6161
subject.params do
62-
build_with Grape::Extensions::Hash::ParamBuilder
62+
build_with :hash
6363
end
6464
subject.get '/declared' do
6565
d = declared(params, include_missing: true)

0 commit comments

Comments
 (0)