From 1f23f003acfec3496a750ebf006cabb24a42d780 Mon Sep 17 00:00:00 2001 From: Conrad Date: Fri, 9 Oct 2026 11:50:22 -0400 Subject: [PATCH 1/5] build: Bump clodius pin for remote-source bbi tilesets, add fsspec clodius#19 adds a path-or-factory Source abstraction to clodius.tiles_v2.bbi, letting a bigInteract tileset read its upstream URL directly over HTTP Range requests instead of requiring a local file. fsspec is what builds that factory on cfdb's side. --- pyproject.toml | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 329cd03..9ef7e15 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,11 @@ dependencies = [ "click", "debugpy", "fastapi", + # bigInteract tileset serving (issue #120) reads its source directly + # over HTTPS Range requests rather than a local copy — `aiohttp` + # (already a dependency above) is what fsspec's `http`/`https` + # filesystem implementation uses. + "fsspec", "grpcio>=1.81.1", "motor", "pydantic>=2,<3", @@ -58,12 +63,17 @@ dev = ["allpairspy", "cloudpickle", "cryptography", "debugpy", "httpx", "hypothe [tool.uv.sources] # Pinned to a sha, not a branch: clodius considers ``tiles_v2`` unstable # and cfdb is its first consumer, so what runs must be exactly what was -# validated. This sha is the tip of the fork's ``main`` — the -# 6-restore-dropped-guards branch (restored ``tiles_v2`` validation -# guards, see ``cfdb.tilesets.backend.ClodiusBackend.tile_error``, now -# bound to ``TilesetError``, and the tile-omission contract change -# documented on ``cfdb.tilesets.service.TilesetService._read_tiles``) has -# been merged there. Stay pinned to a commit sha rather than a branch +# validated. This sha is the tip of the fork's ``main`` as of clodius#19 +# ("Accept a file-like source in the bbi tilesets", issue #120's +# dependency), which lands a shared ``Source`` abstraction (path, or a +# zero-argument factory returning a fresh seekable binary handle) in +# ``clodius/tiles_v2/bbi.py`` — what lets cfdb serve a bigInteract tile +# straight from its upstream URL instead of a local copy. The same +# abstraction landed for the HDF5-backed tilesets (``cooler``, +# ``multivec``) in the prior clodius#18; cfdb does not use that side of +# it yet (``_open_cooler`` still hydrates to local disk — see +# ``cfdb.tilesets.service``), but the pin brings both along, since they +# share one commit. Stay pinned to a commit sha rather than a branch # reference regardless — a branch reference floats, and what runs must # be exactly what was validated. # @@ -74,7 +84,7 @@ dev = ["allpairspy", "cloudpickle", "cryptography", "debugpy", "httpx", "hypothe # only, which does not fit this corpus at any disk budget. Serving # ``.hic`` at all is tracked as separate follow-up work, not something # this pin is expected to grow. -clodius = { git = "https://github.com/abdenlab/clodius", rev = "07f043bea3380bbc38e76f0970d93abed0be4f69" } +clodius = { git = "https://github.com/abdenlab/clodius", rev = "610777e89aad82dceaffcbf49ed2b5df1861f6b2" } [project.scripts] cfdb = "cfdb.cli:cli" From ee1d57dd40a7075c63f6da5140c72acb12d0077d Mon Sep 17 00:00:00 2001 From: Conrad Date: Fri, 9 Oct 2026 11:50:36 -0400 Subject: [PATCH 2/5] fix: Update tileset-lifetime tests for clodius's self-releasing construction The clodius bump changes CoolerTileset's close semantics: construction now reads the file's chromsizes and releases the handle immediately, reopening lazily on first real use (clodius#14/#18's "registration is free" property), rather than holding it open until an explicit close. TestOpenTilesetLifetime's exact close() call-count assertions predated that behavior and failed against the new pin. --- tests/test_tilesets/test_service.py | 50 ++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/tests/test_tilesets/test_service.py b/tests/test_tilesets/test_service.py index 5dacc31..6bdcf00 100644 --- a/tests/test_tilesets/test_service.py +++ b/tests/test_tilesets/test_service.py @@ -931,14 +931,23 @@ async def test_should_close_a_handle_evicted_past_the_open_cap( async with service.checkout(UID, first_doc) as first: assert first.info() is not None # force the handle open - close_spy.assert_not_called() + # Constructing `first` already released its own validation-time + # handle — clodius's construction reads the file's chromsizes + # then releases, reopening lazily on the first real call + # (clodius#14/#18's "registration is free" property) — not the + # eviction this test is about. + close_spy.assert_called_once() + assert close_spy.call_args.args[0] is first + close_spy.reset_mock() # Act await service.tileset_info("4dn/4DNFIXYZ789", second_doc) - # Assert - close_spy.assert_called_once() - assert close_spy.call_args.args[0] is first + # Assert — `second`'s own construction releases itself the same + # way; the eviction this test targets is the call right after it, + # against `first`. + assert close_spy.call_count == 2 + assert close_spy.call_args_list[-1].args[0] is first @pytest.mark.asyncio async def test_should_defer_closing_a_handle_that_is_being_read( @@ -974,11 +983,24 @@ async def test_should_defer_closing_a_handle_that_is_being_read( # Act & Assert async with service.checkout(UID, first_doc) as held: assert held.info() is not None # force the handle open + # Constructing `held` already released its own + # validation-time handle (clodius#14/#18's "registration is + # free" property) — not the eviction this test is about. + close_spy.assert_called_once() + close_spy.reset_mock() + await service.tileset_info("4dn/4DNFIXYZ789", second_doc) - close_spy.assert_not_called() + # `second`'s own construction releases itself the same way. + # The real eviction must still defer, because `held` is + # still checked out — it must not be among these calls. + close_spy.assert_called_once() + assert close_spy.call_args.args[0] is not held + close_spy.reset_mock() + assert held.info() is not None # still serving the reader + close_spy.assert_not_called() - # Assert — the last reader out performs the close + # Assert — the last reader out performs the (deferred) close close_spy.assert_called_once() assert close_spy.call_args.args[0] is held @@ -1022,12 +1044,18 @@ async def grab(): remote.gate.set() await asyncio.gather(first, second) - # Assert + # Assert — the winner is closed exactly once: its own + # construction-time self-release (clodius#14/#18's + # "registration is free" property), never a second time while + # it's the shared, still-open handle. The loser is closed twice + # (that same self-release, plus the explicit dedup cleanup) — + # this test only cares that those never land on the winner. assert seen[0] is seen[1] - assert close_spy.call_count <= 1 - assert all( - call.args[0] is not seen[0] for call in close_spy.call_args_list - ) + winner = seen[0] + winner_closes = [ + call for call in close_spy.call_args_list if call.args[0] is winner + ] + assert len(winner_closes) == 1 @pytest.mark.asyncio async def test_should_survive_aclose_while_a_reader_is_inside_checkout( From 93651cfbb0dd456bc527249eb251bc6eb5d49619 Mon Sep 17 00:00:00 2001 From: Conrad Date: Fri, 9 Oct 2026 11:51:03 -0400 Subject: [PATCH 3/5] feat: Serve bigInteract tilesets directly from remote storage Add /tileset_info and /tiles support for bigInteract files, closing the gap left when #94 moved them off the bigBed/index pipeline. Unlike a contact map, a bigInteract tileset is never materialized: _open_bbi_interaction resolves the file's upstream URL (reusing the same DRS/allowlist resolution /data runs, now public as resolve_download_url) and hands clodius an fsspec-backed factory, so the tile reads happen straight against the source over HTTP Range requests. Two uids per file select clodius's two presentations of the same data: the default uid opens the 2D rectangle-domain tileset, and a ":links" suffix opens the 1D arc/link tileset. The suffix is stripped before the Mongo lookup and never reaches clodius's own tile-id parsing. A new TilesetSourceUnavailable error carries the specific HTTP status an upstream resolution failure maps to on /data (404/403/504/502/400), so a solo request surfaces the same status /data would for the same underlying cause, while a batch entry still degrades to an inline error object like every other per-dataset tileset failure. --- src/cfdb/api/routers/tiles.py | 28 +++++-- src/cfdb/tilesets/backend.py | 51 ++++++++++++ src/cfdb/tilesets/errors.py | 26 ++++++ src/cfdb/tilesets/formats.py | 41 ++++++++++ src/cfdb/tilesets/service.py | 144 +++++++++++++++++++++++++++++----- src/cfdb/workflows/fetcher.py | 11 ++- 6 files changed, 272 insertions(+), 29 deletions(-) diff --git a/src/cfdb/api/routers/tiles.py b/src/cfdb/api/routers/tiles.py index c4be393..4f5f6ae 100644 --- a/src/cfdb/api/routers/tiles.py +++ b/src/cfdb/api/routers/tiles.py @@ -40,6 +40,7 @@ TilesetHydrationTimeout, TilesetTooLarge, ) +from cfdb.tilesets.formats import split_bbi_presentation from cfdb.tilesets.wire import error_payload logger = logging.getLogger(__name__) @@ -92,6 +93,20 @@ def _split_uid(uid: str) -> tuple[str, str]: return dcc, local_id +def _dcc_and_local_id(uid: str) -> tuple[str, str]: + """Split a uid for the file-document lookup, ignoring any presentation. + + A bigInteract uid may carry a ``:links`` suffix (see + ``cfdb.tilesets.formats.split_bbi_presentation``) that selects which + tileset class serves it — that suffix is meaningless to the Mongo + lookup, which is keyed on ``(dcc, local_id)`` alone, so it is + stripped here. ``uid`` itself (suffix intact) is still what's passed + to ``TilesetService``, which re-derives the presentation on its own. + """ + base_uid, _ = split_bbi_presentation(uid) + return _split_uid(base_uid) + + def _require_service(): """Return the tile service, or explain why there isn't one. @@ -171,9 +186,12 @@ def _render_dataset_failure( """ _reraise_infrastructure(exc) if solo: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=str(exc) - ) + # TilesetSourceUnavailable carries the specific status /data + # already maps the same upstream failure to (404/403/504/502); + # every other per-dataset TilesetError has no such attribute and + # keeps the plain 404. + status_code = getattr(exc, "status_code", status.HTTP_404_NOT_FOUND) + raise HTTPException(status_code=status_code, detail=str(exc)) logger.info(f"Tileset {uid} unavailable: {exc}") return error_payload(str(exc)) @@ -196,7 +214,7 @@ async def tileset_info( response: dict[str, Any] = {} for uid in uids: - dcc, local_id = _split_uid(uid) + dcc, local_id = _dcc_and_local_id(uid) _, file_doc = await resolve_file_doc(dcc, local_id) try: response[uid] = await service.tileset_info(uid, file_doc) @@ -235,7 +253,7 @@ async def tiles( response: dict[str, Any] = {} for uid, uid_tile_ids in grouped.items(): - dcc, local_id = _split_uid(uid) + dcc, local_id = _dcc_and_local_id(uid) _, file_doc = await resolve_file_doc(dcc, local_id) try: response.update(await service.tiles(uid, file_doc, uid_tile_ids)) diff --git a/src/cfdb/tilesets/backend.py b/src/cfdb/tilesets/backend.py index 5f35cab..07cf41c 100644 --- a/src/cfdb/tilesets/backend.py +++ b/src/cfdb/tilesets/backend.py @@ -119,6 +119,57 @@ def load_hic_tileset() -> type: return HicTileset +def load_bbi_interaction_tilesets() -> tuple[type, type, type, type[Exception]]: + """Resolve the bigInteract tileset classes, ``LinkPolicy``, and the + pybigtools read-error type. + + Kept separate from :class:`ClodiusBackend`, same reasoning as + :func:`load_hic_tileset`: this is a second, independently-gated + capability rather than a core field every caller needs. Unlike + ``.hic``, there is no permanent refusal behind this gate — pybigtools + (the reader these classes use) has native remote-HTTP support, so a + clodius new enough to carry them is enough to serve bigInteract tiles + straight from the upstream URL. See + :meth:`~cfdb.tilesets.service.TilesetService._open_bbi_interaction`. + + ``clodius.tiles_v2.bbi`` also carries a third interaction tileset, + ``BBIInteractionTileset`` ("paired intervals") — the base both classes + below subclass. It is deliberately not resolved here: clodius's own + docstring for it says to prefer the two concrete subclasses, which is + what every caller in this codebase does. + + ``pybigtools.BBIReadError`` is resolved here rather than imported + directly in ``service.py``, for the same reason the tileset classes + are: ``pybigtools`` is pulled in transitively through clodius, not a + direct cfdb dependency, so every name from it goes through this one + gate. + + Raises: + TileBackendUnavailable: when the installed clodius predates + clodius#15/#19 and carries no ``clodius.tiles_v2.bbi`` + interaction tileset classes. + """ + try: + import pybigtools + from clodius.core.policies import LinkPolicy + from clodius.tiles_v2.bbi import ( + BBIInteraction2DTileset, + BBIInteractionLinksTileset, + ) + except ImportError as exc: + raise TileBackendUnavailable( + "Serving bigInteract tiles requires clodius.tiles_v2.bbi's " + "interaction tileset classes, which the installed clodius " + "does not provide." + ) from exc + return ( + BBIInteraction2DTileset, + BBIInteractionLinksTileset, + LinkPolicy, + pybigtools.BBIReadError, + ) + + def is_tile_error(backend: ClodiusBackend, exc: BaseException) -> bool: """True when ``exc`` is a clodius per-tile error rather than a bug. diff --git a/src/cfdb/tilesets/errors.py b/src/cfdb/tilesets/errors.py index fab4a56..1362004 100644 --- a/src/cfdb/tilesets/errors.py +++ b/src/cfdb/tilesets/errors.py @@ -65,3 +65,29 @@ class TilesetHydrationTimeout(TilesetError): Rendered as 503 with ``Retry-After``: the download may well finish, and a retry will find it. """ + + +class TilesetSourceUnavailable(TilesetError): + """Resolving or reading a remote-source tileset's upstream URL failed. + + Only raised by formats served without local materialization (today, + bigInteract — see + :meth:`~cfdb.tilesets.service.TilesetService._open_bbi_interaction`). + Distinct from :class:`TilesetNotReady`: there is no "not yet built" + state here, no preparation channel to ask — the failure is in + reaching the DCC's own upstream file. ``status_code`` is not one + fixed value, and its two sources differ in how closely they track + ``/data`` (``cfdb.api.routers.data``): a DRS-resolution failure + (object not found, access denied, upstream timeout or error) carries + the same 404/403/504/502 that failure maps to on ``/data``, while a + malformed or disallowed URL (the SSRF allowlist, a malformed DRS + URI, or no usable access method) carries 400 as cfdb's own choice + for the tileset path specifically — ``/data`` has no 400 case for + any of these, so that one status is not a claim of parity. A batch + entry still renders as a plain ``{"error": ...}`` object regardless + of which, like every other per-dataset ``TilesetError``. + """ + + def __init__(self, message: str, *, status_code: int = 502) -> None: + super().__init__(message) + self.status_code = status_code diff --git a/src/cfdb/tilesets/formats.py b/src/cfdb/tilesets/formats.py index f89b579..6221a74 100644 --- a/src/cfdb/tilesets/formats.py +++ b/src/cfdb/tilesets/formats.py @@ -94,3 +94,44 @@ def matrix_source_kind(file_meta: dict[str, Any]) -> MatrixSource | None: def needs_materialization(file_meta: dict[str, Any]) -> bool: """True when this file must be preprocessed before it can be tiled.""" return matrix_source_kind(file_meta) in MATERIALIZED_SOURCES + + +#: ``file_format.name`` for a bigInteract file — minted rather than an EDAM +#: term (``services/ontology_mappings.py``), specifically so it is distinct +#: from plain ``bigBed`` and from this module's own ``HDF5_FORMAT_NAME``. +#: Unlike :class:`MatrixSource`, a bigInteract file needs no filename-suffix +#: dance and no materialization: the name alone identifies it, and it is +#: read in place from its upstream URL — see +#: :meth:`~cfdb.tilesets.service.TilesetService._open_bbi_interaction`. +BBI_INTERACTION_FORMAT_NAME = "bigInteract" + + +def is_bbi_interaction_source(file_meta: dict[str, Any]) -> bool: + """True when ``file_meta`` is a bigInteract file.""" + file_format = file_meta.get("file_format") + return ( + isinstance(file_format, dict) + and file_format.get("name") == BBI_INTERACTION_FORMAT_NAME + ) + + +#: Suffix on a tileset uid that selects the 1D arc/link presentation of a +#: bigInteract file rather than its default 2D rectangle-domain one. ``:`` +#: rather than ``.`` deliberately: a uid half must stay free of ``.`` +#: (clodius parses a tile id by splitting on it), and this never reaches +#: clodius at all — ``split_bbi_presentation`` strips it before anything +#: downstream sees the uid. +LINKS_PRESENTATION_SUFFIX = ":links" + + +def split_bbi_presentation(uid: str) -> tuple[str, str]: + """Split a bigInteract uid into its base uid and presentation mode. + + Returns: + ``(base_uid, presentation)``, where ``presentation`` is + ``"links"`` when ``uid`` ends with :data:`LINKS_PRESENTATION_SUFFIX` + and ``"rectangles"`` (the default) otherwise. + """ + if uid.endswith(LINKS_PRESENTATION_SUFFIX): + return uid[: -len(LINKS_PRESENTATION_SUFFIX)], "links" + return uid, "rectangles" diff --git a/src/cfdb/tilesets/service.py b/src/cfdb/tilesets/service.py index 1a111e2..cbbfc01 100644 --- a/src/cfdb/tilesets/service.py +++ b/src/cfdb/tilesets/service.py @@ -29,20 +29,35 @@ from pathlib import Path from typing import Any -from cfdb.tilesets.backend import load_backend, load_hic_tileset +import fsspec + +from cfdb.services import drs +from cfdb.tilesets.backend import ( + load_backend, + load_bbi_interaction_tilesets, + load_hic_tileset, +) from cfdb.tilesets.errors import ( TileBackendUnavailable, TilesetHydrationTimeout, TilesetIdentityIncomplete, TilesetNotReady, + TilesetSourceUnavailable, TilesetUnsupported, ) -from cfdb.tilesets.formats import MatrixSource, matrix_source_kind +from cfdb.tilesets.formats import ( + MatrixSource, + is_bbi_interaction_source, + matrix_source_kind, + split_bbi_presentation, +) from cfdb.tilesets.store import LocalTilesetStore from cfdb.tilesets.wire import error_payload, tileset_info_payload from cfdb.workflows.cache import CacheBackend +from cfdb.workflows.fetcher import resolve_download_url from cfdb.workflows.models import ArtifactKind from cfdb.workflows.processors.matrix import MatrixTilesetProcessor +from cfdb.workflows.urlsafe import UnsafeOutboundURL logger = logging.getLogger(__name__) @@ -87,9 +102,13 @@ def __init__( self._open: OrderedDict[str, _OpenTileset] = OrderedDict() self._open_lock = asyncio.Lock() #: ``(cache_key, tile_id.raw)`` to payload, least-recently-used - #: first. Keyed on the cache key rather than the uid because the - #: key embeds ``md5-v{processor_version}``, so an upstream byte - #: change or a processor bump invalidates every tile for free. + #: first. Keyed on the cache key rather than the uid because for + #: an mcool/cool/hic the key embeds ``md5-v{processor_version}``, + #: so an upstream byte change or a processor bump invalidates + #: every tile for free. A bigInteract's key has no such identity + #: (see ``_cache_key``) — its tiles are only invalidated by the + #: ordinary LRU/byte-budget eviction, same as an open handle + #: outliving an upstream change for any remote-read source. self._tiles: OrderedDict[tuple[str, str], dict[str, Any]] = OrderedDict() self._tile_bytes = 0 store.set_release_hook(self._release_for_eviction) @@ -115,7 +134,7 @@ async def tiles( position's slot rather than raising or being omitted — one bad id in a batch must not blank the whole track. """ - cache_key = self._cache_key(file_doc) + cache_key = self._cache_key(uid, file_doc) payloads: dict[str, dict[str, Any]] = {} pending: list[str] = [] @@ -180,7 +199,7 @@ async def aclose(self) -> None: async def _acquire(self, uid: str, file_doc: dict[str, Any]) -> _OpenTileset: """Return an open tileset for ``uid``, opening one if needed.""" - cache_key = self._cache_key(file_doc) + cache_key = self._cache_key(uid, file_doc) async with self._open_lock: entry = self._open.get(cache_key) @@ -231,11 +250,13 @@ async def _open_tileset( ) -> Any: """Construct a tileset for ``file_doc``, without dispatching anything.""" kind = matrix_source_kind(file_doc) - if kind is None: - raise TilesetUnsupported(f"{uid} is not a contact map") - if kind is MatrixSource.HIC: - return await self._open_hic(uid, file_doc) - return await self._open_cooler(uid, cache_key) + if kind is not None: + if kind is MatrixSource.HIC: + return await self._open_hic(uid, file_doc) + return await self._open_cooler(uid, cache_key) + if is_bbi_interaction_source(file_doc): + return await self._open_bbi_interaction(uid, file_doc) + raise TilesetUnsupported(f"{uid} is not a tileable source") async def _open_cooler(self, uid: str, cache_key: str) -> Any: """Open the cached mcool artifact for ``uid``.""" @@ -300,6 +321,71 @@ async def _open_hic(self, uid: str, file_doc: dict[str, Any]) -> Any: "4DN .mcool contact maps are unaffected." ) + async def _open_bbi_interaction(self, uid: str, file_doc: dict[str, Any]) -> Any: + """Open a bigInteract tileset directly from its upstream URL. + + Unlike :meth:`_open_cooler`, there is no cache artifact and no + ``LocalTilesetStore`` interaction at all here — pybigtools (the + reader clodius's bbi tilesets use) has native remote-HTTP + support, so the tileset reads the upstream URL itself over HTTP + Range requests via an ``fsspec``-backed factory, with no local + copy ever made. + """ + base_uid, presentation = split_bbi_presentation(uid) + rect_cls, links_cls, link_policy, bbi_read_error = ( + load_bbi_interaction_tilesets() + ) + + access_url = file_doc.get("access_url") + if not access_url: + raise TilesetUnsupported(f"{base_uid} has no access_url") + + # The 404/403/504/502 branches below genuinely mirror what the + # same DRS failure maps to on /data. The 400 branches do not — + # /data has no 400 case for either UnsafeOutboundURL or a bare + # ValueError from the DRS layer; 400 is cfdb's own choice for a + # malformed or disallowed URL on this path specifically. + try: + resolved_url = await resolve_download_url(access_url) + except UnsafeOutboundURL as exc: + raise TilesetSourceUnavailable(str(exc), status_code=400) from exc + except drs.DRSNotFound as exc: + raise TilesetSourceUnavailable(str(exc), status_code=404) from exc + except drs.DRSForbidden as exc: + raise TilesetSourceUnavailable(str(exc), status_code=403) from exc + except drs.DRSTimeout as exc: + raise TilesetSourceUnavailable(str(exc), status_code=504) from exc + except (drs.DRSRedirectBlocked, drs.DRSUpstreamError, drs.DRSError) as exc: + raise TilesetSourceUnavailable(str(exc), status_code=502) from exc + except ValueError as exc: + # parse_drs_uri (malformed drs:// URI) and + # get_https_download_url (no usable access method, e.g. a + # Globus-only object) both raise a bare ValueError that + # isn't UnsafeOutboundURL — latent today, since bigInteract + # access_urls are always plain https:// in practice, but not + # guarded against by anything in the data model. + raise TilesetSourceUnavailable(str(exc), status_code=400) from exc + + def factory(): + return fsspec.open(resolved_url, "rb").open() + + # Resolving the URL is not the same as reading it: constructing + # the tileset is what performs the actual network request + # (clodius's "registration is free" construction reads the file + # header immediately). A real fetch failure here raises a raw + # fsspec/pybigtools exception, not a TilesetError, so it must be + # translated the same way the resolution step above is — + # otherwise it escapes the router's per-dataset handling and + # 500s the whole batch rather than failing just this uid. + try: + if presentation == "links": + return await self._run( + lambda: links_cls(factory, link_policy=link_policy.EITHER) + ) + return await self._run(lambda: rect_cls(factory)) + except (OSError, bbi_read_error) as exc: + raise TilesetSourceUnavailable(str(exc), status_code=502) from exc + async def _close(self, entry: _OpenTileset) -> None: """Close a handle off the event loop.""" try: @@ -401,13 +487,20 @@ def tile_cache_bytes_used(self) -> int: # --- plumbing ----------------------------------------------------------- - def _cache_key(self, file_doc: dict[str, Any]) -> str: - """The TILESET cache key for ``file_doc``. - - Derived through the processor rather than re-implemented, so the - writer, the readiness probe, and this reader agree by construction - rather than by three formulas kept in sync. + def _cache_key(self, uid: str, file_doc: dict[str, Any]) -> str: + """The open-handle-pool key for ``uid``. + + For an mcool/cool/hic, derived through the processor rather than + re-implemented, so the writer, the readiness probe, and this + reader agree by construction rather than by three formulas kept + in sync. A bigInteract file has no such artifact at all — nothing + is cached, there is no processor-derived identity to borrow — so + its key is just the uid itself (already unique per presentation, + since the rectangles/links uids differ by their ``:links`` + suffix); it needs no md5, unlike the processor-derived path. """ + if is_bbi_interaction_source(file_doc): + return f"bbi:{uid}" try: return self._processor.cache_key_for(file_doc, ArtifactKind.TILESET) except ValueError as exc: @@ -418,12 +511,21 @@ async def _run(self, fn): return await asyncio.get_running_loop().run_in_executor(self._pool, fn) -def _payload_bytes(payload: dict[str, Any]) -> int: +def _payload_bytes(payload: Any) -> int: """Approximate on-heap size of a tile payload. - The base64 ``dense`` string dominates by orders of magnitude; the - handful of floats beside it are not worth measuring. + A matrix tile (``dict``, cooler/hic) is dominated by its base64 + ``dense`` string by orders of magnitude; the handful of floats + beside it are not worth measuring. A bigInteract tile has no such + field at all — clodius's ``BBIInteraction*Tileset`` returns a + ``list`` of record dicts instead of a ``dense`` matrix block — so + it is charged by its rendered content length instead. Either shape + may also be a per-tile error ``dict`` (``{"error": ..., "error_type": + ...}``), which falls through the ``dense`` lookup to 0, same as + today. """ + if isinstance(payload, list): + return sum(len(str(record)) for record in payload) dense = payload.get("dense") return len(dense) if isinstance(dense, (str, bytes)) else 0 diff --git a/src/cfdb/workflows/fetcher.py b/src/cfdb/workflows/fetcher.py index 831ed95..e347c94 100644 --- a/src/cfdb/workflows/fetcher.py +++ b/src/cfdb/workflows/fetcher.py @@ -39,7 +39,7 @@ async def download_source(file_meta: dict[str, Any], dest: Path) -> Path: raise ValueError("file_meta has no access_url — cannot fetch source") dest.parent.mkdir(parents=True, exist_ok=True) - download_url = await _resolve_download_url(access_url) + download_url = await resolve_download_url(access_url) part = dest.with_suffix(dest.suffix + ".part") bytes_written = 0 @@ -90,7 +90,7 @@ async def peek_decompressed_prefix( if not access_url: raise ValueError("file_meta has no access_url — cannot peek source") - download_url = await _resolve_download_url(access_url) + download_url = await resolve_download_url(access_url) range_header = f"bytes=0-{max_compressed_bytes - 1}" decompressor = None @@ -117,7 +117,7 @@ async def peek_decompressed_prefix( return bytes(out) -async def _resolve_download_url(access_url: str) -> str: +async def resolve_download_url(access_url: str) -> str: """Return a direct HTTPS URL for ``access_url``. Direct ``https://`` URLs pass through. ``drs://`` URIs are resolved @@ -125,6 +125,11 @@ async def _resolve_download_url(access_url: str) -> str: URI and the resolved HTTPS URL go through ``validate_outbound_url`` so a poisoned ``access_url`` or a DRS object pointing at an internal host gets rejected before any worker-side fetch. + + Public (not module-private) because ``cfdb.tilesets.service`` calls + it too, to resolve a bigInteract file's upstream URL for remote tile + reads — the same resolution step ``download_source`` runs before + writing bytes to disk, just without the download. """ validate_outbound_url(access_url) if access_url.startswith("drs://"): From fb313f1cc72270eb1ed32d1ec5eeb268ed411585 Mon Sep 17 00:00:00 2001 From: Conrad Date: Fri, 9 Oct 2026 11:51:33 -0400 Subject: [PATCH 4/5] test: Cover bigInteract tileset serving end to end Add a real bed5+13 fixture (via pybigtools's writer, since its own Python wrapper only ever returns a read-only reader) and a local HTTP server fixture, so the new code path is exercised against real bytes over real Range requests rather than mocks standing in for clodius. Covers both presentations end to end through the service and the router, the DRS-failure-to-TilesetSourceUnavailable status mapping, independent cache keys per presentation, the format classifier and uid-presentation split in isolation, and the error type's own constructor contract. --- tests/conftest.py | 52 +++ tests/fixtures/biginteract.py | 112 ++++++ tests/test_tiles.py | 167 +++++++++ tests/test_tilesets/test_backend.py | 58 +++ tests/test_tilesets/test_errors.py | 43 ++- tests/test_tilesets/test_formats.py | 225 ++++++++++++ tests/test_tilesets/test_service.py | 551 ++++++++++++++++++++++++++++ 7 files changed, 1207 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/biginteract.py diff --git a/tests/conftest.py b/tests/conftest.py index bdc7874..690a8bd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -499,3 +499,55 @@ def _make(**kwargs): return build_cool(tmp_path / name, **kwargs) return _make + + +# --- bigInteract fixtures (issue #120) -------------------------------------- +# +# bigInteract tiles are read directly from the file's upstream URL rather +# than a local cache artifact (see ``TilesetService._open_bbi_interaction``), +# so exercising the real read path needs a real file served over real HTTP +# — a mocked byte stream would not prove the fsspec Range-read factory +# actually works. Session-scoped: the file and the server are read-only and +# every consuming test only reads from them. + + +@pytest.fixture(scope="session") +def tiny_biginteract(tmp_path_factory): + """A real bed5+13 bigInteract file with a handful of interactions.""" + from tests.fixtures.biginteract import build_biginteract + + return build_biginteract(tmp_path_factory.mktemp("biginteract") / "tiny.bb") + + +@pytest.fixture(scope="session") +def biginteract_server(tiny_biginteract): + """Serve ``tiny_biginteract``'s directory over real HTTP on 127.0.0.1. + + Mirrors ``tests/integration/conftest.py``'s ``sample_server`` fixture, + at a much smaller scale — this one file, no wool worker, no sample + generation. Yields the file's full URL directly rather than a base + URL, since every consumer wants exactly this one file. + """ + import http.server + import socketserver + import threading + + directory = str(tiny_biginteract.parent) + filename = tiny_biginteract.name + + class _Handler(http.server.SimpleHTTPRequestHandler): + def __init__(self, *args, **kwargs): + super().__init__(*args, directory=directory, **kwargs) + + def log_message(self, *_args, **_kwargs) -> None: # pragma: no cover + return + + server = socketserver.ThreadingTCPServer(("127.0.0.1", 0), _Handler) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{port}/{filename}" + finally: + server.shutdown() + server.server_close() diff --git a/tests/fixtures/biginteract.py b/tests/fixtures/biginteract.py new file mode 100644 index 0000000..95f07a6 --- /dev/null +++ b/tests/fixtures/biginteract.py @@ -0,0 +1,112 @@ +"""Synthetic bigInteract (bed5+13) builder for the bigInteract tile tests. + +Plain function, not a pytest fixture, matching ``coolers.py``'s convention. + +``pybigtools``'s own Python wrapper (``pybigtools.open``) unconditionally +returns a read-only ``BBIReader``, regardless of the ``mode`` argument — it +drops write-mode support rather than dispatching to ``BigBedWriter``. The +writer is only reachable through the compiled extension module directly, +``pybigtools.pybigtools.open(path, "w")``; that is what this module does. +``BigBedWriter`` itself cannot be constructed via its class (pybigtools +raises "No constructor defined" — it has no public ``__new__``), so there +is no more direct route. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import NamedTuple + +import pytest + +#: Standard UCSC ``bigInteract`` AutoSql schema (bed5+13). Declared +#: explicitly, rather than left to ``autosql=None``, so the extra fields +#: are typed the way a real UCSC-produced file's are, not generic strings. +INTERACT_AUTOSQL = """table interact +"Interaction between two regions" + ( + string chrom; "Chromosome (or contig, scaffold, etc.)" + uint chromStart; "Start position of lower region" + uint chromEnd; "End position of upper region" + string name; "Name of item, for display" + uint score; "Score (0-1000)" + double value; "Strength of interaction" + string exp; "Experiment name" + string color; "Item color" + string sourceChrom; "Chromosome of source region" + uint sourceStart; "Start position of source region" + uint sourceEnd; "End position of source region" + string sourceName; "Identifier of source region" + string sourceStrand; "Orientation of source region" + string targetChrom; "Chromosome of target region" + uint targetStart; "Start position of target region" + uint targetEnd; "End position of target region" + string targetName; "Identifier of target region" + string targetStrand; "Orientation of target region" + ) +""" + +#: Tiny single-chromosome genome, matching the scale of ``coolers.py``'s +#: fixtures — just large enough to exercise real tile geometry. +INTERACT_CHROMSIZES: dict[str, int] = {"chr1": 10_000} + + +class InteractionRecord(NamedTuple): + """One bigInteract record — the fields a test wants to assert against.""" + + source_start: int + source_end: int + target_start: int + target_end: int + value: float + + +#: Deterministic records spread across the genome, far enough apart that +#: they land in different tiles at low zoom and the same tile at high zoom. +CANONICAL_RECORDS: list[InteractionRecord] = [ + InteractionRecord( + 100 + i * 500, 200 + i * 500, 3000 + i * 500, 3100 + i * 500, 1.5 + i + ) + for i in range(5) +] + + +def build_biginteract( + path: Path, + records: list[InteractionRecord] = CANONICAL_RECORDS, + chromsizes: dict[str, int] = INTERACT_CHROMSIZES, + chrom: str = "chr1", +) -> Path: + """Write a real bed5+13 bigInteract file at ``path``.""" + from pybigtools import pybigtools as bt + + pytest.importorskip("pybigtools") + + vals = [] + for i, rec in enumerate(records): + lo = min(rec.source_start, rec.target_start) + hi = max(rec.source_end, rec.target_end) + rest = "\t".join( + [ + f"rec{i}", + "500", + str(rec.value), + "exp1", + "0,0,0", + chrom, + str(rec.source_start), + str(rec.source_end), + f"src{i}", + ".", + chrom, + str(rec.target_start), + str(rec.target_end), + f"tgt{i}", + ".", + ] + ) + vals.append((chrom, lo, hi, rest)) + + writer = bt.open(str(path), "w") + writer.write(dict(chromsizes), vals, autosql=INTERACT_AUTOSQL) + return path diff --git a/tests/test_tiles.py b/tests/test_tiles.py index 2e2560f..cd508e5 100644 --- a/tests/test_tiles.py +++ b/tests/test_tiles.py @@ -1050,3 +1050,170 @@ async def test_should_503_when_the_workflow_subsystem_is_disabled( await tileset_info(d=[UID]) assert exc_info.value.status_code == 503 assert exc_info.value.headers["Retry-After"] == "30" + + +def _biginteract_doc(access_url: str, **overrides) -> dict: + """Return a projected document for an ENCODE bigInteract file.""" + doc = { + "submission": "encode", + "local_id": "ENCFF000BIG", + "md5": FIXTURE_MD5, + "filename": "sample.bigInteract", + "file_format": {"name": "bigInteract"}, + "access_url": access_url, + "dcc": {"dcc_abbreviation": "ENCODE"}, + } + doc.update(overrides) + return doc + + +class TestBbiInteraction: + """``/tileset_info`` and ``/tiles`` for bigInteract files. + + ``cfdb.tilesets.service.resolve_download_url`` is mocked for the same + reason ``tests/test_tilesets/test_service.py``'s equivalent class + mocks it — the SSRF allowlist's loopback escape hatch is read into a + module constant at import time, already evaluated before this test + module loads. Everything downstream of that one hop — real service + dispatch, a real ``fsspec`` factory, real clodius reads — runs for + real against ``biginteract_server``. + """ + + BIGINTERACT_UID = "encode/ENCFF000BIG" + + @pytest.mark.asyncio + async def test_should_serve_both_presentations_from_one_uid( + self, tile_env, biginteract_server, mocker + ): + """Test that the rectangle and links uids resolve to the same file. + + Given: + One bigInteract document, requested under its default uid and + its ``:links`` uid. + When: + tileset_info is requested for both. + Then: + Each should report the datatype for its own presentation, and + both should be keyed by the exact uid that was requested. + """ + # Arrange + _, mock_db, _ = tile_env + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + doc = _biginteract_doc(biginteract_server) + mock_db.file.docs = [doc] + links_uid = f"{self.BIGINTERACT_UID}:links" + + # Act + response = await tileset_info(d=[self.BIGINTERACT_UID, links_uid]) + + # Assert + assert response[self.BIGINTERACT_UID]["datatype"] == "2d-rectangle-domains" + assert response[links_uid]["datatype"] == "bedlike" + assert response[self.BIGINTERACT_UID]["uuid"] == self.BIGINTERACT_UID + assert response[links_uid]["uuid"] == links_uid + + @pytest.mark.asyncio + async def test_should_serve_real_tiles_for_both_presentations( + self, tile_env, biginteract_server, mocker + ): + """Test a real tile read through the router for each presentation. + + Given: + The same bigInteract document, requested as a rectangle + ``z.x.y`` tile id and a links ``z.x`` tile id. + When: + tiles is requested for both. + Then: + Each should carry the real interaction records clodius read + over HTTP, not an error object. + """ + # Arrange + _, mock_db, _ = tile_env + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + doc = _biginteract_doc(biginteract_server) + mock_db.file.docs = [doc] + rect_raw = f"{self.BIGINTERACT_UID}.0.0.0" + links_raw = f"{self.BIGINTERACT_UID}:links.0.0" + + # Act + response = await tiles(d=[rect_raw, links_raw]) + + # Assert + assert isinstance(response[rect_raw], list) and response[rect_raw] + assert isinstance(response[links_raw], list) and response[links_raw] + + @pytest.mark.asyncio + async def test_should_surface_the_specific_status_for_a_solo_request( + self, tile_env, mocker + ): + """Test that a solo request preserves TilesetSourceUnavailable's status. + + Given: + A bigInteract document whose upstream resolution fails with + DRSForbidden — the same condition /data maps to 403. + When: + tileset_info is requested for just that one uid. + Then: + It should raise HTTPException(403), not the generic 404 every + other per-dataset TilesetError gets. + """ + # Arrange + from cfdb.services import drs + + _, mock_db, _ = tile_env + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + side_effect=drs.DRSForbidden("denied"), + ) + doc = _biginteract_doc("https://example.org/irrelevant.bb") + mock_db.file.docs = [doc] + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await tileset_info(d=[self.BIGINTERACT_UID]) + assert exc_info.value.status_code == 403 + + @pytest.mark.asyncio + async def test_should_render_a_batch_entry_as_an_error_object_regardless( + self, tile_env, mocker + ): + """Test that a resolution failure stays per-dataset inside a batch. + + Given: + Two bigInteract documents, both hitting the same upstream + resolution failure, requested together. + When: + tileset_info is requested for both at once. + Then: + Both should render as inline error objects — proving the + batch is not aborted by a TilesetSourceUnavailable the way a + solo request would be — rather than either raising and + blanking the whole response. + """ + # Arrange + from cfdb.services import drs + + _, mock_db, _ = tile_env + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + side_effect=drs.DRSForbidden("denied"), + ) + first = _biginteract_doc("https://example.org/irrelevant.bb") + second = _biginteract_doc( + "https://example.org/irrelevant.bb", local_id="ENCFF111SIB" + ) + mock_db.file.docs = [first, second] + other_uid = "encode/ENCFF111SIB" + + # Act + response = await tileset_info(d=[self.BIGINTERACT_UID, other_uid]) + + # Assert + assert "error" in response[self.BIGINTERACT_UID] + assert "error" in response[other_uid] diff --git a/tests/test_tilesets/test_backend.py b/tests/test_tilesets/test_backend.py index 02a3131..a0131c2 100644 --- a/tests/test_tilesets/test_backend.py +++ b/tests/test_tilesets/test_backend.py @@ -10,6 +10,7 @@ is_available, is_tile_error, load_backend, + load_bbi_interaction_tilesets, load_hic_tileset, ) from cfdb.tilesets.errors import TileBackendUnavailable @@ -146,6 +147,63 @@ def test_should_raise_when_the_hic_module_is_missing(self, mocker): assert isinstance(excinfo.value.__cause__, ImportError) +class TestLoadBbiInteractionTilesets: + """The bigInteract tileset gate, resolved separately from the cooler one.""" + + def test_should_raise_when_the_bbi_module_is_missing(self, mocker): + """Test the error reported by a clodius that predates bigInteract. + + Given: + ``sys.modules`` patched so ``clodius.tiles_v2.bbi`` raises + ImportError while the cooler tileset stays importable. + When: + load_bbi_interaction_tilesets is called. + Then: + It should raise TileBackendUnavailable naming the bigInteract + interaction tileset classes, chained from the ImportError. + """ + # Arrange + mocker.patch.dict(sys.modules, {"clodius.tiles_v2.bbi": None}) + + # Act & assert + with pytest.raises( + TileBackendUnavailable, match="interaction tileset classes" + ) as excinfo: + load_bbi_interaction_tilesets() + assert isinstance(excinfo.value.__cause__, ImportError) + + def test_should_resolve_the_bbi_interaction_classes(self): + """Test the resolved-names contract of a build with bigInteract support. + + Given: + An environment where clodius carries ``clodius.tiles_v2.bbi``. + When: + load_bbi_interaction_tilesets is called. + Then: + It should return the 2D rectangle tileset, the links tileset, + clodius's LinkPolicy enum, and pybigtools's read-error type, + matching real clodius/pybigtools names. + """ + # Arrange + import pybigtools + from clodius.core.policies import LinkPolicy + from clodius.tiles_v2.bbi import ( + BBIInteraction2DTileset, + BBIInteractionLinksTileset, + ) + + # Act + rect_cls, links_cls, link_policy, bbi_read_error = ( + load_bbi_interaction_tilesets() + ) + + # Assert + assert rect_cls is BBIInteraction2DTileset + assert links_cls is BBIInteractionLinksTileset + assert link_policy is LinkPolicy + assert bbi_read_error is pybigtools.BBIReadError + + class TestIsTileError: """The classification that splits per-tile payloads from 500s. diff --git a/tests/test_tilesets/test_errors.py b/tests/test_tilesets/test_errors.py index e9aff7b..5e34ebc 100644 --- a/tests/test_tilesets/test_errors.py +++ b/tests/test_tilesets/test_errors.py @@ -5,7 +5,7 @@ import inspect import cfdb.tilesets.errors as errors_module -from cfdb.tilesets.errors import TilesetError +from cfdb.tilesets.errors import TilesetError, TilesetSourceUnavailable class TestTilesetErrorHierarchy: @@ -54,3 +54,44 @@ def test_should_derive_every_public_exception_from_tileset_error(self): "TilesetTooLarge", "TilesetHydrationTimeout", } <= names + + +class TestTilesetSourceUnavailable: + """The one TilesetError carrying its own HTTP status code.""" + + def test_should_default_the_status_code_to_502(self): + """Test the constructor's default when no status_code is given. + + Given: + TilesetSourceUnavailable constructed with only a message. + When: + Its status_code attribute is read. + Then: + It should be 502 — every real call site passes an explicit + status_code, so this default is otherwise never exercised. + """ + # Act + exc = TilesetSourceUnavailable("upstream unreachable") + + # Assert + assert exc.status_code == 502 + + def test_should_preserve_an_explicit_status_code(self): + """Test that a passed-in status_code is stored as given. + + Given: + TilesetSourceUnavailable constructed with an explicit + status_code. + When: + Its status_code attribute is read. + Then: + It should equal the value passed in, and the instance + should still be a TilesetError. + """ + # Act + exc = TilesetSourceUnavailable("not found upstream", status_code=404) + + # Assert + assert exc.status_code == 404 + assert isinstance(exc, TilesetError) + assert str(exc) == "not found upstream" diff --git a/tests/test_tilesets/test_formats.py b/tests/test_tilesets/test_formats.py index 861c400..e3bf1c5 100644 --- a/tests/test_tilesets/test_formats.py +++ b/tests/test_tilesets/test_formats.py @@ -9,9 +9,12 @@ from hypothesis import strategies as st from cfdb.tilesets.formats import ( + LINKS_PRESENTATION_SUFFIX, MatrixSource, + is_bbi_interaction_source, matrix_source_kind, needs_materialization, + split_bbi_presentation, ) #: The suffixes the module claims, restated literally so the property @@ -382,3 +385,225 @@ def test_should_not_import_the_workflow_or_aws_stack(self): # Assert assert imported <= {"__future__", "enum", "pathlib", "typing"}, imported + + +class TestIsBbiInteractionSource: + """bigInteract gets its own minted format name, not an EDAM term.""" + + def test_should_recognize_a_biginteract_document(self): + """Test that an exact format-name match is recognized. + + Given: + A document whose ``file_format.name`` is exactly + ``"bigInteract"``. + When: + is_bbi_interaction_source is called. + Then: + It should return True. + """ + # Arrange + doc = {"file_format": {"name": "bigInteract"}} + + # Act + result = is_bbi_interaction_source(doc) + + # Assert + assert result is True + + @pytest.mark.parametrize( + "doc", + [ + {"file_format": {"name": "bigBed"}}, + {"file_format": {"name": "BIGINTERACT"}}, + {"file_format": {"name": "biginteract"}}, + {}, + {"file_format": None}, + {"file_format": "bigInteract"}, + {"file_format": {}}, + ], + ids=[ + "plain-bigbed", + "upper-cased", + "lower-cased", + "no-format", + "null-format", + "format-not-a-dict", + "format-missing-name", + ], + ) + def test_should_reject_everything_else(self, doc): + """Test that only an exact, correctly-shaped match is recognized. + + Given: + A document that is not a bigInteract file — wrong format + name, wrong casing, or a malformed/missing ``file_format``. + When: + is_bbi_interaction_source is called. + Then: + It should return False rather than raising. The match is + deliberately case-sensitive: cfdb mints this name itself + (``services/ontology_mappings.py``), so there is no upstream + casing variance to tolerate, unlike ``HDF5_FORMAT_NAME``. + """ + # Act + result = is_bbi_interaction_source(doc) + + # Assert + assert result is False + + @settings(max_examples=50) + @given( + doc=st.fixed_dictionaries( + {}, + optional={ + "file_format": st.one_of( + _json_scalars, + st.dictionaries(st.text(), _json_scalars, max_size=3), + st.just({"name": "bigInteract"}), + st.just({"name": "BIGINTERACT"}), + st.just({"name": "biginteract"}), + st.just({"name": "bigBed"}), + ) + }, + ) + ) + def test_should_classify_totally_and_soundly(self, doc): + """Test totality and soundness over arbitrary documents. + + Given: + An arbitrary document whose ``file_format`` is possibly a + non-dict, missing, or a dict with a name near the + bigInteract/bigBed boundary. + When: + is_bbi_interaction_source is called. + Then: + It should never raise, and return True exactly when + file_format is a dict with name == "bigInteract" exactly. + """ + # Act + result = is_bbi_interaction_source(doc) + + # Assert + file_format = doc.get("file_format") + expected = ( + isinstance(file_format, dict) and file_format.get("name") == "bigInteract" + ) + assert result is expected + + +class TestSplitBbiPresentation: + """The ``:links`` uid suffix that selects the 1D presentation.""" + + def test_should_default_to_rectangles_with_no_suffix(self): + """Test the default presentation for a plain uid. + + Given: + A uid with no ``:links`` suffix. + When: + split_bbi_presentation is called. + Then: + It should return the uid unchanged, paired with + "rectangles". + """ + # Arrange + uid = "encode/ENCFF000BIG" + + # Act + base_uid, presentation = split_bbi_presentation(uid) + + # Assert + assert base_uid == uid + assert presentation == "rectangles" + + def test_should_select_links_and_strip_the_suffix(self): + """Test the links presentation for a suffixed uid. + + Given: + The same uid with a ``:links`` suffix appended. + When: + split_bbi_presentation is called. + Then: + It should return the base uid with the suffix removed, + paired with "links". + """ + # Arrange + uid = "encode/ENCFF000BIG:links" + + # Act + base_uid, presentation = split_bbi_presentation(uid) + + # Assert + assert base_uid == "encode/ENCFF000BIG" + assert presentation == "links" + + def test_should_split_a_uid_that_is_only_the_suffix(self): + """Test the degenerate case where the whole uid is the suffix. + + Given: + A uid equal to exactly ``:links``, with nothing before it. + When: + split_bbi_presentation is called. + Then: + It should return an empty base uid paired with "links" — + the suffix still anchors at the end, even with nothing to + strip it from. + """ + # Arrange + uid = ":links" + + # Act + base_uid, presentation = split_bbi_presentation(uid) + + # Assert + assert base_uid == "" + assert presentation == "links" + + def test_should_not_match_the_suffix_in_the_middle_of_a_uid(self): + """Test that the suffix must anchor at the end, not appear anywhere. + + Given: + A uid that contains the literal text ``:links`` followed by + more characters, rather than ending with it. + When: + split_bbi_presentation is called. + Then: + It should return the uid unchanged, paired with + "rectangles" — a uid is only detected as the links + presentation when ``:links`` is its trailing suffix. + """ + # Arrange + uid = "encode/ENCFF000BIG:linksextra" + + # Act + base_uid, presentation = split_bbi_presentation(uid) + + # Assert + assert base_uid == uid + assert presentation == "rectangles" + + @settings(max_examples=100) + @given(uid=st.text()) + def test_should_split_totally_and_invertibly(self, uid): + """Test totality and the round-trip property over arbitrary strings. + + Given: + Any string. + When: + split_bbi_presentation is called. + Then: + It should never raise; presentation should be "links" iff + the input ends with the exact suffix; and rejoining + base_uid with the suffix should recover the original + whenever presentation is "links", while base_uid should + equal the original string otherwise. + """ + # Act + base_uid, presentation = split_bbi_presentation(uid) + + # Assert + if uid.endswith(LINKS_PRESENTATION_SUFFIX): + assert presentation == "links" + assert base_uid + LINKS_PRESENTATION_SUFFIX == uid + else: + assert presentation == "rectangles" + assert base_uid == uid diff --git a/tests/test_tilesets/test_service.py b/tests/test_tilesets/test_service.py index 6bdcf00..cb25d5d 100644 --- a/tests/test_tilesets/test_service.py +++ b/tests/test_tilesets/test_service.py @@ -10,16 +10,21 @@ from hypothesis import HealthCheck, example, given, settings from hypothesis import strategies as st +from cfdb.services import drs from cfdb.tilesets.errors import ( TilesetHydrationTimeout, TilesetIdentityIncomplete, TilesetNotReady, + TilesetSourceUnavailable, TilesetUnsupported, ) +from cfdb.tilesets.backend import load_bbi_interaction_tilesets from cfdb.tilesets.service import build_service from cfdb.workflows.cache import LocalFsCache from cfdb.workflows.models import ArtifactKind from cfdb.workflows.processors.matrix import MatrixTilesetProcessor +from cfdb.workflows.urlsafe import UnsafeOutboundURL +from tests.fixtures.biginteract import CANONICAL_RECORDS from tests.fixtures.remote_cache import FakeRemoteCache from tests.test_workflows import FIXTURE_MD5, FIXTURE_MD5_ALT @@ -1134,3 +1139,549 @@ async def test_should_close_the_held_handle_after_aclose_under_a_reader( # Assert close_spy.assert_called_once() assert close_spy.call_args.args[0] is held + + +def _biginteract_doc(access_url: str, **overrides) -> dict: + """Return a projected document for an ENCODE bigInteract file.""" + doc = { + "submission": "encode", + "local_id": "ENCFF000BIG", + "md5": FIXTURE_MD5, + "filename": "sample.bigInteract", + "file_format": {"name": "bigInteract"}, + "access_url": access_url, + "dcc": {"dcc_abbreviation": "ENCODE"}, + } + doc.update(overrides) + return doc + + +class TestBbiInteraction: + """bigInteract: read straight from the upstream URL, never the cache. + + ``resolve_download_url`` is mocked throughout rather than left to run + for real against ``http://127.0.0.1`` — the SSRF allowlist's loopback + escape hatch is read into a module constant at import time + (``cfdb.workflows.urlsafe._ALLOW_HTTP_LOOPBACK``), which has already + been evaluated False by the time this test module loads, so setting + the env var here would do nothing. Mocking the one URL-resolution hop + still exercises everything that matters: real service dispatch, a + real ``fsspec`` factory, and real clodius tile reads over real HTTP + against ``biginteract_server``. + """ + + @pytest.mark.asyncio + async def test_should_serve_tileset_info_for_the_rectangle_presentation( + self, service_factory, biginteract_server, mocker + ): + """Test the default (no-suffix) uid's tileset_info document. + + Given: + A bigInteract file whose uid carries no presentation suffix. + When: + tileset_info is requested. + Then: + It should report the 2D rectangle-domain datatype, with no + cache or local store ever consulted. + """ + # Arrange + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + + def _unreachable_cache(): + raise AssertionError("the cache must not be consulted for bigInteract") + + service = build(cache_provider=_unreachable_cache) + doc = _biginteract_doc(biginteract_server) + uid = "encode/ENCFF000BIG" + + # Act + info = await service.tileset_info(uid, doc) + + # Assert + assert info["datatype"] == "2d-rectangle-domains" + assert info["uuid"] == uid + + @pytest.mark.asyncio + async def test_should_serve_tileset_info_for_the_links_presentation( + self, service_factory, biginteract_server, mocker + ): + """Test the ``:links``-suffixed uid's tileset_info document. + + Given: + The same bigInteract file, requested under its ``:links`` + uid. + When: + tileset_info is requested. + Then: + It should report the 1D "bedlike" datatype rather than the + default rectangle one — a different tileset class entirely, + selected from the same uid string. + """ + # Arrange + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build() + doc = _biginteract_doc(biginteract_server) + uid = "encode/ENCFF000BIG:links" + + # Act + info = await service.tileset_info(uid, doc) + + # Assert + assert info["datatype"] == "bedlike" + assert info["uuid"] == uid + + @pytest.mark.asyncio + async def test_should_read_a_real_rectangle_tile( + self, service_factory, biginteract_server, mocker + ): + """Test a real 2D tile read over the fsspec factory. + + Given: + The rectangle-presentation uid and a ``z.x.y`` tile id + covering the whole genome at zoom 0. + When: + tiles is requested. + Then: + It should return the real interaction records clodius read + over HTTP, not an error payload or an empty list. + """ + # Arrange + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build() + doc = _biginteract_doc(biginteract_server) + uid = "encode/ENCFF000BIG" + raw = f"{uid}.0.0.0" + + # Act + payloads = await service.tiles(uid, doc, [raw]) + + # Assert + payload = payloads[raw] + assert isinstance(payload, list) + assert len(payload) == len(CANONICAL_RECORDS) + assert all("xStart" in record and "yStart" in record for record in payload) + + @pytest.mark.asyncio + async def test_should_read_a_real_links_tile( + self, service_factory, biginteract_server, mocker + ): + """Test a real 1D tile read over the fsspec factory. + + Given: + The links-presentation uid and a ``z.x`` (one-coordinate) + tile id covering the whole genome at zoom 0. + When: + tiles is requested. + Then: + It should return the same real records — the links + presentation shares the same underlying data, just a + different id grammar. + """ + # Arrange + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build() + doc = _biginteract_doc(biginteract_server) + uid = "encode/ENCFF000BIG:links" + raw = f"{uid}.0.0" + + # Act + payloads = await service.tiles(uid, doc, [raw]) + + # Assert + payload = payloads[raw] + assert isinstance(payload, list) + assert len(payload) == len(CANONICAL_RECORDS) + + def test_should_read_tiles_via_genuine_range_requests(self, tiny_biginteract): + """Test that the fsspec factory performs real, partial HTTP reads. + + Given: + A bigInteract file served by a handler that genuinely + implements ``Range``/``206`` (unlike ``biginteract_server``'s + plain ``SimpleHTTPRequestHandler``, which ignores ``Range`` + entirely and always returns the whole file), read through a + small ``block_size`` so a read spanning more than one block + must issue more than one HTTP request. + When: + A tile is read from a tileset built over this factory. + Then: + More than one distinct byte range should have been + requested, and at least one of them should start past byte + 0 — the property that distinguishes "read remotely in + pieces" from "download the whole file on first touch", + which ``biginteract_server`` cannot exercise at its size. + """ + import http.server + import re + import threading + + import fsspec + from clodius.tiles_v2.bbi import BBIInteraction2DTileset + + # Arrange + data = tiny_biginteract.read_bytes() + ranges_seen: list[tuple[int, int] | str] = [] + + class _RangeHandler(http.server.BaseHTTPRequestHandler): + def log_message(self, *_args, **_kwargs) -> None: + return + + def do_HEAD(self) -> None: + self._respond(head_only=True) + + def do_GET(self) -> None: + self._respond(head_only=False) + + def _respond(self, *, head_only: bool) -> None: + range_header = self.headers.get("Range") + if range_header: + match = re.match(r"bytes=(\d+)-(\d*)", range_header) + start = int(match.group(1)) + end = int(match.group(2)) if match.group(2) else len(data) - 1 + end = min(end, len(data) - 1) + ranges_seen.append((start, end)) + chunk = data[start : end + 1] + self.send_response(206) + self.send_header("Content-Type", "application/octet-stream") + self.send_header( + "Content-Range", f"bytes {start}-{end}/{len(data)}" + ) + self.send_header("Content-Length", str(len(chunk))) + self.send_header("Accept-Ranges", "bytes") + self.end_headers() + if not head_only: + self.wfile.write(chunk) + else: + ranges_seen.append("full") + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Length", str(len(data))) + self.send_header("Accept-Ranges", "bytes") + self.end_headers() + if not head_only: + self.wfile.write(data) + + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _RangeHandler) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + url = f"http://127.0.0.1:{port}/tiny.bb" + + def factory(): + return fsspec.open(url, "rb", block_size=32).open() + + # Act + tileset = BBIInteraction2DTileset(factory) + tile_id = tileset.parse_tile_id("u.0.0.0") + tileset.tiles([tile_id]) + + # Assert + byte_ranges = [r for r in ranges_seen if isinstance(r, tuple)] + assert len(byte_ranges) > 1 + assert any(start > 0 for start, _ in byte_ranges) + finally: + server.shutdown() + server.server_close() + + @pytest.mark.asyncio + async def test_should_key_the_two_presentations_independently( + self, service_factory, biginteract_server, mocker + ): + """Test that rectangles and links open distinct tileset handles. + + Given: + Both uids for the same bigInteract file, read under an + open-tileset cap of one. + When: + The links presentation is opened right after the rectangle + one. + Then: + Opening the second should evict and close the first — the + only way that happens is if the ``:links`` suffix changed + the cache key, since a shared key would just reuse the + already-open handle and close nothing. + """ + # Arrange + rect_cls, _, _, _ = load_bbi_interaction_tilesets() + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build(open_max=1) + doc = _biginteract_doc(biginteract_server) + close_spy = mocker.spy(rect_cls, "close") + + await service.tileset_info("encode/ENCFF000BIG", doc) + # Construction self-releases its own validation-time handle + # (clodius's "registration is free" property) — not the + # eviction this test is about. + close_spy.assert_called_once() + close_spy.reset_mock() + + # Act + await service.tileset_info("encode/ENCFF000BIG:links", doc) + + # Assert — the rectangle presentation's handle was evicted and + # closed to make room for the links one, proving they held + # independent cache entries. + close_spy.assert_called_once() + + @pytest.mark.asyncio + async def test_should_close_a_bbi_handle_evicted_past_the_open_cap( + self, service_factory, biginteract_server, mocker + ): + """Test that the open-handle pool bounds bigInteract tilesets too. + + Given: + An open-tileset cap of one and two distinct bigInteract + files. + When: + Both are opened. + Then: + The first handle should be closed — the same eviction + contract the cooler path already has, now exercised against + clodius's BBI-specific close()/read-lock semantics instead + of h5py's. + """ + # Arrange + rect_cls, _, _, _ = load_bbi_interaction_tilesets() + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build(open_max=1) + first_doc = _biginteract_doc(biginteract_server) + second_doc = _biginteract_doc(biginteract_server, local_id="ENCFF111SIB") + close_spy = mocker.spy(rect_cls, "close") + + async with service.checkout("encode/ENCFF000BIG", first_doc) as first: + assert first.info() is not None # force the handle open + close_spy.assert_called_once() + assert close_spy.call_args.args[0] is first + close_spy.reset_mock() + + # Act + await service.tileset_info("encode/ENCFF111SIB", second_doc) + + # Assert — second's own construction releases itself too; the + # eviction this test targets is the call right after it, + # against first. + assert close_spy.call_count == 2 + assert close_spy.call_args_list[-1].args[0] is first + + @pytest.mark.asyncio + async def test_should_defer_closing_a_bbi_handle_that_is_being_read( + self, service_factory, biginteract_server, mocker + ): + """Test the reference-counted close for a live bigInteract reader. + + Given: + A bigInteract tileset checked out by a reader, and a second + distinct bigInteract file opened that evicts it past the + cap. + When: + The eviction lands while the first reader is still inside + its checkout. + Then: + The handle must stay open until that reader leaves, then be + closed exactly once — clodius's BBI tilesets take their own + read lock around close(), so this is a real, not assumed, + property of the new code path. + """ + # Arrange + rect_cls, _, _, _ = load_bbi_interaction_tilesets() + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=biginteract_server, + ) + service = build(open_max=1) + first_doc = _biginteract_doc(biginteract_server) + second_doc = _biginteract_doc(biginteract_server, local_id="ENCFF111SIB") + close_spy = mocker.spy(rect_cls, "close") + + # Act & Assert + async with service.checkout("encode/ENCFF000BIG", first_doc) as held: + assert held.info() is not None # force the handle open + close_spy.assert_called_once() + close_spy.reset_mock() + + await service.tileset_info("encode/ENCFF111SIB", second_doc) + close_spy.assert_called_once() + assert close_spy.call_args.args[0] is not held + close_spy.reset_mock() + + assert held.info() is not None # still serving the reader + close_spy.assert_not_called() + + # Assert — the last reader out performs the deferred close + close_spy.assert_called_once() + assert close_spy.call_args.args[0] is held + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("exc_factory", "expected_status"), + [ + (lambda: drs.DRSNotFound("missing"), 404), + (lambda: drs.DRSForbidden("denied"), 403), + (lambda: drs.DRSTimeout("slow"), 504), + (lambda: drs.DRSUpstreamError("broke"), 502), + (lambda: UnsafeOutboundURL("bad host"), 400), + (lambda: ValueError("malformed drs:// URI"), 400), + ], + ) + async def test_should_translate_a_resolution_failure( + self, service_factory, mocker, exc_factory, expected_status + ): + """Test that each upstream-resolution failure maps to the right status. + + Given: + resolve_download_url raising each DRS exception (mapped to + the same status /data maps the same failure to) or a bare + ValueError (a malformed DRS URI or an unusable access + method — cfdb's own 400, not a /data mirror). + When: + tileset_info is requested. + Then: + It should raise TilesetSourceUnavailable carrying the + expected status code, so the router can preserve it for a + solo request. + """ + # Arrange + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + side_effect=exc_factory(), + ) + service = build() + doc = _biginteract_doc("https://example.org/irrelevant.bb") + + # Act & Assert + with pytest.raises(TilesetSourceUnavailable) as excinfo: + await service.tileset_info("encode/ENCFF000BIG", doc) + assert excinfo.value.status_code == expected_status + + @pytest.mark.asyncio + async def test_should_translate_a_real_fetch_failure( + self, service_factory, biginteract_server, mocker + ): + """Test that a failure in the real remote read, not just resolution, is caught. + + Given: + resolve_download_url succeeding, but the URL it resolves to + 404ing on the actual fetch — the construction step that + reads the file header, not the resolution step above it. + When: + tileset_info is requested. + Then: + It should raise TilesetSourceUnavailable(502) rather than + letting the raw FileNotFoundError escape uncaught. + """ + # Arrange + _, build = service_factory + missing_url = f"{biginteract_server}-does-not-exist" + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=missing_url, + ) + service = build() + doc = _biginteract_doc(missing_url) + + # Act & Assert + with pytest.raises(TilesetSourceUnavailable) as excinfo: + await service.tileset_info("encode/ENCFF000BIG", doc) + assert excinfo.value.status_code == 502 + + @pytest.mark.asyncio + async def test_should_translate_a_malformed_response( + self, service_factory, tmp_path, mocker + ): + """Test that a 200 response of non-bigBed bytes is caught too. + + Given: + A server that resolves the URL and answers 200, but whose + body is not a real bigBed file — the failure pybigtools + itself raises during header parsing, not an OSError. + When: + tileset_info is requested. + Then: + It should raise TilesetSourceUnavailable(502) rather than + letting the raw pybigtools.BBIReadError escape uncaught. + """ + import http.server + import socketserver + import threading + + # Arrange + garbage = tmp_path / "garbage.bb" + garbage.write_bytes(b"not a bigbed file") + + class _Handler(http.server.SimpleHTTPRequestHandler): + def __init__(self, *args, **kwargs): + super().__init__(*args, directory=str(tmp_path), **kwargs) + + def log_message(self, *_args, **_kwargs) -> None: + return + + server = socketserver.ThreadingTCPServer(("127.0.0.1", 0), _Handler) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + url = f"http://127.0.0.1:{port}/garbage.bb" + try: + _, build = service_factory + mocker.patch( + "cfdb.tilesets.service.resolve_download_url", + return_value=url, + ) + service = build() + doc = _biginteract_doc(url) + + # Act & Assert + with pytest.raises(TilesetSourceUnavailable) as excinfo: + await service.tileset_info("encode/ENCFF000BIG", doc) + assert excinfo.value.status_code == 502 + finally: + server.shutdown() + server.server_close() + + @pytest.mark.asyncio + async def test_should_refuse_a_document_with_no_access_url(self, service_factory): + """Test the guard against a bigInteract document with no access_url. + + Given: + A bigInteract document whose access_url is falsy — nothing + to resolve or open. + When: + tileset_info is requested. + Then: + It should raise TilesetUnsupported rather than attempting + URL resolution at all. + """ + # Arrange + _, build = service_factory + service = build() + doc = _biginteract_doc(None) + + # Act & Assert + with pytest.raises(TilesetUnsupported): + await service.tileset_info("encode/ENCFF000BIG", doc) From 5bf9672d69a9a25a9d5ae242c25ea6f2c4116ab0 Mon Sep 17 00:00:00 2001 From: Conrad Date: Fri, 9 Oct 2026 11:51:41 -0400 Subject: [PATCH 5/5] docs: Document interaction tile serving and the restored index gap Add a README section covering the two-uid convention, the lack of any preparation step, and the per-dataset-vs-infrastructure error split. Update ENCODE-SUPPLEMENT's note on bigInteract's lost /index coverage to say what replaces it; bedpe's /index regression still has nothing behind it. --- ENCODE-SUPPLEMENT.md | 4 ++-- README.md | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ENCODE-SUPPLEMENT.md b/ENCODE-SUPPLEMENT.md index e309fe7..108d3f3 100644 --- a/ENCODE-SUPPLEMENT.md +++ b/ENCODE-SUPPLEMENT.md @@ -55,9 +55,9 @@ ENCODE uses human-readable strings for file formats, assay types, output types, `cfdb:` marks a term minted here because EDAM has none (verified against OLS4). The alternative — aliasing an unrepresented format onto the nearest EDAM term — is what produced the starch/BED conflation behind #69 and #72: the format becomes indistinguishable from the one it was aliased to, and the processor claiming that term picks it up and mangles it. -`bedpe` and `bigInteract` both pair two loci per record. Routed into the BED tabix pipeline they would be sorted and indexed by the first locus alone, committing a cached artifact that looks successful and is wrong — and the byte-sniff guard from #71 does not catch it, since both are plaintext or gzip. The distinct **name** is what does the work: processor lookup keys on `file_format.name`, and neither name is in any processor's `supported_formats`, so `GET /data/...` streams the raw upstream file rather than a mangled index. Tileset endpoints that understand these formats are planned separately. +`bedpe` and `bigInteract` both pair two loci per record. Routed into the BED tabix pipeline they would be sorted and indexed by the first locus alone, committing a cached artifact that looks successful and is wrong — and the byte-sniff guard from #71 does not catch it, since both are plaintext or gzip. The distinct **name** is what does the work: processor lookup keys on `file_format.name`, and neither name is in any processor's `supported_formats`, so `GET /data/...` streams the raw upstream file rather than a mangled index. -**`bigInteract` loses a working index in the meantime.** The two formats were not equally broken before this change. A `bedpe` routed through tabix produced an index that was simply wrong — the second mate of every record was unindexed and invisible. A `bigInteract` routed through `bigBedToBed` produced one that was *range-coherent*: the leading three columns are a genuine interval, so the index answered range queries correctly while silently degrading each interaction to a single locus. Re-typing both means `GET /index/...` now returns 404 for either, so `bigInteract` files that previously returned a usable-if-degraded index return nothing at all, and that applies to `bigInteract` already in the experiment corpus as well as to newly ingested annotation files. The tradeoff is accepted deliberately: a 404 states the truth, where the degraded index quietly answered a question the caller did not ask. The tileset work is what restores the capability. +**`bigInteract` loses a working index, but gets tileset serving in exchange.** The two formats were not equally broken before this change. A `bedpe` routed through tabix produced an index that was simply wrong — the second mate of every record was unindexed and invisible. A `bigInteract` routed through `bigBedToBed` produced one that was *range-coherent*: the leading three columns are a genuine interval, so the index answered range queries correctly while silently degrading each interaction to a single locus. Re-typing both means `GET /index/...` now returns 404 for either, so `bigInteract` files that previously returned a usable-if-degraded index return nothing at all, and that applies to `bigInteract` already in the experiment corpus as well as to newly ingested annotation files. The tradeoff is accepted deliberately: a 404 states the truth, where the degraded index quietly answered a question the caller did not ask. `GET /tileset_info` and `GET /tiles` now serve bigInteract files directly (issue #120) — see the README's "Interaction tile serving (bigInteract)" section — which is the capability `/index`'s degraded answer stood in for. `bedpe` has no such replacement yet; its own tileset endpoints remain planned separately, and its `/index` 404 is still a bare regression with no capability behind it. This applies to `.bedpe` already in the ENCODE experiment corpus, not only to annotation files — but it reaches no further than ENCODE. `FILE_FORMAT_TO_EDAM` and `get_file_format` have exactly one consumer, the ENCODE transform; 4DN and HuBMAP take `file_format` from their upstream C2M2 datapackage or portal API and never consult the table. A 4DN `.bedpe` whose upstream declares BED therefore still carries `file_format.name == "BED"`, is still claimed by `TabixIntervalProcessor`, and still gets an index built from its first mate. Closing that means routing incoming formats from every DCC through the same table, or refusing a `.bedpe` filename at the processor regardless of declared format; both are follow-up work. diff --git a/README.md b/README.md index a85b006..55fe576 100644 --- a/README.md +++ b/README.md @@ -999,6 +999,27 @@ This is invisible to the client, for two reasons that were verified rather than - **The `tileset` artifact for an `.mcool` is a byte-identical copy of the upstream file.** Across all 773 4DN mcools that is a material S3 footprint if every one is ever browsed; the workers stack's `CacheArtifactExpirationDays` (30 days) trims it, at the cost of re-materializing anything revisited after a month. - **The local tileset copies are per-API-task.** `DesiredCount` is 1 today; scaling the API horizontally multiplies both the disk footprint and the cold-open cost, and an ALB without session affinity scatters one user's tiles across tasks. +### Interaction tile serving (bigInteract) + +`GET /tileset_info` and `GET /tiles` also serve `bigInteract` files — UCSC's bed5+13 format for paired-region interactions (chromatin loops, regulatory element–gene pairs, and the like) — through the same uid scheme as contact maps, but with none of that section's local-materialization machinery: a bigInteract tileset is read straight from the file's upstream URL over HTTP Range requests, via `fsspec`, every time. There is no cache artifact, no `POST /tilesets/{dcc}/{local_id}` preparation step, and no `CFDB_TILESET_DISK_CACHE_BYTES` budget to exhaust — this format never touches `LocalTilesetStore`. + +**Two presentations, two uids.** clodius exposes bigInteract three different ways; cfdb resolves two of them and serves both rather than picking one: + +| uid | Tileset | `datatype` | Tile id shape | +|-----|---------|------------|----------------| +| `{dcc}/{local_id}` | 2D rectangle domains | `2d-rectangle-domains` | `z.x.y` (same convention as a contact map) | +| `{dcc}/{local_id}:links` | 1D arc/link track | `bedlike` | `z.x` | + +clodius's third presentation, `BBIInteractionTileset` ("paired intervals" — the base class the two rows above subclass), is deliberately not resolved: clodius's own docstring for it recommends preferring the two concrete subclasses, and cfdb has no caller for the raw shape. + +The `:links` suffix lives only on the wire — it selects which clodius tileset class opens the file, and is stripped before the `(dcc, local_id)` Mongo lookup runs; the file document itself is identical either way. `:` rather than `.` is deliberate: the existing restriction on a `.` inside a uid half (clodius parses `uid.z.x[.y]` by splitting on it) still applies, and `:` never collides with it. + +**No `.hic`-style refusal.** `.hic` stays unservable because `hictkpy` has no remote-read entry point at all (see the Limitations above). bigInteract's reader, `pybigtools`, has native HTTP support, which is what makes reading it in place — rather than materializing a local copy — the *default* and only path here, not a future optimization. + +**No preparation or readiness probe.** `GET /tilesets/{dcc}/{local_id}/status` and `POST /tilesets/{dcc}/{local_id}` are specific to the contact-map materialization pipeline and do not apply to bigInteract files — `/tileset_info` and `/tiles` are the whole interface. + +A bigInteract upstream-resolution failure surfaces through the same per-dataset-vs-infrastructure split every other tileset failure does: a solo request gets a specific HTTP status, and a batch entry renders as an inline `{"error": ...}` object beside its siblings rather than failing the whole request. The status itself has two sources. A DRS lookup that 404s, times out, is forbidden, or errors upstream carries the same 404/403/504/502 that failure would get on `/data` — that part is genuine parity. A malformed or disallowed URL (the SSRF allowlist, a malformed DRS URI, or no usable access method) carries 400, which is cfdb's own choice for the tileset path rather than a mirror of `/data` — `/data` has no 400 case for any of these. A failure in the actual remote read itself (the URL resolves but the fetch then fails, or the bytes aren't a real bigBed) carries 502. + ### Preprocessing & indexing workflow Many upstream files are not directly consumable by Gosling Designer without preprocessing (e.g., sort+index for BAM, bgzip+tabix for VCF/GFF/BED). When `/data` or `/index` is called for a format that needs preprocessing and the processed artifact is not yet in cache, the API dispatches a workflow and returns `202 Accepted` with a `Location` header pointing to a job status endpoint. A subsequent call, after the workflow completes, streams the processed artifact from cache (with `Range` support). Both endpoints share a single workflow per source file via a Mongo-backed mutex.