diff --git a/app/models/upload.rb b/app/models/upload.rb index 869e69fb582..f935e924101 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -36,6 +36,13 @@ class Upload < ActiveRecord::Base optimized_images << thumbnail if thumbnail end + def delete + Upload.transaction do + Upload.remove_file url + super + end + end + def self.create_for(user_id, file) # compute the sha sha1 = Digest::SHA1.file(file.tempfile).hexdigest @@ -74,6 +81,11 @@ class Upload < ActiveRecord::Base return LocalStore.store_file(file, sha1, image_info, upload_id) end + def self.remove_file(url) + S3.remove_file(url) if SiteSetting.enable_s3_uploads? + LocalStore.remove_file(url) + end + def self.uploaded_regex /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/ end diff --git a/lib/local_store.rb b/lib/local_store.rb index 11ae1a6ea8d..37539b1f97b 100644 --- a/lib/local_store.rb +++ b/lib/local_store.rb @@ -15,4 +15,9 @@ module LocalStore return Discourse::base_uri + "#{url_root}/#{clean_name}" end + def self.remove_file(url) + File.delete("#{Rails.root}/public#{url}") + rescue Errno::ENOENT + end + end diff --git a/lib/s3.rb b/lib/s3.rb index 7ef07e0e63e..c7c0c791efc 100644 --- a/lib/s3.rb +++ b/lib/s3.rb @@ -1,22 +1,44 @@ module S3 def self.store_file(file, sha1, image_info, upload_id) - raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? - raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank? - raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank? + S3.check_missing_site_settings - @fog_loaded = require 'fog' unless @fog_loaded + directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket) remote_filename = "#{upload_id}#{sha1}.#{image_info.type}" - options = S3.generate_options - directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket, options) # if this fails, it will throw an exception file = S3.upload(file, remote_filename, directory) - return "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/#{remote_filename}" end + def self.remove_file(url) + S3.check_missing_site_settings + + directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket) + + file = S3.destroy(url, directory) + end + + def self.check_missing_site_settings + raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? + raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank? + raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank? + end + + def self.get_or_create_directory(name) + @fog_loaded = require 'fog' unless @fog_loaded + + options = S3.generate_options + + fog = Fog::Storage.new(options) + + directory = fog.directories.get(name) + directory = fog.directories.create(key: name) unless directory + + directory + end + def self.generate_options options = { provider: 'AWS', @@ -28,14 +50,6 @@ module S3 options end - def self.get_or_create_directory(name, options) - fog = Fog::Storage.new(options) - directory = fog.directories.get(name) - directory = fog.directories.create(key: name) unless directory - - directory - end - def self.upload(file, name, directory) directory.files.create( key: name, @@ -45,4 +59,8 @@ module S3 ) end + def self.destroy(name, directory) + directory.files.destroy(key: name) + end + end diff --git a/lib/tasks/images.rake b/lib/tasks/images.rake index d1376bd3ea0..4fcac3e67e9 100644 --- a/lib/tasks/images.rake +++ b/lib/tasks/images.rake @@ -30,3 +30,22 @@ task "images:reindex" => :environment do end puts "\ndone." end + +desc "clean orphan uploaded files" +task "images:clean_orphans" => :environment do + RailsMultisite::ConnectionManagement.each_connection do |db| + puts "Cleaning up #{db}" + # ligthweight safety net to prevent users from wiping all their uploads out + if PostUpload.count == 0 && Upload.count > 0 + puts "The reverse index is empty. Make sure you run the `images:reindex` task" + next + end + Upload.joins("LEFT OUTER JOIN post_uploads ON uploads.id = post_uploads.upload_id") + .where("post_uploads.upload_id IS NULL") + .find_each do |u| + u.delete + putc "." + end + end + puts "\ndone." +end diff --git a/spec/components/local_store_spec.rb b/spec/components/local_store_spec.rb index 7421969d69a..2b79ef67666 100644 --- a/spec/components/local_store_spec.rb +++ b/spec/components/local_store_spec.rb @@ -15,7 +15,7 @@ describe LocalStore do let(:image_info) { FastImage.new(file) } - it 'returns the url of the S3 upload if successful' do + it 'returns the url of the uploaded file if successful' do # prevent the tests from creating directories & files... FileUtils.stubs(:mkdir_p) File.stubs(:open) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 68d47f72089..42e22f2fd4d 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -62,17 +62,17 @@ describe Upload do it "identifies internal or relatives urls" do Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com") - Upload.has_been_uploaded?("http://discuss.site.com/upload/1234/42/ABCD.jpg").should == true - Upload.has_been_uploaded?("/upload/42/ABCD.jpg").should == true + Upload.has_been_uploaded?("http://discuss.site.com/upload/1234/42/0123456789ABCDEF.jpg").should == true + Upload.has_been_uploaded?("/upload/42/0123456789ABCDEF.jpg").should == true end it "identifies internal urls when using a CDN" do ActionController::Base.expects(:asset_host).returns("http://my.cdn.com").twice - Upload.has_been_uploaded?("http://my.cdn.com/upload/1234/42/ABCD.jpg").should == true + Upload.has_been_uploaded?("http://my.cdn.com/upload/1234/42/0123456789ABCDEF.jpg").should == true end it "identifies external urls" do - Upload.has_been_uploaded?("http://domain.com/upload/1234/42/ABCD.jpg").should == false + Upload.has_been_uploaded?("http://domain.com/upload/1234/42/0123456789ABCDEF.jpg").should == false end end