Simplify pageurl tag

This doesn't change functionality, but simplifies the tag implementation which may improve performance in some edge cases.

Also add some `assertNumQueries` for future work.

wagtail/templatetags/wagtailcore_tags.py

@@ -23,25 +23,10 @@ def pageurl(context, page, fallback=None):
     if page is None and fallback:
         return resolve_url(fallback)
 
-    if not hasattr(page, "relative_url"):
+    if not isinstance(page, Page):
         raise ValueError("pageurl tag expected a Page object, got %r" % page)
 
-    try:
-        site = Site.find_for_request(context["request"])
-        current_site = site
-    except KeyError:
-        # request not available in the current context; fall back on page.url
-        return page.url
-
-    if current_site is None:
-        # request does not correspond to a recognised site; fall back on page.url
-        return page.url
-
-    # Pass page.relative_url the request object, which may contain a cached copy of
-    # Site.get_site_root_paths()
-    # This avoids page.relative_url having to make a database/cache fetch for this list
-    # each time it's called.
-    return page.relative_url(current_site, request=context.get("request"))
+    return page.get_url(request=context.get("request"))
 
 
 @register.simple_tag(takes_context=True)

wagtail/tests/tests.py

@@ -24,7 +24,8 @@ class TestPageUrlTags(TestCase):
         tpl = template.Template(
             """{% load wagtailcore_tags %}<a href="{% pageurl page fallback='fallback' %}">Fallback</a>"""
         )
-        result = tpl.render(template.Context({"page": None}))
+        with self.assertNumQueries(0):
+            result = tpl.render(template.Context({"page": None}))
         self.assertIn('<a href="/fallback/">Fallback</a>', result)
 
     def test_pageurl_with_get_absolute_url_object_fallback(self):
@@ -81,11 +82,30 @@ class TestPageUrlTags(TestCase):
         )
 
         # no 'request' object in context
-        result = tpl.render(template.Context({"page": page}))
+        with self.assertNumQueries(7):
+            result = tpl.render(template.Context({"page": page}))
         self.assertIn('<a href="/events/">Events</a>', result)
 
         # 'request' object in context, but no 'site' attribute
-        result = tpl.render(template.Context({"page": page, "request": HttpRequest()}))
+        result = tpl.render(
+            template.Context({"page": page, "request": get_dummy_request()})
+        )
+        self.assertIn('<a href="/events/">Events</a>', result)
+
+    def test_pageurl_caches(self):
+        page = Page.objects.get(url_path="/home/events/")
+        tpl = template.Template(
+            """{% load wagtailcore_tags %}<a href="{% pageurl page %}">{{ page.title }}</a>"""
+        )
+
+        request = get_dummy_request()
+
+        with self.assertNumQueries(8):
+            result = tpl.render(template.Context({"page": page, "request": request}))
+        self.assertIn('<a href="/events/">Events</a>', result)
+
+        with self.assertNumQueries(0):
+            result = tpl.render(template.Context({"page": page, "request": request}))
         self.assertIn('<a href="/events/">Events</a>', result)
 
     @override_settings(ALLOWED_HOSTS=["testserver", "localhost", "unknown.example.com"])
@@ -98,7 +118,8 @@ class TestPageUrlTags(TestCase):
         # 'request' object in context, but site is None
         request = get_dummy_request()
         request.META["HTTP_HOST"] = "unknown.example.com"
-        result = tpl.render(template.Context({"page": page, "request": request}))
+        with self.assertNumQueries(8):
+            result = tpl.render(template.Context({"page": page, "request": request}))
         self.assertIn('<a href="/events/">Events</a>', result)
 
     def test_bad_pageurl(self):
@@ -161,7 +182,10 @@ class TestPageUrlTags(TestCase):
         self.assertEqual(result, "/events/")
 
         # 'request' object in context, but no 'site' attribute
-        result = slugurl(template.Context({"request": HttpRequest()}), "events")
+        with self.assertNumQueries(3):
+            result = slugurl(
+                template.Context({"request": get_dummy_request()}), "events"
+            )
         self.assertEqual(result, "/events/")
 
     @override_settings(ALLOWED_HOSTS=["testserver", "localhost", "unknown.example.com"])