diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index 6c6ea76f87a..111c8b5ce19 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -111,37 +111,39 @@ class S3Helper end destination = get_path_for_s3_upload(destination) - if !Rails.configuration.multisite - options[:copy_source] = File.join(@s3_bucket_name, source) + source_object = if !Rails.configuration.multisite || source.include?(multisite_upload_path) || source.include?(@tombstone_prefix) + s3_bucket.object(source) + elsif @s3_bucket_folder_path + folder, filename = source.split("/", 2) + s3_bucket.object(File.join(folder, multisite_upload_path, filename)) else - if source.include?(multisite_upload_path) || source.include?(@tombstone_prefix) - options[:copy_source] = File.join(@s3_bucket_name, source) - elsif @s3_bucket_folder_path - folder, filename = begin - source.split("/", 2) - end - options[:copy_source] = File.join(@s3_bucket_name, folder, multisite_upload_path, filename) - else - options[:copy_source] = File.join(@s3_bucket_name, multisite_upload_path, source) - end + s3_bucket.object(File.join(multisite_upload_path, source)) + end + + if source_object.size > FIFTEEN_MEGABYTES + options[:multipart_copy] = true + options[:content_length] = source_object.size end destination_object = s3_bucket.object(destination) - # TODO: copy_source is a legacy option here and may become unsupported - # in later versions, we should change to use Aws::S3::Client#copy_object - # at some point. - # - # See https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-s3/lib/aws-sdk-s3/customizations/object.rb#L67-L74 - # - # ---- - # - # Also note, any options for metadata (e.g. content_disposition, content_type) - # will not be applied unless the metadata_directive = "REPLACE" option is passed - # in. If this is not passed in, the source object's metadata will be used. - response = destination_object.copy_from(options) + # Note for small files that do not use multipart copy: Any options for metadata + # (e.g. content_disposition, content_type) will not be applied unless the + # metadata_directive = "REPLACE" option is passed in. If this is not passed in, + # the source object's metadata will be used. + # For larger files it copies the metadata from the source file and merges it + # with values from the copy call. + response = destination_object.copy_from(source_object, options) - [destination, response.copy_object_result.etag.gsub('"', '')] + etag = if response.respond_to?(:copy_object_result) + # small files, regular copy + response.copy_object_result.etag + else + # larger files, multipart copy + response.data.etag + end + + [destination, etag.gsub('"', '')] end # Several places in the application need certain CORS rules to exist diff --git a/spec/lib/s3_helper_spec.rb b/spec/lib/s3_helper_spec.rb index f225c3bda09..f7beacf590e 100644 --- a/spec/lib/s3_helper_spec.rb +++ b/spec/lib/s3_helper_spec.rb @@ -100,26 +100,55 @@ describe "S3Helper" do let(:destination_key) { "original/1X/destination.jpg" } let(:s3_helper) { S3Helper.new("test-bucket", "", client: client) } - it "can copy an object from the source to the destination" do + it "can copy a small object from the source to the destination" do + source_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: source_key, client: client) + source_stub.stubs(:size).returns(5 * 1024 * 1024) + s3_helper.send(:s3_bucket).expects(:object).with(source_key).returns(source_stub) + destination_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: destination_key, client: client) s3_helper.send(:s3_bucket).expects(:object).with(destination_key).returns(destination_stub) - destination_stub.expects(:copy_from).with(copy_source: "test-bucket/#{source_key}").returns( - stub(copy_object_result: stub(etag: "\"etag\"")) + + destination_stub.expects(:copy_from).with(source_stub, {}).returns( + stub(copy_object_result: stub(etag: '"etag"')) ) + + response = s3_helper.copy(source_key, destination_key) + expect(response.first).to eq(destination_key) + expect(response.second).to eq("etag") + end + + it "can copy a large object from the source to the destination" do + source_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: source_key, client: client) + source_stub.stubs(:size).returns(20 * 1024 * 1024) + s3_helper.send(:s3_bucket).expects(:object).with(source_key).returns(source_stub) + + destination_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: destination_key, client: client) + s3_helper.send(:s3_bucket).expects(:object).with(destination_key).returns(destination_stub) + + options = { multipart_copy: true, content_length: source_stub.size } + destination_stub.expects(:copy_from).with(source_stub, options).returns( + stub(data: stub(etag: '"etag"')) + ) + response = s3_helper.copy(source_key, destination_key) expect(response.first).to eq(destination_key) expect(response.second).to eq("etag") end it "puts the metadata from options onto the destination if apply_metadata_to_destination" do - content_disposition = "attachment; filename=\"source.jpg\"; filename*=UTF-8''source.jpg" + source_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: source_key, client: client) + source_stub.stubs(:size).returns(5 * 1024 * 1024) + s3_helper.send(:s3_bucket).expects(:object).with(source_key).returns(source_stub) + destination_stub = Aws::S3::Object.new(bucket_name: "test-bucket", key: destination_key, client: client) s3_helper.send(:s3_bucket).expects(:object).with(destination_key).returns(destination_stub) - destination_stub.expects(:copy_from).with( - copy_source: "test-bucket/#{source_key}", content_disposition: content_disposition, metadata_directive: "REPLACE" - ).returns( - stub(copy_object_result: stub(etag: "\"etag\"")) + + content_disposition = "attachment; filename=\"source.jpg\"; filename*=UTF-8''source.jpg" + options = { content_disposition: content_disposition, metadata_directive: "REPLACE" } + destination_stub.expects(:copy_from).with(source_stub, options).returns( + stub(data: stub(etag: '"etag"')) ) + response = s3_helper.copy( source_key, destination_key, options: { apply_metadata_to_destination: true, content_disposition: content_disposition }