diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index d33adb32ee2..7614b30f5db 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -81,7 +81,8 @@ class UploadsController < ApplicationController if filename == "image.png" && SiteSetting.convert_pasted_images_to_hq_jpg jpeg_path = "#{File.dirname(tempfile.path)}/image.jpg" OptimizedImage.ensure_safe_paths!(tempfile.path, jpeg_path) - `convert #{tempfile.path} -quality #{SiteSetting.convert_pasted_images_quality} #{jpeg_path}` + + Discourse::Utils.execute_command('convert', tempfile.path, '-quality', SiteSetting.convert_pasted_images_quality.to_s, jpeg_path) # only change the format of the image when JPG is at least 5% smaller if File.size(jpeg_path) < File.size(tempfile.path) * 0.95 filename = "image.jpg" diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 8af90fded90..5f7f085addd 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -215,8 +215,7 @@ class OptimizedImage < ActiveRecord::Base end def self.convert_with(instructions, to) - `#{instructions.join(" ")} &> /dev/null` - return false if $?.exitstatus != 0 + return false unless system(instructions.join(" "), '&> /dev/null') ImageOptim.new.optimize_image!(to) true diff --git a/app/models/upload.rb b/app/models/upload.rb index 1200d2487eb..d9aa27eaf39 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -253,7 +253,7 @@ class Upload < ActiveRecord::Base end def self.fix_image_orientation(path) - `convert #{path} -auto-orient #{path}` + Discourse::Utils.execute_command('convert', path, '-auto-orient', path) end def self.migrate_to_new_scheme(limit=nil) diff --git a/lib/backup_restore/backup_restore.rb b/lib/backup_restore/backup_restore.rb index 0b50e094fbd..12a9ecd21dd 100644 --- a/lib/backup_restore/backup_restore.rb +++ b/lib/backup_restore/backup_restore.rb @@ -1,4 +1,3 @@ -require "backup_restore/utils" require "backup_restore/backuper" require "backup_restore/restorer" diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb index bc885e8fe12..37bb897a7e6 100644 --- a/lib/backup_restore/backuper.rb +++ b/lib/backup_restore/backuper.rb @@ -1,8 +1,6 @@ module BackupRestore class Backuper - include BackupRestore::Utils - attr_reader :success def initialize(user_id, opts={}) @@ -198,7 +196,7 @@ module BackupRestore def move_dump_backup log "Finalizing database dump file: #{@backup_filename}" - execute_command( + Discourse::Utils.execute_command( 'mv', @dump_filename, File.join(@archive_directory, @backup_filename), failure_message: "Failed to move database dump file." ) @@ -212,15 +210,15 @@ module BackupRestore tar_filename = "#{@archive_basename}.tar" log "Making sure archive does not already exist..." - execute_command('rm', '-f', tar_filename) - execute_command('rm', '-f', "#{tar_filename}.gz") + Discourse::Utils.execute_command('rm', '-f', tar_filename) + Discourse::Utils.execute_command('rm', '-f', "#{tar_filename}.gz") log "Creating empty archive..." - execute_command('tar', '--create', '--file', tar_filename, '--files-from', '/dev/null') + Discourse::Utils.execute_command('tar', '--create', '--file', tar_filename, '--files-from', '/dev/null') log "Archiving data dump..." FileUtils.cd(File.dirname(@dump_filename)) do - execute_command( + Discourse::Utils.execute_command( 'tar', '--append', '--dereference', '--file', tar_filename, File.basename(@dump_filename), failure_message: "Failed to archive data dump." ) @@ -231,7 +229,7 @@ module BackupRestore log "Archiving uploads..." FileUtils.cd(File.join(Rails.root, "public")) do if File.directory?(upload_directory) - execute_command( + Discourse::Utils.execute_command( 'tar', '--append', '--dereference', '--file', tar_filename, upload_directory, failure_message: "Failed to archive uploads." ) @@ -243,7 +241,7 @@ module BackupRestore remove_tmp_directory log "Gzipping archive, this may take a while..." - execute_command('gzip', '-5', tar_filename, failure_message: "Failed to gzip archive.") + Discourse::Utils.execute_command('gzip', '-5', tar_filename, failure_message: "Failed to gzip archive.") end def after_create_hook @@ -259,11 +257,11 @@ module BackupRestore def notify_user log "Notifying '#{@user.username}' of the end of the backup..." - if @success - SystemMessage.create_from_system_user(@user, :backup_succeeded, logs: pretty_logs(@logs)) - else - SystemMessage.create_from_system_user(@user, :backup_failed, logs: pretty_logs(@logs)) - end + status = @success ? :backup_succeeded : :backup_failed + + SystemMessage.create_from_system_user(@user, status, + logs: Discouse::Utils.pretty_logs(@logs) + ) end def clean_up diff --git a/lib/backup_restore/restorer.rb b/lib/backup_restore/restorer.rb index f7606e871e1..de6e133df13 100644 --- a/lib/backup_restore/restorer.rb +++ b/lib/backup_restore/restorer.rb @@ -6,8 +6,6 @@ module BackupRestore class FilenameMissingError < RuntimeError; end class Restorer - include BackupRestore::Utils - attr_reader :success def initialize(user_id, opts={}) @@ -166,7 +164,7 @@ module BackupRestore def copy_archive_to_tmp_directory log "Copying archive to tmp directory..." - execute_command('cp', @source_filename, @archive_filename, failure_message: "Failed to copy archive to tmp directory.") + Discourse::Utils.execute_command('cp', @source_filename, @archive_filename, failure_message: "Failed to copy archive to tmp directory.") end def unzip_archive @@ -175,7 +173,7 @@ module BackupRestore log "Unzipping archive, this may take a while..." FileUtils.cd(@tmp_directory) do - execute_command('gzip', '--decompress', @archive_filename, failure_message: "Failed to unzip archive.") + Discourse::Utils.execute_command('gzip', '--decompress', @archive_filename, failure_message: "Failed to unzip archive.") end end @@ -185,7 +183,7 @@ module BackupRestore @metadata = if system('tar', '--list', '--file', @tar_filename, BackupRestore::METADATA_FILE) FileUtils.cd(@tmp_directory) do - execute_command( + Discourse::Utils.execute_command( 'tar', '--extract', '--file', @tar_filename, BackupRestore::METADATA_FILE, failure_message: "Failed to extract metadata file." ) @@ -233,7 +231,7 @@ module BackupRestore log "Extracting dump file..." FileUtils.cd(@tmp_directory) do - execute_command( + Discourse::Utils.execute_command( 'tar', '--extract', '--file', @tar_filename, File.basename(@dump_filename), failure_message: "Failed to extract dump file." ) @@ -364,7 +362,7 @@ module BackupRestore log "Extracting uploads..." FileUtils.cd(@tmp_directory) do - execute_command( + Discourse::Utils.execute_command( 'tar', '--extract', '--keep-newer-files', '--file', @tar_filename, 'uploads/', failure_message: "Failed to extract uploads." ) @@ -379,7 +377,7 @@ module BackupRestore previous_db_name = File.basename(tmp_uploads_path) current_db_name = RailsMultisite::ConnectionManagement.current_db - execute_command( + Discourse::Utils.execute_command( 'rsync', '-avp', '--safe-links', "#{tmp_uploads_path}/", "uploads/#{current_db_name}/", failure_message: "Failed to restore uploads." ) @@ -404,11 +402,11 @@ module BackupRestore def notify_user if user = User.find_by(email: @user_info[:email]) log "Notifying '#{user.username}' of the end of the restore..." - if @success - SystemMessage.create_from_system_user(user, :restore_succeeded, logs: pretty_logs(@logs)) - else - SystemMessage.create_from_system_user(user, :restore_failed, logs: pretty_logs(@logs)) - end + status = @success ? :restore_succeeded : :restore_failed + + SystemMessage.create_from_system_user(user, status, + logs: Discourse::Utils.pretty_logs(@logs) + ) else log "Could not send notification to '#{@user_info[:username]}' (#{@user_info[:email]}), because the user does not exists..." end diff --git a/lib/backup_restore/utils.rb b/lib/backup_restore/utils.rb deleted file mode 100644 index b782f61bee6..00000000000 --- a/lib/backup_restore/utils.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'open3' - -module BackupRestore - module Utils - def execute_command(*command, failure_message: "") - stdout, stderr, status = Open3.capture3(*command) - - if !status.success? - failure_message = "#{failure_message}\n" if !failure_message.blank? - raise "#{failure_message}#{stderr}" - end - - stdout - end - - def pretty_logs(logs) - logs.join("\n") - end - end -end diff --git a/lib/discourse.rb b/lib/discourse.rb index d8e66fa405a..086ed28b72b 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -1,4 +1,5 @@ require 'cache' +require 'open3' require_dependency 'plugin/instance' require_dependency 'auth/default_current_user_provider' require_dependency 'version' @@ -16,6 +17,23 @@ module Discourse extend Sidekiq::ExceptionHandler end + class Utils + def self.execute_command(*command, failure_message: "") + stdout, stderr, status = Open3.capture3(*command) + + if !status.success? + failure_message = "#{failure_message}\n" if !failure_message.blank? + raise "#{failure_message}#{stderr}" + end + + stdout + end + + def self.pretty_logs(logs) + logs.join("\n".freeze) + end + end + # Log an exception. # # If your code is in a scheduled job, it is recommended to use the diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index 411b93ed0d2..5b23505998f 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -59,7 +59,7 @@ module FileStore end def purge_tombstone(grace_period) - `find #{tombstone_dir} -mtime +#{grace_period} -type f -delete` + Discourse::Utils.execute_command('find', tombstone_dir, '-mtime', "+#{grace_period}", '-type f -delete') end def get_path_for(type, upload_id, sha, extension) diff --git a/lib/letter_avatar.rb b/lib/letter_avatar.rb index 79e1a9b884e..ffb02e188af 100644 --- a/lib/letter_avatar.rb +++ b/lib/letter_avatar.rb @@ -82,7 +82,7 @@ class LetterAvatar '#{filename}' } - `convert #{instructions.join(" ")}` + Discourse::Utils.execute_command('convert', instructions.join(" ".freeze)) ## do not optimize image, it will end up larger than original filename diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index af1174aab22..e6b8c047565 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -356,11 +356,12 @@ JS public_data = File.dirname(path) + "/public" if Dir.exists?(public_data) target = Rails.root.to_s + "/public/plugins/" - `mkdir -p #{target}` + + Discourse::Utils.execute_command('mkdir', '-p', target) target << name.gsub(/\s/,"_") # TODO a cleaner way of registering and unregistering - `rm -f #{target}` - `ln -s #{public_data} #{target}` + Discourse::Utils.execute_command('rm', '-f', target) + Discourse::Utils.execute_command('ln', '-s', public_data, target) end end