From 1d44a698dec9049e16c6b3fb29f71dd14cae5ac4 Mon Sep 17 00:00:00 2001 From: Alexander Berger Date: Mon, 4 Nov 2024 11:26:10 -0500 Subject: [PATCH 1/4] Fixes for QA feedback on API changes --- src/geneweaver/api/controller/genesets.py | 79 ++++------------------- src/geneweaver/api/services/geneset.py | 15 ++++- 2 files changed, 24 insertions(+), 70 deletions(-) diff --git a/src/geneweaver/api/controller/genesets.py b/src/geneweaver/api/controller/genesets.py index f6f96f2..f1998d9 100644 --- a/src/geneweaver/api/controller/genesets.py +++ b/src/geneweaver/api/controller/genesets.py @@ -20,6 +20,7 @@ from typing_extensions import Annotated from . import message as api_message +from .utilities import raise_http_error router = APIRouter(prefix="/genesets", tags=["genesets"]) @@ -141,11 +142,7 @@ def get_visible_genesets( offset=offset, ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(response) return response @@ -189,11 +186,7 @@ def get_geneset( cursor=cursor, geneset_id=geneset_id, user=user, in_threshold=in_threshold ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(response) return response @@ -217,11 +210,7 @@ def get_geneset_values( in_threshold=in_threshold, ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(response) if response.get("data") is None: raise HTTPException( @@ -254,10 +243,7 @@ def get_export_geneset_by_id_type( response = genset_service.get_geneset(cursor, geneset_id, user) if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(response) id_type = response.get("gene_identifier_type") if id_type: @@ -296,11 +282,7 @@ def get_geneset_metadata( cursor, geneset_id, user, include_pub_info ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(response) return response @@ -317,10 +299,7 @@ def get_publication_for_geneset( geneset_resp = genset_service.get_geneset_metadata(cursor, geneset_id, user, True) if "error" in geneset_resp: - if geneset_resp.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - else: - raise HTTPException(status_code=500, detail=api_message.UNEXPECTED_ERROR) + raise_http_error(geneset_resp) geneset = geneset_resp.get("geneset") if geneset is None: @@ -352,9 +331,7 @@ def put_geneset_threshold( cursor, geneset_id, gene_score_type, user ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) + raise_http_error(response) @router.get("/{geneset_id}/ontologies") @@ -388,14 +365,7 @@ def get_geneset_ontology_terms( cursor, geneset_id, user, limit, offset ) - if "error" in terms_resp: - if terms_resp.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - - if terms_resp.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN: - raise HTTPException( - status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN - ) + raise_http_error(terms_resp) return terms_resp @@ -423,22 +393,7 @@ def put_geneset_ontology_term( user=user, ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - - if response.get("message") == api_message.RECORD_NOT_FOUND_ERROR: - raise HTTPException( - status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR - ) - - if response.get("message") == api_message.RECORD_EXISTS: - raise HTTPException(status_code=412, detail=api_message.RECORD_EXISTS) - - if response.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN: - raise HTTPException( - status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN - ) + raise_http_error(response) @router.delete("/{geneset_id}/ontologies/{ontology_id}", status_code=204) @@ -464,16 +419,4 @@ def delete_geneset_ontology_term( user=user, ) - if "error" in response: - if response.get("message") == api_message.ACCESS_FORBIDDEN: - raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN) - - if response.get("message") == api_message.RECORD_NOT_FOUND_ERROR: - raise HTTPException( - status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR - ) - - if response.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN: - raise HTTPException( - status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN - ) + raise_http_error(response) diff --git a/src/geneweaver/api/services/geneset.py b/src/geneweaver/api/services/geneset.py index b1c40ef..c908a15 100644 --- a/src/geneweaver/api/services/geneset.py +++ b/src/geneweaver/api/services/geneset.py @@ -196,6 +196,10 @@ def get_geneset_metadata( gs_id=geneset_id, with_publication_info=include_pub_info, ) + + if len(results) <= 0: + return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} + return {"geneset": results[0]} except Exception as err: @@ -221,8 +225,9 @@ def get_geneset( gs_id=geneset_id, with_publication_info=False, ) + if len(results) <= 0: - return {"data": None} + return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} geneset = results[0] geneset_values = db_geneset_value.by_geneset_id( @@ -260,8 +265,9 @@ def get_geneset_gene_values( is_readable_by=determine_user_id(user), with_publication_info=False, ) + if len(results) <= 0: - return {"data": None} + return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} # If gene id type is given, check gene species homology to # construct proper gene species mapping @@ -316,6 +322,10 @@ def get_geneset_w_gene_id_type( gs_id=geneset_id, with_publication_info=False, ) + + if len(results) <= 0: + return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} + geneset = results[0] geneset_values = get_gsv_w_gene_homology_update( cursor=cursor, @@ -444,6 +454,7 @@ def get_geneset_ontology_terms( is_gs_readable = db_geneset.is_readable( cursor=cursor, user_id=determine_user_id(user), geneset_id=geneset_id ) + if is_gs_readable is False: return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} From 99008e24b7c6aab28749dec4bb58e12033fce816 Mon Sep 17 00:00:00 2001 From: Alexander Berger Date: Tue, 5 Nov 2024 10:26:06 -0500 Subject: [PATCH 2/4] Fixing tests --- tests/services/test_genset.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/services/test_genset.py b/tests/services/test_genset.py index 4dda38f..d4a9b68 100644 --- a/tests/services/test_genset.py +++ b/tests/services/test_genset.py @@ -38,13 +38,15 @@ def test_get_geneset_no_user_access(mock_db_geneset): """Test get geneset by ID with no user access.""" mock_db_geneset.get.return_value = [] response = geneset.get_geneset(None, 1234, None) - assert "data" in response - assert response["data"] is None + assert "error" in response + assert "data" not in response + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} mock_db_geneset.get.return_value = [] response = geneset.get_geneset(None, 1234, mock_user) - assert "data" in response - assert response["data"] is None + assert "error" in response + assert "data" not in response + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} @patch("geneweaver.api.services.geneset.db_geneset") @@ -138,18 +140,20 @@ def test_get_geneset_w_gene_id_type_no_user(mock_db_geneset, mock_gsv): response = geneset.get_geneset_w_gene_id_type(None, 1234, None, GeneIdentifier(2)) assert response is not None assert isinstance(response, dict) - assert "gene_identifier_type" in response - assert "geneset" in response - assert "geneset_values" in response + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} + assert "gene_identifier_type" not in response + assert "geneset" not in response + assert "geneset_values" not in response response = geneset.get_geneset_w_gene_id_type( None, 1234, User(id=None), GeneIdentifier(2) ) assert response is not None assert isinstance(response, dict) - assert "gene_identifier_type" in response - assert "geneset" in response - assert "geneset_values" in response + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} + assert "gene_identifier_type" not in response + assert "geneset" not in response + assert "geneset_values" not in response @patch("geneweaver.api.services.geneset.db_geneset") @@ -402,8 +406,9 @@ def test_get_geneset_gene_values_invalid_user( response = geneset.get_geneset_gene_values( None, user=None, geneset_id=1234, gene_id_type=None ) - assert "data" in response - assert response["data"] is None + assert "error" in response + assert "data" not in response + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} @patch("geneweaver.api.services.geneset.db_geneset") @@ -499,7 +504,7 @@ def test_geneset_gene_value_w_gene_id_type_none_resp2(mock_db_geneset): response = geneset.get_geneset_gene_values( None, user=mock_user, geneset_id=1234, gene_id_type=GeneIdentifier(2) ) - assert response == {"data": None} + assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN} @patch("geneweaver.api.services.geneset.db_geneset") From e59082fa7fa2fbced1bae204010de09e506c6a34 Mon Sep 17 00:00:00 2001 From: Alexander Berger Date: Tue, 5 Nov 2024 10:28:35 -0500 Subject: [PATCH 3/4] Adding controller utilities module --- src/geneweaver/api/controller/utilities.py | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 src/geneweaver/api/controller/utilities.py diff --git a/src/geneweaver/api/controller/utilities.py b/src/geneweaver/api/controller/utilities.py new file mode 100644 index 0000000..2481026 --- /dev/null +++ b/src/geneweaver/api/controller/utilities.py @@ -0,0 +1,36 @@ +"""Utilities for FastAPI Controller.""" + +from fastapi import HTTPException + +from . import message as api_message + +EXCEPTION_MAP = { + api_message.ACCESS_FORBIDDEN: HTTPException( + status_code=403, detail=api_message.ACCESS_FORBIDDEN + ), + api_message.INACCESSIBLE_OR_FORBIDDEN: HTTPException( + status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN + ), + api_message.RECORD_NOT_FOUND_ERROR: HTTPException( + status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR + ), + api_message.RECORD_EXISTS: HTTPException( + status_code=412, detail=api_message.RECORD_EXISTS + ), +} + + +def raise_http_error(response: dict) -> None: + """Raise HTTPException based on response message. + + :param response: dict + + raises: HTTPException if "error" key is in response dict. + """ + if "error" in response: + try: + raise EXCEPTION_MAP[response.get("message")] + except KeyError: + raise HTTPException( + status_code=500, detail=api_message.UNEXPECTED_ERROR + ) from None From f90acea78b097aae69cf66ed5b58edd24ba7a6de Mon Sep 17 00:00:00 2001 From: Alexander Berger Date: Tue, 5 Nov 2024 10:50:00 -0500 Subject: [PATCH 4/4] Bumping pre-release version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9e5f997..3fec539 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "geneweaver-api" -version = "0.11.0a0" +version = "0.11.0a1" description = "The Geneweaver API" authors = [ "Alexander Berger ",