Add documentation and fix type safety bugs
This commit is contained in:
parent
2579ce9f18
commit
23da63f6d1
4 changed files with 82 additions and 5 deletions
34
ISSUE.md
Normal file
34
ISSUE.md
Normal file
|
|
@ -0,0 +1,34 @@
|
||||||
|
# Bug report
|
||||||
|
|
||||||
|
**Describe the bug**
|
||||||
|
|
||||||
|
The recent refactor introduced a missing type safety and potential runtime error:
|
||||||
|
- `remove_ctx` and `add_ctx` lacked documentation, making their purpose unclear.
|
||||||
|
- `get_send_file_max_age` returned a value without proper type casting, triggering `type: ignore` comments.
|
||||||
|
- The static file route used a lambda referencing a weakref; if the app was garbage‑collected it could raise an obscure error.
|
||||||
|
- `raise_routing_exception` raised `request.routing_exception` without guaranteeing it was not `None`, leading to a possible `TypeError`.
|
||||||
|
|
||||||
|
These issues manifested as type‑checking failures and potential crashes when serving static files or handling routing exceptions.
|
||||||
|
|
||||||
|
**Steps to reproduce**
|
||||||
|
|
||||||
|
1. Run the test suite (`pytest`).
|
||||||
|
2. Observe `type: ignore` warnings and potential failures in `test_regression.py` when static files are accessed.
|
||||||
|
3. Manually trigger a routing exception (e.g., abort with a redirect) and notice that `raise_routing_exception` may raise `None`.
|
||||||
|
4. Access a static file after the app has been garbage‑collected (unlikely in normal use but possible in long‑running processes).
|
||||||
|
|
||||||
|
**Expected behavior**
|
||||||
|
|
||||||
|
- Functions should have clear docstrings.
|
||||||
|
- `get_send_file_max_age` should return an `int` or `None` with proper type casting.
|
||||||
|
- The static file view should raise a clear `RuntimeError` if the app is unavailable.
|
||||||
|
- `raise_routing_exception` should assert the exception exists before raising.
|
||||||
|
|
||||||
|
**Environment**
|
||||||
|
|
||||||
|
- Python version: 3.12
|
||||||
|
- Flask version: 3.2.0.dev
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
*This issue was created automatically using the repository's issue template.*
|
||||||
|
|
@ -70,6 +70,11 @@ T_template_test = t.TypeVar("T_template_test", bound=ft.TemplateTestCallable)
|
||||||
|
|
||||||
|
|
||||||
def _make_timedelta(value: timedelta | int | None) -> timedelta | None:
|
def _make_timedelta(value: timedelta | int | None) -> timedelta | None:
|
||||||
|
"""Coerce the value to a timedelta.
|
||||||
|
|
||||||
|
:param value: The value to coerce. Can be a timedelta, an integer
|
||||||
|
(seconds), or None.
|
||||||
|
"""
|
||||||
if value is None or isinstance(value, timedelta):
|
if value is None or isinstance(value, timedelta):
|
||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
@ -82,6 +87,11 @@ F = t.TypeVar("F", bound=t.Callable[..., t.Any])
|
||||||
# Other methods may call the overridden method with the new ctx arg. Remove it
|
# Other methods may call the overridden method with the new ctx arg. Remove it
|
||||||
# and call the method with the remaining args.
|
# and call the method with the remaining args.
|
||||||
def remove_ctx(f: F) -> F:
|
def remove_ctx(f: F) -> F:
|
||||||
|
"""Decorator that removes the 'ctx' argument from the arguments list.
|
||||||
|
|
||||||
|
This is used when a method signature has been updated to take 'ctx',
|
||||||
|
but the overridden method in a subclass has not.
|
||||||
|
"""
|
||||||
def wrapper(self: Flask, *args: t.Any, **kwargs: t.Any) -> t.Any:
|
def wrapper(self: Flask, *args: t.Any, **kwargs: t.Any) -> t.Any:
|
||||||
if args and isinstance(args[0], AppContext):
|
if args and isinstance(args[0], AppContext):
|
||||||
args = args[1:]
|
args = args[1:]
|
||||||
|
|
@ -94,6 +104,11 @@ def remove_ctx(f: F) -> F:
|
||||||
# The overridden method may call super().base_method without the new ctx arg.
|
# The overridden method may call super().base_method without the new ctx arg.
|
||||||
# Add it to the args for the call.
|
# Add it to the args for the call.
|
||||||
def add_ctx(f: F) -> F:
|
def add_ctx(f: F) -> F:
|
||||||
|
"""Decorator that adds the current context to the arguments list.
|
||||||
|
|
||||||
|
This is used when a method calls a super method that expects 'ctx',
|
||||||
|
but the subclass method does not provide it.
|
||||||
|
"""
|
||||||
def wrapper(self: Flask, *args: t.Any, **kwargs: t.Any) -> t.Any:
|
def wrapper(self: Flask, *args: t.Any, **kwargs: t.Any) -> t.Any:
|
||||||
if not args:
|
if not args:
|
||||||
args = (app_ctx._get_current_object(),)
|
args = (app_ctx._get_current_object(),)
|
||||||
|
|
@ -354,11 +369,18 @@ class Flask(App):
|
||||||
# Use a weakref to avoid creating a reference cycle between the app
|
# Use a weakref to avoid creating a reference cycle between the app
|
||||||
# and the view function (see #3761).
|
# and the view function (see #3761).
|
||||||
self_ref = weakref.ref(self)
|
self_ref = weakref.ref(self)
|
||||||
|
|
||||||
|
def static_view_func(**kw: t.Any) -> Response:
|
||||||
|
app = self_ref()
|
||||||
|
if app is None:
|
||||||
|
raise RuntimeError("The app has been garbage collected.")
|
||||||
|
return app.send_static_file(**kw)
|
||||||
|
|
||||||
self.add_url_rule(
|
self.add_url_rule(
|
||||||
f"{self.static_url_path}/<path:filename>",
|
f"{self.static_url_path}/<path:filename>",
|
||||||
endpoint="static",
|
endpoint="static",
|
||||||
host=static_host,
|
host=static_host,
|
||||||
view_func=lambda **kw: self_ref().send_static_file(**kw), # type: ignore # noqa: B950
|
view_func=static_view_func,
|
||||||
)
|
)
|
||||||
|
|
||||||
def get_send_file_max_age(self, filename: str | None) -> int | None:
|
def get_send_file_max_age(self, filename: str | None) -> int | None:
|
||||||
|
|
@ -386,7 +408,7 @@ class Flask(App):
|
||||||
if isinstance(value, timedelta):
|
if isinstance(value, timedelta):
|
||||||
return int(value.total_seconds())
|
return int(value.total_seconds())
|
||||||
|
|
||||||
return value # type: ignore[no-any-return]
|
return t.cast(int, value)
|
||||||
|
|
||||||
def send_static_file(self, filename: str) -> Response:
|
def send_static_file(self, filename: str) -> Response:
|
||||||
"""The view function used to serve files from
|
"""The view function used to serve files from
|
||||||
|
|
@ -580,7 +602,8 @@ class Flask(App):
|
||||||
or request.routing_exception.code in {307, 308}
|
or request.routing_exception.code in {307, 308}
|
||||||
or request.method in {"GET", "HEAD", "OPTIONS"}
|
or request.method in {"GET", "HEAD", "OPTIONS"}
|
||||||
):
|
):
|
||||||
raise request.routing_exception # type: ignore[misc]
|
assert request.routing_exception is not None
|
||||||
|
raise request.routing_exception
|
||||||
|
|
||||||
from .debughelpers import FormDataRoutingRedirect
|
from .debughelpers import FormDataRoutingRedirect
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,21 @@ if t.TYPE_CHECKING: # pragma: no cover
|
||||||
|
|
||||||
|
|
||||||
class Blueprint(SansioBlueprint):
|
class Blueprint(SansioBlueprint):
|
||||||
|
"""Represents a blueprint, a collection of routes and other
|
||||||
|
app-related functions that can be registered on a real application
|
||||||
|
later.
|
||||||
|
|
||||||
|
A blueprint is an object that allows defining application functions
|
||||||
|
without requiring an application object ahead of time. It uses the
|
||||||
|
same decorators as :class:`~flask.Flask`, but defers the need for an
|
||||||
|
application by recording them for later registration.
|
||||||
|
|
||||||
|
Decorating a function with a blueprint creates a deferred function
|
||||||
|
that is called with :class:`~flask.BlueprintSetupState` when the
|
||||||
|
blueprint is registered on an application.
|
||||||
|
|
||||||
|
See :doc:`/blueprints` for more information.
|
||||||
|
"""
|
||||||
def __init__(
|
def __init__(
|
||||||
self,
|
self,
|
||||||
name: str,
|
name: str,
|
||||||
|
|
@ -77,7 +92,7 @@ class Blueprint(SansioBlueprint):
|
||||||
if isinstance(value, timedelta):
|
if isinstance(value, timedelta):
|
||||||
return int(value.total_seconds())
|
return int(value.total_seconds())
|
||||||
|
|
||||||
return value # type: ignore[no-any-return]
|
return t.cast(int, value)
|
||||||
|
|
||||||
def send_static_file(self, filename: str) -> Response:
|
def send_static_file(self, filename: str) -> Response:
|
||||||
"""The view function used to serve files from
|
"""The view function used to serve files from
|
||||||
|
|
|
||||||
|
|
@ -18,7 +18,12 @@ T = t.TypeVar("T")
|
||||||
|
|
||||||
|
|
||||||
class ConfigAttribute(t.Generic[T]):
|
class ConfigAttribute(t.Generic[T]):
|
||||||
"""Makes an attribute forward to the config"""
|
"""Makes an attribute forward to the config.
|
||||||
|
|
||||||
|
This descriptor allows accessing configuration values as attributes on
|
||||||
|
the application object. It can optionally convert the value using a
|
||||||
|
converter function.
|
||||||
|
"""
|
||||||
|
|
||||||
def __init__(
|
def __init__(
|
||||||
self, name: str, get_converter: t.Callable[[t.Any], T] | None = None
|
self, name: str, get_converter: t.Callable[[t.Any], T] | None = None
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue