diff --git a/docs/changelog.rst b/docs/changelog.rst index 4e3433e2..abe6cd76 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -2,6 +2,15 @@ Changelog ######### +********* +**1.4.2** +********* + +- **FIXED:** fixed a bug that causes a ``ModelViewSet`` generated from models with nested ``ForeignKey`` to output + models named ``Nested`` into the ``definitions`` section (:issue:`59`, :pr:`65`) +- **FIXED:** ``Response`` objects without a ``schema`` are now properly handled when passed through + ``swagger_auto_schema`` (:issue:`66`) + ********* **1.4.1** ********* diff --git a/docs/custom_spec.rst b/docs/custom_spec.rst index 2d940aee..3c9938dc 100644 --- a/docs/custom_spec.rst +++ b/docs/custom_spec.rst @@ -172,7 +172,11 @@ You can define some per-serializer options by adding a ``Meta`` class to your se Currently, the only option you can add here is * ``ref_name`` - a string which will be used as the model definition name for this serializer class; setting it to - ``None`` will force the serializer to be generated as an inline model everywhere it is used + ``None`` will force the serializer to be generated as an inline model everywhere it is used. If two serializers + have the same ``ref_name``, both their usages will be replaced with a reference to the same definition. + If this option is not specified, all serializers have an implicit name derived from their class name, minus any + ``Serializer`` suffix (e.g. ``UserSerializer`` -> ``User``, ``SerializerWithSuffix`` -> ``SerializerWithSuffix``) + ************************* Subclassing and extending @@ -301,3 +305,25 @@ A second example, of a :class:`~.inspectors.FieldInspector` that removes the ``t This means that you should generally avoid view or method-specific ``FieldInspector``\ s if you are dealing with references (a.k.a named models), because you can never know which view will be the first to generate the schema for a given serializer. + + **IMPORTANT:** nested fields on ``ModelSerializer``\ s that are generated from model ``ForeignKeys`` will always be + output by value. If you want the by-reference behaviour you have to explictly set the serializer class of nested + fields instead of letting ``ModelSerializer`` generate one automatically; for example: + + .. code-block:: python + + class OneSerializer(serializers.ModelSerializer): + class Meta: + model = SomeModel + fields = ('id',) + + + class AnotherSerializer(serializers.ModelSerializer): + chilf = OneSerializer() + + class Meta: + model = SomeParentModel + fields = ('id', 'child') + + Another caveat that stems from this is that any serializer named "``NestedSerializer``" will be forced inline + unless it has a ``ref_name`` set explicitly. diff --git a/src/drf_yasg/inspectors/base.py b/src/drf_yasg/inspectors/base.py index f23f1782..040ec057 100644 --- a/src/drf_yasg/inspectors/base.py +++ b/src/drf_yasg/inspectors/base.py @@ -234,7 +234,8 @@ def SwaggerType(existing_object=None, **instance_kwargs): if default is not None: instance_kwargs['default'] = default - instance_kwargs.setdefault('title', title) + if instance_kwargs.get('type', None) != openapi.TYPE_ARRAY: + instance_kwargs.setdefault('title', title) instance_kwargs.setdefault('description', description) instance_kwargs.update(kwargs) diff --git a/src/drf_yasg/inspectors/field.py b/src/drf_yasg/inspectors/field.py index e7b84aba..cfe15a87 100644 --- a/src/drf_yasg/inspectors/field.py +++ b/src/drf_yasg/inspectors/field.py @@ -1,3 +1,4 @@ +import logging import operator from collections import OrderedDict from decimal import Decimal @@ -12,6 +13,8 @@ from ..utils import decimal_as_float, filter_none from .base import FieldInspector, NotHandled, SerializerInspector +logger = logging.getLogger(__name__) + class InlineSerializerInspector(SerializerInspector): """Provides serializer conversions using :meth:`.FieldInspector.field_to_swagger_object`.""" @@ -54,10 +57,14 @@ def field_to_swagger_object(self, field, swagger_object_type, use_references, ** serializer = field serializer_meta = getattr(serializer, 'Meta', None) + serializer_name = type(serializer).__name__ if hasattr(serializer_meta, 'ref_name'): ref_name = serializer_meta.ref_name + elif serializer_name == 'NestedSerializer' and isinstance(serializer, serializers.ModelSerializer): + logger.debug("Forcing inline output for ModelSerializer named 'NestedSerializer': " + str(serializer)) + ref_name = None else: - ref_name = type(serializer).__name__ + ref_name = serializer_name if ref_name.endswith('Serializer'): ref_name = ref_name[:-len('Serializer')] @@ -77,11 +84,17 @@ def make_schema_definition(): if child.required: required.append(property_name) - return SwaggerType( + result = SwaggerType( type=openapi.TYPE_OBJECT, properties=properties, required=required or None, ) + if not ref_name: + # on an inline model, the title is derived from the field name + # but is visually displayed like the model named, which is confusing + # it is better to just remove title from inline models + del result.title + return result if not ref_name or not use_references: return make_schema_definition() diff --git a/testproj/testproj/settings/base.py b/testproj/testproj/settings/base.py index 1f630726..0e1d9e06 100644 --- a/testproj/testproj/settings/base.py +++ b/testproj/testproj/settings/base.py @@ -26,6 +26,7 @@ 'snippets', 'users', 'articles', + 'todo', ] MIDDLEWARE = [ diff --git a/testproj/testproj/urls.py b/testproj/testproj/urls.py index 7e664116..c7cb6a10 100644 --- a/testproj/testproj/urls.py +++ b/testproj/testproj/urls.py @@ -62,5 +62,6 @@ def root_redirect(request): url(r'^snippets/', include('snippets.urls')), url(r'^articles/', include('articles.urls')), url(r'^users/', include('users.urls')), + url(r'^todo/', include('todo.urls')), url(r'^plain/', plain_view), ] diff --git a/testproj/todo/__init__.py b/testproj/todo/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testproj/todo/migrations/0001_initial.py b/testproj/todo/migrations/0001_initial.py new file mode 100644 index 00000000..cecc6e46 --- /dev/null +++ b/testproj/todo/migrations/0001_initial.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11 on 2018-02-21 23:26 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='Todo', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=50)), + ], + ), + migrations.CreateModel( + name='TodoAnother', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=50)), + ('todo', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='todo.Todo')), + ], + ), + migrations.CreateModel( + name='TodoYetAnother', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=50)), + ('todo', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='todo.TodoAnother')), + ], + ), + ] diff --git a/testproj/todo/migrations/__init__.py b/testproj/todo/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testproj/todo/models.py b/testproj/todo/models.py new file mode 100644 index 00000000..cc29546c --- /dev/null +++ b/testproj/todo/models.py @@ -0,0 +1,15 @@ +from django.db import models + + +class Todo(models.Model): + title = models.CharField(max_length=50) + + +class TodoAnother(models.Model): + todo = models.ForeignKey(Todo, on_delete=models.CASCADE) + title = models.CharField(max_length=50) + + +class TodoYetAnother(models.Model): + todo = models.ForeignKey(TodoAnother, on_delete=models.CASCADE) + title = models.CharField(max_length=50) diff --git a/testproj/todo/serializer.py b/testproj/todo/serializer.py new file mode 100644 index 00000000..b23e3186 --- /dev/null +++ b/testproj/todo/serializer.py @@ -0,0 +1,24 @@ +from rest_framework import serializers + +from .models import Todo, TodoAnother, TodoYetAnother + + +class TodoSerializer(serializers.ModelSerializer): + class Meta: + model = Todo + fields = ('title',) + + +class TodoAnotherSerializer(serializers.ModelSerializer): + todo = TodoSerializer() + + class Meta: + model = TodoAnother + fields = ('title', 'todo') + + +class TodoYetAnotherSerializer(serializers.ModelSerializer): + class Meta: + model = TodoYetAnother + fields = ('title', 'todo') + depth = 2 diff --git a/testproj/todo/urls.py b/testproj/todo/urls.py new file mode 100644 index 00000000..391b5bf1 --- /dev/null +++ b/testproj/todo/urls.py @@ -0,0 +1,10 @@ +from rest_framework import routers + +from todo import views + +router = routers.DefaultRouter() +router.register(r'', views.TodoViewSet) +router.register(r'another', views.TodoAnotherViewSet) +router.register(r'yetanother', views.TodoYetAnotherViewSet) + +urlpatterns = router.urls diff --git a/testproj/todo/views.py b/testproj/todo/views.py new file mode 100644 index 00000000..2e2d26ca --- /dev/null +++ b/testproj/todo/views.py @@ -0,0 +1,19 @@ +from rest_framework import viewsets + +from .models import Todo, TodoAnother, TodoYetAnother +from .serializer import TodoAnotherSerializer, TodoSerializer, TodoYetAnotherSerializer + + +class TodoViewSet(viewsets.ReadOnlyModelViewSet): + queryset = Todo.objects.all() + serializer_class = TodoSerializer + + +class TodoAnotherViewSet(viewsets.ReadOnlyModelViewSet): + queryset = TodoAnother.objects.all() + serializer_class = TodoAnotherSerializer + + +class TodoYetAnotherViewSet(viewsets.ReadOnlyModelViewSet): + queryset = TodoYetAnother.objects.all() + serializer_class = TodoYetAnotherSerializer diff --git a/tests/reference.yaml b/tests/reference.yaml index bded5917..f6bb2331 100644 --- a/tests/reference.yaml +++ b/tests/reference.yaml @@ -339,6 +339,105 @@ paths: description: A unique integer value identifying this snippet. required: true type: integer + /todo/: + get: + operationId: todo_list + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + type: array + items: + $ref: '#/definitions/Todo' + tags: + - todo + parameters: [] + /todo/another/: + get: + operationId: todo_another_list + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + type: array + items: + $ref: '#/definitions/TodoAnother' + tags: + - todo + parameters: [] + /todo/another/{id}/: + get: + operationId: todo_another_read + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + $ref: '#/definitions/TodoAnother' + tags: + - todo + parameters: + - name: id + in: path + description: A unique integer value identifying this todo another. + required: true + type: integer + /todo/yetanother/: + get: + operationId: todo_yetanother_list + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + type: array + items: + $ref: '#/definitions/TodoYetAnother' + tags: + - todo + parameters: [] + /todo/yetanother/{id}/: + get: + operationId: todo_yetanother_read + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + $ref: '#/definitions/TodoYetAnother' + tags: + - todo + parameters: + - name: id + in: path + description: A unique integer value identifying this todo yet another. + required: true + type: integer + /todo/{id}/: + get: + operationId: todo_read + description: '' + parameters: [] + responses: + '200': + description: '' + schema: + $ref: '#/definitions/Todo' + tags: + - todo + parameters: + - name: id + in: path + description: A unique integer value identifying this todo. + required: true + type: integer /users/: get: operationId: users_list @@ -538,7 +637,6 @@ definitions: title: Linenos type: boolean language: - title: Language description: Sample help text for language type: object properties: @@ -983,7 +1081,6 @@ definitions: - zephir default: python styles: - title: Styles type: array items: type: string @@ -1020,12 +1117,10 @@ definitions: default: - friendly lines: - title: Lines type: array items: type: integer exampleProjects: - title: Example projects type: array items: $ref: '#/definitions/Project' @@ -1047,6 +1142,64 @@ definitions: format: decimal default: 0.0 minimum: 0.0 + Todo: + required: + - title + type: object + properties: + title: + title: Title + type: string + maxLength: 50 + TodoAnother: + required: + - title + - todo + type: object + properties: + title: + title: Title + type: string + maxLength: 50 + todo: + $ref: '#/definitions/Todo' + TodoYetAnother: + required: + - title + type: object + properties: + title: + title: Title + type: string + maxLength: 50 + todo: + required: + - title + type: object + properties: + id: + title: ID + type: integer + readOnly: true + title: + title: Title + type: string + maxLength: 50 + todo: + required: + - title + type: object + properties: + id: + title: ID + type: integer + readOnly: true + title: + title: Title + type: string + maxLength: 50 + readOnly: true + readOnly: true UserSerializerrr: required: - username @@ -1071,13 +1224,11 @@ definitions: format: email maxLength: 254 articles: - title: Articles type: array items: type: integer uniqueItems: true snippets: - title: Snippets type: array items: type: integer @@ -1095,7 +1246,6 @@ definitions: format: date readOnly: true article_slugs: - title: Article slugs type: array items: type: string diff --git a/tests/test_reference_schema.py b/tests/test_reference_schema.py index 638821b9..4ff0aff6 100644 --- a/tests/test_reference_schema.py +++ b/tests/test_reference_schema.py @@ -46,3 +46,9 @@ def set_inspectors(inspectors, setting_name): json_bytes = codec_json.encode(swagger) swagger_dict = json.loads(json_bytes.decode('utf-8'), object_pairs_hook=OrderedDict) compare_schemas(swagger_dict, reference_schema) + + +def test_no_nested_model(swagger_dict): + # ForeignKey models in deep ModelViewSets might wrongly be labeled as 'Nested' in the definitions section + # see https://github.com/axnsan12/drf-yasg/issues/59 + assert 'Nested' not in swagger_dict['definitions']