diff --git a/news/7068.deprecation.md b/news/7068.deprecation.md new file mode 100644 index 00000000000..feee20812a1 --- /dev/null +++ b/news/7068.deprecation.md @@ -0,0 +1 @@ +Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the specific router var instead, e.g. `deps=["router_url"]`. diff --git a/news/7068.performance.md b/news/7068.performance.md new file mode 100644 index 00000000000..3c56ecabc0e --- /dev/null +++ b/news/7068.performance.md @@ -0,0 +1 @@ +Store router data in separate base vars (session, headers, page, url, route_id) so a navigation delta only re-sends the fields that changed instead of the whole router, and gather the connection-scoped router data (headers, client IP, session id) once at connect time rather than on every event. `State.router` is unchanged for app code. diff --git a/packages/reflex-base/news/7068.performance.md b/packages/reflex-base/news/7068.performance.md new file mode 100644 index 00000000000..41445cf7826 --- /dev/null +++ b/packages/reflex-base/news/7068.performance.md @@ -0,0 +1 @@ +The event processor now refreshes only the router vars whose backing `router_data` keys actually changed, so a navigation no longer rebuilds and re-sends the connection-scoped session and header data. `ROUTER_VARS` names the per-field router vars that replaced the single `router` var on the root state. diff --git a/packages/reflex-base/src/reflex_base/constants/__init__.py b/packages/reflex-base/src/reflex_base/constants/__init__.py index f83bcfd1e25..3f46a794461 100644 --- a/packages/reflex-base/src/reflex_base/constants/__init__.py +++ b/packages/reflex-base/src/reflex_base/constants/__init__.py @@ -60,6 +60,12 @@ ROUTER, ROUTER_DATA, ROUTER_DATA_INCLUDE, + ROUTER_HEADERS, + ROUTER_PAGE, + ROUTER_ROUTE_ID, + ROUTER_SESSION, + ROUTER_URL, + ROUTER_VARS, DefaultPage, Page404, RouteArgType, @@ -86,6 +92,12 @@ "ROUTER", "ROUTER_DATA", "ROUTER_DATA_INCLUDE", + "ROUTER_HEADERS", + "ROUTER_PAGE", + "ROUTER_ROUTE_ID", + "ROUTER_SESSION", + "ROUTER_URL", + "ROUTER_VARS", "ROUTE_NOT_FOUND", "SESSION_STORAGE", "SETTER_PREFIX", diff --git a/packages/reflex-base/src/reflex_base/constants/route.py b/packages/reflex-base/src/reflex_base/constants/route.py index 30e7b32170e..02281381e58 100644 --- a/packages/reflex-base/src/reflex_base/constants/route.py +++ b/packages/reflex-base/src/reflex_base/constants/route.py @@ -11,10 +11,28 @@ class RouteArgType(SimpleNamespace): LIST = "arg_list" -# the name of the backend var containing path and client information +# the name of the state attribute exposing path and client information ROUTER = "router" ROUTER_DATA = "router_data" +# The names of the per-field base vars holding router data on the root state. +# Session and headers are constant for the lifetime of a websocket connection, +# while page, url, and route_id change on every navigation; keeping them in +# separate vars means a navigation delta only re-sends the navigation fields. +ROUTER_SESSION = "router_session" +ROUTER_HEADERS = "router_headers" +ROUTER_PAGE = "router_page" +ROUTER_URL = "router_url" +ROUTER_ROUTE_ID = "router_route_id" + +ROUTER_VARS = ( + ROUTER_SESSION, + ROUTER_HEADERS, + ROUTER_PAGE, + ROUTER_URL, + ROUTER_ROUTE_ID, +) + class RouteVar(SimpleNamespace): """Names of variables used in the router_data dict stored in State.""" diff --git a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py index b9f1a982e72..f8990ac6283 100644 --- a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py +++ b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py @@ -13,7 +13,6 @@ from time import perf_counter from typing import TYPE_CHECKING, Any -from reflex.istate.data import RouterData from reflex.istate.manager.token import BaseStateToken from reflex.istate.proxy import StateProxy from reflex.utils import types @@ -431,12 +430,25 @@ async def _execute_event( ) # re-assign only when the value is set and different - if router_data and state.router_data != router_data: - # assignment will recurse into substates and force recalculation of - # dependent ComputedVar (dynamic route variables) - state.router_data = router_data - if state.router != (router := RouterData.from_router_data(router_data)): - state.router = router + if ( + router_data + and (previous_router_data := state.router_data) != router_data + ): + # only the router vars whose backing keys changed are rebuilt + # and re-sent; session/headers stay put across navigations. + merged_router_data = state._update_router_vars( + router_data, previous_router_data + ) + # Store what it merged rather than the payload: a partial one + # would otherwise drop the keys it omits for the next event. + # Only when that actually differs, though -- a payload that + # merges to what is already there changed nothing, and the + # assignment would still dirty router_data and mark the state + # touched, persisting it for an event that moved nothing. + # The assignment recurses into substates and forces + # recalculation of dependent ComputedVar (dynamic route vars). + if merged_router_data != previous_router_data: + state.router_data = merged_router_data # Preprocess the event. if ( diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 3d541ada126..86b7f966acd 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -720,6 +720,21 @@ def _get_all_var_data(self) -> VarData | None: """ return self._var_data + def _dependency_field_names(self) -> tuple[str, ...]: + """The state field names a ComputedVar depending on this Var must track. + + A Var normally stands for a single state field, the one named by its + VarData. A Var composed of several state fields must name all of them, + or a ``deps=[that_var]`` dependency would only track the one field + VarData.merge happened to surface, leaving the computed var stale when + any of the others change. + + Returns: + The field names to register the dependency against. + """ + all_var_data = self._get_all_var_data() + return (all_var_data.field_name if all_var_data is not None else "",) + def __deepcopy__(self, memo: dict[int, Any]) -> Self: """Deepcopy the var. @@ -2411,10 +2426,11 @@ def _add_static_dep( else None ) if all_var_data is not None: - var_name = all_var_data.field_name + # A composite Var names every state field it is built from. + var_names = dep._dependency_field_names() else: - var_name = dep._js_expr - deps.setdefault(state_name, set()).add(var_name) + var_names = (dep._js_expr,) + deps.setdefault(state_name, set()).update(var_names) elif isinstance(dep, str) and dep != "": deps.setdefault(None, set()).add(dep) else: @@ -2692,18 +2708,20 @@ def add_dependency(self, objclass: type[BaseState], dep: Var): if all_var_data := dep._get_all_var_data(): state_name = all_var_data.state if state_name: - var_name = all_var_data.field_name - if var_name: - self._static_deps.setdefault(state_name, set()).add(var_name) + # A composite Var names every state field it is built from. + var_names = tuple(filter(None, dep._dependency_field_names())) + if var_names: + self._static_deps.setdefault(state_name, set()).update(var_names) target_state_class = objclass.get_root_state().get_class_substate( state_name ) - target_state_class._var_dependencies.setdefault( - var_name, set() - ).add(( - objclass.get_full_name(), - self._name, - )) + for var_name in var_names: + target_state_class._var_dependencies.setdefault( + var_name, set() + ).add(( + objclass.get_full_name(), + self._name, + )) target_state_class._potentially_dirty_states.add( objclass.get_full_name() ) diff --git a/reflex/app.py b/reflex/app.py index 3e4c601b7f7..9bc97d8372f 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -73,7 +73,7 @@ from reflex.app_mixins import AppMixin, LifespanMixin, MiddlewareMixin from reflex.compiler import compiler from reflex.compiler.compiler import readable_name_from_component -from reflex.istate.data import RouterData +from reflex.istate.data import SessionData from reflex.istate.manager import StateManager, StateModificationContext from reflex.istate.manager.token import BaseStateToken from reflex.route import ( @@ -2034,6 +2034,10 @@ def __init__(self, namespace: str, app: App): # Number of client_error reports logged per SID, for rate limiting. self._client_error_counts: dict[str, int] = {} + # Connection-scoped router_data entries per SID, computed once at + # connect time instead of for every event on the connection. + self._static_router_data: dict[str, dict[str, Any]] = {} + # Start time and count of the current process-wide client_error window. self._client_error_window_start = 0.0 self._client_error_window_count = 0 @@ -2085,6 +2089,51 @@ async def on_connect(self, sid: str, environ: dict): if otel.enabled: otel.record_connection(1) + # Headers, client IP, and session id cannot change for the lifetime of + # the connection; compute them once instead of on every event. + self._static_router_data[sid] = self._build_static_router_data(sid, environ) + + def _build_static_router_data(self, sid: str, environ: dict) -> dict[str, Any]: + """Build the connection-scoped router_data entries for a socket. + + Args: + sid: The Socket.IO session id. + environ: The request information, including HTTP headers. + + Returns: + The router_data entries that are constant for the connection. + """ + asgi_scope = environ.get("asgi.scope", {}) + + # Get the client headers. + headers = { + k.decode("utf-8"): v.decode("utf-8") + for (k, v) in asgi_scope.get("headers", []) + } + + # Get the client IP + try: + client_ip = asgi_scope["client"][0] + headers["asgi-scope-client"] = client_ip + except (KeyError, IndexError): + client_ip = environ.get("REMOTE_ADDR", "0.0.0.0") + + # Unroll reverse proxy forwarded headers. + client_ip = ( + headers + .get( + "x-forwarded-for", + client_ip, + ) + .partition(",")[0] + .strip() + ) + return { + constants.RouteVar.SESSION_ID: sid, + constants.RouteVar.HEADERS: headers, + constants.RouteVar.CLIENT_IP: client_ip, + } + def on_disconnect(self, sid: str) -> asyncio.Task | None: """Event for when the websocket disconnects. @@ -2097,6 +2146,7 @@ def on_disconnect(self, sid: str) -> asyncio.Task | None: if otel.enabled: otel.record_connection(-1) self._client_error_counts.pop(sid, None) + self._static_router_data.pop(sid, None) # Get token before cleaning up disconnect_token = self.sid_to_token.get(sid) if disconnect_token: @@ -2189,45 +2239,33 @@ async def on_event(self, sid: str, data: Any): msg = f"Failed to deserialize event data: {fields}." raise exceptions.EventDeserializationError(msg) from ex - # Get the event environment. - if self.app.sio is None: - msg = "Socket.IO is not initialized." - raise RuntimeError(msg) - environ = self.app.sio.get_environ(sid, self.namespace) - if environ is None: - msg = "Socket.IO environ is not initialized." - raise RuntimeError(msg) - - # Get the client headers. - headers = { - k.decode("utf-8"): v.decode("utf-8") - for (k, v) in environ["asgi.scope"]["headers"] - } - - # Get the client IP - try: - client_ip = environ["asgi.scope"]["client"][0] - headers["asgi-scope-client"] = client_ip - except (KeyError, IndexError): - client_ip = environ.get("REMOTE_ADDR", "0.0.0.0") - - # Unroll reverse proxy forwarded headers. - client_ip = ( - headers - .get( - "x-forwarded-for", - client_ip, + static_router_data = self._static_router_data.get(sid) + if static_router_data is None: + # The connection was not seen by on_connect (e.g. namespace created + # after the socket connected); fall back to the connection environ. + if self.app.sio is None: + msg = "Socket.IO is not initialized." + raise RuntimeError(msg) + environ = self.app.sio.get_environ(sid, self.namespace) + if environ is None: + msg = "Socket.IO environ is not initialized." + raise RuntimeError(msg) + static_router_data = self._static_router_data[sid] = ( + self._build_static_router_data(sid, environ) ) - .partition(",")[0] - .strip() - ) router_data = event.router_data + router_data.update(static_router_data) + # The cached headers reach the event, and from there `state.router_data`, + # which is a plain mutable dict: sharing the mapping would let a handler + # mutating `self.router_data["headers"]` corrupt the connection cache for + # every later event on this socket. The shallow copy is ~17x cheaper than + # the per-event header decode it replaced, so the cache still pays off. + router_data[constants.RouteVar.HEADERS] = static_router_data[ + constants.RouteVar.HEADERS + ].copy() router_data.update({ constants.RouteVar.QUERY: format.format_query_params(event.router_data), constants.RouteVar.CLIENT_TOKEN: token, - constants.RouteVar.SESSION_ID: sid, - constants.RouteVar.HEADERS: headers, - constants.RouteVar.CLIENT_IP: client_ip, }) router_data[constants.RouteVar.PATH] = "/" + ( self.app.router(path) or "404" @@ -2349,4 +2387,16 @@ async def link_token_to_sid(self, sid: str, token: str): BaseStateToken(ident=new_token or token, cls=self.app._state) ) as state: state.router_data[constants.RouteVar.SESSION_ID] = sid - state.router = RouterData.from_router_data(state.router_data) + # The state is loaded under this identity, so record it rather + # than waiting for the first event to fill it in: duplicate-token + # handling hands back a fresh token here, and until router_data + # carries it, anything reading router_session.client_token (a + # background task, a shared-state link) addresses the wrong tree. + state.router_data[constants.RouteVar.CLIENT_TOKEN] = new_token or token + # Rebuild from router_data (rather than replacing the field on + # the existing value) to keep the session var and router_data + # in step, the same way the event processor refreshes it. + if ( + session := SessionData.from_router_data(state.router_data) + ) != state.router_session: + state.router_session = session diff --git a/reflex/istate/data.py b/reflex/istate/data.py index 74e13c60899..3eb9737a483 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -3,7 +3,7 @@ import dataclasses from collections.abc import Mapping from types import MappingProxyType -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Final from urllib.parse import _NetlocResultMixinStr, parse_qsl, urlsplit from reflex_base import constants @@ -382,6 +382,86 @@ def _serialize_page_data(obj: PageData) -> dict: return {key.name: getattr(obj, key.name) for key in dataclasses.fields(obj)} +def _url_from_router_data(router_data: dict) -> ReflexURL: + """Build the browser URL for the page described by a router_data dict. + + Args: + router_data: the router_data dict. + + Returns: + The parsed browser URL (origin header + prefixed path). + """ + return ReflexURL( + router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") + + get_config().prepend_frontend_path( + router_data.get(constants.RouteVar.ORIGIN, "") + ) + ) + + +@dataclasses.dataclass(frozen=True) +class URLData: + """The parsed components of the current page URL. + + Storage form of ``RouterData.url`` in the state: unlike ``ReflexURL`` (a + ``str`` subclass, which ``json.dumps`` would serialize as a bare string), + a dataclass goes through the registered serializer, so the frontend + receives the parsed component dict. + """ + + scheme: str = "" + netloc: str = "" + origin: str = "" + path: str = "" + query: str = "" + query_parameters: Mapping[str, str] = dataclasses.field( + default_factory=_FrozenDictStrStr + ) + fragment: str = "" + # Annotated str so the frontend var for this field renders the raw href + # string, but always holds a ReflexURL at runtime so the backend keeps + # parsed-component access without re-splitting the URL. + href: str = ReflexURL("") + + @classmethod + def from_url(cls, url: ReflexURL) -> "URLData": + """Create a URLData object from an already-parsed ReflexURL. + + Args: + url: the parsed URL. + + Returns: + A URLData object mirroring the URL's components. + """ + return cls( + scheme=url.scheme, + netloc=url.netloc, + origin=url.origin, + path=url.path, + query=url.query, + query_parameters=url.query_parameters, + fragment=url.fragment, + href=url, + ) + + @classmethod + def from_router_data(cls, router_data: dict) -> "URLData": + """Create a URLData object from the given router_data. + + Args: + router_data: the router_data dict. + + Returns: + A URLData object for the page described by the router_data. + """ + return cls.from_url(_url_from_router_data(router_data)) + + +@serializer(to=dict) +def _serialize_url_data(obj: URLData) -> dict: + return {key.name: getattr(obj, key.name) for key in dataclasses.fields(obj)} + + @dataclasses.dataclass(frozen=True) class SessionData: """An object containing session data.""" @@ -451,16 +531,21 @@ def from_router_data(cls, router_data: dict) -> "RouterData": session=SessionData.from_router_data(router_data), headers=HeaderData.from_router_data(router_data), _page=PageData.from_router_data(router_data), - url=ReflexURL( - router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") - + get_config().prepend_frontend_path( - router_data.get(constants.RouteVar.ORIGIN, "") - ) - ), + url=_url_from_router_data(router_data), route_id=router_data.get(constants.RouteVar.PATH, ""), ) +# Keys of the serialized RouterData: the object shape the frontend receives. +# `serialize_router_data` emits it, and `RouterDataVar` composes the same shape +# when the whole router is rendered, so the two must not drift apart. +SESSION_KEY: Final = "session" +HEADERS_KEY: Final = "headers" +PAGE_KEY: Final = "page" +URL_KEY: Final = "url" +ROUTE_ID_KEY: Final = "route_id" + + @serializer(to=dict) def serialize_router_data(obj: RouterData) -> dict: """Serialize a RouterData object to a dict. @@ -472,13 +557,180 @@ def serialize_router_data(obj: RouterData) -> dict: A dict representation of the RouterData object. """ return { - "session": obj.session, - "headers": obj.headers, - "page": obj._page, + SESSION_KEY: obj.session, + HEADERS_KEY: obj.headers, + PAGE_KEY: obj._page, # ReflexURL is a str subclass, so json.dumps handles it natively and # never invokes the `default=serialize` hook. Call the URL serializer # eagerly here so the frontend receives the parsed component dict # instead of just the raw URL string. - "url": _serialize_reflex_url(obj.url), - "route_id": obj.route_id, + URL_KEY: _serialize_reflex_url(obj.url), + ROUTE_ID_KEY: obj.route_id, } + + +def _null_var() -> Var: + """Placeholder default for RouterDataVar component fields. + + Returns: + A null Var. + """ + return Var(_js_expr="null", _var_type=None) + + +@dataclasses.dataclass( + eq=False, + frozen=True, + slots=True, +) +class RouterDataVar(CachedVarOperation, ObjectVar[RouterData]): + """Switchboard Var for ``State.router``. + + Router data is stored in separate per-field base vars on the root state + (session, headers, page, url, route_id) so that unchanged + connection-scoped data is not re-sent in the delta on every navigation. + This var stitches them back together: each attribute resolves directly to + the underlying per-field base var, and rendering the var itself produces + an object literal matching the pre-split serialized router shape. + """ + + _url_var: Var = dataclasses.field(default_factory=_null_var) + _page_var: Var = dataclasses.field(default_factory=_null_var) + _session_var: Var = dataclasses.field(default_factory=_null_var) + _headers_var: Var = dataclasses.field(default_factory=_null_var) + _route_id_var: Var = dataclasses.field(default_factory=_null_var) + _default_var_type: ClassVar[Any] = RouterData + + @cached_property_no_lock + def _cached_var_name(self) -> str: + """Render the router as an object literal over the per-field vars. + + Returns: + The JS expression for the assembled router object. + """ + return ( + "({ " + + ", ".join(f'"{key}": {var!s}' for key, var in self._wire_fields().items()) + + " })" + ) + + def _wire_fields(self) -> dict[str, Var]: + """Map each serialized RouterData key to the var backing it. + + Returns: + The keys of the serialized router shape, in order, to their vars. + """ + return { + SESSION_KEY: self._session_var, + HEADERS_KEY: self._headers_var, + PAGE_KEY: self._page_var, + URL_KEY: self._url_var, + ROUTE_ID_KEY: self._route_id_var, + } + + def _dependency_field_names(self) -> tuple[str, ...]: + """Name every per-field router var backing this switchboard. + + VarData.merge surfaces only the first non-empty field name, so without + this a ``deps=[State.router]`` dependency would track one router var + and leave the computed var stale when any of the others changed (a + reconnect updates the session without touching the URL, for example). + + Returns: + The field names of all five per-field router vars. + """ + return tuple( + field_name + for var in ( + self._session_var, + self._headers_var, + self._page_var, + self._url_var, + self._route_id_var, + ) + if (all_var_data := var._get_all_var_data()) is not None + and (field_name := all_var_data.field_name) + ) + + @property + def session(self) -> ObjectVar[SessionData]: + """The per-connection session data. + + Returns: + ObjectVar for the ``router_session`` base var. + """ + return self._session_var.to(ObjectVar, SessionData) + + @property + def headers(self) -> ObjectVar[HeaderData]: + """The headers of the websocket connection request. + + Returns: + ObjectVar for the ``router_headers`` base var. + """ + return self._headers_var.to(ObjectVar, HeaderData) + + @property + def page(self) -> ObjectVar[PageData]: + """The page data for the current page (deprecated, use ``url``). + + Returns: + ObjectVar for the ``router_page`` base var. + """ + return self._page_var.to(ObjectVar, PageData) + + # RouterData exposes the page data under both `page` and `_page`. + _page = page + + @property + def url(self) -> ReflexURLCastedVar: + """The parsed URL of the current page. + + Returns: + ReflexURLCastedVar over the ``router_url`` base var. + """ + return ReflexURLCastedVar.create(self._url_var) + + @property + def route_id(self) -> StringVar: + """The route pattern that matched the current page. + + Returns: + StringVar for the ``router_route_id`` base var. + """ + return self._route_id_var.to(str) + + @classmethod + def create( + cls, + *, + session: Var, + headers: Var, + page: Var, + url: Var, + route_id: Var, + _var_data: VarData | None = None, + ) -> "RouterDataVar": + """Create a RouterDataVar over the per-field router base vars. + + Args: + session: The ``router_session`` base var. + headers: The ``router_headers`` base var. + page: The ``router_page`` base var. + url: The ``router_url`` base var. + route_id: The ``router_route_id`` base var. + _var_data: Additional VarData to merge in. + + Returns: + The new RouterDataVar. + """ + return cls( + _js_expr="", + _var_type=RouterData, + _var_data=_var_data, + _url_var=url, + _page_var=page, + _session_var=session, + _headers_var=headers, + _route_id_var=route_id, + ) diff --git a/reflex/istate/shared.py b/reflex/istate/shared.py index 432cdadbb52..bf5642c2a10 100644 --- a/reflex/istate/shared.py +++ b/reflex/istate/shared.py @@ -6,7 +6,7 @@ from collections.abc import AsyncIterator from typing import TypeVar -from reflex_base.constants import ROUTER_DATA +from reflex_base.constants import ROUTER_DATA, ROUTER_VARS from reflex_base.event import Event, get_hydrate_event from reflex_base.registry import RegistrationContext from reflex_base.utils.exceptions import ReflexRuntimeError @@ -109,7 +109,7 @@ async def _patch_state( linked_state._mark_dirty() # Apply the updates into the existing state tree for rehydrate. root_state = original_state._get_root_state() - root_state.dirty_vars.add("router") + root_state.dirty_vars.update(ROUTER_VARS) root_state.dirty_vars.add(ROUTER_DATA) root_state._mark_dirty() await root_state._get_resolved_delta() @@ -240,7 +240,7 @@ async def _link_to(self, token: str) -> Self: return self # already linked to this token if self._linked_to and self._linked_to != token: # Disassociate from previous linked token since unlink will not be called. - self._linked_from.discard(self.router.session.client_token) + self._linked_from.discard(self.router_session.client_token) # TODO: Change StateManager to accept token + class instead of combining them in a string. if "_" in token: msg = f"Invalid token {token} for linking state {self.get_full_name()}, cannot use underscore (_) in the token name." @@ -275,12 +275,12 @@ async def _unlink(self): # Break the linkage for future events. self._reflex_internal_links.pop(state_name) - self._linked_from.discard(self.router.session.client_token) + self._linked_from.discard(self.router_session.client_token) # Patch in the original state, apply updates, then rehydrate. private_root_state = await get_state_manager().get_state( BaseStateToken( - ident=self.router.session.client_token, + ident=self.router_session.client_token, cls=type(self), ) ) @@ -329,14 +329,13 @@ async def _internal_patch_linked_state( # Set client_token on the linked root so that subsequent get_state # calls when directly modifying a linked token will load the # associated instance. - if linked_root_state.router.session.client_token != token: + if ( + session := linked_root_state.router_session + ).client_token != token: import dataclasses as dc - linked_root_state.router = dc.replace( - linked_root_state.router, - session=dc.replace( - linked_root_state.router.session, client_token=token - ), + linked_root_state.router_session = dc.replace( + session, client_token=token ) if linked_root_state is None: linked_root_state = await get_state_manager().get_state( @@ -349,8 +348,8 @@ async def _internal_patch_linked_state( # Avoid unnecessary dirtiness of shared state when there are no changes. if type(self) not in self._held_locks[token]: self._held_locks[token][type(self)] = linked_state - if self.router.session.client_token not in linked_state._linked_from: - linked_state._linked_from.add(self.router.session.client_token) + if self.router_session.client_token not in linked_state._linked_from: + linked_state._linked_from.add(self.router_session.client_token) if linked_state._linked_to != token: linked_state._linked_to = token await self._exit_stack.enter_async_context( @@ -441,7 +440,7 @@ async def _modify_linked_states( affected_tokens.update( token for token in linked_state._linked_from - if token != self.router.session.client_token + if token != self.router_session.client_token ) # When modifying a shared token directly (empty _reflex_internal_links), # the held locks will be empty. Check SharedState substates for linked diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..fa5256b319c 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -25,7 +25,9 @@ Final, ParamSpec, TypeVar, + cast, get_type_hints, + overload, ) from reflex_base import constants @@ -73,7 +75,15 @@ import reflex.istate.dynamic from reflex import event from reflex.istate import HANDLED_PICKLE_ERRORS, debug_failed_pickles -from reflex.istate.data import RouterData +from reflex.istate.data import ( + HeaderData, + PageData, + ReflexURL, + RouterData, + RouterDataVar, + SessionData, + URLData, +) from reflex.istate.proxy import ImmutableMutableProxy as ImmutableMutableProxy from reflex.istate.proxy import MutableProxy, is_mutable_type from reflex.istate.storage import ClientStorageBase @@ -376,6 +386,124 @@ def _is_user_descriptor(value: Any) -> bool: return not is_computed_var(value) +def _router_fget(self: BaseState) -> RouterData: + """Assemble the RouterData view over the per-field router vars. + + Args: + self: The state instance. + + Returns: + The RouterData for the current connection and page. + """ + return RouterData( + session=self.router_session, + headers=self.router_headers, + _page=self.router_page, + # URLData.href always holds a ReflexURL at runtime (see URLData). + url=cast("ReflexURL", self.router_url.href), + route_id=self.router_route_id, + ) + + +def _router_fset(self: BaseState, value: RouterData) -> None: + """Decompose a RouterData assignment into the per-field router vars. + + Args: + self: The state instance. + value: The RouterData to store. + """ + self.router_session = value.session + self.router_headers = value.headers + self.router_page = value._page + self.router_url = URLData.from_url(value.url) + self.router_route_id = value.route_id + + +def _get_router_var(cls: type[BaseState]) -> RouterDataVar: + """Get (or build and cache) the router switchboard var for a state class. + + Args: + cls: The state class the ``router`` attribute was accessed on. + + Returns: + The RouterDataVar over the root state's per-field router vars. + """ + root_cls = cls.get_root_state() + router_var = root_cls.__dict__.get("_reflex_router_var") + if router_var is None: + base_vars = root_cls.base_vars + if constants.ROUTER_SESSION not in base_vars: + # BaseState itself and mixins never initialize base vars; give + # introspection-style access an unbound switchboard. + return RouterDataVar(_js_expr="", _var_type=RouterData) + router_var = RouterDataVar.create( + session=base_vars[constants.ROUTER_SESSION], + headers=base_vars[constants.ROUTER_HEADERS], + page=base_vars[constants.ROUTER_PAGE], + url=base_vars[constants.ROUTER_URL], + route_id=base_vars[constants.ROUTER_ROUTE_ID], + ) + setattr(root_cls, "_reflex_router_var", router_var) # noqa: B010 + return router_var + + +class _RouterDescriptor(property): + """Property exposing the per-field router vars as a single ``router`` attribute. + + Instance access composes a ``RouterData`` view from the per-field router + vars and assignment decomposes one into them, so existing reads and writes + of ``state.router`` keep working unchanged. Class-level access returns the + ``RouterDataVar`` switchboard, resolving ``State.router.`` to the + underlying per-field base var. Subclassing ``property`` keeps the state + field machinery from treating this as a base var and lets ComputedVar + dependency tracking recurse into the getter, so any computed var reading + ``self.router`` depends on the per-field vars. + """ + + if TYPE_CHECKING: + + @overload + def __get__(self, instance: None, owner: type, /) -> RouterDataVar: ... + + @overload + def __get__(self, instance: BaseState, owner: type, /) -> RouterData: ... + + def __get__(self, instance: Any, owner: type | None = None, /) -> Any: + """Get the switchboard var (class) or RouterData view (instance). + + Args: + instance: The state instance, or None for class access. + owner: The class through which the attribute was accessed. + + Returns: + The RouterDataVar for class access, or the RouterData view. + """ + + def __set__(self, instance: Any, value: RouterData) -> None: + """Set the router data on the instance. + + Args: + instance: The state instance. + value: The RouterData to store. + """ + + else: + + def __get__(self, instance: Any, owner: type | None = None, /): + """Get the switchboard var (class) or RouterData view (instance). + + Args: + instance: The state instance, or None for class access. + owner: The class through which the attribute was accessed. + + Returns: + The RouterDataVar for class access, or the RouterData view. + """ + if instance is None: + return _get_router_var(owner) + return super().__get__(instance, owner) + + all_base_state_classes: dict[str, None] = {} # Instance bookkeeping fields and framework methods read on every event. They @@ -494,8 +622,27 @@ class BaseState(EvenMoreBasicBaseState): default_factory=builtins.dict, is_var=False ) - # The router data for the current page - router: Field[RouterData] = field(default_factory=RouterData) + # The per-connection session data (constant for the socket lifetime). + router_session: Field[SessionData] = field(default_factory=SessionData) + + # The headers of the connection request (constant for the socket lifetime). + router_headers: Field[HeaderData] = field(default_factory=HeaderData) + + # The page data for the current page (deprecated; params feeds dynamic route vars). + router_page: Field[PageData] = field(default_factory=PageData) + + # The parsed URL of the current page. + router_url: Field[URLData] = field(default_factory=URLData) + + # The route pattern that matched the current page. + router_route_id: Field[str] = field(default="") + + # Switchboard for the router vars above: instance reads compose a + # RouterData view, writes decompose into the per-field vars, and class + # access returns the RouterDataVar. Deliberately not a Field: storing each + # kind of router data in its own base var means a navigation delta only + # re-sends the navigation-scoped vars, not session/headers. + router = _RouterDescriptor(_router_fget, _router_fset) # Whether the state has ever been touched since instantiation. _was_touched: bool = field(default=False, is_var=False) @@ -730,6 +877,11 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): **cls.inherited_vars, **cls.base_vars, **cls.computed_vars, + # `router` is a switchboard over the per-field router vars rather + # than a field of its own, but it is usable as a Var everywhere one + # is accepted, so it is listed here (and thus inherited by + # substates). It has no backing field, so it never reaches a delta. + constants.ROUTER: _get_router_var(cls), } cls.event_handlers = {} @@ -995,6 +1147,19 @@ def _init_var_dependency_dicts(cls): # Do not perform dep calculation when cache=False (these are always dirty). continue for state_name, dvar_set in cvar._deps(objclass=cls).items(): + if constants.ROUTER in dvar_set: + # Legacy explicit dependency on the pre-split `router` var: + # depend on all the per-field router vars instead. + console.deprecate( + feature_name='ComputedVar deps=["router"]', + reason="the router var was split; depend on the specific" + ' router var instead (e.g. deps=["router_url"]).', + deprecation_version="0.9.9", + removal_version="1.0", + ) + dvar_set = (dvar_set - {constants.ROUTER}) | set( + constants.ROUTER_VARS + ) state_cls = cls.get_root_state().get_class_substate(state_name) for dvar in dvar_set: defining_state_cls = state_cls @@ -1091,7 +1256,9 @@ def _check_overridden_basevars(cls): """ hints = cls._get_type_hints() for name, computed_var_ in cls._get_computed_vars(): - if name in hints: + # `router` is not a field, but shadowing the descriptor would + # silently break router access for the whole state tree. + if name in hints or name == constants.ROUTER: msg = f"The computed var name `{computed_var_._js_expr}` shadows a base var in {cls.__module__}.{cls.__name__}; use a different name instead" raise ComputedVarShadowsBaseVarsError(msg) @@ -1125,6 +1292,11 @@ def get_skip_vars(cls) -> set[str]: "dirty_vars", "dirty_substates", "router_data", + # Listed in `vars` but backed by no field of its own, so a + # `router` annotation must never become a base var that would + # half-shadow the descriptor. Substates are already covered by + # `inherited_vars` above; this catches a root state class. + constants.ROUTER, } | types.RESERVED_BACKEND_VAR_NAMES ) @@ -1511,7 +1683,7 @@ def setup_dynamic_args(cls, args: builtins.dict[str, str]): def argsingle_factory(param: str): def inner_func(self: BaseState) -> str: - return self.router._page.params.get(param, "") + return self.router_page.params.get(param, "") inner_func.__name__ = param @@ -1519,7 +1691,7 @@ def inner_func(self: BaseState) -> str: def arglist_factory(param: str): def inner_func(self: BaseState) -> list[str]: - return self.router._page.params.get(param, []) + return self.router_page.params.get(param, []) inner_func.__name__ = param @@ -1536,7 +1708,7 @@ def inner_func(self: BaseState) -> list[str]: dynamic_vars[param] = DynamicRouteVar( fget=func, auto_deps=False, - deps=["router"], + deps=[constants.ROUTER_PAGE], _var_data=VarData.from_state(cls, param), ) setattr(cls, param, dynamic_vars[param]) @@ -1714,7 +1886,7 @@ def reset(self): # Reset the base vars. fields = self.get_fields() for prop_name in self.base_vars: - if prop_name == constants.ROUTER: + if prop_name in constants.ROUTER_VARS: continue # never reset the router data field = fields[prop_name] if default_factory := field.default_factory: @@ -1732,6 +1904,86 @@ def reset(self): for substate in self.substates.values(): substate.reset() + def _update_router_vars( + self, + router_data: builtins.dict[str, Any], + previous_router_data: builtins.dict[str, Any], + ) -> builtins.dict[str, Any]: + """Update the per-field router vars from a new router_data dict. + + Each var is rebuilt only when the router_data keys it derives from + changed, so connection-scoped data (session, headers) is not recomputed + on every navigation, and is then assigned only when the rebuilt value + actually differs -- different keys can still yield an equal value (an + absent key and an empty one both produce the default), and assigning + regardless would dirty the var, mark the state touched, and persist it. + + A key missing from ``router_data`` carries no information about the + value it feeds, so the previous one is carried forward rather than + letting the constructors default it away: a payload holding only the + navigation keys must not empty the connection-scoped vars, nor rebuild + the page and URL without the origin header that gives them their host. + + Args: + router_data: The new router_data dict. + previous_router_data: The router_data dict this state last saw. + + Returns: + The router_data to store on the state: the new values over the + previous ones, so a partial payload does not drop keys for the + next comparison either. + """ + # Merging also makes an absent key compare equal to what it replaced, + # so it is not read as a change without a special case for it. + merged = ( + {**previous_router_data, **router_data} + if previous_router_data + else router_data + ) + get = merged.get + prev_get = previous_router_data.get + + headers_changed = prev_get(constants.RouteVar.HEADERS) != get( + constants.RouteVar.HEADERS + ) + # Only the origin header feeds the URL/page host, so the navigation + # vars must not be rebuilt for a change to any other header. + origin_changed = headers_changed and ( + prev_get(constants.RouteVar.HEADERS, {}).get("origin", "") + != get(constants.RouteVar.HEADERS, {}).get("origin", "") + ) + + if ( + any( + prev_get(key) != get(key) + for key in ( + constants.RouteVar.CLIENT_TOKEN, + constants.RouteVar.SESSION_ID, + constants.RouteVar.CLIENT_IP, + ) + ) + and (session := SessionData.from_router_data(merged)) != self.router_session + ): + self.router_session = session + if ( + headers_changed + and (headers := HeaderData.from_router_data(merged)) != self.router_headers + ): + self.router_headers = headers + if ( + origin_changed + or prev_get(constants.RouteVar.PATH) != get(constants.RouteVar.PATH) + or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) + or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) + ): + if (page := PageData.from_router_data(merged)) != self.router_page: + self.router_page = page + if (url := URLData.from_router_data(merged)) != self.router_url: + self.router_url = url + if (route_id := get(constants.RouteVar.PATH, "")) != self.router_route_id: + self.router_route_id = route_id + return merged + @classmethod @functools.lru_cache def _is_client_storage(cls, prop_name_or_field: str | Field) -> bool: @@ -1844,7 +2096,7 @@ async def _get_state_from_redis(self, state_cls: type[T_STATE]) -> T_STATE: ) raise RuntimeError(msg) state_in_redis = await state_manager.get_state( - token=BaseStateToken(ident=self.router.session.client_token, cls=state_cls), + token=BaseStateToken(ident=self.router_session.client_token, cls=state_cls), top_level=False, for_state_instance=self, ) @@ -2222,7 +2474,8 @@ def __getstate__(self): state = state.copy() if state.get("parent_state") is not None: # Do not serialize router data in substates (only the root state). - state.pop("router", None) + for router_var in constants.ROUTER_VARS: + state.pop(router_var, None) state.pop("router_data", None) # Never serialize parent_state or substates. state.pop("parent_state", None) @@ -2243,6 +2496,10 @@ def __setstate__(self, state: builtins.dict[str, Any]): """ state["parent_state"] = None state["substates"] = {} + # Pre-split pickles stored a RouterData under `router`, which is now a + # descriptor; drop it so unpickling does not route through the setter. + # The schema check in _deserialize discards such states anyway. + state.pop("router", None) for key, value in state.items(): object.__setattr__(self, key, value) @@ -2619,7 +2876,7 @@ def on_load_internal(self) -> list[Event | EventSpec | event.EventCallback] | No The list of events to queue for on load handling. """ load_events = RegistrationContext.get().app.get_load_events( - self.router.url.path + self.router_url.path ) if not load_events: self.is_hydrated = True diff --git a/tests/benchmarks/test_event_processing.py b/tests/benchmarks/test_event_processing.py index 15acf8094d4..e912e86a143 100644 --- a/tests/benchmarks/test_event_processing.py +++ b/tests/benchmarks/test_event_processing.py @@ -120,3 +120,85 @@ def test_process_event( @benchmark def _(): loop.run_until_complete(run_events(num_events=3, num_expected_deltas=3)) + + +@pytest.fixture +def on_event_harness(): + """Set up an EventNamespace with a connected socket for benchmarking on_event. + + The event processor's enqueue is mocked out so the benchmark isolates the + per-event router_data preparation (which reuses the connection-scoped + data gathered once in on_connect). + + Yields: + An async callable that feeds the given number of events through + ``EventNamespace.on_event``, and the event loop to drive it with. + """ + from reflex.app import App, EventNamespace + + app = App() + app._event_processor = mock.Mock(enqueue=mock.AsyncMock()) + namespace = EventNamespace("/event", app) + + sid = "benchmark-sid" + environ = { + "QUERY_STRING": "token=benchmark-token", + "asgi.scope": { + "headers": [ + (b"host", b"localhost:3000"), + (b"origin", b"http://localhost:3000"), + (b"user-agent", b"Mozilla/5.0 (X11; Linux x86_64) benchmark"), + (b"accept-encoding", b"gzip, deflate, br"), + (b"accept-language", b"en-US,en;q=0.9"), + (b"cookie", b"session=abc123; theme=dark"), + (b"upgrade", b"websocket"), + (b"connection", b"Upgrade"), + (b"sec-websocket-version", b"13"), + (b"sec-websocket-key", b"dGhlIHNhbXBsZSBub25jZQ=="), + (b"x-forwarded-for", b"203.0.113.7, 10.0.0.1"), + ], + "client": ("127.0.0.1", 54321), + }, + } + + async def run_events(num_events: int) -> None: + """Feed events through on_event. + + Args: + num_events: Number of events to process. + """ + for _ in range(num_events): + await namespace.on_event( + sid, + { + "name": "state.hydrate", + "router_data": {"pathname": "/", "query": {}, "asPath": "/"}, + "payload": {}, + }, + ) + + loop = asyncio.new_event_loop() + loop.run_until_complete(namespace.on_connect(sid, environ)) + yield run_events, loop + loop.close() + + +def test_on_event_router_data( + on_event_harness, + benchmark: BenchmarkFixture, +): + """Benchmark the per-event router_data preparation in on_event. + + Headers and client IP are gathered once at connect time, so the + per-event path is reduced to merging the cached connection-scoped dict + with the event's navigation data. + + Args: + on_event_harness: The run_events async callable and its event loop. + benchmark: The codspeed benchmark fixture. + """ + run_events, loop = on_event_harness + + @benchmark + def _(): + loop.run_until_complete(run_events(num_events=10)) diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index 6ff0b6e805a..81c58f111cb 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -146,3 +146,89 @@ def test_router_url_var_renders_as_href_at_top_level(): """ url_var = rx.State.router.url assert str(url_var) == f'{url_var._original!s}?.["href"]' + + +def test_url_data_serializes_like_reflex_url(): + """URLData (the per-field storage form of the router URL) must serialize + to the same component dict shape as the eager ReflexURL serialization, so + the frontend var access patterns are unchanged by the router var split. + """ + import json + + from reflex_base.utils.format import json_dumps + + from reflex.istate.data import URLData, _serialize_reflex_url + + url = ReflexURL(SAMPLE_URL) + payload = json.loads(json_dumps(URLData.from_url(url))) + assert payload == json.loads(json_dumps(_serialize_reflex_url(url))) + # The runtime value of href keeps parsed-component access on the backend. + assert isinstance(URLData.from_url(url).href, ReflexURL) + + +def test_router_var_resolves_to_per_field_base_vars(): + """State.router is a switchboard: each attribute must resolve directly to + the per-field base var, so a navigation delta that only carries the + navigation-scoped vars still updates every rendered router expression. + """ + prefix = "reflex___state____state" + assert ( + str(rx.State.router.session.client_token) + == f'{prefix}.router_session_rx_state_?.["client_token"]' + ) + assert ( + str(rx.State.router.headers.user_agent) + == f'{prefix}.router_headers_rx_state_?.["user_agent"]' + ) + assert ( + str(rx.State.router.page.raw_path) + == f'{prefix}.router_page_rx_state_?.["raw_path"]' + ) + assert str(rx.State.router.url) == f'{prefix}.router_url_rx_state_?.["href"]' + assert str(rx.State.router.url.path) == f'{prefix}.router_url_rx_state_?.["path"]' + assert str(rx.State.router.route_id) == f"{prefix}.router_route_id_rx_state_" + + +def test_router_var_renders_composed_object(): + """Rendering State.router itself produces an object literal over the + per-field vars, matching the pre-split serialized router shape. + """ + prefix = "reflex___state____state" + assert str(rx.State.router) == ( + "({ " + f'"session": {prefix}.router_session_rx_state_, ' + f'"headers": {prefix}.router_headers_rx_state_, ' + f'"page": {prefix}.router_page_rx_state_, ' + f'"url": {prefix}.router_url_rx_state_, ' + f'"route_id": {prefix}.router_route_id_rx_state_' + " })" + ) + + +def test_router_var_shape_matches_the_serializer(): + """The composed router literal and the serializer must emit the same keys. + + Rendering `State.router` as a whole has to produce the object shape the + backend serializes a `RouterData` into, or a component reading the whole + router would see different keys from the ones the delta carries. The two + are built in different places, so pin them to each other. + """ + import json + + from reflex_base.utils.format import json_dumps + + from reflex.istate.data import RouterData, serialize_router_data + + rendered_keys = list(rx.State.router._wire_fields()) + assert rendered_keys == list(serialize_router_data(RouterData())) + # And that is what actually reaches the client for a whole-router value. + assert rendered_keys == list(json.loads(json_dumps(RouterData()))) + + +def test_router_var_carries_state_var_data(): + """The switchboard var must merge the per-field vars' VarData so hooks + and context wiring for the root state are set up when it renders. + """ + var_data = rx.State.router._get_all_var_data() + assert var_data is not None + assert var_data.state == rx.State.get_full_name() diff --git a/tests/units/reflex_base/event/processor/test_base_state_processor.py b/tests/units/reflex_base/event/processor/test_base_state_processor.py index 6e0db179927..fc8b2448445 100644 --- a/tests/units/reflex_base/event/processor/test_base_state_processor.py +++ b/tests/units/reflex_base/event/processor/test_base_state_processor.py @@ -1338,3 +1338,163 @@ def noop(self): for p in metric_points(otel_metrics, otel.METRIC_STATE_ACQUIRE_DURATION) } assert Event.from_event_type(AcquireState.noop())[0].name in names + + +async def test_no_op_partial_router_data_leaves_the_state_untouched( + wired_app: App, + real_base_state_processor: BaseStateEventProcessor, + emitted_deltas: list, + token: str, +): + """A payload that merges to what is already there must not touch the state. + + A partial router_data (only the navigation keys, as `fix_events` produces) + is never equal to the full dict the state holds, so it reaches the merge. + If it merges to the same thing, nothing moved: assigning it anyway would + dirty router_data, mark the state touched, and persist it for an event + that changed nothing. + + Args: + wired_app: The App wired to the processor's state manager. + real_base_state_processor: The unmocked BaseStateEventProcessor. + emitted_deltas: List of deltas captured from the processor. + token: The client token. + """ + + class NoOpRouterState(State): + n: int = 0 + + @event + def bump(self): + self.n += 1 + + full_view = { + "pathname": "/a", + "asPath": "/a", + "query": {}, + "token": token, + "sid": "sid1", + "ip": "127.0.0.1", + "headers": {"origin": "http://localhost:3000"}, + } + # Same navigation, but carrying only the keys a chained event keeps. + navigation_only = {"pathname": "/a", "asPath": "/a", "query": {}} + + def client_event(router_data: dict[str, Any]) -> Event: + return dataclasses.replace( + Event.from_event_type(NoOpRouterState.bump())[0], router_data=router_data + ) + + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(full_view)) + await processor.join(10) + + root_ctx = real_base_state_processor._root_context + assert root_ctx is not None + state = await root_ctx.state_manager.get_state( + BaseStateToken(ident=token, cls=State) + ) + state._was_touched = False + emitted_deltas.clear() + + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(navigation_only)) + await processor.join(10) + + # The connection-scoped data survived the partial payload... + assert state.router_data["headers"] == full_view["headers"] + assert state.router_session.client_token == token + # ...and nothing about the router was re-sent or marked dirty. + assert not any( + key.startswith("router") + for _token, delta in emitted_deltas + for key in delta.get(State.get_full_name(), {}) + ) + assert not state._get_was_touched() + + +async def test_navigation_delta_elides_connection_scoped_router_vars( + wired_app: App, + real_base_state_processor: BaseStateEventProcessor, + emitted_deltas: list, + token: str, +): + """A navigation only re-sends the navigation-scoped router vars. + + Session and headers cannot change without going through a reconnect, so + re-shipping them in the delta of every client event is pure overhead. + The router is stored in per-field base vars precisely so that a + navigation marks only page/url/route_id dirty; a reconnect (new sid) + marks only the session dirty. + + Args: + wired_app: The App wired to the processor's state manager. + real_base_state_processor: The unmocked BaseStateEventProcessor. + emitted_deltas: List of deltas captured from the processor. + token: The client token. + """ + + class NavState(State): + n: int = 0 + + @event + def bump(self): + self.n += 1 + + headers = {"origin": "http://localhost:3000", "user-agent": "test-agent"} + + def view(path: str, sid: str = "sid1") -> dict[str, Any]: + return { + "pathname": path, + "asPath": path, + "query": {}, + "token": token, + "sid": sid, + "ip": "127.0.0.1", + "headers": headers, + } + + def client_event(router_data: dict[str, Any]) -> Event: + return dataclasses.replace( + Event.from_event_type(NavState.bump())[0], router_data=router_data + ) + + def router_vars_in_deltas() -> set[str]: + return { + key.removesuffix(FIELD_MARKER) + for _token, delta in emitted_deltas + for key in delta.get(State.get_full_name(), {}) + if key.startswith("router") + } + + async def run_event(router_data: dict[str, Any]) -> None: + emitted_deltas.clear() + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(router_data)) + await processor.join(10) + + # First event on the connection populates every router var. + await run_event(view("/a")) + assert router_vars_in_deltas() == { + "router_session", + "router_headers", + "router_page", + "router_url", + "router_route_id", + } + + # A navigation only re-sends the navigation-scoped vars. + await run_event(view("/b")) + assert router_vars_in_deltas() == { + "router_page", + "router_url", + "router_route_id", + } + + # An event without a route change re-sends no router vars at all. + await run_event(view("/b")) + assert router_vars_in_deltas() == set() + + # A reconnect (new sid, same headers) re-sends only the session. + await run_event(view("/b", sid="sid2")) + assert router_vars_in_deltas() == {"router_session"} diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 330b95f09c4..1af2fb6f06f 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -12,14 +12,15 @@ import re import unittest.mock import uuid -from collections.abc import Generator +from collections.abc import AsyncGenerator, Generator from contextlib import nullcontext as does_not_raise from importlib.util import find_spec from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from unittest.mock import AsyncMock, Mock import pytest +import pytest_asyncio import reflex_base from opentelemetry import trace from opentelemetry.sdk.trace import TracerProvider @@ -68,7 +69,7 @@ ) from reflex.compiler.plugins import default_page_plugins from reflex.environment import environment -from reflex.istate.data import RouterData +from reflex.istate.data import RouterData, URLData from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory from reflex.istate.manager.redis import StateManagerRedis @@ -316,9 +317,9 @@ def test_add_page_set_route_dynamic(index_page: ComponentCallable): assert app._pages.keys() == {"test/[dynamic]"} assert "dynamic" in app._state.computed_vars assert app._state.computed_vars["dynamic"]._deps(objclass=EmptyState) == { - EmptyState.get_full_name(): {constants.ROUTER}, + EmptyState.get_full_name(): {"router_page"}, } - assert constants.ROUTER in app._state()._var_dependencies + assert "router_page" in app._state()._var_dependencies def test_add_page_set_route_nested(app: App, index_page: ComponentCallable): @@ -1947,9 +1948,9 @@ async def test_dynamic_route_var_route_change_completed_on_load( assert arg_name in app._state.vars assert arg_name in app._state.computed_vars assert app._state.computed_vars[arg_name]._deps(objclass=DynamicState) == { - DynamicState.get_full_name(): {constants.ROUTER}, + DynamicState.get_full_name(): {"router_page"}, } - assert constants.ROUTER in app._state()._var_dependencies + assert "router_page" in app._state()._var_dependencies substate_token = BaseStateToken(ident=token, cls=DynamicState) exp_vals = ["foo", "foobar", "baz"] @@ -1983,6 +1984,16 @@ def _dynamic_state_event(name, val, **kwargs): val=exp_val, ) exp_router = RouterData.from_router_data(on_load_internal.router_data) + # Only the navigation-scoped router vars change (no session/headers in + # the router_data), so only those land in the delta. + exp_router_delta = { + "router_page" + FIELD_MARKER: exp_router._page, + "router_url" + FIELD_MARKER: URLData.from_url(exp_router.url), + } + if exp_index == 0: + # Every navigation here matches the same route, so the route_id + # only changes on the first one. + exp_router_delta["router_route_id" + FIELD_MARKER] = exp_router.route_id async with mock_base_state_event_processor as processor: await processor.enqueue( token, @@ -1996,7 +2007,7 @@ def _dynamic_state_event(name, val, **kwargs): State.get_full_name(): { arg_name + FIELD_MARKER: exp_val, constants.CompileVars.IS_HYDRATED + FIELD_MARKER: False, - "router" + FIELD_MARKER: exp_router, + **exp_router_delta, }, DynamicState.get_full_name(): { f"comp_{arg_name}" + FIELD_MARKER: exp_val, @@ -4461,6 +4472,210 @@ def test_client_error_constants_match_frontend(): ) +@pytest_asyncio.fixture +async def event_namespace_with_processor_mock() -> AsyncGenerator[EventNamespace, None]: + """An EventNamespace whose app has a mocked event processor. + + Yields: + The EventNamespace instance. + """ + app = App() + app._event_processor = Mock(enqueue=AsyncMock()) + event_namespace = EventNamespace("/event", app) + yield event_namespace + # The token manager is backed by redis when one is configured; drop the + # tokens these tests link so they do not show up in another test's + # enumeration of the shared instance. Awaited rather than run in a fresh + # loop via asyncio.run: the redis client is bound to the test's loop. + await event_namespace._token_manager.disconnect_all() + + +def _connect_environ(token: str) -> dict[str, Any]: + return { + "QUERY_STRING": f"token={token}", + "asgi.scope": { + "headers": [ + (b"origin", b"http://localhost:3000"), + (b"user-agent", b"test-agent"), + ], + "client": ("127.0.0.1", 1234), + }, + } + + +def _client_event_payload() -> dict[str, Any]: + return { + "name": "state.hydrate", + "router_data": {"pathname": "/", "query": {}, "asPath": "/"}, + "payload": {}, + } + + +@pytest.mark.asyncio +async def test_on_event_uses_connect_time_router_data( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """on_event merges the connection-scoped router_data gathered at connect. + + Headers, client IP, and session id are computed once in on_connect; the + per-event path must not re-read the connection environ at all. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace.on_connect("sid1", _connect_environ(token)) + assert "sid1" in event_namespace._static_router_data + + # The per-event path must not re-read the connection environ. + event_namespace.app.sio = Mock( + get_environ=Mock(side_effect=AssertionError("environ must not be consulted")) + ) + await event_namespace.on_event("sid1", _client_event_payload()) + + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + enqueue_mock.assert_called_once() + enqueued_token, event = enqueue_mock.call_args[0] + assert enqueued_token == token + assert event.router_data[constants.RouteVar.CLIENT_TOKEN] == token + assert event.router_data[constants.RouteVar.SESSION_ID] == "sid1" + assert event.router_data[constants.RouteVar.CLIENT_IP] == "127.0.0.1" + assert event.router_data[constants.RouteVar.HEADERS] == { + "origin": "http://localhost:3000", + "user-agent": "test-agent", + "asgi-scope-client": "127.0.0.1", + } + assert event.router_data[constants.RouteVar.PATH] == "/404" + assert event.router_data[constants.RouteVar.QUERY] == {} + + # Disconnect drops the cached connection data. + event_namespace.on_disconnect("sid1") + assert "sid1" not in event_namespace._static_router_data + + +@pytest.mark.asyncio +async def test_link_token_to_sid_records_the_connecting_identity( + token: str, + event_namespace_with_processor_mock: EventNamespace, + mocker: MockerFixture, +): + """The session var carries the token the state was loaded under. + + Duplicate-token handling hands back a fresh token, and the state is loaded + under it. Leaving `router_session.client_token` empty until the first event + would let anything reading it in between -- a background task, a + shared-state link -- address the wrong state tree. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + mocker: pytest-mock fixture. + """ + event_namespace = event_namespace_with_processor_mock + state = Mock() + state.router_data = {} + mocker.patch.object( + event_namespace.app.state_manager, + "modify_state", + Mock(return_value=AsyncMock(__aenter__=AsyncMock(return_value=state))), + ) + + # No duplicate: the connecting token is recorded. + await event_namespace.link_token_to_sid("sid1", token) + assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == token + assert state.router_session.client_token == token + assert state.router_session.session_id == "sid1" + + # Duplicate: the *new* token is recorded, not the one the client sent. + # The duplicate branch emits the replacement token to the client, which + # needs a server the bare namespace does not have. + event_namespace.emit = AsyncMock() # pyright: ignore[reportAttributeAccessIssue] + new_token = "a-fresh-token" + mocker.patch.object( + event_namespace._token_manager, + "link_token_to_sid", + AsyncMock(return_value=new_token), + ) + await event_namespace.link_token_to_sid("sid2", token) + assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == new_token + assert state.router_session.client_token == new_token + assert state.router_session.session_id == "sid2" + + +@pytest.mark.asyncio +async def test_on_event_does_not_share_the_cached_headers( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """Each event gets its own headers mapping, not the cached one. + + The headers reach `state.router_data`, a plain mutable dict, so sharing + the cached mapping would let a handler mutating it corrupt the connection + cache for every later event on the socket. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace.on_connect("sid1", _connect_environ(token)) + cached_headers = event_namespace._static_router_data["sid1"][ + constants.RouteVar.HEADERS + ] + + await event_namespace.on_event("sid1", _client_event_payload()) + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + _, event = enqueue_mock.call_args[0] + event_headers = event.router_data[constants.RouteVar.HEADERS] + + assert event_headers == cached_headers + assert event_headers is not cached_headers + # Mutating what the handler sees must not reach the connection cache. + event_headers["user-agent"] = "mutated" + assert cached_headers["user-agent"] == "test-agent" + + enqueue_mock.reset_mock() + await event_namespace.on_event("sid1", _client_event_payload()) + _, next_event = enqueue_mock.call_args[0] + assert ( + next_event.router_data[constants.RouteVar.HEADERS]["user-agent"] == "test-agent" + ) + + +@pytest.mark.asyncio +async def test_on_event_falls_back_to_environ_without_connect( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """on_event computes and caches the static router_data if connect was missed. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace._token_manager.link_token_to_sid(token, "sid1") + event_namespace.app.sio = Mock( + get_environ=Mock(return_value=_connect_environ(token)) + ) + + await event_namespace.on_event("sid1", _client_event_payload()) + await event_namespace.on_event("sid1", _client_event_payload()) + + # The environ is only consulted once; the result is cached for the sid. + event_namespace.app.sio.get_environ.assert_called_once() + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + assert enqueue_mock.call_count == 2 + for call in enqueue_mock.call_args_list: + _, event = call[0] + assert event.router_data[constants.RouteVar.SESSION_ID] == "sid1" + assert ( + event.router_data[constants.RouteVar.HEADERS]["user-agent"] == "test-agent" + ) + + @pytest.mark.parametrize("compile_raises", [False, True]) def test_compile_releases_memo_naming_caches( mocker: MockerFixture, compile_raises: bool diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 37c0ed2fc4c..ff06d4023da 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -43,7 +43,13 @@ import reflex as rx from reflex.app import App from reflex.environment import environment -from reflex.istate.data import HeaderData, RouterData, _FrozenDictStrStr +from reflex.istate.data import ( + HeaderData, + RouterData, + RouterDataVar, + URLData, + _FrozenDictStrStr, +) from reflex.istate.manager import StateManager from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory @@ -72,20 +78,24 @@ LOCK_EXPIRE_SLEEP = 2.5 if CI else 0.4 -formatted_router = { - "route_id": "", - "url": { +formatted_router_vars = { + "router_route_id" + FIELD_MARKER: "", + "router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "://", + "origin": "", "path": "", "query": "", "query_parameters": {}, "fragment": "", "href": "", }, - "session": {"client_token": "", "client_ip": "", "session_id": ""}, - "headers": { + "router_session" + FIELD_MARKER: { + "client_token": "", + "client_ip": "", + "session_id": "", + }, + "router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -101,7 +111,7 @@ "accept_language": "", "raw_headers": {}, }, - "page": { + "router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", @@ -380,7 +390,8 @@ def test_class_vars(test_state): """ cls = type(test_state) assert cls.vars.keys() == { - "router", + constants.ROUTER, + *constants.ROUTER_VARS, "num1", "num2", "key", @@ -461,8 +472,10 @@ def test_dict(test_state: TestState): } test_state_dict = test_state.dict() assert set(test_state_dict) == substates + # Only vars with a backing field are serialized; `router` is a switchboard + # over the per-field router vars and has no field of its own. assert set(test_state_dict[test_state.get_name()]) == { - var + FIELD_MARKER for var in test_state.vars + var + FIELD_MARKER for var in (*test_state.base_vars, *test_state.computed_vars) } assert set(test_state.dict(include_computed=False)[test_state.get_name()]) == { var + FIELD_MARKER for var in test_state.base_vars @@ -1217,7 +1230,8 @@ def test_interdependent_state_initial_dict() -> None: s = InterdependentState() state_name = s.get_name() d = s.dict(initial=True)[state_name] - d.pop("router" + FIELD_MARKER) + for router_var in constants.ROUTER_VARS: + d.pop(router_var + FIELD_MARKER) assert d == { "x" + FIELD_MARKER: 0, "v1" + FIELD_MARKER: 0, @@ -1510,19 +1524,19 @@ def dep_v(self) -> int: dict1 = json.loads(json_dumps(ps.dict())) assert dict1[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 1, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict1[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 2} dict2 = json.loads(json_dumps(ps.dict())) assert dict2[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 3, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict2[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 4} dict3 = json.loads(json_dumps(ps.dict())) assert dict3[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 5, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict3[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 6} assert counter == 6 @@ -2398,7 +2412,13 @@ async def test_state_proxy( ( token, { - TestState.get_full_name(): {"router" + FIELD_MARKER: router_data}, + TestState.get_full_name(): { + "router_session" + FIELD_MARKER: router_data.session, + "router_headers" + FIELD_MARKER: router_data.headers, + "router_page" + FIELD_MARKER: router_data._page, + "router_url" + FIELD_MARKER: URLData.from_url(router_data.url), + "router_route_id" + FIELD_MARKER: router_data.route_id, + }, grandchild_state.get_full_name(): { "value2" + FIELD_MARKER: "42", }, @@ -3095,7 +3115,7 @@ class MutableContainsBase(BaseState): assert json.loads(val) == { MutableContainsBase.get_full_name(): { f"items{FIELD_MARKER}": [{"tags": ["123", "456"]}], - f"router{FIELD_MARKER}": formatted_router, + **formatted_router_vars, } } @@ -3394,7 +3414,10 @@ def index(): assert len(emitted_deltas) == 1 + len(expected) first_token, first_delta = emitted_deltas[0] assert first_token == token - assert first_delta[State.get_full_name()].pop("router" + FIELD_MARKER) is not None + first_state_delta = first_delta[State.get_full_name()] + assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + for router_var in constants.ROUTER_VARS: + first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) # Find the deltas containing the test handler's state change @@ -3454,7 +3477,10 @@ def index(): # First delta: router + is_hydrated=False assert len(emitted_deltas) >= 2 first_delta = emitted_deltas[0][1] - assert first_delta[State.get_full_name()].pop("router" + FIELD_MARKER) is not None + first_state_delta = first_delta[State.get_full_name()] + assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + for router_var in constants.ROUTER_VARS: + first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) # Find deltas containing the test handler's state change (num incremented twice) @@ -3727,12 +3753,15 @@ def foo(self) -> str: foo = RouterVarDepState.computed_vars["foo"] State._init_var_dependency_dicts() + # Reading self.router recurses into the router property getter, so the + # dependency lands on each of the per-field router vars. assert foo._deps(objclass=RouterVarDepState) == { - RouterVarDepState.get_full_name(): {"router"} + RouterVarDepState.get_full_name(): set(constants.ROUTER_VARS) } - assert (RouterVarDepState.get_full_name(), "foo") in State._var_dependencies[ - "router" - ] + for router_var in constants.ROUTER_VARS: + assert (RouterVarDepState.get_full_name(), "foo") in State._var_dependencies[ + router_var + ] # Get state from state manager. rx_state = await state_manager.get_state(BaseStateToken(ident=token, cls=State)) @@ -3745,10 +3774,249 @@ def foo(self) -> str: # Reassign router var state.router = state.router - assert rx_state.dirty_vars == {"router"} + assert rx_state.dirty_vars == set(constants.ROUTER_VARS) assert state.dirty_vars == {"foo"} assert parent_state.dirty_substates == {RouterVarDepState.get_name()} + # The locally-defined states above registered themselves in the class-level + # dependency maps on State, which outlive this test. Left behind, a later + # test that dirties a router var on a fresh State tree resolves the stale + # entry and raises on the missing substate. Drop them. + for dep_set in State._var_dependencies.values(): + dep_set.difference_update({ + (RouterVarDepState.get_full_name(), "foo"), + }) + State._potentially_dirty_states.discard(RouterVarDepState.get_full_name()) + + +def test_router_var_dep_legacy_string() -> None: + """An explicit deps=["router"] still fires when any router var changes. + + The `router` base var was split into per-field vars; a legacy string dep + on "router" is expanded to all of them (with a deprecation warning). + """ + + class LegacyRouterDepState(State): + """A state with a legacy string dependency on the router var.""" + + @rx.var(deps=["router"], auto_deps=False) + def foo(self) -> str: + return self.router.url.path + + for router_var in constants.ROUTER_VARS: + assert ( + LegacyRouterDepState.get_full_name(), + "foo", + ) in State._var_dependencies[router_var] + assert "router" not in State._var_dependencies + + # Drop the class-level registrations this locally-defined state made; see + # the note in test_router_var_dep. + for dep_set in State._var_dependencies.values(): + dep_set.discard((LegacyRouterDepState.get_full_name(), "foo")) + State._potentially_dirty_states.discard(LegacyRouterDepState.get_full_name()) + + +def test_router_var_dep_whole_router() -> None: + """deps=[State.router] must track every per-field router var. + + The switchboard's VarData surfaces only one field name, so without the + composite dependency hook a cached var declaring the whole router would go + stale when any other router field changed -- a reconnect updates the + session without touching the URL, for instance. + """ + + class WholeRouterDepState(State): + """A state depending on the whole router var.""" + + @rx.var(deps=[State.router], auto_deps=False) + def summary(self) -> str: + return "" + + assert WholeRouterDepState.computed_vars["summary"]._static_deps == { + State.get_full_name(): set(constants.ROUTER_VARS) + } + for router_var in constants.ROUTER_VARS: + assert ( + WholeRouterDepState.get_full_name(), + "summary", + ) in State._var_dependencies[router_var] + + # Drop the class-level registrations; see the note in test_router_var_dep. + for dep_set in State._var_dependencies.values(): + dep_set.discard((WholeRouterDepState.get_full_name(), "summary")) + State._potentially_dirty_states.discard(WholeRouterDepState.get_full_name()) + + +def test_router_is_listed_as_a_var_and_inherited_by_substates() -> None: + """`router` is usable as a Var, so it is listed in vars and inherited. + + It has no backing field of its own, so it must stay out of anything that + serializes vars: the switchboard resolves to the root state's per-field + base vars instead. + """ + + class RouterVarListingState(State): + """A substate that only inherits the router.""" + + assert constants.ROUTER in State.vars + assert constants.ROUTER in RouterVarListingState.inherited_vars + assert constants.ROUTER not in State.base_vars + assert constants.ROUTER not in State.computed_vars + + # The substate's entry is the root's switchboard, resolving to the root's + # per-field base vars rather than to anything on the substate. + router_var = RouterVarListingState.vars[constants.ROUTER] + assert isinstance(router_var, RouterDataVar) + assert router_var.equals(State.router) + assert str(router_var.route_id) == str(State.router_route_id) + + +def test_update_router_vars_ignores_omitted_static_keys( + test_state: TestState, +) -> None: + """A navigation-only payload must not reset the connection-scoped vars. + + A router_data carrying only the navigation keys says nothing about the + session or headers; treating the omission as a change would wipe them to + their defaults and ship a destructive delta. + + Args: + test_state: A state. + """ + full_router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.CLIENT_TOKEN: "tok", + RouteVar.SESSION_ID: "sid1", + RouteVar.CLIENT_IP: "127.0.0.1", + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "a=b"}, + } + test_state._update_router_vars(full_router_data, {}) + test_state._clean() + + navigation_only = { + RouteVar.PATH: "/b", + RouteVar.ORIGIN: "/b", + RouteVar.QUERY: {}, + } + merged = test_state._update_router_vars(navigation_only, full_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_page", + "router_url", + "router_route_id", + } + assert test_state.router.session.client_token == "tok" + assert test_state.router.session.session_id == "sid1" + assert test_state.router.headers.cookie == "a=b" + # The rebuilt navigation vars keep the host from the headers the payload + # omitted, rather than being reconstructed from the partial dict alone. + assert test_state.router.url.origin == "http://localhost:3000" + assert test_state.router.url.path == "/b" + assert test_state.router.page.host == "http://localhost:3000" + # The merged data is what the caller stores, so the omitted keys are still + # there to compare against next time. + assert merged[RouteVar.CLIENT_TOKEN] == "tok" + assert merged[RouteVar.HEADERS] == full_router_data[RouteVar.HEADERS] + + # A second consecutive partial payload still has the full picture. + test_state._clean() + merged2 = test_state._update_router_vars( + {RouteVar.PATH: "/c", RouteVar.ORIGIN: "/c", RouteVar.QUERY: {}}, merged + ) + assert test_state.router.url.origin == "http://localhost:3000" + assert test_state.router.session.client_token == "tok" + assert merged2[RouteVar.HEADERS] == full_router_data[RouteVar.HEADERS] + + +def test_update_router_vars_non_origin_header_leaves_navigation_clean( + test_state: TestState, +) -> None: + """Only the origin header feeds the page/URL, so other headers leave them alone. + + Args: + test_state: A state. + """ + router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "a=b"}, + } + test_state._update_router_vars(router_data, {}) + test_state._clean() + + new_cookie = { + **router_data, + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "c=d"}, + } + test_state._update_router_vars(new_cookie, router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_headers"} + + +def test_update_router_vars_granular_delta(test_state: TestState) -> None: + """_update_router_vars only dirties the vars whose source keys changed. + + Args: + test_state: A state. + """ + full_router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.CLIENT_TOKEN: "tok", + RouteVar.SESSION_ID: "sid1", + RouteVar.CLIENT_IP: "127.0.0.1", + RouteVar.HEADERS: {"origin": "http://localhost:3000"}, + } + test_state._update_router_vars(full_router_data, {}) + assert set(constants.ROUTER_VARS) <= test_state.dirty_vars + test_state._clean() + + # Navigation: only the navigation-scoped vars are rebuilt. + nav_router_data = {**full_router_data, RouteVar.PATH: "/b", RouteVar.ORIGIN: "/b"} + test_state._update_router_vars(nav_router_data, full_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_page", + "router_url", + "router_route_id", + } + assert test_state.router.url.path == "/b" + assert test_state.router.session.session_id == "sid1" + test_state._clean() + + # Reconnect: only the session var is rebuilt. + reconnect_router_data = {**nav_router_data, RouteVar.SESSION_ID: "sid2"} + test_state._update_router_vars(reconnect_router_data, nav_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_session"} + assert test_state.router.session.session_id == "sid2" + test_state._clean() + + # Header change: headers, and the page/URL whose host derives from them. + # route_id derives from the path alone, so it is left clean. + new_headers_router_data = { + **reconnect_router_data, + RouteVar.HEADERS: {"origin": "http://example.com"}, + } + test_state._update_router_vars(new_headers_router_data, reconnect_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_headers", + "router_page", + "router_url", + } + assert test_state.router.url.origin == "http://example.com" + test_state._clean() + + # Keys that differ but derive the same values leave every var clean: an + # absent key and an empty one both produce the default, and dirtying on + # that alone would mark the state touched and persist it. + equivalent_router_data = { + k: v for k, v in new_headers_router_data.items() if k != RouteVar.QUERY + } + test_state._update_router_vars(equivalent_router_data, new_headers_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == set() + @pytest.mark.asyncio async def test_setvar( diff --git a/tests/units/utils/test_format.py b/tests/units/utils/test_format.py index 83e718411fd..1b062d0f8f2 100644 --- a/tests/units/utils/test_format.py +++ b/tests/units/utils/test_format.py @@ -657,20 +657,24 @@ def test_format_query_params(input, output): assert format.format_query_params(input) == output -formatted_router = { - "route_id": "", - "url": { +formatted_router_vars = { + "router_route_id" + FIELD_MARKER: "", + "router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "://", + "origin": "", "path": "", "query": "", "query_parameters": {}, "fragment": "", "href": "", }, - "session": {"client_token": "", "client_ip": "", "session_id": ""}, - "headers": { + "router_session" + FIELD_MARKER: { + "client_token": "", + "client_ip": "", + "session_id": "", + }, + "router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -686,7 +690,7 @@ def test_format_query_params(input, output): "accept_language": "", "raw_headers": {}, }, - "page": { + "router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", @@ -720,7 +724,7 @@ def test_format_query_params(input, output): "obj" + FIELD_MARKER: {"prop1": 42, "prop2": "hello"}, "sum" + FIELD_MARKER: 3.15, "upper" + FIELD_MARKER: "", - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, "asynctest" + FIELD_MARKER: 0, }, ChildState.get_full_name(): { @@ -742,7 +746,7 @@ def test_format_query_params(input, output): "dt" + FIELD_MARKER: "1989-11-09 18:53:00+01:00", "t" + FIELD_MARKER: "18:53:00+01:00", "td" + FIELD_MARKER: "11 days, 0:11:00", - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, }, }, ),