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

Update handling of status code <100, 204 or 304 #353

Merged
merged 3 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ While supporting function based route handlers, Starlite also supports and promo
controllers:

```python title="my_app/controllers/user.py"
from typing import List, Optional
from typing import List, Optional, NoReturn

from pydantic import UUID4
from starlite import Controller, Partial, get, post, put, patch, delete
Expand Down Expand Up @@ -140,7 +140,7 @@ class UserController(Controller):
...

@delete(path="/{user_id:uuid}")
async def delete_user(self, user_id: UUID4) -> User:
async def delete_user(self, user_id: UUID4) -> NoReturn:
...
```

Expand Down
4 changes: 2 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class User:
**Define a Controller** for your data model:

```python title="my_app/controllers/user.py"
from typing import List
from typing import List, NoReturn

from pydantic import UUID4
from starlite import Controller, Partial, get, post, put, patch, delete
Expand Down Expand Up @@ -134,7 +134,7 @@ class UserController(Controller):
...

@delete(path="/{user_id:uuid}")
async def delete_user(self, user_id: UUID4) -> User:
async def delete_user(self, user_id: UUID4) -> NoReturn:
...
```

Expand Down
3 changes: 2 additions & 1 deletion docs/usage/1-routing/3-controllers.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ from pydantic import BaseModel, UUID4
from starlite.controller import Controller
from starlite.handlers import get, post, patch, delete
from starlite.types import Partial
from typing import NoReturn


class UserOrder(BaseModel):
Expand All @@ -34,7 +35,7 @@ class UserOrderController(Controller):
...

@delete(path="/{order_id:uuid}")
async def delete_user_order(self, order_id: UUID4) -> UserOrder:
async def delete_user_order(self, order_id: UUID4) -> NoReturn:
...
```

Expand Down
5 changes: 5 additions & 0 deletions docs/usage/5-responses/2-status-codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ If `status_code` is not set by the user, the following defaults are used:
- DELETE: 204 (No Content)
- GET, PATCH, PUT: 200 (Ok)

<!-- prettier-ignore -->
!!! important
For status codes < 100 or 204, 304 statuses, no response body is allowed. If you specify a return annotation other
than `typing.NoReturn` or `None` in those cases, an `ImproperlyConfiguredException` will be raised.

<!-- prettier-ignore -->
!!! note
When using the `route` decorator with multiple http methods, the default status code is `200`.
Expand Down
68 changes: 34 additions & 34 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 16 additions & 4 deletions starlite/handlers/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@
from contextlib import suppress
from enum import Enum
from inspect import Signature, isawaitable, isclass
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type, Union, cast
from typing import TYPE_CHECKING, Any, Dict, List, NoReturn, Optional, Type, Union, cast

from pydantic import validate_arguments
from starlette.responses import Response as StarletteResponse
from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT
from starlette.status import (
HTTP_200_OK,
HTTP_201_CREATED,
HTTP_204_NO_CONTENT,
HTTP_304_NOT_MODIFIED,
)

from starlite.constants import REDIRECT_STATUS_CODES
from starlite.datastructures import (
Expand Down Expand Up @@ -407,9 +412,16 @@ def _validate_handler_function(self) -> None:
signature = Signature.from_callable(cast("AnyCallable", self.fn))
return_annotation = signature.return_annotation
if return_annotation is Signature.empty:
raise ValidationException(
raise ImproperlyConfiguredException(
"A return value of a route handler function should be type annotated."
"If your function doesn't return a value or returns None, annotate it as returning None."
"If your function doesn't return a value or returns None, annotate it as returning 'NoReturn' or 'None' respectively."
)
if (
self.status_code < 200 or self.status_code in {HTTP_204_NO_CONTENT, HTTP_304_NOT_MODIFIED}
) and return_annotation not in [NoReturn, None]:
raise ImproperlyConfiguredException(
"A status code 204, 304 or in the range below 200 does not support a response body."
"If the function should return a value, change the route handler status code to an appropriate value.",
)
if isclass(return_annotation):
with suppress(TypeError):
Expand Down
9 changes: 7 additions & 2 deletions starlite/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
Dict,
Generic,
List,
NoReturn,
Optional,
TypeVar,
Union,
Expand All @@ -15,7 +16,7 @@
from pydantic import BaseModel
from pydantic_openapi_schema.v3_1_0.open_api import OpenAPI
from starlette.responses import Response as StarletteResponse
from starlette.status import HTTP_204_NO_CONTENT
from starlette.status import HTTP_204_NO_CONTENT, HTTP_304_NOT_MODIFIED

from starlite.enums import MediaType, OpenAPIMediaType
from starlite.exceptions import ImproperlyConfiguredException
Expand Down Expand Up @@ -87,7 +88,11 @@ def render(self, content: Any) -> bytes:
An encoded bytes string
"""
try:
if content is None and self.status_code == HTTP_204_NO_CONTENT:
if (
content is None
or content is NoReturn
and (self.status_code < 100 or self.status_code in {HTTP_204_NO_CONTENT, HTTP_304_NOT_MODIFIED})
):
return b""
if self.media_type == MediaType.JSON:
return dumps(content, default=self.serializer, option=OPT_SERIALIZE_NUMPY | OPT_OMIT_MICROSECONDS)
Expand Down
9 changes: 7 additions & 2 deletions tests/handlers/http/test_delete.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
from typing import Any, NoReturn

import pytest

from starlite import delete
from starlite.testing import create_test_client


def test_handler_return_none_and_204_status_response_empty() -> None:
@pytest.mark.parametrize("return_annotation", (None, NoReturn))
def test_handler_return_none_and_204_status_response_empty(return_annotation: Any) -> None:
@delete(path="/")
async def route() -> None:
async def route() -> return_annotation: # type: ignore[valid-type]
return None

with create_test_client(route_handlers=[route]) as client:
Expand Down
28 changes: 26 additions & 2 deletions tests/handlers/http/test_validations.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import pytest
from pydantic import ValidationError
from starlette.status import HTTP_200_OK, HTTP_307_TEMPORARY_REDIRECT
from starlette.status import (
HTTP_100_CONTINUE,
HTTP_200_OK,
HTTP_304_NOT_MODIFIED,
HTTP_307_TEMPORARY_REDIRECT,
)

from starlite import (
File,
Expand All @@ -9,6 +14,7 @@
Redirect,
Response,
WebSocket,
delete,
get,
route,
)
Expand Down Expand Up @@ -49,7 +55,7 @@ class SpecialResponse(Response):

@pytest.mark.asyncio()
async def test_function_validation() -> None:
with pytest.raises(ValidationException):
with pytest.raises(ImproperlyConfiguredException):

@get(path="/")
def method_with_no_annotation(): # type: ignore
Expand All @@ -61,6 +67,24 @@ def method_with_no_annotation(): # type: ignore
def redirect_method_without_proper_status() -> Redirect:
pass

with pytest.raises(ImproperlyConfiguredException):

@delete(path="/")
def method_with_no_content() -> dict:
pass

with pytest.raises(ImproperlyConfiguredException):

@get(path="/", status_code=HTTP_304_NOT_MODIFIED)
def method_with_not_modified() -> dict:
pass

with pytest.raises(ImproperlyConfiguredException):

@get(path="/", status_code=HTTP_100_CONTINUE)
def method_with_status_lower_than_200() -> dict:
pass

@get(path="/", status_code=HTTP_307_TEMPORARY_REDIRECT)
def redirect_method() -> Redirect:
return Redirect("/test") # type: ignore
Expand Down
17 changes: 9 additions & 8 deletions tests/test_controller.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Type, Union
from typing import Any, NoReturn, Type, Union

import pytest
from starlette.status import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT
Expand All @@ -10,19 +10,20 @@


@pytest.mark.parametrize(
"decorator, http_method, expected_status_code",
"decorator, http_method, expected_status_code, return_annotation",
[
(get, HttpMethod.GET, HTTP_200_OK),
(post, HttpMethod.POST, HTTP_201_CREATED),
(put, HttpMethod.PUT, HTTP_200_OK),
(patch, HttpMethod.PATCH, HTTP_200_OK),
(delete, HttpMethod.DELETE, HTTP_204_NO_CONTENT),
(get, HttpMethod.GET, HTTP_200_OK, Person),
(post, HttpMethod.POST, HTTP_201_CREATED, Person),
(put, HttpMethod.PUT, HTTP_200_OK, Person),
(patch, HttpMethod.PATCH, HTTP_200_OK, Person),
(delete, HttpMethod.DELETE, HTTP_204_NO_CONTENT, NoReturn),
],
)
def test_controller_http_method(
decorator: Union[Type[get], Type[post], Type[put], Type[patch], Type[delete]],
http_method: HttpMethod,
expected_status_code: int,
return_annotation: Any,
) -> None:
test_path = "/person"
person_instance = PersonFactory.build()
Expand All @@ -31,7 +32,7 @@ class MyController(Controller):
path = test_path

@decorator()
def test_method(self) -> Person:
def test_method(self) -> return_annotation: # type: ignore[valid-type]
return person_instance

with create_test_client(MyController) as client:
Expand Down