Skip to content

Commit

Permalink
bug(URI template): allow complex vars coexist with simple var
Browse files Browse the repository at this point in the history
Allow situation like this:

```python
api.add_route('/files/{file_id}', resource_1)
api.add_route('/files/{file_id}.{ext}', resource_2)
```

Which ValueError was raised due to only one var node was allowed whether
other nodes are complex or not. This fix changes the behaviour so now it
only applys to simple vars.

See #564
  • Loading branch information
philiptzou committed Nov 3, 2015
1 parent bc470f6 commit e6e2d5a
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 9 deletions.
25 changes: 16 additions & 9 deletions falcon/routing/compiled.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,20 @@ def line(text, indent_offset=0):
level_indent = indent
found_simple = False

# NOTE(kgriffs): Sort static nodes before var nodes so that
# none of them get masked. False sorts before True.
nodes = sorted(nodes, key=lambda node: node.is_var)
# NOTE(kgriffs & philiptzou): Sort nodes in this sequence:
# static nodes(0), complex var nodes(1) and simple var nodes(2).
# so that none of them get masked.
nodes = sorted(
nodes, key=lambda node: node.is_var + (node.is_var and
not node.is_complex))

for node in nodes:
if node.is_var:
if node.is_complex:
# NOTE(richardolsson): Complex nodes are nodes which contain
# anything more than a single literal or variable, and they
# need to be checked using a pre-compiled regular expression.
# NOTE(richardolsson): Complex nodes are nodes which
# contain anything more than a single literal or variable,
# and they need to be checked using a pre-compiled regular
# expression.
expression_idx = len(self._expressions)
self._expressions.append(node.var_regex)

Expand All @@ -143,7 +147,8 @@ def line(text, indent_offset=0):
# /foo/{id}/bar
# /foo/{name}/bar
#
assert len([node for node in nodes if node.is_var]) == 1
assert len([_node for _node in nodes
if _node.is_var and not _node.is_complex]) == 1
found_simple = True

else:
Expand Down Expand Up @@ -289,7 +294,8 @@ def conflicts_with(self, segment):

return False
else:
# NOTE(kgriffs): Falcon does not support the following:
# NOTE(kgriffs & philiptzou): Falcon does not accept multiple
# simple var nodes exist at the same level as following:
#
# /foo/{thing1}
# /foo/{thing2}
Expand All @@ -298,8 +304,9 @@ def conflicts_with(self, segment):
#
# /foo/{thing1}
# /foo/all
# /foo/{thing1}.{ext}
#
return other.is_var
return other.is_var and not other.is_complex

# NOTE(kgriffs): If self is a static string match, then all the cases
# for other are False, so no need to check.
Expand Down
44 changes: 44 additions & 0 deletions tests/test_uri_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,28 @@ def on_get(self, req, resp, id, name51):
self.called = True


class FileResource(object):
def __init__(self):
self.file_id = None
self.called = False

def on_get(self, req, resp, file_id):
self.file_id = file_id
self.called = True


class FileDetailsResource(object):
def __init__(self):
self.file_id = None
self.ext = None
self.called = False

def on_get(self, req, resp, file_id, ext):
self.file_id = file_id
self.ext = ext
self.called = True


class TestUriTemplates(testing.TestBase):

def before(self):
Expand Down Expand Up @@ -219,3 +241,25 @@ def test_relative_path(self):

self.assertRaises(ValueError, self.api.add_route,
'no/leading_slash', self.resource)

def test_same_level_complex_var(self):
resource = FileResource()
details_resource = FileDetailsResource()
self.api.add_route('/files/{file_id}', resource)
self.api.add_route('/files/{file_id}.{ext}', details_resource)

# dots cause ambiguous in filenames
file_id_1 = self.getUniqueString().replace('.', '-')
file_id_2 = self.getUniqueString().replace('.', '-')
ext = self.getUniqueString().replace('.', '-')
path_1 = '/files/' + file_id_1
path_2 = '/files/' + file_id_2 + '.' + ext

self.simulate_request(path_1)
self.assertTrue(resource.called)
self.assertEqual(resource.file_id, file_id_1)

self.simulate_request(path_2)
self.assertTrue(details_resource.called)
self.assertEqual(details_resource.file_id, file_id_2)
self.assertEqual(details_resource.ext, ext)

0 comments on commit e6e2d5a

Please sign in to comment.