From c1a2e83a5ea4c3cd735d480d0e3e91d35bfff772 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 20 May 2022 17:10:19 -0500 Subject: [PATCH] Move away from ModuleSession/ModuleInput/ModuleOutput toward SessionProxy --- shiny/_modules.py | 150 ++++---------------------------------- shiny/_namespaces.py | 51 +++++++------ shiny/session/_session.py | 94 ++++++++++++++++++------ tests/test_modules.py | 109 ++++++++------------------- 4 files changed, 143 insertions(+), 261 deletions(-) diff --git a/shiny/_modules.py b/shiny/_modules.py index 16702db76..8d81672fe 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -1,90 +1,24 @@ __all__ = ("namespaced_id", "module_ui", "module_server") -from typing import Any, Callable, Optional, TypeVar, ParamSpec, Concatenate +import sys +from typing import Callable, TypeVar -from htmltools import TagChildArg +if sys.version_info < (3, 10): + from typing_extensions import ParamSpec, Concatenate +else: + from typing import ParamSpec, Concatenate -from ._docstring import add_example -from ._namespaces import namespaced_id +from ._namespaces import namespaced_id, namespace_context, get_current_namespaces from .session import Inputs, Outputs, Session, require_active_session, session_context - -class ModuleInputs(Inputs): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_inputs: Inputs): - self._ns = namespaced_id(ns, parent_inputs._ns) # Support nested modules - # Don't set _parent attribute like the other classes since Inputs redefines - # __setattr__ - self._map = parent_inputs._map - - -class ModuleOutputs(Outputs): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_outputs: Outputs): - self._ns = namespaced_id(ns, parent_outputs._ns) # Support nested modules - self._parent = parent_outputs - - def __getattr__(self, attr: str) -> Any: - return getattr(self._parent, attr) - - -class ModuleSession(Session): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_session: Session): - self._ns: str = namespaced_id(ns, parent_session._ns) # Support nested modules - self._parent: Session = parent_session - self.input: ModuleInputs = ModuleInputs(ns, parent_session.input) - self.output: ModuleOutputs = ModuleOutputs(ns, parent_session.output) - - def __getattr__(self, attr: str) -> Any: - return getattr(self._parent, attr) - - -class MockModuleSession(ModuleSession): - def __init__(self, ns: str): - self._ns = ns - - P = ParamSpec("P") R = TypeVar("R") def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - with session_context(MockModuleSession(ns)): + # TODO: what should happen if this is called *inside* of a session? Do we tack on the parent session's namespace as well? + with namespace_context(get_current_namespaces() + [ns]): return fn(*args, **kwargs) return wrapper @@ -94,67 +28,9 @@ def module_server( fn: Callable[Concatenate[Inputs, Outputs, Session, P], R] ) -> Callable[Concatenate[str, P], R]: def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - mod_sess = ModuleSession(ns, require_active_session(None)) - with session_context(mod_sess): - return fn(mod_sess.input, mod_sess.output, mod_sess, *args, **kwargs) + sess = require_active_session(None) + child_sess = sess.make_scope(ns) + with session_context(child_sess): + return fn(child_sess.input, child_sess.output, child_sess, *args, **kwargs) return wrapper - - -# @add_example() -# class Module: -# """ -# Modularize UI and server-side logic. -# -# Parameters -# ---------- -# ui -# The module's UI definition. -# server -# The module's server-side logic. -# """ -# -# def __init__( -# self, -# ui: Callable[..., TagChildArg], -# server: Callable[[ModuleInputs, ModuleOutputs, ModuleSession], None], -# ) -> None: -# self._ui: Callable[..., TagChildArg] = ui -# self._server: Callable[ -# [ModuleInputs, ModuleOutputs, ModuleSession], None -# ] = server -# -# def ui(self, ns: str, *args: Any) -> TagChildArg: -# """ -# Render the module's UI. -# -# Parameters -# ---------- -# namespace -# A namespace for the module. -# args -# Additional arguments to pass to the module's UI definition. -# """ -# -# # Create a fake session so that namespaced_id() knows -# # what the relevant namespace is -# with session_context(MockModuleSession(ns)): -# return self._ui(*args) -# -# def server(self, ns: str, *, session: Optional[Session] = None) -> None: -# """ -# Invoke the module's server-side logic. -# -# Parameters -# ---------- -# ns -# A namespace for the module. -# session -# A :class:`~shiny.Session` instance. If not provided, it is inferred via -# :func:`~shiny.session.get_current_session`. -# """ -# -# mod_sess = ModuleSession(ns, require_active_session(session)) -# with session_context(mod_sess): -# return self._server(mod_sess.input, mod_sess.output, mod_sess) -# diff --git a/shiny/_namespaces.py b/shiny/_namespaces.py index c6c876167..38204f8e2 100644 --- a/shiny/_namespaces.py +++ b/shiny/_namespaces.py @@ -1,34 +1,39 @@ -# TODO: make this available under the shiny.modules API -__all__ = ("namespaced_id",) +from contextlib import contextmanager +from contextvars import ContextVar, Token +from typing import Union, List -from typing import Union, Optional -from .types import MISSING, MISSING_TYPE +class ResolvedId(str): + pass -def namespaced_id(id: str, ns: Union[str, MISSING_TYPE, None] = MISSING) -> str: - """ - Namespace an ID based on the current ``Module()``'s namespace. +Id = Union[str, ResolvedId] - Parameters - ---------- - id - The ID to namespace.. - """ - if isinstance(ns, MISSING_TYPE): - ns = get_current_namespace() - if ns is None: +def namespaced_id(id: str) -> str: + return namespaced_id_ns(id, get_current_namespaces()) + + +def namespaced_id_ns(id: Id, namespaces: List[str] = []) -> str: + if isinstance(id, ResolvedId) or len(namespaces) == 0: return id else: - return ns + "_" + id + return ResolvedId("_".join(namespaces) + "_" + id) -def get_current_namespace() -> Optional[str]: - from .session import get_current_session +def get_current_namespaces() -> List[str]: + return _current_namespaces.get() - session = get_current_session() - if session is None: - return None - else: - return session._ns + +_current_namespaces: ContextVar[List[str]] = ContextVar( + "current_namespaces", default=[] +) + + +@contextmanager +def namespace_context(namespaces: List[str]): + token: Token[List[str]] = _current_namespaces.set(namespaces) + try: + yield + finally: + _current_namespaces.reset(token) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 4002f01db..f3e483e51 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -50,7 +50,7 @@ from .._fileupload import FileInfo, FileUploadManager from ..http_staticfiles import FileResponse from ..input_handler import input_handlers -from .._namespaces import namespaced_id +from .._namespaces import namespaced_id_ns from ..reactive import Value, Effect, Effect_, isolate, flush from ..reactive._core import lock from ..types import SafeException, SilentCancelOutputException, SilentException @@ -113,7 +113,14 @@ def empty_outbound_message_queues() -> OutBoundMessageQueues: return {"values": [], "input_messages": [], "errors": []} -class Session: +# Makes isinstance(x, Session) also return True when x is a SessionProxy (i.e., a module +# session) +class SessionMeta(type): + def __instancecheck__(self, __instance: Any) -> bool: + return isinstance(__instance, SessionProxy) + + +class Session(object, metaclass=SessionMeta): """ A class representing a user session. @@ -143,8 +150,6 @@ def __init__( self.input: Inputs = Inputs() self.output: Outputs = Outputs(self) - self._ns: Optional[str] = None # Only relevant for ModuleSession - self.user: Union[str, None] = None self.groups: Union[List[str], None] = None credentials_json: str = "" @@ -479,7 +484,7 @@ def send_input_message(self, id: str, message: Dict[str, object]) -> None: message The message to send. """ - msg: Dict[str, object] = {"id": namespaced_id(id, self._ns), "message": message} + msg: Dict[str, object] = {"id": id, "message": message} self._outbound_message_queues["input_messages"].append(msg) self._request_flush() @@ -631,7 +636,7 @@ async def _unhandled_error(self, e: Exception) -> None: await self.close() # TODO: probably name should be id - # TODO: anything to be done here for module support? + @add_example() def download( self, @@ -720,6 +725,46 @@ def _process_ui(self, ui: TagChildArg) -> RenderedDeps: return {"deps": deps, "html": res["html"]} + @staticmethod + def ns(id: str) -> str: + return id + + def make_scope(self, id: str) -> "Session": + ns = create_ns_func(id) + return SessionProxy(parent=self, ns=ns) # type: ignore + + +class SessionProxy: + def __init__(self, parent: Session, ns: Callable[[str], str]) -> None: + self._parent = parent + self.ns = ns + self.input = Inputs(values=parent.input._map, ns=ns) + self.output = Outputs( + session=cast(Session, self), + effects=self.output._effects, + suspend_when_hidden=self.output._suspend_when_hidden, + ns=ns, + ) + + def __getattr__(self, attr: str) -> Any: + return getattr(self._parent, attr) + + def send_input_message(self, id: str, message: Dict[str, object]) -> None: + return self._parent.send_input_message(self.ns(id), message) + + # TODO: test this actually works (will handle_request() need an override?) + def download( + self, name: str, **kwargs: object + ) -> Callable[[DownloadHandler], None]: + return self._parent.download(self.ns(name), **kwargs) + + def make_scope(self, id: str) -> Session: + return self._parent.make_scope(self.ns(id)) + + +def create_ns_func(namespace: str) -> Callable[[str], str]: + return lambda x: namespaced_id_ns(x, [namespace]) + # ====================================================================================== # Inputs @@ -739,31 +784,30 @@ class Inputs: for type checking reasons). """ - def __init__(self, **kwargs: object) -> None: - self._map: dict[str, Value[Any]] = {} - for key, value in kwargs.items(): - self._map[key] = Value(value, read_only=True) - - self._ns: Optional[str] = None # Only relevant for ModuleInputs() + def __init__( + self, values: Dict[str, Value[Any]] = {}, ns: Callable[[str], str] = lambda x: x + ) -> None: + self._map = values + self._ns = ns def __setitem__(self, key: str, value: Value[Any]) -> None: if not isinstance(value, Value): raise TypeError("`value` must be a reactive.Value object.") - self._map[namespaced_id(key, self._ns)] = value + self._map[self._ns(key)] = value def __getitem__(self, key: str) -> Value[Any]: - key = namespaced_id(key, self._ns) + key = self._ns(key) # Auto-populate key if accessed but not yet set. Needed to take reactive # dependencies on input values that haven't been received from client # yet. if key not in self._map: - self._map[key] = Value(read_only=True) + self._map[key] = cast(Value[Any], Value(read_only=True)) return self._map[key] def __delitem__(self, key: str) -> None: - del self._map[namespaced_id(key, self._ns)] + del self._map[self._ns(key)] # Allow access of values as attributes. def __setattr__(self, attr: str, value: Value[Any]) -> None: @@ -797,11 +841,17 @@ class Outputs: for type checking reasons). """ - def __init__(self, session: Session) -> None: - self._effects: Dict[str, Effect_] = {} - self._suspend_when_hidden: Dict[str, bool] = {} - self._session: Session = session - self._ns: Optional[str] = None # Only relevant for ModuleOutputs() + def __init__( + self, + session: Session, + ns: Callable[[str], str] = lambda x: x, + effects: Dict[str, Effect_] = {}, + suspend_when_hidden: Dict[str, bool] = {}, + ) -> None: + self._session = session + self._ns = ns + self._effects = effects + self._suspend_when_hidden = suspend_when_hidden def __call__( self, @@ -812,7 +862,7 @@ def __call__( ) -> Callable[[render.RenderFunction], None]: def set_fn(fn: render.RenderFunction) -> None: # Get the (possibly namespaced) output id - fn_name = namespaced_id(name or fn.__name__, self._ns) + fn_name = self._ns(name or fn.__name__) # fn is either a regular function or a RenderFunction object. If # it's the latter, we can give it a bit of metadata, which can be diff --git a/tests/test_modules.py b/tests/test_modules.py index 769a3af36..25790646a 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -2,90 +2,41 @@ from typing import Dict, Union, cast -import pytest from shiny import * from shiny.session import get_current_session from shiny._connection import MockConnection -from shiny._modules import ModuleInputs, ModuleSession from shiny._utils import run_coro_sync -from htmltools import TagChildArg +from htmltools import TagList, Tag -def mod_ui() -> TagChildArg: - return ui.TagList( - ui.input_action_button(id="button", label="module1"), - ui.output_text_verbatim(id="out"), +@module_ui +def mod_inner() -> TagList: + return TagList( + ui.input_action_button("button", label="inner"), + ui.output_text("out"), ) -# Note: We currently can't test Session; this is just here for future use. -def mod_server(input: Inputs, output: Outputs, session: Session): - count: reactive.Value[int] = reactive.Value(0) - - @reactive.Effect() - @event(session.input.button) - def _(): - count.set(count() + 1) - - @output() - @render_text() - def out() -> str: - return f"Click count is {count()}" - - -mod = Module(mod_ui, mod_server) +@module_ui +def mod_outer() -> TagList: + return TagList(mod_inner("inner"), ui.output_text("out2")) def test_module_ui(): - x = cast(ui.TagList, mod.ui("mod1")) - assert cast(ui.Tag, x[0]).attrs["id"] == "mod1_button" - assert cast(ui.Tag, x[1]).attrs["id"] == "mod1_out" - - -@pytest.mark.asyncio -async def test_inputs_proxy(): - input = Inputs(a=1) - input_proxy = ModuleInputs("mod1", input) - - with reactive.isolate(): - assert input.a() == 1 - # Different ways of accessing "a" from the input proxy. - assert input_proxy.a.is_set() is False - assert input_proxy["a"].is_set() is False - assert input["mod1_a"].is_set() is False + x = mod_inner("inner") + assert cast(Tag, x[0]).attrs["id"] == "inner_button" + assert cast(Tag, x[1]).attrs["id"] == "inner_out" + y = mod_outer("outer") + assert cast(Tag, y[0]).attrs["id"] == "outer_inner_button" + assert cast(Tag, y[1]).attrs["id"] == "outer_inner_out" + assert cast(Tag, y[2]).attrs["id"] == "outer_out2" - input_proxy.a._set(2) - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - assert input_proxy["a"]() == 2 - assert input["mod1_a"]() == 2 +def test_session_scoping(): - # Nested input proxies - input_proxy_proxy = ModuleInputs("mod2", input_proxy) - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - # Different ways of accessing "a" from the input proxy. - assert input_proxy_proxy.a.is_set() is False - assert input_proxy_proxy["a"].is_set() is False - assert input_proxy["mod1_a"].is_set() is False - - input_proxy_proxy.a._set(3) - - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - assert input_proxy_proxy.a() == 3 - assert input_proxy_proxy["a"]() == 3 - assert input["mod1_mod2_a"]() == 3 - - -def test_current_session(): - - sessions: Dict[str, Union[Session, None]] = {} + sessions: Dict[str, Union[Session, None, str]] = {} + @module_server def inner(input: Inputs, output: Outputs, session: Session): @reactive.Calc() def out(): @@ -96,9 +47,9 @@ def _(): sessions["inner"] = session sessions["inner_current"] = get_current_session() sessions["inner_calc_current"] = out() + sessions["inner_id"] = session.ns("foo") - mod_inner = Module(ui.TagList, inner) - + @module_server def outer(input: Inputs, output: Outputs, session: Session): @reactive.Calc() def out(): @@ -106,15 +57,14 @@ def out(): @reactive.Effect() def _(): - mod_inner.server("mod_inner") + inner("mod_inner") sessions["outer"] = session sessions["outer_current"] = get_current_session() sessions["outer_calc_current"] = out() - - mod_outer = Module(ui.TagList, outer) + sessions["outer_id"] = session.ns("foo") def server(input: Inputs, output: Outputs, session: Session): - mod_outer.server("mod_outer") + outer("mod_outer") @reactive.Calc() def out(): @@ -125,21 +75,22 @@ def _(): sessions["top"] = session sessions["top_current"] = get_current_session() sessions["top_calc_current"] = out() + sessions["top_id"] = session.ns("foo") App(ui.TagList(), server)._create_session(MockConnection()) run_coro_sync(reactive.flush()) assert sessions["inner"] is sessions["inner_current"] assert sessions["inner_current"] is sessions["inner_calc_current"] - assert isinstance(sessions["inner_current"], ModuleSession) - assert sessions["inner_current"]._ns == "mod_outer_mod_inner" + assert sessions["inner_id"] == "mod_outer_mod_inner_foo" + assert isinstance(sessions["inner_current"], Session) assert sessions["outer"] is sessions["outer_current"] assert sessions["outer_current"] is sessions["outer_calc_current"] - assert isinstance(sessions["outer_current"], ModuleSession) - assert sessions["outer_current"]._ns == "mod_outer" + assert isinstance(sessions["outer_current"], Session) + assert sessions["outer_id"] == "mod_outer_foo" assert sessions["top"] is sessions["top_current"] assert sessions["top_current"] is sessions["top_calc_current"] + assert sessions["top_id"] == "foo" assert isinstance(sessions["top_current"], Session) - assert not isinstance(sessions["top_current"], ModuleSession)