upload: Stop requiring callers pass in the file size.

This can be calculated because we have the contents.

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]]
 

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,

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
     )
 
 

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(
+    url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-        "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile
-    )
     attachment_id = url.replace("/user_uploads/", "").split("/")[0]
 
     return {"attachment_id": attachment_id}

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),

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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile
-        )
         attachment_path_id = url.replace("/user_uploads/", "")
         attachment = claim_attachment(
             path_id=attachment_path_id,

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(
+            url = upload_message_attachment(f"dummy{n}.txt", "text/plain", content, hamlet)
-                f"dummy{n}.txt", len(content), "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(
+            url = upload_message_attachment(f"dummy{n}.txt", "text/plain", content, owner)
-                f"dummy{n}.txt", len(content), "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))

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("dummy1.txt", "text/plain", b"zulip1!", hamlet)
-        upload_message_attachment("dummy2.txt", len(b"zulip2!"), "text/plain", b"zulip2!", othello)
+        upload_message_attachment("dummy2.txt", "text/plain", b"zulip2!", othello)
 
         with self.assertLogs(level="INFO"):
             transfer_message_files_to_s3(1)

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())
 
 

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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+            url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-                "dummy.txt", len(b"zulip!"), "text/plain", b"zulip!", user_profile
-            )
             base = "/user_uploads/"
             self.assertEqual(base, url[: len(base)])
             path_id = re.sub(r"/user_uploads/", "", url)

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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+        url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-            "dummy.txt", len(b"zulip!"), "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(
+            url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-                "dummy.txt", len(b"zulip!"), "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(
+            url = upload_message_attachment("dummy.txt", "text/plain", b"zulip!", user_profile)
-                "dummy.txt", len(b"zulip!"), "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))

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})