Use Active support functions (#2326)

ericproulx · web-flow · commit 280e5b3aab35 · 2023-05-15T22:43:48.000+02:00
* Replace deep_mergeable_hash.rb and deep_symbolize_hash.rb by ActiveSupport functions
Use each_with_object instead of each/inject etc ...

* Replace if env[...] by env.key?
Use deep_dup instead of dup for rack_params

* Use presence instead of ternary
Replace unless present? by if blank?

* Replace !include? by exclude?

* Use ActiveSupport duplicable?

* Fix rubocop and update CHANGELOG

* Add minimal version for ActiveSupport &gt;=5

* Remove extra * in CHANGELOG

* Next version 1.8.0
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,8 @@
-### 1.7.2 (Next)
+### 1.8.0 (Next)
 
 #### Features
 
+* [#2326](https://github.com/ruby-grape/grape/pull/2326): Use ActiveSupport extensions - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes
diff --git a/README.md b/README.md
@@ -159,7 +159,7 @@ content negotiation, versioning and much more.
 
 ## Stable Release
 
-You're reading the documentation for the next release of Grape, which should be **1.7.2**.
+You're reading the documentation for the next release of Grape, which should be **1.8.0**.
 Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version.
 The current stable release is [1.7.1](https://github.com/ruby-grape/grape/blob/v1.7.1/README.md).
 
diff --git a/grape.gemspec b/grape.gemspec
@@ -20,7 +20,7 @@ Gem::Specification.new do |s|
     'source_code_uri' => "https://github.com/ruby-grape/grape/tree/v#{s.version}"
   }
 
-  s.add_runtime_dependency 'activesupport'
+  s.add_runtime_dependency 'activesupport', '>= 5'
   s.add_runtime_dependency 'builder'
   s.add_runtime_dependency 'dry-types', '>= 1.1'
   s.add_runtime_dependency 'mustermann-grape', '~> 1.0.0'
diff --git a/lib/grape.rb b/lib/grape.rb
@@ -20,9 +20,11 @@
 require 'active_support/core_ext/hash/deep_merge'
 require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/hash/indifferent_access'
+require 'active_support/core_ext/hash/keys'
 require 'active_support/core_ext/hash/reverse_merge'
 require 'active_support/core_ext/hash/slice'
 require 'active_support/core_ext/object/blank'
+require 'active_support/core_ext/object/duplicable'
 require 'active_support/dependencies/autoload'
 require 'active_support/notifications'
 require 'i18n'
@@ -92,8 +94,6 @@ module Exceptions
   module Extensions
     extend ::ActiveSupport::Autoload
     eager_autoload do
-      autoload :DeepMergeableHash
-      autoload :DeepSymbolizeHash
       autoload :Hash
     end
     module ActiveSupport
diff --git a/lib/grape/content_types.rb b/lib/grape/content_types.rb
@@ -17,17 +17,11 @@ module ContentTypes
 
     class << self
       def content_types_for_settings(settings)
-        return if settings.blank?
-
-        settings.each_with_object({}) { |value, result| result.merge!(value) }
+        settings&.inject(:merge!)
       end
 
       def content_types_for(from_settings)
-        if from_settings.present?
-          from_settings
-        else
-          Grape::ContentTypes::CONTENT_TYPES.merge(default_elements)
-        end
+        from_settings.presence || Grape::ContentTypes::CONTENT_TYPES.merge(default_elements)
       end
     end
   end
diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
@@ -103,7 +103,7 @@ def handle_passed_param(params_nested_path, has_passed_children = false, &_block
 
           if type == 'Hash' && !has_children
             {}
-          elsif type == 'Array' || (type&.start_with?('[') && !type&.include?(','))
+          elsif type == 'Array' || (type&.start_with?('[') && type&.exclude?(','))
             []
           elsif type == 'Set' || type&.start_with?('#<Set')
             Set.new
@@ -433,7 +433,7 @@ def entity_class_for_obj(object, options)
       #   the given entity_class.
       def entity_representation_for(entity_class, object, options)
         embeds = { env: env }
-        embeds[:version] = env[Grape::Env::API_VERSION] if env[Grape::Env::API_VERSION]
+        embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION)
         entity_class.represent(object, **embeds.merge(options))
       end
     end
diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb
@@ -109,13 +109,9 @@ def namespace_reverse_stackable_with_hash(key)
         settings = get_or_set :namespace_reverse_stackable, key, nil
         return if settings.blank?
 
-        result = {}
-        settings.each do |setting|
-          setting.each do |field, value|
-            result[field] ||= value
-          end
+        settings.each_with_object({}) do |setting, result|
+          result.merge!(setting) { |_k, s1, _s2| s1 }
         end
-        result
       end
 
       # (see #unset_global_setting)
diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
@@ -171,10 +171,9 @@ def to_routes
     end
 
     def prepare_routes_requirements
-      endpoint_requirements = options[:route_options][:requirements] || {}
-      all_requirements = (namespace_stackable(:namespace).map(&:requirements) << endpoint_requirements)
-      all_requirements.reduce({}) do |base_requirements, single_requirements|
-        base_requirements.merge!(single_requirements)
+      {}.merge!(*namespace_stackable(:namespace).map(&:requirements)).tap do |requirements|
+        endpoint_requirements = options.dig(:route_options, :requirements)
+        requirements.merge!(endpoint_requirements) if endpoint_requirements
       end
     end
 
diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb
@@ -27,7 +27,7 @@ def present(message, env)
 
         if presenter
           embeds = { env: env }
-          embeds[:version] = env[Grape::Env::API_VERSION] if env[Grape::Env::API_VERSION]
+          embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION)
           presented_message = presenter.represent(presented_message, embeds).serializable_hash
         end
 
diff --git a/lib/grape/exceptions/base.rb b/lib/grape/exceptions/base.rb
@@ -73,11 +73,11 @@ def translate(key, **options)
         options = options.dup
         options[:default] &&= options[:default].to_s
         message = ::I18n.translate(key, **options)
-        message.present? ? message : fallback_message(key, **options)
+        message.presence || fallback_message(key, **options)
       end
 
       def fallback_message(key, **options)
-        if ::I18n.enforce_available_locales && !::I18n.available_locales.include?(FALLBACK_LOCALE)
+        if ::I18n.enforce_available_locales && ::I18n.available_locales.exclude?(FALLBACK_LOCALE)
           key
         else
           ::I18n.translate(key, locale: FALLBACK_LOCALE, **options)
diff --git a/lib/grape/exceptions/validation_errors.rb b/lib/grape/exceptions/validation_errors.rb
@@ -13,12 +13,7 @@ class ValidationErrors < Grape::Exceptions::Base
       attr_reader :errors
 
       def initialize(errors: [], headers: {}, **_options)
-        @errors = {}
-        errors.each do |validation_error|
-          @errors[validation_error.params] ||= []
-          @errors[validation_error.params] << validation_error
-        end
-
+        @errors = errors.group_by(&:params)
         super message: full_messages.join(', '), status: 400, headers: headers
       end
 
diff --git a/lib/grape/extensions/active_support/hash_with_indifferent_access.rb b/lib/grape/extensions/active_support/hash_with_indifferent_access.rb
@@ -16,9 +16,9 @@ def params_builder
           end
 
           def build_params
-            params = ::ActiveSupport::HashWithIndifferentAccess.new(rack_params)
-            params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS]
-            params
+            ::ActiveSupport::HashWithIndifferentAccess.new(rack_params).tap do |params|
+              params.deep_merge!(grape_routing_args) if env.key?(Grape::Env::GRAPE_ROUTING_ARGS)
+            end
           end
         end
       end
diff --git a/lib/grape/extensions/deep_mergeable_hash.rb b/lib/grape/extensions/deep_mergeable_hash.rb
diff --git a/lib/grape/extensions/deep_symbolize_hash.rb b/lib/grape/extensions/deep_symbolize_hash.rb
diff --git a/lib/grape/extensions/hash.rb b/lib/grape/extensions/hash.rb
@@ -11,13 +11,10 @@ module ParamBuilder
         end
 
         def build_params
-          params = Grape::Extensions::DeepMergeableHash[rack_params]
-          params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS]
-          post_process_params(params)
-        end
-
-        def post_process_params(params)
-          Grape::Extensions::DeepSymbolizeHash.deep_symbolize_keys_in(params)
+          rack_params.deep_dup.tap do |params|
+            params.deep_merge!(grape_routing_args) if env.key?(Grape::Env::GRAPE_ROUTING_ARGS)
+            params.deep_symbolize_keys!
+          end
         end
       end
     end
diff --git a/lib/grape/extensions/hashie/mash.rb b/lib/grape/extensions/hashie/mash.rb
@@ -15,9 +15,9 @@ def params_builder
           end
 
           def build_params
-            params = ::Hashie::Mash.new(rack_params)
-            params.deep_merge!(grape_routing_args) if env[Grape::Env::GRAPE_ROUTING_ARGS]
-            params
+            ::Hashie::Mash.new(rack_params).tap do |params|
+              params.deep_merge!(grape_routing_args) if env.key?(Grape::Env::GRAPE_ROUTING_ARGS)
+            end
           end
         end
       end
diff --git a/lib/grape/formatter/serializable_hash.rb b/lib/grape/formatter/serializable_hash.rb
@@ -15,24 +15,24 @@ def call(object, _env)
         private
 
         def serializable?(object)
-          object.respond_to?(:serializable_hash) || (object.is_a?(Array) && object.all? { |o| o.respond_to? :serializable_hash }) || object.is_a?(Hash)
+          object.respond_to?(:serializable_hash) || array_serializable?(object) || object.is_a?(Hash)
         end
 
         def serialize(object)
           if object.respond_to? :serializable_hash
             object.serializable_hash
-          elsif object.is_a?(Array) && object.all? { |o| o.respond_to? :serializable_hash }
+          elsif array_serializable?(object)
             object.map(&:serializable_hash)
           elsif object.is_a?(Hash)
-            h = {}
-            object.each_pair do |k, v|
-              h[k] = serialize(v)
-            end
-            h
+            object.transform_values { |v| serialize(v) }
           else
             object
           end
         end
+
+        def array_serializable?(object)
+          object.is_a?(Array) && object.all? { |o| o.respond_to? :serializable_hash }
+        end
       end
     end
   end
diff --git a/lib/grape/middleware/auth/base.rb b/lib/grape/middleware/auth/base.rb
@@ -28,7 +28,7 @@ def _call(env)
 
             strategy_info = Grape::Middleware::Auth::Strategies[options[:type]]
 
-            throw(:error, status: 401, message: 'API Authorization Failed.') unless strategy_info.present?
+            throw(:error, status: 401, message: 'API Authorization Failed.') if strategy_info.blank?
 
             strategy = strategy_info.create(@app, options) do |*args|
               auth_proc_context.instance_exec(*args, &auth_proc)
diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb
@@ -103,7 +103,7 @@ def read_rack_input(body)
           begin
             body = (env[Grape::Env::API_REQUEST_BODY] = parser.call(body, env))
             if body.is_a?(Hash)
-              env[Grape::Env::RACK_REQUEST_FORM_HASH] = if env[Grape::Env::RACK_REQUEST_FORM_HASH]
+              env[Grape::Env::RACK_REQUEST_FORM_HASH] = if env.key?(Grape::Env::RACK_REQUEST_FORM_HASH)
                                                           env[Grape::Env::RACK_REQUEST_FORM_HASH].merge(body)
                                                         else
                                                           body
diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb
@@ -57,8 +57,7 @@ def strict_accept_header_presence_check
         end
 
         def strict_version_vendor_accept_header_presence_check
-          return unless versions.present?
-          return if an_accept_header_with_version_and_vendor_is_present?
+          return if versions.blank? || an_accept_header_with_version_and_vendor_is_present?
 
           fail_with_invalid_accept_header!('API vendor or version not found.')
         end
@@ -101,25 +100,18 @@ def fail_with_invalid_version_header!(message)
         end
 
         def available_media_types
-          available_media_types = []
-
-          content_types.each_key do |extension|
-            versions.reverse_each do |version|
-              available_media_types += [
-                "application/vnd.#{vendor}-#{version}+#{extension}",
-                "application/vnd.#{vendor}-#{version}"
-              ]
+          [].tap do |available_media_types|
+            content_types.each_key do |extension|
+              versions.reverse_each do |version|
+                available_media_types << "application/vnd.#{vendor}-#{version}+#{extension}"
+                available_media_types << "application/vnd.#{vendor}-#{version}"
+              end
+              available_media_types << "application/vnd.#{vendor}+#{extension}"
             end
-            available_media_types << "application/vnd.#{vendor}+#{extension}"
-          end
-
-          available_media_types << "application/vnd.#{vendor}"
 
-          content_types.each_value do |media_type|
-            available_media_types << media_type
+            available_media_types << "application/vnd.#{vendor}"
+            available_media_types.concat(content_types.values.flatten)
           end
-
-          available_media_types.flatten
         end
 
         def headers_contain_wrong_vendor?
@@ -130,7 +122,7 @@ def headers_contain_wrong_vendor?
 
         def headers_contain_wrong_version?
           header.values.all? do |header_value|
-            version?(header_value) && !versions.include?(request_version(header_value))
+            version?(header_value) && versions.exclude?(request_version(header_value))
           end
         end
 
diff --git a/lib/grape/util/lazy_value.rb b/lib/grape/util/lazy_value.rb
@@ -70,29 +70,21 @@ def initialize(array)
       end
 
       def evaluate
-        evaluated = []
-        @value_hash.each_with_index do |value, index|
-          evaluated[index] = value.evaluate
-        end
-        evaluated
+        @value_hash.map(&:evaluate)
       end
     end
 
     class LazyValueHash < LazyValueEnumerable
       def initialize(hash)
         super
-        @value_hash = {}.with_indifferent_access
+        @value_hash = ActiveSupport::HashWithIndifferentAccess.new
         hash.each do |key, value|
           self[key] = value
         end
       end
 
       def evaluate
-        evaluated = {}.with_indifferent_access
-        @value_hash.each do |key, value|
-          evaluated[key] =  value.evaluate
-        end
-        evaluated
+        @value_hash.transform_values(&:evaluate)
       end
     end
   end
diff --git a/lib/grape/util/strict_hash_configuration.rb b/lib/grape/util/strict_hash_configuration.rb
diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb
diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb
diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb
diff --git a/lib/grape/validations/validators/default_validator.rb b/lib/grape/validations/validators/default_validator.rb
diff --git a/lib/grape/version.rb b/lib/grape/version.rb
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ Gem::Specification.new do \|s\|`
`20`	`20`	`'source_code_uri' => "https://github.com/ruby-grape/grape/tree/v#{s.version}"`
`21`	`21`	`}`
`22`	`22`
`23`		`- s.add_runtime_dependency 'activesupport'`
	`23`	`+ s.add_runtime_dependency 'activesupport', '>= 5'`
`24`	`24`	`s.add_runtime_dependency 'builder'`
`25`	`25`	`s.add_runtime_dependency 'dry-types', '>= 1.1'`
`26`	`26`	`s.add_runtime_dependency 'mustermann-grape', '~> 1.0.0'`