Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate legacy endpoints #36

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions biothings_annotator/application/views/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
from typing import Dict, List
from biothings_annotator.application.views.annotator import (
BatchCurieView,
CurieLegacyView,
CurieView,
StatusView,
TrapiLegacyView,
TrapiView,
)
from biothings_annotator.application.views.metadata import MetadataView, VersionView
Expand Down Expand Up @@ -36,12 +34,6 @@ def build_routes() -> List[Dict]:
"name": "curie_endpoint",
}

curie_route_mirror = {
"handler": CurieLegacyView.as_view(),
"uri": r"/annotator/<curie:str>",
"name": "curie_endpoint_mirror",
}

batch_curie_route = {
"handler": BatchCurieView.as_view(),
"uri": r"/curie/",
Expand All @@ -50,7 +42,6 @@ def build_routes() -> List[Dict]:

# --- TRAPI ROUTES ---
trapi_route_main = {"handler": TrapiView.as_view(), "uri": "/trapi/", "name": "trapi_endpoint"}
trapi_route_mirror = {"handler": TrapiLegacyView.as_view(), "uri": "/annotator/", "name": "trapi_endpoint_mirror"}

# --- METADATA ROUTES ---
metadata_route = {"handler": MetadataView.as_view(), "uri": "/metadata/openapi", "name": "metadata"}
Expand All @@ -65,10 +56,8 @@ def build_routes() -> List[Dict]:
status_route_main,
version_route_main,
curie_route_main,
curie_route_mirror,
batch_curie_route,
trapi_route_main,
trapi_route_mirror,
metadata_route,
]
return route_collection
19 changes: 0 additions & 19 deletions biothings_annotator/application/views/annotator.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,22 +430,3 @@ async def post(self, request: Request):
}
general_error_response = sanic.json(error_context, status=400)
return general_error_response


# --- Legacy Redirects ---
# The /annotator endpoint has been deprcated so we setup these redirects:
# GET /anotator/<CURIE_ID> -> /curie/<CURIE_ID> (Singular CURIE ID)
# POST /anotator/ -> /trapi/
class CurieLegacyView(HTTPMethodView):
@openapi.deprecated()
async def get(self, request: Request, curie: str):
redirect_response = response.redirect(f"/curie/{curie}", status=302)
return redirect_response


class TrapiLegacyView(HTTPMethodView):
@openapi.deprecated()
async def post(self, request: Request):
trapi_view = TrapiView()
redirect_response = await trapi_view.post(request)
return redirect_response
153 changes: 24 additions & 129 deletions tests/test_application_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@


@pytest.mark.unit
@pytest.mark.parametrize("endpoint", ["/status/"])
def test_status_get(test_annotator: sanic.Sanic, endpoint: str):
def test_status_get(test_annotator: sanic.Sanic):
"""
Tests the Status endpoint GET when the response is HTTP 200
"""
endpoint = "/status/"
request, response = test_annotator.test_client.request(endpoint, http_method="get")

assert request.method == "GET"
Expand All @@ -37,11 +37,11 @@ def test_status_get(test_annotator: sanic.Sanic, endpoint: str):


@pytest.mark.unit
@pytest.mark.parametrize("endpoint", ["/status/"])
def test_status_head(test_annotator: sanic.Sanic, endpoint: str):
def test_status_head(test_annotator: sanic.Sanic):
"""
Tests the Status endpoint HEAD when the response is HTTP 200
"""
endpoint = "/status/"
request, response = test_annotator.test_client.request(endpoint, http_method="head")

assert request.method == "HEAD"
Expand All @@ -60,12 +60,12 @@ def test_status_head(test_annotator: sanic.Sanic, endpoint: str):


@pytest.mark.unit
@pytest.mark.parametrize("endpoint", ["/status/"])
def test_status_get_error(test_annotator: sanic.Sanic, endpoint: str):
def test_status_get_error(test_annotator: sanic.Sanic):
"""
Tests the Status endpoint GET when an Exception is raised
Mocking the annotate_curie method to raise an exception
"""
endpoint = "/status/"
with patch.object(Annotator, "annotate_curie", side_effect=Exception("Simulated error")):
request, response = test_annotator.test_client.request(endpoint, http_method="get")

Expand All @@ -86,12 +86,13 @@ def test_status_get_error(test_annotator: sanic.Sanic, endpoint: str):


@pytest.mark.unit
@pytest.mark.parametrize("endpoint", ["/status/"])
def test_status_get_failed_data_check(test_annotator: sanic.Sanic, endpoint: str):
def test_status_get_failed_data_check(test_annotator: sanic.Sanic):
"""
Tests the Status endpoint GET when the data check fails
Mocking the annotate_curie method to return a value that doesn't contain "NCBIGene:1017"
"""
endpoint = "/status/"

# Mock the return value to simulate the data check failure
with patch.object(Annotator, "annotate_curie", return_value={"_id": "some_other_id"}):
request, response = test_annotator.test_client.request(endpoint, http_method="get")
Expand Down Expand Up @@ -231,40 +232,35 @@ def test_curie_get(temporary_data_storage: Union[str, Path], test_annotator: san


@pytest.mark.parametrize(
"endpoint, batch_curie",
"batch_curie",
(
[
"/curie/",
[
"NCBIGene:695",
"MONDO:0001222",
"DOID:6034",
"CHEMBL.COMPOUND:821",
"PUBCHEM.COMPOUND:3406",
"CHEBI:192712",
"CHEMBL.COMPOUND:3707246",
],
{
"ids": [
"NCBIGene:695",
"MONDO:0001222",
"DOID:6034",
"CHEMBL.COMPOUND:821",
"PUBCHEM.COMPOUND:3406",
"CHEBI:192712",
"CHEMBL.COMPOUND:3707246",
],
],
[
"/curie/",
{
"ids": [
"NCBIGene:695",
"MONDO:0001222",
"DOID:6034",
"CHEMBL.COMPOUND:821",
"PUBCHEM.COMPOUND:3406",
"CHEBI:192712",
"CHEMBL.COMPOUND:3707246",
]
},
],
]
},
),
)
def test_curie_post(test_annotator: sanic.Sanic, endpoint: str, batch_curie: Union[List, Dict]):
def test_curie_post(test_annotator: sanic.Sanic, batch_curie: Union[List, Dict]):
"""
Tests the CURIE endpoint POST
"""
endpoint = "/curie/"
if isinstance(batch_curie, list):
curie_ids = set(batch_curie)
elif isinstance(batch_curie, dict):
Expand Down Expand Up @@ -345,104 +341,3 @@ def test_trapi_post(temporary_data_storage: Union[str, Path], test_annotator: sa
assert identifier is not None
score = values.get("_score", None)
assert score is not None


@pytest.mark.unit
@pytest.mark.parametrize("data_store", ["expected_curie.json"])
def test_annotator_get_redirect(
temporary_data_storage: Union[str, Path], test_annotator: sanic.Sanic, data_store: Dict
):
"""
Tests the legacy endpoint /annotator with a redirect to the
/curie/ endpoint
"""
data_file_path = temporary_data_storage.joinpath(data_store)
with open(str(data_file_path), "r", encoding="utf-8") as file_handle:
expected_curie_body = json.load(file_handle)

endpoint = "/annotator/"
curie_id = "NCBIGene:1017"
url = f"{endpoint}{curie_id}"
request, response = test_annotator.test_client.request(
url, http_method="get", follow_redirects=True, allow_redirects=True
)

assert request.method == "GET"
assert request.is_safe
assert request.query_string == ""
assert request.scheme == "http"
assert request.server_path == url

response_body = response.json
response_body[curie_id][0].pop("_score")
assert response.json == expected_curie_body

assert response.http_version == "HTTP/1.1"
assert response.content_type == "application/json"
assert response.is_success
assert not response.is_error
assert response.is_closed
assert response.status_code == 200
assert response.encoding == "utf-8"


@pytest.mark.unit
@pytest.mark.parametrize("data_store", ["trapi_request.json"])
def test_annotator_post_redirect(
temporary_data_storage: Union[str, Path], test_annotator: sanic.Sanic, data_store: Dict
):
"""
Tests the annotator redirect for the /trapi/ POST endpoint
"""
data_file_path = temporary_data_storage.joinpath(data_store)
with open(str(data_file_path), "r", encoding="utf-8") as file_handle:
trapi_body = json.load(file_handle)

endpoint = "/annotator/"
request, response = test_annotator.test_client.request(
endpoint, http_method="post", json=trapi_body, follow_redirects=True, allow_redirects=True
)

assert request.method == "POST"
assert request.query_string == ""
assert request.scheme == "http"
assert request.server_path == endpoint

assert response.http_version == "HTTP/1.1"
assert response.content_type == "application/json"
assert response.is_success
assert not response.is_error
assert response.is_closed
assert response.status_code == 200
assert response.encoding == "utf-8"

node_set = set(trapi_body["message"]["knowledge_graph"]["nodes"].keys())

annotation = response.json
assert isinstance(annotation, dict)
for key, attribute in annotation.items():
assert key in node_set
if isinstance(attribute, dict):
attribute = [attribute]
assert isinstance(attribute, list)
for subattribute in attribute:
assert isinstance(subattribute, dict)
subattribute_collection = subattribute.get("attributes", None)

assert isinstance(subattribute_collection, list)
for subattributes in subattribute_collection:
assert subattributes["attribute_type_id"] == "biothings_annotations"
assert isinstance(subattributes["value"], list)
for values in subattributes["value"]:
notfound = values.get("notfound", False)
if notfound:
curie_prefix, query = utils.parse_curie(key)
assert values == {"query": query, "notfound": True}
else:
assert isinstance(values, dict)
query = values.get("query", None)
assert query is not None
identifier = values.get("_id", None)
assert identifier is not None
score = values.get("_score", None)
assert score is not None
Loading