diff --git a/src/ga4gh/vrs/dataproxy.py b/src/ga4gh/vrs/dataproxy.py index 32b8d561..bb23e039 100644 --- a/src/ga4gh/vrs/dataproxy.py +++ b/src/ga4gh/vrs/dataproxy.py @@ -16,6 +16,8 @@ import requests from bioutils.accessions import coerce_namespace +from ga4gh.vrs.models import Range + _logger = logging.getLogger(__name__) @@ -181,6 +183,56 @@ def validate_ref_seq( if require_validation: raise DataProxyValidationError(err_msg) + def validate_location_bounds( + self, + sequence_id: str, + start_pos: int | Range | None, + end_pos: int | Range | None, + ) -> None: + """Ensure that ``start_pos`` and ``end_pos`` are representable on ``sequence_id``. + + Each defined coordinate must be within ``[0, len(sequence)]`` (inter-residue). + Undefined (``None``) endpoints are skipped, and for a ``Range`` every defined + member is checked. ``start_pos`` and ``end_pos`` are checked independently, so + ``start_pos > end_pos`` (circular sequences) is permitted. + + Unlike ``validate_ref_seq``, there is no ``require_validation`` option: an + out-of-bounds location has no meaning, so the error is always raised. Sequence + backends may silently truncate out-of-range fetches, so this check must be made + before relying on fetched sequence. + + :param sequence_id: Sequence ID to use + :param start_pos: Start pos (inter-residue) on the sequence_id + :param end_pos: End pos (inter-residue) on the sequence_id + :raises DataProxyValidationError: If a defined coordinate is out of bounds + :raises KeyError: If ``sequence_id`` is not found + """ + # same cache key as derive_refget_accession + sequence_id = coerce_namespace(sequence_id) + md = self.get_metadata(sequence_id) + seq_len = md["length"] + bad = [] + for name, pos in (("start", start_pos), ("end", end_pos)): + values = pos.root if isinstance(pos, Range) else [pos] + if any(v is not None and not 0 <= v <= seq_len for v in values): + bad.append((name, pos)) + if bad: + # Range is shown as its list form, e.g. end=[4500, 4600] + detail = ", ".join( + f"{name}={pos.root if isinstance(pos, Range) else pos}" + for name, pos in bad + ) + # Name the sequence as given, plus the refget accession it resolved to + refget_ac = next((a for a in md["aliases"] if a.startswith("ga4gh:")), None) + seq_name = sequence_id + if refget_ac and refget_ac != sequence_id: + seq_name += f" ({refget_ac})" + err_msg = ( + f"Location out of bounds on {seq_name}: {detail} " + f"not within [0, {seq_len}]" + ) + raise DataProxyValidationError(err_msg) + class _SeqRepoDataProxyBase(_DataProxy): # wraps seqreqpo classes in order to provide translation to/from diff --git a/src/ga4gh/vrs/extras/translator.py b/src/ga4gh/vrs/extras/translator.py index 91c45532..120eaee4 100644 --- a/src/ga4gh/vrs/extras/translator.py +++ b/src/ga4gh/vrs/extras/translator.py @@ -203,6 +203,8 @@ def _create_allele(self, values: dict, **kwargs) -> models.Allele: Args: values (dict): The values to use for creating the allele object. + 'sequence_id' (str): The sequence identifier from the input + expression, used to validate `start` and `end`. 'refget_accession' (str): The accession ID of the reference genome. 'start' (int): The start position of the allele. 'end' (int): The end position of the allele. @@ -213,6 +215,9 @@ def _create_allele(self, values: dict, **kwargs) -> models.Allele: models.Allele: The created allele object. """ + self.data_proxy.validate_location_bounds( + values["sequence_id"], values["start"], values["end"] + ) seq_ref = models.SequenceReference(refgetAccession=values["refget_accession"]) location = models.SequenceLocation( sequenceReference=seq_ref, start=values["start"], end=values["end"] @@ -276,6 +281,7 @@ def _from_beacon(self, beacon_expr: str, **kwargs) -> models.Allele | None: ins_seq = alt values = { + "sequence_id": sequence, "refget_accession": refget_accession, "start": start, "end": end, @@ -340,6 +346,9 @@ def _from_gnomad(self, gnomad_expr: str, **kwargs) -> models.Allele | None: ins_seq = alt # validation checks + # Bounds must be checked before the ref check: an out-of-bounds fetch may be + # silently truncated, which would be misreported as a reference mismatch + self.data_proxy.validate_location_bounds(sequence, start, end) self.data_proxy.validate_ref_seq( sequence, start, @@ -349,6 +358,7 @@ def _from_gnomad(self, gnomad_expr: str, **kwargs) -> models.Allele | None: ) values = { + "sequence_id": sequence, "refget_accession": refget_accession, "start": start, "end": end, @@ -414,6 +424,7 @@ def _from_spdi(self, spdi_expr: str, **kwargs) -> models.Allele | None: ins_seq = g["ins_seq"] values = { + "sequence_id": g["ac"], "refget_accession": refget_accession, "start": start, "end": end, @@ -570,12 +581,16 @@ def _from_hgvs( if not refget_accession: return None + start = sv.posedit.pos.start.base - 1 + end = sv.posedit.pos.end.base + self.data_proxy.validate_location_bounds(sv.ac, start, end) + location = models.SequenceLocation( sequenceReference=models.SequenceReference( refgetAccession=refget_accession ), - start=sv.posedit.pos.start.base - 1, - end=sv.posedit.pos.end.base, + start=start, + end=end, ) copies = kwargs.get("copies") diff --git a/src/ga4gh/vrs/normalize.py b/src/ga4gh/vrs/normalize.py index fad73d87..9747febd 100644 --- a/src/ga4gh/vrs/normalize.py +++ b/src/ga4gh/vrs/normalize.py @@ -111,6 +111,8 @@ def _normalize_allele( of the `sequence`. To exclude `sequence` from the response, set to 0. For no limit, set to `None`. + :raises DataProxyValidationError: If the allele location is out of bounds on its + sequence """ # Algorithm applies to LiteralSequenceExpression alleles only; other states are returned unchanged if not isinstance(input_allele.state, models.LiteralSequenceExpression): @@ -130,6 +132,14 @@ def _normalize_allele( # 0: Get reference sequence and interval ref_seq = SequenceProxy(data_proxy, alias) + + # Reject locations that do not exist on the sequence before anything is fetched, + # since out-of-range fetches may be silently truncated by the sequence backend. + # Done before the early returns below, which skip definite ranges. + data_proxy.validate_location_bounds( + alias, input_allele.location.start, input_allele.location.end + ) + start = _get_allele_location_pos(input_allele, use_start=True) if start is None: return input_allele diff --git a/src/ga4gh/vrs/utils/hgvs_tools.py b/src/ga4gh/vrs/utils/hgvs_tools.py index 503d4022..5ff3f38f 100644 --- a/src/ga4gh/vrs/utils/hgvs_tools.py +++ b/src/ga4gh/vrs/utils/hgvs_tools.py @@ -182,6 +182,7 @@ def extract_allele_values(self, hgvs_expr: str) -> dict | None: (start, end, state) = self.get_position_and_state(sv) return { + "sequence_id": sv.ac, "refget_accession": refget_accession, "start": start, "end": end, diff --git a/tests/extras/cassettes/test_in_bounds[spdi-insertion-at-end].yaml b/tests/extras/cassettes/test_in_bounds[spdi-insertion-at-end].yaml new file mode 100644 index 00000000..b88723ee --- /dev/null +++ b/tests/extras/cassettes/test_in_bounds[spdi-insertion-at-end].yaml @@ -0,0 +1,58 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/refseq:NM_000551.3 + response: + body: + string: "{\n \"added\": \"2016-08-24T05:03:11Z\",\n \"aliases\": [\n \"MD5:215137b1973c1a5afcf86be7d999574a\",\n + \ \"NCBI:NM_000551.3\",\n \"refseq:NM_000551.3\",\n \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n + \ \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n + \ \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n + \ ],\n \"alphabet\": \"ACGT\",\n \"length\": 4560\n}\n" + headers: {} + status: + code: 200 + message: OK +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_ + response: + body: + string: "{\n \"added\": \"2016-08-24T05:03:11Z\",\n \"aliases\": [\n \"MD5:215137b1973c1a5afcf86be7d999574a\",\n + \ \"NCBI:NM_000551.3\",\n \"refseq:NM_000551.3\",\n \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n + \ \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n + \ \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n + \ ],\n \"alphabet\": \"ACGT\",\n \"length\": 4560\n}\n" + headers: {} + status: + code: 200 + message: OK +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/sequence/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_?start=4560&end=4560 + response: + body: + string: '' + headers: {} + status: + code: 200 + message: OK +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/sequence/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_?start=4559&end=4560 + response: + body: + string: G + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[beacon-past-end].yaml b/tests/extras/cassettes/test_out_of_bounds[beacon-past-end].yaml new file mode 100644 index 00000000..2c11ee79 --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[beacon-past-end].yaml @@ -0,0 +1,25 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1 + response: + body: + string: "{\n \"added\": \"2016-08-27T21:17:00Z\",\n \"aliases\": [\n \"GRCh38:1\",\n + \ \"GRCh38:chr1\",\n \"GRCh38.p1:1\",\n \"GRCh38.p1:chr1\",\n \"GRCh38.p10:1\",\n + \ \"GRCh38.p10:chr1\",\n \"GRCh38.p11:1\",\n \"GRCh38.p11:chr1\",\n + \ \"GRCh38.p12:1\",\n \"GRCh38.p12:chr1\",\n \"GRCh38.p2:1\",\n \"GRCh38.p2:chr1\",\n + \ \"GRCh38.p3:1\",\n \"GRCh38.p3:chr1\",\n \"GRCh38.p4:1\",\n \"GRCh38.p4:chr1\",\n + \ \"GRCh38.p5:1\",\n \"GRCh38.p5:chr1\",\n \"GRCh38.p6:1\",\n \"GRCh38.p6:chr1\",\n + \ \"GRCh38.p7:1\",\n \"GRCh38.p7:chr1\",\n \"GRCh38.p8:1\",\n \"GRCh38.p8:chr1\",\n + \ \"GRCh38.p9:1\",\n \"GRCh38.p9:chr1\",\n \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n + \ \"NCBI:NC_000001.11\",\n \"refseq:NC_000001.11\",\n \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n + \ \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n + \ \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n + \ ],\n \"alphabet\": \"ACGMNRT\",\n \"length\": 248956422\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[cnv-hgvs-copy-number-change-past-end].yaml b/tests/extras/cassettes/test_out_of_bounds[cnv-hgvs-copy-number-change-past-end].yaml new file mode 100644 index 00000000..1e5e9d6b --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[cnv-hgvs-copy-number-change-past-end].yaml @@ -0,0 +1,25 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/refseq:NC_000007.14 + response: + body: + string: "{\n \"added\": \"2016-08-27T21:23:35Z\",\n \"aliases\": [\n \"GRCh38:7\",\n + \ \"GRCh38:chr7\",\n \"GRCh38.p1:7\",\n \"GRCh38.p1:chr7\",\n \"GRCh38.p10:7\",\n + \ \"GRCh38.p10:chr7\",\n \"GRCh38.p11:7\",\n \"GRCh38.p11:chr7\",\n + \ \"GRCh38.p12:7\",\n \"GRCh38.p12:chr7\",\n \"GRCh38.p2:7\",\n \"GRCh38.p2:chr7\",\n + \ \"GRCh38.p3:7\",\n \"GRCh38.p3:chr7\",\n \"GRCh38.p4:7\",\n \"GRCh38.p4:chr7\",\n + \ \"GRCh38.p5:7\",\n \"GRCh38.p5:chr7\",\n \"GRCh38.p6:7\",\n \"GRCh38.p6:chr7\",\n + \ \"GRCh38.p7:7\",\n \"GRCh38.p7:chr7\",\n \"GRCh38.p8:7\",\n \"GRCh38.p8:chr7\",\n + \ \"GRCh38.p9:7\",\n \"GRCh38.p9:chr7\",\n \"MD5:cc044cc2256a1141212660fb07b6171e\",\n + \ \"NCBI:NC_000007.14\",\n \"refseq:NC_000007.14\",\n \"SEGUID:4+JjCcBVhPCr8vdIhUKFycPv8bY\",\n + \ \"SHA1:e3e26309c05584f0abf2f748854285c9c3eff1b6\",\n \"VMC:GS_F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\",\n + \ \"sha512t24u:F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\",\n \"ga4gh:SQ.F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\"\n + \ ],\n \"alphabet\": \"ACGNRSTY\",\n \"length\": 159345973\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[gnomad-negative-start].yaml b/tests/extras/cassettes/test_out_of_bounds[gnomad-negative-start].yaml new file mode 100644 index 00000000..2c11ee79 --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[gnomad-negative-start].yaml @@ -0,0 +1,25 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1 + response: + body: + string: "{\n \"added\": \"2016-08-27T21:17:00Z\",\n \"aliases\": [\n \"GRCh38:1\",\n + \ \"GRCh38:chr1\",\n \"GRCh38.p1:1\",\n \"GRCh38.p1:chr1\",\n \"GRCh38.p10:1\",\n + \ \"GRCh38.p10:chr1\",\n \"GRCh38.p11:1\",\n \"GRCh38.p11:chr1\",\n + \ \"GRCh38.p12:1\",\n \"GRCh38.p12:chr1\",\n \"GRCh38.p2:1\",\n \"GRCh38.p2:chr1\",\n + \ \"GRCh38.p3:1\",\n \"GRCh38.p3:chr1\",\n \"GRCh38.p4:1\",\n \"GRCh38.p4:chr1\",\n + \ \"GRCh38.p5:1\",\n \"GRCh38.p5:chr1\",\n \"GRCh38.p6:1\",\n \"GRCh38.p6:chr1\",\n + \ \"GRCh38.p7:1\",\n \"GRCh38.p7:chr1\",\n \"GRCh38.p8:1\",\n \"GRCh38.p8:chr1\",\n + \ \"GRCh38.p9:1\",\n \"GRCh38.p9:chr1\",\n \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n + \ \"NCBI:NC_000001.11\",\n \"refseq:NC_000001.11\",\n \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n + \ \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n + \ \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n + \ ],\n \"alphabet\": \"ACGMNRT\",\n \"length\": 248956422\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end-no-require-validation].yaml b/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end-no-require-validation].yaml new file mode 100644 index 00000000..2c11ee79 --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end-no-require-validation].yaml @@ -0,0 +1,25 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1 + response: + body: + string: "{\n \"added\": \"2016-08-27T21:17:00Z\",\n \"aliases\": [\n \"GRCh38:1\",\n + \ \"GRCh38:chr1\",\n \"GRCh38.p1:1\",\n \"GRCh38.p1:chr1\",\n \"GRCh38.p10:1\",\n + \ \"GRCh38.p10:chr1\",\n \"GRCh38.p11:1\",\n \"GRCh38.p11:chr1\",\n + \ \"GRCh38.p12:1\",\n \"GRCh38.p12:chr1\",\n \"GRCh38.p2:1\",\n \"GRCh38.p2:chr1\",\n + \ \"GRCh38.p3:1\",\n \"GRCh38.p3:chr1\",\n \"GRCh38.p4:1\",\n \"GRCh38.p4:chr1\",\n + \ \"GRCh38.p5:1\",\n \"GRCh38.p5:chr1\",\n \"GRCh38.p6:1\",\n \"GRCh38.p6:chr1\",\n + \ \"GRCh38.p7:1\",\n \"GRCh38.p7:chr1\",\n \"GRCh38.p8:1\",\n \"GRCh38.p8:chr1\",\n + \ \"GRCh38.p9:1\",\n \"GRCh38.p9:chr1\",\n \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n + \ \"NCBI:NC_000001.11\",\n \"refseq:NC_000001.11\",\n \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n + \ \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n + \ \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n + \ ],\n \"alphabet\": \"ACGMNRT\",\n \"length\": 248956422\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end].yaml b/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end].yaml new file mode 100644 index 00000000..2c11ee79 --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[gnomad-past-end].yaml @@ -0,0 +1,25 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1 + response: + body: + string: "{\n \"added\": \"2016-08-27T21:17:00Z\",\n \"aliases\": [\n \"GRCh38:1\",\n + \ \"GRCh38:chr1\",\n \"GRCh38.p1:1\",\n \"GRCh38.p1:chr1\",\n \"GRCh38.p10:1\",\n + \ \"GRCh38.p10:chr1\",\n \"GRCh38.p11:1\",\n \"GRCh38.p11:chr1\",\n + \ \"GRCh38.p12:1\",\n \"GRCh38.p12:chr1\",\n \"GRCh38.p2:1\",\n \"GRCh38.p2:chr1\",\n + \ \"GRCh38.p3:1\",\n \"GRCh38.p3:chr1\",\n \"GRCh38.p4:1\",\n \"GRCh38.p4:chr1\",\n + \ \"GRCh38.p5:1\",\n \"GRCh38.p5:chr1\",\n \"GRCh38.p6:1\",\n \"GRCh38.p6:chr1\",\n + \ \"GRCh38.p7:1\",\n \"GRCh38.p7:chr1\",\n \"GRCh38.p8:1\",\n \"GRCh38.p8:chr1\",\n + \ \"GRCh38.p9:1\",\n \"GRCh38.p9:chr1\",\n \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n + \ \"NCBI:NC_000001.11\",\n \"refseq:NC_000001.11\",\n \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n + \ \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n + \ \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n + \ ],\n \"alphabet\": \"ACGMNRT\",\n \"length\": 248956422\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[hgvs-n-insertion-past-end].yaml b/tests/extras/cassettes/test_out_of_bounds[hgvs-n-insertion-past-end].yaml new file mode 100644 index 00000000..ab28a89d --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[hgvs-n-insertion-past-end].yaml @@ -0,0 +1,18 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/refseq:NM_000551.3 + response: + body: + string: "{\n \"added\": \"2016-08-24T05:03:11Z\",\n \"aliases\": [\n \"MD5:215137b1973c1a5afcf86be7d999574a\",\n + \ \"NCBI:NM_000551.3\",\n \"refseq:NM_000551.3\",\n \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n + \ \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n + \ \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n + \ ],\n \"alphabet\": \"ACGT\",\n \"length\": 4560\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[hgvs-p-ter-at-length-plus-one].yaml b/tests/extras/cassettes/test_out_of_bounds[hgvs-p-ter-at-length-plus-one].yaml new file mode 100644 index 00000000..dba45e9a --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[hgvs-p-ter-at-length-plus-one].yaml @@ -0,0 +1,26 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/refseq:NP_001346993.1 + response: + body: + string: "{\n \"added\": \"2016-08-24T04:59:23Z\",\n \"aliases\": [\n \"Ensembl:ENSP00000480268.1\",\n + \ \"ensembl:ENSP00000480268.1\",\n \"Ensembl:ENSP00000491180.1\",\n \"ensembl:ENSP00000491180.1\",\n + \ \"Ensembl:ENSP00000491338.1\",\n \"ensembl:ENSP00000491338.1\",\n \"Ensembl:ENSP00000491353.1\",\n + \ \"ensembl:ENSP00000491353.1\",\n \"Ensembl:ENSP00000492701.1\",\n \"ensembl:ENSP00000492701.1\",\n + \ \"Ensembl:ENSP00000498790.1\",\n \"ensembl:ENSP00000498790.1\",\n \"MD5:fecf2eee2cdc50588a641e472e062be1\",\n + \ \"NCBI:NP_001346993.1\",\n \"refseq:NP_001346993.1\",\n \"NCBI:NP_001347000.1\",\n + \ \"refseq:NP_001347000.1\",\n \"NCBI:NP_001355060.1\",\n \"refseq:NP_001355060.1\",\n + \ \"NCBI:XP_011534418.1\",\n \"refseq:XP_011534418.1\",\n \"NCBI:XP_024302319.1\",\n + \ \"refseq:XP_024302319.1\",\n \"NCBI:XP_054212402.1\",\n \"refseq:XP_054212402.1\",\n + \ \"NCBI:XP_054212403.1\",\n \"refseq:XP_054212403.1\",\n \"SEGUID:8Lnknw+hAAOAjiLGHus4bfyDy0k\",\n + \ \"SHA1:f0b9e49f0fa10003808e22c61eeb386dfc83cb49\",\n \"VMC:GS_IPAWzkahAXVA3fBdoFluaU4NA3xTYUer\",\n + \ \"sha512t24u:IPAWzkahAXVA3fBdoFluaU4NA3xTYUer\",\n \"ga4gh:SQ.IPAWzkahAXVA3fBdoFluaU4NA3xTYUer\"\n + \ ],\n \"alphabet\": \"ACDEFGHIKLMNPQRSTVWY\",\n \"length\": 193\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/cassettes/test_out_of_bounds[spdi-insertion-past-end].yaml b/tests/extras/cassettes/test_out_of_bounds[spdi-insertion-past-end].yaml new file mode 100644 index 00000000..ab28a89d --- /dev/null +++ b/tests/extras/cassettes/test_out_of_bounds[spdi-insertion-past-end].yaml @@ -0,0 +1,18 @@ +interactions: +- request: + body: null + headers: {} + method: GET + uri: http://localhost:5000/seqrepo/1/metadata/refseq:NM_000551.3 + response: + body: + string: "{\n \"added\": \"2016-08-24T05:03:11Z\",\n \"aliases\": [\n \"MD5:215137b1973c1a5afcf86be7d999574a\",\n + \ \"NCBI:NM_000551.3\",\n \"refseq:NM_000551.3\",\n \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n + \ \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n + \ \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n + \ ],\n \"alphabet\": \"ACGT\",\n \"length\": 4560\n}\n" + headers: {} + status: + code: 200 + message: OK +version: 1 diff --git a/tests/extras/test_location_bounds.py b/tests/extras/test_location_bounds.py new file mode 100644 index 00000000..d3e88e56 --- /dev/null +++ b/tests/extras/test_location_bounds.py @@ -0,0 +1,147 @@ +"""Out-of-bounds SequenceLocations are rejected on every translator input path +except ``vrs``, whose input is trusted as-is + +Sequence lengths used below: + NM_000551.3 4560 + NP_001346993.1 193 + NC_000007.14 159345973 + GRCh38:1 248956422 +""" + +import os +import re + +import pytest + +from ga4gh.vrs.dataproxy import DataProxyValidationError, SeqRepoRESTDataProxy +from ga4gh.vrs.extras.translator import AlleleTranslator, CnvTranslator + +# Refget accession each input sequence resolves to, named alongside it in errors +REFGET_ACCESSIONS = { + "GRCh38:1": "ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO", + "refseq:NC_000007.14": "ga4gh:SQ.F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul", + "refseq:NM_000551.3": "ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_", + "refseq:NP_001346993.1": "ga4gh:SQ.IPAWzkahAXVA3fBdoFluaU4NA3xTYUer", +} + + +@pytest.fixture +def data_proxy() -> SeqRepoRESTDataProxy: + # Metadata lookups are lru_cached per dataproxy instance, so a fresh instance per + # test keeps each cassette self-contained regardless of test order + return SeqRepoRESTDataProxy( + base_url=os.environ.get("SEQREPO_REST_URL", "http://localhost:5000/seqrepo"), + disable_healthcheck=True, + ) + + +@pytest.fixture +def allele_tlr(data_proxy: SeqRepoRESTDataProxy) -> AlleleTranslator: + return AlleleTranslator(data_proxy=data_proxy) + + +@pytest.fixture +def cnv_tlr(data_proxy: SeqRepoRESTDataProxy) -> CnvTranslator: + return CnvTranslator(data_proxy=data_proxy) + + +def _bounds_msg(sequence_id: str, detail: str, seq_len: int) -> str: + return ( + f"Location out of bounds on {sequence_id} ({REFGET_ACCESSIONS[sequence_id]}): " + f"{detail} not within [0, {seq_len}]" + ) + + +out_of_bounds_cases = [ + { + "id": "hgvs-n-insertion-past-end", + "tlr_fixture": "allele_tlr", + "fmt": "hgvs", + "var": "NM_000551.3:n.4561_4562insA", + "msg": _bounds_msg("refseq:NM_000551.3", "start=4561, end=4561", 4560), + }, + # ClinVar references the stop codon, which is not part of the protein sequence + { + "id": "hgvs-p-ter-at-length-plus-one", + "tlr_fixture": "allele_tlr", + "fmt": "hgvs", + "var": "NP_001346993.1:p.Ter194del", + "msg": _bounds_msg("refseq:NP_001346993.1", "end=194", 193), + }, + # Zero-width: an out-of-range fetch returns "" and would compare equal to the + # empty reference, so only a coordinate check can catch this + { + "id": "spdi-insertion-past-end", + "tlr_fixture": "allele_tlr", + "fmt": "spdi", + "var": "NM_000551.3:5000:0:AAA", + "msg": _bounds_msg("refseq:NM_000551.3", "start=5000, end=5000", 4560), + }, + # Must report the bounds error, not "Reference mismatch ... correct ref is ''" + { + "id": "gnomad-past-end", + "tlr_fixture": "allele_tlr", + "fmt": "gnomad", + "var": "1-248956423-A-T", + "msg": _bounds_msg("GRCh38:1", "end=248956423", 248956422), + }, + { + "id": "gnomad-past-end-no-require-validation", + "tlr_fixture": "allele_tlr", + "fmt": "gnomad", + "var": "1-248956423-A-T", + "kwargs": {"require_validation": False}, + "msg": _bounds_msg("GRCh38:1", "end=248956423", 248956422), + }, + { + "id": "gnomad-negative-start", + "tlr_fixture": "allele_tlr", + "fmt": "gnomad", + "var": "1-0-A-T", + "msg": _bounds_msg("GRCh38:1", "start=-1", 248956422), + }, + { + "id": "beacon-past-end", + "tlr_fixture": "allele_tlr", + "fmt": "beacon", + "var": "1 : 248956423 A > T", + "msg": _bounds_msg("GRCh38:1", "end=248956423", 248956422), + }, + { + "id": "cnv-hgvs-copy-number-change-past-end", + "tlr_fixture": "cnv_tlr", + "fmt": "hgvs", + "var": "NC_000007.14:g.159400000_159400100del", + "msg": _bounds_msg( + "refseq:NC_000007.14", "start=159399999, end=159400100", 159345973 + ), + }, +] + + +in_bounds_cases = [ + { + "id": "spdi-insertion-at-end", + "tlr_fixture": "allele_tlr", + "fmt": "spdi", + "var": "NM_000551.3:4560:0:AAA", + "expected_location": {"start": 4560, "end": 4560}, + }, +] + + +@pytest.mark.parametrize("case", out_of_bounds_cases, ids=lambda c: c["id"]) +@pytest.mark.vcr +def test_out_of_bounds(request: pytest.FixtureRequest, case: dict) -> None: + tlr = request.getfixturevalue(case["tlr_fixture"]) + with pytest.raises(DataProxyValidationError, match=f"^{re.escape(case['msg'])}$"): + tlr.translate_from(case["var"], fmt=case["fmt"], **case.get("kwargs", {})) + + +@pytest.mark.parametrize("case", in_bounds_cases, ids=lambda c: c["id"]) +@pytest.mark.vcr +def test_in_bounds(request: pytest.FixtureRequest, case: dict) -> None: + tlr = request.getfixturevalue(case["tlr_fixture"]) + location = tlr.translate_from(case["var"], fmt=case["fmt"]).location.model_dump() + expected_location = case["expected_location"] + assert {k: location[k] for k in expected_location} == expected_location diff --git a/tests/test_dataproxy.py b/tests/test_dataproxy.py index f306c27a..4752424a 100644 --- a/tests/test_dataproxy.py +++ b/tests/test_dataproxy.py @@ -3,7 +3,12 @@ import pytest -from ga4gh.vrs.dataproxy import create_dataproxy +from ga4gh.vrs import models +from ga4gh.vrs.dataproxy import ( + DataProxyValidationError, + _DataProxy, + create_dataproxy, +) @pytest.mark.parametrize("dp", ["rest_dataproxy", "dataproxy"]) @@ -65,3 +70,154 @@ def test_data_proxy_configs(): ), ): create_dataproxy("file:///path/to/seqrepo/root") + + +class _StubDataProxy(_DataProxy): + """Dataproxy serving only metadata, keyed by identifier""" + + def __init__(self, metadata: dict[str, dict]) -> None: + self.metadata = metadata + + def get_sequence( + self, identifier: str, start: int | None = None, end: int | None = None + ) -> str: + raise NotImplementedError + + def get_metadata(self, identifier: str) -> dict: + return self.metadata[identifier] + + +BOUNDS_SEQ_ID = "refseq:NM_000551.3" +BOUNDS_REFGET_ID = "ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_" +BOUNDS_SEQ_LEN = 4560 + + +def _bounds_dp(aliases: list[str]) -> _StubDataProxy: + """Stub serving one sequence's metadata under each of its aliases""" + md = {"length": BOUNDS_SEQ_LEN, "aliases": aliases} + return _StubDataProxy(dict.fromkeys(aliases, md)) + + +# Locations representable on a sequence of length BOUNDS_SEQ_LEN +location_bounds_valid_cases = [ + {"id": "interior", "start": 10, "end": 20}, + {"id": "zero-width-at-start", "start": 0, "end": 0}, + {"id": "terminal-residue", "start": 4559, "end": 4560}, + {"id": "insertion-at-end", "start": 4560, "end": 4560}, + {"id": "circular-start-gt-end", "start": 4000, "end": 5}, + { + "id": "indefinite-end-open-upper", + "start": 4400, + "end": models.Range([4500, None]), + }, + { + "id": "indefinite-start-open-lower", + "start": models.Range([None, 10]), + "end": 20, + }, + { + "id": "definite-ranges", + "start": models.Range([0, 10]), + "end": models.Range([4500, 4560]), + }, + {"id": "start-undefined", "start": None, "end": 10}, + {"id": "end-undefined", "start": 10, "end": None}, + {"id": "both-undefined", "start": None, "end": None}, +] + +# Locations not representable on a sequence of length BOUNDS_SEQ_LEN, with the +# offending coordinates as reported in the error message +location_bounds_invalid_cases = [ + {"id": "one-past-end", "start": 4559, "end": 4561, "detail": "end=4561"}, + { + "id": "zero-width-past-end", + "start": 5000, + "end": 5000, + "detail": "start=5000, end=5000", + }, + { + "id": "start-past-end-with-start-gt-end", + "start": 99999999, + "end": 5, + "detail": "start=99999999", + }, + {"id": "negative-start", "start": -1, "end": 1, "detail": "start=-1"}, + {"id": "negative-end", "start": 0, "end": -1, "detail": "end=-1"}, + { + "id": "definite-end-past-end", + "start": 4400, + "end": models.Range([4500, 4600]), + "detail": "end=[4500, 4600]", + }, + { + "id": "indefinite-end-lower-bound-past-end", + "start": 4400, + "end": models.Range([4561, None]), + "detail": "end=[4561, None]", + }, + { + "id": "range-with-negative-member", + "start": models.Range([-5, 10]), + "end": 20, + "detail": "start=[-5, 10]", + }, +] + + +@pytest.mark.parametrize("case", location_bounds_valid_cases, ids=lambda c: c["id"]) +def test_validate_location_bounds_valid(case: dict) -> None: + dp = _bounds_dp([BOUNDS_SEQ_ID, BOUNDS_REFGET_ID]) + dp.validate_location_bounds(BOUNDS_SEQ_ID, case["start"], case["end"]) + + +@pytest.mark.parametrize("case", location_bounds_invalid_cases, ids=lambda c: c["id"]) +def test_validate_location_bounds_invalid(case: dict) -> None: + dp = _bounds_dp([BOUNDS_SEQ_ID, BOUNDS_REFGET_ID]) + expected_msg = ( + f"Location out of bounds on {BOUNDS_SEQ_ID} ({BOUNDS_REFGET_ID}): " + f"{case['detail']} not within [0, {BOUNDS_SEQ_LEN}]" + ) + with pytest.raises(DataProxyValidationError, match=f"^{re.escape(expected_msg)}$"): + dp.validate_location_bounds(BOUNDS_SEQ_ID, case["start"], case["end"]) + + +# Name used for the sequence in the error message, given the sequence_id passed in +# and the aliases of the sequence +location_bounds_sequence_name_cases = [ + { + "id": "input-and-refget", + "sequence_id": BOUNDS_SEQ_ID, + "aliases": [BOUNDS_SEQ_ID, BOUNDS_REFGET_ID], + "seq_name": f"{BOUNDS_SEQ_ID} ({BOUNDS_REFGET_ID})", + }, + { + "id": "bare-accession-coerced", + "sequence_id": "NM_000551.3", + "aliases": [BOUNDS_SEQ_ID, BOUNDS_REFGET_ID], + "seq_name": f"{BOUNDS_SEQ_ID} ({BOUNDS_REFGET_ID})", + }, + { + "id": "refget-input-named-once", + "sequence_id": BOUNDS_REFGET_ID, + "aliases": [BOUNDS_SEQ_ID, BOUNDS_REFGET_ID], + "seq_name": BOUNDS_REFGET_ID, + }, + { + "id": "no-refget-alias", + "sequence_id": BOUNDS_SEQ_ID, + "aliases": [BOUNDS_SEQ_ID], + "seq_name": BOUNDS_SEQ_ID, + }, +] + + +@pytest.mark.parametrize( + "case", location_bounds_sequence_name_cases, ids=lambda c: c["id"] +) +def test_validate_location_bounds_sequence_name(case: dict) -> None: + dp = _bounds_dp(case["aliases"]) + expected_prefix = f"Location out of bounds on {case['seq_name']}: " + with pytest.raises( + DataProxyValidationError, match=f"^{re.escape(expected_prefix)}" + ): + dp.validate_location_bounds(case["sequence_id"], 0, BOUNDS_SEQ_LEN + 1) diff --git a/tests/test_vrs_normalize.py b/tests/test_vrs_normalize.py index 83c27cdd..d6f62545 100644 --- a/tests/test_vrs_normalize.py +++ b/tests/test_vrs_normalize.py @@ -1,6 +1,9 @@ +import re + import pytest from ga4gh.vrs import models, normalize +from ga4gh.vrs.dataproxy import DataProxyValidationError, SeqRepoDataProxy # Single nucleotide same-as-reference allele. allele_dict1 = { @@ -929,3 +932,76 @@ def test_normalize_partial_rle_del_ins(rest_dataproxy): tail_del_4 = models.Allele(**tail_del_4bp) tail_del_4_norm = normalize(tail_del_4, rest_dataproxy, rle_seq_limit=0) assert tail_del_4_norm == models.Allele(**tail_del_4bp_normalized) + + +# NM_000551.3, length 4560. Present in the test SeqRepo, so these use the local +# dataproxy fixture and need no cassettes. +BOUNDS_REFGET_AC = "SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_" +BOUNDS_SEQ_LEN = 4560 + + +def _bounds_allele( + start: int | list[int | None], end: int | list[int | None], sequence: str +) -> models.Allele: + return models.Allele( + location=models.SequenceLocation( + sequenceReference=models.SequenceReference( + refgetAccession=BOUNDS_REFGET_AC + ), + start=start, + end=end, + ), + state=models.LiteralSequenceExpression(sequence=sequence), + ) + + +normalize_location_in_bounds_cases = [ + # an undefined outer endpoint is representable and must not be rejected + # (the deletion is also rolled right by one base by normalization) + { + "id": "indefinite-ranges-open-outward", + "start": [None, 4400], + "end": [4500, None], + "sequence": "", + "expected_start": [None, 4400], + "expected_end": [4501, None], + }, +] + + +@pytest.mark.parametrize( + "case", normalize_location_in_bounds_cases, ids=lambda c: c["id"] +) +def test_normalize_location_in_bounds(dataproxy: SeqRepoDataProxy, case: dict) -> None: + allele = _bounds_allele(case["start"], case["end"], case["sequence"]) + location = normalize(allele, dataproxy).location.model_dump() + assert (location["start"], location["end"]) == ( + case["expected_start"], + case["expected_end"], + ) + + +normalize_location_out_of_bounds_cases = [ + # Definite ranges are otherwise returned without normalization, so the + # bounds check must run before that early return + { + "id": "definite-range-end-past-end", + "start": 4400, + "end": [4500, 4600], + "detail": "end=[4500, 4600]", + }, +] + + +@pytest.mark.parametrize( + "case", normalize_location_out_of_bounds_cases, ids=lambda c: c["id"] +) +def test_normalize_location_out_of_bounds( + dataproxy: SeqRepoDataProxy, case: dict +) -> None: + expected_msg = ( + f"Location out of bounds on ga4gh:{BOUNDS_REFGET_AC}: {case['detail']} " + f"not within [0, {BOUNDS_SEQ_LEN}]" + ) + with pytest.raises(DataProxyValidationError, match=f"^{re.escape(expected_msg)}$"): + normalize(_bounds_allele(case["start"], case["end"], "A"), dataproxy)