Skip to content

Commit df5b15d

Browse files
committed
Add a cache-baced 'lookahead' to know whether it's worth hitting the 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
1 parent fce56d5 commit df5b15d

File tree

10 files changed

+450
-5
lines changed

10 files changed

+450
-5
lines changed

bedrock/cms/apps.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,24 @@
33
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

55
from django.apps import AppConfig
6+
from django.db import connection
7+
from django.db.models.signals import post_migrate
68

79

810
class CmsConfig(AppConfig):
911
default_auto_field = "django.db.models.BigAutoField"
1012
name = "bedrock.cms"
13+
14+
def ready(self):
15+
import bedrock.cms.signal_handlers # noqa: F401
16+
from bedrock.cms.utils import warm_page_path_cache
17+
18+
if "wagtailcore_page" in connection.introspection.table_names():
19+
# The route to take if the DB already exists in a viable state
20+
warm_page_path_cache()
21+
else:
22+
# The route to take the DB isn't ready yet (eg tests or an empty DB)
23+
post_migrate.connect(
24+
bedrock.cms.signal_handlers.trigger_cache_warming,
25+
sender=self,
26+
)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# This Source Code Form is subject to the terms of the Mozilla Public
2+
# License, v. 2.0. If a copy of the MPL was not distributed with this
3+
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
# Custom version of wagtail_urls that wraps the wagtail_serve route
6+
# with a decorator that does a lookahead to see if Wagtail will 404 or not
7+
# (based on a precomputed cache of URLs in the CMS)
8+
9+
from django.urls import re_path
10+
11+
from wagtail.urls import urlpatterns as wagtail_urlpatterns
12+
from wagtail.views import serve as wagtail_serve
13+
14+
from bedrock.cms.decorators import pre_check_for_cms_404
15+
16+
# Modify the wagtail_urlpatterns to replace `wagtail_serve` with a decorated
17+
# version of the same view, so we can pre-empt Wagtail looking up a page
18+
# that we know will be a 404
19+
custom_wagtail_urls = []
20+
21+
for pattern in wagtail_urlpatterns:
22+
if hasattr(pattern.callback, "__name__") and pattern.callback.__name__ == "serve":
23+
custom_wagtail_urls.append(
24+
re_path(
25+
# This is a RegexPattern not a RoutePattern, which is why we use re_path not path
26+
pattern.pattern,
27+
pre_check_for_cms_404(wagtail_serve),
28+
name=pattern.name,
29+
)
30+
)
31+
else:
32+
custom_wagtail_urls.append(pattern)
33+
34+
# Note: custom_wagtail_urls is imported into the main project urls.py instead of wagtail_urls

bedrock/cms/decorators.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,23 @@
22
# License, v. 2.0. If a copy of the MPL was not distributed with this
33
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5+
import logging
56
from functools import wraps
67

8+
from django.conf import settings
79
from django.http import Http404
810

911
from wagtail.views import serve as wagtail_serve
1012

1113
from bedrock.base.i18n import remove_lang_prefix
14+
from bedrock.cms.utils import path_exists_in_cms
1215
from lib.l10n_utils.fluent import get_active_locales
1316

1417
from .utils import get_cms_locales_for_path
1518

19+
logger = logging.getLogger(__name__)
20+
21+
1622
HTTP_200_OK = 200
1723

1824

@@ -176,3 +182,27 @@ def wrapped_view(request, *args, **kwargs):
176182
else:
177183
# Otherwise, apply the decorator directly to view_func
178184
return decorator(view_func)
185+
186+
187+
def pre_check_for_cms_404(view):
188+
"""
189+
Decorator intended to avoid going through the Wagtail's serve view
190+
for a route that we know will be a 404. How do we know? We have a
191+
pre-warmed cache of all the pages of _live_ pages known to Wagtail
192+
- see bedrock.cms.utils for that.
193+
194+
This decorator can be skipped if settings.CMS_DO_PAGE_PATH_PRECHECK is
195+
set to False via env vars.
196+
"""
197+
198+
def wrapped_view(request, *args, **kwargs):
199+
_path_to_check = request.path
200+
if settings.CMS_DO_PAGE_PATH_PRECHECK:
201+
if not path_exists_in_cms(_path_to_check):
202+
logger.info(f"Raising early 404 for {_path_to_check} because it doesn't exist in the CMS")
203+
raise Http404
204+
205+
# Proceed to the original view
206+
return view(request, *args, **kwargs)
207+
208+
return wrapped_view

bedrock/cms/signal_handlers.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# This Source Code Form is subject to the terms of the Mozilla Public
2+
# License, v. 2.0. If a copy of the MPL was not distributed with this
3+
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
import logging
6+
from typing import TYPE_CHECKING, Type
7+
8+
from django.conf import settings
9+
from django.contrib.auth import get_user_model
10+
from django.core.mail import send_mail
11+
from django.template.loader import render_to_string
12+
13+
from wagtail.signals import page_published, page_unpublished, post_page_move
14+
from wagtail_localize_smartling.signals import individual_translation_imported
15+
16+
from bedrock.cms.utils import warm_page_path_cache
17+
18+
if TYPE_CHECKING:
19+
from wagtail_localize.models import Translation
20+
from wagtail_localize_smartling.models import Job
21+
22+
23+
logger = logging.getLogger(__name__)
24+
25+
26+
def notify_of_imported_translation(
27+
sender: Type["Job"],
28+
instance: "Job",
29+
translation: "Translation",
30+
**kwargs,
31+
):
32+
"""
33+
Signal handler for receiving news that a translation has landed from
34+
Smartling.
35+
36+
For now, sends a notification email to all Admins
37+
"""
38+
UserModel = get_user_model()
39+
40+
admin_emails = UserModel.objects.filter(
41+
is_superuser=True,
42+
is_active=True,
43+
).values_list("email", flat=True)
44+
admin_emails = [email for email in admin_emails if email] # Safety check to ensure no empty email addresses are included
45+
46+
if not admin_emails:
47+
logger.warning("Unable to send translation-imported email alerts: no admins in system")
48+
return
49+
50+
email_subject = "New translations imported into Bedrock CMS"
51+
52+
job_name = instance.name
53+
translation_source_name = str(instance.translation_source.get_source_instance())
54+
55+
smartling_cms_dashboard_url = f"{settings.WAGTAILADMIN_BASE_URL}/cms-admin/smartling-jobs/inspect/{instance.pk}/"
56+
57+
email_body = render_to_string(
58+
template_name="cms/email/notifications/individual_translation_imported__body.txt",
59+
context={
60+
"job_name": job_name,
61+
"translation_source_name": translation_source_name,
62+
"translation_target_language_code": translation.target_locale.language_code,
63+
"smartling_cms_dashboard_url": smartling_cms_dashboard_url,
64+
},
65+
)
66+
67+
send_mail(
68+
subject=email_subject,
69+
message=email_body,
70+
from_email=settings.DEFAULT_FROM_EMAIL,
71+
recipient_list=admin_emails,
72+
)
73+
logger.info(f"Translation-imported notification sent to {len(admin_emails)} admins")
74+
75+
76+
individual_translation_imported.connect(notify_of_imported_translation, weak=False)
77+
78+
79+
def trigger_cache_warming(sender, **kwargs):
80+
# Run after the post-migrate signal is sent for the `cms` app
81+
warm_page_path_cache()
82+
83+
84+
def rebuild_path_cache_after_page_move(sender, **kwargs):
85+
# Check if a page has moved up or down within the tree
86+
# (rather than just being reordered). If it has really moved
87+
# then we should update the cache
88+
if kwargs["url_path_before"] == kwargs["url_path_after"]:
89+
# No URLs are changing - nothing to do here!
90+
return
91+
92+
# The page is moving, so we need to rebuild the entire pre-empting-lookup cache
93+
warm_page_path_cache()
94+
95+
96+
post_page_move.connect(rebuild_path_cache_after_page_move)
97+
98+
page_published.connect(trigger_cache_warming)
99+
page_unpublished.connect(trigger_cache_warming)

bedrock/cms/tests/conftest.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import pytest
66
import wagtail_factories
7+
from wagtail.contrib.redirects.models import Redirect
78
from wagtail.models import Locale, Site
89

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

7677
en_us_root_page = site.root_page
78+
7779
fr_root_page = en_us_root_page.copy_for_translation(fr_locale)
80+
rev = fr_root_page.save_revision()
81+
fr_root_page.publish(rev)
82+
7883
pt_br_root_page = en_us_root_page.copy_for_translation(pt_br_locale)
84+
rev = pt_br_root_page.save_revision()
85+
pt_br_root_page.publish(rev)
7986

8087
en_us_homepage = SimpleRichTextPageFactory(
8188
title="Test Page",
@@ -148,3 +155,21 @@ def tiny_localized_site():
148155
assert fr_homepage.live is True
149156
assert fr_child.live is True
150157
assert fr_grandchild.live is True
158+
159+
160+
@pytest.fixture
161+
def tiny_localized_site_redirects():
162+
"""Some test redirects that complement the tiny_localized_site fixture.
163+
164+
Useful for things like the tests for the cache-based lookup
165+
in bedrock.cms.tests.test_utils.test_path_exists_in_cms
166+
"""
167+
168+
Redirect.add_redirect(
169+
old_path="/fr/moved-page/",
170+
redirect_to="/fr/test-page/child-page/",
171+
)
172+
Redirect.add_redirect(
173+
old_path="/en-US/deeper/nested/moved-page/",
174+
redirect_to="/fr/test-page/",
175+
)

bedrock/cms/tests/test_decorators.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

55

6+
from django.http import Http404, HttpResponse
67
from django.urls import path
78

89
import pytest
910
from wagtail.rich_text import RichText
1011

1112
from bedrock.base.i18n import bedrock_i18n_patterns
12-
from bedrock.cms.decorators import prefer_cms
13+
from bedrock.cms.decorators import pre_check_for_cms_404, prefer_cms
1314
from bedrock.cms.tests import decorator_test_views
1415
from bedrock.urls import urlpatterns as mozorg_urlpatterns
1516

@@ -448,3 +449,58 @@ def test_prefer_cms_rejects_invalid_setup(mocker, config, expect_exeption):
448449
prefer_cms(view_func=fake_view, **config)
449450
else:
450451
prefer_cms(view_func=fake_view, **config)
452+
453+
454+
def dummy_view(request, *args, **kwargs):
455+
return HttpResponse("Hello, world!")
456+
457+
458+
@pytest.mark.parametrize("pretend_that_path_exists", [True, False])
459+
def test_pre_check_for_cms_404(
460+
pretend_that_path_exists,
461+
mocker,
462+
rf,
463+
settings,
464+
client,
465+
tiny_localized_site,
466+
):
467+
settings.CMS_DO_PAGE_PATH_PRECHECK = True
468+
mocked_view = mocker.spy(dummy_view, "__call__") # Spy on the test view
469+
mocked_path_exists_in_cms = mocker.patch("bedrock.cms.decorators.path_exists_in_cms")
470+
mocked_path_exists_in_cms.return_value = pretend_that_path_exists
471+
472+
decorated_view = pre_check_for_cms_404(mocked_view)
473+
request = rf.get("/path/is/irrelevant/because/we/are/mocking/path_exists_in_cms")
474+
475+
if pretend_that_path_exists:
476+
response = decorated_view(request)
477+
# Assert: Verify the original view was called
478+
mocked_view.assert_called_once_with(request)
479+
assert response.content == b"Hello, world!"
480+
else:
481+
with pytest.raises(Http404): # Expect an Http404 since path_exists_in_cms returns False
482+
decorated_view(request)
483+
mocked_view.assert_not_called()
484+
485+
486+
@pytest.mark.parametrize("pretend_that_path_exists", [True, False])
487+
def test_pre_check_for_cms_404__show_can_be_disabled_with_settings(
488+
pretend_that_path_exists,
489+
mocker,
490+
rf,
491+
settings,
492+
client,
493+
tiny_localized_site,
494+
):
495+
settings.CMS_DO_PAGE_PATH_PRECHECK = False
496+
mocked_view = mocker.spy(dummy_view, "__call__") # Spy on the test view
497+
mocked_path_exists_in_cms = mocker.patch("bedrock.cms.decorators.path_exists_in_cms")
498+
mocked_path_exists_in_cms.return_value = pretend_that_path_exists
499+
500+
decorated_view = pre_check_for_cms_404(mocked_view)
501+
request = rf.get("/path/is/irrelevant/because/we/are/mocking/path_exists_in_cms")
502+
503+
# The fake view will always be called because the pre-check isn't being used
504+
response = decorated_view(request)
505+
mocked_view.assert_called_once_with(request)
506+
assert response.content == b"Hello, world!"

0 commit comments

Comments
 (0)