Skip to content

Commit

Permalink
Fix streaming CSV export
Browse files Browse the repository at this point in the history
Streaming was not working either in development (disable_streaming off)
nor on production Heroku. Adding a Last-Modified header made it work.

The problem would only manifest when using rack 2.2 or higher, and was
caused by rack/rack#1416, where the `Rack::ETag`
middleware changed it's behaviour to buffer the response and calculate
an ETag digest even though the `Cache-Control: no-cache` header is set.
This breaks the CSV streaming responses so to avoid that we use the hack
outlined in rack/rack#1619 and set a Last-Modified header which triggers
the `skip_caching?` condition in the `Rack::ETag` middleware.

See activeadmin#6451 and alphagov/e-petitions@46413a7
  • Loading branch information
varyonic committed Nov 22, 2024
1 parent 7754f42 commit a7b92e0
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
4 changes: 3 additions & 1 deletion features/step_definitions/format_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
# Check first rows of the displayed CSV.
Then /^I should download a CSV file with "([^"]*)" separator for "([^"]*)" containing:$/ do |sep, resource_name, table|
body = page.driver.response.body
content_type_header, content_disposition_header = %w[Content-Type Content-Disposition].map do |header_name|
content_type_header, content_disposition_header, last_modified_header = %w[Content-Type Content-Disposition Last-Modified].map do |header_name|
page.response_headers[header_name]
end
expect(content_type_header).to eq 'text/csv; charset=utf-8'
expect(content_disposition_header).to match /\Aattachment; filename=".+?\.csv"\z/
expect(last_modified_header).to_not be_nil
expect(Date.strptime(last_modified_header, "%a, %d %b %Y %H:%M:%S GMT")).to be_a(Date)

csv = CSV.parse(body, col_sep: sep)
table.raw.each_with_index do |expected_row, row_index|
Expand Down
1 change: 1 addition & 0 deletions lib/active_admin/resource_controller/streaming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def index
def stream_resource(&block)
headers['X-Accel-Buffering'] = 'no'
headers['Cache-Control'] = 'no-cache'
headers["Last-Modified"] = Time.current.httpdate

if ActiveAdmin.application.disable_streaming_in.include? Rails.env
self.response_body = block[String.new]
Expand Down

0 comments on commit a7b92e0

Please sign in to comment.