Skip to content

Commit

Permalink
Refactor URL Dispatcher
Browse files Browse the repository at this point in the history
  • Loading branch information
@klen
klen committed Mar 13, 2015
1 parent f0caa65 commit f4b5552
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 68 deletions.
68 changes: 25 additions & 43 deletions aiohttp/web.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,7 @@ class UrlDispatcher(AbstractRouter, collections.abc.Mapping):
DYN_WITH_RE = re.compile(
r'^\{(?P<var>[a-zA-Z][_a-zA-Z0-9]*):(?P<re>.+)\}$')
GOOD = r'[^{}/]+'
PLAIN = re.compile('^' + GOOD + '$')
ROUTE_RE = re.compile(r'(\{[_a-zA-Z][^{}]*(?:\{[^{}]*\}[^{}]*)*\})')

METHODS = {'POST', 'GET', 'PUT', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'}

Expand Down Expand Up @@ -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

Expand Down
42 changes: 17 additions & 25 deletions tests/test_urldispatch.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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<to>+++)': nothing to repeat"), s)
self.assertEqual(
"Bad pattern '\/handler\/(?P<to>+++)': nothing to repeat",
str(ctx.exception))
self.assertIsNone(ctx.exception.__cause__)

def test_route_dynamic_with_regex_spec(self):
Expand Down Expand Up @@ -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<var>.*)$'), 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')

0 comments on commit f4b5552

Please sign in to comment.