diff --git a/.claude/rules/database.md b/.claude/rules/database.md index 74f29ab80..db630809f 100644 --- a/.claude/rules/database.md +++ b/.claude/rules/database.md @@ -1,5 +1,6 @@ --- paths: + - "src/mavedb/db/**" - "src/mavedb/models/**" - "src/mavedb/lib/**" - "src/mavedb/routers/**" @@ -12,3 +13,4 @@ paths: - Write new queries in SQLAlchemy 2.0 `select()` style. Older code still uses `db.query()`; don't copy it. - ValidTime tables never return only live rows by default. Filter with `Model.current` or `Model.live_at(...)`. - Replace ValidTime rows with `supersede_with` or `supersede_live_where`, never a separate retire-then-insert, so the handoff has no gap. +- Mixins shared by models live in `models/mixins/.py`, such as `ValidTime`. `db/` holds what every table builds on: `Base`, `MaterializedView` and sessions. Each top-level `models/` module defines a table. diff --git a/alembic/versions/2dfdc4a88870_store_identifiers_on_owner_rows.py b/alembic/versions/2dfdc4a88870_store_identifiers_on_owner_rows.py new file mode 100644 index 000000000..cc781c81a --- /dev/null +++ b/alembic/versions/2dfdc4a88870_store_identifiers_on_owner_rows.py @@ -0,0 +1,197 @@ +"""store DOI, raw read and target gene identifiers on the rows that use them + +Revision ID: 2dfdc4a88870 +Revises: 6987f253e02a +Create Date: 2026-10-21 + +DOI, raw read (SRA), Ensembl, RefSeq and UniProt identifiers each lived in a shared table that records linked +to by id. Every column but the identifier was a constant, always null, or derivable from it, so the identifier +moves onto the row that uses it (#944): + +- The six DOI and raw read link tables hold the identifier string, keyed by ``(_id, identifier)``. + DOIs are stored bare and upper-cased, as the api now stores new ones: ``doi:`` and resolver prefixes are + stripped, and upper case is the form the DOI registry stores, since DOIs are case-insensitive and a record + shouldn't cite one twice. +- ``ensembl_offsets``, ``refseq_offsets`` and ``uniprot_offsets`` merge into + ``target_gene_external_identifiers``, keyed by ``(target_gene_id, db_name)``: a target gene has at most one + identifier per database. +- The five shared identifier tables are dropped, with any rows no record linked to. Urls are computed when + serializing. + +Under row-level security (#808), these become owned rows, visible exactly when their record is. + +The tables are small (a few thousand rows in all), so the rewrites hold their locks briefly. + +The downgrade rebuilds the shared tables from the link rows, so the unlinked rows don't come back. Ids are +reassigned, the dates are the day of the downgrade, and urls are rebuilt the way the api used to build them, so +gene identifier urls come back null. +""" + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "2dfdc4a88870" +down_revision = "6987f253e02a" +branch_labels = None +depends_on = None + +# The api accepted DOIs with a ``doi:`` or resolver prefix before #944 and stored them as given. This strips the +# prefixes idutils.normalize_doi strips, which is what the api applies to new DOIs. +DOI_NORMALIZE = r"upper(regexp_replace({}, '^(doi:\s*|(https?://)?(dx\.)?doi\.org/)', '', 'i'))" + +# (link table, parent column, shared table, shared id column) +DOI_LINKS = [ + ("scoreset_doi_identifiers", "scoreset_id", "doi_identifiers", "doi_identifier_id"), + ("experiment_doi_identifiers", "experiment_id", "doi_identifiers", "doi_identifier_id"), + ("experiment_set_doi_identifiers", "experiment_set_id", "doi_identifiers", "doi_identifier_id"), +] +RAW_READ_LINKS = [ + ("scoreset_sra_identifiers", "scoreset_id", "sra_identifiers", "sra_identifier_id"), + ("experiment_sra_identifiers", "experiment_id", "sra_identifiers", "sra_identifier_id"), + ("experiment_set_sra_identifiers", "experiment_set_id", "sra_identifiers", "sra_identifier_id"), +] + +# (db_name, offset table, identifier table, whether the identifier table has reference_html) +GENE_IDENTIFIERS = [ + ("Ensembl", "ensembl_offsets", "ensembl_identifiers", False), + ("RefSeq", "refseq_offsets", "refseq_identifiers", True), + ("UniProt", "uniprot_offsets", "uniprot_identifiers", True), +] + + +def _fold_link_table(link: str, parent: str, shared: str, shared_id: str, normalize: str) -> None: + op.execute(f"ALTER TABLE {link} ADD COLUMN identifier character varying") + op.execute( + f"UPDATE {link} l SET identifier = {normalize.format('s.identifier')} FROM {shared} s WHERE s.id = l.{shared_id}" + ) + + # Two shared rows differing only in case or prefix would collide once normalized. There are none today. + op.execute( + f"DELETE FROM {link} a USING {link} b " + f"WHERE a.{parent} = b.{parent} AND a.identifier = b.identifier AND a.ctid > b.ctid" + ) + + op.execute(f"ALTER TABLE {link} DROP CONSTRAINT {link}_pkey") + op.execute(f"ALTER TABLE {link} DROP COLUMN {shared_id}") + op.execute(f"ALTER TABLE {link} ALTER COLUMN identifier SET NOT NULL") + op.execute(f"ALTER TABLE {link} ADD CONSTRAINT {link}_pkey PRIMARY KEY ({parent}, identifier)") + + +def _create_shared_table(shared: str, with_reference_html: bool = False) -> None: + reference_html = "reference_html character varying," if with_reference_html else "" + op.execute( + f""" + CREATE TABLE {shared} ( + id serial NOT NULL, + identifier character varying NOT NULL, + db_name character varying NOT NULL, + db_version character varying, + url character varying, + {reference_html} + creation_date date NOT NULL, + modification_date date NOT NULL, + CONSTRAINT {shared}_pkey PRIMARY KEY (id) + ) + """ + ) + + +def _unfold_link_table(link: str, parent: str, shared: str, shared_id: str) -> None: + op.execute(f"ALTER TABLE {link} ADD COLUMN {shared_id} integer") + op.execute(f"UPDATE {link} l SET {shared_id} = s.id FROM {shared} s WHERE s.identifier = l.identifier") + op.execute(f"ALTER TABLE {link} DROP CONSTRAINT {link}_pkey") + op.execute(f"ALTER TABLE {link} DROP COLUMN identifier") + op.execute(f"ALTER TABLE {link} ALTER COLUMN {shared_id} SET NOT NULL") + op.execute(f"ALTER TABLE {link} ADD CONSTRAINT {link}_pkey PRIMARY KEY ({parent}, {shared_id})") + op.execute( + f"ALTER TABLE {link} ADD CONSTRAINT {link}_{shared_id}_fkey FOREIGN KEY ({shared_id}) REFERENCES {shared}(id)" + ) + + +def upgrade() -> None: + for link, parent, shared, shared_id in DOI_LINKS: + _fold_link_table(link, parent, shared, shared_id, normalize=DOI_NORMALIZE) + for link, parent, shared, shared_id in RAW_READ_LINKS: + _fold_link_table(link, parent, shared, shared_id, normalize="{}") + + op.execute( + """ + CREATE TABLE target_gene_external_identifiers ( + target_gene_id integer NOT NULL, + db_name character varying NOT NULL, + identifier character varying NOT NULL, + "offset" integer NOT NULL, + CONSTRAINT target_gene_external_identifiers_pkey PRIMARY KEY (target_gene_id, db_name), + CONSTRAINT target_gene_external_identifiers_target_gene_id_fkey + FOREIGN KEY (target_gene_id) REFERENCES target_genes(id) ON DELETE CASCADE, + CONSTRAINT ck_target_gene_external_identifiers_db_name_valid + CHECK (db_name IN ('Ensembl', 'RefSeq', 'UniProt')) + ) + """ + ) + for db_name, offsets, identifiers, _ in GENE_IDENTIFIERS: + op.execute( + f""" + INSERT INTO target_gene_external_identifiers (target_gene_id, db_name, identifier, "offset") + SELECT o.target_gene_id, '{db_name}', i.identifier, o."offset" + FROM {offsets} o JOIN {identifiers} i ON i.id = o.identifier_id + """ + ) + op.execute(f"DROP TABLE {offsets}") + op.execute(f"DROP TABLE {identifiers}") + + op.execute("DROP TABLE doi_identifiers") + op.execute("DROP TABLE sra_identifiers") + + +def downgrade() -> None: + for shared, links, db_name, url_prefix in [ + ("doi_identifiers", DOI_LINKS, "DOI", "https://doi.org/"), + ("sra_identifiers", RAW_READ_LINKS, "SRA", "http://www.ebi.ac.uk/ena/data/view/"), + ]: + _create_shared_table(shared) + every_identifier = " UNION ".join(f"SELECT identifier FROM {link}" for link, *_ in links) + op.execute( + f""" + INSERT INTO {shared} (identifier, db_name, url, creation_date, modification_date) + SELECT identifier, '{db_name}', '{url_prefix}' || identifier, current_date, current_date + FROM ({every_identifier}) AS every_identifier + ORDER BY identifier + """ + ) + for link, parent, _, shared_id in links: + _unfold_link_table(link, parent, shared, shared_id) + + for db_name, offsets, identifiers, with_reference_html in GENE_IDENTIFIERS: + _create_shared_table(identifiers, with_reference_html) + op.execute( + f""" + INSERT INTO {identifiers} (identifier, db_name, creation_date, modification_date) + SELECT DISTINCT identifier, db_name, current_date, current_date + FROM target_gene_external_identifiers WHERE db_name = '{db_name}' + ORDER BY identifier + """ + ) + op.execute( + f""" + CREATE TABLE {offsets} ( + identifier_id integer NOT NULL, + target_gene_id integer NOT NULL, + "offset" integer NOT NULL, + CONSTRAINT {offsets}_pkey PRIMARY KEY (identifier_id, target_gene_id), + CONSTRAINT {offsets}_identifier_id_fkey FOREIGN KEY (identifier_id) REFERENCES {identifiers}(id), + CONSTRAINT {offsets}_target_gene_id_fkey FOREIGN KEY (target_gene_id) REFERENCES target_genes(id) + ) + """ + ) + op.execute( + f""" + INSERT INTO {offsets} (identifier_id, target_gene_id, "offset") + SELECT i.id, t.target_gene_id, t."offset" + FROM target_gene_external_identifiers t + JOIN {identifiers} i ON i.identifier = t.identifier + WHERE t.db_name = '{db_name}' + """ + ) + + op.execute("DROP TABLE target_gene_external_identifiers") diff --git a/src/mavedb/lib/external_identifier_urls.py b/src/mavedb/lib/external_identifier_urls.py new file mode 100644 index 000000000..8a9411ab3 --- /dev/null +++ b/src/mavedb/lib/external_identifier_urls.py @@ -0,0 +1,47 @@ +"""Links to the external databases that records cite through :class:`ExternalIdentifier` models. + +MaveDB has no other code for these databases, so their link rules live together here. A source MaveDB also +talks to, such as gnomAD or ClinGen, keeps its link with that code. This module imports nothing from mavedb, so +models can use it. +""" + +import re + +import idutils + +# NCBI keeps RefSeq proteins (these prefixes) in its `protein` database and everything else in `nuccore`, its +# nucleotide database. A link to the wrong one finds nothing. +REFSEQ_PROTEIN_ACCESSION = re.compile(r"^(AP|NP|WP|XP|YP)_") + + +def doi_url(identifier: str) -> str: + """The resolver link for a DOI.""" + return f"https://doi.org/{identifier}" + + +def raw_read_url(identifier: str) -> str: + """The page for a raw read accession, chosen by its accession type (#445).""" + if idutils.is_bioproject(identifier): + return f"https://www.ncbi.nlm.nih.gov/bioproject/{identifier}" + if idutils.is_geo(identifier): + return f"https://www.ncbi.nlm.nih.gov/geo/query/acc.cgi?acc={identifier}" + if idutils.is_arrayexpress_experiment(identifier): + return f"https://www.ebi.ac.uk/biostudies/arrayexpress/studies/{identifier}" + if idutils.is_arrayexpress_array(identifier): + return f"https://www.ebi.ac.uk/biostudies/arrayexpress/arrays/{identifier}" + + # SRA accessions (SRR, ERR, DRR and their study and experiment forms) are shared across the INSDC archives, + # and ENA's browser resolves all of them. It's also where every raw read url pointed before #445, so + # accessions that match none of the types above keep working. + return f"https://www.ebi.ac.uk/ena/browser/view/{identifier}" + + +def external_gene_identifier_url(db_name: str, identifier: str) -> str: + """The page for a target gene's identifier in Ensembl, RefSeq or UniProt.""" + if db_name == "Ensembl": + return f"https://www.ensembl.org/id/{identifier}" + if db_name == "UniProt": + return f"https://www.uniprot.org/uniprotkb/{identifier}" + + database = "protein" if REFSEQ_PROTEIN_ACCESSION.match(identifier) else "nuccore" + return f"https://www.ncbi.nlm.nih.gov/{database}/{identifier}" diff --git a/src/mavedb/lib/identifier_search.py b/src/mavedb/lib/identifier_search.py new file mode 100644 index 000000000..4a91b1c83 --- /dev/null +++ b/src/mavedb/lib/identifier_search.py @@ -0,0 +1,103 @@ +"""Search for the external identifiers that records cite, limited to records the caller may read. + +This is kept apart from :mod:`mavedb.lib.identifiers` because it needs the permission checks, which import the +web framework, and view models import that module. +""" + +from typing import Any, Sequence, Union + +from sqlalchemy import Select, func, select +from sqlalchemy.orm import Session + +from mavedb.lib.permissions.experiment import ExperimentViewer +from mavedb.lib.permissions.experiment_set import ExperimentSetViewer +from mavedb.lib.permissions.principal import Principal +from mavedb.lib.permissions.score_set import ScoreSetViewer +from mavedb.lib.permissions.viewer import Viewer +from mavedb.models.doi_identifier import ExperimentDoiIdentifier, ExperimentSetDoiIdentifier, ScoreSetDoiIdentifier +from mavedb.models.experiment import Experiment +from mavedb.models.experiment_set import ExperimentSet +from mavedb.models.mixins.external_identifier import ExternalIdentifier +from mavedb.models.raw_read_identifier import ( + ExperimentRawReadIdentifier, + ExperimentSetRawReadIdentifier, + ScoreSetRawReadIdentifier, +) +from mavedb.models.score_set import ScoreSet + +IDENTIFIER_SEARCH_LIMIT = 50 + +# A table to search for identifiers: its link model, the viewer for the record that governs each row's +# visibility, and a query selecting ``(link row, record)``. +IdentifierSource = tuple[type[ExternalIdentifier], type[Viewer[Any]], Select] + + +def search_identifiers_on_readable_records( + db: Session, + principal: Principal, + text: str, + sources: Sequence[IdentifierSource], +) -> list[ExternalIdentifier]: + """Find identifiers containing ``text`` that are attached to records the caller may read. + + Identifiers are stored on the rows that link them to their records, so one identifier can appear on many + rows. This returns one row per distinct identifier, sorted by identifier. + + Args: + db: An active database session. + principal: The caller. + text: The text to search for, matched case-insensitively anywhere in the identifier. + sources: The tables to search. + + Returns: + Up to ``IDENTIFIER_SEARCH_LIMIT`` link rows, one per distinct identifier. + """ + # Read access to a record is decided in Python, so the filter runs before the limit. These tables hold a + # few hundred rows. Once row-level security enforces reads (#808), the database does this filtering. + found: dict[str, ExternalIdentifier] = {} + for link, viewer_class, query in sources: + viewer = principal.viewer_for(viewer_class) + rows = db.execute(query.where(func.lower(link.identifier).contains(text.lower(), autoescape=True))).all() + for link_row, record in rows: + if link_row.identifier in found: + continue + if viewer.may_read(record): + found[link_row.identifier] = link_row + + return [found[identifier] for identifier in sorted(found)][:IDENTIFIER_SEARCH_LIMIT] + + +def identifier_sources_on_records( + score_set_link: Union[type[ScoreSetDoiIdentifier], type[ScoreSetRawReadIdentifier]], + experiment_link: Union[type[ExperimentDoiIdentifier], type[ExperimentRawReadIdentifier]], + experiment_set_link: Union[type[ExperimentSetDoiIdentifier], type[ExperimentSetRawReadIdentifier]], +) -> list[IdentifierSource]: + """The sources for an identifier that score sets, experiments and experiment sets each cite. + + Args: + score_set_link: The model linking the identifier to score sets. + experiment_link: The model linking the identifier to experiments. + experiment_set_link: The model linking the identifier to experiment sets. + + Returns: + A source for each record type, for :func:`search_identifiers_on_readable_records`. + """ + return [ + ( + score_set_link, + ScoreSetViewer, + select(score_set_link, ScoreSet).join(ScoreSet, ScoreSet.id == score_set_link.score_set_id), + ), + ( + experiment_link, + ExperimentViewer, + select(experiment_link, Experiment).join(Experiment, Experiment.id == experiment_link.experiment_id), + ), + ( + experiment_set_link, + ExperimentSetViewer, + select(experiment_set_link, ExperimentSet).join( + ExperimentSet, ExperimentSet.id == experiment_set_link.experiment_set_id + ), + ), + ] diff --git a/src/mavedb/lib/identifiers.py b/src/mavedb/lib/identifiers.py index 7855b6af7..436ef8b36 100644 --- a/src/mavedb/lib/identifiers.py +++ b/src/mavedb/lib/identifiers.py @@ -1,7 +1,7 @@ import json import logging import os -from typing import Mapping, Optional, Union +from typing import Iterable, Mapping, Optional, Union import eutils # type: ignore import requests @@ -15,36 +15,12 @@ from mavedb.lib.exceptions import AmbiguousIdentifierError, NonexistentIdentifierError from mavedb.lib.external_publications import Crossref, CrossrefWork, PublicationAuthors, Rxiv, RxivContentDetail from mavedb.lib.validation.publication import identifier_valid_for, infer_identifier_from_url, validate_db_name -from mavedb.models.doi_identifier import DoiIdentifier -from mavedb.models.ensembl_identifier import EnsemblIdentifier -from mavedb.models.ensembl_offset import EnsemblOffset from mavedb.models.publication_identifier import PublicationIdentifier -from mavedb.models.raw_read_identifier import RawReadIdentifier -from mavedb.models.refseq_identifier import RefseqIdentifier -from mavedb.models.refseq_offset import RefseqOffset -from mavedb.models.target_gene import TargetGene -from mavedb.models.uniprot_identifier import UniprotIdentifier -from mavedb.models.uniprot_offset import UniprotOffset +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier +from mavedb.view_models.external_gene_identifier_offset import ExternalGeneIdentifierOffsetCreate logger = logging.getLogger(__name__) -# XXX these classes all have an "identifier" attribute but there's no superclass -# to unify them ... - -EXTERNAL_GENE_IDENTIFIER_CLASSES = { - "Ensembl": EnsemblIdentifier, - "RefSeq": RefseqIdentifier, - "UniProt": UniprotIdentifier, -} - -EXTERNAL_GENE_IDENTIFIER_OFFSET_CLASSES = {"Ensembl": EnsemblOffset, "RefSeq": RefseqOffset, "UniProt": UniprotOffset} - -EXTERNAL_GENE_IDENTIFIER_OFFSET_ATTRIBUTES = { - "Ensembl": "ensembl_offset", - "RefSeq": "refseq_offset", - "UniProt": "uniprot_offset", -} - class ExternalPublication: """ @@ -174,20 +150,6 @@ def reference_html(self) -> str: ) -async def find_or_create_doi_identifier(db: Session, identifier: str): - """ - Find an existing DOI identifier record with the specified identifier string, or create a new one. - - :param db: An active database session - :param identifier: A valid DOI identifier - :return: An existing DoiIdentifier containing the specified identifier string, or a new, unsaved DoiIdentifier - """ - doi_identifier = db.query(DoiIdentifier).filter(DoiIdentifier.identifier == identifier).one_or_none() - if not doi_identifier: - doi_identifier = DoiIdentifier(identifier=identifier, db_name="DOI", url=f"https://doi.org/{identifier}") - return doi_identifier - - async def fetch_pubmed_article(identifier: str) -> Optional[PubmedArticle]: """ Fetch an existing PubMed article from NCBI @@ -408,55 +370,33 @@ async def find_or_create_publication_identifier( return create_generic_article(matched_article) -async def find_or_create_raw_read_identifier(db: Session, identifier: str): - raw_read_identifier = db.query(RawReadIdentifier).filter(RawReadIdentifier.identifier == identifier).one_or_none() - if not raw_read_identifier: - raw_read_identifier = RawReadIdentifier( - identifier=identifier, db_name="SRA", url=f"http://www.ebi.ac.uk/ena/data/view/{identifier}" - ) - return raw_read_identifier - - -async def find_or_create_external_gene_identifier(db: Session, db_name: str, identifier: str): - """ - Find an existing gene identifier record with the specified gene database name and identifier string, or create a new - one. +def distinct_identifiers(identifiers: Iterable[str]) -> list[str]: + """The distinct identifier strings in a submitted list, in the order given. - :param db: An active database session - :param identifier: A valid identifier - :return: An existing EnsemblIdentifier, RefseqIdentifier, or UniprotIdentifier containing the specified identifier - string, or a new, unsaved instance of one of these classes + A record stores each identifier once, so a repeat in the submission would collide with itself. """ + return list(dict.fromkeys(identifiers)) - # TODO Handle key errors. - identifier_class = EXTERNAL_GENE_IDENTIFIER_CLASSES[db_name] - assert hasattr(identifier_class, "identifier") - external_gene_identifier = ( - db.query(identifier_class).filter(identifier_class.identifier == identifier).one_or_none() - ) +def target_gene_external_identifiers( + external_identifiers: Iterable[ExternalGeneIdentifierOffsetCreate], +) -> list[TargetGeneExternalIdentifier]: + """Rows for a target gene's submitted external identifiers. - if not external_gene_identifier: - external_gene_identifier = identifier_class( - identifier=identifier, + A target gene holds one identifier per database. When a submission names a database twice, the last one + wins, as it did when each database had its own offset table. + """ + by_db_name = { + external_identifier.identifier.db_name: external_identifier for external_identifier in external_identifiers + } + return [ + TargetGeneExternalIdentifier( db_name=db_name, - # TODO Set URL from identifier - # url=f'https://doi.org/{identifier}' + identifier=external_identifier.identifier.identifier, + offset=external_identifier.offset, ) - return external_gene_identifier - - -async def create_external_gene_identifier_offset( - db: Session, target_gene: TargetGene, db_name: str, identifier: str, offset: int -): - external_gene_identifier = await find_or_create_external_gene_identifier(db, db_name, identifier) - - # TODO Handle key errors. - offset_class = EXTERNAL_GENE_IDENTIFIER_OFFSET_CLASSES[external_gene_identifier.db_name] - external_gene_identifier_offset = offset_class(identifier=external_gene_identifier, offset=offset) - - identifier_offset_attribute = EXTERNAL_GENE_IDENTIFIER_OFFSET_ATTRIBUTES[external_gene_identifier.db_name] - setattr(target_gene, identifier_offset_attribute, external_gene_identifier_offset) + for db_name, external_identifier in by_db_name.items() + ] def sanitize_target_sequence_label(label: str): diff --git a/src/mavedb/lib/permissions/experiment.py b/src/mavedb/lib/permissions/experiment.py index 2c7e6bdbb..3ca8c8b2c 100644 --- a/src/mavedb/lib/permissions/experiment.py +++ b/src/mavedb/lib/permissions/experiment.py @@ -1,9 +1,11 @@ +from dataclasses import dataclass from typing import Optional 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.utils import deny_action_for_entity, roles_permitted +from mavedb.lib.permissions.viewer import Viewer from mavedb.lib.types.authentication import UserData from mavedb.models.enums.user_role import UserRole from mavedb.models.experiment import Experiment @@ -71,6 +73,15 @@ def has_permission(user_data: Optional[UserData], entity: Experiment, action: Ac ) +@dataclass(frozen=True) +class ExperimentViewer(Viewer[Experiment]): + """Which experiments a caller may read, for reads that reach an experiment through something else.""" + + @staticmethod + def _has_permission(user_data: Optional[UserData], entity: Experiment, action: Action) -> PermissionResponse: + return has_permission(user_data, entity, action) + + def _handle_read_action( user_data: Optional[UserData], entity: Experiment, diff --git a/src/mavedb/lib/permissions/experiment_set.py b/src/mavedb/lib/permissions/experiment_set.py index d8cbecc3b..a0b643ed7 100644 --- a/src/mavedb/lib/permissions/experiment_set.py +++ b/src/mavedb/lib/permissions/experiment_set.py @@ -1,9 +1,11 @@ +from dataclasses import dataclass from typing import Optional 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.utils import deny_action_for_entity, roles_permitted +from mavedb.lib.permissions.viewer import Viewer from mavedb.lib.types.authentication import UserData from mavedb.models.enums.user_role import UserRole from mavedb.models.experiment_set import ExperimentSet @@ -72,6 +74,15 @@ def has_permission(user_data: Optional[UserData], entity: ExperimentSet, action: ) +@dataclass(frozen=True) +class ExperimentSetViewer(Viewer[ExperimentSet]): + """Which experiment sets a caller may read, for reads that reach an experiment set through something else.""" + + @staticmethod + def _has_permission(user_data: Optional[UserData], entity: ExperimentSet, action: Action) -> PermissionResponse: + return has_permission(user_data, entity, action) + + def _handle_read_action( user_data: Optional[UserData], entity: ExperimentSet, diff --git a/src/mavedb/lib/permissions/score_set.py b/src/mavedb/lib/permissions/score_set.py index e54b6b87f..38b32ca8d 100644 --- a/src/mavedb/lib/permissions/score_set.py +++ b/src/mavedb/lib/permissions/score_set.py @@ -1,9 +1,11 @@ +from dataclasses import dataclass from typing import Optional 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.utils import deny_action_for_entity, roles_permitted +from mavedb.lib.permissions.viewer import Viewer from mavedb.lib.types.authentication import UserData from mavedb.models.enums.user_role import UserRole from mavedb.models.score_set import ScoreSet @@ -73,6 +75,15 @@ def has_permission(user_data: Optional[UserData], entity: ScoreSet, action: Acti ) +@dataclass(frozen=True) +class ScoreSetViewer(Viewer[ScoreSet]): + """Which score sets a caller may read, for reads that reach a score set through something else.""" + + @staticmethod + def _has_permission(user_data: Optional[UserData], entity: ScoreSet, action: Action) -> PermissionResponse: + return has_permission(user_data, entity, action) + + def _handle_read_action( user_data: Optional[UserData], entity: ScoreSet, diff --git a/src/mavedb/lib/score_sets.py b/src/mavedb/lib/score_sets.py index 64b6dd034..e43e3a7d2 100644 --- a/src/mavedb/lib/score_sets.py +++ b/src/mavedb/lib/score_sets.py @@ -27,9 +27,7 @@ from mavedb.models.allele import Allele from mavedb.models.contributor import Contributor from mavedb.models.controlled_keyword import ControlledKeyword -from mavedb.models.doi_identifier import DoiIdentifier -from mavedb.models.ensembl_identifier import EnsemblIdentifier -from mavedb.models.ensembl_offset import EnsemblOffset +from mavedb.models.doi_identifier import ScoreSetDoiIdentifier from mavedb.models.experiment import Experiment from mavedb.models.experiment_controlled_keyword import ExperimentControlledKeywordAssociation from mavedb.models.experiment_publication_identifier import ExperimentPublicationIdentifierAssociation @@ -37,18 +35,15 @@ from mavedb.models.mapping_record import MappingRecord from mavedb.models.mapping_record_allele import MappingRecordAllele from mavedb.models.publication_identifier import PublicationIdentifier -from mavedb.models.refseq_identifier import RefseqIdentifier -from mavedb.models.refseq_offset import RefseqOffset from mavedb.models.score_set import ScoreSet from mavedb.models.score_set_publication_identifier import ( ScoreSetPublicationIdentifierAssociation, ) from mavedb.models.target_accession import TargetAccession from mavedb.models.target_gene import TargetGene +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.models.target_sequence import TargetSequence from mavedb.models.taxonomy import Taxonomy -from mavedb.models.uniprot_identifier import UniprotIdentifier -from mavedb.models.uniprot_offset import UniprotOffset from mavedb.models.user import User from mavedb.models.variant import Variant from mavedb.view_models.search import ControlledKeywordFilterOption, ScoreSetsSearch @@ -141,26 +136,10 @@ def build_search_score_sets_query_filter( f"""$[*].name ? (@ like_regex "{lower_search_text}" flag "i")""", ) ), - ScoreSet.doi_identifiers.any(func.lower(DoiIdentifier.identifier).icontains(lower_search_text)), + ScoreSet.doi_identifiers.any(func.lower(ScoreSetDoiIdentifier.identifier).icontains(lower_search_text)), ScoreSet.target_genes.any( - TargetGene.uniprot_offset.has( - UniprotOffset.identifier.has( - func.lower(UniprotIdentifier.identifier).icontains(lower_search_text) - ) - ) - ), - ScoreSet.target_genes.any( - TargetGene.refseq_offset.has( - RefseqOffset.identifier.has( - func.lower(RefseqIdentifier.identifier).icontains(lower_search_text) - ) - ) - ), - ScoreSet.target_genes.any( - TargetGene.ensembl_offset.has( - EnsemblOffset.identifier.has( - func.lower(EnsemblIdentifier.identifier).icontains(lower_search_text) - ) + TargetGene.external_identifiers.any( + func.lower(TargetGeneExternalIdentifier.identifier).icontains(lower_search_text) ) ), ) @@ -262,9 +241,7 @@ def search_score_sets(db: Session, owner_or_contributor: Optional[User], search: ScoreSetPublicationIdentifierAssociation.publication ), joinedload(ScoreSet.target_genes).options( - joinedload(TargetGene.ensembl_offset).joinedload(EnsemblOffset.identifier), - joinedload(TargetGene.refseq_offset).joinedload(RefseqOffset.identifier), - joinedload(TargetGene.uniprot_offset).joinedload(UniprotOffset.identifier), + selectinload(TargetGene.external_identifiers), joinedload(TargetGene.target_sequence).joinedload(TargetSequence.taxonomy), joinedload(TargetGene.target_accession), ), @@ -276,9 +253,7 @@ def search_score_sets(db: Session, owner_or_contributor: Optional[User], search: ScoreSetPublicationIdentifierAssociation.publication ), selectinload(ScoreSet.target_genes).options( - joinedload(TargetGene.ensembl_offset).joinedload(EnsemblOffset.identifier), - joinedload(TargetGene.refseq_offset).joinedload(RefseqOffset.identifier), - joinedload(TargetGene.uniprot_offset).joinedload(UniprotOffset.identifier), + selectinload(TargetGene.external_identifiers), joinedload(TargetGene.target_sequence).joinedload(TargetSequence.taxonomy), joinedload(TargetGene.target_accession), ), diff --git a/src/mavedb/lib/validation/transform.py b/src/mavedb/lib/validation/transform.py index 32c2dff12..c7f4b5940 100644 --- a/src/mavedb/lib/validation/transform.py +++ b/src/mavedb/lib/validation/transform.py @@ -18,9 +18,7 @@ from mavedb.models.score_calibration_publication_identifier import ScoreCalibrationPublicationIdentifierAssociation from mavedb.models.score_set import ScoreSet from mavedb.models.score_set_publication_identifier import ScoreSetPublicationIdentifierAssociation -from mavedb.models.target_gene import TargetGene from mavedb.models.user import User -from mavedb.view_models.external_gene_identifier_offset import ExternalGeneIdentifierOffset from mavedb.view_models.publication_identifier import PublicationIdentifier @@ -133,13 +131,6 @@ def transform_score_calibration_publication_identifiers( return transformed_publication_identifiers -def transform_external_identifier_offsets_to_list(data: TargetGene) -> list[ExternalGeneIdentifierOffset]: - ensembl_offset = data.ensembl_offset - refseq_offset = data.refseq_offset - uniprot_offset = data.uniprot_offset - return list(filter(lambda x: x is not None, [ensembl_offset, refseq_offset, uniprot_offset])) # type:ignore - - # We're unable to create a dynamically typed TypedDict which references enum keys and enforces totality of the enum, so this # is simply typed as dict[str, User] to avoid coupling and the need to rewrite such a TypedDict any time the enum changes. # See https://stackoverflow.com/a/72083153 for details about why this isn't possible. diff --git a/src/mavedb/models/__init__.py b/src/mavedb/models/__init__.py index 534ff0ec5..c9c187376 100644 --- a/src/mavedb/models/__init__.py +++ b/src/mavedb/models/__init__.py @@ -8,8 +8,6 @@ "clinvar_allele_link", "controlled_keyword", "doi_identifier", - "ensembl_identifier", - "ensembl_offset", "experiment", "experiment_set", "genome_identifier", @@ -29,8 +27,6 @@ "published_variant", "variant_annotation_view", "raw_read_identifier", - "refseq_identifier", - "refseq_offset", "role", "score_calibration_functional_classification_variant_association", "score_calibration_functional_classification", @@ -38,11 +34,10 @@ "score_calibration", "score_set", "target_gene", + "target_gene_external_identifier", "target_gene_mapping", "target_sequence", "taxonomy", - "uniprot_identifier", - "uniprot_offset", "urn_redirect", "user", "variant_annotation_status", diff --git a/src/mavedb/models/clinvar_allele_link.py b/src/mavedb/models/clinvar_allele_link.py index 941ee2f22..97c859e13 100644 --- a/src/mavedb/models/clinvar_allele_link.py +++ b/src/mavedb/models/clinvar_allele_link.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime if TYPE_CHECKING: from .allele import Allele diff --git a/src/mavedb/models/doi_identifier.py b/src/mavedb/models/doi_identifier.py index 9cf755870..0b27f9174 100644 --- a/src/mavedb/models/doi_identifier.py +++ b/src/mavedb/models/doi_identifier.py @@ -1,17 +1,37 @@ -from datetime import date - -from sqlalchemy import Column, Date, Integer, String +from sqlalchemy import Column, ForeignKey, Integer, String from mavedb.db.base import Base +from mavedb.lib.external_identifier_urls import doi_url +from mavedb.models.mixins.external_identifier import ExternalIdentifier + + +class DoiIdentifierLink(ExternalIdentifier): + """A DOI cited by one record. DOIs are case-insensitive, so the identifier is stored upper-cased, as the DOI + registry stores it.""" + + db_name = "DOI" + + @property + def url(self) -> str: + return doi_url(self.identifier) + + +class ScoreSetDoiIdentifier(DoiIdentifierLink, Base): + __tablename__ = "scoreset_doi_identifiers" + + score_set_id = Column("scoreset_id", Integer, ForeignKey("scoresets.id"), primary_key=True) + identifier = Column(String, primary_key=True) + + +class ExperimentDoiIdentifier(DoiIdentifierLink, Base): + __tablename__ = "experiment_doi_identifiers" + + experiment_id = Column(Integer, ForeignKey("experiments.id"), primary_key=True) + identifier = Column(String, primary_key=True) -class DoiIdentifier(Base): - __tablename__ = "doi_identifiers" +class ExperimentSetDoiIdentifier(DoiIdentifierLink, Base): + __tablename__ = "experiment_set_doi_identifiers" - id = Column(Integer, primary_key=True) - identifier = Column(String, nullable=False) - db_name = Column(String, nullable=False) - db_version = Column(String, nullable=True) - url = Column(String, nullable=True) - creation_date = Column(Date, nullable=False, default=date.today) - modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) + experiment_set_id = Column(Integer, ForeignKey("experiment_sets.id"), primary_key=True) + identifier = Column(String, primary_key=True) diff --git a/src/mavedb/models/ensembl_identifier.py b/src/mavedb/models/ensembl_identifier.py deleted file mode 100644 index 95189b076..000000000 --- a/src/mavedb/models/ensembl_identifier.py +++ /dev/null @@ -1,17 +0,0 @@ -from datetime import date - -from sqlalchemy import Column, Date, Integer, String - -from mavedb.db.base import Base - - -class EnsemblIdentifier(Base): - __tablename__ = "ensembl_identifiers" - - id = Column(Integer, primary_key=True) - identifier = Column(String, nullable=False) - db_name = Column(String, nullable=False) - db_version = Column(String, nullable=True) - url = Column(String, nullable=True) - creation_date = Column(Date, nullable=False, default=date.today) - modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) diff --git a/src/mavedb/models/ensembl_offset.py b/src/mavedb/models/ensembl_offset.py deleted file mode 100644 index 28bb09a48..000000000 --- a/src/mavedb/models/ensembl_offset.py +++ /dev/null @@ -1,19 +0,0 @@ -from sqlalchemy import Column, ForeignKey, Integer -from sqlalchemy.orm import Mapped, backref, relationship - -from mavedb.db.base import Base -from mavedb.models.ensembl_identifier import EnsemblIdentifier -from mavedb.models.target_gene import TargetGene - - -class EnsemblOffset(Base): - __tablename__ = "ensembl_offsets" - - identifier_id = Column(Integer, ForeignKey("ensembl_identifiers.id"), nullable=False, primary_key=True) - identifier: Mapped[EnsemblIdentifier] = relationship( - "EnsemblIdentifier", backref=backref("target_gene_offsets", uselist=True) - ) - target_gene_id = Column(Integer, ForeignKey("target_genes.id"), nullable=False, primary_key=True) - target_gene: Mapped[TargetGene] = relationship(back_populates="ensembl_offset") - - offset = Column(Integer, nullable=False) diff --git a/src/mavedb/models/experiment.py b/src/mavedb/models/experiment.py index eb8bb2cf2..48ddebf38 100644 --- a/src/mavedb/models/experiment.py +++ b/src/mavedb/models/experiment.py @@ -12,7 +12,7 @@ from mavedb.lib.temp_urns import generate_temp_urn from mavedb.models.contributor import Contributor from mavedb.models.controlled_keyword import ControlledKeyword -from mavedb.models.doi_identifier import DoiIdentifier +from mavedb.models.doi_identifier import ExperimentDoiIdentifier from mavedb.models.experiment_controlled_keyword import ( ExperimentControlledKeywordAssociation, ) @@ -22,7 +22,7 @@ from mavedb.models.experiment_set import ExperimentSet from mavedb.models.legacy_keyword import LegacyKeyword from mavedb.models.publication_identifier import PublicationIdentifier -from mavedb.models.raw_read_identifier import RawReadIdentifier +from mavedb.models.raw_read_identifier import ExperimentRawReadIdentifier from mavedb.models.user import User if TYPE_CHECKING: @@ -36,13 +36,6 @@ Column("contributor_id", ForeignKey("contributors.id"), primary_key=True), ) -experiments_doi_identifiers_association_table = Table( - "experiment_doi_identifiers", - Base.metadata, - Column("experiment_id", ForeignKey("experiments.id"), primary_key=True), - Column("doi_identifier_id", ForeignKey("doi_identifiers.id"), primary_key=True), -) - experiments_legacy_keywords_association_table = Table( "experiment_keywords", Base.metadata, @@ -50,14 +43,6 @@ Column("keyword_id", ForeignKey("keywords.id"), primary_key=True), ) -# experiments_sra_identifiers_association_table = Table( -experiments_raw_read_identifiers_association_table = Table( - "experiment_sra_identifiers", - Base.metadata, - Column("experiment_id", ForeignKey("experiments.id"), primary_key=True), - Column("sra_identifier_id", ForeignKey("sra_identifiers.id"), primary_key=True), -) - class Experiment(Base): __tablename__ = "experiments" @@ -117,10 +102,8 @@ class Experiment(Base): secondary=experiments_legacy_keywords_association_table, backref="experiments", ) - doi_identifiers: Mapped[list[DoiIdentifier]] = relationship( - "DoiIdentifier", - secondary=experiments_doi_identifiers_association_table, - backref="experiments", + doi_identifiers: Mapped[list[ExperimentDoiIdentifier]] = relationship( + cascade="all, delete-orphan", order_by=ExperimentDoiIdentifier.identifier ) publication_identifier_associations: Mapped[list[ExperimentPublicationIdentifierAssociation]] = relationship( "ExperimentPublicationIdentifierAssociation", @@ -133,11 +116,8 @@ class Experiment(Base): creator=lambda p: ExperimentPublicationIdentifierAssociation(publication=p, primary=p.primary), ) - # sra_identifiers = relationship('SraIdentifier', secondary=experiments_sra_identifiers_association_table, backref='experiments') - raw_read_identifiers: Mapped[list[RawReadIdentifier]] = relationship( - "RawReadIdentifier", - secondary=experiments_raw_read_identifiers_association_table, - backref="experiments", + raw_read_identifiers: Mapped[list[ExperimentRawReadIdentifier]] = relationship( + cascade="all, delete-orphan", order_by=ExperimentRawReadIdentifier.identifier ) # Unfortunately, we can't use association_proxy here, because in spite of what the documentation seems to imply, it diff --git a/src/mavedb/models/experiment_set.py b/src/mavedb/models/experiment_set.py index a413cf04c..f540d7c21 100644 --- a/src/mavedb/models/experiment_set.py +++ b/src/mavedb/models/experiment_set.py @@ -10,10 +10,10 @@ from mavedb.lib.temp_urns import generate_temp_urn from .contributor import Contributor -from .doi_identifier import DoiIdentifier +from .doi_identifier import ExperimentSetDoiIdentifier from .legacy_keyword import LegacyKeyword from .publication_identifier import PublicationIdentifier -from .raw_read_identifier import RawReadIdentifier +from .raw_read_identifier import ExperimentSetRawReadIdentifier from .user import User if TYPE_CHECKING: @@ -27,13 +27,6 @@ ) -experiment_sets_doi_identifiers_association_table = Table( - "experiment_set_doi_identifiers", - Base.metadata, - Column("experiment_set_id", ForeignKey("experiment_sets.id"), primary_key=True), - Column("doi_identifier_id", ForeignKey("doi_identifiers.id"), primary_key=True), -) - experiment_sets_legacy_keywords_association_table = Table( "experiment_set_keywords", Base.metadata, @@ -48,14 +41,6 @@ Column("publication_identifier_id", ForeignKey("publication_identifiers.id"), primary_key=True), ) -# experiment_sets_sra_identifiers_association_table = Table( -experiment_sets_raw_read_identifiers_association_table = Table( - "experiment_set_sra_identifiers", - Base.metadata, - Column("experiment_set_id", ForeignKey("experiment_sets.id"), primary_key=True), - Column("sra_identifier_id", ForeignKey("sra_identifiers.id"), primary_key=True), -) - class ExperimentSet(Base): __tablename__ = "experiment_sets" @@ -89,15 +74,14 @@ class ExperimentSet(Base): contributors: Mapped[list["Contributor"]] = relationship( "Contributor", secondary=experiment_sets_contributors_association_table, backref="experiment_sets" ) - doi_identifiers: Mapped[list[DoiIdentifier]] = relationship( - "DoiIdentifier", secondary=experiment_sets_doi_identifiers_association_table, backref="experiment_sets" + doi_identifiers: Mapped[list[ExperimentSetDoiIdentifier]] = relationship( + cascade="all, delete-orphan", order_by=ExperimentSetDoiIdentifier.identifier ) publication_identifiers: Mapped[list[PublicationIdentifier]] = relationship( "PublicationIdentifier", secondary=experiment_sets_publication_identifiers_association_table, backref="experiment_sets", ) - # sra_identifiers = relationship('SraIdentifier', secondary=experiment_sets_sra_identifiers_association_table, backref='experiment_sets') - raw_read_identifiers: Mapped[list[RawReadIdentifier]] = relationship( - "RawReadIdentifier", secondary=experiment_sets_raw_read_identifiers_association_table, backref="experiment_sets" + raw_read_identifiers: Mapped[list[ExperimentSetRawReadIdentifier]] = relationship( + cascade="all, delete-orphan", order_by=ExperimentSetRawReadIdentifier.identifier ) diff --git a/src/mavedb/models/gnomad_allele_link.py b/src/mavedb/models/gnomad_allele_link.py index 75294ba66..12f01c51f 100644 --- a/src/mavedb/models/gnomad_allele_link.py +++ b/src/mavedb/models/gnomad_allele_link.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime if TYPE_CHECKING: from .allele import Allele diff --git a/src/mavedb/models/mapping_record.py b/src/mavedb/models/mapping_record.py index d0838ab27..1fa3cf7ea 100644 --- a/src/mavedb/models/mapping_record.py +++ b/src/mavedb/models/mapping_record.py @@ -7,7 +7,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime from mavedb.lib.hgvs import extract_accession from mavedb.models.enums.sequence_level import SequenceLevel diff --git a/src/mavedb/models/mapping_record_allele.py b/src/mavedb/models/mapping_record_allele.py index b19823067..b5c890246 100644 --- a/src/mavedb/models/mapping_record_allele.py +++ b/src/mavedb/models/mapping_record_allele.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime if TYPE_CHECKING: from .allele import Allele diff --git a/src/mavedb/models/mixins/__init__.py b/src/mavedb/models/mixins/__init__.py new file mode 100644 index 000000000..c9c215b88 --- /dev/null +++ b/src/mavedb/models/mixins/__init__.py @@ -0,0 +1,3 @@ +""" +Mixins shared by models. +""" diff --git a/src/mavedb/models/mixins/external_identifier.py b/src/mavedb/models/mixins/external_identifier.py new file mode 100644 index 000000000..2f67c2edb --- /dev/null +++ b/src/mavedb/models/mixins/external_identifier.py @@ -0,0 +1,14 @@ +class ExternalIdentifier: + """An identifier in an external database that a record cites, and where it resolves. + + Subclasses store ``identifier`` on the citing record's row, supply ``db_name`` as a constant or a column, and + compute ``url`` from them. Annotation sources (ClinVar, gnomAD) aren't citations: their rows hold fetched, + versioned data keyed by ``db_identifier``, so they don't use this. + """ + + identifier: str + db_name: str + + @property + def url(self) -> str: + raise NotImplementedError diff --git a/src/mavedb/db/mixins.py b/src/mavedb/models/mixins/valid_time.py similarity index 99% rename from src/mavedb/db/mixins.py rename to src/mavedb/models/mixins/valid_time.py index c9b17f4b3..578090400 100644 --- a/src/mavedb/db/mixins.py +++ b/src/mavedb/models/mixins/valid_time.py @@ -1,4 +1,4 @@ -"""Reusable declarative mixins.""" +"""Valid-time versioning for rows that change over time.""" from datetime import datetime from typing import ClassVar, Optional, Sequence, TypeVar diff --git a/src/mavedb/models/projection_group.py b/src/mavedb/models/projection_group.py index 89e2a93cf..9abf42280 100644 --- a/src/mavedb/models/projection_group.py +++ b/src/mavedb/models/projection_group.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime if TYPE_CHECKING: from .mapping_record import MappingRecord diff --git a/src/mavedb/models/raw_read_identifier.py b/src/mavedb/models/raw_read_identifier.py index 3bc2a776f..9303fac87 100644 --- a/src/mavedb/models/raw_read_identifier.py +++ b/src/mavedb/models/raw_read_identifier.py @@ -1,17 +1,36 @@ -from datetime import date - -from sqlalchemy import Column, Date, Integer, String +from sqlalchemy import Column, ForeignKey, Integer, String from mavedb.db.base import Base +from mavedb.lib.external_identifier_urls import raw_read_url +from mavedb.models.mixins.external_identifier import ExternalIdentifier + + +class RawReadIdentifierLink(ExternalIdentifier): + """A raw read accession (SRA, BioProject, GEO or ArrayExpress) cited by one record.""" + + db_name = "SRA" + + @property + def url(self) -> str: + return raw_read_url(self.identifier) + + +class ScoreSetRawReadIdentifier(RawReadIdentifierLink, Base): + __tablename__ = "scoreset_sra_identifiers" + + score_set_id = Column("scoreset_id", Integer, ForeignKey("scoresets.id"), primary_key=True) + identifier = Column(String, primary_key=True) + + +class ExperimentRawReadIdentifier(RawReadIdentifierLink, Base): + __tablename__ = "experiment_sra_identifiers" + + experiment_id = Column(Integer, ForeignKey("experiments.id"), primary_key=True) + identifier = Column(String, primary_key=True) -class RawReadIdentifier(Base): - __tablename__ = "sra_identifiers" +class ExperimentSetRawReadIdentifier(RawReadIdentifierLink, Base): + __tablename__ = "experiment_set_sra_identifiers" - id = Column(Integer, primary_key=True) - identifier = Column(String, nullable=False) - db_name = Column(String, nullable=False) - db_version = Column(String, nullable=True) - url = Column(String, nullable=True) - creation_date = Column(Date, nullable=False, default=date.today) - modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) + experiment_set_id = Column(Integer, ForeignKey("experiment_sets.id"), primary_key=True) + identifier = Column(String, primary_key=True) diff --git a/src/mavedb/models/refseq_identifier.py b/src/mavedb/models/refseq_identifier.py deleted file mode 100644 index e01d14c08..000000000 --- a/src/mavedb/models/refseq_identifier.py +++ /dev/null @@ -1,18 +0,0 @@ -from datetime import date - -from sqlalchemy import Column, Date, Integer, String - -from mavedb.db.base import Base - - -class RefseqIdentifier(Base): - __tablename__ = "refseq_identifiers" - - id = Column(Integer, primary_key=True) - identifier = Column(String, nullable=False) - db_name = Column(String, nullable=False) - db_version = Column(String, nullable=True) - url = Column(String, nullable=True) - reference_html = Column(String, nullable=True) - creation_date = Column(Date, nullable=False, default=date.today) - modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) diff --git a/src/mavedb/models/refseq_offset.py b/src/mavedb/models/refseq_offset.py deleted file mode 100644 index ba26b3594..000000000 --- a/src/mavedb/models/refseq_offset.py +++ /dev/null @@ -1,16 +0,0 @@ -from sqlalchemy import Column, ForeignKey, Integer -from sqlalchemy.orm import Mapped, backref, relationship - -from mavedb.db.base import Base -from mavedb.models.refseq_identifier import RefseqIdentifier -from mavedb.models.target_gene import TargetGene - - -class RefseqOffset(Base): - __tablename__ = "refseq_offsets" - - identifier_id = Column(Integer, ForeignKey("refseq_identifiers.id"), nullable=False, primary_key=True) - identifier: Mapped[RefseqIdentifier] = relationship(backref=backref("target_gene_offsets", uselist=True)) - target_gene_id = Column(Integer, ForeignKey("target_genes.id"), nullable=False, primary_key=True) - target_gene: Mapped[TargetGene] = relationship(back_populates="refseq_offset", single_parent=True) - offset = Column(Integer, nullable=False) diff --git a/src/mavedb/models/score_set.py b/src/mavedb/models/score_set.py index 1c71bf6b5..d4b8a1b91 100644 --- a/src/mavedb/models/score_set.py +++ b/src/mavedb/models/score_set.py @@ -10,7 +10,7 @@ import mavedb.models.score_set_publication_identifier from mavedb.db.base import Base from mavedb.models.contributor import Contributor -from mavedb.models.doi_identifier import DoiIdentifier +from mavedb.models.doi_identifier import ScoreSetDoiIdentifier from mavedb.models.enums.mapping_state import MappingState from mavedb.models.enums.processing_state import ProcessingState from mavedb.models.experiment import Experiment @@ -25,7 +25,6 @@ from mavedb.models.target_gene import TargetGene from mavedb.models.variant import Variant -# from .raw_read_identifier import SraIdentifier from mavedb.lib.temp_urns import generate_temp_urn # TODO Reformat code without removing dependencies whose use is not detected. @@ -38,14 +37,6 @@ ) -score_sets_doi_identifiers_association_table = Table( - "scoreset_doi_identifiers", - Base.metadata, - Column("scoreset_id", ForeignKey("scoresets.id"), primary_key=True), - Column("doi_identifier_id", ForeignKey("doi_identifiers.id"), primary_key=True), -) - - score_sets_legacy_keywords_association_table = Table( "scoreset_keywords", Base.metadata, @@ -62,15 +53,6 @@ ) -# score_sets_sra_identifiers_association_table = Table( -score_sets_raw_read_identifiers_association_table = Table( - "scoreset_sra_identifiers", - Base.metadata, - Column("scoreset_id", ForeignKey("scoresets.id"), primary_key=True), - Column("sra_identifier_id", ForeignKey("sra_identifiers.id"), primary_key=True), -) - - class ScoreSet(Base): __tablename__ = "scoresets" @@ -161,10 +143,8 @@ class ScoreSet(Base): secondary=score_sets_contributors_association_table, backref="score_sets", ) - doi_identifiers: Mapped[list["DoiIdentifier"]] = relationship( - "DoiIdentifier", - secondary=score_sets_doi_identifiers_association_table, - backref="score_sets", + doi_identifiers: Mapped[list[ScoreSetDoiIdentifier]] = relationship( + cascade="all, delete-orphan", order_by=ScoreSetDoiIdentifier.identifier ) publication_identifier_associations: Mapped[ list[mavedb.models.score_set_publication_identifier.ScoreSetPublicationIdentifierAssociation] @@ -180,8 +160,6 @@ class ScoreSet(Base): publication=p, primary=p.primary ), ) - # sra_identifiers = relationship('SraIdentifier', secondary=score_sets_sra_identifiers_association_table, backref='score_sets') - # raw_read_identifiers = relationship('RawReadIdentifier', secondary=score_sets_raw_read_identifiers_association_table,backref='score_sets') meta_analyzes_score_sets: Mapped[list["ScoreSet"]] = relationship( "ScoreSet", secondary=score_sets_meta_analysis_score_sets_association_table, diff --git a/src/mavedb/models/target_gene.py b/src/mavedb/models/target_gene.py index 5419c3478..36c399f95 100644 --- a/src/mavedb/models/target_gene.py +++ b/src/mavedb/models/target_gene.py @@ -12,10 +12,8 @@ from mavedb.models.target_sequence import TargetSequence if TYPE_CHECKING: - from mavedb.models.ensembl_offset import EnsemblOffset - from mavedb.models.refseq_offset import RefseqOffset + from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.models.target_gene_mapping import TargetGeneMapping - from mavedb.models.uniprot_offset import UniprotOffset # TODO Reformat code without removing dependencies whose use is not detected. @@ -54,9 +52,11 @@ class TargetGene(Base): creation_date = Column(Date, nullable=False, default=date.today) modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) - ensembl_offset: Mapped["EnsemblOffset"] = relationship(back_populates="target_gene", cascade="all, delete-orphan") - refseq_offset: Mapped["RefseqOffset"] = relationship(back_populates="target_gene", cascade="all, delete-orphan") - uniprot_offset: Mapped["UniprotOffset"] = relationship(back_populates="target_gene", cascade="all, delete-orphan") + external_identifiers: Mapped[list["TargetGeneExternalIdentifier"]] = relationship( + back_populates="target_gene", + cascade="all, delete-orphan", + order_by="TargetGeneExternalIdentifier.db_name", + ) target_gene_mappings: Mapped[list["TargetGeneMapping"]] = relationship( "TargetGeneMapping", diff --git a/src/mavedb/models/target_gene_external_identifier.py b/src/mavedb/models/target_gene_external_identifier.py new file mode 100644 index 000000000..5f0c36495 --- /dev/null +++ b/src/mavedb/models/target_gene_external_identifier.py @@ -0,0 +1,37 @@ +from typing import TYPE_CHECKING + +from sqlalchemy import CheckConstraint, Column, ForeignKey, Integer, String +from sqlalchemy.orm import Mapped, relationship + +from mavedb.db.base import Base +from mavedb.lib.external_identifier_urls import external_gene_identifier_url +from mavedb.lib.validation.constants.identifier import valid_dbnames +from mavedb.models.mixins.external_identifier import ExternalIdentifier + +if TYPE_CHECKING: + from mavedb.models.target_gene import TargetGene + + +class TargetGeneExternalIdentifier(ExternalIdentifier, Base): + """A target gene's identifier in one external database, and the offset of the target within it. + + A target gene has at most one identifier per database. + """ + + __tablename__ = "target_gene_external_identifiers" + __table_args__ = ( + CheckConstraint( + "db_name IN ({})".format(", ".join(f"'{db_name}'" for db_name in valid_dbnames)), + name="ck_target_gene_external_identifiers_db_name_valid", + ), + ) + + target_gene_id = Column(Integer, ForeignKey("target_genes.id", ondelete="CASCADE"), primary_key=True) + target_gene: Mapped["TargetGene"] = relationship(back_populates="external_identifiers") + db_name: Mapped[str] = Column(String, primary_key=True) + identifier: Mapped[str] = Column(String, nullable=False) + offset: Mapped[int] = Column(Integer, nullable=False) + + @property + def url(self) -> str: + return external_gene_identifier_url(self.db_name, self.identifier) diff --git a/src/mavedb/models/uniprot_identifier.py b/src/mavedb/models/uniprot_identifier.py deleted file mode 100644 index f44f334e0..000000000 --- a/src/mavedb/models/uniprot_identifier.py +++ /dev/null @@ -1,18 +0,0 @@ -from datetime import date - -from sqlalchemy import Column, Date, Integer, String - -from mavedb.db.base import Base - - -class UniprotIdentifier(Base): - __tablename__ = "uniprot_identifiers" - - id = Column(Integer, primary_key=True) - identifier = Column(String, nullable=False) - db_name = Column(String, nullable=False) - db_version = Column(String, nullable=True) - url = Column(String, nullable=True) - reference_html = Column(String, nullable=True) - creation_date = Column(Date, nullable=False, default=date.today) - modification_date = Column(Date, nullable=False, default=date.today, onupdate=date.today) diff --git a/src/mavedb/models/uniprot_offset.py b/src/mavedb/models/uniprot_offset.py deleted file mode 100644 index d97fdb2ca..000000000 --- a/src/mavedb/models/uniprot_offset.py +++ /dev/null @@ -1,19 +0,0 @@ -from sqlalchemy import Column, ForeignKey, Integer -from sqlalchemy.orm import Mapped, backref, relationship - -from mavedb.db.base import Base -from mavedb.models.target_gene import TargetGene -from mavedb.models.uniprot_identifier import UniprotIdentifier - - -class UniprotOffset(Base): - __tablename__ = "uniprot_offsets" - - identifier_id = Column(Integer, ForeignKey("uniprot_identifiers.id"), nullable=False, primary_key=True) - identifier: Mapped[UniprotIdentifier] = relationship( - "UniprotIdentifier", backref=backref("target_gene_offsets", uselist=True) - ) - target_gene_id = Column(Integer, ForeignKey("target_genes.id"), nullable=False, primary_key=True) - target_gene: Mapped[TargetGene] = relationship(back_populates="uniprot_offset", single_parent=True) - - offset = Column(Integer, nullable=False) diff --git a/src/mavedb/models/vep_allele_consequence.py b/src/mavedb/models/vep_allele_consequence.py index 8d6eaeacb..aac1aa0fe 100644 --- a/src/mavedb/models/vep_allele_consequence.py +++ b/src/mavedb/models/vep_allele_consequence.py @@ -6,7 +6,7 @@ from sqlalchemy.orm import Mapped, relationship from mavedb.db.base import Base -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime from mavedb.models.enums.vep import VepConsequenceSource if TYPE_CHECKING: diff --git a/src/mavedb/routers/doi_identifiers.py b/src/mavedb/routers/doi_identifiers.py index a13bb804e..762df7bae 100644 --- a/src/mavedb/routers/doi_identifiers.py +++ b/src/mavedb/routers/doi_identifiers.py @@ -1,11 +1,13 @@ from typing import Any, List from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import func from sqlalchemy.orm import Session from mavedb import deps -from mavedb.models.doi_identifier import DoiIdentifier +from mavedb.lib.authorization import get_principal +from mavedb.lib.identifier_search import identifier_sources_on_records, search_identifiers_on_readable_records +from mavedb.lib.permissions.principal import Principal +from mavedb.models.doi_identifier import ExperimentDoiIdentifier, ExperimentSetDoiIdentifier, ScoreSetDoiIdentifier from mavedb.routers.shared import BASE_400_RESPONSE, PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX from mavedb.view_models import doi_identifier from mavedb.view_models.search import TextSearch @@ -31,20 +33,20 @@ responses={**BASE_400_RESPONSE}, summary="Search DOI identifiers", ) -def search_doi_identifiers(search: TextSearch, db: Session = Depends(deps.get_db)) -> Any: +def search_doi_identifiers( + search: TextSearch, + db: Session = Depends(deps.get_db), + principal: Principal = Depends(get_principal), +) -> Any: """ - Search DOI identifiers based on the provided text. + Search the DOI identifiers cited by records you can read. """ - - query = db.query(DoiIdentifier) - - if search.text and len(search.text.strip()) > 0: - lower_search_text = search.text.strip().lower() - query = query.filter(func.lower(DoiIdentifier.identifier).contains(lower_search_text)) - else: + if not search.text or not search.text.strip(): raise HTTPException(status_code=400, detail="Search text is required") - items = query.order_by(DoiIdentifier.identifier).limit(50).all() - if not items: - items = [] - return items + return search_identifiers_on_readable_records( + db, + principal, + search.text.strip(), + identifier_sources_on_records(ScoreSetDoiIdentifier, ExperimentDoiIdentifier, ExperimentSetDoiIdentifier), + ) diff --git a/src/mavedb/routers/experiments.py b/src/mavedb/routers/experiments.py index a58d127b9..4108757c6 100644 --- a/src/mavedb/routers/experiments.py +++ b/src/mavedb/routers/experiments.py @@ -16,11 +16,7 @@ from mavedb.lib.collections import readable_official_collections from mavedb.lib.experiments import enrich_experiment_with_num_score_sets from mavedb.lib.experiments import search_experiments as _search_experiments -from mavedb.lib.identifiers import ( - find_or_create_doi_identifier, - find_or_create_publication_identifier, - find_or_create_raw_read_identifier, -) +from mavedb.lib.identifiers import distinct_identifiers, find_or_create_publication_identifier from mavedb.lib.keywords import search_keyword from mavedb.lib.logging import LoggedRoute from mavedb.lib.logging.context import logging_context, save_to_logging_context @@ -32,9 +28,11 @@ from mavedb.lib.validation.exceptions import ValidationError from mavedb.lib.validation.keywords import validate_keyword_list from mavedb.models.contributor import Contributor +from mavedb.models.doi_identifier import ExperimentDoiIdentifier from mavedb.models.experiment import Experiment from mavedb.models.experiment_controlled_keyword import ExperimentControlledKeywordAssociation from mavedb.models.experiment_set import ExperimentSet +from mavedb.models.raw_read_identifier import ExperimentRawReadIdentifier from mavedb.models.score_set import ScoreSet from mavedb.routers.shared import ( ACCESS_CONTROL_ERROR_RESPONSES, @@ -323,15 +321,16 @@ async def create_experiment( logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context()) raise HTTPException(status_code=404, detail=str(e)) + doi_identifiers = [ + ExperimentDoiIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in item_create.doi_identifiers or []) + ] + raw_read_identifiers = [ + ExperimentRawReadIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in item_create.raw_read_identifiers or []) + ] + try: - doi_identifiers = [ - await find_or_create_doi_identifier(db, identifier.identifier) - for identifier in item_create.doi_identifiers or [] - ] - raw_read_identifiers = [ - await find_or_create_raw_read_identifier(db, identifier.identifier) - for identifier in item_create.raw_read_identifiers or [] - ] primary_publication_identifiers = [ await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name) for identifier in item_create.primary_publication_identifiers or [] @@ -474,16 +473,16 @@ async def update_experiment( logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context()) raise HTTPException(status_code=404, detail=str(e)) - try: - doi_identifiers = [ - await find_or_create_doi_identifier(db, identifier.identifier) - for identifier in item_update.doi_identifiers or [] - ] - raw_read_identifiers = [ - await find_or_create_raw_read_identifier(db, identifier.identifier) - for identifier in item_update.raw_read_identifiers or [] - ] + doi_identifiers = [ + ExperimentDoiIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in item_update.doi_identifiers or []) + ] + raw_read_identifiers = [ + ExperimentRawReadIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in item_update.raw_read_identifiers or []) + ] + try: primary_publication_identifiers = [ await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name) for identifier in item_update.primary_publication_identifiers or [] diff --git a/src/mavedb/routers/genes.py b/src/mavedb/routers/genes.py index 5e6b10a08..04fabf86b 100644 --- a/src/mavedb/routers/genes.py +++ b/src/mavedb/routers/genes.py @@ -11,16 +11,13 @@ from mavedb.lib.hgnc.client import fetch_gene_info from mavedb.lib.logging import LoggedRoute from mavedb.lib.logging.context import logging_context, save_to_logging_context -from mavedb.models.ensembl_offset import EnsemblOffset from mavedb.models.experiment import Experiment from mavedb.models.experiment_controlled_keyword import ExperimentControlledKeywordAssociation from mavedb.models.experiment_publication_identifier import ExperimentPublicationIdentifierAssociation -from mavedb.models.refseq_offset import RefseqOffset from mavedb.models.score_set import ScoreSet from mavedb.models.score_set_publication_identifier import ScoreSetPublicationIdentifierAssociation from mavedb.models.target_gene import TargetGene from mavedb.models.target_sequence import TargetSequence -from mavedb.models.uniprot_offset import UniprotOffset from mavedb.routers.shared import GATEWAY_ERROR_RESPONSES, PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX from mavedb.view_models.gene import GeneResponse from mavedb.view_models.score_set import ShortScoreSet @@ -89,9 +86,7 @@ def _score_set_load_options(): ScoreSetPublicationIdentifierAssociation.publication ), selectinload(ScoreSet.target_genes).options( - joinedload(TargetGene.ensembl_offset).joinedload(EnsemblOffset.identifier), - joinedload(TargetGene.refseq_offset).joinedload(RefseqOffset.identifier), - joinedload(TargetGene.uniprot_offset).joinedload(UniprotOffset.identifier), + selectinload(TargetGene.external_identifiers), joinedload(TargetGene.target_sequence).joinedload(TargetSequence.taxonomy), joinedload(TargetGene.target_accession), ), diff --git a/src/mavedb/routers/raw_read_identifiers.py b/src/mavedb/routers/raw_read_identifiers.py index 90ff207b5..3762fd857 100644 --- a/src/mavedb/routers/raw_read_identifiers.py +++ b/src/mavedb/routers/raw_read_identifiers.py @@ -1,11 +1,17 @@ from typing import Any, List from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import func from sqlalchemy.orm import Session from mavedb import deps -from mavedb.models.raw_read_identifier import RawReadIdentifier +from mavedb.lib.authorization import get_principal +from mavedb.lib.identifier_search import identifier_sources_on_records, search_identifiers_on_readable_records +from mavedb.lib.permissions.principal import Principal +from mavedb.models.raw_read_identifier import ( + ExperimentRawReadIdentifier, + ExperimentSetRawReadIdentifier, + ScoreSetRawReadIdentifier, +) from mavedb.routers.shared import BASE_400_RESPONSE, PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX from mavedb.view_models import raw_read_identifier from mavedb.view_models.search import TextSearch @@ -31,20 +37,22 @@ responses={**BASE_400_RESPONSE}, summary="Search Raw Read identifiers", ) -def search_raw_read_identifiers(search: TextSearch, db: Session = Depends(deps.get_db)) -> Any: +def search_raw_read_identifiers( + search: TextSearch, + db: Session = Depends(deps.get_db), + principal: Principal = Depends(get_principal), +) -> Any: """ - Search Raw Read identifiers. + Search the Raw Read identifiers cited by records you can read. """ - - query = db.query(RawReadIdentifier) - - if search.text and len(search.text.strip()) > 0: - lower_search_text = search.text.strip().lower() - query = query.filter(func.lower(RawReadIdentifier.identifier).contains(lower_search_text)) - else: + if not search.text or not search.text.strip(): raise HTTPException(status_code=400, detail="Search text is required") - items = query.order_by(RawReadIdentifier.identifier).limit(50).all() - if not items: - items = [] - return items + return search_identifiers_on_readable_records( + db, + principal, + search.text.strip(), + identifier_sources_on_records( + ScoreSetRawReadIdentifier, ExperimentRawReadIdentifier, ExperimentSetRawReadIdentifier + ), + ) diff --git a/src/mavedb/routers/score_sets.py b/src/mavedb/routers/score_sets.py index 01ed631dc..e623fe796 100644 --- a/src/mavedb/routers/score_sets.py +++ b/src/mavedb/routers/score_sets.py @@ -57,9 +57,9 @@ from mavedb.lib.experiments import enrich_experiment_with_num_score_sets from mavedb.lib.gnomad import get_gnomad_variants_with_variant_urns from mavedb.lib.identifiers import ( - create_external_gene_identifier_offset, - find_or_create_doi_identifier, + distinct_identifiers, find_or_create_publication_identifier, + target_gene_external_identifiers, ) from mavedb.lib.logging import LoggedRoute from mavedb.lib.logging.context import logging_context, save_to_logging_context @@ -95,6 +95,7 @@ from mavedb.lib.variant_detail import load_variant_detail_inputs from mavedb.lib.workflow.kickoff import enqueue_pipeline_for_score_set from mavedb.models.contributor import Contributor +from mavedb.models.doi_identifier import ScoreSetDoiIdentifier from mavedb.models.enums.processing_state import ProcessingState from mavedb.models.experiment import Experiment from mavedb.models.license import License @@ -299,7 +300,8 @@ async def score_set_update( DoiIdentifierCreate(**identifier) for identifier in item_update_dict.get("doi_identifiers") or [] ] item.doi_identifiers = [ - await find_or_create_doi_identifier(db, identifier.identifier) for identifier in doi_identifiers_list + ScoreSetDoiIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in doi_identifiers_list) ] if any(key in item_update_dict for key in ["primary_publication_identifiers", "secondary_publication_identifiers"]): @@ -477,16 +479,7 @@ async def score_set_update( logger.info(msg="Failed to create score set; Could not infer target type.", extra=logging_context()) raise ValueError("One of either `target_accession` or `target_gene` should be present") - for external_gene_identifier_offset_create in gene.external_identifiers: - offset = external_gene_identifier_offset_create.offset - identifier_create = external_gene_identifier_offset_create.identifier - await create_external_gene_identifier_offset( - db, - target_gene, - identifier_create.db_name, - identifier_create.identifier, - offset, - ) + target_gene.external_identifiers = target_gene_external_identifiers(gene.external_identifiers) targets.append(target_gene) @@ -2115,11 +2108,12 @@ async def create_score_set( logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context()) raise HTTPException(status_code=404, detail=str(e)) + doi_identifiers = [ + ScoreSetDoiIdentifier(identifier=identifier) + for identifier in distinct_identifiers(i.identifier for i in item_create.doi_identifiers or []) + ] + try: - doi_identifiers = [ - await find_or_create_doi_identifier(db, identifier.identifier) - for identifier in item_create.doi_identifiers or [] - ] primary_publication_identifiers = [ await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name) for identifier in item_create.primary_publication_identifiers or [] @@ -2268,16 +2262,7 @@ async def create_score_set( logger.info(msg="Failed to create score set; Could not infer target type.", extra=logging_context()) raise ValueError("One of either `target_accession` or `target_gene` should be present") - for external_gene_identifier_offset_create in gene.external_identifiers: - offset = external_gene_identifier_offset_create.offset - identifier_create = external_gene_identifier_offset_create.identifier - await create_external_gene_identifier_offset( - db, - target_gene, - identifier_create.db_name, - identifier_create.identifier, - offset, - ) + target_gene.external_identifiers = target_gene_external_identifiers(gene.external_identifiers) targets.append(target_gene) diff --git a/src/mavedb/routers/statistics.py b/src/mavedb/routers/statistics.py index 5deea5636..dfda40150 100644 --- a/src/mavedb/routers/statistics.py +++ b/src/mavedb/routers/statistics.py @@ -9,33 +9,20 @@ from mavedb.deps import get_db from mavedb.models.controlled_keyword import ControlledKeyword -from mavedb.models.doi_identifier import DoiIdentifier -from mavedb.models.ensembl_identifier import EnsemblIdentifier -from mavedb.models.ensembl_offset import EnsemblOffset -from mavedb.models.experiment import ( - Experiment, - experiments_doi_identifiers_association_table, - experiments_raw_read_identifiers_association_table, -) +from mavedb.models.doi_identifier import ExperimentDoiIdentifier, ScoreSetDoiIdentifier +from mavedb.models.experiment import Experiment from mavedb.models.experiment_controlled_keyword import ExperimentControlledKeywordAssociation from mavedb.models.experiment_publication_identifier import ExperimentPublicationIdentifierAssociation from mavedb.models.publication_identifier import PublicationIdentifier from mavedb.models.published_variant import PublishedVariantsMV -from mavedb.models.raw_read_identifier import RawReadIdentifier -from mavedb.models.refseq_identifier import RefseqIdentifier -from mavedb.models.refseq_offset import RefseqOffset -from mavedb.models.score_set import ( - ScoreSet, - score_sets_doi_identifiers_association_table, - score_sets_raw_read_identifiers_association_table, -) +from mavedb.models.raw_read_identifier import ExperimentRawReadIdentifier, ScoreSetRawReadIdentifier +from mavedb.models.score_set import ScoreSet from mavedb.models.score_set_publication_identifier import ScoreSetPublicationIdentifierAssociation from mavedb.models.target_accession import TargetAccession from mavedb.models.target_gene import TargetGene +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.models.target_sequence import TargetSequence from mavedb.models.taxonomy import Taxonomy -from mavedb.models.uniprot_identifier import UniprotIdentifier -from mavedb.models.uniprot_offset import UniprotOffset from mavedb.models.user import User from mavedb.routers.shared import PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX @@ -62,6 +49,12 @@ type[ExperimentPublicationIdentifierAssociation], type[ScoreSetPublicationIdentifierAssociation], ] +RecordIdentifierLinks = Union[ + type[ExperimentDoiIdentifier], + type[ExperimentRawReadIdentifier], + type[ScoreSetDoiIdentifier], + type[ScoreSetRawReadIdentifier], +] ## Enum classes hold valid endpoints for different statistics routes. @@ -101,15 +94,11 @@ def _model_and_association_from_record_field( } record_to_assc_map: dict[RecordNames, dict[RecordFields, RecordAssociationTables]] = { RecordNames.experiment: { - RecordFields.doiIdentifiers: experiments_doi_identifiers_association_table, RecordFields.publicationIdentifiers: ExperimentPublicationIdentifierAssociation, - RecordFields.rawReadIdentifiers: experiments_raw_read_identifiers_association_table, RecordFields.keywords: ExperimentControlledKeywordAssociation, }, RecordNames.scoreSet: { - RecordFields.doiIdentifiers: score_sets_doi_identifiers_association_table, RecordFields.publicationIdentifiers: ScoreSetPublicationIdentifierAssociation, - RecordFields.rawReadIdentifiers: score_sets_raw_read_identifiers_association_table, }, } @@ -130,6 +119,40 @@ def _count_for_identifier_in_query(db: Session, query: Select[tuple[Any, int]]) return {value: count for value, count in db.execute(query).all() if value is not None} +# Identifiers are stored on the rows that link them to their records, so these count the link rows themselves. +RECORD_IDENTIFIER_LINKS: dict[RecordNames, dict[RecordFields, RecordIdentifierLinks]] = { + RecordNames.experiment: { + RecordFields.doiIdentifiers: ExperimentDoiIdentifier, + RecordFields.rawReadIdentifiers: ExperimentRawReadIdentifier, + }, + RecordNames.scoreSet: { + RecordFields.doiIdentifiers: ScoreSetDoiIdentifier, + RecordFields.rawReadIdentifiers: ScoreSetRawReadIdentifier, + }, +} + + +def _count_identifiers_on_published_records(db: Session, record: RecordNames, field: RecordFields) -> dict[str, int]: + queried_model, _ = _model_and_association_from_record_field(record, None) + link = RECORD_IDENTIFIER_LINKS[record][field] + query = _join_model_and_filter_unpublished( + select(link.identifier, func.count(link.identifier)), queried_model + ).group_by(link.identifier) + + return _count_for_identifier_in_query(db, query) + + +def _count_target_gene_identifiers_on_published_score_sets(db: Session, db_name: str) -> dict[str, int]: + query = _join_model_and_filter_unpublished( + select(TargetGeneExternalIdentifier.identifier, func.count(TargetGeneExternalIdentifier.identifier)) + .where(TargetGeneExternalIdentifier.db_name == db_name) + .join(TargetGene), + ScoreSet, + ).group_by(TargetGeneExternalIdentifier.identifier) + + return _count_for_identifier_in_query(db, query) + + ######################################################################################## # Record statistics ######################################################################################## @@ -216,21 +239,10 @@ def experiment_publication_identifier_statistics( ) def experiment_raw_read_identifier_statistics(record: RecordNames, db: Session = Depends(get_db)) -> dict[str, int]: """ - Returns a dictionary of counts for the distinct values of the `identifier` field (member of the `raw_read_identifiers` table). + Returns a dictionary of counts for each distinct raw read identifier. Don't include any raw read identifiers from unpublished experiments. """ - queried_model, queried_assc = _model_and_association_from_record_field(record, RecordFields.rawReadIdentifiers) - - if queried_assc is None: - raise HTTPException( - 500, "No association table associated with the raw read identifiers field when one was expected." - ) - - query = _join_model_and_filter_unpublished( - select(RawReadIdentifier.identifier, func.count(RawReadIdentifier.identifier)).join(queried_assc), queried_model - ).group_by(RawReadIdentifier.identifier) - - return _count_for_identifier_in_query(db, query) + return _count_identifiers_on_published_records(db, record, RecordFields.rawReadIdentifiers) @router.get( @@ -241,21 +253,10 @@ def experiment_raw_read_identifier_statistics(record: RecordNames, db: Session = ) def experiment_doi_identifiers_statistics(record: RecordNames, db: Session = Depends(get_db)) -> dict[str, int]: """ - Returns a dictionary of counts for the distinct values of the `identifier` field (member of the `doi_identifiers` table). + Returns a dictionary of counts for each distinct DOI identifier. Don't include any DOI identifiers from unpublished experiments. """ - queried_model, queried_assc = _model_and_association_from_record_field(record, RecordFields.doiIdentifiers) - - if queried_assc is None: - raise HTTPException( - 500, "No association table associated with the doi identifiers field when one was expected." - ) - - query = _join_model_and_filter_unpublished( - select(DoiIdentifier.identifier, func.count(DoiIdentifier.identifier)).join(queried_assc), queried_model - ).group_by(DoiIdentifier.identifier) - - return _count_for_identifier_in_query(db, query) + return _count_identifiers_on_published_records(db, record, RecordFields.doiIdentifiers) @router.get( @@ -519,17 +520,10 @@ def target_genes_organism_counts(db: Session = Depends(get_db)) -> dict[str, int ) def target_genes_ensembl_identifier_counts(db: Session = Depends(get_db)) -> dict[str, int]: """ - Returns a dictionary of counts for the distinct values of the `identifier` field (member of the `ensembl_identifiers` table). + Returns a dictionary of counts for each distinct Ensembl identifier of a target gene. Don't include any NULL field values. Don't include any targets from unpublished score sets. """ - query = _join_model_and_filter_unpublished( - select(EnsemblIdentifier.identifier, func.count(EnsemblIdentifier.identifier)) - .join(EnsemblOffset) - .join(TargetGene), - ScoreSet, - ).group_by(EnsemblIdentifier.identifier) - - return _count_for_identifier_in_query(db, query) + return _count_target_gene_identifiers_on_published_score_sets(db, "Ensembl") @router.get( @@ -540,17 +534,10 @@ def target_genes_ensembl_identifier_counts(db: Session = Depends(get_db)) -> dic ) def target_genes_refseq_identifier_counts(db: Session = Depends(get_db)) -> dict[str, int]: """ - Returns a dictionary of counts for the distinct values of the `identifier` field (member of the `refseq_identifiers` table). + Returns a dictionary of counts for each distinct RefSeq identifier of a target gene. Don't include any NULL field values. Don't include any targets from unpublished score sets. """ - query = _join_model_and_filter_unpublished( - select(RefseqIdentifier.identifier, func.count(RefseqIdentifier.identifier)) - .join(RefseqOffset) - .join(TargetGene), - ScoreSet, - ).group_by(RefseqIdentifier.identifier) - - return _count_for_identifier_in_query(db, query) + return _count_target_gene_identifiers_on_published_score_sets(db, "RefSeq") @router.get( @@ -561,17 +548,10 @@ def target_genes_refseq_identifier_counts(db: Session = Depends(get_db)) -> dict ) def target_genes_uniprot_identifier_counts(db: Session = Depends(get_db)) -> dict[str, int]: """ - Returns a dictionary of counts for the distinct values of the `identifier` field (member of the `uniprot_identifiers` table). + Returns a dictionary of counts for each distinct UniProt identifier of a target gene. Don't include any NULL field values. Don't include any targets from unpublished score sets. """ - query = _join_model_and_filter_unpublished( - select(UniprotIdentifier.identifier, func.count(UniprotIdentifier.identifier)) - .join(UniprotOffset) - .join(TargetGene), - ScoreSet, - ).group_by(UniprotIdentifier.identifier) - - return _count_for_identifier_in_query(db, query) + return _count_target_gene_identifiers_on_published_score_sets(db, "UniProt") @router.get( diff --git a/src/mavedb/routers/target_gene_identifiers.py b/src/mavedb/routers/target_gene_identifiers.py index 061802096..88387633b 100644 --- a/src/mavedb/routers/target_gene_identifiers.py +++ b/src/mavedb/routers/target_gene_identifiers.py @@ -1,11 +1,18 @@ from typing import Any, List from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import func +from sqlalchemy import select from sqlalchemy.orm import Session from mavedb import deps -from mavedb.lib.identifiers import EXTERNAL_GENE_IDENTIFIER_CLASSES +from mavedb.lib.authorization import get_principal +from mavedb.lib.identifier_search import search_identifiers_on_readable_records +from mavedb.lib.permissions.principal import Principal +from mavedb.lib.permissions.score_set import ScoreSetViewer +from mavedb.lib.validation.constants.identifier import valid_dbnames +from mavedb.models.score_set import ScoreSet +from mavedb.models.target_gene import TargetGene +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.routers.shared import BASE_400_RESPONSE, PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX from mavedb.view_models import external_gene_identifier from mavedb.view_models.search import TextSearch @@ -31,28 +38,36 @@ summary="Search target gene identifiers", responses={**BASE_400_RESPONSE}, ) -def search_target_gene_identifiers(db_name: str, search: TextSearch, db: Session = Depends(deps.get_db)) -> Any: +def search_target_gene_identifiers( + db_name: str, + search: TextSearch, + db: Session = Depends(deps.get_db), + principal: Principal = Depends(get_principal), +) -> Any: """ - Search target gene identifiers. + Search the target gene identifiers of score sets you can read. """ - if db_name not in EXTERNAL_GENE_IDENTIFIER_CLASSES: + if db_name not in valid_dbnames: raise HTTPException( status_code=422, - detail=f"Unexpected db_name: {db_name}. Expected one of: {list(EXTERNAL_GENE_IDENTIFIER_CLASSES.keys())}", + detail=f"Unexpected db_name: {db_name}. Expected one of: {valid_dbnames}", ) - identifier_class = EXTERNAL_GENE_IDENTIFIER_CLASSES[db_name] - assert hasattr(identifier_class, "identifier") - - query = db.query(identifier_class) - - if search.text and len(search.text.strip()) > 0: - lower_search_text = search.text.strip().lower() - query = query.filter(func.lower(identifier_class.identifier).contains(lower_search_text)) - else: + if not search.text or not search.text.strip(): raise HTTPException(status_code=400, detail="Search text is required") - items = query.order_by(identifier_class.identifier).limit(50).all() - if not items: - items = [] - return items + return search_identifiers_on_readable_records( + db, + principal, + search.text.strip(), + [ + ( + TargetGeneExternalIdentifier, + ScoreSetViewer, + select(TargetGeneExternalIdentifier, ScoreSet) + .join(TargetGeneExternalIdentifier.target_gene) + .join(TargetGene.score_set) + .where(TargetGeneExternalIdentifier.db_name == db_name), + ) + ], + ) diff --git a/src/mavedb/view_models/components/doi.py b/src/mavedb/view_models/components/doi.py new file mode 100644 index 000000000..9995e9b3a --- /dev/null +++ b/src/mavedb/view_models/components/doi.py @@ -0,0 +1,30 @@ +from typing import Annotated + +import idutils +from pydantic import AfterValidator + +from mavedb.lib.validation.exceptions import ValidationError + + +def canonical_doi(value: str) -> str: + """Validate a DOI and return its canonical form. + + Args: + value: A DOI, bare or with a ``doi:`` or resolver URL prefix. + + Returns: + The bare DOI, upper-cased. + + Raises: + ValidationError: If ``value`` is not a DOI. + """ + if not idutils.is_doi(value): + raise ValidationError("'{}' is not a valid DOI identifier.".format(value)) + + # DOIs are case-insensitive, and the DOI registry stores them upper-cased. Storing that form keeps a record + # from citing the same DOI twice. + return idutils.normalize_doi(value).upper() + + +Doi = Annotated[str, AfterValidator(canonical_doi)] +"""A DOI field, validated and stored bare and upper-cased.""" diff --git a/src/mavedb/view_models/doi_identifier.py b/src/mavedb/view_models/doi_identifier.py index d9afdb24d..f94f172c2 100644 --- a/src/mavedb/view_models/doi_identifier.py +++ b/src/mavedb/view_models/doi_identifier.py @@ -1,10 +1,6 @@ -import idutils - -from pydantic import field_validator - -from mavedb.lib.validation.exceptions import ValidationError from mavedb.view_models import record_type_validator, set_record_type from mavedb.view_models.base.base import BaseModel +from mavedb.view_models.components.doi import Doi class DoiIdentifierBase(BaseModel): @@ -12,16 +8,11 @@ class DoiIdentifierBase(BaseModel): class DoiIdentifierCreate(DoiIdentifierBase): - @field_validator("identifier") - def must_be_valid_doi(cls, v: str): - if not idutils.is_doi(v): - raise ValidationError("'{}' is not a valid DOI identifier.".format(v)) - return v + identifier: Doi # Properties shared by models stored in DB class SavedDoiIdentifier(DoiIdentifierBase): - id: int record_type: str = None # type: ignore url: str diff --git a/src/mavedb/view_models/external_gene_identifier.py b/src/mavedb/view_models/external_gene_identifier.py index 0dd8a7e2f..738a898ed 100644 --- a/src/mavedb/view_models/external_gene_identifier.py +++ b/src/mavedb/view_models/external_gene_identifier.py @@ -30,7 +30,6 @@ class SavedExternalGeneIdentifier(ExternalGeneIdentifierBase): record_type: str = None # type: ignore db_version: Optional[str] = None url: Optional[str] = None - reference_html: Optional[str] = None _record_type_factory = record_type_validator()(set_record_type) diff --git a/src/mavedb/view_models/external_gene_identifier_offset.py b/src/mavedb/view_models/external_gene_identifier_offset.py index 51876ae7c..e49313b13 100644 --- a/src/mavedb/view_models/external_gene_identifier_offset.py +++ b/src/mavedb/view_models/external_gene_identifier_offset.py @@ -1,5 +1,8 @@ -from pydantic import field_validator +from typing import Any +from pydantic import field_validator, model_validator + +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.view_models import external_gene_identifier, record_type_validator, set_record_type from mavedb.view_models.base.base import BaseModel @@ -29,6 +32,18 @@ class SavedExternalGeneIdentifierOffset(ExternalGeneIdentifierOffsetBase): class Config: from_attributes = True + # A stored row holds the identifier beside its offset. The api nests it, as it did when each database had + # its own identifier table. + @model_validator(mode="before") + @classmethod + def nest_stored_identifier(cls, data: Any) -> Any: + if isinstance(data, TargetGeneExternalIdentifier): + return { + "identifier": {"db_name": data.db_name, "identifier": data.identifier, "url": data.url}, + "offset": data.offset, + } + return data + # Properties to return to non-admin clients class ExternalGeneIdentifierOffset(SavedExternalGeneIdentifierOffset): diff --git a/src/mavedb/view_models/raw_read_identifier.py b/src/mavedb/view_models/raw_read_identifier.py index 2a273ac15..d91f3e4ee 100644 --- a/src/mavedb/view_models/raw_read_identifier.py +++ b/src/mavedb/view_models/raw_read_identifier.py @@ -12,7 +12,6 @@ class RawReadIdentifierCreate(RawReadIdentifierBase): # Properties shared by models stored in DB class SavedRawReadIdentifier(RawReadIdentifierBase): - id: int record_type: str = None # type: ignore url: str diff --git a/src/mavedb/view_models/target_gene.py b/src/mavedb/view_models/target_gene.py index eb27ca867..5e62f2f99 100644 --- a/src/mavedb/view_models/target_gene.py +++ b/src/mavedb/view_models/target_gene.py @@ -5,7 +5,7 @@ from typing_extensions import Self from mavedb.lib.validation.exceptions import ValidationError -from mavedb.lib.validation.transform import transform_external_identifier_offsets_to_list, transform_score_set_to_urn +from mavedb.lib.validation.transform import transform_score_set_to_urn from mavedb.models.enums.target_category import TargetCategory from mavedb.view_models import external_gene_identifier_offset, record_type_validator, set_record_type from mavedb.view_models.base.base import BaseModel @@ -69,20 +69,6 @@ class Config: from_attributes = True arbitrary_types_allowed = True - # These 'synthetic' fields are generated from other model properties. Transform data from other properties as needed, setting - # the appropriate field on the model itself. Then, proceed with Pydantic ingestion once fields are created. Only perform these - # transformations if the relevant attributes are present on the input data (i.e., when creating from an ORM object). - @model_validator(mode="before") - def generate_external_identifiers_list(cls, data: Any): - if hasattr(data, "ensembl_offset") or hasattr(data, "refseq_offset") or hasattr(data, "uniprot_offset"): - try: - data.__setattr__("external_identifiers", transform_external_identifier_offsets_to_list(data)) - except (AttributeError, KeyError) as exc: - raise ValidationError( - f"Unable to coerce external identifiers for {cls.__name__}: {exc}." # type: ignore - ) - return data - @model_validator(mode="after") def check_seq_or_accession(self) -> Self: if self.target_sequence is None and self.target_accession is None: diff --git a/tests/helpers/constants.py b/tests/helpers/constants.py index f1d7b81ff..03d0ecc93 100644 --- a/tests/helpers/constants.py +++ b/tests/helpers/constants.py @@ -261,7 +261,6 @@ "recordType": "DoiIdentifier", "identifier": TEST_CROSSREF_IDENTIFIER, "url": f"https://doi.org/{TEST_CROSSREF_IDENTIFIER}", - "id": 1, } # VRS 1.X @@ -2244,7 +2243,6 @@ TEST_SAVED_MINIMAL_RAW_READ_IDENTIFIER = { **TEST_MINIMAL_RAW_READ_IDENTIFIER, - "id": 1, "url": "https://www.test.rawreadidentifiers.org", } diff --git a/tests/lib/permissions/test_experiment.py b/tests/lib/permissions/test_experiment.py index d1d11b2b7..8becca56f 100644 --- a/tests/lib/permissions/test_experiment.py +++ b/tests/lib/permissions/test_experiment.py @@ -11,6 +11,7 @@ from mavedb.lib.permissions.actions import Action from mavedb.lib.permissions.experiment import ( + ExperimentViewer, _handle_add_score_set_action, _handle_delete_action, _handle_read_action, @@ -287,3 +288,25 @@ def test_handle_add_score_set_action(self, test_case: PermissionTest, entity_hel assert result.permitted == test_case.should_be_permitted if not test_case.should_be_permitted and test_case.expected_code: assert result.http_code == test_case.expected_code + + +class TestExperimentViewer: + """Test that the viewer applies experiment READ rules. The caching and fail-closed behaviour it inherits are + covered by test_viewer.py.""" + + @pytest.mark.parametrize( + "user_type, entity_state, expected", + [ + ("anonymous", "private", False), + ("other_user", "private", False), + ("owner", "private", True), + ("anonymous", "published", True), + ], + ids=["anonymous-private", "other-user-private", "owner-private", "anonymous-published"], + ) + def test_may_read_follows_experiment_read_rules( + self, entity_helper: EntityTestHelper, user_type: str, entity_state: str, expected: bool + ) -> None: + viewer = ExperimentViewer(entity_helper.create_user_data(user_type)) + + assert viewer.may_read(entity_helper.create_experiment(entity_state)) is expected diff --git a/tests/lib/permissions/test_experiment_set.py b/tests/lib/permissions/test_experiment_set.py index b47d08e16..467e1fbf7 100644 --- a/tests/lib/permissions/test_experiment_set.py +++ b/tests/lib/permissions/test_experiment_set.py @@ -11,6 +11,7 @@ from mavedb.lib.permissions.actions import Action from mavedb.lib.permissions.experiment_set import ( + ExperimentSetViewer, _handle_add_experiment_action, _handle_delete_action, _handle_read_action, @@ -293,3 +294,25 @@ def test_handle_add_experiment_action(self, test_case: PermissionTest, entity_he assert result.permitted == test_case.should_be_permitted if not test_case.should_be_permitted and test_case.expected_code: assert result.http_code == test_case.expected_code + + +class TestExperimentSetViewer: + """Test that the viewer applies experiment set READ rules. The caching and fail-closed behaviour it inherits are + covered by test_viewer.py.""" + + @pytest.mark.parametrize( + "user_type, entity_state, expected", + [ + ("anonymous", "private", False), + ("other_user", "private", False), + ("owner", "private", True), + ("anonymous", "published", True), + ], + ids=["anonymous-private", "other-user-private", "owner-private", "anonymous-published"], + ) + def test_may_read_follows_experiment_set_read_rules( + self, entity_helper: EntityTestHelper, user_type: str, entity_state: str, expected: bool + ) -> None: + viewer = ExperimentSetViewer(entity_helper.create_user_data(user_type)) + + assert viewer.may_read(entity_helper.create_experiment_set(entity_state)) is expected diff --git a/tests/lib/permissions/test_score_set.py b/tests/lib/permissions/test_score_set.py index fd004b90f..f0dfa9d90 100644 --- a/tests/lib/permissions/test_score_set.py +++ b/tests/lib/permissions/test_score_set.py @@ -11,6 +11,7 @@ from mavedb.lib.permissions.actions import Action from mavedb.lib.permissions.score_set import ( + ScoreSetViewer, _handle_add_calibration_action, _handle_delete_action, _handle_publish_action, @@ -385,3 +386,25 @@ def test_handle_add_calibration_action(self, test_case: PermissionTest, entity_h assert result.permitted == test_case.should_be_permitted if not test_case.should_be_permitted and test_case.expected_code: assert result.http_code == test_case.expected_code + + +class TestScoreSetViewer: + """Test that the viewer applies score set READ rules. The caching and fail-closed behaviour it inherits are + covered by test_viewer.py.""" + + @pytest.mark.parametrize( + "user_type, entity_state, expected", + [ + ("anonymous", "private", False), + ("other_user", "private", False), + ("owner", "private", True), + ("anonymous", "published", True), + ], + ids=["anonymous-private", "other-user-private", "owner-private", "anonymous-published"], + ) + def test_may_read_follows_score_set_read_rules( + self, entity_helper: EntityTestHelper, user_type: str, entity_state: str, expected: bool + ) -> None: + viewer = ScoreSetViewer(entity_helper.create_user_data(user_type)) + + assert viewer.may_read(entity_helper.create_score_set(entity_state)) is expected diff --git a/tests/lib/test_external_identifier_urls.py b/tests/lib/test_external_identifier_urls.py new file mode 100644 index 000000000..3b4f95601 --- /dev/null +++ b/tests/lib/test_external_identifier_urls.py @@ -0,0 +1,40 @@ +import pytest + +from mavedb.lib.external_identifier_urls import doi_url, external_gene_identifier_url, raw_read_url + + +@pytest.mark.unit +def test_doi_url_links_to_the_doi_resolver(): + assert doi_url("10.7554/ELIFE.58026") == "https://doi.org/10.7554/ELIFE.58026" + + +@pytest.mark.unit +@pytest.mark.parametrize( + "identifier, expected_url", + [ + ("PRJNA941815", "https://www.ncbi.nlm.nih.gov/bioproject/PRJNA941815"), + ("GSE226732", "https://www.ncbi.nlm.nih.gov/geo/query/acc.cgi?acc=GSE226732"), + ("E-MTAB-1234", "https://www.ebi.ac.uk/biostudies/arrayexpress/studies/E-MTAB-1234"), + ("A-AFFY-44", "https://www.ebi.ac.uk/biostudies/arrayexpress/arrays/A-AFFY-44"), + ("SRP002725", "https://www.ebi.ac.uk/ena/browser/view/SRP002725"), + ("not-an-accession", "https://www.ebi.ac.uk/ena/browser/view/not-an-accession"), + ], + ids=["bioproject", "geo", "arrayexpress-experiment", "arrayexpress-array", "sra", "unrecognized"], +) +def test_raw_read_url_links_by_accession_type(identifier, expected_url): + assert raw_read_url(identifier) == expected_url + + +@pytest.mark.unit +@pytest.mark.parametrize( + "db_name, identifier, expected_url", + [ + ("Ensembl", "ENSG00000103275", "https://www.ensembl.org/id/ENSG00000103275"), + ("UniProt", "Q9Y617", "https://www.uniprot.org/uniprotkb/Q9Y617"), + ("RefSeq", "NP_478059.1", "https://www.ncbi.nlm.nih.gov/protein/NP_478059.1"), + ("RefSeq", "NM_003352.8", "https://www.ncbi.nlm.nih.gov/nuccore/NM_003352.8"), + ], + ids=["ensembl", "uniprot", "refseq-protein", "refseq-nucleotide"], +) +def test_external_gene_identifier_url_links_to_the_database_page(db_name, identifier, expected_url): + assert external_gene_identifier_url(db_name, identifier) == expected_url diff --git a/tests/models/mixins/__init__.py b/tests/models/mixins/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/db/test_mixins.py b/tests/models/mixins/test_valid_time.py similarity index 99% rename from tests/db/test_mixins.py rename to tests/models/mixins/test_valid_time.py index cd7ef373d..aa6876809 100644 --- a/tests/db/test_mixins.py +++ b/tests/models/mixins/test_valid_time.py @@ -21,7 +21,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import aliased, declarative_base, relationship -from mavedb.db.mixins import ValidTime +from mavedb.models.mixins.valid_time import ValidTime VtBase = declarative_base() diff --git a/tests/routers/test_record_identifiers.py b/tests/routers/test_record_identifiers.py new file mode 100644 index 000000000..d11f0016d --- /dev/null +++ b/tests/routers/test_record_identifiers.py @@ -0,0 +1,243 @@ +# ruff: noqa: E402 + +import pytest + +arq = pytest.importorskip("arq") +cdot = pytest.importorskip("cdot") +fastapi = pytest.importorskip("fastapi") + +from copy import deepcopy + +from sqlalchemy import select + +from mavedb.models.doi_identifier import ExperimentDoiIdentifier, ScoreSetDoiIdentifier +from mavedb.models.raw_read_identifier import ExperimentRawReadIdentifier +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier +from tests.helpers.constants import TEST_MINIMAL_EXPERIMENT, TEST_MINIMAL_SEQ_SCORESET +from tests.helpers.dependency_overrider import DependencyOverrider +from tests.helpers.util.experiment import create_experiment +from tests.helpers.util.score_set import create_seq_score_set + + +def _target_genes_with(external_identifiers): + target_genes = deepcopy(TEST_MINIMAL_SEQ_SCORESET["targetGenes"]) + target_genes[0]["externalIdentifiers"] = external_identifiers + return target_genes + + +ENSEMBL = {"identifier": {"dbName": "Ensembl", "identifier": "ENSG00000103275"}, "offset": 0} +UNIPROT = {"identifier": {"dbName": "UniProt", "identifier": "Q9Y617"}, "offset": 3} +REFSEQ = {"identifier": {"dbName": "RefSeq", "identifier": "NP_478059.1"}, "offset": 6} + + +def test_create_experiment_with_identifiers_stores_one_upper_cased_doi_and_typed_raw_read_urls( + session, client, setup_router_db +): + experiment = create_experiment( + client, + { + "doiIdentifiers": [{"identifier": "10.7554/eLife.58026"}, {"identifier": "10.7554/ELIFE.58026"}], + "rawReadIdentifiers": [ + {"identifier": "SRP002725"}, + {"identifier": "PRJNA941815"}, + {"identifier": "GSE226732"}, + {"identifier": "E-MTAB-1234"}, + ], + }, + ) + + assert experiment["doiIdentifiers"] == [ + { + "recordType": "DoiIdentifier", + "identifier": "10.7554/ELIFE.58026", + "url": "https://doi.org/10.7554/ELIFE.58026", + } + ] + assert {identifier["identifier"]: identifier["url"] for identifier in experiment["rawReadIdentifiers"]} == { + "SRP002725": "https://www.ebi.ac.uk/ena/browser/view/SRP002725", + "PRJNA941815": "https://www.ncbi.nlm.nih.gov/bioproject/PRJNA941815", + "GSE226732": "https://www.ncbi.nlm.nih.gov/geo/query/acc.cgi?acc=GSE226732", + "E-MTAB-1234": "https://www.ebi.ac.uk/biostudies/arrayexpress/studies/E-MTAB-1234", + } + assert session.scalars(select(ExperimentDoiIdentifier.identifier)).all() == ["10.7554/ELIFE.58026"] + + +def test_update_experiment_identifiers_keeps_resubmitted_and_drops_omitted(session, client, setup_router_db): + experiment = create_experiment( + client, + { + "doiIdentifiers": [{"identifier": "10.1000/182"}], + "rawReadIdentifiers": [{"identifier": "SRP002725"}], + }, + ) + + payload = deepcopy(TEST_MINIMAL_EXPERIMENT) + payload.update( + { + "doiIdentifiers": [{"identifier": "10.1000/182"}, {"identifier": "10.1000/183"}], + "rawReadIdentifiers": [], + } + ) + response = client.put(f"/api/v1/experiments/{experiment['urn']}", json=payload) + + assert response.status_code == 200 + assert [identifier["identifier"] for identifier in response.json()["doiIdentifiers"]] == [ + "10.1000/182", + "10.1000/183", + ] + assert response.json()["rawReadIdentifiers"] == [] + assert session.scalars(select(ExperimentRawReadIdentifier)).all() == [] + + +def test_create_score_set_with_doi_stores_it_on_the_score_set(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"], {"doiIdentifiers": [{"identifier": "10.1000/abc"}]}) + + assert score_set["doiIdentifiers"] == [ + {"recordType": "DoiIdentifier", "identifier": "10.1000/ABC", "url": "https://doi.org/10.1000/ABC"} + ] + assert session.scalars(select(ScoreSetDoiIdentifier.identifier)).all() == ["10.1000/ABC"] + + +def test_create_score_set_with_external_identifiers_returns_them_nested_with_urls(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set( + client, experiment["urn"], {"targetGenes": _target_genes_with([UNIPROT, ENSEMBL, REFSEQ])} + ) + + assert score_set["targetGenes"][0]["externalIdentifiers"] == [ + { + "recordType": "ExternalGeneIdentifierOffset", + "offset": 0, + "identifier": { + "recordType": "ExternalGeneIdentifier", + "dbName": "Ensembl", + "identifier": "ENSG00000103275", + "url": "https://www.ensembl.org/id/ENSG00000103275", + }, + }, + { + "recordType": "ExternalGeneIdentifierOffset", + "offset": 6, + "identifier": { + "recordType": "ExternalGeneIdentifier", + "dbName": "RefSeq", + "identifier": "NP_478059.1", + "url": "https://www.ncbi.nlm.nih.gov/protein/NP_478059.1", + }, + }, + { + "recordType": "ExternalGeneIdentifierOffset", + "offset": 3, + "identifier": { + "recordType": "ExternalGeneIdentifier", + "dbName": "UniProt", + "identifier": "Q9Y617", + "url": "https://www.uniprot.org/uniprotkb/Q9Y617", + }, + }, + ] + + +def test_update_score_set_external_identifiers_replaces_the_target_genes_set(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"], {"targetGenes": _target_genes_with([UNIPROT, ENSEMBL])}) + + payload = deepcopy(TEST_MINIMAL_SEQ_SCORESET) + payload.update( + { + "experimentUrn": experiment["urn"], + "targetGenes": _target_genes_with([{**UNIPROT, "offset": 9}]), + } + ) + response = client.put(f"/api/v1/score-sets/{score_set['urn']}", json=payload) + + assert response.status_code == 200 + rows = session.execute(select(TargetGeneExternalIdentifier.db_name, TargetGeneExternalIdentifier.offset)).all() + assert rows == [("UniProt", 9)] + + +@pytest.mark.parametrize( + "route, text, identifier, make_record", + [ + ( + "/api/v1/doi-identifiers/search", + "10.1000", + "10.1000/182", + lambda client: create_experiment(client, {"doiIdentifiers": [{"identifier": "10.1000/182"}]}), + ), + ( + "/api/v1/raw-read-identifiers/search", + "SRP", + "SRP002725", + lambda client: create_experiment(client, {"rawReadIdentifiers": [{"identifier": "SRP002725"}]}), + ), + ( + "/api/v1/target-gene-identifiers/search?db_name=UniProt", + "Q9Y", + "Q9Y617", + lambda client: create_seq_score_set( + client, create_experiment(client)["urn"], {"targetGenes": _target_genes_with([UNIPROT])} + ), + ), + ], + ids=["doi", "raw-read", "target-gene"], +) +def test_search_identifiers_on_private_record_visible_only_to_those_who_can_read_it( + client, setup_router_db, anonymous_app_overrides, extra_user_app_overrides, route, text, identifier, make_record +): + make_record(client) + + owner_response = client.post(route, json={"text": text}) + with DependencyOverrider(anonymous_app_overrides): + anonymous_response = client.post(route, json={"text": text}) + with DependencyOverrider(extra_user_app_overrides): + other_user_response = client.post(route, json={"text": text}) + + assert owner_response.status_code == 200 + assert [result["identifier"] for result in owner_response.json()] == [identifier] + assert anonymous_response.json() == [] + assert other_user_response.json() == [] + + +def test_search_doi_identifiers_cited_twice_returns_one_result(client, setup_router_db): + experiment = create_experiment(client, {"doiIdentifiers": [{"identifier": "10.1000/182"}]}) + create_seq_score_set(client, experiment["urn"], {"doiIdentifiers": [{"identifier": "10.1000/182"}]}) + + response = client.post("/api/v1/doi-identifiers/search", json={"text": "10.1000"}) + + assert response.status_code == 200 + assert response.json() == [ + {"recordType": "DoiIdentifier", "identifier": "10.1000/182", "url": "https://doi.org/10.1000/182"} + ] + + +@pytest.mark.parametrize("text", ["%", "_"], ids=["percent", "underscore"]) +def test_search_doi_identifiers_with_like_wildcard_matches_it_literally(client, setup_router_db, text): + create_experiment(client, {"doiIdentifiers": [{"identifier": "10.1000/182"}]}) + + response = client.post("/api/v1/doi-identifiers/search", json={"text": text}) + + assert response.status_code == 200 + assert response.json() == [] + + +@pytest.mark.parametrize( + "route", + [ + "/api/v1/doi-identifiers/search", + "/api/v1/raw-read-identifiers/search", + "/api/v1/target-gene-identifiers/search?db_name=UniProt", + ], + ids=["doi", "raw-read", "target-gene"], +) +def test_search_identifiers_without_text_returns_400(client, setup_router_db, route): + response = client.post(route, json={"text": " "}) + + assert response.status_code == 400 + + +def test_search_target_gene_identifiers_unknown_database_returns_422(client, setup_router_db): + response = client.post("/api/v1/target-gene-identifiers/search?db_name=GenBank", json={"text": "Q9Y"}) + + assert response.status_code == 422 diff --git a/tests/routers/test_statistics.py b/tests/routers/test_statistics.py index 6618eaf76..1d0dab107 100644 --- a/tests/routers/test_statistics.py +++ b/tests/routers/test_statistics.py @@ -1,5 +1,7 @@ # ruff: noqa: E402 +from copy import deepcopy + import pytest from humps import camelize from unittest.mock import patch @@ -241,14 +243,16 @@ def test_target_gene_field_statistics_seq(client, field, desired_field_value, se # Target gene identifier based statistics behave similarly enough to parametrize. @pytest.mark.parametrize("field_value", TARGET_GENE_IDENTIFIER_FIELDS) @pytest.mark.parametrize( - "target", (TEST_MINIMAL_ACC_SCORESET["targetGenes"][0], TEST_MINIMAL_SEQ_SCORESET["targetGenes"][0]) + "score_set_template", (TEST_MINIMAL_ACC_SCORESET, TEST_MINIMAL_SEQ_SCORESET), ids=["accession", "sequence"] ) -def test_target_gene_identifier_statistiscs( - session, data_provider, client, setup_router_db, data_files, field_value, target +def test_target_gene_identifier_statistics_published_score_set_counts_its_identifiers( + session, data_provider, client, setup_router_db, data_files, field_value, score_set_template ): """Test target gene identifier statistics endpoint for published score sets.""" - record_update = {"externalIdentifiers": [v for v in EXTERNAL_IDENTIFIERS.values()]} - target.update(record_update) + # Copy the target rather than updating the shared constant, which later tests in this process would inherit. + target = deepcopy(score_set_template["targetGenes"][0]) + target["externalIdentifiers"] = list(EXTERNAL_IDENTIFIERS.values()) + score_set_update = {"targetGenes": [target]} # Create whichever score set target type is being tested. experiment = create_experiment(client) @@ -256,13 +260,13 @@ def test_target_gene_identifier_statistiscs( with patch.object( cdot.hgvs.dataproviders.RESTDataProvider, "_get_transcript", return_value=TEST_NT_CDOT_TRANSCRIPT ): - unpublished_score_set = create_acc_score_set(client, experiment["urn"]) + unpublished_score_set = create_acc_score_set(client, experiment["urn"], score_set_update) unpublished_score_set = mock_worker_variant_insertion( client, session, data_provider, unpublished_score_set, data_files / "scores_acc.csv" ) elif "targetSequence" in target: - unpublished_score_set = create_seq_score_set(client, experiment["urn"]) + unpublished_score_set = create_seq_score_set(client, experiment["urn"], score_set_update) unpublished_score_set = mock_worker_variant_insertion( client, session, data_provider, unpublished_score_set, data_files / "scores.csv" ) diff --git a/tests/view_models/test_doi_identifier.py b/tests/view_models/test_doi_identifier.py index 09bf9cd45..48cdba10d 100644 --- a/tests/view_models/test_doi_identifier.py +++ b/tests/view_models/test_doi_identifier.py @@ -20,3 +20,17 @@ def test_invalid_doi_fails(): ) assert f"'{invalid_doi}' is not a valid DOI identifier" in str(exc_info.value) + + +@pytest.mark.parametrize( + "doi, expected", + [ + ("10.7554/eLife.58026", "10.7554/ELIFE.58026"), + ("https://doi.org/10.7554/elife.58026", "10.7554/ELIFE.58026"), + ], + ids=["mixed-case", "url"], +) +def test_create_doi_identifier_mixed_case_is_upper_cased(doi, expected): + doi_identifier = DoiIdentifierCreate(identifier=doi) + + assert doi_identifier.identifier == expected diff --git a/tests/view_models/test_score_set.py b/tests/view_models/test_score_set.py index 036989743..2c9079e3c 100644 --- a/tests/view_models/test_score_set.py +++ b/tests/view_models/test_score_set.py @@ -291,10 +291,7 @@ def test_saved_score_set_synthetic_properties(): score_set.pop("secondaryPublicationIdentifiers") # Convert fields expecting an object to attributed objects - external_identifiers = {"refseq_offset": None, "ensembl_offset": None, "uniprot_offset": None} - target_genes = [ - dummy_attributed_object_from_dict({**target, **external_identifiers}) for target in score_set["targetGenes"] - ] + target_genes = [dummy_attributed_object_from_dict(target) for target in score_set["targetGenes"]] score_set["targetGenes"] = [SavedTargetGene.model_validate(target) for target in target_genes] # Set synthetic properties with dummy attributed objects to mock SQLAlchemy model objects. @@ -397,10 +394,7 @@ def test_cannot_create_saved_score_set_without_all_attributed_properties(exclude score_set.pop("secondaryPublicationIdentifiers") # Convert fields expecting an object to attributed objects - external_identifiers = {"refseq_offset": None, "ensembl_offset": None, "uniprot_offset": None} - target_genes = [ - dummy_attributed_object_from_dict({**target, **external_identifiers}) for target in score_set["targetGenes"] - ] + target_genes = [dummy_attributed_object_from_dict(target) for target in score_set["targetGenes"]] score_set["targetGenes"] = [SavedTargetGene.model_validate(target) for target in target_genes] # Set synthetic properties with dummy attributed objects to mock SQLAlchemy model objects. diff --git a/tests/view_models/test_target_gene.py b/tests/view_models/test_target_gene.py index 71b497b9f..db71974fb 100644 --- a/tests/view_models/test_target_gene.py +++ b/tests/view_models/test_target_gene.py @@ -1,5 +1,6 @@ import pytest +from mavedb.models.target_gene_external_identifier import TargetGeneExternalIdentifier from mavedb.view_models.target_gene import SavedTargetGene, TargetGene, TargetGeneCreate, TargetGeneWithScoreSetUrn from tests.helpers.constants import ( SEQUENCE, @@ -142,46 +143,39 @@ def test_cannot_create_target_gene_with_both_sequence_and_accession(): assert "Expected either a `target_sequence` or a `target_accession`, not both." in str(exc_info.value) +def _stored_external_identifier(identifier, offset): + return TargetGeneExternalIdentifier( + db_name=identifier["db_name"], identifier=identifier["identifier"], offset=offset + ) + + @pytest.mark.parametrize( "identifier", - [ - {"key": "ensembl_offset", "value": TEST_ENSEMBLE_EXTERNAL_IDENTIFIER}, - {"key": "refseq_offset", "value": TEST_REFSEQ_EXTERNAL_IDENTIFIER}, - {"key": "uniprot_offset", "value": TEST_UNIPROT_EXTERNAL_IDENTIFIER}, - ], + [TEST_ENSEMBLE_EXTERNAL_IDENTIFIER, TEST_REFSEQ_EXTERNAL_IDENTIFIER, TEST_UNIPROT_EXTERNAL_IDENTIFIER], + ids=["ensembl", "refseq", "uniprot"], ) -def test_external_identifiers_parsed_into_containing_list(identifier): - target_gene = { - "id": 1, - "name": "UBE2I", - "category": "regulatory", - "ensembl_offset": None, - "refseq_offset": None, - "uniprot_offset": None, - } - target_gene["target_sequence"] = dummy_attributed_object_from_dict( +def test_saved_target_gene_from_stored_external_identifier_nests_identifier_beside_offset(identifier): + offset = 10 + target_gene = dummy_attributed_object_from_dict( { - "sequenceType": "dna", - "sequence": SEQUENCE, - "taxonomy": TEST_SAVED_TAXONOMY, + "id": 1, + "name": "UBE2I", + "category": "regulatory", + "external_identifiers": [_stored_external_identifier(identifier, offset)], + "target_sequence": dummy_attributed_object_from_dict( + { + "sequenceType": "dna", + "sequence": SEQUENCE, + "taxonomy": TEST_SAVED_TAXONOMY, + } + ), } ) - - # Tranform external identifier constant into an attributed object, with an added offset property - external_identifier = identifier["key"] - offset = 10 - target_gene[external_identifier] = { - "offset": offset, - "identifier": dummy_attributed_object_from_dict(identifier["value"]), - } - target_gene[external_identifier] = dummy_attributed_object_from_dict(target_gene[external_identifier]) - - target_gene = dummy_attributed_object_from_dict(target_gene) saved_target_gene = SavedTargetGene.model_validate(target_gene) assert len(saved_target_gene.external_identifiers) == 1 - assert saved_target_gene.external_identifiers[0].identifier.identifier == identifier["value"]["identifier"] - assert saved_target_gene.external_identifiers[0].identifier.db_name == identifier["value"]["db_name"] + assert saved_target_gene.external_identifiers[0].identifier.identifier == identifier["identifier"] + assert saved_target_gene.external_identifiers[0].identifier.db_name == identifier["db_name"] assert saved_target_gene.external_identifiers[0].offset == offset @@ -190,9 +184,7 @@ def test_cannot_create_saved_target_without_seq_or_acc(): "id": 1, "name": "UBE2I", "category": "regulatory", - "ensembl_offset": None, - "refseq_offset": None, - "uniprot_offset": None, + "external_identifiers": [], } target_gene = dummy_attributed_object_from_dict(target_gene) @@ -208,11 +200,7 @@ def test_saved_target_gene_can_be_created_from_orm(): "id": 1, "name": "UBE2I", "category": "regulatory", - "ensembl_offset": dummy_attributed_object_from_dict( - {"offset": 1, "identifier": dummy_attributed_object_from_dict(TEST_ENSEMBLE_EXTERNAL_IDENTIFIER)} - ), - "refseq_offset": None, - "uniprot_offset": None, + "external_identifiers": [_stored_external_identifier(TEST_ENSEMBLE_EXTERNAL_IDENTIFIER, 1)], "target_sequence": dummy_attributed_object_from_dict( { "sequenceType": "dna", @@ -236,14 +224,7 @@ def test_target_gene_with_score_set_urn_can_be_created_from_orm(): "id": 1, "name": "UBE2I", "category": "regulatory", - "ensembl_offset": dummy_attributed_object_from_dict( - { - "offset": 1, - "identifier": dummy_attributed_object_from_dict(TEST_ENSEMBLE_EXTERNAL_IDENTIFIER), - } - ), - "refseq_offset": None, - "uniprot_offset": None, + "external_identifiers": [_stored_external_identifier(TEST_ENSEMBLE_EXTERNAL_IDENTIFIER, 1)], "target_sequence": dummy_attributed_object_from_dict( { "sequenceType": "dna",