Skip to content

Commit

Permalink
FEATURE: Gz to zip for exports (#7889)
Browse files Browse the repository at this point in the history
* Revert "Revert "FEATURE: admin/user exports are compressed using the zip format (#7784)""

This reverts commit f89bd55.

* Replace .tar.zip with .zip
  • Loading branch information
romanrizzi committed Jul 18, 2019
1 parent c1b5861 commit f5c707c
Show file tree
Hide file tree
Showing 15 changed files with 126 additions and 37 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Expand Up @@ -203,6 +203,8 @@ gem "sassc-rails"
gem 'rotp'
gem 'rqrcode'

gem 'rubyzip', require: false

gem 'sshkey', require: false

gem 'rchardet', require: false
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Expand Up @@ -351,6 +351,7 @@ GEM
guess_html_encoding (>= 0.0.4)
nokogiri (>= 1.6.0)
ruby_dep (1.5.0)
rubyzip (1.2.3)
safe_yaml (1.0.5)
sanitize (5.0.0)
crass (~> 1.0.2)
Expand Down Expand Up @@ -516,6 +517,7 @@ DEPENDENCIES
rubocop
ruby-prof
ruby-readability
rubyzip
sanitize
sassc
sassc-rails
Expand Down
Expand Up @@ -44,7 +44,7 @@

{{#if local}}
<div class="inputs">
<input onchange={{action "uploadLocaleFile"}} type="file" id="file-input" accept='.dcstyle.json,application/json,.tar.gz,application/x-gzip'><br>
<input onchange={{action "uploadLocaleFile"}} type="file" id="file-input" accept='.dcstyle.json,application/json,.tar.gz,application/x-gzip,.zip,application/zip'><br>
<span class="description">{{i18n 'admin.customize.theme.import_file_tip'}}</span>
</div>
{{/if}}
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/admin/themes_controller.rb
Expand Up @@ -88,7 +88,7 @@ def import
rescue RemoteTheme::ImportError => e
render_json_error e.message
end
elsif params[:bundle] || (params[:theme] && ["application/x-gzip", "application/gzip"].include?(params[:theme].content_type))
elsif params[:bundle] || (params[:theme] && ["application/x-gzip", "application/gzip", "application/zip"].include?(params[:theme].content_type))
# params[:bundle] used by theme CLI. params[:theme] used by admin UI
bundle = params[:bundle] || params[:theme]
theme_id = params[:theme_id]
Expand Down Expand Up @@ -252,6 +252,7 @@ def export

exporter = ThemeStore::TgzExporter.new(@theme)
file_path = exporter.package_filename

headers['Content-Length'] = File.size(file_path).to_s
send_data File.read(file_path),
filename: File.basename(file_path),
Expand Down
12 changes: 7 additions & 5 deletions app/jobs/regular/export_csv_file.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'csv'
require 'zip'
require_dependency 'system_message'
require_dependency 'upload_creator'

Expand Down Expand Up @@ -53,18 +54,19 @@ def execute(args)
# ensure directory exists
FileUtils.mkdir_p(UserExport.base_directory) unless Dir.exists?(UserExport.base_directory)

# write to CSV file
CSV.open(absolute_path, "w") do |csv|
# Generate a compressed CSV file
csv_to_export = CSV.generate do |csv|
csv << get_header if @entity != "report"
public_send(export_method).each { |d| csv << d }
end

# compress CSV file
system('gzip', '-5', absolute_path)
compressed_file_path = "#{absolute_path}.zip"
Zip::File.open(compressed_file_path, Zip::File::CREATE) do |zipfile|
zipfile.get_output_stream(file_name) { |f| f.puts csv_to_export }
end

# create upload
upload = nil
compressed_file_path = "#{absolute_path}.gz"

if File.exist?(compressed_file_path)
File.open(compressed_file_path) do |file|
Expand Down
2 changes: 1 addition & 1 deletion config/locales/client.en.yml
Expand Up @@ -3517,7 +3517,7 @@ en:
delete_upload_confirm: "Delete this upload? (Theme CSS may stop working!)"
import_web_tip: "Repository containing theme"
import_web_advanced: "Advanced..."
import_file_tip: ".tar.gz or .dcstyle.json file containing theme"
import_file_tip: ".tar.gz, .zip, or .dcstyle.json file containing theme"
is_private: "Theme is in a private git repository"
remote_branch: "Branch name (optional)"
public_key: "Grant the following public key access to the repo:"
Expand Down
2 changes: 1 addition & 1 deletion config/locales/server.en.yml
Expand Up @@ -2671,7 +2671,7 @@ en:
The above download link will be valid for 48 hours.
The data is compressed as a gzip archive. If the archive does not extract itself when you open it, use the tools recommended here: https://www.gzip.org/#faq4
The data is compressed as a zip archive. If the archive does not extract itself when you open it, use the tool recommended here: https://www.7-zip.org/
csv_export_failed:
title: "CSV Export Failed"
Expand Down
2 changes: 1 addition & 1 deletion config/site_settings.yml
Expand Up @@ -1057,7 +1057,7 @@ files:
list_type: compact
export_authorized_extensions:
hidden: true
default: "gz"
default: "zip"
type: list
list_type: compact
responsive_post_image_sizes:
Expand Down
58 changes: 58 additions & 0 deletions lib/import_export/zip_utils.rb
@@ -0,0 +1,58 @@
# frozen_string_literal: true

require 'zip'

class ZipUtils
def zip_directory(path, export_name)
zip_filename = "#{export_name}.zip"
absolute_path = "#{path}/#{export_name}"
entries = Dir.entries(absolute_path) - %w[. ..]

Zip::File.open(zip_filename, Zip::File::CREATE) do |zipfile|
write_entries(entries, absolute_path, '', zipfile)
end

"#{absolute_path}.zip"
end

def unzip_directory(path, zip_filename, allow_non_root_folder: false)
Zip::File.open(zip_filename) do |zip_file|
root = root_folder_present?(zip_file, allow_non_root_folder) ? '' : 'unzipped/'
zip_file.each do |entry|
entry_path = File.join(path, "#{root}#{entry.name}")
FileUtils.mkdir_p(File.dirname(entry_path))
entry.extract(entry_path)
end
end
end

private

def root_folder_present?(filenames, allow_non_root_folder)
filenames.map { |p| p.name.split('/').first }.uniq!.size == 1 || allow_non_root_folder
end

# A helper method to make the recursion work.
def write_entries(entries, base_path, path, zipfile)
entries.each do |e|
zipfile_path = path == '' ? e : File.join(path, e)
disk_file_path = File.join(base_path, zipfile_path)

if File.directory? disk_file_path
recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
else
put_into_archive(disk_file_path, zipfile, zipfile_path)
end
end
end

def recursively_deflate_directory(disk_file_path, zipfile, base_path, zipfile_path)
zipfile.mkdir zipfile_path
subdir = Dir.entries(disk_file_path) - %w[. ..]
write_entries subdir, base_path, zipfile_path, zipfile
end

def put_into_archive(disk_file_path, zipfile, zipfile_path)
zipfile.add(zipfile_path, disk_file_path)
end
end
10 changes: 3 additions & 7 deletions lib/theme_store/tgz_exporter.rb
Expand Up @@ -56,14 +56,10 @@ def export_to_folder
end

private

def export_package
export_to_folder
Dir.chdir(@temp_folder) do
tar_filename = "#{@export_name}.tar"
Discourse::Utils.execute_command('tar', '--create', '--file', tar_filename, @export_name, failure_message: "Failed to tar theme.")
Discourse::Utils.execute_command('gzip', '-5', tar_filename, failure_message: "Failed to gzip archive.")
"#{@temp_folder}/#{tar_filename}.gz"
end
end

Dir.chdir(@temp_folder) { ZipUtils.new.zip_directory(@temp_folder, @export_name) }

This comment has been minimized.

Copy link
@davidtaylorhq

davidtaylorhq Jul 28, 2019

Member

ThemeStore::TgzExporter should be renamed to ZipExporter, to avoid confusion

end
end
14 changes: 12 additions & 2 deletions lib/theme_store/tgz_importer.rb
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'import_export/zip_utils'

module ThemeStore; end

class ThemeStore::TgzImporter

This comment has been minimized.

Copy link
@davidtaylorhq

davidtaylorhq Jul 28, 2019

Member

This should be renamed to something generic, like ArchiveImporter. It now does zip as well as Tgz

Expand All @@ -13,8 +15,16 @@ def initialize(filename)

def import!
FileUtils.mkdir(@temp_folder)

Dir.chdir(@temp_folder) do
Discourse::Utils.execute_command("tar", "-xzvf", @filename, "--strip", "1")
if @filename.include?('.zip')
ZipUtils.new.unzip_directory(@temp_folder, @filename)

# --strip 1 equivalent
FileUtils.mv(Dir.glob("#{@temp_folder}/*/*"), @temp_folder)
else
Discourse::Utils.execute_command("tar", "-xzvf", @filename, "--strip", "1")
end
end
rescue RuntimeError
raise RemoteTheme::ImportError, I18n.t("themes.import_error.unpack_failed")
Expand Down Expand Up @@ -44,7 +54,7 @@ def real_path(relative)

def all_files
Dir.chdir(@temp_folder) do
Dir.glob("**/*").reject { |f| File.directory?(f) }
Dir.glob("**/**").reject { |f| File.directory?(f) }
end
end

Expand Down
15 changes: 9 additions & 6 deletions spec/components/theme_store/tgz_exporter_spec.rb
Expand Up @@ -2,6 +2,7 @@

require 'rails_helper'
require 'theme_store/tgz_exporter'
require 'import_export/zip_utils'

describe ThemeStore::TgzExporter do
let!(:theme) do
Expand Down Expand Up @@ -55,15 +56,17 @@
filename = exporter.package_filename
FileUtils.cp(filename, dir)
exporter.cleanup!
"#{dir}/discourse-header-icons.tar.gz"
"#{dir}/discourse-header-icons.zip"
end

it "exports the theme correctly" do
package
Dir.chdir("#{dir}") do
`tar -xzf discourse-header-icons.tar.gz`
end
Dir.chdir("#{dir}/discourse-header-icons") 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)
`rm #{file}`

folders = Dir.glob("**/*").reject { |f| File.file?(f) }
expect(folders).to contain_exactly("assets", "common", "locales", "mobile")

Expand Down Expand Up @@ -121,7 +124,7 @@
exporter = ThemeStore::TgzExporter.new(theme)
filename = exporter.package_filename
exporter.cleanup!
expect(filename).to end_with "/discourse-header-icons.tar.gz"
expect(filename).to end_with "/discourse-header-icons.zip"
end

end
29 changes: 22 additions & 7 deletions spec/components/theme_store/tgz_importer_spec.rb
Expand Up @@ -4,26 +4,41 @@

require 'rails_helper'
require 'theme_store/tgz_importer'
require 'import_export/zip_utils'

describe ThemeStore::TgzImporter do
before do
@temp_folder = "#{Pathname.new(Dir.tmpdir).realpath}/discourse_theme_#{SecureRandom.hex}"

FileUtils.mkdir(@temp_folder)
Dir.chdir(@temp_folder) do
FileUtils.mkdir_p('test/a')
File.write("test/hello.txt", "hello world")
File.write("test/a/inner", "hello world inner")
end
end

after do
FileUtils.rm_rf @temp_folder
end

it "can import a simple theme" do
it "can import a simple zipped theme" do
Dir.chdir(@temp_folder) do
ZipUtils.new.zip_directory(@temp_folder, 'test')
FileUtils.rm_rf('test/')
end

FileUtils.mkdir(@temp_folder)
importer = ThemeStore::TgzImporter.new("#{@temp_folder}/test.zip")
importer.import!

Dir.chdir(@temp_folder) do
FileUtils.mkdir('test/')
File.write("test/hello.txt", "hello world")
FileUtils.mkdir('test/a')
File.write("test/a/inner", "hello world inner")
expect(importer["hello.txt"]).to eq("hello world")
expect(importer["a/inner"]).to eq("hello world inner")

importer.cleanup!
end

it "can import a simple gzipped theme" do
Dir.chdir(@temp_folder) do
`tar -cvzf test.tar.gz test/* 2> /dev/null`
end

Expand Down
6 changes: 3 additions & 3 deletions spec/components/validators/upload_validator_spec.rb
Expand Up @@ -22,14 +22,14 @@
it "allows 'gz' as extension when uploading export file" do
SiteSetting.authorized_extensions = ""

expect(UploadCreator.new(csv_file, "#{filename}.gz", for_export: true).create_for(user.id)).to be_valid
expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid
end

it "allows uses max_export_file_size_kb when uploading export file" do
SiteSetting.max_attachment_size_kb = "0"
SiteSetting.authorized_extensions = "gz"
SiteSetting.authorized_extensions = "zip"

expect(UploadCreator.new(csv_file, "#{filename}.gz", for_export: true).create_for(user.id)).to be_valid
expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid
end

describe 'when allow_staff_to_upload_any_file_in_pm is true' do
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/admin/themes_controller_spec.rb
Expand Up @@ -51,10 +51,10 @@
expect(response.status).to eq(200)

# Save the output in a temp file (automatically cleaned up)
file = Tempfile.new('archive.tar.gz')
file = Tempfile.new('archive.tar.zip')
file.write(response.body)
file.rewind
uploaded_file = Rack::Test::UploadedFile.new(file.path, "application/x-gzip")
uploaded_file = Rack::Test::UploadedFile.new(file.path, "application/zip")

# Now import it again
expect do
Expand Down

1 comment on commit f5c707c

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

David Taylor posted:

Found a few small issues when working on discourse/discourse_theme@de0940b. I've commented two above, and also the content-type of the download should be changed from application/x-gzip to application/zip

Please sign in to comment.