tusd: Do not delete the .info files.

These files are necessary for the protocol to verify that the file
upload was completed successfully.  Rather than delete them, we update
their StorageClass if it is non-STANDARD.
This commit is contained in:
Alex Vandiver 2024-10-02 19:16:04 +00:00 committed by Tim Abbott
parent 1ff14fd0f1
commit b4b1551b81
2 changed files with 20 additions and 10 deletions

View File

@ -1,6 +1,5 @@
import os import os
import botocore
import orjson import orjson
from django.conf import settings from django.conf import settings
from django.test import override_settings from django.test import override_settings
@ -338,9 +337,11 @@ class TusdPreFinishTest(ZulipTestCase):
self.assertEqual(attachment.size, len("zulip!")) self.assertEqual(attachment.size, len("zulip!"))
self.assertEqual(attachment.content_type, "text/plain") self.assertEqual(attachment.content_type, "text/plain")
# Assert that the .info file is still there -- tusd needs it
# to verify that the upload completed successfully
assert settings.LOCAL_FILES_DIR is not None assert settings.LOCAL_FILES_DIR is not None
self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, path_id))) self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, path_id)))
self.assertFalse(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, f"{path_id}.info"))) self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, f"{path_id}.info")))
def test_no_metadata(self) -> None: def test_no_metadata(self) -> None:
self.login("hamlet") self.login("hamlet")
@ -394,7 +395,7 @@ class TusdPreFinishTest(ZulipTestCase):
assert settings.LOCAL_FILES_DIR is not None assert settings.LOCAL_FILES_DIR is not None
self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, path_id))) self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, path_id)))
self.assertFalse(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, f"{path_id}.info"))) self.assertTrue(os.path.exists(os.path.join(settings.LOCAL_FILES_DIR, f"{path_id}.info")))
@use_s3_backend @use_s3_backend
@override_settings(S3_UPLOADS_STORAGE_CLASS="STANDARD_IA") @override_settings(S3_UPLOADS_STORAGE_CLASS="STANDARD_IA")
@ -467,5 +468,6 @@ class TusdPreFinishTest(ZulipTestCase):
response["Metadata"], response["Metadata"],
{"realm_id": str(hamlet.realm_id), "user_profile_id": str(hamlet.id)}, {"realm_id": str(hamlet.realm_id), "user_profile_id": str(hamlet.id)},
) )
with self.assertRaises(botocore.exceptions.ClientError):
bucket.Object(f"{path_id}.info").get() response = bucket.Object(f"{path_id}.info").get()
self.assertEqual(response["ContentType"], "binary/octet-stream")

View File

@ -21,7 +21,6 @@ from zerver.lib.upload import (
attachment_vips_source, attachment_vips_source,
check_upload_within_quota, check_upload_within_quota,
create_attachment, create_attachment,
delete_message_attachment,
sanitize_name, sanitize_name,
upload_backend, upload_backend,
) )
@ -167,10 +166,19 @@ def handle_upload_pre_finish_hook(
StorageClass=settings.S3_UPLOADS_STORAGE_CLASS, StorageClass=settings.S3_UPLOADS_STORAGE_CLASS,
) )
# https://tus.github.io/tusd/storage-backends/overview/#storage-format # https://tus.github.io/tusd/storage-backends/overview/#storage-format
# tusd creates a .info file next to the upload, which we do not # tusd also creates a .info file next to the upload, which
# need to keep. Clean it up. # must be preserved for HEAD requests (to check for upload
delete_message_attachment(f"{path_id}.info") # state) to work. These files are inaccessible via Zulip, and
# small enough to not pose any notable storage use; but we
# should store them with the right StorageClass.
if settings.S3_UPLOADS_STORAGE_CLASS != "STANDARD":
info_key = upload_backend.uploads_bucket.Object(path_id + ".info")
info_key.copy_from(
CopySource={"Bucket": settings.S3_AUTH_UPLOADS_BUCKET, "Key": path_id + ".info"},
MetadataDirective="COPY",
StorageClass=settings.S3_UPLOADS_STORAGE_CLASS,
)
with transaction.atomic(): with transaction.atomic():
create_attachment( create_attachment(