diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 87e4acad282..70b4a94ee32 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -5,13 +5,21 @@ module Jobs def execute(args) return unless SiteSetting.clean_up_uploads? + base_url = Discourse.store.internal? ? Discourse.store.relative_base_url : Discourse.store.absolute_base_url + s3_hostname = URI.parse(base_url).hostname + s3_cdn_hostname = URI.parse(SiteSetting.s3_cdn_url || "").hostname + # Any URLs in site settings are fair game ignore_urls = [ SiteSetting.logo_url, SiteSetting.logo_small_url, SiteSetting.favicon_url, - SiteSetting.apple_touch_icon_url - ] + SiteSetting.apple_touch_icon_url, + ].map do |url| + url = url.dup + url.gsub!(s3_cdn_hostname, s3_hostname) if s3_cdn_hostname.present? + url[base_url] && url[url.index(base_url)..-1] + end.compact.uniq grace_period = [SiteSetting.clean_orphan_uploads_grace_period_hours, 1].max @@ -30,7 +38,8 @@ module Jobs .where("up.profile_background IS NULL AND up.card_background IS NULL") .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL") .where("ce.upload_id IS NULL AND tf.upload_id IS NULL") - .where("uploads.url NOT IN (?)", ignore_urls) + + result = result.where("uploads.url NOT IN (?)", ignore_urls) if ignore_urls.present? result.find_each do |upload| next if QueuedPost.where("raw LIKE '%#{upload.sha1}%'").exists? diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index e9f649cbe16..d04a9eda8bd 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -33,6 +33,18 @@ describe Jobs::CleanUpUploads do expect(Upload.find_by(id: logo_upload.id)).to eq(logo_upload) end + it "does not clean up uploads in site settings when they use the CDN" do + Discourse.stubs(:asset_host).returns("//my.awesome.cdn") + + logo_small_upload = fabricate_upload + SiteSetting.logo_small_url = "#{Discourse.asset_host}#{logo_small_upload.url}" + + Jobs::CleanUpUploads.new.execute(nil) + + expect(Upload.find_by(id: @upload.id)).to eq(nil) + expect(Upload.find_by(id: logo_small_upload.id)).to eq(logo_small_upload) + end + it "does not delete profile background uploads" do profile_background_upload = fabricate_upload UserProfile.last.update_attributes!(profile_background: profile_background_upload.url)