use correct params class in declared

use correct params class in declared add changelog entry add tests for declared class fix rubocop make changelog entry better remove puts in tests one more puts change test names conform changelog entry return dynamic class name parse response instead remove params Instrument validators with ActiveSupport::Notifications Suppress `warning: method redefined` Bugfix: Correctly handle `given` in Array params (#1625) * Bugfix: Correctly handle `given` in Array params Array parameters are handled as a parameter that opens a scope with `type: Array`; `given` opens up a new scope, but was always setting the type to Hash. This patch fixes that, as well as correctly labeling the error messages associated with array fields. * Code review fix * Add CHANGELOG entry Add ability to include an array of modules as helpers Changing helpers DSL to allow the inclusion of many modules. This attemps to bring a better readability, since it seems to be more intuitive to send a list of modules when the message in question is called helpers. ensure we return a string from test return json in tests Update README according to new `helpers` macro interface Silence warnings (#1632) * silence warnings initialize vars in initializers subclass hashie mash to silence warnings rubocop fixes add changelog entry Revert "use correct params class in declared" This reverts commit 61f0c8e. fix tests * remove disable_warnings in hashie mash * make rubocop happy * fix hashie tests
ruby-grape · Jun 12, 2017 · 5278eca · 5278eca
1 parent c644607
commit 5278eca
Show file tree

Hide file tree

Showing 15 changed files with 206 additions and 67 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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