Mercurial > piecrust2
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])