From aafff740d2abb628fed0bc848a6b414527ad45ab Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 8 Aug 2018 11:26:05 +0800 Subject: [PATCH] Add `FileStore::S3Store#copy_file`. --- lib/file_store/s3_store.rb | 5 +++ lib/s3_helper.rb | 25 +++++++++----- spec/components/file_store/s3_store_spec.rb | 37 ++++++++++++++++++--- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 483b41e02f0..076ea2171ab 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -50,6 +50,11 @@ module FileStore @s3_helper.remove(path, true) end + def copy_file(url, source, destination) + return unless has_been_uploaded?(url) + @s3_helper.copy(source, destination) + end + def has_been_uploaded?(url) return false if url.blank? diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index 9e1ae3ec4e8..61d58559544 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -30,20 +30,25 @@ class S3Helper end def remove(s3_filename, copy_to_tombstone = false) - bucket = s3_bucket - # copy the file in tombstone if copy_to_tombstone && @tombstone_prefix.present? - bucket - .object(File.join(@tombstone_prefix, s3_filename)) - .copy_from(copy_source: File.join(@s3_bucket_name, get_path_for_s3_upload(s3_filename))) + self.copy( + File.join(@tombstone_prefix, s3_filename), + get_path_for_s3_upload(s3_filename) + ) end # delete the file - bucket.object(get_path_for_s3_upload(s3_filename)).delete + s3_bucket.object(get_path_for_s3_upload(s3_filename)).delete rescue Aws::S3::Errors::NoSuchKey end + def copy(source, destination) + s3_bucket + .object(source) + .copy_from(copy_source: File.join(@s3_bucket_name, destination)) + end + # make sure we have a cors config for assets # otherwise we will have no fonts def ensure_cors! @@ -186,9 +191,11 @@ class S3Helper end def s3_bucket - bucket = s3_resource.bucket(@s3_bucket_name) - bucket.create unless bucket.exists? - bucket + @s3_bucket ||= begin + bucket = s3_resource.bucket(@s3_bucket_name) + bucket.create unless bucket.exists? + bucket + end end def check_missing_site_options diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index 00df4f36f91..58cecf5e00c 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -41,7 +41,7 @@ describe FileStore::S3Store do describe "#store_upload" do it "returns an absolute schemaless url" do store.expects(:get_depth_for).with(upload.id).returns(0) - s3_helper.expects(:s3_bucket).returns(s3_bucket) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once s3_object = stub s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object) @@ -109,13 +109,40 @@ describe FileStore::S3Store do end end + context 'copying files in S3' do + include_context "s3 helpers" + + describe '#copy_file' do + it "copies the from in S3 with the right paths" do + s3_helper.expects(:s3_bucket).returns(s3_bucket) + + upload.update!( + url: "//s3-upload-bucket.s3-us-west-1.amazonaws.com/original/1X/#{upload.sha1}.png" + ) + + source = Discourse.store.get_path_for_upload(upload) + destination = Discourse.store.get_path_for_upload(upload).sub('.png', '.jpg') + + s3_object = stub + + s3_bucket.expects(:object).with(source).returns(s3_object) + + s3_object.expects(:copy_from).with( + copy_source: "s3-upload-bucket/#{destination}" + ) + + store.copy_file(upload.url, source, destination) + end + end + end + context 'removal from s3' do include_context "s3 helpers" describe "#remove_upload" do it "removes the file from s3 with the right paths" do store.expects(:get_depth_for).with(upload.id).returns(0) - s3_helper.expects(:s3_bucket).returns(s3_bucket) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once upload.update_attributes!(url: "//s3-upload-bucket.s3-us-west-1.amazonaws.com/original/1X/#{upload.sha1}.png") s3_object = stub @@ -134,7 +161,7 @@ describe FileStore::S3Store do it "removes the file from s3 with the right paths" do store.expects(:get_depth_for).with(upload.id).returns(0) - s3_helper.expects(:s3_bucket).returns(s3_bucket) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once upload.update_attributes!(url: "//s3-upload-bucket.s3-us-west-1.amazonaws.com/discourse-uploads/original/1X/#{upload.sha1}.png") s3_object = stub @@ -158,7 +185,7 @@ describe FileStore::S3Store do it "removes the file from s3 with the right paths" do store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0) - s3_helper.expects(:s3_bucket).returns(s3_bucket) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once s3_object = stub s3_bucket.expects(:object).with("tombstone/optimized/1X/#{upload.sha1}_1_100x200.png").returns(s3_object) @@ -176,7 +203,7 @@ describe FileStore::S3Store do it "removes the file from s3 with the right paths" do store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0) - s3_helper.expects(:s3_bucket).returns(s3_bucket) + s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once s3_object = stub s3_bucket.expects(:object).with("discourse-uploads/tombstone/optimized/1X/#{upload.sha1}_1_100x200.png").returns(s3_object)