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:
parent
d295a16dab
commit
e0102a533a
|
@ -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
|
||||||
)
|
)
|
||||||
|
|
|
@ -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)
|
||||||
|
|
|
@ -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
|
||||||
|
|
Loading…
Reference in New Issue