Merge pull request #936 from dm1try/fix_weird_behaviour_with_optional_groups

fix weird behaviour for optional groups with default params

Author: dblock

CHANGELOG.md

@@ -1,6 +1,10 @@
 0.11.1 (Next)
 =============
 
+#### Fixes
+
+* [#936](https://github.com/intridea/grape/pull/936): Fixed default params processing for optional groups - [@dm1try](https://github.com/dm1try).
+
 * Your contribution here.
 
 0.11.0 (2/23/2015)

lib/grape/validations/validators/default.rb

@@ -11,12 +11,12 @@ def validate_param!(attr_name, params)
       end
 
       def validate!(params)
+        return unless @scope.should_validate?(params)
+
         attrs = AttributesIterator.new(self, @scope, params)
-        parent_element = @scope.element
         attrs.each do |resource_params, attr_name|
           if resource_params[attr_name].nil?
             validate_param!(attr_name, resource_params)
-            params[parent_element] = resource_params if parent_element && params[parent_element].nil?
           end
         end
       end

spec/grape/validations/validators/default_spec.rb

@@ -40,20 +40,6 @@ class API < Grape::API
           { random_number: params[:random], non_random_number: params[:non_random_number] }
         end
 
-        params do
-          # NOTE: The :foo parameter could be made required with json body
-          # params, and then an empty hash would be valid. With query parameters
-          # it must be optional if it isn't provided at all, as otherwise
-          # the validaton for the Hash itself fails because there is no such
-          # thing as an empty hash.
-          optional :foo, type: Hash do
-            optional :bar, default: 'foo-bar'
-          end
-        end
-        get '/group' do
-          { foo_bar: params[:foo][:bar] }
-        end
-
         params do
           optional :array, type: Array do
             requires :name
@@ -107,15 +93,119 @@ def app
     expect(before['random_number']).not_to eq(after['random_number'])
   end
 
-  it 'set default values for optional grouped params' do
-    get('/group')
-    expect(last_response.status).to eq(200)
-    expect(last_response.body).to eq({ foo_bar: 'foo-bar' }.to_json)
-  end
-
   it 'sets default values for grouped arrays' do
     get('/array?array[][name]=name&array[][name]=name2&array[][with_default]=bar2')
     expect(last_response.status).to eq(200)
     expect(last_response.body).to eq({ array: [{ name: 'name', with_default: 'default' }, { name: 'name2', with_default: 'bar2' }] }.to_json)
   end
+
+  context 'optional group with defaults' do
+    subject do
+      Class.new(Grape::API) do
+        default_format :json
+      end
+    end
+
+    def app
+      subject
+    end
+
+    context 'optional array without default value includes optional param with default value' do
+      before do
+        subject.params do
+          optional :optional_array, type: Array do
+            optional :foo_in_optional_array, default: 'bar'
+          end
+        end
+        subject.post '/optional_array' do
+          { optional_array: params[:optional_array] }
+        end
+      end
+
+      it 'returns nil for optional array if param is not provided' do
+        post '/optional_array'
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ optional_array: nil }.to_json)
+      end
+    end
+
+    context 'optional array with default value includes optional param with default value' do
+      before do
+        subject.params do
+          optional :optional_array_with_default, type: Array, default: [] do
+            optional :foo_in_optional_array, default: 'bar'
+          end
+        end
+        subject.post '/optional_array_with_default' do
+          { optional_array_with_default: params[:optional_array_with_default] }
+        end
+      end
+
+      it 'sets default value for optional array if param is not provided' do
+        post '/optional_array_with_default'
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ optional_array_with_default: [] }.to_json)
+      end
+    end
+
+    context 'optional hash without default value includes optional param with default value' do
+      before do
+        subject.params do
+          optional :optional_hash_without_default, type: Hash do
+            optional :foo_in_optional_hash, default: 'bar'
+          end
+        end
+        subject.post '/optional_hash_without_default' do
+          { optional_hash_without_default: params[:optional_hash_without_default] }
+        end
+      end
+
+      it 'returns nil for optional hash if param is not provided' do
+        post '/optional_hash_without_default'
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ optional_hash_without_default: nil }.to_json)
+      end
+    end
+
+    context 'optional hash with default value includes optional param with default value' do
+      before do
+        subject.params do
+          optional :optional_hash_with_default, type: Hash, default: {} do
+            optional :foo_in_optional_hash, default: 'bar'
+          end
+        end
+        subject.post '/optional_hash_with_default_empty_hash' do
+          { optional_hash_with_default: params[:optional_hash_with_default] }
+        end
+
+        subject.params do
+          optional :optional_hash_with_default, type: Hash, default: { foo_in_optional_hash: 'parent_default' } do
+            optional :some_param
+            optional :foo_in_optional_hash, default: 'own_default'
+          end
+        end
+        subject.post '/optional_hash_with_default_inner_params' do
+          { foo_in_optional_hash: params[:optional_hash_with_default][:foo_in_optional_hash] }
+        end
+      end
+
+      it 'sets default value for optional hash if param is not provided' do
+        post '/optional_hash_with_default_empty_hash'
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ optional_hash_with_default: {} }.to_json)
+      end
+
+      it 'sets default value from parent defaults for inner param if parent param is not provided' do
+        post '/optional_hash_with_default_inner_params'
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ foo_in_optional_hash: 'parent_default' }.to_json)
+      end
+
+      it 'sets own default value for inner param if parent param is provided' do
+        post '/optional_hash_with_default_inner_params', optional_hash_with_default: { some_param: 'param' }
+        expect(last_response.status).to eq(201)
+        expect(last_response.body).to eq({ foo_in_optional_hash: 'own_default' }.to_json)
+      end
+    end
+  end
 end