Add logic to make dynamic route merging work
This is by no means the final solution but it's a start in the right direction. Eventually what needs to happen is we need to reduce the complexity of the routing. CompsitionView can probably be removed later on in favor of better Route objects. Also in the next version of sanic we need to move merge_route and add_parameter out of the add_route logic and just have them as standalone methods. The tests should cover everything that we need so that if any changes are made we can identify regression.
This commit is contained in:
parent
d3344da9c5
commit
0a5fa72099
|
@ -149,7 +149,22 @@ class Router:
|
|||
handler=view, methods=methods.union(route.methods))
|
||||
return route
|
||||
|
||||
route = self.routes_all.get(uri)
|
||||
if parameters:
|
||||
# TODO: This is too complex, we need to reduce the complexity
|
||||
if properties['unhashable']:
|
||||
routes_to_check = self.routes_always_check
|
||||
ndx, route = self.check_dynamic_route_exists(
|
||||
pattern, routes_to_check)
|
||||
else:
|
||||
routes_to_check = self.routes_dynamic[url_hash(uri)]
|
||||
ndx, route = self.check_dynamic_route_exists(
|
||||
pattern, routes_to_check)
|
||||
if ndx != -1:
|
||||
# Pop the ndx of the route, no dups of the same route
|
||||
routes_to_check.pop(ndx)
|
||||
else:
|
||||
route = self.routes_all.get(uri)
|
||||
|
||||
if route:
|
||||
route = merge_route(route, methods, handler)
|
||||
else:
|
||||
|
@ -165,6 +180,14 @@ class Router:
|
|||
else:
|
||||
self.routes_static[uri] = route
|
||||
|
||||
@staticmethod
|
||||
def check_dynamic_route_exists(pattern, routes_to_check):
|
||||
for ndx, route in enumerate(routes_to_check):
|
||||
if route.pattern == pattern:
|
||||
return ndx, route
|
||||
else:
|
||||
return -1, None
|
||||
|
||||
def remove(self, uri, clean_cache=True, host=None):
|
||||
if host is not None:
|
||||
uri = host + uri
|
||||
|
|
46
tests/test_dynamic_routes.py
Normal file
46
tests/test_dynamic_routes.py
Normal file
|
@ -0,0 +1,46 @@
|
|||
from sanic import Sanic
|
||||
from sanic.response import text
|
||||
from sanic.utils import sanic_endpoint_test
|
||||
from sanic.router import RouteExists
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.mark.parametrize("method,attr, expected", [
|
||||
("get", "text", "OK1 test"),
|
||||
("post", "text", "OK2 test"),
|
||||
("put", "text", "OK2 test"),
|
||||
("delete", "status", 405),
|
||||
])
|
||||
def test_overload_dynamic_routes(method, attr, expected):
|
||||
app = Sanic('test_dynamic_route')
|
||||
|
||||
@app.route('/overload/<param>', methods=['GET'])
|
||||
async def handler1(request, param):
|
||||
return text('OK1 ' + param)
|
||||
|
||||
@app.route('/overload/<param>', methods=['POST', 'PUT'])
|
||||
async def handler2(request, param):
|
||||
return text('OK2 ' + param)
|
||||
|
||||
request, response = sanic_endpoint_test(
|
||||
app, method, uri='/overload/test')
|
||||
assert getattr(response, attr) == expected
|
||||
|
||||
|
||||
def test_overload_dynamic_routes_exist():
|
||||
app = Sanic('test_dynamic_route')
|
||||
|
||||
@app.route('/overload/<param>', methods=['GET'])
|
||||
async def handler1(request, param):
|
||||
return text('OK1 ' + param)
|
||||
|
||||
@app.route('/overload/<param>', methods=['POST', 'PUT'])
|
||||
async def handler2(request, param):
|
||||
return text('OK2 ' + param)
|
||||
|
||||
# if this doesn't raise an error, than at least the below should happen:
|
||||
# assert response.text == 'Duplicated'
|
||||
with pytest.raises(RouteExists):
|
||||
@app.route('/overload/<param>', methods=['PUT', 'DELETE'])
|
||||
async def handler3(request):
|
||||
return text('Duplicated')
|
Loading…
Reference in New Issue
Block a user