mirror of
https://github.com/zulip/zulip.git
synced 2025-11-04 14:03:30 +00:00
performance: Use ORM to fetch sender in render_markdown.
In 709493cd75 (Feb 2017)
I added code to render_markdown that re-fetched the
sender of the message, to detect whether the message is
a bot.
It's better to just let the ORM fetch this. The
message object should already have sender.
The diff makes it look like we are saving round trips
to the database, which is true in some cases. For
the main message-send codepath, though, we are only
saving a trip to memcached, since the middleware
will have put our sender's user object into the
cache. The test_message_send test calls internally
to check_send_stream_message, so it was actually
hitting the database in render_markdown (prior to
my change).
This commit is contained in:
@@ -48,7 +48,6 @@ from zerver.models import (
|
|||||||
UserMessage,
|
UserMessage,
|
||||||
UserProfile,
|
UserProfile,
|
||||||
get_display_recipient_by_id,
|
get_display_recipient_by_id,
|
||||||
get_user_profile_by_id,
|
|
||||||
get_usermessage_by_message_id,
|
get_usermessage_by_message_id,
|
||||||
query_for_ids,
|
query_for_ids,
|
||||||
)
|
)
|
||||||
@@ -685,7 +684,7 @@ def render_markdown(message: Message,
|
|||||||
if realm is None:
|
if realm is None:
|
||||||
realm = message.get_realm()
|
realm = message.get_realm()
|
||||||
|
|
||||||
sender = get_user_profile_by_id(message.sender_id)
|
sender = message.sender
|
||||||
sent_by_bot = sender.is_bot
|
sent_by_bot = sender.is_bot
|
||||||
translate_emoticons = sender.translate_emoticons
|
translate_emoticons = sender.translate_emoticons
|
||||||
|
|
||||||
|
|||||||
@@ -960,8 +960,8 @@ class EditMessageTest(ZulipTestCase):
|
|||||||
'propagate_mode': 'change_all',
|
'propagate_mode': 'change_all',
|
||||||
'topic': 'new topic',
|
'topic': 'new topic',
|
||||||
})
|
})
|
||||||
self.assertEqual(len(queries), 48)
|
self.assertEqual(len(queries), 47)
|
||||||
self.assertEqual(len(cache_tries), 13)
|
self.assertEqual(len(cache_tries), 11)
|
||||||
|
|
||||||
messages = get_topic_messages(user_profile, old_stream, "test")
|
messages = get_topic_messages(user_profile, old_stream, "test")
|
||||||
self.assertEqual(len(messages), 1)
|
self.assertEqual(len(messages), 1)
|
||||||
|
|||||||
@@ -1194,7 +1194,7 @@ class StreamMessagesTest(ZulipTestCase):
|
|||||||
body=content,
|
body=content,
|
||||||
)
|
)
|
||||||
|
|
||||||
self.assert_length(queries, 13)
|
self.assert_length(queries, 12)
|
||||||
|
|
||||||
def test_stream_message_dict(self) -> None:
|
def test_stream_message_dict(self) -> None:
|
||||||
user_profile = self.example_user('iago')
|
user_profile = self.example_user('iago')
|
||||||
|
|||||||
@@ -651,13 +651,13 @@ class LoginTest(ZulipTestCase):
|
|||||||
with queries_captured() as queries, cache_tries_captured() as cache_tries:
|
with queries_captured() as queries, cache_tries_captured() as cache_tries:
|
||||||
self.register(self.nonreg_email('test'), "test")
|
self.register(self.nonreg_email('test'), "test")
|
||||||
# Ensure the number of queries we make is not O(streams)
|
# Ensure the number of queries we make is not O(streams)
|
||||||
self.assertEqual(len(queries), 73)
|
self.assertEqual(len(queries), 72)
|
||||||
|
|
||||||
# We can probably avoid a couple cache hits here, but there doesn't
|
# We can probably avoid a couple cache hits here, but there doesn't
|
||||||
# seem to be any O(N) behavior. Some of the cache hits are related
|
# seem to be any O(N) behavior. Some of the cache hits are related
|
||||||
# to sending messages, such as getting the welcome bot, looking up
|
# to sending messages, such as getting the welcome bot, looking up
|
||||||
# the alert words for a realm, etc.
|
# the alert words for a realm, etc.
|
||||||
self.assertEqual(len(cache_tries), 16)
|
self.assertEqual(len(cache_tries), 15)
|
||||||
|
|
||||||
user_profile = self.nonreg_user('test')
|
user_profile = self.nonreg_user('test')
|
||||||
self.assert_logged_in_user_id(user_profile.id)
|
self.assert_logged_in_user_id(user_profile.id)
|
||||||
|
|||||||
@@ -2984,7 +2984,7 @@ class SubscriptionAPITest(ZulipTestCase):
|
|||||||
streams_to_sub,
|
streams_to_sub,
|
||||||
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
||||||
)
|
)
|
||||||
self.assert_length(queries, 36)
|
self.assert_length(queries, 35)
|
||||||
|
|
||||||
self.assert_length(events, 5)
|
self.assert_length(events, 5)
|
||||||
for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]:
|
for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]:
|
||||||
@@ -3817,7 +3817,7 @@ class SubscriptionAPITest(ZulipTestCase):
|
|||||||
[new_streams[0]],
|
[new_streams[0]],
|
||||||
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
||||||
)
|
)
|
||||||
self.assert_length(queries, 36)
|
self.assert_length(queries, 35)
|
||||||
|
|
||||||
# Test creating private stream.
|
# Test creating private stream.
|
||||||
with queries_captured() as queries:
|
with queries_captured() as queries:
|
||||||
@@ -3827,7 +3827,7 @@ class SubscriptionAPITest(ZulipTestCase):
|
|||||||
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
dict(principals=orjson.dumps([user1.id, user2.id]).decode()),
|
||||||
invite_only=True,
|
invite_only=True,
|
||||||
)
|
)
|
||||||
self.assert_length(queries, 38)
|
self.assert_length(queries, 37)
|
||||||
|
|
||||||
# Test creating a public stream with announce when realm has a notification stream.
|
# Test creating a public stream with announce when realm has a notification stream.
|
||||||
notifications_stream = get_stream(self.streams[0], self.test_realm)
|
notifications_stream = get_stream(self.streams[0], self.test_realm)
|
||||||
@@ -3842,7 +3842,7 @@ class SubscriptionAPITest(ZulipTestCase):
|
|||||||
principals=orjson.dumps([user1.id, user2.id]).decode(),
|
principals=orjson.dumps([user1.id, user2.id]).decode(),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
self.assert_length(queries, 44)
|
self.assert_length(queries, 43)
|
||||||
|
|
||||||
class GetStreamsTest(ZulipTestCase):
|
class GetStreamsTest(ZulipTestCase):
|
||||||
def test_streams_api_for_bot_owners(self) -> None:
|
def test_streams_api_for_bot_owners(self) -> None:
|
||||||
|
|||||||
@@ -769,8 +769,8 @@ class QueryCountTest(ZulipTestCase):
|
|||||||
prereg_user=prereg_user,
|
prereg_user=prereg_user,
|
||||||
)
|
)
|
||||||
|
|
||||||
self.assert_length(queries, 72)
|
self.assert_length(queries, 70)
|
||||||
self.assert_length(cache_tries, 22)
|
self.assert_length(cache_tries, 20)
|
||||||
self.assert_length(events, 7)
|
self.assert_length(events, 7)
|
||||||
|
|
||||||
peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"]
|
peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"]
|
||||||
|
|||||||
Reference in New Issue
Block a user