DEV: Improve `script/downsize_uploads.rb` (#13508)

DEV: Improve script/downsize_uploads.rb (#13508)

  • Only shrink images that are used in Posts and no other models
  • Don’t save the upload if the size is the same
diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb
index 7b3ee54..5522331 100644
--- a/app/jobs/scheduled/clean_up_uploads.rb
+++ b/app/jobs/scheduled/clean_up_uploads.rb
@@ -29,36 +29,9 @@ module Jobs
         .where("uploads.retain_hours IS NULL OR uploads.created_at < current_timestamp - interval '1 hour' * uploads.retain_hours")
         .where("uploads.created_at < ?", grace_period.hour.ago)
         .where("uploads.access_control_post_id IS NULL")
-        .joins(<<~SQL)
-          LEFT JOIN site_settings ss
-          ON NULLIF(ss.value, '')::integer = uploads.id
-          AND ss.data_type = #{SiteSettings::TypeSupervisor.types[:upload].to_i}
-        SQL
         .joins("LEFT JOIN post_uploads pu ON pu.upload_id = uploads.id")
-        .joins("LEFT JOIN users u ON u.uploaded_avatar_id = uploads.id")
-        .joins("LEFT JOIN user_avatars ua ON ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id")
-        .joins("LEFT JOIN user_profiles up ON up.profile_background_upload_id = uploads.id OR up.card_background_upload_id = uploads.id")
-        .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id")
-        .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id")
-        .joins("LEFT JOIN theme_fields tf ON tf.upload_id = uploads.id")
-        .joins("LEFT JOIN user_exports ue ON ue.upload_id = uploads.id")
-        .joins("LEFT JOIN groups g ON g.flair_upload_id = uploads.id")
-        .joins("LEFT JOIN badges b ON b.image_upload_id = uploads.id")
         .where("pu.upload_id IS NULL")
-        .where("u.uploaded_avatar_id IS NULL")
-        .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL")
-        .where("up.profile_background_upload_id IS NULL AND up.card_background_upload_id IS NULL")
-        .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL")
-        .where("ce.upload_id IS NULL")
-        .where("tf.upload_id IS NULL")
-        .where("ue.upload_id IS NULL")
-        .where("g.flair_upload_id IS NULL")
-        .where("b.image_upload_id IS NULL")
-        .where("ss.value IS NULL")
-
-      if SiteSetting.selectable_avatars.present?
-        result = result.where.not(id: SiteSetting.selectable_avatars.map(&:id))
-      end
+        .with_no_non_post_relations
 
       result.find_each do |upload|
         if upload.sha1.present?
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 0f1902c..e660073 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -64,6 +64,40 @@ class Upload < ActiveRecord::Base
     )
   end
 
+  def self.with_no_non_post_relations
+    scope = self
+      .joins(<<~SQL)
+        LEFT JOIN site_settings ss
+        ON NULLIF(ss.value, '')::integer = uploads.id
+        AND ss.data_type = #{SiteSettings::TypeSupervisor.types[:upload].to_i}
+      SQL
+      .where("ss.value IS NULL")
+      .joins("LEFT JOIN users u ON u.uploaded_avatar_id = uploads.id")
+      .where("u.uploaded_avatar_id IS NULL")
+      .joins("LEFT JOIN user_avatars ua ON ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id")
+      .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL")
+      .joins("LEFT JOIN user_profiles up ON up.profile_background_upload_id = uploads.id OR up.card_background_upload_id = uploads.id")
+      .where("up.profile_background_upload_id IS NULL AND up.card_background_upload_id IS NULL")
+      .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id")
+      .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL")
+      .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id")
+      .where("ce.upload_id IS NULL")
+      .joins("LEFT JOIN theme_fields tf ON tf.upload_id = uploads.id")
+      .where("tf.upload_id IS NULL")
+      .joins("LEFT JOIN user_exports ue ON ue.upload_id = uploads.id")
+      .where("ue.upload_id IS NULL")
+      .joins("LEFT JOIN groups g ON g.flair_upload_id = uploads.id")
+      .where("g.flair_upload_id IS NULL")
+      .joins("LEFT JOIN badges b ON b.image_upload_id = uploads.id")
+      .where("b.image_upload_id IS NULL")
+
+    if SiteSetting.selectable_avatars.present?
+      scope = scope.where.not(id: SiteSetting.selectable_avatars.map(&:id))
+    end
+
+    scope
+  end
+
   def to_s
     self.url
   end
diff --git a/lib/shrink_uploaded_image.rb b/lib/shrink_uploaded_image.rb
index 483a325..4581766 100644
--- a/lib/shrink_uploaded_image.rb
+++ b/lib/shrink_uploaded_image.rb
@@ -12,6 +12,20 @@ class ShrinkUploadedImage
   end
 
   def perform
+    # Neither #dup or #clone provide a complete copy
+    original_upload = Upload.find_by(id: upload.id)
+    unless original_upload
+      log "Upload is missing"
+      return false
+    end
+
+    posts = Post.unscoped.joins(:post_uploads).where(post_uploads: { upload_id: original_upload.id }).uniq.sort_by(&:created_at)
+
+    if posts.empty?
+      log "Upload not used in any posts"
+      return false
+    end
+
     OptimizedImage.downsize(path, path, "#{@max_pixels}@", filename: upload.original_filename)
     sha1 = Upload.generate_digest(path)
 
@@ -27,13 +41,6 @@ class ShrinkUploadedImage
       return false
     end
 
-    # Neither #dup or #clone provide a complete copy
-    original_upload = Upload.find_by(id: upload.id)
-    unless original_upload
-      log "Upload is missing"
-      return false
-    end
-
     ww, hh = ImageSizer.resize(w, h)
 
     # A different upload record that matches the sha1 of the downsized image
@@ -49,7 +56,7 @@ class ShrinkUploadedImage
       filesize: File.size(path)
     }
 
-    if upload.filesize > upload.filesize_was
+    if upload.filesize >= upload.filesize_was
       log "No filesize reduction"
       return false
     end
@@ -70,7 +77,6 @@ class ShrinkUploadedImage
     log "(an existing upload)" if existing_upload
 
     success = true
-    posts = Post.unscoped.joins(:post_uploads).where(post_uploads: { upload_id: original_upload.id }).uniq.sort_by(&:created_at)
 
     posts.each do |post|
       transform_post(post, original_upload, upload)
@@ -99,32 +105,6 @@ class ShrinkUploadedImage
       log "#{Discourse.base_url}/p/#{post.id}"
     end
 
-    if posts.empty?
-      log "Upload not used in any posts"
-
-      if User.where(uploaded_avatar_id: original_upload.id).exists?
-        log "Used as a User avatar"
-      elsif UserAvatar.where(gravatar_upload_id: original_upload.id).exists?
-        log "Used as a UserAvatar gravatar"
-      elsif UserAvatar.where(custom_upload_id: original_upload.id).exists?
-        log "Used as a UserAvatar custom upload"
-      elsif UserProfile.where(profile_background_upload_id: original_upload.id).exists?
-        log "Used as a UserProfile profile background"
-      elsif UserProfile.where(card_background_upload_id: original_upload.id).exists?
-        log "Used as a UserProfile card background"
-      elsif Category.where(uploaded_logo_id: original_upload.id).exists?
-        log "Used as a Category logo"
-      elsif Category.where(uploaded_background_id: original_upload.id).exists?
-        log "Used as a Category background"
-      elsif CustomEmoji.where(upload_id: original_upload.id).exists?
-        log "Used as a CustomEmoji"
-      elsif ThemeField.where(upload_id: original_upload.id).exists?
-        log "Used as a ThemeField"
-      else
-        success = false
-      end
-    end
-
     unless success
       if @interactive
         print "Press any key to continue with the upload"
@@ -157,16 +137,6 @@ class ShrinkUploadedImage
         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)

[... diff too long, it was truncated ...]

GitHub sha: 046a875222405a3fd312cff6cf9adbb1393c279d

This commit appears in #13508 which was approved by eviltrout. It was merged by CvX.