diff --git a/src/ga4gh/vrs/models.py b/src/ga4gh/vrs/models.py index ad2978d5..495261b2 100644 --- a/src/ga4gh/vrs/models.py +++ b/src/ga4gh/vrs/models.py @@ -243,6 +243,28 @@ def _recurse_ga4gh_serialize(obj): return obj +def _recurse_ga4gh_serialize_no_store(obj, recompute: bool = False): + """Serialize without mutating any object. + + Mirrors `_recurse_ga4gh_serialize`, but digest computation never stores + digests: nested identifiable objects contribute their existing digest when + set (unless `recompute`), otherwise a digest computed with `store=False`. + """ + if isinstance(obj, Ga4ghIdentifiableObject): + if obj.digest is not None and not recompute: + return obj.digest + return obj._compute_digest_no_store(recompute) + if isinstance(obj, _ValueObject): + return obj._ga4gh_serialize_no_store(recompute) + if isinstance(obj, RootModel): + return _recurse_ga4gh_serialize_no_store(obj.model_dump(), recompute) + if isinstance(obj, str): + return obj + if isinstance(obj, list): + return [_recurse_ga4gh_serialize_no_store(x, recompute) for x in obj] + return obj + + class _ValueObject(Entity, ABC): """A contextual value whose equality is based on value, not identity. See https://en.wikipedia.org/wiki/Value_object for more on Value Objects. @@ -258,6 +280,15 @@ def ga4gh_serialize(self) -> dict: out[k] = _recurse_ga4gh_serialize(v) return out + def _ga4gh_serialize_no_store(self, recompute: bool = False) -> dict: + """Serialize like `ga4gh_serialize`, but digest computation for nested + identifiable objects never stores digests on those objects.""" + out = OrderedDict() + for k in self.ga4gh.inherent: + v = getattr(self, k) + out[k] = _recurse_ga4gh_serialize_no_store(v, recompute) + return out + class ga4gh: # noqa: N801 inherent: list[str] @@ -327,7 +358,10 @@ def get_or_create_ga4gh_identifier( - 'always': this will update the vro.id field any time the identifier is computed - 'never': the vro.id field will not be edited in-place, - even when empty + even when empty. The vro object is not mutated in any way: + digests are computed with ``store=False`` at every level of the + serialization, so neither the vro.digest field nor digest fields + on nested objects are set. Digests will be recalculated even if present if recompute is True. @@ -344,7 +378,11 @@ def get_or_create_ga4gh_identifier( elif in_place == "always": self.id = self.compute_ga4gh_identifier(recompute) elif in_place == "never": - return self.compute_ga4gh_identifier(recompute) + # Compute the identifier without mutating the object: digests are + # computed with store=False at every level, so the caller's + # object (including its digest fields) is left untouched. + digest = self._compute_digest_no_store(recompute) + return f"{CURIE_NAMESPACE}{CURIE_SEP}{self.ga4gh.prefix}{GA4GH_PREFIX_SEP}{digest}" else: msg = "Expected 'in_place' to be one of 'default', 'always', or 'never'" raise ValueError(msg) @@ -376,6 +414,16 @@ def get_or_create_digest(self, recompute: bool = False) -> str: return self.compute_digest() return self.digest + def _compute_digest_no_store(self, recompute: bool = False) -> str: + """Compute a sha512t24u digest without mutating any object. + + Like `compute_digest(store=False)`, but the serialization step also + avoids storing digests on nested identifiable objects. + """ + return sha512t24u( + encode_canonical_json(self._ga4gh_serialize_no_store(recompute)) + ) + class ga4gh(_ValueObject.ga4gh): # noqa: N801 prefix: str @@ -738,6 +786,11 @@ def ga4gh_serialize(self) -> dict: out["members"] = sorted(out["members"]) return out + def _ga4gh_serialize_no_store(self, recompute: bool = False) -> dict: + out = _ValueObject._ga4gh_serialize_no_store(self, recompute) + out["members"] = sorted(out["members"]) + return out + class ga4gh(Ga4ghIdentifiableObject.ga4gh): prefix = "CPB" inherent = ["members", "type"] diff --git a/tests/test_vrs.py b/tests/test_vrs.py index 975854ab..add14df4 100644 --- a/tests/test_vrs.py +++ b/tests/test_vrs.py @@ -194,6 +194,44 @@ def test_cpb(): assert ga4gh_identify(cpb_431012) == "ga4gh:CPB.x8GH5G73cPMs37jy1-9mJjWynu324rxI" +def test_identify_in_place_never_does_not_mutate(): + """ga4gh_identify(..., in_place="never") must not mutate the input object. + + Regression test for https://github.com/ga4gh/vrs-python/issues/440: + digest computation used to set `digest` fields on the object and its + nested identifiable objects even when in_place="never". + """ + allele = models.Allele(**allele_dict) + before = allele.model_dump_json(exclude_none=True) + assert allele.digest is None + assert allele.location.digest is None + + obj_id = ga4gh_identify(allele, in_place="never") + + assert obj_id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + assert allele.id is None + assert allele.digest is None + assert allele.location.digest is None + assert allele.model_dump_json(exclude_none=True) == before + + +def test_identify_in_place_modes_still_mutate(): + """Sanity check: in_place="default"/"always" keep their mutating behavior.""" + allele = models.Allele(**allele_dict) + assert ( + ga4gh_identify(allele, in_place="default") + == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + ) + assert allele.id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + + allele = models.Allele(**allele_dict) + assert ( + ga4gh_identify(allele, in_place="always") + == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + ) + assert allele.id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + + def test_ga4gh_iri(): iri = models.iriReference.model_construct( "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE"