Merge pull request #243 from VariantEffect/release2024.2.1

Release 2024.2.1

Author: bencap

src/mavedb/lib/authentication.py

@@ -158,11 +158,8 @@ async def get_current_user(
             email=email,
             is_first_login=True,
         )
-        logger.info(f"Creating new user with username {user.username}")
 
-        db.add(user)
-        db.commit()
-        db.refresh(user)
+        logger.info(f"Creating new user with username {user.username}")
 
     elif not user.is_active:
         return None

src/mavedb/lib/score_sets.py

@@ -71,7 +71,7 @@ def search_score_sets(db: Session, owner: Optional[User], search: ScoreSetsSearc
             query = query.filter(ScoreSet.published_date.is_(None))
 
     if search.text:
-        lower_search_text = search.text.lower()
+        lower_search_text = search.text.lower().strip()
         query = query.filter(
             or_(
                 ScoreSet.urn.icontains(lower_search_text),

src/mavedb/routers/experiments.py

@@ -125,7 +125,7 @@ def get_experiment_score_sets(
     #
     # TODO(#182): A side effect of this implementation is that only the user who has created the experiment may view all the Score sets
     # associated with a given experiment. This could be solved with user impersonation for certain user roles.
-    score_sets = db.query(ScoreSet).filter(ScoreSet.experiment_id == experiment.id)
+    score_sets = db.query(ScoreSet).filter(ScoreSet.experiment_id == experiment.id).filter(~ScoreSet.superseding_score_set.has())
     if user_data is not None:
         score_set_result = score_sets.filter(
             or_(ScoreSet.private.is_(False), and_(ScoreSet.private.is_(True), ScoreSet.created_by == user_data.user))

src/mavedb/routers/publication_identifiers.py

@@ -1,16 +1,23 @@
+from enum import Enum
 from typing import Any, List
 
 from fastapi import APIRouter, Depends, HTTPException
 from sqlalchemy import func, or_
 from sqlalchemy.exc import MultipleResultsFound
 from sqlalchemy.orm import Session
+from starlette.convertors import Convertor, register_url_convertor
 
 from mavedb import deps
-from mavedb.lib.identifiers import find_generic_article, find_or_create_publication_identifier
+from mavedb.lib.identifiers import find_generic_article
+from mavedb.lib.validation.constants.publication import valid_dbnames
 from mavedb.models.publication_identifier import PublicationIdentifier
 from mavedb.view_models import publication_identifier
 from mavedb.view_models.search import TextSearch
 
+# I don't think we can escape the type: ignore hint here on a dynamically created enumerated type.
+PublicationDatabases = Enum("PublicationDataBases", ((x, x) for x in valid_dbnames))  # type: ignore
+
+
 router = APIRouter(
     prefix="/api/v1/publication-identifiers",
     tags=["publication identifiers"],
@@ -27,8 +34,31 @@ def list_publications(*, db: Session = Depends(deps.get_db)) -> Any:
     return items
 
 
+# See https://github.com/tiangolo/fastapi/discussions/7328, which describes that slashes are currently un-escapable in FastAPI/Starlette
+# implementations.
+#
+# Workaround here by defining a custom type convertor (see https://www.starlette.io/routing/#path-parameters) whose
+# RegEx match matches our accepted publication identifiers. When the API is queried and the string matches the convertor, we enter the
+# route on which the convertor captured a match, use the match as our variable of interest, and resolve the route. By capturing the match,
+# we retain the ability to add routes such as /db_name/identifier or /identifier/title since we can now match to these unescapable slashes
+# without matching the rest of the path as a path type, as the `:path` convertor would have. In the OpenAPI spec, this type will still be
+# represented as a 'path'. -capodb 2024.05.30
+class PublicationIdentifierConverter(Convertor):
+    # RegEx structure: "DOI | PubMed | bioRxiv | medRxiv"
+    regex = "10[.][0-9]{4,9}\/[-._;()\/:A-z0-9]+|[0-9]{0,8}|[0-9]{4}[.][0-9]{2}[.][0-9]{2}[.][0-9]{6}|[0-9]{4}[.][0-9]{2}[.][0-9]{2}[.][0-9]{8}"
+
+    def convert(self, value: str) -> str:
+        return value
+
+    def to_string(self, value: str) -> str:
+        return str(value)
+
+
+register_url_convertor("publication", PublicationIdentifierConverter())
+
+
 @router.get(
-    "/{identifier}",
+    "/{identifier:publication}",
     status_code=200,
     response_model=publication_identifier.PublicationIdentifier,
     responses={404: {}},
@@ -51,14 +81,14 @@ def fetch_publication_by_identifier(*, identifier: str, db: Session = Depends(de
 
 
 @router.get(
-    "/{db_name}/{identifier}",
+    "/{db_name:str}/{identifier:publication}",
     status_code=200,
     response_model=publication_identifier.PublicationIdentifier,
     responses={404: {}},
 )
 def fetch_publication_by_dbname_and_identifier(
     *,
-    db_name: str,
+    db_name: PublicationDatabases,
     identifier: str,
     db: Session = Depends(deps.get_db),
 ) -> PublicationIdentifier:
@@ -68,18 +98,19 @@ def fetch_publication_by_dbname_and_identifier(
     try:
         item = (
             db.query(PublicationIdentifier)
-            .filter(PublicationIdentifier.identifier == identifier and PublicationIdentifier.db_name == db_name)
+            .filter(PublicationIdentifier.identifier == identifier)
+            .filter(PublicationIdentifier.db_name == db_name.name)
             .one_or_none()
         )
     except MultipleResultsFound:
         raise HTTPException(
             status_code=500,
-            detail=f"Multiple publications with identifier {identifier} and database name {db_name} were found.",
+            detail=f"Multiple publications with identifier {identifier} and database name {db_name.name} were found.",
         )
     if not item:
         raise HTTPException(
             status_code=404,
-            detail=f"No publication with identifier {identifier} and database name {db_name} were found.",
+            detail=f"No publication with identifier {identifier} and database name {db_name.name} were found.",
         )
     return item
 
@@ -191,7 +222,7 @@ async def search_publications_by_identifier(*, identifier: str, db: Session = De
 @router.get(
     "/search/{db_name}/{identifier}",
     status_code=200,
-    response_model=publication_identifier.PublicationIdentifier,
+    response_model=list[publication_identifier.PublicationIdentifier],
     responses={404: {}, 500: {}},
 )
 async def search_publications_by_identifier_and_db(
@@ -201,7 +232,7 @@ async def search_publications_by_identifier_and_db(
     db: Session = Depends(deps.get_db),
 ) -> Any:
     """
-    Search publication identifiers via their identifier and database.
+    Search all of the publication identifiers via their identifier and database.
     """
     query = (
         db.query(PublicationIdentifier)

tests/helpers/constants.py

@@ -21,6 +21,12 @@
     "is_first_login": True,
 }
 
+TEST_USER_DECODED_JWT = {
+    "sub": TEST_USER["username"],
+    "given_name": TEST_USER["first_name"],
+    "family_name": TEST_USER["last_name"],
+}
+
 EXTRA_USER = {
     "username": "1234-5678-8765-4321",
     "first_name": "Extra",
@@ -32,6 +38,12 @@
     "is_first_login": True,
 }
 
+EXTRA_USER_DECODED_JWT = {
+    "sub": EXTRA_USER["username"],
+    "given_name": EXTRA_USER["first_name"],
+    "family_name": EXTRA_USER["last_name"],
+}
+
 ADMIN_USER = {
     "username": "9999-9999-9999-9999",
     "first_name": "Admin",
@@ -43,6 +55,12 @@
     "is_first_login": True,
 }
 
+ADMIN_USER_DECODED_JWT = {
+    "sub": ADMIN_USER["username"],
+    "given_name": ADMIN_USER["first_name"],
+    "family_name": ADMIN_USER["last_name"],
+}
+
 TEST_EXPERIMENT = {
     "title": "Test Title",
     "short_description": "Test experiment",

tests/helpers/util.py

@@ -175,3 +175,26 @@ def publish_score_set(client, score_set_urn):
     response_data = response.json()
     jsonschema.validate(instance=response_data, schema=ScoreSet.schema())
     return response_data
+
+
+def create_api_key_for_current_user(client):
+    response = client.post("api/v1/users/me/access-keys")
+    assert response.status_code == 200
+    return response.json()["keyId"]
+
+
+def create_admin_key_for_current_user(client):
+    response = client.post("api/v1/users/me/access-keys/admin")
+    assert response.status_code == 200
+    return response.json()["keyId"]
+
+
+def mark_user_inactive(session, username):
+    user = session.query(User).where(User.username == username).one()
+    user.is_active = False
+
+    session.add(user)
+    session.commit()
+    session.refresh(user)
+
+    return user

tests/lib/conftest.py

@@ -3,8 +3,10 @@
 from mavedb.models.license import License
 from mavedb.models.taxonomy import Taxonomy
 from mavedb.models.user import User
+from mavedb.models.role import Role
+from mavedb.models.enums.user_role import UserRole
 
-from tests.helpers.constants import EXTRA_USER, TEST_LICENSE, TEST_TAXONOMY, TEST_USER
+from tests.helpers.constants import ADMIN_USER, EXTRA_USER, TEST_LICENSE, TEST_TAXONOMY, TEST_USER
 
 
 @pytest.fixture
@@ -16,6 +18,7 @@ def setup_lib_db(session):
     db = session
     db.add(User(**TEST_USER))
     db.add(User(**EXTRA_USER))
+    db.add(User(**ADMIN_USER, role_objs=[Role(name=UserRole.admin)]))
     db.add(Taxonomy(**TEST_TAXONOMY))
     db.add(License(**TEST_LICENSE))
     db.commit()