Skip to content

Commit

Permalink
fix(OpenAPI): Query-only properties included in path, cookie and head…
Browse files Browse the repository at this point in the history
…er parameter schema and response headers (#3908) (#3909)
  • Loading branch information
provinzkraut authored Dec 22, 2024
1 parent 3ce30b6 commit 11df596
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 21 deletions.
27 changes: 25 additions & 2 deletions litestar/datastructures/response_header.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import TYPE_CHECKING, Any

from litestar.exceptions import ImproperlyConfiguredException
from litestar.utils import warn_deprecation

if TYPE_CHECKING:
from litestar.openapi.spec import Example
Expand Down Expand Up @@ -46,7 +47,7 @@ class ResponseHeader:
Default value is `false`.
"""

allow_empty_value: bool = False
allow_empty_value: bool = None # type: ignore[assignment]
"""Sets the ability to pass empty-valued parameters. This is valid only for
`query` parameters and allows sending a parameter with an empty value.
Default value is `false`. If.
Expand Down Expand Up @@ -80,7 +81,7 @@ class ResponseHeader:
For all other styles, the default value is `false`.
"""

allow_reserved: bool = False
allow_reserved: bool = None # type: ignore[assignment]
"""Determines whether the parameter value SHOULD allow reserved characters,
as defined by.
Expand Down Expand Up @@ -121,5 +122,27 @@ def __post_init__(self) -> None:
if not self.documentation_only and self.value is None:
raise ImproperlyConfiguredException("value must be set if documentation_only is false")

if self.allow_reserved is None:
self.allow_reserved = False # type: ignore[unreachable]
else:
warn_deprecation(
"2.13.1",
"allow_reserved",
kind="parameter",
removal_in="4",
info="This property is invalid for headers and will be ignored",
)

if self.allow_empty_value is None:
self.allow_empty_value = False # type: ignore[unreachable]
else:
warn_deprecation(
"2.13.1",
"allow_empty_value",
kind="parameter",
removal_in="4",
info="This property is invalid for headers and will be ignored",
)

def __hash__(self) -> int:
return hash(self.name)
7 changes: 7 additions & 0 deletions litestar/openapi/spec/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def _normalize_value(value: Any) -> Any:
class BaseSchemaObject:
"""Base class for schema spec objects"""

@property
def _exclude_fields(self) -> set[str]:
return set()

def _iter_fields(self) -> Iterator[Field[Any]]:
yield from fields(self)

Expand All @@ -50,8 +54,11 @@ def to_schema(self) -> dict[str, Any]:
recursively.
"""
result: dict[str, Any] = {}
exclude = self._exclude_fields

for field in self._iter_fields():
if field.name in exclude:
continue
value = _normalize_value(getattr(self, field.name, None))

if value is not None:
Expand Down
40 changes: 31 additions & 9 deletions litestar/openapi/spec/header.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from __future__ import annotations

from dataclasses import Field, dataclass
from typing import TYPE_CHECKING, Any, Iterator, Literal

from typing_extensions import override
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, Literal

from litestar.openapi.spec.base import BaseSchemaObject
from litestar.utils import warn_deprecation

if TYPE_CHECKING:
from litestar.openapi.spec.example import Example
Expand Down Expand Up @@ -55,7 +54,7 @@ class OpenAPIHeader(BaseSchemaObject):
deprecated: bool = False
"""Specifies that a parameter is deprecated and SHOULD be transitioned out of usage. Default value is ``False``."""

allow_empty_value: bool = False
allow_empty_value: bool = None # type: ignore[assignment]
"""Sets the ability to pass empty-valued parameters. This is valid only for ``query`` parameters and allows sending
a parameter with an empty value. Default value is ``False``. If
`style <https://spec.openapis.org/oas/v3.1.0#parameterStyle>`__ is used, and if behavior is ``n/a`` (cannot be
Expand Down Expand Up @@ -87,7 +86,7 @@ class OpenAPIHeader(BaseSchemaObject):
other styles, the default value is ``False``.
"""

allow_reserved: bool = False
allow_reserved: bool = None # type: ignore[assignment]
"""Determines whether the parameter value SHOULD allow reserved characters, as defined by. :rfc:`3986`
(``:/?#[]@!$&'()*+,;=``) to be included without percent-encoding.
Expand Down Expand Up @@ -122,6 +121,29 @@ class OpenAPIHeader(BaseSchemaObject):
The key is the media type and the value describes it. The map MUST only contain one entry.
"""

@override
def _iter_fields(self) -> Iterator[Field[Any]]:
yield from (f for f in super()._iter_fields() if f.name not in {"name", "param_in"})
@property
def _exclude_fields(self) -> set[str]:
return {"name", "param_in", "allow_reserved", "allow_empty_value"}

def __post_init__(self) -> None:
if self.allow_reserved is None:
self.allow_reserved = False # type: ignore[unreachable]
else:
warn_deprecation(
"2.13.1",
"allow_reserved",
kind="parameter",
removal_in="4",
info="This property is invalid for headers and will be ignored",
)

if self.allow_empty_value is None:
self.allow_empty_value = False # type: ignore[unreachable]
else:
warn_deprecation(
"2.13.1",
"allow_empty_value",
kind="parameter",
removal_in="4",
info="This property is invalid for headers and will be ignored",
)
9 changes: 9 additions & 0 deletions litestar/openapi/spec/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,12 @@ class Parameter(BaseSchemaObject):
The key is the media type and the value describes it. The map MUST only contain one entry.
"""

@property
def _exclude_fields(self) -> set[str]:
exclude = set()
if self.param_in != "query":
# these are only allowed in query params
exclude.update({"allow_empty_value", "allow_reserved"})

return exclude
2 changes: 0 additions & 2 deletions tests/e2e/test_openapi/test_spec_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ def test_included_header_fields() -> None:

assert app1.openapi_schema.to_schema()["paths"]["/"]["get"]["responses"]["200"]["headers"] == {
"X-Version": {
"allowEmptyValue": False,
"allowReserved": False,
"deprecated": False,
"description": "Test",
"required": False,
Expand Down
4 changes: 0 additions & 4 deletions tests/unit/test_openapi/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@ def test_merged_components_correct() -> None:
"one": {
"required": False,
"deprecated": False,
"allowEmptyValue": False,
"allowReserved": False,
},
"two": {
"required": False,
"deprecated": False,
"allowEmptyValue": False,
"allowReserved": False,
},
},
}
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/test_openapi/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,3 +445,33 @@ async def handler(

testmodel_schema_name = app.openapi_schema.paths["/"].get.parameters[0].schema.value # type: ignore[index, union-attr]
assert app.openapi_schema.components.schemas[testmodel_schema_name].properties["param"].type == OpenAPIType.STRING # type: ignore[index, union-attr]


def test_query_param_only_properties() -> None:
# https://github.com/litestar-org/litestar/issues/3908
@get("/{path_param:str}")
def handler(
path_param: str,
query_param: str,
header_param: Annotated[str, Parameter(header="header_param")],
cookie_param: Annotated[str, Parameter(cookie="cookie_param")],
) -> None:
pass

app = Litestar([handler])
params = {p.name: p for p in app.openapi_schema.paths["/{path_param}"].get.parameters} # type: ignore[union-attr, index]

for key in ["path_param", "header_param", "cookie_param"]:
schema = params[key].to_schema()
assert "allowEmptyValue" not in schema
assert "allowReserved" not in schema

assert params["query_param"].to_schema() == {
"name": "query_param",
"in": "query",
"schema": {"type": "string"},
"required": True,
"deprecated": False,
"allowEmptyValue": False,
"allowReserved": False,
}
32 changes: 28 additions & 4 deletions tests/unit/test_openapi/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,12 @@ def handler() -> list:

assert isinstance(response.headers, dict)
assert isinstance(response.headers["special-header"], OpenAPIHeader)
assert response.headers["special-header"].description == "super-duper special"
headers_schema = response.headers["special-header"].schema
assert isinstance(headers_schema, Schema)
assert headers_schema.type == OpenAPIType.STRING
assert response.headers["special-header"].to_schema() == {
"schema": {"type": "string"},
"description": "super-duper special",
"required": False,
"deprecated": False,
}


def test_create_success_response_with_cookies(create_factory: CreateFactoryFixture) -> None:
Expand Down Expand Up @@ -538,3 +540,25 @@ def handler() -> File:

response = create_factory(handler).create_success_response()
assert next(iter(response.content.values())).schema.content_media_type == expected # type: ignore[union-attr]


def test_response_header_deprecated_properties() -> None:
assert ResponseHeader(name="foo", value="bar").allow_empty_value is False
assert ResponseHeader(name="foo", value="bar").allow_reserved is False

with pytest.warns(DeprecationWarning, match="property is invalid for headers"):
ResponseHeader(name="foo", value="bar", allow_empty_value=True)

with pytest.warns(DeprecationWarning, match="property is invalid for headers"):
ResponseHeader(name="foo", value="bar", allow_reserved=True)


def test_header_deprecated_properties() -> None:
assert OpenAPIHeader().allow_empty_value is False
assert OpenAPIHeader().allow_reserved is False

with pytest.warns(DeprecationWarning, match="property is invalid for headers"):
OpenAPIHeader(allow_empty_value=True)

with pytest.warns(DeprecationWarning, match="property is invalid for headers"):
OpenAPIHeader(allow_reserved=True)

0 comments on commit 11df596

Please sign in to comment.