From 89edfbd3a452ce80b1865b136df8a13aad7835b4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 18 Jun 2014 19:16:30 -0700 Subject: Partition routes during setup. Partitioning of all the routes is currently being done during the first request. Since there is no need to clear the cache for `partitioned_routes` when adding a new route. We can move the partitioning of the routes during setup time. --- actionpack/CHANGELOG.md | 5 +++++ actionpack/lib/action_dispatch/journey/router.rb | 2 +- actionpack/lib/action_dispatch/journey/routes.rb | 19 ++++++++++++------- actionpack/test/journey/routes_test.rb | 21 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1083f4a92d..a569e0eb6b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Partitioning of routes is now done when the routes are being drawn. This + helps to decrease the time spent filtering the routes during the first request. + + *Guo Xiang Tan* + * Fix regression in functional tests. Responses should have default headers assigned. diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index e9df984c86..ed63a5bd8b 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -88,7 +88,7 @@ module ActionDispatch end def custom_routes - partitioned_routes.last + routes.custom_routes end def filter_routes(path) diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb index 80e3818ccd..a6d1980db2 100644 --- a/actionpack/lib/action_dispatch/journey/routes.rb +++ b/actionpack/lib/action_dispatch/journey/routes.rb @@ -5,13 +5,14 @@ module ActionDispatch class Routes # :nodoc: include Enumerable - attr_reader :routes, :named_routes + attr_reader :routes, :named_routes, :custom_routes, :anchored_routes def initialize @routes = [] @named_routes = {} @ast = nil - @partitioned_routes = nil + @anchored_routes = [] + @custom_routes = [] @simulator = nil end @@ -30,18 +31,22 @@ module ActionDispatch def clear routes.clear + anchored_routes.clear + custom_routes.clear named_routes.clear end - def partitioned_routes - @partitioned_routes ||= routes.partition do |r| - r.path.anchored && r.ast.grep(Nodes::Symbol).all?(&:default_regexp?) + def partition_route(route) + if route.path.anchored && route.ast.grep(Nodes::Symbol).all?(&:default_regexp?) + anchored_routes << route + else + custom_routes << route end end def ast @ast ||= begin - asts = partitioned_routes.first.map(&:ast) + asts = anchored_routes.map(&:ast) Nodes::Or.new(asts) unless asts.empty? end end @@ -60,6 +65,7 @@ module ActionDispatch route.precedence = routes.length routes << route named_routes[name] = route if name && !named_routes[name] + partition_route(route) clear_cache! route end @@ -68,7 +74,6 @@ module ActionDispatch def clear_cache! @ast = nil - @partitioned_routes = nil @simulator = nil end end diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb index a4efc82b8c..6ff055af1d 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -3,6 +3,10 @@ require 'abstract_unit' module ActionDispatch module Journey class TestRoutes < ActiveSupport::TestCase + setup do + @routes = Routes.new + end + def test_clear routes = Routes.new exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] @@ -36,6 +40,23 @@ module ActionDispatch assert_not_equal sim, routes.simulator end + def test_partition_route + path = Path::Pattern.from_string '/hello' + + anchored_route = @routes.add_route nil, path, {}, {}, {} + assert_equal [anchored_route], @routes.anchored_routes + assert_equal [], @routes.custom_routes + + strexp = Router::Strexp.build( + "/hello/:who", { who: /\d/ }, ['/', '.', '?'] + ) + path = Path::Pattern.new strexp + + custom_route = @routes.add_route nil, path, {}, {}, {} + assert_equal [custom_route], @routes.custom_routes + assert_equal [anchored_route], @routes.anchored_routes + end + def test_first_name_wins #def add_route app, path, conditions, defaults, name = nil routes = Routes.new -- cgit v1.2.3