markdown: Remove logic for creating markdown engines for all realms.

This logic likely never ran due to a combination of bugs.

* Running `maybe_update_markdown_engines` unconditionally meant that
  `if md_engine_key in md_engines` was likely always true.
* Introduced in 65838bb: DEFAULT_MARKDOWN_KEY could never be in
  md_engines, so should we have ever reached that code path, we'd have
  tried to rebuild all markdown engines every time.

And it also wasn't clearly helpful -- because we fetch all linkifiers
for a realm on every request anyway, we don't really save database
queries by doing a bulk fetch on startup, and doing so would likely
result in a material regression to Zulip's overall startup time that
we were creating markdown engines for large numbers of realms in bulk
during process startup.
This commit is contained in:
Tim Abbott 2021-04-13 08:42:48 -07:00
parent c6a50499f7
commit 2e928a0853
3 changed files with 15 additions and 74 deletions

View File

@ -73,7 +73,6 @@ from zerver.models import (
UserGroup, UserGroup,
UserGroupMembership, UserGroupMembership,
UserProfile, UserProfile,
all_linkifiers_for_installation,
get_active_streams, get_active_streams,
linkifiers_for_realm, linkifiers_for_realm,
) )
@ -2373,32 +2372,22 @@ def topic_links(linkifiers_key: int, topic_name: str) -> List[Dict[str, str]]:
return [{k: str(v) for k, v in match.items() if k != "index"} for match in matches] return [{k: str(v) for k, v in match.items() if k != "index"} for match in matches]
def maybe_update_markdown_engines(linkifiers_key: Optional[int], email_gateway: bool) -> None: def maybe_update_markdown_engines(linkifiers_key: int, email_gateway: bool) -> None:
# If linkifiers_key is None, load all linkifiers
global linkifier_data global linkifier_data
if linkifiers_key is None:
all_linkifiers = all_linkifiers_for_installation()
all_linkifiers[DEFAULT_MARKDOWN_KEY] = []
for linkifiers_key, linkifiers in all_linkifiers.items():
linkifier_data[linkifiers_key] = linkifiers
make_md_engine(linkifiers_key, email_gateway)
# Hack to ensure that linkifiers_key is right for mirrored Zephyrs
linkifier_data[ZEPHYR_MIRROR_MARKDOWN_KEY] = []
make_md_engine(ZEPHYR_MIRROR_MARKDOWN_KEY, False)
else:
linkifiers = linkifiers_for_realm(linkifiers_key)
if linkifiers_key not in linkifier_data or linkifier_data[linkifiers_key] != linkifiers:
# Linkifier data has changed, update `linkifier_data` and any
# of the existing Markdown engines using this set of linkifiers.
linkifier_data[linkifiers_key] = linkifiers
for email_gateway_flag in [True, False]:
if (linkifiers_key, email_gateway_flag) in md_engines:
# Update only existing engines(if any), don't create new one.
make_md_engine(linkifiers_key, email_gateway_flag)
if (linkifiers_key, email_gateway) not in md_engines: linkifiers = linkifiers_for_realm(linkifiers_key)
# Markdown engine corresponding to this key doesn't exists so create one. if linkifiers_key not in linkifier_data or linkifier_data[linkifiers_key] != linkifiers:
make_md_engine(linkifiers_key, email_gateway) # Linkifier data has changed, update `linkifier_data` and any
# of the existing Markdown engines using this set of linkifiers.
linkifier_data[linkifiers_key] = linkifiers
for email_gateway_flag in [True, False]:
if (linkifiers_key, email_gateway_flag) in md_engines:
# Update only existing engines(if any), don't create new one.
make_md_engine(linkifiers_key, email_gateway_flag)
if (linkifiers_key, email_gateway) not in md_engines:
# Markdown engine corresponding to this key doesn't exists so create one.
make_md_engine(linkifiers_key, email_gateway)
# We want to log Markdown parser failures, but shouldn't log the actual input # We want to log Markdown parser failures, but shouldn't log the actual input
@ -2578,14 +2567,7 @@ def do_convert(
maybe_update_markdown_engines(linkifiers_key, email_gateway) maybe_update_markdown_engines(linkifiers_key, email_gateway)
md_engine_key = (linkifiers_key, email_gateway) md_engine_key = (linkifiers_key, email_gateway)
_md_engine = md_engines[md_engine_key]
if md_engine_key in md_engines:
_md_engine = md_engines[md_engine_key]
else:
if DEFAULT_MARKDOWN_KEY not in md_engines:
maybe_update_markdown_engines(linkifiers_key=None, email_gateway=False)
_md_engine = md_engines[(DEFAULT_MARKDOWN_KEY, email_gateway)]
# Reset the parser; otherwise it will get slower over time. # Reset the parser; otherwise it will get slower over time.
_md_engine.reset() _md_engine.reset()

View File

@ -3,13 +3,11 @@ import datetime
import re import re
import secrets import secrets
import time import time
from collections import defaultdict
from datetime import timedelta from datetime import timedelta
from typing import ( from typing import (
AbstractSet, AbstractSet,
Any, Any,
Callable, Callable,
DefaultDict,
Dict, Dict,
Iterable, Iterable,
List, List,
@ -938,20 +936,6 @@ def linkifiers_for_realm_remote_cache(realm_id: int) -> List[LinkifierDict]:
return filters return filters
def all_linkifiers_for_installation() -> Dict[int, List[LinkifierDict]]:
filters: DefaultDict[int, List[LinkifierDict]] = defaultdict(list)
for linkifier in RealmFilter.objects.all():
filters[linkifier.realm_id].append(
LinkifierDict(
pattern=linkifier.pattern,
url_format=linkifier.url_format_string,
id=linkifier.id,
)
)
return filters
def flush_linkifiers(sender: Any, **kwargs: Any) -> None: def flush_linkifiers(sender: Any, **kwargs: Any) -> None:
realm_id = kwargs["instance"].realm_id realm_id = kwargs["instance"].realm_id
cache_delete(get_linkifiers_cache_key(realm_id)) cache_delete(get_linkifiers_cache_key(realm_id))

View File

@ -45,7 +45,6 @@ from zerver.lib.message import render_markdown
from zerver.lib.request import JsonableError from zerver.lib.request import JsonableError
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.tex import render_tex from zerver.lib.tex import render_tex
from zerver.lib.types import LinkifierDict
from zerver.lib.user_groups import create_user_group from zerver.lib.user_groups import create_user_group
from zerver.models import ( from zerver.models import (
MAX_MESSAGE_LENGTH, MAX_MESSAGE_LENGTH,
@ -1420,30 +1419,6 @@ class MarkdownTest(ZulipTestCase):
], ],
) )
def test_maybe_update_markdown_engines(self) -> None:
realm = get_realm("zulip")
url_format_string = r"https://trac.example.com/ticket/%(id)s"
linkifier = RealmFilter(
realm=realm, pattern=r"#(?P<id>[0-9]{2,8})", url_format_string=url_format_string
)
linkifier.save()
import zerver.lib.markdown
zerver.lib.markdown.linkifier_data = {}
maybe_update_markdown_engines(None, False)
all_linkifiers = zerver.lib.markdown.linkifier_data
zulip_linkifiers = all_linkifiers[realm.id]
self.assertEqual(len(zulip_linkifiers), 1)
self.assertDictEqual(
zulip_linkifiers[0],
LinkifierDict(
pattern="#(?P<id>[0-9]{2,8})",
url_format="https://trac.example.com/ticket/%(id)s",
id=linkifier.id,
),
)
def test_flush_linkifier(self) -> None: def test_flush_linkifier(self) -> None:
realm = get_realm("zulip") realm = get_realm("zulip")