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. 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" 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://"): 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 5dacc31..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 @@ -931,14 +936,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 +988,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 +1049,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( @@ -1106,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)