From 8b59f61b9a10a0d8568ecc69a375d217f487993f Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 16:15:36 -0400 Subject: [PATCH 1/3] deepcopy annotations in SubmissionStatus._set_last_persistent_instance --- synapseclient/models/submission_status.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapseclient/models/submission_status.py b/synapseclient/models/submission_status.py index 9db745552..280609e3c 100644 --- a/synapseclient/models/submission_status.py +++ b/synapseclient/models/submission_status.py @@ -1,3 +1,4 @@ +from copy import deepcopy from dataclasses import dataclass, field, replace from datetime import date, datetime from typing import Optional, Protocol, Union @@ -435,7 +436,14 @@ def has_changed(self) -> bool: def _set_last_persistent_instance(self) -> None: """Stash the last time this object interacted with Synapse. This is used to determine if the object has been changed and needs to be updated in Synapse.""" + del self._last_persistent_instance self._last_persistent_instance = replace(self) + self._last_persistent_instance.annotations = ( + deepcopy(self.annotations) if self.annotations else {} + ) + self._last_persistent_instance.submission_annotations = ( + deepcopy(self.submission_annotations) if self.submission_annotations else {} + ) def fill_from_dict( self, From e492ac21d3df59b6f44d8a620bdf8a72e2dd3d23 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Fri, 26 Jun 2026 14:16:05 -0400 Subject: [PATCH 2/3] test: add cases for in-place mutations of annotations & submission_annotations --- .../async/test_submission_status_async.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/integration/synapseclient/models/async/test_submission_status_async.py b/tests/integration/synapseclient/models/async/test_submission_status_async.py index f345fa2e8..641b039e5 100644 --- a/tests/integration/synapseclient/models/async/test_submission_status_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_status_async.py @@ -390,6 +390,58 @@ async def test_store_submission_status_without_id(self): ): await submission_status.store_async(synapse_client=self.syn) + async def test_in_place_mutation_of_submission_annotations_detected_and_stored( + self, test_submission_status: SubmissionStatus + ): + """ + Test that in-place mutations (e.g. update()) on submission_annotations are + detected as a change and persisted to Synapse. + """ + # GIVEN a submission status with an initial submission annotation stored + test_submission_status.submission_annotations = { + "initial_key": ["initial_value"] + } + stored = await test_submission_status.store_async(synapse_client=self.syn) + version_after_first_store = stored.status_version + assert not stored.has_changed + + # WHEN I mutate submission_annotations in-place via .update() + stored.submission_annotations.update({"added_key": ["added_value"]}) + + # THEN has_changed should detect the mutation (not silently ignore it) + assert stored.has_changed + + # WHEN I store the mutated status + updated = await stored.store_async(synapse_client=self.syn) + + # THEN the changes should be persisted (status_version increments and both keys present) + assert updated.status_version > version_after_first_store + assert "initial_key" in updated.submission_annotations + assert "added_key" in updated.submission_annotations + + async def test_in_place_mutation_of_annotations_detected_and_stored( + self, test_submission_status: SubmissionStatus + ): + """ + Test that in-place mutations (e.g. update()) on annotations are + detected as a change and persisted to Synapse. + """ + # GIVEN a submission status with no legacy annotations (empty dict) + assert not test_submission_status.has_changed + + # WHEN I mutate annotations in-place via .update() + test_submission_status.annotations.update({"added_key": "added_value"}) + + # THEN has_changed should detect the mutation (not silently ignore it) + assert test_submission_status.has_changed + + # WHEN I store the mutated status + version_before = test_submission_status.status_version + updated = await test_submission_status.store_async(synapse_client=self.syn) + + # THEN the store should have been sent (status_version increments) + assert updated.status_version > version_before + async def test_store_submission_status_without_changes( self, test_submission_status: SubmissionStatus ): From 474d9c95a1241694bffd52a73caf0bb781267e98 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Fri, 26 Jun 2026 15:51:05 -0400 Subject: [PATCH 3/3] refactor: turn integration tests into unit tests --- .../async/test_submission_status_async.py | 52 --------------- .../unit_test_submission_status_async.py | 65 +++++++++++++++++++ 2 files changed, 65 insertions(+), 52 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_submission_status_async.py b/tests/integration/synapseclient/models/async/test_submission_status_async.py index 641b039e5..f345fa2e8 100644 --- a/tests/integration/synapseclient/models/async/test_submission_status_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_status_async.py @@ -390,58 +390,6 @@ async def test_store_submission_status_without_id(self): ): await submission_status.store_async(synapse_client=self.syn) - async def test_in_place_mutation_of_submission_annotations_detected_and_stored( - self, test_submission_status: SubmissionStatus - ): - """ - Test that in-place mutations (e.g. update()) on submission_annotations are - detected as a change and persisted to Synapse. - """ - # GIVEN a submission status with an initial submission annotation stored - test_submission_status.submission_annotations = { - "initial_key": ["initial_value"] - } - stored = await test_submission_status.store_async(synapse_client=self.syn) - version_after_first_store = stored.status_version - assert not stored.has_changed - - # WHEN I mutate submission_annotations in-place via .update() - stored.submission_annotations.update({"added_key": ["added_value"]}) - - # THEN has_changed should detect the mutation (not silently ignore it) - assert stored.has_changed - - # WHEN I store the mutated status - updated = await stored.store_async(synapse_client=self.syn) - - # THEN the changes should be persisted (status_version increments and both keys present) - assert updated.status_version > version_after_first_store - assert "initial_key" in updated.submission_annotations - assert "added_key" in updated.submission_annotations - - async def test_in_place_mutation_of_annotations_detected_and_stored( - self, test_submission_status: SubmissionStatus - ): - """ - Test that in-place mutations (e.g. update()) on annotations are - detected as a change and persisted to Synapse. - """ - # GIVEN a submission status with no legacy annotations (empty dict) - assert not test_submission_status.has_changed - - # WHEN I mutate annotations in-place via .update() - test_submission_status.annotations.update({"added_key": "added_value"}) - - # THEN has_changed should detect the mutation (not silently ignore it) - assert test_submission_status.has_changed - - # WHEN I store the mutated status - version_before = test_submission_status.status_version - updated = await test_submission_status.store_async(synapse_client=self.syn) - - # THEN the store should have been sent (status_version increments) - assert updated.status_version > version_before - async def test_store_submission_status_without_changes( self, test_submission_status: SubmissionStatus ): diff --git a/tests/unit/synapseclient/models/async/unit_test_submission_status_async.py b/tests/unit/synapseclient/models/async/unit_test_submission_status_async.py index ff4137c9e..7c122b1c9 100644 --- a/tests/unit/synapseclient/models/async/unit_test_submission_status_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_submission_status_async.py @@ -478,6 +478,71 @@ async def test_batch_update_with_batch_token(self) -> None: assert request_body["batchToken"] == batch_token assert request_body["isFirstBatch"] is False + def test_in_place_mutation_of_submission_annotations_detected_by_has_changed( + self, + ) -> None: + """ + Test that in-place .update() on submission_annotations are + detected as a change by has_changed. + """ + # GIVEN a status with submission_annotations and a stashed persistent instance + submission_status = SubmissionStatus( + id=SUBMISSION_STATUS_ID, + submission_annotations={"initial_key": ["initial_value"]}, + ) + submission_status._set_last_persistent_instance() + assert not submission_status.has_changed + + # WHEN I mutate submission_annotations in-place via .update() + submission_status.submission_annotations.update({"added_key": ["added_value"]}) + + # THEN has_changed should detect the mutation + assert submission_status.has_changed + + def test_in_place_mutation_of_annotations_detected_by_has_changed(self) -> None: + """ + That that in-place .update() on annotations are detected as + a change by has_changed. + """ + # GIVEN a status with annotations and a stashed persistent instance + submission_status = SubmissionStatus( + id=SUBMISSION_STATUS_ID, + annotations={"initial_key": "initial_value"}, + ) + submission_status._set_last_persistent_instance() + assert not submission_status.has_changed + + # WHEN I mutate annotations in-place via .update() + submission_status.annotations.update({"added_key": "added_value"}) + + # THEN has_changed should detect the mutation + assert submission_status.has_changed + + async def test_store_async_sends_request_after_in_place_mutation(self) -> None: + """Regression test: store_async must call the API when submission_annotations + was mutated in-place via .update(), not skip it as 'no changes'.""" + # GIVEN a status with a stashed persistent instance + submission_status = SubmissionStatus( + id=SUBMISSION_STATUS_ID, + etag=ETAG, + status_version=STATUS_VERSION, + submission_annotations={"initial_key": ["initial_value"]}, + ) + submission_status._set_last_persistent_instance() + + # WHEN I mutate in-place and store + submission_status.submission_annotations.update({"added_key": ["added_value"]}) + + with patch( + "synapseclient.api.evaluation_services.update_submission_status", + new_callable=AsyncMock, + return_value=self.get_example_submission_status_dict(), + ) as mock_update: + await submission_status.store_async(synapse_client=self.syn) + + # THEN the API should have been called (not skipped) + mock_update.assert_called_once() + def test_set_last_persistent_instance(self) -> None: """Test setting the last persistent instance.""" # GIVEN a SubmissionStatus