Refactor router architecture for better encapsulation

- Convert Route classes to use explicit parameters instead of options hashes
- Add Forwardable delegation for cleaner interfaces
- Move Pattern construction to caller for better separation of concerns
- Improve API design with keyword arguments throughout router system
- Replace nested option access (route.options[:endpoint]) with direct methods
- Simplify code with flat_map and remove unnecessary splat operators
- Extract DSL_METHODS constant in ApiDescription for reusability

This refactoring improves code maintainability, testability, and performance
while maintaining backward compatibility with the public API.

Author: ericproulx

lib/grape/api/instance.rb

@@ -177,7 +177,7 @@ def cascade?
       def add_head_not_allowed_methods_and_options_methods
         # The paths we collected are prepared (cf. Path#prepare), so they
         # contain already versioning information when using path versioning.
-        all_routes = self.class.endpoints.map(&:routes).flatten
+        all_routes = self.class.endpoints.flat_map(&:routes)
 
         # Disable versioning so adding a route won't prepend versioning
         # informations again.
@@ -186,23 +186,21 @@ def add_head_not_allowed_methods_and_options_methods
 
       def collect_route_config_per_pattern(all_routes)
         routes_by_regexp = all_routes.group_by(&:pattern_regexp)
+        namespace_inheritable = self.class.inheritable_setting.namespace_inheritable
 
         # Build the configuration based on the first endpoint and the collection of methods supported.
         routes_by_regexp.each_value do |routes|
-          last_route = routes.last # Most of the configuration is taken from the last endpoint
           next if routes.any? { |route| route.request_method == '*' }
 
-          namespace_inheritable = self.class.inheritable_setting.namespace_inheritable
+          last_route = routes.last # Most of the configuration is taken from the last endpoint
           allowed_methods = routes.map(&:request_method)
           allowed_methods |= [Rack::HEAD] if !namespace_inheritable[:do_not_route_head] && allowed_methods.include?(Rack::GET)
 
           allow_header = namespace_inheritable[:do_not_route_options] ? allowed_methods : [Rack::OPTIONS] | allowed_methods
           last_route.app.options[:options_route_enabled] = true unless namespace_inheritable[:do_not_route_options] || allowed_methods.include?(Rack::OPTIONS)
 
-          @router.associate_routes(last_route.pattern, {
-                                     endpoint: last_route.app,
-                                     allow_header: allow_header
-                                   })
+          greedy_route = Grape::Router::GreedyRoute.new(last_route.pattern, endpoint: last_route.app, allow_header: allow_header)
+          @router.associate_routes(greedy_route)
         end
       end
 

lib/grape/dsl/routing.rb

@@ -149,14 +149,14 @@ def route(methods, paths = ['/'], route_options = {}, &block)
         all_route_options.deep_merge!(endpoint_description) if endpoint_description
         all_route_options.deep_merge!(route_options) if route_options&.any?
 
-        endpoint_options = {
+        new_endpoint = Grape::Endpoint.new(
+          inheritable_setting,
           method: method,
           path: paths,
           for: self,
-          route_options: all_route_options
-        }
-
-        new_endpoint = Grape::Endpoint.new(inheritable_setting, endpoint_options, &block)
+          route_options: all_route_options,
+          &block
+        )
         endpoints << new_endpoint unless endpoints.any? { |e| e.equals?(new_endpoint) }
 
         inheritable_setting.route_end
@@ -217,7 +217,7 @@ def reset_endpoints!
       #
       # @param param [Symbol] The name of the parameter you wish to declare.
       # @option options [Regexp] You may supply a regular expression that the declared parameter must meet.
-      def route_param(param, options = {}, &block)
+      def route_param(param, **options, &block)
         options = options.dup
 
         options[:requirements] = {

lib/grape/endpoint.rb

@@ -46,10 +46,7 @@ def run_before_each(endpoint)
     # @note This happens at the time of API definition, so in this context the
     # endpoint does not know if it will be mounted under a different endpoint.
     # @yield a block defining what your API should do when this endpoint is hit
-    def initialize(new_settings, options = {}, &block)
-      require_option(options, :path)
-      require_option(options, :method)
-
+    def initialize(new_settings, **options, &block)
       self.inheritable_setting = new_settings.point_in_time_copy
 
       # now +namespace_stackable(:declared_params)+ contains all params defined for
@@ -67,7 +64,6 @@ def initialize(new_settings, options = {}, &block)
       @options[:path] << '/' if options[:path].empty?
 
       @options[:method] = Array(options[:method])
-      @options[:route_options] ||= {}
 
       @lazy_initialize_lock = Mutex.new
       @lazy_initialized = nil
@@ -88,10 +84,6 @@ def inherit_settings(namespace_stackable)
       endpoints&.each { |e| e.inherit_settings(namespace_stackable) }
     end
 
-    def require_option(options, key)
-      raise Grape::Exceptions::MissingOption.new(key) unless options.key?(key)
-    end
-
     def routes
       @routes ||= endpoints&.collect(&:routes)&.flatten || to_routes
     end
@@ -117,53 +109,6 @@ def mount_in(router)
       end
     end
 
-    def to_routes
-      default_route_options = prepare_default_route_attributes
-
-      map_routes do |method, raw_path|
-        prepared_path = Path.new(raw_path, namespace, prepare_default_path_settings)
-        params = options[:route_options].present? ? options[:route_options].merge(default_route_options) : default_route_options
-        route = Grape::Router::Route.new(method, prepared_path.origin, prepared_path.suffix, params)
-        route.apply(self)
-      end.flatten
-    end
-
-    def prepare_routes_requirements
-      {}.merge!(*inheritable_setting.namespace_stackable[:namespace].map(&:requirements)).tap do |requirements|
-        endpoint_requirements = options.dig(:route_options, :requirements)
-        requirements.merge!(endpoint_requirements) if endpoint_requirements
-      end
-    end
-
-    def prepare_default_route_attributes
-      {
-        namespace: namespace,
-        version: prepare_version,
-        requirements: prepare_routes_requirements,
-        prefix: inheritable_setting.namespace_inheritable[:root_prefix],
-        anchor: options[:route_options].fetch(:anchor, true),
-        settings: inheritable_setting.route.except(:declared_params, :saved_validations),
-        forward_match: options[:forward_match]
-      }
-    end
-
-    def prepare_version
-      version = inheritable_setting.namespace_inheritable[:version]
-      return if version.blank?
-
-      version.length == 1 ? version.first : version
-    end
-
-    def map_routes
-      options[:method].map { |method| options[:path].map { |path| yield method, path } }
-    end
-
-    def prepare_default_path_settings
-      namespace_stackable_hash = inheritable_setting.namespace_stackable.to_hash
-      namespace_inheritable_hash = inheritable_setting.namespace_inheritable.to_hash
-      namespace_stackable_hash.merge!(namespace_inheritable_hash)
-    end
-
     def namespace
       @namespace ||= Namespace.joined_space_path(inheritable_setting.namespace_stackable[:namespace])
     end
@@ -311,6 +256,59 @@ def options?
 
     private
 
+    def to_routes
+      route_options = options[:route_options]
+      default_route_options = prepare_default_route_attributes(route_options)
+      complete_route_options = route_options.merge(default_route_options)
+      path_settings = prepare_default_path_settings
+
+      options[:method].flat_map do |method|
+        options[:path].map do |path|
+          prepared_path = Path.new(path, default_route_options[:namespace], path_settings)
+          pattern = Grape::Router::Pattern.new(
+            origin: prepared_path.origin,
+            suffix: prepared_path.suffix,
+            anchor: default_route_options[:anchor],
+            params: route_options[:params],
+            format: options[:format],
+            version: default_route_options[:version],
+            requirements: default_route_options[:requirements]
+          )
+          Grape::Router::Route.new(self, method, pattern, complete_route_options)
+        end
+      end
+    end
+
+    def prepare_default_route_attributes(route_options)
+      {
+        namespace: namespace,
+        version: prepare_version(inheritable_setting.namespace_inheritable[:version]),
+        requirements: prepare_routes_requirements(route_options[:requirements]),
+        prefix: inheritable_setting.namespace_inheritable[:root_prefix],
+        anchor: route_options.fetch(:anchor, true),
+        settings: inheritable_setting.route.except(:declared_params, :saved_validations),
+        forward_match: options[:forward_match]
+      }
+    end
+
+    def prepare_default_path_settings
+      namespace_stackable_hash = inheritable_setting.namespace_stackable.to_hash
+      namespace_inheritable_hash = inheritable_setting.namespace_inheritable.to_hash
+      namespace_stackable_hash.merge!(namespace_inheritable_hash)
+    end
+
+    def prepare_routes_requirements(route_options_requirements)
+      namespace_requirements = inheritable_setting.namespace_stackable[:namespace].filter_map(&:requirements)
+      namespace_requirements << route_options_requirements if route_options_requirements.present?
+      namespace_requirements.reduce({}, :merge)
+    end
+
+    def prepare_version(namespace_inheritable_version)
+      return if namespace_inheritable_version.blank?
+
+      namespace_inheritable_version.length == 1 ? namespace_inheritable_version.first : namespace_inheritable_version
+    end
+
     def build_stack
       stack = Grape::Middleware::Stack.new
 

lib/grape/router.rb

@@ -38,7 +38,7 @@ def initialize
       @optimized_map = Hash.new { |hash, key| hash[key] = // }
     end
 
-    def compile!
+    def compile! 
       return if compiled
 
       @union = Regexp.union(@neutral_regexes)
@@ -57,11 +57,9 @@ def append(route)
       map[route.request_method] << route
     end
 
-    def associate_routes(pattern, options)
-      Grape::Router::GreedyRoute.new(pattern, options).then do |greedy_route|
-        @neutral_regexes << greedy_route.to_regexp(@neutral_map.length)
-        @neutral_map << greedy_route
-      end
+    def associate_routes(greedy_route)
+      @neutral_regexes << greedy_route.to_regexp(@neutral_map.length)
+      @neutral_map << greedy_route
     end
 
     def call(env)
@@ -91,7 +89,7 @@ def identity(env)
 
     def rotation(env, exact_route = nil)
       response = nil
-      input, method = *extract_input_and_method(env)
+      input, method = extract_input_and_method(env)
       map[method].each do |route|
         next if exact_route == route
         next unless route.match?(input)
@@ -103,7 +101,7 @@ def rotation(env, exact_route = nil)
     end
 
     def transaction(env)
-      input, method = *extract_input_and_method(env)
+      input, method = extract_input_and_method(env)
 
       # using a Proc is important since `return` will exit the enclosing function
       cascade_or_return_response = proc do |response|
@@ -126,7 +124,7 @@ def transaction(env)
 
       route = match?(input, '*')
 
-      return last_neighbor_route.options[:endpoint].call(env) if last_neighbor_route && last_response_cascade && route
+      return last_neighbor_route.call(env) if last_neighbor_route && last_response_cascade && route
 
       last_response_cascade = cascade_or_return_response.call(process_route(route, env)) if route
 
@@ -142,7 +140,8 @@ def process_route(route, env)
 
     def make_routing_args(default_args, route, input)
       args = default_args || { route_info: route }
-      args.merge(route.params(input))
+      route_params = route.params(input)
+      route_params ? args.merge(route_params) : args
     end
 
     def extract_input_and_method(env)
@@ -171,12 +170,12 @@ def greedy_match?(input)
 
     def call_with_allow_headers(env, route)
       prepare_env_from_route(env, route)
-      env[Grape::Env::GRAPE_ALLOWED_METHODS] = route.options[:allow_header]
-      route.options[:endpoint].call(env)
+      env[Grape::Env::GRAPE_ALLOWED_METHODS] = route.allow_header
+      route.call(env)
     end
 
     def prepare_env_from_route(env, route)
-      input, = *extract_input_and_method(env)
+      input, = extract_input_and_method(env)
       env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(env[Grape::Env::GRAPE_ROUTING_ARGS], route, input)
     end
 

lib/grape/router/base_route.rb

@@ -3,22 +3,31 @@
 module Grape
   class Router
     class BaseRoute
+      extend Forwardable
+
       delegate_missing_to :@options
 
-      attr_reader :index, :pattern, :options
+      attr_reader :index, :options, :pattern
+
+      def_delegators :@pattern, :path, :origin
+      def_delegators :@options, :description, :version, :requirements, :prefix, :anchor, :settings, :forward_match, *Grape::Util::ApiDescription::DSL_METHODS
 
-      def initialize(options)
+      def initialize(pattern, options = {})
+        @pattern = pattern
         @options = options.is_a?(ActiveSupport::OrderedOptions) ? options : ActiveSupport::OrderedOptions.new.update(options)
       end
 
-      alias attributes options
+      # see https://github.com/ruby-grape/grape/issues/1348
+      def namespace
+        @options[:namespace]
+      end
 
       def regexp_capture_index
         CaptureIndexCache[index]
       end
 
       def pattern_regexp
-        pattern.to_regexp
+        @pattern.to_regexp
       end
 
       def to_regexp(index)

lib/grape/router/greedy_route.rb

@@ -6,14 +6,20 @@
 module Grape
   class Router
     class GreedyRoute < BaseRoute
-      def initialize(pattern, options)
-        @pattern = pattern
-        super(options)
+      extend Forwardable
+
+      def_delegators :@endpoint, :call
+
+      attr_reader :endpoint, :allow_header
+
+      def initialize(pattern, endpoint:, allow_header:)
+        super(pattern)
+        @endpoint = endpoint
+        @allow_header = allow_header
       end
 
-      # Grape::Router:Route defines params as a function
       def params(_input = nil)
-        options[:params] || {}
+        nil
       end
     end
   end

lib/grape/router/pattern.rb

@@ -13,10 +13,10 @@ class Pattern
       def_delegators :to_regexp, :===
       alias match? ===
 
-      def initialize(origin, suffix, options)
+      def initialize(origin:, suffix:, anchor:, params:, format:, version:, requirements:)
         @origin = origin
-        @path = build_path(origin, options[:anchor], suffix)
-        @pattern = build_pattern(@path, options[:params], options[:format], options[:version], options[:requirements])
+        @path = PatternCache[[build_path_from_pattern(@origin, anchor), suffix]]
+        @pattern = Mustermann::Grape.new(@path, uri_decode: true, params: params, capture: extract_capture(format, version, requirements))
         @to_regexp = @pattern.to_regexp
       end
 
@@ -28,24 +28,10 @@ def captures_default
 
       private
 
-      def build_pattern(path, params, format, version, requirements)
-        Mustermann::Grape.new(
-          path,
-          uri_decode: true,
-          params: params,
-          capture: extract_capture(format, version, requirements)
-        )
-      end
-
-      def build_path(pattern, anchor, suffix)
-        PatternCache[[build_path_from_pattern(pattern, anchor), suffix]]
-      end
-
       def extract_capture(format, version, requirements)
-        capture = {}.tap do |h|
-          h[:format] = map_str(format) if format.present?
-          h[:version] = map_str(version) if version.present?
-        end
+        capture = {}
+        capture[:format] = map_str(format) if format.present?
+        capture[:version] = map_str(version) if version.present?
 
         return capture if requirements.blank?