feat(bookmark): Support bookmarking input files (#1945)

Author: schloerke

MANIFEST.in

@@ -2,6 +2,7 @@ include LICENSE
 
 recursive-include tests *
 recursive-exclude * __pycache__
+recursive-exclude * shiny_bookmarks
 recursive-exclude * *.py[co]
 
 recursive-include docs *.rst conf.py Makefile make.bat *.jpg *.png *.gif

pyproject.toml

@@ -134,6 +134,10 @@ doc = [
     "griffe>=1.3.2",
 ]
 
+[tool.uv.sources]
+# https://github.com/encode/uvicorn/pull/2602
+uvicorn = { git = "https://github.com/schloerke/uvicorn", branch = "reload-exclude-abs-path" }
+
 [project.urls]
 Homepage = "https://github.com/posit-dev/py-shiny"
 Documentation = "https://shiny.posit.co/py/"

shiny/_main.py

@@ -21,6 +21,7 @@
 from . import __version__, _autoreload, _hostenv, _static, _utils
 from ._docstring import no_example
 from ._typing_extensions import NotRequired, TypedDict
+from .bookmark._bookmark_state import shiny_bookmarks_folder_name
 from .express import is_express_app
 from .express._utils import escape_to_var_name
 
@@ -44,7 +45,15 @@ def main() -> None:
     "*.yml",
     "*.yaml",
 )
-RELOAD_EXCLUDES_DEFAULT = (".*", "*.py[cod]", "__pycache__", "env", "venv")
+RELOAD_EXCLUDES_DEFAULT = (
+    ".*",
+    "*.py[cod]",
+    "__pycache__",
+    "env",
+    "venv",
+    ".venv",
+    shiny_bookmarks_folder_name,
+)
 
 
 @main.command(

shiny/bookmark/_bookmark_state.py

@@ -3,11 +3,13 @@
 import os
 from pathlib import Path
 
+shiny_bookmarks_folder_name = "shiny_bookmarks"
+
 
 def _local_dir(id: str) -> Path:
     # Try to save/load from current working directory as we do not know where the
     # app file is located
-    return Path(os.getcwd()) / "shiny_bookmarks" / id
+    return Path(os.getcwd()) / shiny_bookmarks_folder_name / id
 
 
 async def local_save_dir(id: str) -> Path:

shiny/bookmark/_serializers.py

@@ -1,11 +1,15 @@
 from __future__ import annotations
 
+import warnings
 from pathlib import Path
 from shutil import copyfile
-from typing import Any, TypeVar
+from typing import TYPE_CHECKING, Any, TypeVar
 
 from typing_extensions import TypeIs
 
+if TYPE_CHECKING:
+    from ..session import Session
+
 
 class Unserializable: ...
 
@@ -28,22 +32,64 @@ async def serializer_default(value: T, state_dir: Path | None) -> T:
     return value
 
 
-# TODO: Barret - Integrate
-def serializer_file_input(value: Any, state_dir: Path | None) -> Any | Unserializable:
+def serializer_file_input(
+    value: list[dict[str, str | int]], state_dir: Path | None
+) -> Any | Unserializable:
     if state_dir is None:
+        warnings.warn(
+            "`shiny.ui.input_file()` is attempting to save bookmark state. "
+            'However the App\'s `bookmark_store=` is not set to `"server"`. '
+            "Either exclude the input value (`session.bookmark.exclude.append(NAME)`) "
+            'or set `bookmark_store="server"`.',
+            UserWarning,
+            stacklevel=1,
+        )
         return Unserializable()
 
-    # TODO: Barret - Double check this logic!
-
-    # `value` is a data frame. When persisting files, we need to copy the file to
+    # `value` is a "data frame" (list of arrays). When persisting files, we need to copy the file to
     # the persistent dir and then strip the original path before saving.
-    datapath = Path(value["datapath"])
-    new_paths = state_dir / datapath.name
-
-    if new_paths.exists():
-        new_paths.unlink()
-    copyfile(datapath, new_paths)
 
-    value["datapath"] = new_paths.name
-
-    return value
+    if not isinstance(value, list):
+        raise ValueError(
+            f"Invalid value type for file input. Expected list, received: {type(value)}"
+        )
+
+    ret_file_infos = value.copy()
+
+    for i, file_info in enumerate(ret_file_infos):
+        if not isinstance(file_info, dict):
+            raise ValueError(
+                f"Invalid file info type for file input ({i}). "
+                f"Expected dict, received: {type(file_info)}"
+            )
+        if "datapath" not in file_info:
+            raise ValueError(f"Missing 'datapath' key in file info ({i}).")
+        if not isinstance(file_info["datapath"], str):
+            raise TypeError(
+                f"Invalid type for 'datapath' in file info ({i}). "
+                f"Expected str, received: {type(file_info['datapath'])}"
+            )
+
+        datapath = Path(file_info["datapath"])
+        new_path = state_dir / datapath.name
+        if new_path.exists():
+            new_path.unlink()
+        copyfile(datapath, new_path)
+
+        # Store back into the file_info dict to update `ret_file_infos`
+        file_info["datapath"] = new_path.name
+
+    return ret_file_infos
+
+
+def can_serialize_input_file(session: Session) -> bool:
+    """
+    Check if the session can serialize file input.
+
+    Args:
+        session (Session): The current session.
+
+    Returns:
+        bool: True if the session can serialize file input, False otherwise.
+    """
+    return session.bookmark.store == "server"

shiny/input_handler.py

@@ -1,9 +1,11 @@
 from __future__ import annotations
 
 from datetime import date, datetime, timezone
+from pathlib import Path
 from typing import TYPE_CHECKING, Any, Callable, Dict
 
 from .bookmark import serializer_unserializable
+from .bookmark._serializers import can_serialize_input_file, serializer_file_input
 
 if TYPE_CHECKING:
     from .session import Session
@@ -173,33 +175,80 @@ def _(value: Any, name: ResolvedId, session: Session) -> Any:
     if value is None:
         return None
 
-    # TODO: Barret: Input handler for file inputs
+    if not can_serialize_input_file(session):
+        raise ValueError(
+            "`shiny.ui.input_file()` is attempting to restore bookmark state. "
+            'However the App\'s `bookmark_store=` is not set to `"server"`. '
+            "Either exclude the input value (`session.bookmark.exclude.append(NAME)`) "
+            'or set `bookmark_store="server"`.'
+        )
 
-    # # The data will be a named list of lists; convert to a data frame.
-    # val <- as.data.frame(lapply(val, unlist), stringsAsFactors = FALSE)
+    value_obj = value
+
+    # Convert from:
+    # `{name: (n1, n2, n3), size: (s1, s2, s3), type: (t1, t2, t3), datapath: (d1, d2, d3)}`
+    # to:
+    # `[{name: n1, size: s1, type: t1, datapath: d1}, ...]`
+    value_list: list[dict[str, str | int | None]] = []
+    for i in range(len(value_obj["name"])):
+        value_list.append(
+            {
+                "name": value_obj["name"][i],
+                "size": value_obj["size"][i],
+                "type": value_obj["type"][i],
+                "datapath": value_obj["datapath"][i],
+            }
+        )
 
-    # # `val$datapath` should be a filename without a path, for security reasons.
-    # if (basename(val$datapath) != val$datapath) {
-    #   stop("Invalid '/' found in file input path.")
-    # }
+    # Validate the input value
+    for value_item in value_list:
+        if value_item["datapath"] is not None:
+            if not isinstance(value_item["datapath"], str):
+                raise ValueError(
+                    "Invalid type for file input path: ", type(value_item["datapath"])
+                )
+            if Path(value_item["datapath"]).name != value_item["datapath"]:
+                raise ValueError("Invalid '/' found in file input path.")
 
-    # # Prepend the persistent dir
-    # oldfile <- file.path(getCurrentRestoreContext()$dir, val$datapath)
+    import shutil
+    import tempfile
 
-    # # Copy the original file to a new temp dir, so that a restored session can't
-    # # modify the original.
-    # newdir <- file.path(tempdir(), createUniqueId(12))
-    # dir.create(newdir)
-    # val$datapath <- file.path(newdir, val$datapath)
-    # file.copy(oldfile, val$datapath)
+    from shiny._utils import rand_hex
 
-    # # Need to mark this input value with the correct serializer. When a file is
-    # # uploaded the usual way (instead of being restored), this occurs in
-    # # session$`@uploadEnd`.
-    # setSerializer(name, serializerFileInput)
+    from .bookmark._restore_state import get_current_restore_context
+    from .session import session_context
 
-    # snapshotPreprocessInput(name, snapshotPreprocessorFileInput)
+    with session_context(session):
+        restore_ctx = get_current_restore_context()
 
-    # val
+    # These should not fail as we know
+    if restore_ctx is None or restore_ctx.dir is None:
+        raise RuntimeError("No restore context found. Cannot restore file input.")
 
-    return value
+    restore_ctx_dir = Path(restore_ctx.dir)
+
+    if len(value_list) > 0:
+        tempdir_root = tempfile.TemporaryDirectory()
+        session.on_ended(lambda: tempdir_root.cleanup())
+
+        for f in value_list:
+            assert f["datapath"] is not None and isinstance(f["datapath"], str)
+
+            data_path = f["datapath"]
+
+            # Prepend the persistent dir
+            old_file = restore_ctx_dir / data_path
+
+            # Copy the original file to a new temp dir, so that a restored session can't
+            # modify the original.
+            tempdir = Path(tempdir_root.name) / rand_hex(12)
+            tempdir.mkdir(parents=True, exist_ok=True)
+            f["datapath"] = str(tempdir / Path(data_path).name)
+            shutil.copy2(old_file, f["datapath"])
+
+    # Need to mark this input value with the correct serializer. When a file is
+    # uploaded the usual way (instead of being restored), this occurs in
+    # session$`@uploadEnd`.
+    session.input.set_serializer(name, serializer_file_input)
+
+    return value_list

shiny/session/_session.py

@@ -46,6 +46,7 @@
 from ..bookmark import BookmarkApp, BookmarkProxy
 from ..bookmark._button import BOOKMARK_ID
 from ..bookmark._restore_state import RestoreContext
+from ..bookmark._serializers import serializer_file_input
 from ..http_staticfiles import FileResponse
 from ..input_handler import input_handlers
 from ..module import ResolvedId
@@ -824,6 +825,10 @@ async def uploadEnd(job_id: str, input_id: str) -> None:
             # by wrapping it in ResolvedId, otherwise self.input will throw an id
             # validation error.
             self.input[ResolvedId(input_id)]._set(file_data)
+
+            # This also occurs during input handler: shiny.file
+            self.input.set_serializer(input_id, serializer_file_input)
+
             # Explicitly return None to signal that the message was handled.
             return None
 

shiny/ui/_input_file.py

@@ -1,15 +1,18 @@
 from __future__ import annotations
 
-__all__ = ("input_file",)
-
+import warnings
 from typing import Literal, Optional
 
 from htmltools import Tag, TagChild, css, div, span, tags
 
 from .._docstring import add_example
+from ..bookmark import restore_input
+from ..bookmark._utils import to_json_str
 from ..module import resolve_id
 from ._utils import shiny_input_label
 
+__all__ = ("input_file",)
+
 
 @add_example()
 def input_file(
@@ -83,6 +86,28 @@ def input_file(
         accept = [accept]
 
     resolved_id = resolve_id(id)
+    restored_value = restore_input(resolved_id, default=None)
+
+    if restored_value is not None:
+        restored_obj: dict[str, list[str | int | None]] = {
+            "name": [],
+            "size": [],
+            "type": [],
+            "datapath": [],
+        }
+        try:
+            for file in restored_value:
+                restored_obj["name"].append(file.get("name", None))
+                restored_obj["size"].append(file.get("size", None))
+                restored_obj["type"].append(file.get("type", None))
+                restored_obj["datapath"].append(file.get("datapath", None))
+            restored_value = to_json_str(restored_obj)
+        except Exception:
+            warnings.warn(
+                f"Error while restoring file input value for `{resolved_id}`. Resetting to `None`.",
+                stacklevel=1,
+            )
+
     btn_file = span(
         button_label,
         tags.input(
@@ -95,6 +120,7 @@ def input_file(
             # Don't use "display: none;" style, which causes keyboard accessibility issue; instead use the following workaround: https://css-tricks.com/places-its-tempting-to-use-display-none-but-dont/
             style="position: absolute !important; top: -99999px !important; left: -99999px !important;",
             class_="shiny-input-file",
+            **({"data-restore": restored_value} if restored_value else {}),
         ),
         class_="btn btn-default btn-file",
     )

tests/playwright/ai_generated_apps/bookmark/input_file/app-express.py

@@ -1,6 +1,6 @@
 from shiny.express import app_opts, input, module, render, session, ui
 
-app_opts(bookmark_store="url")
+app_opts(bookmark_store="server")
 
 with ui.card():
     ui.card_header("Bookmarking File Input Demo")

tests/playwright/ai_generated_apps/bookmark/input_file/test_input_file_express_bookmarking.py

@@ -1,4 +1,3 @@
-import pytest
 from playwright.sync_api import FilePayload, Page
 
 from shiny.playwright import controller
@@ -8,7 +7,6 @@
 app = create_app_fixture(["app-express.py"])
 
 
-@pytest.mark.skip("Broken test! TODO: Barret")
 def test_file_input_bookmarking(page: Page, app: ShinyAppProc) -> None:
     page.goto(app.url)