Avoid looking up emails when rendering messages.

We now fetch email -> id mappings for messages ONLY if
it potentially uses the !avatar(foo@example.com) syntax.
This commit is contained in:
Steve Howell 2017-09-14 13:11:34 -07:00 committed by Tim Abbott
parent 4e7fce60ee
commit c3032a7fe8
5 changed files with 62 additions and 9 deletions

View File

@ -45,7 +45,6 @@ from zerver.lib.url_preview import preview as link_preview
from zerver.models import ( from zerver.models import (
all_realm_filters, all_realm_filters,
get_active_streams, get_active_streams,
get_active_user_dicts_in_realm,
get_system_bot, get_system_bot,
Message, Message,
Realm, Realm,
@ -78,6 +77,9 @@ if False:
# mypy requires the Optional to be inside Union # mypy requires the Optional to be inside Union
ElementStringNone = Union[Element, Optional[Text]] ElementStringNone = Union[Element, Optional[Text]]
AVATAR_REGEX = r'!avatar\((?P<email>[^)]*)\)'
GRAVATAR_REGEX = r'!gravatar\((?P<email>[^)]*)\)'
class BugdownRenderingException(Exception): class BugdownRenderingException(Exception):
pass pass
@ -718,7 +720,7 @@ class Avatar(markdown.inlinepatterns.Pattern):
profile_id = None profile_id = None
if db_data is not None: if db_data is not None:
user_dict = db_data['by_email'].get(email) user_dict = db_data['email_info'].get(email)
if user_dict is not None: if user_dict is not None:
profile_id = user_dict['id'] profile_id = user_dict['id']
@ -728,6 +730,17 @@ class Avatar(markdown.inlinepatterns.Pattern):
img.set('alt', email) img.set('alt', email)
return img return img
def possible_avatar_emails(content):
# type: (Text) -> Set[Text]
emails = set()
for regex in [AVATAR_REGEX, GRAVATAR_REGEX]:
matches = re.findall(regex, content)
for email in matches:
if email:
emails.add(email)
return emails
path_to_name_to_codepoint = os.path.join(settings.STATIC_ROOT, "generated", "emoji", "name_to_codepoint.json") path_to_name_to_codepoint = os.path.join(settings.STATIC_ROOT, "generated", "emoji", "name_to_codepoint.json")
with open(path_to_name_to_codepoint) as name_to_codepoint_file: with open(path_to_name_to_codepoint) as name_to_codepoint_file:
name_to_codepoint = ujson.load(name_to_codepoint_file) name_to_codepoint = ujson.load(name_to_codepoint_file)
@ -1280,8 +1293,8 @@ class Bugdown(markdown.Extension):
md.parser.blockprocessors.add('indent', ListIndentProcessor(md.parser), '<ulist') md.parser.blockprocessors.add('indent', ListIndentProcessor(md.parser), '<ulist')
# Note that !gravatar syntax should be deprecated long term. # Note that !gravatar syntax should be deprecated long term.
md.inlinePatterns.add('avatar', Avatar(r'!avatar\((?P<email>[^)]*)\)'), '>backtick') md.inlinePatterns.add('avatar', Avatar(AVATAR_REGEX), '>backtick')
md.inlinePatterns.add('gravatar', Avatar(r'!gravatar\((?P<email>[^)]*)\)'), '>backtick') md.inlinePatterns.add('gravatar', Avatar(GRAVATAR_REGEX), '>backtick')
md.inlinePatterns.add('stream_subscribe_button', md.inlinePatterns.add('stream_subscribe_button',
StreamSubscribeButton(r'!_stream_subscribe_button\((?P<stream_name>(?:[^)\\]|\\\)|\\)*)\)'), '>backtick') StreamSubscribeButton(r'!_stream_subscribe_button\((?P<stream_name>(?:[^)\\]|\\\)|\\)*)\)'), '>backtick')
@ -1475,6 +1488,31 @@ def log_bugdown_error(msg):
could cause an infinite exception loop.""" could cause an infinite exception loop."""
logging.getLogger('').error(msg) logging.getLogger('').error(msg)
def get_email_info(realm_id, emails):
# type: (int, Set[Text]) -> Dict[Text, FullNameInfo]
if not emails:
return dict()
q_list = {
Q(email__iexact=email.strip().lower())
for email in emails
}
rows = UserProfile.objects.filter(
realm_id=realm_id
).filter(
functools.reduce(lambda a, b: a | b, q_list),
).values(
'id',
'email',
)
dct = {
row['email'].strip().lower(): row
for row in rows
}
return dct
def get_full_name_info(realm_id, full_names): def get_full_name_info(realm_id, full_names):
# type: (int, Set[Text]) -> Dict[Text, FullNameInfo] # type: (int, Set[Text]) -> Dict[Text, FullNameInfo]
if not full_names: if not full_names:
@ -1541,7 +1579,6 @@ def do_convert(content, message=None, message_realm=None, possible_words=None, s
global db_data global db_data
if message is not None: if message is not None:
assert message_realm is not None # ensured above if message is not None assert message_realm is not None # ensured above if message is not None
realm_users = get_active_user_dicts_in_realm(message_realm)
realm_streams = get_active_streams(message_realm).values('id', 'name') realm_streams = get_active_streams(message_realm).values('id', 'name')
if possible_words is None: if possible_words is None:
@ -1550,8 +1587,11 @@ def do_convert(content, message=None, message_realm=None, possible_words=None, s
full_names = possible_mentions(content) full_names = possible_mentions(content)
full_name_info = get_full_name_info(message_realm.id, full_names) full_name_info = get_full_name_info(message_realm.id, full_names)
emails = possible_avatar_emails(content)
email_info = get_email_info(message_realm.id, emails)
db_data = {'possible_words': possible_words, db_data = {'possible_words': possible_words,
'by_email': dict((user['email'].lower(), user) for user in realm_users), 'email_info': email_info,
'full_name_info': full_name_info, 'full_name_info': full_name_info,
'emoji': message_realm.get_emoji(), 'emoji': message_realm.get_emoji(),
'sent_by_bot': sent_by_bot, 'sent_by_bot': sent_by_bot,

View File

@ -1034,6 +1034,19 @@ class BugdownErrorTests(ZulipTestCase):
class BugdownAvatarTestCase(ZulipTestCase): class BugdownAvatarTestCase(ZulipTestCase):
def test_possible_avatar_emails(self):
# type: () -> None
content = '''
hello !avatar(foo@example.com) my email is ignore@ignore.com
!gravatar(bar@yo.tv)
smushing!avatar(hamlet@example.org) is allowed
'''
self.assertEqual(
bugdown.possible_avatar_emails(content),
{'foo@example.com', 'bar@yo.tv', 'hamlet@example.org'},
)
def test_avatar_with_id(self): def test_avatar_with_id(self):
# type: () -> None # type: () -> None
sender_user_profile = self.example_user('othello') sender_user_profile = self.example_user('othello')

View File

@ -577,7 +577,7 @@ class StreamMessagesTest(ZulipTestCase):
with queries_captured() as queries: with queries_captured() as queries:
send_message() send_message()
self.assert_length(queries, 15) self.assert_length(queries, 14)
def test_stream_message_dict(self): def test_stream_message_dict(self):
# type: () -> None # type: () -> None

View File

@ -314,7 +314,7 @@ class LoginTest(ZulipTestCase):
with queries_captured() as queries: with queries_captured() as queries:
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.assert_length(queries, 65) self.assert_length(queries, 64)
user_profile = self.nonreg_user('test') user_profile = self.nonreg_user('test')
self.assertEqual(get_session_dict_user(self.client.session), user_profile.id) self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
self.assertFalse(user_profile.enable_stream_desktop_notifications) self.assertFalse(user_profile.enable_stream_desktop_notifications)

View File

@ -1629,7 +1629,7 @@ class SubscriptionAPITest(ZulipTestCase):
streams_to_sub, streams_to_sub,
dict(principals=ujson.dumps([email1, email2])), dict(principals=ujson.dumps([email1, email2])),
) )
self.assert_length(queries, 41) self.assert_length(queries, 40)
self.assert_length(events, 7) self.assert_length(events, 7)
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')]: