diff --git a/news/+dev-mode-imports.performance.md b/news/+dev-mode-imports.performance.md new file mode 100644 index 00000000000..0cafbaabae4 --- /dev/null +++ b/news/+dev-mode-imports.performance.md @@ -0,0 +1 @@ +Reduce development startup and reload time and memory by deferring unused database, admin, and compiler imports in the backend launcher and state mutation tracking, and by avoiding redundant app preloads in spawned Granian supervisors. diff --git a/news/7049.bugfix.md b/news/7049.bugfix.md new file mode 100644 index 00000000000..484016d9784 --- /dev/null +++ b/news/7049.bugfix.md @@ -0,0 +1 @@ +Preserve relationship serialization and database usage accounting for apps that use SQLModel directly, without loading unused database integrations. diff --git a/packages/reflex-base/news/+lazy-optional-integrations.performance.md b/packages/reflex-base/news/+lazy-optional-integrations.performance.md new file mode 100644 index 00000000000..e693ec7d556 --- /dev/null +++ b/packages/reflex-base/news/+lazy-optional-integrations.performance.md @@ -0,0 +1 @@ +Load pandas, Plotly, and Pillow serializers on demand and avoid importing SQLAlchemy for generic type helpers, reducing startup time and memory when these integrations are unused. diff --git a/packages/reflex-base/news/7049.bugfix.md b/packages/reflex-base/news/7049.bugfix.md new file mode 100644 index 00000000000..9c55743ed2e --- /dev/null +++ b/packages/reflex-base/news/7049.bugfix.md @@ -0,0 +1 @@ +Preserve SQLModel relationships, ObjectVar field access, and optional serializer overrides on cold startup, including classes without module names and multiple optional-library bases. Avoid import-order failures, fork hangs, and registry corruption during serializer lookup. diff --git a/packages/reflex-base/src/reflex_base/utils/_serializer_types.py b/packages/reflex-base/src/reflex_base/utils/_serializer_types.py new file mode 100644 index 00000000000..f68ef7c1671 --- /dev/null +++ b/packages/reflex-base/src/reflex_base/utils/_serializer_types.py @@ -0,0 +1,39 @@ +"""Optional serializer annotations resolved only when introspected at runtime.""" + +from importlib import import_module +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from pandas import DataFrame as DataFrame + from PIL.Image import Image as Image + from plotly.graph_objects import Figure as Figure + from plotly.graph_objs.layout import Template as Template + from sqlmodel import SQLModel as SQLModel + +_TYPE_MODULES = { + "DataFrame": "pandas", + "Image": "PIL.Image", + "Figure": "plotly.graph_objects", + "Template": "plotly.graph_objs.layout", + "SQLModel": "sqlmodel", +} + + +def __getattr__(name: str) -> type: + """Resolve an optional type without importing unused serializer dependencies. + + Args: + name: The optional type name. + + Returns: + The concrete type from its optional dependency. + + Raises: + AttributeError: If the name is not an optional serializer type. + """ + if name not in _TYPE_MODULES: + msg = f"module {__name__!r} has no attribute {name!r}" + raise AttributeError(msg) + value = getattr(import_module(_TYPE_MODULES[name]), name) + globals()[name] = value + return value diff --git a/packages/reflex-base/src/reflex_base/utils/serializers.py b/packages/reflex-base/src/reflex_base/utils/serializers.py index d4299947499..fcd46b8af3c 100644 --- a/packages/reflex-base/src/reflex_base/utils/serializers.py +++ b/packages/reflex-base/src/reflex_base/utils/serializers.py @@ -2,16 +2,19 @@ from __future__ import annotations -import contextlib +import base64 import dataclasses import decimal import functools import inspect +import io import json import logging +import sys import uuid import warnings from collections.abc import Callable, Mapping, Sequence +from contextlib import suppress from datetime import date, datetime, time, timedelta from enum import Enum from importlib.util import find_spec @@ -20,7 +23,7 @@ from uuid import UUID from reflex_base.constants.colors import Color -from reflex_base.utils import types +from reflex_base.utils import _serializer_types, types logger = logging.getLogger(__name__) @@ -34,8 +37,12 @@ SERIALIZERS: dict[type, Serializer] = {} SERIALIZER_TYPES: dict[type, type] = {} +_OPTIONAL_SERIALIZERS: dict[str, Serializer] = {} +_OPTIONAL_SERIALIZER_TYPES: dict[str, type] = {} +_OPTIONAL_SERIALIZER_MODULES: dict[str, tuple[str, ...]] = {} SERIALIZED_FUNCTION = TypeVar("SERIALIZED_FUNCTION", bound=Serializer) +_REGISTRY_VALUE = TypeVar("_REGISTRY_VALUE") deserializers = { @@ -48,6 +55,25 @@ } +def _get_optional_type_name(type_: type) -> str | None: + """Identify an optional type by identity in an already-loaded module. + + Args: + type_: The value type that may belong to an optional dependency. + + Returns: + The optional type name, or None for an unrelated type. + """ + name = getattr(type_, "__name__", None) + if not isinstance(name, str): + return None + for module_name in _OPTIONAL_SERIALIZER_MODULES.get(name, ()): + module = sys.modules.get(module_name) + if module is not None and vars(module).get(name) is type_: + return name + return None + + @overload def serializer( fn: None = None, @@ -95,6 +121,8 @@ def wrapper(fn: SERIALIZED_FUNCTION) -> SERIALIZED_FUNCTION: # Make sure the type is not already registered. registered_fn = SERIALIZERS.get(type_) + if registered_fn is None and (name := _get_optional_type_name(type_)): + registered_fn = _OPTIONAL_SERIALIZERS[name] if registered_fn is not None and registered_fn != fn and overwrite is not True: message = f"Overwriting serializer for type {type_} from {registered_fn.__module__}:{registered_fn.__qualname__} to {fn.__module__}:{fn.__qualname__}." if overwrite is False: @@ -117,7 +145,7 @@ def wrapper(fn: SERIALIZED_FUNCTION) -> SERIALIZED_FUNCTION: to_type = to or type_hints.get("return") - # Apply type transformation if requested + # Apply type transformation if requested. if to_type: SERIALIZER_TYPES[type_] = to_type get_serializer_type.cache_clear() @@ -181,6 +209,82 @@ def serialize( return serialized +def _find_optional_base( + type_: type, + registry: dict[type, _REGISTRY_VALUE], + optional_defaults: dict[str, _REGISTRY_VALUE], +) -> tuple[_REGISTRY_VALUE | None, int]: + """Resolve the highest-priority optional base class in the MRO. + + Args: + type_: The type to find a serializer for. + registry: Explicit registrations for functions or output types. + optional_defaults: Defaults for the corresponding optional types. + + Returns: + The matching registration and its priority, or None and -1. + """ + best: _REGISTRY_VALUE | None = None + best_priority = -1 + for base in getattr(type_, "__mro__", ()): + if name := _get_optional_type_name(base): + priority = _OPTIONAL_SERIALIZER_ORDER[name] + if priority > best_priority: + best = registry.get(base, optional_defaults[name]) + best_priority = priority + return best, best_priority + + +def _registered_priority(registered_type: type) -> int | None: + """Get the default precedence of a registered type. + + Args: + registered_type: A key of a serializer registry. + + Returns: + The default priority, or None for a user registration. + """ + priority = _DEFAULT_SERIALIZER_ORDER.get(id(registered_type)) + if priority is None and (name := _get_optional_type_name(registered_type)): + priority = _OPTIONAL_SERIALIZER_ORDER[name] + return priority + + +def _find_serializer( + type_: type, + registry: dict[type, _REGISTRY_VALUE], + optional_defaults: dict[str, _REGISTRY_VALUE], +) -> _REGISTRY_VALUE | None: + """Resolve defaults and overrides without importing or mutating the registry. + + Args: + type_: The type to find a serializer for. + registry: Explicit registrations for functions or output types. + optional_defaults: Defaults for the corresponding optional types. + + Returns: + The matching registration, or None if no serializer is registered. + """ + if (registered := registry.get(type_)) is not None: + return registered + if name := _get_optional_type_name(type_): + return optional_defaults[name] + + best, best_priority = _find_optional_base(type_, registry, optional_defaults) + # A private copy permits concurrent/reentrant registration without a lock + # around user-defined hash or subclass callbacks, or destructive reordering. + for registered_type, value in reversed(registry.copy().items()): + priority = _registered_priority(registered_type) + if (priority is None or priority > best_priority) and issubclass( + type_, registered_type + ): + # User registrations follow all defaults, in reverse insertion order. + if priority is None: + return value + best, best_priority = value, priority + return best + + @functools.lru_cache def get_serializer(type_: type) -> Serializer | None: """Get the serializer for the type. @@ -191,18 +295,7 @@ def get_serializer(type_: type) -> Serializer | None: Returns: The serializer for the type, or None if there is no serializer. """ - # First, check if the type is registered. - serializer = SERIALIZERS.get(type_) - if serializer is not None: - return serializer - - # If the type is not registered, check if it is a subclass of a registered type. - for registered_type, serializer in reversed(SERIALIZERS.items()): - if issubclass(type_, registered_type): - return serializer - - # If there is no serializer, return None. - return None + return _find_serializer(type_, SERIALIZERS, _OPTIONAL_SERIALIZERS) @functools.lru_cache @@ -215,18 +308,7 @@ def get_serializer_type(type_: type) -> type | None: Returns: The serialized type for the type, or None if there is no type conversion registered. """ - # First, check if the type is registered. - serializer = SERIALIZER_TYPES.get(type_) - if serializer is not None: - return serializer - - # If the type is not registered, check if it is a subclass of a registered type. - for registered_type, serializer in reversed(SERIALIZER_TYPES.items()): - if issubclass(type_, registered_type): - return serializer - - # If there is no serializer, return None. - return None + return _find_serializer(type_, SERIALIZER_TYPES, _OPTIONAL_SERIALIZER_TYPES) def has_serializer(type_: type, into_type: type | None = None) -> bool: @@ -408,105 +490,161 @@ def serialize_color(color: Color) -> str: return color.__format__("") -with contextlib.suppress(ImportError): - from pandas import DataFrame +def format_dataframe_values(df: _serializer_types.DataFrame) -> list[list[Any]]: + """Format dataframe values to a list of lists. - def format_dataframe_values(df: DataFrame) -> list[list[Any]]: - """Format dataframe values to a list of lists. + Args: + df: The dataframe to format. - Args: - df: The dataframe to format. + Returns: + The dataframe as a list of lists. + """ + return [ + [str(d) if isinstance(d, (list, tuple)) else d for d in data] + for data in list(df.to_numpy().tolist()) + ] - Returns: - The dataframe as a list of lists. - """ - return [ - [str(d) if isinstance(d, (list, tuple)) else d for d in data] - for data in list(df.to_numpy().tolist()) - ] - @serializer - def serialize_dataframe(df: DataFrame) -> dict: - """Serialize a pandas dataframe. +def serialize_dataframe(df: _serializer_types.DataFrame) -> dict: + """Serialize a pandas dataframe. - Args: - df: The dataframe to serialize. + Args: + df: The dataframe to serialize. - Returns: - The serialized dataframe. - """ - return { - "columns": df.columns.tolist(), - "data": format_dataframe_values(df), - } + Returns: + The serialized dataframe. + """ + return { + "columns": df.columns.tolist(), + "data": format_dataframe_values(df), + } -with contextlib.suppress(ImportError): - from plotly.graph_objects import Figure, layout +def serialize_figure(figure: _serializer_types.Figure) -> dict: + """Serialize a plotly figure. + + Args: + figure: The figure to serialize. + + Returns: + The serialized figure. + """ from plotly.io import to_json - @serializer - def serialize_figure(figure: Figure) -> dict: - """Serialize a plotly figure. + return json.loads(str(to_json(figure))) - Args: - figure: The figure to serialize. - Returns: - The serialized figure. - """ - return json.loads(str(to_json(figure))) +def serialize_template(template: _serializer_types.Template) -> dict: + """Serialize a plotly template. - @serializer - def serialize_template(template: layout.Template) -> dict: - """Serialize a plotly template. + Args: + template: The template to serialize. - Args: - template: The template to serialize. + Returns: + The serialized template. + """ + from plotly.io import to_json - Returns: - The serialized template. - """ - return { - "data": json.loads(str(to_json(template.data))), - "layout": json.loads(str(to_json(template.layout))), - } + return { + "data": json.loads(str(to_json(template.data))), + "layout": json.loads(str(to_json(template.layout))), + } -with contextlib.suppress(ImportError): - import base64 - import io +def serialize_image(image: _serializer_types.Image) -> str: + """Serialize a Pillow image as a data URI. + Args: + image: The image to serialize. + + Returns: + The serialized image. + """ from PIL.Image import MIME - from PIL.Image import Image as Img - @serializer - def serialize_image(image: Img) -> str: - """Serialize a plotly figure. + buff = io.BytesIO() + image_format = getattr(image, "format", None) or "PNG" + image.save(buff, format=image_format) + image_bytes = buff.getvalue() + base64_image = base64.b64encode(image_bytes).decode("utf-8") + try: + # Newer method to get the mime type, but does not always work. + mime_type = image.get_format_mimetype() # pyright: ignore [reportAttributeAccessIssue] + except AttributeError: + try: + # Fallback method + mime_type = MIME[image_format] + except KeyError: + # Unknown mime_type: warn and return image/png and hope the browser can sort it out. + warnings.warn( # noqa: B028 + f"Unknown mime type for {image} {image_format}. Defaulting to image/png" + ) + mime_type = "image/png" + + return f"data:{mime_type};base64,{base64_image}" - Args: - image: The image to serialize. - Returns: - The serialized image. - """ - buff = io.BytesIO() - image_format = getattr(image, "format", None) or "PNG" - image.save(buff, format=image_format) - image_bytes = buff.getvalue() - base64_image = base64.b64encode(image_bytes).decode("utf-8") - try: - # Newer method to get the mime type, but does not always work. - mime_type = image.get_format_mimetype() # pyright: ignore [reportAttributeAccessIssue] - except AttributeError: - try: - # Fallback method - mime_type = MIME[image_format] - except KeyError: - # Unknown mime_type: warn and return image/png and hope the browser can sort it out. - warnings.warn( # noqa: B028 - f"Unknown mime type for {image} {image_format}. Defaulting to image/png" - ) - mime_type = "image/png" - - return f"data:{mime_type};base64,{base64_image}" +def serialize_sqlmodel(m: _serializer_types.SQLModel) -> dict[str, Any]: + """Serialize a SQLModel instance, including its loaded relationships. + + Args: + m: The SQLModel instance to serialize. + + Returns: + The model fields and available relationships. + """ + from sqlalchemy.orm.exc import DetachedInstanceError + + fields = m.model_dump() + relationships = {} + for name in m.__sqlmodel_relationships__: + with suppress(DetachedInstanceError): + relationships[name] = getattr(m, name) + return {**fields, **relationships} + + +def _prepare_serializers_for_fork() -> None: + """Finish Plotly JSON initialization before forking a process using it.""" + for name in ("Figure", "Template"): + for module_name in _OPTIONAL_SERIALIZER_MODULES[name]: + module = sys.modules.get(module_name) + if module is not None and isinstance(vars(module).get(name), type): + # JSON engines can defer imports until their first invocation. + # Finish those imports too, without holding a serializer lock. + with suppress(ImportError): + from plotly.io import to_json + + to_json({"data": []}, validate=False) + + return + + +_INITIAL_SERIALIZER_TYPES = tuple(SERIALIZERS) +_DEFAULT_SERIALIZER_ORDER = { + id(type_): index for index, type_ in enumerate(_INITIAL_SERIALIZER_TYPES) +} +_OPTIONAL_SERIALIZERS = { + "DataFrame": serialize_dataframe, + "Figure": serialize_figure, + "Template": serialize_template, + "Image": serialize_image, + "SQLModel": serialize_sqlmodel, +} +_OPTIONAL_SERIALIZER_TYPES = { + "DataFrame": dict, + "Figure": dict, + "Template": dict, + "Image": str, + "SQLModel": dict[str, Any], +} +_OPTIONAL_SERIALIZER_MODULES = { + "DataFrame": ("pandas", "pandas.core.frame"), + "Figure": ("plotly.graph_objs._figure",), + "Template": ("plotly.graph_objs.layout._template",), + "Image": ("PIL.Image",), + "SQLModel": ("sqlmodel.main",), +} +_OPTIONAL_SERIALIZER_ORDER = { + name: len(_INITIAL_SERIALIZER_TYPES) + index + for index, name in enumerate(_OPTIONAL_SERIALIZERS) +} diff --git a/packages/reflex-base/src/reflex_base/utils/types.py b/packages/reflex-base/src/reflex_base/utils/types.py index 8ed26b771c6..63e3851c7fe 100644 --- a/packages/reflex-base/src/reflex_base/utils/types.py +++ b/packages/reflex-base/src/reflex_base/utils/types.py @@ -688,7 +688,7 @@ def get_attribute_access_type( if hasattr(cls, "__fields__") and name in cls.__fields__: # pydantic models return get_field_type(cls, name) - if find_spec("sqlalchemy") and find_spec("sqlalchemy.orm"): + if isinstance(cls, type) and "sqlalchemy.orm" in sys.modules: import sqlalchemy from sqlalchemy.ext.associationproxy import AssociationProxyInstance from sqlalchemy.orm import ( @@ -698,16 +698,9 @@ def get_attribute_access_type( Relationship, ) - from reflex.model import Model + sqlmodel_type = getattr(sys.modules.get("sqlmodel"), "SQLModel", None) - if find_spec("sqlmodel"): - from sqlmodel import SQLModel - - sqlmodel_types = (Model, SQLModel) - else: - sqlmodel_types = (Model,) - - if isinstance(cls, type) and issubclass(cls, DeclarativeBase): + if issubclass(cls, DeclarativeBase): insp = sqlalchemy.inspect(cls) if name in insp.columns: # check for list types @@ -748,9 +741,9 @@ def get_attribute_access_type( ) ] elif ( - isinstance(cls, type) + sqlmodel_type is not None and not is_generic_alias(cls) - and issubclass(cls, sqlmodel_types) + and issubclass(cls, sqlmodel_type) # Probes for unannotated names must not trigger hint resolution, # which may fail on unresolvable ForwardRefs. and declares_annotation(cls, name) diff --git a/reflex/app.py b/reflex/app.py index 3e4c601b7f7..f378a74ab9f 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -1428,6 +1428,10 @@ def _find_route_conflict( def _setup_admin_dash(self): """Setup the admin dash.""" + admin_dash = self.admin_dash + if not admin_dash or not admin_dash.models: + return + try: from starlette_admin.contrib.sqla.admin import Admin from starlette_admin.contrib.sqla.view import ModelView @@ -1440,24 +1444,21 @@ def _setup_admin_dash(self): if not self._api: return - admin_dash = self.admin_dash - - if admin_dash and admin_dash.models: - # Build the admin dashboard - # The first positional argument is `engine` before starlette-admin - # 1.0 and `session_provider` (which still accepts an Engine) after, - # so pass it positionally to support both. - admin = admin_dash.admin or Admin( - get_engine(), - title="Reflex Admin Dashboard", - logo_url="https://reflex.dev/Reflex.svg", - ) + # Build the admin dashboard + # The first positional argument is `engine` before starlette-admin + # 1.0 and `session_provider` (which still accepts an Engine) after, + # so pass it positionally to support both. + admin = admin_dash.admin or Admin( + get_engine(), + title="Reflex Admin Dashboard", + logo_url="https://reflex.dev/Reflex.svg", + ) - for model in admin_dash.models: - view = admin_dash.view_overrides.get(model, ModelView) - admin.add_view(view(model)) + for model in admin_dash.models: + view = admin_dash.view_overrides.get(model, ModelView) + admin.add_view(view(model)) - admin.mount_to(self._api) + admin.mount_to(self._api) def _get_frontend_packages( self, diff --git a/reflex/compiler/utils.py b/reflex/compiler/utils.py index ef437f8d39e..54d27cd201d 100644 --- a/reflex/compiler/utils.py +++ b/reflex/compiler/utils.py @@ -45,6 +45,7 @@ # To re-export this function. merge_imports = imports.merge_imports +write_file = path_ops.write_file def compile_import_statement(fields: list[ImportVar]) -> tuple[str, list[str]]: @@ -879,20 +880,6 @@ def resolve_path_of_web_dir(path: str | Path) -> Path: return (web_dir / path).absolute() -def write_file(path: str | Path, code: str): - """Write the given code to the given path. - - Args: - path: The path to write the code to. - code: The code to write. - """ - path = Path(path) - path.parent.mkdir(parents=True, exist_ok=True) - if path.exists() and path.read_text(encoding="utf-8") == code: - return - path.write_text(code, encoding="utf-8") - - _MEMO_MANIFEST_FILENAME = ".memo-manifest.json" diff --git a/reflex/istate/proxy.py b/reflex/istate/proxy.py index 4a3d9cd35f2..2406c680431 100644 --- a/reflex/istate/proxy.py +++ b/reflex/istate/proxy.py @@ -10,6 +10,7 @@ import json import sys from collections.abc import Callable, Sequence +from importlib import import_module from importlib.util import find_spec from types import MethodType from typing import TYPE_CHECKING, Any, Literal, NoReturn, SupportsIndex, TypeVar, cast @@ -415,21 +416,38 @@ def mark_dirty(self): raise NotImplementedError(msg) -MUTABLE_TYPES = ( +_MUTABLE_BUILTIN_TYPES = ( list, dict, set, ) -if find_spec("sqlalchemy"): - from sqlalchemy.orm import DeclarativeBase +_MUTABLE_MODEL_BASES = ( + ("sqlalchemy.orm.decl_api", "DeclarativeBase"), + ("pydantic.main", "BaseModel"), +) - MUTABLE_TYPES += (DeclarativeBase,) -if find_spec("pydantic"): - from pydantic import BaseModel +def __getattr__(name: str) -> Any: + """Resolve the legacy mutable-types tuple only when explicitly requested. - MUTABLE_TYPES += (BaseModel,) + Args: + name: The module attribute to resolve. + + Returns: + The mutable builtin and model base types. + + Raises: + AttributeError: If the requested attribute is unknown. + """ + if name == "MUTABLE_TYPES": + return _MUTABLE_BUILTIN_TYPES + tuple( + getattr(import_module(module_name), base_name) + for module_name, base_name in _MUTABLE_MODEL_BASES + if find_spec(module_name.partition(".")[0]) + ) + msg = f"module {__name__!r} has no attribute {name!r}" + raise AttributeError(msg) class MutableProxy(wrapt.ObjectProxy): @@ -1012,6 +1030,15 @@ def is_mutable_type(type_: type) -> bool: Returns: Whether the type is mutable and should be wrapped. """ - return issubclass(type_, MUTABLE_TYPES) or ( + if issubclass(type_, _MUTABLE_BUILTIN_TYPES) or ( dataclasses.is_dataclass(type_) and not issubclass(type_, Var) - ) + ): + return True + # A model's defining module is already loaded before its subclasses exist. + # Read its namespace directly so lazy module attributes cannot load packages. + for module_name, base_name in _MUTABLE_MODEL_BASES: + if (module := sys.modules.get(module_name)) is not None: + base = vars(module).get(base_name) + if base is not None and issubclass(type_, base): + return True + return False diff --git a/reflex/model.py b/reflex/model.py index 40a72a99876..34585d853e7 100644 --- a/reflex/model.py +++ b/reflex/model.py @@ -5,14 +5,12 @@ import logging import re from collections import defaultdict -from contextlib import suppress from importlib.util import find_spec from typing import TYPE_CHECKING, Any, ClassVar from reflex_base.config import get_config from reflex_base.environment import environment from reflex_base.utils import console -from reflex_base.utils.serializers import serializer logger = logging.getLogger(__name__) @@ -511,6 +509,7 @@ def migrate(autogenerate: bool = False) -> bool | None: if find_spec("sqlmodel") and find_spec("sqlalchemy") and find_spec("pydantic"): import sqlmodel + from reflex_base.utils.serializers import serialize_sqlmodel as serialize_sqlmodel from sqlmodel.ext.asyncio.session import AsyncSession _AsyncSessionLocal: dict[str | None, sqlalchemy.ext.asyncio.async_sessionmaker] = {} @@ -537,29 +536,6 @@ def get_db_status() -> dict[str, bool]: return {"db": status} - @serializer - def serialize_sqlmodel(m: sqlmodel.SQLModel) -> dict[str, Any]: - """Serialize a SQLModel object to a dictionary. - - Args: - m: The SQLModel object to serialize. - - Returns: - The serialized object as a dictionary. - """ - base_fields = m.model_dump() - relationships = {} - # SQLModel relationships do not appear in __fields__, but should be included if present. - for name in m.__sqlmodel_relationships__: - with suppress( - sqlalchemy.orm.exc.DetachedInstanceError # This happens when the relationship was never loaded and the session is closed. - ): - relationships[name] = getattr(m, name) - return { - **base_fields, - **relationships, - } - def _warn_about_model_deprecation(): console.deprecate( feature_name="reflex.Model", diff --git a/reflex/utils/exec.py b/reflex/utils/exec.py index c93b11949c3..ce6bf5f5eef 100644 --- a/reflex/utils/exec.py +++ b/reflex/utils/exec.py @@ -498,8 +498,18 @@ def run_backend( # Run the backend in development mode. if should_use_granian(): - # We import reflex app because this lets granian cache the module - import reflex.app # noqa: F401 + # Forked workers inherit imported modules from the supervisor. Spawned + # and forkserver workers do not, so preloading the app there only keeps + # the full framework graph resident in the long-lived supervisor. + if not environment.REFLEX_STRICT_HOT_RELOAD.get(): + import multiprocessing + + if multiprocessing.get_start_method() == "fork": + from reflex_base.utils import serializers + + import reflex.app # noqa: F401 + + serializers._prepare_serializers_for_fork() run_granian_backend(host, port, loglevel) else: diff --git a/reflex/utils/frontend_skeleton.py b/reflex/utils/frontend_skeleton.py index 9a5fa3d9ee3..59ab8b20cd0 100644 --- a/reflex/utils/frontend_skeleton.py +++ b/reflex/utils/frontend_skeleton.py @@ -12,8 +12,8 @@ from reflex_base.plugins.embed import get_embed_plugin from reflex.compiler import templates -from reflex.compiler.utils import write_file from reflex.utils import net, path_ops +from reflex.utils.path_ops import write_file from reflex.utils.prerequisites import get_project_hash, get_web_dir from reflex.utils.registry import get_npm_registry diff --git a/reflex/utils/path_ops.py b/reflex/utils/path_ops.py index 6e4da438e7d..fadc4406b65 100644 --- a/reflex/utils/path_ops.py +++ b/reflex/utils/path_ops.py @@ -16,6 +16,20 @@ join = os.linesep.join +def write_file(path: str | Path, code: str): + """Write the given code to the given path, skipping unchanged contents. + + Args: + path: The path to write the code to. + code: The code to write. + """ + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + if path.exists() and path.read_text(encoding="utf-8") == code: + return + path.write_text(code, encoding="utf-8") + + def chmod_rm(path: Path): """Remove a file or directory with chmod. diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index 38b64ebd5f7..d87786f7b92 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -27,7 +27,6 @@ from reflex_base.utils.decorator import once from rich.markup import escape -from reflex import model from reflex.utils import net, path_ops from reflex.utils.misc import get_module_path @@ -788,6 +787,8 @@ def check_schema_up_to_date(): """Check if the sqlmodel metadata matches the current database schema.""" if get_config().db_url is None or not environment.ALEMBIC_CONFIG.get().exists(): return + from reflex import model + with model.get_engine().connect() as connection: from alembic.util.exc import CommandError diff --git a/reflex/utils/telemetry_accounting.py b/reflex/utils/telemetry_accounting.py index 04324b02bb2..57676cdecba 100644 --- a/reflex/utils/telemetry_accounting.py +++ b/reflex/utils/telemetry_accounting.py @@ -3,8 +3,8 @@ from __future__ import annotations import logging +import sys from collections.abc import Iterable, Iterator -from importlib.util import find_spec from typing import TYPE_CHECKING, TypedDict from reflex_base.config import Config, get_config @@ -18,14 +18,11 @@ LocalStorage, SessionStorage, ) -from reflex.model import ModelRegistry from reflex.route import get_route_args from reflex.utils import telemetry logger = logging.getLogger(__name__) -_HAS_SQLALCHEMY = find_spec("sqlalchemy") is not None - __all__ = ["record_compile"] if TYPE_CHECKING: @@ -224,12 +221,32 @@ def _collect_features_used( _walk_state_features(features, user_states) _walk_app_features(features, app) features["upload_count"] = int(Upload.is_used) - if _HAS_SQLALCHEMY: - features["db_model_count"] = len(ModelRegistry.get_models()) + features["db_model_count"] = _get_db_model_count() _record_config_attestations(features, config) return features +def _get_db_model_count() -> int: + """Count nonempty model bases without importing the optional database stack. + + Returns: + The number of nonempty registered model bases, including the shared + SQLModel base for apps that have not loaded Reflex's model integration. + """ + model_module = sys.modules.get("reflex.model") + registry = getattr(model_module, "ModelRegistry", None) + get_models = getattr(registry, "get_models", None) + if get_models is not None: + return len(get_models()) + + # Reflex's default model shares SQLModel's metadata, counting as one base + # regardless of how many tables an app has defined directly with SQLModel. + sqlmodel_module = sys.modules.get("sqlmodel") + sqlmodel_base = getattr(sqlmodel_module, "SQLModel", None) + metadata = getattr(sqlmodel_base, "metadata", None) + return int(metadata is not None and bool(metadata.tables)) + + _STATE_MANAGER_FEATURE: dict[str, FeatureName] = { "disk": "state_manager_disk", "memory": "state_manager_memory", diff --git a/tests/units/compiler/test_compiler_utils.py b/tests/units/compiler/test_compiler_utils.py index 6d1553ff506..8055295b332 100644 --- a/tests/units/compiler/test_compiler_utils.py +++ b/tests/units/compiler/test_compiler_utils.py @@ -8,11 +8,18 @@ from reflex_components_core.el.elements.metadata import Link from reflex.compiler.utils import compile_state, create_document_root +from reflex.compiler.utils import write_file as compiler_write_file from reflex.constants.state import FIELD_MARKER from reflex.state import State +from reflex.utils.path_ops import write_file from reflex.vars.base import computed_var +def test_write_file_reexport() -> None: + """Existing compiler callers retain the shared file-writing helper.""" + assert compiler_write_file is write_file + + def test_document_preloads_the_global_stylesheet(): """Render-blocking CSS should be discoverable alongside early resource hints.""" head = create_document_root().children[0] diff --git a/tests/units/istate/test_proxy.py b/tests/units/istate/test_proxy.py index 5db160714b2..285349688b0 100644 --- a/tests/units/istate/test_proxy.py +++ b/tests/units/istate/test_proxy.py @@ -3,6 +3,8 @@ import asyncio import dataclasses import pickle +import subprocess +import sys from asyncio import CancelledError from contextlib import asynccontextmanager from typing import Any, ClassVar @@ -19,10 +21,103 @@ MutableProxy, ReadOnlyStateProxy, StateProxy, + is_mutable_type, ) from reflex.state import BaseState +def test_proxy_does_not_import_sqlalchemy() -> None: + """State mutation tracking must not load an unused database integration.""" + result = subprocess.run( + [ + sys.executable, + "-c", + """ +import sys +from reflex.istate.proxy import is_mutable_type + +assert is_mutable_type(list) +assert not is_mutable_type(str) +assert "sqlalchemy" not in sys.modules +""", + ], + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize("models_first", [False, True]) +def test_mutable_models_with_either_import_order(models_first: bool) -> None: + """Model classification works before or after importing state tracking.""" + pytest.importorskip("sqlalchemy") + pytest.importorskip("sqlmodel") + script = """ +from pydantic import BaseModel +from pydantic.v1 import BaseModel as LegacyPydanticBase +from sqlalchemy.orm import DeclarativeBase, DeclarativeBaseNoMeta, declarative_base +from sqlmodel import SQLModel +""" + proxy_import = "from reflex.istate import proxy\n" + script = script + proxy_import if models_first else proxy_import + script + script += """ +class DatabaseBase(DeclarativeBase): + pass + +class PydanticModel(BaseModel): + value: int = 1 + +class SQLModelSubclass(SQLModel): + value: int = 1 + +for cls in (DeclarativeBase, DatabaseBase, BaseModel, PydanticModel, SQLModel, SQLModelSubclass): + assert proxy.is_mutable_type(cls), cls + assert proxy.is_mutable_type(cls), cls # Exercise the cached result too. + +for cls in (LegacyPydanticBase, DeclarativeBaseNoMeta, declarative_base()): + assert not proxy.is_mutable_type(cls), cls + +Impostor = type("DeclarativeBase", (), {"__module__": "sqlalchemy.orm.decl_api"}) +assert not proxy.is_mutable_type(Impostor) +assert proxy.MUTABLE_TYPES == (list, dict, set, DeclarativeBase, BaseModel) +assert "MUTABLE_TYPES" not in dir(proxy) +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize( + ("type_", "expected"), + [ + (list, True), + (dict, True), + (set, True), + (type("ListSubclass", (list,), {}), True), + (type("DictSubclass", (dict,), {}), True), + (type("SetSubclass", (set,), {}), True), + (dataclasses.make_dataclass("Data", []), True), + (dataclasses.make_dataclass("FrozenData", [], frozen=True), True), + (rx.Var, False), + (int, False), + (str, False), + (tuple, False), + (frozenset, False), + (object, False), + ], +) +def test_is_mutable_type(type_: type, expected: bool) -> None: + """Keep the existing container, dataclass, and Var classification rules.""" + assert is_mutable_type(type_) is expected + + @dataclasses.dataclass class Item: """Simple picklable object for testing.""" diff --git a/tests/units/reflex_base/utils/test_lazy_serializer_regressions.py b/tests/units/reflex_base/utils/test_lazy_serializer_regressions.py new file mode 100644 index 00000000000..fff95fdd399 --- /dev/null +++ b/tests/units/reflex_base/utils/test_lazy_serializer_regressions.py @@ -0,0 +1,486 @@ +"""Cold-process compatibility regressions for optional serializers.""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + + +def _run_script( + script: str, + *, + cwd: Path | None = None, + timeout: int = 15, +) -> None: + """Run an isolated script and report its captured failure output. + + Args: + script: The Python source to execute in a fresh interpreter. + cwd: The working directory for a temporary application fixture. + timeout: The maximum subprocess lifetime in seconds. + """ + result = subprocess.run( + [sys.executable, "-c", script], + cwd=cwd, + env={**os.environ, "REFLEX_TELEMETRY_ENABLED": "false"}, + capture_output=True, + text=True, + check=False, + timeout=timeout, + ) + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize("custom_type", [None, "SQLModel", "Team", "object"]) +@pytest.mark.parametrize("compatibility_import", [False, True]) +def test_sqlmodel_relationships_from_cold_state( + custom_type: str | None, compatibility_import: bool +) -> None: + """Direct SQLModel usage preserves queried relationships and overrides. + + Args: + custom_type: The model type to override, or None to use the default. + compatibility_import: Whether to import the historical public serializer. + """ + pytest.importorskip("sqlmodel") + if compatibility_import: + pytest.importorskip("alembic") + script = """ +import json +import pickle +import reflex as rx +from typing import Any, get_type_hints +from sqlalchemy.orm import selectinload +from sqlmodel import Field, Relationship, Session, SQLModel, create_engine, select +from reflex_base.utils import serializers +from reflex_base.utils.format import json_dumps +from reflex_base.vars.base import can_use_in_object_var + +class Team(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + name: str + heroes: list["Hero"] = Relationship() + +class Hero(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + name: str + team_id: int | None = Field(default=None, foreign_key="team.id") + +engine = create_engine("sqlite://") +SQLModel.metadata.create_all(engine) +with Session(engine) as session: + session.add(Team(id=1, name="Avengers", heroes=[Hero(id=2, name="Thor", team_id=1)])) + session.commit() + teams = list(session.exec(select(Team).options(selectinload(Team.heroes)))) + +CUSTOM_REGISTRATION + +if COMPATIBILITY_IMPORT: + from reflex.model import serialize_sqlmodel + assert serialize_sqlmodel(m=teams[0])["heroes"][0].name == "Thor" + assert get_type_hints(serialize_sqlmodel)["m"] is SQLModel + assert get_type_hints(serialize_sqlmodel)["return"] == dict[str, Any] + assert pickle.loads(pickle.dumps(serialize_sqlmodel)) is serialize_sqlmodel + assert pickle.loads(b"creflex.model\\nserialize_sqlmodel\\n.") is serialize_sqlmodel + +app = rx.App() + +class State(rx.State): + teams: list[Team] = [] + + @rx.event + def load(self): + self.teams = teams + +state = State(_reflex_internal_init=True) +State.load.fn(state) +payload = json_dumps(state.get_delta()) +assert "Thor" in payload, "loaded SQLModel relationship missing from state delta" +assert "Avengers" in payload +if CUSTOM_ENABLED: + assert serializers.get_serializer(Team) is custom + assert '"custom":true' in payload.replace(" ", "") +else: + assert serializers.get_serializer_type(Team) == dict[str, Any] + assert not serializers.has_serializer(Team, dict) + assert serializers.has_serializer(Team, dict[str, Any]) + assert not serializers.has_serializer(Team, dict[str, int]) + assert not can_use_in_object_var(Team) + assert State.teams[0].name._var_type is str +json.loads(payload) +engine.dispose() +""" + custom_registration = ( + """ +@rx.serializer(overwrite=True) +def custom(value: CUSTOM_TYPE) -> dict: + return { + **value.model_dump(), + "heroes": [hero.name for hero in value.heroes], + "custom": True, + } +""".replace("CUSTOM_TYPE", custom_type) + if custom_type + else "" + ) + _run_script( + script + .replace("CUSTOM_REGISTRATION", custom_registration) + .replace("COMPATIBILITY_IMPORT", repr(compatibility_import)) + .replace("CUSTOM_ENABLED", repr(custom_type is not None)), + timeout=30, + ) + + +def test_custom_serializer_in_optional_dependency_namespace(tmp_path: Path) -> None: + """A user package named pandas must support its own serialized classes. + + Args: + tmp_path: The isolated directory containing the user package. + """ + package = tmp_path / "pandas" + package.mkdir() + (package / "__init__.py").write_text("") + (package / "application.py").write_text( + """ +import reflex as rx + +class Label: + def __init__(self, text="Ready"): + self.text = text + +@rx.serializer +def serialize_label(value: Label) -> str: + return value.text + +class State(rx.State): + label: Label = Label() + + @rx.event + def update(self): + self.label = Label("Updated") +""" + ) + _run_script( + """ +from pandas.application import State +from reflex_base.utils.format import json_dumps + +state = State(_reflex_internal_init=True) +State.update.fn(state) +assert "Updated" in json_dumps(state.get_delta()) +""", + cwd=tmp_path, + ) + + +def test_import_registers_no_fork_hook() -> None: + """Importing the serializers must not install a process-wide fork hook.""" + _run_script( + """ +import os + +hooks = [] +os.register_at_fork = lambda **kwargs: hooks.extend(kwargs.values()) +from reflex_base.utils import serializers + +registered = [hook for hook in hooks if hook.__module__ == serializers.__name__] +assert not registered, registered +""" + ) + + +@pytest.mark.skipif(not hasattr(os, "fork"), reason="requires fork") +@pytest.mark.parametrize( + ("operation", "import_target"), + [ + ("lookup", "plotly.graph_objs.layout._template"), + ("serialize", "plotly.io._json"), + ("serialize", "orjson"), + ], +) +def test_optional_lookup_after_fork_during_import( + operation: str, import_target: str +) -> None: + """Optional serializer use cannot strand an import lock in a forked child. + + Args: + operation: Whether to resolve a serializer or serialize a real figure. + import_target: The dependency whose initial import overlaps the fork. + """ + pytest.importorskip("plotly") + if import_target == "orjson": + pytest.importorskip("orjson") + _run_script( + """ +import importlib.abc +import importlib.machinery +import os +import select +import signal +import sys +import threading +import warnings + +entered = threading.Event() +release = threading.Event() + +class PauseLoader(importlib.abc.Loader): + def __init__(self, inner): + self.inner = inner + + def create_module(self, spec): + return self.inner.create_module(spec) + + def exec_module(self, module): + if threading.current_thread().name == "optional-first-use": + entered.set() + assert release.wait(10) + self.inner.exec_module(module) + +class PauseFinder(importlib.abc.MetaPathFinder): + def find_spec(self, name, path=None, target=None): + if name == IMPORT_TARGET: + spec = importlib.machinery.PathFinder.find_spec(name, path, target) + spec.loader = PauseLoader(spec.loader) + return spec + +sys.meta_path.insert(0, PauseFinder()) +from reflex_base.utils import serializers +from plotly.graph_objects import Figure + +figure = Figure() if OPERATION == "serialize" else None +errors = [] + +def use_serializer(): + if OPERATION == "serialize": + result = serializers.serialize(figure) + assert isinstance(result, dict) + assert "data" in result + else: + assert serializers.get_serializer(Figure) is serializers.serialize_figure + +def first_use(): + try: + use_serializer() + except Exception as error: + errors.append(str(error)) + +thread = threading.Thread(target=first_use, name="optional-first-use", daemon=True) +thread.start() +# Implementations that do not import Template need not reach the pause. +entered.wait(0.5) +read_fd, write_fd = os.pipe() +# Pre-fork preparation waits for the active import to finish. +timer = threading.Timer(0.5, release.set) +timer.start() +serializers._prepare_serializers_for_fork() +with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + pid = os.fork() +if pid == 0: + os.close(read_fd) + use_serializer() + os.write(write_fd, b"OK") + os._exit(0) + +os.close(write_fd) +try: + release.set() + thread.join(5) + assert not thread.is_alive() + assert not errors, errors + ready, _, _ = select.select([read_fd], [], [], 3) + assert ready, "forked child hung during optional serializer lookup" + assert os.read(read_fd, 2) == b"OK" +finally: + release.set() + timer.join(5) + os.close(read_fd) + finished, _ = os.waitpid(pid, os.WNOHANG) + if not finished: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) +""".replace("OPERATION", repr(operation)).replace("IMPORT_TARGET", repr(import_target)), + timeout=30, + ) + + +def test_custom_metaclass_import_does_not_deadlock_registration() -> None: + """An import from a type hash can coexist with a lookup in that import.""" + _run_script( + """ +import importlib.abc +import importlib.util +import os +import sys +import threading +from reflex_base.utils import serializers + +module_entered = threading.Event() +hash_entered = threading.Event() +errors = [] + +class Loader(importlib.abc.Loader): + def create_module(self, spec): + return None + + def exec_module(self, module): + module_entered.set() + assert hash_entered.wait(5) + serializers.get_serializer(int) + module.done = True + +class Finder(importlib.abc.MetaPathFinder): + def find_spec(self, name, path=None, target=None): + if name == "serializer_held_import": + return importlib.util.spec_from_loader(name, Loader()) + +sys.meta_path.insert(0, Finder()) + +class Meta(type): + def __hash__(cls): + hash_entered.set() + import serializer_held_import + return type.__hash__(cls) + +class Target(metaclass=Meta): + pass + +def custom(value: Target) -> str: + return "custom" + +def run(action): + try: + action() + except Exception as error: + errors.append(str(error)) + +importer = threading.Thread(target=run, args=(lambda: __import__("serializer_held_import"),), daemon=True) +importer.start() +assert module_entered.wait(5) +registrar = threading.Thread(target=run, args=(lambda: serializers.serializer(custom),), daemon=True) +registrar.start() +registrar.join(3) +importer.join(0.5) +if registrar.is_alive() or importer.is_alive(): + print("serializer registration deadlocked with an import", flush=True) + os._exit(1) +assert not errors, errors +assert serializers.serialize(Target()) == "custom" +""" + ) + + +@pytest.mark.parametrize("raising_hash_call", [2, 4]) +def test_optional_lookup_hash_failure_preserves_unrelated_serializers( + raising_hash_call: int, +) -> None: + """A user type hash failure cannot remove unrelated serialization behavior. + + Args: + raising_hash_call: The hash invocation that raises during optional lookup. + """ + pytest.importorskip("pandas") + _run_script( + """ +from reflex_base.utils import serializers +from pandas import DataFrame + +armed = False + +class Meta(type): + calls = 0 + + def __hash__(cls): + if armed: + Meta.calls += 1 + if Meta.calls == RAISING_HASH_CALL: + raise RuntimeError("custom metaclass hash failed") + return type.__hash__(cls) + +class Fragile(metaclass=Meta): + pass + +class Stable: + pass + +@serializers.serializer +def fragile(value: Fragile) -> str: + return "fragile" + +@serializers.serializer +def stable(value: Stable) -> str: + return "stable" + +armed = True +try: + serializers.get_serializer(DataFrame) +except RuntimeError: + pass +finally: + armed = False + +assert serializers.serialize(Stable()) == "stable" +assert serializers.get_serializer_type(Stable) is str +""".replace("RAISING_HASH_CALL", str(raising_hash_call)) + ) + + +def test_reentrant_plugin_serializer_registration_survives_optional_lookup() -> None: + """Importing a serializer plugin from a type hash preserves its registration.""" + pytest.importorskip("pandas") + _run_script( + """ +import importlib.abc +import importlib.util +import sys +from reflex_base.utils import serializers +from pandas import DataFrame + +armed = False + +class Late: + pass + +def late(value: Late) -> str: + return "late" + +class Loader(importlib.abc.Loader): + def create_module(self, spec): + return None + + def exec_module(self, module): + serializers.serializer(late) + module.done = True + +class Finder(importlib.abc.MetaPathFinder): + def find_spec(self, name, path=None, target=None): + if name == "serializer_plugin": + return importlib.util.spec_from_loader(name, Loader()) + +sys.meta_path.insert(0, Finder()) + +class Meta(type): + def __hash__(cls): + if armed: + import serializer_plugin + return type.__hash__(cls) + +class Trigger(metaclass=Meta): + pass + +@serializers.serializer +def trigger(value: Trigger) -> str: + return "trigger" + +armed = True +serializers.get_serializer(DataFrame) +# A lookup that does not touch unrelated hashes may leave the plugin unloaded. +import serializer_plugin +assert serializers.serialize(Late()) == "late" +assert serializers.get_serializer_type(Late) is str +""" + ) diff --git a/tests/units/reflex_base/utils/test_types.py b/tests/units/reflex_base/utils/test_types.py index 2b26bb25259..b42cb81a204 100644 --- a/tests/units/reflex_base/utils/test_types.py +++ b/tests/units/reflex_base/utils/test_types.py @@ -79,6 +79,25 @@ def test_property_classes_wildcard_import_compatibility(module_name: str): assert result.stdout.strip() == "True" +def test_import_does_not_load_sqlalchemy() -> None: + """Generic type helpers must not import optional database support.""" + script = """ +import sys + +from reflex_base.utils import types # noqa: F401 + +assert "sqlalchemy" not in sys.modules, "SQLAlchemy imported eagerly" +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr + + def _type_alias_types() -> list[type]: """Collect the TypeAliasType classes available on this Python. diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 330b95f09c4..801f004bdde 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import builtins import contextlib import contextvars import functools @@ -250,6 +251,23 @@ def test_default_app(app: App): assert app.admin_dash is None +def test_setup_admin_dash_skips_optional_imports_without_config( + app: App, monkeypatch: pytest.MonkeyPatch +) -> None: + """A default app must not load the optional admin and database stacks.""" + real_import = builtins.__import__ + + def import_without_admin(name, *args, **kwargs): + if name.startswith("starlette_admin") or name == "reflex.model": + msg = f"unexpected optional import: {name}" + raise AssertionError(msg) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", import_without_admin) + + app._setup_admin_dash() + + def test_multiple_states_error( monkeypatch: pytest.MonkeyPatch, test_state: BaseState, diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 923cff9af40..3152b4abafd 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -1,6 +1,8 @@ import importlib.metadata import json import shutil +import subprocess +import sys import tempfile import uuid from collections.abc import Callable, Generator @@ -300,6 +302,26 @@ def test_check_latest_package_version_can_be_disabled( assert json.loads(version_check_file.read_text()) == {} +def test_prerequisites_does_not_import_database_stack() -> None: + """Importing general prerequisites must not load optional database support.""" + script = """ +import sys + +from reflex.utils import prerequisites # noqa: F401 + +loaded = [name for name in ("reflex.model", "alembic", "sqlmodel") if name in sys.modules] +assert not loaded, f"database modules imported eagerly: {loaded}" +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr + + def _patch_web_dir(monkeypatch: pytest.MonkeyPatch, web_dir: Path): monkeypatch.setattr(frontend_skeleton, "get_web_dir", lambda: web_dir) monkeypatch.setattr(js_runtimes, "get_web_dir", lambda: web_dir) diff --git a/tests/units/test_reflex.py b/tests/units/test_reflex.py index bd539ab727b..14f2f2514fc 100644 --- a/tests/units/test_reflex.py +++ b/tests/units/test_reflex.py @@ -7,6 +7,7 @@ import subprocess import sys +import click import click.testing import pytest @@ -122,6 +123,30 @@ def test_cli_startup_does_not_import_runtime_modules( assert outcome["loaded"] == [] +def test_backend_launcher_does_not_import_compiler_or_state() -> None: + """The backend supervisor must not load the worker's compiler and state.""" + result = subprocess.run( + [ + sys.executable, + "-c", + """ +import sys +from reflex import reflex +from reflex.istate.manager import reset_disk_state_manager +from reflex.utils import build, exec, telemetry + +unexpected = {"reflex.state", "reflex.compiler.utils", "sqlalchemy"} & sys.modules.keys() +assert not unexpected, unexpected +""", + ], + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + + def test_cloud_commands_registered(): """The hosting CLI commands import, resolve, and dispatch only on demand.""" probe = """ diff --git a/tests/units/utils/test_exec.py b/tests/units/utils/test_exec.py index 5dfc677c094..0e8d8d40db2 100644 --- a/tests/units/utils/test_exec.py +++ b/tests/units/utils/test_exec.py @@ -1,17 +1,76 @@ """Tests for development backend launchers in ``reflex.utils.exec``.""" +import builtins +import multiprocessing import os from pathlib import Path import pytest from pytest_mock import MockerFixture from reflex_base.environment import environment +from reflex_base.utils import serializers from reflex.utils import exec as exec_utils DEV_BACKEND_RELOAD_ENV_NAME = environment.REFLEX_DEV_BACKEND_RELOAD_ACTIVE.name +def test_run_backend_skips_app_preload_for_spawn( + tmp_path: Path, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Spawned Granian workers cannot reuse modules imported by the supervisor.""" + mocker.patch.object(exec_utils, "get_web_dir", return_value=tmp_path) + mocker.patch.object(exec_utils, "should_use_granian", return_value=True) + run_granian = mocker.patch.object(exec_utils, "run_granian_backend") + mocker.patch.object(exec_utils, "notify_backend") + mocker.patch.object(multiprocessing, "get_start_method", return_value="spawn") + monkeypatch.setenv(environment.REFLEX_STRICT_HOT_RELOAD.name, "False") + + real_import = builtins.__import__ + + def import_without_app_preload(name, *args, **kwargs): + if name == "reflex.app": + msg = "reflex.app was preloaded in a spawn-based supervisor" + raise AssertionError(msg) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", import_without_app_preload) + prepare_fork = mocker.patch.object(serializers, "_prepare_serializers_for_fork") + + exec_utils.run_backend("127.0.0.1", 8000) + + run_granian.assert_called_once() + prepare_fork.assert_not_called() + + +def test_run_backend_preloads_app_for_fork( + tmp_path: Path, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Forked Granian workers reuse the supervisor's imported app modules.""" + mocker.patch.object(exec_utils, "get_web_dir", return_value=tmp_path) + mocker.patch.object(exec_utils, "should_use_granian", return_value=True) + mocker.patch.object(exec_utils, "run_granian_backend") + mocker.patch.object(exec_utils, "notify_backend") + mocker.patch.object(multiprocessing, "get_start_method", return_value="fork") + monkeypatch.setenv(environment.REFLEX_STRICT_HOT_RELOAD.name, "False") + + imported: list[str] = [] + real_import = builtins.__import__ + + def track_app_preload(name, *args, **kwargs): + if name == "reflex.app": + imported.append(name) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", track_app_preload) + prepare_fork = mocker.patch.object(serializers, "_prepare_serializers_for_fork") + + exec_utils.run_backend("127.0.0.1", 8000) + + assert imported == ["reflex.app"] + prepare_fork.assert_called_once_with() + + def test_run_uvicorn_backend_sets_reload_env_var_and_clears_marker( tmp_path: Path, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ): diff --git a/tests/units/utils/test_path_ops.py b/tests/units/utils/test_path_ops.py new file mode 100644 index 00000000000..f877a310c80 --- /dev/null +++ b/tests/units/utils/test_path_ops.py @@ -0,0 +1,41 @@ +"""Tests for path operations.""" + +from pathlib import Path + +import pytest + +from reflex.utils.path_ops import write_file + + +@pytest.mark.parametrize("string_path", [False, True]) +def test_write_file_creates_parents(tmp_path: Path, string_path: bool) -> None: + """Write UTF-8 content with either a string or Path and missing parents.""" + path = tmp_path / "nested" / "source.js" + content = 'const message = "hello 🌍";\n' + write_file(str(path) if string_path else path, content) + assert path.read_text(encoding="utf-8") == content + + +def test_write_file_preserves_unchanged_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Identical content must not write or trigger a file-watcher reload.""" + path = tmp_path / "source.js" + write_file(path, "unchanged") + original_mtime = path.stat().st_mtime_ns + + def unexpected_write(*args, **kwargs): + """Reject writes to an unchanged file.""" + pytest.fail("An unchanged file was rewritten") + + monkeypatch.setattr(Path, "write_text", unexpected_write) + write_file(path, "unchanged") + assert path.stat().st_mtime_ns == original_mtime + + +def test_write_file_updates_changed_file(tmp_path: Path) -> None: + """Existing files receive changed content.""" + path = tmp_path / "source.js" + write_file(path, "before") + write_file(path, "after") + assert path.read_text(encoding="utf-8") == "after" diff --git a/tests/units/utils/test_serializers.py b/tests/units/utils/test_serializers.py index 01e77d64cc9..289b73b5525 100644 --- a/tests/units/utils/test_serializers.py +++ b/tests/units/utils/test_serializers.py @@ -1,6 +1,9 @@ import datetime import decimal import json +import os +import subprocess +import sys from enum import Enum from pathlib import Path from typing import Any @@ -17,6 +20,334 @@ from pydantic import BaseModel as Base +@pytest.mark.skipif(not hasattr(os, "fork"), reason="requires fork") +@pytest.mark.parametrize("lookup", ["get_serializer", "get_serializer_type"]) +def test_serializer_lookup_after_fork(lookup: str) -> None: + """Forking during a lookup must not leave the child with an orphaned lock. + + Args: + lookup: The serializer lookup to exercise. + """ + script = """ +import os +import select +import signal +import threading +import warnings +from reflex_base.utils import serializers + +entered = threading.Event() +release = threading.Event() + +class HashMeta(type): + calls = 0 + + def __hash__(cls): + if threading.current_thread().name == "lookup": + HashMeta.calls += 1 + # First hash is the cache key; second is the locked registry lookup. + if HashMeta.calls == 2: + entered.set() + assert release.wait(5) + return type.__hash__(cls) + +class Target(metaclass=HashMeta): + pass + +lookup = getattr(serializers, LOOKUP) +thread = threading.Thread(target=lookup, args=(Target,), name="lookup") +thread.start() +assert entered.wait(5) +# Let a pre-fork synchronization hook wait for the active lookup to finish. +timer = threading.Timer(0.5, release.set) +timer.start() +read_fd, write_fd = os.pipe() +with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + pid = os.fork() +if pid == 0: + os.close(read_fd) + lookup(int) + os.write(write_fd, b"OK") + os._exit(0) + +os.close(write_fd) +try: + ready, _, _ = select.select([read_fd], [], [], 3) + assert ready, "forked child hung in serializer lookup" + assert os.read(read_fd, 2) == b"OK" +finally: + release.set() + thread.join(5) + timer.join(5) + os.close(read_fd) + finished, _ = os.waitpid(pid, os.WNOHANG) + if not finished: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) +""".replace("LOOKUP", repr(lookup)) + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +def test_serializer_with_no_module_name() -> None: + """Custom serializers must accept classes without a string module name.""" + script = """ +import reflex as rx +from reflex_base.utils import serializers + +class Value: + __module__ = None + +assert serializers.get_serializer(Value) is None +assert serializers.get_serializer_type(Value) is None + +@rx.serializer +def serialize_value(value: Value) -> str: + return "custom" + +assert serializers.serialize(Value(), get_type=True) == ("custom", str) +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize("reverse_bases", [False, True]) +@pytest.mark.parametrize("first_lookup", ["get_serializer", "get_serializer_type"]) +@pytest.mark.parametrize("module_name", [None, "application"]) +def test_optional_multiple_inheritance_precedence( + reverse_bases: bool, first_lookup: str, module_name: str | None +) -> None: + """Optional base serializers must have stable precedence on first use. + + Args: + reverse_bases: Whether to reverse the optional base classes. + first_lookup: The lookup to perform before either library is registered. + module_name: The optional subclass's module name. + """ + pytest.importorskip("pandas") + pytest.importorskip("PIL") + script = ( + """ +from pandas import DataFrame +from PIL.Image import Image +from reflex_base.utils import serializers + +bases = (Image, DataFrame) if REVERSE_BASES else (DataFrame, Image) +Both = type("Both", bases, {"__module__": MODULE_NAME}) +expected = { + "get_serializer": serializers.serialize_image, + "get_serializer_type": str, +} +assert getattr(serializers, FIRST_LOOKUP)(Both) is expected[FIRST_LOOKUP] +for value_type in (DataFrame, Image): + serializers.get_serializer(value_type) + serializers.get_serializer_type(value_type) + serializers.get_serializer.cache_clear() + serializers.get_serializer_type.cache_clear() + assert serializers.get_serializer(Both) is serializers.serialize_image + assert serializers.get_serializer_type(Both) is str + +@serializers.serializer +def fallback(value: object) -> int: + return 1 + +assert serializers.get_serializer(Both) is fallback +assert serializers.get_serializer_type(Both) is int +assert serializers.get_serializer(Image) is serializers.serialize_image + +@serializers.serializer +def exact(value: Both) -> str: + return "custom" + +assert serializers.get_serializer(Both) is exact +assert serializers.get_serializer_type(Both) is str +""" + .replace("REVERSE_BASES", repr(reverse_bases)) + .replace("FIRST_LOOKUP", repr(first_lookup)) + .replace("MODULE_NAME", repr(module_name)) + ) + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +def test_optional_serializer_dependencies_are_lazy() -> None: + """Importing serializers must not import heavyweight optional libraries.""" + script = """ +import sys + +from reflex_base.utils import serializers # noqa: F401 + +optional_modules = ("pandas", "plotly", "PIL", "sqlmodel", "sqlalchemy") +loaded = [name for name in optional_modules if name in sys.modules] +assert not loaded, f"optional serializer dependencies imported eagerly: {loaded}" +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + + assert result.returncode == 0, result.stderr + + +def test_lazy_serializer_preserves_custom_registration() -> None: + """Loading an optional output type must preserve custom serializers.""" + pytest.importorskip("pandas") + script = """ +from pandas import DataFrame +from reflex_base.utils import serializers + +@serializers.serializer(overwrite=True) +def custom_dataframe(value: DataFrame): + return "custom" + +assert serializers.get_serializer(DataFrame) is custom_dataframe +assert serializers.get_serializer_type(DataFrame) is dict +serializers.get_serializer.cache_clear() +assert serializers.get_serializer(DataFrame) is custom_dataframe +assert serializers.serialize(DataFrame()) == "custom" +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +def test_lazy_serializer_preserves_exact_type_precedence() -> None: + """A broad fallback must not mask a built-in exact-type serializer.""" + pytest.importorskip("pandas") + script = """ +from pandas import DataFrame +from reflex_base.utils import serializers + +class CustomFrame(DataFrame): + pass + +@serializers.serializer +def fallback(value: object) -> str: + return "fallback" + +assert serializers.get_serializer(DataFrame) is serializers.serialize_dataframe +assert serializers.get_serializer_type(DataFrame) is dict +assert serializers.get_serializer(CustomFrame) is fallback +assert serializers.get_serializer_type(CustomFrame) is str +serializers.get_serializer.cache_clear() +assert serializers.get_serializer(CustomFrame) is fallback +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +def test_optional_serializer_annotations_resolve() -> None: + """Runtime introspection must resolve optional serializer argument types.""" + pytest.importorskip("pandas") + pytest.importorskip("plotly") + pytest.importorskip("PIL") + script = """ +from typing import get_type_hints +from reflex_base.utils import serializers + +for name in ( + "format_dataframe_values", "serialize_dataframe", "serialize_figure", + "serialize_template", "serialize_image", +): + hints = get_type_hints(getattr(serializers, name)) + assert all(isinstance(value, type) for key, value in hints.items() if key != "return") +""" + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize("lookup", ["get_serializer", "get_serializer_type"]) +def test_optional_registration_during_subclass_lookup(lookup: str) -> None: + """Concurrent first use must not invalidate an active registry iteration. + + Args: + lookup: The serializer lookup to exercise. + """ + pytest.importorskip("pandas") + script = """ +from concurrent.futures import ThreadPoolExecutor +from threading import Event +from pandas import DataFrame +from reflex_base.utils import serializers + +entered = Event() +registered = Event() + +class Target: + pass + +class BlockingMeta(type): + def __subclasscheck__(cls, candidate): + if candidate is Target: + entered.set() + assert registered.wait(5), "optional registration blocked" + return False + +class Sentinel(metaclass=BlockingMeta): + pass + +@serializers.serializer +def sentinel(value: Sentinel) -> str: + return "sentinel" + +with ThreadPoolExecutor(max_workers=1) as pool: + pending = pool.submit(getattr(serializers, LOOKUP), Target) + assert entered.wait(5), "subclass lookup did not start" + try: + assert serializers.get_serializer(DataFrame) is serializers.serialize_dataframe + finally: + registered.set() + assert pending.result(timeout=5) is None +""".replace("LOOKUP", repr(lookup)) + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert result.returncode == 0, result.stderr + + @pytest.mark.parametrize( ("type_", "expected"), [(Enum, True)], diff --git a/tests/units/utils/test_telemetry_accounting.py b/tests/units/utils/test_telemetry_accounting.py index 760c713571a..677cbf13e7f 100644 --- a/tests/units/utils/test_telemetry_accounting.py +++ b/tests/units/utils/test_telemetry_accounting.py @@ -1,9 +1,12 @@ """Tests for ``reflex.utils.telemetry_accounting``.""" +import subprocess +import sys from types import SimpleNamespace from typing import cast from unittest.mock import MagicMock +import pytest from pytest_mock import MockerFixture from reflex_base.config import Config from reflex_base.plugins.sitemap import SitemapPlugin @@ -21,6 +24,96 @@ from reflex.utils import telemetry_accounting +def _run_in_subprocess(script: str) -> None: + """Run import-sensitive assertions in a fresh interpreter. + + Args: + script: Python source containing the assertions to run. + """ + result = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + + +def test_import_does_not_load_database_model() -> None: + """Compile accounting must not import database support just to report zero.""" + script = """ +import sys + +from reflex.utils import telemetry_accounting + +assert telemetry_accounting._get_db_model_count() == 0 +for module in ("reflex.model", "sqlmodel", "sqlalchemy"): + assert module not in sys.modules, f"{module} imported eagerly" +""" + _run_in_subprocess(script) + + +def test_get_db_model_count_for_direct_sqlmodel_tables() -> None: + """Direct SQLModel tables count as one base without loading reflex.model.""" + pytest.importorskip("sqlmodel") + script = """ +import sys + +from reflex.utils import telemetry_accounting +from sqlmodel import Field, SQLModel + +assert telemetry_accounting._get_db_model_count() == 0 + +class User(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + +assert telemetry_accounting._get_db_model_count() == 1 + +class Post(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + +assert telemetry_accounting._get_db_model_count() == 1 +assert "reflex.model" not in sys.modules, "database model imported eagerly" +""" + _run_in_subprocess(script) + + +def test_get_db_model_count_preserves_registered_bases() -> None: + """Count nonempty registered bases without double counting SQLModel.""" + pytest.importorskip("sqlmodel") + script = """ +from reflex.model import ModelRegistry +from reflex.utils import telemetry_accounting +from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column +from sqlmodel import Field, SQLModel + +class User(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + +class Post(SQLModel, table=True): + id: int | None = Field(default=None, primary_key=True) + +@ModelRegistry.register +class CustomBase(DeclarativeBase): + pass + +class CustomTable(CustomBase): + __tablename__ = "custom_table" + id: Mapped[int] = mapped_column(primary_key=True) + +@ModelRegistry.register +class EmptyBase(DeclarativeBase): + pass + +assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 2 + +ModelRegistry.models.clear() +assert telemetry_accounting._get_db_model_count() == 0 +""" + _run_in_subprocess(script) + + def _fake_config(**overrides) -> Config: """Build a stand-in config pre-populated with defaults the collector reads. @@ -374,11 +467,11 @@ def reflex_internal_task(): def test_collect_features_used_counts_registered_db_models(mocker: MockerFixture): """A non-empty ``ModelRegistry`` reads into ``db_model_count``.""" - mocker.patch.object(telemetry_accounting, "_HAS_SQLALCHEMY", True) - # Replace ModelRegistry wholesale so the test works whether sqlalchemy is - # installed (real ModelRegistry) or not (the _ClassThatErrorsOnInit stub). fake_registry = SimpleNamespace(get_models=lambda: {object(), object()}) - mocker.patch.object(telemetry_accounting, "ModelRegistry", fake_registry) + mocker.patch.dict( + sys.modules, + {"reflex.model": SimpleNamespace(ModelRegistry=fake_registry)}, + ) features = telemetry_accounting._collect_features_used( _fake_app(), # pyright: ignore[reportArgumentType]