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.

Sign up for GitHub

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

Jump to bottom

Handle Swagger slash basepath within Swagger UI Middleware #1835

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+27 −3
Open

Handle Swagger slash basepath within Swagger UI Middleware #1835

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3b64142
Handle Swagger slash basepath within Swagger UI Middleware
nielsbox Dec 11, 2023
862ba68
Fix tests
nielsbox Dec 11, 2023
af3cd11
Fix tests
nielsbox Dec 11, 2023
75600e9
Fix tests
nielsbox Dec 11, 2023
04e0fee
Fix Linters
nielsbox Dec 11, 2023
53e95a5
Fix Linters
nielsbox Dec 11, 2023
f7c5037
Adress comments
nielsbox Jan 12, 2024
701c7eb
Merge branch 'main' into bugfix/swagger-slash-base-path
nielsbox Jan 12, 2024
306d5ee
Merge branch 'main' into bugfix/swagger-slash-base-path
RobbeSneyders Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion connexion/spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def base_path(self):

@base_path.setter
def base_path(self, base_path):
base_path = canonical_base_path(base_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function seems to be fairly specific for this usecase, can we adjust it instead or create a different util function that adjust the behaviour of canonical_base_path in this way?

Then it's also easier to test because we can just write a unit test for that util function

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit wider comment: the property itself still uses the canonical_base_path function, so the "getter" returns a different value, there's also the added complexity that the base path is used as Flask blueprint name, which has its own rules

base_path = "/" + canonical_base_path(base_path).lstrip("/")
self._raw_spec["basePath"] = base_path
self._spec["basePath"] = base_path

Expand Down
17 changes: 14 additions & 3 deletions tests/api/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from starlette.middleware.cors import CORSMiddleware
from starlette.types import Receive, Scope, Send

from conftest import OPENAPI3_SPEC, build_app_from_fixture
from conftest import BASEPATH_SLASH_SPEC, OPENAPI3_SPEC, build_app_from_fixture


@pytest.fixture(scope="session")
Expand All @@ -24,11 +24,22 @@ def simple_openapi_app(app_class):


@pytest.fixture(scope="session")
def swagger_ui_app(app_class):
def swagger_ui_app(spec, app_class):
return build_app_from_fixture(
"simple",
app_class=app_class,
spec_file=OPENAPI3_SPEC,
spec_file=spec,
validate_responses=True,
swagger_ui_options=SwaggerUIOptions(spec_path="/spec.json"),
)


@pytest.fixture(scope="session")
def swagger_ui_basepath_app(app_class):
return build_app_from_fixture(
"simple",
app_class=app_class,
spec_file=BASEPATH_SLASH_SPEC,
nielsbox marked this conversation as resolved.
Show resolved Hide resolved
validate_responses=True,
swagger_ui_options=SwaggerUIOptions(spec_path="/spec.json"),
)
Expand Down
11 changes: 11 additions & 0 deletions tests/api/test_swagger_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
from connexion.spec import Specification


def test_simple(swagger_ui_app):
app_client = swagger_ui_app.test_client()
response = app_client.get("/v1.0/spec.json")
assert response.status_code == 200
Specification.from_dict(response.json())
Ruwann marked this conversation as resolved.
Show resolved Hide resolved


def test_basepath(swagger_ui_basepath_app):
app_client = swagger_ui_basepath_app.test_client()
response = app_client.get("/spec.json")
assert response.status_code == 200
Specification.from_dict(response.json())
2 changes: 2 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
SPEC_FOLDER = TEST_FOLDER / "fakeapi"
OPENAPI2_SPEC = "swagger.yaml"
OPENAPI3_SPEC = "openapi.yaml"
BASEPATH_SLASH_SPEC = "basepath-slash.yaml"
SPECS = [OPENAPI2_SPEC, OPENAPI3_SPEC]
SWAGGER_UI_SPECS = [OPENAPI2_SPEC, OPENAPI3_SPEC, BASEPATH_SLASH_SPEC]
nielsbox marked this conversation as resolved.
Show resolved Hide resolved
METHOD_VIEW_RESOLVERS = [MethodResolver, MethodViewResolver]
APP_CLASSES = [FlaskApp, AsyncApp]

Expand Down
Loading