From 1bd1a21525e58768c2188a291488d0b3ba2832c5 Mon Sep 17 00:00:00 2001 From: James McCarthy Date: Sun, 9 Apr 2017 16:12:54 -0400 Subject: [PATCH] Replace Hashie::Mash with ActiveSupport::HashWithIndifferentAccess. --- .gitignore | 3 + CHANGELOG.md | 1 + Gemfile | 1 + README.md | 49 +++-- UPGRADING.md | 27 +++ gemfiles/rack_1.5.2.gemfile | 1 + gemfiles/rack_edge.gemfile | 1 + gemfiles/rails_3.gemfile | 1 + gemfiles/rails_4.gemfile | 1 + gemfiles/rails_5.gemfile | 1 + gemfiles/rails_edge.gemfile | 1 + grape.gemspec | 1 - lib/grape.rb | 17 +- lib/grape/dsl/inside_route.rb | 4 +- lib/grape/dsl/parameters.rb | 25 +++ lib/grape/endpoint.rb | 7 +- lib/grape/extensions/deep_mergeable_hash.rb | 19 ++ lib/grape/extensions/deep_symbolize_hash.rb | 22 ++ lib/grape/extensions/hash.rb | 17 ++ .../hash_with_indifferent_access.rb | 17 ++ lib/grape/extensions/hashie/mash.rb | 19 ++ lib/grape/middleware/globals.rb | 2 +- lib/grape/request.rb | 21 +- .../validations/types/custom_type_coercer.rb | 22 +- lib/grape/validations/types/file.rb | 5 +- lib/grape/validations/validators/coerce.rb | 8 +- spec/grape/endpoint_spec.rb | 200 ++++++++++++++++++ spec/grape/request_spec.rb | 16 +- .../validations/validators/coerce_spec.rb | 88 +++++++- 29 files changed, 536 insertions(+), 61 deletions(-) create mode 100644 lib/grape/extensions/deep_mergeable_hash.rb create mode 100644 lib/grape/extensions/deep_symbolize_hash.rb create mode 100644 lib/grape/extensions/hash.rb create mode 100644 lib/grape/extensions/hash_with_indifferent_access.rb create mode 100644 lib/grape/extensions/hashie/mash.rb diff --git a/.gitignore b/.gitignore index af485e01f0..1d5c25bcce 100644 --- a/.gitignore +++ b/.gitignore @@ -26,8 +26,11 @@ coverage doc pkg .rvmrc +.ruby-version +.ruby-gemset .bundle .yardoc/* +.byebug_history dist Gemfile.lock gemfiles/*.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a171f9ce3..cc806c5af2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#1594](https://github.com/ruby-grape/grape/pull/1594): Replace `Hashie::Mash` parameters with `ActiveSupport::HashWithIndifferentAccess` - [@james2m](https://github.com/james2m), [@dblock](https://github.com/dblock). * Your contribution here. #### Fixes diff --git a/Gemfile b/Gemfile index 58a721ea7a..406fe34eba 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/README.md b/README.md index 0958a5f3d7..f085bb1759 100644 --- a/README.md +++ b/README.md @@ -495,7 +495,24 @@ In the case of conflict between either of: * `GET`, `POST` and `PUT` parameters * the contents of the request body on `POST` and `PUT` -route string parameters will have precedence. +Route string parameters will have precedence. + +### Params Class + +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] + +The class can be overridden on individual parameter blocks using `build_with` as follows. + +```ruby +params do + build_with Grape::Extensions::Hash::ParamBuilder + optional :color, type: String +end +``` + +In the example above, `params["color"]` will return `nil` since `params` is a plain `Hash`. ### Declared @@ -509,7 +526,7 @@ post 'users/signup' do end ```` -If we do not specify any params, `declared` will return an empty `Hashie::Mash` instance. +If we do not specify any params, `declared` will return an empty `ActiveSupport::HashWithIndifferentAccess` hash. **Request** @@ -526,8 +543,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 params[: +] ````ruby format :json @@ -562,17 +579,11 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d } ```` -The returned hash is a `Hashie::Mash` instance, allowing you to access parameters via dot notation: - -```ruby - declared(params).user == declared(params)['user'] -``` - +The returned hash is a `ActiveSupport::HashWithIndifferentAccess` hash. -The `#declared` method is not available to `before` filters, as those are evaluated prior -to parameter coercion. +The `#declared` method is not available to `before` filters, as those are evaluated prior to parameter coercion. -### Include parent namespaces +### Include Parent Namespaces By default `declared(params)` includes parameters that were defined in all parent namespaces. If you want to return only parameters from your current namespace, you can set `include_parent_namespaces` option to `false`. @@ -897,18 +908,16 @@ end ### Multipart File Parameters -Grape makes use of `Rack::Request`'s built-in support for multipart -file parameters. Such parameters can be declared with `type: File`: +Grape makes use of `Rack::Request`'s built-in support for multipart file parameters. Such parameters can be declared with `type: File`: ```ruby params do requires :avatar, type: File end post '/' do - # Parameter will be wrapped using Hashie: - params.avatar.filename # => 'avatar.png' - params.avatar.type # => 'image/png' - params.avatar.tempfile # => # + params[:avatar][:filename] # => 'avatar.png' + params[:avatar][:avatar] # => 'image/png' + params[:avatar][:tempfile] # => # end ``` @@ -1381,7 +1390,7 @@ class Admin < Grape::Validations::Base # @attrs is a list containing the attribute we are currently validating # in our sample case this method once will get called with # @attrs being [:admin_field] and once with @attrs being [:admin_false_field] - return unless request.params.key? @attrs.first + return unless request.params.key?(@attrs.first) # check if admin flag is set to true return unless @option # check if user is admin or not diff --git a/UPGRADING.md b/UPGRADING.md index c517dc6092..1a481f60e8 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,33 @@ Upgrading Grape =============== +### Upgrading to >= 1.0.0 + +#### 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`. + +[TODO] + +This behavior can also be overridden on individual parameter blocks using `build_with`. + +```ruby +params do + build_with Grape::Extensions::Hash::ParamBuilder + optional :color, type: String +end +``` + +If you're constructing your own `Grape::Request` in a middleware, you can pass different parameter handlers to create the desired `params` class with `build_params_with`. + +```ruby +def request + Grape::Request.new(env, build_params_with: Grape::Extensions::Hashie::Mash::ParamBuilder) +end +``` + +See [#1594](https://github.com/ruby-grape/grape/pull/1594) for more information. + ### Upgrading to >= 0.19.1 #### DELETE now defaults to status code 200 for responses with a body, or 204 otherwise diff --git a/gemfiles/rack_1.5.2.gemfile b/gemfiles/rack_1.5.2.gemfile index 632a14311c..ae9e8174d4 100644 --- a/gemfiles/rack_1.5.2.gemfile +++ b/gemfiles/rack_1.5.2.gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/gemfiles/rack_edge.gemfile b/gemfiles/rack_edge.gemfile index b05ce40319..589bf59c45 100644 --- a/gemfiles/rack_edge.gemfile +++ b/gemfiles/rack_edge.gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/gemfiles/rails_3.gemfile b/gemfiles/rails_3.gemfile index 705be15125..29f753f494 100644 --- a/gemfiles/rails_3.gemfile +++ b/gemfiles/rails_3.gemfile @@ -9,6 +9,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index c3dc4ae0ad..e3cab94687 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile index 66a30197a6..e00ea92243 100644 --- a/gemfiles/rails_5.gemfile +++ b/gemfiles/rails_5.gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/gemfiles/rails_edge.gemfile b/gemfiles/rails_edge.gemfile index 08dd3d6469..972f7fdee7 100644 --- a/gemfiles/rails_edge.gemfile +++ b/gemfiles/rails_edge.gemfile @@ -8,6 +8,7 @@ group :development, :test do gem 'bundler' gem 'rake' gem 'rubocop', '0.47.0' + gem 'hashie' end group :development do diff --git a/grape.gemspec b/grape.gemspec index 7a1caae813..93e7777690 100644 --- a/grape.gemspec +++ b/grape.gemspec @@ -18,7 +18,6 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'activesupport' s.add_runtime_dependency 'multi_json', '>= 1.3.2' s.add_runtime_dependency 'multi_xml', '>= 0.5.2' - s.add_runtime_dependency 'hashie', '>= 2.1.0' s.add_runtime_dependency 'virtus', '>= 1.0.0' s.add_runtime_dependency 'builder' diff --git a/lib/grape.rb b/lib/grape.rb index 8a24429fe5..e3f3ddd1c4 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -4,7 +4,6 @@ require 'rack/accept' require 'rack/auth/basic' require 'rack/auth/digest/md5' -require 'hashie' require 'set' require 'active_support/version' require 'active_support/core_ext/hash/indifferent_access' @@ -78,6 +77,21 @@ module Exceptions autoload :MethodNotAllowed end + module Extensions + extend ActiveSupport::Autoload + + autoload :DeepMergeableHash + autoload :DeepSymbolizeHash + autoload :Hash + autoload :HashWithIndifferentAccess + + module Hashie + extend ActiveSupport::Autoload + + autoload :Mash + end + end + module Middleware extend ActiveSupport::Autoload autoload :Base @@ -85,6 +99,7 @@ module Middleware autoload :Formatter autoload :Error autoload :Globals + autoload :Stack module Auth extend ActiveSupport::Autoload diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 5cc6e13be7..281ff834ec 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -46,7 +46,7 @@ def declared_array(passed_params, options, declared_params) end def declared_hash(passed_params, options, declared_params) - declared_params.each_with_object(Hashie::Mash.new) do |declared_param, memo| + declared_params.each_with_object({}) do |declared_param, memo| # If it is not a Hash then it does not have children. # Find its value or set it to nil. if !declared_param.is_a?(Hash) @@ -56,7 +56,7 @@ def declared_hash(passed_params, options, declared_params) declared_param.each_pair do |declared_parent_param, declared_children_params| next unless options[:include_missing] || passed_params.key?(declared_parent_param) - passed_children_params = passed_params[declared_parent_param] || Hashie::Mash.new + passed_children_params = passed_params[declared_parent_param] || {} memo[optioned_param_key(declared_parent_param, options)] = declared(passed_children_params, options, declared_children_params) end end diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 7d3986ac09..44eb99a9fb 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -8,6 +8,31 @@ module DSL module Parameters extend ActiveSupport::Concern + # 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) + # * Grape::Extensions::Hash::ParamBuilder + # * Grape::Extensions::Hashie::Mash::ParamBuilder + # + # @example + # + # require 'grape/extenstions/hashie_mash' + # class API < Grape::API + # desc "Get collection" + # params do + # build_with Grape::Extensions::Hashie::Mash::ParamBuilder + # requires :user_id, type: Integer + # end + # get do + # params['user_id'] + # end + # end + def build_with(build_with = nil) + @api.namespace_inheritable(:build_with, build_with) + end + # Include reusable params rules among current. # You can define reusable params with helpers method. # diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 9792199edd..523dbdfde2 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -1,5 +1,3 @@ -require 'grape/middleware/stack' - module Grape # An Endpoint is the proxy scope in which all routing # blocks are executed. In other words, any methods @@ -239,15 +237,12 @@ def equals?(e) def run ActiveSupport::Notifications.instrument('endpoint_run.grape', endpoint: self, env: env) do @header = {} - - @request = Grape::Request.new(env) + @request = Grape::Request.new(env, build_params_with: namespace_inheritable(:build_with)) @params = @request.params @headers = @request.headers cookies.read(@request) - self.class.run_before_each(self) - run_filters befores, :before if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS]) diff --git a/lib/grape/extensions/deep_mergeable_hash.rb b/lib/grape/extensions/deep_mergeable_hash.rb new file mode 100644 index 0000000000..8f9903d62f --- /dev/null +++ b/lib/grape/extensions/deep_mergeable_hash.rb @@ -0,0 +1,19 @@ +module Grape + module Extensions + class DeepMergeableHash < ::Hash + def deep_merge!(other_hash) + other_hash.each_pair do |current_key, other_value| + this_value = self[current_key] + + self[current_key] = if this_value.is_a?(::Hash) && other_value.is_a?(::Hash) + this_value.deep_merge(other_value) + else + other_value + end + end + + self + end + end + end +end diff --git a/lib/grape/extensions/deep_symbolize_hash.rb b/lib/grape/extensions/deep_symbolize_hash.rb new file mode 100644 index 0000000000..8c54ebc46c --- /dev/null +++ b/lib/grape/extensions/deep_symbolize_hash.rb @@ -0,0 +1,22 @@ +module Grape + module Extensions + module DeepSymbolizeHash + def self.deep_symbolize_keys_in(object) + case object + when ::Hash + object.each_with_object({}) do |(key, value), new_hash| + new_hash[symbolize_key(key)] = deep_symbolize_keys_in(value) + end + when ::Array + object.map { |element| deep_symbolize_keys_in(element) } + else + object + end + end + + def self.symbolize_key(key) + key.respond_to?(:to_sym) ? key.to_sym : key + end + end + end +end diff --git a/lib/grape/extensions/hash.rb b/lib/grape/extensions/hash.rb new file mode 100644 index 0000000000..9785fd68c8 --- /dev/null +++ b/lib/grape/extensions/hash.rb @@ -0,0 +1,17 @@ +module Grape + module Extensions + module Hash + module ParamBuilder + def build_params + params = Grape::Extensions::DeepMergeableHash[rack_params] + params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS] + post_process_params(params) + end + + def post_process_params(params) + Grape::Extensions::DeepSymbolizeHash.deep_symbolize_keys_in(params) + end + end + end + end +end diff --git a/lib/grape/extensions/hash_with_indifferent_access.rb b/lib/grape/extensions/hash_with_indifferent_access.rb new file mode 100644 index 0000000000..14608a0ef1 --- /dev/null +++ b/lib/grape/extensions/hash_with_indifferent_access.rb @@ -0,0 +1,17 @@ +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 new file mode 100644 index 0000000000..e60677bfce --- /dev/null +++ b/lib/grape/extensions/hashie/mash.rb @@ -0,0 +1,19 @@ +module Grape + module Extensions + module Hashie + module Mash + def params_builder + Grape::Extensions::Hashie::Mash::ParamBuilder + end + + module ParamBuilder + def build_params + params = ::Hashie::Mash.new(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/middleware/globals.rb b/lib/grape/middleware/globals.rb index 961536bf59..f356030e34 100644 --- a/lib/grape/middleware/globals.rb +++ b/lib/grape/middleware/globals.rb @@ -4,7 +4,7 @@ module Grape module Middleware class Globals < Base def before - request = Grape::Request.new(@env) + request = Grape::Request.new(@env, build_params_with: @options[:build_params_with]) @env[Grape::Env::GRAPE_REQUEST] = request @env[Grape::Env::GRAPE_REQUEST_HEADERS] = request.headers @env[Grape::Env::GRAPE_REQUEST_PARAMS] = request.params if @env[Grape::Env::RACK_INPUT] diff --git a/lib/grape/request.rb b/lib/grape/request.rb index 21f3c00869..deef61c454 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -4,6 +4,11 @@ class Request < Rack::Request alias rack_params params + def initialize(env, options = {}) + extend options[:build_params_with] || Grape::Extensions::HashWithIndifferentAccess::ParamBuilder + super(env) + end + def params @params ||= build_params end @@ -14,16 +19,12 @@ def headers private - def build_params - params = Hashie::Mash.new(rack_params) - if env[Grape::Env::GRAPE_ROUTING_ARGS] - args = env[Grape::Env::GRAPE_ROUTING_ARGS].dup - # preserve version from query string parameters - args.delete(:version) - args.delete(:route_info) - params.deep_merge!(args) - end - params + def grape_routing_args + args = env[Grape::Env::GRAPE_ROUTING_ARGS].dup + # preserve version from query string parameters + args.delete(:version) + args.delete(:route_info) + args end def build_headers diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb index d273375fcc..a67bd1db73 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 - # are passed through +Hashie.symbolize_keys!+. + # have their keys recursively symbolized # This helps common libs such as JSON to work easily. # # @param type see #new @@ -161,8 +161,8 @@ def enforce_symbolized_keys(type, method) if type == Array || type == Set lambda do |val| method.call(val).tap do |new_value| - new_value.each do |item| - Hashie.symbolize_keys!(item) if item.is_a? Hash + new_value.map do |item| + item.is_a?(Hash) ? symbolize_keys(item) : item end end end @@ -170,7 +170,7 @@ def enforce_symbolized_keys(type, method) # Hash objects are processed directly elsif type == Hash lambda do |val| - Hashie.symbolize_keys! method.call(val) + symbolize_keys method.call(val) end # Simple types are not processed. @@ -179,6 +179,20 @@ def enforce_symbolized_keys(type, method) method end end + + def symbolize_keys!(hash) + hash.each_key do |key| + hash[key.to_sym] = hash.delete(key) if key.respond_to?(:to_sym) + end + hash + end + + def symbolize_keys(hash) + hash.inject({}) do |new_hash, (key, value)| + new_key = key.respond_to?(:to_sym) ? key.to_sym : key + new_hash.merge!(new_key => value) + end + end end end end diff --git a/lib/grape/validations/types/file.rb b/lib/grape/validations/types/file.rb index be8b7be636..fe1aedc327 100644 --- a/lib/grape/validations/types/file.rb +++ b/lib/grape/validations/types/file.rb @@ -17,10 +17,9 @@ def coerce(input) def value_coerced?(value) # Rack::Request creates a Hash with filename, - # content type and an IO object. Grape wraps that - # using hashie for convenience. Do a bit of basic + # content type and an IO object. Do a bit of basic # duck-typing. - value.is_a?(Hashie::Mash) && value.key?(:tempfile) + value.is_a?(::Hash) && value.key?(:tempfile) end end end diff --git a/lib/grape/validations/validators/coerce.rb b/lib/grape/validations/validators/coerce.rb index f4710a6bb6..b93834fe85 100644 --- a/lib/grape/validations/validators/coerce.rb +++ b/lib/grape/validations/validators/coerce.rb @@ -10,11 +10,16 @@ def initialize(*_args) @converter = Types.build_coercer(type, @option[:method]) end + def validate(request) + @request = request + super + end + 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] = new_value + params[attr_name] = @request.respond_to?(:post_process_params) ? @request.post_process_params(new_value) : new_value end private @@ -33,7 +38,6 @@ 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 744555fe2a..270923ca73 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -249,6 +249,117 @@ def app end 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 + params.class + end + + get '/foo' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('ActiveSupport::HashWithIndifferentAccess') + end + end + + context 'sets a value to params' do + it 'params' do + subject.params do + requires :a, type: String + end + subject.get '/foo' do + params[:a] = 'bar' + end + + get '/foo', a: 'foo' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('bar') + end + end + end + describe '#declared' do before do subject.format :json @@ -775,6 +886,7 @@ def app end end end + it 'parse email param with provided requirements for params' do get '/outer/abc@example.com' expect(last_response.body).to eq('abc@example.com') @@ -911,6 +1023,94 @@ def app expect(JSON.parse(last_response.body)['params']).to eq '123' 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 + requires :a, type: String + end + subject.get '/foo' do + params[:a] = 'bar' + end + + get '/foo', a: 'foo' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('bar') + end + end end describe '#error!' do diff --git a/spec/grape/request_spec.rb b/spec/grape/request_spec.rb index 45b9ef0264..b763532ca4 100644 --- a/spec/grape/request_spec.rb +++ b/spec/grape/request_spec.rb @@ -30,8 +30,18 @@ module Grape } end - it 'returns params' do - expect(request.params).to eq('a' => '123', 'b' => 'xyz') + it 'by default returns stringified parameter keys' do + expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new('a' => '123', 'b' => 'xyz')) + end + + context 'when build_params_with: Grape::Extensions::Hash::ParamBuilder is specified' do + let(:request) do + Grape::Request.new(env, build_params_with: Grape::Extensions::Hash::ParamBuilder) + end + + it 'returns symbolized params' do + expect(request.params).to eq(a: '123', b: 'xyz') + end end describe 'with grape.routing_args' do @@ -47,7 +57,7 @@ module Grape end it 'cuts version and route_info' do - expect(request.params).to eq('a' => '123', 'b' => 'xyz', 'c' => 'ccc') + expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc')) end end end diff --git a/spec/grape/validations/validators/coerce_spec.rb b/spec/grape/validations/validators/coerce_spec.rb index ca7ef224ae..f4f723d48a 100644 --- a/spec/grape/validations/validators/coerce_spec.rb +++ b/spec/grape/validations/validators/coerce_spec.rb @@ -285,7 +285,7 @@ class User requires :file, type: Rack::Multipart::UploadedFile end subject.post '/upload' do - params[:file].filename + params[:file][:filename] end post '/upload', file: Rack::Test::UploadedFile.new(__FILE__) @@ -302,7 +302,7 @@ class User requires :file, coerce: File end subject.post '/upload' do - params[:file].filename + params[:file][:filename] end post '/upload', file: Rack::Test::UploadedFile.new(__FILE__) @@ -625,7 +625,7 @@ class User get '/', a: %w(the other) expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('["the", "other"]') get '/', a: { a: 1, b: 2 } expect(last_response.status).to eq(400) @@ -633,27 +633,27 @@ class User get '/', a: [1, 2, 3] expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('["1", "2", "3"]') end it 'allows multiple collection types' do get '/', b: [1, 2, 3] expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('[1, 2, 3]') get '/', b: %w(1 2 3) expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('[1, 2, 3]') get '/', b: [1, true, 'three'] expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('["1", "true", "three"]') end it 'allows collections with multiple types' do get '/', c: [1, '2', true, 'three'] expect(last_response.status).to eq(200) - expect(last_response.body).to eq('#') + expect(last_response.body).to eq('[1, 2, "true", "three"]') get '/', d: '1' expect(last_response.status).to eq(200) @@ -669,6 +669,78 @@ class User end end + context 'when params is Hashie::Mash' do + context 'for primitive collections' do + before do + subject.params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder + optional :a, types: [String, Array[String]] + optional :b, types: [Array[Integer], Array[String]] + optional :c, type: Array[Integer, String] + optional :d, types: [Integer, String, Set[Integer, String]] + end + subject.get '/' do + ( + params.a || + params.b || + params.c || + params.d + ).inspect + end + end + + it 'allows singular form declaration' do + get '/', a: 'one way' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('"one way"') + + get '/', a: %w(the other) + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + + get '/', a: { a: 1, b: 2 } + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('a is invalid') + + get '/', a: [1, 2, 3] + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + end + + it 'allows multiple collection types' do + get '/', b: [1, 2, 3] + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + + get '/', b: %w(1 2 3) + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + + get '/', b: [1, true, 'three'] + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + end + + it 'allows collections with multiple types' do + get '/', c: [1, '2', true, 'three'] + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + + get '/', d: '1' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('1') + + get '/', d: 'one' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('"one"') + + get '/', d: %w(1 two) + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('#') + end + end + end + context 'custom coercion rules' do before do subject.params do