Oh hai!
\n"), ), ) @@ -84,13 +93,23 @@ def test_changelog_api_view__prerenders_markup( markdown: Optional[str], markup: str, ) -> None: + cache = get_cache("markdown_render") v = PackageVersionFactory(changelog=markdown) + assert cache.get(f"rendered_html:changelog:{v.id}") is None + response = api_client.get( f"/api/cyberstorm/package/{v.package.namespace}/{v.package.name}/latest/changelog/", ) actual = response.json() + if markup == "": + # We ignore empty strings and dont render them so no need for cache + assert cache.get(f"rendered_html:changelog:{v.id}") == None + else: + assert cache.get(f"rendered_html:changelog:{v.id}") == markup + assert cache.get(f"rendering_status:changelog:{v.id}") is None + assert cache.get(f"lock.rendered_html:changelog:{v.id}") is None assert actual["html"] == markup diff --git a/django/thunderstore/api/cyberstorm/views/markdown.py b/django/thunderstore/api/cyberstorm/views/markdown.py index 548940615..040784a48 100644 --- a/django/thunderstore/api/cyberstorm/views/markdown.py +++ b/django/thunderstore/api/cyberstorm/views/markdown.py @@ -5,8 +5,8 @@ from rest_framework.generics import RetrieveAPIView, get_object_or_404 from thunderstore.api.utils import CyberstormAutoSchemaMixin -from thunderstore.markdown.templatetags.markdownify import render_markdown from thunderstore.repository.models import Package, PackageVersion +from thunderstore.repository.services.markdown import render_markdown_service class CyberstormMarkdownResponseSerializer(serializers.Serializer): @@ -29,7 +29,11 @@ def get_object(self): version_number=self.kwargs.get("version_number"), ) - return {"html": render_markdown(package_version.readme)} + return render_markdown_service( + markdown=package_version.readme, + key="readme", + object_id=package_version.id, + ) class PackageVersionChangelogAPIView(CyberstormAutoSchemaMixin, RetrieveAPIView): @@ -48,10 +52,15 @@ def get_object(self): version_number=self.kwargs.get("version_number"), ) - if package_version.changelog is None: - raise Http404 + changelog = package_version.changelog + if changelog is None: + raise Http404("CHANGELOG not found for this package version.") - return {"html": render_markdown(package_version.changelog)} + return render_markdown_service( + markdown=changelog, + key="changelog", + object_id=package_version.id, + ) def get_package_version( diff --git a/django/thunderstore/core/settings.py b/django/thunderstore/core/settings.py index fe898eeeb..5c44fee88 100644 --- a/django/thunderstore/core/settings.py +++ b/django/thunderstore/core/settings.py @@ -109,6 +109,7 @@ REDIS_URL_LEGACY=(str, None), REDIS_URL_PROFILES=(str, None), REDIS_URL_DOWNLOADS=(str, None), + REDIS_MARKDOWN_RENDER=(str, None), DB_CERT_DIR=(str, ""), DB_CLIENT_CERT=(str, ""), DB_CLIENT_KEY=(str, ""), @@ -390,6 +391,7 @@ class CeleryQueues: BackgroundCache = "background.cache" BackgroundTask = "background.task" BackgroundLongRunning = "background.long_running" + BackgroundMarkdownRender = "background.markdown_render" CELERY_BROKER_URL = env.str("CELERY_BROKER_URL") @@ -509,6 +511,10 @@ def get_redis_cache(env_key: str, fallback_key: Optional[str] = None): **get_redis_cache("REDIS_URL_DOWNLOADS", "REDIS_URL"), "TIMEOUT": None, }, + "markdown_render": { + **get_redis_cache("REDIS_MARKDOWN_RENDER", "REDIS_URL"), + "TIMEOUT": None, + }, } diff --git a/django/thunderstore/core/tests/test_celery.py b/django/thunderstore/core/tests/test_celery.py index e62c1db04..184fcb1b8 100644 --- a/django/thunderstore/core/tests/test_celery.py +++ b/django/thunderstore/core/tests/test_celery.py @@ -40,6 +40,7 @@ def test_task(): "thunderstore.repository.tasks.process_package_submission", "thunderstore.repository.tasks.cleanup_package_submissions", "thunderstore.repository.tasks.log_version_download", + "thunderstore.repository.tasks.render_markdown", "thunderstore.webhooks.tasks.process_audit_event", ) diff --git a/django/thunderstore/repository/services/__init__.py b/django/thunderstore/repository/services/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/django/thunderstore/repository/services/markdown.py b/django/thunderstore/repository/services/markdown.py new file mode 100644 index 000000000..155620447 --- /dev/null +++ b/django/thunderstore/repository/services/markdown.py @@ -0,0 +1,34 @@ +from celery.result import AsyncResult + +from thunderstore.cache.utils import get_cache +from thunderstore.repository.tasks.markdown import render_markdown_to_html + +cache = get_cache("markdown_render") + + +def render_markdown_service(markdown: str, key: str, object_id: int) -> dict: + if markdown.strip() == "": + return {"html": ""} + + cache_key = f"rendered_html:{key}:{object_id}" + status_key = f"rendering_status:{key}:{object_id}" + + if (html := cache.get(cache_key)) is not None: + return {"html": html} + + if task_id := cache.get(status_key): + task = AsyncResult(id=task_id) + else: + task = render_markdown_to_html.delay(markdown=markdown, cache_key=cache_key) + cache.set(status_key, task.id, timeout=300) + + try: + result = task.get(timeout=5) + cache.delete(status_key) + return {"html": result} + except TimeoutError: + cache.delete(status_key) + raise TimeoutError("Markdown rendering task timed out.") + except Exception as error: + cache.delete(status_key) + raise error diff --git a/django/thunderstore/repository/tasks/markdown.py b/django/thunderstore/repository/tasks/markdown.py new file mode 100644 index 000000000..ecdd34ffb --- /dev/null +++ b/django/thunderstore/repository/tasks/markdown.py @@ -0,0 +1,28 @@ +from celery import shared_task + +from thunderstore.cache.utils import get_cache +from thunderstore.core.settings import CeleryQueues +from thunderstore.markdown.templatetags.markdownify import render_markdown + +cache = get_cache("markdown_render") + + +@shared_task( + queue=CeleryQueues.BackgroundMarkdownRender, + name="thunderstore.repository.tasks.render_markdown", +) +def render_markdown_to_html( + markdown: str, + cache_key: str, +) -> str: + cached_html = cache.get(cache_key) + if cached_html is not None: + return cached_html + + try: + html = render_markdown(markdown) + except Exception as error: + raise error + + cache.set(cache_key, html) + return html diff --git a/django/thunderstore/repository/tasks/tests/test_markdown.py b/django/thunderstore/repository/tasks/tests/test_markdown.py new file mode 100644 index 000000000..63f287625 --- /dev/null +++ b/django/thunderstore/repository/tasks/tests/test_markdown.py @@ -0,0 +1,36 @@ +from unittest.mock import patch + +import pytest + +from thunderstore.cache.utils import get_cache +from thunderstore.repository.tasks.markdown import render_markdown_to_html + +RENDER_MARKDOWN_PATH = "thunderstore.markdown.templatetags.markdownify.render_markdown" + + +def test_markdown_cache_hit_returns_html(): + cache = get_cache("markdown_render") + cache_key = "rendered_html:test_key:1" + cache.set(cache_key, "Cached HTML
") + + result = render_markdown_to_html("", cache_key) + assert result == "Cached HTML
" + + +def test_markdown_cache_miss_triggers_rendering_task(): + cache = get_cache("markdown_render") + cache_key = "rendered_html:test_key:1" + + result = render_markdown_to_html("test markdown", cache_key) + assert result == "test markdown
\n" + assert cache.get(cache_key) == "test markdown
\n" + + +def test_markdown_rendering_exception(): + cache = get_cache("markdown_render") + cache_key = "rendered_html:test_key:1" + + with pytest.raises(Exception): + render_markdown_to_html(None, cache_key) + + assert cache.get(cache_key) is None diff --git a/django/thunderstore/repository/tests/test_markdown_service.py b/django/thunderstore/repository/tests/test_markdown_service.py new file mode 100644 index 000000000..85c9e6339 --- /dev/null +++ b/django/thunderstore/repository/tests/test_markdown_service.py @@ -0,0 +1,63 @@ +from unittest.mock import patch + +import pytest + +from thunderstore.cache.utils import get_cache +from thunderstore.repository.services.markdown import render_markdown_service + + +@pytest.mark.django_db +@pytest.mark.parametrize("markdown, expected_html", [("", ""), (" ", "")]) +def test_render_markdown_empty_input(markdown, expected_html): + result = render_markdown_service(markdown, "changelog", 1) + assert result == {"html": expected_html} + + +@pytest.mark.django_db +def test_render_markdown_cached_html(package_version): + cache = get_cache("markdown_render") + cache_key = f"rendered_html:changelog:{package_version.id}" + cache.set(cache_key, "Cached HTML
") + + result = render_markdown_service( + package_version.changelog, "changelog", package_version.id + ) + assert result == {"html": "Cached HTML
"} + + +@pytest.mark.django_db +def test_render_markdown_no_cached_html(package_version): + cache = get_cache("markdown_render") + cache_key = f"rendered_html:changelog:{package_version.id}" + cache.delete(cache_key) + + result = render_markdown_service( + package_version.changelog, "changelog", package_version.id + ) + assert result == {"html": "