diff --git a/ENCODE-SUPPLEMENT.md b/ENCODE-SUPPLEMENT.md index ce87a61..e309fe7 100644 --- a/ENCODE-SUPPLEMENT.md +++ b/ENCODE-SUPPLEMENT.md @@ -1,14 +1,30 @@ # ENCODE Metadata Mapping -Field mapping from the ENCODE metadata TSV to the CFDB data model. ENCODE does not use C2M2 — all data is fetched from the ENCODE REST API and pre-materialized directly into the `files` collection, bypassing the C2M2 load and Rust materializer steps. +Field mapping from the ENCODE metadata TSVs to the CFDB data model. ENCODE does not use C2M2 — all data is fetched from the ENCODE REST API and pre-materialized directly into the `files` collection, bypassing the C2M2 load and Rust materializer steps. + +Two TSVs are ingested: released **experiments** and released **annotations** of configured types. They share most of their mapping; the sections below describe the experiment TSV, and [Annotation Mapping](#annotation-mapping) states every way an annotation row differs. ## Data Source | Source | URL | What It Provides | |--------|-----|------------------| -| ENCODE metadata TSV | `GET /metadata/?type=Experiment&status=released` | Streaming TSV of all released experiment files (~700k rows, hundreds of MB) | +| ENCODE experiment metadata TSV | `GET /metadata/?type=Experiment&status=released` | Streaming TSV of all released experiment files (~700k rows, hundreds of MB), 59 columns | +| ENCODE annotation metadata TSV | `GET /metadata/?type=Annotation&status=released&annotation_type=` | Streaming TSV of all released files of one annotation type, 32 columns. Requested once per configured type | + +The TSV is streamed line-by-line to keep memory usage constant. Each row represents one file with its experiment/dataset, biosample, library, and donor metadata denormalized inline. + +### Which annotation types are ingested + +`ENCODE_ANNOTATION_TYPES` — comma-separated `annotation_type` values. Unset yields the default allowlist below; set to an empty value to disable annotation ingest entirely, which logs a warning since `ENCODE_METADATA_TIMEOUT_SECONDS` reads an empty value the other way, as "unset, use the default". Entries are trimmed, blanks dropped, and repeats collapsed — one ingest phase runs per distinct entry, and since ENCODE files are written with `insert_many` into a collection with no unique key, a duplicated token would otherwise load every file of that type twice. + +| `annotation_type` | Datasets | Files | Formats | Assemblies | +|---|---|---|---|---| +| `candidate Cis-Regulatory Elements` | 6,230 | 12,448 | `bed bed9+`, `bigBed bed9+`, `bed bed3+`, `bigBed bed3+`, `bigBed bed9` | GRCh38, mm10, hg19 | +| `element gene regulatory interaction predictions` | 1,543 | 16,692 | `bed bed3+`, `bedpe`, `bigInteract`, `bed bed3` | GRCh38 | -The TSV is streamed line-by-line to keep memory usage constant. Each row represents one file with its experiment, biosample, library, and donor metadata denormalized inline. +This is an allowlist rather than a filter applied after the fact. ENCODE publishes 580,910 annotation datasets, 86% of them footprints; ingesting the type space wholesale and pruning later would be a corpus-scale mistake to undo. + +One request per type, not one request carrying repeated `annotation_type` parameters: a failure or timeout on one type then costs only that type, and each response stays small enough for the portal to assemble without a gateway timeout. ## Ontology Mappings @@ -28,11 +44,25 @@ ENCODE uses human-readable strings for file formats, assay types, output types, | `broadPeak` | `format:3614` | BroadPeak | | `bigWig` | `format:3006` | bigWig | | `bigBed` | `format:3004` | bigBed | +| `bedpe` | `cfdb:bedpe` | bedpe | +| `bigInteract` | `cfdb:biginteract` | bigInteract | | `vcf` | `format:3016` | VCF | | `gtf` | `format:2306` | GTF | | `tsv` | `format:3475` | TSV | | `hdf5` | `format:3590` | HDF5 | +#### Minted (non-EDAM) format terms + +`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. + +**`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. + +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. + +**Cache artifacts left behind.** Files of these two formats that were indexed before the re-typing still have their incorrect `.tbi` artifacts in the workflow cache. Nothing reads them any more — `lookup_for` returns `None` and the router bails before probing the cache — and nothing purges them either, so they are unreachable storage cost until someone sweeps them. They also make a latent cache-key hazard concrete: `cache_key` identifies a processor only by its `processor_version`, not by which processor it is, so a future paired-interval processor sharing a version number with `TabixIntervalProcessor` would derive the same key and read those stale artifacts back as cache hits. A processor identity has to be folded into the key before that processor lands; see the warning on `cfdb.workflows.keys.cache_key`. + ### Output Type -> EDAM Data 53 mappings from ENCODE `Output type` strings to EDAM data CV terms. @@ -47,6 +77,13 @@ ENCODE uses human-readable strings for file formats, assay types, output types, | `methylation state at CpG` | `data:1772` | Methylation data | | `variant calls` | `data:3498` | Sequence variations | | `contact matrix` | `data:2082` | Matrix | +| `candidate Cis-Regulatory Elements` | `data:1255` | Sequence features | +| `elements reference` | `data:1255` | Sequence features | +| `element gene links` | `data:0006` | Data | +| `thresholded element gene links` | `data:0006` | Data | +| `thresholded links` | `data:0006` | Data | + +The last five are the complete `Output type` domain of the two ingested annotation types, verified against the live TSVs. Without them every annotation file would carry `data_type: null`. The element/gene link types resolve to the generic `data:0006` because EDAM has no term for a predicted regulatory relationship between two loci — the pre-existing `chromatin interactions` entry already made that call the same way. ### Assay -> OBI @@ -150,6 +187,9 @@ Experiment-level fields stored on `collection.extra.encode` (`EnrichedEncodeColl | `extra.encode.platform` | `Platform` | | `extra.encode.dbxrefs` | `dbxrefs` | | `extra.encode.rbns_protein_concentration` | `RBNS protein concentration` | +| `extra.encode.annotation_type` | `Annotation type` (annotation TSV only) | +| `extra.encode.software_used` | `Software used` (annotation TSV only) | +| `extra.encode.encyclopedia_version` | `Encyclopedia Version` (annotation TSV only) | ### Biosample @@ -161,6 +201,9 @@ One biosample per file, nested inside the collection: | `anatomy` | `Biosample term id` + `Biosample term name` | `{id, name}` object | | `subjects[]` | `Donor(s)` | Same subjects as collection | | `extra.encode.biosample_type` | `Biosample type` | e.g., `"primary cell"`, `"tissue"`, `"cell line"` | +| `extra.encode.life_stage` | `Life stage` | Annotation TSV only. e.g., `"embryonic"`, `"adult"`, `"young adult"`, `"unknown"` | +| `extra.encode.age` | `Age` | Annotation TSV only. Kept as the upstream string — the released corpus contains `"2-4"` and `"unknown"` alongside decimals, and distinguishes `"10.5"` from `"10.50"` | +| `extra.encode.age_units` | `Age units` | Annotation TSV only. `year` / `month` / `week` / `day`; blank when `age` is absent or a sentinel | | `extra.encode.biosample_treatments` | `Biosample treatments` | Treatment details | | `extra.encode.biosample_treatments_amount` | `Biosample treatments amount` | Dosage | | `extra.encode.biosample_treatments_duration` | `Biosample treatments duration` | Duration | @@ -215,8 +258,10 @@ All stored on `file.extra.encode` (`EnrichedEncodeFile`). Every field is `Option |------------|-------------------| | `extra.encode.genome_annotation` | `Genome annotation` | | `extra.encode.controlled_by` | `Controlled by` | -| `extra.encode.s3_uri` | `s3_uri` | +| `extra.encode.s3_uri` | `s3_uri` (annotation TSV: `S3 URL`) | | `extra.encode.azure_url` | `Azure URL` | +| `extra.encode.organism` | `Biosample organism` (annotation TSV: `Organism`) | +| `extra.encode.annotation_type` | `Annotation type` (annotation TSV only) | ### Analysis Metadata @@ -233,6 +278,52 @@ All stored on `file.extra.encode` (`EnrichedEncodeFile`). Every field is `Option | `extra.encode.audit_not_compliant` | `Audit NOT_COMPLIANT` | | `extra.encode.audit_error` | `Audit ERROR` | +## Annotation Mapping + +Everything above applies to an annotation row too, except as stated here. `transform_annotation_to_c2m2` renames the annotation TSV's columns to their experiment equivalents, runs the shared transformation, then applies the annotation-only columns — so one mapping serves both TSVs. + +### Renamed columns + +Applied via `ANNOTATION_COLUMN_ALIASES` in `encode.py`. Verified against the live headers of both ingested types (both 32 columns, identical to each other). Every other shared column already agrees by name. + +| Annotation TSV | Experiment TSV | Lands on | +|---|---|---| +| `Dataset accession` | `Experiment accession` | `collections[].local_id`, `accession_id`, `name`, `persistent_id` | +| `Assay term name` | `Assay` | `assay_type`, `collections[].experiment_type` | +| `Assembly` | `File assembly` | `genome_assembly` | +| `Dataset date released` | `Experiment date released` | `creation_time` | +| `S3 URL` | `s3_uri` | `extra.encode.s3_uri` | +| `Organism` | `Biosample organism` | `extra.encode.organism`, `subjects[].taxonomy` | + +### Annotation-only columns + +| Annotation TSV | Lands on | +|---|---| +| `Annotation type` | `extra.encode.annotation_type` **and** `collections[].extra.encode.annotation_type` | +| `Software used` | `collections[].extra.encode.software_used` | +| `Encyclopedia Version` | `collections[].extra.encode.encyclopedia_version` | +| `Targets` | `collections[].experiment_target` | +| `Life stage` | `…biosamples[].extra.encode.life_stage`, when the row names a biosample | +| `Age` | `…biosamples[].extra.encode.age`, when the row names a biosample | +| `Age units` | `…biosamples[].extra.encode.age_units`, when the row names a biosample | + +`annotation_type` is stored on the file as well as the dataset. The dataset is where the property belongs, but filtering *files* by it is the actual use case — "give me the cCRE files" — and routing that through a collection subdocument on every query is not worth avoiding one duplicated string. + +`Targets` reuses the scalar `experiment_target` rather than adding a parallel list field; the TSV hands over a string either way. Note that ENCODE does not order it — both `"CTCF-human, H3K4me3-human"` and `"H3K4me3-human, CTCF-human"` occur in the released corpus — so an equality filter on the whole value matches one permutation only. + +`Life stage` / `Age` / `Age units` go to the biosample, not to `Subject.age_at_sampling`. They are biosample-scoped in ENCODE's own model (resolved from the donor upstream), which is why the annotation TSV publishes them despite having no `Donor(s)` column — and `age_at_sampling` is a float in years, which could represent neither the `"2-4"` ranges nor the `"unknown"` sentinels the corpus contains. + +The biosample is the destination, so a row that names no `Biosample term name` has nowhere to put them and they are dropped. That is exactly the row the annotation transform relaxed `require_biosample` for, so the caveat matters more here than on the experiment path: an annotation row with donor traits but no biosample loses them. Nothing was lost against the corpus as last checked — of the biosample-less rows in the two configured annotation types, none published any of the three — and inventing a biosample to hold them would be worse than dropping them. `test_transform_annotation_to_c2m2_should_drop_donor_traits_with_no_biosample` pins the behavior. + +### Experiment-only columns + +Absent from the annotation TSV, and therefore **unset** on annotation documents rather than derived: all eight `Library *`, all six `Biosample genetic modifications *`, `Biosample treatments *`, `Biological`/`Technical replicate(s)`, `Donor(s)`, `Experiment target`, `File analysis title`/`status`, `File format type`, `Genome annotation`, `Index of`, `Read length`, `Mapped read length`, `Run type`, `Paired end`, `Paired with`, `Platform`, `RBNS protein concentration`, `Controlled by`. + +Two consequences worth naming: + +- **No subjects.** With no `Donor(s)` column there is nothing to key a `Subject` on, so annotation documents carry `collections[].subjects == []` and no donor is fabricated. The organism that would have reached `subjects[].taxonomy` is on `extra.encode.organism` instead — which is the only way the organism of a multi-organism result set (the released cCREs span *Homo sapiens* and *Mus musculus*) is recoverable without inspecting filenames. +- **The dataset does not require a biosample.** Unlike the experiment path, the annotation collection is built from `Dataset accession` alone. 48 released cCRE files name no biosample term; gating on one would leave 24 dataset accessions unqueryable. Its `persistent_id` points at `/annotations/`, not `/experiments/`. + ## Sync Flow ENCODE sync bypasses the C2M2 ZIP pipeline entirely. Files are pre-materialized during ingest. @@ -243,19 +334,37 @@ ENCODE sync bypasses the C2M2 ZIP pipeline entirely. Files are pre-materialized ### Data Flow +`_sync_encode` runs one phase for released experiments plus one per configured `annotation_type`: + ```text -fetch_encode_metadata() # Streaming TSV from ENCODE API - │ # Yields one dict per row +_sync_encode() + ├─ clear ENCODE data + upsert the DCC record (inside the cutover lock) + │ + ├─ phase "experiment" + │ fetch_encode_metadata() # Streaming TSV from ENCODE API + │ └─> transform_to_c2m2(row) # Yields one dict per row + │ + ├─ phase "annotation[]" × one per configured type + │ fetch_encode_annotation_metadata(type) + │ └─> transform_annotation_to_c2m2(row) + │ └─ rename columns, then the shared transformation │ - └─> transform_to_c2m2(row) # Per-row transformation + └─ each transformation: ├─ Map File format -> EDAM # ontology_mappings.get_file_format() ├─ Derive compression -> EDAM # from the download URL's suffix ├─ Map Output type -> EDAM # ontology_mappings.get_data_type() ├─ Map Assay -> OBI # ontology_mappings.get_assay_type() ├─ Map Organism -> NCBI # ontology_mappings.get_taxonomy() ├─ Build collection + biosample + subjects inline - ├─ Build extra.encode dict (21 file fields) └─ Insert into files collection (batches of 1000) ``` +**Phase isolation.** A phase that raises is logged and recorded, and the remaining phases still run — a transient failure on one stream does not cost the corpus the others. A stream that dies *mid-flight* (the shape an `asyncio.TimeoutError` against the metadata budget takes) has its trailing partial batch committed rather than discarded, and still reports the rows it loaded, so the per-phase counts, the summary tallies and the collection all agree. The indexes are ensured unconditionally afterwards, so whatever did load is servable without a full collection scan. The sync then fails, naming the phases that broke: reporting a clean sync over a partially loaded collection would be worse than a visible failure. + +**Each phase replaces only its own slice.** `files` is not cleared corpus-wide before the fan-out. Each phase owns a slice — experiments are the documents with no `extra.encode.annotation_type`, each annotation phase the documents carrying its type — and deletes that slice only once it has replacement rows in hand. A phase that fails before delivering any row, the shape a portal 504 or an exhausted download budget takes, therefore leaves its previous rows being served instead of emptying them; clearing up front meant a failed experiment phase left the API serving the ~29k annotation documents as the entire corpus. A stream that drains cleanly with no rows is the opposite case and does clear, so a type ENCODE stopped publishing stops being served. A phase that dies partway through still leaves its own slice partially loaded — only loading into a shadow collection and swapping on success would close that, and it is not part of this change. + +**One budget for the fan-out.** `ENCODE_METADATA_TIMEOUT_SECONDS` (default 3600) bounds the whole sync, not each stream within it. The budget is resolved once and every phase shares a single deadline, each receiving whatever remains; a phase that finds the budget spent fails immediately rather than opening a request it has no time to finish. Per-stream budgets would have multiplied the ceiling by the number of phases — three by default, and unbounded as the allowlist grows — while the whole fan-out sits inside the cutover lock that gates the read surface. That matters beyond availability: the sync lock treats itself as abandoned after one hour (`STALE_LOCK_THRESHOLD`) with nothing refreshing `started_at`, so a run that outlives the threshold admits a second sync, which clears the ENCODE corpus while the first is still inserting into it. + +**Queryability.** `extra.encode.annotation_type` and `extra.encode.organism` are indexed by `materialized_files_index_specs()` — the ENCODE sync writes `files` directly and never reaches the Rust materializer that owns the rest, so nothing else would create them — and both are in `ALLOWED_DISTINCT_FIELDS`, so a client can enumerate the available annotation types rather than having to know ENCODE's exact spelling in advance. + No post-ingest enrichment pass. All metadata is captured during the initial TSV transformation. diff --git a/README.md b/README.md index 06445b8..1452b2c 100644 --- a/README.md +++ b/README.md @@ -560,11 +560,13 @@ Anatomy, FileFormat, DataType, and AssayType share an identical schema: `id` (st | Entity | Ontology Source | |--------|----------------| | Anatomy | UBERON (Uber-anatomy ontology) | -| FileFormat | EDAM CV `format:` terms | +| FileFormat | EDAM CV `format:` terms, or a `cfdb:`-prefixed token minted where EDAM has none | | DataType | EDAM CV `data:` terms | | AssayType | OBI (Ontology for Biomedical Investigations) | | NcbiTaxonomy | NCBI Taxonomy Database | +Every `id` above resolves in its source ontology except the minted `FileFormat` tokens. EDAM has no term for a few formats cfdb ingests — `bedpe` and `bigInteract` at present — and aliasing them onto the nearest EDAM term would make each indistinguishable from the format it was aliased to, so a `cfdb:` token is minted instead. A client resolving `file_format.id` against EDAM should skip ids carrying that prefix rather than treat them as resolvable. + #### Subject A human or organism from which biosamples are derived. diff --git a/schema.graphql b/schema.graphql index 079dcaf..7497264 100644 --- a/schema.graphql +++ b/schema.graphql @@ -156,6 +156,9 @@ type EnrichedCollectionType { input EnrichedEncodeBiosampleInput { biosampleType: [String!] = null + lifeStage: [String!] = null + age: [String!] = null + ageUnits: [String!] = null biosampleTreatments: [String!] = null biosampleTreatmentsAmount: [String!] = null biosampleTreatmentsDuration: [String!] = null @@ -172,6 +175,9 @@ input EnrichedEncodeBiosampleInput { type EnrichedEncodeBiosampleType { biosampleType: String + lifeStage: String + age: String + ageUnits: String biosampleTreatments: String biosampleTreatmentsAmount: String biosampleTreatmentsDuration: String @@ -192,6 +198,9 @@ input EnrichedEncodeCollectionInput { platform: [String!] = null dbxrefs: [String!] = null rbnsProteinConcentration: [String!] = null + annotationType: [String!] = null + softwareUsed: [String!] = null + encyclopediaVersion: [String!] = null } type EnrichedEncodeCollectionType { @@ -199,10 +208,15 @@ type EnrichedEncodeCollectionType { platform: String dbxrefs: String rbnsProteinConcentration: String + annotationType: String + softwareUsed: String + encyclopediaVersion: String } input EnrichedEncodeFileInput { assembly: [String!] = null + annotationType: [String!] = null + organism: [String!] = null fileFormatType: [String!] = null outputType: [String!] = null genomeAnnotation: [String!] = null @@ -225,6 +239,8 @@ input EnrichedEncodeFileInput { type EnrichedEncodeFileType { assembly: String + annotationType: String + organism: String fileFormatType: String outputType: String genomeAnnotation: String diff --git a/src/cfdb/api/gql/inputs.py b/src/cfdb/api/gql/inputs.py index 2c1189d..fb9871a 100644 --- a/src/cfdb/api/gql/inputs.py +++ b/src/cfdb/api/gql/inputs.py @@ -70,6 +70,9 @@ class SubjectInput: @strawberry.input class EnrichedEncodeBiosampleInput: biosample_type: list[str] | None = None + life_stage: list[str] | None = None + age: list[str] | None = None + age_units: list[str] | None = None biosample_treatments: list[str] | None = None biosample_treatments_amount: list[str] | None = None biosample_treatments_duration: list[str] | None = None @@ -134,6 +137,9 @@ class EnrichedEncodeCollectionInput: platform: list[str] | None = None dbxrefs: list[str] | None = None rbns_protein_concentration: list[str] | None = None + annotation_type: list[str] | None = None + software_used: list[str] | None = None + encyclopedia_version: list[str] | None = None @strawberry.input @@ -216,6 +222,8 @@ class EnrichedFourdnFileInput: @strawberry.input class EnrichedEncodeFileInput: assembly: list[str] | None = None + annotation_type: list[str] | None = None + organism: list[str] | None = None file_format_type: list[str] | None = None output_type: list[str] | None = None genome_annotation: list[str] | None = None diff --git a/src/cfdb/api/gql/schema.py b/src/cfdb/api/gql/schema.py index e5eef05..8402a38 100644 --- a/src/cfdb/api/gql/schema.py +++ b/src/cfdb/api/gql/schema.py @@ -78,6 +78,18 @@ def process_errors( "output_type", "status", "data_access_level", + # ENCODE annotation facets. All three are small closed vocabularies + # -- a handful of annotation types, organisms and assemblies -- + # which is what this allowlist is for; unlike accession_id, + # deliberately absent because enumerating it would return the whole + # corpus. Without annotation_type here a client can discover which + # assemblies exist but not which annotation types do, which is the + # facet the annotation ingest exists to expose (issue #94). + # ``assembly`` mirrors the core ``genome_assembly`` above; both are + # written by the ENCODE ingest and either may be filtered on. + "extra.encode.annotation_type", + "extra.encode.organism", + "extra.encode.assembly", } ) diff --git a/src/cfdb/indexes.py b/src/cfdb/indexes.py index 74f534f..5e0cedd 100644 --- a/src/cfdb/indexes.py +++ b/src/cfdb/indexes.py @@ -173,10 +173,30 @@ def materialized_files_index_specs() -> list[IndexSpec]: collection on a public endpoint. Ensuring just the accession keys there costs nothing when the materializer has already created them: identical keys derive identical default names, so the create is a no-op. + + The ``extra.encode`` keys are here for the same reason and are likewise + not the materializer's: they are written only by the ENCODE ingest, and + ``annotation_type`` in particular is the filter the whole annotation + corpus is meant to be reached through (issue #94). Without an index that + is a full scan of ~300k documents on an unauthenticated endpoint. + + ``genome_assembly`` is the exception to "not the materializer's": it is + a core field every DCC populates and the materializer does index it. + It is repeated here because an ENCODE-only database never runs the + materializer, and assembly is the other half of the same acceptance + criterion as organism -- narrowing to GRCh38 is the first move a client + makes. The `extra.encode.assembly` mirror is indexed alongside it since + the ENCODE ingest writes both and either may be filtered on. Repeating + a key the materializer also creates costs nothing: identical keys derive + identical default names, so the create is a no-op. """ return [ IndexSpec("files", [("accession_id", 1)]), IndexSpec("files", [("collections.accession_id", 1)]), + IndexSpec("files", [("extra.encode.annotation_type", 1)]), + IndexSpec("files", [("extra.encode.organism", 1)]), + IndexSpec("files", [("extra.encode.assembly", 1)]), + IndexSpec("files", [("genome_assembly", 1)]), ] diff --git a/src/cfdb/models.py b/src/cfdb/models.py index 73ad107..bb7d09d 100644 --- a/src/cfdb/models.py +++ b/src/cfdb/models.py @@ -130,9 +130,33 @@ class EnrichedFourdnFile(BaseModel): class EnrichedEncodeFile(BaseModel): - """ENCODE file-level metadata from metadata TSV.""" + """ENCODE file-level metadata from metadata TSV. + + Populated from either the Experiment or the Annotation metadata TSV. + The two share only part of their column sets, so a field sourced from a + column the other TSV does not publish is None on those documents rather + than derived from something else -- see the annotation mapping in + :mod:`cfdb.services.encode`. + + Attributes: + annotation_type: + The kind of annotation this file belongs to (e.g. "candidate + Cis-Regulatory Elements"). The field that gives an annotation + file its meaning, and the one a client filters on to ask for + cCREs without string-matching filenames. None on experiment + files, whose TSV has no such column. + + organism: + Scientific name of the source organism (e.g. "Homo sapiens"). + Also reaches ``subjects[].taxonomy`` on experiment files, but + annotation rows name no donor and so build no subject -- this is + the only place the organism of a multi-organism annotation + result set is queryable. + """ assembly: Optional[str] = None + annotation_type: Optional[str] = None + organism: Optional[str] = None file_format_type: Optional[str] = None output_type: Optional[str] = None genome_annotation: Optional[str] = None @@ -227,12 +251,37 @@ def empty_string_to_none(cls, v): class EnrichedEncodeCollection(BaseModel): - """ENCODE experiment-level metadata from metadata TSV.""" + """ENCODE dataset-level metadata from metadata TSV. + + A "dataset" is an Experiment or an Annotation depending on which TSV the + document came from; ``platform`` and ``rbns_protein_concentration`` are + experiment-only, ``annotation_type``, ``software_used`` and + ``encyclopedia_version`` annotation-only. + + Attributes: + annotation_type: + The annotation kind this dataset publishes, mirroring + :attr:`EnrichedEncodeFile.annotation_type`. Held on the dataset + as well as the file because that is the entity the property + actually describes. + + software_used: + Software that produced the annotation, as a comma-separated + list (e.g. "ABC-Enhancer-Gene-Prediction, Distal regulation + ENCODE-rE2G"). Blank for some annotation types. + + encyclopedia_version: + The ENCODE Encyclopedia release the annotation belongs to (e.g. + "ENCODE v4", "ENCODE v3, current"). + """ project: Optional[str] = None platform: Optional[str] = None dbxrefs: Optional[str] = None rbns_protein_concentration: Optional[str] = None + annotation_type: Optional[str] = None + software_used: Optional[str] = None + encyclopedia_version: Optional[str] = None class EnrichedFourdnCollection(BaseModel): @@ -298,9 +347,31 @@ class EnrichedEncodeBiosample(BaseModel): ENCODE biosample-level metadata from metadata TSV. Contains biosample classification, treatment, and library information. + + Attributes: + life_stage: + Developmental stage of the source organism when the biosample + was taken (e.g. "embryonic", "adult", "young adult", "unknown"). + + age: + Age of the source organism at sampling, in ``age_units``, kept + as the upstream string. NOT parsed to a number: the released + annotation corpus contains "2-4" and "unknown" alongside plain + decimals, and it distinguishes "10.5" from "10.50". This is also + why the value does not go to :attr:`Subject.age_at_sampling`, + which is a float in years -- that field could represent neither + the ranges nor the sentinels, and an annotation row names no + donor to build a Subject from in the first place. + + age_units: + Unit for :attr:`age` ("year", "month", "week", "day"). Blank + when ``age`` is absent or a sentinel. """ biosample_type: Optional[str] = None + life_stage: Optional[str] = None + age: Optional[str] = None + age_units: Optional[str] = None biosample_treatments: Optional[str] = None biosample_treatments_amount: Optional[str] = None biosample_treatments_duration: Optional[str] = None @@ -393,7 +464,9 @@ class FileMetadataModel(BaseModel): file_format: An EDAM CV term identifying the digital format of this file - (e.g., TSV or FASTQ). If compressed, this is the uncompressed format. + (e.g., TSV or FASTQ), or a ``cfdb:``-prefixed token where EDAM + has no term for it; see :class:`FileFormat`. If compressed, + this is the uncompressed format. compression_format: An EDAM CV term ID identifying compression that is extrinsic to @@ -571,20 +644,34 @@ class AssayType(BaseModel): class FileFormat(BaseModel): """ - An EDAM CV 'format:' term. + An EDAM CV 'format:' term, or a term minted here where EDAM has none. Describes the digital format of C2M2 files. + Most ids are EDAM CV ``format:`` terms and resolve at edamontology.org. + A few formats EDAM does not cover -- ``bedpe`` and ``bigInteract`` at + the time of writing -- carry a token minted here instead, prefixed + ``cfdb:`` (:data:`cfdb.services.ontology_mappings.MINTED_FORMAT_PREFIX`, + the stable discriminator to test against). Those ids resolve nowhere: + aliasing such a format onto the nearest EDAM term would make it + indistinguishable from the format it was aliased to, and the processor + claiming that term would pick it up and mangle it. A consumer resolving + ids against EDAM must therefore skip the minted prefix rather than + assume every id is resolvable. + Attributes: id: - An EDAM CV format term identifier. + An EDAM CV format term identifier, or a ``cfdb:``-prefixed + token minted where EDAM has no term for the format. name: - A short, human-readable, machine-read-friendly label for this EDAM - format term. + A short, human-readable, machine-read-friendly label for this + format term. Distinct per format even where the id is minted: + workflow processor routing keys on this field, so two formats + sharing a name share a pipeline. description: - A human-readable description of this EDAM format term. + A human-readable description of this format term. """ id: str = str() diff --git a/src/cfdb/services/encode.py b/src/cfdb/services/encode.py index 5dbafed..f591aca 100644 --- a/src/cfdb/services/encode.py +++ b/src/cfdb/services/encode.py @@ -1,11 +1,15 @@ """ENCODE metadata TSV client and CFDB transformation service. -Fetches the released-experiment metadata TSV from ENCODE and transforms -each row into a CFDB file document. +Fetches the released-experiment and released-annotation metadata TSVs from +ENCODE and transforms each row into a CFDB file document. -Metadata URL ------------- +Metadata URLs +------------- https://www.encodeproject.org/metadata/?type=Experiment&status=released +https://www.encodeproject.org/metadata/?type=Annotation&status=released&annotation_type= + +The annotation URL is requested once per configured ``annotation_type`` +(``ENCODE_ANNOTATION_TYPES``; see :func:`annotation_types_from_env`). Field Mapping (ENCODE TSV → CFDB) ---------------------------------- @@ -43,6 +47,7 @@ Controlled by → extra.encode.controlled_by s3_uri → extra.encode.s3_uri Azure URL → extra.encode.azure_url +Biosample organism → extra.encode.organism File analysis title → extra.encode.file_analysis_title File analysis status → extra.encode.file_analysis_status Audit WARNING → extra.encode.audit_warning @@ -100,14 +105,72 @@ dcc.dcc_description, dcc.contact_email, dcc.contact_name, dcc.dcc_url, dcc.project_id_namespace, dcc.project_local_id + + +Annotation TSV +-------------- + +The Annotation TSV publishes 32 columns to the Experiment TSV's 59, and the +overlap is partial. Everything above applies to an annotation row too, except +as stated here. + +Renamed columns (annotation name → the experiment name the mapping uses). +Applied by ``transform_annotation_to_c2m2`` via ANNOTATION_COLUMN_ALIASES +before the shared transformation, so the table above covers both TSVs: + +Dataset accession → Experiment accession +Assay term name → Assay +Assembly → File assembly +Dataset date released → Experiment date released +S3 URL → s3_uri +Organism → Biosample organism + +Annotation-only columns: + +Annotation type → extra.encode.annotation_type, and + collections[].extra.encode.annotation_type +Software used → collections[].extra.encode.software_used +Encyclopedia Version → collections[].extra.encode.encyclopedia_version +Targets → collections[].experiment_target (reuses the + scalar; ENCODE does not order the value, so + an equality filter matches one permutation) +Life stage → …biosamples[].extra.encode.life_stage +Age → …biosamples[].extra.encode.age +Age units → …biosamples[].extra.encode.age_units + +The three donor traits land on the biosample, so a row naming no +``Biosample term name`` -- the row the annotation transform relaxed +``require_biosample`` for -- has nowhere to put them and drops them. +No biosample-less row in the configured annotation types publishes any +of the three today, and inventing a biosample to hold them would be +worse than dropping them. + +Columns the Annotation TSV does not publish, left unset rather than derived: +all eight ``Library *``, all six ``Biosample genetic modifications *``, +``Biosample treatments *``, ``Biological/Technical replicate(s)``, +``Donor(s)``, ``Experiment target``, ``File analysis title``/``status``, +``File format type``, ``Genome annotation``, ``Index of``, ``Read length``, +``Mapped read length``, ``Run type``, ``Paired end``, ``Paired with``, +``Platform``, ``RBNS protein concentration``, ``Controlled by``. + +Two consequences worth naming: + +* No ``Donor(s)`` column means no subjects are built, so an annotation + document has ``collections[].subjects == []`` and no donor is invented. + The organism it would have carried is on ``extra.encode.organism``. +* The dataset collection is built from ``Dataset accession`` alone, without + requiring a biosample term -- 48 released cCRE files name no biosample, + and gating on one would make 24 dataset accessions unqueryable. Its + ``persistent_id`` points at ``/annotations/``, not ``/experiments/``. """ import asyncio import logging import os import re +from contextlib import aclosing from typing import AsyncGenerator, Optional -from urllib.parse import unquote, urlsplit +from urllib.parse import unquote, urlencode, urlsplit import aiohttp @@ -151,27 +214,121 @@ def _timeout_from_env(name: str, default: int) -> int: #: a malformed value should fail the sync that reads it, not the import of #: this module, which would take the whole API down over a knob only the #: sync uses. +#: +#: The budget covers *the whole sync*, not each stream in it. A sync runs one +#: stream per configured ``annotation_type`` plus one for experiments, all +#: inside a single cutover lock that gates the read surface, so a per-stream +#: budget would multiply the outage by the number of phases and put the worst +#: case past the sync lock's one-hour stale threshold -- at which point a +#: second sync is admitted and clears the corpus while the first is still +#: writing into it. Callers share one deadline; see +#: :func:`metadata_budget_seconds`. _METADATA_TIMEOUT_ENV = "ENCODE_METADATA_TIMEOUT_SECONDS" _METADATA_TIMEOUT_DEFAULT_SECONDS = 3600 -async def fetch_encode_metadata() -> AsyncGenerator[dict, None]: +def metadata_budget_seconds() -> int: + """Resolve the whole-sync metadata download budget, in seconds. + + Exposed so a sync running several streams resolves the budget once and + shares one deadline across them, rather than granting each stream its + own full allowance. """ - Fetch all released experiment files from ENCODE metadata TSV endpoint. + return _timeout_from_env( + _METADATA_TIMEOUT_ENV, _METADATA_TIMEOUT_DEFAULT_SECONDS + ) - Uses the /metadata/ endpoint which returns a single TSV file containing - all matching records, avoiding the need for paginated JSON API calls. - The response is streamed line-by-line to avoid loading the full TSV - (hundreds of MB) into memory. +#: Environment variable overriding which ``annotation_type`` values are +#: ingested, and the allowlist applied when it is unset. An allowlist rather +#: than a filter retrofitted later: ENCODE publishes 580,910 annotation +#: datasets and 86% of them are footprints, so ingesting the type space +#: wholesale and pruning afterwards would be a corpus-scale mistake to undo. +#: The two defaults are ~7,773 datasets, proportionate against the 27,043 +#: experiments already ingested. +_ANNOTATION_TYPES_ENV = "ENCODE_ANNOTATION_TYPES" +_ANNOTATION_TYPES_DEFAULT: tuple[str, ...] = ( + "candidate Cis-Regulatory Elements", + "element gene regulatory interaction predictions", +) - Yields: - Dicts keyed by TSV column names for each file row + +def annotation_types_from_env() -> tuple[str, ...]: + """Resolve the ``annotation_type`` allowlist to ingest. + + Reads a comma-separated ``ENCODE_ANNOTATION_TYPES``, stripping each + entry and dropping blanks. An unset variable yields the default + allowlist; a variable set to an empty (or all-blank) value yields an + empty tuple, which disables annotation ingest entirely. That distinction + is deliberate -- turning the annotation path off is a legitimate + operator choice, and having to edit code to make it is worse than + honoring an explicitly empty setting. It is also the opposite of what + :func:`_timeout_from_env` does with an empty value, so the empty case + logs a warning rather than leaving the operator to infer it from an + absence in the per-phase log. + + Repeats are collapsed, first occurrence winning, because the sync runs + one ingest phase per returned entry and ENCODE files are written with + ``insert_many`` into a collection with no unique key: a duplicated token + in a task definition would otherwise insert every file of that type + twice, with nothing to reject it and no way back short of a full + re-sync. + + Resolved per call rather than at import, matching + :func:`_timeout_from_env`: the value is only meaningful to the sync, so + a bad one should fail the sync rather than the API's import of this + module. """ - config = get_dcc_config("encode") - api_base = config["api_base"] + raw = os.getenv(_ANNOTATION_TYPES_ENV) + if raw is None: + return _ANNOTATION_TYPES_DEFAULT + entries = (entry.strip() for entry in raw.split(",")) + # dict.fromkeys rather than a set: order is the ingest order, and a + # reordered allowlist would shuffle the log and the phase sequence. + allowlist = tuple(dict.fromkeys(entry for entry in entries if entry)) + + if not allowlist: + # Warned about because the neighbouring :func:`_timeout_from_env` + # reads an empty value as "unset, use the default" and this one + # reads it as "ingest nothing" -- a difference an operator has no + # reason to expect between two variables of the same module. Empty + # values arrive by accident routinely: an unset CloudFormation + # parameter, a docker-compose ``${VAR}`` that expanded to nothing, + # an ECS task definition entry with an empty ``value``. Disabling + # the annotation ingest is a legitimate choice, so this stays a + # warning rather than an error -- but silently loading no + # annotations shows up only as an absence in the per-phase log, + # and an absence is what nobody notices. + logger.warning( + "%s is set but empty: annotation ingest is disabled and no " + "annotation files will be loaded. Unset it entirely to restore " + "the default allowlist (%s).", + _ANNOTATION_TYPES_ENV, + ", ".join(_ANNOTATION_TYPES_DEFAULT), + ) - metadata_url = f"{api_base}/metadata/?type=Experiment&status=released" + return allowlist + + +async def _stream_metadata_tsv( + metadata_url: str, label: str, deadline: float | None = None +) -> AsyncGenerator[dict, None]: + """Stream one ENCODE ``/metadata/`` TSV, yielding a dict per data row. + + ``label`` names the stream in log lines so that, with several streams + per sync, a progress or timeout message identifies which one it came + from. + Args: + metadata_url: Fully-formed ``/metadata/`` URL to stream. + label: Human-readable name for this stream, used in log messages. + deadline: Event-loop clock reading (:meth:`asyncio.AbstractEventLoop.time`) + by which this stream must finish, shared with every other stream + in the same sync. ``None`` grants a fresh full budget, which is + correct only for a stream running on its own. + + Yields: + Dicts keyed by TSV column names for each file row + """ headers = { "User-Agent": "cfdb/1.0", } @@ -184,9 +341,21 @@ async def fetch_encode_metadata() -> AsyncGenerator[dict, None]: # was not enough against DocumentDB -- the sync aborted around 230,000 of # ~810,000 rows and, because the DCC is cleared before reloading, left the # corpus smaller than it started. - timeout_seconds = _timeout_from_env( - _METADATA_TIMEOUT_ENV, _METADATA_TIMEOUT_DEFAULT_SECONDS - ) + timeout_seconds: float + if deadline is None: + timeout_seconds = metadata_budget_seconds() + else: + timeout_seconds = deadline - asyncio.get_running_loop().time() + if timeout_seconds <= 0: + # Refused rather than clamped to zero: aiohttp reads a + # non-positive total as "no timeout", so passing the exhausted + # remainder through would turn a spent budget into an unbounded + # request -- the exact failure the budget exists to prevent. + raise asyncio.TimeoutError( + f"ENCODE {label} metadata fetch not attempted: the " + f"{_METADATA_TIMEOUT_ENV} budget was already spent by an " + "earlier phase of this sync" + ) async with aiohttp.ClientSession() as session: try: @@ -196,9 +365,13 @@ async def fetch_encode_metadata() -> AsyncGenerator[dict, None]: timeout=aiohttp.ClientTimeout(total=timeout_seconds), ) as response: if response.status != 200: - logger.error(f"ENCODE metadata API error: HTTP {response.status}") + logger.error( + f"ENCODE {label} metadata API error: " + f"HTTP {response.status}" + ) raise Exception( - f"ENCODE metadata API error: HTTP {response.status}" + f"ENCODE {label} metadata API error: " + f"HTTP {response.status}" ) # Stream line-by-line to keep memory usage constant @@ -219,11 +392,13 @@ async def fetch_encode_metadata() -> AsyncGenerator[dict, None]: if total_fetched % 50000 == 0: logger.info( - f"Parsed {total_fetched} ENCODE metadata rows..." + f"Parsed {total_fetched} ENCODE {label} " + "metadata rows..." ) logger.info( - f"ENCODE metadata fetch complete: {total_fetched} rows" + f"ENCODE {label} metadata fetch complete: " + f"{total_fetched} rows" ) except asyncio.TimeoutError: @@ -233,16 +408,105 @@ async def fetch_encode_metadata() -> AsyncGenerator[dict, None]: # is the one fact that distinguishes "the budget is too small" # from "the endpoint is down". logger.error( - f"ENCODE metadata fetch timed out after {total_fetched} rows " - f"({timeout_seconds}s budget); the files collection is left " - f"partially loaded. Raise {_METADATA_TIMEOUT_ENV} and re-run " - "the sync." + f"ENCODE {label} metadata fetch timed out after " + f"{total_fetched} rows ({timeout_seconds:.0f}s of the " + f"{_METADATA_TIMEOUT_ENV} budget remained when it started); " + "the files collection is left partially loaded. Raise " + f"{_METADATA_TIMEOUT_ENV} and re-run the sync." ) raise except aiohttp.ClientError as e: - logger.error(f"ENCODE metadata API network error: {e}") - raise Exception(f"ENCODE metadata API network error: {e}") + logger.error(f"ENCODE {label} metadata API network error: {e}") + raise Exception(f"ENCODE {label} metadata API network error: {e}") + + +async def fetch_encode_metadata( + deadline: float | None = None, +) -> AsyncGenerator[dict, None]: + """ + Fetch all released experiment files from ENCODE metadata TSV endpoint. + + Uses the /metadata/ endpoint which returns a single TSV file containing + all matching records, avoiding the need for paginated JSON API calls. + The response is streamed line-by-line to avoid loading the full TSV + (hundreds of MB) into memory. + + Args: + deadline: Event-loop clock reading by which this fetch must finish, + shared with the other streams of the same sync. ``None`` grants + a fresh full budget; see :func:`metadata_budget_seconds`. + + Yields: + Dicts keyed by TSV column names for each file row + """ + config = get_dcc_config("encode") + api_base = config["api_base"] + + metadata_url = f"{api_base}/metadata/?type=Experiment&status=released" + + # ``aclosing`` because ``async for`` does not close the iterator it + # abandons: without it, closing this generator raises GeneratorExit here + # and leaves the inner one -- which owns the aiohttp session -- suspended + # at its yield with the session still open. + async with aclosing( + _stream_metadata_tsv(metadata_url, "experiment", deadline) + ) as stream: + async for row in stream: + yield row + + +async def fetch_encode_annotation_metadata( + annotation_type: str, + deadline: float | None = None, +) -> AsyncGenerator[dict, None]: + """ + Fetch all released annotation files of one ``annotation_type``. + + The same ``/metadata/`` endpoint as the experiment fetch, against + ``type=Annotation``. The two TSVs do not share a column set -- Annotation + publishes 32 columns to Experiment's 59, with six of the shared ones + renamed -- so rows from here must go through + :func:`transform_annotation_to_c2m2`, not :func:`transform_to_c2m2`. + + One request per annotation type, rather than one request carrying + repeated ``annotation_type`` parameters, so that a failure or a timeout + on one type costs only that type. It also keeps each response small + enough to come back: the portal returns 504 on requests that take too + long to assemble. + + Args: + annotation_type: An ENCODE ``annotation_type`` value, e.g. + "candidate Cis-Regulatory Elements". Passed through URL + encoding, so spaces and punctuation need no pre-escaping. + deadline: Event-loop clock reading by which this fetch must finish, + shared with the other streams of the same sync. ``None`` grants + a fresh full budget; see :func:`metadata_budget_seconds`. + + Yields: + Dicts keyed by TSV column names for each file row + """ + config = get_dcc_config("encode") + api_base = config["api_base"] + + query = urlencode( + { + "type": "Annotation", + "status": "released", + "annotation_type": annotation_type, + } + ) + metadata_url = f"{api_base}/metadata/?{query}" + + # See fetch_encode_metadata for why the inner stream is closed + # explicitly rather than left to ``async for``. + async with aclosing( + _stream_metadata_tsv( + metadata_url, f"annotation[{annotation_type}]", deadline + ) + ) as stream: + async for row in stream: + yield row def _nonempty(value: str | None) -> str | None: @@ -380,12 +644,45 @@ def _extract_donor_ids(donors_str: str | None) -> list[str]: return ids -def transform_to_c2m2(row: dict) -> Optional[dict]: +# Annotation TSV column -> the Experiment TSV column holding the same datum. +# Verified against the live headers of both ingested annotation types (both +# 32 columns, identical to each other). Applied by +# :func:`transform_annotation_to_c2m2` before the shared transformation, so +# that a rename is the only thing standing between the two TSVs and one +# mapping serves both. Every other shared column already agrees by name. +ANNOTATION_COLUMN_ALIASES = { + "Dataset accession": "Experiment accession", + "Assay term name": "Assay", + "Assembly": "File assembly", + "Dataset date released": "Experiment date released", + "S3 URL": "s3_uri", + "Organism": "Biosample organism", +} + + +def _transform_row( + row: dict, *, dataset_path: str, require_biosample: bool +) -> Optional[dict]: """ Transform an ENCODE metadata TSV row to C2M2-compatible document for MongoDB. + Shared by the experiment and annotation ingest paths. Every field is read + under its *experiment* column name; the annotation path renames its + columns to match before calling here (see + :data:`ANNOTATION_COLUMN_ALIASES`). A column the caller's TSV does not + publish is simply absent from ``row``, so the corresponding field comes + out unset rather than derived from something else. + Args: row: Dict keyed by TSV column names + dataset_path: Path segment for the collection's persistent_id -- + "experiments" or "annotations". ENCODE serves the two dataset + kinds under different URLs, and a link to the wrong one 404s. + require_biosample: When True, a row with no ``Biosample term name`` + produces no collection at all -- preserving the experiment + path's long-standing behavior. When False, the collection is + built from the dataset accession alone and simply carries no + biosamples, so that the accession stays queryable. Returns: C2M2-compatible dict for insertion into files collection, or None if invalid @@ -474,10 +771,19 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: biosample_organism = _nonempty(row.get("Biosample organism")) donors_raw = _nonempty(row.get("Donor(s)")) - if biosample_term_name: + # A biosample can only be built from a biosample term. Whether its + # absence also suppresses the *collection* is the caller's call: 48 of + # the released cCRE files name no biosample term, and dropping their + # collection would make 24 dataset accessions unqueryable. + build_biosample = bool(biosample_term_name) + build_collection = build_biosample or ( + not require_biosample and bool(experiment_accession) + ) + + if build_collection: # Build anatomy from biosample term anatomy = None - if biosample_term_id: + if biosample_term_id and biosample_term_name: anatomy = {"id": biosample_term_id, "name": biosample_term_name} # Build subjects from donor(s) and organism @@ -501,6 +807,22 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: if biosample_type: biosample_extra["biosample_type"] = biosample_type + # Donor characteristics, which ENCODE resolves onto the biosample + # upstream -- which is why the annotation TSV publishes them despite + # having no Donor(s) column. Kept as the upstream strings; see + # EnrichedEncodeBiosample for why they are not parsed into + # Subject.age_at_sampling. + # + # Note these are attached only under ``if build_biosample`` below, + # so a row naming no biosample term parses them and drops them. + # Deliberate: the biosample is their destination and inventing one + # to hold them would be worse. No biosample-less row in the + # configured annotation types published any of the three when the + # released corpus was last checked. + _add_extra(biosample_extra, "life_stage", row.get("Life stage")) + _add_extra(biosample_extra, "age", row.get("Age")) + _add_extra(biosample_extra, "age_units", row.get("Age units")) + # Treatment fields treatments = _nonempty(row.get("Biosample treatments")) if treatments: @@ -555,19 +877,22 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: ) # Build biosample - biosample = { - "id_namespace": id_namespace, - "local_id": f"biosample:{biosample_term_name}", - "project_id_namespace": id_namespace, - "project_local_id": "ENCODE", - "subjects": subjects, - } - if anatomy: - biosample["anatomy"] = anatomy - if biosample_extra: - biosample["extra"] = {"encode": biosample_extra} + biosamples = [] + if build_biosample: + biosample = { + "id_namespace": id_namespace, + "local_id": f"biosample:{biosample_term_name}", + "project_id_namespace": id_namespace, + "project_local_id": "ENCODE", + "subjects": subjects, + } + if anatomy: + biosample["anatomy"] = anatomy + if biosample_extra: + biosample["extra"] = {"encode": biosample_extra} + biosamples.append(biosample) - # Build collection extra (experiment-level fields) + # Build collection extra (dataset-level fields) collection_encode_extra = {} _add_extra(collection_encode_extra, "project", row.get("Project")) _add_extra(collection_encode_extra, "platform", row.get("Platform")) @@ -578,12 +903,13 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: row.get("RBNS protein concentration"), ) - # Build collection — keyed by experiment accession, fallback to biosample + # Build collection — keyed by dataset accession, fallback to biosample if experiment_accession: collection_local_id = experiment_accession collection_name = experiment_accession collection_persistent_id = ( - f"https://www.encodeproject.org/experiments/{experiment_accession}/" + f"https://www.encodeproject.org/{dataset_path}/" + f"{experiment_accession}/" ) else: collection_local_id = f"biosample:{biosample_term_name}" @@ -594,12 +920,12 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: "id_namespace": id_namespace, "local_id": collection_local_id, "name": collection_name, - "biosamples": [biosample], + "biosamples": biosamples, "subjects": subjects, } - # Only the experiment-keyed branch has an accession. The + # Only the dataset-keyed branch has an accession. The # ``biosample:``-keyed fallback collection is synthesized locally and - # names no ENCODE experiment, so it is left unset rather than given a + # names no ENCODE dataset, so it is left unset rather than given a # fabricated accession. if experiment_accession: collection["accession_id"] = normalize_accession(experiment_accession) @@ -650,6 +976,23 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: _add_extra(extra, "s3_uri", row.get("s3_uri")) _add_extra(extra, "azure_url", row.get("Azure URL")) + # Mirrors the top-level genome_assembly under the DCC namespace, the way + # extra.fourdn.genome_assembly and extra.hubmap.genome_assembly mirror it + # for their DCCs. The field was declared and published in the SDL but + # never written, so a client reading the schema and reaching for + # extra.encode.assembly -- the natural move once cCREs made assembly a + # filter people actually use -- silently matched nothing. + _add_extra(extra, "assembly", row.get("File assembly")) + + # Organism, from the same column that feeds subjects[].taxonomy above. + # Recorded here as well because an annotation row names no donor, so it + # builds no subject -- without this, the organism of a multi-organism + # result set (the released cCREs span Homo sapiens and Mus musculus) + # would be recoverable only by inspecting filenames. Set on experiment + # rows too, from the identical datum, so the filter means the same thing + # across the whole corpus rather than only over annotations. + _add_extra(extra, "organism", row.get("Biosample organism")) + # Analysis metadata _add_extra(extra, "file_analysis_title", row.get("File analysis title")) _add_extra(extra, "file_analysis_status", row.get("File analysis status")) @@ -678,6 +1021,81 @@ def transform_to_c2m2(row: dict) -> Optional[dict]: return doc +def transform_to_c2m2(row: dict) -> Optional[dict]: + """ + Transform an ENCODE *experiment* metadata TSV row to a C2M2 document. + + Args: + row: Dict keyed by TSV column names, as yielded by + :func:`fetch_encode_metadata` + + Returns: + C2M2-compatible dict for insertion into files collection, or None if invalid + """ + return _transform_row(row, dataset_path="experiments", require_biosample=True) + + +def transform_annotation_to_c2m2(row: dict) -> Optional[dict]: + """ + Transform an ENCODE *annotation* metadata TSV row to a C2M2 document. + + Renames the annotation TSV's columns to their experiment equivalents + (:data:`ANNOTATION_COLUMN_ALIASES`), runs the shared transformation, then + applies the seven annotation-only columns. Experiment-only fields are + left unset rather than derived: the aliased row simply does not carry + those keys, so, for instance, ``_extract_donor_ids`` receives nothing and + the document ends up with no subjects rather than an invented donor. + + Args: + row: Dict keyed by TSV column names, as yielded by + :func:`fetch_encode_annotation_metadata` + + Returns: + C2M2-compatible dict for insertion into files collection, or None if invalid + """ + aliased = dict(row) + for annotation_column, experiment_column in ANNOTATION_COLUMN_ALIASES.items(): + if annotation_column in aliased: + aliased[experiment_column] = aliased.pop(annotation_column) + + doc = _transform_row( + aliased, dataset_path="annotations", require_biosample=False + ) + if doc is None: + return None + + annotation_type = _nonempty(row.get("Annotation type")) + + # On the file as well as the dataset. The dataset is where the property + # belongs, but filtering files by annotation_type is the actual use case + # -- "give me the cCRE files" -- and routing that through a collection + # subdocument for every query is not worth the single duplicated string. + if annotation_type: + doc.setdefault("extra", {}).setdefault("encode", {})["annotation_type"] = ( + annotation_type + ) + + for collection in doc.get("collections", []): + collection_extra = collection.setdefault("extra", {}).setdefault("encode", {}) + if annotation_type: + collection_extra["annotation_type"] = annotation_type + _add_extra(collection_extra, "software_used", row.get("Software used")) + _add_extra( + collection_extra, "encyclopedia_version", row.get("Encyclopedia Version") + ) + if not collection_extra: + del collection["extra"] + + # Reuses the scalar experiment_target rather than adding a parallel + # list field: the TSV hands over a string either way. Note that + # ENCODE does not order it -- both "CTCF-human, H3K4me3-human" and + # "H3K4me3-human, CTCF-human" occur in the released corpus -- so an + # equality filter on the whole value matches one permutation only. + _add_extra(collection, "experiment_target", row.get("Targets")) + + return doc + + def _add_extra(extra: dict, key: str, value: str | None) -> None: """Add a non-empty value to the extra dict.""" v = _nonempty(value) diff --git a/src/cfdb/services/ontology_mappings.py b/src/cfdb/services/ontology_mappings.py index 70fd217..adebb0c 100644 --- a/src/cfdb/services/ontology_mappings.py +++ b/src/cfdb/services/ontology_mappings.py @@ -1,5 +1,14 @@ """Ontology mappings for ENCODE metadata transformation to C2M2 format.""" +# Prefix marking a CV term minted here rather than drawn from EDAM. EDAM has +# no term for every format ENCODE publishes, and the alternative -- aliasing +# an unrepresented format onto the nearest EDAM term -- is what produced the +# starch/BED conflation (#69, #72): the format becomes indistinguishable from +# the one it was aliased to, and the processor that claims that EDAM term +# picks it up and mangles it. A minted token says "no standard term exists" +# without lying about what the file is. +MINTED_FORMAT_PREFIX = "cfdb:" + # ENCODE file_format to EDAM format CV terms # Reference: https://edamontology.org/page/formats FILE_FORMAT_TO_EDAM = { @@ -12,7 +21,16 @@ "cram": {"id": "format:3462", "name": "CRAM"}, # Genomic interval formats "bed": {"id": "format:3003", "name": "BED"}, - "bedpe": {"id": "format:3003", "name": "BED"}, # BED paired-end + # Deliberately NOT format:3003/BED. BEDPE columns are chrom1/start1/end1/ + # chrom2/start2/end2, so the BED tabix pipeline would sort and index the + # first mate and leave the second unindexed -- a cached artifact that + # looks successful and is wrong. EDAM has no BEDPE term (OLS4: + # q=bedpe&ontology=edam returns nothing), so the token is minted. The + # distinct *name* is what matters operationally: processor lookup keys on + # ``file_format.name`` (``processors.tools.format_name``), and "bedpe" is + # in no processor's ``supported_formats``, so /data streams the raw + # upstream file instead of a mangled index until a real processor exists. + "bedpe": {"id": f"{MINTED_FORMAT_PREFIX}bedpe", "name": "bedpe"}, "broadpeak": {"id": "format:3614", "name": "BroadPeak"}, "narrowpeak": {"id": "format:3613", "name": "NarrowPeak"}, "gappedpeak": {"id": "format:3003", "name": "BED"}, # gappedPeak is BED variant @@ -52,7 +70,18 @@ "database": {"id": "format:2330", "name": "Plain text"}, "starch": {"id": "format:3003", "name": "BED"}, # BEDOPS compressed BED archive "tagalign": {"id": "format:3003", "name": "BED"}, # tagAlign is a BED variant - "biginteract": {"id": "format:3004", "name": "bigBed"}, # bigInteract is a bigBed variant + # Structurally a bigBed, but its trailing columns encode an interaction's + # source and target endpoints. Extracting it with bigBedToBed and indexing + # the leading three columns is range-coherent but silently degrades the + # interaction to an interval, so it is kept distinct for the same reason + # as "bedpe" above. EDAM has no bigInteract term either. + # + # Unlike bedpe, this trades a working capability for an honest one: + # the old index answered range queries correctly, so /index on these + # files goes from degraded-but-usable to 404 until the tileset + # endpoints land. That reaches bigInteract already in the experiment + # corpus, not only newly ingested annotation files. + "biginteract": {"id": f"{MINTED_FORMAT_PREFIX}biginteract", "name": "bigInteract"}, "csfasta": {"id": "format:1929", "name": "FASTA"}, # color-space FASTA (SOLiD) "csqual": {"id": "format:2330", "name": "Plain text"}, # color-space quality scores "h5ad": {"id": "format:3590", "name": "HDF5"}, # AnnData HDF5 format @@ -115,6 +144,24 @@ "transcription start sites": {"id": "data:1255", "name": "Sequence features"}, "enhancer predictions": {"id": "data:1255", "name": "Sequence features"}, "long range chromatin interactions": {"id": "data:0006", "name": "Data"}, + # Annotation datasets (type=Annotation ingest) + # + # Verified against the live annotation TSVs: these five values are the + # complete Output type domain of the two ingested annotation_types, and + # none of them appeared above, so without these entries every annotation + # file would carry data_type=None. + # + # The element/gene link types resolve to the generic "data:0006" rather + # than to a features or track term. EDAM has nothing for a predicted + # regulatory relationship between two loci, and the pre-existing + # "chromatin interactions" entry already made that call the same way. A + # deliberately vague term is preferable to claiming these files are + # something they are not. + "candidate Cis-Regulatory Elements": {"id": "data:1255", "name": "Sequence features"}, + "elements reference": {"id": "data:1255", "name": "Sequence features"}, + "element gene links": {"id": "data:0006", "name": "Data"}, + "thresholded element gene links": {"id": "data:0006", "name": "Data"}, + "thresholded links": {"id": "data:0006", "name": "Data"}, # Reference data "genome reference": {"id": "data:2340", "name": "Genome identifier"}, "sequence alignability": {"id": "data:0006", "name": "Data"}, diff --git a/src/cfdb/services/sync.py b/src/cfdb/services/sync.py index 884e37a..f9a76b8 100644 --- a/src/cfdb/services/sync.py +++ b/src/cfdb/services/sync.py @@ -8,10 +8,12 @@ import shutil import subprocess from collections import Counter +from contextlib import aclosing from copy import copy from dataclasses import dataclass, field from datetime import datetime from enum import Enum +from functools import partial from pathlib import Path from typing import Optional @@ -123,7 +125,16 @@ async def _run_sync(task: SyncTask) -> None: async def _sync_dccs(task: SyncTask) -> None: - """Core sync implementation for API.""" + """Core sync implementation for API. + + Each DCC is isolated the same way ``_sync_encode`` isolates its phases: + one that raises is logged and recorded, and the remaining DCCs still + run. Letting the first failure escape meant every DCC ordered after it + was skipped -- ``get_all_dcc_names`` sorts, so one failed ENCODE phase + cost the whole HuBMAP sync -- along with the data-collection indexes + built at the end. The run still fails once every DCC has been attempted, + naming the ones that broke. + """ if api.db is None: raise RuntimeError("Database not initialized") @@ -132,15 +143,28 @@ async def _sync_dccs(task: SyncTask) -> None: downloads_path = data_path / "downloads" downloads_path.mkdir(exist_ok=True) + failures: list[tuple[str, Exception]] = [] + for dcc in task.dcc_names: task.current_dcc = dcc dcc_type = get_dcc_type(dcc) - # Branch on DCC type - if dcc_type == "rest_api" and dcc == "encode": - await _sync_encode(task) - else: - await _sync_c2m2_zip(task, data_path, downloads_path) + try: + # Branch on DCC type + if dcc_type == "rest_api" and dcc == "encode": + await _sync_encode(task) + else: + await _sync_c2m2_zip(task, data_path, downloads_path) + except Exception as exc: + # Narrower than BaseException on purpose, so a CancelledError + # still cancels the run rather than being recorded as one DCC's + # failure and followed by every remaining DCC. + logger.exception( + "%s sync failed; continuing with the remaining DCCs", + dcc.upper(), + ) + failures.append((dcc, exc)) + continue logger.info(f"{dcc.upper()} synced successfully") await _log_accession_coverage(dcc) @@ -150,12 +174,26 @@ async def _sync_dccs(task: SyncTask) -> None: # (operational indexes only) so we never build these against an # empty database on a cold start; likewise skip when no DCC was # actually synced so an empty request doesn't build them either. + # Runs even when a DCC failed: whatever did load still has to be + # queryable without a full collection scan. if task.dcc_names: task.current_step = "indexing" task.progress = "Ensuring data indexes..." logger.info(task.progress) await ensure_indexes(api.db, data_index_specs()) + if failures: + failed_names = ", ".join(dcc for dcc, _ in failures) + task.progress = ( + f"Sync incomplete: {len(task.dcc_names) - len(failures)} of " + f"{len(task.dcc_names)} DCCs synced" + ) + logger.info(task.progress) + raise RuntimeError( + f"Sync completed with {len(failures)} failed DCC(s) " + f"({failed_names})" + ) from failures[0][1] + task.progress = "All DCCs synced successfully" logger.info(task.progress) @@ -1000,16 +1038,195 @@ async def _enrich_hubmap_files(dataset_metadata: dict[str, dict]) -> None: ) +async def _ingest_encode_rows( + task: SyncTask, + label: str, + rows, + transform, + compression_counts: Counter[str | None], + annotation_type_counts: Counter[str], + counts_by_phase: dict[str, int], + stale_filter: dict, +) -> None: + """Transform and batch-insert one ENCODE metadata stream. + + Shared by the experiment and annotation phases so both use the same + batching, the same skip-on-None handling, and contribute to the same + corpus-wide tallies. + + The row count is reported by mutating ``counts_by_phase`` rather than by + returning, and a partial batch left buffered by a dead stream is flushed + in a ``finally``. Both exist for the same reason: a stream can die + *mid-flight* -- an ``asyncio.TimeoutError`` against the metadata budget + is the known ENCODE failure mode -- and a return value is lost when it + does. Reporting by return meant a failed phase contributed nothing to + the caller's total even though its completed batches were already + committed, so the sync under-reported the corpus by up to everything + that phase had loaded, while the tallies below (mutated per row, not per + batch) over-reported it by the discarded partial batch. Three numbers in + one log block that disagreed with each other and with the database. + + The *clean-path* flush is deliberately not in that ``finally``. A + ``finally`` runs whether or not an exception is in flight, so suppressing + the flush's failure there suppressed it on the success path too: a stream + that drained cleanly and then failed to commit its trailing rows returned + normally and the sync reported a clean load over a corpus short by up to + ``BATCH_SIZE - 1`` rows, against a DCC cleared before the load. + + Args: + task: Sync task whose ``progress`` is updated per batch. + label: Names this stream in progress and log lines. + rows: Async iterable of TSV row dicts. + transform: Row -> document callable; None means skip the row. + compression_counts: Mutated with each document's compression term. + annotation_type_counts: Mutated with each document's annotation type. + counts_by_phase: Mutated with this phase's committed row count, on + the failure path as well as the success path. + stale_filter: Selects the slice of ``files`` this phase owns, deleted + once replacement rows are in hand. See ``clear_stale_once``. + """ + batch: list[dict] = [] + count = 0 + stale_cleared = False + + async def clear_stale_once() -> None: + """Delete the slice this phase owns, once, before its first write. + + Deferred to the first batch rather than run before the stream opens. + The known ENCODE failures -- a 504 while the portal assembles the + response, an exhausted download budget -- strike before any row + arrives, and clearing up front would delete the slice and then have + nothing to put back, serving an empty one until the next successful + sync. Deferring means the previous rows keep being served instead. + A stream that dies *mid*-load still leaves a partial slice; only a + shadow-and-swap would fix that. + """ + nonlocal stale_cleared + if stale_cleared: + return + stale_cleared = True + deleted = await api.db.files.delete_many( + {"submission": "encode", **stale_filter} + ) + logger.info( + "ENCODE %s: cleared %d stale files", label, deleted.deleted_count + ) + + async def commit(pending: list[dict]) -> None: + """Send one batch, discounting it from the tally if it does not land.""" + nonlocal count + await clear_stale_once() + try: + await api.db.files.insert_many(pending) + except Exception: + count -= len(pending) + raise + + try: + # ``aclosing`` because the caller isolates phase failures with a + # broad ``except``, which abandons this generator mid-iteration. + # The stream owns an aiohttp session inside its own ``async with``, + # released only when the generator is finalized -- left to the + # garbage collector that is neither prompt nor guaranteed in a + # long-lived API process, and the annotation fan-out turned one + # stream per sync into N+1. + async with aclosing(rows): + async for row in rows: + doc = transform(row) + if doc is None: + continue + + compression_counts[doc.get("compression_format")] += 1 + annotation_type = ( + doc.get("extra", {}).get("encode", {}).get("annotation_type") + ) + if annotation_type: + annotation_type_counts[annotation_type] += 1 + + batch.append(doc) + count += 1 + + # Insert in batches. The buffer is detached *before* the + # await rather than cleared after it, so an insert that + # raises leaves nothing behind for the flush below to submit + # a second time. Re-submitting is not harmless: the driver + # stamps ``_id`` in place on these dicts before sending, so + # a retry collides on the committed prefix and silently + # drops the batch's uncommitted suffix. + if len(batch) >= BATCH_SIZE: + pending, batch = batch, [] + await commit(pending) + task.progress = f"Inserted {count} ENCODE {label} files..." + logger.info(task.progress) + + # The stream drained without error, so its result is authoritative + # even when it is empty: clear here too, or a type ENCODE stopped + # publishing would keep serving last sync's rows indefinitely. + await clear_stale_once() + + # The clean-path flush: inside the ``try``, so a sink failure with no + # exception in flight fails the phase rather than being swallowed. + if batch: + pending, batch = batch, [] + await commit(pending) + except asyncio.CancelledError: + # Never write on the way out of a cancellation. The decision to stop + # has already been made, and the flush below could not protect the + # unwind anyway: its ``except Exception`` does not catch a + # re-delivered CancelledError, which would replace the original and + # skip the ``counts_by_phase`` record. + count -= len(batch) + batch.clear() + raise + finally: + # Only reachable with rows still buffered when the stream itself died + # mid-flight -- the detach above guarantees they were never + # submitted. They are already counted in the tallies, and the DCC was + # cleared before the load, so discarding them would lose data for no + # gain. + if batch: + try: + await commit(batch) + except Exception: + # Never let the flush replace the exception that caused it. + logger.exception( + "ENCODE %s: failed to commit the final %d-row batch", + label, + len(batch), + ) + counts_by_phase[label] = count + logger.info(f"ENCODE {label} ingest complete: {count} files") + + async def _sync_encode(task: SyncTask) -> None: """ Sync ENCODE data from REST API. Unlike C2M2 ZIP sources, ENCODE data is fetched from the REST API and pre-materialized directly into the files collection. + + Runs one phase for released experiments plus one per configured + ``annotation_type``. Each phase is isolated: a phase that raises is + logged and recorded, and the remaining phases still run, so a transient + failure on one stream does not cost the corpus the others. The task is + still failed at the end if any phase failed -- the alternative would + report a clean sync over a partially loaded collection. + + Isolation covers the destructive half too. Each phase owns a slice of + ``files`` and deletes only that slice, and only once it has rows to put + back, so a phase that fails before delivering any -- the shape a portal + 504 or an exhausted download budget takes -- leaves its previous rows + being served rather than emptying them. A phase that dies partway + through still leaves its own slice partially loaded; nothing short of + loading into a shadow collection and swapping would change that. """ from cfdb.services.encode import ( + annotation_types_from_env, build_encode_dcc_record, + fetch_encode_annotation_metadata, fetch_encode_metadata, + metadata_budget_seconds, + transform_annotation_to_c2m2, transform_to_c2m2, ) @@ -1021,8 +1238,28 @@ async def _sync_encode(task: SyncTask) -> None: task.progress = "Clearing existing ENCODE data..." logger.info(task.progress) + count = 0 + counts_by_phase: dict[str, int] = {} + failures: list[tuple[str, Exception]] = [] + # Tally the derived compression terms. The derivation depends on the + # shape of ENCODE's download URLs, which nobody here controls, so a + # corpus-wide flip (an upstream column rename, a redirect stub) would + # otherwise be invisible behind an unchanged row count. + compression_counts: Counter[str | None] = Counter() + # Same reasoning for annotation_type, and seeded from the configured + # allowlist below so a type that returns nothing is reported as zero + # rather than simply missing from the log. + annotation_type_counts: Counter[str] = Counter() + async with locks.CutoverLock("encode"): - await _clear_dcc_data_async("encode") + # ``files`` is deliberately not cleared here. One corpus-wide delete + # before a fan-out designed to survive a phase failure means a failed + # phase serves an absent slice rather than a stale one -- the + # experiment phase is the ~295k-file bulk and the likeliest to time + # out, and losing it leaves the API serving the ~29k annotation + # documents as the whole corpus. Each phase clears only what it is + # about to reload, immediately before reloading it. + await _clear_dcc_data_async("encode", skip_collections={"files"}) # Step 2: Upsert DCC record task.current_step = "dcc_record" @@ -1036,49 +1273,111 @@ async def _sync_encode(task: SyncTask) -> None: upsert=True, ) - # Step 3: Fetch and transform files from ENCODE API + # Step 3: Fetch and transform files from ENCODE API. The experiment + # stream first, then one per configured annotation type. task.current_step = "fetching" task.progress = "Fetching files from ENCODE API..." logger.info(task.progress) - batch = [] - count = 0 - # Tally the derived compression terms. The derivation depends on the - # shape of ENCODE's download URLs, which nobody here controls, so a - # corpus-wide flip (an upstream column rename, a redirect stub) would - # otherwise be invisible behind an unchanged row count. - compression_counts: Counter[str | None] = Counter() - - async for encode_file in fetch_encode_metadata(): - # Transform to C2M2 format - doc = transform_to_c2m2(encode_file) - if doc is None: - continue - - compression_counts[doc.get("compression_format")] += 1 - batch.append(doc) - count += 1 + # One deadline for the whole fan-out, resolved here rather than per + # stream. Every phase runs inside the cutover lock held above, which + # gates the read surface, so a per-stream budget would multiply the + # outage by the number of phases and carry the worst case past the + # sync lock's one-hour stale threshold -- admitting a second sync + # that clears the corpus while this one is still writing to it. A + # phase that spends what is left fails as an ordinary TimeoutError, + # so the isolation below still applies and its rows are still + # committed; the phases after it fail immediately rather than + # opening a request they have no time to finish. + deadline = asyncio.get_running_loop().time() + metadata_budget_seconds() + + # Streams are built by a factory rather than constructed up front, so + # a phase that is never reached never opens a request. Each phase + # carries the filter selecting the slice of ``files`` it owns, so it + # replaces exactly what it reloads. The experiment slice is defined + # by absence: an annotation row published with a blank ``Annotation + # type`` produces no such key, so the experiment phase sweeps it and + # its own phase reinserts it -- it is lost only if that phase also + # fails. + phases = [ + ( + "experiment", + partial(fetch_encode_metadata, deadline=deadline), + transform_to_c2m2, + {"extra.encode.annotation_type": {"$exists": False}}, + ) + ] + for annotation_type in annotation_types_from_env(): + # Seeded at zero so a configured type that returns nothing logs + # ": 0" rather than vanishing from the distribution. An absent + # key is what nobody notices; a zero is what everybody does. + annotation_type_counts[annotation_type] = 0 + phases.append( + ( + f"annotation[{annotation_type}]", + partial( + fetch_encode_annotation_metadata, + annotation_type, + deadline=deadline, + ), + transform_annotation_to_c2m2, + {"extra.encode.annotation_type": annotation_type}, + ) + ) - # Insert in batches - if len(batch) >= BATCH_SIZE: - await api.db.files.insert_many(batch) - task.progress = f"Inserted {count} ENCODE files..." - logger.info(task.progress) - batch.clear() + for label, open_stream, transform, stale_filter in phases: + try: + await _ingest_encode_rows( + task, + label, + open_stream(), + transform, + compression_counts, + annotation_type_counts, + counts_by_phase, + stale_filter, + ) + except Exception as exc: + # Deliberately broad: the point is that no failure mode of + # one stream -- timeout, HTTP error, malformed TSV -- may + # stop the others from being attempted. Narrower than + # BaseException on purpose, so a CancelledError still + # cancels the sync rather than being logged as a phase + # failure and followed by N more phases. + logger.exception( + "ENCODE %s ingest failed; continuing with the remaining " + "phases", + label, + ) + failures.append((label, exc)) + continue - # Insert remaining batch - if batch: - await api.db.files.insert_many(batch) - logger.info(f"Inserted final batch, total: {count} ENCODE files") + # Summed from the accumulator rather than tracked alongside it, so + # a phase that failed after committing rows still contributes what + # it actually loaded. + count = sum(counts_by_phase.values()) # ENCODE writes straight into the materialized collection and never runs # the materializer, which is the only other creator of files indexes. On # a database where ENCODE is the only DCC synced, that leaves files with - # no indexes at all and makes every accession lookup a full scan. + # no indexes at all and makes every accession lookup a full scan. Run + # unconditionally: whatever a partially failed sync did load still has to + # be servable without a full collection scan. await ensure_indexes(api.db, materialized_files_index_specs()) - task.progress = f"ENCODE sync complete: {count} files" + if failures: + # counts_by_phase carries an entry for every phase that started, + # failed ones included -- they may have committed rows before + # dying -- so the succeeded tally comes from the phase list, not + # from the accumulator's length. + task.progress = ( + f"ENCODE sync incomplete: {count} files from " + f"{len(phases) - len(failures)} of {len(phases)} phases" + ) + else: + task.progress = f"ENCODE sync complete: {count} files" logger.info(task.progress) + logger.info("ENCODE files inserted per phase: %s", counts_by_phase) logger.info( "ENCODE compression_format distribution: %s", { @@ -1086,6 +1385,17 @@ async def _sync_encode(task: SyncTask) -> None: for term, tally in compression_counts.most_common() }, ) + logger.info( + "ENCODE annotation_type distribution: %s", dict(annotation_type_counts) + ) + + if failures: + failed_labels = ", ".join(label for label, _ in failures) + raise RuntimeError( + f"ENCODE sync completed with {len(failures)} failed phase(s) " + f"({failed_labels}); {count} files were loaded from the phases " + "that succeeded" + ) from failures[0][1] async def _materialize_files(submission: str) -> None: @@ -1119,14 +1429,26 @@ async def _materialize_files(submission: str) -> None: raise -async def _clear_dcc_data_async(submission: str) -> None: - """Clear DCC data using async Motor client.""" +async def _clear_dcc_data_async( + submission: str, skip_collections: set[str] | None = None +) -> None: + """Clear DCC data using async Motor client. + + Args: + submission: DCC whose documents are removed. + skip_collections: Collections left untouched. The ENCODE sync passes + ``files`` here because it clears that collection one phase-slice + at a time instead; see :func:`_sync_encode`. + """ if api.db is None: raise RuntimeError("Database not initialized") collection_names = await api.db.list_collection_names() + skip = skip_collections or set() for collection_name in collection_names: + if collection_name in skip: + continue try: result = await api.db[collection_name].delete_many( {"submission": submission} diff --git a/src/cfdb/workflows/keys.py b/src/cfdb/workflows/keys.py index eb8f407..455d7aa 100644 --- a/src/cfdb/workflows/keys.py +++ b/src/cfdb/workflows/keys.py @@ -185,6 +185,21 @@ def cache_key( row for the new key) but does NOT invalidate cached artifacts. To force fresh cache entries for a single processor's outputs, bump that processor's ``processor_version`` instead. + + Warning: + The key identifies the processor only by ``processor_version``, not + by which processor it is. Two processors that claim the same + ``(file, artifact_kind)`` pair at equal ``processor_version`` derive + the *same* key and would read back each other's artifacts as cache + hits -- a wrong answer rather than a miss. This holds today only + because each pair is claimed by at most one processor, which is a + property of the current registry and not of this function. Fold a + processor identity (class name, or a registry-assigned id) into the + key before landing a second processor for any pair. The paired + interval formats make this concrete: ``.bedpe`` and ``bigInteract`` + files carry ``index`` artifacts built by ``TabixIntervalProcessor`` + before they were re-typed, so a future paired-interval processor is + exactly the case that would collide. """ if processor_version < 0: raise ValueError("processor_version must be non-negative") diff --git a/tests/conftest.py b/tests/conftest.py index a711076..24769c1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,14 +15,19 @@ ISSUE_83_SIZE = 6262125716 -def _resolve(doc: dict, key: str): +#: Distinguishes "no such path" from "a path holding None", which ``$exists`` +#: has to tell apart and a bare ``None`` default cannot. +_MISSING = object() + + +def _resolve(doc: dict, key: str, default=None): """Resolve a possibly dot-notated key against a nested dict.""" value = doc for part in key.split("."): - if isinstance(value, dict): - value = value.get(part) + if isinstance(value, dict) and part in value: + value = value[part] else: - return None + return default return value @@ -53,9 +58,12 @@ def _match(doc: dict, query: dict) -> bool: if value == operand: return False elif op == "$exists": - if operand and key not in doc: - return False - if not operand and key in doc: + # Resolved rather than tested with ``in doc``, which sees + # only top-level keys and so reports every dotted path as + # absent -- making {"a.b": {"$exists": False}} match + # everything, including the documents that do have it. + present = _resolve(doc, key, _MISSING) is not _MISSING + if bool(operand) is not present: return False elif op == "$regex": import re diff --git a/tests/test_data.py b/tests/test_data.py index dd1277b..d27bb08 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -12,8 +12,10 @@ from cfdb import api from cfdb.api.routers.data import stream_file, stream_file_status from cfdb.services import drs, locks +from cfdb.services.ontology_mappings import get_file_format from cfdb.workflows.executor import WoolExecutor from cfdb.workflows.processors.bam import BamIndexProcessor +from cfdb.workflows.processors.tabix import TabixIntervalProcessor from tests.test_workflows import FIXTURE_MD5 from cfdb.workflows.processors.registry import ProcessorRegistry, default_registry @@ -592,6 +594,92 @@ async def fake_drs(*args, **kwargs): ) + @pytest.mark.asyncio + @pytest.mark.parametrize("format_name", ["bedpe", "bigInteract"]) + async def test_stream_file_should_serve_a_paired_interval_format_unprocessed( + self, mock_db, mocker, format_name + ): + """Test a paired-interval file reaches the direct streaming path. + + While these formats were aliased to BED and bigBed the tabix + pipeline claimed them and committed an index built from the first + locus of each record. Now that each carries its own format name no + processor claims it, so the request must fall through to the + upstream file rather than dispatch a workflow that would produce + the wrong artifact. + + Given: + A file in a paired-interval format and a wired workflow + subsystem carrying the processors the API registers. + When: + stream_file is called. + Then: + It should reach the direct DRS streaming path rather than + returning a 202 workflow dispatch. + """ + # Arrange + mocker.patch.object(locks, "wait_for_cutover", return_value=None) + registry = default_registry() + registry.register(BamIndexProcessor()) + registry.register(TabixIntervalProcessor()) + mocker.patch.object(api, "processor_registry", registry) + + class _Cache: + async def head(self, _k): + raise AssertionError("an unclaimed format must not consult cache") + + def get(self, *_a, **_kw): + raise AssertionError + + async def put(self, *_a, **_kw): + raise AssertionError + + async def delete(self, _k): + return False + + mocker.patch.object(api, "cache", _Cache()) + mocker.patch.object(api, "executor", object()) + + mock_db.dcc.docs = [ + { + "dcc_abbreviation": "4DN_DCIC", + "project_id_namespace": "tag:4dn.org,2015:", + } + ] + # The minted term is injected rather than produced: get_file_format + # has one consumer, the ENCODE transform, so the 4DN ingest cannot + # mint it and a real 4DN .bedpe still arrives declaring BED. This + # pins registry routing, which keys on the format name irrespective + # of DCC -- not that 4DN paired-interval files are safe today. See + # the paired-interval section of ENCODE-SUPPLEMENT.md. + mock_db.file.docs = [ + { + "submission": "4dn", + "id_namespace": "tag:4dn.org,2015:", + "local_id": "4DNFIPAIR01", + "filename": "x.bedpe.gz", + "md5": FIXTURE_MD5, + "access_url": "drs://4dn/abc", + "file_format": get_file_format(format_name), + } + ] + + drs_calls: list = [] + + async def fake_drs(*args, **kwargs): + drs_calls.append((args, kwargs)) + raise Exception("DRS unavailable") + + mocker.patch.object(drs, "fetch_drs_object", side_effect=fake_drs) + + # Act & assert + with pytest.raises(HTTPException) as exc_info: + await stream_file("4dn", "4DNFIPAIR01", _make_request(), range=None) + + assert exc_info.value.status_code != 202 + assert len(drs_calls) == 1 + + class TestStreamFileStatus: @pytest.mark.asyncio async def test_stream_file_status_should_raise_400_when_dcc_invalid( diff --git a/tests/test_encode.py b/tests/test_encode.py index ff33c31..5c1f08c 100644 --- a/tests/test_encode.py +++ b/tests/test_encode.py @@ -4,20 +4,25 @@ import asyncio import logging +from urllib.parse import parse_qs, urlsplit import aiohttp import pytest -from hypothesis import given, settings +from hypothesis import HealthCheck, assume, given, settings from hypothesis import strategies as st from cfdb.accessions import normalize_accession +from cfdb.models import FileMetadataModel from cfdb.services import encode as encode_module from cfdb.services.encode import ( COMPRESSION_SUFFIX_TO_EDAM, UNCOMPRESSED, UNMAPPABLE_COMPRESSION_SUFFIXES, + annotation_types_from_env, derive_compression_format, + fetch_encode_annotation_metadata, fetch_encode_metadata, + transform_annotation_to_c2m2, transform_to_c2m2, ) @@ -26,6 +31,10 @@ #: the module should fail these tests, not silently follow along. TIMEOUT_ENV = "ENCODE_METADATA_TIMEOUT_SECONDS" +#: The variable bounding which annotation types are ingested. Named here for +#: the same reason as TIMEOUT_ENV. +ANNOTATION_TYPES_ENV = "ENCODE_ANNOTATION_TYPES" + DOWNLOAD_URL = "https://www.encodeproject.org/files/ENCFF123ABC/@@download/{name}" # Every value the derivation is allowed to produce, for closed-domain @@ -59,7 +68,10 @@ class _FakeContent: modelling a stream that dies partway. """ - def __init__(self, lines: list[str], fail_after: int | None = None): + def __init__(self, lines, fail_after: int | None = None): + # Any iterable, not just a list: a body large enough to trip the + # per-50000-row progress log would otherwise have to be + # materialized in memory just to be thrown away line by line. self._lines = lines self._fail_after = fail_after @@ -87,10 +99,10 @@ async def __aexit__(self, *exc): class _StreamingSession: """Fake aiohttp session that serves one streamed TSV body. - Records the kwargs of every ``get`` -- unlike the 4DN fakes, which - discard them -- because the request's timeout is the behavior under - test. ``error`` raises instead of responding, modelling a network - failure before any body arrives. + Records the URL and kwargs of every ``get`` -- unlike the 4DN fakes, + which discard them -- because the request's timeout and the query it + carries are both behaviors under test. ``error`` raises instead of + responding, modelling a network failure before any body arrives. """ def __init__(self, lines=None, status=200, fail_after=None, error=None): @@ -99,14 +111,22 @@ def __init__(self, lines=None, status=200, fail_after=None, error=None): self._fail_after = fail_after self._error = error self.get_kwargs: list[dict] = [] + self.get_urls: list[str] = [] + #: Times ``__aexit__`` ran. The stream owns its session inside an + #: ``async with``, so this is the only observable signal that an + #: abandoned or failed stream released it rather than leaving it to + #: the garbage collector. + self.exit_count = 0 async def __aenter__(self): return self async def __aexit__(self, *exc): + self.exit_count += 1 return False def get(self, url, **kwargs): + self.get_urls.append(url) self.get_kwargs.append(kwargs) if self._error is not None: raise self._error @@ -211,6 +231,70 @@ async def test_fetch_encode_metadata_should_fall_back_to_the_default_when_unset( assert session.get_kwargs[0]["timeout"].total == 3600 +@pytest.mark.asyncio +async def test_fetch_encode_metadata_should_request_only_the_deadline_that_remains( + mocker, monkeypatch +): + """Test a shared deadline bounds the request, not a fresh full budget. + + A sync runs one stream per configured annotation type plus one for + experiments, all inside the cutover lock that gates the read surface. + Granting each its own hour would multiply the outage by the phase count. + + Given: + A deadline half a minute out, against an hour-long budget. + When: + fetch_encode_metadata drains a response. + Then: + It should bound the request by the remaining time, not the budget. + """ + # Arrange + monkeypatch.delenv(TIMEOUT_ENV, raising=False) + session = _StreamingSession(lines=_tsv("ENCFF1\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + deadline = asyncio.get_running_loop().time() + 30 + + # Act + [row async for row in fetch_encode_metadata(deadline=deadline)] + + # Assert + assert session.get_kwargs[0]["timeout"].total <= 30 + + +@pytest.mark.asyncio +async def test_fetch_encode_metadata_should_refuse_a_spent_deadline( + mocker, monkeypatch +): + """Test an exhausted budget stops the fetch instead of going unbounded. + + aiohttp reads a non-positive total as "no timeout", so handing it the + remainder of a spent budget would turn the exhausted case into an + unbounded request -- the failure the budget exists to prevent. + + Given: + A deadline that has already passed. + When: + fetch_encode_metadata is drained. + Then: + It should raise TimeoutError without opening a request. + """ + # Arrange + monkeypatch.delenv(TIMEOUT_ENV, raising=False) + session = _StreamingSession(lines=_tsv("ENCFF1\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + deadline = asyncio.get_running_loop().time() - 1 + + # Act & assert + with pytest.raises(asyncio.TimeoutError, match="budget was already spent"): + [row async for row in fetch_encode_metadata(deadline=deadline)] + + assert session.get_kwargs == [] + + @pytest.mark.asyncio @pytest.mark.parametrize( "value", ["not-a-number", "0", "-1"], ids=["malformed", "zero", "negative"] @@ -398,6 +482,9 @@ async def test_fetch_encode_metadata_should_report_how_far_it_got_when_it_times_ [row async for row in fetch_encode_metadata()] assert "timed out after 2 rows" in caplog.text + # Names its stream: with several streams per sync, a timeout message + # that does not say which one died is not actionable. + assert "experiment" in caplog.text @pytest.mark.asyncio @@ -1104,11 +1191,13 @@ def test_transform_to_c2m2_should_fold_the_experiment_collection_accession(): def test_transform_to_c2m2_should_build_no_collection_without_a_biosample_term(): """Test that an experiment accession alone yields no collection. - The whole collection block is gated on the biosample term name, so a - row shaped like an ENCODE annotation or reference contributes no - collection and its experiment accession is not queryable anywhere. - Pre-existing behavior, pinned here because the accession field turns it - from a cosmetic gap into a data-completeness question. + On the experiment path the whole collection block is gated on the + biosample term name, so such a row contributes no collection and its + accession is queryable nowhere. Long-standing behavior, pinned here as + the other half of ``require_biosample``: the annotation path passes + False and deliberately does build the collection (see + ...should_build_a_dataset_without_a_biosample), so this test is what + stops that choice from silently leaking onto experiments. Given: A row carrying an Experiment accession but no Biosample term name. @@ -1163,3 +1252,1481 @@ def test_transform_to_c2m2_should_store_the_accession_case_folded(accession, pad # Assert assert doc["accession_id"] == normalize_accession(doc["local_id"]) + + +# --------------------------------------------------------------------------- +# Annotation ingest +# --------------------------------------------------------------------------- + +#: Download URL of the real released cCRE file the annotation fixture is +#: modelled on. +ANNOTATION_DOWNLOAD_URL = ( + "https://www.encodeproject.org/files/ENCFF845ITU/@@download/ENCFF845ITU.bigBed" +) + +#: Text an environment variable can actually hold: ``os.environ`` rejects an +#: embedded NUL outright and cannot encode a lone surrogate. Generating +#: either tests the harness rather than the allowlist parser. +ENV_SAFE_TEXT = st.text( + alphabet=st.characters(exclude_characters="\x00", codec="utf-8"), + max_size=80, +) + + +def _annotation_row(**overrides) -> dict: + """Return a realistic ENCODE annotation metadata TSV row. + + The full 32-column annotation header, verified against the live TSVs of + both ingested types, with values from a released cCRE file -- so the + fixture carries the shapes the corpus actually contains: an empty Assay + term name, a multi-valued Targets, a non-human organism. + """ + row = { + "File accession": "ENCFF845ITU", + "File format": "bigBed bed9+", + "Output type": "candidate Cis-Regulatory Elements", + "Assay term name": "", + "Dataset accession": "ENCSR026KJM", + "Annotation type": "candidate Cis-Regulatory Elements", + "Software used": "", + "Encyclopedia Version": "ENCODE v1", + "Biosample term id": "UBERON:0002048", + "Biosample term name": "lung", + "Biosample type": "tissue", + "Life stage": "postnatal", + "Age": "0", + "Age units": "day", + "Organism": "Mus musculus", + "Targets": "H3K4me3-mouse, CTCF-mouse", + "Dataset date released": "2017-09-12", + "Project": "ENCODE", + "Lab": "Zhiping Weng, UMass", + "md5sum": "5ff392dcde69f8ec512ea381928674d9", + "dbxrefs": "", + "File download URL": ANNOTATION_DOWNLOAD_URL, + "Assembly": "mm10", + "Controlled by": "", + "File Status": "released", + "Derived from": "/files/ENCFF728HFF/", + "S3 URL": "https://encode-public.s3.amazonaws.com/2017/ENCFF845ITU.bigBed", + "Azure URL": "", + "Size": "13743825", + "Audit WARNING": "", + "Audit NOT_COMPLIANT": "", + "Audit ERROR": "", + } + row.update(overrides) + return row + + +def test_annotation_types_from_env_should_return_a_bounded_default_when_unset( + monkeypatch, +): + """Test the allowlist is never implicitly the whole annotation space. + + ENCODE publishes 580,910 annotation datasets, 86% of them footprints. + Defaulting to all of them would be a corpus-scale mistake to undo, so + the default has to be an explicit, small set -- asserted by equality, + not by "non-empty and not footprints", which would also pass if a third + type were quietly added to the default. + + Given: + No ENCODE_ANNOTATION_TYPES in the environment. + When: + annotation_types_from_env is called. + Then: + It should return exactly the two documented default types. + """ + # Arrange + monkeypatch.delenv(ANNOTATION_TYPES_ENV, raising=False) + + # Act + types = annotation_types_from_env() + + # Assert + assert types == ( + "candidate Cis-Regulatory Elements", + "element gene regulatory interaction predictions", + ) + + +def test_annotation_types_from_env_should_honor_an_override(monkeypatch): + """Test which types are ingested is configuration, not a code change. + + Given: + ENCODE_ANNOTATION_TYPES naming two types. + When: + annotation_types_from_env is called. + Then: + It should return exactly those two, in order. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, "chromatin state,footprints") + + # Act + types = annotation_types_from_env() + + # Assert + assert types == ("chromatin state", "footprints") + + +def test_annotation_types_from_env_should_strip_padding_and_drop_blanks(monkeypatch): + """Test a human-written list is read the way it was meant. + + Given: + An override written with spaces after the commas and a trailing + comma. + When: + annotation_types_from_env is called. + Then: + It should return the trimmed values with no empty entry. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, " chromatin state , footprints ,") + + # Act + types = annotation_types_from_env() + + # Assert + assert types == ("chromatin state", "footprints") + + +def test_annotation_types_from_env_should_disable_ingest_when_set_empty(monkeypatch): + """Test an operator can turn the annotation path off without a deploy. + + Given: + ENCODE_ANNOTATION_TYPES set to an empty value -- distinct from + unset, which yields the default allowlist. + When: + annotation_types_from_env is called. + Then: + It should return no types at all. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, "") + + # Act + types = annotation_types_from_env() + + # Assert + assert types == () + + +def test_annotation_types_from_env_should_warn_when_set_empty(monkeypatch, caplog): + """Test an accidentally empty allowlist is visible rather than inferable. + + The neighbouring timeout variable reads an empty value as "unset, use + the default" and this one reads it as "ingest nothing" -- a difference + an operator has no reason to expect. Empty values arrive by accident + routinely, from an unset CloudFormation parameter to a docker-compose + expansion, and the only other trace is an absence in the per-phase log. + + Given: + ENCODE_ANNOTATION_TYPES set to an empty value. + When: + annotation_types_from_env is called. + Then: + It should warn, naming the variable and the disabled ingest. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, "") + + # Act + with caplog.at_level(logging.WARNING, logger=encode_module.logger.name): + annotation_types_from_env() + + # Assert + assert ANNOTATION_TYPES_ENV in caplog.text + assert "annotation ingest is disabled" in caplog.text + + +def test_annotation_types_from_env_should_not_warn_when_unset(monkeypatch, caplog): + """Test the default allowlist is not reported as a disabled ingest. + + Given: + No ENCODE_ANNOTATION_TYPES in the environment. + When: + annotation_types_from_env is called. + Then: + It should emit no warning. + """ + # Arrange + monkeypatch.delenv(ANNOTATION_TYPES_ENV, raising=False) + + # Act + with caplog.at_level(logging.WARNING, logger=encode_module.logger.name): + annotation_types_from_env() + + # Assert + assert caplog.text == "" + + +@pytest.mark.asyncio +async def test_fetch_encode_annotation_metadata_should_query_the_requested_type( + mocker, +): + """Test the request selects annotations of one type, not experiments. + + Given: + A streamed metadata response and an annotation type containing the + spaces and capitals ENCODE's vocabulary actually uses. + When: + fetch_encode_annotation_metadata drains it. + Then: + It should have requested released Annotations with the type + URL-encoded rather than interpolated raw, and carry no other + parameter. + """ + # Arrange + session = _StreamingSession(lines=_tsv("ENCFF1\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act + [ + row + async for row in fetch_encode_annotation_metadata( + "candidate Cis-Regulatory Elements" + ) + ] + + # Assert + url = session.get_urls[0] + parsed = urlsplit(url) + assert parsed.path == "/metadata/" + # Equality, not substring containment: a substring check passes against + # a URL that also carries a stray or duplicated parameter. + assert parse_qs(parsed.query) == { + "type": ["Annotation"], + "status": ["released"], + "annotation_type": ["candidate Cis-Regulatory Elements"], + } + assert " " not in url + + +@pytest.mark.asyncio +async def test_fetch_encode_annotation_metadata_should_yield_a_row_per_data_line( + mocker, +): + """Test the annotation stream parses the way the experiment one does. + + Given: + A streamed annotation TSV with a header and two data lines. + When: + fetch_encode_annotation_metadata drains it. + Then: + It should yield one dict per data line, keyed by column name. + """ + # Arrange + session = _StreamingSession(lines=_tsv("ENCFF1\tbed", "ENCFF2\tbigBed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act + rows = [row async for row in fetch_encode_annotation_metadata("chromatin state")] + + # Assert + assert rows == [ + {"File accession": "ENCFF1", "File format": "bed"}, + {"File accession": "ENCFF2", "File format": "bigBed"}, + ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "session_kwargs", + [ + {"status": 503}, + {"error": aiohttp.ClientError("connection reset")}, + {"lines": _tsv("ENCFF1\tbed"), "fail_after": 1}, + ], + ids=["http-error", "network-error", "timeout"], +) +async def test_fetch_encode_annotation_metadata_should_release_the_session_on_failure( + mocker, session_kwargs +): + """Test a failed stream does not leave its HTTP session open. + + The stream owns an aiohttp session inside its own context manager, and + the sync isolates phase failures with a broad except that abandons the + generator mid-iteration. With one stream per sync that was academic; + with one per annotation type plus the experiment, a leak per failure + accumulates in a long-lived API process. + + Given: + An annotation stream that fails by HTTP status, by network error, + or by timeout. + When: + It is drained and the error escapes. + Then: + The session should have been exited in every case. + """ + # Arrange + session = _StreamingSession(**session_kwargs) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act + with pytest.raises((Exception, asyncio.TimeoutError)): + [row async for row in fetch_encode_annotation_metadata("chromatin state")] + + # Assert + assert session.exit_count == 1 + + +@pytest.mark.asyncio +async def test_fetch_encode_annotation_metadata_should_release_the_session_when_abandoned( + mocker, +): + """Test a stream abandoned part-way still releases its session. + + This is the shape the sync's phase isolation produces: the consumer + stops iterating because something else raised, not because the stream + ended. + + Given: + A streamed annotation body of several rows. + When: + The consumer takes one row and closes the generator. + Then: + The session should have been exited. + """ + # Arrange + session = _StreamingSession(lines=_tsv("ENCFF1\tbed", "ENCFF2\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + stream = fetch_encode_annotation_metadata("chromatin state") + + # Act + async for _ in stream: + break + await stream.aclose() + + # Assert + assert session.exit_count == 1 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "session_kwargs, expectation", + [ + ({"status": 503}, "503"), + ({"error": aiohttp.ClientError("connection reset")}, "network error"), + ], + ids=["http-error", "network-error"], +) +async def test_fetch_encode_annotation_metadata_should_name_its_stream_when_it_fails( + mocker, session_kwargs, expectation +): + """Test a failure message identifies which stream produced it. + + With several streams per sync, a message that does not say which one + died leaves an operator to guess between the experiment corpus and any + of the configured annotation types. + + Given: + An annotation stream for a named type that fails. + When: + It is drained. + Then: + The error should name both the failure and the annotation type. + """ + # Arrange + session = _StreamingSession(**session_kwargs) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act & assert + with pytest.raises(Exception, match=r"annotation\[chromatin state\]"): + [row async for row in fetch_encode_annotation_metadata("chromatin state")] + + with pytest.raises(Exception, match=expectation): + [row async for row in fetch_encode_annotation_metadata("chromatin state")] + + +@pytest.mark.asyncio +async def test_fetch_encode_annotation_metadata_should_name_its_stream_on_timeout( + mocker, caplog +): + """Test a timed-out annotation stream names its type and its progress. + + Given: + An annotation stream that times out after one data row. + When: + It is drained. + Then: + The log should name the annotation type alongside the row count. + """ + # Arrange + session = _StreamingSession( + lines=_tsv("ENCFF1\tbed", "ENCFF2\tbed"), fail_after=2 + ) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act & assert + with caplog.at_level(logging.ERROR), pytest.raises(asyncio.TimeoutError): + [row async for row in fetch_encode_annotation_metadata("chromatin state")] + + assert "annotation[chromatin state]" in caplog.text + assert "timed out after 1 rows" in caplog.text + + +@pytest.mark.asyncio +async def test_fetch_encode_annotation_metadata_should_not_let_a_type_inject_parameters( + mocker, +): + """Test a configured value cannot smuggle extra query parameters. + + The annotation type is operator-supplied configuration interpolated + into a URL. Were it not encoded, a value containing an ampersand could + append parameters -- widening status past released, or flipping the + type back to Experiment. + + Given: + An annotation type containing URL metacharacters. + When: + The stream is drained. + Then: + The query should still carry exactly three parameters with the + intended type and status. + """ + # Arrange + hostile = "x&status=deleted&type=Experiment" + session = _StreamingSession(lines=_tsv("ENCFF1\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act + [row async for row in fetch_encode_annotation_metadata(hostile)] + + # Assert + assert parse_qs(urlsplit(session.get_urls[0]).query) == { + "type": ["Annotation"], + "status": ["released"], + "annotation_type": [hostile], + } + + +@pytest.mark.asyncio +async def test_fetch_encode_metadata_should_request_the_released_experiment_corpus( + mocker, +): + """Test the experiment URL is unchanged by the annotation work. + + The annotation fetch builds its query with urlencode while this one + keeps a hand-built literal. The obvious future tidy-up is to unify + them, which would change escaping or parameter order with nothing else + to catch it. + + Given: + A streamed metadata response. + When: + fetch_encode_metadata drains it. + Then: + It should have requested exactly the released-experiment URL. + """ + # Arrange + session = _StreamingSession(lines=_tsv("ENCFF1\tbed")) + mocker.patch.object( + encode_module.aiohttp, "ClientSession", return_value=session + ) + + # Act + [row async for row in fetch_encode_metadata()] + + # Assert + assert session.get_urls[0] == ( + "https://www.encodeproject.org/metadata/?type=Experiment&status=released" + ) + + +@pytest.mark.parametrize("raw", [" ", ",,,", " , , ", "\t"]) +def test_annotation_types_from_env_should_disable_ingest_when_all_entries_blank( + monkeypatch, raw +): + """Test a whitespace-only allowlist disables ingest rather than widening it. + + A blank entry surviving into the allowlist would request + ``annotation_type=``, which the portal is liable to read as unfiltered + -- 580,910 datasets, the exact outcome the allowlist exists to prevent. + + Given: + An override consisting only of separators and whitespace. + When: + annotation_types_from_env is called. + Then: + It should return no types at all. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, raw) + + # Act + types = annotation_types_from_env() + + # Assert + assert types == () + + +def test_annotation_types_from_env_should_collapse_a_repeated_type(monkeypatch): + """Test a repeated entry yields one ingest phase, not two. + + Given: + An override naming the same type twice with differing padding. + When: + annotation_types_from_env is called. + Then: + It should return that type once, keeping first-occurrence order. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, "alpha, beta ,alpha") + + # Act + types = annotation_types_from_env() + + # Assert + assert types == ("alpha", "beta") + + +@given(annotation_type=ENV_SAFE_TEXT.filter(lambda s: s.strip())) +# monkeypatch is function-scoped and so is not reset between generated +# examples. Safe here: every example overwrites the same single variable +# rather than accumulating state, and the fixture still restores the real +# environment once at the end. +@settings( + max_examples=100, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +def test_annotation_types_from_env_should_round_trip_any_single_type( + monkeypatch, annotation_type +): + """Test any comma-free type survives configuration unchanged. + + ENCODE's vocabulary is case- and space-significant, so a well-meaning + normalization would silently stop matching upstream. + + Given: + Any non-blank text containing no comma. + When: + It is set as the allowlist and read back. + Then: + It should come back as its own stripped form, unaltered otherwise. + """ + # Arrange + assume("," not in annotation_type) + monkeypatch.setenv(ANNOTATION_TYPES_ENV, annotation_type) + + # Act + types = annotation_types_from_env() + + # Assert + assert types == (annotation_type.strip(),) + + +@given(raw=ENV_SAFE_TEXT) +@settings( + max_examples=200, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +def test_annotation_types_from_env_should_never_yield_a_blank_entry(monkeypatch, raw): + """Test no configuration can produce an unfiltered annotation query. + + Given: + Any text at all, including comma-and-whitespace soup. + When: + annotation_types_from_env is called. + Then: + Every returned entry should be non-blank and already stripped. + """ + # Arrange + monkeypatch.setenv(ANNOTATION_TYPES_ENV, raw) + + # Act + types = annotation_types_from_env() + + # Assert + assert all(entry and entry == entry.strip() for entry in types) + + +def test_transform_annotation_to_c2m2_should_map_the_renamed_columns(): + """Test the renamed columns reach the fields their twins do. + + The annotation TSV publishes the same data as the experiment TSV under + different column names. A missed rename is silent -- the field simply + ends up unset -- so each one is pinned. + + Given: + An annotation row carrying the renamed columns. + When: + transform_annotation_to_c2m2 is called. + Then: + Each value should land where its experiment-named twin would. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["local_id"] == "ENCSR026KJM" # Dataset accession + assert doc["genome_assembly"] == "mm10" # Assembly + assert doc["creation_time"] == "2017-09-12" # Dataset date released + assert doc["extra"]["encode"]["s3_uri"].endswith("ENCFF845ITU.bigBed") # S3 URL + assert doc["extra"]["encode"]["organism"] == "Mus musculus" # Organism + + +def test_transform_annotation_to_c2m2_should_map_the_assay_term_name_rename(): + """Test Assay term name is read as the experiment path reads Assay. + + Given: + An annotation row whose Assay term name is populated, as the + interaction-prediction type's rows are. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset should carry it as its experiment_type. + """ + # Arrange + row = _annotation_row(**{"Assay term name": "DNase-seq"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["experiment_type"] == "DNase-seq" + + +def test_transform_annotation_to_c2m2_should_make_annotation_type_queryable(): + """Test the field that gives an annotation its meaning is stored. + + Without it a client can only find cCRE files by string-matching + filenames, which is the gap the annotation ingest exists to close. + + Given: + A cCRE annotation row. + When: + transform_annotation_to_c2m2 is called. + Then: + annotation_type should be set on both the file and its dataset. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + expected = "candidate Cis-Regulatory Elements" + assert doc["extra"]["encode"]["annotation_type"] == expected + assert doc["collections"][0]["extra"]["encode"]["annotation_type"] == expected + + +def test_transform_annotation_to_c2m2_should_store_the_dataset_only_fields(): + """Test the dataset-scoped annotation-only columns are preserved. + + Given: + An annotation row naming its encyclopedia version and the software + that produced it. + When: + transform_annotation_to_c2m2 is called. + Then: + Both should be stored on the dataset. + """ + # Arrange + row = _annotation_row( + **{ + "Software used": "ABC-Enhancer-Gene-Prediction", + "Encyclopedia Version": "ENCODE v4", + } + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + dataset_extra = doc["collections"][0]["extra"]["encode"] + assert dataset_extra["software_used"] == "ABC-Enhancer-Gene-Prediction" + assert dataset_extra["encyclopedia_version"] == "ENCODE v4" + + +def test_transform_annotation_to_c2m2_should_carry_targets_as_experiment_target(): + """Test Targets reuses the existing scalar rather than a parallel field. + + Given: + An annotation row whose Targets column lists several targets. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset's experiment_target should hold the value verbatim. + """ + # Arrange + row = _annotation_row(**{"Targets": "H3K4me3-mouse, CTCF-mouse"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["experiment_target"] == "H3K4me3-mouse, CTCF-mouse" + + +def test_transform_annotation_to_c2m2_should_carry_donor_traits_on_the_biosample(): + """Test the age columns are preserved as published. + + They are kept as strings, not parsed: the released corpus contains + "2-4" and "unknown" alongside decimals, which is also why they cannot + go to Subject.age_at_sampling. + + Given: + An annotation row whose Age is a range rather than a number. + When: + transform_annotation_to_c2m2 is called. + Then: + All three should sit on the biosample, verbatim. + """ + # Arrange + row = _annotation_row( + **{"Life stage": "embryonic", "Age": "2-4", "Age units": "week"} + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + biosample_extra = doc["collections"][0]["biosamples"][0]["extra"]["encode"] + assert biosample_extra["life_stage"] == "embryonic" + assert biosample_extra["age"] == "2-4" + assert biosample_extra["age_units"] == "week" + + +def test_transform_annotation_to_c2m2_should_leave_experiment_only_fields_unset(): + """Test fields the annotation TSV cannot supply are absent, not invented. + + Given: + An annotation row, whose TSV publishes none of the library, + replicate, genetic-modification or analysis columns. + When: + transform_annotation_to_c2m2 is called. + Then: + Those fields should be absent rather than filled with a derived or + default value. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + for absent in ( + "genome_annotation", + "output_type_detail", + "biological_replicates", + "technical_replicates", + ): + assert absent not in doc + file_extra = doc["extra"]["encode"] + for absent in ( + "read_length", + "mapped_read_length", + "run_type", + "paired_end", + "paired_with", + "index_of", + "file_analysis_title", + "file_analysis_status", + ): + assert absent not in file_extra + biosample_extra = doc["collections"][0]["biosamples"][0]["extra"]["encode"] + assert not any(key.startswith("library_") for key in biosample_extra) + assert "biosample_genetic_modifications" not in biosample_extra + assert "analyte_class" not in doc["collections"][0] + + +def test_transform_annotation_to_c2m2_should_build_no_subject_without_a_donor(): + """Test no donor is fabricated for a TSV that names none. + + The annotation TSV has no Donor(s) column at all, so there is nothing + to key a Subject on. + + Given: + An annotation row. + When: + transform_annotation_to_c2m2 is called. + Then: + Both the dataset and its biosample should carry no subjects. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + collection = doc["collections"][0] + assert collection["subjects"] == [] + assert collection["biosamples"][0]["subjects"] == [] + + +def test_transform_annotation_to_c2m2_should_link_to_the_annotations_path(): + """Test the dataset's persistent_id resolves. + + ENCODE serves annotations and experiments under different paths, so + reusing the experiment path would mint a link that 404s. + + Given: + An annotation row with a dataset accession. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset's persistent_id should point at /annotations/. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["persistent_id"] == ( + "https://www.encodeproject.org/annotations/ENCSR026KJM/" + ) + + +def test_transform_annotation_to_c2m2_should_build_a_dataset_without_a_biosample(): + """Test a dataset accession stays queryable with no biosample term. + + 48 of the released cCRE files name no biosample term. Gating the + dataset on one -- as the experiment path does -- would leave 24 dataset + accessions unreachable. + + Given: + An annotation row with a dataset accession but no biosample term. + When: + transform_annotation_to_c2m2 is called. + Then: + It should still build the dataset -- addressable, linkable and + labelled -- carrying no biosamples. + """ + # Arrange + row = _annotation_row( + **{"Biosample term name": "", "Biosample term id": "", "Biosample type": ""} + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + collection = doc["collections"][0] + assert collection["local_id"] == "ENCSR026KJM" + assert collection["accession_id"] == "ENCSR026KJM" + assert collection["biosamples"] == [] + # Addressable and labelled, not merely present: a collection a client + # cannot resolve or filter is not what those 24 datasets needed. + assert collection["persistent_id"] == ( + "https://www.encodeproject.org/annotations/ENCSR026KJM/" + ) + assert ( + collection["extra"]["encode"]["annotation_type"] + == "candidate Cis-Regulatory Elements" + ) + + +def test_transform_annotation_to_c2m2_should_fold_the_dataset_accession(): + """Test the dataset accession is folded the way filters are. + + Given: + An annotation row whose dataset accession is lower-cased. + When: + transform_annotation_to_c2m2 is called. + Then: + accession_id should be folded while local_id keeps the raw value. + """ + # Arrange + row = _annotation_row(**{"Dataset accession": "encsr026kjm"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + collection = doc["collections"][0] + assert collection["local_id"] == "encsr026kjm" + assert collection["accession_id"] == normalize_accession("encsr026kjm") + + +def test_transform_annotation_to_c2m2_should_return_none_without_an_accession(): + """Test a row with no file accession is skipped rather than inserted. + + Given: + An annotation row whose File accession is blank. + When: + transform_annotation_to_c2m2 is called. + Then: + It should return None. + """ + # Arrange + row = _annotation_row(**{"File accession": " "}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc is None + + +def test_transform_annotation_to_c2m2_should_keep_bedpe_out_of_the_bed_format(): + """Test a paired-interval annotation file is not labelled plain BED. + + A bedpe labelled BED is routed into the tabix BED pipeline, which + indexes the first mate and silently drops the second. + + Given: + An annotation row published as bedpe. + When: + transform_annotation_to_c2m2 is called. + Then: + Its file_format should name bedpe rather than BED. + """ + # Arrange + row = _annotation_row(**{"File format": "bedpe"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["file_format"]["name"] == "bedpe" + + +def test_transform_annotation_to_c2m2_should_populate_the_required_c2m2_fields(): + """Test annotation documents are as complete as experiment documents. + + Given: + An annotation row. + When: + transform_annotation_to_c2m2 is called. + Then: + The fields a client needs to locate and fetch the file should all + be populated, on the same code paths the experiment ingest uses. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["dcc"]["dcc_abbreviation"] == "ENCODE" + assert doc["file_format"] == {"id": "format:3004", "name": "bigBed"} + # The exact term, so deleting the annotation entries from + # OUTPUT_TYPE_TO_EDAM fails at the ingest boundary too, not only in the + # ontology unit test. + assert doc["data_type"] == {"id": "data:1255", "name": "Sequence features"} + assert doc["access_url"] == ANNOTATION_DOWNLOAD_URL + assert doc["md5"] == "5ff392dcde69f8ec512ea381928674d9" + assert doc["size_in_bytes"] == 13743825 + + +@given(name=st.text(min_size=0, max_size=40)) +@settings(max_examples=100) +def test_transform_annotation_to_c2m2_should_not_raise_for_any_download_url(name): + """Test the annotation path is total over download URLs. + + Given: + Any download filename, including empty and punctuation-only ones. + When: + transform_annotation_to_c2m2 is called. + Then: + It should return a document rather than raising. + """ + # Arrange + row = _annotation_row(**{"File download URL": DOWNLOAD_URL.format(name=name)}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc is not None + + +def test_transform_to_c2m2_should_link_to_the_experiments_path(): + """Test an experiment dataset's persistent_id resolves. + + The URL path became a caller-supplied argument when the annotation + ingest landed. ENCODE serves the two dataset kinds under different + paths, so passing the wrong one would give all ~27,000 experiment + collections a link that 404s -- and the annotation side of the same + interpolation is pinned while this one was not. + + Given: + An experiment row with an accession and a biosample term. + When: + transform_to_c2m2 is called. + Then: + The collection's persistent_id should point at /experiments/. + """ + # Arrange + row = _encode_row( + **{"Experiment accession": "ENCSR918ZSJ", "Biosample term name": "K562"} + ) + + # Act + doc = transform_to_c2m2(row) + + # Assert + assert doc["collections"][0]["persistent_id"] == ( + "https://www.encodeproject.org/experiments/ENCSR918ZSJ/" + ) + + +@pytest.mark.parametrize( + "term_id, expected", + [ + ("UBERON:0002048", {"id": "UBERON:0002048", "name": "lung"}), + ("", None), + ], + ids=["with-term-id", "without-term-id"], +) +def test_transform_to_c2m2_should_derive_anatomy_from_the_biosample_term( + term_id, expected +): + """Test anatomy tracks the term id while the collection tracks the name. + + The guard producing anatomy was rewritten when the annotation path + landed, and nothing asserted the field on either path -- deleting the + anatomy block outright would have failed no test. + + Given: + An experiment row with a biosample term name, with and without a + term id. + When: + transform_to_c2m2 is called. + Then: + Anatomy should appear on both the biosample and the collection + when the term id is present and on neither when it is not, with + the collection built either way. + """ + # Arrange + row = _encode_row( + **{ + "Experiment accession": "ENCSR918ZSJ", + "Biosample term name": "lung", + "Biosample term id": term_id, + } + ) + + # Act + doc = transform_to_c2m2(row) + + # Assert + collection = doc["collections"][0] + assert collection["biosamples"][0].get("anatomy") == expected + assert collection.get("anatomy") == ([expected] if expected else None) + + +def test_transform_to_c2m2_should_not_stamp_annotation_fields_on_an_experiment(): + """Test the annotation-only post-processing stays on its own path. + + Given: + An experiment row that happens to carry an Annotation type column. + When: + transform_to_c2m2 is called. + Then: + No annotation_type should appear on the file or its collection. + """ + # Arrange + row = _encode_row( + **{ + "Experiment accession": "ENCSR918ZSJ", + "Biosample term name": "K562", + "Annotation type": "candidate Cis-Regulatory Elements", + } + ) + + # Act + doc = transform_to_c2m2(row) + + # Assert + assert "annotation_type" not in doc.get("extra", {}).get("encode", {}) + assert "annotation_type" not in doc["collections"][0].get("extra", {}).get( + "encode", {} + ) + + +def test_transform_to_c2m2_should_keep_bedpe_out_of_the_bed_format(): + """Test the remap reaches the experiment corpus, not only annotations. + + ENCODE already publishes .bedpe under type=Experiment, and those files + were the original motivation for not calling a paired-interval format + BED. + + Given: + An experiment row published as bedpe. + When: + transform_to_c2m2 is called. + Then: + Its file_format should name bedpe rather than BED. + """ + # Arrange + row = _encode_row(**{"File format": "bedpe"}) + + # Act + doc = transform_to_c2m2(row) + + # Assert + assert doc["file_format"]["name"] == "bedpe" + + +def test_transform_to_c2m2_should_mirror_the_assembly_under_the_dcc_namespace(): + """Test extra.encode.assembly carries the value the schema promises. + + The field was declared and published in the SDL but never written, so + a client reaching for it -- the natural move once multi-assembly cCREs + made assembly a filter people use -- matched nothing at all. + + Given: + An experiment row naming an assembly. + When: + transform_to_c2m2 is called. + Then: + extra.encode.assembly should mirror the top-level genome_assembly. + """ + # Arrange + row = _encode_row(**{"File assembly": "GRCh38"}) + + # Act + doc = transform_to_c2m2(row) + + # Assert + assert doc["genome_assembly"] == "GRCh38" + assert doc["extra"]["encode"]["assembly"] == "GRCh38" + + +def test_transform_annotation_to_c2m2_should_mirror_the_assembly_under_the_dcc_namespace(): + """Test extra.encode.assembly carries the value the schema promises. + + The annotation TSV names the column "Assembly" rather than "File + assembly", so the two transforms reach the same field by different + routes and each needs its own pin. + + Given: + An annotation row naming an assembly. + When: + transform_annotation_to_c2m2 is called. + Then: + extra.encode.assembly should mirror the top-level genome_assembly. + """ + # Arrange + row = _annotation_row(**{"Assembly": "GRCh38"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["genome_assembly"] == "GRCh38" + assert doc["extra"]["encode"]["assembly"] == "GRCh38" + + +def test_transform_annotation_to_c2m2_should_produce_a_valid_file_document(): + """Test the emitted document is one the read path can deserialize. + + The ingest writes plain dicts and the GraphQL resolver reads them back + through FileMetadataModel on every row, so a key the model does not + declare is dropped silently -- the filter would exist and match + nothing. Nothing else joins the two halves. + + Given: + A full annotation row. + When: + The emitted document is validated as a FileMetadataModel. + Then: + It should validate and expose all the annotation fields on their + enriched models. + """ + # Arrange + row = _annotation_row(**{"Software used": "ABC-Enhancer-Gene-Prediction"}) + + # Act + model = FileMetadataModel(**transform_annotation_to_c2m2(row)) + + # Assert + assert model.extra.encode.annotation_type == "candidate Cis-Regulatory Elements" + assert model.extra.encode.organism == "Mus musculus" + assert model.extra.encode.assembly == "mm10" + dataset = model.collections[0] + assert dataset.extra.encode.annotation_type == ( + "candidate Cis-Regulatory Elements" + ) + assert dataset.extra.encode.software_used == "ABC-Enhancer-Gene-Prediction" + assert dataset.extra.encode.encyclopedia_version == "ENCODE v1" + biosample = dataset.biosamples[0] + assert biosample.extra.encode.life_stage == "postnatal" + assert biosample.extra.encode.age == "0" + assert biosample.extra.encode.age_units == "day" + + +def test_transform_annotation_to_c2m2_should_omit_an_empty_dataset_extra(): + """Test a dataset with nothing to enrich carries no empty extra dict. + + An empty ``{"encode": {}}`` would surface as a null facet in any + distinct-value enumeration over the dataset fields. + + Given: + An annotation row whose every dataset-level source is blank. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset should carry no extra key at all. + """ + # Arrange + row = _annotation_row( + **{ + "Project": "", + "dbxrefs": "", + "Annotation type": "", + "Software used": "", + "Encyclopedia Version": "", + } + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert "extra" not in doc["collections"][0] + + +def test_transform_annotation_to_c2m2_should_skip_a_blank_annotation_type(): + """Test a blank annotation type is absent rather than empty. + + Given: + An annotation row with no Annotation type but a populated + Encyclopedia Version. + When: + transform_annotation_to_c2m2 is called. + Then: + Neither the file nor the dataset should carry annotation_type, + while the encyclopedia version still lands. + """ + # Arrange + row = _annotation_row(**{"Annotation type": " "}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert "annotation_type" not in doc["extra"]["encode"] + dataset_extra = doc["collections"][0]["extra"]["encode"] + assert "annotation_type" not in dataset_extra + assert dataset_extra["encyclopedia_version"] == "ENCODE v1" + + +def test_transform_annotation_to_c2m2_should_enrich_a_biosample_keyed_dataset(): + """Test the locally synthesized dataset still carries its annotation fields. + + Reachable on the annotation path because the collection is no longer + gated on the biosample term: a row with a biosample but no dataset + accession falls back to a ``biosample:``-keyed collection. + + Given: + An annotation row with a biosample term but no dataset accession. + When: + transform_annotation_to_c2m2 is called. + Then: + The fallback dataset should carry annotation_type and + experiment_target, and no fabricated accession or persistent id. + """ + # Arrange + row = _annotation_row(**{"Dataset accession": ""}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + dataset = doc["collections"][0] + assert dataset["local_id"] == "biosample:lung" + assert dataset["extra"]["encode"]["annotation_type"] == ( + "candidate Cis-Regulatory Elements" + ) + assert dataset["experiment_target"] == "H3K4me3-mouse, CTCF-mouse" + assert "accession_id" not in dataset + assert "persistent_id" not in dataset + + +def test_transform_annotation_to_c2m2_should_still_label_a_file_with_no_dataset(): + """Test a file with nothing to group it by is still classified. + + Given: + An annotation row with neither a dataset accession nor a biosample + term. + When: + transform_annotation_to_c2m2 is called. + Then: + It should build no collection while the file still carries its + annotation type. + """ + # Arrange + row = _annotation_row( + **{ + "Dataset accession": "", + "Biosample term name": "", + "Biosample term id": "", + } + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"] == [] + assert doc["extra"]["encode"]["annotation_type"] == ( + "candidate Cis-Regulatory Elements" + ) + + +def test_transform_annotation_to_c2m2_should_not_assay_type_an_empty_assay(): + """Test the shape every released cCRE row actually has. + + All 12,448 of them publish an empty Assay term name, so this is the + dominant annotation shape rather than an edge case. + + Given: + An annotation row whose Assay term name is empty. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset should be built with no experiment_type and the file + with no assay_type, rather than either being defaulted. + """ + # Arrange + row = _annotation_row() + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert "assay_type" not in doc + assert "experiment_type" not in doc["collections"][0] + + +def test_transform_annotation_to_c2m2_should_pass_a_multi_valued_assay_through(): + """Test a comma-joined assay list is preserved rather than dropped. + + Some interaction-prediction rows name several assays in one column. + No OBI term matches the joined string, which is correct -- but the + string itself is still the best available description. + + Given: + An annotation row whose Assay term name lists four assays. + When: + transform_annotation_to_c2m2 is called. + Then: + experiment_type should hold it verbatim while assay_type stays + absent. + """ + # Arrange + row = _annotation_row( + **{"Assay term name": "ChIP-seq, RNA-seq, HiC, ATAC-seq"} + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["experiment_type"] == ( + "ChIP-seq, RNA-seq, HiC, ATAC-seq" + ) + assert "assay_type" not in doc + + +def test_transform_annotation_to_c2m2_should_drop_donor_traits_with_no_biosample(): + """Test the donor traits go nowhere when there is no biosample to hold them. + + They are biosample-scoped, so a row with no biosample term has no + honest destination for them short of inventing one. Pinned rather than + fixed: no released annotation row carries both, so nothing is lost + today -- but if ENCODE starts publishing that combination this test is + what makes the loss visible instead of silent. + + Given: + An annotation row naming a life stage and age but no biosample + term. + When: + transform_annotation_to_c2m2 is called. + Then: + The dataset should be built with no biosamples and the traits + should appear nowhere in the document. + """ + # Arrange + row = _annotation_row( + **{ + "Biosample term name": "", + "Biosample term id": "", + "Life stage": "embryonic", + "Age": "10.5", + "Age units": "week", + } + ) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["collections"][0]["biosamples"] == [] + assert "embryonic" not in repr(doc) + assert "10.5" not in repr(doc) + + +def test_transform_annotation_to_c2m2_should_keep_biginteract_out_of_bigbed(): + """Test a bigInteract annotation file is not labelled plain bigBed. + + Extracting it as a bigBed and indexing its leading columns is + range-coherent but degrades the interaction to an interval. + + Given: + An annotation row published as bigInteract. + When: + transform_annotation_to_c2m2 is called. + Then: + Its file_format should name bigInteract rather than bigBed. + """ + # Arrange + row = _annotation_row(**{"File format": "bigInteract"}) + + # Act + doc = transform_annotation_to_c2m2(row) + + # Assert + assert doc["file_format"]["name"] == "bigInteract" + + +def test_transform_to_c2m2_should_store_the_organism_on_the_file(): + """Test the experiment path records organism where annotations do. + + Both TSVs publish the same datum under different names. Populating it + from both keeps the filter meaning the same thing corpus-wide rather + than matching annotation files only. + + Given: + An experiment row naming its Biosample organism. + When: + transform_to_c2m2 is called. + Then: + The file should carry that organism. + """ + # Arrange + row = _encode_row( + **{"Biosample organism": "Homo sapiens", "Biosample term name": "K562"} + ) + + # Act + doc = transform_to_c2m2(row) + + # Assert + assert doc["extra"]["encode"]["organism"] == "Homo sapiens" diff --git a/tests/test_fake_collection.py b/tests/test_fake_collection.py index ba82eae..db8f2b9 100644 --- a/tests/test_fake_collection.py +++ b/tests/test_fake_collection.py @@ -194,16 +194,40 @@ async def test_find_one_should_resolve_dotted_existence_path(self): ) # Assert - # ``$exists`` in the FakeCollection matcher checks ``key in doc``; - # the dotted path ``extra.fourdn.extra_files`` is not a top-level - # key, so the matcher's strict behavior may or may not return the - # doc. Just assert that the call does not raise — the test pins - # the present contract. - # If the matcher does return the doc, confirm the dotted resolver - # delivered the right one. If not, the matcher considers the - # dotted key absent — also valid for the current implementation. - if found is not None: - assert found["local_id"] == "x" + assert found is not None + assert found["local_id"] == "x" + + @pytest.mark.asyncio + async def test_find_one_should_not_match_an_absent_dotted_path(self): + """Test that a negated ``$exists`` excludes docs that have the path. + + Matching by top-level key reports every dotted path as absent, so + this predicate matched the whole collection -- which is how the + ENCODE per-slice clearing selects the documents carrying no + annotation type. A delete built on it would have taken the corpus. + + Given: + One doc carrying a nested path and one without it. + When: + ``find_one`` negates ``$exists`` on that dotted path. + Then: + It should return only the doc that lacks the path. + """ + # Arrange + coll = FakeCollection() + coll.docs.append( + {"local_id": "has", "extra": {"encode": {"annotation_type": "cCRE"}}} + ) + coll.docs.append({"local_id": "lacks", "extra": {"encode": {}}}) + + # Act + found = await coll.find_one( + {"extra.encode.annotation_type": {"$exists": False}} + ) + + # Assert + assert found is not None + assert found["local_id"] == "lacks" @pytest.mark.asyncio async def test_find_one_and_update_should_match_lt_operator_on_datetime(self): diff --git a/tests/test_index.py b/tests/test_index.py index 63adf37..12a10c5 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -10,10 +10,12 @@ from cfdb import api from cfdb.api.routers.index import stream_index_file, stream_index_file_status from cfdb.services import drs, locks +from cfdb.services.ontology_mappings import get_file_format from cfdb.workflows.executor import WoolExecutor from cfdb.workflows.models import ArtifactKind from cfdb.workflows.processors.bam import BamIndexProcessor from cfdb.workflows.processors.registry import ProcessorRegistry, default_registry +from cfdb.workflows.processors.tabix import TabixIntervalProcessor from tests.test_workflows import FIXTURE_MD5 @@ -841,6 +843,65 @@ async def test_stream_index_file_should_return_404_when_no_sidecar_and_no_proces await stream_index_file("4dn", "4DNFIBED01", _make_request(), range=None) assert exc_info.value.status_code == 404 + @pytest.mark.asyncio + @pytest.mark.parametrize("format_name", ["bedpe", "bigInteract"]) + async def test_stream_index_file_should_not_index_a_paired_interval_format( + self, mock_db, mocker, tmp_path, format_name + ): + """Test a paired-interval file is refused rather than mis-indexed. + + Both formats pair two loci per record. While they were aliased to + BED and bigBed the tabix pipeline claimed them and indexed the + first locus alone, committing an artifact that looked successful + and was wrong. Now that each carries its own format name, the + wired registry should claim neither -- so the request fails + cleanly instead. + + Given: + A file in a paired-interval format, no sidecar, and the + workflow subsystem wired with the processors the API + registers. + When: + stream_index_file is called. + Then: + It should raise HTTPException(404) rather than dispatching a + workflow. + """ + # Arrange + from cfdb.workflows.cache import LocalFsCache + + mocker.patch.object(locks, "wait_for_cutover", return_value=None) + # The minted term is injected rather than produced: get_file_format + # has one consumer, the ENCODE transform, so the 4DN ingest cannot + # mint it and a real 4DN .bedpe still arrives declaring BED. This + # pins registry routing, which keys on the format name irrespective + # of DCC -- not that 4DN paired-interval files are safe today. See + # the paired-interval section of ENCODE-SUPPLEMENT.md. + doc = _make_file_doc(file_format=get_file_format(format_name)) + doc.pop("extra", None) + mock_db.files.docs = [] + mock_db.file.docs = [doc] + registry = default_registry() + registry.register(BamIndexProcessor()) + registry.register(TabixIntervalProcessor()) + mocker.patch.object(api, "cache", LocalFsCache(tmp_path / "cache")) + mocker.patch.object(api, "processor_registry", registry) + executor = WoolExecutor( + mock_db, + api.cache, + api.processor_registry, + workdir_root=tmp_path / "jobs", + ) + mocker.patch.object(api, "executor", executor) + ensure = mocker.spy(executor, "ensure_workflow") + + # Act & assert + with pytest.raises(HTTPException) as exc_info: + await stream_index_file("4dn", "4DNFIBED01", _make_request(), range=None) + + assert exc_info.value.status_code == 404 + assert ensure.call_count == 0 + @pytest.mark.asyncio async def test_stream_index_file_should_return_503_when_subsystem_disabled( self, mock_db, mocker diff --git a/tests/test_indexes.py b/tests/test_indexes.py index a4cf19a..97bfcc9 100644 --- a/tests/test_indexes.py +++ b/tests/test_indexes.py @@ -656,6 +656,37 @@ def test_materialized_files_index_specs_should_not_overlap_the_data_specs(): assert {s.collection for s in materialized_files_index_specs()} == {"files"} +def test_materialized_files_index_specs_should_declare_exactly_these_keys(): + """Test the spec list stays a deliberate enumeration. + + The per-concern tests below assert subsets, which reads well but lets an + unintended spec merge unnoticed -- and this list is applied with + ``createIndex`` against a public collection on every ENCODE sync, so an + accidental index is a build cost and a write cost that nothing else + would flag. ``test_all_index_specs_should_not_repeat_a_collection_and_name`` + catches only duplicate names, not extra distinct ones. + + Given: + The materialized files index specs. + When: + Their key tuples are collected. + Then: + They should be exactly the declared set, with no additions. + """ + # Act + keys = {spec.keys for spec in materialized_files_index_specs()} + + # Assert + assert keys == { + (("accession_id", 1),), + (("collections.accession_id", 1),), + (("extra.encode.annotation_type", 1),), + (("extra.encode.organism", 1),), + (("extra.encode.assembly", 1),), + (("genome_assembly", 1),), + } + + def test_materialized_files_index_specs_should_cover_both_accession_paths(): """Test that both queryable accession paths are indexed. @@ -674,7 +705,60 @@ def test_materialized_files_index_specs_should_cover_both_accession_paths(): keys = {spec.keys for spec in materialized_files_index_specs()} # Assert - assert keys == {(("accession_id", 1),), (("collections.accession_id", 1),)} + assert { + (("accession_id", 1),), + (("collections.accession_id", 1),), + } <= keys + + +def test_materialized_files_index_specs_should_cover_the_annotation_facets(): + """Test that the ENCODE annotation filters are indexed. + + ``annotation_type`` is the field the whole annotation corpus is meant + to be reached through, and ``files`` is written directly by the ENCODE + sync, so nothing else would create these. Unindexed, each is a full + scan of the collection on an unauthenticated endpoint. + + Given: + The materialized files index specs. + When: + Their key tuples are collected. + Then: + They should cover the annotation type, organism and assembly paths + that ``to_query`` emits for an ``extra.encode`` filter. + """ + # Act + keys = {spec.keys for spec in materialized_files_index_specs()} + + # Assert + assert { + (("extra.encode.annotation_type", 1),), + (("extra.encode.organism", 1),), + (("extra.encode.assembly", 1),), + } <= keys + + +def test_materialized_files_index_specs_should_cover_the_core_assembly_field(): + """Test the documented assembly filter is indexed, not just its mirror. + + ``genome_assembly`` is the field the schema publishes and the one a + client narrowing to GRCh38 filters on; ``extra.encode.assembly`` is the + DCC mirror. The materializer indexes the core field, but an ENCODE-only + database never runs the materializer, so without this the documented + filter is the one left scanning. + + Given: + The materialized files index specs. + When: + Their key tuples are collected. + Then: + They should cover the core ``genome_assembly`` path. + """ + # Act + keys = {spec.keys for spec in materialized_files_index_specs()} + + # Assert + assert (("genome_assembly", 1),) in keys def test_all_index_specs_should_not_repeat_a_collection_and_name(): diff --git a/tests/test_inputs.py b/tests/test_inputs.py index ffccfca..f301d89 100644 --- a/tests/test_inputs.py +++ b/tests/test_inputs.py @@ -1,8 +1,21 @@ +import pytest from hypothesis import given, settings from hypothesis import strategies as st from cfdb.accessions import normalize_accession -from cfdb.api.gql.inputs import CollectionInput, FileMetadataInput, to_dict, to_query +from cfdb.api.gql.inputs import ( + BiosampleInput, + CollectionInput, + EnrichedBiosampleInput, + EnrichedCollectionInput, + EnrichedEncodeBiosampleInput, + EnrichedEncodeCollectionInput, + EnrichedEncodeFileInput, + EnrichedFileInput, + FileMetadataInput, + to_dict, + to_query, +) #: Alphabet the DCCs actually issue accessions from. _ACCESSION_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" @@ -383,6 +396,159 @@ def test_to_query_should_accept_accession_id_from_the_graphql_inputs(): } +#: Every path the ENCODE annotation ingest writes, paired with the filter +#: that must reach it. A mismatch between the two would leave the whole +#: annotation corpus unfilterable while every ingest test still passed, so +#: these are asserted against the paths ``transform_annotation_to_c2m2`` +#: actually emits rather than against the schema alone. +_ANNOTATION_FILTER_PATHS = [ + ("annotation_type", "extra.encode.annotation_type"), + ("organism", "extra.encode.organism"), + ("assembly", "extra.encode.assembly"), +] + + +@pytest.mark.parametrize("field, path", _ANNOTATION_FILTER_PATHS) +def test_to_query_should_reach_the_file_level_encode_fields(field, path): + """Test a file-level ENCODE filter flattens to the stored path. + + Given: + A FileMetadataInput naming one of the ENCODE file-level fields. + When: + The input is converted with to_dict and then to_query. + Then: + It should produce a predicate on the dotted path the ingest + writes. + """ + # Arrange + payload = FileMetadataInput( + extra=[EnrichedFileInput(encode=[EnrichedEncodeFileInput(**{field: ["v"]})])] + ) + + # Act + query = to_query(to_dict(payload)) + + # Assert + assert query == {path: "v"} + + +@pytest.mark.parametrize( + "field, path", + [ + ("annotation_type", "collections.extra.encode.annotation_type"), + ("software_used", "collections.extra.encode.software_used"), + ("encyclopedia_version", "collections.extra.encode.encyclopedia_version"), + ], +) +def test_to_query_should_reach_the_dataset_level_encode_fields(field, path): + """Test a dataset-level ENCODE filter flattens to the stored path. + + Given: + A FileMetadataInput naming one of the ENCODE dataset-level fields + inside a nested CollectionInput. + When: + The input is converted with to_dict and then to_query. + Then: + It should produce a predicate on the dotted path the ingest + writes. + """ + # Arrange + payload = FileMetadataInput( + collections=[ + CollectionInput( + extra=[ + EnrichedCollectionInput( + encode=[EnrichedEncodeCollectionInput(**{field: ["v"]})] + ) + ] + ) + ] + ) + + # Act + query = to_query(to_dict(payload)) + + # Assert + assert query == {path: "v"} + + +@pytest.mark.parametrize("field", ["life_stage", "age", "age_units"]) +def test_to_query_should_reach_the_biosample_level_encode_fields(field): + """Test the deepest ENCODE filter flattens to the stored path. + + Five segments through two array levels -- the shape most likely to be + got wrong, and the one a client would notice last. + + Given: + A FileMetadataInput naming a donor-trait field nested under + collections.biosamples. + When: + The input is converted with to_dict and then to_query. + Then: + It should produce a predicate on the dotted path the ingest + writes. + """ + # Arrange + payload = FileMetadataInput( + collections=[ + CollectionInput( + biosamples=[ + BiosampleInput( + extra=[ + EnrichedBiosampleInput( + encode=[ + EnrichedEncodeBiosampleInput(**{field: ["v"]}) + ] + ) + ] + ) + ] + ) + ] + ) + + # Act + query = to_query(to_dict(payload)) + + # Assert + assert query == {f"collections.biosamples.extra.encode.{field}": "v"} + + +@given( + value=st.text(min_size=1, max_size=40), + field_and_path=st.sampled_from(_ANNOTATION_FILTER_PATHS), +) +@settings(max_examples=100) +def test_to_query_should_not_fold_an_encode_annotation_filter(value, field_and_path): + """Test the annotation filters match byte-exactly. + + ENCODE's vocabulary is case- and space-significant -- "candidate + Cis-Regulatory Elements" is stored exactly as published -- so folding + any of these the way accessions are folded would make them + permanently unmatchable. + + Given: + Any text value on any ENCODE file-level annotation field. + When: + to_query builds the predicate. + Then: + It should carry the value unaltered under its full dotted path. + """ + # Arrange + field, path = field_and_path + payload = FileMetadataInput( + extra=[ + EnrichedFileInput(encode=[EnrichedEncodeFileInput(**{field: [value]})]) + ] + ) + + # Act + query = to_query(to_dict(payload)) + + # Assert + assert query == {path: value} + + def test_to_query_should_collapse_a_single_clause(): """Test that one set field yields a bare predicate, not a wrapper. diff --git a/tests/test_ontology_mappings.py b/tests/test_ontology_mappings.py index 2835153..18abcc8 100644 --- a/tests/test_ontology_mappings.py +++ b/tests/test_ontology_mappings.py @@ -2,10 +2,29 @@ from __future__ import annotations -from hypothesis import given +import pytest +from hypothesis import given, settings from hypothesis import strategies as st -from cfdb.services.ontology_mappings import get_file_format +from cfdb.services.ontology_mappings import ( + FILE_FORMAT_TO_EDAM, + MINTED_FORMAT_PREFIX, + get_data_type, + get_file_format, +) +from cfdb.workflows.processors.bam import BamIndexProcessor +from cfdb.workflows.processors.registry import default_registry +from cfdb.workflows.processors.tabix import TabixIntervalProcessor + +#: The minted subset of the format table. Extracted so the property tests +#: below share one definition and can size their example budgets from it -- +#: these domains are small closed tables, so exhausting them is both cheaper +#: and stricter than the default budget. +MINTED_ENTRIES = [ + (key, value) + for key, value in FILE_FORMAT_TO_EDAM.items() + if value["id"].startswith(MINTED_FORMAT_PREFIX) +] def test_get_file_format_with_simple_format(): @@ -113,19 +132,196 @@ def test_get_file_format_with_tagalign_case_insensitive(): assert result == {"id": "format:3003", "name": "BED"} -def test_get_file_format_with_biginteract_format(): - """Test bigInteract format maps to bigBed. +def test_get_file_format_should_return_a_minted_term_for_biginteract(): + """Test bigInteract resolves to its own term rather than to bigBed. Given: - The format string "bigInteract" (a bigBed variant). + The format string "bigInteract", a bigBed variant whose trailing + columns encode interaction endpoints. When: get_file_format is called. Then: - It should return the EDAM bigBed term. + It should return a minted term distinct from the EDAM bigBed term, + so the interaction is not silently served as a plain interval. """ + # Act result = get_file_format("bigInteract") - assert result == {"id": "format:3004", "name": "bigBed"} + # Assert + assert result == {"id": "cfdb:biginteract", "name": "bigInteract"} + assert result != get_file_format("bigBed") + + +def test_get_file_format_should_return_a_minted_term_for_bedpe(): + """Test bedpe resolves to its own term rather than to BED. + + Given: + The format string "bedpe", whose columns are chrom1/start1/end1/ + chrom2/start2/end2 rather than a single interval. + When: + get_file_format is called. + Then: + It should return a minted term distinct from the EDAM BED term. + """ + # Act + result = get_file_format("bedpe") + + # Assert + assert result == {"id": "cfdb:bedpe", "name": "bedpe"} + assert result != get_file_format("bed") + + +def _wired_registry(): + """Build the registry the API serves from. + + Mirrors the registrations in ``cfdb.api.main``'s lifespan, so an + assertion about what is claimed holds against the real deployment + rather than a hand-picked pair of classes. + """ + registry = default_registry() + registry.register(BamIndexProcessor()) + registry.register(TabixIntervalProcessor()) + return registry + + +@pytest.mark.parametrize("encode_format", ["bedpe", "bigInteract"]) +def test_get_file_format_should_keep_paired_formats_out_of_the_tabix_pipeline( + encode_format, +): + """Test the paired-interval formats are claimed by no processor. + + Given: + A format whose records pair two loci, which the BED tabix pipeline + would index by the first locus alone -- committing an artifact that + looks successful and is wrong -- and the registry the API wires. + When: + A file document carrying that format is looked up. + Then: + No processor should claim it, so a request streams the raw + upstream file instead of a mangled index. + """ + # Arrange + file_meta = {"file_format": get_file_format(encode_format)} + + # Act + processor = _wired_registry().lookup_for(file_meta) + + # Assert + assert processor is None + + +def test_get_file_format_should_still_route_plain_bed_to_the_tabix_pipeline(): + """Test the minting did not cost the formats that were routed correctly. + + A positive control for the assertion above: "no processor claims it" + would also hold if the registry were empty or lookup were broken. + + Given: + A plain BED format and the registry the API wires. + When: + A file document carrying it is looked up. + Then: + The tabix interval processor should claim it. + """ + # Arrange + file_meta = {"file_format": get_file_format("bed")} + + # Act + processor = _wired_registry().lookup_for(file_meta) + + # Assert + assert isinstance(processor, TabixIntervalProcessor) + + +@settings(max_examples=len(MINTED_ENTRIES)) +@given(entry=st.sampled_from(MINTED_ENTRIES)) +def test_get_file_format_should_derive_a_minted_id_from_its_key(entry): + """Test a minted term's id matches the format it is keyed under. + + A typo'd mint -- "cfdb:biginterract" -- is invisible to every other + assertion, since nothing else compares the id to anything. Asserted + through the accessor rather than against the table directly, so the + property covers what production reads. + + Given: + Any format whose table entry carries a minted id. + When: + get_file_format is called with its key. + Then: + The returned id should be the prefix followed by the key. + """ + # Arrange + key, _ = entry + + # Act + term = get_file_format(key) + + # Assert + assert term["id"] == f"{MINTED_FORMAT_PREFIX}{key}" + + +@settings(max_examples=len(FILE_FORMAT_TO_EDAM)) +@given(entry=st.sampled_from(sorted(FILE_FORMAT_TO_EDAM.items()))) +def test_get_file_format_should_return_only_edam_or_minted_ids(entry): + """Test the accessor admits only two kinds of identifier. + + The minted prefix is the mechanism that keeps an unrepresented format + from being aliased onto a term that means something else. A third id + shape would sit outside it silently. + + Given: + Any format in the table. + When: + get_file_format is called with its key. + Then: + The returned id should be an EDAM format term or a minted token, + and its name should be non-empty. + """ + # Arrange + key, _ = entry + + # Act + term = get_file_format(key) + + # Assert + assert term["id"].startswith("format:") or term["id"].startswith( + MINTED_FORMAT_PREFIX + ) + assert term["name"] + + +@settings(max_examples=len(MINTED_ENTRIES)) +@given(entry=st.sampled_from(MINTED_ENTRIES)) +def test_get_file_format_should_not_let_a_minted_term_borrow_an_edam_name(entry): + """Test a minted term cannot be defeated by reusing an EDAM name. + + Routing keys on the format *name*, not the id, so + ``{"id": "cfdb:x", "name": "BED"}`` would mint an id and still drop + the file into the BED pipeline -- the exact outcome minting exists to + prevent. + + Given: + Any format whose table entry carries a minted id. + When: + get_file_format is called with its key. + Then: + The returned name should be shared with no EDAM entry and claimed + by no processor. + """ + # Arrange + key, _ = entry + edam_names = { + other["name"] + for other in FILE_FORMAT_TO_EDAM.values() + if not other["id"].startswith(MINTED_FORMAT_PREFIX) + } + + # Act + term = get_file_format(key) + + # Assert + assert term["name"] not in edam_names + assert _wired_registry().lookup_for({"file_format": term}) is None def test_get_file_format_with_h5ad_format(): @@ -188,6 +384,48 @@ def test_get_file_format_with_unknown_compound(): assert result is None +@pytest.mark.parametrize( + "output_type, expected", + [ + ( + "candidate Cis-Regulatory Elements", + {"id": "data:1255", "name": "Sequence features"}, + ), + ("elements reference", {"id": "data:1255", "name": "Sequence features"}), + ("element gene links", {"id": "data:0006", "name": "Data"}), + ( + "thresholded element gene links", + {"id": "data:0006", "name": "Data"}, + ), + ("thresholded links", {"id": "data:0006", "name": "Data"}), + ], +) +def test_get_data_type_should_map_every_annotation_output_type(output_type, expected): + """Test the annotation Output type domain resolves to its documented term. + + These five are the complete Output type domain of the two ingested + annotation types, verified against the live TSVs; without them every + annotation file would carry a null data_type. Asserted by exact term + rather than merely non-null, so a copy-paste that pointed cCREs at the + generic Data term fails here. + + Given: + An Output type value published by an ingested annotation type. + When: + get_data_type is called. + Then: + It should return that value's documented CV term. + """ + # Act + result = get_data_type(output_type) + + # Assert + assert result == expected + + +# Unlike the sampled-table properties above, this domain is unbounded, so +# the budget is doing real work rather than exhausting a closed set. +@settings(max_examples=200) @given(st.text()) def test_get_file_format_return_type_invariant(format_string): """Test return type is always None or a dict with id and name keys. diff --git a/tests/test_schema.py b/tests/test_schema.py index 78c5a4b..e0cd244 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -96,6 +96,34 @@ def _make_file_doc( } +def _make_annotation_doc( + local_id: str, + annotation_type: str = "candidate Cis-Regulatory Elements", + organism: str = "Homo sapiens", + genome_assembly: str = "GRCh38", +) -> dict: + """Return an ENCODE annotation file document as the sync writes one.""" + return { + "id_namespace": "ns", + "local_id": local_id, + "project_id_namespace": "ns", + "project_local_id": "proj", + "filename": f"{local_id}.bed.gz", + "submission": "encode", + "data_access_level": "public", + "genome_assembly": genome_assembly, + "dcc": {"dcc_name": "ENCODE", "dcc_abbreviation": "encode"}, + "collections": [], + "extra": { + "encode": { + "annotation_type": annotation_type, + "organism": organism, + "assembly": genome_assembly, + } + }, + } + + def _named_type(type_ref: dict) -> str | None: """Unwrap an introspection type reference down to its named type.""" while type_ref is not None and type_ref.get("name") is None: @@ -912,6 +940,174 @@ async def test_files_should_return_the_uncompressed_sentinel_as_an_empty_string( } assert returned == {"f1": "", "f2": "format:3989"} + @pytest.mark.asyncio + async def test_files_should_filter_documents_by_annotation_type(self, mock_db): + """Test a client can ask for cCRE files without matching filenames. + + The headline promise of the annotation ingest, asserted through + the same GraphQL surface a client uses rather than at the + transform. + + Given: + Two cCRE files, one interaction-prediction file, and one + experiment file carrying no annotation type. + When: + The files query filters on the cCRE annotation type. + Then: + It should return only the two cCRE files. + """ + # Arrange + mock_db.files.docs = [ + _make_annotation_doc("f1", "candidate Cis-Regulatory Elements"), + _make_annotation_doc("f2", "candidate Cis-Regulatory Elements"), + _make_annotation_doc( + "f3", "element gene regulatory interaction predictions" + ), + _make_file_doc("f4", submission="encode"), + ] + + # Act + result = await schema.execute( + """ + query { + files(input: [{ extra: [{ encode: [{ + annotationType: ["candidate Cis-Regulatory Elements"] + }] }] }]) { + totalCount + items { localId } + } + } + """ + ) + + # Assert + assert result.errors is None + assert result.data["files"]["totalCount"] == 2 + assert result.data["files"]["items"] == [{"localId": "f1"}, {"localId": "f2"}] + + @pytest.mark.asyncio + async def test_files_should_filter_documents_by_organism(self, mock_db): + """Test a multi-organism annotation result set can be narrowed. + + The released cCREs span human and mouse, and an annotation row + names no donor -- so without the file-level organism there is + nothing to narrow on but the filename. + + Given: + A human cCRE file and a mouse one. + When: + The files query filters on the mouse organism. + Then: + It should return only the mouse file. + """ + # Arrange + mock_db.files.docs = [ + _make_annotation_doc("f1", organism="Homo sapiens"), + _make_annotation_doc("f2", organism="Mus musculus"), + ] + + # Act + result = await schema.execute( + """ + query { + files(input: [{ extra: [{ encode: [{ + organism: ["Mus musculus"] + }] }] }]) { + totalCount + items { localId } + } + } + """ + ) + + # Assert + assert result.errors is None + assert result.data["files"]["items"] == [{"localId": "f2"}] + + @pytest.mark.asyncio + async def test_files_should_narrow_an_annotation_type_by_assembly(self, mock_db): + """Test the multi-assembly result set the issue describes. + + A client asking for cCREs unqualified gets GRCh38, mm10 and hg19 + together; both fields have to compose for that to be narrowable. + + Given: + cCRE files under GRCh38, mm10 and hg19, plus an mm10 file of a + different annotation type. + When: + The files query filters on the cCRE type and the mm10 + assembly together. + Then: + It should return only the mm10 cCRE file. + """ + # Arrange + mock_db.files.docs = [ + _make_annotation_doc("f1", genome_assembly="GRCh38"), + _make_annotation_doc("f2", genome_assembly="mm10"), + _make_annotation_doc("f3", genome_assembly="hg19"), + _make_annotation_doc( + "f4", + annotation_type="element gene regulatory interaction predictions", + genome_assembly="mm10", + ), + ] + + # Act + result = await schema.execute( + """ + query { + files(input: [{ + genomeAssembly: ["mm10"], + extra: [{ encode: [{ + annotationType: ["candidate Cis-Regulatory Elements"] + }] }] + }]) { + totalCount + items { localId } + } + } + """ + ) + + # Assert + assert result.errors is None + assert result.data["files"]["items"] == [{"localId": "f2"}] + + @pytest.mark.asyncio + async def test_files_should_serialize_the_annotation_fields(self, mock_db): + """Test the annotation fields are readable, not only filterable. + + Given: + One annotation file document. + When: + The files query selects its ENCODE extra fields. + Then: + It should return the annotation type, organism and assembly. + """ + # Arrange + mock_db.files.docs = [_make_annotation_doc("f1")] + + # Act + result = await schema.execute( + """ + query { + files { + items { + extra { encode { annotationType organism assembly } } + } + } + } + """ + ) + + # Assert + assert result.errors is None + assert result.data["files"]["items"][0]["extra"]["encode"] == { + "annotationType": "candidate Cis-Regulatory Elements", + "organism": "Homo sapiens", + "assembly": "GRCh38", + } + @pytest.mark.asyncio async def test_files_should_filter_documents_by_compression_format(self, mock_db): """Test that the derived term is queryable through the input filter. @@ -1298,6 +1494,96 @@ def _patch_cutover(self, mocker): """No-op ``locks.wait_for_cutover`` for every test in this class.""" mocker.patch.object(locks, "wait_for_cutover", return_value=None) + @pytest.mark.asyncio + async def test_distinct_values_should_enumerate_the_annotation_types( + self, mock_db + ): + """Test a client can discover which annotation types exist. + + Filtering by annotation type is only useful if the vocabulary is + discoverable -- otherwise a client has to already know the exact + ENCODE spelling, which is precisely the string-matching the + annotation ingest set out to replace. Low cardinality, so it + belongs in the allowlist where accessions do not. + + Given: + Files spanning two annotation types and one with none. + When: + distinctValues is asked for extra.encode.annotation_type. + Then: + It should return the two types. + """ + # Arrange + mock_db.files.docs = [ + _make_annotation_doc("f1", "candidate Cis-Regulatory Elements"), + _make_annotation_doc("f2", "candidate Cis-Regulatory Elements"), + _make_annotation_doc( + "f3", "element gene regulatory interaction predictions" + ), + _make_distinct_doc("f4", "ENCODE", "encode"), + ] + + # Act + result = await schema.execute( + """ + query { + distinctValues(fields: ["extra.encode.annotation_type"]) { + field + values + } + } + """ + ) + + # Assert + assert result.errors is None + entry = result.data["distinctValues"][0] + assert sorted(entry["values"]) == [ + "candidate Cis-Regulatory Elements", + "element gene regulatory interaction predictions", + ] + + @pytest.mark.asyncio + async def test_distinct_values_should_enumerate_the_assemblies(self, mock_db): + """Test a client can discover which assemblies exist. + + Assembly is named alongside organism in the same acceptance + criterion, and cCREs are published against several of them, so + narrowing to one is the first move a client makes -- which it can + only do knowing the vocabulary. Same small closed vocabulary as its + two sibling facets. + + Given: + ENCODE files spanning two assemblies. + When: + distinctValues is asked for extra.encode.assembly. + Then: + It should return both assemblies. + """ + # Arrange + mock_db.files.docs = [ + _make_annotation_doc("f1", genome_assembly="GRCh38"), + _make_annotation_doc("f2", genome_assembly="GRCh38"), + _make_annotation_doc("f3", genome_assembly="mm10"), + ] + + # Act + result = await schema.execute( + """ + query { + distinctValues(fields: ["extra.encode.assembly"]) { + field + values + } + } + """ + ) + + # Assert + assert result.errors is None + entry = result.data["distinctValues"][0] + assert sorted(entry["values"]) == ["GRCh38", "mm10"] + @pytest.mark.asyncio @pytest.mark.parametrize( "field", ["accession_id", "collections.accession_id"] diff --git a/tests/test_sync.py b/tests/test_sync.py index 46e36a6..e712609 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -6,7 +6,10 @@ import logging import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st +from cfdb import api from cfdb.services import encode as encode_module from cfdb.services import fourdn as fourdn_module from cfdb.services import sync as sync_module @@ -24,6 +27,33 @@ _sync_encode, ) from cfdb.indexes import data_index_specs +from tests.conftest import FakeDB + + +class _EncodeSyncTestBase: + """Base for the classes that drive ``_sync_encode``. + + ``_sync_encode`` fans out over ``annotation_types_from_env()``, whose + default allowlist is non-empty, so a test that mocks only the experiment + fetch would reach the real ENCODE portal once per annotation type and + stream tens of thousands of live rows into its assertions. The autouse + fixture below turns the annotation phases off by default. + + Scoped to these classes rather than the module: nothing in the 4DN or + HuBMAP tests reads the variable, and a module-wide autouse fixture that + silently governs a network boundary is worth keeping close to the tests + that depend on it. + + To opt back in, either set ``ENCODE_ANNOTATION_TYPES`` to the types + under test or ``monkeypatch.delenv`` it to exercise the default + allowlist -- the fixture is function-scoped, so a test's own + monkeypatching wins. + """ + + @pytest.fixture(autouse=True) + def no_annotation_phases(self, monkeypatch): + """Turn the annotation phases off unless a test asks for them.""" + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "") def _encode_metadata_row(accession: str, filename: str) -> dict: @@ -37,12 +67,65 @@ def _encode_metadata_row(accession: str, filename: str) -> dict: } +def _encode_annotation_row(accession: str, dataset: str, filename: str) -> dict: + """Return a minimal ENCODE annotation TSV row, using annotation column names.""" + return { + "File accession": accession, + "File format": "bed", + "Dataset accession": dataset, + "Annotation type": "candidate Cis-Regulatory Elements", + "Assembly": "GRCh38", + "Organism": "Homo sapiens", + "File download URL": ( + f"https://www.encodeproject.org/files/{accession}/@@download/{filename}" + ), + } + + async def _async_iter(rows): """Yield the given rows, standing in for the streaming metadata fetch.""" for row in rows: yield row +async def _async_raise(exc): + """Stand in for a metadata stream that fails before yielding anything.""" + raise exc + yield # pragma: no cover - unreachable, marks this an async generator + + +async def _async_iter_then_raise(rows, exc): + """Stand in for a stream that dies partway through. + + The production failure shape: the metadata fetch's timeout bounds the + whole streamed body, so it fires with rows already delivered and + inserted, not before the first one. + """ + for row in rows: + yield row + raise exc + + +def _fail_insert_on_call(collection, failing_call: int, exc: Exception) -> None: + """Make one of a fake collection's insert_many calls raise. + + Fails the *sink* rather than the stream, which is the half of the ingest + the stream helpers above cannot reach. Keyed by call ordinal so a test can + choose a batch boundary without inspecting the rows. + """ + real = collection.insert_many + calls = 0 + + async def insert_many(docs): + nonlocal calls + calls += 1 + if calls == failing_call: + raise exc + return await real(docs) + + collection.insert_many = insert_many + + class TestSyncDccsDataIndexing: @pytest.mark.asyncio async def test__sync_dccs_should_ensure_data_indexes_after_load( @@ -102,6 +185,199 @@ async def test__sync_dccs_should_skip_indexing_when_no_dccs( ensure_mock.assert_not_awaited() +class TestSyncDccsFailureIsolation: + @pytest.mark.asyncio + async def test__sync_dccs_should_sync_the_remaining_dccs_when_one_fails( + self, mocker, mock_db, tmp_path + ): + """Test one DCC's failure does not cost the others their sync. + + get_all_dcc_names sorts, so ENCODE is attempted before HuBMAP on a + whole-corpus sync. Letting the failure escape the loop meant one + failed ENCODE phase -- three network streams by default, against a + portal that 504s on slow requests -- skipped HuBMAP entirely. + + Given: + A three-DCC sync whose first DCC raises. + When: + _sync_dccs runs. + Then: + It should still attempt both remaining DCCs before failing. + """ + # Arrange + mocker.patch.object(sync_module, "DATA_DIR", str(tmp_path)) + mocker.patch.object(sync_module, "get_dcc_type", return_value="rest_api") + mocker.patch.object( + sync_module, + "_sync_encode", + mocker.AsyncMock(side_effect=RuntimeError("encode died")), + ) + zip_sync = mocker.patch.object( + sync_module, "_sync_c2m2_zip", mocker.AsyncMock() + ) + mocker.patch.object(sync_module, "ensure_indexes", mocker.AsyncMock()) + mocker.patch.object( + sync_module, "_log_accession_coverage", mocker.AsyncMock() + ) + task = SyncTask(id="t1", dcc_names=["encode", "4dn", "hubmap"]) + + # Act & assert + with pytest.raises(RuntimeError, match="encode"): + await _sync_dccs(task) + + assert zip_sync.await_count == 2 + + @pytest.mark.asyncio + async def test__sync_dccs_should_ensure_data_indexes_when_a_dcc_failed( + self, mocker, mock_db, tmp_path + ): + """Test a partial run still leaves what loaded queryable. + + The index build sat after the loop, so the first failure skipped it + and left the DCCs that did load answering every query by collection + scan. + + Given: + A two-DCC sync whose first DCC raises. + When: + _sync_dccs runs. + Then: + It should still ensure the data indexes once. + """ + # Arrange + mocker.patch.object(sync_module, "DATA_DIR", str(tmp_path)) + mocker.patch.object(sync_module, "get_dcc_type", return_value="rest_api") + mocker.patch.object( + sync_module, + "_sync_encode", + mocker.AsyncMock(side_effect=RuntimeError("encode died")), + ) + mocker.patch.object(sync_module, "_sync_c2m2_zip", mocker.AsyncMock()) + ensure_mock = mocker.patch.object( + sync_module, "ensure_indexes", mocker.AsyncMock() + ) + mocker.patch.object( + sync_module, "_log_accession_coverage", mocker.AsyncMock() + ) + task = SyncTask(id="t1", dcc_names=["encode", "hubmap"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_dccs(task) + + ensure_mock.assert_awaited_once() + + @pytest.mark.asyncio + async def test__sync_dccs_should_not_report_a_failed_dcc_as_synced( + self, mocker, mock_db, tmp_path + ): + """Test the per-DCC success reporting skips the DCC that failed. + + Given: + A two-DCC sync whose first DCC raises. + When: + _sync_dccs runs. + Then: + Only the DCC that succeeded should be logged as synced and have + its accession coverage reported. + """ + # Arrange + mocker.patch.object(sync_module, "DATA_DIR", str(tmp_path)) + mocker.patch.object(sync_module, "get_dcc_type", return_value="rest_api") + mocker.patch.object( + sync_module, + "_sync_encode", + mocker.AsyncMock(side_effect=RuntimeError("encode died")), + ) + mocker.patch.object(sync_module, "_sync_c2m2_zip", mocker.AsyncMock()) + mocker.patch.object(sync_module, "ensure_indexes", mocker.AsyncMock()) + coverage = mocker.patch.object( + sync_module, "_log_accession_coverage", mocker.AsyncMock() + ) + task = SyncTask(id="t1", dcc_names=["encode", "hubmap"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_dccs(task) + + assert [call.args[0] for call in coverage.await_args_list] == ["hubmap"] + assert task.progress == "Sync incomplete: 1 of 2 DCCs synced" + + @pytest.mark.asyncio + async def test__sync_dccs_should_chain_the_first_dcc_failure_as_the_cause( + self, mocker, mock_db, tmp_path + ): + """Test the raised error keeps the first failure's traceback. + + Given: + A two-DCC sync where both DCCs raise distinguishable errors. + When: + _sync_dccs runs. + Then: + The error's cause should be the first failure's exception. + """ + # Arrange + first = RuntimeError("encode died") + second = RuntimeError("hubmap died") + mocker.patch.object(sync_module, "DATA_DIR", str(tmp_path)) + mocker.patch.object(sync_module, "get_dcc_type", return_value="rest_api") + mocker.patch.object( + sync_module, "_sync_encode", mocker.AsyncMock(side_effect=first) + ) + mocker.patch.object( + sync_module, "_sync_c2m2_zip", mocker.AsyncMock(side_effect=second) + ) + mocker.patch.object(sync_module, "ensure_indexes", mocker.AsyncMock()) + mocker.patch.object( + sync_module, "_log_accession_coverage", mocker.AsyncMock() + ) + task = SyncTask(id="t1", dcc_names=["encode", "hubmap"]) + + # Act & assert + with pytest.raises(RuntimeError) as excinfo: + await _sync_dccs(task) + + assert excinfo.value.__cause__ is first + + @pytest.mark.asyncio + async def test__sync_dccs_should_propagate_a_cancellation( + self, mocker, mock_db, tmp_path + ): + """Test cancellation cancels the run rather than failing one DCC. + + The per-DCC handler is deliberately broad, and a CancelledError + caught by it would be recorded as a DCC failure and followed by + every remaining DCC -- the opposite of cancelling. + + Given: + A two-DCC sync whose first DCC raises CancelledError. + When: + _sync_dccs runs. + Then: + The CancelledError should propagate and the second DCC should + never run. + """ + # Arrange + mocker.patch.object(sync_module, "DATA_DIR", str(tmp_path)) + mocker.patch.object(sync_module, "get_dcc_type", return_value="rest_api") + mocker.patch.object( + sync_module, + "_sync_encode", + mocker.AsyncMock(side_effect=asyncio.CancelledError()), + ) + zip_sync = mocker.patch.object( + sync_module, "_sync_c2m2_zip", mocker.AsyncMock() + ) + mocker.patch.object(sync_module, "ensure_indexes", mocker.AsyncMock()) + task = SyncTask(id="t1", dcc_names=["encode", "hubmap"]) + + # Act & assert + with pytest.raises(asyncio.CancelledError): + await _sync_dccs(task) + + zip_sync.assert_not_awaited() + + # --------------------------------------------------------------------------- # _prune_non_public_hubmap_raw_records # --------------------------------------------------------------------------- @@ -392,7 +668,7 @@ async def test__load_dataset_async_should_not_add_compression_format_when_absent # --------------------------------------------------------------------------- -class TestSyncEncode: +class TestSyncEncode(_EncodeSyncTestBase): @pytest.mark.asyncio async def test__sync_encode_should_populate_accession_id_on_inserted_docs( self, mock_db, mocker @@ -417,7 +693,7 @@ async def test__sync_encode_should_populate_accession_id_on_inserted_docs( row["Experiment accession"] = "encsr918zsj" row["Biosample term name"] = "K562" mocker.patch.object( - encode_module, "fetch_encode_metadata", lambda: _async_iter([row]) + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([row]) ) # Act @@ -451,7 +727,7 @@ async def test__sync_encode_should_populate_compression_format_on_inserted_docs( _encode_metadata_row("ENCFF003CCC", "ENCFF003CCC.bed.starch"), ] mocker.patch.object( - encode_module, "fetch_encode_metadata", lambda: _async_iter(rows) + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) ) # Act @@ -491,7 +767,7 @@ async def test__sync_encode_should_log_the_compression_format_distribution( _encode_metadata_row("ENCFF003CCC", "ENCFF003CCC.bed.starch"), ] mocker.patch.object( - encode_module, "fetch_encode_metadata", lambda: _async_iter(rows) + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) ) # Act @@ -536,7 +812,7 @@ async def test__sync_encode_should_leave_other_dcc_documents_unchanged( mocker.patch.object( encode_module, "fetch_encode_metadata", - lambda: _async_iter([_encode_metadata_row("ENCFF001AAA", "x.bed.gz")]), + lambda deadline=None: _async_iter([_encode_metadata_row("ENCFF001AAA", "x.bed.gz")]), ) # Act @@ -547,6 +823,1127 @@ async def test__sync_encode_should_leave_other_dcc_documents_unchanged( assert survivors == others +class TestSyncEncodeAnnotationPhases(_EncodeSyncTestBase): + @pytest.mark.asyncio + async def test__sync_encode_should_ingest_annotations_alongside_experiments( + self, mock_db, mocker, monkeypatch + ): + """Test that both metadata streams reach the files collection. + + Given: + One configured annotation type, plus an experiment stream and an + annotation stream each yielding one row. + When: + _sync_encode runs. + Then: + Both documents should be inserted, and the annotation one should + carry the annotation_type that makes it findable. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter([_encode_metadata_row("ENCFF001AAA", "x.bed.gz")]), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + by_id = {d["local_id"]: d for d in mock_db.files.docs} + assert set(by_id) == {"ENCFF001AAA", "ENCFF002BBB"} + annotation = by_id["ENCFF002BBB"] + assert ( + annotation["extra"]["encode"]["annotation_type"] + == "candidate Cis-Regulatory Elements" + ) + assert annotation["collections"][0]["accession_id"] == "ENCSR001AAA" + + @pytest.mark.asyncio + async def test__sync_encode_should_request_only_the_configured_types( + self, mock_db, mocker, monkeypatch + ): + """Test that the allowlist bounds which annotation types are fetched. + + Given: + Two annotation types configured out of the far larger space + ENCODE publishes. + When: + _sync_encode runs. + Then: + The annotation fetch should be called once per configured type + and with no other type. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "chromatin state, footprints") + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + fetch_annotations = mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter([]), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + requested = [call.args[0] for call in fetch_annotations.call_args_list] + assert requested == ["chromatin state", "footprints"] + + @pytest.mark.asyncio + async def test__sync_encode_should_ingest_annotations_when_experiments_fail( + self, mock_db, mocker, monkeypatch + ): + """Test that a failed experiment stream does not cost the annotations. + + Given: + An experiment stream that raises, and a healthy annotation + stream. + When: + _sync_encode runs. + Then: + The annotation document should still be inserted, and the sync + should fail afterwards naming the phase that broke. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(RuntimeError("experiment stream died")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ), + ) + + # Act & assert + with pytest.raises(RuntimeError, match="experiment"): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert [d["local_id"] for d in mock_db.files.docs] == ["ENCFF002BBB"] + + @pytest.mark.asyncio + async def test__sync_encode_should_ingest_experiments_when_an_annotation_fails( + self, mock_db, mocker, monkeypatch + ): + """Test that one failed annotation type does not cost the others. + + Given: + Two configured annotation types where the first stream raises, + alongside a healthy experiment stream. + When: + _sync_encode runs. + Then: + The experiment document and the surviving annotation type's + document should both be inserted. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "broken, healthy") + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter([_encode_metadata_row("ENCFF001AAA", "x.bed.gz")]), + ) + + def _annotation_stream(annotation_type, deadline=None): + if annotation_type == "broken": + return _async_raise(RuntimeError("annotation stream died")) + return _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ) + + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=_annotation_stream, + ) + + # Act & assert + with pytest.raises(RuntimeError, match="annotation\\[broken\\]"): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert {d["local_id"] for d in mock_db.files.docs} == { + "ENCFF001AAA", + "ENCFF002BBB", + } + + @pytest.mark.asyncio + async def test__sync_encode_should_ensure_the_indexes_when_a_phase_failed( + self, mock_db, mocker, monkeypatch + ): + """Test that a partial load is still left queryable. + + Given: + An experiment stream that raises and a healthy annotation + stream, so the sync ends in failure with rows loaded. + When: + _sync_encode runs. + Then: + The accession indexes should still be ensured, so what did load + is not left behind a full collection scan. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(RuntimeError("experiment stream died")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ), + ) + ensure = mocker.patch.object( + sync_module, "ensure_indexes", mocker.AsyncMock(return_value=0) + ) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert ensure.await_count == 1 + + @pytest.mark.asyncio + async def test__sync_encode_should_report_the_rows_a_failed_phase_committed( + self, mock_db, mocker, monkeypatch + ): + """Test the reported total matches what is actually in the database. + + A stream bounded by a wall-clock budget fails mid-flight, with rows + already committed. Reporting a phase's contribution only on its + clean return dropped every one of those rows from the total while + leaving them in the collection, so the sync under-reported the + corpus it had just written. + + Given: + A batch size of 2 and an experiment stream that yields 5 rows + and then dies, alongside a healthy annotation phase. + When: + _sync_encode runs. + Then: + The count in the final progress should equal the ENCODE + documents actually present. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + monkeypatch.setattr(sync_module, "BATCH_SIZE", 2) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") for i in range(5) + ] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter_then_raise(rows, TimeoutError("budget exhausted")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF999ZZZ", "ENCSR001AAA", "y.bed.gz")] + ), + ) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(task) + + inserted = [d for d in mock_db.files.docs if d["submission"] == "encode"] + # Anchored rather than a substring: "6 files" is contained in + # "16 files", so containment passes on an order-of-magnitude error. + assert task.progress.startswith( + f"ENCODE sync incomplete: {len(inserted)} files" + ) + + @pytest.mark.asyncio + async def test__sync_encode_should_commit_the_partial_batch_of_a_failed_phase( + self, mock_db, mocker, monkeypatch + ): + """Test rows transformed before a stream died are not thrown away. + + The DCC is cleared before the load, so a row discarded here is a + row the corpus loses outright until the next full re-sync. + + Given: + A batch size of 2 and a stream that yields 5 rows and then + dies, leaving one row in an uncommitted partial batch. + When: + _sync_encode runs. + Then: + All 5 rows should be committed, not just the two full batches. + """ + # Arrange + monkeypatch.setattr(sync_module, "BATCH_SIZE", 2) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") for i in range(5) + ] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter_then_raise(rows, TimeoutError("budget exhausted")), + ) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert len(mock_db.files.docs) == 5 + + @pytest.mark.asyncio + async def test__sync_encode_should_fail_when_the_trailing_batch_fails_to_commit( + self, mock_db, mocker, monkeypatch + ): + """Test a sink failure on a cleanly drained stream fails the sync. + + The flush of the trailing partial batch used to sit in a ``finally`` + that suppressed its own failure. A ``finally`` also runs when nothing + is propagating, so this reported a clean sync over a corpus short by + the trailing batch -- against a DCC cleared before the load. + + Given: + A batch size of 2 and a stream of 3 rows that drains cleanly, + whose trailing one-row batch fails to insert. + When: + _sync_encode runs. + Then: + It should raise and report only the two rows that committed. + """ + # Arrange + monkeypatch.setattr(sync_module, "BATCH_SIZE", 2) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") for i in range(3) + ] + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) + ) + _fail_insert_on_call(mock_db.files, 2, RuntimeError("mongo failover")) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(task) + + assert len(mock_db.files.docs) == 2 + assert task.progress.startswith("ENCODE sync incomplete: 2 files") + + @pytest.mark.asyncio + async def test__sync_encode_should_not_resubmit_a_batch_the_sink_rejected( + self, mock_db, mocker, monkeypatch + ): + """Test a batch that failed to insert is not sent a second time. + + The buffer used to be cleared only after a successful insert, so a + failed one left the same list for the trailing flush to submit again. + + Given: + A batch size of 2 and a stream of 4 rows whose first batch fails + to insert. + When: + _sync_encode runs. + Then: + The rejected batch should never reach the collection, and the + reported count should exclude it. + """ + # Arrange + monkeypatch.setattr(sync_module, "BATCH_SIZE", 2) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") for i in range(4) + ] + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) + ) + _fail_insert_on_call(mock_db.files, 1, RuntimeError("mongo failover")) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(task) + + assert mock_db.files.docs == [] + assert task.progress.startswith("ENCODE sync incomplete: 0 files") + + @pytest.mark.asyncio + async def test__sync_encode_should_give_every_phase_the_same_deadline( + self, mock_db, mocker, monkeypatch + ): + """Test the download budget bounds the sync rather than each stream. + + Every phase runs inside the one cutover lock that gates the read + surface, so a per-stream budget would multiply the outage by the + phase count and carry the worst case past the sync lock's one-hour + stale threshold -- at which point a second sync is admitted and + clears the corpus while this one is still writing to it. + + Given: + Two configured annotation types, so three phases run. + When: + _sync_encode runs. + Then: + Every phase should be handed one and the same deadline. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha,beta") + deadlines = [] + + def _experiment_stream(deadline=None): + deadlines.append(deadline) + return _async_iter([]) + + def _annotation_stream(annotation_type, deadline=None): + deadlines.append(deadline) + return _async_iter([]) + + mocker.patch.object( + encode_module, "fetch_encode_metadata", _experiment_stream + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=_annotation_stream, + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert len(deadlines) == 3 + assert None not in deadlines + assert len(set(deadlines)) == 1 + + @pytest.mark.asyncio + async def test__sync_encode_should_ingest_a_repeated_type_only_once( + self, mock_db, mocker, monkeypatch + ): + """Test a duplicated allowlist entry does not double-load its type. + + ENCODE documents are written with insert_many into a collection + carrying no unique key, so a repeated token would insert every file + of that type twice with nothing to reject it. + + Given: + ENCODE_ANNOTATION_TYPES naming the same type twice. + When: + _sync_encode runs. + Then: + The annotation fetch should run once and each document should + appear once. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "chromatin state,chromatin state") + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + fetch_annotations = mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert fetch_annotations.call_count == 1 + assert [d["local_id"] for d in mock_db.files.docs] == ["ENCFF002BBB"] + + @pytest.mark.asyncio + async def test__sync_encode_should_report_a_configured_type_that_returned_nothing( + self, mock_db, mocker, monkeypatch, caplog + ): + """Test an empty annotation type is logged as zero, not omitted. + + A type ENCODE stops publishing is the failure this tally exists to + surface. An absent key is what nobody notices when diffing logs; a + zero is what everybody does. + + Given: + Two configured types, one returning rows and one returning + none. + When: + _sync_encode runs. + Then: + The annotation_type distribution should carry the empty type + with a count of zero. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "populated,vanished") + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + + def _annotation_stream(annotation_type, deadline=None): + if annotation_type == "vanished": + return _async_iter([]) + return _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ) + + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=_annotation_stream, + ) + + # Act + with caplog.at_level(logging.INFO): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + distribution = next( + m for m in caplog.messages if "annotation_type distribution" in m + ) + assert "'vanished': 0" in distribution + + @pytest.mark.asyncio + async def test__sync_encode_should_propagate_a_cancellation( + self, mock_db, mocker, monkeypatch + ): + """Test cancellation cancels the sync rather than failing one phase. + + The per-phase handler is deliberately broad, and a CancelledError + caught by it would be logged as a phase failure and followed by + every remaining phase -- the opposite of cancelling. + + Given: + An experiment stream raising CancelledError and a configured + annotation type. + When: + _sync_encode runs. + Then: + The CancelledError should propagate and the annotation phase + should never run. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(asyncio.CancelledError()), + ) + fetch_annotations = mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter([]), + ) + + # Act & assert + with pytest.raises(asyncio.CancelledError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + fetch_annotations.assert_not_called() + + @pytest.mark.asyncio + async def test__sync_encode_should_not_commit_buffered_rows_when_cancelled( + self, mock_db, mocker, monkeypatch + ): + """Test a cancelled sync stops writing rather than flushing on its way out. + + The trailing-batch flush runs from a ``finally``, which a cancellation + unwinds through as readily as a failure. Writing there would commit + rows after the decision to stop, and a cancellation re-delivered by + that await would replace the original and skip the phase's tally. + + Given: + A batch size large enough to leave every row buffered, and a + stream that yields three rows and then raises CancelledError. + When: + _sync_encode runs. + Then: + The cancellation should propagate with the buffered rows never + committed. + """ + # Arrange + monkeypatch.setattr(sync_module, "BATCH_SIZE", 100) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") for i in range(3) + ] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter_then_raise(rows, asyncio.CancelledError()), + ) + + # Act & assert + with pytest.raises(asyncio.CancelledError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert mock_db.files.docs == [] + + @pytest.mark.asyncio + async def test__sync_encode_should_still_index_when_every_phase_fails( + self, mock_db, mocker, monkeypatch + ): + """Test the total-failure path leaves a coherent, queryable state. + + No phase delivered a row, so no phase replaced its slice: the corpus + is last sync's, in full, rather than empty. Emptying it would have + been the one outcome with nothing to recommend it -- the load failed, + so there is nothing to put in its place. + + Given: + An experiment stream and both configured annotation streams + all raising, over a pre-seeded stale ENCODE document. + When: + _sync_encode runs. + Then: + The stale document should survive, the indexes should still be + ensured, and the error should name all three phases. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha,beta") + mock_db.files.docs = [{"submission": "encode", "local_id": "STALE"}] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(RuntimeError("experiment died")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_raise( + RuntimeError(f"{annotation_type} died") + ), + ) + ensure = mocker.patch.object( + sync_module, "ensure_indexes", mocker.AsyncMock(return_value=0) + ) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act & assert + with pytest.raises(RuntimeError) as excinfo: + await _sync_encode(task) + + assert [d["local_id"] for d in mock_db.files.docs] == ["STALE"] + assert ensure.await_count == 1 + for label in ("experiment", "annotation[alpha]", "annotation[beta]"): + assert label in str(excinfo.value) + assert "0 of 3 phases" in task.progress + + @pytest.mark.asyncio + async def test__sync_encode_should_chain_the_first_failure_as_the_cause( + self, mock_db, mocker, monkeypatch + ): + """Test the raised error keeps the first failure's traceback. + + Given: + Two failing phases raising distinguishable exceptions. + When: + _sync_encode runs. + Then: + The RuntimeError's __cause__ should be the first one, so the + traceback points at the failure that started the cascade. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha") + first = RuntimeError("experiment died first") + second = RuntimeError("annotation died second") + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_raise(first) + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_raise(second), + ) + + # Act & assert + with pytest.raises(RuntimeError) as excinfo: + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert excinfo.value.__cause__ is first + + @pytest.mark.asyncio + async def test__sync_encode_should_not_report_a_partial_load_as_complete( + self, mock_db, mocker, monkeypatch + ): + """Test the task's own progress text admits a phase was lost. + + Given: + An experiment stream that raises and a healthy annotation + stream. + When: + _sync_encode runs. + Then: + The task's final progress should say the sync was incomplete, + rather than reporting a clean sync over a partial corpus. + """ + # Arrange + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", "candidate Cis-Regulatory Elements" + ) + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(RuntimeError("experiment stream died")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ), + ) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(task) + + assert "incomplete" in task.progress + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "row_count, expected_inserts", + [(3, 1), (4, 2), (0, 0)], + ids=["exactly-one-batch", "one-batch-plus-remainder", "empty-stream"], + ) + async def test__sync_encode_should_insert_one_batch_per_batch_size_rows( + self, mock_db, mocker, monkeypatch, row_count, expected_inserts + ): + """Test the batching boundary commits every row exactly once. + + The three cases are the boundary itself, the remainder path, and + the empty stream that must not issue a trailing empty insert. + + Given: + A batch size of 3 and a stream of 3, 4 or 0 rows. + When: + _sync_encode runs. + Then: + insert_many should be called the expected number of times and + every row should land exactly once. + """ + # Arrange + monkeypatch.setattr(sync_module, "BATCH_SIZE", 3) + rows = [ + _encode_metadata_row(f"ENCFF00{i}AAA", f"f{i}.bed.gz") + for i in range(row_count) + ] + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) + ) + # Spied, not asserted on by argument: _ingest_encode_rows clears the + # same list it hands to insert_many, so a recorded call reads back + # empty. + spy = mocker.spy(mock_db.files, "insert_many") + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert spy.call_count == expected_inserts + assert len(mock_db.files.docs) == row_count + + @pytest.mark.asyncio + async def test__sync_encode_should_not_count_a_row_it_skipped( + self, mock_db, mocker, caplog + ): + """Test an untransformable row is absent from the corpus and tallies. + + Given: + Three experiment rows where the middle one has no File + accession and so transforms to None. + When: + _sync_encode runs. + Then: + Two documents should be inserted and the compression + distribution should account for two, not three. + """ + # Arrange + rows = [ + _encode_metadata_row("ENCFF001AAA", "a.bed.gz"), + {"File accession": " ", "File format": "bed"}, + _encode_metadata_row("ENCFF003CCC", "c.bed.gz"), + ] + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter(rows) + ) + task = SyncTask(id="t1", dcc_names=["encode"]) + + # Act + with caplog.at_level(logging.INFO): + await _sync_encode(task) + + # Assert + assert len(mock_db.files.docs) == 2 + assert task.progress == "ENCODE sync complete: 2 files" + distribution = next( + m for m in caplog.messages if "compression_format distribution" in m + ) + assert "'format:3989': 2" in distribution + + @pytest.mark.asyncio + async def test__sync_encode_should_not_let_a_phase_clear_another_phases_rows( + self, mock_db, mocker, monkeypatch + ): + """Test each phase's clear reaches only the slice that phase reloads. + + Every phase clears before loading, so a filter that selected more + than its own slice would delete the preceding phases' documents as + each new one started, leaving only the last phase's rows. + + Given: + A stale ENCODE document and three healthy phases. + When: + _sync_encode runs. + Then: + The stale document should be gone and all three phases' + documents should survive together. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha,beta") + mock_db.files.docs = [{"submission": "encode", "local_id": "STALE"}] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter([_encode_metadata_row("ENCFF001AAA", "a.bed.gz")]), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter( + [ + _encode_annotation_row( + f"ENCFF_{annotation_type}", "ENCSR001AAA", "y.bed.gz" + ) + ] + ), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert {d["local_id"] for d in mock_db.files.docs} == { + "ENCFF001AAA", + "ENCFF_alpha", + "ENCFF_beta", + } + + @pytest.mark.asyncio + async def test__sync_encode_should_keep_a_failed_phases_rows_until_it_reloads_them( + self, mock_db, mocker, monkeypatch + ): + """Test a failed phase serves stale data rather than none. + + One corpus-wide clear before the fan-out destroyed every phase's data + before any failure was knowable, so a failed experiment phase left + the API serving the annotation documents as the whole corpus. The + experiment phase is the largest and slowest stream, so it is the + likeliest to be the one that fails. + + Given: + A stale experiment document, an experiment phase that fails + before yielding, and a healthy annotation phase. + When: + _sync_encode runs. + Then: + The stale experiment document should survive alongside the + freshly loaded annotation document. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha") + mock_db.files.docs = [{"submission": "encode", "local_id": "STALE_EXPERIMENT"}] + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_raise(RuntimeError("experiment died")), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter( + [_encode_annotation_row("ENCFF_alpha", "ENCSR001AAA", "y.bed.gz")] + ), + ) + + # Act & assert + with pytest.raises(RuntimeError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + assert {d["local_id"] for d in mock_db.files.docs} == { + "STALE_EXPERIMENT", + "ENCFF_alpha", + } + + @pytest.mark.asyncio + async def test__sync_encode_should_empty_a_slice_whose_stream_returned_nothing( + self, mock_db, mocker, monkeypatch + ): + """Test a type that stopped being published stops being served. + + The clear is deferred until a phase has rows to put back, so that a + phase failing before it delivers any leaves its previous rows alone. + A stream that drains cleanly with nothing in it is the other case + entirely: the empty result is the answer, and keeping last sync's + rows would serve documents ENCODE no longer publishes. + + Given: + A stale document for a configured type whose stream then yields + no rows at all. + When: + _sync_encode runs. + Then: + The stale document should be gone. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "alpha") + mock_db.files.docs = [ + { + "submission": "encode", + "local_id": "STALE_ALPHA", + "extra": {"encode": {"annotation_type": "alpha"}}, + } + ] + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter([]), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert mock_db.files.docs == [] + + @pytest.mark.asyncio + async def test__sync_encode_should_record_a_failed_phase_as_zero_in_the_per_phase_log( + self, mock_db, mocker, monkeypatch, caplog + ): + """Test the per-phase log distinguishes what loaded from what did not. + + Given: + One healthy annotation phase and one that fails before + yielding. + When: + _sync_encode runs. + Then: + The per-phase counts should name the healthy phase and record + the failed one as having loaded nothing. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "healthy,broken") + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + + def _annotation_stream(annotation_type, deadline=None): + if annotation_type == "broken": + return _async_raise(RuntimeError("died")) + return _async_iter( + [_encode_annotation_row("ENCFF002BBB", "ENCSR001AAA", "y.bed.gz")] + ) + + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=_annotation_stream, + ) + + # Act + with caplog.at_level(logging.INFO): + with pytest.raises(RuntimeError): + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + per_phase = next(m for m in caplog.messages if "inserted per phase" in m) + assert "'annotation[healthy]': 1" in per_phase + assert "'annotation[broken]': 0" in per_phase + + @pytest.mark.asyncio + @settings( + max_examples=40, + deadline=None, + suppress_health_check=[HealthCheck.function_scoped_fixture], + ) + @given( + # One (row count, fails) pair per annotation phase, plus one for the + # experiment phase, so the fan-out and the failure pattern both vary. + phase_specs=st.lists( + st.tuples(st.integers(min_value=0, max_value=6), st.booleans()), + min_size=1, + max_size=4, + ), + batch_size=st.integers(min_value=1, max_value=4), + ) + async def test__sync_encode_should_report_exactly_what_it_loaded( + self, mocker, monkeypatch, phase_specs, batch_size + ): + """Test the reported total always equals the documents committed. + + The invariant the mid-stream accounting defect broke, over + arbitrary fan-out and failure patterns rather than the handful of + shapes the example tests fix. + + Given: + Any number of phases with any row counts, any subset of which + die after yielding all their rows, at any batch size. + When: + _sync_encode runs. + Then: + The count in the final progress should equal the ENCODE + documents in the collection. + """ + # Arrange + # A fresh database per generated example rather than the function + # scoped ``mock_db`` fixture, which Hypothesis does not reset between + # examples -- documents would accumulate across them. + db = FakeDB() + monkeypatch.setattr(api, "db", db) + monkeypatch.setattr(sync_module, "BATCH_SIZE", batch_size) + experiment_spec, *annotation_specs = phase_specs + monkeypatch.setenv( + "ENCODE_ANNOTATION_TYPES", + ",".join(f"type{i}" for i in range(len(annotation_specs))), + ) + + def _stream(prefix, spec): + count, fails = spec + rows = [ + _encode_metadata_row(f"ENCFF{prefix}{i:03d}", f"f{i}.bed.gz") + for i in range(count) + ] + if fails: + return _async_iter_then_raise(rows, RuntimeError("died")) + return _async_iter(rows) + + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _stream("E", experiment_spec), + ) + mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _stream( + annotation_type[-1], annotation_specs[int(annotation_type[-1])] + ), + ) + task = SyncTask(id="t1", dcc_names=["encode"]) + any_failure = any(fails for _, fails in phase_specs) + + # Act + if any_failure: + with pytest.raises(RuntimeError): + await _sync_encode(task) + else: + await _sync_encode(task) + + # Assert + committed = len([d for d in db.files.docs if d["submission"] == "encode"]) + # Anchored rather than a substring. The generated counts reach into + # the twenties, so "6 files" would be satisfied by a reported + # "16 files" -- and this assertion is the one standing guard over + # the reported half of the accounting invariant. + state = "incomplete" if any_failure else "complete" + assert task.progress.startswith(f"ENCODE sync {state}: {committed} files") + assert committed == sum(count for count, _ in phase_specs) + + @pytest.mark.asyncio + async def test__sync_encode_should_run_the_default_allowlist_when_unset( + self, mock_db, mocker, monkeypatch + ): + """Test the default allowlist reaches the sync, not just the parser. + + Every other test in this class pins the variable, so without this + one nothing joins ``annotation_types_from_env``'s default to the + phases actually run. + + Given: + No ENCODE_ANNOTATION_TYPES in the environment. + When: + _sync_encode runs. + Then: + It should fetch exactly the two default annotation types. + """ + # Arrange + monkeypatch.delenv("ENCODE_ANNOTATION_TYPES", raising=False) + mocker.patch.object( + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([]) + ) + fetch_annotations = mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter([]), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + assert [call.args[0] for call in fetch_annotations.call_args_list] == [ + "candidate Cis-Regulatory Elements", + "element gene regulatory interaction predictions", + ] + + @pytest.mark.asyncio + async def test__sync_encode_should_skip_annotations_when_the_allowlist_is_empty( + self, mock_db, mocker, monkeypatch + ): + """Test that an explicitly empty allowlist disables the fetch entirely. + + Given: + ENCODE_ANNOTATION_TYPES set to an empty value. + When: + _sync_encode runs. + Then: + No annotation request should be made, and the experiment ingest + should complete normally. + """ + # Arrange + monkeypatch.setenv("ENCODE_ANNOTATION_TYPES", "") + mocker.patch.object( + encode_module, + "fetch_encode_metadata", + lambda deadline=None: _async_iter([_encode_metadata_row("ENCFF001AAA", "x.bed.gz")]), + ) + fetch_annotations = mocker.patch.object( + encode_module, + "fetch_encode_annotation_metadata", + side_effect=lambda annotation_type, deadline=None: _async_iter([]), + ) + + # Act + await _sync_encode(SyncTask(id="t1", dcc_names=["encode"])) + + # Assert + fetch_annotations.assert_not_called() + assert [d["local_id"] for d in mock_db.files.docs] == ["ENCFF001AAA"] + + class TestStamp4dnFileAccessions: @pytest.mark.asyncio async def test__stamp_4dn_file_accessions_should_write_the_raw_file_collection( @@ -1049,7 +2446,7 @@ async def test__log_accession_coverage_should_warn_when_nothing_is_covered( # Assert assert "will return no matches" in caplog.text -class TestSyncEncodeIndexes: +class TestSyncEncodeIndexes(_EncodeSyncTestBase): @pytest.mark.asyncio async def test__sync_encode_should_ensure_the_accession_indexes( self, mocker, mock_db @@ -1072,7 +2469,7 @@ async def test__sync_encode_should_ensure_the_accession_indexes( # Arrange row = _encode_metadata_row("encff001aaa", "encff001aaa.bed.gz") mocker.patch.object( - encode_module, "fetch_encode_metadata", lambda: _async_iter([row]) + encode_module, "fetch_encode_metadata", lambda deadline=None: _async_iter([row]) ) # Act