Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rack.response_finished to Rack::Lint #1802

Closed
wants to merge 9 commits into from
Closed
4 changes: 4 additions & 0 deletions SPEC.rdoc
Expand Up @@ -137,6 +137,10 @@ There are the following restrictions:
set. <tt>PATH_INFO</tt> should be <tt>/</tt> if
<tt>SCRIPT_NAME</tt> is empty.
<tt>SCRIPT_NAME</tt> never should be <tt>/</tt>, but instead be empty.
<tt>rack.response_finished</tt>:: An array of callables run after the HTTP response has been sent.
The array of callables should be called directly after the HTTP response has
been closed. The callables should be called sequentially and synchronously.
BlakeWilliams marked this conversation as resolved.
Show resolved Hide resolved


=== The Input Stream

Expand Down
1 change: 1 addition & 0 deletions lib/rack/constants.rb
Expand Up @@ -52,6 +52,7 @@ module Rack
RACK_RECURSIVE_INCLUDE = 'rack.recursive.include'
RACK_MULTIPART_BUFFER_SIZE = 'rack.multipart.buffer_size'
RACK_MULTIPART_TEMPFILE_FACTORY = 'rack.multipart.tempfile_factory'
RACK_RESPONSE_FINISHED = 'rack.response_finished'
Copy link
Member

@ioquatix ioquatix Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BlakeWilliams @tenderlove looking at other keys, should this be rack.response.finished or rack.response_finished? We don't use rack.request_form_input but rack.request.form_input.

Is there any precedent for appending _callbacks to the name i.e. rack.response.finished_callbacks or something? I don't mind either way, just calling it out for further discussion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of it being "namespaced", like rack.response.finished. I'll update that real quick, but I also like the idea of specifying that it is some kind of array of callbacks. I'll defer to other folks on naming, but I'm 👍 on rack.response.finished_callbacks.

RACK_REQUEST_FORM_INPUT = 'rack.request.form_input'
RACK_REQUEST_FORM_HASH = 'rack.request.form_hash'
RACK_REQUEST_FORM_VARS = 'rack.request.form_vars'
Expand Down
9 changes: 9 additions & 0 deletions lib/rack/lint.rb
Expand Up @@ -359,6 +359,15 @@ def check_environment(env)
unless env[SCRIPT_NAME] != "/"
raise LintError, "SCRIPT_NAME cannot be '/', make it '' and PATH_INFO '/'"
end

## <tt>rack.response_finished</tt>:: An array of callables run after the HTTP response has been sent.
BlakeWilliams marked this conversation as resolved.
Show resolved Hide resolved
if callables = env[RACK_RESPONSE_FINISHED]
raise LintError, "rack.response_finished must be an array of callable objects" unless callables.is_a?(Array)

callables.each do |callable|
raise LintError, "rack.response_finished values must respond to call" unless callable.respond_to?(:call)
BlakeWilliams marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

##
Expand Down
16 changes: 16 additions & 0 deletions test/spec_lint.rb
Expand Up @@ -209,6 +209,16 @@ def obj.error(*) end
Rack::Lint.new(nil).call(env("SCRIPT_NAME" => "/"))
}.must_raise(Rack::Lint::LintError).
message.must_match(/cannot be .* make it ''/)

lambda {
Rack::Lint.new(nil).call(env("rack.response_finished" => "not a callable"))
}.must_raise(Rack::Lint::LintError).
message.must_match(/rack.response_finished must be an array of callable objects/)

lambda {
Rack::Lint.new(nil).call(env("rack.response_finished" => [->{}, "not a callable"]))
}.must_raise(Rack::Lint::LintError).
message.must_match(/rack.response_finished values must respond to call/)
end

it "notice input errors" do
Expand Down Expand Up @@ -708,4 +718,10 @@ def assert_lint(*args)
}).call(env({}))[1]['rack.hijack'].call(StringIO.new).read.must_equal ''
end

it "pass valid rack.response_finished" do
Rack::Lint.new(lambda { |env|
[200, { "rack.response_finished" => [-> {}, lambda {}], "content-length" => "3" }, ["foo"]]
}).call(env({})).first.must_equal 200
end

end