Merge pull request ruby-grape#1961 from ruby-grape/revert-1953-chore/micto-optimization-3

Revert "delete a reversible stackable values class"

Author: dm1try

CHANGELOG.md

@@ -3,7 +3,6 @@
 #### Features
 
 * Your contribution here.
-* [#1953](https://github.com/ruby-grape/grape/pull/1953): Delete a reversible stackable values class - [@dnesteryuk](https://github.com/dnesteryuk).
 * [#1949](https://github.com/ruby-grape/grape/pull/1949): Add support for Ruby 2.7 - [@nbulaj](https://github.com/nbulaj).
 * [#1948](https://github.com/ruby-grape/grape/pull/1948): Relax `dry-types` dependency version - [@nbulaj](https://github.com/nbulaj).
 * [#1944](https://github.com/ruby-grape/grape/pull/1944): Reduces `attribute_translator` string allocations - [@ericproulx](https://github.com/ericproulx).

lib/grape.rb

@@ -140,6 +140,7 @@ module Util
  eager_autoload do
  autoload :InheritableValues
  autoload :StackableValues
+ autoload :ReverseStackableValues
  autoload :InheritableSetting
  autoload :StrictHashConfiguration
  autoload :Registrable

lib/grape/dsl/request_response.rb

@@ -127,7 +127,7 @@ def rescue_from(*args, &block)
  :base_only_rescue_handlers
  end
 
- namespace_stackable handler_type, Hash[args.map { |arg| [arg, handler] }]
+ namespace_reverse_stackable handler_type, Hash[args.map { |arg| [arg, handler] }]
  end
 
  namespace_stackable(:rescue_options, options)

lib/grape/dsl/settings.rb

@@ -96,18 +96,21 @@ def namespace_stackable(key, value = nil)
  get_or_set :namespace_stackable, key, value
  end
 
+ def namespace_reverse_stackable(key, value = nil)
+ get_or_set :namespace_reverse_stackable, key, value
+ end
+
  def namespace_stackable_with_hash(key)
  settings = get_or_set :namespace_stackable, key, nil
  return if settings.blank?
  settings.each_with_object({}) { |value, result| result.deep_merge!(value) }
  end
 
  def namespace_reverse_stackable_with_hash(key)
- settings = get_or_set :namespace_stackable, key, nil
+ settings = get_or_set :namespace_reverse_stackable, key, nil
  return if settings.blank?
-
  result = {}
- settings.reverse_each do |setting|
+ settings.each do |setting|
  setting.each do |field, value|
  result[field] ||= value
  end
@@ -168,11 +171,7 @@ def within_namespace(&_block)
  # the superclass's :inheritable_setting.
  def build_top_level_setting
  Grape::Util::InheritableSetting.new.tap do |setting|
- # Doesn't try to inherit settings from +Grape::API::Instance+ which also responds to
- # +inheritable_setting+, however, it doesn't contain any user-defined settings.
- # Otherwise, it would lead to an extra instance of +Grape::Util::InheritableSetting+
- # in the chain for every endpoint.
- if defined?(superclass) && superclass.respond_to?(:inheritable_setting) && superclass != Grape::API::Instance
+ if defined?(superclass) && superclass.respond_to?(:inheritable_setting) && superclass != Grape::API
  setting.inherit_from superclass.inheritable_setting
  end
  end

lib/grape/endpoint.rb

@@ -386,8 +386,24 @@ def run_filters(filters, type = :other)
  extend post_extension if post_extension
  end
 
- %i[befores before_validations after_validations afters finallies].each do |meth_id|
- define_method(meth_id) { namespace_stackable(meth_id) }
+ def befores
+ namespace_stackable(:befores) || []
+ end
+
+ def before_validations
+ namespace_stackable(:before_validations) || []
+ end
+
+ def after_validations
+ namespace_stackable(:after_validations) || []
+ end
+
+ def afters
+ namespace_stackable(:afters) || []
+ end
+
+ def finallies
+ namespace_stackable(:finallies) || []
  end
 
  def validations

lib/grape/util/inheritable_setting.rb

@@ -6,7 +6,7 @@ module Util
  # and inheritable values (see InheritableValues and StackableValues).
  class InheritableSetting
  attr_accessor :route, :api_class, :namespace
- attr_accessor :namespace_inheritable, :namespace_stackable
+ attr_accessor :namespace_inheritable, :namespace_stackable, :namespace_reverse_stackable
  attr_accessor :parent, :point_in_time_copies
 
  # Retrieve global settings.
@@ -31,6 +31,7 @@ def initialize
  # used with a mount, or should every API::Class be a separate namespace by default?
  self.namespace_inheritable = InheritableValues.new
  self.namespace_stackable = StackableValues.new
+ self.namespace_reverse_stackable = ReverseStackableValues.new
 
  self.point_in_time_copies = []
 
@@ -53,6 +54,7 @@ def inherit_from(parent)
 
  namespace_inheritable.inherited_values = parent.namespace_inheritable
  namespace_stackable.inherited_values = parent.namespace_stackable
+ namespace_reverse_stackable.inherited_values = parent.namespace_reverse_stackable
  self.route = parent.route.merge(route)
 
  point_in_time_copies.map { |cloned_one| cloned_one.inherit_from parent }
@@ -70,6 +72,7 @@ def point_in_time_copy
  new_setting.namespace = namespace.clone
  new_setting.namespace_inheritable = namespace_inheritable.clone
  new_setting.namespace_stackable = namespace_stackable.clone
+ new_setting.namespace_reverse_stackable = namespace_reverse_stackable.clone
  new_setting.route = route.clone
  new_setting.api_class = api_class
 
@@ -90,7 +93,8 @@ def to_hash
  route: route.clone,
  namespace: namespace.to_hash,
  namespace_inheritable: namespace_inheritable.to_hash,
- namespace_stackable: namespace_stackable.to_hash
+ namespace_stackable: namespace_stackable.to_hash,
+ namespace_reverse_stackable: namespace_reverse_stackable.to_hash
  }
  end
  end

lib/grape/util/reverse_stackable_values.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require_relative 'stackable_values'
+
+module Grape
+ module Util
+ class ReverseStackableValues < StackableValues
+ protected
+
+ def concat_values(inherited_value, new_value)
+ [].tap do |value|
+ value.concat(new_value)
+ value.concat(inherited_value)
+ end
+ end
+ end
+ end
+end

lib/grape/util/stackable_values.rb

@@ -13,7 +13,6 @@ def initialize(*_args)
  @frozen_values = {}
  end
 
- # Even if there is no value, an empty array will be returned.
  def [](name)
  return @frozen_values[name] if @frozen_values.key? name
 

lib/grape/validations/attributes_iterator.rb

@@ -11,7 +11,6 @@ def initialize(validator, scope, params)
  @scope = scope
  @attrs = validator.attrs
  @original_params = scope.params(params)
- @array_given = @original_params.is_a?(Array)
  @params = Array.wrap(@original_params)
  end
 
@@ -31,7 +30,7 @@ def do_each(params_to_process, parent_indicies = [], &block)
  end
 
  if @scope.type == Array
- next unless @array_given # do not validate content of array if it isn't array
+ next unless @original_params.is_a?(Array) # do not validate content of array if it isn't array
 
  # fill current and parent scopes with correct array indicies
  parent_scope = @scope.parent

lib/grape/validations/params_scope.rb

@@ -45,10 +45,8 @@ def configuration
  # @return [Boolean] whether or not this entire scope needs to be
  # validated
  def should_validate?(parameters)
- scoped_params = params(parameters)
-
- return false if @optional && (scoped_params.blank? || all_element_blank?(scoped_params))
- return false unless meets_dependency?(scoped_params, parameters)
+ return false if @optional && (params(parameters).blank? || all_element_blank?(parameters))
+ return false unless meets_dependency?(params(parameters), parameters)
  return true if parent.nil?
  parent.should_validate?(parameters)
  end
@@ -448,8 +446,8 @@ def options_key?(type, key, validations)
  validations[type].respond_to?(:key?) && validations[type].key?(key) && !validations[type][key].nil?
  end
 
- def all_element_blank?(scoped_params)
- scoped_params.respond_to?(:all?) && scoped_params.all?(&:blank?)
+ def all_element_blank?(parameters)
+ params(parameters).respond_to?(:all?) && params(parameters).all?(&:blank?)
  end
 
  # Validators don't have access to each other and they don't need, however,