"type" mismatch with "enum" contents about drf-yasg

[davcamer]

Thanks. I was able to get a full client generated using your suggestions for custom field inspectors. I'll close this for now, and link the inspectors if we end up continuing with it.

[axnsan12]

Hello!

I'm not sure I really understand your situation (there's quite a lot of code there!), but:

• drf-yasg generates each model in Models section of the swagger document strictly from serializers - so if you have three different serializers, you should have no problem in getting three different representations generated
• the type generated for serializers.ChoiceField and serializers.MultipleChoiceField is currently hardcoded to string (in ModelSerializers, model fields with choices=... automatically generate ChoiceFields - http://www.django-rest-framework.org/api-guide/fields/#choice-selection-fields).
  • This hardcoding is because I found no obvious way to introspect the expected type of the ChoiceField, and because strings are always accepted as input by the django-rest-framework ChoiceField: https://github.com/encode/django-rest-framework/blob/master/rest_framework/fields.py#L1373. I don't know how/if this could be handled better in drf-yasg - maybe we should also convert all values in the enum key to strings? (just like DRF does when handling input)

However I wasn't really able to follow what is your desired output. Does the type/enum mismatch cause problems for you? Which part exactly would you like to be generated differently?

It would really help if you could provide the exact definitions of each of your problematic serializers (this can be done by printing the serializer itself: http://www.django-rest-framework.org/api-guide/serializers/#inspecting-a-modelserializer), along with what you would want to be generated differently in the swagger schema.

[davcamer]

It is definitely a lot of code. I wanted to share as much as possible up front, in case it was easy to find the problem with enough code. 😆

I'll focus on a single model and accompanying serializers because I think solving the problem for one type will solve it for all the types. Digging in to Racks then. drf-yasg is getting all the different serializers, so that is good.

The type mismatch is causing two problems. One very practical and the other more theoretical:

1. I'm trying to use the swagger.json output to generate a golang client using go-swagger. The type mismatch is choking this code generation. As seen in that issue, go-swagger might be patched already to handle this case though.
2. The generated openapi spec doesn't match the way that the existing API works. I'll detail the specifics of this.

Thanks for pointing out the way to inspect serializers. Here they are for Racks. Type and width are the fields that cause problems for Racks.

The RackSerializer:

RackSerializer():
    id = IntegerField(label='ID', read_only=True)
    name = CharField(max_length=50, required=True)
    facility_id = CharField(allow_blank=True, allow_null=True, label='Facility ID', max_length=50, required=True)
    display_name = ReadOnlyField()
    site = NestedSiteSerializer():
        id = IntegerField(label='ID', read_only=True)
        url = HyperlinkedIdentityField(view_name='dcim-api:site-detail')
        name = CharField(max_length=50, validators=[<UniqueValidator(queryset=Site.objects.all())>])
        slug = SlugField(max_length=50, validators=[<UniqueValidator(queryset=Site.objects.all())>])
    group = NestedRackGroupSerializer():
        id = IntegerField(label='ID', read_only=True)
        url = HyperlinkedIdentityField(view_name='dcim-api:rackgroup-detail')
        name = CharField(max_length=50)
        slug = SlugField(max_length=50)
    tenant = NestedTenantSerializer():
        id = IntegerField(label='ID', read_only=True)
        url = HyperlinkedIdentityField(view_name='tenancy-api:tenant-detail')
        name = CharField(max_length=30, validators=[<UniqueValidator(queryset=Tenant.objects.all())>])
        slug = SlugField(max_length=50, validators=[<UniqueValidator(queryset=Tenant.objects.all())>])
    role = NestedRackRoleSerializer():
        id = IntegerField(label='ID', read_only=True)
        url = HyperlinkedIdentityField(view_name='dcim-api:rackrole-detail')
        name = CharField(max_length=50, validators=[<UniqueValidator(queryset=RackRole.objects.all())>])
        slug = SlugField(max_length=50, validators=[<UniqueValidator(queryset=RackRole.objects.all())>])
    serial = CharField(allow_blank=True, label='Serial number', max_length=50, required=False)
    type = ChoiceFieldSerializer(choices=((100, '2-post frame'), (200, '4-post frame'), (300, '4-post cabinet'), (1000, 'Wall-mounted frame'), (1100, 'Wall-mounted cabinet')))
    width = ChoiceFieldSerializer(choices=((19, '19 inches'), (23, '23 inches')))
    u_height = IntegerField(label='Height (U)', max_value=100, min_value=1, required=False)
    desc_units = BooleanField(help_text='Units are numbered top-to-bottom', label='Descending units', required=False)
    comments = CharField(allow_blank=True, required=False, style={'base_template': 'textarea.html'})
    custom_fields = CustomFieldsSerializer(required=False)
    class Meta:
        validators = [<UniqueTogetherValidator(queryset=Rack.objects.all(), fields=('site', 'name'))>, <UniqueTogetherValidator(queryset=Rack.objects.all(), fields=('site', 'facility_id'))>]

The WritableRackSerializer:

WritableRackSerializer():
    id = IntegerField(label='ID', read_only=True)
    name = CharField(max_length=50)
    facility_id = CharField(allow_blank=True, allow_null=True, label='Facility ID', max_length=50, required=False)
    site = PrimaryKeyRelatedField(queryset=Site.objects.all())
    group = PrimaryKeyRelatedField(allow_null=True, queryset=RackGroup.objects.all(), required=False)
    tenant = PrimaryKeyRelatedField(allow_null=True, queryset=Tenant.objects.all(), required=False)
    role = PrimaryKeyRelatedField(allow_null=True, queryset=RackRole.objects.all(), required=False)
    serial = CharField(allow_blank=True, label='Serial number', max_length=50, required=False)
    type = ChoiceField(allow_null=True, choices=((100, '2-post frame'), (200, '4-post frame'), (300, '4-post cabinet'), (1000, 'Wall-mounted frame'), (1100, 'Wall-mounted cabinet')), required=False, validators=[<django.core.validators.MinValueValidator object>, <django.core.validators.MaxValueValidator object>])
    width = ChoiceField(choices=((19, '19 inches'), (23, '23 inches')), help_text='Rail-to-rail width', required=False, validators=[<django.core.validators.MinValueValidator object>, <django.core.validators.MaxValueValidator object>])
    u_height = IntegerField(label='Height (U)', max_value=100, min_value=1, required=False)
    desc_units = BooleanField(help_text='Units are numbered top-to-bottom', label='Descending units', required=False)
    comments = CharField(allow_blank=True, required=False, style={'base_template': 'textarea.html'})
    custom_fields = CustomFieldsSerializer(required=False)
    class Meta:
        validators = [<UniqueTogetherValidator(queryset=<QuerySet [<Rack: Test Rack (0.0.1)>]>, fields=('site', 'name'))>]

This made me realize that the fields causing the problems are using a custom serializer class in the standard seralizers, derived from rest_framework.serializers.Field:

class ChoiceFieldSerializer(Field):
    """
    Represent a ChoiceField as {'value': <DB value>, 'label': <string>}.
    """
    def __init__(self, choices, **kwargs):
        self._choices = dict()
        for k, v in choices:
            # Unpack grouped choices
            if type(v) in [list, tuple]:
                for k2, v2 in v:
                    self._choices[k2] = v2
            else:
                self._choices[k] = v
        super(ChoiceFieldSerializer, self).__init__(**kwargs)

    def to_representation(self, obj):
        return {'value': obj, 'label': self._choices[obj]}

    def to_internal_value(self, data):
        return self._choices.get(data)

This custom field serializer breaks the fields, such as width, in to a small sub-object:

{
  "id": 1,
  "name": "Test Rack",
  "facility_id": "0.0.1",
  "display_name": "Test Rack (0.0.1)",
  "site": {
    "id": 1,
    "url": "http://localhost:8000/api/dcim/sites/1/",
    "name": "Area 51",
    "slug": "area-51"
  },
  "group": null,
  "tenant": null,
  "role": null,
  "serial": "",
  "type": null,
  "width": {
    "value": 19,
    "label": "19 inches"
  },
  "u_height": 42,
  "desc_units": false,
  "comments": "",
  "custom_fields": {}
}

The generated swagger for this field doesn't show the nested object for the width field, it just shows a flat string:

    "Rack": {
      "required": [
        "name",
        "facility_id",
        "site",
        "group",
        "tenant",
        "role",
        "type",
        "width"
      ],
      "type": "object",
      "properties": {
        "id": {
          "title": "ID",
          "type": "integer",
          "readOnly": true
        },
        "name": {
          "title": "Name",
          "type": "string",
          "maxLength": 50
        },
        "facility_id": {
          "title": "Facility ID",
          "type": "string",
          "maxLength": 50
        },
        "display_name": {
          "title": "Display name",
          "type": "string",
          "readOnly": true
        },
        "site": {
          "$ref": "#/definitions/NestedSite"
        },
        "group": {
          "$ref": "#/definitions/NestedRackGroup"
        },
        "tenant": {
          "$ref": "#/definitions/NestedTenant"
        },
        "role": {
          "$ref": "#/definitions/NestedRackRole"
        },
        "serial": {
          "title": "Serial number",
          "type": "string",
          "maxLength": 50
        },
        "type": {
          "title": "Type",
          "type": "string"
        },
        "width": {
          "title": "Width",
          "type": "string"
        },
        "u_height": {
          "title": "Height (U)",
          "type": "integer",
          "maximum": 100,
          "minimum": 1
        },
        "desc_units": {
          "title": "Descending units",
          "description": "Units are numbered top-to-bottom",
          "type": "boolean"
        },
        "comments": {
          "title": "Comments",
          "type": "string"
        },
        "custom_fields": {
          "title": "Custom fields",
          "type": "string"
        }
      }
    }

I guess this would ideally be a reference to a type that includes the value and label:

        "width": {
          "$ref": "#/definitions/IntegerChoiceField"
        },
...
    "IntegerChoiceField": {
      "required": [
        "label",
        "value"
      ],
      "type": "object",
      "properties": {
        "label": {
          "title": "Label",
          "type": "string",
          "readOnly": true
        },
        "value": {
          "title": "Value",
          "type": "integer",
          "readOnly": true
        }
      }
    },

For the WritableRack serializer, as you pointed out, strings are accepted for input. They are the string form of the value, rather than the label, which I wasn't expecting. On the response though, the fields use integers if the choice values are integers. For example a curl to PATCH the width field returns an integer for width:

$ curl -X PATCH --header 'Content-Type: application/json' --header 'Accept: application/json' --header 'X-CSRFToken: IX6FtawujHl73Qc6qBBsB6KED9HNGUEP1KdfzhzpkK2Fnq2GqKcYf9jpR1zHReFQ' -d '{"width": 19}' 'http://localhost:8000/api/dcim/racks/1/'
{
  "id": 1,
  "name": "Test Rack",
  "facility_id": "0.0.1",
  "site": 1,
  "group": null,
  "tenant": null,
  "role": null,
  "serial": "",
  "type": null,
  "width": 19,
  "u_height": 42,
  "desc_units": false,
  "comments": "",
  "custom_fields": {}
}

So the swagger definition that currently shows string for the field:

    "WritableRack": {
      "required": [
        "name",
        "site"
      ],
      "type": "object",
      "properties": {
        "id": {
          "title": "ID",
          "type": "integer",
          "readOnly": true
        },
        "name": {
          "title": "Name",
          "type": "string",
          "maxLength": 50
        },
        "facility_id": {
          "title": "Facility ID",
          "type": "string",
          "maxLength": 50
        },
        "site": {
          "title": "Site",
          "type": "integer"
        },
        "group": {
          "title": "Group",
          "type": "integer"
        },
        "tenant": {
          "title": "Tenant",
          "type": "integer"
        },
        "role": {
          "title": "Role",
          "type": "integer"
        },
        "serial": {
          "title": "Serial number",
          "type": "string",
          "maxLength": 50
        },
        "type": {
          "title": "Type",
          "type": "string",
          "enum": [
            100,
            200,
            300,
            1000,
            1100
          ]
        },
        "width": {
          "title": "Width",
          "description": "Rail-to-rail width",
          "type": "string",
          "enum": [
            19,
            23
          ]
        },
        "u_height": {
          "title": "Height (U)",
          "type": "integer",
          "maximum": 100,
          "minimum": 1
        },
        "desc_units": {
          "title": "Descending units",
          "description": "Units are numbered top-to-bottom",
          "type": "boolean"
        },
        "comments": {
          "title": "Comments",
          "type": "string"
        },
        "custom_fields": {
          "title": "Custom fields",
          "type": "string"
        }
      }
    }

Should show integer instead:

...
        "width": {
          "title": "Width",
          "description": "Rail-to-rail width",
          "type": "integer",
          "enum": [
            19,
            23
          ]
        },
...

There are also fields backed by booleans, which hopefully could be approached the same way.

I've already learned a lot about how this works by putting together this email. Please let me know if you have ideas about how to handle the custom field serializer, since that seems to be the root of the trouble.

[axnsan12]

In the read case, the fields are generated as flat strings because drf-yasg has no idea about how to handle your ChoiceFieldSerializer field class. The write case is caused by the hardcoding I talked about above. It could be solved inside drf-yasg by peeking inside the underlying model field to inspect its type, but it could prove a little too presumptuous.

For both cases I think you could solve it with a custom FieldInspector added to DEFAULT_FIELD_INSPECTORS. See also FieldInspector.

Probably something along the lines of (kinda-pseudo-code, meaning I didn't try to run it):

Read case:

class CustomChoiceFieldInspector(FieldInspector):
    def field_to_swagger_object(self, field, swagger_object_type, use_references, **kwargs):
        # this returns a callable which extracts title, description and other stuff for you
        # https://drf-yasg.readthedocs.io/en/stable/_modules/drf_yasg/inspectors/base.html#FieldInspector._get_partial_types
        SwaggerType, _ = self._get_partial_types(field, swagger_object_type, use_references, **kwargs)

        if isinstance(field, ChoiceFieldSerializer):  # <---- your custom choices field
             integer_choice_field = SwaggerType(type=openapi.TYPE_OBJECT, required=["label","value"], properties={"label": openapi.Schema(type=openapi.TYPE_STRING, read_only=True), "value": openapi.Schema(type=openapi.TYPE_INTEGER, read_only=True)})

             # if you want to make it a separate reference
             # if not, "return integer_choice_field" would do
             if use_references:
                 # self.components is a ReferenceResolver: https://drf-yasg.readthedocs.io/en/stable/drf_yasg.html#drf_yasg.openapi.ReferenceResolver
                 definitions = self.components.with_scope(openapi.SCHEMA_DEFINITIONS)
                 definitions.setdefault("IntegerChoiceField", lambda: integer_choice_field)
                 return openapi.SchemaRef(definitions, "IntegerChoiceField)
             else:
                 return integer_choice_field
        

        return NotHandled

The write case is more complicated, you would need a way to discern your ChoiceFields from regular ChoiceFields, perhaps by switching on field.parent and field.name?

   ... same class as before
   def process_result(self, result, method_name, field, **kwargs):
         if <somehow figure out which fields you want to change>:
               result.type = <whatever you want>

         return result

[axnsan12]

Hello!

The second part of your issue should be fixed with 1.4.4.

[davcamer]

That's great! Thanks for following up on that.

[davcamer]

Slightly unexpected but not bad result for boolean fields:

        "connection_status": {
          "title": "Connection status",
          "type": "boolean",
          "enum": [
            false,
            true
          ]
        }

The enum indicating true and false are the valid values seems extraneous? But the tool I'm using hasn't had trouble with it.

[davcamer]

The PR to netbox that result from this, if you're interested:
https://github.com/digitalocean/netbox/pull/1930/files

[axnsan12]

The true/false enum does seem a bit extraneous, but I would guess it comes from a model BooleanField with choices=(True, False)? It could be argued that those are extraneous 😄

The netbox PR is interesting indeed!