# HG changeset patch # User Ludovic Chabant # Date 1528261731 25200 # Node ID e94737572542dcc6c55a125a7536d21074b26c9b # Parent 9f3e702a8a69573484de46d5d383ac96ecac8bb1 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. diff -r 9f3e702a8a69 -r e94737572542 piecrust/rendering.py --- 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) diff -r 9f3e702a8a69 -r e94737572542 piecrust/serving/server.py --- 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 diff -r 9f3e702a8a69 -r e94737572542 piecrust/serving/util.py --- 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): diff -r 9f3e702a8a69 -r e94737572542 tests/servings/test_tags.yaml --- /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