FIX: use uniq instead of uniq! when checking for uncompressed root path. Use rails naming convention for ZipUtils

lib/import_export/zip_utils.rb

@@ -2,57 +2,59 @@
 
 require 'zip'
 
-class ZipUtils
+module ImportExport
-  def zip_directory(path, export_name)
+  class ZipUtils
-    zip_filename = "#{export_name}.zip"
+    def zip_directory(path, export_name)
-    absolute_path = "#{path}/#{export_name}"
+      zip_filename = "#{export_name}.zip"
-    entries = Dir.entries(absolute_path) - %w[. ..]
+      absolute_path = "#{path}/#{export_name}"
+      entries = Dir.entries(absolute_path) - %w[. ..]
 
-    Zip::File.open(zip_filename, Zip::File::CREATE) do |zipfile|
+      Zip::File.open(zip_filename, Zip::File::CREATE) do |zipfile|
-      write_entries(entries, absolute_path, '', zipfile)
+        write_entries(entries, absolute_path, '', zipfile)
+      end
+
+      "#{absolute_path}.zip"
     end
 
-    "#{absolute_path}.zip"
+    def unzip_directory(path, zip_filename, allow_non_root_folder: false)
-  end
+      Zip::File.open(zip_filename) do |zip_file|
-
+        root = root_folder_present?(zip_file, allow_non_root_folder) ? '' : 'unzipped/'
-  def unzip_directory(path, zip_filename, allow_non_root_folder: false)
+        zip_file.each do |entry|
-    Zip::File.open(zip_filename) do |zip_file|
+          entry_path = File.join(path, "#{root}#{entry.name}")
-      root = root_folder_present?(zip_file, allow_non_root_folder) ? '' : 'unzipped/'
+          FileUtils.mkdir_p(File.dirname(entry_path))
-      zip_file.each do |entry|
+          entry.extract(entry_path)
-        entry_path = File.join(path, "#{root}#{entry.name}")
+        end
-        FileUtils.mkdir_p(File.dirname(entry_path))
-        entry.extract(entry_path)
       end
     end
-  end
 
-  private
+    private
 
-  def root_folder_present?(filenames, allow_non_root_folder)
+    def root_folder_present?(filenames, allow_non_root_folder)
-    filenames.map { |p| p.name.split('/').first }.uniq!.size == 1 || allow_non_root_folder
+      filenames.map { |p| p.name.split('/').first }.uniq.size == 1 || allow_non_root_folder
-  end
+    end
 
-  # A helper method to make the recursion work.
+    # A helper method to make the recursion work.
-  def write_entries(entries, base_path, path, zipfile)
+    def write_entries(entries, base_path, path, zipfile)
-    entries.each do |e|
+      entries.each do |e|
-      zipfile_path = path == '' ? e : File.join(path, e)
+        zipfile_path = path == '' ? e : File.join(path, e)
-      disk_file_path = File.join(base_path, zipfile_path)
+        disk_file_path = File.join(base_path, zipfile_path)
 
-      if File.directory? disk_file_path
+        if File.directory? disk_file_path
-        recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
+          recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
-      else
+        else
-        put_into_archive(disk_file_path, zipfile, zipfile_path)
+          put_into_archive(disk_file_path, zipfile, zipfile_path)
+        end
       end
     end
-  end
 
-  def recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
+    def recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
-    zipfile.mkdir zipfile_path
+      zipfile.mkdir zipfile_path
-    subdir = Dir.entries(disk_file_path) - %w[. ..]
+      subdir = Dir.entries(disk_file_path) - %w[. ..]
-    write_entries subdir, base_path, zipfile_path, zipfile
+      write_entries subdir, base_path, zipfile_path, zipfile
-  end
+    end
 
-  def put_into_archive(disk_file_path, zipfile, zipfile_path)
+    def put_into_archive(disk_file_path, zipfile, zipfile_path)
-    zipfile.add(zipfile_path, disk_file_path)
+      zipfile.add(zipfile_path, disk_file_path)
+    end
   end
 end

lib/theme_store/tgz_exporter.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'import_export/zip_utils'
+
 module ThemeStore; end
 
 class ThemeStore::TgzExporter
@@ -60,6 +62,6 @@ class ThemeStore::TgzExporter
   def export_package
     export_to_folder
 
-    Dir.chdir(@temp_folder) { ZipUtils.new.zip_directory(@temp_folder, @export_name) }
+    Dir.chdir(@temp_folder) { ImportExport::ZipUtils.new.zip_directory(@temp_folder, @export_name) }
   end
 end

lib/theme_store/tgz_importer.rb

@@ -18,7 +18,7 @@ class ThemeStore::TgzImporter
 
     Dir.chdir(@temp_folder) do
       if @filename.include?('.zip')
-        ZipUtils.new.unzip_directory(@temp_folder, @filename)
+        ImportExport::ZipUtils.new.unzip_directory(@temp_folder, @filename)
 
         # --strip 1 equivalent
         FileUtils.mv(Dir.glob("#{@temp_folder}/*/*"), @temp_folder)

spec/components/theme_store/tgz_exporter_spec.rb

@@ -2,7 +2,6 @@
 
 require 'rails_helper'
 require 'theme_store/tgz_exporter'
-require 'import_export/zip_utils'
 
 describe ThemeStore::TgzExporter do
   let!(:theme) do
@@ -64,7 +63,7 @@ describe ThemeStore::TgzExporter do
     file = 'discourse-header-icons.zip'
     dest = 'discourse-header-icons'
     Dir.chdir(dir) do
-      ZipUtils.new.unzip_directory(dir, file, allow_non_root_folder: true)
+      ImportExport::ZipUtils.new.unzip_directory(dir, file, allow_non_root_folder: true)
       `rm #{file}`
 
       folders = Dir.glob("**/*").reject { |f| File.file?(f) }

spec/components/theme_store/tgz_importer_spec.rb

@@ -24,7 +24,7 @@ describe ThemeStore::TgzImporter do
 
   it "can import a simple zipped theme" do
     Dir.chdir(@temp_folder) do
-      ZipUtils.new.zip_directory(@temp_folder, 'test')
+      ImportExport::ZipUtils.new.zip_directory(@temp_folder, 'test')
       FileUtils.rm_rf('test/')
     end