FIX: Uploading large files (> 5GB) failed when enable_direct_s3_uploads is enabled (#16724)

Larger files require a multipart copy.
This commit is contained in:
Gerhard Schlager
2022-06-28 21:30:00 +02:00
committed by GitHub
parent 0bcc478635
commit 9d870f151c
2 changed files with 64 additions and 33 deletions

View File

@ -111,37 +111,39 @@ class S3Helper
end end
destination = get_path_for_s3_upload(destination) destination = get_path_for_s3_upload(destination)
if !Rails.configuration.multisite source_object = if !Rails.configuration.multisite || source.include?(multisite_upload_path) || source.include?(@tombstone_prefix)
options[:copy_source] = File.join(@s3_bucket_name, source) 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 else
if source.include?(multisite_upload_path) || source.include?(@tombstone_prefix) s3_bucket.object(File.join(multisite_upload_path, source))
options[:copy_source] = File.join(@s3_bucket_name, source) end
elsif @s3_bucket_folder_path
folder, filename = begin if source_object.size > FIFTEEN_MEGABYTES
source.split("/", 2) options[:multipart_copy] = true
end options[:content_length] = source_object.size
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
end end
destination_object = s3_bucket.object(destination) destination_object = s3_bucket.object(destination)
# TODO: copy_source is a legacy option here and may become unsupported # Note for small files that do not use multipart copy: Any options for metadata
# in later versions, we should change to use Aws::S3::Client#copy_object # (e.g. content_disposition, content_type) will not be applied unless the
# at some point. # metadata_directive = "REPLACE" option is passed in. If this is not passed in,
# # the source object's metadata will be used.
# See https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-s3/lib/aws-sdk-s3/customizations/object.rb#L67-L74 # 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)
#
# 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)
[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 end
# Several places in the application need certain CORS rules to exist # Several places in the application need certain CORS rules to exist

View File

@ -100,26 +100,55 @@ describe "S3Helper" do
let(:destination_key) { "original/1X/destination.jpg" } let(:destination_key) { "original/1X/destination.jpg" }
let(:s3_helper) { S3Helper.new("test-bucket", "", client: client) } 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) 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) 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) response = s3_helper.copy(source_key, destination_key)
expect(response.first).to eq(destination_key) expect(response.first).to eq(destination_key)
expect(response.second).to eq("etag") expect(response.second).to eq("etag")
end end
it "puts the metadata from options onto the destination if apply_metadata_to_destination" do 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) 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) 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" content_disposition = "attachment; filename=\"source.jpg\"; filename*=UTF-8''source.jpg"
).returns( options = { content_disposition: content_disposition, metadata_directive: "REPLACE" }
stub(copy_object_result: stub(etag: "\"etag\"")) destination_stub.expects(:copy_from).with(source_stub, options).returns(
stub(data: stub(etag: '"etag"'))
) )
response = s3_helper.copy( response = s3_helper.copy(
source_key, destination_key, source_key, destination_key,
options: { apply_metadata_to_destination: true, content_disposition: content_disposition } options: { apply_metadata_to_destination: true, content_disposition: content_disposition }