Skip to content

Commit

Permalink
Don't resolve id's more than once and keep track of namespaces in a list
Browse files Browse the repository at this point in the history
  • Loading branch information
cpsievert committed May 18, 2022
1 parent 9e01e4f commit 6cc2a44
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 36 deletions.
13 changes: 9 additions & 4 deletions shiny/_modules.py
@@ -1,5 +1,6 @@
__all__ = ("namespaced_id", "module_ui", "module_server")

import copy
import sys
from typing import Any, Callable, TypeVar

Expand All @@ -26,7 +27,8 @@ class ModuleInputs(Inputs):
"""

def __init__(self, ns: str, parent_inputs: Inputs):
self._ns = namespaced_id(ns, parent_inputs._ns) # Support nested modules
self._namespaces = copy.copy(parent_inputs._namespaces)
self._namespaces.insert(0, ns)
# Don't set _parent attribute like the other classes since Inputs redefines
# __setattr__
self._map = parent_inputs._map
Expand All @@ -46,7 +48,8 @@ class ModuleOutputs(Outputs):
"""

def __init__(self, ns: str, parent_outputs: Outputs):
self._ns = namespaced_id(ns, parent_outputs._ns) # Support nested modules
self._namespaces = copy.copy(parent_outputs._namespaces)
self._namespaces.insert(0, ns)
self._parent = parent_outputs

def __getattr__(self, attr: str) -> Any:
Expand All @@ -67,7 +70,8 @@ class ModuleSession(Session):
"""

def __init__(self, ns: str, parent_session: Session):
self._ns: str = namespaced_id(ns, parent_session._ns) # Support nested modules
self._namespaces = copy.copy(parent_session._namespaces)
self._namespaces.append(ns)
self._parent: Session = parent_session
self.input: ModuleInputs = ModuleInputs(ns, parent_session.input)
self.output: ModuleOutputs = ModuleOutputs(ns, parent_session.output)
Expand All @@ -78,7 +82,7 @@ def __getattr__(self, attr: str) -> Any:

class MockModuleSession(ModuleSession):
def __init__(self, ns: str):
self._ns = ns
self._namespaces = [ns]


P = ParamSpec("P")
Expand All @@ -87,6 +91,7 @@ def __init__(self, ns: str):

def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]:
def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R:
# TODO: what should happen if this is called *inside* of a session? Do we pass in the parent session?
with session_context(MockModuleSession(ns)):
return fn(*args, **kwargs)

Expand Down
33 changes: 15 additions & 18 deletions shiny/_namespaces.py
@@ -1,34 +1,31 @@
__all__ = ("namespaced_id",)

from typing import Union, Optional
from typing import Union, List

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 :func:`module_ui`/:func:`module_server`
namespace.

Parameters
----------
id
The ID to namespace..
"""
if isinstance(ns, MISSING_TYPE):
ns = get_current_namespace()
Id = Union[str, ResolvedId]

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]:
def get_current_namespaces() -> List[str]:
from .session import get_current_session

session = get_current_session()
if session is None:
return None
return []
else:
return session._ns
return session._namespaces
25 changes: 14 additions & 11 deletions shiny/session/_session.py
Expand Up @@ -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
Expand Down Expand Up @@ -143,7 +143,7 @@ def __init__(
self.input: Inputs = Inputs()
self.output: Outputs = Outputs(self)

self._ns: Optional[str] = None # Only relevant for ModuleSession
self._namespaces: List[str] = [] # i.e., ModuleSession namespaces

self.user: Union[str, None] = None
self.groups: Union[List[str], None] = None
Expand Down Expand Up @@ -479,7 +479,10 @@ 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": namespaced_id_ns(id, self._namespaces),
"message": message,
}
self._outbound_message_queues["input_messages"].append(msg)
self._request_flush()

Expand Down Expand Up @@ -744,16 +747,16 @@ def __init__(self, **kwargs: object) -> None:
for key, value in kwargs.items():
self._map[key] = Value(value, read_only=True)

self._ns: Optional[str] = None # Only relevant for ModuleInputs()
self._namespaces: List[str] = [] # i.e. ModuleInputs() namespaces

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[namespaced_id_ns(key, self._namespaces)] = value

def __getitem__(self, key: str) -> Value[Any]:
key = namespaced_id(key, self._ns)
key = namespaced_id_ns(key, self._namespaces)
# 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.
Expand All @@ -763,18 +766,18 @@ def __getitem__(self, key: str) -> Value[Any]:
return self._map[key]

def __delitem__(self, key: str) -> None:
del self._map[namespaced_id(key, self._ns)]
del self._map[namespaced_id_ns(key, self._namespaces)]

# Allow access of values as attributes.
def __setattr__(self, attr: str, value: Value[Any]) -> None:
if attr in ("_map", "_ns"):
if attr in ("_map", "_namespaces"):
super().__setattr__(attr, value)
return

self.__setitem__(attr, value)

def __getattr__(self, attr: str) -> Value[Any]:
if attr in ("_map", "_ns"):
if attr in ("_map", "_namespaces"):
return object.__getattribute__(self, attr)
return self.__getitem__(attr)

Expand All @@ -801,7 +804,7 @@ 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()
self._namespaces: List[str] = [] # i.e., ModuleOutputs() namespaces

def __call__(
self,
Expand All @@ -812,7 +815,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 = namespaced_id_ns(name or fn.__name__, self._namespaces)

# 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
Expand Down
5 changes: 2 additions & 3 deletions tests/test_modules.py
Expand Up @@ -78,7 +78,6 @@ async def test_inputs_proxy():
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():
Expand Down Expand Up @@ -129,12 +128,12 @@ def _():
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_current"]._namespaces == ["mod_outer", "mod_inner"]

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 sessions["outer_current"]._namespaces == ["mod_outer"]

assert sessions["top"] is sessions["top_current"]
assert sessions["top_current"] is sessions["top_calc_current"]
Expand Down

0 comments on commit 6cc2a44

Please sign in to comment.