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

fix: resource warnings #3838

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
14 changes: 2 additions & 12 deletions litestar/connection/request.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import warnings
from typing import TYPE_CHECKING, Any, AsyncGenerator, Generic
from typing import TYPE_CHECKING, Any, AsyncGenerator, Generic, cast

from litestar._multipart import parse_content_header, parse_multipart_form
from litestar._parsers import parse_url_encoded_form_data
Expand Down Expand Up @@ -222,17 +222,7 @@ async def form(self) -> FormMultiDict:

self._connection_state.form = form_data

# form_data is a dict[str, list[str] | str | UploadFile]. Convert it to a
# list[tuple[str, str | UploadFile]] before passing it to FormMultiDict so
# multi-keys can be accessed properly
items = []
for k, v in form_data.items():
if isinstance(v, list):
for sv in v:
items.append((k, sv))
else:
items.append((k, v))
self._form = FormMultiDict(items)
self._form = FormMultiDict.from_form_data(cast("dict[str, Any]", form_data))

return self._form

Expand Down
21 changes: 21 additions & 0 deletions litestar/datastructures/multi_dicts.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,27 @@ def copy(self) -> Self: # type: ignore[override]
class FormMultiDict(ImmutableMultiDict[Any]):
"""MultiDict for form data."""

@classmethod
def from_form_data(cls, form_data: dict[str, list[str] | str | UploadFile]) -> FormMultiDict:
"""Create a FormMultiDict from form data.

Args:
form_data: Form data to create the FormMultiDict from.

Returns:
A FormMultiDict instance
"""
# Convert form_data to a list[tuple[str, str | UploadFile]] before passing it
# to FormMultiDict so multi-keys can be accessed properly
items = []
for k, v in form_data.items():
if not isinstance(v, list):
items.append((k, v))
else:
for sv in v:
items.append((k, sv))
return cls(items)

async def close(self) -> None:
"""Close all files in the multi-dict.

Expand Down
2 changes: 2 additions & 0 deletions litestar/datastructures/upload_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@
Returns:
None.
"""
if self.file.closed:
return None

Check warning on line 98 in litestar/datastructures/upload_file.py

View check run for this annotation

Codecov / codecov/patch

litestar/datastructures/upload_file.py#L98

Added line #L98 was not covered by tests
if self.rolled_to_disk:
return await sync_to_thread(self.file.close)
return self.file.close()
Expand Down
12 changes: 4 additions & 8 deletions litestar/routes/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from msgspec.msgpack import decode as _decode_msgpack_plain

from litestar.datastructures.upload_file import UploadFile
from litestar.datastructures.multi_dicts import FormMultiDict
from litestar.enums import HttpMethod, MediaType, ScopeType
from litestar.exceptions import ClientException, ImproperlyConfiguredException, SerializationException
from litestar.handlers.http_handlers import HTTPRouteHandler
Expand Down Expand Up @@ -86,8 +86,10 @@ async def handle(self, scope: HTTPScope, receive: Receive, send: Send) -> None:
if after_response_handler := route_handler.resolve_after_response():
await after_response_handler(request)

if request._form is not Empty:
await request._form.close()
if form_data := scope.get("_form", {}):
await self._cleanup_temporary_files(form_data=cast("dict[str, Any]", form_data))
await FormMultiDict.from_form_data(cast("dict[str, Any]", form_data)).close()

def create_handler_map(self) -> None:
"""Parse the ``router_handlers`` of this route and return a mapping of
Expand Down Expand Up @@ -258,9 +260,3 @@ def options_handler(scope: Scope) -> Response:
include_in_schema=False,
sync_to_thread=False,
)(options_handler)

@staticmethod
async def _cleanup_temporary_files(form_data: dict[str, Any]) -> None:
for v in form_data.values():
if isinstance(v, UploadFile) and not v.file.closed:
await v.close()
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ fail_under = 50
addopts = "--strict-markers --strict-config --dist=loadgroup -m 'not server_integration'"
asyncio_mode = "auto"
filterwarnings = [
"error",
# https://github.com/pytest-dev/pytest-asyncio/issues/724
"default:.*socket.socket:pytest.PytestUnraisableExceptionWarning",
"ignore::trio.TrioDeprecationWarning:anyio._backends._trio*:",
"ignore::DeprecationWarning:pkg_resources.*",
"ignore::DeprecationWarning:google.rpc",
Expand Down
8 changes: 2 additions & 6 deletions tests/e2e/test_routing/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import subprocess
import time
from pathlib import Path
from typing import Callable, List
Expand All @@ -16,16 +15,13 @@ def runner(app: str, server_command: List[str]) -> None:
tmp_path.joinpath("app.py").write_text(app)
monkeypatch.chdir(tmp_path)

proc = psutil.Popen(
server_command,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
)
proc = psutil.Popen(server_command)

def kill() -> None:
for child in proc.children(recursive=True):
child.kill()
proc.kill()
proc.wait()

request.addfinalizer(kill)

Expand Down
14 changes: 7 additions & 7 deletions tests/unit/test_datastructures/test_multi_dicts.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from __future__ import annotations

from unittest.mock import patch

import pytest
from pytest_mock import MockerFixture

from litestar.datastructures import UploadFile
from litestar.datastructures.multi_dicts import FormMultiDict, ImmutableMultiDict, MultiDict
Expand Down Expand Up @@ -34,20 +35,19 @@ def test_immutable_multi_dict_as_mutable() -> None:
assert multi.mutable_copy().dict() == MultiDict(data).dict()


async def test_form_multi_dict_close(mocker: MockerFixture) -> None:
close = mocker.patch("litestar.datastructures.multi_dicts.UploadFile.close")

async def test_form_multi_dict_close() -> None:
multi = FormMultiDict(
[
("foo", UploadFile(filename="foo", content_type="text/plain")),
("bar", UploadFile(filename="foo", content_type="text/plain")),
]
)

with patch("litestar.datastructures.multi_dicts.UploadFile.close") as mock_close:
await multi.close()
assert mock_close.call_count == 2
# calls the real UploadFile.close method to clean up
await multi.close()

assert close.call_count == 2


@pytest.mark.parametrize("type_", [MultiDict, ImmutableMultiDict])
def test_copy(type_: type[MultiDict | ImmutableMultiDict]) -> None:
Expand Down
Loading