This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8497 in repository https://gitbox.apache.org/repos/asf/allura.git
commit 242ce6427c593854921aa5f536b514ae0f6f9c93 Author: Dave Brondsema <[email protected]> AuthorDate: Tue Feb 7 09:48:49 2023 -0500 [#8497] rearrange ForgeMarkdown so it only instantiates a full Markdown+extensions class when needed (cached_convert often does not need it) --- Allura/allura/lib/app_globals.py | 48 ++++++++++++++++++++++-------------- Allura/allura/tasks/mail_tasks.py | 6 ++--- Allura/allura/tests/test_globals.py | 6 ++--- Allura/allura/tests/test_markdown.py | 4 ++- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/Allura/allura/lib/app_globals.py b/Allura/allura/lib/app_globals.py index 7fe9ffb16..8a004d6b0 100644 --- a/Allura/allura/lib/app_globals.py +++ b/Allura/allura/lib/app_globals.py @@ -74,7 +74,26 @@ __all__ = ['Globals'] log = logging.getLogger(__name__) -class ForgeMarkdown(markdown.Markdown): +class ForgeMarkdown: + + def __init__(self, **forge_ext_kwargs): + self.forge_ext_kwargs = forge_ext_kwargs + + def make_markdown_instance(self, **forge_ext_kwargs): + # for further performance improvements, this might be cachable including forge_ext_kwargs? + # but ForgeExtension and the classes it makes often save things on `self` which might not be thread safe + # when multiple threads are using the same instance + # TestCachedMarkdown re-uses the same instance previously, and had a bug that AddCustomClass wasn't running + return markdown.Markdown( + extensions=['markdown.extensions.fenced_code', 'markdown.extensions.codehilite', + 'markdown.extensions.abbr', 'markdown.extensions.def_list', 'markdown.extensions.footnotes', + 'markdown.extensions.md_in_html', + ForgeExtension(**forge_ext_kwargs), + EmojiExtension(), + UserMentionExtension(), + 'markdown.extensions.tables', 'markdown.extensions.toc', 'markdown.extensions.nl2br', + 'markdown_checklist.extension'], + output_format='html') def convert(self, source, render_limit=True): if render_limit and len(source) > asint(config.get('markdown_render_max_length', 80000)): @@ -84,7 +103,7 @@ class ForgeMarkdown(markdown.Markdown): escaped = html.escape(h.really_unicode(source)) return Markup('<pre>%s</pre>' % escaped) try: - return super().convert(source) + return self.make_markdown_instance(**self.forge_ext_kwargs).convert(source) except Exception: log.info('Invalid markdown: %s Upwards trace is %s', source, ''.join(traceback.format_stack()), exc_info=True) @@ -460,29 +479,18 @@ class Globals: else: return Markup(pygments.highlight(text, lexer, formatter)) - def forge_markdown(self, **kwargs): - '''return a markdown.Markdown object on which you can call convert''' - return ForgeMarkdown( - extensions=['markdown.extensions.fenced_code', 'markdown.extensions.codehilite', - 'markdown.extensions.abbr', 'markdown.extensions.def_list', 'markdown.extensions.footnotes', - 'markdown.extensions.md_in_html', - ForgeExtension(**kwargs), EmojiExtension(), UserMentionExtension(), - 'markdown.extensions.tables', 'markdown.extensions.toc', 'markdown.extensions.nl2br', - 'markdown_checklist.extension'], - output_format='html') - @property def markdown(self): - return self.forge_markdown() + return ForgeMarkdown() @property def markdown_wiki(self): if c.project and c.project.is_nbhd_project: - return self.forge_markdown(wiki=True, macro_context='neighborhood-wiki') + return ForgeMarkdown(wiki=True, macro_context='neighborhood-wiki') elif c.project and c.project.is_user_project: - return self.forge_markdown(wiki=True, macro_context='userproject-wiki') + return ForgeMarkdown(wiki=True, macro_context='userproject-wiki') else: - return self.forge_markdown(wiki=True) + return ForgeMarkdown(wiki=True) @property def markdown_commit(self): @@ -490,8 +498,10 @@ class Globals: """ app = getattr(c, 'app', None) - return ForgeMarkdown(extensions=[CommitMessageExtension(app), EmojiExtension(), 'markdown.extensions.nl2br'], - output_format='html') + return markdown.Markdown( + extensions=[CommitMessageExtension(app), EmojiExtension(), 'markdown.extensions.nl2br'], + output_format='html', + ) @property def production_mode(self): diff --git a/Allura/allura/tasks/mail_tasks.py b/Allura/allura/tasks/mail_tasks.py index e2b2a0a15..8aae497c4 100644 --- a/Allura/allura/tasks/mail_tasks.py +++ b/Allura/allura/tasks/mail_tasks.py @@ -16,7 +16,6 @@ # under the License. import html import logging -import six.moves.html_parser import re from tg import tmpl_context as c, app_globals as g, config @@ -27,7 +26,7 @@ from allura.lib import helpers as h from allura.lib.decorators import task from allura.lib import mail_util from allura.lib import exceptions as exc -import six + log = logging.getLogger(__name__) @@ -113,6 +112,7 @@ def create_multipart_msg(text, metalink=None): :param metalink: :return: """ + from allura.lib.app_globals import ForgeMarkdown def replace_html(matchobj): text_within_div = matchobj.group(1) @@ -129,7 +129,7 @@ def create_multipart_msg(text, metalink=None): plain_text = html.unescape(plain_text) # put literal HTML tags back into plaintext plain_msg = mail_util.encode_email_part(plain_text, 'plain') - html_text = g.forge_markdown(email=True).convert(text) + html_text = ForgeMarkdown(email=True).convert(text) if metalink: html_text = html_text + mail_meta_content(metalink) html_msg = mail_util.encode_email_part(html_text, 'html') diff --git a/Allura/allura/tests/test_globals.py b/Allura/allura/tests/test_globals.py index b0e4d56af..9b7497890 100644 --- a/Allura/allura/tests/test_globals.py +++ b/Allura/allura/tests/test_globals.py @@ -482,7 +482,7 @@ class Test(): </code></pre></div> </div>''') assert ( - g.forge_markdown(email=True).convert('[Home]') == + ForgeMarkdown(email=True).convert('[Home]') == # uses localhost: '<div class="markdown_content"><p><a class="alink" href="http://localhost/p/test/wiki/Home/";>[Home]</a></p></div>') assert g.markdown.convert(dedent('''\ @@ -760,7 +760,7 @@ class TestCachedMarkdown(unittest.TestCase): self.md = ForgeMarkdown() self.post = M.Post() self.post.text = '**bold**' - self.expected_html = '<p><strong>bold</strong></p>' + self.expected_html = '<div class="markdown_content"><p><strong>bold</strong></p></div>' def test_bad_source_field_name(self): self.assertRaises(AttributeError, self.md.cached_convert, @@ -774,7 +774,7 @@ class TestCachedMarkdown(unittest.TestCase): @patch.dict('allura.lib.app_globals.config', markdown_cache_threshold='-0.01') def test_non_ascii(self): self.post.text = 'å∫ç' - expected = '<p>å∫ç</p>' + expected = '<div class="markdown_content"><p>å∫ç</p></div>' # test with empty cache self.assertEqual(expected, self.md.cached_convert(self.post, 'text')) # test with primed cache diff --git a/Allura/allura/tests/test_markdown.py b/Allura/allura/tests/test_markdown.py index 992eec8b0..cd74a9c45 100644 --- a/Allura/allura/tests/test_markdown.py +++ b/Allura/allura/tests/test_markdown.py @@ -16,6 +16,8 @@ # under the License. import unittest + +import markdown import mock from allura.lib import markdown_extensions as mde @@ -140,7 +142,7 @@ Not *strong* or _underlined_.""" * <a href="/p/project/tool/2/tree/test.py#l3">source:test.py@2#L3</a></p> <p>Not *strong* or _underlined_.</div>""" - md = ForgeMarkdown( + md = markdown.Markdown( extensions=[mde.CommitMessageExtension(app), 'markdown.extensions.nl2br'], output_format='html') self.assertEqual(md.convert(text), expected_html)
