NamespaceVersioning causes 404 exception in the schema generation

[maerteijn]

So, let's try to describe this correctly here.

When NamespaceVersioning is enabled, the schema endpoint returns a 404 when some of the url definitions contain captured values like a slug, id etc. Example:

    path(
        "productclasses/<slug:slug>/",
        ProductClassAdminDetail.as_view(),
        name="admin-productclass-detail",
    ),

The modify_for_versioning function will then be called in the Schemagenerator:

    elif view.versioning_class:
        version = (
            self.api_version  # generator was explicitly versioned
            or getattr(request, 'version', None)  # incoming request was versioned
            or view.versioning_class.default_version  # fallback
        )
        path = modify_for_versioning(self.inspector.patterns, method, path, view, version)

This will eventually will call a django resolver to modify the mocked request with the (versioned) resolver_match of the url:

    elif issubclass(view.versioning_class, versioning.NamespaceVersioning):
        request.resolver_match = get_resolver(tuple(patterns)).resolve(path)

And here things go wrong when it can't resolve the path. This can happen because the path is already modified by coerse_path earlier in the process , so it won't match:

[[<URLPattern '^productclasses/(?P<slug>[-\w]+)/$' [name='admin-productclass-detail']>]], 'path': 'productclasses/{slug}/'}

And this will raise a django.urls.exceptions.Resolver404 exception, resulting in a 404 result for the complete schema.

I guess the coerce_path method should be called adfer modify_for_versioning, or the 404 exception should be caught here and log a warning but I'm not yet up to speed of the complete internal functionality of drf-spectacular to correctly judge this. I'm looking forward to your view on this.

[tfranzel]

hey @maerteijn . thanks for the detailed explanation. i tried finding the issue, but it is not immediately obvious to me. in fact, the versioning tests go through that path (namespace with a viewset list id replacement) without an issue. i confirmed that with the debugger. i think i need a bit of help to get a grip on that issue.

could you do a reproduction? ideally in either one of those 2 spots?
https://github.com/tfranzel/drf-spectacular/blob/master/tests/test_versioning.py#L91
https://github.com/tfranzel/drf-spectacular/blob/master/tests/test_versioning.py#L169

[maerteijn]

I’ll try to reproduce the issue in one of the testcases, to be continued.

[maerteijn]

So here we go: https://github.com/tfranzel/drf-spectacular/compare/master...maerteijn:reproduce-404-with-namespace-versioning?expand=1

When you checkout my reproduce-404-with-namespace-versioning branch the testrunner will log the errors (and fail) on the console when running:

 pytest tests/test_versioning.py

When you change the newly added NamespaceVersioningAPIView from:

class NamespaceVersioningAPIView(generics.RetrieveUpdateDestroyAPIView):
    versioning_class = NamespaceVersioning
    ...

to

class NamespaceVersioningAPIView(generics.RetrieveUpdateDestroyAPIView):
    versioning_class = None
    ...

the tests will pass

[tfranzel]

awesome, thanks! i will investigate

[tfranzel]

@maerteijn so that was trickier than i expected. the core problem was that namespace versioning uses the native DRF router for finding the correct route (and therefore the namespace). the resolution was super simple and did not account for path variables pattern <int:pk> does not "match" the string <int:pk>, but pattern <pk> will match the string <pk>. long story short, temporarily de-typed the pattern just to find correct namespace.

problem should be solved for <TYPE:foo> and any path regex. please confirm and close, thx.

[maerteijn]

I can confirm this works now in the reproduce-404-with-namespace-versioning branch, and for all the Admin API endpoints of django-oscar-api when using NamespacedVersioning.

👍 Thanks!

[tfranzel]

awesome 👍 sry that i didn't merge any commits of yours. it occurred to me too late.

[maerteijn]

NP, those were just 'showing what's wrong' commits, not 'this fixes the issue' commits 😄 Commit credits go to who fixed it!