From 5a8736ae450c14ab1236386c5b6065b355541d5e Mon Sep 17 00:00:00 2001 From: Christopher Grebs Date: Mon, 12 Aug 2019 20:36:05 +0200 Subject: [PATCH 1/5] Handle 'None' return value of wait() properly during throttling. (#6837) --- rest_framework/views.py | 10 +++++++++- tests/test_throttling.py | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) 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/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 From ec1b14174f8717337fc4402c7cea43f647e2586e Mon Sep 17 00:00:00 2001 From: Min ho Kim Date: Thu, 15 Aug 2019 07:39:45 +1000 Subject: [PATCH 2/5] Fixed typos (#6872) --- requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From f8c16441fa69850fd581c23807a8a0e38f3239d4 Mon Sep 17 00:00:00 2001 From: Reupen Shah Date: Tue, 3 Sep 2019 14:25:44 +0100 Subject: [PATCH 3/5] Add support for pagination in OpenAPI response schemas (#6867) Refs #6846 This provides a way for pagination classes to add pagination properties (`count`, `next`, `results` etc.) to OpenAPI response schemas. A new method `get_paginated_response_schema()` has been added to `BasePagination`. This method is intended to mirror `get_paginated_response()` (which takes a `list` and wraps it in a `dict`). Hence, `get_paginated_response_schema()` takes an unpaginated response schema (of type `array`) and wraps that with a schema object of type `object` containing the relevant properties that the pagination class adds to responses. The default implementation of `BasePagination.get_paginated_response_schema()` simply passes the schema through unmodified, for backwards compatibility. --- rest_framework/pagination.py | 59 ++++++++++++++++++++ rest_framework/schemas/openapi.py | 15 ++++-- tests/schemas/test_openapi.py | 66 ++++++++++++++++++++++- tests/test_pagination.py | 89 +++++++++++++++++++++++++++++++ 4 files changed, 225 insertions(+), 4 deletions(-) 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 0af7510cd..343d0b624 100644 --- a/rest_framework/schemas/openapi.py +++ b/rest_framework/schemas/openapi.py @@ -206,11 +206,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): @@ -423,6 +422,13 @@ 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 _get_serializer(self, method, path): view = self.view @@ -489,6 +495,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/tests/schemas/test_openapi.py b/tests/schemas/test_openapi.py index 78a5609da..e0fe3c9ab 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}/' @@ -288,15 +340,27 @@ class TestOperationIntrospection(TestCase): } 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, 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): """ From 1cc4be47b40a0d0c062fccf28853ca448a8522ab Mon Sep 17 00:00:00 2001 From: Dima Knivets Date: Tue, 3 Sep 2019 16:43:54 +0300 Subject: [PATCH 4/5] Fixed min/max attributes for serializers.ListField (#6866) --- rest_framework/schemas/openapi.py | 18 +++++++++++------- tests/schemas/test_openapi.py | 3 +++ tests/schemas/views.py | 6 ++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/rest_framework/schemas/openapi.py b/rest_framework/schemas/openapi.py index 343d0b624..4fee95439 100644 --- a/rest_framework/schemas/openapi.py +++ b/rest_framework/schemas/openapi.py @@ -377,7 +377,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 @@ -389,13 +389,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): @@ -405,9 +403,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): diff --git a/tests/schemas/test_openapi.py b/tests/schemas/test_openapi.py index e0fe3c9ab..508e7dba8 100644 --- a/tests/schemas/test_openapi.py +++ b/tests/schemas/test_openapi.py @@ -459,6 +459,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),)) From 324242bf4eac07c61123299729c5b8c2a1914d13 Mon Sep 17 00:00:00 2001 From: Dima Knivets Date: Sun, 11 Aug 2019 15:39:35 +0300 Subject: [PATCH 5/5] Schemas: Map renderers/parsers for request/response media-types. --- rest_framework/schemas/openapi.py | 34 ++++++++++++++++++++++++++----- tests/schemas/test_openapi.py | 24 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/rest_framework/schemas/openapi.py b/rest_framework/schemas/openapi.py index 4fee95439..cd70b9d0c 100644 --- a/rest_framework/schemas/openapi.py +++ b/rest_framework/schemas/openapi.py @@ -1,4 +1,5 @@ import warnings +from operator import attrgetter from urllib.parse import urljoin from django.core.validators import ( @@ -8,7 +9,7 @@ from django.core.validators import ( from django.db import models from django.utils.encoding import force_str -from rest_framework import exceptions, serializers +from rest_framework import exceptions, renderers, serializers from rest_framework.compat import uritemplate from rest_framework.fields import _UnvalidatedField, empty @@ -78,7 +79,9 @@ class SchemaGenerator(BaseSchemaGenerator): class AutoSchema(ViewInspector): - content_types = ['application/json'] + request_media_types = [] + response_media_types = [] + method_mapping = { 'get': 'Retrieve', 'post': 'Create', @@ -336,6 +339,12 @@ class AutoSchema(ViewInspector): self._map_min_max(field, content) return content + if isinstance(field, serializers.FileField): + return { + 'type': 'string', + 'format': 'binary' + } + # Simplest cases, default to 'string' type: FIELD_CLASS_SCHEMA_TYPE = { serializers.BooleanField: 'boolean', @@ -430,9 +439,20 @@ class AutoSchema(ViewInspector): 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: + # 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): view = self.view @@ -452,6 +472,8 @@ class AutoSchema(ViewInspector): if method not in ('PUT', 'PATCH', 'POST'): return {} + self.request_media_types = self.map_parsers(path, method) + serializer = self._get_serializer(path, method) if not isinstance(serializer, serializers.Serializer): @@ -469,7 +491,7 @@ class AutoSchema(ViewInspector): return { 'content': { ct: {'schema': content} - for ct in self.content_types + for ct in self.request_media_types } } @@ -482,6 +504,8 @@ class AutoSchema(ViewInspector): } } + self.response_media_types = self.map_renderers(path, method) + item_schema = {} serializer = self._get_serializer(path, method) @@ -509,7 +533,7 @@ class AutoSchema(ViewInspector): '200': { 'content': { ct: {'schema': response_schema} - for ct in self.content_types + for ct in self.response_media_types }, # description is a mandatory property, # https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#responseObject diff --git a/tests/schemas/test_openapi.py b/tests/schemas/test_openapi.py index 508e7dba8..6c9089e05 100644 --- a/tests/schemas/test_openapi.py +++ b/tests/schemas/test_openapi.py @@ -339,6 +339,30 @@ class TestOperationIntrospection(TestCase): }, } + def test_multipart_request_body_generation(self): + """Test that a view's delete method generates a proper response body schema.""" + path = '/{id}/' + method = 'POST' + + class ItemSerializer(serializers.Serializer): + attachment = serializers.FileField() + + class View(generics.CreateAPIView): + serializer_class = ItemSerializer + + view = create_view( + View, + method, + create_request(path), + ) + inspector = AutoSchema() + inspector.view = view + + request_body = inspector._get_request_body(path, method) + assert 'multipart/form-data' in request_body['content'] + attachment = request_body['content']['multipart/form-data']['schema']['properties']['attachment'] + assert attachment['format'] == 'binary' + def test_retrieve_response_body_generation(self): """ Test that a list of properties is returned for retrieve item views.