Skip to content

Commit

Permalink
Support multiple path characteristics for switch_page and page_link (#…
Browse files Browse the repository at this point in the history
…8127)

## Describe your changes

The changes made improve page path calculation, but don't support file
path scenarios with `..`, etc. This change aims to clean up the logic
and ensure it's consistent.

## Testing Plan

- Unit Tests - Tested the file path methods to cover all the use cases
- Manual Testing - Testing on Windows and Mac

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
kmcgrady committed Feb 9, 2024
1 parent 4692bbf commit cf2bc07
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 20 deletions.
14 changes: 4 additions & 10 deletions lib/streamlit/commands/execution_control.py
Expand Up @@ -19,6 +19,7 @@
from streamlit import source_util
from streamlit.deprecation_util import make_deprecated_name_warning
from streamlit.errors import NoSessionContext, StreamlitAPIException
from streamlit.file_util import get_main_script_directory, normalize_path_join
from streamlit.logger import get_logger
from streamlit.runtime.metrics_util import gather_metrics
from streamlit.runtime.scriptrunner import RerunData, get_script_run_ctx
Expand Down Expand Up @@ -138,22 +139,15 @@ def switch_page(page: str) -> NoReturn: # type: ignore[misc]
# This should never be the case
raise NoSessionContext()

normalized_ctx_main_script_path = os.path.normpath(ctx.main_script_path)
main_script_path = os.path.join(os.getcwd(), normalized_ctx_main_script_path)
main_script_directory = os.path.dirname(main_script_path)

# Convenience for handling ./ notation
page = os.path.normpath(page)

# Build full path
requested_page = os.path.join(main_script_directory, page)
main_script_directory = get_main_script_directory(ctx.main_script_path)
requested_page = os.path.realpath(normalize_path_join(main_script_directory, page))
all_app_pages = source_util.get_pages(ctx.main_script_path).values()

matched_pages = [p for p in all_app_pages if p["script_path"] == requested_page]

if len(matched_pages) == 0:
raise StreamlitAPIException(
f"Could not find page: '{page}'. Must be the file path relative to the main script, from the directory: {os.path.basename(main_script_directory)}. Only the main app file and files in the `pages/` directory are supported."
f"Could not find page: `{page}`. Must be the file path relative to the main script, from the directory: `{os.path.basename(main_script_directory)}`. Only the main app file and files in the `pages/` directory are supported."
)

ctx.script_requests.request_rerun(
Expand Down
16 changes: 6 additions & 10 deletions lib/streamlit/elements/widgets/button.py
Expand Up @@ -26,6 +26,7 @@
from streamlit.elements.form import current_form_id, is_in_form
from streamlit.elements.utils import check_callback_rules, check_session_state_rules
from streamlit.errors import StreamlitAPIException
from streamlit.file_util import get_main_script_directory, normalize_path_join
from streamlit.proto.Button_pb2 import Button as ButtonProto
from streamlit.proto.DownloadButton_pb2 import DownloadButton as DownloadButtonProto
from streamlit.proto.LinkButton_pb2 import LinkButton as LinkButtonProto
Expand Down Expand Up @@ -664,15 +665,10 @@ def _page_link(
if ctx:
ctx_main_script = ctx.main_script_path

normalized_ctx_main_script_path = os.path.normpath(ctx_main_script)
main_script_path = os.path.join(os.getcwd(), normalized_ctx_main_script_path)
main_script_directory = os.path.dirname(main_script_path)

# Convenience for handling ./ notation
page = os.path.normpath(page)

# Build full path
requested_page = os.path.join(main_script_directory, page)
main_script_directory = get_main_script_directory(ctx_main_script)
requested_page = os.path.realpath(
normalize_path_join(main_script_directory, page)
)
all_app_pages = source_util.get_pages(ctx_main_script).values()

# Handle retrieving the page_script_hash & page
Expand All @@ -688,7 +684,7 @@ def _page_link(

if page_link_proto.page_script_hash == "":
raise StreamlitAPIException(
f"Could not find page: '{page}'. Must be the file path relative to the main script, from the directory: {os.path.basename(main_script_directory)}. Only the main app file and files in the `pages/` directory are supported."
f"Could not find page: `{page}`. Must be the file path relative to the main script, from the directory: `{os.path.basename(main_script_directory)}`. Only the main app file and files in the `pages/` directory are supported."
)

return self.dg._enqueue("page_link", page_link_proto)
Expand Down
35 changes: 35 additions & 0 deletions lib/streamlit/file_util.py
Expand Up @@ -207,3 +207,38 @@ def file_in_pythonpath(filepath) -> bool:
file_is_in_folder_glob(os.path.normpath(filepath), path)
for path in absolute_paths
)


def normalize_path_join(*args):
"""Return the normalized path of the joined path.
Parameters
----------
*args : str
The path components to join.
Returns
-------
str
The normalized path of the joined path.
"""
return os.path.normpath(os.path.join(*args))


def get_main_script_directory(main_script):
"""Return the full path to the main script directory.
Parameters
----------
main_script : str
The main script path. The path can be an absolute path or a relative
path.
Returns
-------
str
The full path to the main script directory.
"""
main_script_path = normalize_path_join(os.getcwd(), main_script)

return os.path.dirname(main_script_path)
63 changes: 63 additions & 0 deletions lib/tests/streamlit/file_util_test.py
Expand Up @@ -270,3 +270,66 @@ def test_current_directory(self):
self._make_it_absolute("../something_else/module")
)
)

def test_get_main_script_directory(self):
"""Test file_util.get_main_script_directory."""
with patch("os.getcwd", return_value="/some/random"):
self.assertEqual(
file_util.get_main_script_directory("app.py"),
"/some/random",
)
self.assertEqual(
file_util.get_main_script_directory("./app.py"),
"/some/random",
)
self.assertEqual(
file_util.get_main_script_directory("../app.py"),
"/some",
)
self.assertEqual(
file_util.get_main_script_directory("/path/to/my/app.py"),
"/path/to/my",
)

def test_normalize_path_join(self):
"""Test file_util.normalize_path_join."""
self.assertEqual(
file_util.normalize_path_join("/some", "random", "path"),
"/some/random/path",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "path"),
"some/random/path",
)
self.assertEqual(
file_util.normalize_path_join("/some", "random", "./path"),
"/some/random/path",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "./path"),
"some/random/path",
)
self.assertEqual(
file_util.normalize_path_join("/some", "random", "../path"),
"/some/path",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "../path"),
"some/path",
)
self.assertEqual(
file_util.normalize_path_join("/some", "random", "/path"),
"/path",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "/path"),
"/path",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "path", ".."),
"some/random",
)
self.assertEqual(
file_util.normalize_path_join("some", "random", "path", "../.."),
"some",
)

0 comments on commit cf2bc07

Please sign in to comment.