From d4bc24dcb38a7b0832fe39d700ea8f88c336da22 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Sun, 24 May 2020 18:50:18 +0100 Subject: [PATCH] Clean up `OutputStream` internals. There was some fairly odd stuff going on in `put_next_entry` that allowed for data within an `Entry` to be overridden and prevented an `Entry` from being a single point of truth. Fixing this also simplifies the code within `File` and still passes all tests. Also, fixing the above means we can stop passing the compression level around as a parameter and use the value stored in each `Entry` directly. Let's keep `compression_level` out of the `Entry` public API though as it only makes sense when writing an `Entry`: there doesn't seem to be an obvious way to read what level of compression was used when reading an `Entry` from a zip file. --- lib/zip/entry.rb | 16 +++++++--------- lib/zip/output_stream.rb | 19 +++++++------------ test/file_test.rb | 1 + 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/lib/zip/entry.rb b/lib/zip/entry.rb index e216043..dc29a57 100644 --- a/lib/zip/entry.rb +++ b/lib/zip/entry.rb @@ -13,8 +13,7 @@ module Zip :restore_times, :restore_permissions, :restore_ownership, :unix_uid, :unix_gid, :unix_perms, :dirty - attr_reader :ftype, :filepath # :nodoc: - attr_writer :compression_level # :nodoc: + attr_reader :compression_level, :ftype, :filepath # :nodoc: def set_default_vars_values @local_header_offset = 0 @@ -579,14 +578,13 @@ module Zip def write_to_zip_output_stream(zip_output_stream) #:nodoc:all if @ftype == :directory - zip_output_stream.put_next_entry(self, nil, nil, ::Zip::Entry::STORED) + @compression_method = ::Zip::Entry::STORED + zip_output_stream.put_next_entry(self) elsif @filepath - zip_output_stream.put_next_entry( - self, nil, nil, - compression_method || ::Zip::Entry::DEFLATED, - @compression_level || ::Zip.default_compression - ) - get_input_stream { |is| ::Zip::IOExtras.copy_stream(zip_output_stream, is) } + zip_output_stream.put_next_entry(self) + get_input_stream do |is| + ::Zip::IOExtras.copy_stream(zip_output_stream, is) + end else zip_output_stream.copy_raw_entry(self) end diff --git a/lib/zip/output_stream.rb b/lib/zip/output_stream.rb index 4168e60..d4ca32c 100644 --- a/lib/zip/output_stream.rb +++ b/lib/zip/output_stream.rb @@ -95,15 +95,10 @@ module Zip new_entry = if entry_name.kind_of?(Entry) entry_name else - Entry.new(@file_name, entry_name.to_s) + Entry.new(@file_name, entry_name.to_s, comment, extra, 0, 0, compression_method, level) end - new_entry.comment = comment unless comment.nil? - unless extra.nil? - new_entry.extra = extra.kind_of?(ExtraField) ? extra : ExtraField.new(extra.to_s) - end - new_entry.compression_method = compression_method unless compression_method.nil? - new_entry.compression_level = level unless level.nil? - init_next_entry(new_entry, level) + + init_next_entry(new_entry) @current_entry = new_entry end @@ -143,19 +138,19 @@ module Zip @compressor = ::Zip::NullCompressor.instance end - def init_next_entry(entry, level) + def init_next_entry(entry) finalize_current_entry @entry_set << entry entry.write_local_entry(@output_stream) @encrypter.reset! @output_stream << @encrypter.header(entry.mtime) - @compressor = get_compressor(entry, level) + @compressor = get_compressor(entry) end - def get_compressor(entry, level) + def get_compressor(entry) case entry.compression_method when Entry::DEFLATED - ::Zip::Deflater.new(@output_stream, level, @encrypter) + ::Zip::Deflater.new(@output_stream, entry.compression_level, @encrypter) when Entry::STORED ::Zip::PassThruCompressor.new(@output_stream) else diff --git a/test/file_test.rb b/test/file_test.rb index 1d05007..0d72a1d 100644 --- a/test/file_test.rb +++ b/test/file_test.rb @@ -87,6 +87,7 @@ class ZipFileTest < MiniTest::Test assert_equal(custom_entry_args[2], entry.compressed_size) assert_equal(custom_entry_args[3], entry.crc) assert_equal(custom_entry_args[4], entry.compression_method) + assert_equal(custom_entry_args[5], entry.compression_level) assert_equal(custom_entry_args[6], entry.size) assert_equal(custom_entry_args[7], entry.time)