Remove namespace_stackable_with_hash from public interface

ericproulx · ericproulx · commit 3e14d24082da · 2025-09-22T10:46:26.000+02:00
- Move namespace_stackable_with_hash method from DSL::Settings to InheritableSetting
- Update all internal usage to use inheritable_setting.namespace_stackable_with_hash
- Remove public method from DSL::Settings module
- Update tests to reflect the new internal structure
- Increase RuboCop AbcSize limit to accommodate refactored code
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -35,7 +35,7 @@ Style/Send:
   Enabled: true
 
 Metrics/AbcSize:
-  Max: 45
+  Max: 50
 
 Metrics/BlockLength:
   Max: 30
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
 * [#2600](https://github.com/ruby-grape/grape/pull/2600): Refactor versioner middleware: simplify base class and improve consistency - [@ericproulx](https://github.com/ericproulx).
 * [#2601](https://github.com/ruby-grape/grape/pull/2601): Refactor route_setting internal usage to use inheritable_setting.route for improved consistency and performance - [@ericproulx](https://github.com/ericproulx).
 * [#2602](https://github.com/ruby-grape/grape/pull/2602): Remove `namespace_reverse_stackable` from public DSL interface and use direct inheritable_setting access - [@ericproulx](https://github.com/ericproulx).
+* [#2603](https://github.com/ruby-grape/grape/pull/2603): Remove `namespace_stackable_with_hash` from public interface and move to internal InheritableSetting - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes
diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
@@ -389,22 +389,22 @@ def route
       # @return [Class] the located Entity class, or nil if none is found
       def entity_class_for_obj(object, options)
         entity_class = options.delete(:with)
-
-        if entity_class.nil?
-          # entity class not explicitly defined, auto-detect from relation#klass or first object in the collection
-          object_class = if object.respond_to?(:klass)
-                           object.klass
-                         else
-                           object.respond_to?(:first) ? object.first.class : object.class
-                         end
-
-          object_class.ancestors.each do |potential|
-            entity_class ||= (namespace_stackable_with_hash(:representations) || {})[potential]
-          end
-
-          entity_class ||= object_class.const_get(:Entity) if object_class.const_defined?(:Entity) && object_class.const_get(:Entity).respond_to?(:represent)
+        return entity_class if entity_class
+
+        # entity class not explicitly defined, auto-detect from relation#klass or first object in the collection
+        object_class = if object.respond_to?(:klass)
+                         object.klass
+                       else
+                         object.respond_to?(:first) ? object.first.class : object.class
+                       end
+
+        representations = inheritable_setting.namespace_stackable_with_hash(:representations)
+        if representations
+          potential = object_class.ancestors.detect { |potential| representations.key?(potential) }
+          entity_class = representations[potential] if potential
         end
 
+        entity_class = object_class.const_get(:Entity) if !entity_class && object_class.const_defined?(:Entity) && object_class.const_get(:Entity).respond_to?(:represent)
         entity_class
       end
 
diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb
@@ -54,7 +54,7 @@ def build_with(build_with)
       #       end
       #     end
       def use(*names)
-        named_params = @api.namespace_stackable_with_hash(:named_params) || {}
+        named_params = @api.inheritable_setting.namespace_stackable_with_hash(:named_params) || {}
         options = names.extract_options!
         names.each do |name|
           params_block = named_params.fetch(name) do
diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb
@@ -60,7 +60,7 @@ def content_type(key, val)
 
       # All available content types.
       def content_types
-        c_types = namespace_stackable_with_hash(:content_types)
+        c_types = inheritable_setting.namespace_stackable_with_hash(:content_types)
         Grape::ContentTypes.content_types_for c_types
       end
 
diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb
@@ -142,14 +142,17 @@ def mount(mounts, *opts)
       #   end
       def route(methods, paths = ['/'], route_options = {}, &block)
         method = methods == :any ? '*' : methods
-        description = inheritable_setting.route[:description] || {}
+        endpoint_params = inheritable_setting.namespace_stackable_with_hash(:params) || {}
+        endpoint_description = inheritable_setting.route[:description]
+        all_route_options = { params: endpoint_params }
+        all_route_options.deep_merge!(endpoint_description) if endpoint_description
+        all_route_options.deep_merge!(route_options) if route_options&.any?
+
         endpoint_options = {
           method: method,
           path: paths,
           for: self,
-          route_options: {
-            params: namespace_stackable_with_hash(:params) || {}
-          }.deep_merge(description).deep_merge(route_options || {})
+          route_options: all_route_options
         }
 
         new_endpoint = Grape::Endpoint.new(inheritable_setting, endpoint_options, &block)
diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb
@@ -46,13 +46,6 @@ def namespace_setting(key, value = nil)
         get_or_set(inheritable_setting.namespace, key, value)
       end
 
-      def namespace_stackable_with_hash(key)
-        settings = namespace_stackable(key)
-        return if settings.blank?
-
-        settings.each_with_object({}) { |value, result| result.deep_merge!(value) }
-      end
-
       private
 
       # Execute the block within a context where our inheritable settings are forked
diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
@@ -321,7 +321,7 @@ def options?
     def build_stack
       stack = Grape::Middleware::Stack.new
 
-      content_types = namespace_stackable_with_hash(:content_types)
+      content_types = inheritable_setting.namespace_stackable_with_hash(:content_types)
       format = namespace_inheritable(:format)
 
       stack.use Rack::Head
@@ -333,10 +333,10 @@ def build_stack
                 rescue_all: namespace_inheritable(:rescue_all),
                 rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions),
                 default_error_formatter: namespace_inheritable(:default_error_formatter),
-                error_formatters: namespace_stackable_with_hash(:error_formatters),
-                rescue_options: namespace_stackable_with_hash(:rescue_options),
+                error_formatters: inheritable_setting.namespace_stackable_with_hash(:error_formatters),
+                rescue_options: inheritable_setting.namespace_stackable_with_hash(:rescue_options),
                 rescue_handlers: rescue_handlers,
-                base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers),
+                base_only_rescue_handlers: inheritable_setting.namespace_stackable_with_hash(:base_only_rescue_handlers),
                 all_rescue_handler: namespace_inheritable(:all_rescue_handler),
                 grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler)
 
@@ -354,8 +354,8 @@ def build_stack
                 format: format,
                 default_format: namespace_inheritable(:default_format) || :txt,
                 content_types: content_types,
-                formatters: namespace_stackable_with_hash(:formatters),
-                parsers: namespace_stackable_with_hash(:parsers)
+                formatters: inheritable_setting.namespace_stackable_with_hash(:formatters),
+                parsers: inheritable_setting.namespace_stackable_with_hash(:parsers)
 
       builder = stack.build
       builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run }
diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb
@@ -95,6 +95,13 @@ def to_hash
           namespace_reverse_stackable: namespace_reverse_stackable.to_hash
         }
       end
+
+      def namespace_stackable_with_hash(key)
+        data = namespace_stackable[key]
+        return if data.blank?
+
+        data.each_with_object({}) { |value, result| result.deep_merge!(value) }
+      end
     end
   end
 end
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
@@ -116,9 +116,10 @@
     end
 
     it 'adds the association to the :representations setting' do
-      klass = Class.new
-      subject.represent Object, with: klass
-      expect(subject.namespace_stackable_with_hash(:representations)[Object]).to eq(klass)
+      dummy_presenter_klass = Class.new
+      represent_object = Class.new
+      subject.represent represent_object, with: dummy_presenter_klass
+      expect(subject.inheritable_setting.namespace_stackable[:representations]).to eq([represent_object => dummy_presenter_klass])
     end
   end
 
diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb
@@ -6,37 +6,14 @@
   let(:dummy_class) do
     Class.new do
       include Grape::DSL::InsideRoute
+      include Grape::DSL::Settings
 
       attr_reader :env, :request, :new_settings
 
       def initialize
         @env = {}
         @header = {}
-        @new_settings = { namespace_inheritable: namespace_inheritable_hash, namespace_stackable: namespace_stackable_hash }
-      end
-
-      def namespace_inheritable(key, value = nil)
-        if value
-          namespace_inheritable_hash[key] = value
-        else
-          namespace_inheritable_hash[key]
-        end
-      end
-
-      def self.namespace_stackable(key, value = nil)
-        if value
-          namespace_stackable_hash[key] << value
-        else
-          namespace_stackable_hash[key]
-        end
-      end
-
-      def namespace_stackable_with_hash(key, value = nil)
-        if value
-          namespace_stackable_with_hash_hash[key] = value
-        else
-          namespace_stackable_with_hash_hash[key]
-        end
+        @new_settings = { namespace_inheritable: inheritable_setting.namespace_inheritable, namespace_stackable: inheritable_setting.namespace_stackable }
       end
 
       def header(key = nil, val = nil)
@@ -46,20 +23,6 @@ def header(key = nil, val = nil)
           @header ||= Grape::Util::Header.new
         end
       end
-
-      private
-
-      def namespace_inheritable_hash
-        @namespace_inheritable_hash ||= {}
-      end
-
-      def namespace_stackable_hash
-        @namespace_stackable_hash ||= Hash.new { [] }
-      end
-
-      def namespace_stackable_with_hash_hash
-        @namespace_stackable_with_hash_hash ||= {}
-      end
     end
   end
 
diff --git a/spec/grape/dsl/parameters_spec.rb b/spec/grape/dsl/parameters_spec.rb
@@ -68,13 +68,14 @@ def extract_message_option(attrs)
     let(:named_params) { { params_group: proc {} } }
 
     it 'calls processes associated with named params' do
-      allow(subject.api).to receive(:namespace_stackable_with_hash).and_return(named_params)
+      subject.api = Class.new { include Grape::DSL::Settings }.new
+      subject.api.inheritable_setting.namespace_stackable[:named_params] = named_params
       expect(subject).to receive(:instance_exec).with(options).and_yield
       subject.use :params_group, options
     end
 
     it 'raises error when non-existent named param is called' do
-      allow(subject.api).to receive(:namespace_stackable_with_hash).and_return({})
+      subject.api = Class.new { include Grape::DSL::Settings }.new
       expect { subject.use :params_group }.to raise_error('Params :params_group not found!')
     end
   end
