Slow cyberstorm api endpoints (TS-2516)

django/thunderstore/api/cyberstorm/tests/test_markdown.py

@@ -5,6 +5,7 @@
 from rest_framework.test import APIClient
 
 from thunderstore.api.cyberstorm.views.markdown import get_package_version
+from thunderstore.cache.utils import get_cache
 from thunderstore.repository.factories import PackageVersionFactory
 from thunderstore.repository.models import Package
 
@@ -61,21 +62,29 @@ def test_get_package_version__raises_for_inactive_package_version(
 
 @pytest.mark.django_db
 def test_readme_api_view__prerenders_markup(api_client: APIClient) -> None:
+    cache = get_cache("markdown_render")
     v = PackageVersionFactory(readme="# Very **strong** header")
 
+    assert cache.get(f"rendered_html:readme:{v.id}") is None
+
     response = api_client.get(
         f"/api/cyberstorm/package/{v.package.namespace}/{v.package.name}/latest/readme/",
     )
     actual = response.json()
+    expected_html = "<h1>Very <strong>strong</strong> header</h1>\n"
 
-    assert actual["html"] == "<h1>Very <strong>strong</strong> header</h1>\n"
+    assert cache.get(f"rendered_html:readme:{v.id}") == expected_html
+    assert cache.get(f"rendering_status:readme:{v.id}") is None
+    assert cache.get(f"lock.rendered_html:readme:{v.id}") is None
+    assert actual["html"] == expected_html
 
 
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     ("markdown", "markup"),
     (
         ("", ""),
+        ("   ", ""),
         ("Oh hai!", "<p>Oh hai!</p>\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
 
 

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(

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,
+    },
 }
 
 

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",
 )
 

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

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

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, "<p>Cached HTML</p>")
+
+    result = render_markdown_to_html("", cache_key)
+    assert result == "<p>Cached HTML</p>"
+
+
+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 == "<p>test markdown</p>\n"
+    assert cache.get(cache_key) == "<p>test markdown</p>\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

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, "<p>Cached HTML</p>")
+
+    result = render_markdown_service(
+        package_version.changelog, "changelog", package_version.id
+    )
+    assert result == {"html": "<p>Cached HTML</p>"}
+
+
+@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": "<h1>This is an example changelog</h1>\n"}
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "side_effect,expected_exception,exception_message",
+    [
+        (TimeoutError, TimeoutError, "Markdown rendering task timed out."),
+        (Exception, Exception, None),
+    ],
+)
+def test_render_markdown_exceptions(
+    package_version, side_effect, expected_exception, exception_message
+):
+    cache = get_cache("markdown_render")
+    status_key = f"rendering_status:changelog:{package_version.id}"
+    task_id = "mock-task-id"
+    cache.set(status_key, task_id)
+
+    get_path = "celery.result.AsyncResult.get"
+    id_path = "celery.result.AsyncResult.id"
+    with patch(get_path, side_effect=side_effect), patch(id_path, return_value=task_id):
+        with pytest.raises(expected_exception, match=exception_message):
+            render_markdown_service(
+                package_version.changelog, "changelog", package_version.id
+            )
+        assert cache.get(status_key) is None

docker-compose.yml

@@ -8,12 +8,13 @@ x-django-service: &django-service
       BUILD_INSTALL_EXTRAS: ${BUILD_INSTALL_EXTRAS}
   environment:
     CELERY_BROKER_URL: "pyamqp://django:django@rabbitmq/django"
-    CELERY_QUEUES: "celery,background.cache,background.task,background.long_running,log.downloads"
+    CELERY_QUEUES: "celery,background.cache,background.task,background.long_running,log.downloads,background.markdown_render"
     DATABASE_URL: "psql://django:django@dbpool/django"
     REDIS_URL: "redis://redis:6379/0"
     REDIS_URL_LEGACY: "redis://redis:6379/1"
     REDIS_URL_PROFILES: "redis://redis:6379/2"
     REDIS_URL_DOWNLOADS: "redis://redis:6379/3"
+    REDIS_MARKDOWN_RENDER: "redis://redis:6379/4"
     PROTOCOL: "http://"
 
     AWS_ACCESS_KEY_ID: "thunderstore"