This is an automated email from the ASF dual-hosted git repository. kentontaylor pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/allura.git
commit 506eecadd07d369a1802a86e0701f7442266fe50 Author: Dave Brondsema <[email protected]> AuthorDate: Mon Mar 7 14:38:22 2022 -0500 make some markdown macros cacheable --- Allura/allura/lib/app_globals.py | 13 ++++++++--- Allura/allura/lib/macro.py | 45 +++++++++++++++++++++++++++++-------- Allura/allura/tests/test_globals.py | 28 ++++++++++++++++++++++- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/Allura/allura/lib/app_globals.py b/Allura/allura/lib/app_globals.py index 4325ab2..8487d6d 100644 --- a/Allura/allura/lib/app_globals.py +++ b/Allura/allura/lib/app_globals.py @@ -16,6 +16,8 @@ # under the License. from __future__ import annotations +import re + """The application's Globals object""" import logging @@ -60,6 +62,7 @@ from allura.eventslistener import PostEvent from allura.lib import gravatar, plugin, utils from allura.lib import helpers as h +from allura.lib.macro import uncacheable_macros_names from allura.lib.widgets import analytics from allura.lib.security import Credentials from allura.lib.solr import MockSOLR, make_solr_from_config @@ -90,6 +93,11 @@ class ForgeMarkdown(markdown.Markdown): return Markup("""<p><strong>ERROR!</strong> The markdown supplied could not be parsed correctly. Did you forget to surround a code snippet with "~~~~"?</p><pre>%s</pre>""" % escaped) + @LazyProperty + def uncacheable_macro_regex(self): + regex_names = '|'.join(uncacheable_macros_names()) + return re.compile(rf"\[\[\s*({regex_names})\b") + def cached_convert(self, artifact: MappedClass, field_name: str) -> str: """ Convert ``artifact.field_name`` markdown source to html, caching @@ -128,8 +136,7 @@ class ForgeMarkdown(markdown.Markdown): '"markdown_cache_threshold" must be a float.') # Check if contains macro and never cache - # TODO: more precise search for [[include or [[members etc (all _macros). Or even track if one ran or not - if "[[" in source_text: + if self.uncacheable_macro_regex.search(source_text): if render_time > float(config.get('markdown_cache_threshold.nocache', 0.5)): try: details = artifact.index_id() @@ -139,7 +146,7 @@ class ForgeMarkdown(markdown.Markdown): details += ' ' + artifact.url() except Exception: pass - log.info(f'Not saving markdown cache since [[ means it might have a dynamic macro. Took {render_time:.03}s on {details}') + log.info(f'Not saving markdown cache since it has a dynamic macro. Took {render_time:.03}s on {details}') return html if threshold is not None and render_time > threshold: diff --git a/Allura/allura/lib/macro.py b/Allura/allura/lib/macro.py index efccc97..a9ed3a0 100644 --- a/Allura/allura/lib/macro.py +++ b/Allura/allura/lib/macro.py @@ -15,10 +15,15 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import cgi import random import logging import traceback +from dataclasses import dataclass +from typing import Callable + import six.moves.urllib.parse import six.moves.urllib.request import six.moves.urllib.error @@ -41,16 +46,36 @@ import six log = logging.getLogger(__name__) -_macros = {} +@dataclass +class RegisteredMacro: + func: Callable + context: str + cacheable: bool -class macro: - def __init__(self, context=None): +_macros: dict[str, RegisteredMacro] = {} + + +def uncacheable_macros_names(): + return [name for name, m in _macros.items() if not m.cacheable] + + +class macro: + """ + Decorator to declare a function is a custom markdown macro + """ + + def __init__(self, context: str = None, cacheable: bool = False): + """ + :param context: either "neighborhood-wiki" or "userproject-wiki" to limit the macro to be used in those contexts + :param cacheable: indicates if its ok to cache the macro's output permanently + """ self._context = context + self._cacheable = cacheable def __call__(self, func): - _macros[func.__name__] = (func, self._context) + _macros[func.__name__] = RegisteredMacro(func, self._context, self._cacheable) return func @@ -87,9 +112,11 @@ class parse: return f'[[Error parsing {s}: {ex}]]' def _lookup_macro(self, s): - macro, context = _macros.get(s, (None, None)) - if context is None or context == self._context: - return macro + macro = _macros.get(s) + if not macro: + return None + elif macro.context is None or macro.context == self._context: + return macro.func else: return None @@ -394,7 +421,7 @@ def include(ref=None, repo=None, **kw): return response -@macro() +@macro(cacheable=True) def img(src=None, **kw): attrs = ('%s="%s"' % t for t in kw.items()) included = request.environ.setdefault('allura.macro.att_embedded', set()) @@ -439,7 +466,7 @@ def members(limit=20): return response -@macro() +@macro(cacheable=True) def embed(url=None): consumer = oembed.OEmbedConsumer() endpoint = oembed.OEmbedEndpoint('https://www.youtube.com/oembed', diff --git a/Allura/allura/tests/test_globals.py b/Allura/allura/tests/test_globals.py index 2e1ec28..6d1f51e 100644 --- a/Allura/allura/tests/test_globals.py +++ b/Allura/allura/tests/test_globals.py @@ -887,10 +887,36 @@ class TestCachedMarkdown(unittest.TestCase): self.assertEqual(html, self.expected_html) self.assertIsInstance(html, Markup) self.assertFalse(convert_func.called) - self.post.text = "text [[macro]] pass" + self.post.text = "text [[include]] pass" html = self.md.cached_convert(self.post, 'text') self.assertTrue(convert_func.called) + @patch.dict('allura.lib.app_globals.config', markdown_cache_threshold='-0.01') + def test_cacheable_macro(self): + # cachable + self.post.text = "text [[img src=...]] pass" + del self.post.text_cache + self.md.cached_convert(self.post, 'text') + assert self.post.text_cache.html + + # cachable, its not even a macro! + self.post.text = "text [[ blah" + del self.post.text_cache + self.md.cached_convert(self.post, 'text') + assert self.post.text_cache.html + + # not cacheable + self.post.text = "text [[include file=...]] pass" + del self.post.text_cache + self.md.cached_convert(self.post, 'text') + assert not self.post.text_cache.html + + # not cacheable + self.post.text = "text [[ \n include file=... ]] pass" + del self.post.text_cache + self.md.cached_convert(self.post, 'text') + assert not self.post.text_cache.html + @patch.dict('allura.lib.app_globals.config', {}) def test_no_threshold_defined(self): html = self.md.cached_convert(self.post, 'text')
