FIX: Restructure temp/ folders for direct S3 uploads (#14137)

Previously we had temp/ in the middle of the S3 key path like so

* /uploads/default/temp/randomstring/test.png (normal site)
* /sitename/uploads/default/temp/randomstring/test.png (s3 folder path site)
* /standard10/uploads/sitename/temp/randomstring/test.png (multisite site)

However this necessitates making a lifecycle rule to clean up incomplete
S3 multipart uploads for every site, something which we cannot do. It makes
much more sense to have a structure with /temp at the start of the key,
which is what this commit does:

* /temp/uploads/default/randomstring/test.png (normal site)
* /temp/sitename/uploads/default/randomstring/test.png (s3 folder path site)
* /temp/standard10/uploads/sitename/randomstring/test.png (multisite site)
This commit is contained in:
Martin Brennan 2021-08-25 09:22:36 +10:00 committed by GitHub
parent d295a16dab
commit e0102a533a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 7 additions and 7 deletions

View File

@ -41,10 +41,11 @@ module FileStore
File.join(path, "test_#{ENV['TEST_ENV_NUMBER'].presence || '0'}") File.join(path, "test_#{ENV['TEST_ENV_NUMBER'].presence || '0'}")
end end
def temporary_upload_path(file_name) def temporary_upload_path(file_name, folder_prefix: "")
File.join( File.join(
upload_path,
TEMPORARY_UPLOAD_PREFIX, TEMPORARY_UPLOAD_PREFIX,
folder_prefix,
upload_path,
SecureRandom.hex, SecureRandom.hex,
file_name file_name
) )

View File

@ -236,8 +236,7 @@ module FileStore
end end
def temporary_upload_path(file_name) def temporary_upload_path(file_name)
path = super(file_name) s3_bucket_folder_path.nil? ? super(file_name) : super(file_name, folder_prefix: s3_bucket_folder_path)
s3_bucket_folder_path.nil? ? path : File.join(s3_bucket_folder_path, path)
end end
def object_from_path(path) def object_from_path(path)

View File

@ -313,7 +313,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
url = store.signed_url_for_temporary_upload("test.png") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) expect(url).to match(/Amz-Expires/)
expect(key).to match(/uploads\/default\/test_[0-9]\/temp\/[a-zA-z0-9]{0,32}\/test.png/) expect(key).to match(/temp\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
end end
it "presigned url contans the metadata when provided" do it "presigned url contans the metadata when provided" do
@ -329,7 +329,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
url = store.signed_url_for_temporary_upload("test.png") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) expect(url).to match(/Amz-Expires/)
expect(key).to match(/site\/uploads\/default\/test_[0-9]\/temp\/[a-zA-z0-9]{0,32}\/test.png/) expect(key).to match(/temp\/site\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
end end
end end
@ -341,7 +341,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
url = store.signed_url_for_temporary_upload("test.png") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) expect(url).to match(/Amz-Expires/)
expect(key).to match(/standard99\/uploads\/second\/test_[0-9]\/temp\/[a-zA-z0-9]{0,32}\/test.png/) expect(key).to match(/temp\/standard99\/uploads\/second\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
end end
end end
end end