diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 83932da1985..da309f34006 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -12,7 +12,9 @@ module FileStore attr_reader :s3_helper def initialize(s3_helper = nil) - @s3_helper = s3_helper || S3Helper.new(s3_bucket, TOMBSTONE_PREFIX) + @s3_helper = s3_helper || S3Helper.new(s3_bucket, + Rails.configuration.multisite ? multisite_tombstone_prefix : TOMBSTONE_PREFIX + ) end def store_upload(file, upload, content_type = nil) @@ -87,6 +89,10 @@ module FileStore @s3_helper.update_tombstone_lifecycle(grace_period) end + def multisite_tombstone_prefix + File.join("uploads", "tombstone", RailsMultisite::ConnectionManagement.current_db, "/") + end + def path_for(upload) url = upload.try(:url) FileStore::LocalStore.new.path_for(upload) if url && url[/^\/[^\/]/] diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index f12497bc1ae..c3dad0694c0 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -39,14 +39,27 @@ class S3Helper end # delete the file + s3_filename.prepend(multisite_upload_path) if Rails.configuration.multisite s3_bucket.object(get_path_for_s3_upload(s3_filename)).delete rescue Aws::S3::Errors::NoSuchKey end def copy(source, destination, options: {}) + if !Rails.configuration.multisite + options[:copy_source] = File.join(@s3_bucket_name, source) + else + if @s3_bucket_folder_path + bucket_folder, filename = begin + source.split("/".freeze, 2) + end + options[:copy_source] = File.join(@s3_bucket_name, bucket_folder, multisite_upload_path, filename) + else + options[:copy_source] = File.join(@s3_bucket_name, multisite_upload_path, source) + end + end s3_bucket .object(destination) - .copy_from(options.merge(copy_source: File.join(@s3_bucket_name, source))) + .copy_from(options) end # make sure we have a cors config for assets @@ -194,6 +207,10 @@ class S3Helper path end + def multisite_upload_path + File.join("uploads", RailsMultisite::ConnectionManagement.current_db, "/") + end + def s3_resource Aws::S3::Resource.new(@s3_options) end diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb index 2c27a8169bd..6c023d15b69 100644 --- a/spec/multisite/s3_store_spec.rb +++ b/spec/multisite/s3_store_spec.rb @@ -4,18 +4,7 @@ require 'file_store/s3_store' RSpec.describe 'Multisite s3 uploads', type: :multisite do let(:conn) { RailsMultisite::ConnectionManagement } let(:uploaded_file) { file_from_fixtures("smallest.png") } - - let(:upload) do - Fabricate(:upload, sha1: Digest::SHA1.hexdigest(File.read(uploaded_file))) - end - - let(:s3_client) { Aws::S3::Client.new(stub_responses: true) } - - let(:s3_helper) do - S3Helper.new(SiteSetting.s3_upload_bucket, '', client: s3_client) - end - - let(:store) { FileStore::S3Store.new(s3_helper) } + let(:upload) { Fabricate(:upload, sha1: Digest::SHA1.hexdigest(File.read(uploaded_file))) } context 'uploading to s3' do before(:each) do @@ -26,6 +15,10 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do end describe "#store_upload" do + let(:s3_client) { Aws::S3::Client.new(stub_responses: true) } + let(:s3_helper) { S3Helper.new(SiteSetting.s3_upload_bucket, '', client: s3_client) } + let(:store) { FileStore::S3Store.new(s3_helper) } + it "returns the correct url for default and second multisite db" do conn.with_connection('default') do expect(store.store_upload(uploaded_file, upload)).to eq( @@ -41,4 +34,76 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do end end end + + context 'removal from s3' do + before do + SiteSetting.s3_region = 'us-west-1' + SiteSetting.s3_upload_bucket = "s3-upload-bucket" + SiteSetting.s3_access_key_id = "s3-access-key-id" + SiteSetting.s3_secret_access_key = "s3-secret-access-key" + SiteSetting.enable_s3_uploads = true + end + + describe "#remove_upload" do + let(:store) { FileStore::S3Store.new } + let(:client) { Aws::S3::Client.new(stub_responses: true) } + let(:resource) { Aws::S3::Resource.new(client: client) } + let(:s3_bucket) { resource.bucket(SiteSetting.s3_upload_bucket) } + let(:s3_helper) { store.s3_helper } + + it "removes the file from s3 on multisite" do + conn.with_connection('default') do + store.expects(:get_depth_for).with(upload.id).returns(0) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once + upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/default/original/1X/#{upload.sha1}.png") + s3_object = stub + + s3_bucket.expects(:object).with("uploads/tombstone/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/uploads/default/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("uploads/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:delete) + + store.remove_upload(upload) + end + end + + it "removes the file from s3 on another multisite db" do + conn.with_connection('second') do + store.expects(:get_depth_for).with(upload.id).returns(0) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once + upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/second/original/1X/#{upload.sha1}.png") + s3_object = stub + + s3_bucket.expects(:object).with("uploads/tombstone/second/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/uploads/second/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("uploads/second/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:delete) + + store.remove_upload(upload) + end + end + + describe "when s3_upload_bucket includes folders path" do + before do + SiteSetting.s3_upload_bucket = "s3-upload-bucket/discourse-uploads" + end + + it "removes the file from s3 on multisite" do + conn.with_connection('default') do + store.expects(:get_depth_for).with(upload.id).returns(0) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once + upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png") + s3_object = stub + + s3_bucket.expects(:object).with("discourse-uploads/uploads/tombstone/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:delete) + + store.remove_upload(upload) + end + end + end + end + end end