diff --git a/analytics/management/commands/populate_analytics_db.py b/analytics/management/commands/populate_analytics_db.py index f0fccf8f5e..ffbed95dd4 100644 --- a/analytics/management/commands/populate_analytics_db.py +++ b/analytics/management/commands/populate_analytics_db.py @@ -1,4 +1,3 @@ -import os from datetime import timedelta from typing import Any, Dict, List, Mapping, Type, Union @@ -145,10 +144,8 @@ class Command(ZulipBaseCommand): # Create an attachment in the database for set_storage_space_used_statistic. IMAGE_FILE_PATH = static_path("images/test-images/checkbox.png") - file_info = os.stat(IMAGE_FILE_PATH) - file_size = file_info.st_size with open(IMAGE_FILE_PATH, "rb") as fp: - upload_message_attachment_from_request(UploadedFile(fp), shylock, file_size) + upload_message_attachment_from_request(UploadedFile(fp), shylock) FixtureData: TypeAlias = Mapping[Union[str, int, None], List[int]] diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 8eb02518d8..6c74ad1ae5 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -327,7 +327,6 @@ def extract_and_upload_attachments(message: EmailMessage, realm: Realm, sender: if isinstance(attachment, bytes): upload_url = upload_message_attachment( filename, - len(attachment), content_type, attachment, sender, diff --git a/zerver/lib/upload/__init__.py b/zerver/lib/upload/__init__.py index f9bd7c53b3..f22cc5240a 100644 --- a/zerver/lib/upload/__init__.py +++ b/zerver/lib/upload/__init__.py @@ -124,7 +124,6 @@ def sanitize_name(value: str) -> str: def upload_message_attachment( uploaded_file_name: str, - uploaded_file_size: int, content_type: str, file_data: bytes, user_profile: UserProfile, @@ -146,7 +145,7 @@ def upload_message_attachment( path_id, user_profile, target_realm, - uploaded_file_size, + len(file_data), content_type, ) return f"/user_uploads/{path_id}" @@ -175,11 +174,11 @@ def claim_attachment( def upload_message_attachment_from_request( - user_file: UploadedFile, user_profile: UserProfile, user_file_size: int + user_file: UploadedFile, user_profile: UserProfile ) -> str: uploaded_file_name, content_type = get_file_info(user_file) return upload_message_attachment( - uploaded_file_name, user_file_size, content_type, user_file.read(), user_profile + uploaded_file_name, content_type, user_file.read(), user_profile ) diff --git a/zerver/openapi/curl_param_value_generators.py b/zerver/openapi/curl_param_value_generators.py index fb243593c6..34568cc161 100644 --- a/zerver/openapi/curl_param_value_generators.py +++ b/zerver/openapi/curl_param_value_generators.py @@ -350,9 +350,7 @@ def deactivate_own_user() -> Dict[str, object]: @openapi_param_value_generator(["/attachments/{attachment_id}:delete"]) def remove_attachment() -> Dict[str, object]: user_profile = helpers.example_user("iago") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) attachment_id = url.replace("/user_uploads/", "").split("/")[0] return {"attachment_id": attachment_id} diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index b79fb11739..bda1205f7a 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -601,7 +601,6 @@ class TestEmailMirrorMessagesWithAttachments(ZulipTestCase): process_message(incoming_valid_message) upload_message_attachment.assert_called_with( "image.png", - len(image_bytes), "image/png", image_bytes, get_system_bot(settings.EMAIL_GATEWAY_BOT, stream.realm_id), @@ -728,7 +727,6 @@ class TestEmailMirrorMessagesWithAttachments(ZulipTestCase): process_message(incoming_valid_message) upload_message_attachment.assert_called_with( utf8_filename, - len(image_bytes), "image/png", image_bytes, get_system_bot(settings.EMAIL_GATEWAY_BOT, stream.realm_id), @@ -775,7 +773,6 @@ class TestEmailMirrorMessagesWithAttachments(ZulipTestCase): process_message(incoming_valid_message) upload_message_attachment.assert_called_with( "image.png", - len(image_bytes), "image/png", image_bytes, get_system_bot(settings.EMAIL_GATEWAY_BOT, stream.realm_id), diff --git a/zerver/tests/test_import_export.py b/zerver/tests/test_import_export.py index 0127927edd..5720fb934d 100644 --- a/zerver/tests/test_import_export.py +++ b/zerver/tests/test_import_export.py @@ -164,9 +164,7 @@ class ExportFile(ZulipTestCase): self, user_profile: UserProfile, *, emoji_name: str = "whatever" ) -> None: message = most_recent_message(user_profile) - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) attachment_path_id = url.replace("/user_uploads/", "") claim_attachment( path_id=attachment_path_id, @@ -383,9 +381,7 @@ class RealmImportExportTest(ExportFile): # We create an attachment tied to a personal message. That means it shouldn't be # included in a public export, as it's private data. personal_message_id = self.send_personal_message(user_profile, self.example_user("othello")) - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) attachment_path_id = url.replace("/user_uploads/", "") attachment = claim_attachment( path_id=attachment_path_id, diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 3125a10310..5bac17b85d 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -2162,9 +2162,7 @@ class ScrubRealmTest(ZulipTestCase): path_ids = [] for n in range(1, 4): content = f"content{n}".encode() - url = upload_message_attachment( - f"dummy{n}.txt", len(content), "text/plain", content, hamlet - ) + url = upload_message_attachment(f"dummy{n}.txt", "text/plain", content, hamlet) base = "/user_uploads/" self.assertEqual(base, url[: len(base)]) path_id = re.sub(r"/user_uploads/", "", url) @@ -2239,9 +2237,7 @@ class ScrubRealmTest(ZulipTestCase): file_paths = [] for n, owner in enumerate([iago, othello, hamlet, cordelia, king]): content = f"content{n}".encode() - url = upload_message_attachment( - f"dummy{n}.txt", len(content), "text/plain", content, owner - ) + url = upload_message_attachment(f"dummy{n}.txt", "text/plain", content, owner) base = "/user_uploads/" self.assertEqual(base, url[: len(base)]) file_path = os.path.join(settings.LOCAL_FILES_DIR, re.sub(r"/user_uploads/", "", url)) diff --git a/zerver/tests/test_transfer.py b/zerver/tests/test_transfer.py index 4e957795d8..93bfec9913 100644 --- a/zerver/tests/test_transfer.py +++ b/zerver/tests/test_transfer.py @@ -67,8 +67,8 @@ class TransferUploadsToS3Test(ZulipTestCase): hamlet = self.example_user("hamlet") othello = self.example_user("othello") - upload_message_attachment("dummy1.txt", len(b"zulip1!"), "text/plain", b"zulip1!", hamlet) - upload_message_attachment("dummy2.txt", len(b"zulip2!"), "text/plain", b"zulip2!", othello) + upload_message_attachment("dummy1.txt", "text/plain", b"zulip1!", hamlet) + upload_message_attachment("dummy2.txt", "text/plain", b"zulip2!", othello) with self.assertLogs(level="INFO"): transfer_message_files_to_s3(1) diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 0e0b905ed2..069345dadb 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -1836,14 +1836,14 @@ class UploadSpaceTests(UploadSerializeMixin, ZulipTestCase): self.assertEqual(0, cache_get(get_realm_used_upload_space_cache_key(self.realm.id))[0]) data = b"zulip!" - upload_message_attachment("dummy.txt", len(data), "text/plain", data, self.user_profile) + upload_message_attachment("dummy.txt", "text/plain", data, self.user_profile) # notify_attachment_update function calls currently_used_upload_space_bytes which # updates the cache. self.assert_length(data, cache_get(get_realm_used_upload_space_cache_key(self.realm.id))[0]) self.assert_length(data, self.realm.currently_used_upload_space_bytes()) data2 = b"more-data!" - upload_message_attachment("dummy2.txt", len(data2), "text/plain", data2, self.user_profile) + upload_message_attachment("dummy2.txt", "text/plain", data2, self.user_profile) self.assertEqual( len(data) + len(data2), cache_get(get_realm_used_upload_space_cache_key(self.realm.id))[0], @@ -1876,7 +1876,7 @@ class UploadSpaceTests(UploadSerializeMixin, ZulipTestCase): self.assert_length(data2, self.realm.currently_used_upload_space_bytes()) data3 = b"even-more-data!" - upload_message_attachment("dummy3.txt", len(data3), "text/plain", data3, self.user_profile) + upload_message_attachment("dummy3.txt", "text/plain", data3, self.user_profile) self.assertEqual(len(data2) + len(data3), self.realm.currently_used_upload_space_bytes()) diff --git a/zerver/tests/test_upload_local.py b/zerver/tests/test_upload_local.py index b7ab06d60b..8c711a33aa 100644 --- a/zerver/tests/test_upload_local.py +++ b/zerver/tests/test_upload_local.py @@ -30,9 +30,7 @@ from zerver.models.users import get_system_bot class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): def test_upload_message_attachment(self) -> None: user_profile = self.example_user("hamlet") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) base = "/user_uploads/" self.assertEqual(base, url[: len(base)]) @@ -47,9 +45,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): def test_save_attachment_contents(self) -> None: user_profile = self.example_user("hamlet") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) path_id = re.sub(r"/user_uploads/", "", url) output = BytesIO() @@ -68,7 +64,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): self.assertEqual(user_profile.realm, internal_realm) url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile, zulip_realm + "dummy.txt", "text/plain", b"zulip!", user_profile, zulip_realm ) # Ensure the correct realm id of the target realm is used instead of the bot's realm. self.assertTrue(url.startswith(f"/user_uploads/{zulip_realm.id}/")) @@ -96,9 +92,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): user_profile = self.example_user("hamlet") path_ids = [] for n in range(1, 1005): - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) base = "/user_uploads/" self.assertEqual(base, url[: len(base)]) path_id = re.sub(r"/user_uploads/", "", url) diff --git a/zerver/tests/test_upload_s3.py b/zerver/tests/test_upload_s3.py index d1608a8b20..3c43c37e4c 100644 --- a/zerver/tests/test_upload_s3.py +++ b/zerver/tests/test_upload_s3.py @@ -46,9 +46,7 @@ class S3Test(ZulipTestCase): bucket = create_s3_buckets(settings.S3_AUTH_UPLOADS_BUCKET)[0] user_profile = self.example_user("hamlet") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) base = "/user_uploads/" self.assertEqual(base, url[: len(base)]) @@ -67,9 +65,7 @@ class S3Test(ZulipTestCase): def test_save_attachment_contents(self) -> None: create_s3_buckets(settings.S3_AUTH_UPLOADS_BUCKET) user_profile = self.example_user("hamlet") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) path_id = re.sub(r"/user_uploads/", "", url) output = BytesIO() @@ -90,7 +86,7 @@ class S3Test(ZulipTestCase): self.assertEqual(user_profile.realm, internal_realm) url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile, zulip_realm + "dummy.txt", "text/plain", b"zulip!", user_profile, zulip_realm ) # Ensure the correct realm id of the target realm is used instead of the bot's realm. self.assertTrue(url.startswith(f"/user_uploads/{zulip_realm.id}/")) @@ -100,9 +96,7 @@ class S3Test(ZulipTestCase): bucket = create_s3_buckets(settings.S3_AUTH_UPLOADS_BUCKET)[0] user_profile = self.example_user("hamlet") - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) path_id = re.sub(r"/user_uploads/", "", url) self.assertIsNotNone(bucket.Object(path_id).get()) @@ -117,9 +111,7 @@ class S3Test(ZulipTestCase): user_profile = self.example_user("hamlet") path_ids = [] for n in range(1, 5): - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) path_id = re.sub(r"/user_uploads/", "", url) self.assertIsNotNone(bucket.Object(path_id).get()) path_ids.append(path_id) @@ -152,9 +144,7 @@ class S3Test(ZulipTestCase): user_profile = self.example_user("hamlet") path_ids = [] for n in range(1, 5): - url = upload_message_attachment( - "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile - ) + url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile) path_ids.append(re.sub(r"/user_uploads/", "", url)) found_paths = [r[0] for r in all_message_attachments()] self.assertEqual(sorted(found_paths), sorted(path_ids)) diff --git a/zerver/views/upload.py b/zerver/views/upload.py index 8815f0ae5f..0b824dc2e7 100644 --- a/zerver/views/upload.py +++ b/zerver/views/upload.py @@ -320,5 +320,5 @@ def upload_file_backend(request: HttpRequest, user_profile: UserProfile) -> Http ) check_upload_within_quota(user_profile.realm, file_size) - uri = upload_message_attachment_from_request(user_file, user_profile, file_size) + uri = upload_message_attachment_from_request(user_file, user_profile) return json_success(request, data={"uri": uri})