From 49aec7ec3b5d41f1880e566474a08ebc4f0ea58b Mon Sep 17 00:00:00 2001 From: Tom Kane Date: Mon, 21 Sep 2026 15:55:46 +0000 Subject: [PATCH 1/5] fetch remote screens over http so they can be crawled --- src/techui_builder/generate_jsonmap.py | 9 +- src/techui_builder/jsonmap/crawl.py | 127 ++++++++++++++++--------- src/techui_builder/jsonmap/fetch.py | 48 ++++++++++ src/techui_builder/jsonmap/links.py | 46 +++++---- tests/conftest.py | 34 ++++++- tests/jsonmap/test_crawl.py | 59 ++++++++++-- tests/jsonmap/test_links.py | 53 ++++++----- tests/test_generate_jsonmap.py | 7 -- 8 files changed, 277 insertions(+), 106 deletions(-) create mode 100644 src/techui_builder/jsonmap/fetch.py diff --git a/src/techui_builder/generate_jsonmap.py b/src/techui_builder/generate_jsonmap.py index 3fc3c936..15e6d3a4 100644 --- a/src/techui_builder/generate_jsonmap.py +++ b/src/techui_builder/generate_jsonmap.py @@ -11,6 +11,7 @@ from techui_builder._logger import Logger from techui_builder.jsonmap.crawl import CrawlContext, crawl +from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.nodes import ScreenNode, serialise_node from techui_builder.models import TechUi @@ -46,6 +47,7 @@ class JsonMapGenerator: bob_path: Path = field(default=Path("index.bob")) techui: Path = field(default=Path("techui.yaml")) output: Path | None = field(default=None) + fetcher: ScreenFetcher = field(default_factory=ScreenFetcher) def __post_init__(self): # Determine the directory to write the json map file to. @@ -72,7 +74,6 @@ def __post_init__(self): def generate_json_map( self, screen_path: Path, - dest_path: Path, current_component_name: str | None = None, name_elem: str | None = None, ) -> ScreenNode: @@ -80,9 +81,9 @@ def generate_json_map( ctx = CrawlContext( components=self.techui_yaml.components, synoptic_dir=self._parent_path, - link_base_dir=dest_path, + fetcher=self.fetcher, component_name=current_component_name, - service_name="", + screen=screen_path, ) return crawl(screen_path, ctx, link_name=name_elem) @@ -95,7 +96,7 @@ def write_json_map( f"Cannot generate json map for {self.bob_path}. Has it been generated?" ) - json_map = self.generate_json_map(self.bob_path, self._parent_path) + json_map = self.generate_json_map(self.bob_path) with open(self._write_directory / "JsonMap.json", "w") as f: f.write( json.dumps(json_map, indent=4, default=lambda o: serialise_node(o)) diff --git a/src/techui_builder/jsonmap/crawl.py b/src/techui_builder/jsonmap/crawl.py index 3ca47672..fa7742a4 100644 --- a/src/techui_builder/jsonmap/crawl.py +++ b/src/techui_builder/jsonmap/crawl.py @@ -1,18 +1,22 @@ """Recursively crawl a tree of .bob screens into a tree of ScreenNodes.""" +import logging from collections.abc import Mapping -from dataclasses import dataclass, replace +from dataclasses import dataclass, field, replace from pathlib import Path +from urllib.error import HTTPError +from urllib.parse import urlsplit from lxml import etree, objectify from lxml.objectify import ObjectifiedElement +from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.links import ( WidgetLink, WidgetType, - assumed_exists, extract_links, - find_local_screen, + resolve_link, + substitute_macros, ) from techui_builder.jsonmap.naming import ( find_techui_label, @@ -22,30 +26,51 @@ from techui_builder.jsonmap.nodes import ScreenNode from techui_builder.models import Component +logger_ = logging.getLogger(__name__) + @dataclass class CrawlContext: """State passed down the recursion.""" components: Mapping[str, Component] - synoptic_dir: Path # ScreenNode.file is relative to this - link_base_dir: Path # link files are resolved against this; never changes + synoptic_dir: Path # local ScreenNode.file is relative to this + fetcher: ScreenFetcher component_name: str | None - service_name: str + screen: Path | str # the screen being crawled, as a local path or URL + macros: dict[str, str] = field(default_factory=dict) # including inherited ones + + def with_screen(self, screen: Path | str) -> "CrawlContext": + """A copy for crawling a screen's links, in its component if it names one.""" + component_name = self.component_name + # Only a local screen in the synoptic directory can name a component + if component_name is None and isinstance(screen, Path): + if screen.stem in self.components: + component_name = screen.stem + + return replace(self, screen=screen, component_name=component_name) + + +def find_screen_file(screen: Path | str) -> Path: + """The screen's file name, for a local path or a URL with a query or fragment.""" + if isinstance(screen, str): + return Path(urlsplit(screen).path) + return screen + - def with_screen_component(self, screen_path: Path) -> "CrawlContext": - """A copy for the screen's component, if it is one and none is set yet.""" - if self.component_name is not None or screen_path.stem not in self.components: - return self +def format_screen(screen: Path | str, synoptic_dir: Path) -> str: + """The URL, or the local path relative to the synoptic directory.""" + if isinstance(screen, str): + return screen + return str(screen.resolve().relative_to(synoptic_dir.resolve(), walk_up=True)) - component_name = screen_path.stem - # We know from the if statement that it exists - component = self.components.get(component_name) - assert isinstance(component, Component) - # TODO: How to find the screens if PV prefix is not the service name??? - return replace( - self, component_name=component_name, service_name=component.prefix.lower() - ) + +def inherit_macros( + parent_macros: Mapping[str, str], macros: Mapping[str, str] +) -> dict[str, str]: + """The parent's macros overridden by a link's macros, expanded like Phoebus.""" + expanded = {k: substitute_macros(v, parent_macros) for k, v in macros.items()} + return {**parent_macros, **expanded} def crawl_link( @@ -57,42 +82,63 @@ def crawl_link( If it can't be found, a leaf ScreenNode is returned. """ - local_path = find_local_screen(link.file, ctx.link_base_dir, ctx.service_name) + macros = inherit_macros(ctx.macros, link.macros) + screen = resolve_link(link.file, macros, ctx.screen) + leaf = ScreenNode( + format_screen(screen, ctx.synoptic_dir), display_name, macros=macros + ) + + if isinstance(screen, str): + try: + ctx.fetcher.fetch(screen) + except HTTPError as e: + leaf.exists = False + leaf.error = f"Could not fetch screen: {e}" + return leaf + except OSError as e: + # The server may just be unreachable from here, so assume it exists + leaf.error = f"Could not fetch screen: {e}" + return leaf + + elif not screen.is_file(): + leaf.exists = False + logger_.debug(f"Link {link.file} -> {screen}: not found") + return leaf + + logger_.debug(f"Link {link.file} -> {screen}: found") # Crawl the next file - if local_path is not None: - # TODO: investigate non-recursive approaches? - return crawl(local_path, ctx, link_name=link.name) - - return ScreenNode( - link.file, - display_name, - exists=assumed_exists(link.file, link.macros), - ) + # TODO: investigate non-recursive approaches? + node = crawl(screen, replace(ctx, macros=macros), link_name=link.name) + node.macros = macros + return node + + +def parse_screen(screen: Path | str, fetcher: ScreenFetcher) -> ObjectifiedElement: + """Parse a local or remote .bob screen.""" + if isinstance(screen, str): + return objectify.fromstring(fetcher.fetch(screen), base_url=screen) + return objectify.parse(screen.absolute()).getroot() def crawl( - screen_path: Path, ctx: CrawlContext, link_name: str | None = None + screen: Path | str, ctx: CrawlContext, link_name: str | None = None ) -> ScreenNode: """Crawl a .bob screen and the screens it links to into a ScreenNode.""" # Create initial node at top of .bob file current_node = ScreenNode( - str( - screen_path.resolve().relative_to(ctx.synoptic_dir.resolve(), walk_up=True) - ), - display_name=None, + format_screen(screen, ctx.synoptic_dir), display_name=None ) - ctx = ctx.with_screen_component(screen_path) + ctx = ctx.with_screen(screen) try: # Create xml tree from .bob file - tree = objectify.parse(screen_path.absolute()) - root: ObjectifiedElement = tree.getroot() + root = parse_screen(screen, ctx.fetcher) # Label for the linking widget, else the screen's own , else file stem - own_name = name_or_file_stem(root.name.text, screen_path) + own_name = name_or_file_stem(root.name.text, find_screen_file(screen)) label = find_techui_label(ctx.components, ctx.component_name, link_name) current_node.display_name = label if label is not None else own_name @@ -100,22 +146,17 @@ def crawl( # Label, else widget , else file stem label = find_techui_label(ctx.components, ctx.component_name, link.name) display_name = name_or_file_stem( - label if label is not None else link.name, Path(link.file) + label if label is not None else link.name, find_screen_file(link.file) ) child_node = crawl_link(link, display_name, ctx) if link.type == WidgetType.EMBEDDED: for embedded_child in child_node.children: - embedded_child.macros = {**embedded_child.macros, **link.macros} embedded_child.display_name = display_name - embedded_child.exists = "IOC" in link.macros or ( - "https://" in str(embedded_child.file) - ) current_node.children.append(embedded_child) else: - child_node.macros = link.macros # TODO: make this work for only list[ScreenNode] assert isinstance(current_node.children, list) # TODO: fix typing diff --git a/src/techui_builder/jsonmap/fetch.py b/src/techui_builder/jsonmap/fetch.py new file mode 100644 index 00000000..6e92b442 --- /dev/null +++ b/src/techui_builder/jsonmap/fetch.py @@ -0,0 +1,48 @@ +"""Fetching remote .bob screens over http(s).""" + +import logging +from dataclasses import dataclass, field +from urllib.error import HTTPError +from urllib.parse import urlparse +from urllib.request import urlopen + +logger_ = logging.getLogger(__name__) + +TIMEOUT_SECONDS = 10 + + +def download_url(url: str) -> bytes: + """Download the contents of a URL.""" + with urlopen(url, timeout=TIMEOUT_SECONDS) as response: + return response.read() + + +@dataclass +class ScreenFetcher: + """Fetches remote screens.""" + + _screens: dict[str, bytes] = field(default_factory=dict) + _unreachable_hosts: dict[str, OSError] = field(default_factory=dict) + + def fetch(self, url: str) -> bytes: + """Fetch a screen, raising HTTPError if missing or OSError if unreachable.""" + if url in self._screens: + return self._screens[url] + + # Don't wait for a timeout on every screen from a host that is down + host = urlparse(url).netloc + if host in self._unreachable_hosts: + raise self._unreachable_hosts[host] + + try: + screen = download_url(url) + except HTTPError as e: + logger_.warning(f"Could not fetch {url}: {e}") + raise + except OSError as e: + logger_.warning(f"Could not reach {host}, not crawling its screens: {e}") + self._unreachable_hosts[host] = e + raise + + self._screens[url] = screen + return screen diff --git a/src/techui_builder/jsonmap/links.py b/src/techui_builder/jsonmap/links.py index 54c087e6..805c1175 100644 --- a/src/techui_builder/jsonmap/links.py +++ b/src/techui_builder/jsonmap/links.py @@ -1,16 +1,20 @@ """Links from a .bob screen to other screens.""" +import logging import re from collections.abc import Iterator, Mapping from dataclasses import dataclass from enum import StrEnum from pathlib import Path +from urllib.parse import urljoin, urlsplit from lxml.objectify import ObjectifiedElement from techui_builder.utils import _get_action_group, _get_macros, _get_nav_tabs -PVI_FILE_RE = re.compile(r"^(?:\$\(IOC\))\/([a-zA-Z]+[.a-zA-Z]+)$") +logger_ = logging.getLogger(__name__) + +MACRO_RE = re.compile(r"\$(?:\((\w+)\)|\{(\w+)\})") class WidgetType(StrEnum): @@ -39,8 +43,8 @@ def extract_file_text(file_elem: ObjectifiedElement) -> str: def is_bob(file: str) -> bool: - """Whether the file is a .bob screen.""" - return Path(file).suffix == ".bob" + """Whether the link is to a .bob screen""" + return urlsplit(file).path.endswith(".bob") # ignores url queries def extract_links(root: ObjectifiedElement) -> Iterator[WidgetLink]: @@ -87,6 +91,7 @@ def extract_links(root: ObjectifiedElement) -> Iterator[WidgetLink]: file = extract_file_text(file_elem) # Skip links that are not .bob screens if not is_bob(file): + logger_.debug(f"Skipping link to {file}: not a .bob screen") continue yield WidgetLink(file, name, widget_type, macros) @@ -96,29 +101,34 @@ def extract_links(root: ObjectifiedElement) -> Iterator[WidgetLink]: file = extract_file_text(file_elem) # Skip links that are not .bob screens if not is_bob(file): + logger_.debug(f"Skipping link to {file}: not a .bob screen") continue yield WidgetLink(file, name, widget_type, macros) -def resolve_link_path(file: str, base_dir: Path, service_name: str) -> Path: - """Resolve a link's file to a local path.""" +def is_url(file: str) -> bool: + """Whether the file is an http(s) URL.""" + return file.startswith(("http://", "https://")) - match = PVI_FILE_RE.fullmatch(file) - # The file path is a PVI screen, so attempt to find that screen - if match: - file_name = match.group(1) - return base_dir / f"../{service_name}/{file_name}" - return base_dir / file +def substitute_macros(text: str, macros: Mapping[str, str]) -> str: + """Replace $(NAME) and ${NAME} with macro values, leaving unknown macros as-is.""" + return MACRO_RE.sub( + lambda m: macros.get(m.group(1) or m.group(2), m.group(0)), text + ) -def find_local_screen(file: str, base_dir: Path, service_name: str) -> Path | None: - """Resolve a link's file, returning the path if it can be crawled locally.""" - path = resolve_link_path(file, base_dir, service_name) - return path if path.is_file() else None +def resolve_link( + file: str, macros: Mapping[str, str], screen: Path | str +) -> Path | str: + """Resolve a link's file to a URL or local path, relative to the linking screen.""" + file = substitute_macros(file, macros) + if is_url(file): + return file + # Phoebus resolves relative files against the display containing the link + if isinstance(screen, str): + return urljoin(screen, file) -def assumed_exists(file: str, macros: Mapping[str, str]) -> bool: - """Whether a link's file that could not be found locally is assumed to exist.""" - return "IOC" in macros or ("https:/" in file) + return screen.parent / file diff --git a/tests/conftest.py b/tests/conftest.py index fc936ec8..0178d922 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,10 @@ import shutil +from email.message import Message +from io import BytesIO from pathlib import Path from unittest.mock import MagicMock, Mock, patch +from urllib.error import HTTPError, URLError +from urllib.parse import urlparse import pytest from lxml.etree import Element, SubElement, tostring @@ -16,6 +20,32 @@ from techui_builder.status import GenerateStatusPvs from techui_builder.validator import Validator +TESTS_DIR = Path(__file__).parent + +# Local directories standing in for the opis servers of each beamline +OPIS_SERVERS = { + "t01-opis.diamond.ac.uk": TESTS_DIR / "t01-services", + "b01-1-opis.diamond.ac.uk": TESTS_DIR / "test_files", +} + + +def serve_opis(url: str, timeout: float) -> BytesIO: + """Open a URL on one of the local opis servers.""" + parsed = urlparse(url) + if parsed.netloc not in OPIS_SERVERS: + raise URLError("Name or service not known") + path = OPIS_SERVERS[parsed.netloc] / parsed.path.lstrip("/") + if not path.is_file(): + raise HTTPError(url, 404, "Not Found", Message(), None) + return BytesIO(path.read_bytes()) + + +@pytest.fixture(autouse=True) +def no_network(): + """Serve remote screens from local directories instead of the network.""" + with patch("techui_builder.jsonmap.fetch.urlopen", side_effect=serve_opis): + yield + @pytest.fixture def tmp_t01_services(tmp_path) -> Path: @@ -246,7 +276,7 @@ def example_json_map_pvi_screens(): duplicate=False, children=[ ScreenNode( - file="../bl01t-mo-motor-01/pmacAxis.pvi.bob", + file="https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01/pmacAxis.pvi.bob", display_name="X1", exists=True, duplicate=False, @@ -260,7 +290,7 @@ def example_json_map_pvi_screens(): error="", ), ScreenNode( - file="../bl01t-mo-motor-01/pmacAxis.pvi.bob", + file="https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01/pmacAxis.pvi.bob", display_name="A", exists=True, duplicate=False, diff --git a/tests/jsonmap/test_crawl.py b/tests/jsonmap/test_crawl.py index 3c89504d..284fbb9d 100644 --- a/tests/jsonmap/test_crawl.py +++ b/tests/jsonmap/test_crawl.py @@ -3,9 +3,13 @@ import pytest from techui_builder.jsonmap.crawl import CrawlContext, crawl, crawl_link +from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.links import WidgetLink, WidgetType from techui_builder.jsonmap.nodes import ScreenNode +MOTOR_IOC = "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" +DCAM_IOC = "https://b01-1-opis.diamond.ac.uk/bl01c-di-dcam-01" + @pytest.fixture def t01_ctx(json_map_generator) -> CrawlContext: @@ -13,9 +17,9 @@ def t01_ctx(json_map_generator) -> CrawlContext: return CrawlContext( components=json_map_generator.techui_yaml.components, synoptic_dir=synoptic, - link_base_dir=synoptic, + fetcher=ScreenFetcher(), component_name=None, - service_name="", + screen=synoptic / "index.bob", ) @@ -38,8 +42,8 @@ def test_crawl_component_screen(t01_ctx): assert motor.display_name == "Motor Stage" assert [(c.file, c.display_name) for c in motor.children] == [ - ("../bl01t-mo-motor-01/pmacAxis.pvi.bob", "X1"), - ("../bl01t-mo-motor-01/pmacAxis.pvi.bob", "A"), + (f"{MOTOR_IOC}/pmacAxis.pvi.bob", "X1"), + (f"{MOTOR_IOC}/pmacAxis.pvi.bob", "A"), ("techui-support/bob/pmac/pmacController.bob", "pmacController"), ] @@ -61,12 +65,47 @@ def test_crawl_link(t01_ctx): assert missing == ScreenNode("missing.bob", "Missing", exists=False) -def test_with_screen_component(t01_ctx): - motor_ctx = t01_ctx.with_screen_component(Path("motor1.bob")) +def test_crawl_link_remote_screen(t01_ctx): + link = WidgetLink( + f"{DCAM_IOC}/ADUVC.pvi.bob", + "DRV", + WidgetType.ACTION_BUTTON, + {"P": "BL01C-DI-DCAM-01", "R": ":DRV:"}, + ) + camera = crawl_link(link, "DRV", t01_ctx) + missing = crawl_link( + WidgetLink(f"{DCAM_IOC}/missing.pvi.bob", "Missing", link.type, {}), + "Missing", + t01_ctx, + ) - assert motor_ctx.component_name == "motor1" - assert motor_ctx.service_name == "bl01t-mo-motor-01" + assert (camera.file, camera.display_name, camera.error) == ( + link.file, + "ADUVC Camera", + "", + ) + # The subscreen link is relative and has no macros of its own + assert camera.children == [ + ScreenNode(f"{DCAM_IOC}/ADUVC_Advanced.pvi.bob", "Advanced", macros=link.macros) + ] + assert (missing.exists, missing.children) == (False, []) + assert missing.error.startswith("Could not fetch screen") + + +def test_with_screen(t01_ctx): + motor_ctx = t01_ctx.with_screen(Path("motor1.bob")) + + assert (motor_ctx.screen, motor_ctx.component_name) == ( + Path("motor1.bob"), + "motor1", + ) # Not a component - assert t01_ctx.with_screen_component(Path("index.bob")) is t01_ctx + assert t01_ctx.with_screen(Path("index.bob")).component_name is None # Already inside a component - assert motor_ctx.with_screen_component(Path("dcam1.bob")) is motor_ctx + assert motor_ctx.with_screen(Path("dcam1.bob")).component_name == "motor1" + # A remote screen is never a component, but is still the screen being crawled + remote_ctx = t01_ctx.with_screen(f"{MOTOR_IOC}/motor1.bob") + assert (remote_ctx.screen, remote_ctx.component_name) == ( + f"{MOTOR_IOC}/motor1.bob", + None, + ) diff --git a/tests/jsonmap/test_links.py b/tests/jsonmap/test_links.py index 47d3d46b..dcc4aba4 100644 --- a/tests/jsonmap/test_links.py +++ b/tests/jsonmap/test_links.py @@ -5,10 +5,9 @@ from techui_builder.jsonmap.links import ( WidgetLink, WidgetType, - assumed_exists, extract_links, - find_local_screen, - resolve_link_path, + is_bob, + resolve_link, ) @@ -63,26 +62,36 @@ def test_extract_links(): ] -def test_resolve_link_path(): - dest = Path("/beamline/synoptic") - svc = "bl01t-mo-motor-01" +def test_resolve_link(): + screen = Path("/services/synoptic/techui-support/bob/slits/slit.bob") + url = "https://t01-opis.diamond.ac.uk/bl01t-di-cam-01/ADUVC.pvi.bob" + macros = {"IOC": "https://t01-opis.diamond.ac.uk/bl01t-di-cam-01"} - assert resolve_link_path("sub/screen.bob", dest, svc) == dest / "sub/screen.bob" - assert ( - resolve_link_path("$(IOC)/Simple.pvi.bob", dest, svc) - == dest / f"../{svc}/Simple.pvi.bob" + # Local files are relative to the screen containing the link + assert resolve_link("../pmac/motor.bob", macros, screen) == Path( + "/services/synoptic/techui-support/bob/slits/../pmac/motor.bob" + ) + # Macros are substituted, and unknown ones are left alone + assert resolve_link("$(IOC)/ADUVC.pvi.bob", macros, screen) == url + assert resolve_link("${IOC}/ADUVC.pvi.bob", macros, screen) == url + assert resolve_link("$(IOC)/x.bob", {}, screen) == screen.parent / "$(IOC)/x.bob" + # Files in remote screens are relative to the screen's URL + assert resolve_link("ADUVC_Advanced.pvi.bob", {}, url) == url.replace( + "ADUVC.pvi.bob", "ADUVC_Advanced.pvi.bob" + ) + assert resolve_link("http://other.invalid/x.bob", macros, url) == ( + "http://other.invalid/x.bob" ) -def test_find_local_screen(tmp_path: Path): - (tmp_path / "screen.bob").touch() - - assert find_local_screen("screen.bob", tmp_path, "") == tmp_path / "screen.bob" - for file in ["missing.bob", "", "https://example.invalid/x/screen.bob"]: - assert find_local_screen(file, tmp_path, "") is None - - -def test_assumed_exists(): - assert assumed_exists("$(IOC)/x.pvi.bob", {"IOC": "https://example.invalid"}) - assert assumed_exists("https://example.invalid/x/screen.bob", {}) - assert not assumed_exists("missing.bob", {"P": "BL01T-MO-MOTOR-01"}) +def test_is_bob(): + assert is_bob("dcam1.bob") + assert is_bob("$(IOC)/dcam1.bob") + assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob") + # A url may carry a query or fragment that is not part of the file name + assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob?v=2") + assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob#Advanced") + # Screens Phoebus can open but the builder does not crawl + assert not is_bob("dcam1.opi") + assert not is_bob("https://opis.diamond.ac.uk/ioc/index.html") + assert not is_bob("") diff --git a/tests/test_generate_jsonmap.py b/tests/test_generate_jsonmap.py index b64ef3e0..0899c2a0 100644 --- a/tests/test_generate_jsonmap.py +++ b/tests/test_generate_jsonmap.py @@ -67,7 +67,6 @@ def test_write_json_map(json_map_generator, tmp_test_files): def test_generate_json_map(json_map_generator_with_test_files, example_json_map): test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map == example_json_map @@ -92,7 +91,6 @@ def test_generate_json_map_embedded_screen( test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map == example_json_map @@ -114,7 +112,6 @@ def test_generate_json_map_nav_tabs( test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map == example_json_map_root @@ -125,7 +122,6 @@ def test_generate_json_map_child_file_crawl_pvi_screen( ): jsonmap = json_map_generator.generate_json_map( screen_path=tmp_t01_services / "synoptic/motor1.bob", - dest_path=tmp_t01_services / "synoptic", ) assert example_json_map_pvi_screens == jsonmap @@ -150,7 +146,6 @@ def test_generate_json_map_get_macros( test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map == example_json_map @@ -162,7 +157,6 @@ def test_generate_json_map_xml_parse_error( test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map.error.startswith("XML parse error:") @@ -177,7 +171,6 @@ def test_generate_json_map_other_exception( test_json_map = json_map_generator_with_test_files.generate_json_map( json_map_generator_with_test_files.bob_path, - json_map_generator_with_test_files._write_directory, ) assert test_json_map.error != "" From 0ba83ab3b62cb3174857fb5c16a1b9f267f20e54 Mon Sep 17 00:00:00 2001 From: Tom Kane Date: Wed, 23 Sep 2026 17:00:48 +0000 Subject: [PATCH 2/5] use the t01 example beamline in the remote screen crawl test The test fetched a screen from a real beamline's opis server, which meant the hostname, IOC and PV prefix appeared in the test and the screens it crawled were not in the repo. Crawl the t01 motor IOC's index.bob instead: it is already checked in under tests/t01-services and links on to three sub-screens, so it covers the same behaviour without any new fixtures. --- tests/conftest.py | 1 - tests/jsonmap/test_crawl.py | 35 +++++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0178d922..73c33611 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,7 +25,6 @@ # Local directories standing in for the opis servers of each beamline OPIS_SERVERS = { "t01-opis.diamond.ac.uk": TESTS_DIR / "t01-services", - "b01-1-opis.diamond.ac.uk": TESTS_DIR / "test_files", } diff --git a/tests/jsonmap/test_crawl.py b/tests/jsonmap/test_crawl.py index 284fbb9d..9269ad06 100644 --- a/tests/jsonmap/test_crawl.py +++ b/tests/jsonmap/test_crawl.py @@ -8,7 +8,6 @@ from techui_builder.jsonmap.nodes import ScreenNode MOTOR_IOC = "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" -DCAM_IOC = "https://b01-1-opis.diamond.ac.uk/bl01c-di-dcam-01" @pytest.fixture @@ -67,26 +66,38 @@ def test_crawl_link(t01_ctx): def test_crawl_link_remote_screen(t01_ctx): link = WidgetLink( - f"{DCAM_IOC}/ADUVC.pvi.bob", - "DRV", - WidgetType.ACTION_BUTTON, - {"P": "BL01C-DI-DCAM-01", "R": ":DRV:"}, + f"{MOTOR_IOC}/index.bob", "Motors", WidgetType.ACTION_BUTTON, {"P": "BL01T"} ) - camera = crawl_link(link, "DRV", t01_ctx) + ioc = crawl_link(link, "Motors", t01_ctx) missing = crawl_link( - WidgetLink(f"{DCAM_IOC}/missing.pvi.bob", "Missing", link.type, {}), + WidgetLink(f"{MOTOR_IOC}/missing.bob", "Missing", link.type, {}), "Missing", t01_ctx, ) - assert (camera.file, camera.display_name, camera.error) == ( + # The display name comes from the fetched screen, not the link + assert (ioc.file, ioc.display_name, ioc.error) == ( link.file, - "ADUVC Camera", + "bl01t-mo-brick-01", "", ) - # The subscreen link is relative and has no macros of its own - assert camera.children == [ - ScreenNode(f"{DCAM_IOC}/ADUVC_Advanced.pvi.bob", "Advanced", macros=link.macros) + # Sub-screen links are relative to the screen that contains them + assert [(c.file, c.display_name, c.macros) for c in ioc.children] == [ + ( + f"{MOTOR_IOC}/ppmacController.pvi.bob", + "ppmacController", + {"P": "BL01T-MO-BRICK-01"}, + ), + ( + f"{MOTOR_IOC}/pmacAxis.pvi.bob", + "pmacAxis (BL01T-MO-MOTOR-01)", + {"P": "BL01T-MO-MOTOR-01", "M": ":X"}, + ), + ( + f"{MOTOR_IOC}/pmacAxis.pvi.bob", + "pmacAxis (BL01T-MO-MOTOR-01)", + {"P": "BL01T-MO-MOTOR-01", "M": ":A"}, + ), ] assert (missing.exists, missing.children) == (False, []) assert missing.error.startswith("Could not fetch screen") From 45a9200733f4cfc67501798e7b288a489221e54b Mon Sep 17 00:00:00 2001 From: Tom Kane Date: Wed, 23 Sep 2026 17:02:20 +0000 Subject: [PATCH 3/5] import WidgetType from utils rather than through links #273 moved WidgetType to utils.py; the crawl module and its tests were still reaching it through jsonmap.links, which only re-imports it. --- src/techui_builder/jsonmap/crawl.py | 2 +- tests/jsonmap/test_crawl.py | 3 ++- tests/jsonmap/test_links.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/techui_builder/jsonmap/crawl.py b/src/techui_builder/jsonmap/crawl.py index fa7742a4..0264302c 100644 --- a/src/techui_builder/jsonmap/crawl.py +++ b/src/techui_builder/jsonmap/crawl.py @@ -13,7 +13,6 @@ from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.links import ( WidgetLink, - WidgetType, extract_links, resolve_link, substitute_macros, @@ -25,6 +24,7 @@ ) from techui_builder.jsonmap.nodes import ScreenNode from techui_builder.models import Component +from techui_builder.utils import WidgetType logger_ = logging.getLogger(__name__) diff --git a/tests/jsonmap/test_crawl.py b/tests/jsonmap/test_crawl.py index 9269ad06..a666447f 100644 --- a/tests/jsonmap/test_crawl.py +++ b/tests/jsonmap/test_crawl.py @@ -4,8 +4,9 @@ from techui_builder.jsonmap.crawl import CrawlContext, crawl, crawl_link from techui_builder.jsonmap.fetch import ScreenFetcher -from techui_builder.jsonmap.links import WidgetLink, WidgetType +from techui_builder.jsonmap.links import WidgetLink from techui_builder.jsonmap.nodes import ScreenNode +from techui_builder.utils import WidgetType MOTOR_IOC = "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" diff --git a/tests/jsonmap/test_links.py b/tests/jsonmap/test_links.py index dcc4aba4..0890731f 100644 --- a/tests/jsonmap/test_links.py +++ b/tests/jsonmap/test_links.py @@ -4,11 +4,11 @@ from techui_builder.jsonmap.links import ( WidgetLink, - WidgetType, extract_links, is_bob, resolve_link, ) +from techui_builder.utils import WidgetType def button(name: str, *actions: str, widget_type="action_button") -> str: From 99b39d9f8d6d5e6a5146a8a21a41d8753a737457 Mon Sep 17 00:00:00 2001 From: Tom Kane Date: Wed, 23 Sep 2026 17:14:08 +0000 Subject: [PATCH 4/5] stop remote screen test for now --- tests/jsonmap/test_crawl.py | 74 ++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/tests/jsonmap/test_crawl.py b/tests/jsonmap/test_crawl.py index a666447f..259ceea6 100644 --- a/tests/jsonmap/test_crawl.py +++ b/tests/jsonmap/test_crawl.py @@ -65,43 +65,43 @@ def test_crawl_link(t01_ctx): assert missing == ScreenNode("missing.bob", "Missing", exists=False) -def test_crawl_link_remote_screen(t01_ctx): - link = WidgetLink( - f"{MOTOR_IOC}/index.bob", "Motors", WidgetType.ACTION_BUTTON, {"P": "BL01T"} - ) - ioc = crawl_link(link, "Motors", t01_ctx) - missing = crawl_link( - WidgetLink(f"{MOTOR_IOC}/missing.bob", "Missing", link.type, {}), - "Missing", - t01_ctx, - ) - - # The display name comes from the fetched screen, not the link - assert (ioc.file, ioc.display_name, ioc.error) == ( - link.file, - "bl01t-mo-brick-01", - "", - ) - # Sub-screen links are relative to the screen that contains them - assert [(c.file, c.display_name, c.macros) for c in ioc.children] == [ - ( - f"{MOTOR_IOC}/ppmacController.pvi.bob", - "ppmacController", - {"P": "BL01T-MO-BRICK-01"}, - ), - ( - f"{MOTOR_IOC}/pmacAxis.pvi.bob", - "pmacAxis (BL01T-MO-MOTOR-01)", - {"P": "BL01T-MO-MOTOR-01", "M": ":X"}, - ), - ( - f"{MOTOR_IOC}/pmacAxis.pvi.bob", - "pmacAxis (BL01T-MO-MOTOR-01)", - {"P": "BL01T-MO-MOTOR-01", "M": ":A"}, - ), - ] - assert (missing.exists, missing.children) == (False, []) - assert missing.error.startswith("Could not fetch screen") +# def test_crawl_link_remote_screen(t01_ctx): +# link = WidgetLink( +# f"{MOTOR_IOC}/index.bob", "Motors", WidgetType.ACTION_BUTTON, {"P": "BL01T"} +# ) +# ioc = crawl_link(link, "Motors", t01_ctx) +# missing = crawl_link( +# WidgetLink(f"{MOTOR_IOC}/missing.bob", "Missing", link.type, {}), +# "Missing", +# t01_ctx, +# ) + +# # The display name comes from the fetched screen, not the link +# assert (ioc.file, ioc.display_name, ioc.error) == ( +# link.file, +# "bl01t-mo-brick-01", +# "", +# ) +# # Sub-screen links are relative to the screen that contains them +# assert [(c.file, c.display_name, c.macros) for c in ioc.children] == [ +# ( +# f"{MOTOR_IOC}/ppmacController.pvi.bob", +# "ppmacController", +# {"P": "BL01T-MO-BRICK-01"}, +# ), +# ( +# f"{MOTOR_IOC}/pmacAxis.pvi.bob", +# "pmacAxis (BL01T-MO-MOTOR-01)", +# {"P": "BL01T-MO-MOTOR-01", "M": ":X"}, +# ), +# ( +# f"{MOTOR_IOC}/pmacAxis.pvi.bob", +# "pmacAxis (BL01T-MO-MOTOR-01)", +# {"P": "BL01T-MO-MOTOR-01", "M": ":A"}, +# ), +# ] +# assert (missing.exists, missing.children) == (False, []) +# assert missing.error.startswith("Could not fetch screen") def test_with_screen(t01_ctx): From 1925588dbb033aefe1a0a59dff7036758b8dcb64 Mon Sep 17 00:00:00 2001 From: Tom Kane Date: Thu, 24 Sep 2026 17:05:47 +0000 Subject: [PATCH 5/5] remove fetching logic and refactor after --- example/t01-services/synoptic/JsonMap.json | 8 +-- example/t01-services/synoptic/dcam1.bob | 2 +- example/t01-services/synoptic/motor1.bob | 8 +-- src/techui_builder/generate.py | 16 ++++- src/techui_builder/generate_jsonmap.py | 3 - src/techui_builder/jsonmap/crawl.py | 72 +++++-------------- src/techui_builder/jsonmap/fetch.py | 48 ------------- src/techui_builder/jsonmap/links.py | 22 ++---- src/techui_builder/jsonmap/nodes.py | 1 - tests/conftest.py | 41 ++--------- tests/jsonmap/test_crawl.py | 49 +------------ tests/jsonmap/test_links.py | 22 ++---- tests/jsonmap/test_nodes.py | 1 - tests/test_files/widget.xml | 2 +- ...l_screen.xml => widget_service_screen.xml} | 2 +- tests/test_generate.py | 4 +- 16 files changed, 61 insertions(+), 240 deletions(-) delete mode 100644 src/techui_builder/jsonmap/fetch.py rename tests/test_files/{widget_url_screen.xml => widget_service_screen.xml} (88%) diff --git a/example/t01-services/synoptic/JsonMap.json b/example/t01-services/synoptic/JsonMap.json index 061b331b..bc0895cf 100644 --- a/example/t01-services/synoptic/JsonMap.json +++ b/example/t01-services/synoptic/JsonMap.json @@ -5,12 +5,12 @@ "file": "dcam1.bob", "children": [ { - "file": "ProfileCursorGraphs.bob", + "file": "techui-support/bob/ADAravis/ProfileCursorGraphs.bob", "macros": { "P": "BL01T-DI-CAM-01", "R": ":CAM:", "label": "CAM", - "IOC": "https://t01-opis.diamond.ac.uk/bl01t-di-cam-01" + "IOC": "../../../../bl01t-di-cam-01" }, "displayName": "CAM" }, @@ -72,7 +72,7 @@ "M": ":X", "P": "BL01T-MO-MOTOR-01", "label": "X1", - "IOC": "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" + "IOC": "../../../../bl01t-mo-motor-01" }, "displayName": "X1" }, @@ -82,7 +82,7 @@ "M": ":A", "P": "BL01T-MO-MOTOR-01", "label": "A", - "IOC": "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" + "IOC": "../../../../bl01t-mo-motor-01" }, "displayName": "A" }, diff --git a/example/t01-services/synoptic/dcam1.bob b/example/t01-services/synoptic/dcam1.bob index 12797766..ab720dd9 100644 --- a/example/t01-services/synoptic/dcam1.bob +++ b/example/t01-services/synoptic/dcam1.bob @@ -16,7 +16,7 @@

BL01T-DI-CAM-01

:CAM: - https://t01-opis.diamond.ac.uk/bl01t-di-cam-01 + ../../../../bl01t-di-cam-01 0 0 diff --git a/example/t01-services/synoptic/motor1.bob b/example/t01-services/synoptic/motor1.bob index e4abe216..2d93162d 100644 --- a/example/t01-services/synoptic/motor1.bob +++ b/example/t01-services/synoptic/motor1.bob @@ -13,10 +13,10 @@ 120 techui-support/bob/pmac/motor_embed.bob -

BL01T-MO-MOTOR-01

:X +

BL01T-MO-MOTOR-01

- https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01 + ../../../../bl01t-mo-motor-01
0 0 @@ -27,10 +27,10 @@ 120 techui-support/bob/pmac/motor_embed.bob -

BL01T-MO-MOTOR-01

:A +

BL01T-MO-MOTOR-01

- https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01 + ../../../../bl01t-mo-motor-01
0 150 diff --git a/src/techui_builder/generate.py b/src/techui_builder/generate.py index b1dee7f9..59a03030 100644 --- a/src/techui_builder/generate.py +++ b/src/techui_builder/generate.py @@ -181,6 +181,12 @@ def _update_macros(self, component: Entity) -> tuple[str, dict[str, str]]: return component_name, new_macros + def resolve_service_dir(self, service_name: str, screen_dir: Path) -> str: + """A service's screen directory, relative to the screen linking to it.""" + # Service directories sit alongside the synoptic directory, not inside it + service_dir = self.synoptic_dir.resolve().parent / service_name + return str(service_dir.relative_to(screen_dir.resolve(), walk_up=True)) + def _allocate_widget( self, screen_mapping: Mapping, component: Entity ) -> EmbeddedDisplay | ActionButton | None | list[EmbeddedDisplay | ActionButton]: @@ -191,7 +197,8 @@ def _allocate_widget( file = Template(screen_mapping["file"]).render(component.macros) if file.startswith("$(IOC)"): screen_path = support_screen_path = file.replace( - "$(IOC)", f"{self.beamline_url}/{component.service_name}" + "$(IOC)", + self.resolve_service_dir(component.service_name, self.synoptic_dir), ) # Only works with related displays as # embedded displays need to access the file to get dimensions @@ -243,7 +250,12 @@ def _allocate_widget( # TODO: Change this to pvi_button if True: - new_widget.macro("IOC", f"{self.beamline_url}/{component.service_name}") + new_widget.macro( + "IOC", + self.resolve_service_dir( + component.service_name, Path(screen_path).parent + ), + ) # The only other option is for related displays else: diff --git a/src/techui_builder/generate_jsonmap.py b/src/techui_builder/generate_jsonmap.py index 15e6d3a4..5ebc9c03 100644 --- a/src/techui_builder/generate_jsonmap.py +++ b/src/techui_builder/generate_jsonmap.py @@ -11,7 +11,6 @@ from techui_builder._logger import Logger from techui_builder.jsonmap.crawl import CrawlContext, crawl -from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.nodes import ScreenNode, serialise_node from techui_builder.models import TechUi @@ -47,7 +46,6 @@ class JsonMapGenerator: bob_path: Path = field(default=Path("index.bob")) techui: Path = field(default=Path("techui.yaml")) output: Path | None = field(default=None) - fetcher: ScreenFetcher = field(default_factory=ScreenFetcher) def __post_init__(self): # Determine the directory to write the json map file to. @@ -81,7 +79,6 @@ def generate_json_map( ctx = CrawlContext( components=self.techui_yaml.components, synoptic_dir=self._parent_path, - fetcher=self.fetcher, component_name=current_component_name, screen=screen_path, ) diff --git a/src/techui_builder/jsonmap/crawl.py b/src/techui_builder/jsonmap/crawl.py index 0264302c..53d46425 100644 --- a/src/techui_builder/jsonmap/crawl.py +++ b/src/techui_builder/jsonmap/crawl.py @@ -4,13 +4,9 @@ from collections.abc import Mapping from dataclasses import dataclass, field, replace from pathlib import Path -from urllib.error import HTTPError -from urllib.parse import urlsplit from lxml import etree, objectify -from lxml.objectify import ObjectifiedElement -from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.links import ( WidgetLink, extract_links, @@ -34,34 +30,22 @@ class CrawlContext: """State passed down the recursion.""" components: Mapping[str, Component] - synoptic_dir: Path # local ScreenNode.file is relative to this - fetcher: ScreenFetcher + synoptic_dir: Path # ScreenNode.file is relative to this component_name: str | None - screen: Path | str # the screen being crawled, as a local path or URL + screen: Path # the screen being crawled macros: dict[str, str] = field(default_factory=dict) # including inherited ones - def with_screen(self, screen: Path | str) -> "CrawlContext": + def with_screen(self, screen: Path) -> "CrawlContext": """A copy for crawling a screen's links, in its component if it names one.""" component_name = self.component_name - # Only a local screen in the synoptic directory can name a component - if component_name is None and isinstance(screen, Path): - if screen.stem in self.components: - component_name = screen.stem + if component_name is None and screen.stem in self.components: + component_name = screen.stem return replace(self, screen=screen, component_name=component_name) -def find_screen_file(screen: Path | str) -> Path: - """The screen's file name, for a local path or a URL with a query or fragment.""" - if isinstance(screen, str): - return Path(urlsplit(screen).path) - return screen - - -def format_screen(screen: Path | str, synoptic_dir: Path) -> str: - """The URL, or the local path relative to the synoptic directory.""" - if isinstance(screen, str): - return screen +def format_screen(screen: Path, synoptic_dir: Path) -> str: + """The screen's path, relative to the synoptic directory.""" return str(screen.resolve().relative_to(synoptic_dir.resolve(), walk_up=True)) @@ -84,26 +68,15 @@ def crawl_link( """ macros = inherit_macros(ctx.macros, link.macros) screen = resolve_link(link.file, macros, ctx.screen) - leaf = ScreenNode( - format_screen(screen, ctx.synoptic_dir), display_name, macros=macros - ) - if isinstance(screen, str): - try: - ctx.fetcher.fetch(screen) - except HTTPError as e: - leaf.exists = False - leaf.error = f"Could not fetch screen: {e}" - return leaf - except OSError as e: - # The server may just be unreachable from here, so assume it exists - leaf.error = f"Could not fetch screen: {e}" - return leaf - - elif not screen.is_file(): - leaf.exists = False + if not screen.is_file(): logger_.debug(f"Link {link.file} -> {screen}: not found") - return leaf + return ScreenNode( + format_screen(screen, ctx.synoptic_dir), + display_name, + exists=False, + macros=macros, + ) logger_.debug(f"Link {link.file} -> {screen}: found") @@ -114,16 +87,7 @@ def crawl_link( return node -def parse_screen(screen: Path | str, fetcher: ScreenFetcher) -> ObjectifiedElement: - """Parse a local or remote .bob screen.""" - if isinstance(screen, str): - return objectify.fromstring(fetcher.fetch(screen), base_url=screen) - return objectify.parse(screen.absolute()).getroot() - - -def crawl( - screen: Path | str, ctx: CrawlContext, link_name: str | None = None -) -> ScreenNode: +def crawl(screen: Path, ctx: CrawlContext, link_name: str | None = None) -> ScreenNode: """Crawl a .bob screen and the screens it links to into a ScreenNode.""" # Create initial node at top of .bob file @@ -135,10 +99,10 @@ def crawl( try: # Create xml tree from .bob file - root = parse_screen(screen, ctx.fetcher) + root = objectify.parse(screen.absolute()).getroot() # Label for the linking widget, else the screen's own , else file stem - own_name = name_or_file_stem(root.name.text, find_screen_file(screen)) + own_name = name_or_file_stem(root.name.text, screen) label = find_techui_label(ctx.components, ctx.component_name, link_name) current_node.display_name = label if label is not None else own_name @@ -146,7 +110,7 @@ def crawl( # Label, else widget , else file stem label = find_techui_label(ctx.components, ctx.component_name, link.name) display_name = name_or_file_stem( - label if label is not None else link.name, find_screen_file(link.file) + label if label is not None else link.name, Path(link.file) ) child_node = crawl_link(link, display_name, ctx) diff --git a/src/techui_builder/jsonmap/fetch.py b/src/techui_builder/jsonmap/fetch.py deleted file mode 100644 index 6e92b442..00000000 --- a/src/techui_builder/jsonmap/fetch.py +++ /dev/null @@ -1,48 +0,0 @@ -"""Fetching remote .bob screens over http(s).""" - -import logging -from dataclasses import dataclass, field -from urllib.error import HTTPError -from urllib.parse import urlparse -from urllib.request import urlopen - -logger_ = logging.getLogger(__name__) - -TIMEOUT_SECONDS = 10 - - -def download_url(url: str) -> bytes: - """Download the contents of a URL.""" - with urlopen(url, timeout=TIMEOUT_SECONDS) as response: - return response.read() - - -@dataclass -class ScreenFetcher: - """Fetches remote screens.""" - - _screens: dict[str, bytes] = field(default_factory=dict) - _unreachable_hosts: dict[str, OSError] = field(default_factory=dict) - - def fetch(self, url: str) -> bytes: - """Fetch a screen, raising HTTPError if missing or OSError if unreachable.""" - if url in self._screens: - return self._screens[url] - - # Don't wait for a timeout on every screen from a host that is down - host = urlparse(url).netloc - if host in self._unreachable_hosts: - raise self._unreachable_hosts[host] - - try: - screen = download_url(url) - except HTTPError as e: - logger_.warning(f"Could not fetch {url}: {e}") - raise - except OSError as e: - logger_.warning(f"Could not reach {host}, not crawling its screens: {e}") - self._unreachable_hosts[host] = e - raise - - self._screens[url] = screen - return screen diff --git a/src/techui_builder/jsonmap/links.py b/src/techui_builder/jsonmap/links.py index 149d70b6..070e6bc5 100644 --- a/src/techui_builder/jsonmap/links.py +++ b/src/techui_builder/jsonmap/links.py @@ -5,7 +5,6 @@ from collections.abc import Iterator, Mapping from dataclasses import dataclass from pathlib import Path -from urllib.parse import urljoin, urlsplit from lxml.objectify import ObjectifiedElement @@ -33,13 +32,13 @@ class WidgetLink: def extract_file_text(file_elem: ObjectifiedElement) -> str: """The stripped text of a element.""" - # Keep raw string to preserve urls + # Keep the raw string; macros are expanded later return file_elem.text.strip() if file_elem.text else "" def is_bob(file: str) -> bool: """Whether the link is to a .bob screen""" - return urlsplit(file).path.endswith(".bob") # ignores url queries + return file.endswith(".bob") def extract_links(root: ObjectifiedElement) -> Iterator[WidgetLink]: @@ -106,11 +105,6 @@ def extract_links(root: ObjectifiedElement) -> Iterator[WidgetLink]: yield WidgetLink(file, name, widget_type, macros) -def is_url(file: str) -> bool: - """Whether the file is an http(s) URL.""" - return file.startswith(("http://", "https://")) - - def substitute_macros(text: str, macros: Mapping[str, str]) -> str: """Replace $(NAME) and ${NAME} with macro values, leaving unknown macros as-is.""" return MACRO_RE.sub( @@ -118,16 +112,8 @@ def substitute_macros(text: str, macros: Mapping[str, str]) -> str: ) -def resolve_link( - file: str, macros: Mapping[str, str], screen: Path | str -) -> Path | str: - """Resolve a link's file to a URL or local path, relative to the linking screen.""" +def resolve_link(file: str, macros: Mapping[str, str], screen: Path) -> Path: + """Resolve a link's file to a local path, relative to the linking screen.""" file = substitute_macros(file, macros) - if is_url(file): - return file - # Phoebus resolves relative files against the display containing the link - if isinstance(screen, str): - return urljoin(screen, file) - return screen.parent / file diff --git a/src/techui_builder/jsonmap/nodes.py b/src/techui_builder/jsonmap/nodes.py index 849f4f3c..8e964f4c 100644 --- a/src/techui_builder/jsonmap/nodes.py +++ b/src/techui_builder/jsonmap/nodes.py @@ -11,7 +11,6 @@ class ScreenNode: file: str display_name: str | None exists: bool = True - duplicate: bool = False children: list["ScreenNode"] = field(default_factory=list) macros: dict[str, str] = field(default_factory=dict) error: str = "" diff --git a/tests/conftest.py b/tests/conftest.py index 4b482459..dcea3c6e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,6 @@ import shutil -from email.message import Message -from io import BytesIO from pathlib import Path from unittest.mock import MagicMock, Mock, patch -from urllib.error import HTTPError, URLError -from urllib.parse import urlparse import pytest from lxml.etree import Element, SubElement, tostring @@ -22,29 +18,6 @@ TESTS_DIR = Path(__file__).parent -# Local directories standing in for the opis servers of each beamline -OPIS_SERVERS = { - "t01-opis.diamond.ac.uk": TESTS_DIR / "t01-services", -} - - -def serve_opis(url: str, timeout: float) -> BytesIO: - """Open a URL on one of the local opis servers.""" - parsed = urlparse(url) - if parsed.netloc not in OPIS_SERVERS: - raise URLError("Name or service not known") - path = OPIS_SERVERS[parsed.netloc] / parsed.path.lstrip("/") - if not path.is_file(): - raise HTTPError(url, 404, "Not Found", Message(), None) - return BytesIO(path.read_bytes()) - - -@pytest.fixture(autouse=True) -def no_network(): - """Serve remote screens from local directories instead of the network.""" - with patch("techui_builder.jsonmap.fetch.urlopen", side_effect=serve_opis): - yield - @pytest.fixture def tmp_t01_services(tmp_path) -> Path: @@ -272,33 +245,30 @@ def example_json_map_pvi_screens(): file="motor1.bob", display_name="motor1", exists=True, - duplicate=False, children=[ ScreenNode( - file="https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01/pmacAxis.pvi.bob", + file="../bl01t-mo-motor-01/pmacAxis.pvi.bob", display_name="X1", exists=True, - duplicate=False, children=[], macros={ "M": ":X", "P": "BL01T-MO-MOTOR-01", "label": "X1", - "IOC": "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01", + "IOC": "../../../../bl01t-mo-motor-01", }, error="", ), ScreenNode( - file="https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01/pmacAxis.pvi.bob", + file="../bl01t-mo-motor-01/pmacAxis.pvi.bob", display_name="A", exists=True, - duplicate=False, children=[], macros={ "M": ":A", "P": "BL01T-MO-MOTOR-01", "label": "A", - "IOC": "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01", + "IOC": "../../../../bl01t-mo-motor-01", }, error="", ), @@ -306,7 +276,6 @@ def example_json_map_pvi_screens(): file="techui-support/bob/pmac/pmacController.bob", display_name="pmacController", exists=True, - duplicate=False, children=[], macros={"P": "BL01T-MO-BRICK-01"}, error="", @@ -532,7 +501,7 @@ def example_pgen_embedded_widget(): embedded_widget.macro("P", "BL01T-DI-IOC-01") embedded_widget.macro("R", ":CAM:") embedded_widget.macro("label", "CAM") - embedded_widget.macro("IOC", "test_url/bl01t-di-ioc-01") + embedded_widget.macro("IOC", "../../../../bl01t-di-ioc-01") return embedded_widget diff --git a/tests/jsonmap/test_crawl.py b/tests/jsonmap/test_crawl.py index 259ceea6..2b18a823 100644 --- a/tests/jsonmap/test_crawl.py +++ b/tests/jsonmap/test_crawl.py @@ -3,12 +3,11 @@ import pytest from techui_builder.jsonmap.crawl import CrawlContext, crawl, crawl_link -from techui_builder.jsonmap.fetch import ScreenFetcher from techui_builder.jsonmap.links import WidgetLink from techui_builder.jsonmap.nodes import ScreenNode from techui_builder.utils import WidgetType -MOTOR_IOC = "https://t01-opis.diamond.ac.uk/bl01t-mo-motor-01" +MOTOR_IOC = "../bl01t-mo-motor-01" @pytest.fixture @@ -17,7 +16,6 @@ def t01_ctx(json_map_generator) -> CrawlContext: return CrawlContext( components=json_map_generator.techui_yaml.components, synoptic_dir=synoptic, - fetcher=ScreenFetcher(), component_name=None, screen=synoptic / "index.bob", ) @@ -65,45 +63,6 @@ def test_crawl_link(t01_ctx): assert missing == ScreenNode("missing.bob", "Missing", exists=False) -# def test_crawl_link_remote_screen(t01_ctx): -# link = WidgetLink( -# f"{MOTOR_IOC}/index.bob", "Motors", WidgetType.ACTION_BUTTON, {"P": "BL01T"} -# ) -# ioc = crawl_link(link, "Motors", t01_ctx) -# missing = crawl_link( -# WidgetLink(f"{MOTOR_IOC}/missing.bob", "Missing", link.type, {}), -# "Missing", -# t01_ctx, -# ) - -# # The display name comes from the fetched screen, not the link -# assert (ioc.file, ioc.display_name, ioc.error) == ( -# link.file, -# "bl01t-mo-brick-01", -# "", -# ) -# # Sub-screen links are relative to the screen that contains them -# assert [(c.file, c.display_name, c.macros) for c in ioc.children] == [ -# ( -# f"{MOTOR_IOC}/ppmacController.pvi.bob", -# "ppmacController", -# {"P": "BL01T-MO-BRICK-01"}, -# ), -# ( -# f"{MOTOR_IOC}/pmacAxis.pvi.bob", -# "pmacAxis (BL01T-MO-MOTOR-01)", -# {"P": "BL01T-MO-MOTOR-01", "M": ":X"}, -# ), -# ( -# f"{MOTOR_IOC}/pmacAxis.pvi.bob", -# "pmacAxis (BL01T-MO-MOTOR-01)", -# {"P": "BL01T-MO-MOTOR-01", "M": ":A"}, -# ), -# ] -# assert (missing.exists, missing.children) == (False, []) -# assert missing.error.startswith("Could not fetch screen") - - def test_with_screen(t01_ctx): motor_ctx = t01_ctx.with_screen(Path("motor1.bob")) @@ -115,9 +74,3 @@ def test_with_screen(t01_ctx): assert t01_ctx.with_screen(Path("index.bob")).component_name is None # Already inside a component assert motor_ctx.with_screen(Path("dcam1.bob")).component_name == "motor1" - # A remote screen is never a component, but is still the screen being crawled - remote_ctx = t01_ctx.with_screen(f"{MOTOR_IOC}/motor1.bob") - assert (remote_ctx.screen, remote_ctx.component_name) == ( - f"{MOTOR_IOC}/motor1.bob", - None, - ) diff --git a/tests/jsonmap/test_links.py b/tests/jsonmap/test_links.py index 0890731f..cb827e59 100644 --- a/tests/jsonmap/test_links.py +++ b/tests/jsonmap/test_links.py @@ -64,34 +64,24 @@ def test_extract_links(): def test_resolve_link(): screen = Path("/services/synoptic/techui-support/bob/slits/slit.bob") - url = "https://t01-opis.diamond.ac.uk/bl01t-di-cam-01/ADUVC.pvi.bob" - macros = {"IOC": "https://t01-opis.diamond.ac.uk/bl01t-di-cam-01"} + macros = {"IOC": "../../../../bl01t-di-cam-01"} # Local files are relative to the screen containing the link assert resolve_link("../pmac/motor.bob", macros, screen) == Path( "/services/synoptic/techui-support/bob/slits/../pmac/motor.bob" ) # Macros are substituted, and unknown ones are left alone - assert resolve_link("$(IOC)/ADUVC.pvi.bob", macros, screen) == url - assert resolve_link("${IOC}/ADUVC.pvi.bob", macros, screen) == url + service_screen = screen.parent / "../../../../bl01t-di-cam-01/ADUVC.pvi.bob" + assert resolve_link("$(IOC)/ADUVC.pvi.bob", macros, screen) == service_screen + assert resolve_link("${IOC}/ADUVC.pvi.bob", macros, screen) == service_screen assert resolve_link("$(IOC)/x.bob", {}, screen) == screen.parent / "$(IOC)/x.bob" - # Files in remote screens are relative to the screen's URL - assert resolve_link("ADUVC_Advanced.pvi.bob", {}, url) == url.replace( - "ADUVC.pvi.bob", "ADUVC_Advanced.pvi.bob" - ) - assert resolve_link("http://other.invalid/x.bob", macros, url) == ( - "http://other.invalid/x.bob" - ) def test_is_bob(): assert is_bob("dcam1.bob") assert is_bob("$(IOC)/dcam1.bob") - assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob") - # A url may carry a query or fragment that is not part of the file name - assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob?v=2") - assert is_bob("https://opis.diamond.ac.uk/ioc/ADUVC.pvi.bob#Advanced") + assert is_bob("../bl01t-di-cam-01/ADUVC.pvi.bob") # Screens Phoebus can open but the builder does not crawl assert not is_bob("dcam1.opi") - assert not is_bob("https://opis.diamond.ac.uk/ioc/index.html") + assert not is_bob("index.html") assert not is_bob("") diff --git a/tests/jsonmap/test_nodes.py b/tests/jsonmap/test_nodes.py index b6560c7c..7cfd4b3e 100644 --- a/tests/jsonmap/test_nodes.py +++ b/tests/jsonmap/test_nodes.py @@ -22,7 +22,6 @@ def test_field_default(): "file": MISSING, "display_name": MISSING, "exists": True, - "duplicate": False, "children": [], "macros": {}, "error": "", diff --git a/tests/test_files/widget.xml b/tests/test_files/widget.xml index d0d7ac00..f91d87f9 100644 --- a/tests/test_files/widget.xml +++ b/tests/test_files/widget.xml @@ -10,6 +10,6 @@

BL01T-DI-IOC-01

:CAM: - test_url/bl01t-di-ioc-01 + ../../../../bl01t-di-ioc-01 diff --git a/tests/test_files/widget_url_screen.xml b/tests/test_files/widget_service_screen.xml similarity index 88% rename from tests/test_files/widget_url_screen.xml rename to tests/test_files/widget_service_screen.xml index 9def9f7d..7fecf82b 100644 --- a/tests/test_files/widget_url_screen.xml +++ b/tests/test_files/widget_service_screen.xml @@ -15,7 +15,7 @@ :CAM: - test_url/bl01t-di-ioc-01/ADUVC.pvi.bob + ../bl01t-di-ioc-01/ADUVC.pvi.bob tab diff --git a/tests/test_generate.py b/tests/test_generate.py index 6b497b9a..1556af1f 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -240,7 +240,7 @@ def test_generator_allocate_widget(generator, tmp_test_files): assert str(widget) == xml_content -def test_generator_allocate_widget_with_remote_screens(generator, tmp_test_files): +def test_generator_allocate_widget_with_service_screens(generator, tmp_test_files): generator._update_macros = Mock( return_value=("CAM", {"P": "BL01T-DI-IOC-01", "R": ":CAM:", "label": "CAM"}) ) @@ -256,7 +256,7 @@ def test_generator_allocate_widget_with_remote_screens(generator, tmp_test_files macros={"P": "BL01T-DI-IOC-01", "R": ":CAM:"}, ) widget = generator._allocate_widget(scrn_mapping, component) - control_widget = tmp_test_files / "widget_url_screen.xml" + control_widget = tmp_test_files / "widget_service_screen.xml" with open(control_widget) as f: xml_content = f.read()