From f4b5552c49a43359d3e2a14dc7d0d44262f9e41f Mon Sep 17 00:00:00 2001 From: Kirill Klenov Date: Fri, 13 Mar 2015 19:41:45 +0300 Subject: [PATCH] Refactor URL Dispatcher --- aiohttp/web.py | 68 ++++++++++++++------------------------- tests/test_urldispatch.py | 42 ++++++++++-------------- 2 files changed, 42 insertions(+), 68 deletions(-) diff --git a/aiohttp/web.py b/aiohttp/web.py index fe38569c2c..bb5242847f 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -1383,7 +1383,7 @@ class UrlDispatcher(AbstractRouter, collections.abc.Mapping): DYN_WITH_RE = re.compile( r'^\{(?P[a-zA-Z][_a-zA-Z0-9]*):(?P.+)\}$') GOOD = r'[^{}/]+' - PLAIN = re.compile('^' + GOOD + '$') + ROUTE_RE = re.compile(r'(\{[_a-zA-Z][^{}]*(?:\{[^{}]*\}[^{}]*)*\})') METHODS = {'POST', 'GET', 'PUT', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'} @@ -1444,59 +1444,41 @@ def add_route(self, method, path, handler, method = method.upper() assert method in self.METHODS, method - if isinstance(path, retype): - route = DynamicRoute( - method, handler, name, path, - path.pattern, expect_handler=expect_handler) - + if not ('{' in path or '}' in path or self.ROUTE_RE.search(path)): + route = PlainRoute( + method, handler, name, path, expect_handler=expect_handler) self._register_endpoint(route) return route - assert path.startswith('/') - parts = [] - factory = PlainRoute - format_parts = [] - for part in path.split('/'): - if not part: - continue + pattern = '' + formatter = '' + for part in self.ROUTE_RE.split(path): match = self.DYN.match(part) if match: - parts.append('(?P<' + match.group('var') + '>' + - self.GOOD + ')') - factory = DynamicRoute - format_parts.append('{'+match.group('var')+'}') + pattern += '(?P<' + match.group('var') + '>' + self.GOOD + ')' + formatter += '{%s}' % match.group('var') continue match = self.DYN_WITH_RE.match(part) if match: - parts.append('(?P<' + match.group('var') + '>' + - match.group('re') + ')') - factory = DynamicRoute - format_parts.append('{'+match.group('var')+'}') - continue - if self.PLAIN.match(part): - parts.append(re.escape(part)) - format_parts.append(part) + pattern += '(?P<%(var)s>%(re)s)' % match.groupdict() + formatter += '{%s}' % match.group('var') continue - raise ValueError("Invalid path '{}'['{}']".format(path, part)) - if factory is PlainRoute: - route = PlainRoute( - method, handler, name, path, expect_handler=expect_handler) - else: - pattern = '/' + '/'.join(parts) - formatter = '/' + '/'.join(format_parts) - if path.endswith('/') and pattern != '/': - pattern += '/' - formatter += '/' - try: - compiled = re.compile('^' + pattern + '$') - except re.error as exc: - raise ValueError( - "Bad pattern '{}': {}".format(pattern, exc)) from None - route = DynamicRoute( - method, handler, name, compiled, - formatter, expect_handler=expect_handler) + if '{' in part or '}' in part: + raise ValueError("Invalid path '{}'['{}']".format(path, part)) + + formatter += part + pattern += re.escape(part) + + try: + compiled = re.compile('^' + pattern + '$') + except re.error as exc: + raise ValueError( + "Bad pattern '{}': {}".format(pattern, exc)) from None + route = DynamicRoute( + method, handler, name, compiled, + formatter, expect_handler=expect_handler) self._register_endpoint(route) return route diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 2f7e102df2..eea66448be 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -112,11 +112,6 @@ def test_add_url_invalid4(self): with self.assertRaises(ValueError): self.router.add_route('post', '/post/{id"}', handler) - def test_add_url_invalid5(self): - handler = self.make_handler() - with self.assertRaises(ValueError): - self.router.add_route('post', '/post"{id}', handler) - def test_add_url_escaping(self): handler = self.make_handler() self.router.add_route('GET', '/+$', handler) @@ -298,6 +293,20 @@ def test_add_route_with_re(self): self.assertIsNotNone(info) self.assertEqual({'to': '1234'}, info) + def test_add_route_with_re_and_slashes(self): + handler = self.make_handler() + self.router.add_route('GET', r'/handler/{to:[^/]+/?}', handler) + req = self.make_request('GET', '/handler/1234/') + info = self.loop.run_until_complete(self.router.resolve(req)) + self.assertIsNotNone(info) + self.assertEqual({'to': '1234/'}, info) + + self.router.add_route('GET', r'/handler/{to:.+}', handler) + req = self.make_request('GET', '/handler/1234/5/6/7') + info = self.loop.run_until_complete(self.router.resolve(req)) + self.assertIsNotNone(info) + self.assertEqual({'to': '1234/5/6/7'}, info) + def test_add_route_with_re_not_match(self): handler = self.make_handler() self.router.add_route('GET', r'/handler/{to:\d+}', handler) @@ -322,9 +331,9 @@ def test_add_route_with_invalid_re(self): handler = self.make_handler() with self.assertRaises(ValueError) as ctx: self.router.add_route('GET', r'/handler/{to:+++}', handler) - s = str(ctx.exception) - self.assertTrue(s.startswith( - "Bad pattern '/handler/(?P+++)': nothing to repeat"), s) + self.assertEqual( + "Bad pattern '\/handler\/(?P+++)': nothing to repeat", + str(ctx.exception)) self.assertIsNone(ctx.exception.__cause__) def test_route_dynamic_with_regex_spec(self): @@ -426,20 +435,3 @@ def handler(request): self.assertRaises( AssertionError, self.router.add_route, 'GET', '/', self.make_handler(), expect_handler=handler) - - def test_raw_regexp(self): - import re - - handler = self.make_handler() - self.router.add_route('GET', re.compile('^/test/(.*)$'), handler) - req = self.make_request('GET', '/test/foo/bar') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertIsNotNone(info) - self.assertEqual(0, len(info)) - self.assertIs(handler, info.handler) - self.assertIsNone(info.route.name) - - self.router.add_route('GET', re.compile('^/var/(?P.*)$'), handler) - req = self.make_request('GET', '/var/foo/bar') - info = self.loop.run_until_complete(self.router.resolve(req)) - self.assertEqual(info.get('var'), 'foo/bar')