changeset 1145:e94737572542

serve: Fix an issue where false positive matches were rendered as the requested page. Now we try to render the page, but also try to detect for the most common "empty" pages.
author Ludovic Chabant <ludovic@chabant.com>
date Tue, 05 Jun 2018 22:08:51 -0700
parents 9f3e702a8a69
children 3516759ea1b2
files piecrust/rendering.py piecrust/serving/server.py piecrust/serving/util.py tests/servings/test_tags.yaml
diffstat 4 files changed, 103 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/piecrust/rendering.py	Tue Jun 05 22:05:46 2018 -0700
+++ b/piecrust/rendering.py	Tue Jun 05 22:08:51 2018 -0700
@@ -54,6 +54,7 @@
     return {
         'used_source_names': {'segments': [], 'layout': []},
         'used_pagination': False,
+        'pagination_has_items': False,
         'pagination_has_more': False,
         'used_assets': False,
     }
@@ -95,6 +96,7 @@
             raise Exception("Pagination has already been used.")
         assert paginator.is_loaded
         ri['used_pagination'] = True
+        ri['pagination_has_items'] = paginator.has_items
         ri['pagination_has_more'] = paginator.has_more
         self.addUsedSource(paginator._source)
 
--- a/piecrust/serving/server.py	Tue Jun 05 22:05:46 2018 -0700
+++ b/piecrust/serving/server.py	Tue Jun 05 22:08:51 2018 -0700
@@ -10,10 +10,11 @@
 from werkzeug.wrappers import Request, Response
 from jinja2 import FileSystemLoader, Environment
 from piecrust import CACHE_DIR, RESOURCES_DIR
+from piecrust.page import PageNotFoundError
 from piecrust.rendering import RenderingContext, render_page
 from piecrust.routing import RouteNotFoundError
 from piecrust.serving.util import (
-    content_type_map, make_wrapped_file_response, get_requested_page,
+    content_type_map, make_wrapped_file_response, get_requested_pages,
     get_app_for_server)
 from piecrust.sources.base import SourceNotFoundError
 
@@ -152,22 +153,44 @@
 
     def _try_serve_page(self, app, environ, request):
         # Find a matching page.
-        req_page = get_requested_page(app, request.path)
+        req_pages, not_founds = get_requested_pages(app, request.path)
+
+        rendered_page = None
+
+        for req_page in req_pages:
+            # We have a page, let's try to render it.
+            render_ctx = RenderingContext(req_page.page,
+                                          sub_num=req_page.sub_num,
+                                          force_render=True)
+            req_page.page.source.prepareRenderContext(render_ctx)
+
+            # Render the page.
+            this_rendered_page = render_page(render_ctx)
+
+            # We might have rendered a page that technically exists, but
+            # has no interesting content, like a tag page for a tag that
+            # isn't used by any page. To eliminate these false positives,
+            # we check if there was pagination used, and if so, if it had
+            # anything in it.
+            # TODO: we might need a more generic system for other cases.
+            render_info = this_rendered_page.render_info
+            if (render_info['used_pagination'] and
+                    not render_info['pagination_has_items']):
+                not_founds.append(PageNotFoundError(
+                    ("Rendered '%s' (page %d) in source '%s' "
+                     "but got empty content:\n\n%s\n\n") %
+                    (req_page.req_path, req_page.sub_num,
+                     req_page.page.source.name, this_rendered_page.content)))
+                continue
+
+            rendered_page = this_rendered_page
+            break
 
         # If we haven't found any good match, report all the places we didn't
         # find it at.
-        if req_page.page is None:
+        if rendered_page is None:
             msg = "Can't find path for '%s':" % request.path
-            raise MultipleNotFound(msg, req_page.not_found_errors)
-
-        # We have a page, let's try to render it.
-        render_ctx = RenderingContext(req_page.page,
-                                      sub_num=req_page.sub_num,
-                                      force_render=True)
-        req_page.page.source.prepareRenderContext(render_ctx)
-
-        # Render the page.
-        rendered_page = render_page(render_ctx)
+            raise MultipleNotFound(msg, not_founds)
 
         # Start doing stuff.
         page = rendered_page.page
--- a/piecrust/serving/util.py	Tue Jun 05 22:05:46 2018 -0700
+++ b/piecrust/serving/util.py	Tue Jun 05 22:08:51 2018 -0700
@@ -39,19 +39,22 @@
         uri_no_sub, sub_num = decomposed_uri
 
     res = []
+
     for route in routes:
         route_params = route.matchUri(uri)
         if route_params is not None:
             res.append((route, route_params, 1))
 
-        if sub_num > 1:
+    if sub_num > 1:
+        for route in routes:
             route_params = route.matchUri(uri_no_sub)
             if route_params is not None:
                 res.append((route, route_params, sub_num))
+
     return res
 
 
-def get_requested_page(app, req_path):
+def get_requested_pages(app, req_path):
     # Remove the trailing slash to simplify how we parse URLs.
     root_url = app.config.get('site/root')
     if req_path != root_url:
@@ -65,7 +68,9 @@
     if len(routes) == 0:
         raise RouteNotFoundError("Can't find route for: %s" % req_path)
 
-    req_page = RequestedPage()
+    req_pages = []
+    not_founds = []
+
     for route, route_params, route_sub_num in routes:
         cur_req_path = req_path
         if route_sub_num > 1:
@@ -73,16 +78,27 @@
 
         page = _get_requested_page_for_route(app, route, route_params)
         if page is not None:
+            req_page = RequestedPage()
             req_page.page = page
             req_page.sub_num = route_sub_num
             req_page.req_path = cur_req_path
-            break
+            req_pages.append(req_page)
+        else:
+            not_founds.append(PageNotFoundError(
+                "No path found for '%s' in source '%s'." %
+                (cur_req_path, route.source_name)))
+
+    return req_pages, not_founds
+
 
-        req_page.not_found_errors.append(PageNotFoundError(
-            "No path found for '%s' in source '%s'." %
-            (cur_req_path, route.source_name)))
+def get_requested_page(app, req_path):
+    req_pages, not_founds = get_requested_pages(app, req_path)
+    if req_pages:
+        return req_pages[0]
 
-    return req_page
+    nfrp = RequestedPage()
+    nfrp.not_found_errors = not_founds
+    return nfrp
 
 
 def _get_requested_page_for_route(app, route, route_params):
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/servings/test_tags.yaml	Tue Jun 05 22:08:51 2018 -0700
@@ -0,0 +1,41 @@
+---
+url: /tag/foo.html
+config:
+    site:
+        posts_per_page: 3
+in:
+    templates/_tag.html: |
+        Posts with {{tag}}
+        {% for post in pagination.posts -%}
+        {{post.url}}
+        {% endfor %}
+    posts/2016-06-01_post0.html: "---\ntags: [foo]\n---\nPost 0"
+    posts/2016-06-02_post1.html: "---\ntags: [foo]\n---\nPost 1"
+    posts/2016-06-03_post2.html: "---\ntags: [foo]\n---\nPost 2"
+    posts/2016-06-04_post3.html: "---\ntags: [foo]\n---\nPost 3"
+    posts/2016-06-05_post4.html: "---\ntags: [foo]\n---\nPost 4"
+out: |
+    Posts with foo
+    /2016/06/05/post4.html
+    /2016/06/04/post3.html
+    /2016/06/03/post2.html
+---
+url: /tag/foo/2.html
+config:
+    site:
+        posts_per_page: 3
+in:
+    templates/_tag.html: |
+        Posts with {{tag}}
+        {% for post in pagination.posts -%}
+        {{post.url}}
+        {% endfor %}
+    posts/2016-06-01_post0.html: "---\ntags: [foo]\n---\nPost 0"
+    posts/2016-06-02_post1.html: "---\ntags: [foo]\n---\nPost 1"
+    posts/2016-06-03_post2.html: "---\ntags: [foo]\n---\nPost 2"
+    posts/2016-06-04_post3.html: "---\ntags: [foo]\n---\nPost 3"
+    posts/2016-06-05_post4.html: "---\ntags: [foo]\n---\nPost 4"
+out: |
+    Posts with foo
+    /2016/06/02/post1.html
+    /2016/06/01/post0.html