changeset 787:f6f9a284a5f3

routing: Simplify how route functions are declared and handled. * Site config now only has to declare the function name. * Simply code for running route functions.
author Ludovic Chabant <ludovic@chabant.com>
date Thu, 01 Sep 2016 23:00:58 -0700
parents 97c1dc568810
children 276030ea7972
files piecrust/__init__.py piecrust/appconfig.py piecrust/commands/builtin/info.py piecrust/routing.py piecrust/templating/jinjaengine.py tests/test_routing.py
diffstat 6 files changed, 95 insertions(+), 117 deletions(-) [+]
line wrap: on
line diff
--- a/piecrust/__init__.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/piecrust/__init__.py	Thu Sep 01 23:00:58 2016 -0700
@@ -18,7 +18,7 @@
 
 PIECRUST_URL = 'https://bolt80.com/piecrust/'
 
-CACHE_VERSION = 27
+CACHE_VERSION = 28
 
 try:
     from piecrust.__version__ import APP_VERSION
--- a/piecrust/appconfig.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/piecrust/appconfig.py	Thu Sep 01 23:00:58 2016 -0700
@@ -287,7 +287,7 @@
                 {
                     'url': '/%path:slug%',
                     'source': 'theme_pages',
-                    'func': 'pcurl(slug)'
+                    'func': 'pcurl'
                     }
                 ],
             'theme_tag_page': 'theme_pages:_tag.%ext%',
@@ -339,7 +339,7 @@
             'default_post_layout': 'post',
             'post_url': '/%int4:year%/%int2:month%/%int2:day%/%slug%',
             'year_url': '/archives/%int4:year%',
-            'tag_url': '/tag/%path:tag%',
+            'tag_url': '/tag/%+tag%',
             'category_url': '/%category%',
             'posts_per_page': 5
             })
@@ -365,7 +365,7 @@
                     {
                         'url': '/%path:slug%',
                         'source': 'pages',
-                        'func': 'pcurl(slug)'
+                        'func': 'pcurl'
                         }
                     ],
                 'taxonomies': collections.OrderedDict([
@@ -467,14 +467,12 @@
                     {
                         'url': post_url,
                         'source': blog_name,
-                        'func': (
-                            '%sposturl(int:year,int:month,int:day,slug)' %
-                            tpl_func_prefix)
+                        'func': ('%sposturl' % tpl_func_prefix)
                         },
                     {
                         'url': year_url,
                         'generator': ('%s_archives' % blog_name),
-                        'func': ('%syearurl(year)' % tpl_func_prefix)
+                        'func': ('%syearurl' % tpl_func_prefix)
                         }
                     ]
                 })
@@ -510,19 +508,15 @@
                 (values, 'site/%s' % tax_url_cfg_name),
                 default=('%s/%%%s%%' % (term, term)))
         tax_url = '/' + url_prefix + tax_url.lstrip('/')
-        term_arg = term
-        if tax_cfg.get('multiple') is True:
-            term_arg = '+' + term
         tax_func_name = try_get_dict_values(
                 (user_overrides, 'site/taxonomies/%s/func_name' % tax_name),
                 (values, 'site/taxonomies/%s/func_name' % tax_name),
                 default=('%s%surl' % (tpl_func_prefix, term)))
-        tax_func = '%s(%s)' % (tax_func_name, term_arg)
         tax_route = collections.OrderedDict({
             'url': tax_url,
             'generator': tax_gen_name,
             'taxonomy': tax_name,
-            'func': tax_func
+            'func': tax_func_name
             })
         cfg['site']['routes'].append(tax_route)
 
--- a/piecrust/commands/builtin/info.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/piecrust/commands/builtin/info.py	Thu Sep 01 23:00:58 2016 -0700
@@ -84,8 +84,8 @@
             logger.info("    generator: %s" % (route.generator_name or ''))
             logger.info("    regex: %s" % route.uri_re.pattern)
             logger.info("    function: %s(%s)" % (
-                route.template_func_name,
-                ', '.join(route.template_func_args)))
+                route.func_name,
+                ', '.join(route.func_parameters)))
 
 
 class ShowPathsCommand(ChefCommand):
--- a/piecrust/routing.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/piecrust/routing.py	Thu Sep 01 23:00:58 2016 -0700
@@ -9,10 +9,8 @@
 logger = logging.getLogger(__name__)
 
 
-route_re = re.compile(r'%((?P<qual>[\w\d]+):)?(?P<name>\w+)%')
-route_esc_re = re.compile(r'\\%((?P<qual>[\w\d]+)\\:)?(?P<name>\w+)\\%')
-template_func_re = re.compile(r'^(?P<name>\w+)\((?P<args>.*)\)\s*$')
-template_func_arg_re = re.compile(r'((?P<qual>[\w\d]+):)?(?P<arg>\+?\w+)')
+route_re = re.compile(r'%((?P<qual>[\w\d]+):)?(?P<var>\+)?(?P<name>\w+)%')
+route_esc_re = re.compile(r'\\%((?P<qual>[\w\d]+)\\:)?(?P<var>\\\+)?(?P<name>\w+)\\%')
 ugly_url_cleaner = re.compile(r'\.html$')
 
 
@@ -94,15 +92,24 @@
         else:
             self.uri_re_no_path = None
 
-        self.required_route_metadata = set()
+        # Determine the parameters for the route function.
+        self.func_name = self._validateFuncName(cfg.get('func'))
+        self.func_parameters = []
+        self.func_has_variadic_parameter = False
+        variadic_param_idx = -1
         for m in route_re.finditer(self.uri_pattern):
-            self.required_route_metadata.add(m.group('name'))
+            name = m.group('name')
+            if m.group('var'):
+                self.func_has_variadic_parameter = True
+                variadic_param_idx = len(self.func_parameters)
 
-        self.template_func = None
-        self.template_func_name = None
-        self.template_func_args = []
-        self.template_func_vararg = None
-        self._createTemplateFunc(cfg.get('func'))
+            self.func_parameters.append(name)
+
+        if (variadic_param_idx >= 0 and
+                variadic_param_idx != len(self.func_parameters) - 1):
+            raise Exception(
+                "Only the last route URL parameter can be variadic. "
+                "Got: %s" % self.uri_pattern)
 
     @property
     def route_type(self):
@@ -142,7 +149,7 @@
                 self.generator_name, self.uri))
 
     def matchesMetadata(self, route_metadata):
-        return self.required_route_metadata.issubset(route_metadata.keys())
+        return set(self.func_parameters).issubset(route_metadata.keys())
 
     def matchUri(self, uri, strict=False):
         if not uri.startswith(self.uri_root):
@@ -171,7 +178,7 @@
             # say, a route's pattern is `/foo/%slug%`, and we're matching an
             # URL like `/foo`.
             matched_keys = set(route_metadata.keys())
-            missing_keys = self.required_route_metadata - matched_keys
+            missing_keys = set(self.func_parameters) - matched_keys
             for k in missing_keys:
                 route_metadata[k] = ''
 
@@ -231,6 +238,35 @@
 
         return uri
 
+    def execTemplateFunc(self, *args):
+        fixed_param_count = len(self.func_parameters)
+        if self.func_has_variadic_parameter:
+            fixed_param_count -= 1
+
+        if len(args) < fixed_param_count:
+            raise Exception(
+                    "Route function '%s' expected %d arguments, "
+                    "got %d: %s" %
+                    (self.func_name, fixed_param_count, len(args), args))
+
+        if self.func_has_variadic_parameter:
+            coerced_args = list(args[:fixed_param_count])
+            if len(args) > fixed_param_count:
+                var_arg = tuple(args[fixed_param_count:])
+                coerced_args.append(var_arg)
+        else:
+            coerced_args = args
+
+        metadata = {}
+        for arg_name, arg_val in zip(self.func_parameters, coerced_args):
+            metadata[arg_name] = self._coerceRouteParameter(
+                    arg_name, arg_val)
+
+        if self.is_generator_route:
+            self.generator.onRouteFunctionUsed(self, metadata)
+
+        return self.getUri(metadata)
+
     def _uriFormatRepl(self, m):
         qual = m.group('qual')
         name = m.group('name')
@@ -243,7 +279,7 @@
     def _uriPatternRepl(self, m):
         name = m.group('name')
         qual = m.group('qual')
-        if qual == 'path':
+        if qual == 'path' or m.group('var'):
             return r'(?P<%s>[^\?]*)' % name
         elif qual == 'int4':
             return r'(?P<%s>\d{4})' % name
@@ -269,99 +305,47 @@
                 raise Exception(
                         "Expected route parameter '%s' to be of type "
                         "'%s', but was: %s" %
-                        (k, param_type, route_metadata[k]))
+                        (name, param_type, val))
         if param_type == 'path':
             return val
         raise Exception("Unknown route parameter type: %s" % param_type)
 
-    def _createTemplateFunc(self, func_def):
-        if func_def is None:
-            return
-
-        m = template_func_re.match(func_def)
-        if m is None:
-            raise Exception("Template function definition for route '%s' "
-                            "has invalid syntax: %s" %
-                            (self.uri_pattern, func_def))
-
-        self.template_func_name = m.group('name')
-        self.template_func_args = []
-        arg_list = m.group('args')
-        if arg_list:
-            self.template_func_args = []
-            for m2 in template_func_arg_re.finditer(arg_list):
-                self.template_func_args.append(m2.group('arg'))
-            for i in range(len(self.template_func_args) - 1):
-                if self.template_func_args[i][0] == '+':
-                    raise Exception("Only the last route parameter can be a "
-                                    "variable argument (prefixed with `+`)")
-
-        if (self.template_func_args and
-                self.template_func_args[-1][0] == '+'):
-            self.template_func_vararg = self.template_func_args[-1][1:]
-
-        def template_func(*args):
-            is_variable = (self.template_func_vararg is not None)
-            if not is_variable and len(args) != len(self.template_func_args):
-                raise Exception(
-                        "Route function '%s' expected %d arguments, "
-                        "got %d: %s" %
-                        (func_def, len(self.template_func_args),
-                            len(args), args))
-            elif is_variable and len(args) < len(self.template_func_args):
-                raise Exception(
-                        "Route function '%s' expected at least %d arguments, "
-                        "got %d: %s" %
-                        (func_def, len(self.template_func_args),
-                            len(args), args))
-
-            metadata = {}
-            non_var_args = list(self.template_func_args)
-            if is_variable:
-                del non_var_args[-1]
-
-            for arg_name, arg_val in zip(non_var_args, args):
-                metadata[arg_name] = self._coerceRouteParameter(
-                        arg_name, arg_val)
-
-            if is_variable:
-                metadata[self.template_func_vararg] = []
-                for i in range(len(non_var_args), len(args)):
-                    metadata[self.template_func_vararg].append(args[i])
-
-            if self.is_generator_route:
-                self.generator.onRouteFunctionUsed(self, metadata)
-
-            return self.getUri(metadata)
-
-        self.template_func = template_func
+    def _validateFuncName(self, name):
+        if not name:
+            return None
+        i = name.find('(')
+        if i >= 0:
+            name = name[:i]
+            logger.warning(
+                "Route function names shouldn't contain the list of arguments "
+                "anymore -- just specify '%s'." % name)
+        return name
 
 
 class CompositeRouteFunction(object):
     def __init__(self):
-        self._funcs = []
+        self._routes = []
         self._arg_names = None
 
     def addFunc(self, route):
         if self._arg_names is None:
-            self._arg_names = sorted(route.template_func_args)
+            self._arg_names = list(route.func_parameters)
 
-        if sorted(route.template_func_args) != self._arg_names:
+        if route.func_parameters != self._arg_names:
             raise Exception("Cannot merge route function with arguments '%s' "
                             "with route function with arguments '%s'." %
-                            (route.template_func_args, self._arg_names))
-        self._funcs.append((route, route.template_func))
+                            (route.func_parameters, self._arg_names))
+        self._routes.append(route)
 
     def __call__(self, *args, **kwargs):
-        if len(self._funcs) == 1 or len(args) == len(self._arg_names):
-            f = self._funcs[0][1]
-            return f(*args, **kwargs)
+        if len(self._routes) == 1 or len(args) == len(self._arg_names):
+            return self._routes[0].execTemplateFunc(*args, **kwargs)
 
         if len(args) == len(self._arg_names) + 1:
             f_args = args[:-1]
-            for r, f in self._funcs:
+            for r in self._routes:
                 if r.source_name == args[-1]:
-                    return f(*f_args, **kwargs)
+                    return r.execTemplateFunc(*f_args, **kwargs)
             raise Exception("No such source: %s" % args[-1])
 
         raise Exception("Incorrect number of arguments for route function. "
--- a/piecrust/templating/jinjaengine.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/piecrust/templating/jinjaengine.py	Thu Sep 01 23:00:58 2016 -0700
@@ -236,7 +236,7 @@
 
         # Add route functions.
         for route in app.routes:
-            name = route.template_func_name
+            name = route.func_name
             func = self.globals.get(name)
             if func is None:
                 func = CompositeRouteFunction()
--- a/tests/test_routing.py	Thu Sep 01 22:58:31 2016 -0700
+++ b/tests/test_routing.py	Thu Sep 01 23:00:58 2016 -0700
@@ -26,28 +26,28 @@
 
 
 @pytest.mark.parametrize(
-        'site_root, route_pattern, expected_required_metadata',
+        'site_root, route_pattern, expected_func_parameters',
         [
-            ('/', '/%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'])),
-            ('/~johndoe', '/%foo%', set(['foo'])),
-            ('/~johndoe', '/%path:foo%', set(['foo'])),
-            ('/~johndoe', '/%foo%/%bar%', set(['foo', 'bar'])),
-            ('/~johndoe', '/%foo%/%path:bar%', set(['foo', 'bar']))
+            ('/', '/%foo%', ['foo']),
+            ('/', '/%path:foo%', ['foo']),
+            ('/', '/%foo%/%bar%', ['foo', 'bar']),
+            ('/', '/%foo%/%path:bar%', ['foo', 'bar']),
+            ('/something', '/%foo%', ['foo']),
+            ('/something', '/%path:foo%', ['foo']),
+            ('/something', '/%foo%/%bar%', ['foo', 'bar']),
+            ('/something', '/%foo%/%path:bar%', ['foo', 'bar']),
+            ('/~johndoe', '/%foo%', ['foo']),
+            ('/~johndoe', '/%path:foo%', ['foo']),
+            ('/~johndoe', '/%foo%/%bar%', ['foo', 'bar']),
+            ('/~johndoe', '/%foo%/%path:bar%', ['foo', 'bar'])
             ])
 def test_required_metadata(site_root, route_pattern,
-                           expected_required_metadata):
+                           expected_func_parameters):
     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_route_metadata == expected_required_metadata
+    assert route.func_parameters == expected_func_parameters
 
 
 @pytest.mark.parametrize(