diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 7b3ee54f075..55223318d5e 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 0f1902c21be..e660073eef3 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 483a325ae48..45817669d11 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) - 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) else upload.optimized_images.each(&:destroy!) end diff --git a/script/downsize_uploads.rb b/script/downsize_uploads.rb index 7fcb1e64313..ce7adc2efda 100644 --- a/script/downsize_uploads.rb +++ b/script/downsize_uploads.rb @@ -35,7 +35,11 @@ def process_uploads dimensions_count = 0 downsized_count = 0 - scope = Upload.by_users.where("LOWER(extension) IN ('jpg', 'jpeg', 'gif', 'png')") + scope = Upload + .by_users + .with_no_non_post_relations + .where("LOWER(extension) IN ('jpg', 'jpeg', 'gif', 'png')") + scope = scope.where(<<-SQL, MAX_IMAGE_PIXELS) COALESCE(width, 0) = 0 OR COALESCE(height, 0) = 0 OR diff --git a/spec/lib/shrink_uploaded_image_spec.rb b/spec/lib/shrink_uploaded_image_spec.rb index d343d639391..b5cb80a4eab 100644 --- a/spec/lib/shrink_uploaded_image_spec.rb +++ b/spec/lib/shrink_uploaded_image_spec.rb @@ -66,6 +66,18 @@ describe ShrinkUploadedImage do expect(result).to be(false) end + + it "returns false when the upload is not used in any posts" do + Fabricate(:user, uploaded_avatar: upload) + + result = ShrinkUploadedImage.new( + upload: upload, + path: Discourse.store.path_for(upload), + max_pixels: 10_000 + ).perform + + expect(result).to be(false) + end end context "when S3 uploads are enabled" do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 9071bac3fa1..a628d2e7056 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -3,9 +3,7 @@ require 'rails_helper' describe Upload do - let(:upload) { build(:upload) } - let(:user_id) { 1 } let(:image_filename) { "logo.png" } @@ -20,8 +18,34 @@ describe Upload do let(:attachment_path) { __FILE__ } let(:attachment) { File.new(attachment_path) } - context ".create_thumbnail!" do + describe '.with_no_non_post_relations' do + it "does not find non-post related uploads" do + post_upload = Fabricate(:upload) + post = Fabricate(:post, raw: "") + post.link_post_uploads + badge_upload = Fabricate(:upload) + Fabricate(:badge, image_upload: badge_upload) + + avatar_upload = Fabricate(:upload) + Fabricate(:user, uploaded_avatar: avatar_upload) + + site_setting_upload = Fabricate(:upload) + SiteSetting.create!( + name: "logo", + data_type: SiteSettings::TypeSupervisor.types[:upload], + value: site_setting_upload.id + ) + + upload_ids = Upload + .by_users + .with_no_non_post_relations + .pluck(:id) + expect(upload_ids).to eq([post_upload.id]) + end + end + + context ".create_thumbnail!" do it "does not create a thumbnail when disabled" do SiteSetting.create_thumbnails = false OptimizedImage.expects(:create_for).never