Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When DEFAULT_VERSIONING_CLASS is set to "AcceptHeaderVersioning" the SpectacularSwaggerView is inaccessible #1036

Closed
PureTryOut opened this issue Jul 26, 2023 · 10 comments

Comments

@PureTryOut
Copy link

Describe the bug
I'm trying to version the endpoints:

REST_FRAMEWORK = {
    "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning",
    "ALLOWED_VERSIONS": ["v1"],
    "VERSION_PARAM": "version",
   ...
}

With these settings the SpectacularSwaggerView becomes inaccessible, as it is internally a DRF view which now requires the accept header to contain version=v1. The browser of course doesn't set that so when navigating to that page I now get 406 Not Acceptable.
Versioning makes sense for every endpoint except for the Swagger view itself. If I set the default version the view works fine.

To Reproduce
Set DEFAULT_VERSIONING_CLASS to rest_framework.versioning.AcceptHeaderVersioning and make sure no default version is set.

Expected behavior
The Swagger view to be available at all times, no matter if the API version is set in the request headers or not.

@tfranzel
Copy link
Owner

tfranzel commented Jul 27, 2023

Good point @PureTryOut, the SpectacularSwaggerView should not be versioned. I guess this was never found because people either provide a DEFAULT_VERSION or don't provide a ALLOWED_VERSIONS restriction when using versioning everywhere. We should probable null out the versioning_class by default.

In the meantime you can do

path('api/schema/swagger-ui/', SpectacularSwaggerView.as_view(url_name='schema', versioning_class=None), name='swagger-ui'),

, which should have the same effect as the upcoming fix.

@tfranzel tfranzel added the bug Something isn't working label Jul 28, 2023
@PureTryOut
Copy link
Author

Hmm that doesn't seem to work. Well Swagger loads again, but then complains that Not Acceptable /api/schema/. I guess it doesn't pass a version a long? I'd expect a dropdown of some kind in the Swagger view to select the version of the API you want to see.

@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2023

That was to be expected, since you do not have a default version and that makes perfect sense. You hard require a version for your API while not setting a default. The schema should not work as we cannot know what you want. There are two options.

hardcode the version

path('api/schema/', SpectacularAPIView.as_view(api_version="v1"), name='schema'),

OR use the swagger topbar:

SPECTACULAR_SETTINGS = {
    "SERVE_INCLUDE_SCHEMA": False,
    "SWAGGER_UI_SETTINGS": '''{
        deepLinking: true,
        urls: [{url: "/api/schema/?version=v1", name: "v1"}, {url: "/api/schema/?version=v2", name: "v2"}],
        presets: [SwaggerUIBundle.presets.apis, SwaggerUIStandalonePreset],
        layout: "StandaloneLayout",
        persistAuthorization: true,  
        displayOperationId: true,
        filter: true 
    }''',
}

#784 (comment)

Note that you still need the SpectacularSwaggerView hotfix until we fix it upstream.

@PureTryOut
Copy link
Author

I'd like to use that topbar, but those urls examples don't work with AcceptHeaderVersioning do they? Endpoints do not expect a version scheme in the url, but as part of a request header. Indeed the Swagger UI still fails to load the scheme and the error returned by the scheme endpoint is Invalid version in "Accept" header.

@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2023

I'd like to use that topbar, but those urls examples don't work with AcceptHeaderVersioning do they?

they do (indirectly). that is a feature of the schema endpoint. that version param should be handed over to the appropriate places. It certainly is not just forwarded as a param to the API endpoints.

Invalid version in "Accept" header

Hmm that should not be hit anymore. Strange. Have you tried the hardcoded version also? Internally they are almost the same and that route might be easier to debug.

@PureTryOut
Copy link
Author

I could hardcode the version for the schema, e.g. path("api/schema/", SpectacularAPIView.as_view(api_version="v1"), name="schema"),, but that kinda defeats the point doesn't it? 😅

@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2023

I'm not suggesting you should should choose that as a permanent solution. I'm just asking you to test this variant to further investigate the reasons why this is not working for you.

this is just a more straight forward override of the version with one step less that is internally identical:

version = self.api_version or request.version or self._get_version_parameter(request)

@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2023

nevermind my last comment, it will not work either way. I think i understood it.

When using AcceptHeaderVersioning on SpectacularAPIView with your specific settings, the request gets rejected early inside DRF long before it reaches any spectacular code. We have no way of massaging the version as DRF flat out rejects the request due to missing version info.

Your settings require a ALLOWED_VERSIONS and you don't have a DEFAULT_VERSION, so the schema request can never succeed without an explicitly given version. This is okay for the API but makes no sense for the schema endpoint.

3 options:

  • change settings to be graceful with default version
  • use adapted versioning class with default version on schema view only
  • explicitly version schema endpoints without versioning class
        path("api/schema/v1/", SpectacularAPIView.as_view(versioning_class=None, api_version="v1")
        path("api/schema/v2/", SpectacularAPIView.as_view(versioning_class=None, api_version="v2")
    

This is not a bug but a (non-obviously) broken configuration on your part. There is basically nothing we can do to mitigate this in our end.

@tfranzel tfranzel removed the bug Something isn't working label Aug 1, 2023
@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2023

ahh wait you can even do

path("api/schema/", SpectacularAPIView.as_view(versioning_class=None),
path('api/schema/swagger-ui/', SpectacularSwaggerView.as_view(url_name='schema', versioning_class=None), name='swagger-ui'),

and on swagger config

urls: [{url: "/api/schema/?version=v1", name: "v1"}, {url: "/api/schema/?version=v2", name: "v2"}],

and it should work. the version gets picked up from the url and used even if you do not have a versioning class set on our views. This circumvents rejected requests due to a missing header while still utilizing the version from the query param.

@PureTryOut
Copy link
Author

That works great! Thank you for the help, this makes things a lot easier 😄

tfranzel added a commit that referenced this issue Sep 23, 2023
Even though this change makes sense in principle,
it may be a regression and having the HTML views have versioning
classes does make sense in certain situations.
Revert the change as this is not a clear improvement on closer
inspection.

This reverts commit 388da8d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants