diff --git a/README.md b/README.md index f085bb1759..b44ec6f2ba 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ - [Param](#param) - [Describing Methods](#describing-methods) - [Parameters](#parameters) + - [Params Class](#params-class) - [Declared](#declared) - [Include Missing](#include-missing) - [Parameter Validation and Coercion](#parameter-validation-and-coercion) @@ -501,9 +502,19 @@ Route string parameters will have precedence. By default parameters are available as `ActiveSupport::HashWithIndifferentAccess`. This can be changed to, for example, Ruby `Hash` or `Hashie::Mash` for the entire API. -[TODO] +```ruby +class API < Grape::API + include Grape::Extensions::Hashie::Mash::ParamBuilder + + params do + optional :color, type: String + end + get do + params.color # instead of params[:color] + end +``` -The class can be overridden on individual parameter blocks using `build_with` as follows. +The class can also be overridden on individual parameter blocks using `build_with` as follows. ```ruby params do @@ -514,6 +525,8 @@ end In the example above, `params["color"]` will return `nil` since `params` is a plain `Hash`. +Available parameter builders are `Grape::Extensions::Hash::ParamBuilder`, `Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder` and `Grape::Extensions::Hashie::Mash::ParamBuilder`. + ### Declared Grape allows you to access only the parameters that have been declared by your `params` block. It filters out the params that have been passed, but are not allowed. Consider the following API endpoint: @@ -526,7 +539,7 @@ post 'users/signup' do end ```` -If we do not specify any params, `declared` will return an empty `ActiveSupport::HashWithIndifferentAccess` hash. +If you do not specify any parameters, `declared` will return an empty hash. **Request** @@ -543,8 +556,8 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d ```` -Once we add parameters requirements, grape will start returning only the declared params[: -] +Once we add parameters requirements, grape will start returning only the declared parameters. + ````ruby format :json @@ -579,7 +592,7 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d } ```` -The returned hash is a `ActiveSupport::HashWithIndifferentAccess` hash. +The returned hash is an `ActiveSupport::HashWithIndifferentAccess`. The `#declared` method is not available to `before` filters, as those are evaluated prior to parameter coercion. diff --git a/UPGRADING.md b/UPGRADING.md index 1a481f60e8..165e03443a 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -5,9 +5,33 @@ Upgrading Grape #### Changes in Parameter Class -The default class for `params` has changed from `Hashie::Mash` to `ActiveSupport::HashWithIndifferentAccess` and the `hashie` dependency has been removed. To restore the behavior of prior versions, add `hashie` to your `Gemfile` and use `TODO`. +The default class for `params` has changed from `Hashie::Mash` to `ActiveSupport::HashWithIndifferentAccess` and the `hashie` dependency has been removed. This means that by default you can no longer access parameters by method name. -[TODO] +``` +class API < Grape::API + params do + optional :color, type: String + end + get do + params[:color] # use params[:color] instead of params.color + end +end +``` + +To restore the behavior of prior versions, add `hashie` to your `Gemfile` and `include Grape::Extensions::Hashie::Mash::ParamBuilder` in your API. + +``` +class API < Grape::API + include Grape::Extensions::Hashie::Mash::ParamBuilder + + params do + optional :color, type: String + end + get do + # params.color works + end +end +``` This behavior can also be overridden on individual parameter blocks using `build_with`. @@ -26,7 +50,7 @@ def request end ``` -See [#1594](https://github.com/ruby-grape/grape/pull/1594) for more information. +See [#1610](https://github.com/ruby-grape/grape/pull/1610) for more information. ### Upgrading to >= 0.19.1 diff --git a/lib/grape.rb b/lib/grape.rb index e3f3ddd1c4..e5e9e7636f 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -26,7 +26,7 @@ I18n.load_path << File.expand_path('../grape/locale/en.yml', __FILE__) module Grape - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload eager_autoload do autoload :API @@ -47,14 +47,14 @@ module Grape end module Http - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload eager_autoload do autoload :Headers end end module Exceptions - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Base autoload :Validation autoload :ValidationArrayErrors @@ -78,22 +78,27 @@ module Exceptions end module Extensions - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :DeepMergeableHash autoload :DeepSymbolizeHash autoload :Hash - autoload :HashWithIndifferentAccess + + module ActiveSupport + extend ::ActiveSupport::Autoload + + autoload :HashWithIndifferentAccess + end module Hashie - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Mash end end module Middleware - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Base autoload :Versioner autoload :Formatter @@ -102,7 +107,7 @@ module Middleware autoload :Stack module Auth - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Base autoload :DSL autoload :StrategyInfo @@ -110,7 +115,7 @@ module Auth end module Versioner - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Path autoload :Header autoload :Param @@ -119,7 +124,7 @@ module Versioner end module Util - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :InheritableValues autoload :StackableValues autoload :ReverseStackableValues @@ -129,7 +134,7 @@ module Util end module ErrorFormatter - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Base autoload :Json autoload :Txt @@ -137,7 +142,7 @@ module ErrorFormatter end module Formatter - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Json autoload :SerializableHash autoload :Txt @@ -145,13 +150,13 @@ module Formatter end module Parser - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Json autoload :Xml end module DSL - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload eager_autoload do autoload :API autoload :Callbacks @@ -170,17 +175,17 @@ module DSL end class API - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Helpers end module Presenters - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :Presenter end module ServeFile - extend ActiveSupport::Autoload + extend ::ActiveSupport::Autoload autoload :FileResponse autoload :FileBody autoload :SendfileResponse diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 44eb99a9fb..2f427ad4fb 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -11,8 +11,9 @@ module Parameters # Set the module used to build the request.params. # # @param build_with the ParamBuilder module to use when building request.params - # Available builders are; - # * Grape::Extensions::HashWithIndifferentAccess::ParamBuilder (default) + # Available builders are: + # + # * Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder (default) # * Grape::Extensions::Hash::ParamBuilder # * Grape::Extensions::Hashie::Mash::ParamBuilder # @@ -30,7 +31,7 @@ module Parameters # end # end def build_with(build_with = nil) - @api.namespace_inheritable(:build_with, build_with) + @api.namespace_inheritable(:build_params_with, build_with) end # Include reusable params rules among current. diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 523dbdfde2..3ff2d6c380 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -237,7 +237,7 @@ def equals?(e) def run ActiveSupport::Notifications.instrument('endpoint_run.grape', endpoint: self, env: env) do @header = {} - @request = Grape::Request.new(env, build_params_with: namespace_inheritable(:build_with)) + @request = Grape::Request.new(env, build_params_with: namespace_inheritable(:build_params_with)) @params = @request.params @headers = @request.headers diff --git a/lib/grape/extensions/active_support/hash_with_indifferent_access.rb b/lib/grape/extensions/active_support/hash_with_indifferent_access.rb new file mode 100644 index 0000000000..291e34decd --- /dev/null +++ b/lib/grape/extensions/active_support/hash_with_indifferent_access.rb @@ -0,0 +1,25 @@ +module Grape + module Extensions + module ActiveSupport + module HashWithIndifferentAccess + extend ::ActiveSupport::Concern + + included do + namespace_inheritable(:build_params_with, Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder) + end + + def params_builder + Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + end + + module ParamBuilder + def build_params + params = ::ActiveSupport::HashWithIndifferentAccess[rack_params] + params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS] + params + end + end + end + end + end +end diff --git a/lib/grape/extensions/hash.rb b/lib/grape/extensions/hash.rb index 9785fd68c8..77d0757630 100644 --- a/lib/grape/extensions/hash.rb +++ b/lib/grape/extensions/hash.rb @@ -2,6 +2,12 @@ module Grape module Extensions module Hash module ParamBuilder + extend ::ActiveSupport::Concern + + included do + namespace_inheritable(:build_params_with, Grape::Extensions::Hash::ParamBuilder) + end + def build_params params = Grape::Extensions::DeepMergeableHash[rack_params] params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS] diff --git a/lib/grape/extensions/hash_with_indifferent_access.rb b/lib/grape/extensions/hash_with_indifferent_access.rb deleted file mode 100644 index 14608a0ef1..0000000000 --- a/lib/grape/extensions/hash_with_indifferent_access.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Grape - module Extensions - module HashWithIndifferentAccess - def params_builder - Grape::Extensions::HashWithIndifferentAccess::ParamBuilder - end - - module ParamBuilder - def build_params - params = ActiveSupport::HashWithIndifferentAccess[rack_params] - params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS] - params - end - end - end - end -end diff --git a/lib/grape/extensions/hashie/mash.rb b/lib/grape/extensions/hashie/mash.rb index e60677bfce..50958f3224 100644 --- a/lib/grape/extensions/hashie/mash.rb +++ b/lib/grape/extensions/hashie/mash.rb @@ -2,11 +2,17 @@ module Grape module Extensions module Hashie module Mash - def params_builder - Grape::Extensions::Hashie::Mash::ParamBuilder - end - module ParamBuilder + extend ::ActiveSupport::Concern + + included do + namespace_inheritable(:build_params_with, Grape::Extensions::Hashie::Mash::ParamBuilder) + end + + def params_builder + Grape::Extensions::Hashie::Mash::ParamBuilder + end + def build_params params = ::Hashie::Mash.new(rack_params) params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS] diff --git a/lib/grape/request.rb b/lib/grape/request.rb index deef61c454..0f11dff9e6 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -5,7 +5,7 @@ class Request < Rack::Request alias rack_params params def initialize(env, options = {}) - extend options[:build_params_with] || Grape::Extensions::HashWithIndifferentAccess::ParamBuilder + extend options[:build_params_with] || Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder super(env) end diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb index a67bd1db73..da28474eb0 100644 --- a/lib/grape/validations/types/custom_type_coercer.rb +++ b/lib/grape/validations/types/custom_type_coercer.rb @@ -147,7 +147,7 @@ def infer_type_check(type) # Enforce symbolized keys for complex types # by wrapping the coercion method such that # any Hash objects in the immediate heirarchy - # have their keys recursively symbolized + # have their keys recursively symbolized. # This helps common libs such as JSON to work easily. # # @param type see #new diff --git a/lib/grape/validations/validators/coerce.rb b/lib/grape/validations/validators/coerce.rb index b93834fe85..76db19d47c 100644 --- a/lib/grape/validations/validators/coerce.rb +++ b/lib/grape/validations/validators/coerce.rb @@ -11,7 +11,6 @@ def initialize(*_args) end def validate(request) - @request = request super end @@ -19,7 +18,7 @@ def validate_param!(attr_name, params) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:coerce) unless params.is_a? Hash new_value = coerce_value(params[attr_name]) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:coerce) unless valid_type?(new_value) - params[attr_name] = @request.respond_to?(:post_process_params) ? @request.post_process_params(new_value) : new_value + params[attr_name] = new_value end private @@ -38,6 +37,7 @@ def valid_type?(val) # Allow nil, to ignore when a parameter is absent return true if val.nil? + converter.value_coerced? val end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 270923ca73..1af9b73b94 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -250,88 +250,6 @@ def app end describe '#params' do - context 'build params with Hash' do - it 'should be Hash' do - subject.params do - build_with Grape::Extensions::Hash::ParamBuilder - end - - subject.get '/foo' do - params.class - end - - get '/foo' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Hash') - end - end - - context 'build params with HashWithIndifferentAccess' do - it 'should be ActiveSupport::HashWithIndifferentAccess' do - subject.params do - build_with Grape::Extensions::HashWithIndifferentAccess::ParamBuilder - end - - subject.get '/foo' do - params.class - end - - get '/foo' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('ActiveSupport::HashWithIndifferentAccess') - end - - it 'parse sub hash params' do - subject.params do - build_with Grape::Extensions::HashWithIndifferentAccess::ParamBuilder - - optional :a, type: Hash do - optional :b, type: Hash do - optional :c, type: String - end - optional :d, type: Array - end - end - subject.get '/foo' do - [params[:a]['b'][:c], params['a'][:d]] - end - - get '/foo', a: { b: { c: 'bar' }, d: ['foo'] } - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", ["foo"]]') - end - end - - context 'build params with Hashie::Mash' do - it 'should be Hashie::Mash' do - subject.params do - build_with Grape::Extensions::Hashie::Mash::ParamBuilder - end - - subject.get '/foo' do - params.class - end - - get '/foo' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Hashie::Mash') - end - - it 'is indifferent to key or symbol access' do - subject.params do - build_with Grape::Extensions::Hashie::Mash::ParamBuilder - requires :a, type: String - end - subject.get '/foo' do - [params[:a], params['a']] - end - - get '/foo', a: 'bar' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", "bar"]') - end - end - context 'default class' do it 'should be a ActiveSupport::HashWithIndifferentAccess' do subject.get '/foo' do @@ -1024,79 +942,6 @@ def app end end - context 'params presented as a plain Ruby Hash' do - it 'symbolizes params keys' do - subject.params do - optional :a, type: Hash do - optional :b, type: Hash do - optional :c, type: String - end - optional :d, type: Array - end - end - - subject.get '/foo' do - [params[:a][:b][:c], params[:a][:d]] - end - - get '/foo', 'a' => { b: { c: 'bar' }, 'd' => ['foo'] } - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", ["foo"]]') - end - - it 'symbolizes the params' do - subject.params do - build_with Grape::Extensions::Hash::ParamBuilder - requires :a, type: String - end - - subject.get '/foo' do - [params[:a], params['a']] - end - - get '/foo', a: 'bar' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", nil]') - end - end - - context 'params presented as a HashWithIndifferentAccess' do - it 'params are indifferent to symbol or string keys' do - subject.params do - build_with Grape::Extensions::HashWithIndifferentAccess::ParamBuilder - optional :a, type: Hash do - optional :b, type: Hash do - optional :c, type: String - end - optional :d, type: Array - end - end - - subject.get '/foo' do - [params[:a]['b'][:c], params['a'][:d]] - end - - get '/foo', 'a' => { b: { c: 'bar' }, 'd' => ['foo'] } - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", ["foo"]]') - end - - it 'responds to string keys' do - subject.params do - build_with Grape::Extensions::HashWithIndifferentAccess::ParamBuilder - requires :a, type: String - end - - subject.get '/foo' do - [params[:a], params['a']] - end - - get '/foo', a: 'bar' - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('["bar", "bar"]') - end - end - context 'sets a value to params' do it 'params' do subject.params do diff --git a/spec/grape/extensions/param_builders/hash_spec.rb b/spec/grape/extensions/param_builders/hash_spec.rb new file mode 100644 index 0000000000..27371fd983 --- /dev/null +++ b/spec/grape/extensions/param_builders/hash_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe Grape::Extensions::Hash::ParamBuilder do + subject { Class.new(Grape::API) } + + def app + subject + end + + describe 'in an endpoint' do + context '#params' do + before do + subject.params do + build_with Grape::Extensions::Hash::ParamBuilder + end + + subject.get do + params.class + end + end + + it 'should be of type Hash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Hash') + end + end + end + + describe 'in an api' do + before do + subject.send(:include, Grape::Extensions::Hash::ParamBuilder) + end + + context '#params' do + before do + subject.get do + params.class + end + end + + it 'should be Hash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Hash') + end + end + + it 'symbolizes params keys' do + subject.params do + optional :a, type: Hash do + optional :b, type: Hash do + optional :c, type: String + end + optional :d, type: Array + end + end + + subject.get '/foo' do + [params[:a][:b][:c], params[:a][:d]] + end + + get '/foo', 'a' => { b: { c: 'bar' }, 'd' => ['foo'] } + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", ["foo"]]') + end + + it 'symbolizes the params' do + subject.params do + build_with Grape::Extensions::Hash::ParamBuilder + requires :a, type: String + end + + subject.get '/foo' do + [params[:a], params['a']] + end + + get '/foo', a: 'bar' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", nil]') + 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 new file mode 100644 index 0000000000..1c2ea2c4f9 --- /dev/null +++ b/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder do + subject { Class.new(Grape::API) } + + def app + subject + end + + describe 'in an endpoint' do + context '#params' do + before do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + end + + subject.get do + params.class + end + end + + it 'should be of type Hash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('ActiveSupport::HashWithIndifferentAccess') + end + end + end + + describe 'in an api' do + before do + subject.send(:include, Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder) + end + + context '#params' do + before do + subject.get do + params.class + end + end + + it 'is a Hash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('ActiveSupport::HashWithIndifferentAccess') + end + + it 'parses sub hash params' do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + + optional :a, type: Hash do + optional :b, type: Hash do + optional :c, type: String + end + optional :d, type: Array + end + end + + subject.get '/foo' do + [params[:a]['b'][:c], params['a'][:d]] + end + + get '/foo', a: { b: { c: 'bar' }, d: ['foo'] } + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", ["foo"]]') + end + + it 'params are indifferent to symbol or string keys' do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + optional :a, type: Hash do + optional :b, type: Hash do + optional :c, type: String + end + optional :d, type: Array + end + end + + subject.get '/foo' do + [params[:a]['b'][:c], params['a'][:d]] + end + + get '/foo', 'a' => { b: { c: 'bar' }, 'd' => ['foo'] } + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", ["foo"]]') + end + + it 'responds to string keys' do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + requires :a, type: String + end + + subject.get '/foo' do + [params[:a], params['a']] + end + + get '/foo', a: 'bar' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", "bar"]') + end + 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 new file mode 100644 index 0000000000..0a5975f2d5 --- /dev/null +++ b/spec/grape/extensions/param_builders/hashie/mash_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Grape::Extensions::Hashie::Mash::ParamBuilder do + subject { Class.new(Grape::API) } + + def app + subject + end + + describe 'in an endpoint' do + context '#params' do + before do + subject.params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder + end + + subject.get do + params.class + end + end + + it 'should be of type Hashie::Mash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Hashie::Mash') + end + end + end + + describe 'in an api' do + before do + subject.send(:include, Grape::Extensions::Hashie::Mash::ParamBuilder) + end + + context '#params' do + before do + subject.get do + params.class + end + end + + it 'should be Hashie::Mash' do + get '/' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Hashie::Mash') + end + end + + context 'in a nested namespace api' do + before do + subject.namespace :foo do + get do + params.class + end + end + end + + it 'should be Hashie::Mash' do + get '/foo' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Hashie::Mash') + end + end + + it 'is indifferent to key or symbol access' do + subject.params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder + requires :a, type: String + end + subject.get '/foo' do + [params[:a], params['a']] + end + + get '/foo', a: 'bar' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('["bar", "bar"]') + end + end +end