From ca50b5dac7444469799dfe67b7e52262dcec962d Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Fri, 18 Apr 2025 11:34:12 +0000 Subject: [PATCH] attachments: Do not fetch complete owner object. We just need to compare the user profile id and the owner id, we will save 1 query call this way. --- zerver/lib/attachments.py | 2 +- zerver/tests/test_upload.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/zerver/lib/attachments.py b/zerver/lib/attachments.py index 32502561e4..fd13810724 100644 --- a/zerver/lib/attachments.py +++ b/zerver/lib/attachments.py @@ -120,7 +120,7 @@ def validate_attachment_request( ) attachment.refresh_from_db() - if user_profile == attachment.owner: + if user_profile.id == attachment.owner_id: # If you own the file, you can access it. return True, attachment if ( diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 6da2a59ac8..f31b1ca8dd 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -803,7 +803,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): # Owner user should be able to view file self.login_user(hamlet) - with self.assert_database_query_count(6): + with self.assert_database_query_count(5): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") @@ -811,7 +811,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): # Subscribed user who received the message should be able to view file self.login_user(cordelia) - with self.assert_database_query_count(9): + with self.assert_database_query_count(8): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") @@ -864,7 +864,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): # Owner user should be able to view file self.login_user(user) - with self.assert_database_query_count(6): + with self.assert_database_query_count(5): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") @@ -872,7 +872,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): # Originally subscribed user should be able to view file self.login_user(polonius) - with self.assert_database_query_count(9): + with self.assert_database_query_count(8): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") @@ -880,7 +880,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): # Subscribed user who did not receive the message should also be able to view file self.login_user(late_subscribed_user) - with self.assert_database_query_count(10): + with self.assert_database_query_count(9): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") @@ -890,7 +890,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): def assert_cannot_access_file(user: UserProfile) -> None: self.login_user(user) # It takes a few extra queries to verify lack of access with shared history. - with self.assert_database_query_count(9): + with self.assert_database_query_count(8): response = self.client_get(url) self.assertEqual(response.status_code, 403) self.assert_in_response("You are not authorized to view this file.", response) @@ -931,7 +931,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): user = self.example_user("aaron") self.login_user(user) - with self.assert_database_query_count(9): + with self.assert_database_query_count(8): response = self.client_get(url) self.assertEqual(response.status_code, 403) self.assert_in_response("You are not authorized to view this file.", response) @@ -940,12 +940,12 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): self.subscribe(user, "test-subscribe 2") # If we were accidentally one query per message, this would be 20+ - with self.assert_database_query_count(10): + with self.assert_database_query_count(9): response = self.client_get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.getvalue(), b"zulip!") - with self.assert_database_query_count(6): + with self.assert_database_query_count(5): self.assertTrue(validate_attachment_request(user, fp_path_id)[0]) self.logout()