From 7c3ff27e83a4e91f5112a5b9b70a0ad3367ebd08 Mon Sep 17 00:00:00 2001
From: Eric Proulx <eproulx@petalmd.com>
Date: Mon, 20 May 2024 22:51:22 +0200
Subject: [PATCH] Optimize memory alloc and retained (#2441)

* Fix Cache in namespace and path
Compile! skip non defined method
Pattern, optimize capture_default
Use delete_if instead of -

* Revert root_prefix and to_regexp

* Refactor path

* Fix prefix, api string allocation

* Drop AttributeTranslator in favor of OrderedOptions
Manage Route regexp in Route class

* Add cache for capture_index

* Drop attribute_translator
Remove useless alias

* Fix all Rubocop Lint/MissingSuper
Add changelog
---
 .rubocop_todo.yml                             | 12 +---
 CHANGELOG.md                                  |  1 +
 lib/grape.rb                                  |  2 +
 lib/grape/api/instance.rb                     |  3 +-
 lib/grape/dsl/routing.rb                      |  4 +-
 lib/grape/endpoint.rb                         |  7 ++-
 .../exceptions/validation_array_errors.rb     |  1 +
 lib/grape/namespace.rb                        |  5 +-
 lib/grape/path.rb                             | 51 +++++++--------
 lib/grape/router.rb                           | 25 ++++----
 lib/grape/router/attribute_translator.rb      | 63 -------------------
 lib/grape/router/base_route.rb                | 39 ++++++++++++
 lib/grape/router/greedy_route.rb              | 17 ++---
 lib/grape/router/pattern.rb                   | 20 +++---
 lib/grape/router/route.rb                     | 15 ++---
 lib/grape/util/base_inheritable.rb            |  8 +--
 lib/grape/util/reverse_stackable_values.rb    |  5 +-
 lib/grape/util/stackable_values.rb            |  5 +-
 lib/grape/validations/params_scope.rb         | 13 ++--
 spec/grape/path_spec.rb                       |  2 +-
 .../grape/router/attribute_translator_spec.rb | 26 --------
 spec/grape/router/greedy_route_spec.rb        | 10 +--
 22 files changed, 129 insertions(+), 205 deletions(-)
 delete mode 100644 lib/grape/router/attribute_translator.rb
 create mode 100644 lib/grape/router/base_route.rb
 delete mode 100644 spec/grape/router/attribute_translator_spec.rb

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 44b3ed850d..74dd51f3c9 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-05-10 16:10:58 UTC using RuboCop version 1.63.2.
+# on 2024-05-20 14:55:33 UTC using RuboCop version 1.63.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -40,16 +40,6 @@ Lint/EmptyClass:
   Exclude:
     - 'lib/grape/dsl/parameters.rb'
 
-# Offense count: 5
-# Configuration parameters: AllowedParentClasses.
-Lint/MissingSuper:
-  Exclude:
-    - 'lib/grape/api/instance.rb'
-    - 'lib/grape/exceptions/validation_array_errors.rb'
-    - 'lib/grape/namespace.rb'
-    - 'lib/grape/path.rb'
-    - 'lib/grape/router/pattern.rb'
-
 # Offense count: 1
 # Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 60f7a77ea9..9b409b9131 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -44,6 +44,7 @@
 * [#2414](https://github.com/ruby-grape/grape/pull/2414): Fix Rack::Lint missing content-type - [@ericproulx](https://github.com/ericproulx).
 * [#2378](https://github.com/ruby-grape/grape/pull/2378): Do not overwrite `route_param` with a regular one if they share same name - [@arg](https://github.com/arg).
 * [#2444](https://github.com/ruby-grape/grape/pull/2444): Replace method_missing in endpoint - [@ericproulx](https://github.com/ericproulx).
+* [#2441](https://github.com/ruby-grape/grape/pull/2441): Optimize memory alloc and retained - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.0.0 (2023/11/11)
diff --git a/lib/grape.rb b/lib/grape.rb
index 2c99d377a8..963e37364f 100644
--- a/lib/grape.rb
+++ b/lib/grape.rb
@@ -16,6 +16,7 @@
 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/module/delegation'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/object/deep_dup'
 require 'active_support/core_ext/object/duplicable'
@@ -23,6 +24,7 @@
 require 'active_support/core_ext/string/exclude'
 require 'active_support/deprecation'
 require 'active_support/inflector'
+require 'active_support/ordered_options'
 require 'active_support/notifications'
 
 require 'English'
diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb
index 8047b4e256..a17c8e63f0 100644
--- a/lib/grape/api/instance.rb
+++ b/lib/grape/api/instance.rb
@@ -125,6 +125,7 @@ def evaluate_as_instance_with_configuration(block, lazy: false)
         end
 
         def inherited(subclass)
+          super
           subclass.reset!
           subclass.logger = logger.clone
         end
@@ -220,7 +221,7 @@ def add_head_not_allowed_methods_and_options_methods
 
       def collect_route_config_per_pattern
         all_routes       = self.class.endpoints.map(&:routes).flatten
-        routes_by_regexp = all_routes.group_by { |route| route.pattern.to_regexp }
+        routes_by_regexp = all_routes.group_by(&:pattern_regexp)
 
         # Build the configuration based on the first endpoint and the collection of methods supported.
         routes_by_regexp.values.map do |routes|
diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb
index a422c34d04..812ddd1d08 100644
--- a/lib/grape/dsl/routing.rb
+++ b/lib/grape/dsl/routing.rb
@@ -30,7 +30,7 @@ def version(*args, &block)
           if args.any?
             options = args.extract_options!
             options = options.reverse_merge(using: :path)
-            requested_versions = args.flatten
+            requested_versions = args.flatten.map(&:to_s)
 
             raise Grape::Exceptions::MissingVendorOption.new if options[:using] == :header && !options.key?(:vendor)
 
@@ -54,7 +54,7 @@ def version(*args, &block)
 
         # Define a root URL prefix for your entire API.
         def prefix(prefix = nil)
-          namespace_inheritable(:root_prefix, prefix)
+          namespace_inheritable(:root_prefix, prefix&.to_s)
         end
 
         # Create a scope without affecting the URL.
diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
index b04f47e1cf..7fc83fc437 100644
--- a/lib/grape/endpoint.rb
+++ b/lib/grape/endpoint.rb
@@ -190,10 +190,11 @@ def prepare_default_route_attributes
     end
 
     def prepare_version
-      version = namespace_inheritable(:version) || []
+      version = namespace_inheritable(:version)
+      return unless version
       return if version.empty?
 
-      version.length == 1 ? version.first.to_s : version
+      version.length == 1 ? version.first : version
     end
 
     def merge_route_options(**default)
@@ -206,7 +207,7 @@ def map_routes
 
     def prepare_path(path)
       path_settings = inheritable_setting.to_hash[:namespace_stackable].merge(inheritable_setting.to_hash[:namespace_inheritable])
-      Path.prepare(path, namespace, path_settings)
+      Path.new(path, namespace, path_settings)
     end
 
     def namespace
diff --git a/lib/grape/exceptions/validation_array_errors.rb b/lib/grape/exceptions/validation_array_errors.rb
index d596c88770..d7815b1f6a 100644
--- a/lib/grape/exceptions/validation_array_errors.rb
+++ b/lib/grape/exceptions/validation_array_errors.rb
@@ -6,6 +6,7 @@ class ValidationArrayErrors < Base
       attr_reader :errors
 
       def initialize(errors)
+        super()
         @errors = errors
       end
     end
diff --git a/lib/grape/namespace.rb b/lib/grape/namespace.rb
index 8375e3a561..537e7ff663 100644
--- a/lib/grape/namespace.rb
+++ b/lib/grape/namespace.rb
@@ -31,13 +31,14 @@ def self.joined_space(settings)
     # Join the namespaces from a list of settings to create a path prefix.
     # @param settings [Array] list of Grape::Util::InheritableSettings.
     def self.joined_space_path(settings)
-      Grape::Router.normalize_path(JoinedSpaceCache[joined_space(settings)])
+      JoinedSpaceCache[joined_space(settings)]
     end
 
     class JoinedSpaceCache < Grape::Util::Cache
       def initialize
+        super
         @cache = Hash.new do |h, joined_space|
-          h[joined_space] = -joined_space.join('/')
+          h[joined_space] = Grape::Router.normalize_path(joined_space.join('/'))
         end
       end
     end
diff --git a/lib/grape/path.rb b/lib/grape/path.rb
index e6fa540e7e..fd05778930 100644
--- a/lib/grape/path.rb
+++ b/lib/grape/path.rb
@@ -3,10 +3,6 @@
 module Grape
   # Represents a path to an endpoint.
   class Path
-    def self.prepare(raw_path, namespace, settings)
-      Path.new(raw_path, namespace, settings)
-    end
-
     attr_reader :raw_path, :namespace, :settings
 
     def initialize(raw_path, namespace, settings)
@@ -20,31 +16,27 @@ def mount_path
     end
 
     def root_prefix
-      split_setting(:root_prefix)
+      settings[:root_prefix]
     end
 
     def uses_specific_format?
-      if settings.key?(:format) && settings.key?(:content_types)
-        settings[:format] && Array(settings[:content_types]).size == 1
-      else
-        false
-      end
+      return false unless settings.key?(:format) && settings.key?(:content_types)
+
+      settings[:format] && Array(settings[:content_types]).size == 1
     end
 
     def uses_path_versioning?
-      if settings.key?(:version) && settings[:version_options] && settings[:version_options].key?(:using)
-        settings[:version] && settings[:version_options][:using] == :path
-      else
-        false
-      end
+      return false unless settings.key?(:version) && settings[:version_options]&.key?(:using)
+
+      settings[:version] && settings[:version_options][:using] == :path
     end
 
     def namespace?
-      namespace&.match?(/^\S/) && namespace != '/'
+      namespace&.match?(/^\S/) && not_slash?(namespace)
     end
 
     def path?
-      raw_path&.match?(/^\S/) && raw_path != '/'
+      raw_path&.match?(/^\S/) && not_slash?(raw_path)
     end
 
     def suffix
@@ -58,7 +50,7 @@ def suffix
     end
 
     def path
-      Grape::Router.normalize_path(PartsCache[parts])
+      PartsCache[parts]
     end
 
     def path_with_suffix
@@ -73,24 +65,29 @@ def to_s
 
     class PartsCache < Grape::Util::Cache
       def initialize
+        super
         @cache = Hash.new do |h, parts|
-          h[parts] = -parts.join('/')
+          h[parts] = Grape::Router.normalize_path(parts.join('/'))
         end
       end
     end
 
     def parts
-      parts = [mount_path, root_prefix].compact
-      parts << ':version' if uses_path_versioning?
-      parts << namespace.to_s
-      parts << raw_path.to_s
-      parts.flatten.reject { |part| part == '/' }
+      [].tap do |parts|
+        add_part(parts, mount_path)
+        add_part(parts, root_prefix)
+        parts << ':version' if uses_path_versioning?
+        add_part(parts, namespace)
+        add_part(parts, raw_path)
+      end
     end
 
-    def split_setting(key)
-      return if settings[key].nil?
+    def add_part(parts, value)
+      parts << value if value && not_slash?(value)
+    end
 
-      settings[key].to_s.split('/')
+    def not_slash?(value)
+      value != '/'
     end
   end
 end
diff --git a/lib/grape/router.rb b/lib/grape/router.rb
index ba02073957..e1adf63bb0 100644
--- a/lib/grape/router.rb
+++ b/lib/grape/router.rb
@@ -12,10 +12,6 @@ def self.normalize_path(path)
       path
     end
 
-    def self.supported_methods
-      @supported_methods ||= Grape::Http::Headers::SUPPORTED_METHODS + ['*']
-    end
-
     def initialize
       @neutral_map = []
       @neutral_regexes = []
@@ -28,13 +24,12 @@ def compile!
 
       @union = Regexp.union(@neutral_regexes)
       @neutral_regexes = nil
-      self.class.supported_methods.each do |method|
+      (Grape::Http::Headers::SUPPORTED_METHODS + ['*']).each do |method|
+        next unless map.key?(method)
+
         routes = map[method]
-        @optimized_map[method] = routes.map.with_index do |route, index|
-          route.index = index
-          Regexp.new("(?<_#{index}>#{route.pattern.to_regexp})")
-        end
-        @optimized_map[method] = Regexp.union(@optimized_map[method])
+        optimized_map = routes.map.with_index { |route, index| route.to_regexp(index) }
+        @optimized_map[method] = Regexp.union(optimized_map)
       end
       @compiled = true
     end
@@ -44,8 +39,10 @@ def append(route)
     end
 
     def associate_routes(pattern, **options)
-      @neutral_regexes << Regexp.new("(?<_#{@neutral_map.length}>)#{pattern.to_regexp}")
-      @neutral_map << Grape::Router::GreedyRoute.new(pattern: pattern, index: @neutral_map.length, **options)
+      Grape::Router::GreedyRoute.new(pattern: pattern, **options).then do |greedy_route|
+        @neutral_regexes << greedy_route.to_regexp(@neutral_map.length)
+        @neutral_map << greedy_route
+      end
     end
 
     def call(env)
@@ -145,11 +142,11 @@ def default_response
     end
 
     def match?(input, method)
-      @optimized_map[method].match(input) { |m| @map[method].detect { |route| m["_#{route.index}"] } }
+      @optimized_map[method].match(input) { |m| @map[method].detect { |route| m[route.regexp_capture_index] } }
     end
 
     def greedy_match?(input)
-      @union.match(input) { |m| @neutral_map.detect { |route| m["_#{route.index}"] } }
+      @union.match(input) { |m| @neutral_map.detect { |route| m[route.regexp_capture_index] } }
     end
 
     def call_with_allow_headers(env, route)
diff --git a/lib/grape/router/attribute_translator.rb b/lib/grape/router/attribute_translator.rb
deleted file mode 100644
index 2197e0efe9..0000000000
--- a/lib/grape/router/attribute_translator.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-# frozen_string_literal: true
-
-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
-      ROUTE_ATTRIBUTES = (%i[
-        allow_header
-        anchor
-        endpoint
-        format
-        forward_match
-        namespace
-        not_allowed_method
-        prefix
-        request_method
-        requirements
-        settings
-        suffix
-        version
-      ] | Grape::DSL::Desc::ROUTE_ATTRIBUTES).freeze
-
-      def initialize(**attributes)
-        @attributes = attributes
-      end
-
-      ROUTE_ATTRIBUTES.each do |attr|
-        define_method attr do
-          @attributes[attr]
-        end
-
-        define_method(:"#{attr}=") do |val|
-          @attributes[attr] = val
-        end
-      end
-
-      def to_h
-        @attributes
-      end
-
-      def method_missing(method_name, *args)
-        if setter?(method_name)
-          @attributes[method_name.to_s.chomp('=').to_sym] = args.first
-        else
-          @attributes[method_name]
-        end
-      end
-
-      def respond_to_missing?(method_name, _include_private = false)
-        return true if setter?(method_name)
-
-        @attributes.key?(method_name)
-      end
-
-      private
-
-      def setter?(method_name)
-        method_name.end_with?('=')
-      end
-    end
-  end
-end
diff --git a/lib/grape/router/base_route.rb b/lib/grape/router/base_route.rb
new file mode 100644
index 0000000000..9d19c87209
--- /dev/null
+++ b/lib/grape/router/base_route.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module Grape
+  class Router
+    class BaseRoute
+      delegate_missing_to :@options
+
+      attr_reader :index, :pattern, :options
+
+      def initialize(**options)
+        @options = ActiveSupport::OrderedOptions.new.update(options)
+      end
+
+      alias attributes options
+
+      def regexp_capture_index
+        CaptureIndexCache[index]
+      end
+
+      def pattern_regexp
+        pattern.to_regexp
+      end
+
+      def to_regexp(index)
+        @index = index
+        Regexp.new("(?<#{regexp_capture_index}>#{pattern_regexp})")
+      end
+
+      class CaptureIndexCache < Grape::Util::Cache
+        def initialize
+          super
+          @cache = Hash.new do |h, index|
+            h[index] = "_#{index}"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/grape/router/greedy_route.rb b/lib/grape/router/greedy_route.rb
index 787c1265b6..a999c1b906 100644
--- a/lib/grape/router/greedy_route.rb
+++ b/lib/grape/router/greedy_route.rb
@@ -5,24 +5,15 @@
 
 module Grape
   class Router
-    class GreedyRoute
-      extend Forwardable
-
-      attr_reader :index, :pattern, :options, :attributes
-
-      # params must be handled in this class to avoid method redefined warning
-      delegate Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES - [:params] => :@attributes
-
-      def initialize(index:, pattern:, **options)
-        @index = index
+    class GreedyRoute < BaseRoute
+      def initialize(pattern:, **options)
         @pattern = pattern
-        @options = options
-        @attributes = Grape::Router::AttributeTranslator.new(**options)
+        super(**options)
       end
 
       # Grape::Router:Route defines params as a function
       def params(_input = nil)
-        @attributes.params || {}
+        options[:params] || {}
       end
     end
   end
diff --git a/lib/grape/router/pattern.rb b/lib/grape/router/pattern.rb
index 0f646bea90..7b5b5276f0 100644
--- a/lib/grape/router/pattern.rb
+++ b/lib/grape/router/pattern.rb
@@ -3,9 +3,12 @@
 module Grape
   class Router
     class Pattern
-      attr_reader :origin, :path, :pattern, :to_regexp, :captures_default
-
       extend Forwardable
+
+      DEFAULT_CAPTURES = %w[format version].freeze
+
+      attr_reader :origin, :path, :pattern, :to_regexp
+
       def_delegators :pattern, :named_captures, :params
       def_delegators :to_regexp, :===
       alias match? ===
@@ -15,7 +18,12 @@ def initialize(pattern, **options)
         @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
+
+      def captures_default
+        to_regexp.names
+                 .delete_if { |n| DEFAULT_CAPTURES.include?(n) }
+                 .to_h { |k| [k, ''] }
       end
 
       private
@@ -43,11 +51,6 @@ def extract_capture(**options)
         options[:requirements].merge(sliced_options)
       end
 
-      def regex_captures_default(regex)
-        names = regex.names - %w[format version] # remove default format and version
-        names.to_h { |k| [k, ''] }
-      end
-
       def build_path_from_pattern(pattern, anchor: false)
         if pattern.end_with?('*path')
           pattern.dup.insert(pattern.rindex('/') + 1, '?')
@@ -62,6 +65,7 @@ def build_path_from_pattern(pattern, anchor: false)
 
       class PatternCache < Grape::Util::Cache
         def initialize
+          super
           @cache = Hash.new do |h, (pattern, suffix)|
             h[[pattern, suffix]] = -"#{pattern}#{suffix}"
           end
diff --git a/lib/grape/router/route.rb b/lib/grape/router/route.rb
index 2ee3bb89fb..335ecb712f 100644
--- a/lib/grape/router/route.rb
+++ b/lib/grape/router/route.rb
@@ -2,20 +2,17 @@
 
 module Grape
   class Router
-    class Route
+    class Route < BaseRoute
       extend Forwardable
 
-      attr_reader :app, :pattern, :options, :attributes
-      attr_accessor :index
+      attr_reader :app, :request_method
 
       def_delegators :pattern, :path, :origin
-      # params must be handled in this class to avoid method redefined warning
-      delegate Grape::Router::AttributeTranslator::ROUTE_ATTRIBUTES - [:params] => :attributes
 
       def initialize(method, pattern, **options)
-        @options = options
+        @request_method = upcase_method(method)
         @pattern = Grape::Router::Pattern.new(pattern, **options)
-        @attributes = Grape::Router::AttributeTranslator.new(**options, request_method: upcase_method(method))
+        super(**options)
       end
 
       def exec(env)
@@ -30,7 +27,7 @@ def apply(app)
       def match?(input)
         return false if input.blank?
 
-        attributes.forward_match ? input.start_with?(pattern.origin) : pattern.match?(input)
+        options[:forward_match] ? input.start_with?(pattern.origin) : pattern.match?(input)
       end
 
       def params(input = nil)
@@ -50,7 +47,7 @@ def params_without_input
 
       def upcase_method(method)
         method_s = method.to_s
-        Grape::Http::Headers.find_supported_method(method_s) || method_s.upcase
+        Grape::Http::Headers::SUPPORTED_METHODS.detect { |m| m.casecmp(method_s).zero? } || method_s.upcase
       end
     end
   end
diff --git a/lib/grape/util/base_inheritable.rb b/lib/grape/util/base_inheritable.rb
index 5db6e3455d..cc68ababfe 100644
--- a/lib/grape/util/base_inheritable.rb
+++ b/lib/grape/util/base_inheritable.rb
@@ -26,10 +26,10 @@ def initialize_copy(other)
 
       def keys
         if new_values.any?
-          combined = inherited_values.keys
-          combined.concat(new_values.keys)
-          combined.uniq!
-          combined
+          inherited_values.keys.tap do |combined|
+            combined.concat(new_values.keys)
+            combined.uniq!
+          end
         else
           inherited_values.keys
         end
diff --git a/lib/grape/util/reverse_stackable_values.rb b/lib/grape/util/reverse_stackable_values.rb
index 3b008a926c..43da1ead53 100644
--- a/lib/grape/util/reverse_stackable_values.rb
+++ b/lib/grape/util/reverse_stackable_values.rb
@@ -8,10 +8,7 @@ class ReverseStackableValues < StackableValues
       def concat_values(inherited_value, new_value)
         return inherited_value unless new_value
 
-        [].tap do |value|
-          value.concat(new_value)
-          value.concat(inherited_value)
-        end
+        new_value + inherited_value
       end
     end
   end
diff --git a/lib/grape/util/stackable_values.rb b/lib/grape/util/stackable_values.rb
index c19e2f5828..64336182c0 100644
--- a/lib/grape/util/stackable_values.rb
+++ b/lib/grape/util/stackable_values.rb
@@ -29,10 +29,7 @@ def to_hash
       def concat_values(inherited_value, new_value)
         return inherited_value unless new_value
 
-        [].tap do |value|
-          value.concat(inherited_value)
-          value.concat(new_value)
-        end
+        inherited_value + new_value
       end
     end
   end
diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb
index 9e1e28b84f..403b0ef09b 100644
--- a/lib/grape/validations/params_scope.rb
+++ b/lib/grape/validations/params_scope.rb
@@ -209,11 +209,11 @@ def push_renamed_param(path, new_name)
 
       def require_required_and_optional_fields(context, opts)
         if context == :all
-          optional_fields = Array(opts[:except])
-          required_fields = opts[:using].keys - optional_fields
+          optional_fields = Array.wrap(opts[:except])
+          required_fields = opts[:using].keys.delete_if { |f| optional_fields.include?(f) }
         else # context == :none
-          required_fields = Array(opts[:except])
-          optional_fields = opts[:using].keys - required_fields
+          required_fields = Array.wrap(opts[:except])
+          optional_fields = opts[:using].keys.delete_if { |f| required_fields.include?(f) }
         end
         required_fields.each do |field|
           field_opts = opts[:using][field]
@@ -229,7 +229,10 @@ def require_required_and_optional_fields(context, opts)
 
       def require_optional_fields(context, opts)
         optional_fields = opts[:using].keys
-        optional_fields -= Array(opts[:except]) unless context == :all
+        unless context == :all
+          except_fields = Array.wrap(opts[:except])
+          optional_fields.delete_if { |f| except_fields.include?(f) }
+        end
         optional_fields.each do |field|
           field_opts = opts[:using][field]
           optional(field, field_opts) if field_opts
diff --git a/spec/grape/path_spec.rb b/spec/grape/path_spec.rb
index a222a2c400..afad438df4 100644
--- a/spec/grape/path_spec.rb
+++ b/spec/grape/path_spec.rb
@@ -49,7 +49,7 @@ module Grape
 
       it 'splits the mount path' do
         path = described_class.new(anything, anything, root_prefix: 'hello/world')
-        expect(path.root_prefix).to eql(%w[hello world])
+        expect(path.root_prefix).to eql('hello/world')
       end
     end
 
diff --git a/spec/grape/router/attribute_translator_spec.rb b/spec/grape/router/attribute_translator_spec.rb
deleted file mode 100644
index 54e22dd64b..0000000000
--- a/spec/grape/router/attribute_translator_spec.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-# frozen_string_literal: true
-
-describe Grape::Router::AttributeTranslator do
-  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')
-        expect(translator.public_send(attribute)).to eq('value')
-      end
-
-      it "returns nil from #{attribute} key if missing" do
-        translator = described_class.new
-        expect(translator.public_send(attribute)).to be_nil
-      end
-    end
-
-    describe "##{attribute}=" do
-      it "sets value for #{attribute}", :aggregate_failures do
-        translator = described_class.new(attribute => 'value')
-        expect do
-          translator.public_send(:"#{attribute}=", 'new_value')
-        end.to change(translator, attribute).from('value').to('new_value')
-      end
-    end
-  end
-end
diff --git a/spec/grape/router/greedy_route_spec.rb b/spec/grape/router/greedy_route_spec.rb
index add108ac50..29e966280e 100644
--- a/spec/grape/router/greedy_route_spec.rb
+++ b/spec/grape/router/greedy_route_spec.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe Grape::Router::GreedyRoute do
-  let(:instance) { described_class.new(index: index, pattern: pattern, **options) }
+  let(:instance) { described_class.new(pattern: pattern, **options) }
   let(:index) { 0 }
   let(:pattern) { :pattern }
   let(:params) do
@@ -11,12 +11,6 @@
     { params: params }.freeze
   end
 
-  describe '#index' do
-    subject { instance.index }
-
-    it { is_expected.to eq(index) }
-  end
-
   describe '#pattern' do
     subject { instance.pattern }
 
@@ -38,6 +32,6 @@
   describe '#attributes' do
     subject { instance.attributes }
 
-    it { is_expected.to be_a(Grape::Router::AttributeTranslator) }
+    it { is_expected.to eq(options) }
   end
 end