input request on schema generator shadows actual required request

[sergei-maertens]

drf-spectacular/drf_spectacular/generators.py

Line 185 in 8b9d0ca

if input_request:

Note that this is untested on the current master branch, but I believe the same problem exists since the method signature hasn't really changed from 0.12.0 (which I'm on):

def parse(self, request, public):
        """ Iterate endpoints generating per method path operations. """
        result = {}
        self._initialise_endpoints()

        for path, path_regex, method, view in self._get_paths_and_endpoints(None if public else request):
            if not self.has_view_permissions(path, method, view):
                continue

            # mocked request to allow certain operations in get_queryset and get_serializer[_class]
            # without exceptions being raised due to no request.
            if not request:
                request = spectacular_settings.GET_MOCK_REQUEST(method, path, view, request)

            view.request = request

This is causing our generated API spec (interactively) to be wrong, because of the different request serializer class based on HTTP method:

    def get_serializer(self, *args, **kwargs):
        if self.request.method == "PATCH":
            return UpdateZaakDocumentSerializer(*args, **kwargs)
        return AddZaakDocumentSerializer(*args, **kwargs)

When you generate the schema from the CLI, a mock request is present with the correct method, from:

for path, path_regex, method, view in self._get_paths_and_endpoints(None if public else request):

However, if you are using the schema view (hooked up in urls.py), then request in

def parse(self, request, public):

is actually the GET request to /api in our case, i.e. the request that fetches/renders the schema.

So, we have a mismatch here, because of:

            if not request:
                request = spectacular_settings.GET_MOCK_REQUEST(method, path, view, request)

            view.request = request

-> method is PATCH, but request.method is GET

This causes the API schema to pick up the AddZaakDocumentSerializer instead of the UpdateZaakDocumentSerializer. Of course, we can fix this with @extend_schema, but that's a bit of a shame.

Is there a reason we cannot always use the mock request for schema generation?

[sergei-maertens]

Looks like #250 might fix this, I'll try it out first

[tfranzel]

most recent release should also be fine for testing this, as master does not contain any related changes.

at least it looks like #250 addressed this issue. i believe there are still ways to break it so we may have to still iterate.

[sergei-maertens]

Tested on 0.15.1 and this version does not fix it, unfortunately :( it is using the input_request instead.

[sergei-maertens]

It's a tricky one, since you want the input request with the actual authenticated user if you only want to display API documentation they have access to.

Perhaps you could grab the input_request, clone it, and alter the relevant attributes: method and path?

def build_mock_request(method, path, view, original_request, **kwargs): does seem to be prepared for this, with the (unused) original_request parameter.

Overriding that setting doesn't work as a workaround, because the mock request function is only called if there's no input_request.

So I propose:

1. Always call the mock request factory, people with different needs can already swap this out for another callable
2. Change the current implementation of the mock request factory to essentially clone the input request if available, and alter method + path, and use the current behaviour if there's no input_request

I'd be fine with just 1. as well, since then I can just provide my own callable

[tfranzel]

haha, wishful thinking didn't help. 😄

Perhaps you could grab the input_request, clone it, and alter the relevant attributes: method and path?

the solution is most likely something to that effect. i'll also look into it.

fyi: you should update in any case as we had dozens of fixes/features since 0.12.0

[sergei-maertens]

I've edited by previous comment, but it seems that we're roughly on the same line :)

fyi: you should update in any case as we had dozens of fixes/features since 0.12.0

done that just now, and it went pretty smooth thanks to swagger_fake_view :)

[tfranzel]

@sergei-maertens the change was a bit more involved, but overall reduced complexity imho. this should now behave exactly as one would supect.

• GET_MOCK_REQUEST is now always called no matter what.
• most headers are carried over
• authenticated user is carried over

please check if that works for you.

[tfranzel]

@sergei-maertens did you have time to give this a try? i would like to have someone else check this before i do a release.

[sergei-maertens]

I haven't had time yet! Maybe next week I can try it out, but no guarantees since I'm involved with other projects atm :P

[sergei-maertens]

@tfranzel just checked out 0.16.0 locally and it appears to work as intended! I'll create a new issue if I spot any new bugs :)