From a04a095738bbd56353f1d406976473e474bb7b1d Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 14 Sep 2016 15:24:44 -0700 Subject: [PATCH] Speed up alert word detection during message sends. We no longer use all the alert words for all the users in the entire realm when we look for alert words in a newly sent/edited message. Now we limit the search to only all the alert words for all the users who will get UserMessage records. This will hopefully make a big difference for big realms where most messages are only sent to a small subset of users. --- zerver/lib/actions.py | 8 +++++--- zerver/models.py | 19 +++++++++++++------ zerver/tests/test_bugdown.py | 4 +++- zerver/views/messages.py | 14 ++++++++++++-- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 2006779fd3..732d1c09ff 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -628,13 +628,14 @@ def do_send_message(message, rendered_content = None, no_log = False, stream = N 'stream': stream, 'local_id': local_id}])[0] -def render_incoming_message(message, content): - # type: (Message, text_type) -> text_type +def render_incoming_message(message, content, message_users): + # type: (Message, text_type, Set[UserProfile]) -> text_type realm_alert_words = alert_words_in_realm(message.get_realm()) try: rendered_content = message.render_markdown( content=content, realm_alert_words=realm_alert_words, + message_users=message_users, ) except BugdownRenderingException: raise JsonableError(_('Unable to render message')) @@ -704,7 +705,8 @@ def do_send_messages(messages): assert message['message'].rendered_content is None rendered_content = render_incoming_message( message['message'], - message['message'].content) + message['message'].content, + message_users=message['active_recipients']) message['message'].set_rendered_content(rendered_content) for message in messages: diff --git a/zerver/models.py b/zerver/models.py index d7c0f328cd..17afb94ce3 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -809,8 +809,8 @@ class Message(ModelReprMixin, models.Model): # type: () -> Realm return self.sender.realm - def render_markdown(self, content, domain=None, realm_alert_words=None): - # type: (text_type, Optional[text_type], Optional[RealmAlertWords]) -> text_type + def render_markdown(self, content, domain=None, realm_alert_words=None, message_users=None): + # type: (text_type, Optional[text_type], Optional[RealmAlertWords], Set[UserProfile]) -> text_type """Return HTML for given markdown. Bugdown may add properties to the message object such as `mentions_user_ids` and `mentions_wildcard`. These are only on this Django object and are not saved in the @@ -822,6 +822,11 @@ class Message(ModelReprMixin, models.Model): import zerver.lib.bugdown as bugdown # 'from zerver.lib import bugdown' gives mypy error in python 3 mode. + if message_users is None: + message_user_ids = set() # type: Set[int] + else: + message_user_ids = {u.id for u in message_users} + self.mentions_wildcard = False self.is_me_message = False self.mentions_user_ids = set() # type: Set[int] @@ -836,8 +841,9 @@ class Message(ModelReprMixin, models.Model): possible_words = set() # type: Set[text_type] if realm_alert_words is not None: - for words in realm_alert_words.values(): - possible_words.update(set(words)) + for user_id, words in realm_alert_words.items(): + if user_id in message_user_ids: + possible_words.update(set(words)) # DO MAIN WORK HERE -- call bugdown to convert rendered_content = bugdown.convert(content, domain, self, possible_words) @@ -846,8 +852,9 @@ class Message(ModelReprMixin, models.Model): if realm_alert_words is not None: for user_id, words in realm_alert_words.items(): - if set(words).intersection(self.alert_words): - self.user_ids_with_alert_words.add(user_id) + if user_id in message_user_ids: + if set(words).intersection(self.alert_words): + self.user_ids_with_alert_words.add(user_id) self.is_me_message = Message.is_status_message(content, rendered_content) diff --git a/zerver/tests/test_bugdown.py b/zerver/tests/test_bugdown.py index a3248133b9..ec191da3cc 100644 --- a/zerver/tests/test_bugdown.py +++ b/zerver/tests/test_bugdown.py @@ -421,7 +421,9 @@ class BugdownTest(TestCase): realm_alert_words = alert_words_in_realm(user_profile.realm) def render(msg, content): - return msg.render_markdown(content, realm_alert_words=realm_alert_words) + return msg.render_markdown(content, + realm_alert_words=realm_alert_words, + message_users={user_profile}) content = "We have an ALERTWORD day today!" self.assertEqual(render(msg, content), "

We have an ALERTWORD day today!

") diff --git a/zerver/views/messages.py b/zerver/views/messages.py index f9f65a5af5..46aafb7df9 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -33,7 +33,7 @@ from zerver.models import Message, UserProfile, Stream, Subscription, \ parse_usermessage_flags, to_dict_cache_key_id, extract_message_dict, \ stringify_message_dict, \ resolve_email_to_domain, get_realm, get_active_streams, \ - bulk_get_streams + bulk_get_streams, get_user_profile_by_id from sqlalchemy import func from sqlalchemy.sql import select, join, column, literal_column, literal, and_, \ @@ -911,8 +911,18 @@ def update_message_backend(request, user_profile, raise JsonableError(_("Content can't be empty")) content = truncate_body(content) + # We exclude UserMessage.flags.historical rows since those + # users did not receive the message originally, and thus + # probably are not relevant for reprocessed alert_words, + # mentions and similar rendering features. This may be a + # decision we change in the future. + ums = UserMessage.objects.filter(message=message.id, + flags=~UserMessage.flags.historical) + message_users = {get_user_profile_by_id(um.user_profile_id) for um in ums} # If rendering fails, the called code will raise a JsonableError. - rendered_content = render_incoming_message(message, content) + rendered_content = render_incoming_message(message, + content=content, + message_users=message_users) do_update_message(user_profile, message, subject, propagate_mode, content, rendered_content) return json_success()