From c6448263c33860becdfd2391373cc95a6976da7a Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 29 Dec 2021 12:52:27 +0000 Subject: [PATCH] refactor: Add MentionBackend. We will eventually use this to avoid redundant queries. The diff is slightly noisy here, but there are no logic changes. --- zerver/lib/actions.py | 8 +++-- zerver/lib/markdown/__init__.py | 5 +-- zerver/lib/mention.py | 49 ++++++++++++++++---------- zerver/tests/test_events.py | 8 +++-- zerver/tests/test_markdown.py | 9 +++-- zerver/tests/test_message_fetch.py | 5 +-- zerver/tests/test_notification_data.py | 18 ++++++---- 7 files changed, 64 insertions(+), 38 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 260dda9a2d..15cc6dea5d 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -100,7 +100,7 @@ from zerver.lib.hotspots import get_next_hotspots from zerver.lib.i18n import get_language_name from zerver.lib.markdown import MessageRenderingResult, topic_links from zerver.lib.markdown import version as markdown_version -from zerver.lib.mention import MentionData, silent_mention_syntax_for_user +from zerver.lib.mention import MentionBackend, MentionData, silent_mention_syntax_for_user from zerver.lib.message import ( MessageDict, SendMessageRequest, @@ -1863,8 +1863,9 @@ def build_message_send_dict( if realm is None: realm = message.sender.realm + mention_backend = MentionBackend(realm.id) mention_data = MentionData( - realm_id=realm.id, + mention_backend=mention_backend, content=message.content, ) @@ -3065,8 +3066,9 @@ def check_update_message( content = "(deleted)" content = normalize_body(content) + mention_backend = MentionBackend(user_profile.realm_id) mention_data = MentionData( - realm_id=user_profile.realm.id, + mention_backend=mention_backend, content=content, ) user_info = get_user_info_for_message_updates(message.id) diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py index 98910a6102..45c8a902bc 100644 --- a/zerver/lib/markdown/__init__.py +++ b/zerver/lib/markdown/__init__.py @@ -54,7 +54,7 @@ from zerver.lib.emoji import EMOTICON_RE, codepoint_to_name, name_to_codepoint, from zerver.lib.exceptions import MarkdownRenderingException from zerver.lib.markdown import fenced_code from zerver.lib.markdown.fenced_code import FENCE_RE -from zerver.lib.mention import FullNameInfo, MentionData, get_stream_name_map +from zerver.lib.mention import FullNameInfo, MentionBackend, MentionData, get_stream_name_map from zerver.lib.outgoing_http import OutgoingSession from zerver.lib.subdomains import is_static_or_current_realm_url from zerver.lib.tex import render_tex @@ -2508,7 +2508,8 @@ def do_convert( # are uncommon enough that it's a useful optimization. if mention_data is None: - mention_data = MentionData(message_realm.id, content) + mention_backend = MentionBackend(message_realm.id) + mention_data = MentionData(mention_backend, content) stream_names = possible_linked_stream_names(content) stream_name_info = get_stream_name_map(message_realm, stream_names) diff --git a/zerver/lib/mention.py b/zerver/lib/mention.py index cc6a183b5e..8fb0baa922 100644 --- a/zerver/lib/mention.py +++ b/zerver/lib/mention.py @@ -37,6 +37,29 @@ class UserFilter: raise AssertionError("totally empty filter makes no sense") +@dataclass +class MentionBackend: + realm_id: int + + def get_full_name_info_list(self, user_filters: List[UserFilter]) -> List[FullNameInfo]: + q_list = [user_filter.Q() for user_filter in user_filters] + + rows = ( + UserProfile.objects.filter( + realm_id=self.realm_id, + is_active=True, + ) + .filter( + functools.reduce(lambda a, b: a | b, q_list), + ) + .only( + "id", + "full_name", + ) + ) + return [FullNameInfo(id=row.id, full_name=row.full_name) for row in rows] + + def user_mention_matches_wildcard(mention: str) -> bool: return mention in wildcards @@ -65,7 +88,9 @@ def possible_user_group_mentions(content: str) -> Set[str]: return {m.group("match") for m in USER_GROUP_MENTIONS_RE.finditer(content)} -def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[FullNameInfo]: +def get_possible_mentions_info( + mention_backend: MentionBackend, mention_texts: Set[str] +) -> List[FullNameInfo]: if not mention_texts: return [] @@ -88,28 +113,14 @@ def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[F # For **name** syntax. user_filters.append(UserFilter(full_name=mention_text, id=None)) - q_list = [user_filter.Q() for user_filter in user_filters] - - rows = ( - UserProfile.objects.filter( - realm_id=realm_id, - is_active=True, - ) - .filter( - functools.reduce(lambda a, b: a | b, q_list), - ) - .only( - "id", - "full_name", - ) - ) - return [FullNameInfo(id=row.id, full_name=row.full_name) for row in rows] + return mention_backend.get_full_name_info_list(user_filters) class MentionData: - def __init__(self, realm_id: int, content: str) -> None: + def __init__(self, mention_backend: MentionBackend, content: str) -> None: + realm_id = mention_backend.realm_id mention_texts, has_wildcards = possible_mentions(content) - possible_mentions_info = get_possible_mentions_info(realm_id, mention_texts) + possible_mentions_info = get_possible_mentions_info(mention_backend, mention_texts) self.full_name_info = {row.full_name.lower(): row for row in possible_mentions_info} self.user_id_info = {row.id: row for row in possible_mentions_info} self.init_user_group_data(realm_id=realm_id, content=content) diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 30de1f3016..e749a221b3 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -171,7 +171,7 @@ from zerver.lib.events import ( fetch_initial_state_data, post_process_state, ) -from zerver.lib.mention import MentionData +from zerver.lib.mention import MentionBackend, MentionData from zerver.lib.message import render_markdown from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import ( @@ -437,8 +437,9 @@ class NormalActionsTest(BaseAction): content = "new content" rendering_result = render_markdown(pm, content) prior_mention_user_ids: Set[int] = set() + mention_backend = MentionBackend(self.user_profile.realm_id) mention_data = MentionData( - realm_id=self.user_profile.realm_id, + mention_backend=mention_backend, content=content, ) @@ -496,8 +497,9 @@ class NormalActionsTest(BaseAction): content = "new content" rendering_result = render_markdown(message, content) prior_mention_user_ids: Set[int] = set() + mention_backend = MentionBackend(self.user_profile.realm_id) mention_data = MentionData( - realm_id=self.user_profile.realm_id, + mention_backend=mention_backend, content=content, ) diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index 50b3c4d75e..58266c5291 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -42,6 +42,7 @@ from zerver.lib.markdown.fenced_code import FencedBlockPreprocessor from zerver.lib.mdiff import diff_strings from zerver.lib.mention import ( FullNameInfo, + MentionBackend, MentionData, get_possible_mentions_info, possible_mentions, @@ -232,8 +233,9 @@ class MarkdownMiscTest(ZulipTestCase): fred4 = make_user("fred4@example.com", "Fred Flintstone") + mention_backend = MentionBackend(realm.id) lst = get_possible_mentions_info( - realm.id, {"Fred Flintstone", "Cordelia, LEAR's daughter", "Not A User"} + mention_backend, {"Fred Flintstone", "Cordelia, LEAR's daughter", "Not A User"} ) set_of_names = set(map(lambda x: x.full_name.lower(), lst)) self.assertEqual(set_of_names, {"fred flintstone", "cordelia, lear's daughter"}) @@ -259,7 +261,8 @@ class MarkdownMiscTest(ZulipTestCase): hamlet = self.example_user("hamlet") cordelia = self.example_user("cordelia") content = "@**King Hamlet** @**Cordelia, lear's daughter**" - mention_data = MentionData(realm.id, content) + mention_backend = MentionBackend(realm.id) + mention_data = MentionData(mention_backend, content) self.assertEqual(mention_data.get_user_ids(), {hamlet.id, cordelia.id}) self.assertEqual( mention_data.get_user_by_id(hamlet.id), @@ -275,7 +278,7 @@ class MarkdownMiscTest(ZulipTestCase): self.assertFalse(mention_data.message_has_wildcards()) content = "@**King Hamlet** @**Cordelia, lear's daughter** @**all**" - mention_data = MentionData(realm.id, content) + mention_data = MentionData(mention_backend, content) self.assertTrue(mention_data.message_has_wildcards()) def test_invalid_katex_path(self) -> None: diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py index ae2246b534..a5eff2ba6a 100644 --- a/zerver/tests/test_message_fetch.py +++ b/zerver/tests/test_message_fetch.py @@ -21,7 +21,7 @@ from zerver.lib.actions import ( ) from zerver.lib.avatar import avatar_url from zerver.lib.exceptions import JsonableError -from zerver.lib.mention import MentionData +from zerver.lib.mention import MentionBackend, MentionData from zerver.lib.message import ( MessageDict, get_first_visible_message_id, @@ -3776,7 +3776,8 @@ class MessageHasKeywordsTest(ZulipTestCase): hamlet = self.example_user("hamlet") realm_id = hamlet.realm.id rendering_result = render_markdown(msg, content) - mention_data = MentionData(realm_id, content) + mention_backend = MentionBackend(realm_id) + mention_data = MentionData(mention_backend, content) do_update_message( hamlet, msg, diff --git a/zerver/tests/test_notification_data.py b/zerver/tests/test_notification_data.py index 51ce3c1009..79926a3457 100644 --- a/zerver/tests/test_notification_data.py +++ b/zerver/tests/test_notification_data.py @@ -1,4 +1,4 @@ -from zerver.lib.mention import MentionData +from zerver.lib.mention import MentionBackend, MentionData from zerver.lib.notification_data import get_user_group_mentions_data from zerver.lib.test_classes import ZulipTestCase from zerver.lib.user_groups import create_user_group @@ -220,11 +220,13 @@ class TestNotificationData(ZulipTestCase): hamlet_only = create_user_group("hamlet_only", [hamlet], realm) hamlet_and_cordelia = create_user_group("hamlet_and_cordelia", [hamlet, cordelia], realm) + mention_backend = MentionBackend(realm.id) + # Base case. No user/user group mentions result = get_user_group_mentions_data( mentioned_user_ids=set(), mentioned_user_group_ids=[], - mention_data=MentionData(realm.id, "no group mentioned"), + mention_data=MentionData(mention_backend, "no group mentioned"), ) self.assertDictEqual(result, {}) @@ -232,7 +234,7 @@ class TestNotificationData(ZulipTestCase): result = get_user_group_mentions_data( mentioned_user_ids=set(), mentioned_user_group_ids=[hamlet_and_cordelia.id], - mention_data=MentionData(realm.id, "hey @*hamlet_and_cordelia*!"), + mention_data=MentionData(mention_backend, "hey @*hamlet_and_cordelia*!"), ) self.assertDictEqual( result, @@ -247,7 +249,9 @@ class TestNotificationData(ZulipTestCase): result = get_user_group_mentions_data( mentioned_user_ids=set(), mentioned_user_group_ids=[hamlet_and_cordelia.id, hamlet_only.id], - mention_data=MentionData(realm.id, "hey @*hamlet_and_cordelia* and @*hamlet_only*"), + mention_data=MentionData( + mention_backend, "hey @*hamlet_and_cordelia* and @*hamlet_only*" + ), ) self.assertDictEqual( result, @@ -262,7 +266,9 @@ class TestNotificationData(ZulipTestCase): result = get_user_group_mentions_data( mentioned_user_ids=set(), mentioned_user_group_ids=[hamlet_only.id, hamlet_and_cordelia.id], - mention_data=MentionData(realm.id, "hey @*hamlet_only* and @*hamlet_and_cordelia*"), + mention_data=MentionData( + mention_backend, "hey @*hamlet_only* and @*hamlet_and_cordelia*" + ), ) self.assertDictEqual( result, @@ -277,7 +283,7 @@ class TestNotificationData(ZulipTestCase): result = get_user_group_mentions_data( mentioned_user_ids={hamlet.id}, mentioned_user_group_ids=[hamlet_and_cordelia.id], - mention_data=MentionData(realm.id, "hey @*hamlet_and_cordelia*!"), + mention_data=MentionData(mention_backend, "hey @*hamlet_and_cordelia*!"), ) self.assertDictEqual( result,