Comments (10)
Immediately obvious solution would be:
def find_route(path, router, basket, extra):
parts = tuple(path[1:].split(router.delimiter))
num = len(parts)
if num > 0:
basket[0] = parts[0]
if num == 2:
if parts[1] == "bar":
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>/bar'
return router.dynamic_routes[('<foo:int>', 'bar')], basket
else: #<- New else condition
raise NotFound #<- New raise NotFound
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>'
return router.dynamic_routes[('<foo:int>',)], basket
raise NotFound
But that seems like cheating, I like the idea that the logic falls through to the bottom for NotFound
.
Maybe it should be this?:
def find_route(path, router, basket, extra):
parts = tuple(path[1:].split(router.delimiter))
num = len(parts)
if num > 0:
basket[0] = parts[0]
if num == 1:
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>'
return router.dynamic_routes[('<foo:int>',)], basket
if num == 2:
if parts[1] == "bar":
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>/bar'
return router.dynamic_routes[('<foo:int>', 'bar')], basket
raise NotFound
from sanic-routing.
Or is it just missing a single else
?
def find_route(path, router, basket, extra):
parts = tuple(path[1:].split(router.delimiter))
num = len(parts)
if num > 0:
basket[0] = parts[0]
if num == 2:
if parts[1] == "bar":
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>/bar'
return router.dynamic_routes[('<foo:int>', 'bar')], basket
else: #<-- single new else
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>'
return router.dynamic_routes[('<foo:int>',)], basket
raise NotFound
from sanic-routing.
Ok, I've been diving into the code and the logic within the router AST generation, and I understand it much better now.
It looks like the optimize()
step where it normally injects raise NotFound
locations is missing one, the find_route_src
should look like this:
def find_route(path, router, basket, extra):
parts = tuple(path[1:].split(router.delimiter))
num = len(parts)
if num > 0:
basket[0] = parts[0]
if num == 2:
if parts[1] == "bar":
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>/bar'
return router.dynamic_routes[('<foo:int>', 'bar')], basket
raise NotFound #<- New raise NotFound
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>'
return router.dynamic_routes[('<foo:int>',)], basket
raise NotFound
But thinking about it further, that won't fix the case where the path is /0/aaa/bbb
because in that case num==3
, so it falls through to the bucket[0]
parameter conversion.
from sanic-routing.
I've narrowed down a test which covers all of the concerns above:
def test_use_route_bug16():
router = Router()
def h1(foo):
return "first"
router.add("/test/<foo:int>", h1)
def h2(foo):
return "second"
router.add("/test/<foo:int>/bar", h2)
router.finalize()
with pytest.raises(NotFound):
(route, handler, params) = router.get("/test/foo/aaaa", "BASE")
with pytest.raises(NotFound):
(route, handler, params) = router.get("/test/0/aaaa", "BASE")
with pytest.raises(NotFound):
(route, handler, params) = router.get("/test/0/aaaa/bbbb", "BASE")
from sanic-routing.
Nice work, I will take a look later. It feels to me like you mentioned that the problem is in the optimize()
method.
from sanic-routing.
I am wondering if we need to do this:
def find_route(path, router, basket, extra):
parts = tuple(path[1:].split(router.delimiter))
num = len(parts)
if num > 0:
basket[0] = parts[0]
if num > 2: #<- New condition
raise NotFound #<- New raise NotFound
elif num == 2: #<- Changed from if
if parts[1] == "bar":
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>/bar'
return router.dynamic_routes[('<foo:int>', 'bar')], basket
raise NotFound #<- New raise NotFound
try:
basket['__params__']['foo'] = int(basket[0])
except ValueError:
...
else:
basket['__raw_path__'] = '<foo:int>'
return router.dynamic_routes[('<foo:int>',)], basket
raise NotFound
from sanic-routing.
Yep, I think it's actually two different bugs here on top of each other.
That fix would take care of both of them, but does introduce a new if
statement into the route, which may hurt router performance a tiny bit.
from sanic-routing.
That fix would take care of both of them, but does introduce a new
if
statement into the route, which may hurt router performance a tiny bit.
Yup, which is something that I want to play with still.
Originally, it only ever used >
as an operator. But, then I realized that we may want to bail out and use ==
if there is nothing deeper in the node chain. We potentially could go back, and remove this:
if (
self.last
and self.route
and not self.children
and not self.route.requirements
):
Or, at least limit it even further in scope where there is nothing at the parent node level as a fallback.
So, in this case it would be:
if num > 0:
basket[0] = parts[0]
if num > 2: #<- Revert condition so it is not applicable since there is the parent level fallback
if parts[1] == "bar":
...
raise NotFound #<- New raise NotFound
...
from sanic-routing.
I was playing yesterday with changing the logic to use >=
for all of the comparisons up the chain, then !=
for NotFound
at the end. But I couldn't get it to work quite how I wanted it, I don't have enough experience with the tree rendering code.
from sanic-routing.
I was playing yesterday with changing the logic to use
>=
for all of the comparisons up the chain, then!=
forNotFound
at the end. But I couldn't get it to work quite how I wanted it, I don't have enough experience with the tree rendering code.
I'll add some comments so it will be easier to follow and we can start iterating/improving.
from sanic-routing.
Related Issues (20)
- `RouteExists` thrown when registering a directory and a missing file to the same route. HOT 1
- path_to_parts parsing bug HOT 6
- Pattern registration auto-compile HOT 1
- Context managed code generation
- Having two or more <path> routes does not work with different methods HOT 6
- Regex params not type cast
- `404`s with parameter type `path` HOT 11
- Invalid declaration: <entity_id>:meta HOT 4
- Incorrect handling of `pattern` argument for `register_pattern` HOT 2
- Unique route names
- [Feature] Fallback route when none match HOT 2
- Colon in uri generate response 404 HOT 2
- parts_to_path, raises exception for part with multiple parameters without type hint HOT 1
- Websocket conflicts with http GET
- Unquoted URLs should be case-insensitive
- Parameter types are broken when using <something:path>
- "unquote=True" breaks int path parameters HOT 1
- Parameterized Route returns 404 instead of 405 HOT 2
- Expandable routes have more priority than static routes when adding a trailing slash to the path HOT 2
- Requirements based on equality
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sanic-routing.