From 68c400a9464d87347331fef138e2636fffc82bf8 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 16:13:35 -0400 Subject: [PATCH 1/7] Simplify URI, add windows tests, add host and name as properties --- README.md | 52 +++++--- src/janelia_pathlib/__init__.py | 2 - src/janelia_pathlib/_macos.py | 12 +- src/janelia_pathlib/_path.py | 35 +++--- src/janelia_pathlib/_registry.py | 173 +++++++++++++++----------- src/janelia_pathlib/_windows.py | 10 +- tests/test_janelia_path.py | 200 ++++++++++++++++++++++++------- 7 files changed, 323 insertions(+), 161 deletions(-) diff --git a/README.md b/README.md index 62f4635..cf11083 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,9 @@ [![codecov](https://codecov.io/gh/JaneliaSciComp/janelia_pathlib/branch/main/graph/badge.svg)](https://codecov.io/gh/JaneliaSciComp/janelia_pathlib) [![PyPI](https://img.shields.io/pypi/v/janelia-pathlib.svg)](https://pypi.org/project/janelia-pathlib) -Translate Janelia file share paths to and from an unambiguous `janelia://` URI, so code can refer to a share without depending on OS-specific mount guesses. +Translate Janelia file share paths to and from an unambiguous `janelia:` URI, so code can refer to a share without depending on OS-specific mount guesses. -Janelia file shares are mounted at different paths on Linux, Mac, and Windows, and on macOS two different shares can even collide on the same guessed mount point (e.g. `/Volumes/`). This package gives you two functions to move between a local path and a stable, host-qualified identity that's never ambiguous. +Janelia file shares are mounted at different paths on Linux, Mac, and Windows, and on macOS two different shares can even collide on the same guessed mount point (e.g. `/Volumes/`). This package gives you two functions to move between a local path and a stable identity that's never ambiguous. ## Installation @@ -27,42 +27,56 @@ Requires Python 3.11+. Zero runtime dependencies beyond `platformdirs`. ```python from janelia_pathlib import to_janelia_uri, from_janelia_uri -# Canonicalize a local path to its unambiguous janelia:// identity +# Canonicalize a local path to its unambiguous janelia: identity uri = to_janelia_uri("/labs/alpha/alphalab/sub/data") -# "janelia://fileserver.example.org/alphalab/sub/data" +# "janelia:/labs/alpha/alphalab/sub/data" -# Resolve a janelia:// URI back to a real, usable local path +# Resolve a janelia: URI back to a real, usable local path path = from_janelia_uri(uri) # "/Volumes/alphalab/sub/data" (on Mac), "/labs/alpha/alphalab/sub/data" (on Linux), ... ``` -## Why `janelia://`? +## Why `janelia:`? A share's local mount point differs by OS, and on macOS it isn't even guaranteed to be unique: two shares with the same share name on different servers (e.g. `host-a.example.org/data` and `host-b.example.org/data`) both nominally want `/Volumes/data` — macOS mounts the second at `/Volumes/data-1` instead. A bare path string like `/Volumes/data` can't tell you which share that actually is. -`janelia://host/share/subpath` encodes the SMB host and share name directly, which is always unique — so it's safe to store, log, or pass between machines, and always resolves back to the *correct* share regardless of what macOS happened to name the mountpoint. +`janelia:/` uses the share's Linux path as its identity. Linux mount points are fixed and admin-assigned, so they're unique and stable — unlike a macOS mountpoint name or a Windows drive letter, and unlike the SMB host, which changes if a share migrates to new hardware. The URI is safe to store, log, or pass between machines, and always resolves back to the *correct* share. + +### Why one slash, not two? + +`//` in a URI opens an [authority component](https://datatracker.ietf.org/doc/html/rfc3986#section-3.2) — the `host` in `https://host/path`. A Linux path has nothing to put there: a share's identity spans a variable number of leading segments (`/labs/alpha/home`, `/nrs/foxtrot`) and is only recoverable by prefix-matching the share table, so there's no fixed cut point. `janelia://labs/alpha/...` would silently parse `labs` — a storage tier — as the authority. The single-slash form says there is no authority, the same way `mailto:` and `urn:` do. ## API ### `to_janelia_uri(path)` -Translate a local path in the **current OS's own format** to its canonical `janelia://host/share/subpath` form. A path in a different OS's format (e.g. a Windows UNC path passed in while running on Linux) is not recognized — only `janelia://` URIs are portable across OSes; a plain path is assumed to describe the machine it's running on. +Translate a path to its canonical `janelia:/` form. + +Three input formats are **self-describing** and resolve on any OS, regardless of the machine you're running on: + +| Format | Example | +|---|---| +| Linux path | `/labs/alpha/alphalab/data` | +| Windows UNC | `\\fileserver.example.org\alphalab\data` | +| SMB URL | `smb://fileserver.example.org/alphalab/data` | ```python from janelia_pathlib import to_janelia_uri -# On Linux: +# All three produce the same identity, on any OS: to_janelia_uri("/labs/alpha/alphalab/data") -# "janelia://fileserver.example.org/alphalab/data" - -# On Mac, once the share is mounted: -to_janelia_uri("/Volumes/alphalab/data") -# "janelia://fileserver.example.org/alphalab/data" +to_janelia_uri("smb://fileserver.example.org/alphalab/data") +# "janelia:/labs/alpha/alphalab/data" ``` -**On macOS**, resolving a `/Volumes/...` path requires the share to be mounted already — `mac_path` alone can't be trusted as a guess (two shares can collide on the same mount name; see "Why `janelia://`?" above), so this checks live `mount` state rather than guessing. If the share isn't mounted, the path isn't recognized and passes through unchanged. Use `from_janelia_uri` first if you need to mount it. +Two formats are **inherently OS-local** and only resolve while running on that OS, with the share mounted: + +| Format | Example | Why | +|---|---|---| +| macOS mountpoint | `/Volumes/alphalab/data` | The name is assigned from the share name alone, so two shares can collide (`/Volumes/data` vs `/Volumes/data-1`). Only the live `mount` table says which is which. | +| Windows drive letter | `Z:\data` | Per-user mapping state; the letter carries no information about the share. | -**Linux** paths always resolve without any live check, since Linux mount points are fixed and never ambiguous. +For these, the live mount table is consulted rather than guessing from `mac_path`/`windows_path` — a guess could be wrong in exactly the way the mount-name collision causes. If the share isn't mounted, the path passes through unchanged; use `from_janelia_uri` first if you need to mount it. Paths that don't match any known share are returned unchanged: @@ -73,12 +87,12 @@ to_janelia_uri("/tmp/local/scratch/file.txt") ### `from_janelia_uri(path_str, *, timeout=30)` -Translate a `janelia://host/share/subpath` string to a real, usable local path. +Translate a `janelia:/` string to a real, usable local path. ```python from janelia_pathlib import from_janelia_uri -from_janelia_uri("janelia://fileserver.example.org/alphalab/data") +from_janelia_uri("janelia:/labs/alpha/alphalab/data") # "/Volumes/alphalab/data" (on Mac) # "/labs/alpha/alphalab/data" (on Linux) ``` @@ -87,7 +101,7 @@ On **macOS**, if the share isn't already mounted, this mounts it on demand (trig On **Linux** and **Windows**, no mounting step is needed — shares are already available at a fixed path (Linux) or accessible directly via UNC path (Windows). -A string that isn't a `janelia://` URI is returned unchanged: +A string that isn't a `janelia:` URI is returned unchanged: ```python from_janelia_uri("/tmp/local/scratch/file.txt") diff --git a/src/janelia_pathlib/__init__.py b/src/janelia_pathlib/__init__.py index 9806d4a..ab3a7e8 100644 --- a/src/janelia_pathlib/__init__.py +++ b/src/janelia_pathlib/__init__.py @@ -1,5 +1,3 @@ -"""Translate Janelia file share paths to/from an unambiguous janelia:// URI.""" - from importlib.metadata import PackageNotFoundError, version from janelia_pathlib._path import from_janelia_uri, to_janelia_uri diff --git a/src/janelia_pathlib/_macos.py b/src/janelia_pathlib/_macos.py index 639b9a5..7e82ad2 100644 --- a/src/janelia_pathlib/_macos.py +++ b/src/janelia_pathlib/_macos.py @@ -9,7 +9,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from collections.abc import Mapping + from collections.abc import Iterable from ._registry import Share @@ -46,9 +46,7 @@ def find_live_mount(host: str, share_name: str) -> str | None: return None -def find_share_for_mac_mountpoint( - mountpoint: str, shares: Mapping[tuple[str, str], Share] -) -> Share | None: +def find_share_for_mac_mountpoint(mountpoint: str, shares: Iterable[Share]) -> Share | None: """Find which of ``shares`` is actually live-mounted at ``mountpoint``. Disambiguates a bare ``/Volumes/`` path when multiple shares would @@ -57,9 +55,11 @@ def find_share_for_mac_mountpoint( second one actually landing at ``/Volumes/-1``) by checking which share's SMB host+name is actually mounted there right now. """ + from ._registry import find_share_by_host_and_name + for mount_host, mount_share, live_point in _live_smb_mounts(): if live_point == mountpoint: - share = shares.get((mount_host, mount_share)) + share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: return share return None @@ -74,7 +74,7 @@ def ensure_mounted(share_info: Share, *, timeout: float = 30) -> Path | None: with the same name on different servers collide, and the second mounted gets suffixed (``/Volumes/-1``). """ - host, share_name = share_info.get_host_and_share_name() + host, share_name = share_info.host, share_info.share_name smb_url = share_info.mac_path existing = find_live_mount(host, share_name) diff --git a/src/janelia_pathlib/_path.py b/src/janelia_pathlib/_path.py index ffdab1f..73cdfd7 100644 --- a/src/janelia_pathlib/_path.py +++ b/src/janelia_pathlib/_path.py @@ -1,35 +1,33 @@ -"""Translation between local paths and the unambiguous janelia:// URI form.""" - from __future__ import annotations from typing import TYPE_CHECKING from ._macos import ensure_mounted -from ._registry import current_os, resolve, resolve_janelia_url +from ._registry import current_os, resolve, resolve_janelia_uri if TYPE_CHECKING: from pathlib import Path def from_janelia_uri(path_str: str, *, timeout: float = 30) -> str: - """Translate a ``janelia://`` URI to a real, usable local path string. - - A plain path is returned unchanged. A - ``janelia://host/share/subpath`` string is resolved to the matching - Janelia share and, on macOS, mounted on demand if not already mounted -- - returning its real local mountpoint rather than a guessed - ``/Volumes/`` path, which may not exist or may belong to a - different, colliding share. + """Translate a ``janelia:`` URI to a real, usable local path string. + + A plain path is returned unchanged. A ``janelia:/`` string + is resolved to the matching Janelia share and, on macOS, mounted on + demand if not already mounted -- returning its real local mountpoint + rather than a guessed ``/Volumes/`` path, which may not exist or + may belong to a different, colliding share. """ - if not path_str.strip().startswith("janelia://"): + if not path_str.strip().startswith("janelia:"): return path_str - result = resolve_janelia_url(path_str) + result = resolve_janelia_uri(path_str) if result is None: - raise RuntimeError(f"No known share for URL: {path_str}") + raise RuntimeError(f"No known share for URI: {path_str}") share_info, subpath = result os_name = current_os() + sep = "/" if os_name == "mac": mount_point = ensure_mounted(share_info, timeout=timeout) if mount_point is None: @@ -38,16 +36,19 @@ def from_janelia_uri(path_str: str, *, timeout: float = 30) -> str: elif os_name == "linux": base = share_info.linux_path else: + # windows_path is a backslash-separated UNC path; joining the + # subpath with "/" would produce a mixed-separator string. base = share_info.windows_path + sep = "\\" - return base if not subpath else base + "/" + subpath + return base if not subpath else base + sep + subpath.replace("/", sep) def to_janelia_uri(path: str | Path) -> str: - """Translate a path to its canonical ``janelia://`` URI form. + """Translate a path to its canonical ``janelia:`` URI form. If the path matches a known Janelia share, returns its unambiguous - ``janelia://host/share/subpath`` identity -- safe to round-trip through + ``janelia:/`` identity -- safe to round-trip through ``from_janelia_uri`` on any OS, unlike a ``/Volumes/`` mountpoint name, which can collide with another share's on macOS. Otherwise returns the plain path string unchanged. diff --git a/src/janelia_pathlib/_registry.py b/src/janelia_pathlib/_registry.py index ad5b051..dc07dec 100644 --- a/src/janelia_pathlib/_registry.py +++ b/src/janelia_pathlib/_registry.py @@ -15,42 +15,82 @@ from ._fetch import fetch_shares, no_fetch, resolve_cache_path if TYPE_CHECKING: - from collections.abc import Mapping + from collections.abc import Iterable, Mapping _OS_MAP = {"Darwin": "mac", "Linux": "linux", "Windows": "windows"} +_SMB_URL_RE = re.compile(r"smb://([^/]+)/(.+)") + + +def parse_smb_url(url: str) -> tuple[str, str] | None: + """Split an ``smb://host/share/...`` URL into ``(host, rest)``. + + Returns None if ``url`` isn't a well-formed ``smb://`` URL. + """ + match = _SMB_URL_RE.match(url.strip().rstrip("/")) + if match is None: + return None + return match.group(1), match.group(2) + @dataclass(frozen=True, slots=True) class Share: - """A single file share entry, matching the Fileglancer API's own field names.""" + """A single file share entry, matching the Fileglancer API's own field names. + + ``host`` and ``share_name`` are parsed from ``mac_path`` once at load + (see ``from_api_entry``) rather than on each lookup. Live mount tables + on macOS and Windows identify a share by that pair. + """ zone: str linux_path: str windows_path: str mac_path: str + host: str + share_name: str - def get_host_and_share_name(self) -> tuple[str, str]: - """Happens to be parsed from mac_path + @classmethod + def from_api_entry(cls, entry: Mapping[str, str]) -> Share: + """Build a Share from one Fileglancer API record. Raises: - ValueError: _description_ - - Returns: - tuple[str, str]: _description_ + ValueError: if ``mac_path`` isn't a well-formed ``smb://`` URL. """ - match = re.match(r"smb://([^/]+)/(.+)", self.mac_path) - if match is None: - raise ValueError("Malformed smb path for mac mounting") - return match.group(1), match.group(2) + mac_path = entry["mac_path"] + parsed = parse_smb_url(mac_path) + if parsed is None: + raise ValueError(f"Malformed smb path for mac mounting: {mac_path}") + host, share_name = parsed + return cls( + zone=entry["zone"], + linux_path=entry["linux_path"], + windows_path=entry["windows_path"], + mac_path=mac_path, + host=host, + share_name=share_name, + ) @property def uri(self) -> str: - host, share_name = self.get_host_and_share_name() - return f"janelia://{host}/{share_name}" + """The share's canonical ``janelia:/`` identity.""" + return f"janelia:{self.linux_path}" + + +def find_share_by_host_and_name( + host: str, share_name: str, shares: Iterable[Share] +) -> Share | None: + """Find the share served as ``share_name`` by ``host``. + + This is how live mount tables identify a share on macOS and Windows. + """ + for share in shares: + if (share.host, share.share_name) == (host, share_name): + return share + return None def _find_share_by_fixed_path( - path_str: str, shares: Mapping[tuple[str, str], Share], attr: str + path_str: str, shares: Iterable[Share], attr: str ) -> tuple[Share, str] | None: """Find which of ``shares`` a path falls under by prefix-matching ``attr``. @@ -64,7 +104,7 @@ def _find_share_by_fixed_path( best_share: Share | None = None best_prefix = "" - for share in shares.values(): + for share in shares: candidate = _to_forward_slash(re.sub(r"[/\\]+$", "", getattr(share, attr) or "")) if not candidate: continue @@ -80,9 +120,7 @@ def _find_share_by_fixed_path( return best_share, subpath[1:] if subpath.startswith("/") else subpath -def find_share_for_linux_mount( - path_str: str, shares: Mapping[tuple[str, str], Share] -) -> tuple[Share, str] | None: +def find_share_for_linux_mount(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: """Find which of ``shares`` a Linux path falls under. Linux mount points are fixed and admin-assigned, so (unlike macOS @@ -92,9 +130,7 @@ def find_share_for_linux_mount( return _find_share_by_fixed_path(path_str, shares, "linux_path") -def find_share_for_windows_unc( - path_str: str, shares: Mapping[tuple[str, str], Share] -) -> tuple[Share, str] | None: +def find_share_for_windows_unc(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: """Find which of ``shares`` a Windows UNC path (\\\\host\\share\\...) falls under. A UNC path already encodes the host, so (unlike a drive letter) it's @@ -103,8 +139,26 @@ def find_share_for_windows_unc( return _find_share_by_fixed_path(path_str, shares, "windows_path") +def find_share_for_smb_url(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: + """Find which of ``shares`` an ``smb://host/share/...`` URL refers to. + + The host and share name are both in the string, so this is an exact + lookup -- no live-state check needed. + """ + parsed = parse_smb_url(path_str) + if parsed is None: + return None + host, rest = parsed + + share_name, _, subpath = rest.partition("/") + share = find_share_by_host_and_name(host, share_name, shares) + if share is None: + return None + return share, subpath + + def find_share_for_mounted_path( - path_str: str, os_name: str, shares: Mapping[tuple[str, str], Share] + path_str: str, os_name: str, shares: Iterable[Share] ) -> tuple[Share, str] | None: """Detect whether ``path_str`` falls under a mounted Janelia share. @@ -152,16 +206,10 @@ def find_share_for_mounted_path( @cache -def _load_shares() -> dict[tuple[str, str], Share]: +def _load_shares() -> tuple[Share, ...]: """Load share data, refreshing it from the network on every call unless JANELIA_PATHLIB_NO_FETCH is set. - Returns a mapping from ``(host, share_name)`` to ``Share`` -- the same - key every exact-match lookup (mac mountpoint, windows drive, janelia:// - URL) needs, so those become O(1) dict lookups instead of linear scans. - Prefix-based matching (linux_path, windows UNC) just iterates - ``.values()``, same as it would a list. - JANELIA_PATHLIB_NO_FETCH means "never touch the network": use the cache file as-is if present (however stale). Without it, a fetch failure (offline, DNS, etc.) falls back to the existing cache file instead. @@ -189,20 +237,11 @@ def _load_shares() -> dict[tuple[str, str], Share]: "access to the Janelia network to populate the cache.", stacklevel=2, ) - return {} + return () data_text = cache_file.read_text() raw = json.loads(data_text) - shares = {} - for entry in raw: - share = Share( - zone=entry["zone"], - linux_path=entry["linux_path"], - windows_path=entry["windows_path"], - mac_path=entry["mac_path"], - ) - shares[share.get_host_and_share_name()] = share - return shares + return tuple(Share.from_api_entry(entry) for entry in raw) def _to_forward_slash(path: str) -> str: @@ -217,37 +256,26 @@ def current_os() -> str: return os_name -def resolve_janelia_url( - path_str: str, shares: Mapping[tuple[str, str], Share] | None = None +def resolve_janelia_uri( + path_str: str, shares: Iterable[Share] | None = None ) -> tuple[Share, str] | None: - """Match a ``janelia://host/share_name/subpath`` string against known shares. + """Match a ``janelia:/`` string against known shares. - Unlike a bare ``/Volumes/`` path, the host in this identity makes - resolution unambiguous even when two shares' mountpoint-name guesses - collide (e.g. two different hosts whose share names happen to match - both nominally want the same ``/Volumes/``). + The URI body is a Linux path, so this is the same prefix match + ``find_share_for_linux_mount`` does. A ``janelia://...`` string + (an older, host-qualified form) is rejected: ``//`` would make the + first path segment parse as a URI authority. """ if shares is None: shares = _load_shares() - match = re.match(r"janelia://([^/]+)/(.+)", path_str) + match = re.match(r"janelia:(/(?!/).*)$", path_str.strip()) if match is None: return None - host, rest = match.group(1), match.group(2).rstrip("/") - - # The share name is always exactly the first path segment (no real - # share name contains a literal "/", confirmed against all live - # Fileglancer data) -- a single O(1) dict lookup, no scanning needed. - share_name, _, subpath = rest.partition("/") - share = shares.get((host, share_name)) - if share is None: - return None - return share, subpath + return find_share_for_linux_mount(match.group(1), shares) -def resolve( - path_str: str, shares: Mapping[tuple[str, str], Share] | None = None -) -> tuple[Share, str] | None: +def resolve(path_str: str, shares: Iterable[Share] | None = None) -> tuple[Share, str] | None: """Match a path string against known shares on any OS. Returns (share, subpath) where subpath is the relative portion after @@ -263,11 +291,12 @@ def resolve( live state can map it to a share. On Linux this is just the static ``linux_path`` match, already fully authoritative. - Falls back to a plain ``linux_path`` prefix match regardless of the - current OS, since it's a fixed, admin-assigned path that's never - ambiguous on any OS -- unlike a mac mountpoint name or a windows drive - letter, which need live state on that OS to verify, and aren't guessed - at when running on a different one. + Then falls back to the forms that are self-describing enough to match + on any OS, no live state needed: a ``linux_path`` prefix (fixed and + admin-assigned), a ``windows_path`` UNC prefix (carries its own host), + and an ``smb://`` URL (likewise). A ``/Volumes/`` mountpoint or a + drive letter can't join them -- resolving either needs the live mount + table, which only exists on the machine holding the mount. """ if shares is None: shares = _load_shares() @@ -276,4 +305,12 @@ def resolve( if live_result is not None: return live_result - return find_share_for_linux_mount(path_str, shares) + for matcher in ( + find_share_for_linux_mount, + find_share_for_windows_unc, + find_share_for_smb_url, + ): + result = matcher(path_str, shares) + if result is not None: + return result + return None diff --git a/src/janelia_pathlib/_windows.py b/src/janelia_pathlib/_windows.py index 4bc9492..bebd629 100644 --- a/src/janelia_pathlib/_windows.py +++ b/src/janelia_pathlib/_windows.py @@ -12,7 +12,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from collections.abc import Mapping + from collections.abc import Iterable from ._registry import Share @@ -56,14 +56,14 @@ def _live_windows_drives() -> list[tuple[str, str, str]]: return drives -def find_share_for_windows_drive( - drive: str, shares: Mapping[tuple[str, str], Share] -) -> Share | None: +def find_share_for_windows_drive(drive: str, shares: Iterable[Share]) -> Share | None: """Find which of ``shares`` is actually mapped to Windows drive letter ``drive``.""" + from ._registry import find_share_by_host_and_name + drive_norm = drive.rstrip("\\").upper() for mount_host, mount_share, local in _live_windows_drives(): if local.rstrip("\\").upper() == drive_norm: - share = shares.get((mount_host, mount_share)) + share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: return share return None diff --git a/tests/test_janelia_path.py b/tests/test_janelia_path.py index 2e043af..7a352af 100644 --- a/tests/test_janelia_path.py +++ b/tests/test_janelia_path.py @@ -12,7 +12,7 @@ import pytest -from janelia_pathlib import _fetch, _registry, from_janelia_uri, to_janelia_uri +from janelia_pathlib import _fetch, _registry, _windows, from_janelia_uri, to_janelia_uri mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS paths") linux_only = pytest.mark.skipif(sys.platform != "linux", reason="Linux paths") @@ -23,10 +23,7 @@ def test_to_janelia_uri_from_linux_path(): - assert ( - to_janelia_uri("/labs/alpha/alphalab/data") - == "janelia://fileserver.example.org/alphalab/data" - ) + assert to_janelia_uri("/labs/alpha/alphalab/data") == "janelia:/labs/alpha/alphalab/data" @mac_only @@ -39,22 +36,40 @@ def test_to_janelia_uri_from_mac_path(): returncode=0, stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", ) - assert ( - to_janelia_uri("/Volumes/alphalab/data") - == "janelia://fileserver.example.org/alphalab/data" - ) + assert to_janelia_uri("/Volumes/alphalab/data") == "janelia:/labs/alpha/alphalab/data" -@windows_only def test_to_janelia_uri_from_windows_path(): + """A UNC path carries its own host, so it resolves without live state -- + including when running on a non-Windows machine.""" assert ( to_janelia_uri("\\\\fileserver.example.org\\alphalab\\data") - == "janelia://fileserver.example.org/alphalab/data" + == "janelia:/labs/alpha/alphalab/data" ) def test_to_janelia_uri_share_root_only(): - assert to_janelia_uri("/labs/alpha/alphalab") == "janelia://fileserver.example.org/alphalab" + assert to_janelia_uri("/labs/alpha/alphalab") == "janelia:/labs/alpha/alphalab" + + +def test_to_janelia_uri_from_smb_url_on_any_os(): + """Same for an smb:// URL -- host and share name are both in the string.""" + assert ( + to_janelia_uri("smb://fileserver.example.org/alphalab/data") + == "janelia:/labs/alpha/alphalab/data" + ) + + +def test_to_janelia_uri_smb_url_share_root_only(): + assert to_janelia_uri("smb://fileserver.example.org/alphalab") == "janelia:/labs/alpha/alphalab" + + +def test_to_janelia_uri_smb_url_with_dollar_share(): + """Hidden shares (trailing $) are ordinary share names, not special syntax.""" + assert ( + to_janelia_uri("smb://fileserver.example.org/alpha$/data") + == "janelia:/labs/alpha/home/data" + ) def test_to_janelia_uri_no_match_passthrough(): @@ -63,22 +78,18 @@ def test_to_janelia_uri_no_match_passthrough(): def test_to_janelia_uri_longest_prefix_wins(): """Both /labs/charlie and /labs/charlie/charlie match; the longer wins.""" - assert ( - to_janelia_uri("/labs/charlie/charlie/data") - == "janelia://fileserver.example.org/charlie/data" - ) + assert to_janelia_uri("/labs/charlie/charlie/data") == "janelia:/labs/charlie/charlie/data" def test_to_janelia_uri_trailing_slash(): - assert to_janelia_uri("/labs/alpha/alphalab/") == "janelia://fileserver.example.org/alphalab" + assert to_janelia_uri("/labs/alpha/alphalab/") == "janelia:/labs/alpha/alphalab" -@windows_only def test_to_janelia_uri_windows_forward_slash(): """Windows paths with forward slashes should still match.""" assert ( to_janelia_uri("//fileserver.example.org/alphalab/data") - == "janelia://fileserver.example.org/alphalab/data" + == "janelia:/labs/alpha/alphalab/data" ) @@ -95,11 +106,8 @@ def test_to_janelia_uri_resolves_colliding_mac_mount_via_live_state(): mock_run.return_value = subprocess.CompletedProcess( args=["mount"], returncode=0, stdout=mount_output ) - assert to_janelia_uri("/Volumes/foxtrot/data") == "janelia://nrs.example.org/foxtrot/data" - assert ( - to_janelia_uri("/Volumes/foxtrot-1/data") - == "janelia://nearline.example.org/foxtrot/data" - ) + assert to_janelia_uri("/Volumes/foxtrot/data") == "janelia:/nrs/foxtrot/data" + assert to_janelia_uri("/Volumes/foxtrot-1/data") == "janelia:/nearline/foxtrot/data" @mac_only @@ -122,15 +130,20 @@ def test_from_janelia_uri_no_match_passthrough(): @linux_only def test_from_janelia_uri_on_linux(): - assert ( - from_janelia_uri("janelia://fileserver.example.org/alphalab/data") - == "/labs/alpha/alphalab/data" - ) + assert from_janelia_uri("janelia:/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" def test_from_janelia_uri_unknown_share_raises(): with pytest.raises(RuntimeError, match="No known share"): - from_janelia_uri("janelia://unknown.example.org/nope/data") + from_janelia_uri("janelia:/nope/not-a-share/data") + + +def test_from_janelia_uri_rejects_double_slash_form(): + """``janelia://host/share/...`` is the old form. ``//`` opens a URI + authority component, which a Linux path has no value for, so it's + rejected rather than silently parsing ``labs`` as an authority.""" + with pytest.raises(RuntimeError, match="No known share"): + from_janelia_uri("janelia://labs/alpha/alphalab/data") @mac_only @@ -145,7 +158,7 @@ def test_from_janelia_uri_mounts_on_demand(): stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", ) mock_run.side_effect = [mount_output_empty, None, mount_output_found] - result = from_janelia_uri("janelia://fileserver.example.org/alphalab/data", timeout=2) + result = from_janelia_uri("janelia:/labs/alpha/alphalab/data", timeout=2) assert result == "/Volumes/alphalab/data" open_call = mock_run.call_args_list[1] args = open_call[0][0] @@ -162,7 +175,7 @@ def test_from_janelia_uri_already_mounted(): returncode=0, stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", ) - result = from_janelia_uri("janelia://fileserver.example.org/alphalab/data") + result = from_janelia_uri("janelia:/labs/alpha/alphalab/data") assert result == "/Volumes/alphalab/data" for call in mock_run.call_args_list: assert call[0][0][0] != "open" @@ -182,22 +195,121 @@ def test_from_janelia_uri_resolves_colliding_share_name(): "//user@nearline.example.org/foxtrot on /Volumes/foxtrot-1 (smbfs, nodev)" ), ) - assert from_janelia_uri("janelia://nrs.example.org/foxtrot/data") == "/Volumes/foxtrot/data" - assert ( - from_janelia_uri("janelia://nearline.example.org/foxtrot/data") - == "/Volumes/foxtrot-1/data" - ) + assert from_janelia_uri("janelia:/nrs/foxtrot/data") == "/Volumes/foxtrot/data" + assert from_janelia_uri("janelia:/nearline/foxtrot/data") == "/Volumes/foxtrot-1/data" -@linux_only -def test_from_janelia_uri_raises_on_linux_mount_failure(): - """Linux never mounts (fixed mount points), so this shouldn't try to.""" +@windows_only +def test_from_janelia_uri_on_windows(): + """Windows resolves to the UNC path -- no mounting step needed.""" + assert ( + from_janelia_uri("janelia:/labs/alpha/alphalab/data") + == "\\\\fileserver.example.org\\alphalab\\data" + ) + + +@windows_only +def test_from_janelia_uri_windows_share_root_only(): + assert ( + from_janelia_uri("janelia:/labs/alpha/alphalab") == "\\\\fileserver.example.org\\alphalab" + ) + + +@windows_only +def test_from_janelia_uri_windows_nested_subpath_separators(): + """The subpath arrives forward-slashed from the matchers; every segment + must come back backslash-separated, not just the first join.""" assert ( - from_janelia_uri("janelia://fileserver.example.org/alphalab/data") - == "/labs/alpha/alphalab/data" + from_janelia_uri("janelia:/labs/alpha/alphalab/sub/deeper/data") + == "\\\\fileserver.example.org\\alphalab\\sub\\deeper\\data" ) +# --- Windows drive-letter mapping --- +# +# These patch PowerShell's Get-SmbMapping output rather than requiring a real +# mapped drive, so they run on any platform -- same approach as the macOS +# `mount` tests above. + + +def _smb_mapping(stdout): + return subprocess.CompletedProcess(args=["powershell"], returncode=0, stdout=stdout) + + +def test_find_share_for_windows_drive_matches_mapping(): + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + share = _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) + assert share is not None + assert share.linux_path == "/labs/alpha/alphalab" + + +def test_find_share_for_windows_drive_is_case_insensitive(): + """Drive letters are case-insensitive, and Get-SmbMapping may report a + trailing backslash that the queried drive string lacks.""" + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:\\"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + share = _windows.find_share_for_windows_drive("z:", _registry._load_shares()) + assert share is not None + assert share.linux_path == "/labs/alpha/alphalab" + + +def test_find_share_for_windows_drive_resolves_colliding_share_names(): + """Two shares named `foxtrot` on different hosts are mapped to different + letters; only the host in RemotePath tells them apart.""" + mapping = json.dumps( + [ + {"RemotePath": "\\\\nrs.example.org\\foxtrot", "LocalPath": "X:"}, + {"RemotePath": "\\\\nearline.example.org\\foxtrot", "LocalPath": "Y:"}, + ] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + shares = _registry._load_shares() + assert _windows.find_share_for_windows_drive("X:", shares).linux_path == "/nrs/foxtrot" + assert _windows.find_share_for_windows_drive("Y:", shares).linux_path == "/nearline/foxtrot" + + +def test_find_share_for_windows_drive_single_mapping_is_json_object(): + """ConvertTo-Json emits a bare object, not a list, for a single mapping.""" + mapping = json.dumps({"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + share = _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) + assert share is not None + assert share.linux_path == "/labs/alpha/alphalab" + + +def test_find_share_for_windows_drive_unmapped_returns_none(): + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + assert _windows.find_share_for_windows_drive("Q:", _registry._load_shares()) is None + + +def test_find_share_for_windows_drive_unknown_share_returns_none(): + """A drive mapped to a share we don't know about isn't ours to translate.""" + mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + assert _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) is None + + +@pytest.mark.parametrize("stdout", ["", " ", "not json at all"]) +def test_live_windows_drives_degrades_on_bad_output(stdout): + """Empty or unparsable PowerShell output yields no mappings, not a crash.""" + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(stdout)): + assert _windows._live_windows_drives() == [] + + +def test_live_windows_drives_degrades_when_powershell_missing(): + """On a non-Windows machine there's no powershell at all -- OSError, not a crash.""" + with patch("janelia_pathlib._windows.subprocess.run", side_effect=OSError): + assert _windows._live_windows_drives() == [] + + # --- round trip --- @@ -251,7 +363,7 @@ def test_no_fetch_degrades_when_cache_missing(monkeypatch, tmp_path): _registry._load_shares.cache_clear() try: with pytest.warns(UserWarning, match="No share data available"): - assert _registry._load_shares() == {} + assert _registry._load_shares() == () # No known shares -> a real share path is left untranslated assert to_janelia_uri("/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" finally: @@ -273,7 +385,7 @@ def boom(*args, **kwargs): _registry._load_shares.cache_clear() try: with pytest.warns(UserWarning, match="No share data available"): - assert _registry._load_shares() == {} + assert _registry._load_shares() == () finally: _registry._load_shares.cache_clear() @@ -332,6 +444,6 @@ def boom(*args, **kwargs): with pytest.warns(UserWarning, match="stale"): shares = _registry._load_shares() assert len(shares) == 1 - assert next(iter(shares.values())).zone == "Alpha" + assert shares[0].zone == "Alpha" finally: _registry._load_shares.cache_clear() From 50ea0c2f1628eb6978d15d5bc6225ce471b2901a Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 16:18:03 -0400 Subject: [PATCH 2/7] Split badly named module into _share and _match --- src/janelia_pathlib/_fetch.py | 2 +- src/janelia_pathlib/_macos.py | 6 +- .../{_registry.py => _match.py} | 151 +++--------------- src/janelia_pathlib/_path.py | 3 +- src/janelia_pathlib/_share.py | 135 ++++++++++++++++ src/janelia_pathlib/_windows.py | 6 +- tests/test_janelia_path.py | 46 +++--- 7 files changed, 185 insertions(+), 164 deletions(-) rename src/janelia_pathlib/{_registry.py => _match.py} (61%) create mode 100644 src/janelia_pathlib/_share.py diff --git a/src/janelia_pathlib/_fetch.py b/src/janelia_pathlib/_fetch.py index 7d3608f..47054b0 100644 --- a/src/janelia_pathlib/_fetch.py +++ b/src/janelia_pathlib/_fetch.py @@ -3,7 +3,7 @@ The fetched shares.json is stored in the per-user cache directory so it never lives in the source tree (which would leak internal Janelia paths to a public repo). Invoked automatically on every use unless -JANELIA_PATHLIB_NO_FETCH is set (see ``_registry._load_shares``), and +JANELIA_PATHLIB_NO_FETCH is set (see ``_share.load_shares``), and manually via the ``janelia-paths-fetch`` CLI. """ diff --git a/src/janelia_pathlib/_macos.py b/src/janelia_pathlib/_macos.py index 7e82ad2..473662e 100644 --- a/src/janelia_pathlib/_macos.py +++ b/src/janelia_pathlib/_macos.py @@ -8,10 +8,12 @@ from pathlib import Path from typing import TYPE_CHECKING +from ._share import find_share_by_host_and_name + if TYPE_CHECKING: from collections.abc import Iterable - from ._registry import Share + from ._share import Share _SMB_MOUNT_LINE = re.compile(r"^//(?:[^@]+@)?([^/]+)/(.+?) on (\S+) \(smbfs") @@ -55,8 +57,6 @@ def find_share_for_mac_mountpoint(mountpoint: str, shares: Iterable[Share]) -> S second one actually landing at ``/Volumes/-1``) by checking which share's SMB host+name is actually mounted there right now. """ - from ._registry import find_share_by_host_and_name - for mount_host, mount_share, live_point in _live_smb_mounts(): if live_point == mountpoint: share = find_share_by_host_and_name(mount_host, mount_share, shares) diff --git a/src/janelia_pathlib/_registry.py b/src/janelia_pathlib/_match.py similarity index 61% rename from src/janelia_pathlib/_registry.py rename to src/janelia_pathlib/_match.py index dc07dec..28bdb74 100644 --- a/src/janelia_pathlib/_registry.py +++ b/src/janelia_pathlib/_match.py @@ -1,92 +1,28 @@ -"""Share registry: loads share data and provides path translation.""" +"""Matching path strings against known shares. + +Which matcher applies depends on how self-describing the path format is. +A Linux path, a UNC path, and an ``smb://`` URL each identify a share on +their own, so they resolve anywhere. A ``/Volumes/`` mountpoint or a +Windows drive letter don't -- only the live mount table on that machine +says which share they point at. +""" from __future__ import annotations -import json -import platform import re -import warnings -from dataclasses import dataclass -from functools import cache from typing import TYPE_CHECKING -from urllib.error import URLError from . import _macos, _windows -from ._fetch import fetch_shares, no_fetch, resolve_cache_path +from ._share import current_os, find_share_by_host_and_name, load_shares, parse_smb_url if TYPE_CHECKING: - from collections.abc import Iterable, Mapping - -_OS_MAP = {"Darwin": "mac", "Linux": "linux", "Windows": "windows"} - -_SMB_URL_RE = re.compile(r"smb://([^/]+)/(.+)") - + from collections.abc import Iterable -def parse_smb_url(url: str) -> tuple[str, str] | None: - """Split an ``smb://host/share/...`` URL into ``(host, rest)``. + from ._share import Share - Returns None if ``url`` isn't a well-formed ``smb://`` URL. - """ - match = _SMB_URL_RE.match(url.strip().rstrip("/")) - if match is None: - return None - return match.group(1), match.group(2) - -@dataclass(frozen=True, slots=True) -class Share: - """A single file share entry, matching the Fileglancer API's own field names. - - ``host`` and ``share_name`` are parsed from ``mac_path`` once at load - (see ``from_api_entry``) rather than on each lookup. Live mount tables - on macOS and Windows identify a share by that pair. - """ - - zone: str - linux_path: str - windows_path: str - mac_path: str - host: str - share_name: str - - @classmethod - def from_api_entry(cls, entry: Mapping[str, str]) -> Share: - """Build a Share from one Fileglancer API record. - - Raises: - ValueError: if ``mac_path`` isn't a well-formed ``smb://`` URL. - """ - mac_path = entry["mac_path"] - parsed = parse_smb_url(mac_path) - if parsed is None: - raise ValueError(f"Malformed smb path for mac mounting: {mac_path}") - host, share_name = parsed - return cls( - zone=entry["zone"], - linux_path=entry["linux_path"], - windows_path=entry["windows_path"], - mac_path=mac_path, - host=host, - share_name=share_name, - ) - - @property - def uri(self) -> str: - """The share's canonical ``janelia:/`` identity.""" - return f"janelia:{self.linux_path}" - - -def find_share_by_host_and_name( - host: str, share_name: str, shares: Iterable[Share] -) -> Share | None: - """Find the share served as ``share_name`` by ``host``. - - This is how live mount tables identify a share on macOS and Windows. - """ - for share in shares: - if (share.host, share.share_name) == (host, share_name): - return share - return None +def _to_forward_slash(path: str) -> str: + return path.replace("\\", "/") def _find_share_by_fixed_path( @@ -205,57 +141,6 @@ def find_share_for_mounted_path( return None -@cache -def _load_shares() -> tuple[Share, ...]: - """Load share data, refreshing it from the network on every call unless - JANELIA_PATHLIB_NO_FETCH is set. - - JANELIA_PATHLIB_NO_FETCH means "never touch the network": use the cache - file as-is if present (however stale). Without it, a fetch failure - (offline, DNS, etc.) falls back to the existing cache file instead. - Either way, if there's no cache file to fall back to, this degrades to - no known shares (with a warning) rather than raising -- paths just pass - through untranslated instead of breaking the caller. - """ - cache_file = resolve_cache_path() - - if not no_fetch(): - try: - fetch_shares(cache_file) - except (URLError, OSError) as e: - if cache_file.is_file(): - warnings.warn( - f"Failed to refresh share data ({e}); using cached data " - f"at {cache_file}, which may be stale.", - stacklevel=2, - ) - - if not cache_file.is_file(): - warnings.warn( - f"No share data available at {cache_file}; paths will not be " - "translated. Run `janelia-paths-fetch` from a machine with " - "access to the Janelia network to populate the cache.", - stacklevel=2, - ) - return () - - data_text = cache_file.read_text() - raw = json.loads(data_text) - return tuple(Share.from_api_entry(entry) for entry in raw) - - -def _to_forward_slash(path: str) -> str: - return path.replace("\\", "/") - - -def current_os() -> str: - """Return the current OS as 'linux', 'mac', or 'windows'.""" - os_name = _OS_MAP.get(platform.system()) - if os_name is None: - raise RuntimeError(f"Unsupported platform: {platform.system()}") - return os_name - - def resolve_janelia_uri( path_str: str, shares: Iterable[Share] | None = None ) -> tuple[Share, str] | None: @@ -267,7 +152,7 @@ def resolve_janelia_uri( first path segment parse as a URI authority. """ if shares is None: - shares = _load_shares() + shares = load_shares() match = re.match(r"janelia:(/(?!/).*)$", path_str.strip()) if match is None: @@ -287,9 +172,9 @@ def resolve(path_str: str, shares: Iterable[Share] | None = None) -> tuple[Share can't: a bare ``/Volumes/`` Mac path can't be told apart from a static string alone when two shares collide on the same mount name (macOS names mountpoints after the share name alone, ignoring the - host), and a Windows drive letter isn't in the registry at all -- only - live state can map it to a share. On Linux this is just the static - ``linux_path`` match, already fully authoritative. + host), and a Windows drive letter isn't in the share data at all -- + only live state can map it to a share. On Linux this is just the + static ``linux_path`` match, already fully authoritative. Then falls back to the forms that are self-describing enough to match on any OS, no live state needed: a ``linux_path`` prefix (fixed and @@ -299,7 +184,7 @@ def resolve(path_str: str, shares: Iterable[Share] | None = None) -> tuple[Share table, which only exists on the machine holding the mount. """ if shares is None: - shares = _load_shares() + shares = load_shares() live_result = find_share_for_mounted_path(path_str, current_os(), shares) if live_result is not None: diff --git a/src/janelia_pathlib/_path.py b/src/janelia_pathlib/_path.py index 73cdfd7..0b6bea3 100644 --- a/src/janelia_pathlib/_path.py +++ b/src/janelia_pathlib/_path.py @@ -3,7 +3,8 @@ from typing import TYPE_CHECKING from ._macos import ensure_mounted -from ._registry import current_os, resolve, resolve_janelia_uri +from ._match import resolve, resolve_janelia_uri +from ._share import current_os if TYPE_CHECKING: from pathlib import Path diff --git a/src/janelia_pathlib/_share.py b/src/janelia_pathlib/_share.py new file mode 100644 index 0000000..6828e4e --- /dev/null +++ b/src/janelia_pathlib/_share.py @@ -0,0 +1,135 @@ +"""The Share type and the share data it's loaded from.""" + +from __future__ import annotations + +import json +import platform +import re +import warnings +from dataclasses import dataclass +from functools import cache +from typing import TYPE_CHECKING +from urllib.error import URLError + +from ._fetch import fetch_shares, no_fetch, resolve_cache_path + +if TYPE_CHECKING: + from collections.abc import Iterable, Mapping + +_OS_MAP = {"Darwin": "mac", "Linux": "linux", "Windows": "windows"} + +_SMB_URL_RE = re.compile(r"smb://([^/]+)/(.+)") + + +def current_os() -> str: + """Return the current OS as 'linux', 'mac', or 'windows'.""" + os_name = _OS_MAP.get(platform.system()) + if os_name is None: + raise RuntimeError(f"Unsupported platform: {platform.system()}") + return os_name + + +def parse_smb_url(url: str) -> tuple[str, str] | None: + """Split an ``smb://host/share/...`` URL into ``(host, rest)``. + + Returns None if ``url`` isn't a well-formed ``smb://`` URL. + """ + match = _SMB_URL_RE.match(url.strip().rstrip("/")) + if match is None: + return None + return match.group(1), match.group(2) + + +@dataclass(frozen=True, slots=True) +class Share: + """A single file share entry, matching the Fileglancer API's own field names. + + ``host`` and ``share_name`` are parsed from ``mac_path`` once at load + (see ``from_api_entry``) rather than on each lookup. Live mount tables + on macOS and Windows identify a share by that pair. + """ + + zone: str + linux_path: str + windows_path: str + mac_path: str + host: str + share_name: str + + @classmethod + def from_api_entry(cls, entry: Mapping[str, str]) -> Share: + """Build a Share from one Fileglancer API record. + + Raises: + ValueError: if ``mac_path`` isn't a well-formed ``smb://`` URL. + """ + mac_path = entry["mac_path"] + parsed = parse_smb_url(mac_path) + if parsed is None: + raise ValueError(f"Malformed smb path for mac mounting: {mac_path}") + host, share_name = parsed + return cls( + zone=entry["zone"], + linux_path=entry["linux_path"], + windows_path=entry["windows_path"], + mac_path=mac_path, + host=host, + share_name=share_name, + ) + + @property + def uri(self) -> str: + """The share's canonical ``janelia:/`` identity.""" + return f"janelia:{self.linux_path}" + + +def find_share_by_host_and_name( + host: str, share_name: str, shares: Iterable[Share] +) -> Share | None: + """Find the share served as ``share_name`` by ``host``. + + This is how live mount tables identify a share on macOS and Windows. + """ + for share in shares: + if (share.host, share.share_name) == (host, share_name): + return share + return None + + +@cache +def load_shares() -> tuple[Share, ...]: + """Load share data, refreshing it from the network on every call unless + JANELIA_PATHLIB_NO_FETCH is set. + + JANELIA_PATHLIB_NO_FETCH means "never touch the network": use the cache + file as-is if present (however stale). Without it, a fetch failure + (offline, DNS, etc.) falls back to the existing cache file instead. + Either way, if there's no cache file to fall back to, this degrades to + no known shares (with a warning) rather than raising -- paths just pass + through untranslated instead of breaking the caller. + """ + cache_file = resolve_cache_path() + + if not no_fetch(): + try: + fetch_shares(cache_file) + except (URLError, OSError) as e: + if cache_file.is_file(): + warnings.warn( + f"Failed to refresh share data ({e}); using cached data " + f"at {cache_file}, which may be stale.", + stacklevel=2, + ) + + if not cache_file.is_file(): + warnings.warn( + f"No share data available at {cache_file}; paths will not be " + "translated. Run `janelia-paths-fetch` from a machine with " + "access to the Janelia network to populate the cache.", + stacklevel=2, + ) + return () + + data_text = cache_file.read_text() + raw = json.loads(data_text) + return tuple(Share.from_api_entry(entry) for entry in raw) diff --git a/src/janelia_pathlib/_windows.py b/src/janelia_pathlib/_windows.py index bebd629..87965dc 100644 --- a/src/janelia_pathlib/_windows.py +++ b/src/janelia_pathlib/_windows.py @@ -11,10 +11,12 @@ import subprocess from typing import TYPE_CHECKING +from ._share import find_share_by_host_and_name + if TYPE_CHECKING: from collections.abc import Iterable - from ._registry import Share + from ._share import Share DRIVE_RE = re.compile(r"^([A-Za-z]:)(\\.*)?$") @@ -58,8 +60,6 @@ def _live_windows_drives() -> list[tuple[str, str, str]]: def find_share_for_windows_drive(drive: str, shares: Iterable[Share]) -> Share | None: """Find which of ``shares`` is actually mapped to Windows drive letter ``drive``.""" - from ._registry import find_share_by_host_and_name - drive_norm = drive.rstrip("\\").upper() for mount_host, mount_share, local in _live_windows_drives(): if local.rstrip("\\").upper() == drive_norm: diff --git a/tests/test_janelia_path.py b/tests/test_janelia_path.py index 7a352af..24b051d 100644 --- a/tests/test_janelia_path.py +++ b/tests/test_janelia_path.py @@ -12,7 +12,7 @@ import pytest -from janelia_pathlib import _fetch, _registry, _windows, from_janelia_uri, to_janelia_uri +from janelia_pathlib import _fetch, _share, _windows, from_janelia_uri, to_janelia_uri mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS paths") linux_only = pytest.mark.skipif(sys.platform != "linux", reason="Linux paths") @@ -241,7 +241,7 @@ def test_find_share_for_windows_drive_matches_mapping(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) + share = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) assert share is not None assert share.linux_path == "/labs/alpha/alphalab" @@ -253,7 +253,7 @@ def test_find_share_for_windows_drive_is_case_insensitive(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:\\"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("z:", _registry._load_shares()) + share = _windows.find_share_for_windows_drive("z:", _share.load_shares()) assert share is not None assert share.linux_path == "/labs/alpha/alphalab" @@ -268,7 +268,7 @@ def test_find_share_for_windows_drive_resolves_colliding_share_names(): ] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - shares = _registry._load_shares() + shares = _share.load_shares() assert _windows.find_share_for_windows_drive("X:", shares).linux_path == "/nrs/foxtrot" assert _windows.find_share_for_windows_drive("Y:", shares).linux_path == "/nearline/foxtrot" @@ -277,7 +277,7 @@ def test_find_share_for_windows_drive_single_mapping_is_json_object(): """ConvertTo-Json emits a bare object, not a list, for a single mapping.""" mapping = json.dumps({"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) + share = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) assert share is not None assert share.linux_path == "/labs/alpha/alphalab" @@ -287,14 +287,14 @@ def test_find_share_for_windows_drive_unmapped_returns_none(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - assert _windows.find_share_for_windows_drive("Q:", _registry._load_shares()) is None + assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None def test_find_share_for_windows_drive_unknown_share_returns_none(): """A drive mapped to a share we don't know about isn't ours to translate.""" mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - assert _windows.find_share_for_windows_drive("Z:", _registry._load_shares()) is None + assert _windows.find_share_for_windows_drive("Z:", _share.load_shares()) is None @pytest.mark.parametrize("stdout", ["", " ", "not json at all"]) @@ -360,14 +360,14 @@ def test_no_fetch_degrades_when_cache_missing(monkeypatch, tmp_path): monkeypatch.setenv("JANELIA_PATHLIB_NO_FETCH", "1") monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(tmp_path / "missing.json")) - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() try: with pytest.warns(UserWarning, match="No share data available"): - assert _registry._load_shares() == () + assert _share.load_shares() == () # No known shares -> a real share path is left untranslated assert to_janelia_uri("/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" finally: - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() def test_missing_cache_degrades_without_no_fetch(monkeypatch, tmp_path): @@ -382,12 +382,12 @@ def boom(*args, **kwargs): raise URLError("offline") monkeypatch.setattr("urllib.request.urlopen", boom) - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() try: with pytest.warns(UserWarning, match="No share data available"): - assert _registry._load_shares() == () + assert _share.load_shares() == () finally: - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() def test_load_shares_refetches_every_call(monkeypatch, tmp_path): @@ -403,17 +403,17 @@ def fake_fetch_shares(path): call_count["n"] += 1 path.write_text("[]") - monkeypatch.setattr(_registry, "fetch_shares", fake_fetch_shares) - _registry._load_shares.cache_clear() + monkeypatch.setattr(_share, "fetch_shares", fake_fetch_shares) + _share.load_shares.cache_clear() try: - _registry._load_shares.cache_clear() - _registry._load_shares() + _share.load_shares.cache_clear() + _share.load_shares() assert call_count["n"] == 1 - _registry._load_shares.cache_clear() - _registry._load_shares() + _share.load_shares.cache_clear() + _share.load_shares() assert call_count["n"] == 2 finally: - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() def test_fetch_failure_falls_back_to_stale_cache(monkeypatch, tmp_path): @@ -439,11 +439,11 @@ def boom(*args, **kwargs): raise URLError("offline") monkeypatch.setattr("urllib.request.urlopen", boom) - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() try: with pytest.warns(UserWarning, match="stale"): - shares = _registry._load_shares() + shares = _share.load_shares() assert len(shares) == 1 assert shares[0].zone == "Alpha" finally: - _registry._load_shares.cache_clear() + _share.load_shares.cache_clear() From b21c93b8181e488e608db703c31a623314b7d003 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 16:20:47 -0400 Subject: [PATCH 3/7] Rename _path to _translate --- src/janelia_pathlib/__init__.py | 2 +- src/janelia_pathlib/_match.py | 9 --------- src/janelia_pathlib/_share.py | 2 -- src/janelia_pathlib/{_path.py => _translate.py} | 0 4 files changed, 1 insertion(+), 12 deletions(-) rename src/janelia_pathlib/{_path.py => _translate.py} (100%) diff --git a/src/janelia_pathlib/__init__.py b/src/janelia_pathlib/__init__.py index ab3a7e8..524cc66 100644 --- a/src/janelia_pathlib/__init__.py +++ b/src/janelia_pathlib/__init__.py @@ -1,6 +1,6 @@ from importlib.metadata import PackageNotFoundError, version -from janelia_pathlib._path import from_janelia_uri, to_janelia_uri +from janelia_pathlib._translate import from_janelia_uri, to_janelia_uri try: __version__ = version("janelia-pathlib") diff --git a/src/janelia_pathlib/_match.py b/src/janelia_pathlib/_match.py index 28bdb74..5fc7fda 100644 --- a/src/janelia_pathlib/_match.py +++ b/src/janelia_pathlib/_match.py @@ -1,12 +1,3 @@ -"""Matching path strings against known shares. - -Which matcher applies depends on how self-describing the path format is. -A Linux path, a UNC path, and an ``smb://`` URL each identify a share on -their own, so they resolve anywhere. A ``/Volumes/`` mountpoint or a -Windows drive letter don't -- only the live mount table on that machine -says which share they point at. -""" - from __future__ import annotations import re diff --git a/src/janelia_pathlib/_share.py b/src/janelia_pathlib/_share.py index 6828e4e..c070e65 100644 --- a/src/janelia_pathlib/_share.py +++ b/src/janelia_pathlib/_share.py @@ -1,5 +1,3 @@ -"""The Share type and the share data it's loaded from.""" - from __future__ import annotations import json diff --git a/src/janelia_pathlib/_path.py b/src/janelia_pathlib/_translate.py similarity index 100% rename from src/janelia_pathlib/_path.py rename to src/janelia_pathlib/_translate.py From dcc28c0c4952f6fbc687f0ce1029f43057b56366 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 17:08:54 -0400 Subject: [PATCH 4/7] Tons of nitpicks of Claude decisions --- README.md | 11 +++ src/janelia_pathlib/_macos.py | 61 +++++++++--- src/janelia_pathlib/_match.py | 142 ++++++++------------------ src/janelia_pathlib/_share.py | 36 ++++++- src/janelia_pathlib/_translate.py | 64 +++++++++--- src/janelia_pathlib/_windows.py | 24 ++++- tests/fixtures/shares.json | 18 +++- tests/test_janelia_path.py | 159 +++++++++++++++++++++++++++--- 8 files changed, 360 insertions(+), 155 deletions(-) diff --git a/README.md b/README.md index cf11083..59a857d 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,17 @@ for long; override it with: export JANELIA_PATHLIB_FETCH_TIMEOUT=5 ``` +On macOS, `from_janelia_uri` waits up to 30s for an on-demand mount to appear. +That wait covers a native credential prompt, so it budgets for someone typing a +password — raise it for a slow authenticator, or set it near zero in headless +contexts where nobody is there to answer: + +```bash +export JANELIA_PATHLIB_MOUNT_TIMEOUT=90 +``` + +Passing `timeout=` to `from_janelia_uri` overrides it per call. + ## Development This project uses [uv](https://docs.astral.sh/uv/) and diff --git a/src/janelia_pathlib/_macos.py b/src/janelia_pathlib/_macos.py index 473662e..880347d 100644 --- a/src/janelia_pathlib/_macos.py +++ b/src/janelia_pathlib/_macos.py @@ -1,7 +1,6 @@ -"""macOS-specific share resolution and on-demand mounting.""" - from __future__ import annotations +import os import re import subprocess import time @@ -17,6 +16,22 @@ _SMB_MOUNT_LINE = re.compile(r"^//(?:[^@]+@)?([^/]+)/(.+?) on (\S+) \(smbfs") +_ENV_MOUNT_TIMEOUT = "JANELIA_PATHLIB_MOUNT_TIMEOUT" +_DEFAULT_MOUNT_TIMEOUT = 30.0 + + +def default_mount_timeout() -> float: + """Seconds to wait for an on-demand mount, from ``JANELIA_PATHLIB_MOUNT_TIMEOUT``. + + The wait covers a native SMB credential prompt, so it budgets for a + person typing a password -- raise it for a slow authenticator, or drop + it to near-zero in headless contexts where no one is there to answer. + """ + try: + return float(os.environ.get(_ENV_MOUNT_TIMEOUT, _DEFAULT_MOUNT_TIMEOUT)) + except ValueError: + return _DEFAULT_MOUNT_TIMEOUT + def _live_smb_mounts() -> list[tuple[str, str, str]]: """List live SMB mounts as ``(host, share_name, mountpoint)`` from `mount` output.""" @@ -48,24 +63,33 @@ def find_live_mount(host: str, share_name: str) -> str | None: return None -def find_share_for_mac_mountpoint(mountpoint: str, shares: Iterable[Share]) -> Share | None: - """Find which of ``shares`` is actually live-mounted at ``mountpoint``. +def find_share_for_mac_path(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: + """Find which of ``shares`` a ``/Volumes//...`` path points at. Disambiguates a bare ``/Volumes/`` path when multiple shares would guess the same mountpoint name (e.g. two shares on different hosts that happen to share a name both default to ``/Volumes/``, with the second one actually landing at ``/Volumes/-1``) by checking which share's SMB host+name is actually mounted there right now. + + Returns (share, subpath), or None if the path isn't under a live mount. """ + normalized = re.sub(r"/+$", "", path_str.strip()) + parts = normalized.split("/") + # /Volumes//rest... -> mountpoint is /Volumes/ + if len(parts) < 3 or parts[1] != "Volumes": + return None + mountpoint = "/".join(parts[:3]) + for mount_host, mount_share, live_point in _live_smb_mounts(): if live_point == mountpoint: share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: - return share + return share, normalized[len(mountpoint) :].lstrip("/") return None -def ensure_mounted(share_info: Share, *, timeout: float = 30) -> Path | None: +def ensure_mounted(share_info: Share, *, timeout: float | None = None) -> Path | None: """Mount ``share_info`` on macOS if needed and return its real local mountpoint. Resolution is keyed on ``(host, share_name)`` from the share's ``mac_path`` @@ -73,23 +97,28 @@ def ensure_mounted(share_info: Share, *, timeout: float = 30) -> Path | None: because macOS names mountpoints after the share name alone: two shares with the same name on different servers collide, and the second mounted gets suffixed (``/Volumes/-1``). + + ``timeout`` defaults to ``default_mount_timeout()``. """ + if timeout is None: + timeout = default_mount_timeout() host, share_name = share_info.host, share_info.share_name - smb_url = share_info.mac_path - existing = find_live_mount(host, share_name) - if existing is not None: - return Path(existing) + found = find_live_mount(host, share_name) + if found is not None: + return Path(found) - subprocess.run(["open", smb_url], check=True) + # `open` goes through the native SMB client so it can use Keychain + # credentials or prompt for them; unlike `mount_smbfs` it returns as + # soon as the mount is *requested*. There's no handle to wait on, and + # `mount` only prints a snapshot, so poll until the share shows up. + subprocess.run(["open", share_info.mac_path], check=True) - # Wait for the mount to appear deadline = time.monotonic() + timeout - while time.monotonic() < deadline: + while True: found = find_live_mount(host, share_name) if found is not None: return Path(found) + if time.monotonic() >= deadline: + return None time.sleep(0.5) - - found = find_live_mount(host, share_name) - return Path(found) if found is not None else None diff --git a/src/janelia_pathlib/_match.py b/src/janelia_pathlib/_match.py index 5fc7fda..c2add64 100644 --- a/src/janelia_pathlib/_match.py +++ b/src/janelia_pathlib/_match.py @@ -16,54 +16,32 @@ def _to_forward_slash(path: str) -> str: return path.replace("\\", "/") -def _find_share_by_fixed_path( +def find_share_by_path_field( path_str: str, shares: Iterable[Share], attr: str ) -> tuple[Share, str] | None: """Find which of ``shares`` a path falls under by prefix-matching ``attr``. - For path fields that are already unambiguous on their own (``linux_path``, - fixed and admin-assigned; ``windows_path``, a host-qualified UNC path) -- - unlike a macOS mountpoint name or Windows drive letter, neither needs a - live-state check. + Works for the path fields that identify a share on their own, with no + live state: ``linux_path`` (fixed and admin-assigned) and + ``windows_path`` (a host-qualified UNC path). Since neither depends on + the machine it's read on, both are worth trying regardless of which OS + is running. """ normalized = _to_forward_slash(path_str.strip()) normalized = re.sub(r"/+$", "", normalized) - best_share: Share | None = None - best_prefix = "" for share in shares: candidate = _to_forward_slash(re.sub(r"[/\\]+$", "", getattr(share, attr) or "")) - if not candidate: + if not candidate or not normalized.startswith(candidate): continue - if normalized.startswith(candidate) and len(candidate) > len(best_prefix): - rest = normalized[len(candidate) :] - if rest == "" or rest.startswith("/"): - best_share = share - best_prefix = candidate + # Only a match on a segment boundary: /labs/golf must not claim + # /labs/golflab/data. No share nests under another (asserted at + # load), so the first boundary match is the only one. + rest = normalized[len(candidate) :] + if rest == "" or rest.startswith("/"): + return share, rest.lstrip("/") - if best_share is None: - return None - subpath = normalized[len(best_prefix) :] - return best_share, subpath[1:] if subpath.startswith("/") else subpath - - -def find_share_for_linux_mount(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: - """Find which of ``shares`` a Linux path falls under. - - Linux mount points are fixed and admin-assigned, so (unlike macOS - mount-name collisions or Windows drive letters) a static ``linux_path`` - prefix match is already fully authoritative -- no live state to check. - """ - return _find_share_by_fixed_path(path_str, shares, "linux_path") - - -def find_share_for_windows_unc(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: - """Find which of ``shares`` a Windows UNC path (\\\\host\\share\\...) falls under. - - A UNC path already encodes the host, so (unlike a drive letter) it's - unambiguous on its own -- no live-state check needed. - """ - return _find_share_by_fixed_path(path_str, shares, "windows_path") + return None def find_share_for_smb_url(path_str: str, shares: Iterable[Share]) -> tuple[Share, str] | None: @@ -87,58 +65,32 @@ def find_share_for_smb_url(path_str: str, shares: Iterable[Share]) -> tuple[Shar def find_share_for_mounted_path( path_str: str, os_name: str, shares: Iterable[Share] ) -> tuple[Share, str] | None: - """Detect whether ``path_str`` falls under a mounted Janelia share. + """Find which of ``shares`` a local mountpoint path points at. - Dispatches to the OS-specific check: a live mount-table/SMB-mapping - lookup on macOS and Windows (the only reliable way to resolve a path - whose mountpoint name doesn't uniquely determine the share -- macOS - mount-name collisions, Windows drive-letter mappings), or a plain - static ``linux_path`` prefix match on Linux (already fully - authoritative there, no live state needed). + Only handles the two formats that *can't* be resolved from the string + alone -- a macOS ``/Volumes/`` mountpoint and a Windows drive + letter. Both name a share indirectly, via per-machine state: macOS + names mountpoints after the share name only, so two shares on + different hosts collide and the second gets suffixed; a drive letter + is an arbitrary per-user mapping. Only the live mount table resolves + either, and only on the machine holding the mount -- hence ``os_name``. Returns (share, subpath), or None if nothing matches. """ - if os_name == "linux": - return find_share_for_linux_mount(path_str, shares) - - normalized = _to_forward_slash(path_str.strip()) - normalized = re.sub(r"/+$", "", normalized) - if os_name == "mac": - parts = normalized.split("/") - # /Volumes//rest... -> mountpoint is /Volumes/ - if len(parts) >= 3 and parts[1] == "Volumes": - mountpoint = "/".join(parts[:3]) - share = _macos.find_share_for_mac_mountpoint(mountpoint, shares) - if share is not None: - subpath = normalized[len(mountpoint) :] - return share, subpath[1:] if subpath.startswith("/") else subpath - return None - + return _macos.find_share_for_mac_path(path_str, shares) if os_name == "windows": - match = _windows.DRIVE_RE.match(path_str.strip()) - if match is not None: - drive, rest = match.group(1), match.group(2) or "" - share = _windows.find_share_for_windows_drive(drive, shares) - if share is not None: - subpath = _to_forward_slash(rest) - return share, subpath[1:] if subpath.startswith("/") else subpath - return None - - # Not a drive letter -- a UNC path (\\host\share\...) is already - # unambiguous, same as linux_path, so no live-state check is needed. - return find_share_for_windows_unc(path_str, shares) - + return _windows.find_share_for_windows_drive(path_str, shares) return None -def resolve_janelia_uri( +def find_share_for_janelia_uri( path_str: str, shares: Iterable[Share] | None = None ) -> tuple[Share, str] | None: """Match a ``janelia:/`` string against known shares. - The URI body is a Linux path, so this is the same prefix match - ``find_share_for_linux_mount`` does. A ``janelia://...`` string + The URI body is a Linux path, so this is a ``linux_path`` prefix + match like any other. A ``janelia://...`` string (an older, host-qualified form) is rejected: ``//`` would make the first path segment parse as a URI authority. """ @@ -148,31 +100,22 @@ def resolve_janelia_uri( match = re.match(r"janelia:(/(?!/).*)$", path_str.strip()) if match is None: return None - return find_share_for_linux_mount(match.group(1), shares) + return find_share_by_path_field(match.group(1), shares, "linux_path") -def resolve(path_str: str, shares: Iterable[Share] | None = None) -> tuple[Share, str] | None: - """Match a path string against known shares on any OS. +def find_share_for_path( + path_str: str, shares: Iterable[Share] | None = None +) -> tuple[Share, str] | None: + """Match a path string against known shares, in any OS's format. Returns (share, subpath) where subpath is the relative portion after the share mount prefix, or None if no share matches. - First checks whether the path falls under a mounted share for the - current OS (``find_share_for_mounted_path``) -- on macOS and Windows - this consults live mount state to resolve cases a static string alone - can't: a bare ``/Volumes/`` Mac path can't be told apart from a - static string alone when two shares collide on the same mount name - (macOS names mountpoints after the share name alone, ignoring the - host), and a Windows drive letter isn't in the share data at all -- - only live state can map it to a share. On Linux this is just the - static ``linux_path`` match, already fully authoritative. - - Then falls back to the forms that are self-describing enough to match - on any OS, no live state needed: a ``linux_path`` prefix (fixed and - admin-assigned), a ``windows_path`` UNC prefix (carries its own host), - and an ``smb://`` URL (likewise). A ``/Volumes/`` mountpoint or a - drive letter can't join them -- resolving either needs the live mount - table, which only exists on the machine holding the mount. + A Linux path, a UNC path, and an ``smb://`` URL each name a share on + their own, so all three are tried regardless of which OS is running -- + a UNC path passed in on Linux still resolves. Only a macOS + ``/Volumes/`` mountpoint and a Windows drive letter need the + live mount table, so those are tried first and only on their own OS. """ if shares is None: shares = load_shares() @@ -181,12 +124,9 @@ def resolve(path_str: str, shares: Iterable[Share] | None = None) -> tuple[Share if live_result is not None: return live_result - for matcher in ( - find_share_for_linux_mount, - find_share_for_windows_unc, - find_share_for_smb_url, - ): - result = matcher(path_str, shares) + for attr in ("linux_path", "windows_path"): + result = find_share_by_path_field(path_str, shares, attr) if result is not None: return result - return None + + return find_share_for_smb_url(path_str, shares) diff --git a/src/janelia_pathlib/_share.py b/src/janelia_pathlib/_share.py index c070e65..5364c5b 100644 --- a/src/janelia_pathlib/_share.py +++ b/src/janelia_pathlib/_share.py @@ -6,6 +6,7 @@ import warnings from dataclasses import dataclass from functools import cache +from itertools import pairwise from typing import TYPE_CHECKING from urllib.error import URLError @@ -14,16 +15,15 @@ if TYPE_CHECKING: from collections.abc import Iterable, Mapping -_OS_MAP = {"Darwin": "mac", "Linux": "linux", "Windows": "windows"} - _SMB_URL_RE = re.compile(r"smb://([^/]+)/(.+)") def current_os() -> str: """Return the current OS as 'linux', 'mac', or 'windows'.""" - os_name = _OS_MAP.get(platform.system()) + system = platform.system() + os_name = {"Darwin": "mac", "Linux": "linux", "Windows": "windows"}.get(system) if os_name is None: - raise RuntimeError(f"Unsupported platform: {platform.system()}") + raise RuntimeError(f"Unsupported platform: {system}") return os_name @@ -81,6 +81,28 @@ def uri(self) -> str: return f"janelia:{self.linux_path}" +def _warn_if_nested(shares: tuple[Share, ...], attr: str, sep: str) -> None: + """Warn if any share's ``attr`` path sits inside another share's. + + Matching takes the first share whose path prefixes the input, which is + only unambiguous while no share nests under another. That holds across + all live Fileglancer data (a share mounted inside another share's mount + would be odd to administer), but it's an assumption about data we don't + control, so check rather than trust. + + Sorting puts any nested path directly after the one containing it, so + comparing neighbours is enough -- no quadratic scan over every pair. + """ + paths = sorted({getattr(share, attr) for share in shares if getattr(share, attr)}) + for outer, inner in pairwise(paths): + if inner.startswith(outer.rstrip(sep) + sep): + warnings.warn( + f"Share {inner!r} is nested inside {outer!r}; paths under it may " + f"resolve to the wrong share.", + stacklevel=2, + ) + + def find_share_by_host_and_name( host: str, share_name: str, shares: Iterable[Share] ) -> Share | None: @@ -130,4 +152,8 @@ def load_shares() -> tuple[Share, ...]: data_text = cache_file.read_text() raw = json.loads(data_text) - return tuple(Share.from_api_entry(entry) for entry in raw) + shares = tuple(Share.from_api_entry(entry) for entry in raw) + + _warn_if_nested(shares, "linux_path", "/") + _warn_if_nested(shares, "windows_path", "\\") + return shares diff --git a/src/janelia_pathlib/_translate.py b/src/janelia_pathlib/_translate.py index 0b6bea3..aaf571c 100644 --- a/src/janelia_pathlib/_translate.py +++ b/src/janelia_pathlib/_translate.py @@ -3,26 +3,47 @@ from typing import TYPE_CHECKING from ._macos import ensure_mounted -from ._match import resolve, resolve_janelia_uri +from ._match import find_share_for_janelia_uri, find_share_for_path from ._share import current_os if TYPE_CHECKING: from pathlib import Path -def from_janelia_uri(path_str: str, *, timeout: float = 30) -> str: +def from_janelia_uri(path_str: str, *, timeout: float | None = None) -> str: """Translate a ``janelia:`` URI to a real, usable local path string. - A plain path is returned unchanged. A ``janelia:/`` string - is resolved to the matching Janelia share and, on macOS, mounted on - demand if not already mounted -- returning its real local mountpoint - rather than a guessed ``/Volumes/`` path, which may not exist or - may belong to a different, colliding share. + The returned path is in the running OS's own format: a ``/Volumes`` + mountpoint on macOS, ``linux_path`` on Linux, a UNC path on Windows. + + On macOS the share is mounted on demand if it isn't already, which may + trigger a credential prompt. The mountpoint is read back from the live + mount table rather than guessed from the share name, since two shares + with the same name on different hosts both want ``/Volumes/`` + and the second one actually lands at ``/Volumes/-1``. Linux and + Windows need no mounting step -- shares are already at a fixed path or + reachable directly by UNC path. + + Args: + path_str: A ``janelia:/`` URI. Any other string is + returned unchanged, so this is safe to call on paths that may + or may not be URIs. + timeout: Seconds to wait for an on-demand macOS mount. Defaults to + ``JANELIA_PATHLIB_MOUNT_TIMEOUT`` (30s), which budgets for + someone typing a password. Ignored off macOS. + + Returns: + A local path string, or ``path_str`` unchanged if it isn't a + ``janelia:`` URI. + + Raises: + RuntimeError: if the URI names no known share, or if a macOS mount + fails or times out. """ if not path_str.strip().startswith("janelia:"): return path_str - result = resolve_janelia_uri(path_str) + result = find_share_for_janelia_uri(path_str) if result is None: raise RuntimeError(f"No known share for URI: {path_str}") share_info, subpath = result @@ -48,13 +69,28 @@ def from_janelia_uri(path_str: str, *, timeout: float = 30) -> str: def to_janelia_uri(path: str | Path) -> str: """Translate a path to its canonical ``janelia:`` URI form. - If the path matches a known Janelia share, returns its unambiguous - ``janelia:/`` identity -- safe to round-trip through - ``from_janelia_uri`` on any OS, unlike a ``/Volumes/`` mountpoint - name, which can collide with another share's on macOS. Otherwise - returns the plain path string unchanged. + The URI is built from the share's Linux path, which is fixed and + admin-assigned, so it stays valid across machines and across a share + moving to new hardware -- unlike a ``/Volumes/`` mountpoint + name, which is per-machine and can collide with another share's. + Store or pass around the URI, and call ``from_janelia_uri`` at the + point of use to get something the filesystem will accept. + + A Linux path, a Windows UNC path and an ``smb://`` URL each identify a + share on their own, so all three are recognized on any OS. A macOS + ``/Volumes`` mountpoint or a Windows drive letter names a share only + via per-machine state, so those are recognized only while running on + that OS with the share mounted, and pass through unchanged otherwise. + + Args: + path: A local path in any of the formats above. Anything that + doesn't match a known share is returned unchanged. + + Returns: + A ``janelia:/`` URI, or ``str(path)`` unchanged if it + doesn't fall under a known share. """ - result = resolve(str(path)) + result = find_share_for_path(str(path)) if result is None: return str(path) share_info, subpath = result diff --git a/src/janelia_pathlib/_windows.py b/src/janelia_pathlib/_windows.py index 87965dc..375a146 100644 --- a/src/janelia_pathlib/_windows.py +++ b/src/janelia_pathlib/_windows.py @@ -18,7 +18,7 @@ from ._share import Share -DRIVE_RE = re.compile(r"^([A-Za-z]:)(\\.*)?$") +_DRIVE_RE = re.compile(r"^([A-Za-z]:)(\\.*)?$") def _live_windows_drives() -> list[tuple[str, str, str]]: @@ -58,12 +58,26 @@ def _live_windows_drives() -> list[tuple[str, str, str]]: return drives -def find_share_for_windows_drive(drive: str, shares: Iterable[Share]) -> Share | None: - """Find which of ``shares`` is actually mapped to Windows drive letter ``drive``.""" - drive_norm = drive.rstrip("\\").upper() +def find_share_for_windows_drive( + path_str: str, shares: Iterable[Share] +) -> tuple[Share, str] | None: + """Find which of ``shares`` a mapped drive path (``X:\\...``) points at. + + A drive letter is an arbitrary per-user mapping that says nothing about + the share on its own, so this checks which share is actually mapped to + it right now. + + Returns (share, subpath), or None if the path isn't a mapped drive. + """ + match = _DRIVE_RE.match(path_str.strip()) + if match is None: + return None + drive_norm = match.group(1).rstrip("\\").upper() + subpath = (match.group(2) or "").replace("\\", "/").strip("/") + for mount_host, mount_share, local in _live_windows_drives(): if local.rstrip("\\").upper() == drive_norm: share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: - return share + return share, subpath return None diff --git a/tests/fixtures/shares.json b/tests/fixtures/shares.json index 6a252df..22ff1e3 100644 --- a/tests/fixtures/shares.json +++ b/tests/fixtures/shares.json @@ -35,7 +35,7 @@ "zone": "Charlie", "group": "charlie", "storage": "home", - "linux_path": "/labs/charlie", + "linux_path": "/labs/charlie/home", "windows_path": "\\\\fileserver.example.org\\charlie$", "mac_path": "smb://fileserver.example.org/charlie$" }, @@ -47,6 +47,22 @@ "windows_path": "\\\\fileserver.example.org\\charlie", "mac_path": "smb://fileserver.example.org/charlie" }, + { + "zone": "Golf", + "group": "golf", + "storage": "primary", + "linux_path": "/labs/golf", + "windows_path": "\\\\fileserver.example.org\\golf", + "mac_path": "smb://fileserver.example.org/golf" + }, + { + "zone": "Golflab", + "group": "golflab", + "storage": "primary", + "linux_path": "/labs/golflab", + "windows_path": "\\\\fileserver.example.org\\golflab", + "mac_path": "smb://fileserver.example.org/golflab" + }, { "zone": "Delta", "group": "delta", diff --git a/tests/test_janelia_path.py b/tests/test_janelia_path.py index 24b051d..fc4a795 100644 --- a/tests/test_janelia_path.py +++ b/tests/test_janelia_path.py @@ -7,12 +7,13 @@ import json import subprocess import sys +import warnings from unittest.mock import patch from urllib.error import URLError import pytest -from janelia_pathlib import _fetch, _share, _windows, from_janelia_uri, to_janelia_uri +from janelia_pathlib import _fetch, _macos, _share, _windows, from_janelia_uri, to_janelia_uri mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS paths") linux_only = pytest.mark.skipif(sys.platform != "linux", reason="Linux paths") @@ -72,13 +73,54 @@ def test_to_janelia_uri_smb_url_with_dollar_share(): ) +@pytest.mark.parametrize("os_name", ["linux", "mac", "windows"]) +@pytest.mark.parametrize( + "path", + [ + "/labs/alpha/alphalab/data", + "\\\\fileserver.example.org\\alphalab\\data", + "smb://fileserver.example.org/alphalab/data", + ], +) +def test_self_describing_formats_resolve_on_every_os(path, os_name): + """A linux path, a UNC path and an smb:// URL each name a share on + their own, so none of them should depend on which OS is running.""" + with patch("janelia_pathlib._match.current_os", return_value=os_name): + assert to_janelia_uri(path) == "janelia:/labs/alpha/alphalab/data" + + def test_to_janelia_uri_no_match_passthrough(): assert to_janelia_uri("/tmp/local/scratch/file.txt") == "/tmp/local/scratch/file.txt" -def test_to_janelia_uri_longest_prefix_wins(): - """Both /labs/charlie and /labs/charlie/charlie match; the longer wins.""" +def test_to_janelia_uri_sibling_shares_under_one_group(): + """A group's shares sit side by side (/labs/charlie/home, .../charlie), + never nested, so each resolves to itself.""" assert to_janelia_uri("/labs/charlie/charlie/data") == "janelia:/labs/charlie/charlie/data" + assert to_janelia_uri("/labs/charlie/home/data") == "janelia:/labs/charlie/home/data" + + +def test_to_janelia_uri_substring_share_name_is_not_a_prefix(): + """/labs/golf is a string prefix of /labs/golflab but not a path prefix: + matching must respect segment boundaries, or every golflab path would + resolve to the golf share.""" + assert to_janelia_uri("/labs/golflab/data") == "janelia:/labs/golflab/data" + assert to_janelia_uri("/labs/golf/data") == "janelia:/labs/golf/data" + assert to_janelia_uri("/labs/golflab") == "janelia:/labs/golflab" + assert to_janelia_uri("/labs/golf") == "janelia:/labs/golf" + + +def test_to_janelia_uri_substring_share_name_unc(): + """Same boundary rule on the windows_path field.""" + assert ( + to_janelia_uri("\\\\fileserver.example.org\\golflab\\data") == "janelia:/labs/golflab/data" + ) + + +def test_to_janelia_uri_partial_segment_matches_nothing(): + """A path that shares a prefix with a share but diverges mid-segment + belongs to no share at all -- it must not fall back to the shorter one.""" + assert to_janelia_uri("/labs/golfcourse/data") == "/labs/golfcourse/data" def test_to_janelia_uri_trailing_slash(): @@ -241,9 +283,11 @@ def test_find_share_for_windows_drive_matches_mapping(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) - assert share is not None + result = _windows.find_share_for_windows_drive("Z:\\data", _share.load_shares()) + assert result is not None + share, subpath = result assert share.linux_path == "/labs/alpha/alphalab" + assert subpath == "data" def test_find_share_for_windows_drive_is_case_insensitive(): @@ -253,9 +297,9 @@ def test_find_share_for_windows_drive_is_case_insensitive(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:\\"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("z:", _share.load_shares()) - assert share is not None - assert share.linux_path == "/labs/alpha/alphalab" + result = _windows.find_share_for_windows_drive("z:", _share.load_shares()) + assert result is not None + assert result[0].linux_path == "/labs/alpha/alphalab" def test_find_share_for_windows_drive_resolves_colliding_share_names(): @@ -269,17 +313,19 @@ def test_find_share_for_windows_drive_resolves_colliding_share_names(): ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): shares = _share.load_shares() - assert _windows.find_share_for_windows_drive("X:", shares).linux_path == "/nrs/foxtrot" - assert _windows.find_share_for_windows_drive("Y:", shares).linux_path == "/nearline/foxtrot" + assert _windows.find_share_for_windows_drive("X:", shares)[0].linux_path == "/nrs/foxtrot" + assert ( + _windows.find_share_for_windows_drive("Y:", shares)[0].linux_path == "/nearline/foxtrot" + ) def test_find_share_for_windows_drive_single_mapping_is_json_object(): """ConvertTo-Json emits a bare object, not a list, for a single mapping.""" mapping = json.dumps({"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - share = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) - assert share is not None - assert share.linux_path == "/labs/alpha/alphalab" + result = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) + assert result is not None + assert result[0].linux_path == "/labs/alpha/alphalab" def test_find_share_for_windows_drive_unmapped_returns_none(): @@ -290,6 +336,21 @@ def test_find_share_for_windows_drive_unmapped_returns_none(): assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None +def test_find_share_for_windows_drive_ignores_non_drive_paths(): + """A UNC or linux path isn't a drive mapping -- that's the static + matchers' job, so this shouldn't claim it.""" + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + shares = _share.load_shares() + assert _windows.find_share_for_windows_drive("/labs/alpha/alphalab", shares) is None + assert ( + _windows.find_share_for_windows_drive("\\\\fileserver.example.org\\alphalab", shares) + is None + ) + + def test_find_share_for_windows_drive_unknown_share_returns_none(): """A drive mapped to a share we don't know about isn't ours to translate.""" mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) @@ -329,6 +390,78 @@ def test_round_trip_non_janelia_path_is_untouched(): # --- Offline / fetch robustness --- +def test_load_shares_warns_on_nested_shares(monkeypatch, tmp_path): + """First-match-wins is only unambiguous while no share sits inside + another, so a nested pair in the API data should say so loudly rather + than silently resolving to whichever came first.""" + cache_file = tmp_path / "shares.json" + cache_file.write_text( + json.dumps( + [ + { + "zone": "Nest", + "linux_path": "/labs/outer", + "windows_path": "\\\\fileserver.example.org\\outer", + "mac_path": "smb://fileserver.example.org/outer", + }, + { + "zone": "Nest", + "linux_path": "/labs/outer/inner", + "windows_path": "\\\\fileserver.example.org\\inner", + "mac_path": "smb://fileserver.example.org/inner", + }, + ] + ) + ) + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) + _share.load_shares.cache_clear() + try: + with pytest.warns(UserWarning, match="nested inside"): + _share.load_shares() + finally: + _share.load_shares.cache_clear() + + +def test_load_shares_quiet_for_sibling_shares(tmp_path): + """Sibling shares under one group share a prefix but don't nest.""" + _share.load_shares.cache_clear() + try: + with warnings.catch_warnings(): + warnings.simplefilter("error") + _share.load_shares() + finally: + _share.load_shares.cache_clear() + + +def test_default_mount_timeout_falls_back_without_env(monkeypatch): + monkeypatch.delenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", raising=False) + assert _macos.default_mount_timeout() == 30.0 + + +def test_default_mount_timeout_reads_env(monkeypatch): + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "90") + assert _macos.default_mount_timeout() == 90.0 + + +def test_default_mount_timeout_ignores_garbage(monkeypatch): + """A malformed value shouldn't crash a path translation.""" + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "soon please") + assert _macos.default_mount_timeout() == 30.0 + + +@mac_only +def test_ensure_mounted_honors_env_timeout(monkeypatch): + """With a zero timeout, a share that never appears gives up immediately + rather than waiting out the 30s default.""" + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "0") + share = next(s for s in _share.load_shares() if s.share_name == "alphalab") + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with patch("janelia_pathlib._macos.time.sleep") as mock_sleep: + assert _macos.ensure_mounted(share) is None + mock_sleep.assert_not_called() + + def test_fetch_shares_uses_timeout(monkeypatch, tmp_path): """fetch_shares passes a timeout to urlopen so it can't hang indefinitely.""" From 2af6f3322e27dc9c4e4cf129652eaee334a33396 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 17:23:16 -0400 Subject: [PATCH 5/7] Warn when passing mount-looking path that isn't mounted --- src/janelia_pathlib/_macos.py | 15 +++++++++++ src/janelia_pathlib/_windows.py | 14 +++++++++++ tests/test_janelia_path.py | 44 ++++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/janelia_pathlib/_macos.py b/src/janelia_pathlib/_macos.py index 880347d..d6f37a6 100644 --- a/src/janelia_pathlib/_macos.py +++ b/src/janelia_pathlib/_macos.py @@ -4,6 +4,7 @@ import re import subprocess import time +import warnings from pathlib import Path from typing import TYPE_CHECKING @@ -86,6 +87,20 @@ def find_share_for_mac_path(path_str: str, shares: Iterable[Share]) -> tuple[Sha share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: return share, normalized[len(mountpoint) :].lstrip("/") + + # Not mounted, so it passes through untranslated -- indistinguishable to + # the caller from a path that was never a share. If the mountpoint name + # matches a known share, say so. macOS suffixes a colliding mountpoint + # (/Volumes/foo-1), so match the stem too. + mount_name = parts[2] + stem = re.sub(r"-\d+$", "", mount_name) + if any(share.share_name in (mount_name, stem) for share in shares): + warnings.warn( + f"{path_str!r} looks like Janelia share {stem!r}, but it isn't mounted " + f"here, so it can't be translated. Mount it, or use from_janelia_uri() " + f"to mount it on demand.", + stacklevel=3, + ) return None diff --git a/src/janelia_pathlib/_windows.py b/src/janelia_pathlib/_windows.py index 375a146..9ad5be9 100644 --- a/src/janelia_pathlib/_windows.py +++ b/src/janelia_pathlib/_windows.py @@ -9,6 +9,7 @@ import json import re import subprocess +import warnings from typing import TYPE_CHECKING from ._share import find_share_by_host_and_name @@ -80,4 +81,17 @@ def find_share_for_windows_drive( share = find_share_by_host_and_name(mount_host, mount_share, shares) if share is not None: return share, subpath + # Mapped, but to a share we don't know -- not ours to translate. + return None + + # A drive letter carries no share name, so unlike a /Volumes path there's + # nothing to match against to tell a lapsed Janelia mapping from any other + # local drive. Warn anyway: a caller passing a drive letter here is asking + # for it to be translated, and silent passthrough hides why it wasn't. + warnings.warn( + f"{path_str!r} is not a mapped network drive, so it can't be " + f"translated. Map it, or use from_janelia_uri() with the share's " + f"janelia: URI.", + stacklevel=3, + ) return None diff --git a/tests/test_janelia_path.py b/tests/test_janelia_path.py index fc4a795..4fcdc55 100644 --- a/tests/test_janelia_path.py +++ b/tests/test_janelia_path.py @@ -152,6 +152,37 @@ def test_to_janelia_uri_resolves_colliding_mac_mount_via_live_state(): assert to_janelia_uri("/Volumes/foxtrot-1/data") == "janelia:/nearline/foxtrot/data" +@mac_only +def test_to_janelia_uri_warns_when_known_share_is_unmounted(): + """An unmounted share passes through untranslated, which otherwise looks + identical to a path that was never a share -- so say which it was.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with pytest.warns(UserWarning, match="isn't mounted"): + assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" + + +@mac_only +def test_to_janelia_uri_warns_for_suffixed_collision_mountpoint(): + """macOS suffixes a colliding mountpoint, so /Volumes/foxtrot-1 should + still be recognized as share `foxtrot`.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with pytest.warns(UserWarning, match="foxtrot"): + to_janelia_uri("/Volumes/foxtrot-1/data") + + +@mac_only +def test_to_janelia_uri_quiet_for_non_janelia_volume(): + """A USB stick or disk image isn't ours -- no warning for /Volumes names + that match no known share.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert to_janelia_uri("/Volumes/MyBackupDrive/x") == "/Volumes/MyBackupDrive/x" + + @mac_only def test_to_janelia_uri_mac_mount_not_live_passes_through_unchanged(): """If live `mount` output has nothing at this mountpoint (e.g. not actually @@ -160,7 +191,8 @@ def test_to_janelia_uri_mac_mount_not_live_passes_through_unchanged(): exactly the same way the original mount-name collision bug was.""" with patch("janelia_pathlib._macos.subprocess.run") as mock_run: mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" + with pytest.warns(UserWarning, match="isn't mounted"): + assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" # --- from_janelia_uri() tests --- @@ -333,7 +365,8 @@ def test_find_share_for_windows_drive_unmapped_returns_none(): [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] ) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None + with pytest.warns(UserWarning, match="not a mapped network drive"): + assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None def test_find_share_for_windows_drive_ignores_non_drive_paths(): @@ -352,10 +385,13 @@ def test_find_share_for_windows_drive_ignores_non_drive_paths(): def test_find_share_for_windows_drive_unknown_share_returns_none(): - """A drive mapped to a share we don't know about isn't ours to translate.""" + """A drive mapped to a share we don't know about isn't ours to translate -- + and unlike an unmapped letter, nothing is wrong, so it shouldn't warn.""" mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - assert _windows.find_share_for_windows_drive("Z:", _share.load_shares()) is None + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert _windows.find_share_for_windows_drive("Z:", _share.load_shares()) is None @pytest.mark.parametrize("stdout", ["", " ", "not json at all"]) From 9361c1393d90b0a6b347390fdbc1f712f4a5b675 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 17:28:29 -0400 Subject: [PATCH 6/7] Split tests into per-module files --- tests/conftest.py | 11 +- tests/test_janelia_path.py | 618 ------------------------------------- tests/test_share.py | 200 ++++++++++++ tests/test_translate.py | 305 ++++++++++++++++++ tests/test_windows.py | 119 +++++++ 5 files changed, 634 insertions(+), 619 deletions(-) delete mode 100644 tests/test_janelia_path.py create mode 100644 tests/test_share.py create mode 100644 tests/test_translate.py create mode 100644 tests/test_windows.py diff --git a/tests/conftest.py b/tests/conftest.py index a11888b..54536ce 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,15 +1,24 @@ """Pytest configuration: point janelia-pathlib at a fake shares file.""" import os +import sys from pathlib import Path +import pytest + # Set before janelia_pathlib is imported by any test module so the registry # loads from the fixture instead of the user cache dir (which may be empty # or contain real Janelia share data we don't want tests to depend on). os.environ["JANELIA_PATHLIB_SHARES_PATH"] = str(Path(__file__).parent / "fixtures" / "shares.json") -# _load_shares() refreshes from the network on every call by default; tests +# load_shares() refreshes from the network on every call by default; tests # must never depend on (or overwrite) real network data, so disable fetching # globally. Tests that specifically exercise fetch behavior override this # themselves via monkeypatch. os.environ["JANELIA_PATHLIB_NO_FETCH"] = "1" + +# Only for tests whose *output* is OS-specific. Tests that patch a live mount +# table run everywhere -- see test_windows.py. +mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS paths") +linux_only = pytest.mark.skipif(sys.platform != "linux", reason="Linux paths") +windows_only = pytest.mark.skipif(sys.platform != "win32", reason="Windows paths") diff --git a/tests/test_janelia_path.py b/tests/test_janelia_path.py deleted file mode 100644 index 4fcdc55..0000000 --- a/tests/test_janelia_path.py +++ /dev/null @@ -1,618 +0,0 @@ -"""Tests for janelia-pathlib. - -Tests run against a fictional share fixture (see tests/fixtures/shares.json -and tests/conftest.py). No real Janelia share data is referenced here. -""" - -import json -import subprocess -import sys -import warnings -from unittest.mock import patch -from urllib.error import URLError - -import pytest - -from janelia_pathlib import _fetch, _macos, _share, _windows, from_janelia_uri, to_janelia_uri - -mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS paths") -linux_only = pytest.mark.skipif(sys.platform != "linux", reason="Linux paths") -windows_only = pytest.mark.skipif(sys.platform != "win32", reason="Windows paths") - - -# --- to_janelia_uri() tests --- - - -def test_to_janelia_uri_from_linux_path(): - assert to_janelia_uri("/labs/alpha/alphalab/data") == "janelia:/labs/alpha/alphalab/data" - - -@mac_only -def test_to_janelia_uri_from_mac_path(): - """Resolving a Mac path requires live mount state (mac_path alone could - be ambiguous), so this must be live-checked, not a static guess.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess( - args=["mount"], - returncode=0, - stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", - ) - assert to_janelia_uri("/Volumes/alphalab/data") == "janelia:/labs/alpha/alphalab/data" - - -def test_to_janelia_uri_from_windows_path(): - """A UNC path carries its own host, so it resolves without live state -- - including when running on a non-Windows machine.""" - assert ( - to_janelia_uri("\\\\fileserver.example.org\\alphalab\\data") - == "janelia:/labs/alpha/alphalab/data" - ) - - -def test_to_janelia_uri_share_root_only(): - assert to_janelia_uri("/labs/alpha/alphalab") == "janelia:/labs/alpha/alphalab" - - -def test_to_janelia_uri_from_smb_url_on_any_os(): - """Same for an smb:// URL -- host and share name are both in the string.""" - assert ( - to_janelia_uri("smb://fileserver.example.org/alphalab/data") - == "janelia:/labs/alpha/alphalab/data" - ) - - -def test_to_janelia_uri_smb_url_share_root_only(): - assert to_janelia_uri("smb://fileserver.example.org/alphalab") == "janelia:/labs/alpha/alphalab" - - -def test_to_janelia_uri_smb_url_with_dollar_share(): - """Hidden shares (trailing $) are ordinary share names, not special syntax.""" - assert ( - to_janelia_uri("smb://fileserver.example.org/alpha$/data") - == "janelia:/labs/alpha/home/data" - ) - - -@pytest.mark.parametrize("os_name", ["linux", "mac", "windows"]) -@pytest.mark.parametrize( - "path", - [ - "/labs/alpha/alphalab/data", - "\\\\fileserver.example.org\\alphalab\\data", - "smb://fileserver.example.org/alphalab/data", - ], -) -def test_self_describing_formats_resolve_on_every_os(path, os_name): - """A linux path, a UNC path and an smb:// URL each name a share on - their own, so none of them should depend on which OS is running.""" - with patch("janelia_pathlib._match.current_os", return_value=os_name): - assert to_janelia_uri(path) == "janelia:/labs/alpha/alphalab/data" - - -def test_to_janelia_uri_no_match_passthrough(): - assert to_janelia_uri("/tmp/local/scratch/file.txt") == "/tmp/local/scratch/file.txt" - - -def test_to_janelia_uri_sibling_shares_under_one_group(): - """A group's shares sit side by side (/labs/charlie/home, .../charlie), - never nested, so each resolves to itself.""" - assert to_janelia_uri("/labs/charlie/charlie/data") == "janelia:/labs/charlie/charlie/data" - assert to_janelia_uri("/labs/charlie/home/data") == "janelia:/labs/charlie/home/data" - - -def test_to_janelia_uri_substring_share_name_is_not_a_prefix(): - """/labs/golf is a string prefix of /labs/golflab but not a path prefix: - matching must respect segment boundaries, or every golflab path would - resolve to the golf share.""" - assert to_janelia_uri("/labs/golflab/data") == "janelia:/labs/golflab/data" - assert to_janelia_uri("/labs/golf/data") == "janelia:/labs/golf/data" - assert to_janelia_uri("/labs/golflab") == "janelia:/labs/golflab" - assert to_janelia_uri("/labs/golf") == "janelia:/labs/golf" - - -def test_to_janelia_uri_substring_share_name_unc(): - """Same boundary rule on the windows_path field.""" - assert ( - to_janelia_uri("\\\\fileserver.example.org\\golflab\\data") == "janelia:/labs/golflab/data" - ) - - -def test_to_janelia_uri_partial_segment_matches_nothing(): - """A path that shares a prefix with a share but diverges mid-segment - belongs to no share at all -- it must not fall back to the shorter one.""" - assert to_janelia_uri("/labs/golfcourse/data") == "/labs/golfcourse/data" - - -def test_to_janelia_uri_trailing_slash(): - assert to_janelia_uri("/labs/alpha/alphalab/") == "janelia:/labs/alpha/alphalab" - - -def test_to_janelia_uri_windows_forward_slash(): - """Windows paths with forward slashes should still match.""" - assert ( - to_janelia_uri("//fileserver.example.org/alphalab/data") - == "janelia:/labs/alpha/alphalab/data" - ) - - -@mac_only -def test_to_janelia_uri_resolves_colliding_mac_mount_via_live_state(): - """Two shares (nrs/nearline) both nominally want /Volumes/foxtrot on macOS. - Static string matching alone can't tell them apart -- live `mount` output, - which records the actual SMB host for each mountpoint, must be checked.""" - mount_output = ( - "//user@nrs.example.org/foxtrot on /Volumes/foxtrot (smbfs, nodev)\n" - "//user@nearline.example.org/foxtrot on /Volumes/foxtrot-1 (smbfs, nodev)" - ) - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess( - args=["mount"], returncode=0, stdout=mount_output - ) - assert to_janelia_uri("/Volumes/foxtrot/data") == "janelia:/nrs/foxtrot/data" - assert to_janelia_uri("/Volumes/foxtrot-1/data") == "janelia:/nearline/foxtrot/data" - - -@mac_only -def test_to_janelia_uri_warns_when_known_share_is_unmounted(): - """An unmounted share passes through untranslated, which otherwise looks - identical to a path that was never a share -- so say which it was.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - with pytest.warns(UserWarning, match="isn't mounted"): - assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" - - -@mac_only -def test_to_janelia_uri_warns_for_suffixed_collision_mountpoint(): - """macOS suffixes a colliding mountpoint, so /Volumes/foxtrot-1 should - still be recognized as share `foxtrot`.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - with pytest.warns(UserWarning, match="foxtrot"): - to_janelia_uri("/Volumes/foxtrot-1/data") - - -@mac_only -def test_to_janelia_uri_quiet_for_non_janelia_volume(): - """A USB stick or disk image isn't ours -- no warning for /Volumes names - that match no known share.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - with warnings.catch_warnings(): - warnings.simplefilter("error") - assert to_janelia_uri("/Volumes/MyBackupDrive/x") == "/Volumes/MyBackupDrive/x" - - -@mac_only -def test_to_janelia_uri_mac_mount_not_live_passes_through_unchanged(): - """If live `mount` output has nothing at this mountpoint (e.g. not actually - mounted, or `mount` unavailable), the path is left unresolved rather than - guessed from the static mac_path field -- a guess here could be wrong in - exactly the same way the original mount-name collision bug was.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - with pytest.warns(UserWarning, match="isn't mounted"): - assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" - - -# --- from_janelia_uri() tests --- - - -def test_from_janelia_uri_no_match_passthrough(): - assert from_janelia_uri("/tmp/local/scratch/file.txt") == "/tmp/local/scratch/file.txt" - - -@linux_only -def test_from_janelia_uri_on_linux(): - assert from_janelia_uri("janelia:/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" - - -def test_from_janelia_uri_unknown_share_raises(): - with pytest.raises(RuntimeError, match="No known share"): - from_janelia_uri("janelia:/nope/not-a-share/data") - - -def test_from_janelia_uri_rejects_double_slash_form(): - """``janelia://host/share/...`` is the old form. ``//`` opens a URI - authority component, which a Linux path has no value for, so it's - rejected rather than silently parsing ``labs`` as an authority.""" - with pytest.raises(RuntimeError, match="No known share"): - from_janelia_uri("janelia://labs/alpha/alphalab/data") - - -@mac_only -def test_from_janelia_uri_mounts_on_demand(): - """from_janelia_uri should trigger `open smb://...` when the share isn't - mounted yet, then resolve the real mountpoint from live `mount` output.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mount_output_empty = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - mount_output_found = subprocess.CompletedProcess( - args=["mount"], - returncode=0, - stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", - ) - mock_run.side_effect = [mount_output_empty, None, mount_output_found] - result = from_janelia_uri("janelia:/labs/alpha/alphalab/data", timeout=2) - assert result == "/Volumes/alphalab/data" - open_call = mock_run.call_args_list[1] - args = open_call[0][0] - assert args[0] == "open" - assert args[1] == "smb://fileserver.example.org/alphalab" - - -@mac_only -def test_from_janelia_uri_already_mounted(): - """from_janelia_uri should return the existing mountpoint without calling open.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess( - args=["mount"], - returncode=0, - stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", - ) - result = from_janelia_uri("janelia:/labs/alpha/alphalab/data") - assert result == "/Volumes/alphalab/data" - for call in mock_run.call_args_list: - assert call[0][0][0] != "open" - - -@mac_only -def test_from_janelia_uri_resolves_colliding_share_name(): - """When two shares share a mount-name (nrs/nearline foxtrot collision), - from_janelia_uri must match on (host, share_name), not on the guessed - /Volumes/ path.""" - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess( - args=["mount"], - returncode=0, - stdout=( - "//user@nrs.example.org/foxtrot on /Volumes/foxtrot (smbfs, nodev)\n" - "//user@nearline.example.org/foxtrot on /Volumes/foxtrot-1 (smbfs, nodev)" - ), - ) - assert from_janelia_uri("janelia:/nrs/foxtrot/data") == "/Volumes/foxtrot/data" - assert from_janelia_uri("janelia:/nearline/foxtrot/data") == "/Volumes/foxtrot-1/data" - - -@windows_only -def test_from_janelia_uri_on_windows(): - """Windows resolves to the UNC path -- no mounting step needed.""" - assert ( - from_janelia_uri("janelia:/labs/alpha/alphalab/data") - == "\\\\fileserver.example.org\\alphalab\\data" - ) - - -@windows_only -def test_from_janelia_uri_windows_share_root_only(): - assert ( - from_janelia_uri("janelia:/labs/alpha/alphalab") == "\\\\fileserver.example.org\\alphalab" - ) - - -@windows_only -def test_from_janelia_uri_windows_nested_subpath_separators(): - """The subpath arrives forward-slashed from the matchers; every segment - must come back backslash-separated, not just the first join.""" - assert ( - from_janelia_uri("janelia:/labs/alpha/alphalab/sub/deeper/data") - == "\\\\fileserver.example.org\\alphalab\\sub\\deeper\\data" - ) - - -# --- Windows drive-letter mapping --- -# -# These patch PowerShell's Get-SmbMapping output rather than requiring a real -# mapped drive, so they run on any platform -- same approach as the macOS -# `mount` tests above. - - -def _smb_mapping(stdout): - return subprocess.CompletedProcess(args=["powershell"], returncode=0, stdout=stdout) - - -def test_find_share_for_windows_drive_matches_mapping(): - mapping = json.dumps( - [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] - ) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - result = _windows.find_share_for_windows_drive("Z:\\data", _share.load_shares()) - assert result is not None - share, subpath = result - assert share.linux_path == "/labs/alpha/alphalab" - assert subpath == "data" - - -def test_find_share_for_windows_drive_is_case_insensitive(): - """Drive letters are case-insensitive, and Get-SmbMapping may report a - trailing backslash that the queried drive string lacks.""" - mapping = json.dumps( - [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:\\"}] - ) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - result = _windows.find_share_for_windows_drive("z:", _share.load_shares()) - assert result is not None - assert result[0].linux_path == "/labs/alpha/alphalab" - - -def test_find_share_for_windows_drive_resolves_colliding_share_names(): - """Two shares named `foxtrot` on different hosts are mapped to different - letters; only the host in RemotePath tells them apart.""" - mapping = json.dumps( - [ - {"RemotePath": "\\\\nrs.example.org\\foxtrot", "LocalPath": "X:"}, - {"RemotePath": "\\\\nearline.example.org\\foxtrot", "LocalPath": "Y:"}, - ] - ) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - shares = _share.load_shares() - assert _windows.find_share_for_windows_drive("X:", shares)[0].linux_path == "/nrs/foxtrot" - assert ( - _windows.find_share_for_windows_drive("Y:", shares)[0].linux_path == "/nearline/foxtrot" - ) - - -def test_find_share_for_windows_drive_single_mapping_is_json_object(): - """ConvertTo-Json emits a bare object, not a list, for a single mapping.""" - mapping = json.dumps({"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - result = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) - assert result is not None - assert result[0].linux_path == "/labs/alpha/alphalab" - - -def test_find_share_for_windows_drive_unmapped_returns_none(): - mapping = json.dumps( - [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] - ) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - with pytest.warns(UserWarning, match="not a mapped network drive"): - assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None - - -def test_find_share_for_windows_drive_ignores_non_drive_paths(): - """A UNC or linux path isn't a drive mapping -- that's the static - matchers' job, so this shouldn't claim it.""" - mapping = json.dumps( - [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] - ) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - shares = _share.load_shares() - assert _windows.find_share_for_windows_drive("/labs/alpha/alphalab", shares) is None - assert ( - _windows.find_share_for_windows_drive("\\\\fileserver.example.org\\alphalab", shares) - is None - ) - - -def test_find_share_for_windows_drive_unknown_share_returns_none(): - """A drive mapped to a share we don't know about isn't ours to translate -- - and unlike an unmapped letter, nothing is wrong, so it shouldn't warn.""" - mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): - with warnings.catch_warnings(): - warnings.simplefilter("error") - assert _windows.find_share_for_windows_drive("Z:", _share.load_shares()) is None - - -@pytest.mark.parametrize("stdout", ["", " ", "not json at all"]) -def test_live_windows_drives_degrades_on_bad_output(stdout): - """Empty or unparsable PowerShell output yields no mappings, not a crash.""" - with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(stdout)): - assert _windows._live_windows_drives() == [] - - -def test_live_windows_drives_degrades_when_powershell_missing(): - """On a non-Windows machine there's no powershell at all -- OSError, not a crash.""" - with patch("janelia_pathlib._windows.subprocess.run", side_effect=OSError): - assert _windows._live_windows_drives() == [] - - -# --- round trip --- - - -def test_round_trip_through_janelia_uri(): - uri = to_janelia_uri("/labs/alpha/alphalab/data") - # On Linux, from_janelia_uri needs no mounting and resolves purely from - # static registry data, so the round trip works without any live state. - if sys.platform == "linux": - assert from_janelia_uri(uri) == "/labs/alpha/alphalab/data" - - -def test_round_trip_non_janelia_path_is_untouched(): - p = "/tmp/local/scratch/file.txt" - assert from_janelia_uri(to_janelia_uri(p)) == p - - -# --- Offline / fetch robustness --- - - -def test_load_shares_warns_on_nested_shares(monkeypatch, tmp_path): - """First-match-wins is only unambiguous while no share sits inside - another, so a nested pair in the API data should say so loudly rather - than silently resolving to whichever came first.""" - cache_file = tmp_path / "shares.json" - cache_file.write_text( - json.dumps( - [ - { - "zone": "Nest", - "linux_path": "/labs/outer", - "windows_path": "\\\\fileserver.example.org\\outer", - "mac_path": "smb://fileserver.example.org/outer", - }, - { - "zone": "Nest", - "linux_path": "/labs/outer/inner", - "windows_path": "\\\\fileserver.example.org\\inner", - "mac_path": "smb://fileserver.example.org/inner", - }, - ] - ) - ) - monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) - _share.load_shares.cache_clear() - try: - with pytest.warns(UserWarning, match="nested inside"): - _share.load_shares() - finally: - _share.load_shares.cache_clear() - - -def test_load_shares_quiet_for_sibling_shares(tmp_path): - """Sibling shares under one group share a prefix but don't nest.""" - _share.load_shares.cache_clear() - try: - with warnings.catch_warnings(): - warnings.simplefilter("error") - _share.load_shares() - finally: - _share.load_shares.cache_clear() - - -def test_default_mount_timeout_falls_back_without_env(monkeypatch): - monkeypatch.delenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", raising=False) - assert _macos.default_mount_timeout() == 30.0 - - -def test_default_mount_timeout_reads_env(monkeypatch): - monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "90") - assert _macos.default_mount_timeout() == 90.0 - - -def test_default_mount_timeout_ignores_garbage(monkeypatch): - """A malformed value shouldn't crash a path translation.""" - monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "soon please") - assert _macos.default_mount_timeout() == 30.0 - - -@mac_only -def test_ensure_mounted_honors_env_timeout(monkeypatch): - """With a zero timeout, a share that never appears gives up immediately - rather than waiting out the 30s default.""" - monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "0") - share = next(s for s in _share.load_shares() if s.share_name == "alphalab") - with patch("janelia_pathlib._macos.subprocess.run") as mock_run: - mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") - with patch("janelia_pathlib._macos.time.sleep") as mock_sleep: - assert _macos.ensure_mounted(share) is None - mock_sleep.assert_not_called() - - -def test_fetch_shares_uses_timeout(monkeypatch, tmp_path): - """fetch_shares passes a timeout to urlopen so it can't hang indefinitely.""" - - captured = {} - - class _Resp: - def __enter__(self): - return self - - def __exit__(self, *exc): - return False - - def read(self): - return b'{"paths": []}' - - def fake_urlopen(url, *args, **kwargs): - captured["timeout"] = kwargs.get("timeout") - return _Resp() - - monkeypatch.setattr("urllib.request.urlopen", fake_urlopen) - _fetch.fetch_shares(tmp_path / "shares.json") - assert captured["timeout"] is not None - assert captured["timeout"] > 0 - - -def test_no_fetch_degrades_when_cache_missing(monkeypatch, tmp_path): - """With JANELIA_PATHLIB_NO_FETCH set and no cache, load returns no shares and - paths pass through untranslated instead of raising.""" - - monkeypatch.setenv("JANELIA_PATHLIB_NO_FETCH", "1") - monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(tmp_path / "missing.json")) - _share.load_shares.cache_clear() - try: - with pytest.warns(UserWarning, match="No share data available"): - assert _share.load_shares() == () - # No known shares -> a real share path is left untranslated - assert to_janelia_uri("/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" - finally: - _share.load_shares.cache_clear() - - -def test_missing_cache_degrades_without_no_fetch(monkeypatch, tmp_path): - """Without the opt-out, a missing cache + failed fetch still degrades to no - known shares (with a warning) instead of raising -- same end state as the - no_fetch case, just reached via a failed fetch attempt instead of an - explicit opt-out.""" - monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) - monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(tmp_path / "missing.json")) - - def boom(*args, **kwargs): - raise URLError("offline") - - monkeypatch.setattr("urllib.request.urlopen", boom) - _share.load_shares.cache_clear() - try: - with pytest.warns(UserWarning, match="No share data available"): - assert _share.load_shares() == () - finally: - _share.load_shares.cache_clear() - - -def test_load_shares_refetches_every_call(monkeypatch, tmp_path): - """Without JANELIA_PATHLIB_NO_FETCH, _load_shares() always attempts a fresh - fetch -- not just when the cache file is missing.""" - monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) - cache_file = tmp_path / "shares.json" - monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) - - call_count = {"n": 0} - - def fake_fetch_shares(path): - call_count["n"] += 1 - path.write_text("[]") - - monkeypatch.setattr(_share, "fetch_shares", fake_fetch_shares) - _share.load_shares.cache_clear() - try: - _share.load_shares.cache_clear() - _share.load_shares() - assert call_count["n"] == 1 - _share.load_shares.cache_clear() - _share.load_shares() - assert call_count["n"] == 2 - finally: - _share.load_shares.cache_clear() - - -def test_fetch_failure_falls_back_to_stale_cache(monkeypatch, tmp_path): - """If the network fetch fails but a cache file already exists, use the - stale cache (with a warning) instead of raising.""" - monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) - cache_file = tmp_path / "shares.json" - cache_file.write_text( - json.dumps( - [ - { - "zone": "Alpha", - "linux_path": "/labs/alpha/alphalab", - "windows_path": "\\\\fileserver.example.org\\alphalab", - "mac_path": "smb://fileserver.example.org/alphalab", - } - ] - ) - ) - monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) - - def boom(*args, **kwargs): - raise URLError("offline") - - monkeypatch.setattr("urllib.request.urlopen", boom) - _share.load_shares.cache_clear() - try: - with pytest.warns(UserWarning, match="stale"): - shares = _share.load_shares() - assert len(shares) == 1 - assert shares[0].zone == "Alpha" - finally: - _share.load_shares.cache_clear() diff --git a/tests/test_share.py b/tests/test_share.py new file mode 100644 index 0000000..3e7ae56 --- /dev/null +++ b/tests/test_share.py @@ -0,0 +1,200 @@ +"""Tests for loading share data, and for the knobs that control it.""" + +import json +import subprocess +import warnings +from unittest.mock import patch +from urllib.error import URLError + +import pytest +from conftest import mac_only + +from janelia_pathlib import _fetch, _macos, _share, to_janelia_uri + + +def _share_entry(zone, linux_path, share_name): + return { + "zone": zone, + "linux_path": linux_path, + "windows_path": f"\\\\fileserver.example.org\\{share_name}", + "mac_path": f"smb://fileserver.example.org/{share_name}", + } + + +# --- nesting check --- + + +def test_load_shares_warns_on_nested_shares(monkeypatch, tmp_path): + """First-match-wins is only unambiguous while no share sits inside + another, so a nested pair in the API data should say so loudly rather + than silently resolving to whichever came first.""" + cache_file = tmp_path / "shares.json" + cache_file.write_text( + json.dumps( + [ + _share_entry("Nest", "/labs/outer", "outer"), + _share_entry("Nest", "/labs/outer/inner", "inner"), + ] + ) + ) + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) + _share.load_shares.cache_clear() + try: + with pytest.warns(UserWarning, match="nested inside"): + _share.load_shares() + finally: + _share.load_shares.cache_clear() + + +def test_load_shares_quiet_for_sibling_shares(): + """Sibling shares under one group share a prefix but don't nest.""" + _share.load_shares.cache_clear() + try: + with warnings.catch_warnings(): + warnings.simplefilter("error") + _share.load_shares() + finally: + _share.load_shares.cache_clear() + + +# --- mount timeout --- + + +def test_default_mount_timeout_falls_back_without_env(monkeypatch): + monkeypatch.delenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", raising=False) + assert _macos.default_mount_timeout() == 30.0 + + +def test_default_mount_timeout_reads_env(monkeypatch): + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "90") + assert _macos.default_mount_timeout() == 90.0 + + +def test_default_mount_timeout_ignores_garbage(monkeypatch): + """A malformed value shouldn't crash a path translation.""" + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "soon please") + assert _macos.default_mount_timeout() == 30.0 + + +@mac_only +def test_ensure_mounted_honors_env_timeout(monkeypatch): + """With a zero timeout, a share that never appears gives up immediately + rather than waiting out the 30s default.""" + monkeypatch.setenv("JANELIA_PATHLIB_MOUNT_TIMEOUT", "0") + share = next(s for s in _share.load_shares() if s.share_name == "alphalab") + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with patch("janelia_pathlib._macos.time.sleep") as mock_sleep: + assert _macos.ensure_mounted(share) is None + mock_sleep.assert_not_called() + + +# --- offline / fetch robustness --- + + +def test_fetch_shares_uses_timeout(monkeypatch, tmp_path): + """fetch_shares passes a timeout to urlopen so it can't hang indefinitely.""" + + captured = {} + + class _Resp: + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def read(self): + return b'{"paths": []}' + + def fake_urlopen(url, *args, **kwargs): + captured["timeout"] = kwargs.get("timeout") + return _Resp() + + monkeypatch.setattr("urllib.request.urlopen", fake_urlopen) + _fetch.fetch_shares(tmp_path / "shares.json") + assert captured["timeout"] is not None + assert captured["timeout"] > 0 + + +def test_no_fetch_degrades_when_cache_missing(monkeypatch, tmp_path): + """With JANELIA_PATHLIB_NO_FETCH set and no cache, load returns no shares and + paths pass through untranslated instead of raising.""" + + monkeypatch.setenv("JANELIA_PATHLIB_NO_FETCH", "1") + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(tmp_path / "missing.json")) + _share.load_shares.cache_clear() + try: + with pytest.warns(UserWarning, match="No share data available"): + assert _share.load_shares() == () + # No known shares -> a real share path is left untranslated + assert to_janelia_uri("/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" + finally: + _share.load_shares.cache_clear() + + +def test_missing_cache_degrades_without_no_fetch(monkeypatch, tmp_path): + """Without the opt-out, a missing cache + failed fetch still degrades to no + known shares (with a warning) instead of raising -- same end state as the + no_fetch case, just reached via a failed fetch attempt instead of an + explicit opt-out.""" + monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(tmp_path / "missing.json")) + + def boom(*args, **kwargs): + raise URLError("offline") + + monkeypatch.setattr("urllib.request.urlopen", boom) + _share.load_shares.cache_clear() + try: + with pytest.warns(UserWarning, match="No share data available"): + assert _share.load_shares() == () + finally: + _share.load_shares.cache_clear() + + +def test_load_shares_refetches_every_call(monkeypatch, tmp_path): + """Without JANELIA_PATHLIB_NO_FETCH, load_shares() always attempts a fresh + fetch -- not just when the cache file is missing.""" + monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) + cache_file = tmp_path / "shares.json" + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) + + call_count = {"n": 0} + + def fake_fetch_shares(path): + call_count["n"] += 1 + path.write_text("[]") + + monkeypatch.setattr(_share, "fetch_shares", fake_fetch_shares) + _share.load_shares.cache_clear() + try: + _share.load_shares() + assert call_count["n"] == 1 + _share.load_shares.cache_clear() + _share.load_shares() + assert call_count["n"] == 2 + finally: + _share.load_shares.cache_clear() + + +def test_fetch_failure_falls_back_to_stale_cache(monkeypatch, tmp_path): + """If the network fetch fails but a cache file already exists, use the + stale cache (with a warning) instead of raising.""" + monkeypatch.delenv("JANELIA_PATHLIB_NO_FETCH", raising=False) + cache_file = tmp_path / "shares.json" + cache_file.write_text(json.dumps([_share_entry("Alpha", "/labs/alpha/alphalab", "alphalab")])) + monkeypatch.setenv("JANELIA_PATHLIB_SHARES_PATH", str(cache_file)) + + def boom(*args, **kwargs): + raise URLError("offline") + + monkeypatch.setattr("urllib.request.urlopen", boom) + _share.load_shares.cache_clear() + try: + with pytest.warns(UserWarning, match="stale"): + shares = _share.load_shares() + assert len(shares) == 1 + assert shares[0].zone == "Alpha" + finally: + _share.load_shares.cache_clear() diff --git a/tests/test_translate.py b/tests/test_translate.py new file mode 100644 index 0000000..cdedbcb --- /dev/null +++ b/tests/test_translate.py @@ -0,0 +1,305 @@ +"""Tests for the public to_janelia_uri / from_janelia_uri conversions. + +Tests run against a fictional share fixture (see tests/fixtures/shares.json +and tests/conftest.py). No real Janelia share data is referenced here. +""" + +import subprocess +import sys +import warnings +from unittest.mock import patch + +import pytest +from conftest import linux_only, mac_only, windows_only + +from janelia_pathlib import from_janelia_uri, to_janelia_uri + +# --- to_janelia_uri(): formats that resolve on any OS --- + + +def test_to_janelia_uri_from_linux_path(): + assert to_janelia_uri("/labs/alpha/alphalab/data") == "janelia:/labs/alpha/alphalab/data" + + +def test_to_janelia_uri_from_windows_path(): + """A UNC path carries its own host, so it resolves without live state -- + including when running on a non-Windows machine.""" + assert ( + to_janelia_uri("\\\\fileserver.example.org\\alphalab\\data") + == "janelia:/labs/alpha/alphalab/data" + ) + + +def test_to_janelia_uri_from_smb_url_on_any_os(): + """Same for an smb:// URL -- host and share name are both in the string.""" + assert ( + to_janelia_uri("smb://fileserver.example.org/alphalab/data") + == "janelia:/labs/alpha/alphalab/data" + ) + + +@pytest.mark.parametrize("os_name", ["linux", "mac", "windows"]) +@pytest.mark.parametrize( + "path", + [ + "/labs/alpha/alphalab/data", + "\\\\fileserver.example.org\\alphalab\\data", + "smb://fileserver.example.org/alphalab/data", + ], +) +def test_self_describing_formats_resolve_on_every_os(path, os_name): + """A linux path, a UNC path and an smb:// URL each name a share on + their own, so none of them should depend on which OS is running.""" + with patch("janelia_pathlib._match.current_os", return_value=os_name): + assert to_janelia_uri(path) == "janelia:/labs/alpha/alphalab/data" + + +def test_to_janelia_uri_share_root_only(): + assert to_janelia_uri("/labs/alpha/alphalab") == "janelia:/labs/alpha/alphalab" + + +def test_to_janelia_uri_smb_url_share_root_only(): + assert to_janelia_uri("smb://fileserver.example.org/alphalab") == "janelia:/labs/alpha/alphalab" + + +def test_to_janelia_uri_smb_url_with_dollar_share(): + """Hidden shares (trailing $) are ordinary share names, not special syntax.""" + assert ( + to_janelia_uri("smb://fileserver.example.org/alpha$/data") + == "janelia:/labs/alpha/home/data" + ) + + +def test_to_janelia_uri_trailing_slash(): + assert to_janelia_uri("/labs/alpha/alphalab/") == "janelia:/labs/alpha/alphalab" + + +def test_to_janelia_uri_windows_forward_slash(): + """Windows paths with forward slashes should still match.""" + assert ( + to_janelia_uri("//fileserver.example.org/alphalab/data") + == "janelia:/labs/alpha/alphalab/data" + ) + + +def test_to_janelia_uri_no_match_passthrough(): + assert to_janelia_uri("/tmp/local/scratch/file.txt") == "/tmp/local/scratch/file.txt" + + +# --- to_janelia_uri(): prefix matching is per path segment --- + + +def test_to_janelia_uri_sibling_shares_under_one_group(): + """A group's shares sit side by side (/labs/charlie/home, .../charlie), + never nested, so each resolves to itself.""" + assert to_janelia_uri("/labs/charlie/charlie/data") == "janelia:/labs/charlie/charlie/data" + assert to_janelia_uri("/labs/charlie/home/data") == "janelia:/labs/charlie/home/data" + + +def test_to_janelia_uri_substring_share_name_is_not_a_prefix(): + """/labs/golf is a string prefix of /labs/golflab but not a path prefix: + matching must respect segment boundaries, or every golflab path would + resolve to the golf share.""" + assert to_janelia_uri("/labs/golflab/data") == "janelia:/labs/golflab/data" + assert to_janelia_uri("/labs/golf/data") == "janelia:/labs/golf/data" + assert to_janelia_uri("/labs/golflab") == "janelia:/labs/golflab" + assert to_janelia_uri("/labs/golf") == "janelia:/labs/golf" + + +def test_to_janelia_uri_substring_share_name_unc(): + """Same boundary rule on the windows_path field.""" + assert ( + to_janelia_uri("\\\\fileserver.example.org\\golflab\\data") == "janelia:/labs/golflab/data" + ) + + +def test_to_janelia_uri_partial_segment_matches_nothing(): + """A path that shares a prefix with a share but diverges mid-segment + belongs to no share at all -- it must not fall back to the shorter one.""" + assert to_janelia_uri("/labs/golfcourse/data") == "/labs/golfcourse/data" + + +# --- to_janelia_uri(): macOS mountpoints need live state --- + + +@mac_only +def test_to_janelia_uri_from_mac_path(): + """Resolving a Mac path requires live mount state (mac_path alone could + be ambiguous), so this must be live-checked, not a static guess.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=["mount"], + returncode=0, + stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", + ) + assert to_janelia_uri("/Volumes/alphalab/data") == "janelia:/labs/alpha/alphalab/data" + + +@mac_only +def test_to_janelia_uri_resolves_colliding_mac_mount_via_live_state(): + """Two shares (nrs/nearline) both nominally want /Volumes/foxtrot on macOS. + Static string matching alone can't tell them apart -- live `mount` output, + which records the actual SMB host for each mountpoint, must be checked.""" + mount_output = ( + "//user@nrs.example.org/foxtrot on /Volumes/foxtrot (smbfs, nodev)\n" + "//user@nearline.example.org/foxtrot on /Volumes/foxtrot-1 (smbfs, nodev)" + ) + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=["mount"], returncode=0, stdout=mount_output + ) + assert to_janelia_uri("/Volumes/foxtrot/data") == "janelia:/nrs/foxtrot/data" + assert to_janelia_uri("/Volumes/foxtrot-1/data") == "janelia:/nearline/foxtrot/data" + + +@mac_only +def test_to_janelia_uri_warns_when_known_share_is_unmounted(): + """An unmounted share is left unresolved rather than guessed from the + static mac_path field -- a guess could be wrong in exactly the way the + mount-name collision causes. But passing through silently looks identical + to a path that was never a share, so say which it was.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with pytest.warns(UserWarning, match="isn't mounted"): + assert to_janelia_uri("/Volumes/alphalab/data") == "/Volumes/alphalab/data" + + +@mac_only +def test_to_janelia_uri_warns_for_suffixed_collision_mountpoint(): + """macOS suffixes a colliding mountpoint, so /Volumes/foxtrot-1 should + still be recognized as share `foxtrot`.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with pytest.warns(UserWarning, match="foxtrot"): + to_janelia_uri("/Volumes/foxtrot-1/data") + + +@mac_only +def test_to_janelia_uri_quiet_for_non_janelia_volume(): + """A USB stick or disk image isn't ours -- no warning for /Volumes names + that match no known share.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert to_janelia_uri("/Volumes/MyBackupDrive/x") == "/Volumes/MyBackupDrive/x" + + +# --- from_janelia_uri() --- + + +def test_from_janelia_uri_no_match_passthrough(): + assert from_janelia_uri("/tmp/local/scratch/file.txt") == "/tmp/local/scratch/file.txt" + + +def test_from_janelia_uri_unknown_share_raises(): + with pytest.raises(RuntimeError, match="No known share"): + from_janelia_uri("janelia:/nope/not-a-share/data") + + +def test_from_janelia_uri_rejects_double_slash_form(): + """``janelia://host/share/...`` is the old form. ``//`` opens a URI + authority component, which a Linux path has no value for, so it's + rejected rather than silently parsing ``labs`` as an authority.""" + with pytest.raises(RuntimeError, match="No known share"): + from_janelia_uri("janelia://labs/alpha/alphalab/data") + + +@linux_only +def test_from_janelia_uri_on_linux(): + assert from_janelia_uri("janelia:/labs/alpha/alphalab/data") == "/labs/alpha/alphalab/data" + + +@mac_only +def test_from_janelia_uri_mounts_on_demand(): + """from_janelia_uri should trigger `open smb://...` when the share isn't + mounted yet, then resolve the real mountpoint from live `mount` output.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mount_output_empty = subprocess.CompletedProcess(args=["mount"], returncode=0, stdout="") + mount_output_found = subprocess.CompletedProcess( + args=["mount"], + returncode=0, + stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", + ) + mock_run.side_effect = [mount_output_empty, None, mount_output_found] + result = from_janelia_uri("janelia:/labs/alpha/alphalab/data", timeout=2) + assert result == "/Volumes/alphalab/data" + open_call = mock_run.call_args_list[1] + args = open_call[0][0] + assert args[0] == "open" + assert args[1] == "smb://fileserver.example.org/alphalab" + + +@mac_only +def test_from_janelia_uri_already_mounted(): + """from_janelia_uri should return the existing mountpoint without calling open.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=["mount"], + returncode=0, + stdout="//user@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)", + ) + result = from_janelia_uri("janelia:/labs/alpha/alphalab/data") + assert result == "/Volumes/alphalab/data" + for call in mock_run.call_args_list: + assert call[0][0][0] != "open" + + +@mac_only +def test_from_janelia_uri_resolves_colliding_share_name(): + """When two shares share a mount-name (nrs/nearline foxtrot collision), + from_janelia_uri must match on (host, share_name), not on the guessed + /Volumes/ path.""" + with patch("janelia_pathlib._macos.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=["mount"], + returncode=0, + stdout=( + "//user@nrs.example.org/foxtrot on /Volumes/foxtrot (smbfs, nodev)\n" + "//user@nearline.example.org/foxtrot on /Volumes/foxtrot-1 (smbfs, nodev)" + ), + ) + assert from_janelia_uri("janelia:/nrs/foxtrot/data") == "/Volumes/foxtrot/data" + assert from_janelia_uri("janelia:/nearline/foxtrot/data") == "/Volumes/foxtrot-1/data" + + +@windows_only +def test_from_janelia_uri_on_windows(): + """Windows resolves to the UNC path -- no mounting step needed.""" + assert ( + from_janelia_uri("janelia:/labs/alpha/alphalab/data") + == "\\\\fileserver.example.org\\alphalab\\data" + ) + + +@windows_only +def test_from_janelia_uri_windows_share_root_only(): + assert ( + from_janelia_uri("janelia:/labs/alpha/alphalab") == "\\\\fileserver.example.org\\alphalab" + ) + + +@windows_only +def test_from_janelia_uri_windows_nested_subpath_separators(): + """The subpath arrives forward-slashed from the matchers; every segment + must come back backslash-separated, not just the first join.""" + assert ( + from_janelia_uri("janelia:/labs/alpha/alphalab/sub/deeper/data") + == "\\\\fileserver.example.org\\alphalab\\sub\\deeper\\data" + ) + + +# --- round trip --- + + +def test_round_trip_through_janelia_uri(): + uri = to_janelia_uri("/labs/alpha/alphalab/data") + # On Linux, from_janelia_uri needs no mounting and resolves purely from + # static share data, so the round trip works without any live state. + if sys.platform == "linux": + assert from_janelia_uri(uri) == "/labs/alpha/alphalab/data" + + +def test_round_trip_non_janelia_path_is_untouched(): + p = "/tmp/local/scratch/file.txt" + assert from_janelia_uri(to_janelia_uri(p)) == p diff --git a/tests/test_windows.py b/tests/test_windows.py new file mode 100644 index 0000000..4c69f11 --- /dev/null +++ b/tests/test_windows.py @@ -0,0 +1,119 @@ +"""Tests for Windows drive-letter mapping. + +These patch PowerShell's Get-SmbMapping output rather than requiring a real +mapped drive, so they run on any platform -- the same approach the macOS +`mount` tests in test_translate.py use. +""" + +import json +import subprocess +import warnings +from unittest.mock import patch + +import pytest + +from janelia_pathlib import _share, _windows + + +def _smb_mapping(stdout): + return subprocess.CompletedProcess(args=["powershell"], returncode=0, stdout=stdout) + + +def test_find_share_for_windows_drive_matches_mapping(): + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + result = _windows.find_share_for_windows_drive("Z:\\data", _share.load_shares()) + assert result is not None + share, subpath = result + assert share.linux_path == "/labs/alpha/alphalab" + assert subpath == "data" + + +def test_find_share_for_windows_drive_is_case_insensitive(): + """Drive letters are case-insensitive, and Get-SmbMapping may report a + trailing backslash that the queried drive string lacks.""" + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:\\"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + result = _windows.find_share_for_windows_drive("z:", _share.load_shares()) + assert result is not None + assert result[0].linux_path == "/labs/alpha/alphalab" + + +def test_find_share_for_windows_drive_resolves_colliding_share_names(): + """Two shares named `foxtrot` on different hosts are mapped to different + letters; only the host in RemotePath tells them apart.""" + mapping = json.dumps( + [ + {"RemotePath": "\\\\nrs.example.org\\foxtrot", "LocalPath": "X:"}, + {"RemotePath": "\\\\nearline.example.org\\foxtrot", "LocalPath": "Y:"}, + ] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + shares = _share.load_shares() + assert _windows.find_share_for_windows_drive("X:", shares)[0].linux_path == "/nrs/foxtrot" + assert ( + _windows.find_share_for_windows_drive("Y:", shares)[0].linux_path == "/nearline/foxtrot" + ) + + +def test_find_share_for_windows_drive_single_mapping_is_json_object(): + """ConvertTo-Json emits a bare object, not a list, for a single mapping.""" + mapping = json.dumps({"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + result = _windows.find_share_for_windows_drive("Z:", _share.load_shares()) + assert result is not None + assert result[0].linux_path == "/labs/alpha/alphalab" + + +def test_find_share_for_windows_drive_unmapped_warns(): + """A drive letter carries no share name, so there's nothing to match on -- + but a caller passing one here wants it translated, and silent passthrough + hides why it wasn't.""" + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + with pytest.warns(UserWarning, match="not a mapped network drive"): + assert _windows.find_share_for_windows_drive("Q:", _share.load_shares()) is None + + +def test_find_share_for_windows_drive_unknown_share_returns_none(): + """A drive mapped to a share we don't know about isn't ours to translate -- + and unlike an unmapped letter, nothing is wrong, so it shouldn't warn.""" + mapping = json.dumps([{"RemotePath": "\\\\other.example.org\\nope", "LocalPath": "Z:"}]) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert _windows.find_share_for_windows_drive("Z:", _share.load_shares()) is None + + +def test_find_share_for_windows_drive_ignores_non_drive_paths(): + """A UNC or linux path isn't a drive mapping -- that's the static + matchers' job, so this shouldn't claim it.""" + mapping = json.dumps( + [{"RemotePath": "\\\\fileserver.example.org\\alphalab", "LocalPath": "Z:"}] + ) + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(mapping)): + shares = _share.load_shares() + assert _windows.find_share_for_windows_drive("/labs/alpha/alphalab", shares) is None + assert ( + _windows.find_share_for_windows_drive("\\\\fileserver.example.org\\alphalab", shares) + is None + ) + + +@pytest.mark.parametrize("stdout", ["", " ", "not json at all"]) +def test_live_windows_drives_degrades_on_bad_output(stdout): + """Empty or unparsable PowerShell output yields no mappings, not a crash.""" + with patch("janelia_pathlib._windows.subprocess.run", return_value=_smb_mapping(stdout)): + assert _windows._live_windows_drives() == [] + + +def test_live_windows_drives_degrades_when_powershell_missing(): + """On a non-Windows machine there's no powershell at all -- OSError, not a crash.""" + with patch("janelia_pathlib._windows.subprocess.run", side_effect=OSError): + assert _windows._live_windows_drives() == [] From 3e16b25b2efd195cb5812327f72c6d5ffd23c065 Mon Sep 17 00:00:00 2001 From: Caroline Malin-Mayor Date: Mon, 26 Oct 2026 17:41:06 -0400 Subject: [PATCH 7/7] Add argparse type helper :grin: and NO_MOUNT env var --- README.md | 55 ++++++++++++++++-- src/janelia_pathlib/__init__.py | 2 + src/janelia_pathlib/_argparse.py | 41 +++++++++++++ src/janelia_pathlib/_macos.py | 19 ++++++- src/janelia_pathlib/_translate.py | 15 ++++- tests/test_argparse.py | 95 +++++++++++++++++++++++++++++++ tests/test_share.py | 18 ++++++ 7 files changed, 235 insertions(+), 10 deletions(-) create mode 100644 src/janelia_pathlib/_argparse.py create mode 100644 tests/test_argparse.py diff --git a/README.md b/README.md index 59a857d..cb34875 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ to_janelia_uri("/tmp/local/scratch/file.txt") # "/tmp/local/scratch/file.txt" ``` -### `from_janelia_uri(path_str, *, timeout=30)` +### `from_janelia_uri(path_str, *, timeout=None)` Translate a `janelia:/` string to a real, usable local path. @@ -97,7 +97,7 @@ from_janelia_uri("janelia:/labs/alpha/alphalab/data") # "/labs/alpha/alphalab/data" (on Linux) ``` -On **macOS**, if the share isn't already mounted, this mounts it on demand (triggering the native SMB mount, which may prompt for credentials) and waits up to `timeout` seconds for the mount to appear, returning its *real* local mountpoint — not a guessed `/Volumes/` path, which may not exist yet or may belong to a different, colliding share. Raises `RuntimeError` if the URI doesn't match a known share, or if mounting fails or times out. +On **macOS**, if the share isn't already mounted, this mounts it on demand (triggering the native SMB mount, which may prompt for credentials) and waits up to `timeout` seconds for the mount to appear, returning its *real* local mountpoint — not a guessed `/Volumes/` path, which may not exist yet or may belong to a different, colliding share. `timeout` defaults to `JANELIA_PATHLIB_MOUNT_TIMEOUT` (30s). Raises `RuntimeError` if the URI doesn't match a known share, or if mounting fails or times out. On **Linux** and **Windows**, no mounting step is needed — shares are already available at a fixed path (Linux) or accessible directly via UNC path (Windows). @@ -110,6 +110,31 @@ from_janelia_uri("/tmp/local/scratch/file.txt") Both functions round-trip: `from_janelia_uri(to_janelia_uri(p))` resolves back to a valid local path for `p`'s share, and non-Janelia paths pass through both functions untouched. +### `janelia_path(path_str)` + +An [`argparse`](https://docs.python.org/3/library/argparse.html) `type=` converter, for scripts that take a path on the command line: + +```python +from janelia_pathlib import janelia_path + +parser.add_argument("--data", type=janelia_path) +``` + +```console +$ my-script --data janelia:/labs/alpha/alphalab/data +``` + +The script receives `args.data` already converted to a local path, so it never has to care which OS it's on. Ordinary paths still work — anything that isn't a `janelia:` URI is passed through — so this is safe to add to an existing argument. + +Failures become normal argparse errors rather than tracebacks: + +```console +$ my-script --data janelia:/labs/nope/data +demo: error: argument --data: No known share for URI: janelia:/labs/nope/data +``` + +On macOS this mounts the share during `parse_args()`, which may block on a credential prompt. Set [`JANELIA_PATHLIB_NO_MOUNT`](#never-mounting) when nobody is there to answer. + ## Share data Share data is **not** bundled with the package — it's fetched from the [Fileglancer API](https://fileglancer.int.janelia.org/api/file-share-paths) and cached in your user cache directory: @@ -152,10 +177,28 @@ for long; override it with: export JANELIA_PATHLIB_FETCH_TIMEOUT=5 ``` -On macOS, `from_janelia_uri` waits up to 30s for an on-demand mount to appear. -That wait covers a native credential prompt, so it budgets for someone typing a -password — raise it for a slow authenticator, or set it near zero in headless -contexts where nobody is there to answer: +### Never mounting + +On macOS, `from_janelia_uri` mounts a share that isn't mounted yet, which can +block on a credential prompt. To forbid that — so an unmounted share raises +instead — set: + +```bash +export JANELIA_PATHLIB_NO_MOUNT=1 +``` + +Already-mounted shares still resolve; this only stops new mounts. Use it in +cron jobs, CI, containers, or anywhere nobody is there to answer a prompt. + +Whether a prompt can be answered is a property of the machine, not of the code +calling it, which is why this is an environment setting rather than a function +argument — the person running a script decides, not whoever wrote it. + +### Mount timeout + +When mounting *is* allowed, `from_janelia_uri` waits up to 30s for the mount to +appear. That wait covers a credential prompt, so it budgets for someone typing +a password — raise it for a slow authenticator: ```bash export JANELIA_PATHLIB_MOUNT_TIMEOUT=90 diff --git a/src/janelia_pathlib/__init__.py b/src/janelia_pathlib/__init__.py index 524cc66..aa8960a 100644 --- a/src/janelia_pathlib/__init__.py +++ b/src/janelia_pathlib/__init__.py @@ -1,5 +1,6 @@ from importlib.metadata import PackageNotFoundError, version +from janelia_pathlib._argparse import janelia_path from janelia_pathlib._translate import from_janelia_uri, to_janelia_uri try: @@ -12,5 +13,6 @@ __all__ = [ "__version__", "from_janelia_uri", + "janelia_path", "to_janelia_uri", ] diff --git a/src/janelia_pathlib/_argparse.py b/src/janelia_pathlib/_argparse.py new file mode 100644 index 0000000..c34df05 --- /dev/null +++ b/src/janelia_pathlib/_argparse.py @@ -0,0 +1,41 @@ +from __future__ import annotations + +from argparse import ArgumentTypeError + +from ._translate import from_janelia_uri + + +def janelia_path(path_str: str) -> str: + """An ``argparse`` ``type=`` converter that turns a ``janelia:`` URI into a local path. + + :: + + parser.add_argument("--data", type=janelia_path) + + Anything that isn't a ``janelia:`` URI is passed through unchanged, so + an argument declared this way still accepts ordinary local paths. + + On macOS this mounts the share if it isn't already, which may block on + a credential prompt during ``parse_args()``. Set + ``JANELIA_PATHLIB_NO_MOUNT`` to turn that into an argument error + instead -- useful in headless or scripted contexts where nobody is + there to answer. Whether that applies is a property of the machine, not + of the script, so it's an environment setting. + + Args: + path_str: The argument value, passed by ``argparse``. + + Returns: + A local path in this OS's format, or ``path_str`` unchanged if it + isn't a ``janelia:`` URI. + + Raises: + ArgumentTypeError: if the URI names no known share, or the share + can't be mounted. + """ + try: + return from_janelia_uri(path_str) + except RuntimeError as e: + # argparse only renders ArgumentTypeError/ValueError/TypeError as a + # clean "argument --x: ..." message; anything else is a traceback. + raise ArgumentTypeError(str(e)) from e diff --git a/src/janelia_pathlib/_macos.py b/src/janelia_pathlib/_macos.py index d6f37a6..00b25e7 100644 --- a/src/janelia_pathlib/_macos.py +++ b/src/janelia_pathlib/_macos.py @@ -18,9 +18,22 @@ _SMB_MOUNT_LINE = re.compile(r"^//(?:[^@]+@)?([^/]+)/(.+?) on (\S+) \(smbfs") _ENV_MOUNT_TIMEOUT = "JANELIA_PATHLIB_MOUNT_TIMEOUT" +_ENV_NO_MOUNT = "JANELIA_PATHLIB_NO_MOUNT" _DEFAULT_MOUNT_TIMEOUT = 30.0 +def no_mount() -> bool: + """Whether on-demand mounting is disabled via ``JANELIA_PATHLIB_NO_MOUNT``. + + Mounting a share can block on a credential prompt, which is unwanted in + a headless or scripted context where nobody is there to answer. Whether + that's the case is a property of the machine, not of the calling code, + so it's an environment setting rather than an argument. + """ + val = os.environ.get(_ENV_NO_MOUNT, "").strip().lower() + return val not in ("", "0", "false", "no") + + def default_mount_timeout() -> float: """Seconds to wait for an on-demand mount, from ``JANELIA_PATHLIB_MOUNT_TIMEOUT``. @@ -113,7 +126,8 @@ def ensure_mounted(share_info: Share, *, timeout: float | None = None) -> Path | with the same name on different servers collide, and the second mounted gets suffixed (``/Volumes/-1``). - ``timeout`` defaults to ``default_mount_timeout()``. + ``timeout`` defaults to ``default_mount_timeout()``. Returns None if the + share isn't mounted and ``JANELIA_PATHLIB_NO_MOUNT`` forbids mounting it. """ if timeout is None: timeout = default_mount_timeout() @@ -123,6 +137,9 @@ def ensure_mounted(share_info: Share, *, timeout: float | None = None) -> Path | if found is not None: return Path(found) + if no_mount(): + return None + # `open` goes through the native SMB client so it can use Keychain # credentials or prompt for them; unlike `mount_smbfs` it returns as # soon as the mount is *requested*. There's no handle to wait on, and diff --git a/src/janelia_pathlib/_translate.py b/src/janelia_pathlib/_translate.py index aaf571c..ff52a5a 100644 --- a/src/janelia_pathlib/_translate.py +++ b/src/janelia_pathlib/_translate.py @@ -2,7 +2,7 @@ from typing import TYPE_CHECKING -from ._macos import ensure_mounted +from ._macos import ensure_mounted, no_mount from ._match import find_share_for_janelia_uri, find_share_for_path from ._share import current_os @@ -24,6 +24,9 @@ def from_janelia_uri(path_str: str, *, timeout: float | None = None) -> str: Windows need no mounting step -- shares are already at a fixed path or reachable directly by UNC path. + Set ``JANELIA_PATHLIB_NO_MOUNT`` to never mount: an unmounted share + raises instead. Useful where no one is there to answer a prompt. + Args: path_str: A ``janelia:/`` URI. Any other string is returned unchanged, so this is safe to call on paths that may @@ -37,8 +40,9 @@ def from_janelia_uri(path_str: str, *, timeout: float | None = None) -> str: ``janelia:`` URI. Raises: - RuntimeError: if the URI names no known share, or if a macOS mount - fails or times out. + RuntimeError: if the URI names no known share, if a macOS mount + fails or times out, or if the share isn't mounted and + ``JANELIA_PATHLIB_NO_MOUNT`` forbids mounting it. """ if not path_str.strip().startswith("janelia:"): return path_str @@ -53,6 +57,11 @@ def from_janelia_uri(path_str: str, *, timeout: float | None = None) -> str: if os_name == "mac": mount_point = ensure_mounted(share_info, timeout=timeout) if mount_point is None: + if no_mount(): + raise RuntimeError( + f"Share {share_info.share_name!r} isn't mounted and " + f"JANELIA_PATHLIB_NO_MOUNT is set, so it can't be mounted." + ) raise RuntimeError(f"Failed to mount share: {share_info.zone}") base = str(mount_point) elif os_name == "linux": diff --git a/tests/test_argparse.py b/tests/test_argparse.py new file mode 100644 index 0000000..0180d1f --- /dev/null +++ b/tests/test_argparse.py @@ -0,0 +1,95 @@ +import argparse +import subprocess +from unittest.mock import patch + +import pytest +from conftest import mac_only + +from janelia_pathlib import janelia_path + + +def _parser(): + parser = argparse.ArgumentParser(prog="demo", exit_on_error=False) + parser.add_argument("--data", type=janelia_path) + return parser + + +def _mount_output(mounted): + stdout = "//u@fileserver.example.org/alphalab on /Volumes/alphalab (smbfs, nodev)" + return subprocess.CompletedProcess( + args=["mount"], returncode=0, stdout=stdout if mounted else "" + ) + + +@pytest.mark.parametrize( + ("os_name", "expected"), + [ + ("linux", "/labs/alpha/alphalab/data"), + ("windows", "\\\\fileserver.example.org\\alphalab\\data"), + ], +) +def test_janelia_path_converts_uri(os_name, expected): + """Linux and Windows need no mounting, so a URI converts outright.""" + with patch("janelia_pathlib._translate.current_os", return_value=os_name): + args = _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + assert args.data == expected + + +def test_janelia_path_passes_through_plain_paths(): + """An argument declared with this type still accepts ordinary paths.""" + args = _parser().parse_args(["--data", "/tmp/local/x"]) + assert args.data == "/tmp/local/x" + + +def test_janelia_path_unknown_share_is_an_argument_error(): + """argparse renders ArgumentTypeError as `argument --data: ...`; the + RuntimeError from_janelia_uri raises would escape as a traceback.""" + with pytest.raises(argparse.ArgumentError, match="No known share"): + _parser().parse_args(["--data", "janelia:/nope/not-a-share"]) + + +@mac_only +def test_janelia_path_resolves_mounted_share(): + with patch("janelia_pathlib._macos.subprocess.run", return_value=_mount_output(True)): + args = _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + assert args.data == "/Volumes/alphalab/data" + + +@mac_only +def test_janelia_path_mounts_on_demand(): + with patch("janelia_pathlib._macos.subprocess.run") as run: + run.side_effect = [_mount_output(False), None, _mount_output(True)] + args = _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + assert args.data == "/Volumes/alphalab/data" + assert run.call_args_list[1][0][0][0] == "open" + + +@mac_only +def test_janelia_path_mount_failure_is_an_argument_error(): + """A mount that never appears must surface as an argument error.""" + monkey = {"JANELIA_PATHLIB_MOUNT_TIMEOUT": "0"} + with patch.dict("os.environ", monkey): + with patch("janelia_pathlib._macos.subprocess.run", return_value=_mount_output(False)): + with pytest.raises(argparse.ArgumentError, match="Failed to mount"): + _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + + +@mac_only +def test_no_mount_env_turns_unmounted_share_into_an_error(monkeypatch): + """The person running a script -- not its author -- knows whether there's + anyone around to answer a credential prompt.""" + monkeypatch.setenv("JANELIA_PATHLIB_NO_MOUNT", "1") + with patch("janelia_pathlib._macos.subprocess.run", return_value=_mount_output(False)) as run: + with pytest.raises(argparse.ArgumentError, match="NO_MOUNT"): + _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + for call in run.call_args_list: + assert call[0][0][0] != "open" + + +@mac_only +def test_no_mount_env_still_resolves_an_already_mounted_share(monkeypatch): + """NO_MOUNT forbids mounting, not resolving.""" + monkeypatch.setenv("JANELIA_PATHLIB_NO_MOUNT", "1") + with patch("janelia_pathlib._macos.subprocess.run", return_value=_mount_output(True)): + args = _parser().parse_args(["--data", "janelia:/labs/alpha/alphalab/data"]) + assert args.data == "/Volumes/alphalab/data" diff --git a/tests/test_share.py b/tests/test_share.py index 3e7ae56..92557c0 100644 --- a/tests/test_share.py +++ b/tests/test_share.py @@ -76,6 +76,24 @@ def test_default_mount_timeout_ignores_garbage(monkeypatch): assert _macos.default_mount_timeout() == 30.0 +@pytest.mark.parametrize("value", ["1", "true", "yes", "anything"]) +def test_no_mount_is_set(monkeypatch, value): + monkeypatch.setenv("JANELIA_PATHLIB_NO_MOUNT", value) + assert _macos.no_mount() is True + + +@pytest.mark.parametrize("value", ["", "0", "false", "no", " FALSE "]) +def test_no_mount_is_unset(monkeypatch, value): + """Same falsey spellings JANELIA_PATHLIB_NO_FETCH accepts.""" + monkeypatch.setenv("JANELIA_PATHLIB_NO_MOUNT", value) + assert _macos.no_mount() is False + + +def test_no_mount_defaults_to_false(monkeypatch): + monkeypatch.delenv("JANELIA_PATHLIB_NO_MOUNT", raising=False) + assert _macos.no_mount() is False + + @mac_only def test_ensure_mounted_honors_env_timeout(monkeypatch): """With a zero timeout, a share that never appears gives up immediately