diff --git a/odinweb/containers.py b/odinweb/containers.py index 1892824..d8d81a9 100644 --- a/odinweb/containers.py +++ b/odinweb/containers.py @@ -14,11 +14,6 @@ from odin.exceptions import ValidationError from odin.utils import getmeta -# Imports for typing support -from typing import Union, Tuple, Any, Generator, Dict, Type, Optional # noqa -from odin import Resource # noqa -from .data_structures import BaseHttpRequest # noqa - from . import _compat from . import content_type_resolvers from .constants import Method, HTTPStatus @@ -28,6 +23,11 @@ from .helpers import resolve_content_type, create_response from .resources import Error +# Imports for typing support +from typing import Union, Tuple, Any, Generator, Dict, Type, Optional # noqa: F401 +from odin import Resource # noqa: F401 +from .data_structures import BaseHttpRequest # noqa: F401 + logger = logging.getLogger(__name__) @@ -274,7 +274,7 @@ def __init__(self, *containers, **options): raise ValueError("Path prefix must be an absolute path (eg start with a '/')") def handle_500(self, request, exception): - # type: (BaseHttpRequest, BaseException) + # type: (BaseHttpRequest, BaseException) -> Any """ Handle an *un-handled* exception. """ diff --git a/odinweb/decorators.py b/odinweb/decorators.py index 334383b..5492308 100644 --- a/odinweb/decorators.py +++ b/odinweb/decorators.py @@ -7,6 +7,8 @@ """ from __future__ import absolute_import +import odin + from odin.utils import force_tuple, lazy_property, getmeta from .constants import HTTPStatus, Method, Type @@ -16,9 +18,8 @@ from .utils import to_bool, dict_filter # Imports for typing support -from typing import Callable, Union, Tuple, Dict, Any, Generator, List, Set, Iterable # noqa +from typing import Callable, Union, Tuple, Dict, Any, Generator, List, Set, Iterable # noqa: F401 from .data_structures import BaseHttpRequest -from odin import Resource # noqa # Type definitions Tags = Union[str, Iterable[str]] @@ -70,7 +71,7 @@ def inner(callback): def __init__(self, callback, path=NoPath, methods=Method.GET, resource=None, tags=None, summary=None, middleware=None): - # type: (Callable, Path, Methods, Type[Resource], Tags, str, List[Any]) -> None + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> None """ :param callback: Function we are routing :param path: A sub path that can be used as a action. @@ -275,7 +276,7 @@ def inner(c): def action(callback=None, name=None, path=None, methods=Method.GET, resource=None, tags=None, summary=None, middleware=None): - # type: (Callable, Path, Path, Methods, Type[Resource], Tags, str, List[Any]) -> Operation + # type: (Callable, Path, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> Operation """ Decorator to apply an action to a resource. An action is applied to a `detail` operation. """ @@ -332,6 +333,12 @@ def __init__(self, *args, **kwargs): super(WrappedListOperation, self).__init__(*args, **kwargs) + # TODO: This is a workaround until params can validate and type convert themselves. + self.offset_param = odin.IntegerField(name='offset', min_value=0, default=self.default_offset) + self.limit_param = odin.IntegerField(name='limit', min_value=1, max_value=self.max_limit, + default=self.default_limit) + self.bare_param = odin.BooleanField(name='bare', default=False) + # Apply documentation self.parameters.add(Param.query('offset', Type.Integer, "Offset to start listing from.", default=self.default_offset)) @@ -342,20 +349,17 @@ def __init__(self, *args, **kwargs): def execute(self, request, *args, **path_args): # type: (BaseHttpRequest, *Any, **Any) -> Any # Get paging args from query string - offset = int(request.query.get('offset', self.default_offset)) - if offset < 0: - offset = 0 + param = self.offset_param + offset = param.clean(request.GET.get(param.name, param.default)) path_args['offset'] = offset - max_limit = self.max_limit - limit = int(request.query.get('limit', self.default_limit)) - if limit < 1: - limit = 1 - elif max_limit and limit > max_limit: - limit = max_limit + param = self.limit_param + limit = param.clean(request.GET.get(param.name, param.default)) path_args['limit'] = limit - bare = to_bool(request.query.get('bare', False)) + param = self.bare_param + bare = param.clean(request.GET.get(param.name, param.default)) + path_args['bare'] = bare # Run base execute result = super(WrappedListOperation, self).execute(request, *args, **path_args) @@ -405,6 +409,11 @@ def __init__(self, *args, **kwargs): super(ListOperation, self).__init__(*args, **kwargs) + # TODO: This is a workaround until params can validate and type convert themselves. + self.offset_param = odin.IntegerField(name='offset', min_value=0, default=self.default_offset) + self.limit_param = odin.IntegerField(name='limit', min_value=1, max_value=self.max_limit, + default=self.default_limit) + # Apply documentation self.parameters.add(Param.query('offset', Type.Integer, "Offset to start listing from.", default=self.default_offset)) @@ -414,17 +423,12 @@ def __init__(self, *args, **kwargs): def execute(self, request, *args, **path_args): # type: (BaseHttpRequest, *Any, **Any) -> Any # Get paging args from query string - offset = int(request.GET.get('offset', self.default_offset)) - if offset < 0: - offset = 0 + param = self.offset_param + offset = param.clean(request.GET.get(param.name, param.default)) path_args['offset'] = offset - max_limit = self.max_limit - limit = int(request.GET.get('limit', self.default_limit)) - if limit < 1: - limit = 1 - elif max_limit and limit > max_limit: - limit = max_limit + param = self.limit_param + limit = param.clean(request.GET.get(param.name, param.default)) path_args['limit'] = limit # Run base execute @@ -473,7 +477,7 @@ def execute(self, request, *args, **path_args): def listing(callback=None, path=None, method=Method.GET, resource=None, tags=None, summary="List resources", middleware=None, default_limit=50, max_limit=None, use_wrapper=True): - # type: (Callable, Path, Methods, Resource, Tags, str, List[Any], int, int) -> Operation + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any], int, int) -> Operation """ Decorator to configure an operation that returns a list of resources. """ @@ -489,7 +493,7 @@ def inner(c): def create(callback=None, path=None, method=Method.POST, resource=None, tags=None, summary="Create a new resource", middleware=None): - # type: (Callable, Path, Methods, Resource, Tags, str, List[Any]) -> Operation + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> Operation """ Decorator to configure an operation that creates a resource. """ @@ -503,7 +507,7 @@ def inner(c): def detail(callback=None, path=None, method=Method.GET, resource=None, tags=None, summary="Get specified resource.", middleware=None): - # type: (Callable, Path, Methods, Resource, Tags, str, List[Any]) -> Operation + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> Operation """ Decorator to configure an operation that fetches a resource. """ @@ -517,7 +521,7 @@ def inner(c): def update(callback=None, path=None, method=Method.PUT, resource=None, tags=None, summary="Update specified resource.", middleware=None): - # type: (Callable, Path, Methods, Resource, Tags, str, List[Any]) -> Operation + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> Operation """ Decorator to configure an operation that updates a resource. """ @@ -532,7 +536,7 @@ def inner(c): def patch(callback=None, path=None, method=Method.PATCH, resource=None, tags=None, summary="Patch specified resource.", middleware=None): - # type: (Callable, Path, Methods, Resource, Tags, str, List[Any]) -> Operation + # type: (Callable, Path, Methods, Type[odin.Resource], Tags, str, List[Any]) -> Operation """ Decorator to configure an operation that patches a resource. """ diff --git a/tests/test_decorators.py b/tests/test_decorators.py index d562e7a..0f02ca4 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -3,6 +3,7 @@ import pytest from collections import defaultdict +from odin.exceptions import ValidationError from odinweb import decorators from odinweb.constants import * @@ -134,21 +135,15 @@ def my_func(request): @pytest.mark.parametrize('options, query, offset, limit, bare', ( ({}, {}, 0, 50, False), - ({}, {'offset': 10}, 10, 50, False), - ({}, {'limit': 20}, 0, 20, False), + ({}, {'offset': '10'}, 10, 50, False), + ({}, {'limit': '20'}, 0, 20, False), ({}, {'bare': 'F'}, 0, 50, False), ({}, {'bare': 'T'}, 0, 50, True), ({}, {'bare': 'yes'}, 0, 50, True), - ({}, {'offset': 10, 'limit': 20, 'bare': '1'}, 10, 20, True), + ({}, {'offset': '10', 'limit': 20, 'bare': '1'}, 10, 20, True), # Max limit ({'max_limit': 100}, {}, 0, 50, False), ({'max_limit': 100}, {'offset': 10, 'limit': 100}, 10, 100, False), - ({'max_limit': 100}, {'offset': 10, 'limit': 102}, 10, 100, False), - # Silly values? - ({}, {'offset': -1}, 0, 50, False), - ({}, {'limit': -1}, 0, 1, False), - ({}, {'limit': 0}, 0, 1, False), - ({}, {'offset': -1, 'limit': -1}, 0, 1, False), )) def test_options_handled(self, options, query, offset, limit, bare): mock_request = MockRequest(query=query) @@ -172,11 +167,36 @@ def my_func(request, **kwargs): assert result.limit == limit assert result.total_count is None + @pytest.mark.parametrize('options, query', ( + ({}, {'offset': '-1'}), + ({}, {'offset': ''}), + ({}, {'offset': 'Xyz'}), + ({}, {'offset': '?$#'}), + ({}, {'limit': '-1'}), + ({}, {'limit': '0'}), + ({}, {'limit': ''}), + ({}, {'limit': 'Xyz'}), + ({}, {'limit': '?$#'}), + ({'max_limit': 20}, {'limit': '21'}), + ({}, {'bare': 'eek'}), + ({}, {'bare': ''}), + ({}, {'bare': '32'}), + )) + def test_options_handled__invalid(self, options, query): + mock_request = MockRequest(query=query) + + @decorators.WrappedListOperation(**options) + def my_func(*_): + pass + + with pytest.raises(ValidationError): + my_func(mock_request, {}) + def test_returning_total_count(self): mock_request = MockRequest() @decorators.WrappedListOperation - def my_func(request, foo, offset, limit): + def my_func(request, foo, offset, limit, bare): assert foo == 'bar' assert offset == 0 assert limit == 50 @@ -214,12 +234,6 @@ def my_func(request): # Max limit ({'max_limit': 100}, {}, 0, 50), ({'max_limit': 100}, {'offset': 10, 'limit': 100}, 10, 100), - ({'max_limit': 100}, {'offset': 10, 'limit': 102}, 10, 100), - # Silly values? - ({}, {'offset': -1}, 0, 50), - ({}, {'limit': -1}, 0, 1), - ({}, {'limit': 0}, 0, 1), - ({}, {'offset': -1, 'limit': -1}, 0, 1), )) def test_options_handled(self, options, query, offset, limit): mock_request = MockRequest(query=query) @@ -240,6 +254,28 @@ def my_func(request, **kwargs): assert result['X-Page-Limit'] == str(limit) assert 'X-Total-Count' not in result.headers + @pytest.mark.parametrize('options, query', ( + ({}, {'offset': '-1'}), + ({}, {'offset': ''}), + ({}, {'offset': 'Xyz'}), + ({}, {'offset': '?$#'}), + ({}, {'limit': '-1'}), + ({}, {'limit': '0'}), + ({}, {'limit': ''}), + ({}, {'limit': 'Xyz'}), + ({}, {'limit': '?$#'}), + ({'max_limit': 20}, {'limit': '21'}), + )) + def test_options_handled__invalid(self, options, query): + mock_request = MockRequest(query=query) + + @decorators.WrappedListOperation(**options) + def my_func(*_): + pass + + with pytest.raises(ValidationError): + my_func(mock_request, {}) + def test_returning_total_count(self): mock_request = MockRequest()