Merge pull request #44 from TheJacksonLaboratory/task/add-api-standar…

…ds-tests

Adding api standards tests

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "geneweaver-api"
-version = "0.4.0a5"
+version = "0.4.0a6"
 description = "The Geneweaver API"
 authors = [
     "Alexander Berger <[email protected]>",
@@ -18,10 +18,10 @@ packages = [
 [tool.poetry.dependencies]
 python = "^3.9"
 
-geneweaver-core = "^0.9.0a1"
+geneweaver-core = "^0.9.0a8"
 fastapi = {extras = ["all"], version = "^0.99.1"}
 uvicorn = {extras = ["standard"], version = "^0.24.0"}
-geneweaver-db = "^0.3.0a12"
+geneweaver-db = "^0.3.0a16"
 psycopg-pool = "^3.1.7"
 requests = "^2.31.0"
 python-jose = {extras = ["cryptography"], version = "^3.3.0"}

src/geneweaver/api/controller/genes.py

@@ -70,7 +70,7 @@ def get_gene_preferred(
     return response
 
 
-@router.post("/homologs", response_model=GeneIdMappingResp)
+@router.post("/homologs", response_model=GeneIdMappingResp, deprecated=True)
 def get_related_gene_ids(
     gene_id_mapping: GeneIdHomologReq,
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
@@ -91,7 +91,7 @@ def get_related_gene_ids(
     return gene_id_mapping_resp
 
 
-@router.post("/mapping", response_model=GeneIdMappingResp)
+@router.post("/mappings", response_model=GeneIdMappingResp)
 def get_genes_mapping(
     gene_id_mapping: GeneIdMappingReq,
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
@@ -110,7 +110,7 @@ def get_genes_mapping(
     return gene_id_mapping_resp
 
 
-@router.post("/mapping/aon", response_model=GeneIdMappingResp)
+@router.post("/mappings/aon", response_model=GeneIdMappingResp)
 def get_genes_mapping_aon(
     gene_id_mapping: GeneIdMappingAonReq,
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),

src/geneweaver/api/controller/genesets.py

@@ -26,7 +26,7 @@ def get_visible_genesets(
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
 ) -> dict:
     """Get all visible genesets."""
-    user_genesets = db_geneset.by_user_id(cursor, user.id)
+    user_genesets = db_geneset.by_owner_id(cursor, user.id)
     return {"genesets": user_genesets}
 
 

src/geneweaver/api/controller/publications.py

@@ -11,28 +11,21 @@
 router = APIRouter(prefix="/publications", tags=["publications"])
 
 
-@router.get("/{pub_id}")
+@router.get("/{publication_id}")
 def get_publication_by_id(
-    pub_id: Annotated[
+    publication_id: Annotated[
         int, Path(format="int64", minimum=0, maxiumum=9223372036854775807)
     ],
+    as_pubmed_id: Optional[bool] = True,
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
 ) -> dict:
     """Get a publication by id."""
-    response = publication_service.get_publication(cursor, pub_id)
-
-    if response.get("publication") is None:
-        raise HTTPException(status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR)
-
-    return response
-
-
-@router.get("/pubmed/{pubmed_id}")
-def get_publication_by_pubmed_id(
-    pubmed_id: str, cursor: Optional[deps.Cursor] = Depends(deps.cursor)
-) -> dict:
-    """Get a publication by id."""
-    response = publication_service.get_publication_by_pubmed_id(cursor, pubmed_id)
+    if as_pubmed_id:
+        response = publication_service.get_publication_by_pubmed_id(
+            cursor, publication_id
+        )
+    else:
+        response = publication_service.get_publication(cursor, publication_id)
 
     if response.get("publication") is None:
         raise HTTPException(status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR)

src/geneweaver/api/controller/species.py

@@ -1,34 +1,31 @@
 """Endpoints related to species."""
-from typing import Optional
+from typing import List, Optional
 
 from fastapi import APIRouter, Depends, Query
 from geneweaver.api import dependencies as deps
 from geneweaver.api.services import species as species_service
 from geneweaver.core.enum import GeneIdentifier, Species
+from geneweaver.core.schema.species import Species as SpeciesSchema
 from typing_extensions import Annotated
 
 router = APIRouter(prefix="/species", tags=["species"])
 
 
-@router.get("/")
+@router.get("")
 def get_species(
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
     taxonomy_id: Annotated[
         Optional[int], Query(format="int64", minimum=0, maxiumum=9223372036854775807)
     ] = None,
     reference_gene_id_type: Optional[GeneIdentifier] = None,
-) -> dict:
+) -> List[SpeciesSchema]:
     """Get species."""
-    response = species_service.get_species(cursor, taxonomy_id, reference_gene_id_type)
-
-    return response
+    return species_service.get_species(cursor, taxonomy_id, reference_gene_id_type)
 
 
 @router.get("/{species_id}")
 def get_species_by_id(
     species_id: Species, cursor: Optional[deps.Cursor] = Depends(deps.cursor)
-) -> dict:
+) -> SpeciesSchema:
     """Get species."""
-    response = species_service.get_species_by_id(cursor, species_id)
-
-    return response
+    return species_service.get_species_by_id(cursor, species_id)

src/geneweaver/api/services/species.py

@@ -12,7 +12,7 @@ def get_species(
     cursor: Cursor,
     taxonomy_id: Optional[int] = None,
     reference_gene_id_type: Optional[GeneIdentifier] = None,
-) -> dict:
+) -> list:
     """Get species from DB.
 
     @param cursor: DB cursor
@@ -21,11 +21,7 @@ def get_species(
     @return: dictionary response (species).
     """
     try:
-        species = db_species.get(cursor, taxonomy_id, reference_gene_id_type)
-        for species_record in species:
-            decode_gene_identifier(species_record)
-
-        return {"species": species}
+        return db_species.get(cursor, taxonomy_id, reference_gene_id_type)
 
     except Exception as err:
         logger.error(err)
@@ -40,10 +36,7 @@ def get_species_by_id(cursor: Cursor, species: Species) -> dict:
     @return: dictionary response (species).
     """
     try:
-        species_rsp = db_species.get_by_id(cursor, species)
-        decode_gene_identifier(species_rsp)
-
-        return {"species": species_rsp}
+        return db_species.get_by_id(cursor, species)
 
     except Exception as err:
         logger.error(err)

tests/controllers/test_api_standards.py

@@ -0,0 +1,130 @@
+"""API Standards Tests.
+
+https://devops.jax.org/Development/Best_Practices/API_Standards/http_standards/
+"""
+import re
+
+import pytest
+from fastapi.testclient import TestClient
+from geneweaver.api.core.config_class import GeneweaverAPIConfig
+
+CURLY_BRACE_REGEX = re.compile(r"\{[^}]*\}|[^{}_]+")
+
+
+def get_openapi_json():
+    """Get the openapi json file for parameterizing tests."""
+    from unittest.mock import patch
+
+    config = GeneweaverAPIConfig(
+        DB_HOST="localhost",
+        DB_USERNAME="postgres",
+        DB_PASSWORD="postgres",
+        DB_NAME="geneweaver",
+    )
+
+    with (
+        patch("geneweaver.api.core.config_class.GeneweaverAPIConfig", lambda: config),
+        patch("geneweaver.api.dependencies.lifespan", lambda app: None),
+    ):
+        from geneweaver.api.main import app
+
+        test_app = TestClient(app)
+        resp = test_app.get(f"{config.API_PREFIX}/openapi.json")
+
+    return resp.json()
+
+
+OPENAPI_JSON = get_openapi_json()
+PATH_KEYS = OPENAPI_JSON["paths"].keys()
+
+
+@pytest.fixture()
+def openapi_json():
+    """Provide the auto-rendered openapi.json file from the test client."""
+    return OPENAPI_JSON
+
+
+@pytest.fixture(params=PATH_KEYS)
+def openapi_json_path(request):
+    """Return a path from the openapi.json file."""
+    return request.param
+
+
+def test_defines_2xx_response(openapi_json, openapi_json_path):
+    """Test that the OpenAPI spec defines a 2xx response."""
+    path = openapi_json_path
+    path_details = openapi_json["paths"][path]
+    for method, method_details in path_details.items():
+        assert any(
+            "2" in response_code for response_code in method_details["responses"].keys()
+        ), (
+            f"Response for {method.upper()} {path} "
+            f"does not contain a 2xx status code"
+        )
+
+
+def test_no_underscores(openapi_json_path):
+    """Test that no underscores are present in API endpoint names."""
+    segments = re.findall(CURLY_BRACE_REGEX, openapi_json_path)
+
+    assert not any(
+        "_" in segment
+        for segment in segments
+        if "{" not in segment and "}" not in segment
+    ), (openapi_json_path + "contains underscores in endpoint name")
+
+
+def test_no_trailing_slashes(openapi_json_path):
+    """Test that no trailing slashes are present in API endpoint names."""
+    assert not openapi_json_path.endswith(
+        "/"
+    ), f"{openapi_json_path} contains a trailing slash"
+
+
+def test_endpoint_names_are_plural(openapi_json_path):
+    """Test that all endpoint names are in plural form."""
+    url = openapi_json_path
+    segments = url.split("/")
+    n_segments = len(segments)
+    while len(segments) > 0:
+        segment = segments.pop()
+        if segment == "":
+            continue
+        if "{" in segment:
+            continue
+        if segment == "api":
+            continue
+        if len(segments) == n_segments - 1:
+            continue
+        else:
+            assert segment.endswith("s"), f"{segment} in {url} is not in plural form"
+
+
+def test_no_uppercase_letters(openapi_json_path):
+    """Test that no uppercase letters are present in API endpoint names."""
+    url = openapi_json_path
+    assert url == url.lower(), f"{url} contains uppercase letters"
+
+
+@pytest.mark.skip(reason="Not all endpoints have a response schema defined")
+def test_should_define_response_schema(openapi_json, openapi_json_path):
+    """Test that all endpoints define a response schema."""
+    path = openapi_json_path
+    path_details = openapi_json["paths"][path]
+    for method, method_details in path_details.items():
+        for response_code, response_details in method_details["responses"].items():
+            if response_code == "200":
+                if "content" not in response_details:
+                    continue
+                assert (
+                    "schema" in response_details["content"]["application/json"]
+                ), f"Response for {path} {method} does not contain a schema"
+                assert (
+                    response_details["content"]["application/json"]["schema"].get(
+                        "type"
+                    )
+                    != "object"
+                ), (
+                    f"Response for {method.upper()} {path} uses 'object' type "
+                    "as schema. This is too vague."
+                )