Fix validations nested under a given

Multi-param and default validators, all of which implemented their
own `validate!` method, were not respecting the `given` dependency
as expected. This change copies the check into all those validators.

The bug was likely introduced by #1625, which required that the
dependency check be moved out of the scope itself and into the
validators. Rolling back that change would require fundamentally
re-architecting how array parameters are validated (possibly with
a new kind of scope).

CHANGELOG.md

@@ -6,7 +6,7 @@
 
 #### Fixes
 
-* Your contribution here.
+* [#1691](https://github.com/ruby-grape/grape/pull/1691): Fix validations nested under `given` - [@rnubel](https://github.com/rnubel).
 
 ### 1.0.1 (9/8/2017)
 

lib/grape/validations/validators/all_or_none.rb

@@ -4,16 +4,20 @@ module Validations
     class AllOrNoneOfValidator < MultipleParamsBase
       def validate!(params)
         super
-        if scope_requires_params && only_subset_present
+        if scope_requires_params && only_subset_present(params)
           raise Grape::Exceptions::Validation, params: all_keys, message: message(:all_or_none)
         end
         params
       end
 
       private
 
-      def only_subset_present
-        scoped_params.any? { |resource_params| !keys_in_common(resource_params).empty? && keys_in_common(resource_params).length < attrs.length }
+      def only_subset_present(params)
+        scoped_params.any? do |resource_params|
+          return unless scope_should_validate?(resource_params, params)
+
+          !keys_in_common(resource_params).empty? && keys_in_common(resource_params).length < attrs.length
+        end
       end
     end
   end

lib/grape/validations/validators/at_least_one_of.rb

@@ -4,16 +4,20 @@ module Validations
     class AtLeastOneOfValidator < MultipleParamsBase
       def validate!(params)
         super
-        if scope_requires_params && no_exclusive_params_are_present
+        if scope_requires_params && no_exclusive_params_are_present(params)
           raise Grape::Exceptions::Validation, params: all_keys, message: message(:at_least_one)
         end
         params
       end
 
       private
 
-      def no_exclusive_params_are_present
-        scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? }
+      def no_exclusive_params_are_present(params)
+        scoped_params.any? do |resource_params|
+          next unless scope_should_validate?(resource_params, params)
+
+          keys_in_common(resource_params).empty?
+        end
       end
     end
   end

lib/grape/validations/validators/default.rb

@@ -20,6 +20,8 @@ def validate_param!(attr_name, params)
       def validate!(params)
         attrs = AttributesIterator.new(self, @scope, params)
         attrs.each do |resource_params, attr_name|
+          next unless @scope.meets_dependency?(resource_params, params)
+
           if resource_params.is_a?(Hash) && resource_params[attr_name].nil?
             validate_param!(attr_name, resource_params)
           end

lib/grape/validations/validators/exactly_one_of.rb

@@ -4,7 +4,7 @@ module Validations
     class ExactlyOneOfValidator < MutualExclusionValidator
       def validate!(params)
         super
-        if scope_requires_params && none_of_restricted_params_is_present
+        if scope_requires_params && none_of_restricted_params_is_present(params)
           raise Grape::Exceptions::Validation, params: all_keys, message: message(:exactly_one)
         end
         params
@@ -21,8 +21,12 @@ def message(default_key = nil)
 
       private
 
-      def none_of_restricted_params_is_present
-        scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? }
+      def none_of_restricted_params_is_present(params)
+        scoped_params.any? do |resource_params|
+          next unless scope_should_validate?(resource_params, params)
+
+          keys_in_common(resource_params).empty?
+        end
       end
     end
   end

lib/grape/validations/validators/multiple_params_base.rb

@@ -10,6 +10,10 @@ def validate!(params)
 
       private
 
+      def scope_should_validate?(scoped_params, params)
+        @scope.meets_dependency?(scoped_params, params)
+      end
+
       def scope_requires_params
         @scope.required? || scoped_params.any?(&:any?)
       end

lib/grape/validations/validators/mutual_exclusion.rb

@@ -6,16 +6,18 @@ class MutualExclusionValidator < MultipleParamsBase
 
       def validate!(params)
         super
-        if two_or_more_exclusive_params_are_present
+        if two_or_more_exclusive_params_are_present(params)
           raise Grape::Exceptions::Validation, params: processing_keys_in_common, message: message(:mutual_exclusion)
         end
         params
       end
 
       private
 
-      def two_or_more_exclusive_params_are_present
+      def two_or_more_exclusive_params_are_present(params)
         scoped_params.any? do |resource_params|
+          next unless scope_should_validate?(resource_params, params)
+
           @processing_keys_in_common = keys_in_common(resource_params)
           @processing_keys_in_common.length > 1
         end

spec/grape/validations/params_scope_spec.rb

@@ -407,6 +407,60 @@ def initialize(value)
       expect(last_response.status).to eq(200)
     end
 
+    it 'sets default values only if the dependency is met' do
+      subject.params do
+        optional :a
+        given :a do
+          optional :b, default: 'default value'
+        end
+      end
+      subject.get('/defaults') { params.to_json }
+
+      get '/defaults'
+      expect(last_response.status).to eq(200)
+      expect(last_response.body).to eq('{}')
+
+      get '/defaults', a: true
+      expect(last_response.status).to eq(200)
+      expect(JSON.parse(last_response.body)).to eq({ "a" => "true", "b" => "default value" })
+    end
+
+    it 'conditionally applies all multi-param validators' do
+      subject.params do
+        optional :a
+        given :a do
+          optional :b, :c, :d, :e, :f, :g
+
+          exactly_one_of :b, :c
+          all_or_none_of :d, :e
+          at_least_one_of :f, :g
+        end
+      end
+      subject.get('/exactly_one') { params.to_json }
+
+      get '/exactly_one'
+      expect(last_response.status).to eq(200)
+      expect(last_response.body).to eq('{}')
+
+      get '/exactly_one', a: true, b: true, f: true
+      expect(last_response.status).to eq(200)
+
+      get '/exactly_one', a: true, c: true, f: true
+      expect(last_response.status).to eq(200)
+
+      get '/exactly_one', a: true, b: true, c: true, f: true
+      expect(last_response.status).to eq(400)
+
+      get '/exactly_one', d: true
+      expect(last_response.status).to eq(200)
+
+      get '/exactly_one', a: true, b: true, d: true, e: true, f: true
+      expect(last_response.status).to eq(200)
+
+      get '/exactly_one', a: true, b: true, f: true, g: true
+      expect(last_response.status).to eq(200)
+    end
+
     it 'applies only the appropriate validation' do
       subject.params do
         optional :a