diff --git a/requirements.txt b/requirements.txt index 0b41cd50b..b4e5ff579 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,8 +2,8 @@ # just Django, but for the purposes of development and testing # there are a number of packages that are useful to install. -# Laying these out as seperate requirements files, allows us to -# only included the relevent sets when running tox, and ensures +# Laying these out as separate requirements files, allows us to +# only included the relevant sets when running tox, and ensures # we are only ever declaring our dependencies in one place. -r requirements/requirements-optionals.txt diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index a68e8ab12..1a1ba2f65 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -138,6 +138,9 @@ class BasePagination: def get_paginated_response(self, data): # pragma: no cover raise NotImplementedError('get_paginated_response() must be implemented.') + def get_paginated_response_schema(self, schema): + return schema + def to_html(self): # pragma: no cover raise NotImplementedError('to_html() must be implemented to display page controls.') @@ -222,6 +225,26 @@ class PageNumberPagination(BasePagination): ('results', data) ])) + def get_paginated_response_schema(self, schema): + return { + 'type': 'object', + 'properties': { + 'count': { + 'type': 'integer', + 'example': 123, + }, + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': schema, + }, + } + def get_page_size(self, request): if self.page_size_query_param: try: @@ -369,6 +392,26 @@ class LimitOffsetPagination(BasePagination): ('results', data) ])) + def get_paginated_response_schema(self, schema): + return { + 'type': 'object', + 'properties': { + 'count': { + 'type': 'integer', + 'example': 123, + }, + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': schema, + }, + } + def get_limit(self, request): if self.limit_query_param: try: @@ -840,6 +883,22 @@ class CursorPagination(BasePagination): ('results', data) ])) + def get_paginated_response_schema(self, schema): + return { + 'type': 'object', + 'properties': { + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': schema, + }, + } + def get_html_context(self): return { 'previous_url': self.get_previous_link(), diff --git a/rest_framework/schemas/openapi.py b/rest_framework/schemas/openapi.py index 62dafd59c..cd70b9d0c 100644 --- a/rest_framework/schemas/openapi.py +++ b/rest_framework/schemas/openapi.py @@ -209,11 +209,10 @@ class AutoSchema(ViewInspector): if not is_list_view(path, method, view): return [] - pagination = getattr(view, 'pagination_class', None) - if not pagination: + paginator = self._get_pagninator() + if not paginator: return [] - paginator = view.pagination_class() return paginator.get_schema_operation_parameters(view) def _map_field(self, field): @@ -387,7 +386,7 @@ class AutoSchema(ViewInspector): schema['default'] = field.default if field.help_text: schema['description'] = str(field.help_text) - self._map_field_validators(field.validators, schema) + self._map_field_validators(field, schema) properties[field.field_name] = schema @@ -399,13 +398,11 @@ class AutoSchema(ViewInspector): return result - def _map_field_validators(self, validators, schema): + def _map_field_validators(self, field, schema): """ map field validators - :param list:validators: list of field validators - :param dict:schema: schema that the validators get added to """ - for v in validators: + for v in field.validators: # "Formats such as "email", "uuid", and so on, MAY be used even though undefined by this specification." # https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#data-types if isinstance(v, EmailValidator): @@ -415,9 +412,15 @@ class AutoSchema(ViewInspector): if isinstance(v, RegexValidator): schema['pattern'] = v.regex.pattern elif isinstance(v, MaxLengthValidator): - schema['maxLength'] = v.limit_value + attr_name = 'maxLength' + if isinstance(field, serializers.ListField): + attr_name = 'maxItems' + schema[attr_name] = v.limit_value elif isinstance(v, MinLengthValidator): - schema['minLength'] = v.limit_value + attr_name = 'minLength' + if isinstance(field, serializers.ListField): + attr_name = 'minItems' + schema[attr_name] = v.limit_value elif isinstance(v, MaxValueValidator): schema['maximum'] = v.limit_value elif isinstance(v, MinValueValidator): @@ -432,15 +435,22 @@ class AutoSchema(ViewInspector): schema['maximum'] = int(digits * '9') + 1 schema['minimum'] = -schema['maximum'] + def _get_pagninator(self): + pagination_class = getattr(self.view, 'pagination_class', None) + if pagination_class: + return pagination_class() + return None + def map_parsers(self, path, method): return list(map(attrgetter('media_type'), self.view.parser_classes)) def map_renderers(self, path, method): media_types = [] for renderer in self.view.renderer_classes: - # I assume this is not relevant to OpenAPI spec - if renderer != renderers.BrowsableAPIRenderer: - media_types.append(renderer.media_type) + # BrowsableAPIRenderer not relevant to OpenAPI spec + if renderer == renderers.BrowsableAPIRenderer: + continue + media_types.append(renderer.media_type) return media_types def _get_serializer(self, method, path): @@ -513,6 +523,9 @@ class AutoSchema(ViewInspector): 'type': 'array', 'items': item_schema, } + paginator = self._get_pagninator() + if paginator: + response_schema = paginator.get_paginated_response_schema(response_schema) else: response_schema = item_schema diff --git a/rest_framework/views.py b/rest_framework/views.py index 832f17233..bec10560a 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -356,7 +356,15 @@ class APIView(View): throttle_durations.append(throttle.wait()) if throttle_durations: - self.throttled(request, max(throttle_durations)) + # Filter out `None` values which may happen in case of config / rate + # changes, see #1438 + durations = [ + duration for duration in throttle_durations + if duration is not None + ] + + duration = max(durations, default=None) + self.throttled(request, duration) def determine_version(self, request, *args, **kwargs): """ diff --git a/tests/schemas/test_openapi.py b/tests/schemas/test_openapi.py index 5be61448c..6c9089e05 100644 --- a/tests/schemas/test_openapi.py +++ b/tests/schemas/test_openapi.py @@ -264,6 +264,58 @@ class TestOperationIntrospection(TestCase): }, } + def test_paginated_list_response_body_generation(self): + """Test that pagination properties are added for a paginated list view.""" + path = '/' + method = 'GET' + + class Pagination(pagination.BasePagination): + def get_paginated_response_schema(self, schema): + return { + 'type': 'object', + 'item': schema, + } + + class ItemSerializer(serializers.Serializer): + text = serializers.CharField() + + class View(generics.GenericAPIView): + serializer_class = ItemSerializer + pagination_class = Pagination + + view = create_view( + View, + method, + create_request(path), + ) + inspector = AutoSchema() + inspector.view = view + + responses = inspector._get_responses(path, method) + assert responses == { + '200': { + 'description': '', + 'content': { + 'application/json': { + 'schema': { + 'type': 'object', + 'item': { + 'type': 'array', + 'items': { + 'properties': { + 'text': { + 'type': 'string', + }, + }, + 'required': ['text'], + }, + }, + }, + }, + }, + }, + } + def test_delete_response_body_generation(self): """Test that a view's delete method generates a proper response body schema.""" path = '/{id}/' @@ -312,15 +364,27 @@ class TestOperationIntrospection(TestCase): assert attachment['format'] == 'binary' def test_retrieve_response_body_generation(self): - """Test that a list of properties is returned for retrieve item views.""" + """ + Test that a list of properties is returned for retrieve item views. + + Pagination properties should not be added as the view represents a single item. + """ path = '/{id}/' method = 'GET' + class Pagination(pagination.BasePagination): + def get_paginated_response_schema(self, schema): + return { + 'type': 'object', + 'item': schema, + } + class ItemSerializer(serializers.Serializer): text = serializers.CharField() class View(generics.GenericAPIView): serializer_class = ItemSerializer + pagination_class = Pagination view = create_view( View, @@ -419,6 +483,9 @@ class TestOperationIntrospection(TestCase): assert properties['string']['minLength'] == 2 assert properties['string']['maxLength'] == 10 + assert properties['lst']['minItems'] == 2 + assert properties['lst']['maxItems'] == 10 + assert properties['regex']['pattern'] == r'[ABC]12{3}' assert properties['regex']['description'] == 'must have an A, B, or C followed by 1222' diff --git a/tests/schemas/views.py b/tests/schemas/views.py index 273f1d30a..d1fc75eb8 100644 --- a/tests/schemas/views.py +++ b/tests/schemas/views.py @@ -85,6 +85,12 @@ class ExampleValidatedSerializer(serializers.Serializer): ), help_text='must have an A, B, or C followed by 1222' ) + lst = serializers.ListField( + validators=( + MaxLengthValidator(limit_value=10), + MinLengthValidator(limit_value=2), + ) + ) decimal1 = serializers.DecimalField(max_digits=6, decimal_places=2) decimal2 = serializers.DecimalField(max_digits=5, decimal_places=0, validators=(DecimalValidator(max_digits=17, decimal_places=4),)) diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 11fd6844d..cd84c8151 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -259,6 +259,37 @@ class TestPageNumberPagination: with pytest.raises(exceptions.NotFound): self.paginate_queryset(request) + def test_get_paginated_response_schema(self): + unpaginated_schema = { + 'type': 'object', + 'item': { + 'properties': { + 'test-property': { + 'type': 'integer', + }, + }, + }, + } + + assert self.pagination.get_paginated_response_schema(unpaginated_schema) == { + 'type': 'object', + 'properties': { + 'count': { + 'type': 'integer', + 'example': 123, + }, + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': unpaginated_schema, + }, + } + class TestPageNumberPaginationOverride: """ @@ -535,6 +566,37 @@ class TestLimitOffset: assert content.get('next') == next_url assert content.get('previous') == prev_url + def test_get_paginated_response_schema(self): + unpaginated_schema = { + 'type': 'object', + 'item': { + 'properties': { + 'test-property': { + 'type': 'integer', + }, + }, + }, + } + + assert self.pagination.get_paginated_response_schema(unpaginated_schema) == { + 'type': 'object', + 'properties': { + 'count': { + 'type': 'integer', + 'example': 123, + }, + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': unpaginated_schema, + }, + } + class CursorPaginationTestsMixin: @@ -834,6 +896,33 @@ class CursorPaginationTestsMixin: assert current == [1, 1, 1, 1, 1] assert next == [1, 2, 3, 4, 4] + def test_get_paginated_response_schema(self): + unpaginated_schema = { + 'type': 'object', + 'item': { + 'properties': { + 'test-property': { + 'type': 'integer', + }, + }, + }, + } + + assert self.pagination.get_paginated_response_schema(unpaginated_schema) == { + 'type': 'object', + 'properties': { + 'next': { + 'type': 'string', + 'nullable': True, + }, + 'previous': { + 'type': 'string', + 'nullable': True, + }, + 'results': unpaginated_schema, + }, + } + class TestCursorPagination(CursorPaginationTestsMixin): """ diff --git a/tests/test_throttling.py b/tests/test_throttling.py index 3c172e263..d5a61232d 100644 --- a/tests/test_throttling.py +++ b/tests/test_throttling.py @@ -159,6 +159,27 @@ class ThrottlingTests(TestCase): assert response.status_code == 429 assert int(response['retry-after']) == 58 + def test_throttle_rate_change_negative(self): + self.set_throttle_timer(MockView_DoubleThrottling, 0) + request = self.factory.get('/') + for dummy in range(24): + response = MockView_DoubleThrottling.as_view()(request) + assert response.status_code == 429 + assert int(response['retry-after']) == 60 + + previous_rate = User3SecRateThrottle.rate + try: + User3SecRateThrottle.rate = '1/sec' + + for dummy in range(24): + response = MockView_DoubleThrottling.as_view()(request) + + assert response.status_code == 429 + assert int(response['retry-after']) == 60 + finally: + # reset + User3SecRateThrottle.rate = previous_rate + def ensure_response_header_contains_proper_throttle_field(self, view, expected_headers): """ Ensure the response returns an Retry-After field with status and next attributes