From 31af1f30fbde554c0e52ef50baa8fe7e1294a581 Mon Sep 17 00:00:00 2001 From: Benjamin Capodanno Date: Thu, 24 Sep 2026 09:22:26 -0700 Subject: [PATCH] fix(score-sets): only reuse meta-analysis experiments the caller can add to --- src/mavedb/lib/score_sets.py | 7 +- src/mavedb/routers/score_sets.py | 31 +++++-- tests/routers/test_score_set.py | 155 +++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 8 deletions(-) diff --git a/src/mavedb/lib/score_sets.py b/src/mavedb/lib/score_sets.py index 698bc515a..8a3624543 100644 --- a/src/mavedb/lib/score_sets.py +++ b/src/mavedb/lib/score_sets.py @@ -444,10 +444,12 @@ def find_meta_analyses_for_experiment_sets(db: Session, urns: list[str]) -> list """ Find all score sets that are meta-analyses for score sets from a specified collection of experiment sets. + Results are not filtered by visibility or ownership; callers must check permissions before using them. + :param db: An active database session. :param urns: A list of experiment set URNS. - :return: A score set that is a meta-analysis for score sets belonging to exactly the collection of experiment sets - specified by urns; or None if there is no such meta-analysis. + :return: The score sets, ordered by ID, that are meta-analyses for score sets belonging to exactly the collection + of experiment sets specified by urns. """ # Ensure that URNs are not repeated in the list. urns = list(set(urns)) @@ -480,6 +482,7 @@ def find_meta_analyses_for_experiment_sets(db: Session, urns: list[str]) -> list .filter(*urn_filters) .group_by(ScoreSet.id) .having(func.count(func.distinct(analyzed_experiment_set.id)) == len(urns)) + .order_by(ScoreSet.id) .all() ) diff --git a/src/mavedb/routers/score_sets.py b/src/mavedb/routers/score_sets.py index aff713429..1bab6230c 100644 --- a/src/mavedb/routers/score_sets.py +++ b/src/mavedb/routers/score_sets.py @@ -1817,8 +1817,9 @@ async def create_score_set( ) if len(meta_analyzes_score_sets) > 0: - # If any existing score set is a meta-analysis for score sets in the same collection of experiment sets, use its - # experiment as the parent of our new meta-analysis. Otherwise, create a new experiment. + # If an existing score set is a meta-analysis for score sets in the same collection of experiment sets, and the + # user may add score sets to its experiment, use that experiment as the parent of our new meta-analysis. + # Otherwise, create a new experiment. meta_analyzes_experiment_sets = list( set( ( @@ -1830,13 +1831,31 @@ async def create_score_set( ) meta_analyzes_experiment_set_urns = [es.urn for es in meta_analyzes_experiment_sets if es.urn is not None] existing_meta_analyses = find_meta_analyses_for_experiment_sets(db, meta_analyzes_experiment_set_urns) + reusable_experiment = next( + ( + meta_analysis.experiment + for meta_analysis in existing_meta_analyses + if has_permission(user_data, meta_analysis.experiment, Action.ADD_SCORE_SET).permitted + ), + None, + ) - if len(existing_meta_analyses) > 0: - experiment = existing_meta_analyses[0].experiment + if reusable_experiment is not None: + experiment = reusable_experiment elif len(meta_analyzes_experiment_sets) == 1: # The analyzed score sets all belong to one experiment set, so the meta-analysis should go in that - # experiment set's meta-analysis experiment. But there is no meta-analysis experiment (or else we would - # have found it by looking at existing_meta_analyses[0].experiment), so we will create one. + # experiment set's meta-analysis experiment. An experiment set holds at most one meta-analysis experiment + # (see generate_experiment_urn), so if another user's private one exists we cannot create a second. + if len(existing_meta_analyses) > 0: + logger.info( + msg="Failed to create score set; Another user's private meta-analysis experiment exists for the requested meta-analyzed score sets.", + extra=logging_context(), + ) + raise HTTPException( + status_code=409, + detail="A meta-analysis of these score sets is in progress by another user.", + ) + meta_analyzes_experiment_set = meta_analyzes_experiment_sets[0] experiment = Experiment( experiment_set=meta_analyzes_experiment_set, diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index 7d9290a9a..a24d536d2 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -2477,6 +2477,161 @@ def test_multiple_score_set_meta_analysis_multiple_experiment_sets_with_differen assert isinstance(MAVEDB_SCORE_SET_URN_RE.fullmatch(published_meta_score_set["urn"]), re.Match) +def test_meta_analysis_single_experiment_set_conflicts_with_other_users_private_meta_analysis( + session, data_provider, client, setup_router_db, data_files, extra_user_app_overrides +): + 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) as worker_queue: + published_score_set = publish_score_set(client, score_set["urn"]) + worker_queue.assert_called_once() + + private_meta_score_set = create_seq_score_set( + client, + None, + update={ + "title": "Private Meta Analysis Title", + "abstractText": "Private meta-analysis abstract", + "methodText": "Private meta-analysis methods", + "metaAnalyzesScoreSetUrns": [published_score_set["urn"]], + }, + ) + private_experiment_urn = private_meta_score_set["experiment"]["urn"] + + score_set_post_payload = deepcopy(TEST_MINIMAL_SEQ_SCORESET) + score_set_post_payload.update( + {"title": "Test Meta Analysis", "metaAnalyzesScoreSetUrns": [published_score_set["urn"]]} + ) + with DependencyOverrider(extra_user_app_overrides): + response = client.post("/api/v1/score-sets/", json=score_set_post_payload) + + assert response.status_code == 409 + assert response.json()["detail"] == "A meta-analysis of these score sets is in progress by another user." + for private_value in ( + private_experiment_urn, + private_meta_score_set["urn"], + "Private Meta Analysis Title", + "Private meta-analysis abstract", + "Private meta-analysis methods", + ): + assert private_value not in response.text + + private_experiment_score_sets = session.scalars( + select(ScoreSetDbModel).join(ExperimentDbModel).where(ExperimentDbModel.urn == private_experiment_urn) + ).all() + assert [ss.urn for ss in private_experiment_score_sets] == [private_meta_score_set["urn"]] + + +def test_meta_analysis_multiple_experiment_sets_does_not_reuse_other_users_private_meta_analysis( + session, data_provider, client, setup_router_db, data_files, extra_user_app_overrides +): + experiment_1 = create_experiment(client, {"title": "Experiment 1"}) + experiment_2 = create_experiment(client, {"title": "Experiment 2"}) + score_set_1 = create_seq_score_set(client, experiment_1["urn"], update={"title": "Score Set 1"}) + score_set_1 = mock_worker_variant_insertion(client, session, data_provider, score_set_1, data_files / "scores.csv") + score_set_2 = create_seq_score_set(client, experiment_2["urn"], update={"title": "Score Set 2"}) + score_set_2 = mock_worker_variant_insertion(client, session, data_provider, score_set_2, data_files / "scores.csv") + + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue: + published_score_set_1 = publish_score_set(client, score_set_1["urn"]) + published_score_set_2 = publish_score_set(client, score_set_2["urn"]) + worker_queue.assert_called() + + meta_analyzes_score_set_urns = [published_score_set_1["urn"], published_score_set_2["urn"]] + private_meta_score_set = create_seq_score_set( + client, + None, + update={ + "title": "Private Meta Analysis Title", + "abstractText": "Private meta-analysis abstract", + "methodText": "Private meta-analysis methods", + "metaAnalyzesScoreSetUrns": meta_analyzes_score_set_urns, + }, + ) + private_experiment = private_meta_score_set["experiment"] + + with DependencyOverrider(extra_user_app_overrides): + meta_score_set = create_seq_score_set( + client, + None, + update={"title": "Test Meta Analysis", "metaAnalyzesScoreSetUrns": meta_analyzes_score_set_urns}, + ) + meta_score_set = mock_worker_variant_insertion( + client, session, data_provider, meta_score_set, data_files / "scores.csv" + ) + + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue: + published_meta_score_set = publish_score_set(client, meta_score_set["urn"]) + worker_queue.assert_called_once() + + assert meta_score_set["experiment"]["urn"] != private_experiment["urn"] + assert meta_score_set["experiment"]["experimentSetUrn"] != private_experiment["experimentSetUrn"] + for private_value in ( + private_experiment["urn"], + private_experiment["experimentSetUrn"], + private_meta_score_set["urn"], + "Private Meta Analysis Title", + "Private meta-analysis abstract", + "Private meta-analysis methods", + ): + assert private_value not in json.dumps(meta_score_set) + + assert published_meta_score_set["urn"] == "urn:mavedb:00000003-0-1" + + private_experiment_record = session.scalars( + select(ExperimentDbModel).where(ExperimentDbModel.urn == private_experiment["urn"]) + ).one() + assert private_experiment_record.private + assert private_experiment_record.experiment_set.private + assert private_experiment_record.experiment_set.urn == private_experiment["experimentSetUrn"] + + +def test_meta_analysis_joins_other_users_published_meta_analysis_experiment( + session, data_provider, client, setup_router_db, data_files, extra_user_app_overrides +): + 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) as worker_queue: + published_score_set = publish_score_set(client, score_set["urn"]) + worker_queue.assert_called_once() + + other_meta_score_set = create_seq_score_set( + client, + None, + update={"title": "Other Meta Analysis", "metaAnalyzesScoreSetUrns": [published_score_set["urn"]]}, + ) + other_meta_score_set = mock_worker_variant_insertion( + client, session, data_provider, other_meta_score_set, data_files / "scores.csv" + ) + + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue: + published_other_meta_score_set = publish_score_set(client, other_meta_score_set["urn"]) + worker_queue.assert_called_once() + + assert published_other_meta_score_set["experiment"]["urn"] == "urn:mavedb:00000001-0" + + with DependencyOverrider(extra_user_app_overrides): + meta_score_set = create_seq_score_set( + client, + None, + update={"title": "Test Meta Analysis", "metaAnalyzesScoreSetUrns": [published_score_set["urn"]]}, + ) + meta_score_set = mock_worker_variant_insertion( + client, session, data_provider, meta_score_set, data_files / "scores.csv" + ) + + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue: + published_meta_score_set = publish_score_set(client, meta_score_set["urn"]) + worker_queue.assert_called_once() + + assert meta_score_set["experiment"]["urn"] == published_other_meta_score_set["experiment"]["urn"] + assert published_meta_score_set["urn"] == "urn:mavedb:00000001-0-2" + + ######################################################################################################################## # Score set search ########################################################################################################################