aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md5
-rw-r--r--actionpack/lib/action_controller/metal/helpers.rb4
-rw-r--r--actionpack/lib/action_dispatch/http/request.rb2
-rw-r--r--actionpack/lib/action_dispatch/http/response.rb4
-rw-r--r--actionpack/lib/action_dispatch/journey/router.rb2
-rw-r--r--actionpack/lib/action_dispatch/journey/routes.rb19
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb30
-rw-r--r--actionpack/lib/action_dispatch/testing/test_response.rb2
-rw-r--r--actionpack/test/abstract_unit.rb4
-rw-r--r--actionpack/test/controller/live_stream_test.rb2
-rw-r--r--actionpack/test/dispatch/debug_exceptions_test.rb4
-rw-r--r--actionpack/test/dispatch/response_test.rb2
-rw-r--r--actionpack/test/journey/routes_test.rb22
13 files changed, 72 insertions, 30 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 0134bf7cab..8c4ba2a154 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_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb
index a9c3e438fb..819837e767 100644
--- a/actionpack/lib/action_controller/metal/helpers.rb
+++ b/actionpack/lib/action_controller/metal/helpers.rb
@@ -93,6 +93,10 @@ module ActionController
super(args)
end
+ # Return a list of helper names in specific path.
+ #
+ # ActionController::Base.all_helpers_from_path 'app/helpers'
+ # # => ["application", "chart", "rubygems"]
def all_helpers_from_path(path)
helpers = Array(path).flat_map do |_path|
extract = /^#{Regexp.quote(_path.to_s)}\/?(.*)_helper.rb$/
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 07b3814ca4..732ee67268 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -114,7 +114,7 @@ module ActionDispatch
end
def engine_script_name(_routes) # :nodoc:
- env["ROUTES_#{_routes.object_id}_SCRIPT_NAME"]
+ env[_routes.env_key]
end
def request_method=(request_method) #:nodoc:
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index a895d1ab18..5fe544c60c 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -113,7 +113,9 @@ module ActionDispatch # :nodoc:
# The underlying body, as a streamable object.
attr_reader :stream
- def initialize(status = 200, header = {}, body = [], default_headers: self.class.default_headers)
+ # Ruby 2.2 bug https://bugs.ruby-lang.org/issues/10685 prevents
+ # default_headers from being a keyword argument.
+ def initialize(status = 200, header = {}, body = [], default_headers = self.class.default_headers)
super()
header = merge_default_headers(header, default_headers)
diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb
index 2b036796ab..cc4bd6105d 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/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index ce04f0b08a..58b3fff9d7 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -199,12 +199,9 @@ module ActionDispatch
private
def optimized_helper(args)
- params = parameterize_args(args)
- missing_keys = missing_keys(params)
-
- unless missing_keys.empty?
- raise_generation_error(params, missing_keys)
- end
+ params = parameterize_args(args) { |k|
+ raise_generation_error(args)
+ }
@route.format params
end
@@ -215,16 +212,21 @@ module ActionDispatch
def parameterize_args(args)
params = {}
- @required_parts.zip(args.map(&:to_param)) { |k,v| params[k] = v }
+ @arg_size.times { |i|
+ key = @required_parts[i]
+ value = args[i].to_param
+ yield key if value.nil? || value.empty?
+ params[key] = value
+ }
params
end
- def missing_keys(args)
- args.select{ |part, arg| arg.nil? || arg.empty? }.keys
- end
-
- def raise_generation_error(args, missing_keys)
- constraints = Hash[@route.requirements.merge(args).sort]
+ def raise_generation_error(args)
+ missing_keys = []
+ params = parameterize_args(args) { |missing_key|
+ missing_keys << missing_key
+ }
+ constraints = Hash[@route.requirements.merge(params).sort]
message = "No route matches #{constraints.inspect}"
message << " missing required keys: #{missing_keys.sort.inspect}"
@@ -308,6 +310,7 @@ module ActionDispatch
attr_accessor :formatter, :set, :named_routes, :default_scope, :router
attr_accessor :disable_clear_and_finalize, :resources_path_names
attr_accessor :default_url_options, :request_class
+ attr_reader :env_key
alias :routes :set
@@ -325,6 +328,7 @@ module ActionDispatch
@prepend = []
@disable_clear_and_finalize = false
@finalized = false
+ @env_key = "ROUTES_#{object_id}_SCRIPT_NAME".freeze
@set = Journey::Routes.new
@router = Journey::Router.new @set
diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb
index a9b88ac5fd..527784885c 100644
--- a/actionpack/lib/action_dispatch/testing/test_response.rb
+++ b/actionpack/lib/action_dispatch/testing/test_response.rb
@@ -7,7 +7,7 @@ module ActionDispatch
# See Response for more information on controller response objects.
class TestResponse < Response
def self.from_response(response)
- new response.status, response.headers, response.body, default_headers: nil
+ new response.status, response.headers, response.body, nil
end
# Was the response successful?
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 918589f916..f5dd9d76af 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -95,7 +95,7 @@ end
module ActiveSupport
class TestCase
include ActionDispatch::DrawOnce
- if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0
+ if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0
parallelize_me!
end
end
@@ -479,7 +479,7 @@ class ForkingExecutor
end
end
-if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0
+if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0
# Use N processes (N defaults to 4)
Minitest.parallel_executor = ForkingExecutor.new(PROCESS_COUNT)
end
diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb
index 7fd1276e98..0c65270ec1 100644
--- a/actionpack/test/controller/live_stream_test.rb
+++ b/actionpack/test/controller/live_stream_test.rb
@@ -276,6 +276,8 @@ module ActionController
end
def test_async_stream
+ rubinius_skip "https://github.com/rubinius/rubinius/issues/2934"
+
@controller.latch = ActiveSupport::Concurrency::Latch.new
parts = ['hello', 'world']
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 7921f05688..a867aee7ec 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -305,7 +305,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
assert_response 500
assert_select '#Application-Trace' do
- assert_select 'pre code', /\(eval\):1: syntax error, unexpected/
+ assert_select 'pre code', /syntax error, unexpected/
end
end
@@ -332,7 +332,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
assert_response 500
assert_select '#Application-Trace' do
- assert_select 'pre code', /\(eval\):1: syntax error, unexpected/
+ assert_select 'pre code', /syntax error, unexpected/
end
end
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index f4c0b421d6..c61423dce4 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -172,7 +172,7 @@ class ResponseTest < ActiveSupport::TestCase
original = ActionDispatch::Response.default_charset
begin
ActionDispatch::Response.default_charset = 'utf-16'
- resp = ActionDispatch::Response.new(200, { "Content-Type" => "text/xml" }, default_headers: nil)
+ resp = ActionDispatch::Response.new(200, { "Content-Type" => "text/xml" })
assert_equal('utf-16', resp.charset)
ensure
ActionDispatch::Response.default_charset = original
diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb
index a4efc82b8c..b54d961f66 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,8 +40,24 @@ 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
one = Path::Pattern.from_string '/hello'