Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router tweaks #2031

Merged
merged 7 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions sanic/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,17 +392,22 @@ def url_for(self, view_name: str, **kwargs):
if getattr(route.ctx, "static", None):
filename = kwargs.pop("filename", "")
# it's static folder
if "file_uri" in uri:
folder_ = uri.split("<file_uri:", 1)[0]
if "__file_uri__" in uri:
folder_ = uri.split("<__file_uri__:", 1)[0]
if folder_.endswith("/"):
folder_ = folder_[:-1]

if filename.startswith("/"):
filename = filename[1:]

kwargs["file_uri"] = filename
kwargs["__file_uri__"] = filename

if uri != "/" and uri.endswith("/"):
if (
uri != "/"
and uri.endswith("/")
and not route.strict
and not route.raw_path[:-1]
):
uri = uri[:-1]

if not uri.startswith("/"):
Expand Down Expand Up @@ -572,25 +577,27 @@ async def handle_request(self, request: Request):
# Define `response` var here to remove warnings about
# allocation before assignment below.
response = None
name = None
try:
# Fetch handler from router
(
route,
handler,
kwargs,
uri,
name,
ignore_body,
) = self.router.get(request)
request.name = name

request._match_info = kwargs
request.route = route
request.name = route.name
request.uri_template = f"/{route.path}"
request.endpoint = request.name

if (
request.stream
and request.stream.request_body
and not ignore_body
and not route.ctx.ignore_body
):
if self.router.is_stream_handler(request):

if hasattr(handler, "is_stream"):
# Streaming handler: lift the size limit
request.stream.request_max_size = float("inf")
else:
Expand All @@ -601,15 +608,15 @@ async def handle_request(self, request: Request):
# Request Middleware
# -------------------------------------------- #
response = await self._run_request_middleware(
request, request_name=name
request, request_name=route.name
)

# No middleware results
if not response:
# -------------------------------------------- #
# Execute Handler
# -------------------------------------------- #

request.uri_template = f"/{uri}"
if handler is None:
raise ServerError(
(
Expand All @@ -618,12 +625,11 @@ async def handle_request(self, request: Request):
)
)

request.endpoint = request.name

# Run response handler
response = handler(request, **kwargs)
if isawaitable(response):
response = await response

if response:
response = await request.respond(response)
else:
Expand Down
24 changes: 14 additions & 10 deletions sanic/mixins/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ def _generate_name(self, *objects) -> str:
else:
break

if not name: # noq
if not name: # noqa
raise ValueError("Could not generate a name for handler")

if not name.startswith(f"{self.name}."):
Expand All @@ -627,19 +627,19 @@ async def _static_request_handler(
stream_large_files,
request,
content_type=None,
file_uri=None,
__file_uri__=None,
):
# Using this to determine if the URL is trying to break out of the path
# served. os.path.realpath seems to be very slow
if file_uri and "../" in file_uri:
if __file_uri__ and "../" in __file_uri__:
raise InvalidUsage("Invalid URL")
# Merge served directory and requested file if provided
# Strip all / that in the beginning of the URL to help prevent python
# from herping a derp and treating the uri as an absolute path
root_path = file_path = file_or_directory
if file_uri:
if __file_uri__:
file_path = path.join(
file_or_directory, sub("^[/]*", "", file_uri)
file_or_directory, sub("^[/]*", "", __file_uri__)
)

# URL decode the path sent by the browser otherwise we won't be able to
Expand All @@ -648,10 +648,12 @@ async def _static_request_handler(
if not file_path.startswith(path.abspath(unquote(root_path))):
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={file_uri}"
f"relative_url={__file_uri__}"
)
raise FileNotFound(
"File not found", path=file_or_directory, relative_url=file_uri
"File not found",
path=file_or_directory,
relative_url=__file_uri__,
)
try:
headers = {}
Expand Down Expand Up @@ -719,10 +721,12 @@ async def _static_request_handler(
except Exception:
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={file_uri}"
f"relative_url={__file_uri__}"
)
raise FileNotFound(
"File not found", path=file_or_directory, relative_url=file_uri
"File not found",
path=file_or_directory,
relative_url=__file_uri__,
)

def _register_static(
Expand Down Expand Up @@ -772,7 +776,7 @@ def _register_static(
# If we're not trying to match a file directly,
# serve from the folder
if not path.isfile(file_or_directory):
uri += "/<file_uri>"
uri += "/<__file_uri__>"

# special prefix for static files
# if not static.name.startswith("_static_"):
Expand Down
5 changes: 5 additions & 0 deletions sanic/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
Union,
)

from sanic_routing.route import Route # type: ignore


if TYPE_CHECKING:
from sanic.server import ConnInfo
Expand Down Expand Up @@ -104,6 +106,7 @@ class Request:
"parsed_forwarded",
"raw_url",
"request_middleware_started",
"route",
"stream",
"transport",
"uri_template",
Expand Down Expand Up @@ -151,6 +154,7 @@ def __init__(
self._match_info: Dict[str, Any] = {}
self.stream: Optional[Http] = None
self.endpoint: Optional[str] = None
self.route: Optional[Route] = None

def __repr__(self):
class_name = self.__class__.__name__
Expand Down Expand Up @@ -431,6 +435,7 @@ def cookies(self) -> Dict[str, str]:
:return: Incoming cookies on the request
:rtype: Dict[str, str]
"""

if self._cookies is None:
cookie = self.headers.get("Cookie")
if cookie is not None:
Expand Down
49 changes: 22 additions & 27 deletions sanic/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
from sanic_routing.route import Route # type: ignore

from sanic.constants import HTTP_METHODS
from sanic.exceptions import MethodNotSupported, NotFound
from sanic.exceptions import MethodNotSupported, NotFound, SanicException
from sanic.handlers import RouteHandler
from sanic.request import Request


ROUTER_CACHE_SIZE = 1024
ALLOWED_LABELS = ("__file_uri__",)


class Router(BaseRouter):
Expand All @@ -33,7 +34,7 @@ class Router(BaseRouter):
@lru_cache(maxsize=ROUTER_CACHE_SIZE)
def _get(
self, path, method, host
) -> Tuple[RouteHandler, Dict[str, Any], str, str, bool]:
) -> Tuple[Route, RouteHandler, Dict[str, Any]]:
try:
route, handler, params = self.resolve(
path=path,
Expand All @@ -50,14 +51,14 @@ def _get(
)

return (
route,
handler,
params,
route.path,
route.name,
route.ctx.ignore_body,
)

def get(self, request: Request):
def get( # type: ignore
self, request: Request
) -> Tuple[Route, RouteHandler, Dict[str, Any]]:
"""
Retrieve a `Route` object containg the details about how to handle
a response for a given request
Expand All @@ -66,14 +67,13 @@ def get(self, request: Request):
:type request: Request
:return: details needed for handling the request and returning the
correct response
:rtype: Tuple[ RouteHandler, Tuple[Any, ...], Dict[str, Any], str, str,
Optional[str], bool, ]
:rtype: Tuple[ Route, RouteHandler, Dict[str, Any]]
"""
return self._get(
request.path, request.method, request.headers.get("host")
)

def add(
def add( # type: ignore
self,
uri: str,
methods: Iterable[str],
Expand Down Expand Up @@ -138,7 +138,7 @@ def add(
if host:
params.update({"requirements": {"host": host}})

route = super().add(**params)
route = super().add(**params) # type: ignore
route.ctx.ignore_body = ignore_body
route.ctx.stream = stream
route.ctx.hosts = hosts
Expand All @@ -150,23 +150,6 @@ def add(
return routes[0]
return routes

def is_stream_handler(self, request) -> bool:
"""
Handler for request is stream or not.

:param request: Request object
:return: bool
"""
try:
handler = self.get(request)[0]
except (NotFound, MethodNotSupported):
return False
if hasattr(handler, "view_class") and hasattr(
handler.view_class, request.method.lower()
):
handler = getattr(handler.view_class, request.method.lower())
return hasattr(handler, "is_stream")

@lru_cache(maxsize=ROUTER_CACHE_SIZE)
def find_route_by_view_name(self, view_name, name=None):
"""
Expand Down Expand Up @@ -204,3 +187,15 @@ def routes_dynamic(self):
@property
def routes_regex(self):
return self.regex_routes

def finalize(self, *args, **kwargs):
super().finalize(*args, **kwargs)

for route in self.dynamic_routes.values():
if any(
label.startswith("__") and label not in ALLOWED_LABELS
for label in route.labels
):
raise SanicException(
f"Invalid route: {route}. Parameter names cannot use '__'."
)
4 changes: 2 additions & 2 deletions tests/benchmark/test_route_resolution_benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async def test_resolve_route_no_arg_string_path(
iterations=1000,
rounds=1000,
)
assert await result[0](None) == 1
assert await result[1](None) == 1

@mark.asyncio
async def test_resolve_route_with_typed_args(
Expand Down Expand Up @@ -72,4 +72,4 @@ async def test_resolve_route_with_typed_args(
iterations=1000,
rounds=1000,
)
assert await result[0](None) == 1
assert await result[1](None) == 1
4 changes: 2 additions & 2 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from inspect import isawaitable
from os import environ
from unittest.mock import patch
from unittest.mock import Mock, patch

import pytest

Expand Down Expand Up @@ -109,7 +109,7 @@ async def handler():

def test_app_handle_request_handler_is_none(app, monkeypatch):
def mockreturn(*args, **kwargs):
return None, {}, "", "", False
return Mock(), None, {}

# Not sure how to make app.router.get() return None, so use mock here.
monkeypatch.setattr(app.router, "get", mockreturn)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_blueprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ def test_static_blueprint_name(static_file_directory, file_name):
app.blueprint(bp)

uri = app.url_for("static", name="static.testing")
assert uri == "/static/test.file"
assert uri == "/static/test.file/"

_, response = app.test_client.get("/static/test.file")
assert response.status == 404
Expand Down
1 change: 0 additions & 1 deletion tests/test_exceptions_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ def test_html_traceback_output_in_debug_mode():
assert response.status == 500
soup = BeautifulSoup(response.body, "html.parser")
html = str(soup)
print(html)

assert "response = handler(request, **kwargs)" in html
assert "handler_4" in html
Expand Down
1 change: 0 additions & 1 deletion tests/test_reloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ def complete(*args):
def scanner(proc):
for line in proc.stdout:
line = line.decode().strip()
print(">", line)
if line.startswith("complete"):
yield line

Expand Down
9 changes: 9 additions & 0 deletions tests/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,12 @@ async def get(request):
"/", headers={"SOME-OTHER-REQUEST-ID": f"{REQUEST_ID}"}
)
assert request.id == REQUEST_ID * 2


def test_route_assigned_to_request(app):
@app.get("/")
async def get(request):
return response.empty()

request, _ = app.test_client.get("/")
assert request.route is list(app.router.routes.values())[0]
1 change: 0 additions & 1 deletion tests/test_request_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,6 @@ async def read_chunk():
data = await reader.read(4096)
assert data
buffer += data
print(res)
assert buffer[size : size + 2] == b"\r\n"
ret, buffer = buffer[:size], buffer[size + 2 :]
return ret
Expand Down
Loading