Skip to content

Commit

Permalink
Refactor the create logic in Zip::File.
Browse files Browse the repository at this point in the history
Make the internal @create varible more consistent and actually match the
documentation.

Zip::File::CREATE is now true, rather than 1.

A new test is added to check if passing 1 in still works to ensure backwards
compatibility.
  • Loading branch information
hainesr committed Sep 5, 2016
1 parent 63ed0d9 commit fc23f68
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/zip/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module Zip
# interface for accessing the filesystem, ie. the File and Dir classes.

class File < CentralDirectory
CREATE = 1
CREATE = true
SPLIT_SIGNATURE = 0x08074b50
ZIP64_EOCD_SIGNATURE = 0x06064b50
MAX_SEGMENT_SIZE = 3_221_225_472
Expand All @@ -64,19 +64,19 @@ class File < CentralDirectory

# Opens a zip archive. Pass true as the second parameter to create
# a new archive if it doesn't exist already.
def initialize(file_name, create = nil, buffer = false, options = {})
def initialize(file_name, create = false, buffer = false, options = {})
super()
@name = file_name
@comment = ''
@create = create
@create = create ? true : false # allow any truthy value to mean true
case
when !buffer && ::File.size?(file_name)
@create = nil
@create = false
@file_permissions = ::File.stat(file_name).mode
::File.open(name, 'rb') do |f|
read_from_stream(f)
end
when create
when @create
@entry_set = EntrySet.new
when ::File.zero?(file_name)
raise Error, "File #{file_name} has zero size. Did you mean to pass the create flag?"
Expand All @@ -94,7 +94,7 @@ class << self
# Same as #new. If a block is passed the ZipFile object is passed
# to the block and is automatically closed afterwards just as with
# ruby's builtin File.open method.
def open(file_name, create = nil)
def open(file_name, create = false)
zf = ::Zip::File.new(file_name, create)
return zf unless block_given?
begin
Expand Down Expand Up @@ -339,7 +339,7 @@ def commit_required?
@entry_set.each do |e|
return true if e.dirty
end
@comment != @stored_comment || @entry_set != @stored_entries || @create == ::Zip::File::CREATE
@comment != @stored_comment || @entry_set != @stored_entries || @create
end

# Searches for entry with the specified name. Returns nil if
Expand Down Expand Up @@ -407,7 +407,7 @@ def on_success_replace
begin
if yield tmp_filename
::File.rename(tmp_filename, name)
::File.chmod(@file_permissions, name) if @create.nil?
::File.chmod(@file_permissions, name) unless @create
end
ensure
::File.unlink(tmp_filename) if ::File.exist?(tmp_filename)
Expand Down
14 changes: 14 additions & 0 deletions test/file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ def test_create_from_scratch
assert_equal(2, zfRead.entries.length)
end

def test_create_from_scratch_with_old_create_parameter
comment = 'a short comment'

zf = ::Zip::File.new(EMPTY_FILENAME, 1)
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
zf.mkdir('dir1')
zf.comment = comment
zf.close

zfRead = ::Zip::File.new(EMPTY_FILENAME)
assert_equal(comment, zfRead.comment)
assert_equal(2, zfRead.entries.length)
end

def test_get_output_stream
entryCount = nil
::Zip::File.open(TEST_ZIP.zip_name) do |zf|
Expand Down

0 comments on commit fc23f68

Please sign in to comment.