diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1d7f66c286..83021bf3dd 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,35 +1,30 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2017-02-19 15:40:46 -0500 using RuboCop version 0.47.1. +# on 2017-06-12 13:25:24 -0600 using RuboCop version 0.47.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 43 +# Offense count: 44 Metrics/AbcSize: Max: 44 -# Offense count: 265 +# Offense count: 277 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: Max: 3104 -# Offense count: 1 -# Configuration parameters: CountBlocks. -Metrics/BlockNesting: - Max: 4 - # Offense count: 8 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 281 + Max: 287 -# Offense count: 26 +# Offense count: 28 Metrics/CyclomaticComplexity: Max: 14 -# Offense count: 993 +# Offense count: 1098 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: @@ -40,12 +35,12 @@ Metrics/LineLength: Metrics/MethodLength: Max: 33 -# Offense count: 9 +# Offense count: 10 # Configuration parameters: CountComments. Metrics/ModuleLength: Max: 212 -# Offense count: 16 +# Offense count: 17 Metrics/PerceivedComplexity: Max: 14 diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dccef1db0..7f6ec47e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,16 @@ * [#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). * [#1622](https://github.com/ruby-grape/grape/pull/1622): Add `except_values` validator to replace `except` option of `values` validator - [@jlfaber](https://github.com/jlfaber). +* [#1635](https://github.com/ruby-grape/grape/pull/1635): Instrument validators with ActiveSupport::Notifications - [@ktimothy](https://github.com/ktimothy). +* [#1646](https://github.com/ruby-grape/grape/pull/1646): Add ability to include an array of modules as helpers - [@pablonahuelgomez](https://github.com/pablonahuelgomez). * Your contribution here. #### Fixes +* [#1631](https://github.com/ruby-grape/grape/pull/1631): Declared now returns declared options using the class that params is set to use - [@thogg4](https://github.com/thogg4). +* [#1632](https://github.com/ruby-grape/grape/pull/1632): Silence warnings - [@thogg4](https://github.com/thogg4). * [#1615](https://github.com/ruby-grape/grape/pull/1615): Fix default and type validator when values is a Hash with no value attribute - [@jlfaber](https://github.com/jlfaber). +* [#1625](https://github.com/ruby-grape/grape/pull/1625): Handle `given` correctly when nested in Array params - [@rnubel](https://github.com/rnubel), [@avellable](https://github.com/avellable). * Your contribution here. ### 0.19.2 (4/12/2017) diff --git a/README.md b/README.md index c7dda05922..a3cc7bfc41 100644 --- a/README.md +++ b/README.md @@ -1643,7 +1643,7 @@ end ## Helpers You can define helper methods that your endpoints can use with the `helpers` -macro by either giving a block or a module. +macro by either giving a block or an array of modules. ```ruby module StatusHelpers @@ -1652,6 +1652,12 @@ module StatusHelpers end end +module HttpCodesHelpers + def unauthorized + 401 + end +end + class API < Grape::API # define helpers with a block helpers do @@ -1660,8 +1666,12 @@ class API < Grape::API end end - # or mix in a module - helpers StatusHelpers + # or mix in an array of modules + helpers StatusHelpers, HttpCodesHelpers + + before do + error!('Access Denied', unauthorized) unless current_user + end get 'info' do # helpers available in your endpoint and filters @@ -3367,6 +3377,14 @@ The execution of the main content block of the endpoint. * *filters* - The filters being executed * *type* - The type of filters (before, before_validation, after_validation, after) +#### endpoint_run_validators.grape + +The execution of validators. + +* *endpoint* - The endpoint instance +* *validators* - The validators being executed +* *request* - The request being validated + See the [ActiveSupport::Notifications documentation](http://api.rubyonrails.org/classes/ActiveSupport/Notifications.html) for information on how to subscribe to these events. ### Monitoring Products diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb index 9d2200a7cd..d58e2cf020 100644 --- a/lib/grape/dsl/helpers.rb +++ b/lib/grape/dsl/helpers.rb @@ -13,7 +13,7 @@ module ClassMethods # When called without a block, all known helpers within this scope # are included. # - # @param [Module] new_mod optional module of methods to include + # @param [Array] new_modules optional array of modules to include # @param [Block] block optional block of methods to include # # @example Define some helpers. @@ -26,28 +26,42 @@ module ClassMethods # end # end # - def helpers(new_mod = nil, &block) - if block_given? || new_mod - mod = new_mod || Module.new - define_boolean_in_mod(mod) - inject_api_helpers_to_mod(mod) if new_mod + # @example Include many modules + # + # class ExampleAPI < Grape::API + # helpers Authentication, Mailer, OtherModule + # end + # + def helpers(*new_modules, &block) + include_new_modules(new_modules) if new_modules.any? + include_block(block) if block_given? + include_all_in_scope if !block_given? && new_modules.empty? + end - inject_api_helpers_to_mod(mod) do - mod.class_eval(&block) - end if block_given? + protected - namespace_stackable(:helpers, mod) - else - mod = Module.new - namespace_stackable(:helpers).each do |mod_to_include| - mod.send :include, mod_to_include - end - change! - mod + def include_new_modules(modules) + modules.each { |mod| make_inclusion(mod) } + end + + def include_block(block) + Module.new.tap do |mod| + make_inclusion(mod) { mod.class_eval(&block) } end end - protected + def make_inclusion(mod, &block) + define_boolean_in_mod(mod) + inject_api_helpers_to_mod(mod, &block) + namespace_stackable(:helpers, mod) + end + + def include_all_in_scope + Module.new.tap do |mod| + namespace_stackable(:helpers).each { |mod_to_include| mod.send :include, mod_to_include } + change! + end + end def define_boolean_in_mod(mod) return if defined? mod::Boolean diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 281ff834ec..d060b3e10f 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({}) do |declared_param, memo| + declared_params.each_with_object(passed_params.class.new) 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] || {} + passed_children_params = passed_params[declared_parent_param] || passed_params.class.new memo[optioned_param_key(declared_parent_param, options)] = declared(passed_children_params, options, declared_children_params) end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 24b9d5f3a5..2f3236d7de 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -7,7 +7,7 @@ module Routing include Grape::DSL::Configuration module ClassMethods - attr_reader :endpoints, :routes + attr_reader :endpoints # Specify an API version. # diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb index f83d61645f..8d94cff555 100644 --- a/lib/grape/dsl/settings.rb +++ b/lib/grape/dsl/settings.rb @@ -9,7 +9,7 @@ module DSL module Settings extend ActiveSupport::Concern - attr_accessor :inheritable_setting, :top_level_setting + attr_writer :inheritable_setting, :top_level_setting # Fetch our top-level settings, which apply to all endpoints in the API. def top_level_setting diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 3ff2d6c380..6e5df43785 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -96,6 +96,11 @@ def initialize(new_settings, options = {}, &block) @lazy_initialized = nil @block = nil + @status = nil + @file = nil + @body = nil + @proc = nil + return unless block_given? @source = block @@ -336,15 +341,17 @@ def lazy_initialize! def run_validators(validators, request) validation_errors = [] - validators.each do |validator| - begin - validator.validate(request) - rescue Grape::Exceptions::Validation => e - validation_errors << e - break if validator.fail_fast? - rescue Grape::Exceptions::ValidationArrayErrors => e - validation_errors += e.errors - break if validator.fail_fast? + ActiveSupport::Notifications.instrument('endpoint_run_validators.grape', endpoint: self, validators: validators, request: request) do + validators.each do |validator| + begin + validator.validate(request) + rescue Grape::Exceptions::Validation => e + validation_errors << e + break if validator.fail_fast? + rescue Grape::Exceptions::ValidationArrayErrors => e + validation_errors += e.errors + break if validator.fail_fast? + end end end diff --git a/lib/grape/extensions/hashie/mash.rb b/lib/grape/extensions/hashie/mash.rb index 50958f3224..6afa106d95 100644 --- a/lib/grape/extensions/hashie/mash.rb +++ b/lib/grape/extensions/hashie/mash.rb @@ -4,7 +4,6 @@ module Hashie module Mash module ParamBuilder extend ::ActiveSupport::Concern - included do namespace_inheritable(:build_params_with, Grape::Extensions::Hashie::Mash::ParamBuilder) end diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index c57ca5f142..c4c109688a 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -42,37 +42,45 @@ def should_validate?(parameters) return false if @optional && (params(parameters).blank? || any_element_blank?(parameters)) + return true if parent.nil? + parent.should_validate?(parameters) + end + + def meets_dependency?(params) + return true unless @dependent_on + + params = params.with_indifferent_access + @dependent_on.each do |dependency| if dependency.is_a?(Hash) dependency_key = dependency.keys[0] proc = dependency.values[0] - return false unless proc.call(params(parameters).try(:[], dependency_key)) - elsif params(parameters).try(:[], dependency).blank? + return false unless proc.call(params.try(:[], dependency_key)) + elsif params.respond_to?(:key?) && params.try(:[], dependency).blank? return false end - end if @dependent_on + end - return true if parent.nil? - parent.should_validate?(parameters) + true end # @return [String] the proper attribute name, with nesting considered. - def full_name(name) + def full_name(name, index: nil) if nested? # Find our containing element's name, and append ours. - "#{@parent.full_name(@element)}#{array_index}[#{name}]" + [@parent.full_name(@element), [@index || index, name].map(&method(:brackets))].compact.join elsif lateral? - # Find the name of the element as if it was at the - # same nesting level as our parent. - @parent.full_name(name) + # Find the name of the element as if it was at the same nesting level + # as our parent. We need to forward our index upward to achieve this. + @parent.full_name(name, index: @index) else # We must be the root scope, so no prefix needed. name.to_s end end - def array_index - "[#{@index}]" if @index.present? + def brackets(val) + "[#{val}]" if val end # @return [Boolean] whether or not this scope is the root-level scope @@ -187,7 +195,7 @@ def new_lateral_scope(options, &block) element: nil, parent: self, options: @optional, - type: Hash, + type: type == Array ? Array : Hash, dependent_on: options[:dependent_on], &block ) diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 7b3a2e9655..ad3b440fa7 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -41,6 +41,7 @@ def validate!(params) array_errors = [] attributes.each do |resource_params, attr_name| next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name)) + next unless @scope.meets_dependency?(resource_params) begin validate_param!(attr_name, resource_params) diff --git a/lib/grape/validations/validators/values.rb b/lib/grape/validations/validators/values.rb index 2204692252..776dc314ad 100644 --- a/lib/grape/validations/validators/values.rb +++ b/lib/grape/validations/validators/values.rb @@ -14,6 +14,9 @@ def initialize(attrs, options, required, scope, opts = {}) warn '[DEPRECATION] The values validator proc option is deprecated. ' \ 'The lambda expression can now be assigned directly to values.' if @proc else + @excepts = nil + @values = nil + @proc = nil @values = options end super diff --git a/spec/grape/dsl/helpers_spec.rb b/spec/grape/dsl/helpers_spec.rb index 8611140882..ba4e48ec4c 100644 --- a/spec/grape/dsl/helpers_spec.rb +++ b/spec/grape/dsl/helpers_spec.rb @@ -6,8 +6,12 @@ module HelpersSpec class Dummy include Grape::DSL::Helpers - def self.mod - namespace_stackable(:helpers).first + def self.mods + namespace_stackable(:helpers) + end + + def self.first_mod + mods.first end end end @@ -36,23 +40,38 @@ def test expect(subject).to receive(:namespace_stackable).with(:helpers).and_call_original subject.helpers(&proc) - expect(subject.mod.instance_methods).to include(:test) + expect(subject.first_mod.instance_methods).to include(:test) end it 'uses provided modules' do mod = Module.new - expect(subject).to receive(:namespace_stackable).with(:helpers, kind_of(Grape::DSL::Helpers::BaseHelper)).and_call_original + expect(subject).to receive(:namespace_stackable).with(:helpers, kind_of(Grape::DSL::Helpers::BaseHelper)).and_call_original.exactly(2).times expect(subject).to receive(:namespace_stackable).with(:helpers).and_call_original subject.helpers(mod, &proc) - expect(subject.mod).to eq mod + expect(subject.first_mod).to eq mod + end + + it 'uses many provided modules' do + mod = Module.new + mod2 = Module.new + mod3 = Module.new + + expect(subject).to receive(:namespace_stackable).with(:helpers, kind_of(Grape::DSL::Helpers::BaseHelper)).and_call_original.exactly(4).times + expect(subject).to receive(:namespace_stackable).with(:helpers).and_call_original.exactly(3).times + + subject.helpers(mod, mod2, mod3, &proc) + + expect(subject.mods).to include(mod) + expect(subject.mods).to include(mod2) + expect(subject.mods).to include(mod3) end context 'with an external file' do it 'sets Boolean as a Virtus::Attribute::Boolean' do subject.helpers BooleanParam - expect(subject.mod::Boolean).to eq Virtus::Attribute::Boolean + expect(subject.first_mod::Boolean).to eq Virtus::Attribute::Boolean end end end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 1af9b73b94..8d678b6a9f 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -298,6 +298,47 @@ def app end end + context 'when params are not built with default class' do + it 'returns an object that corresponds with the params class - hash with indifferent access' do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('ActiveSupport::HashWithIndifferentAccess') + end + + it 'returns an object that corresponds with the params class - hashie mash' do + subject.params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('Hashie::Mash') + end + + it 'returns an object that corresponds with the params class - hash' do + subject.params do + build_with Grape::Extensions::Hash::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('Hash') + end + end + it 'should show nil for nested params if include_missing is true' do subject.get '/declared' do declared(params, include_missing: true) @@ -1403,6 +1444,9 @@ def memoized have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), filters: [], type: :before_validation }), + have_attributes(name: 'endpoint_run_validators.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), + validators: [], + request: a_kind_of(Grape::Request) }), have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), filters: [], type: :after_validation }), @@ -1424,6 +1468,9 @@ def memoized have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), filters: [], type: :before_validation }), + have_attributes(name: 'endpoint_run_validators.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), + validators: [], + request: a_kind_of(Grape::Request) }), have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(Grape::Endpoint), filters: [], type: :after_validation }), diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 5b1c4e41a5..7b659e5cee 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -473,6 +473,29 @@ def initialize(value) end end + context 'when validations are dependent on a parameter within an array param' do + before do + subject.params do + requires :foos, type: Array do + optional :foo_type, :baz_type + given :foo_type do + requires :bar + end + end + end + subject.post('/test') { declared(params).to_json } + end + + it 'applies the constraint within each value' do + post '/test', + { foos: [{ foo_type: 'a' }, { baz_type: 'c' }] }.to_json, + 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('foos[0][bar] is missing') + end + end + context 'when validations are dependent on a parameter with specific value' do # build test cases from all combinations of declarations and options a_decls = %i(optional requires)