From 8a82ceb3bc19c37eadb79111f88257d77339a308 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Mon, 27 Jan 2020 03:31:11 +0100 Subject: [PATCH] FIX: Improve downsize_uploads (#8409) With this change the script: * Actually removes original large-sized images * Doesn't save processed files if their size has increased * Prevents inconsistent state --- .../javascripts/discourse/lib/lock-on.js.es6 | 2 +- app/jobs/regular/process_post.rb | 2 +- script/downsize_uploads.rb | 203 ++++++++++++------ 3 files changed, 141 insertions(+), 66 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/lock-on.js.es6 b/app/assets/javascripts/discourse/lib/lock-on.js.es6 index 13720bfeece..90cc641dfed 100644 --- a/app/assets/javascripts/discourse/lib/lock-on.js.es6 +++ b/app/assets/javascripts/discourse/lib/lock-on.js.es6 @@ -72,7 +72,7 @@ export default class LockOn { previousTop = top; } - // Commit suicide after a little while + // Stop after a little while if (new Date().getTime() - startedAt > LOCK_DURATION_MS) { return this.clearLock(interval); } diff --git a/app/jobs/regular/process_post.rb b/app/jobs/regular/process_post.rb index 266072de015..20aa499e228 100644 --- a/app/jobs/regular/process_post.rb +++ b/app/jobs/regular/process_post.rb @@ -30,7 +30,7 @@ module Jobs if cooked != (recooked || orig_cooked) if orig_cooked.present? && cooked.blank? - # TODO suicide if needed, let's gather a few here first + # TODO stop/restart the worker if needed, let's gather a few here first Rails.logger.warn("Cooked post processor in FATAL state, bypassing. You need to urgently restart sidekiq\norig: #{orig_cooked}\nrecooked: #{recooked}\ncooked: #{cooked}\npost id: #{post.id}") else post.update_column(:cooked, cp.html) diff --git a/script/downsize_uploads.rb b/script/downsize_uploads.rb index a9983f80b9c..95d65c24946 100644 --- a/script/downsize_uploads.rb +++ b/script/downsize_uploads.rb @@ -5,71 +5,33 @@ require File.expand_path("../../config/environment", __FILE__) # no less than 1 megapixel max_image_pixels = [ARGV[0].to_i, 1_000_000].max -puts "", "Fixing all images dimensions in the database", "" - -count = 0 - -Upload - .where("LOWER(extension) IN ('jpg', 'jpeg', 'gif', 'png')") - .where("COALESCE(width, 0) = 0 OR COALESCE(height, 0) = 0 OR COALESCE(thumbnail_width, 0) = 0 OR COALESCE(thumbnail_height, 0) = 0") - .find_each do |upload| - - count += 1 - print "\r%8d".freeze % count - - next unless source = upload.local? ? Discourse.store.path_for(upload) : "https:#{upload.url}" - - w, h = FastImage.size(source) - ww, hh = ImageSizer.resize(w, h) - - next if w == 0 || h == 0 || ww == 0 || hh == 0 - - upload.update!( - width: w, - height: h, - thumbnail_width: ww, - thumbnail_height: hh, - ) -end - puts "", "Downsizing images to no more than #{max_image_pixels} pixels" -count = 0 +dimensions_count = 0 +downsized_count = 0 -Upload - .where("LOWER(extension) IN ('jpg', 'jpeg', 'gif', 'png')") - .where("width * height > ?", max_image_pixels) - .find_each do |upload| - - count += 1 - print "\r%8d".freeze % count - - next unless source = upload.local? ? Discourse.store.path_for(upload) : "https:#{upload.url}" - next unless size = (FastImage.size(source) rescue nil) - - if size.reduce(:*) < max_image_pixels - ww, hh = ImageSizer.resize(*size) - - upload.update!( - width: size[0], - height: size[1], - thumbnail_width: ww, - thumbnail_height: hh, - ) - - next - end - - next unless path = upload.local? ? source : (Discourse.store.download(upload) rescue nil)&.path +def downsize_upload(upload, path, max_image_pixels) + # Make sure the filesize is up to date + upload.filesize = File.size(path) OptimizedImage.downsize(path, path, "#{max_image_pixels}@", filename: upload.original_filename) - - previous_short_url = upload.short_url - sha1 = Upload.generate_digest(path) - w, h = FastImage.size(path) - ww, hh = ImageSizer.resize(w, h) + if sha1 == upload.sha1 + puts "no sha1 change" if ENV["VERBOSE"] + return + end + + w, h = FastImage.size(path, timeout: 10, raise_on_failure: true) + + if !w || !h + puts "invalid image dimensions after resizing" if ENV["VERBOSE"] + return + end + + # Neither #dup or #clone provide a complete copy + original_upload = Upload.find(upload.id) + ww, hh = ImageSizer.resize(w, h) new_file = true if existing_upload = Upload.find_by(sha1: sha1) @@ -77,23 +39,136 @@ Upload new_file = false end + before = upload.filesize upload.filesize = File.size(path) + + if upload.filesize > before + puts "no filesize reduction" if ENV["VERBOSE"] + return + end + upload.sha1 = sha1 upload.width = w upload.height = h upload.thumbnail_width = ww upload.thumbnail_height = hh - next unless upload.save! if new_file - next unless url = Discourse.store.store_upload(File.new(path), upload) - next unless upload.update!(url: url) + url = Discourse.store.store_upload(File.new(path), upload) + + unless url + puts "couldn't store the upload" if ENV["VERBOSE"] + return + end + + upload.url = url end - upload.posts.each do |post| - post.update!(raw: post.raw.gsub(previous_short_url, upload.short_url)) if new_file - Jobs.enqueue(:process_post, post_id: post.id, bypass_bump: true, cook: true) + if ENV["VERBOSE"] + puts "base62: #{original_upload.base62_sha1} -> #{Upload.base62_sha1(sha1)}" + puts "sha1: #{original_upload.sha1} -> #{sha1}" + puts "is a new file: #{new_file}" end + + upload.save! + + if new_file + upload.optimized_images.each(&:destroy!) + Discourse.store.remove_upload(original_upload) + else + begin + PostUpload.where(upload_id: original_upload.id).update_all(upload_id: upload.id) + rescue ActiveRecord::RecordNotUnique, PG::UniqueViolation + end + + User.where(uploaded_avatar_id: original_upload.id).update_all(uploaded_avatar_id: upload.id) + UserAvatar.where(gravatar_upload_id: original_upload.id).update_all(gravatar_upload_id: upload.id) + UserAvatar.where(custom_upload_id: original_upload.id).update_all(custom_upload_id: upload.id) + UserProfile.where(profile_background_upload_id: original_upload.id).update_all(profile_background_upload_id: upload.id) + UserProfile.where(card_background_upload_id: original_upload.id).update_all(card_background_upload_id: upload.id) + Category.where(uploaded_logo_id: original_upload.id).update_all(uploaded_logo_id: upload.id) + Category.where(uploaded_background_id: original_upload.id).update_all(uploaded_background_id: upload.id) + CustomEmoji.where(upload_id: original_upload.id).update_all(upload_id: upload.id) + ThemeField.where(upload_id: original_upload.id).update_all(upload_id: upload.id) + end + + original_upload.posts.each do |post| + post.raw.gsub!(/upload:\/\/#{original_upload.base62_sha1}(\.#{original_upload.extension})?/, upload.short_url) + post.raw.gsub!(Discourse.store.cdn_url(original_upload.url), Discourse.store.cdn_url(upload.url)) + + if post.raw_changed? + puts "updating post #{post.id}" if ENV["VERBOSE"] + post.save! + else + puts "Could find the upload path in post.raw (post_id: #{post.id})" if ENV["VERBOSE"] + end + + post.rebake! + end + + original_upload.reload.destroy! unless new_file + + puts "" if ENV["VERBOSE"] + + true +end + +scope = Upload + .where("LOWER(extension) IN ('jpg', 'jpeg', 'gif', 'png')") + .where("COALESCE(width, 0) = 0 OR COALESCE(height, 0) = 0 OR COALESCE(thumbnail_width, 0) = 0 OR COALESCE(thumbnail_height, 0) = 0 OR width * height > ?", max_image_pixels) + +puts "Uploads to process: #{scope.count}" + +scope.find_each do |upload| + print "\rFixed dimensions: %8d Downsized: %8d (upload id: #{upload.id})".freeze % [dimensions_count, downsized_count] + puts "\n" if ENV["VERBOSE"] + + source = upload.local? ? Discourse.store.path_for(upload) : "https:#{upload.url}" + + unless source + puts "no path or URL" if ENV["VERBOSE"] + next + end + + w, h = FastImage.size(source, timeout: 10) + + if !w || !h + puts "invalid image dimensions" if ENV["VERBOSE"] + next + end + + ww, hh = ImageSizer.resize(w, h) + + if w == 0 || h == 0 || ww == 0 || hh == 0 + puts "invalid image dimensions" if ENV["VERBOSE"] + next + end + + if upload.read_attribute(:width) != w || upload.read_attribute(:height) != h || upload.read_attribute(:thumbnail_width) != ww || upload.read_attribute(:thumbnail_height) != hh + puts "Correcting the upload dimensions" if ENV["VERBOSE"] + dimensions_count += 1 + + upload.update!( + width: w, + height: h, + thumbnail_width: ww, + thumbnail_height: hh, + ) + end + + if w * h < max_image_pixels + puts "image size within allowed range" if ENV["VERBOSE"] + next + end + + path = upload.local? ? source : (Discourse.store.download(upload) rescue nil)&.path + + unless path + puts "no image path" if ENV["VERBOSE"] + next + end + + downsized_count += 1 if downsize_upload(upload, path, max_image_pixels) end puts "", "Done"