changeset 329:422052d2e978

internal: Try handling URLs in a consistent way. * Now URLs passed to, and returned from, routes will always be absolute URLs, i.e. URLs including the site root. * Validate the site root at config loading time to make sure it starts and ends with a slash. * Get rid of unused stuff. * Add tests.
author Ludovic Chabant <ludovic@chabant.com>
date Tue, 31 Mar 2015 23:03:28 -0700
parents 2a5996e0d3ec
children de4903457bed
files piecrust/app.py piecrust/baking/single.py piecrust/data/assetor.py piecrust/data/base.py piecrust/data/builder.py piecrust/rendering.py piecrust/routing.py piecrust/uriutil.py tests/test_baking_baker.py tests/test_data_assetor.py tests/test_routing.py tests/test_uriutil.py
diffstat 12 files changed, 174 insertions(+), 71 deletions(-) [+]
line wrap: on
line diff
--- a/piecrust/app.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/app.py	Tue Mar 31 23:03:28 2015 -0700
@@ -28,7 +28,7 @@
 logger = logging.getLogger(__name__)
 
 
-CACHE_VERSION = 17
+CACHE_VERSION = 18
 
 
 class VariantNotFoundError(Exception):
@@ -136,6 +136,12 @@
         cachec = collections.OrderedDict()
         values['__cache'] = cachec
 
+        # Make sure the site root starts and ends with a slash.
+        if not sitec['root'].startswith('/'):
+            raise ConfigurationError("The `site/root` setting must start "
+                                     "with a slash.")
+        sitec['root'] = sitec['root'].rstrip('/') + '/'
+
         # Cache auto-format regexes.
         if not isinstance(sitec['auto_formats'], dict):
             raise ConfigurationError("The 'site/auto_formats' setting must be "
--- a/piecrust/baking/single.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/baking/single.py	Tue Mar 31 23:03:28 2015 -0700
@@ -11,6 +11,7 @@
         PASS_FORMATTING, PASS_RENDERING)
 from piecrust.sources.base import (PageFactory,
         REALM_NAMES, REALM_USER, REALM_THEME)
+from piecrust.uriutil import split_uri
 
 
 logger = logging.getLogger(__name__)
@@ -41,8 +42,10 @@
         self.pagination_suffix = app.config.get('site/pagination_suffix')
 
     def getOutputPath(self, uri):
+        uri_root, uri_path = split_uri(self.app, uri)
+
         bake_path = [self.out_dir]
-        decoded_uri = urllib.parse.unquote(uri.lstrip('/'))
+        decoded_uri = urllib.parse.unquote(uri_path)
         if self.pretty_urls:
             bake_path.append(decoded_uri)
             bake_path.append('index.html')
@@ -71,8 +74,7 @@
 
         # Generate the URL using the route.
         page = factory.buildPage()
-        uri = route.getUri(route_metadata, provider=page,
-                           include_site_root=False)
+        uri = route.getUri(route_metadata, provider=page)
 
         override = self.record.getOverrideEntry(factory, uri)
         if override is not None:
@@ -125,7 +127,7 @@
 
         while has_more_subs:
             sub_uri = route.getUri(route_metadata, sub_num=cur_sub,
-                                   provider=page, include_site_root=False)
+                                   provider=page)
             out_path = self.getOutputPath(sub_uri)
 
             # Check for up-to-date outputs.
--- a/piecrust/data/assetor.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/data/assetor.py	Tue Mar 31 23:03:28 2015 -0700
@@ -26,9 +26,8 @@
             {
                 '%path%': rel_assets_path,
                 '%uri%': uri})
-    # TEMP HACK
-    # TODO: looks like we can get here with differently formatted URLs... :(
-    return app.config.get('site/root') + base_url.strip('/') + '/'
+
+    return base_url.rstrip('/') + '/'
 
 
 class Assetor(object):
--- a/piecrust/data/base.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/data/base.py	Tue Mar 31 23:03:28 2015 -0700
@@ -1,7 +1,7 @@
 import time
 import logging
 from piecrust.data.assetor import Assetor
-from piecrust.uriutil import get_slug
+from piecrust.uriutil import split_uri
 
 
 logger = logging.getLogger(__name__)
@@ -129,8 +129,9 @@
 
     def _loadCustom(self):
         page_url = self._get_uri()
+        _, slug = split_uri(self.page.app, page_url)
         self._setValue('url', page_url)
-        self._setValue('slug', get_slug(self._page.app, page_url))
+        self._setValue('slug', slug)
         self._setValue(
                 'timestamp',
                 time.mktime(self.page.datetime.timetuple()))
--- a/piecrust/data/builder.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/data/builder.py	Tue Mar 31 23:03:28 2015 -0700
@@ -7,7 +7,7 @@
 from piecrust.data.debug import build_debug_info
 from piecrust.data.linker import PageLinkerData
 from piecrust.data.paginator import Paginator
-from piecrust.uriutil import get_slug, split_sub_uri
+from piecrust.uriutil import split_uri, split_sub_uri
 
 
 logger = logging.getLogger(__name__)
@@ -22,14 +22,15 @@
         self.pagination_filter = None
 
     @property
-    def slug(self):
-        return get_slug(self.page.app, self.uri)
+    def app(self):
+        return self.page.app
 
 
 def build_page_data(ctx):
     page = ctx.page
     app = page.app
     first_uri, _ = split_sub_uri(app, ctx.uri)
+    _, slug = split_uri(app, ctx.uri)
 
     pc_data = PieCrustData()
     pgn_source = ctx.pagination_source or get_default_pagination_source(page)
@@ -46,7 +47,7 @@
             }
     page_data = data['page']
     page_data['url'] = ctx.uri
-    page_data['slug'] = ctx.slug
+    page_data['slug'] = slug
     page_data['timestamp'] = time.mktime(page.datetime.timetuple())
     date_format = app.config.get('site/date_format')
     if date_format:
--- a/piecrust/rendering.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/rendering.py	Tue Mar 31 23:03:28 2015 -0700
@@ -8,7 +8,6 @@
         page_value_accessor)
 from piecrust.sources.base import PageSource
 from piecrust.templating.base import TemplateNotFoundError, TemplatingError
-from piecrust.uriutil import get_slug
 
 
 logger = logging.getLogger(__name__)
@@ -66,10 +65,6 @@
         return self.page.app
 
     @property
-    def slug(self):
-        return get_slug(self.page.app, self.uri)
-
-    @property
     def source_metadata(self):
         return self.page.source_metadata
 
--- a/piecrust/routing.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/routing.py	Tue Mar 31 23:03:28 2015 -0700
@@ -7,7 +7,9 @@
 
 
 route_re = re.compile(r'%((?P<qual>path):)?(?P<name>\w+)%')
-template_func_re = re.compile(r'^(?P<name>\w+)\((?P<first_arg>\w+)(?P<other_args>.*)\)\s*$')
+route_esc_re = re.compile(r'\\%((?P<qual>path)\\:)?(?P<name>\w+)\\%')
+template_func_re = re.compile(r'^(?P<name>\w+)\((?P<first_arg>\w+)'
+                              r'(?P<other_args>.*)\)\s*$')
 template_func_arg_re = re.compile(r',\s*(?P<arg>\w+)')
 ugly_url_cleaner = re.compile(r'\.html$')
 
@@ -29,7 +31,7 @@
         self.trailing_slash = app.config.get('site/trailing_slash')
         self.pagination_suffix_format = app.config.get(
                 '__cache/pagination_suffix_format')
-        self.uri_root = app.config.get('site/root').rstrip('/') + '/'
+        self.uri_root = app.config.get('site/root')
 
         uri = cfg['url']
         self.uri_pattern = uri.lstrip('/')
@@ -38,8 +40,8 @@
             self.uri_format += '?!debug'
 
         # Get the straight-forward regex for matching this URI pattern.
-        re_suffix = '$'
-        p = route_re.sub(self._uriPatternRepl, self.uri_pattern) + re_suffix
+        p = route_esc_re.sub(self._uriPatternRepl,
+                             re.escape(self.uri_pattern)) + '$'
         self.uri_re = re.compile(p)
 
         # If the URI pattern has a 'path'-type component, we'll need to match
@@ -54,16 +56,18 @@
                 .replace('//', '/')
                 .rstrip('/'))
         if uri_pattern_no_path != self.uri_pattern:
-            p = route_re.sub(self._uriPatternRepl, uri_pattern_no_path) + '$'
+            p = route_esc_re.sub(self._uriPatternRepl,
+                                 re.escape(uri_pattern_no_path)) + '$'
             self.uri_re_no_path = re.compile(p)
         else:
             self.uri_re_no_path = None
 
+        self.required_source_metadata = set()
+        for m in route_re.finditer(self.uri_pattern):
+            self.required_source_metadata.add(m.group('name'))
+
         self.source_name = cfg['source']
         self.taxonomy = cfg.get('taxonomy')
-        self.required_source_metadata = set()
-        for m in route_re.finditer(uri):
-            self.required_source_metadata.add(m.group('name'))
         self.template_func = None
         self.template_func_name = None
         self.template_func_args = []
@@ -85,6 +89,10 @@
         return self.required_source_metadata.issubset(source_metadata.keys())
 
     def matchUri(self, uri):
+        if not uri.startswith(self.uri_root):
+            raise Exception("The given URI is not absolute: %s" % uri)
+        uri = uri[len(self.uri_root):]
+
         if not self.pretty_urls:
             uri = ugly_url_cleaner.sub('', uri)
         elif self.trailing_slash:
@@ -99,8 +107,7 @@
                 return m.groupdict()
         return None
 
-    def getUri(self, source_metadata, *, sub_num=1, provider=None,
-               include_site_root=True):
+    def getUri(self, source_metadata, *, sub_num=1, provider=None):
         if provider:
             source_metadata = dict(source_metadata)
             source_metadata.update(provider.getRouteMetadata())
@@ -147,9 +154,7 @@
                 else:
                     uri = base_uri + ext
 
-        if include_site_root:
-            uri = self.uri_root + uri
-
+        uri = self.uri_root + uri
         return uri
 
     def _uriFormatRepl(self, m):
--- a/piecrust/uriutil.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/piecrust/uriutil.py	Tue Mar 31 23:03:28 2015 -0700
@@ -74,13 +74,20 @@
     return pattern.sub(lambda m: reps[re.escape(m.group(0))], text)
 
 
-def get_slug(app, uri):
-    site_root = app.config.get('site/root')
-    uri = uri[len(site_root):]
-    return uri.lstrip('/')
+def split_uri(app, uri):
+    root = app.config.get('site/root')
+    uri_root = uri[:len(root)]
+    if uri_root != root:
+        raise Exception("URI '%s' is not a full URI." % uri)
+    uri = uri[len(root):]
+    return uri_root, uri
 
 
 def split_sub_uri(app, uri):
+    root = app.config.get('site/root')
+    if not uri.startswith(root):
+        raise Exception("URI '%s' is not a full URI." % uri)
+
     pretty_urls = app.config.get('site/pretty_urls')
     if not pretty_urls:
         uri, ext = os.path.splitext(uri)
@@ -97,5 +104,5 @@
     if not pretty_urls:
         uri += ext
 
-    return (uri, page_num)
+    return uri, page_num
 
--- a/tests/test_baking_baker.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/tests/test_baking_baker.py	Tue Mar 31 23:03:28 2015 -0700
@@ -40,11 +40,13 @@
         app.config.set('site/pretty_urls', True)
     assert app.config.get('site/pretty_urls') == pretty
 
-    baker = PageBaker(app, '/destination')
-    path = baker.getOutputPath(uri)
-    expected = os.path.normpath(
-            os.path.join('/destination', expected))
-    assert expected == path
+    for site_root in ['/', '/whatever/']:
+        app.config.set('site/root', site_root)
+        baker = PageBaker(app, '/destination')
+        path = baker.getOutputPath(site_root + uri)
+        expected = os.path.normpath(
+                os.path.join('/destination', expected))
+        assert expected == path
 
 
 def test_empty_bake():
@@ -60,10 +62,18 @@
         assert list(structure.keys()) == ['index.html']
 
 
-def test_simple_bake():
+@pytest.mark.parametrize(
+        'site_root',
+        [
+            ('/'), ('/whatever')
+            ])
+def test_simple_bake(site_root):
+    pconf = {'layout': 'none', 'format': 'none'}
     fs = (mock_fs()
-            .withPage('posts/2010-01-01_post1.md', {'layout': 'none', 'format': 'none'}, 'post one')
-            .withPage('pages/_index.md', {'layout': 'none', 'format': 'none'}, "something"))
+          .withConfig({'site': {'root': site_root}})
+          .withPage('posts/2010-01-01_post1.md', pconf, 'post one')
+          .withPage('pages/about.md', pconf, 'URL: {{page.url}}')
+          .withPage('pages/_index.md', pconf, "something"))
     with mock_fs_scope(fs):
         out_dir = fs.path('kitchen/_counter')
         app = fs.getApp()
@@ -72,6 +82,8 @@
         structure = fs.getStructure('kitchen/_counter')
         assert structure == {
                 '2010': {'01': {'01': {'post1.html': 'post one'}}},
+                'about.html': 'URL: %s' % (
+                        site_root.rstrip('/') + '/about.html'),
                 'index.html': 'something'}
 
 
@@ -147,7 +159,8 @@
         out_dir = fs.path('kitchen/_counter')
         app = fs.getApp()
         baker = Baker(app, out_dir)
-        baker.bake()
+        r = baker.bake()
+        assert r.success is True
 
         s = fs.getStructure('kitchen/_counter/tag')
         assert s['foo.html'] == "Pages in foo\nPost 3\nPost 1\n"
--- a/tests/test_data_assetor.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/tests/test_data_assetor.py	Tue Mar 31 23:03:28 2015 -0700
@@ -1,32 +1,48 @@
 import pytest
 from mock import MagicMock
-from piecrust.data.assetor import (Assetor, UnsupportedAssetsError,
-        build_base_url)
+from piecrust.data.assetor import (
+        Assetor, UnsupportedAssetsError, build_base_url)
 from .mockutil import mock_fs, mock_fs_scope
 
 
-@pytest.mark.parametrize('fs, expected', [
-        (mock_fs().withPage('pages/foo/bar'), {}),
+@pytest.mark.parametrize('fs, site_root, expected', [
+        (mock_fs().withPage('pages/foo/bar'), '/', {}),
         (mock_fs()
             .withPage('pages/foo/bar')
             .withPageAsset('pages/foo/bar', 'one.txt', 'one'),
+            '/',
             {'one': 'one'}),
         (mock_fs()
             .withPage('pages/foo/bar')
             .withPageAsset('pages/foo/bar', 'one.txt', 'one')
             .withPageAsset('pages/foo/bar', 'two.txt', 'two'),
+            '/',
+            {'one': 'one', 'two': 'two'}),
+
+        (mock_fs().withPage('pages/foo/bar'), '/whatever', {}),
+        (mock_fs()
+            .withPage('pages/foo/bar')
+            .withPageAsset('pages/foo/bar', 'one.txt', 'one'),
+            '/whatever',
+            {'one': 'one'}),
+        (mock_fs()
+            .withPage('pages/foo/bar')
+            .withPageAsset('pages/foo/bar', 'one.txt', 'one')
+            .withPageAsset('pages/foo/bar', 'two.txt', 'two'),
+            '/whatever',
             {'one': 'one', 'two': 'two'})
         ])
-def test_assets(fs, expected):
+def test_assets(fs, site_root, expected):
+    fs.withConfig({'site': {'root': site_root}})
     with mock_fs_scope(fs):
         page = MagicMock()
         page.app = fs.getApp(cache=False)
         page.app.env.base_asset_url_format = '%uri%'
         page.path = fs.path('/kitchen/pages/foo/bar.md')
-        assetor = Assetor(page, '/foo/bar')
+        assetor = Assetor(page, site_root.rstrip('/') + '/foo/bar')
         for en in expected.keys():
             assert hasattr(assetor, en)
-            path = '/foo/bar/%s.txt' % en
+            path = site_root.rstrip('/') + '/foo/bar/%s.txt' % en
             assert getattr(assetor, en) == path
             assert assetor[en] == path
 
--- a/tests/test_routing.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/tests/test_routing.py	Tue Mar 31 23:03:28 2015 -0700
@@ -1,4 +1,3 @@
-import mock
 import pytest
 from piecrust.routing import Route
 from .mockutil import get_mock_app
@@ -17,8 +16,8 @@
                 {'zoo': 'zar'}, False)
             ])
 def test_matches_metadata(config, metadata, expected):
-    app = mock.Mock()
-    app.config = {'site/root': '/'}
+    app = get_mock_app()
+    app.config.set('site/root', '/')
     config.setdefault('source', 'blah')
     route = Route(app, config)
     m = route.matchesMetadata(metadata)
@@ -26,41 +25,99 @@
 
 
 @pytest.mark.parametrize(
-        'config, uri, expected_match',
+        'site_root, route_pattern, expected_required_metadata',
         [
-            ({'url': '/%foo%'},
+            ('/', '/%foo%', set(['foo'])),
+            ('/', '/%path:foo%', set(['foo'])),
+            ('/', '/%foo%/%bar%', set(['foo', 'bar'])),
+            ('/', '/%foo%/%path:bar%', set(['foo', 'bar'])),
+            ('/something', '/%foo%', set(['foo'])),
+            ('/something', '/%path:foo%', set(['foo'])),
+            ('/something', '/%foo%/%bar%', set(['foo', 'bar'])),
+            ('/something', '/%foo%/%path:bar%', set(['foo', 'bar']))
+            ])
+def test_required_metadata(site_root, route_pattern,
+                           expected_required_metadata):
+    app = get_mock_app()
+    app.config.set('site/root', site_root.rstrip('/') + '/')
+    config = {'url': route_pattern, 'source': 'blah'}
+    route = Route(app, config)
+    assert route.required_source_metadata == expected_required_metadata
+
+
+@pytest.mark.parametrize(
+        'site_root, config, uri, expected_match',
+        [
+            ('/', {'url': '/%foo%'},
                 'something',
                 {'foo': 'something'}),
-            ({'url': '/%foo%'},
+            ('/', {'url': '/%foo%'},
                 'something/other',
                 None),
-            ({'url': '/%path:foo%'},
+            ('/', {'url': '/%path:foo%'},
                 'something/other',
                 {'foo': 'something/other'}),
-            ({'url': '/%path:foo%'},
+            ('/', {'url': '/%path:foo%'},
                 '',
                 {'foo': ''}),
-            ({'url': '/prefix/%path:foo%'},
+            ('/', {'url': '/prefix/%path:foo%'},
                 'prefix/something/other',
                 {'foo': 'something/other'}),
-            ({'url': '/prefix/%path:foo%'},
+            ('/', {'url': '/prefix/%path:foo%'},
                 'prefix/',
                 {'foo': ''}),
-            ({'url': '/prefix/%path:foo%'},
+            ('/', {'url': '/prefix/%path:foo%'},
+                'prefix',
+                {}),
+
+            ('/blah', {'url': '/%foo%'},
+                'something',
+                {'foo': 'something'}),
+            ('/blah', {'url': '/%foo%'},
+                'something/other',
+                None),
+            ('/blah', {'url': '/%path:foo%'},
+                'something/other',
+                {'foo': 'something/other'}),
+            ('/blah', {'url': '/%path:foo%'},
+                '',
+                {'foo': ''}),
+            ('/blah', {'url': '/prefix/%path:foo%'},
+                'prefix/something/other',
+                {'foo': 'something/other'}),
+            ('/blah', {'url': '/prefix/%path:foo%'},
+                'prefix/',
+                {'foo': ''}),
+            ('/blah', {'url': '/prefix/%path:foo%'},
                 'prefix',
                 {}),
             ])
-def test_match_uri(config, uri, expected_match):
-    app = mock.Mock()
-    app.config = {'site/root': '/'}
+def test_match_uri(site_root, config, uri, expected_match):
+    site_root = site_root.rstrip('/') + '/'
+    app = get_mock_app()
+    app.config.set('site/root', site_root)
     config.setdefault('source', 'blah')
     route = Route(app, config)
     assert route.uri_pattern == config['url'].lstrip('/')
-    m = route.matchUri(uri)
+    m = route.matchUri(site_root + uri)
     assert m == expected_match
 
 
 @pytest.mark.parametrize(
+        'site_root',
+        [
+            ('/'), ('/whatever')
+            ])
+def test_match_uri_requires_absolute_uri(site_root):
+    with pytest.raises(Exception):
+        app = get_mock_app()
+        app.config.set('site/root', site_root.rstrip('/') + '/')
+        config = {'url': '/%path:slug%', 'source': 'blah'}
+        route = Route(app, config)
+        route.matchUri('notabsuri')
+
+
+@pytest.mark.parametrize(
         'slug, page_num, pretty, expected',
         [
             # Pretty URLs
@@ -92,7 +149,7 @@
             ])
 def test_get_uri(slug, page_num, pretty, expected):
     app = get_mock_app()
-    app.config.set('site/root', '/blah')
+    app.config.set('site/root', '/blah/')
     app.config.set('site/pretty_urls', pretty)
     app.config.set('site/trailing_slash', False)
     app.config.set('__cache/pagination_suffix_format', '/%(num)d')
--- a/tests/test_uriutil.py	Tue Mar 31 22:38:56 2015 -0700
+++ b/tests/test_uriutil.py	Tue Mar 31 23:03:28 2015 -0700
@@ -39,8 +39,9 @@
 def test_split_sub_uri(uri, expected, pretty_urls):
     app = mock.MagicMock()
     app.config = {
+            'site/root': '/whatever/',
             'site/pretty_urls': pretty_urls,
             '__cache/pagination_suffix_re': '/(?P<num>\\d+)$'}
-    actual = split_sub_uri(app, uri)
-    assert actual == expected
+    actual = split_sub_uri(app, '/whatever/' + uri)
+    assert actual == ('/whatever/' + expected[0], expected[1])