Merge pull request rails#54526 from skipkayhil/hm-avoid-array-methods

More Journey optimizations

Author: rafaelfranca

actionpack/lib/action_dispatch/journey/route.rb

@@ -38,29 +38,50 @@ def self.call(_); true; end
           def self.verb; ""; end
         end
 
+        class Or
+          attr_reader :verb
+
+          def initialize(verbs)
+            @verbs = verbs
+            @verb = @verbs.map(&:verb).join("|")
+          end
+
+          def call(req)
+            @verbs.any? { |v| v.call req }
+          end
+        end
+
         VERB_TO_CLASS = VERBS.each_with_object(all: All) do |verb, hash|
           klass = const_get verb
           hash[verb]                 = klass
           hash[verb.downcase]        = klass
           hash[verb.downcase.to_sym] = klass
         end
-      end
 
-      def self.verb_matcher(verb)
-        VerbMatchers::VERB_TO_CLASS.fetch(verb) do
+        VERB_TO_CLASS.default_proc = proc do |_, verb|
           VerbMatchers::Unknown.new verb.to_s.dasherize.upcase
         end
+
+        def self.for(verbs)
+          if verbs.any? { |v| VERB_TO_CLASS[v] == All }
+            All
+          elsif verbs.one?
+            VERB_TO_CLASS[verbs.first]
+          else
+            Or.new(verbs.map { |v| VERB_TO_CLASS[v] })
+          end
+        end
       end
 
       ##
       # +path+ is a path constraint.
       # `constraints` is a hash of constraints to be applied to this route.
-      def initialize(name:, app: nil, path:, constraints: {}, required_defaults: [], defaults: {}, request_method_match: nil, precedence: 0, scope_options: {}, internal: false, source_location: nil)
+      def initialize(name:, app: nil, path:, constraints: {}, required_defaults: [], defaults: {}, via: nil, precedence: 0, scope_options: {}, internal: false, source_location: nil)
         @name        = name
         @app         = app
         @path        = path
 
-        @request_method_match = request_method_match
+        @request_method_match = via && VerbMatchers.for(via)
         @constraints = constraints
         @defaults    = defaults
         @required_defaults = nil
@@ -146,43 +167,36 @@ def dispatcher?
       end
 
       def matches?(request)
-        match_verb(request) &&
-        constraints.all? { |method, value|
-          case value
-          when Regexp, String
-            value === request.send(method).to_s
-          when Array
-            value.include?(request.send(method))
-          when TrueClass
-            request.send(method).present?
-          when FalseClass
-            request.send(method).blank?
-          else
-            value === request.send(method)
-          end
-        }
+        @request_method_match.call(request) && (
+          constraints.empty? ||
+          constraints.all? { |method, value|
+            case value
+            when Regexp, String
+              value === request.send(method).to_s
+            when Array
+              value.include?(request.send(method))
+            when TrueClass
+              request.send(method).present?
+            when FalseClass
+              request.send(method).blank?
+            else
+              value === request.send(method)
+            end
+          }
+        )
       end
 
       def ip
         constraints[:ip] || //
       end
 
       def requires_matching_verb?
-        !@request_method_match.all? { |x| x == VerbMatchers::All }
+        @request_method_match != VerbMatchers::All
       end
 
       def verb
-        verbs.join("|")
+        @request_method_match.verb
       end
-
-      private
-        def verbs
-          @request_method_match.map(&:verb)
-        end
-
-        def match_verb(request)
-          @request_method_match.any? { |m| m.call request }
-        end
     end
   end
   # :startdoc:

actionpack/lib/action_dispatch/journey/router.rb

@@ -38,17 +38,12 @@ def serve(req)
             req.path_info = "/" + req.path_info unless req.path_info.start_with? "/"
           end
 
-          tmp_params = set_params.merge route.defaults
-          parameters.each_pair { |key, val|
-            tmp_params[key] = val.force_encoding(::Encoding::UTF_8)
-          }
-
-          req.path_parameters = tmp_params
+          req.path_parameters = parameters
           req.route = route
 
           _, headers, _ = response = route.app.serve(req)
 
-          if "pass" == headers[Constants::X_CASCADE]
+          if headers[Constants::X_CASCADE] == "pass"
             req.script_name     = script_name
             req.path_info       = path_info
             req.path_parameters = set_params
@@ -69,7 +64,6 @@ def recognize(rails_req)
             rails_req.path_info   = "/" + rails_req.path_info unless rails_req.path_info.start_with? "/"
           end
 
-          parameters = route.defaults.merge parameters
           yield(route, parameters)
         end
       end
@@ -107,8 +101,10 @@ def filter_routes(path)
 
         def find_routes(req)
           path_info = req.path_info
-          routes = filter_routes(path_info).concat custom_routes.find_all { |r|
-            r.path.match?(path_info)
+          routes = filter_routes(path_info)
+
+          custom_routes.each { |r|
+            routes << r if r.path.match?(path_info)
           }
 
           if req.head?
@@ -125,15 +121,19 @@ def find_routes(req)
 
           routes.each do |r|
             match_data = r.path.match(path_info)
-            path_parameters = {}
+
+            path_parameters = req.path_parameters.merge r.defaults
+
             index = 1
             match_data.names.each do |name|
               if val = match_data[index]
-                path_parameters[name.to_sym] = if val.include?("%")
+                val = if val.include?("%")
                   CGI.unescapeURIComponent(val)
                 else
                   val
                 end
+                val.force_encoding(::Encoding::UTF_8)
+                path_parameters[name.to_sym] = val
               end
               index += 1
             end

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -183,7 +183,7 @@ def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:,
         def make_route(name, precedence)
           Journey::Route.new(name: name, app: application, path: path, constraints: conditions,
                              required_defaults: required_defaults, defaults: defaults,
-                             request_method_match: request_method, precedence: precedence,
+                             via: @via, precedence: precedence,
                              scope_options: scope_options, internal: @internal, source_location: route_source_location)
         end
 
@@ -204,11 +204,6 @@ def build_conditions(current_conditions, request_class)
         end
         private :build_conditions
 
-        def request_method
-          @via.map { |x| Journey::Route.verb_matcher(x) }
-        end
-        private :request_method
-
         private
           def intern(object)
             object.is_a?(String) ? -object : object

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -932,6 +932,7 @@ def recognize_path_with_request(req, path, extras, raise_on_missing: true)
               params[key] = URI::RFC2396_PARSER.unescape(value)
             end
           end
+          original_path_parameters = req.path_parameters
           req.path_parameters = params
           app = route.app
           if app.matches?(req) && app.dispatcher?
@@ -945,6 +946,8 @@ def recognize_path_with_request(req, path, extras, raise_on_missing: true)
           elsif app.matches?(req) && app.engine?
             path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras, raise_on_missing: false)
             return path_parameters if path_parameters
+          else
+            req.path_parameters = original_path_parameters
           end
         end