Skip to content

Commit

Permalink
Fix memory leak in logging middleware (#1138)
Browse files Browse the repository at this point in the history
applied fix to pop values from state
  • Loading branch information
Goldziher committed Feb 3, 2023
1 parent d9cb8be commit 354942e
Show file tree
Hide file tree
Showing 34 changed files with 33 additions and 51 deletions.
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ repos:
- id: isort
exclude: "docs"
- repo: https://github.com/psf/black
rev: 22.12.0
rev: 23.1.0
hooks:
- id: black
args: [--config=./pyproject.toml]
Expand Down Expand Up @@ -86,7 +86,7 @@ repos:
- id: slotscheck
exclude: "test_*|docs"
- repo: https://github.com/pycqa/pylint
rev: "v2.16.0b1"
rev: "v2.16.0"
hooks:
- id: pylint
exclude: "tests|examples|test_apps|docs|tools"
Expand Down Expand Up @@ -158,7 +158,7 @@ repos:
jsbeautifier,
]
- repo: https://github.com/RobertCraigie/pyright-python
rev: v1.1.291
rev: v1.1.292
hooks:
- id: pyright
exclude: "test_apps|tools|docs"
Expand Down
1 change: 0 additions & 1 deletion docs/examples/tests/test_startup_and_shutdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class FakeAsyncEngine:


async def test_startup_and_shutdown_example(monkeypatch: "MonkeyPatch") -> None:

monkeypatch.setattr(startup_and_shutdown, "create_async_engine", MagicMock(return_value=FakeAsyncEngine))

@get("/")
Expand Down
13 changes: 10 additions & 3 deletions poetry.lock

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

1 change: 0 additions & 1 deletion starlite/asgi/routing_trie/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ def add_route_to_trie(

else:
for component in route.path_components:

if isinstance(component, PathParameterDefinition):
current_node.is_path_param_node = True
next_node_key: Union[Type[PathParameterSentinel], str] = PathParameterSentinel
Expand Down
1 change: 0 additions & 1 deletion starlite/asgi/routing_trie/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from typing import TYPE_CHECKING, Dict, Literal, NamedTuple, Set, Tuple, Type, Union

if TYPE_CHECKING:

from starlite.types import ASGIApp, Method, RouteHandlerType
from starlite.types.internal_types import PathParameterDefinition

Expand Down
1 change: 0 additions & 1 deletion starlite/connection/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from starlite.types.empty import Empty

if TYPE_CHECKING:

from typing import NoReturn

from pydantic import BaseModel
Expand Down
1 change: 0 additions & 1 deletion starlite/connection/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from starlite.utils.serialization import decode_json, decode_msgpack

if TYPE_CHECKING:

from starlite.handlers.http import HTTPRouteHandler # noqa: F401
from starlite.types.asgi_types import HTTPScope, Method, Receive, Scope, Send

Expand Down
1 change: 0 additions & 1 deletion starlite/middleware/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
)

if TYPE_CHECKING:

from starlite.types import ASGIApp, Receive, Scope, Scopes, Send


Expand Down
1 change: 0 additions & 1 deletion starlite/middleware/cors.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ async def wrapped_send(message: "Message") -> None:
if (self.config.is_allow_all_origins and has_cookie) or (
not self.config.is_allow_all_origins and self.config.is_origin_allowed(origin=origin)
):

headers["Access-Control-Allow-Origin"] = origin
headers["Vary"] = "Origin"

Expand Down
7 changes: 3 additions & 4 deletions starlite/middleware/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ def log_message(self, values: Dict[str, Any]) -> None:
"""
message = values.pop("message")
if self.is_struct_logger:

self.logger.info(message, **values)
else:
self.logger.info(f"{message}: " + ", ".join([f"{key}={value}" for key, value in values.items()]))
Expand Down Expand Up @@ -193,11 +192,11 @@ def extract_response_data(self, scope: "Scope") -> Dict[str, Any]:
serializer = get_serializer_from_scope(scope) or default_serializer
extracted_data = self.response_extractor(
messages=(
get_starlite_scope_state(scope, HTTP_RESPONSE_START),
get_starlite_scope_state(scope, HTTP_RESPONSE_BODY),
get_starlite_scope_state(scope, HTTP_RESPONSE_START, pop=True),
get_starlite_scope_state(scope, HTTP_RESPONSE_BODY, pop=True),
),
)
response_body_compressed = get_starlite_scope_state(scope, SCOPE_STATE_RESPONSE_COMPRESSED)
response_body_compressed = get_starlite_scope_state(scope, SCOPE_STATE_RESPONSE_COMPRESSED, default=False)
for key in self.config.response_log_fields:
value: Any
value = extracted_data.get(key)
Expand Down
1 change: 0 additions & 1 deletion starlite/middleware/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from starlite.exceptions import ImproperlyConfiguredException

if TYPE_CHECKING:

from starlite.types import Scope, Scopes


Expand Down
1 change: 0 additions & 1 deletion starlite/multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ def parse_multipart_form(body: bytes, boundary: bytes) -> Dict[str, Any]:
fields: DefaultDict[str, List[Any]] = defaultdict(list)

if body and boundary:

form_parts = body.split(boundary)
for form_part in form_parts[1:-1]:
file_name = None
Expand Down
1 change: 0 additions & 1 deletion starlite/openapi/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from starlite.utils.serialization import encode_json

if TYPE_CHECKING:

from pydantic_openapi_schema.v3_1_0.open_api import OpenAPI

MSG_OPENAPI_NOT_INITIALIZED = "Starlite has not been instantiated with OpenAPIConfig"
Expand Down
2 changes: 0 additions & 2 deletions starlite/openapi/responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
from starlite.utils import get_enum_string_value, get_name, is_class_and_subclass

if TYPE_CHECKING:

from pydantic_openapi_schema.v3_1_0.responses import Responses

from starlite.datastructures.cookie import Cookie
Expand Down Expand Up @@ -199,7 +198,6 @@ def create_additional_responses(
return

for status_code, additional_response in route_handler.responses.items():

schema = create_schema(
field=SignatureField.create(field_type=additional_response.model),
generate_examples=additional_response.generate_examples,
Expand Down
1 change: 0 additions & 1 deletion starlite/openapi/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from starlite.openapi.enums import OpenAPIType

if TYPE_CHECKING:

from starlite.signature.models import SignatureField

CAPITAL_LETTERS_PATTERN = re.compile(r"(?=[A-Z])")
Expand Down
1 change: 0 additions & 1 deletion starlite/plugins/sql_alchemy/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
raise MissingDependencyException("sqlalchemy is not installed") from e

if TYPE_CHECKING:

from starlite.types import Message, Scope

IsolationLevel = Literal["AUTOCOMMIT", "READ COMMITTED", "READ UNCOMMITTED", "REPEATABLE READ", "SERIALIZABLE"]
Expand Down
1 change: 0 additions & 1 deletion starlite/response/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
)

if TYPE_CHECKING:

from starlite.datastructures import BackgroundTask, BackgroundTasks
from starlite.types import (
HTTPResponseBodyEvent,
Expand Down
1 change: 0 additions & 1 deletion starlite/response/redirect.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from starlite.status_codes import HTTP_307_TEMPORARY_REDIRECT

if TYPE_CHECKING:

from starlite.datastructures import BackgroundTask, BackgroundTasks
from starlite.types import ResponseCookies

Expand Down
1 change: 0 additions & 1 deletion starlite/static_files/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from starlite.utils.file import FileSystemAdapter

if TYPE_CHECKING:

from starlite.types import Receive, Scope, Send
from starlite.types.composite_types import PathType
from starlite.types.file_types import FileInfo, FileSystemProtocol
Expand Down
1 change: 0 additions & 1 deletion starlite/testing/websocket_test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from starlite.utils.serialization import decode_json, encode_json

if TYPE_CHECKING:

from starlite.testing.client.sync_client import TestClient
from starlite.types import (
WebSocketConnectEvent,
Expand Down
1 change: 0 additions & 1 deletion starlite/types/builtin_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from typing import TYPE_CHECKING

if TYPE_CHECKING:

from typing import Type, Union

from pydantic_factories.protocols import DataclassProtocol
Expand Down
1 change: 0 additions & 1 deletion starlite/utils/predicates.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
UNION_TYPES = {Union}

if TYPE_CHECKING:

from starlite.types.builtin_types import (
DataclassClass,
DataclassClassOrInstance,
Expand Down
10 changes: 4 additions & 6 deletions starlite/utils/scope.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from typing import TYPE_CHECKING, Any, Optional

from starlite.constants import SCOPE_STATE_NAMESPACE
from starlite.types import Empty

if TYPE_CHECKING:
from starlite.types import Scope, Serializer
Expand Down Expand Up @@ -33,7 +32,7 @@ def get_serializer_from_scope(scope: "Scope") -> Optional["Serializer"]:
return None


def get_starlite_scope_state(scope: "Scope", key: str, default: Any = Empty) -> Any:
def get_starlite_scope_state(scope: "Scope", key: str, default: Any = None, pop: bool = False) -> Any:
"""Get an internal value from connection scope state.
Note:
Expand All @@ -46,15 +45,14 @@ def get_starlite_scope_state(scope: "Scope", key: str, default: Any = Empty) ->
Args:
scope: The connection scope.
key: Key to get from internal namespace in scope state.
default: Value set in internal namespace and returned if ``key`` doesn't exist.
default: Default value to return.
pop: Boolean flag dictating whether the value should be deleted from the state.
Returns:
Value mapped to ``key`` in internal connection scope namespace.
"""
namespace = scope["state"].setdefault(SCOPE_STATE_NAMESPACE, {})
if default is Empty:
return namespace.get(key)
return namespace.setdefault(key, default)
return namespace.get(key, default) if not pop else namespace.pop(key, default)


def set_starlite_scope_state(scope: "Scope", key: str, value: Any) -> None:
Expand Down
2 changes: 0 additions & 2 deletions tests/cli/test_run_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ def test_run_command_with_autodiscover_app_factory(
patch_autodiscovery_paths: Callable[[List[str]], None],
create_app_file: CreateAppFileFixture,
) -> None:

patch_autodiscovery_paths([file_name])
path = create_app_file(file_name, content=file_content)
result = runner.invoke(cli_command, "run")
Expand All @@ -122,7 +121,6 @@ def test_run_command_with_app_factory(
mock_uvicorn_run: MagicMock,
create_app_file: CreateAppFileFixture,
) -> None:

path = create_app_file("_create_app_with_path.py", content=CREATE_APP_FILE_CONTENT)
app_path = f"{path.stem}:create_app"
result = runner.invoke(cli_command, ["--app", app_path, "run"])
Expand Down
1 change: 0 additions & 1 deletion tests/connection/request/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ def handler(request: Request[Any, Any]) -> dict:

def test_request_cookies() -> None:
async def app(scope: "Scope", receive: "Receive", send: "Send") -> None:

request = Request[Any, Any](scope, receive)
mycookie = request.cookies.get("mycookie")
if mycookie:
Expand Down
1 change: 0 additions & 1 deletion tests/connection/websocket/test_websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from starlite.testing import TestClient, create_test_client

if TYPE_CHECKING:

from starlite.types import Receive, Scope, Send


Expand Down
1 change: 0 additions & 1 deletion tests/contrib/jwt/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from tests import User, UserFactory

if TYPE_CHECKING:

from starlite.cache import SimpleCacheBackend


Expand Down
1 change: 0 additions & 1 deletion tests/kwargs/test_multipart_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,5 @@ async def hello_world(data: Optional[UploadFile] = Body(media_type=RequestEncodi
await data.read()

with create_test_client(route_handlers=[hello_world]) as client:

response = client.post("/")
assert response.status_code == HTTP_201_CREATED
1 change: 0 additions & 1 deletion tests/middleware/test_base_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from starlite.testing import create_test_client

if TYPE_CHECKING:

from starlite.types import Message, Receive, Scope, Send


Expand Down
1 change: 0 additions & 1 deletion tests/response/test_serialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ def test_response_serialization_structured_types(content: Any, response_type: An
elif isinstance(value, dict) and "path" in value:
assert content.__class__(**value)["path"] == str(content["path"])
elif isinstance(value, dict):

assert content.__class__(**value) == content
else:
assert [content[0].__class__(**value[0])] == content
Expand Down
1 change: 0 additions & 1 deletion tests/template/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from starlite.testing import create_test_client

if TYPE_CHECKING:

from starlite.template import TemplateEngineProtocol


Expand Down
4 changes: 1 addition & 3 deletions tests/testing/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from tests import Pet, PetFactory

if TYPE_CHECKING:

from starlite.middleware.session.base import (
BaseBackendConfig,
ServerSideSessionConfig,
Expand Down Expand Up @@ -84,7 +83,7 @@ def test_request_factory_build_headers() -> None:

assert len(built_headers) == len(headers.keys())

for (key, value) in built_headers:
for key, value in built_headers:
decoded_key = key.decode("latin1")
decoded_value = value.decode("latin1")
assert decoded_key in headers
Expand Down Expand Up @@ -259,7 +258,6 @@ def test_test_client_set_session_data(
session_config: "BaseBackendConfig",
test_client_backend: "AnyIOBackend",
) -> None:

session_data = {"foo": "bar"}

if with_domain:
Expand Down
1 change: 0 additions & 1 deletion tests/utils/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from starlite.utils.file import BaseLocalFileSystem, FileSystemAdapter

if TYPE_CHECKING:

from starlite.types import FileSystemProtocol


Expand Down
Loading

0 comments on commit 354942e

Please sign in to comment.