TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Enforce middleware ordering with a test, instead of comments

Browse Source 
We want the actual order to be very predictable, so it's rightly defined
in code -- not with an on-the-fly tsort.

But we can do the tsort here, and then verify that it matches the
implemented ordering. This way we don't leave future readers guessing
which parts of the ordering are deliberate and which are arbitrary.
This commit is contained in:
Matthew Draper 2016-12-31 08:20:35 +10:30
parent 2c5190e329
commit 6b126ffdcd
2 changed files with 35 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
railties/lib/rails/application/default_middleware_stack.rb
View File

@ -41,10 +41,8 @@ def build_stack
middleware.use ::Rack::Runtime
middleware.use ::Rack::MethodOverride unless config.api_only
middleware.use ::ActionDispatch::RequestId
middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
# Must come after Rack::MethodOverride to properly log overridden methods
# Must come after ActionDispatch::RemoteIP to properly log ip address
middleware.use ::Rails::Rack::Logger, config.log_tags
middleware.use ::ActionDispatch::ShowExceptions, show_exceptions_app
middleware.use ::ActionDispatch::DebugExceptions, app, config.debug_exception_response_format

37
railties/test/application/middleware_test.rb
View File

@ -30,8 +30,8 @@ def app
"Rack::Runtime",
"Rack::MethodOverride",
"ActionDispatch::RequestId",
"ActionDispatch::RemoteIp", # Must come before Rails::Rack::Logger to properly log request_id
"Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
"ActionDispatch::RemoteIp",
"Rails::Rack::Logger",
"ActionDispatch::ShowExceptions",
"ActionDispatch::DebugExceptions",
"ActionDispatch::Reloader",
@ -59,7 +59,7 @@ def app
"Rack::Runtime",
"ActionDispatch::RequestId",
"ActionDispatch::RemoteIp",
"Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
"Rails::Rack::Logger",
"ActionDispatch::ShowExceptions",
"ActionDispatch::DebugExceptions",
"ActionDispatch::Reloader",
@ -70,6 +70,37 @@ def app
], middleware
end
test "middleware dependencies" do
boot!
# The following array-of-arrays describes dependencies between
# middlewares: the first item in each list depends on the
# remaining items (and therefore must occur later in the
# middleware stack).
dependencies = [
# Logger needs a fully "corrected" request environment
%w(Rails::Rack::Logger Rack::MethodOverride ActionDispatch::RequestId ActionDispatch::RemoteIp),
# Serving public/ doesn't invoke user code, so it should skip
# locks etc
%w(ActionDispatch::Executor ActionDispatch::Static),
# Errors during reload must be reported
%w(ActionDispatch::Reloader ActionDispatch::ShowExceptions ActionDispatch::DebugExceptions),
# Outright dependencies
%w(ActionDispatch::Static Rack::Sendfile),
%w(ActionDispatch::Flash ActionDispatch::Session::CookieStore),
%w(ActionDispatch::Session::CookieStore ActionDispatch::Cookies),
]
require "tsort"
sorted = TSort.tsort((middleware | dependencies.flatten).method(:each),
lambda { |n, &b| dependencies.each { |m, *ds| ds.each(&b) if m == n } })
assert_equal sorted, middleware
end
test "Rack::Cache is not included by default" do
boot!