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

Versioning support making up non-existent versioned url #1327

Open
stefanofusai opened this issue Nov 11, 2024 · 5 comments
Open

Versioning support making up non-existent versioned url #1327

stefanofusai opened this issue Nov 11, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@stefanofusai
Copy link

stefanofusai commented Nov 11, 2024

Introduction
The urls.py file for one of my apps, let's call it foo, looks (somewhat) like this:

from django.urls import include, re_path
from rest_framework.routers import SimpleRouter

from .views import ConfigAPIView, MyViewSet, MyAPIView

app_name = "foo"

handler400 = "rest_framework.exceptions.bad_request"
handler500 = "rest_framework.exceptions.server_error"

router = SimpleRouter()
router.register("bar", MyViewSet, basename="bar")
...
urlpatterns = [
	...,
    re_path(r"^(?P<version>v4)/config/$", ConfigAPIView.as_view(), name="config"),
    re_path(
        r"^(?P<version>v[34])/baz/$", MyAPIView.as_view(), name="baz"
    ),
    re_path(r"^(?P<version>v[34])/", include(router.urls)),
]

And the urls.py file for my Django project looks like this:

from django.conf import settings
from django.conf.urls.static import static
from django.contrib import admin
from django.urls import include, path
from drf_spectacular.views import SpectacularAPIView

from core.views import SpectacularSwaggerView

urlpatterns = [
   	...,
    path("foo/", include("foo.urls")),
    path("qux/", include("qux.urls")),
    path(
        "docs/", SpectacularSwaggerView.as_view(url_name="schema-v4"), name="swagger-ui"
    ),
    path(
        "schema/v3/",
        SpectacularAPIView.as_view(
            api_version="v3", custom_settings={"VERSION": "3.0.0"}
        ),
        name="schema-v3",
    ),
    path(
        "schema/v4/",
        SpectacularAPIView.as_view(
            api_version="v4", custom_settings={"VERSION": "4.0.0"}
        ),
        name="schema-v4",
    ),
    ...
]

As you can see from the code snippet, all endpoints of the foo app support versioning (either v3 or v4), except for one endpoint (config) which was introduced in the v4 API and only supports the v4 version.

Here's a screenshot of the foo app's urls via Django's URLs in DEBUG mode:
image

The bug
For some reason, the schema-v3 endpoint includes a /v3/config/ path which is not supposed to be there and obviously results in a 404 when trying it out via Swagger UI. The schema-v4 endpoint correctly includes the /v4/config/ path.

image image

Expected behavior
The schema-v3 endpoint doesn't include a /v3/config/ path.

Additional information

settings.py

REST_FRAMEWORK = {
    ...
    "DEFAULT_SCHEMA_CLASS": "core.openapi.AutoSchema",
    "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.URLPathVersioning",
	...
}
SPECTACULAR_SETTINGS = {
    "AUTHENTICATION_WHITELIST": [
        "rest_framework_simplejwt.authentication.JWTAuthentication"
    ],
    ...
    "ENUM_NAME_OVERRIDES": {
        "ValidationErrorEnum": "drf_standardized_errors.openapi_serializers.ValidationErrorEnum.choices",
        "ClientErrorEnum": "drf_standardized_errors.openapi_serializers.ClientErrorEnum.choices",
        "ServerErrorEnum": "drf_standardized_errors.openapi_serializers.ServerErrorEnum.choices",
        "ErrorCode401Enum": "drf_standardized_errors.openapi_serializers.ErrorCode401Enum.choices",
        "ErrorCode403Enum": "drf_standardized_errors.openapi_serializers.ErrorCode403Enum.choices",
        "ErrorCode404Enum": "drf_standardized_errors.openapi_serializers.ErrorCode404Enum.choices",
        "ErrorCode405Enum": "drf_standardized_errors.openapi_serializers.ErrorCode405Enum.choices",
        "ErrorCode406Enum": "drf_standardized_errors.openapi_serializers.ErrorCode406Enum.choices",
        "ErrorCode415Enum": "drf_standardized_errors.openapi_serializers.ErrorCode415Enum.choices",
        "ErrorCode429Enum": "drf_standardized_errors.openapi_serializers.ErrorCode429Enum.choices",
        "ErrorCode500Enum": "drf_standardized_errors.openapi_serializers.ErrorCode500Enum.choices",
    },
    "OAS_VERSION": "3.1.0",
    "POSTPROCESSING_HOOKS": [
        "drf_standardized_errors.openapi_hooks.postprocess_schema_enums"
    ],
    "SCHEMA_PATH_PREFIX": r"^(/auth/|/foo/qux/v[34]/)$",
    "SECURITY": [{"jwtAuth": []}],
    "SERVERS": [{"url": "http://127.0.0.1:8000", "description": "Localhost"}],
    "SERVE_INCLUDE_SCHEMA": False,
    "SERVE_PUBLIC": False,
    "SWAGGER_UI_SETTINGS": """{
        defaultModelsExpandDepth: -1,
        deepLinking: true,
        displayRequestDuration: true,
        layout: "StandaloneLayout",
        persistAuthorization: true,
        presets: [SwaggerUIBundle.presets.apis, SwaggerUIStandalonePreset],
        requestSnippetsEnabled: true,
        tagsSorter: "alpha",
        urls: [{url: "/schema/v4/", name: "v4"}, {url: "/schema/v3/", name: "v3"}]
    }""",
	...
}

core/openapi.py

from django.conf import settings
from drf_spectacular.utils import Direction, OpenApiParameter, OpenApiTypes
from drf_standardized_errors.openapi import AutoSchema as BaseAutoSchema
from rest_framework.serializers import Serializer

from .mixins import RequiredHeadersMixin


class AutoSchema(BaseAutoSchema):
    def get_override_parameters(self) -> list[OpenApiParameter]:
        override_parameters = []

        if any(
            f"/{version}/" in self.path
            for version in set(
                settings.FOO_API_VERSIONS + settings.QUX_API_VERSIONS
            )
        ):
            override_parameters += [
                OpenApiParameter(
                    name=header.name,
                    type=OpenApiTypes.STR,
                    location=OpenApiParameter.HEADER,
                    required=True,
                    enum=header.choices,
                )
                for header in RequiredHeadersMixin.get_required_headers(
                    self.view, self.method
                )
            ]

        return override_parameters

    def get_serializer_name(self, serializer: Serializer, direction: Direction) -> str:
        return f"{serializer.__class__.__module__}.{serializer.__class__.__name__}"
@stefanofusai
Copy link
Author

stefanofusai commented Nov 11, 2024

Note: this also happens when defining the url in foo.urls.urlpatterns without versioning, as: path("v4/config/", ConfigAPIView.as_view(), name="config")

@tfranzel
Copy link
Owner

Hey,

so on first sight this looks pretty good. Everything seems setup properly. FYI we do fully support this. Tests are here:

https://github.com/tfranzel/drf-spectacular/blob/master/tests/test_versioning.py

DEFAULT_VERSIONING_CLASS looks good and should be used everywhere including spectacular.

Can you step into the the debugger and see if the version on any view actually parses correctly?

wait a sec.. i think it is expected that the config view shows up if it is mounted. the view should then deal with the version and raise if not supported I think. However, thats bad for the schema. I have to think about whats the right thing to do here, but for now I think this behaves "as expected".

Note: this also happens when defining the url in foo.urls.urlpatterns without versioning, as: path("v4/config/", ConfigAPIView.as_view(), name="config")

without versioning enabled, everything is shown. that okay afais

@stefanofusai
Copy link
Author

stefanofusai commented Nov 11, 2024

Thank you for the quick reply!

Just to make sure we're on the same page, what I'm expecting is for the config view to only be present in the v4 schema and not in the v3 schema.

Even without versioning enabled on the view, from my previous comment, it appears in both schemas.

Let me know if I can help you with more information!

@tfranzel
Copy link
Owner

Just to make sure we're on the same page, what I'm expecting is for the config view to only be present in the v4 schema and not in the v3 schema.

Yes, that makes sense. My statement earlier was a bit imprecise.

You are correct and I kind of missed this aspect. We support 4 different versioning classes and the concept of route filtering based on "populated" params apparently only makes sense for URLPathVersioning. Each versioning base class has it's on quirks and since this makes no sense for the others (e.g.HeaderVersioning, NamespaceVersioning), it probably got overlooked.

This likely needs to be handled individually in the generator.

@stefanofusai
Copy link
Author

Sounds good! I guess this can then be marked as a bug?

Let me know if I can be of further help. Thanks again for the prompt replies, much appreciated as always!

@tfranzel tfranzel added the bug Something isn't working label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants