From d614823013b38ae2026c7a7937ab0667c5dddfaf Mon Sep 17 00:00:00 2001 From: Suby Raman Date: Thu, 2 Feb 2017 16:24:16 -0500 Subject: [PATCH 1/4] rebase --- sanic/router.py | 38 +++++++++++++++++++++++++----- sanic/sanic.py | 12 +++++++++- sanic/views.py | 21 ++++++++--------- tests/test_views.py | 56 ++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 107 insertions(+), 20 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 24d0438f..7ae28f09 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -4,8 +4,7 @@ from functools import lru_cache from .exceptions import NotFound, InvalidUsage from .views import CompositionView -Route = namedtuple( - 'Route', +Route = namedtuple('Route', ['handler', 'methods', 'pattern', 'parameters', 'name']) Parameter = namedtuple('Parameter', ['name', 'cast']) @@ -70,6 +69,28 @@ class Router: self.routes_always_check = [] self.hosts = None + def __str__(self): + """ + The typical user inspecting the router will likely want to see + the routes available. Provide a simple representation. + """ + def _route_to_str(uri, route): + out = 'name={0.name}, methods={0.methods}, URI={1}>\n'.format( + route, uri) + + if route.handler.__doc__: + out += '{}\n'.format(route.handler.__doc__) + + out += '\n' + + return out + + out = '' + for uri, route in self.routes_all.items(): + out += _route_to_str(uri, route) + + return out + def parse_parameter_string(self, parameter_string): """ Parse a parameter string into its constituent name, type, and pattern @@ -130,11 +151,16 @@ class Router: properties = {"unhashable": None} def add_parameter(match): + # We could receive NAME or NAME:PATTERN name = match.group(1) - name, _type, pattern = self.parse_parameter_string(name) + pattern = 'string' + if ':' in name: + name, pattern = name.split(':', 1) - parameter = Parameter( - name=name, cast=_type) + default = (str, pattern) + # Pull from pre-configured types + _type, pattern = REGEX_TYPES.get(pattern, default) + parameter = Parameter(name=name, cast=_type) parameters.append(parameter) # Mark the whole route as unhashable if it has the hash key in it @@ -146,7 +172,7 @@ class Router: return '({})'.format(pattern) - pattern_string = re.sub(self.parameter_pattern, add_parameter, uri) + pattern_string = re.sub(r'<(.+?)>', add_parameter, uri) pattern = re.compile(r'^{}$'.format(pattern_string)) def merge_route(route, methods, handler): diff --git a/sanic/sanic.py b/sanic/sanic.py index 4ad4734a..079049ca 100644 --- a/sanic/sanic.py +++ b/sanic/sanic.py @@ -17,6 +17,7 @@ from .response import HTTPResponse from .router import Router from .server import serve, serve_multiple, HttpProtocol from .static import register as static_register +from .views import CompositionView class Sanic: @@ -120,7 +121,16 @@ class Sanic: """ # Handle HTTPMethodView differently if hasattr(handler, 'view_class'): - methods = frozenset(HTTP_METHODS) + methods = set() + + for method in HTTP_METHODS: + if getattr(handler.view_class, method.lower(), None): + methods.add(method) + + # handle composition view differently + if isinstance(handler, CompositionView): + methods = handler.handlers.keys() + self.route(uri=uri, methods=methods, host=host)(handler) return handler diff --git a/sanic/views.py b/sanic/views.py index 5d8e9d40..d6da9145 100644 --- a/sanic/views.py +++ b/sanic/views.py @@ -1,4 +1,5 @@ from .exceptions import InvalidUsage +from .constants import HTTP_METHODS class HTTPMethodView: @@ -40,11 +41,7 @@ class HTTPMethodView: def dispatch_request(self, request, *args, **kwargs): handler = getattr(self, request.method.lower(), None) - if handler: - return handler(request, *args, **kwargs) - raise InvalidUsage( - 'Method {} not allowed for URL {}'.format( - request.method, request.url), status_code=405) + return handler(request, *args, **kwargs) @classmethod def as_view(cls, *class_args, **class_kwargs): @@ -89,15 +86,15 @@ class CompositionView: def add(self, methods, handler): for method in methods: + if method not in HTTP_METHODS: + raise InvalidUsage( + '{} is not a valid HTTP method.'.format(method)) + if method in self.handlers: - raise KeyError( - 'Method {} already is registered.'.format(method)) + raise InvalidUsage( + 'Method {} is already registered.'.format(method)) self.handlers[method] = handler def __call__(self, request, *args, **kwargs): - handler = self.handlers.get(request.method.upper(), None) - if handler is None: - raise InvalidUsage( - 'Method {} not allowed for URL {}'.format( - request.method, request.url), status_code=405) + handler = self.handlers[request.method.upper()] return handler(request, *args, **kwargs) diff --git a/tests/test_views.py b/tests/test_views.py index 4e5b17f0..456b714d 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,8 +1,9 @@ import pytest as pytest from sanic import Sanic +from sanic.exceptions import InvalidUsage from sanic.response import text, HTTPResponse -from sanic.views import HTTPMethodView +from sanic.views import HTTPMethodView, CompositionView from sanic.blueprints import Blueprint from sanic.request import Request from sanic.utils import sanic_endpoint_test @@ -196,3 +197,56 @@ def test_with_decorator(): request, response = sanic_endpoint_test(app, method="get") assert response.text == 'I am get method' assert results[0] == 1 + + +def test_composition_view_rejects_incorrect_methods(): + def foo(request): + return text('Foo') + + view = CompositionView() + + with pytest.raises(InvalidUsage) as e: + view.add(['GET', 'FOO'], foo) + + assert str(e.value) == 'FOO is not a valid HTTP method.' + + +def test_composition_view_rejects_duplicate_methods(): + def foo(request): + return text('Foo') + + view = CompositionView() + + with pytest.raises(InvalidUsage) as e: + view.add(['GET', 'POST', 'GET'], foo) + + assert str(e.value) == 'Method GET is already registered.' + + +def test_composition_view_runs_methods_as_expected(): + app = Sanic('test_composition_view') + + view = CompositionView() + view.add(['GET', 'POST', 'PUT'], lambda x: text('first method')) + view.add(['DELETE', 'PATCH'], lambda x: text('second method')) + + app.add_route(view, '/') + + for method in ['GET', 'POST', 'PUT']: + request, response = sanic_endpoint_test(app, uri='/', method=method) + assert response.text == 'first method' + + for method in ['DELETE', 'PATCH']: + request, response = sanic_endpoint_test(app, uri='/', method=method) + assert response.text == 'second method' + +def test_composition_view_rejects_invalid_methods(): + app = Sanic('test_composition_view') + + view = CompositionView() + view.add(['GET', 'POST', 'PUT'], lambda x: text('first method')) + + app.add_route(view, '/') + for method in ['DELETE', 'PATCH']: + request, response = sanic_endpoint_test(app, uri='/', method=method) + assert response.status == 405 From 4d6f9ffd7cfd4b6f929fdf2a16d661719497e14b Mon Sep 17 00:00:00 2001 From: Suby Raman Date: Mon, 13 Feb 2017 11:45:55 -0500 Subject: [PATCH 2/4] rebase --- sanic/router.py | 11 +++-------- tests/test_views.py | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 7ae28f09..621d3733 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -151,16 +151,11 @@ class Router: properties = {"unhashable": None} def add_parameter(match): - # We could receive NAME or NAME:PATTERN name = match.group(1) - pattern = 'string' - if ':' in name: - name, pattern = name.split(':', 1) + name, _type, pattern = self.parse_parameter_string(name) - default = (str, pattern) - # Pull from pre-configured types - _type, pattern = REGEX_TYPES.get(pattern, default) - parameter = Parameter(name=name, cast=_type) + parameter = Parameter( + name=name, cast=_type) parameters.append(parameter) # Mark the whole route as unhashable if it has the hash key in it diff --git a/tests/test_views.py b/tests/test_views.py index 456b714d..afa52d8b 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -240,6 +240,7 @@ def test_composition_view_runs_methods_as_expected(): request, response = sanic_endpoint_test(app, uri='/', method=method) assert response.text == 'second method' + def test_composition_view_rejects_invalid_methods(): app = Sanic('test_composition_view') From 051ff2b325529a3d4953228df1d4de7d30236a23 Mon Sep 17 00:00:00 2001 From: Suby Raman Date: Mon, 13 Feb 2017 11:50:09 -0500 Subject: [PATCH 3/4] remove repr stuff --- sanic/router.py | 22 ---------------------- tests/test_views.py | 18 +++++++++++++----- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 621d3733..c115b362 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -69,28 +69,6 @@ class Router: self.routes_always_check = [] self.hosts = None - def __str__(self): - """ - The typical user inspecting the router will likely want to see - the routes available. Provide a simple representation. - """ - def _route_to_str(uri, route): - out = 'name={0.name}, methods={0.methods}, URI={1}>\n'.format( - route, uri) - - if route.handler.__doc__: - out += '{}\n'.format(route.handler.__doc__) - - out += '\n' - - return out - - out = '' - for uri, route in self.routes_all.items(): - out += _route_to_str(uri, route) - - return out - def parse_parameter_string(self, parameter_string): """ Parse a parameter string into its constituent name, type, and pattern diff --git a/tests/test_views.py b/tests/test_views.py index afa52d8b..ec0b91b8 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -223,7 +223,8 @@ def test_composition_view_rejects_duplicate_methods(): assert str(e.value) == 'Method GET is already registered.' -def test_composition_view_runs_methods_as_expected(): +@pytest.mark.parametrize('method', HTTP_METHODS) +def test_composition_view_runs_methods_as_expected(method): app = Sanic('test_composition_view') view = CompositionView() @@ -232,22 +233,29 @@ def test_composition_view_runs_methods_as_expected(): app.add_route(view, '/') - for method in ['GET', 'POST', 'PUT']: + if method in ['GET', 'POST', 'PUT']: request, response = sanic_endpoint_test(app, uri='/', method=method) assert response.text == 'first method' - for method in ['DELETE', 'PATCH']: + if method in ['DELETE', 'PATCH']: request, response = sanic_endpoint_test(app, uri='/', method=method) assert response.text == 'second method' -def test_composition_view_rejects_invalid_methods(): +@pytest.mark.parametrize('method', HTTP_METHODS) +def test_composition_view_rejects_invalid_methods(method): app = Sanic('test_composition_view') view = CompositionView() view.add(['GET', 'POST', 'PUT'], lambda x: text('first method')) app.add_route(view, '/') - for method in ['DELETE', 'PATCH']: + + if method in ['GET', 'POST', 'PUT']: + request, response = sanic_endpoint_test(app, uri='/', method=method) + assert response.status == 200 + assert response.text == 'first method' + + if method in ['DELETE', 'PATCH']: request, response = sanic_endpoint_test(app, uri='/', method=method) assert response.status == 405 From b2be821637b18a6ab65ff67a1ce18ccf11d9367d Mon Sep 17 00:00:00 2001 From: Suby Raman Date: Mon, 13 Feb 2017 11:55:00 -0500 Subject: [PATCH 4/4] reverse router changes --- sanic/router.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index c115b362..24d0438f 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -4,7 +4,8 @@ from functools import lru_cache from .exceptions import NotFound, InvalidUsage from .views import CompositionView -Route = namedtuple('Route', +Route = namedtuple( + 'Route', ['handler', 'methods', 'pattern', 'parameters', 'name']) Parameter = namedtuple('Parameter', ['name', 'cast']) @@ -145,7 +146,7 @@ class Router: return '({})'.format(pattern) - pattern_string = re.sub(r'<(.+?)>', add_parameter, uri) + pattern_string = re.sub(self.parameter_pattern, add_parameter, uri) pattern = re.compile(r'^{}$'.format(pattern_string)) def merge_route(route, methods, handler):