simplify logic for defining declared params

Now, route settings get declared params when an endpoint gets initialized, the `Grape::Validations::ParamsScope` doesn't need to worry about that anymore. Since the endpoint takes care of finalizing declared params, the `declared_params` method works with a structure which doesn't require any processing.
ruby-grape · Jun 27, 2020 · 12786e6 · 12786e6
1 parent 9198f7e
commit 12786e6
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 * [#1520](https://github.com/ruby-grape/grape/pull/1520): Un-deprecate stream-like objects - [@urkle](https://github.com/urkle).
 * [#2060](https://github.com/ruby-grape/grape/pull/2060): Drop support for Ruby 2.4 - [@dblock](https://github.com/dblock).
 * [#2060](https://github.com/ruby-grape/grape/pull/2060): Upgraded Rubocop to 0.84.0 - [@dblock](https://github.com/dblock).
+* [#2077](https://github.com/ruby-grape/grape/pull/2077): Simplify logic for defining declared params - [@dnesteryuk](https://github.com/dnesteryuk).
 * Your contribution here.
 
 #### Fixes

diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
@@ -124,10 +124,10 @@ def route_options_params_key(params_nested_path)
         def optioned_declared_params(**options)
           declared_params = if options[:include_parent_namespaces]
                               # Declared params including parent namespaces
-                              route_setting(:saved_declared_params).flatten | Array(route_setting(:declared_params))
+                              route_setting(:declared_params)
                             else
                               # Declared params at current namespace
-                              route_setting(:saved_declared_params).last & Array(route_setting(:declared_params))
+                              namespace_stackable(:declared_params).last || []
                             end
 
           raise ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params

diff --git a/lib/grape/dsl/validations.rb b/lib/grape/dsl/validations.rb
@@ -10,7 +10,24 @@ module Validations
       include Grape::DSL::Configuration
 
       module ClassMethods
-        # Clears all defined parameters and validations.
+        # Clears all defined parameters and validations. The main purpose of it is to clean up
+        # settings, so next endpoint won't interfere with previous one.
+        #
+        #    params do
+        #      # params for the endpoint below this block
+        #    end
+        #    post '/current' do
+        #      # whatever
+        #    end
+        #
+        #    # somewhere between them the reset_validations! method gets called
+        #
+        #    params do
+        #      # params for the endpoint below this block
+        #    end
+        #    post '/next' do
+        #      # whatever
+        #    end
         def reset_validations!
           unset_namespace_stackable :declared_params
           unset_namespace_stackable :validations

diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
@@ -80,7 +80,10 @@ def initialize(new_settings, options = {}, &block)
 
       self.inheritable_setting = new_settings.point_in_time_copy
 
-      route_setting(:saved_declared_params, namespace_stackable(:declared_params))
+      # now +namespace_stackable(:declared_params)+ contains all params defined for
+      # this endpoint and its parents, but later it will be cleaned up,
+      # see +reset_validations!+ in lib/grape/dsl/validations.rb
+      route_setting(:declared_params, namespace_stackable(:declared_params).flatten)
       route_setting(:saved_validations, namespace_stackable(:validations))
 
       namespace_stackable(:representations, []) unless namespace_stackable(:representations)
@@ -116,7 +119,6 @@ def inherit_settings(namespace_stackable)
       parent_declared_params = namespace_stackable[:declared_params]
 
       if parent_declared_params
-        inheritable_setting.route[:declared_params] ||= []
         inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten)
       end
 
@@ -190,7 +192,7 @@ def prepare_default_route_attributes
         requirements: prepare_routes_requirements,
         prefix: namespace_inheritable(:root_prefix),
         anchor: options[:route_options].fetch(:anchor, true),
-        settings: inheritable_setting.route.except(:saved_declared_params, :saved_validations),
+        settings: inheritable_setting.route.except(:declared_params, :saved_validations),
         forward_match: options[:forward_match]
       }
     end

diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb
@@ -237,10 +237,10 @@ def configure_declared_params
           @parent.push_declared_params [element => @declared_params]
         else
           @api.namespace_stackable(:declared_params, @declared_params)
-
-          @api.route_setting(:declared_params, []) unless @api.route_setting(:declared_params)
-          @api.route_setting(:declared_params, @api.namespace_stackable(:declared_params).flatten)
         end
+
+        # params were stored in settings, it can be cleaned from the params scope
+        @declared_params = nil
       end
 
       def validates(attrs, validations)

diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb
@@ -9,6 +9,10 @@ def app
     subject
   end
 
+  def declared_params
+    subject.namespace_stackable(:declared_params).flatten
+  end
+
   describe 'params' do
     context 'optional' do
       before do
@@ -41,7 +45,7 @@ def app
         subject.params do
           optional :some_param
         end
-        expect(subject.route_setting(:declared_params)).to eq([:some_param])
+        expect(declared_params).to eq([:some_param])
       end
     end
 
@@ -61,7 +65,7 @@ def define_optional_using
 
       it 'adds entity documentation to declared params' do
         define_optional_using
-        expect(subject.route_setting(:declared_params)).to eq(%i[field_a field_b])
+        expect(declared_params).to eq(%i[field_a field_b])
       end
 
       it 'works when field_a and field_b are not present' do
@@ -108,7 +112,7 @@ def define_optional_using
         subject.params do
           requires :some_param
         end
-        expect(subject.route_setting(:declared_params)).to eq([:some_param])
+        expect(declared_params).to eq([:some_param])
       end
 
       it 'works when required field is present but nil' do
@@ -193,7 +197,7 @@ def define_requires_all
 
       it 'adds entity documentation to declared params' do
         define_requires_all
-        expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
+        expect(declared_params).to eq(%i[required_field optional_field])
       end
 
       it 'errors when required_field is not present' do
@@ -228,7 +232,7 @@ def define_requires_none
 
       it 'adds entity documentation to declared params' do
         define_requires_none
-        expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
+        expect(declared_params).to eq(%i[required_field optional_field])
       end
 
       it 'errors when required_field is not present' do
@@ -258,7 +262,7 @@ def define_requires_all
 
         it 'adds only the entity documentation to declared params, nothing more' do
           define_requires_all
-          expect(subject.route_setting(:declared_params)).to eq(%i[required_field optional_field])
+          expect(declared_params).to eq(%i[required_field optional_field])
         end
       end
 
@@ -324,7 +328,7 @@ def define_requires_none
             requires :key
           end
         end
-        expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
+        expect(declared_params).to eq([items: [:key]])
       end
     end
 
@@ -396,7 +400,7 @@ def define_requires_none
             requires :key
           end
         end
-        expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
+        expect(declared_params).to eq([items: [:key]])
       end
     end
 
@@ -459,7 +463,7 @@ def define_requires_none
             requires :key
           end
         end
-        expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
+        expect(declared_params).to eq([items: [:key]])
       end
     end
 
@@ -813,7 +817,7 @@ def validate_param!(attr_name, params)
             requires :key
           end
         end
-        expect(subject.route_setting(:declared_params)).to eq([items: [:key]])
+        expect(declared_params).to eq([items: [:key]])
       end
     end
 
@@ -877,7 +881,7 @@ def validate_param!(attr_name, params)
             requires(:required_subitems, type: Array) { requires :value }
           end
         end
-        expect(subject.route_setting(:declared_params)).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])
+        expect(declared_params).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])
       end
     end
 
@@ -1122,14 +1126,14 @@ def validate_param!(attr_name, params)
           subject.params do
             use :pagination
           end
-          expect(subject.route_setting(:declared_params)).to eq %i[page per_page]
+          expect(declared_params).to eq %i[page per_page]
         end
 
         it 'by #use with multiple params' do
           subject.params do
             use :pagination, :period
           end
-          expect(subject.route_setting(:declared_params)).to eq %i[page per_page start_date end_date]
+          expect(declared_params).to eq %i[page per_page start_date end_date]
         end
       end