Optimize AttributeTranslator (#2393)

* Add GreedyRoute
Define setter methods in AttributeTranslator
Combine default route attributes + desc attributes
Refactor Pattern
Remove description in settings

* Add greedy_route_spec.rb
Remove delete options

* Revert settings description

* Rubocop

* Remove details and replace spec `details` with detail

* Add CHANGELOG.md entry

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#2383](https://github.com/ruby-grape/grape/pull/2383): Use regex block instead of if - [@ericproulx](https://github.com/ericproulx).
 * [#2384](https://github.com/ruby-grape/grape/pull/2384): Allow to use `before/after/rescue_from` methods in any order when using `mount` - [@jcagarcia](https://github.com/jcagarcia).
 * [#2390](https://github.com/ruby-grape/grape/pull/2390): Drop support for Ruby 2.6 and Rails 5 - [@ericproulx](https://github.com/ericproulx).
+* [#2393](https://github.com/ruby-grape/grape/pull/2393): Optimize AttributeTranslator - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/dsl/desc.rb

@@ -5,6 +5,27 @@ module DSL
     module Desc
       include Grape::DSL::Settings
 
+      ROUTE_ATTRIBUTES = %i[
+        body_name
+        consumes
+        default
+        deprecated
+        description
+        detail
+        entity
+        headers
+        hidden
+        http_codes
+        is_array
+        named
+        nickname
+        params
+        produces
+        security
+        summary
+        tags
+      ].freeze
+
       # Add a description to the next namespace or function.
       # @param description [String] descriptive string for this endpoint
       #   or namespace
@@ -81,26 +102,7 @@ def desc(description, options = {}, &config_block)
       # Returns an object which configures itself via an instance-context DSL.
       def desc_container(endpoint_configuration)
         Module.new do
-          include Grape::Util::StrictHashConfiguration.module(
-            :summary,
-            :description,
-            :detail,
-            :params,
-            :entity,
-            :http_codes,
-            :named,
-            :body_name,
-            :headers,
-            :hidden,
-            :deprecated,
-            :is_array,
-            :nickname,
-            :produces,
-            :consumes,
-            :security,
-            :tags,
-            :default
-          )
+          include Grape::Util::StrictHashConfiguration.module(*ROUTE_ATTRIBUTES)
           config_context.define_singleton_method(:configuration) do
             endpoint_configuration
           end

lib/grape/router.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'grape/router/route'
+require 'grape/router/greedy_route'
 require 'grape/util/cache'
 
 module Grape
@@ -48,7 +49,7 @@ def append(route)
 
     def associate_routes(pattern, **options)
       @neutral_regexes << Regexp.new("(?<_#{@neutral_map.length}>)#{pattern.to_regexp}")
-      @neutral_map << Grape::Router::AttributeTranslator.new(**options, pattern: pattern, index: @neutral_map.length)
+      @neutral_map << Grape::Router::GreedyRoute.new(pattern: pattern, index: @neutral_map.length, **options)
     end
 
     def call(env)
@@ -122,7 +123,7 @@ def process_route(route, env)
 
     def make_routing_args(default_args, route, input)
       args = default_args || { route_info: route }
-      args.merge(route.params(input) || {})
+      args.merge(route.params(input))
     end
 
     def extract_input_and_method(env)

lib/grape/router/attribute_translator.rb

@@ -3,54 +3,54 @@
 module Grape
   class Router
     # this could be an OpenStruct, but doesn't work in Ruby 2.3.0, see https://bugs.ruby-lang.org/issues/12251
+    # fixed >= 3.0
     class AttributeTranslator
-      attr_reader :attributes
-
-      ROUTE_ATTRIBUTES = %i[
-        prefix
-        version
-        settings
+      ROUTE_ATTRIBUTES = (%i[
+        allow_header
+        anchor
+        endpoint
         format
-        description
-        http_codes
-        headers
-        entity
-        details
-        requirements
-        request_method
+        forward_match
         namespace
-      ].freeze
-
-      ROUTER_ATTRIBUTES = %i[pattern index].freeze
+        not_allowed_method
+        prefix
+        request_method
+        requirements
+        settings
+        suffix
+        version
+      ] | Grape::DSL::Desc::ROUTE_ATTRIBUTES).freeze
 
       def initialize(**attributes)
         @attributes = attributes
       end
 
-      (ROUTER_ATTRIBUTES + ROUTE_ATTRIBUTES).each do |attr|
+      ROUTE_ATTRIBUTES.each do |attr|
         define_method attr do
-          attributes[attr]
+          @attributes[attr]
+        end
+
+        define_method("#{attr}=") do |val|
+          @attributes[attr] = val
         end
       end
 
       def to_h
-        attributes
+        @attributes
       end
 
       def method_missing(method_name, *args)
         if setter?(method_name)
-          attributes[method_name.to_s.chomp('=').to_sym] = args.first
+          @attributes[method_name.to_s.chomp('=').to_sym] = args.first
         else
-          attributes[method_name]
+          @attributes[method_name]
         end
       end
 
       def respond_to_missing?(method_name, _include_private = false)
-        if setter?(method_name)
-          true
-        else
-          @attributes.key?(method_name)
-        end
+        return true if setter?(method_name)
+
+        @attributes.key?(method_name)
       end
 
       private

lib/grape/router/greedy_route.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'grape/router/attribute_translator'
+require 'forwardable'
+
+# Act like a Grape::Router::Route but for greedy_match
+# see @neutral_map
+
+module Grape
+  class Router
+    class GreedyRoute
+      extend Forwardable
+
+      attr_reader :index, :pattern, :options, :attributes
+
+      delegate Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES => :@attributes
+
+      def initialize(index:, pattern:, **options)
+        @index = index
+        @pattern = pattern
+        @options = options
+        @attributes = Grape::Router::AttributeTranslator.new(**options)
+      end
+
+      # Grape::Router:Route defines params as a function
+      def params(_input = nil)
+        @attributes.params || {}
+      end
+    end
+  end
+end

lib/grape/router/pattern.rb

@@ -7,53 +7,60 @@
 module Grape
   class Router
     class Pattern
-      DEFAULT_PATTERN_OPTIONS   = { uri_decode: true }.freeze
-      DEFAULT_SUPPORTED_CAPTURE = %i[format version].freeze
-
-      attr_reader :origin, :path, :pattern, :to_regexp
+      attr_reader :origin, :path, :pattern, :to_regexp, :captures_default
 
       extend Forwardable
       def_delegators :pattern, :named_captures, :params
       def_delegators :to_regexp, :===
       alias match? ===
 
       def initialize(pattern, **options)
-        @origin  = pattern
-        @path    = build_path(pattern, **options)
-        @pattern = Mustermann::Grape.new(@path, **pattern_options(options))
+        @origin = pattern
+        @path = build_path(pattern, anchor: options[:anchor], suffix: options[:suffix])
+        @pattern = build_pattern(@path, options)
         @to_regexp = @pattern.to_regexp
+        @captures_default = regex_captures_default(@to_regexp)
       end
 
       private
 
-      def pattern_options(options)
-        capture = extract_capture(**options)
-        params = options[:params]
-        options = DEFAULT_PATTERN_OPTIONS.dup
-        options[:capture] = capture if capture.present?
-        options[:params] = params if params.present?
-        options
+      def build_pattern(path, options)
+        Mustermann::Grape.new(
+          path,
+          uri_decode: true,
+          params: options[:params],
+          capture: extract_capture(**options)
+        )
       end
 
-      def build_path(pattern, anchor: false, suffix: nil, **_options)
-        unless anchor || pattern.end_with?('*path')
-          pattern = +pattern
-          pattern << '/' unless pattern.end_with?('/')
-          pattern << '*path'
-        end
+      def build_path(pattern, anchor: false, suffix: nil)
+        PatternCache[[build_path_from_pattern(pattern, anchor: anchor), suffix]]
+      end
 
-        pattern = -pattern.split('/').tap do |parts|
-          parts[parts.length - 1] = "?#{parts.last}"
-        end.join('/') if pattern.end_with?('*path')
+      def extract_capture(**options)
+        sliced_options = options
+                         .slice(:format, :version)
+                         .delete_if { |_k, v| v.blank? }
+                         .transform_values { |v| Array.wrap(v).map(&:to_s) }
+        return sliced_options if options[:requirements].blank?
+
+        options[:requirements].merge(sliced_options)
+      end
 
-        PatternCache[[pattern, suffix]]
+      def regex_captures_default(regex)
+        names = regex.names - %w[format version] # remove default format and version
+        names.to_h { |k| [k, ''] }
       end
 
-      def extract_capture(requirements: {}, **options)
-        requirements = {}.merge(requirements)
-        DEFAULT_SUPPORTED_CAPTURE.each_with_object(requirements) do |field, capture|
-          option = Array(options[field])
-          capture[field] = option.map(&:to_s) if option.present?
+      def build_path_from_pattern(pattern, anchor: false)
+        if pattern.end_with?('*path')
+          pattern.dup.insert(pattern.rindex('/') + 1, '?')
+        elsif anchor
+          pattern
+        elsif pattern.end_with?('/')
+          "#{pattern}?*path"
+        else
+          "#{pattern}/?*path"
         end
       end
 

lib/grape/router/route.rb

@@ -7,23 +7,18 @@
 module Grape
   class Router
     class Route
-      FIXED_NAMED_CAPTURES = %w[format version].freeze
-
-      attr_accessor :pattern, :translator, :app, :index, :options
+      extend Forwardable
 
-      alias attributes translator
+      attr_reader :app, :pattern, :options, :attributes
+      attr_accessor :index
 
-      extend Forwardable
       def_delegators :pattern, :path, :origin
       delegate Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES => :attributes
 
       def initialize(method, pattern, **options)
-        method_s = method.to_s
-        method_upcase = Grape::Http::Headers.find_supported_method(method_s) || method_s.upcase
-
-        @options    = options.merge(method: method_upcase)
-        @pattern    = Pattern.new(pattern, **options)
-        @translator = AttributeTranslator.new(**options, request_method: method_upcase)
+        @options = options
+        @pattern = Grape::Router::Pattern.new(pattern, **options)
+        @attributes = Grape::Router::AttributeTranslator.new(**options, request_method: upcase_method(method))
       end
 
       def exec(env)
@@ -36,18 +31,29 @@ def apply(app)
       end
 
       def match?(input)
-        translator.respond_to?(:forward_match) && translator.forward_match ? input.start_with?(pattern.origin) : pattern.match?(input)
+        return if input.blank?
+
+        attributes.forward_match ? input.start_with?(pattern.origin) : pattern.match?(input)
       end
 
       def params(input = nil)
-        if input.nil?
-          pattern.named_captures.keys.each_with_object(translator.params) do |(key), defaults|
-            defaults[key] ||= '' unless FIXED_NAMED_CAPTURES.include?(key) || defaults.key?(key)
-          end
-        else
-          parsed = pattern.params(input)
-          parsed ? parsed.delete_if { |_, value| value.nil? }.symbolize_keys : {}
-        end
+        return params_without_input if input.blank?
+
+        parsed = pattern.params(input)
+        return {} unless parsed
+
+        parsed.delete_if { |_, value| value.nil? }.symbolize_keys
+      end
+
+      private
+
+      def params_without_input
+        pattern.captures_default.merge(attributes.params)
+      end
+
+      def upcase_method(method)
+        method_s = method.to_s
+        Grape::Http::Headers.find_supported_method(method_s) || method_s.upcase
       end
     end
   end

spec/grape/api_spec.rb

@@ -3100,13 +3100,13 @@ def static
       ]
     end
 
-    it 'includes details' do
-      subject.desc 'method', details: 'method details'
+    it 'includes detail' do
+      subject.desc 'method', detail: 'method details'
       subject.get 'method'
       expect(subject.routes.map do |route|
-        { description: route.description, details: route.details, params: route.params }
+        { description: route.description, detail: route.detail, params: route.params }
       end).to eq [
-        { description: 'method', details: 'method details', params: {} }
+        { description: 'method', detail: 'method details', params: {} }
       ]
     end
 

spec/grape/router/attribute_translator_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe Grape::Router::AttributeTranslator do
-  (Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES + Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES).each do |attribute|
+  described_class::ROUTE_ATTRIBUTES.each do |attribute|
     describe "##{attribute}" do
       it "returns value from #{attribute} key if present" do
         translator = described_class.new(attribute => 'value')