f4b8b73c3d
When a single segment in a requested path can match more than one
node at that branch in the routing tree, and the first branch taken
happens to be the wrong one (i.e., the subsequent nodes do not match,
but they would have under a different branch), do not mask the other
branches that could result in a successful resolution of the requested
path.
I chose to implement this by not immediately returning None when a
branch prediction turns out to be false. Alternatively, I could have
introduced functions but that would likely incur more overhead vs.
performing a few additional node comparisons on the way back up the
routing tree. I did, however, include a simple optimization that
allows a "fast return" when only simple nodes are involved in the
branching, since in that case there is only one possible route.
More advanced optimizations are possible, and may be attempted in
the future. For example:
* Graft the alternate branches directly into each point where the
routing tree would otherwise 'return None'. This would balloon the
size of the generated code, but would be akin to inline expansion
of a routing representing the traversal of each branch where
branch mispredictions are possible.
* Introspect competing regular expressions to determine whether it
is actually possible for them to match the same string given in
the path.
* Set a flag that can be referenced when returning from a branch
misprediction in order to short-circuit checks. This may or may
not be faster that some of the checks (i.e., those that check
path length would not benefit from this flag, while those that
index into the path list and perform a string comparison may be).
* Rewrite the bytecode to allow direct jumps to alternate branches,
ala https://github.com/snoack/python-goto
Fixes #702
298 lines
10 KiB
Python
298 lines
10 KiB
Python
import ddt
|
|
|
|
from falcon.routing import DefaultRouter
|
|
import falcon.testing as testing
|
|
|
|
|
|
class ResourceWithId(object):
|
|
def __init__(self, resource_id):
|
|
self.resource_id = resource_id
|
|
|
|
def __repr__(self):
|
|
return 'ResourceWithId({0})'.format(self.resource_id)
|
|
|
|
def on_get(self, req, resp):
|
|
resp.body = self.resource_id
|
|
|
|
|
|
class TestRegressionCases(testing.TestBase):
|
|
"""Test specific repros reported by users of the framework."""
|
|
|
|
def before(self):
|
|
self.router = DefaultRouter()
|
|
|
|
def test_versioned_url(self):
|
|
self.router.add_route('/{version}/messages', {}, ResourceWithId(2))
|
|
|
|
resource, method_map, params = self.router.find('/v2/messages')
|
|
self.assertEqual(resource.resource_id, 2)
|
|
|
|
self.router.add_route('/v2', {}, ResourceWithId(1))
|
|
|
|
resource, method_map, params = self.router.find('/v2')
|
|
self.assertEqual(resource.resource_id, 1)
|
|
|
|
resource, method_map, params = self.router.find('/v2/messages')
|
|
self.assertEqual(resource.resource_id, 2)
|
|
|
|
resource, method_map, params = self.router.find('/v1/messages')
|
|
self.assertEqual(resource.resource_id, 2)
|
|
|
|
resource, method_map, params = self.router.find('/v1')
|
|
self.assertIs(resource, None)
|
|
|
|
def test_recipes(self):
|
|
self.router.add_route(
|
|
'/recipes/{activity}/{type_id}', {}, ResourceWithId(1))
|
|
self.router.add_route(
|
|
'/recipes/baking', {}, ResourceWithId(2))
|
|
|
|
resource, method_map, params = self.router.find('/recipes/baking/4242')
|
|
self.assertEqual(resource.resource_id, 1)
|
|
|
|
resource, method_map, params = self.router.find('/recipes/baking')
|
|
self.assertEqual(resource.resource_id, 2)
|
|
|
|
resource, method_map, params = self.router.find('/recipes/grilling')
|
|
self.assertIs(resource, None)
|
|
|
|
|
|
@ddt.ddt
|
|
class TestComplexRouting(testing.TestBase):
|
|
def before(self):
|
|
self.router = DefaultRouter()
|
|
|
|
self.router.add_route(
|
|
'/repos', {}, ResourceWithId(1))
|
|
self.router.add_route(
|
|
'/repos/{org}', {}, ResourceWithId(2))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}', {}, ResourceWithId(3))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/commits', {}, ResourceWithId(4))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}',
|
|
{}, ResourceWithId(5))
|
|
|
|
self.router.add_route(
|
|
'/teams/{id}', {}, ResourceWithId(6))
|
|
self.router.add_route(
|
|
'/teams/{id}/members', {}, ResourceWithId(7))
|
|
|
|
self.router.add_route(
|
|
'/teams/default', {}, ResourceWithId(19))
|
|
self.router.add_route(
|
|
'/teams/default/members/thing', {}, ResourceWithId(19))
|
|
|
|
self.router.add_route(
|
|
'/user/memberships', {}, ResourceWithId(8))
|
|
self.router.add_route(
|
|
'/emojis', {}, ResourceWithId(9))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}/full',
|
|
{}, ResourceWithId(10))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/all', {}, ResourceWithId(11))
|
|
|
|
# NOTE(kgriffs): The ordering of these calls is significant; we
|
|
# need to test that the {id} field does not match the other routes,
|
|
# regardless of the order they are added.
|
|
self.router.add_route(
|
|
'/emojis/signs/0', {}, ResourceWithId(12))
|
|
self.router.add_route(
|
|
'/emojis/signs/{id}', {}, ResourceWithId(13))
|
|
self.router.add_route(
|
|
'/emojis/signs/42', {}, ResourceWithId(14))
|
|
self.router.add_route(
|
|
'/emojis/signs/42/small', {}, ResourceWithId(14.1))
|
|
self.router.add_route(
|
|
'/emojis/signs/78/small', {}, ResourceWithId(22))
|
|
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/{usr0}:{branch0}...{usr1}:{branch1}/part',
|
|
{}, ResourceWithId(15))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/{usr0}:{branch0}',
|
|
{}, ResourceWithId(16))
|
|
self.router.add_route(
|
|
'/repos/{org}/{repo}/compare/{usr0}:{branch0}/full',
|
|
{}, ResourceWithId(17))
|
|
|
|
self.router.add_route(
|
|
'/gists/{id}/{representation}', {}, ResourceWithId(21))
|
|
self.router.add_route(
|
|
'/gists/{id}/raw', {}, ResourceWithId(18))
|
|
self.router.add_route(
|
|
'/gists/first', {}, ResourceWithId(20))
|
|
|
|
@ddt.data(
|
|
'/teams/{collision}', # simple vs simple
|
|
'/emojis/signs/{id_too}', # another simple vs simple
|
|
'/repos/{org}/{repo}/compare/{complex}:{vs}...{complex2}:{collision}',
|
|
)
|
|
def test_collision(self, template):
|
|
self.assertRaises(
|
|
ValueError,
|
|
self.router.add_route, template, {}, ResourceWithId(-1)
|
|
)
|
|
|
|
@ddt.data(
|
|
'/repos/{org}/{repo}/compare/{simple-vs-complex}',
|
|
'/repos/{complex}.{vs}.{simple}',
|
|
'/repos/{org}/{repo}/compare/{complex}:{vs}...{complex2}/full',
|
|
)
|
|
def test_non_collision(self, template):
|
|
self.router.add_route(template, {}, ResourceWithId(-1))
|
|
|
|
def test_dump(self):
|
|
print(self.router._src)
|
|
|
|
def test_override(self):
|
|
self.router.add_route('/emojis/signs/0', {}, ResourceWithId(-1))
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/0')
|
|
self.assertEqual(resource.resource_id, -1)
|
|
|
|
def test_literal_segment(self):
|
|
resource, method_map, params = self.router.find('/emojis/signs/0')
|
|
self.assertEqual(resource.resource_id, 12)
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/1')
|
|
self.assertEqual(resource.resource_id, 13)
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/42')
|
|
self.assertEqual(resource.resource_id, 14)
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/42/small')
|
|
self.assertEqual(resource.resource_id, 14.1)
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/1/small')
|
|
self.assertEqual(resource, None)
|
|
|
|
@ddt.data(
|
|
'/teams',
|
|
'/emojis/signs',
|
|
'/gists',
|
|
'/gists/42',
|
|
)
|
|
def test_dead_segment(self, template):
|
|
resource, method_map, params = self.router.find(template)
|
|
self.assertIs(resource, None)
|
|
|
|
def test_malformed_pattern(self):
|
|
resource, method_map, params = self.router.find(
|
|
'/repos/racker/falcon/compare/foo')
|
|
self.assertIs(resource, None)
|
|
|
|
resource, method_map, params = self.router.find(
|
|
'/repos/racker/falcon/compare/foo/full')
|
|
self.assertIs(resource, None)
|
|
|
|
def test_literal(self):
|
|
resource, method_map, params = self.router.find('/user/memberships')
|
|
self.assertEqual(resource.resource_id, 8)
|
|
|
|
def test_variable(self):
|
|
resource, method_map, params = self.router.find('/teams/42')
|
|
self.assertEqual(resource.resource_id, 6)
|
|
self.assertEqual(params, {'id': '42'})
|
|
|
|
resource, method_map, params = self.router.find('/emojis/signs/stop')
|
|
self.assertEqual(params, {'id': 'stop'})
|
|
|
|
resource, method_map, params = self.router.find('/gists/42/raw')
|
|
self.assertEqual(params, {'id': '42'})
|
|
|
|
@ddt.data(
|
|
('/teams/default', 19),
|
|
('/teams/default/members', 7),
|
|
('/teams/foo', 6),
|
|
('/teams/foo/members', 7),
|
|
('/gists/first', 20),
|
|
('/gists/first/raw', 18),
|
|
('/gists/first/pdf', 21),
|
|
('/gists/1776/pdf', 21),
|
|
('/emojis/signs/78', 13),
|
|
('/emojis/signs/78/small', 22),
|
|
)
|
|
@ddt.unpack
|
|
def test_literal_vs_variable(self, path, expected_id):
|
|
resource, method_map, params = self.router.find(path)
|
|
self.assertEqual(resource.resource_id, expected_id)
|
|
|
|
@ddt.data(
|
|
# Misc.
|
|
'/this/does/not/exist',
|
|
'/user/bogus',
|
|
'/repos/racker/falcon/compare/johndoe:master...janedoe:dev/bogus',
|
|
|
|
# Literal vs variable (teams)
|
|
'/teams',
|
|
'/teams/42/members/undefined',
|
|
'/teams/42/undefined',
|
|
'/teams/42/undefined/segments',
|
|
'/teams/default/members/undefined',
|
|
'/teams/default/members/thing/undefined',
|
|
'/teams/default/members/thing/undefined/segments',
|
|
'/teams/default/undefined',
|
|
'/teams/default/undefined/segments',
|
|
|
|
# Literal vs variable (emojis)
|
|
'/emojis/signs',
|
|
'/emojis/signs/0/small',
|
|
'/emojis/signs/0/undefined',
|
|
'/emojis/signs/0/undefined/segments',
|
|
'/emojis/signs/20/small',
|
|
'/emojis/signs/20/undefined',
|
|
'/emojis/signs/42/undefined',
|
|
'/emojis/signs/78/undefined',
|
|
)
|
|
def test_not_found(self, path):
|
|
resource, method_map, params = self.router.find(path)
|
|
self.assertIs(resource, None)
|
|
|
|
def test_subsegment_not_found(self):
|
|
resource, method_map, params = self.router.find('/emojis/signs/0/x')
|
|
self.assertIs(resource, None)
|
|
|
|
def test_multivar(self):
|
|
resource, method_map, params = self.router.find(
|
|
'/repos/racker/falcon/commits')
|
|
self.assertEqual(resource.resource_id, 4)
|
|
self.assertEqual(params, {'org': 'racker', 'repo': 'falcon'})
|
|
|
|
resource, method_map, params = self.router.find(
|
|
'/repos/racker/falcon/compare/all')
|
|
self.assertEqual(resource.resource_id, 11)
|
|
self.assertEqual(params, {'org': 'racker', 'repo': 'falcon'})
|
|
|
|
@ddt.data(('', 5), ('/full', 10), ('/part', 15))
|
|
@ddt.unpack
|
|
def test_complex(self, url_postfix, resource_id):
|
|
uri = '/repos/racker/falcon/compare/johndoe:master...janedoe:dev'
|
|
resource, method_map, params = self.router.find(uri + url_postfix)
|
|
|
|
self.assertEqual(resource.resource_id, resource_id)
|
|
self.assertEqual(params, {
|
|
'org': 'racker',
|
|
'repo': 'falcon',
|
|
'usr0': 'johndoe',
|
|
'branch0': 'master',
|
|
'usr1': 'janedoe',
|
|
'branch1': 'dev',
|
|
})
|
|
|
|
@ddt.data(('', 16), ('/full', 17))
|
|
@ddt.unpack
|
|
def test_complex_alt(self, url_postfix, resource_id):
|
|
uri = '/repos/falconry/falcon/compare/johndoe:master'
|
|
resource, method_map, params = self.router.find(uri + url_postfix)
|
|
|
|
self.assertEqual(resource.resource_id, resource_id)
|
|
self.assertEqual(params, {
|
|
'org': 'falconry',
|
|
'repo': 'falcon',
|
|
'usr0': 'johndoe',
|
|
'branch0': 'master',
|
|
})
|