Skip to content

Commit

Permalink
Add a cache-baced 'lookahead' to know whether it's worth hitting the …
Browse files Browse the repository at this point in the history
…CMS for a page

...because if the page isn't in the lookahead, we can avoid touching the DB
just to ultimately return a 404
  • Loading branch information
stevejalim committed Jan 13, 2025
1 parent fce56d5 commit 845f97e
Show file tree
Hide file tree
Showing 10 changed files with 450 additions and 5 deletions.
16 changes: 16 additions & 0 deletions bedrock/cms/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,24 @@
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

from django.apps import AppConfig
from django.db import connection
from django.db.models.signals import post_migrate


class CmsConfig(AppConfig):
default_auto_field = "django.db.models.BigAutoField"
name = "bedrock.cms"

def ready(self):
import bedrock.cms.signal_handlers # noqa: F401
from bedrock.cms.utils import warm_page_path_cache

if "wagtailcore_page" in connection.introspection.table_names():
# The route to take if the DB already exists in a viable state
warm_page_path_cache()
else:
# The route to take the DB isn't ready yet (eg tests or an empty DB)
post_migrate.connect(
bedrock.cms.signal_handlers.trigger_cache_warming,
sender=self,
)
34 changes: 34 additions & 0 deletions bedrock/cms/bedrock_wagtail_urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

# Custom version of wagtail_urls that wraps the wagtail_serve route
# with a decorator that does a lookahead to see if Wagtail will 404 or not
# (based on a precomputed cache of URLs in the CMS)

from django.urls import re_path

from wagtail.urls import urlpatterns as wagtail_urlpatterns
from wagtail.views import serve as wagtail_serve

from bedrock.cms.decorators import pre_check_for_cms_404

# Modify the wagtail_urlpatterns to replace `wagtail_serve` with a decorated
# version of the same view, so we can pre-empt Wagtail looking up a page
# that we know will be a 404
custom_wagtail_urls = []

for pattern in wagtail_urlpatterns:
if hasattr(pattern.callback, "__name__") and pattern.callback.__name__ == "serve":
custom_wagtail_urls.append(
re_path(
# This is a RegexPattern not a RoutePattern, which is why we use re_path not path
pattern.pattern,
pre_check_for_cms_404(wagtail_serve),
name=pattern.name,
)
)
else:
custom_wagtail_urls.append(pattern)

# Note: custom_wagtail_urls is imported into the main project urls.py instead of wagtail_urls
30 changes: 30 additions & 0 deletions bedrock/cms/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,23 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

import logging
from functools import wraps

from django.conf import settings
from django.http import Http404

from wagtail.views import serve as wagtail_serve

from bedrock.base.i18n import remove_lang_prefix
from bedrock.cms.utils import path_exists_in_cms
from lib.l10n_utils.fluent import get_active_locales

from .utils import get_cms_locales_for_path

logger = logging.getLogger(__name__)


HTTP_200_OK = 200


Expand Down Expand Up @@ -176,3 +182,27 @@ def wrapped_view(request, *args, **kwargs):
else:
# Otherwise, apply the decorator directly to view_func
return decorator(view_func)


def pre_check_for_cms_404(view):
"""
Decorator intended to avoid going through the Wagtail's serve view
for a route that we know will be a 404. How do we know? We have a
pre-warmed cache of all the pages of _live_ pages known to Wagtail
- see bedrock.cms.utils for that.
This decorator can be skipped if settings.CMS_DO_PAGE_PATH_PRECHECK is
set to False via env vars.
"""

def wrapped_view(request, *args, **kwargs):
_path_to_check = request.path
if settings.CMS_DO_PAGE_PATH_PRECHECK:
if not path_exists_in_cms(_path_to_check):
logger.info(f"Raising early 404 for {_path_to_check} because it doesn't exist in the CMS")
raise Http404

# Proceed to the original view
return view(request, *args, **kwargs)

return wrapped_view
99 changes: 99 additions & 0 deletions bedrock/cms/signal_handlers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

import logging
from typing import TYPE_CHECKING, Type

from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.mail import send_mail
from django.template.loader import render_to_string

from wagtail.signals import page_published, page_unpublished, post_page_move
from wagtail_localize_smartling.signals import translation_imported

from bedrock.cms.utils import warm_page_path_cache

if TYPE_CHECKING:
from wagtail_localize.models import Translation
from wagtail_localize_smartling.models import Job


logger = logging.getLogger(__name__)


def notify_of_imported_translation(
sender: Type["Job"],
instance: "Job",
translation: "Translation",
**kwargs,
):
"""
Signal handler for receiving news that a translation has landed from
Smartling.
For now, sends a notification email to all Admins
"""
UserModel = get_user_model()

admin_emails = UserModel.objects.filter(
is_superuser=True,
is_active=True,
).values_list("email", flat=True)
admin_emails = [email for email in admin_emails if email] # Safety check to ensure no empty email addresses are included

if not admin_emails:
logger.warning("Unable to send translation-imported email alerts: no admins in system")
return

email_subject = "New translations imported into Bedrock CMS"

job_name = instance.name
translation_source_name = str(instance.translation_source.get_source_instance())

smartling_cms_dashboard_url = f"{settings.WAGTAILADMIN_BASE_URL}/cms-admin/smartling-jobs/inspect/{instance.pk}/"

email_body = render_to_string(
template_name="cms/email/notifications/translation_imported__body.txt",
context={
"job_name": job_name,
"translation_source_name": translation_source_name,
"translation_target_language_code": translation.target_locale.language_code,
"smartling_cms_dashboard_url": smartling_cms_dashboard_url,
},
)

send_mail(
subject=email_subject,
message=email_body,
from_email=settings.DEFAULT_FROM_EMAIL,
recipient_list=admin_emails,
)
logger.info(f"Translation-imported notification sent to {len(admin_emails)} admins")


translation_imported.connect(notify_of_imported_translation, weak=False)


def trigger_cache_warming(sender, **kwargs):
# Run after the post-migrate signal is sent for the `cms` app
warm_page_path_cache()


def rebuild_path_cache_after_page_move(sender, **kwargs):
# Check if a page has moved up or down within the tree
# (rather than just being reordered). If it has really moved
# then we should update the cache
if kwargs["url_path_before"] == kwargs["url_path_after"]:
# No URLs are changing - nothing to do here!
return

# The page is moving, so we need to rebuild the entire pre-empting-lookup cache
warm_page_path_cache()


post_page_move.connect(rebuild_path_cache_after_page_move)

page_published.connect(trigger_cache_warming)
page_unpublished.connect(trigger_cache_warming)
25 changes: 25 additions & 0 deletions bedrock/cms/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import pytest
import wagtail_factories
from wagtail.contrib.redirects.models import Redirect
from wagtail.models import Locale, Site

from bedrock.cms.tests.factories import LocaleFactory, SimpleRichTextPageFactory
Expand Down Expand Up @@ -74,8 +75,14 @@ def tiny_localized_site():
site = Site.objects.get(is_default_site=True)

en_us_root_page = site.root_page

fr_root_page = en_us_root_page.copy_for_translation(fr_locale)
rev = fr_root_page.save_revision()
fr_root_page.publish(rev)

pt_br_root_page = en_us_root_page.copy_for_translation(pt_br_locale)
rev = pt_br_root_page.save_revision()
pt_br_root_page.publish(rev)

en_us_homepage = SimpleRichTextPageFactory(
title="Test Page",
Expand Down Expand Up @@ -148,3 +155,21 @@ def tiny_localized_site():
assert fr_homepage.live is True
assert fr_child.live is True
assert fr_grandchild.live is True


@pytest.fixture
def tiny_localized_site_redirects():
"""Some test redirects that complement the tiny_localized_site fixture.
Useful for things like the tests for the cache-based lookup
in bedrock.cms.tests.test_utils.test_path_exists_in_cms
"""

Redirect.add_redirect(
old_path="/fr/moved-page/",
redirect_to="/fr/test-page/child-page/",
)
Redirect.add_redirect(
old_path="/en-US/deeper/nested/moved-page/",
redirect_to="/fr/test-page/",
)
58 changes: 57 additions & 1 deletion bedrock/cms/tests/test_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
# file, You can obtain one at https://mozilla.org/MPL/2.0/.


from django.http import Http404, HttpResponse
from django.urls import path

import pytest
from wagtail.rich_text import RichText

from bedrock.base.i18n import bedrock_i18n_patterns
from bedrock.cms.decorators import prefer_cms
from bedrock.cms.decorators import pre_check_for_cms_404, prefer_cms
from bedrock.cms.tests import decorator_test_views
from bedrock.urls import urlpatterns as mozorg_urlpatterns

Expand Down Expand Up @@ -448,3 +449,58 @@ def test_prefer_cms_rejects_invalid_setup(mocker, config, expect_exeption):
prefer_cms(view_func=fake_view, **config)
else:
prefer_cms(view_func=fake_view, **config)


def dummy_view(request, *args, **kwargs):
return HttpResponse("Hello, world!")


@pytest.mark.parametrize("pretend_that_path_exists", [True, False])
def test_pre_check_for_cms_404(
pretend_that_path_exists,
mocker,
rf,
settings,
client,
tiny_localized_site,
):
settings.CMS_DO_PAGE_PATH_PRECHECK = True
mocked_view = mocker.spy(dummy_view, "__call__") # Spy on the test view
mocked_path_exists_in_cms = mocker.patch("bedrock.cms.decorators.path_exists_in_cms")
mocked_path_exists_in_cms.return_value = pretend_that_path_exists

decorated_view = pre_check_for_cms_404(mocked_view)
request = rf.get("/path/is/irrelevant/because/we/are/mocking/path_exists_in_cms")

if pretend_that_path_exists:
response = decorated_view(request)
# Assert: Verify the original view was called
mocked_view.assert_called_once_with(request)
assert response.content == b"Hello, world!"
else:
with pytest.raises(Http404): # Expect an Http404 since path_exists_in_cms returns False
decorated_view(request)
mocked_view.assert_not_called()


@pytest.mark.parametrize("pretend_that_path_exists", [True, False])
def test_pre_check_for_cms_404__show_can_be_disabled_with_settings(
pretend_that_path_exists,
mocker,
rf,
settings,
client,
tiny_localized_site,
):
settings.CMS_DO_PAGE_PATH_PRECHECK = False
mocked_view = mocker.spy(dummy_view, "__call__") # Spy on the test view
mocked_path_exists_in_cms = mocker.patch("bedrock.cms.decorators.path_exists_in_cms")
mocked_path_exists_in_cms.return_value = pretend_that_path_exists

decorated_view = pre_check_for_cms_404(mocked_view)
request = rf.get("/path/is/irrelevant/because/we/are/mocking/path_exists_in_cms")

# The fake view will always be called because the pre-check isn't being used
response = decorated_view(request)
mocked_view.assert_called_once_with(request)
assert response.content == b"Hello, world!"
Loading

0 comments on commit 845f97e

Please sign in to comment.