Fix #28 and #103

README.md

@@ -68,6 +68,24 @@ Zip::File.open(zipfile_name, Zip::File::CREATE) do |zipfile|
 end
 ```
 
+### Save zip archive entries in sorted by name state
+
+To saving zip archives in sorted order like below you need to set `::Zip.sort_entries` to `true`
+
+```
+Vegetable/
+Vegetable/bean
+Vegetable/carrot
+Vegetable/celery
+fruit/
+fruit/apple
+fruit/kiwi
+fruit/mango
+fruit/orange
+```
+
+After this entries in zip archive will be saved in ordered state.
+
 ## Known issues
 
 ### Modify docx file with rubyzip

lib/zip.rb

@@ -34,13 +34,14 @@ end
 
 module Zip
   extend self
-  attr_accessor :unicode_names, :on_exists_proc, :continue_on_exists_proc
+  attr_accessor :unicode_names, :on_exists_proc, :continue_on_exists_proc, :sort_entries
 
   def reset!
     @_ran_once = false
     @unicode_names = false
     @on_exists_proc = false
     @continue_on_exists_proc = false
+    @sort_entries = false
   end
 
   def setup

lib/zip/entry_set.rb

@@ -6,7 +6,6 @@ module Zip
     def initialize(an_enumerable = [])
       super()
       @entry_set   = {}
-      @entry_order = []
       an_enumerable.each { |o| push(o) }
     end
 
@@ -19,8 +18,6 @@ module Zip
     end
 
     def <<(entry)
-      @entry_order.delete(to_key(entry))
-      @entry_order << to_key(entry)
       @entry_set[to_key(entry)] = entry
     end
 
@@ -33,7 +30,7 @@ module Zip
     alias :length :size
 
     def delete(entry)
-      if @entry_order.delete(to_key(entry)) && @entry_set.delete(to_key(entry))
+      if @entry_set.delete(to_key(entry))
         entry
       else
         nil
@@ -41,23 +38,27 @@ module Zip
     end
 
     def each(&block)
-      @entry_order.each do |key|
-        block.call @entry_set[key]
+      @entry_set = @entry_set.dup.each do |_, value|
+        block.call(value)
       end
     end
 
     def entries
-      @entry_order.map { |key| @entry_set[key] }
+      if ::Zip.sort_entries == true
+        @entry_set.values.sort_by{|x| x.name}
+      else
+        @entry_set.values
+      end
     end
 
     # deep clone
     def dup
-      EntrySet.new(@entry_order.map { |key| @entry_set[key].dup })
+      EntrySet.new(@entry_set.map { |key, value| value.dup })
     end
 
     def ==(other)
       return false unless other.kind_of?(EntrySet)
-      @entry_set == other.entry_set && @entry_order == other.entry_order
+      @entry_set.values == other.entry_set.values
     end
 
     def parent(entry)

lib/zip/file.rb

@@ -314,11 +314,10 @@ module Zip
 
     # Write buffer write changes to buffer and return
     def write_buffer
-      buffer = OutputStream.write_buffer do |zos|
+      OutputStream.write_buffer do |zos|
         @entry_set.each { |e| e.write_to_zip_output_stream(zos) }
         zos.comment = comment
       end
-      return buffer
     end
 
     # Closes the zip file committing any changes that has been made.
@@ -349,14 +348,14 @@ module Zip
     # Searches for an entry just as find_entry, but throws Errno::ENOENT
     # if no entry is found.
     def get_entry(entry)
-      selectedEntry = find_entry(entry)
-      unless selectedEntry
+      selected_entry = find_entry(entry)
+      unless selected_entry
         raise Errno::ENOENT, entry
       end
-      selectedEntry.restore_ownership   = @restore_ownership
-      selectedEntry.restore_permissions = @restore_permissions
-      selectedEntry.restore_times       = @restore_times
-      selectedEntry
+      selected_entry.restore_ownership   = @restore_ownership
+      selected_entry.restore_permissions = @restore_permissions
+      selected_entry.restore_times       = @restore_times
+      selected_entry
     end
 
     # Creates a directory
@@ -366,7 +365,7 @@ module Zip
       end
       entryName = entryName.dup.to_s
       entryName << '/' unless entryName.end_with?('/')
-      @entry_set << StreamableDirectory.new(@name, entryName, nil, permissionInt)
+      @entry_set << ::Zip::StreamableDirectory.new(@name, entryName, nil, permissionInt)
     end
 
     private

test/ziptest.rb

@@ -886,8 +886,8 @@ end
 class ZipEntrySetTest < Test::Unit::TestCase
   ZIP_ENTRIES = [
     ::Zip::Entry.new("zipfile.zip", "name1", "comment1"),
-    ::Zip::Entry.new("zipfile.zip", "name2", "comment1"),
     ::Zip::Entry.new("zipfile.zip", "name3", "comment1"),
+    ::Zip::Entry.new("zipfile.zip", "name2", "comment1"),
     ::Zip::Entry.new("zipfile.zip", "name4", "comment1"),
     ::Zip::Entry.new("zipfile.zip", "name5", "comment1"),
     ::Zip::Entry.new("zipfile.zip", "name6", "comment1")
@@ -941,7 +941,14 @@ class ZipEntrySetTest < Test::Unit::TestCase
   end
 
   def test_entries
-    assert_equal(ZIP_ENTRIES.sort, @zipEntrySet.entries.sort)
+    assert_equal(ZIP_ENTRIES, @zipEntrySet.entries)
+  end
+
+  def test_entries_with_sort
+    ::Zip.sort_entries = true
+    assert_equal(ZIP_ENTRIES.sort, @zipEntrySet.entries)
+    ::Zip.sort_entries = false
+    assert_equal(ZIP_ENTRIES, @zipEntrySet.entries)
   end
 
   def test_compound
@@ -1368,6 +1375,39 @@ class ZipFileTest < Test::Unit::TestCase
     zfRead.close
   end
 
+  def test_rename_with_each
+    zf_name = 'test_rename_zip.zip'
+    if ::File.exist?(zf_name)
+      ::File.unlink(zf_name)
+    end
+    arr = []
+    arr_renamed = []
+    ::Zip::File.open(zf_name, ::Zip::File::CREATE) do |zf|
+      zf.mkdir('test')
+      arr << 'test/'
+      arr_renamed << 'Ztest/'
+      %w(a b c d).each do |f|
+        zf.get_output_stream("test/#{f}") {|file| file.puts 'aaaa'}
+        arr << "test/#{f}"
+        arr_renamed << "Ztest/#{f}"
+      end
+    end
+    zf = ::Zip::File.open(zf_name)
+    assert_equal(zf.entries.map(&:name), arr)
+    zf.close
+    Zip::File.open(zf_name, "wb") do |z|
+      z.each do |f|
+        z.rename(f, "Z#{f.name}")
+      end
+    end
+    zf = ::Zip::File.open(zf_name)
+    assert_equal(zf.entries.map(&:name), arr_renamed)
+    zf.close
+    if ::File.exist?(zf_name)
+      ::File.unlink(zf_name)
+    end
+  end
+
   def test_renameToExistingEntry
     oldEntries = nil
     ::Zip::File.open(TEST_ZIP.zip_name) { |zf| oldEntries = zf.entries }