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

Replace zip_tricks with zip_kit #100

Merged
merged 2 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ A gem to stream dynamically generated zip files from a rails application. Unlike
- Removes need for large disk space or memory allocation to generate zips, even huge zips. So it works on Heroku.
- The user begins downloading immediately, which decreaceses latency, download time, and timeouts on Heroku.

Zipline now depends on [zip tricks](https://github.com/WeTransfer/zip_tricks), and you might want to just use that directly if you have more advanced use cases.
Zipline now depends on [zip_kit](https://github.com/julik/zip_kit), and you might want to just use that directly if you have more advanced use cases.

## Installation

Expand Down Expand Up @@ -43,7 +43,7 @@ class MyController < ApplicationController
files = users.map{ |user| [user.avatar, "#{user.username}.png", modification_time: 1.day.ago] }

# we can force duplicate file names to be renamed, or raise an error
# we can also pass in our own writer if required to conform with the Delegated [ZipTricks::Streamer object](https://github.com/WeTransfer/zip_tricks/blob/main/lib/zip_tricks/streamer.rb#L147) object.
# we can also pass in our own writer if required to conform with the delegated [ZipKit::Streamer object](https://github.com/julik/zip_kit/blob/main/lib/zip_kit/streamer.rb#L147) object.
zipline(files, 'avatars.zip', auto_rename_duplicate_filenames: true)
end
end
Expand Down Expand Up @@ -93,7 +93,7 @@ For directories, just give the files names like "directory/file".

```Ruby
avatars = [
# remote_url zip_path zip_tricks_options
# remote_url zip_path write_file options for Streamer
[ 'http://www.example.com/user1.png', 'avatars/user1.png', modification_time: Time.now.utc ]
[ 'http://www.example.com/user2.png', 'avatars/user2.png', modification_time: 1.day.ago ]
[ 'http://www.example.com/user3.png', 'avatars/user3.png' ]
Expand Down
31 changes: 5 additions & 26 deletions lib/zipline.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
require 'content_disposition'
require 'zip_kit'
require 'zipline/version'
require 'zip_tricks'
require 'zipline/zip_generator'
require 'zipline/chunked_body'
require 'zipline/tempfile_body'

# class MyController < ApplicationController
# include Zipline
Expand All @@ -15,7 +13,6 @@
# end
module Zipline
def zipline(files, zipname = 'zipline.zip', **kwargs_for_new)
zip_generator = ZipGenerator.new(files, **kwargs_for_new)
headers['Content-Disposition'] = ContentDisposition.format(disposition: 'attachment', filename: zipname)
headers['Content-Type'] = Mime::Type.lookup_by_extension('zip').to_s
response.sending_file = true
Expand All @@ -24,32 +21,14 @@ def zipline(files, zipname = 'zipline.zip', **kwargs_for_new)
# Disables Rack::ETag if it is enabled (prevent buffering)
# see https://github.com/rack/rack/issues/1619#issuecomment-606315714
self.response.headers['Last-Modified'] = Time.now.httpdate

if request.get_header("HTTP_VERSION") == "HTTP/1.0"
# If HTTP/1.0 is used it is not possible to stream, and if that happens it usually will be
# unclear why buffering is happening. Some info in the log is the least one can do.
logger.warn { "The downstream HTTP proxy/LB insists on HTTP/1.0 protocol, ZIP response will be buffered." } if logger

# If we use Rack::ContentLength it would run through our ZIP block twice - once to calculate the content length
# of the response, and once - to serve. We can trade performance for disk space and buffer the response into a Tempfile
# since we are already buffering.
tempfile_body = TempfileBody.new(request.env, zip_generator)
headers["Content-Length"] = tempfile_body.size.to_s
headers["X-Zipline-Output"] = "buffered"
self.response_body = tempfile_body
else
# Disable buffering for both nginx and Google Load Balancer, see
# https://cloud.google.com/appengine/docs/flexible/how-requests-are-handled?tab=python#x-accel-buffering
response.headers["X-Accel-Buffering"] = "no"

# Make sure Rack::ContentLength does not try to compute a content length,
# and remove the one already set
headers.delete("Content-Length")

# and send out in chunked encoding
headers["Transfer-Encoding"] = "chunked"
headers["X-Zipline-Output"] = "streamed"
self.response_body = Chunked.new(zip_generator)
end

zip_generator = ZipGenerator.new(request.env, files, **kwargs_for_new)
response.headers.merge!(zip_generator.headers)
self.response_body = zip_generator
end
end
31 changes: 0 additions & 31 deletions lib/zipline/chunked_body.rb

This file was deleted.

52 changes: 0 additions & 52 deletions lib/zipline/tempfile_body.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/zipline/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Zipline
VERSION = "1.6.0"
VERSION = "1.7.0"
end
40 changes: 26 additions & 14 deletions lib/zipline/zip_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,22 @@
module Zipline
class ZipGenerator
# takes an array of pairs [[uploader, filename], ... ]
def initialize(files, **kwargs_for_streamer)
# Use RackBody as it has buffering built-in in zip_tricks 5.x+
@body = ZipTricks::RackBody.new(**kwargs_for_streamer) do |streamer|
def initialize(env, files, **kwargs_for_streamer)
zip_enumerator = ZipKit::OutputEnumerator.new(**kwargs_for_streamer) do |streamer|
files.each do |file, name, options = {}|
handle_file(streamer, file, name.to_s, options)
end
end

@headers, @body = recording_stream_exceptions do
# If buffering is used, the streaming block will be executed immediately to compute Content-Length
zip_enumerator.to_headers_and_rack_response_body(env)
end
end

def each(&block)
return to_enum(:each) unless block_given?
@body.each(&block)
rescue => e
# Since most APM packages do not trace errors occurring within streaming
# Rack bodies, it can be helpful to print the error to the Rails log at least
error_message = "zipline: an exception (#{e.inspect}) was raised when serving the ZIP body."
error_message += " The error occurred when handling #{@filename.inspect}" if @filename
logger.error(error_message)
raise
recording_stream_exceptions { @body.each(&block) }
end

def handle_file(streamer, file, name, options)
Expand Down Expand Up @@ -72,7 +69,7 @@ def normalize(file)
end

def write_file(streamer, file, name, options)
streamer.write_deflated_file(name, **options.slice(:modification_time)) do |writer_for_file|
streamer.write_file(name, **options.slice(:modification_time)) do |writer_for_file|
if file[:url]
the_remote_uri = URI(file[:url])

Expand All @@ -92,12 +89,27 @@ def write_file(streamer, file, name, options)
end
end

def is_io?(io_ish)
io_ish.respond_to? :read
def headers
@headers
end

private

def recording_stream_exceptions
yield
rescue => e
# Since most APM packages do not trace errors occurring within streaming
# Rack bodies, it can be helpful to print the error to the Rails log at least
error_message = "zipline: an exception (#{e.inspect}) was raised when serving the ZIP body."
error_message += " The error occurred when handling #{@filename.inspect}" if @filename
logger.error(error_message)
raise
end

def is_io?(io_ish)
io_ish.respond_to? :read
end

def logger
# Rails is not defined in our tests, and might as well not be defined
# elsewhere - or the logger might not be configured correctly
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/zipline/zip_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def to_s
let(:file){ directory.files.create(file_attributes) }

describe '.normalize' do
let(:generator){ Zipline::ZipGenerator.new([])}
let(:generator){ Zipline::ZipGenerator.new(_env = {}, _files = [])}
context "CarrierWave" do
context "Remote" do
let(:file){ CarrierWave::Storage::Fog::File.new(nil,nil,nil) }
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/zipline/zipline_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def download_zip_with_error_during_streaming
end
end

it 'passes keyword parameters to ZipTricks::Streamer' do
it 'passes keyword parameters to ZipKit::Streamer' do
fake_rack_env = {
"HTTP_VERSION" => "HTTP/1.0",
"REQUEST_METHOD" => "GET",
Expand All @@ -39,7 +39,7 @@ def download_zip_with_error_during_streaming
"SERVER_NAME" => "host.example",
"rack.input" => StringIO.new,
}
expect(ZipTricks::Streamer).to receive(:new).with(anything, auto_rename_duplicate_filenames: false).and_call_original
expect(ZipKit::Streamer).to receive(:new).with(anything, auto_rename_duplicate_filenames: false).and_call_original

status, headers, body = FakeController.action(:download_zip).call(fake_rack_env)

Expand All @@ -56,13 +56,13 @@ def download_zip_with_error_during_streaming
"SERVER_NAME" => "host.example",
"rack.input" => StringIO.new,
}
expect(ZipTricks::Streamer).to receive(:new).with(anything, auto_rename_duplicate_filenames: false).and_call_original
fake_logger = double()
expect(Logger).to receive(:new).and_return(fake_logger)
expect(fake_logger).to receive(:error).with(instance_of(String))

expect {
FakeController.action(:download_zip_with_error_during_streaming).call(fake_rack_env)
status, headers, body = FakeController.action(:download_zip_with_error_during_streaming).call(fake_rack_env)
body.each { }
}.to raise_error(/Something wonky/)
end
end
2 changes: 1 addition & 1 deletion zipline.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Gem::Specification.new do |gem|

gem.add_dependency 'actionpack', ['>= 6.0', '< 8.0']
gem.add_dependency 'content_disposition', '~> 1.0'
gem.add_dependency 'zip_tricks', ['>= 4.8.3', '< 6'] # Minimum to 4.8.3 which is the last-released MIT version
gem.add_dependency 'zip_kit', ['~> 6', '>= 6.0.1', '< 7']

gem.add_development_dependency 'rspec', '~> 3'
gem.add_development_dependency 'fog-aws'
Expand Down
Loading