From 7977b4180a195bdbc6f6ea7c751141a89fa57e87 Mon Sep 17 00:00:00 2001
From: Dmitriy Nesteryuk <dmitriy.nesteryuk@gmail.com>
Date: Sun, 31 May 2020 20:49:35 +0300
Subject: [PATCH] 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.
---
 lib/grape/dsl/inside_route.rb         |  4 ++--
 lib/grape/dsl/validations.rb          | 19 ++++++++++++++++-
 lib/grape/endpoint.rb                 |  8 ++++---
 lib/grape/validations/params_scope.rb |  3 ---
 spec/grape/validations_spec.rb        | 30 +++++++++++++++------------
 5 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
index cf218a4c8..7fcf54549 100644
--- 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
index a426a11aa..d2d354fb1 100644
--- 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
index b202c8271..7a8439692 100644
--- 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
index 8a0c84cbb..c6a8f9d21 100644
--- a/lib/grape/validations/params_scope.rb
+++ b/lib/grape/validations/params_scope.rb
@@ -237,9 +237,6 @@ 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
       end
 
diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb
index 7bde6104a..232c23534 100644
--- 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