Return POST /get_setid results as a JSON list, as NodeNorm Redis does

src/nodenorm/handlers/base.py

@@ -2,6 +2,7 @@
 
 import json
 
+import tornado.escape
 from biothings.web.handlers import BaseHandler
 from tornado.web import HTTPError
 
@@ -53,3 +54,13 @@ def load_json_body(self):
             return json.loads(self.request.body)
         except (json.JSONDecodeError, UnicodeDecodeError) as error:
             raise HTTPError(detail=f"The body is not valid JSON: {error}", status_code=400)
+
+    def write(self, chunk):
+        # Tornado refuses a top-level list ("Lists not accepted for security reasons"), a guard
+        # against JSON hijacking through <script> tags in very old browsers. This is a public
+        # JSON API that allows any origin anyway, and NodeNorm Redis returns lists, so encode a
+        # list the way Tornado encodes a dict.
+        if isinstance(chunk, list):
+            self.set_header("Content-Type", "application/json; charset=UTF-8")
+            chunk = tornado.escape.json_encode(chunk)
+        super().write(chunk)

src/nodenorm/handlers/conflations.py

@@ -7,7 +7,7 @@ class ValidConflationsHandler(NodeNormalizationBaseHandler):
     name = "allowed-conflations"
 
     async def get(self):
-        # Wrapped in an object, as NodeNorm Redis does. Tornado also refuses to write a bare list.
+        # Wrapped in an object, as NodeNorm Redis does.
         self.finish({"conflations": ALLOWED_CONFLATIONS})
 
     async def head(self):

src/nodenorm/handlers/set_identifiers.py

@@ -1,5 +1,6 @@
 # set_id.py
 # Code related to generating IDs for sets (as in https://github.com/TranslatorSRI/NodeNormalization/issues/256).
+import asyncio
 import dataclasses
 import logging
 import uuid
@@ -70,18 +71,19 @@ async def post(self):
 
         release = self.select_babel_release(self.get_query_argument("babel_version", None))
 
-        # We have to make a minor change to the API to ensure we're avoiding a security concern
-        # enforced by tornado, so we return a dictionary here instead of a list
-        set_identifiers = {}
-        for index, group in enumerate(post_body):
-            curies = group.get("curies", [])
-            conflations = group.get("conflations", [])
-            set_identifiers[index] = await generate_setid(
-                self.biothings, curies, conflations, search_indices=release.search_indices
+        # One SetIDResponse per input set, in order, as NodeNorm Redis returns. The sets are
+        # independent, so their Elasticsearch lookups run concurrently.
+        set_identifiers = await asyncio.gather(
+            *(
+                generate_setid(
+                    self.biothings,
+                    group.get("curies", []),
+                    group.get("conflations", []),
+                    search_indices=release.search_indices,
+                )
+                for group in post_body
             )
-
-        if not set_identifiers:
-            raise HTTPError(detail="Error occurred during processing.", status_code=500)
+        )
 
         self.finish(set_identifiers)
 

src/nodenorm/webapp/openapi.json

@@ -411,7 +411,7 @@
       "post": {
         "summary": "Normalize and deduplicate several sets of identifiers and return a hash for each set.",
         "tags": ["translator"],
-        "description": "Returns one set ID per input set, keyed by the set's position in the request as a string (\"0\", \"1\", ...). Select one Babel release for the entire batch with the optional babel_version query parameter.",
+        "description": "Returns one set ID per input set, in the order the sets were given. Select one Babel release for the entire batch with the optional babel_version query parameter.",
         "operationId": "get_setid_post",
         "requestBody": {
           "required": true,
@@ -434,12 +434,12 @@
         },
         "responses": {
           "200": {
-            "description": "One set ID per input set, keyed by its index",
+            "description": "One set ID per input set",
             "content": {
               "application/json": {
                 "schema": {
-                  "type": "object",
-                  "additionalProperties": {"$ref": "#/components/schemas/SetIDResponse"}
+                  "type": "array",
+                  "items": {"$ref": "#/components/schemas/SetIDResponse"}
                 }
               }
             },

tests/test_babel_endpoints.py

@@ -177,11 +177,11 @@ def test_setid_post_selects_one_release_for_all_groups_and_conflation(self):
         )
         body = self.assert_selected_release(response, OLD_VERSION)
 
-        assert set(body) == {"0", "1"}
-        assert body["0"]["normalized_curies"] == ["OLD:1", "OLD:2"]
-        assert body["0"]["normalized_string"] == "OLD:1||OLD:2"
-        assert body["1"]["normalized_curies"] == ["OLD:P2"]
-        assert body["1"]["conflations"] == ["GeneProtein"]
+        assert len(body) == 2
+        assert body[0]["normalized_curies"] == ["OLD:1", "OLD:2"]
+        assert body[0]["normalized_string"] == "OLD:1||OLD:2"
+        assert body[1]["normalized_curies"] == ["OLD:P2"]
+        assert body[1]["conflations"] == ["GeneProtein"]
         assert len(self.client.calls) == 3
         assert all(operation == "msearch" and call["index"] == [OLD_INDEX] for operation, call in self.client.calls)
         assert self.namespace.elasticsearch.indices == [DEFAULT_INDEX]
@@ -195,7 +195,7 @@ def test_setid_post_defaults_to_configured_release(self):
         )
 
         body = self.assert_selected_release(response, DEFAULT_VERSION)
-        assert body["0"]["normalized_curies"] == ["NEW:1"]
+        assert body[0]["normalized_curies"] == ["NEW:1"]
 
     def test_setid_rejects_unsupported_release_before_lookup(self):
         self.assert_unsupported(self.fetch("/get_setid?curie=INPUT:1&babel_version=unknown"))

tests/test_base_handler.py

@@ -0,0 +1,25 @@
+import json
+
+import tornado.web
+from tornado.testing import AsyncHTTPTestCase
+
+from nodenorm.handlers.base import NodeNormalizationBaseHandler
+
+
+class ListHandler(NodeNormalizationBaseHandler):
+    async def get(self):
+        self.finish([{"curie": "MESH:D014867"}, {"curie": "NCIT:C34373"}])
+
+
+class TestWriteList(AsyncHTTPTestCase):
+    """Tornado's own write() raises on a list, which made /get_allowed_conflations return HTTP 500."""
+
+    def get_app(self) -> tornado.web.Application:
+        return tornado.web.Application([(r"/list", ListHandler)])
+
+    def test_a_list_is_written_as_json(self):
+        response = self.fetch("/list")
+
+        assert response.code == 200
+        assert response.headers["Content-Type"] == "application/json; charset=UTF-8"
+        assert json.loads(response.body) == [{"curie": "MESH:D014867"}, {"curie": "NCIT:C34373"}]

tests/test_openapi.py

@@ -242,8 +242,8 @@ def test_post_reports_an_unknown_conflation(self):
 
         assert response.code == 200
         result = json.loads(response.body)
-        jsonschema.validate(result["0"], SPEC["components"]["schemas"]["SetIDResponse"])
-        assert "Bogus" in result["0"]["error"]
+        jsonschema.validate(result[0], SPEC["components"]["schemas"]["SetIDResponse"])
+        assert "Bogus" in result[0]["error"]
 
     def test_post_answers_a_set_with_no_curies(self):
         # The other early return in generate_setid(); it too used to return the dataclass (HTTP 500).
@@ -253,12 +253,19 @@ def test_post_answers_a_set_with_no_curies(self):
 
         assert response.code == 200
         result = json.loads(response.body)
-        jsonschema.validate(result["0"], SPEC["components"]["schemas"]["SetIDResponse"])
-        assert result["0"]["normalized_curies"] == [] and result["0"]["setid"] is None
+        jsonschema.validate(result[0], SPEC["components"]["schemas"]["SetIDResponse"])
+        assert result[0]["normalized_curies"] == [] and result[0]["setid"] is None
 
     def test_post_rejects_a_body_that_is_not_an_array_of_objects(self):
         # A single object was the documented body before; it used to fail with a 500.
-        for body in ('{"curies": ["MESH:D014867"]}', '["MESH:D014867"]', "[]", "not json", b'[{"curies": ["\xff"]}]'):
+        for body in (
+            '{"curies": ["MESH:D014867"]}',
+            '["MESH:D014867"]',
+            "[]",
+            "not json",
+            "",
+            b'[{"curies": ["\xff"]}]',
+        ):
             assert self.fetch("/get_setid", method="POST", body=body).code == 400, body
 
 
@@ -446,3 +453,35 @@ def test_post_rejects_a_body_that_is_not_a_json_object(self):
         # Each of these used to fail with a 500.
         for body in ('["MESH:D014867"]', "not json", b'{"curies": ["\xff"]}'):
             assert self.fetch("/get_normalized_nodes", method="POST", body=body).code == 400, body
+
+
+async def fake_generate_setid(biothings, curies, conflations, search_indices=None):
+    return dataclasses.asdict(SetIDResponse(curies=curies, conflations=conflations, normalized_curies=sorted(curies)))
+
+
+class TestSetIdentifierPost(AsyncHTTPTestCase):
+    """Regression: POST returned an index-keyed object because Tornado refused to write a list."""
+
+    def setUp(self):
+        # Patched per test and undone afterwards: assigning the module attribute instead would leak
+        # the fake into tests/test_set_identifiers.py whenever pytest-randomly runs it later.
+        patcher = mock.patch("nodenorm.handlers.set_identifiers.generate_setid", fake_generate_setid)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        super().setUp()
+
+    def get_app(self) -> tornado.web.Application:
+        app = tornado.web.Application([(r"/get_setid", SetIdentifierHandler)])
+        app.biothings = SimpleNamespace(elasticsearch=SimpleNamespace(babel=RELEASES))
+        return app
+
+    def test_returns_one_response_per_set_as_a_list(self):
+        sets = [{"curies": ["MESH:D014867"]}, {"curies": ["NCIT:C34373"], "conflations": ["GeneProtein"]}]
+        response = self.fetch("/get_setid", method="POST", body=json.dumps(sets))
+
+        assert response.code == 200
+        assert response.headers["Content-Type"].startswith("application/json")
+        body = json.loads(response.body)
+        assert isinstance(body, list) and [r["curies"] for r in body] == [s["curies"] for s in sets]
+        for result in body:
+            jsonschema.validate(result, SPEC["components"]["schemas"]["SetIDResponse"])

tests/test_set_identifiers.py

@@ -95,15 +95,15 @@ def test_post_endpoint(self):
         response = yield http_client.fetch(url, self.stop, method="POST", headers=headers, body=body, request_timeout=0)
         body = json.loads(response.body.decode("utf-8"))
 
-        # One SetIDResponse per input set, keyed by the set's index in the request.
-        expected_body = {
-            "0": {
+        # One SetIDResponse per input set, as a list (matching NodeNorm Redis).
+        expected_body = [
+            {
                 "curies": ["MESH:D014867", "NCIT:C34373", "UNII:63M8RYN44N", "RUBBISH:1234"],
                 "conflations": ["GeneProtein", "DrugChemical"],
                 "error": None,
                 "normalized_curies": ["CHEBI:15377", "MONDO:0004976", "RUBBISH:1234"],
                 "normalized_string": "CHEBI:15377||MONDO:0004976||RUBBISH:1234",
                 "setid": "uuid:771d3c09-9a8c-5c46-8b85-97f481a90d40",
             }
-        }
+        ]
         assert json.loads(response.body.decode("utf-8")) == expected_body