Potential bug with OPTIONS request returning all fields of a nested serializer, despite explicit fields passed to `fields` arg.

[resurrexi]

I've lately been using the OPTIONS request to get metadata on the fields that I defined for my serializer. However, after sending the request, I'll get metadata on all fields from a nested serializer field, even if I specifically request only a specific set of fields from that nested field.

For example, if I have a serializer:

class SerializerA(DynamicFieldsMixin, NestedModelSerializer):
    field = NestedField(SerializerB, read_only=True, fields=["id", "field1"])

And then I send an OPTIONS request to the list URL endpoint, I'll get a response like:

{
    "name": "ModelA List",
    "description": "",
    "renders": [
        "application/json",
        "text/html"
    ],
    "parses": [
        "application/json",
        "application/x-www-form-urlencoded",
        "multipart/form-data"
    ],
    "actions": {
        "POST": {
            "id": {
                "type": "integer",
                "required": false,
                "read_only": true,
                "label": "ID"
            },
            "field": {
                "type": "nested object",
                "required": false,
                "read_only": true,
                "label": "My field",
                "children": {
                    "id": {
                        "type": "integer",
                        "required": false,
                        "read_only": true,
                        "label": "ID"
                    },
                    "field1": {
                        "type": "datetime",
                        "required": false,
                        "read_only": false,
                        "label": "Field1"
                    },
                    "field2": {
                        "type": "string",
                        "required": false,
                        "read_only": true,
                        "label": "Field2"
                    },
                    "field3": {
                        "type": "string",
                        "required": false,
                        "read_only": true,
                        "label": "Field3"
                    }
                }
            }
        }
    }
}

I don't know if the intended design of django-restql was to ignore the fields argument when using OPTIONS requests, but I ideally would like for it to obey it, because I recently ran into a recursive maximum depth issue when sending OPTIONS requests on a self-referencing serializer (see #185).

When I was going through the stack trace, I noticed the likely cause is where DynamicFieldsMixin has a _use_restql_fields attribute that's set to False by default. This means that when fetching the fields of a serializer, it skips resolving of what fields are allowed, and defaults to calling the standard get_fields method from DRF's ModelSerializer.

[yezyilomo]

Thank you @resurrexi for reporting this. Django RESTQL is not primarily designed to handle OPTION requests but the way I see it, it's supposed to work just fine, I think setting _use_restql_fields=False by default is not the problem, in fact it's very important to set its value to False because it allows us to query fields even when we use POST, PUT and PATCH without interfering with data creation/updation, if we don't set _use_restql_fields=False initially query might remove fields which are needed during data creation/updation, check here and here to understand why we are setting _use_restql_fields=False initially and change it to True in to_representation method.

I think the problem is here https://github.com/yezyilomo/django-restql/blob/2709cb92df4587eea83de8b35411e586920b4da5/django_restql/mixins.py#L391-L398 I think it's supposed to be like this

    @property
    def fields(self):
        if self._use_restql_fields:
            # Use restql fields
            return self.restql_fields
        return self.get_allowed_fields()

I have tried to change it, all tests have passed, now we need to know if it works on OPTION requests too. Could you please try it against your project on your side since currently we have not written tests for OPTION requests?.

[resurrexi]

@yezyilomo Thanks for responding so quickly and bringing me up to speed with the _use_restql_fields. I wasn't too familiar with its usage so it was just a guess.

I tested out your recommendation replacing return self._all_fields with return self.get_allowed_fields(). It now respects the fields arguments with the OPTIONS request.

Also despite django-restql not designed for OPTIONS request, this was just a very edgy case because I needed to send OPTIONS requests to get all values and display names for ChoiceFields and I don't want to maintain a seperate file of choices for my frontend app. I imagine there will probably be others out there who will also use OPTIONS requests for this purpose.

[yezyilomo]

Thank you for helping in testing that, now we know that it works we need to write tests for it, I’ve never used OPTION for checking metadata before so if you could assist in writing tests I would very much appreciate.

Also despite django-restql not designed for OPTIONS request, this was just a very edgy case because I needed to send OPTIONS requests to get all values and display names for ‘ChoiceFields’ and I don't want to maintain a seperate file of choices for my frontend app. I imagine there will probably be others out there who will also use OPTIONS requests for this purpose.

Don’t worry, we’re definitely going to support OPTION requests, we started with only GET requests but here we are supporting GET, POST, PUT and PATCH so the aim is to support all actions, we just need use cases to support them, that’s all.

[yezyilomo]

Would like to release v0.12.0 soon which will include this fix, #209 and #211

[resurrexi]

Thank you for helping in testing that, now we know that it works we need to write tests for it, I’ve never used OPTION for checking metadata before so if you could assist in writing tests I would very much appreciate.

I haven't used OPTIONS until now. Also, browsing through the web, it seems like there's no authoritative standard in terms of what an OPTIONS response should be. Even in the official DRF docs, they say:

There are not currently any widely adopted conventions for exactly what style of response should be returned for HTTP OPTIONS requests, so we provide an ad-hoc style that returns some useful information.

However, considering django-restql sits on top of DRF, we can make the assumption that OPTIONS requests should return responses that follow DRF's default response style. That means at a minimum, the response should contain:

1. the response headers with the Allow header being the most important, which tells the client which REST verbs are allowed on the resource, and
2. the fields and underlying metadata available for the resource

I think bullet 2 is probably more relevant to django-restql since restql controls which fields are exposed, and I think this should be tested.

Thoughts?

[yezyilomo]

I think bullet 2 is probably more relevant to django-restql since restql controls which fields are exposed, and I think this should be tested.

I agree with you here, tests should be checking whether django-restql respects ‘exclude’ kwarg when it receives OPTION requests, so I expect it to be something like a serializer which excludes some fields then on a response we check if they’re excluded too or not.