diff --git a/src/mavedb/lib/permissions/score_calibration.py b/src/mavedb/lib/permissions/score_calibration.py index 86b404f5c..d00d01791 100644 --- a/src/mavedb/lib/permissions/score_calibration.py +++ b/src/mavedb/lib/permissions/score_calibration.py @@ -4,6 +4,7 @@ from mavedb.lib.logging.context import save_to_logging_context from mavedb.lib.permissions.actions import Action from mavedb.lib.permissions.models import PermissionResponse +from mavedb.lib.permissions.score_set import has_permission as score_set_has_permission from mavedb.lib.permissions.utils import deny_action_for_entity, roles_permitted from mavedb.lib.permissions.viewer import Viewer from mavedb.lib.types.authentication import UserData @@ -105,9 +106,12 @@ def _handle_read_action( """ Handle READ action permission check for ScoreCalibration entities. - ScoreCalibrations are generally readable by anyone who can access the - associated ScoreSet, as they provide important contextual information - about the score data. + TODO(#876) - Separates the readability concern by adding the notion of a separate + calibration method. For now, A ScoreCalibration is never readable by a user who + cannot read its ScoreSet, because the calibration variant routes return the score + set's variants and scores. Among users who can read the ScoreSet, published + calibrations are readable by all and private calibrations only by their owner, + contributors (for investigator-provided calibrations), and admins. Args: user_data: The user's authentication data. @@ -120,6 +124,10 @@ def _handle_read_action( Returns: PermissionResponse: Permission result with appropriate HTTP status. """ + # Deny as not found so the calibrations of an unreadable score set are not acknowledged. + if not score_set_has_permission(user_data, entity.score_set, Action.READ).permitted: + return deny_action_for_entity(entity, True, user_data, False, "score calibration") + ## Allow read access under the following conditions: # Any user may read a ScoreCalibration if it is not private. if not private: diff --git a/src/mavedb/lib/score_calibrations.py b/src/mavedb/lib/score_calibrations.py index 11e1b2e88..1796b3a8d 100644 --- a/src/mavedb/lib/score_calibrations.py +++ b/src/mavedb/lib/score_calibrations.py @@ -506,7 +506,8 @@ def publish_score_calibration(db: Session, calibration: ScoreCalibration, user: Raises ------ ValueError - If the calibration is already published (i.e., `private` is False). + If the calibration is already published (i.e., `private` is False), or if its + score set is still private. Notes ----- @@ -516,6 +517,9 @@ def publish_score_calibration(db: Session, calibration: ScoreCalibration, user: if not calibration.private: raise ValueError("Calibration is already published.") + if calibration.score_set.private: + raise ValueError("Cannot publish a calibration whose score set is private.") + calibration.private = False calibration.modified_by = user diff --git a/src/mavedb/routers/score_calibrations.py b/src/mavedb/routers/score_calibrations.py index e8b31478c..1df20b5ee 100644 --- a/src/mavedb/routers/score_calibrations.py +++ b/src/mavedb/routers/score_calibrations.py @@ -67,14 +67,23 @@ def list_my_calibrations( db: Session = Depends(deps.get_db), user_data: UserData = Depends(require_current_user), ) -> list[ScoreCalibration]: - """List all score calibrations created by the current user.""" - return ( + """ + List the score calibrations created by the current user that the user may still read. + + Calibrations on score sets the user can no longer read, for example after being removed as a + contributor, are omitted. + """ + calibrations = ( db.query(ScoreCalibration) .filter(ScoreCalibration.created_by_id == user_data.user.id) .options(selectinload(ScoreCalibration.score_set).selectinload(ScoreSet.contributors)) .all() ) + return [ + calibration for calibration in calibrations if has_permission(user_data, calibration, Action.READ).permitted + ] + @router.get( "/{urn}", @@ -672,6 +681,8 @@ def publish_score_calibration_route( ) -> ScoreCalibration: """ Publish a score calibration, making it publicly visible. + + The calibration's score set must already be published. """ save_to_logging_context({"requested_resource": urn, "resource_property": "private"}) @@ -691,15 +702,14 @@ def publish_score_calibration_route( logger.debug("The requested score calibration is already public", extra=logging_context()) return item - # XXX: desired? - # if item.score_set.private: - # logger.debug( - # "Score calibrations associated with private score sets cannot be published", extra=logging_context() - # ) - # raise HTTPException( - # status_code=400, - # detail="Score calibrations associated with private score sets cannot be published. First publish the score set, then calibrations.", - # ) + if item.score_set.private: + logger.debug( + "Score calibrations associated with private score sets cannot be published", extra=logging_context() + ) + raise HTTPException( + status_code=400, + detail="Score calibrations associated with private score sets cannot be published. First publish the score set, then calibrations.", + ) item = publish_score_calibration(db, item, user_data.user) db.commit() diff --git a/tests/helpers/mocks/factories.py b/tests/helpers/mocks/factories.py index 6e83d089c..7c7f7aac7 100644 --- a/tests/helpers/mocks/factories.py +++ b/tests/helpers/mocks/factories.py @@ -153,6 +153,10 @@ def create_mock_score_set( mock_target_gene = create_mock_target_gene() mock_experiment = create_mock_experiment() + # Read by the score set permission check, which calibration visibility depends on. + kwargs.setdefault("private", False) + kwargs.setdefault("contributors", []) + return create_sealed_mock( urn=urn, title=title, diff --git a/tests/helpers/util/score_calibration.py b/tests/helpers/util/score_calibration.py index a535096c2..ab3d0b9c0 100644 --- a/tests/helpers/util/score_calibration.py +++ b/tests/helpers/util/score_calibration.py @@ -48,6 +48,18 @@ def create_test_score_calibration_in_score_set_via_client( return calibration +def force_publish_test_score_calibration(db: "Session", calibration_urn: str) -> None: + """Mark a calibration public without the publish route's checks. + + Reproduces calibrations that were published while their score set was still private, which the publish + route now refuses. + """ + calibration = db.query(ScoreCalibration).filter(ScoreCalibration.urn == calibration_urn).one() + calibration.private = False + db.add(calibration) + db.commit() + + def publish_test_score_calibration_via_client(client: "TestClient", calibration_urn: str): response = client.post(f"/api/v1/score-calibrations/{calibration_urn}/publish") diff --git a/tests/lib/annotation/conftest.py b/tests/lib/annotation/conftest.py index 29a056c67..fbdbf052d 100644 --- a/tests/lib/annotation/conftest.py +++ b/tests/lib/annotation/conftest.py @@ -27,12 +27,12 @@ def make_private(mapped_variant, *, owner_id: int = PRIVATE_CALIBRATION_OWNER_ID """Mark every calibration on a mapped variant's score set private, owned by ``owner_id``. The real permission check reads ``created_by_id`` and the owning score set's contributor list, neither - of which the annotation mocks populate. + of which the annotation mocks populate. The score set stays public so only the calibration is private. """ for calibration in mapped_variant.variant.score_set.score_calibrations: calibration.private = True calibration.created_by_id = owner_id - calibration.score_set = Mock(contributors=[], created_by_id=owner_id, modified_by_id=owner_id) + calibration.score_set = Mock(private=False, contributors=[], created_by_id=owner_id, modified_by_id=owner_id) return mapped_variant diff --git a/tests/lib/csv/test_variant.py b/tests/lib/csv/test_variant.py index 6b475d6fd..37029fd18 100644 --- a/tests/lib/csv/test_variant.py +++ b/tests/lib/csv/test_variant.py @@ -54,6 +54,10 @@ def _add_pathogenicity_calibration(db, score_set, variants_in_abnormal_range, ur Only *variants_in_abnormal_range* are associated with the abnormal range, which is what ``functional_classification_of_variant`` consults to classify a variant. """ + # A public calibration is only readable when its score set is too. + score_set.private = False + db.add(score_set) + calibration = ScoreCalibration( score_set_id=score_set.id, urn=urn, @@ -111,6 +115,10 @@ def _add_rangeless_calibration(db, score_set, urn, title): It can support neither a functional nor a pathogenicity annotation, so every column of its namespace would be NA. """ + # A public calibration is only readable when its score set is too. + score_set.private = False + db.add(score_set) + calibration = ScoreCalibration( score_set_id=score_set.id, urn=urn, diff --git a/tests/lib/permissions/conftest.py b/tests/lib/permissions/conftest.py index 302159f5e..933372a4a 100644 --- a/tests/lib/permissions/conftest.py +++ b/tests/lib/permissions/conftest.py @@ -169,15 +169,22 @@ def create_user(user_id: int = 5): ) @staticmethod - def create_score_calibration(entity_state: str = "private", investigator_provided: bool = False): + def create_score_calibration( + entity_state: str = "private", + investigator_provided: bool = False, + score_set_state: Optional[str] = None, + score_set_owner_id: int = 2, + ): """Create a ScoreCalibration mock for testing. Args: entity_state: "private" or "published" (affects score_set and private property) investigator_provided: True if investigator-provided, False if community-provided + score_set_state: "private" or "published" for the score set; defaults to entity_state + score_set_owner_id: ID of the score set's owner; defaults to the calibration's owner """ private = entity_state == "private" - score_set = EntityTestHelper.create_score_set(entity_state) + score_set = EntityTestHelper.create_score_set(score_set_state or entity_state, owner_id=score_set_owner_id) # ScoreCalibrations have their own private property plus associated ScoreSet return Mock( diff --git a/tests/lib/permissions/test_score_calibration.py b/tests/lib/permissions/test_score_calibration.py index a9ea8370d..9689ef42c 100644 --- a/tests/lib/permissions/test_score_calibration.py +++ b/tests/lib/permissions/test_score_calibration.py @@ -205,6 +205,46 @@ def test_handle_read_action(self, test_case: PermissionTest, entity_helper: Enti assert result.http_code == test_case.expected_code +class TestScoreCalibrationReadRequiresScoreSetRead: + """A calibration is never more visible than its score set, since its variant routes return the score set's scores.""" + + @pytest.mark.parametrize( + "user_type, should_be_permitted", + [ + ("admin", True), + ("owner", True), + ("contributor", True), + ("mapper", True), + ("other_user", False), + ("anonymous", False), + ], + ) + def test_published_calibration_on_private_score_set( + self, entity_helper: EntityTestHelper, user_type: str, should_be_permitted: bool + ) -> None: + score_calibration = entity_helper.create_score_calibration("published", score_set_state="private") + + result = has_permission(entity_helper.create_user_data(user_type), score_calibration, Action.READ) + + assert result.permitted == should_be_permitted + if not should_be_permitted: + assert result.http_code == 404 + + def test_owner_who_cannot_read_the_score_set_cannot_read_their_calibration( + self, entity_helper: EntityTestHelper + ) -> None: + # The calibration's creator is neither the score set's owner nor one of its contributors, e.g. after + # being removed from the contributor list. + score_calibration = entity_helper.create_score_calibration( + "private", investigator_provided=True, score_set_owner_id=7 + ) + + result = has_permission(entity_helper.create_user_data("owner"), score_calibration, Action.READ) + + assert not result.permitted + assert result.http_code == 404 + + class TestScoreCalibrationUpdateActionHandler: """Test the _handle_update_action helper function directly.""" diff --git a/tests/lib/test_score_calibrations.py b/tests/lib/test_score_calibrations.py index 9a5ba43c5..3b080e90d 100644 --- a/tests/lib/test_score_calibrations.py +++ b/tests/lib/test_score_calibrations.py @@ -1193,12 +1193,39 @@ async def test_publish_score_calibration_marks_calibration_public( existing_calibration = await create_test_range_based_score_calibration_in_score_set( session, setup_lib_db_with_score_set.urn, test_user ) + existing_calibration.score_set.private = False assert existing_calibration.private is True published_calibration = publish_score_calibration(session, existing_calibration, test_user) assert published_calibration.private is False +@pytest.mark.asyncio +@pytest.mark.parametrize( + "mock_publication_fetch", + [ + [ + {"dbName": "PubMed", "identifier": TEST_PUBMED_IDENTIFIER}, + {"dbName": "bioRxiv", "identifier": TEST_BIORXIV_IDENTIFIER}, + ], + ], + indirect=["mock_publication_fetch"], +) +async def test_cannot_publish_calibration_when_score_set_is_private( + setup_lib_db_with_score_set, session, mock_publication_fetch +): + test_user = session.execute(select(User)).scalars().first() + + existing_calibration = await create_test_range_based_score_calibration_in_score_set( + session, setup_lib_db_with_score_set.urn, test_user + ) + assert existing_calibration.score_set.private is True + + with pytest.raises(ValueError, match="Cannot publish a calibration whose score set is private."): + publish_score_calibration(session, existing_calibration, test_user) + assert existing_calibration.private is True + + @pytest.mark.asyncio @pytest.mark.parametrize( "mock_publication_fetch", @@ -1218,6 +1245,7 @@ async def test_publish_score_calibration_user_is_set_as_modifier( existing_calibration = await create_test_range_based_score_calibration_in_score_set( session, setup_lib_db_with_score_set.urn, test_user ) + existing_calibration.score_set.private = False publish_user = session.execute(select(User).where(User.id != test_user.id)).scalars().first() published_calibration = publish_score_calibration(session, existing_calibration, publish_user) diff --git a/tests/routers/test_experiments.py b/tests/routers/test_experiments.py index acd949392..e4f56785e 100644 --- a/tests/routers/test_experiments.py +++ b/tests/routers/test_experiments.py @@ -1914,11 +1914,11 @@ def test_experiment_score_sets_serve_published_calibrations_to_anonymous_users( calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) - publish_test_score_calibration_via_client(client, calibration["urn"]) - with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): published = publish_score_set(client, score_set["urn"]) + publish_test_score_calibration_via_client(client, calibration["urn"]) + experiment_urn = published["experiment"]["urn"] with DependencyOverrider(anonymous_app_overrides): diff --git a/tests/routers/test_mapped_variants.py b/tests/routers/test_mapped_variants.py index 51f45452f..6372faf78 100644 --- a/tests/routers/test_mapped_variants.py +++ b/tests/routers/test_mapped_variants.py @@ -1,6 +1,7 @@ # ruff: noqa: E402 import json +from unittest.mock import patch import pytest @@ -32,6 +33,7 @@ from tests.helpers.util.score_set import ( create_seq_score_set_with_mapped_variants, create_seq_score_set_with_variants, + publish_score_set, ) @@ -213,6 +215,8 @@ def test_show_mapped_variant_functional_impact_statement( experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -294,6 +298,8 @@ def test_cannot_show_mapped_variant_functional_impact_statement_when_no_mapping_ experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -360,6 +366,8 @@ def test_show_mapped_variant_clinical_evidence_line( experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -441,6 +449,8 @@ def test_cannot_show_mapped_variant_clinical_evidence_line_when_no_mapping_data_ experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) diff --git a/tests/routers/test_score_calibrations.py b/tests/routers/test_score_calibrations.py index fe1aeba7c..16b4b4c31 100644 --- a/tests/routers/test_score_calibrations.py +++ b/tests/routers/test_score_calibrations.py @@ -32,6 +32,7 @@ from tests.helpers.util.score_calibration import ( create_publish_and_promote_score_calibration, create_test_score_calibration_in_score_set_via_client, + force_publish_test_score_calibration, publish_test_score_calibration_via_client, ) from tests.helpers.util.score_set import create_seq_score_set_with_mapped_variants, publish_score_set @@ -297,6 +298,8 @@ def test_anonymous_user_can_get_score_calibration_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -332,6 +335,8 @@ def test_other_user_can_get_score_calibration_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -367,6 +372,8 @@ def test_creating_user_can_get_score_calibration_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -401,6 +408,8 @@ def test_contributing_user_can_get_score_calibration_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -445,6 +454,8 @@ def test_admin_user_can_get_score_calibration_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -787,11 +798,11 @@ def test_anonymous_user_can_get_score_calibrations_for_score_set_when_public( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) - publish_test_score_calibration_via_client(client, calibration["urn"]) - with patch.object(ArqRedis, "enqueue_job", return_value=None): score_set = publish_score_set(client, score_set["urn"]) + publish_test_score_calibration_via_client(client, calibration["urn"]) + with DependencyOverrider(anonymous_app_overrides): response = client.get(f"/api/v1/score-calibrations/score-set/{score_set['urn']}") @@ -832,11 +843,11 @@ def test_other_user_can_get_score_calibrations_for_score_set_when_public( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) - publish_test_score_calibration_via_client(client, calibration["urn"]) - with patch.object(ArqRedis, "enqueue_job", return_value=None): score_set = publish_score_set(client, score_set["urn"]) + publish_test_score_calibration_via_client(client, calibration["urn"]) + with DependencyOverrider(extra_user_app_overrides): response = client.get(f"/api/v1/score-calibrations/score-set/{score_set['urn']}") @@ -877,7 +888,7 @@ def test_anonymous_user_cannot_get_score_calibrations_for_score_set_when_calibra client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) - publish_test_score_calibration_via_client(client, calibration["urn"]) + force_publish_test_score_calibration(session, calibration["urn"]) with DependencyOverrider(anonymous_app_overrides): response = client.get(f"/api/v1/score-calibrations/score-set/{score_set['urn']}") @@ -917,7 +928,7 @@ def test_other_user_cannot_get_score_calibrations_for_score_set_when_calibration client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) - publish_test_score_calibration_via_client(client, calibration["urn"]) + force_publish_test_score_calibration(session, calibration["urn"]) with DependencyOverrider(extra_user_app_overrides): response = client.get(f"/api/v1/score-calibrations/score-set/{score_set['urn']}") @@ -948,6 +959,8 @@ def test_creating_user_can_get_score_calibrations_for_score_set_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -988,6 +1001,8 @@ def test_contributing_user_can_get_score_calibrations_for_score_set_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -1038,6 +1053,8 @@ def test_admin_user_can_get_score_calibrations_for_score_set_when_public( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -1148,6 +1165,8 @@ def test_get_primary_score_calibration_for_score_set_when_exists( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -1183,6 +1202,8 @@ def test_get_primary_score_calibration_for_score_set_when_multiple_exist( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) @@ -2212,6 +2233,8 @@ def test_cannot_update_published_score_calibration_as_score_set_owner( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -2400,6 +2423,8 @@ def test_can_update_published_score_calibration_as_admin_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -2826,6 +2851,8 @@ def test_cannot_delete_published_score_calibration_as_owner( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -2985,6 +3012,8 @@ def test_can_delete_published_score_calibration_as_admin_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3021,6 +3050,8 @@ def test_cannot_delete_primary_score_calibration( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3069,6 +3100,8 @@ def test_cannot_promote_score_calibration_as_anonymous_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3103,6 +3136,8 @@ def test_cannot_promote_score_calibration_when_score_calibration_not_owned_by_us experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3139,6 +3174,8 @@ def test_can_promote_score_calibration_as_score_set_owner( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3173,6 +3210,8 @@ def test_can_promote_score_calibration_as_score_set_contributor( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3218,6 +3257,8 @@ def test_can_promote_score_calibration_as_admin_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3254,6 +3295,8 @@ def test_can_promote_existing_primary_to_primary( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) primary_calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3288,6 +3331,8 @@ def test_cannot_promote_research_use_only_to_primary( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], @@ -3357,6 +3402,8 @@ def test_cannot_promote_to_primary_if_primary_exists( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3393,6 +3440,8 @@ def test_can_promote_to_primary_if_primary_exists_when_demote_existing_is_true( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) primary_calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3441,6 +3490,8 @@ def test_score_set_owner_can_promote_to_primary_with_demote_existing_flag_on_com experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) with DependencyOverrider(admin_app_overrides): primary_calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) @@ -3501,6 +3552,8 @@ def test_cannot_demote_score_calibration_as_anonymous_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3536,6 +3589,8 @@ def test_cannot_demote_score_calibration_when_score_calibration_not_owned_by_use experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3571,6 +3626,8 @@ def test_can_demote_score_calibration_as_score_set_contributor( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3617,6 +3674,8 @@ def test_can_demote_score_calibration_as_score_set_owner( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3653,6 +3712,8 @@ def test_can_demote_score_calibration_as_admin_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3690,6 +3751,8 @@ def test_can_demote_non_primary_score_calibration( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3826,6 +3889,8 @@ def test_can_publish_score_calibration_as_score_set_owner( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3862,6 +3927,8 @@ def test_can_publish_score_calibration_as_admin_user( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3899,6 +3966,8 @@ def test_can_publish_already_published_calibration( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -3920,6 +3989,45 @@ def test_can_publish_already_published_calibration( assert published_calibration_2["private"] is False +@pytest.mark.parametrize( + "mock_publication_fetch", + [ + [ + {"dbName": "PubMed", "identifier": TEST_PUBMED_IDENTIFIER}, + {"dbName": "bioRxiv", "identifier": TEST_BIORXIV_IDENTIFIER}, + ] + ], + indirect=["mock_publication_fetch"], +) +def test_cannot_publish_score_calibration_when_score_set_is_private( + client, setup_router_db, mock_publication_fetch, session, data_provider, data_files +): + experiment = create_experiment(client) + score_set = create_seq_score_set_with_mapped_variants( + client, + session, + data_provider, + experiment["urn"], + data_files / "scores.csv", + ) + calibration = create_test_score_calibration_in_score_set_via_client( + client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) + ) + + response = client.post( + f"/api/v1/score-calibrations/{calibration['urn']}/publish", + ) + + assert response.status_code == 400 + error = response.json() + assert "Score calibrations associated with private score sets cannot be published" in error["detail"] + + persisted = session.execute( + select(CalibrationDbModel).where(CalibrationDbModel.urn == calibration["urn"]) + ).scalar_one() + assert persisted.private is True + + ########################################################### # GET /score-calibrations/{urn}/functional-classifications/{id}/variants ########################################################### @@ -4206,6 +4314,8 @@ def test_anonymous_user_can_get_variants_for_public_calibration( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, @@ -4480,6 +4590,8 @@ def test_anonymous_user_can_get_all_variants_for_public_calibration( experiment["urn"], data_files / "scores.csv", ) + with patch.object(ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) calibration = create_test_score_calibration_in_score_set_via_client( client, @@ -4503,6 +4615,71 @@ def test_anonymous_user_can_get_all_variants_for_public_calibration( assert len(fc_variants["variants"]) == calibration["functionalClassifications"][i]["variantCount"] +def _create_published_calibration_on_private_score_set(client, session, data_provider, data_files) -> dict: + """Create a calibration marked public on a score set that is still private.""" + experiment = create_experiment(client) + score_set = create_seq_score_set_with_mapped_variants( + client, + session, + data_provider, + experiment["urn"], + data_files / "scores.csv", + ) + calibration = create_test_score_calibration_in_score_set_via_client( + client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) + ) + + force_publish_test_score_calibration(session, calibration["urn"]) + + return calibration + + +@pytest.mark.parametrize( + "mock_publication_fetch", + [ + [ + {"dbName": "PubMed", "identifier": TEST_PUBMED_IDENTIFIER}, + {"dbName": "bioRxiv", "identifier": TEST_BIORXIV_IDENTIFIER}, + ] + ], + indirect=["mock_publication_fetch"], +) +@pytest.mark.parametrize("route", ["", "/variants", "/functional-classifications/{fc_id}/variants"]) +def test_anonymous_user_cannot_read_published_calibration_on_private_score_set( + client, setup_router_db, mock_publication_fetch, session, data_provider, data_files, anonymous_app_overrides, route +): + calibration = _create_published_calibration_on_private_score_set(client, session, data_provider, data_files) + fc_id = calibration["functionalClassifications"][0]["id"] + + with DependencyOverrider(anonymous_app_overrides): + response = client.get(f"/api/v1/score-calibrations/{calibration['urn']}{route.format(fc_id=fc_id)}") + + assert response.status_code == 404 + error = response.json() + assert f"score calibration with URN '{calibration['urn']}' not found" in error["detail"] + + +@pytest.mark.parametrize( + "mock_publication_fetch", + [ + [ + {"dbName": "PubMed", "identifier": TEST_PUBMED_IDENTIFIER}, + {"dbName": "bioRxiv", "identifier": TEST_BIORXIV_IDENTIFIER}, + ] + ], + indirect=["mock_publication_fetch"], +) +def test_score_set_owner_can_read_published_calibration_on_private_score_set( + client, setup_router_db, mock_publication_fetch, session, data_provider, data_files +): + calibration = _create_published_calibration_on_private_score_set(client, session, data_provider, data_files) + + response = client.get(f"/api/v1/score-calibrations/{calibration['urn']}/variants") + + assert response.status_code == 200 + assert len(response.json()) == len(calibration["functionalClassifications"]) + + ########################################################### # Independent calibration creation and publication source validation ########################################################### @@ -4939,6 +5116,54 @@ def test_authenticated_user_sees_own_calibrations( assert calibrations[0]["scoreSetUrn"] == score_set["urn"] +@pytest.mark.parametrize( + "mock_publication_fetch", + [ + [ + {"dbName": "PubMed", "identifier": TEST_PUBMED_IDENTIFIER}, + {"dbName": "bioRxiv", "identifier": TEST_BIORXIV_IDENTIFIER}, + ] + ], + indirect=["mock_publication_fetch"], +) +def test_user_does_not_see_own_calibrations_on_score_sets_they_can_no_longer_read( + client, setup_router_db, mock_publication_fetch, session, data_provider, data_files, extra_user_app_overrides +): + experiment = create_experiment(client) + score_set = create_seq_score_set_with_mapped_variants( + client, + session, + data_provider, + experiment["urn"], + data_files / "scores.csv", + ) + add_contributor( + session, + score_set["urn"], + ScoreSetDbModel, + EXTRA_USER["username"], + EXTRA_USER["first_name"], + EXTRA_USER["last_name"], + ) + with DependencyOverrider(extra_user_app_overrides): + create_test_score_calibration_in_score_set_via_client( + client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) + ) + + score_set_item = session.execute( + select(ScoreSetDbModel).where(ScoreSetDbModel.urn == score_set["urn"]) + ).scalar_one() + score_set_item.contributors = [] + session.add(score_set_item) + session.commit() + + with DependencyOverrider(extra_user_app_overrides): + response = client.get("/api/v1/score-calibrations/me") + + assert response.status_code == 200 + assert response.json() == [] + + @pytest.mark.parametrize( "mock_publication_fetch", [ diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index 7d9290a9a..ab15a4de3 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -1142,9 +1142,15 @@ def test_extra_user_can_only_view_published_score_calibrations_in_score_set( ], indirect=["mock_publication_fetch"], ) -def test_creating_user_can_view_all_score_calibrations_in_score_set(client, setup_router_db, mock_publication_fetch): +def test_creating_user_can_view_all_score_calibrations_in_score_set( + client, setup_router_db, mock_publication_fetch, session, data_provider, data_files +): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) + score_set = mock_worker_variant_insertion(client, session, data_provider, score_set, data_files / "scores.csv") + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) + private_calibration = create_test_score_calibration_in_score_set_via_client( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -4416,6 +4422,8 @@ def test_get_annotated_pathogenicity_evidence_lines_for_score_set( experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -4505,6 +4513,8 @@ def test_get_annotated_pathogenicity_evidence_lines_for_score_set_when_some_vari experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -4547,6 +4557,8 @@ def test_get_annotated_functional_impact_statement_for_score_set( experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) @@ -4638,6 +4650,8 @@ def test_get_annotated_functional_impact_statement_for_score_set_when_some_varia experiment["urn"], data_files / "scores.csv", ) + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None): + score_set = publish_score_set(client, score_set["urn"]) create_publish_and_promote_score_calibration( client, score_set["urn"], deepcamelize(TEST_BRNICH_SCORE_CALIBRATION_RANGE_BASED) ) diff --git a/tests/scripts/conftest.py b/tests/scripts/conftest.py index af77e8011..eb81dee7c 100644 --- a/tests/scripts/conftest.py +++ b/tests/scripts/conftest.py @@ -287,7 +287,7 @@ def make_dump_score_set(session, sample_user, dump_experiment, dump_licenses, du is how a variant the mapper could not place is stored, and which the README documents as yielding a null `annotation`. Note that the shared `TEST_MINIMAL_MAPPED_VARIANT` uses an empty dict here, a shape production never stores and the annotation layer cannot parse. - published: sets published_date, which the dump's selection query requires. + published: sets published_date, which the dump's selection query requires, and clears private. cc0: whether the score set carries the CC0 license the dump requires. """ counter = {"n": 0} @@ -318,6 +318,7 @@ def _make( modified_by=sample_user, licence_id=CC0_LICENSE_ID if cc0 else OTHER_LICENSE_ID, published_date=date(2024, 1, 1) if published else None, + private=not published, dataset_columns={"score_columns": score_columns, "count_columns": list(count_columns)}, target_genes=[ TargetGene(