Repository navigation
Update OpenAPI for SmartAPI registration and preserve multi-Babel behavior - #58
Merged
Merged
Conversation
The document was copied from NodeNorm Redis and never pruned, so it advertised TRAPI 1.5 (info.x-trapi), carried 145 schemas no path referenced, named RENCI's developer as the contact and sent readers to /get_curie_prefixes, which this service has never served. It also failed openapi-spec-validator: the /get_allowed_conflations response had no description, and FilterKgraphTopNParameters.max_edges declared maximum 0 (NCATSTranslator/NodeNormalization#409). Rewritten from scratch against build_handlers(): the six routes this service registers, nothing else. Responses that were `schema: {}` now have schemas (NormalizedNodes, SetIDResponse), the FastAPI 422 responses are replaced by the 400 the Tornado handlers raise, and the examples are taken from the live CI instance. The servers block lists the ci and test deployments at ITRB instead of RENCI. Contact is Will Liu and the team is DOGSURF (#35); infores and termsOfService are unchanged for now. info.version is bumped to 1.1.0 because /status reports it and the response shapes change in the next commit. handlers/curie_prefix.py was an unregistered copy of the semantic-types handler (its class is even named SemanticTypeHandler), so it goes too. Refs #35, #36 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GET /get_allowed_conflations finished a bare list, and Tornado's write()
refuses a top-level list ("Lists not accepted for security reasons"),
so it returned HTTP 500 on every deployment. It now returns
{"conflations": [...]}, which is also what NodeNorm Redis returns.
GET /get_semantic_types lower-cased every class ("biolink:namedthing");
NodeNorm Redis returns the Biolink class URI as is ("biolink:NamedThing").
/openapi.json now redirects to /webapp/openapi.json so the FastAPI
default address used by NodeNorm Redis, SmartAPI and Babel Validation
works here too; it is excluded from OpenTelemetry like the other static
paths.
The POST set-ID test posted a single object where the handler expects
a list of sets, and the normalized-nodes docstring showed a "curie" key
where the handler reads "curies"; both corrected. POST /get_setid still
answers with an object keyed "0", "1", ... (the same Tornado limitation);
returning a list as NodeNorm Redis does is a breaking change, so it is
left to its own pull request.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The document is hand-written and served as a static file, so these tests are what stops it drifting: the spec validates; its paths are exactly the routes build_handlers() registers; each path documents the GET/POST methods its handler defines; each GET documents the query parameters its handler reads (found by scanning the handler source for get_argument calls); and the SetIDResponse schema lists the dataclass's fields. Each was checked to fail when the spec is mutated. Also a regression test for /get_allowed_conflations, which returned HTTP 500 until the previous commit: it mounts just that handler and validates the body against the documented ConflationList schema. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
It ran only against the su10 Elasticsearch host before. This mounts the handler alone with a fake Elasticsearch client and validates the body against the documented schema. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The dot in each redirect pattern was unescaped, so /openapiXjson and /faviconXico redirected too. The OpenTelemetry exclusions already escape it. A test now checks the /openapi.json redirect and that a near-miss path is a 404. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment
The query-parameter check scanned only get() for get_argument calls, so
a parameter read in a helper method would drift unnoticed; it now scans
the whole handler class. A new check compares the fields each post()
reads with .get("...") against the properties of the documented request
body (CurieList, and the SetIDQuery items of the /get_setid array).
Both were confirmed to fail when a field is removed from the document or
a parameter is renamed in a helper.
The checks still only see literal names; the README now says so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
generate_setid() returned the SetIDResponse dataclass itself on the invalid-conflation and no-normalized-CURIEs paths. Tornado can only write dicts, so GET /get_setid?conflation=Bogus answered 500 and the documented `error` field never reached anyone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The body has to be a JSON array of {curies, conflations} objects. A
single object (the shape the old document described) passed the length
check and then failed with an AttributeError, and invalid JSON failed in
json.loads; both were 500s. They are now 400s that say what shape the
body must have.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The index mapping (for the Babel version) was read before the try
block, so an unreachable cluster raised and /status answered 500
instead of the documented {"status": "error"}. The read now happens
inside the try; babel_version and babel_version_url are null when the
mapping couldn't be read, and the document says so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The patterns were written r"/get_setid?", r"/status?" and so on. In a Tornado route that ? makes the last character optional, so /get_seti and /statu reached the handlers though the OpenAPI document lists only the full paths. test_openapi.py hid this by stripping the ? before comparing routes with the document; it now compares them as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The request-body check read only post()'s source, though the README says fields are found by scanning the handler, and the query-parameter check already scans the whole class. A body field read in a helper method was either missed or reported as undocumented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The methods check compared only GET and POST, so the HEAD handler on /get_allowed_conflations, which the document didn't list, went unnoticed. It now compares every method Tornado routes except OPTIONS (answered by the base handler for CORS), and the document lists HEAD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/test_openapi.py imports it directly; it was only installed because openapi-spec-validator happens to depend on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parse_boolean() treated every string except "false" as true, so
conflate=0 or conflate=no turned conflation on. The flags now accept the
strings FastAPI/Pydantic accepted in NodeNorm Redis (true/false, 1/0,
yes/no, on/off, t/f, y/n, in any case) and answer 400 for anything else,
including an empty value. POST bodies go through the same check, so
{"conflate": "false"} is no longer truthy. The document lists the
accepted values.
This is a behavior change: a caller sending an unrecognized value now
gets a 400 where it used to get conflation silently switched on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Covers the second path #47 describes: a set whose CURIEs normalize to nothing. get_normalized_nodes() is patched out, so it needs no host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
https://nodenorm-es.transltr.io/ is now live. It goes first, since Swagger UI and generated clients default to the first server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
json.loads() raises UnicodeDecodeError rather than JSONDecodeError for bytes that aren't valid UTF-8, so such a body escaped the check and became a 500, though the document promises a 400 for a malformed body. The decoding moves to a load_json_body() helper on the base handler so POST /get_normalized_nodes can use it too. Raised by Copilot on #42. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The handler called json.loads() with no error handling and then .get() on the result, so a body that wasn't JSON, wasn't UTF-8, or was an array instead of an object all became 500s. It now uses load_json_body() and checks for an object, and the document lists these under the 400 response. Same cause as the UTF-8 case Copilot raised for POST /get_setid on #42. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_document_is_valid_openapi() reads the file from the package, so a broken /webapp/ static route or /openapi.json redirect, or a wrong Content-Type, went unnoticed. The new test fetches /openapi.json through build_handlers(), follows the redirect, and validates the body. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
biolink/information-resource-registry#140 added infores:nodenorm for NodeNorm ES and renamed infores:sri-node-normalizer to NodeNorm Redis, so the two services no longer share an infores in SmartAPI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep multi-Babel selectors, selected-version headers, and the current status handler with HTTP 503 health failures. Update the OpenAPI sync checks for shared parameter references and per-method Tornado query readers, validate both release response shapes, and advance the API version to 1.2.0. Validation: 290 tests passed, with four Elasticsearch integration tests deselected; 12 subtests passed.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The equivalent-identifier schema rejects null types returned when individual_types=true and a clique’s source type is missing, leaving the response contract inconsistent with handler behavior.
1 open issue
What changed in this PR
Updates the Elasticsearch-backed NodeNorm API for SmartAPI registration while preserving multi-Babel release selection, response headers, and the existing status contract.
Changes:
- Replaces stale OpenAPI metadata and unused TRAPI schemas with endpoint contracts for API version 1.2.0.
- Corrects response serialization, semantic-type casing, input handling, and route matching; exposes
/openapi.json. - Adds offline schema validation and regression coverage, including multi-Babel responses.
| File | Description |
|---|---|
tests/test_set_identifiers.py |
Aligns POST requests and expectations with the batch contract. |
tests/test_openapi.py |
Adds document, handler-contract, and offline response validation. |
tests/test_babel_endpoints.py |
Updates semantic-type expectations across releases. |
tests/test_application.py |
Updates the exact status-route expectation. |
src/nodenorm/webapp/openapi.json |
Refreshes metadata, schemas, and endpoint contracts for version 1.2.0. |
src/nodenorm/handlers/set_identifiers.py |
Validates body structure and serializes early returns as dictionaries. |
src/nodenorm/handlers/semantic_types.py |
Preserves Biolink class casing. |
src/nodenorm/handlers/normalized_nodes.py |
Validates JSON body structure and parses boolean flags strictly. |
src/nodenorm/handlers/curie_prefix.py |
Removes the unused, unregistered handler. |
src/nodenorm/handlers/conflations.py |
Returns an object-wrapped conflation list and supports HEAD. |
src/nodenorm/handlers/base.py |
Adds shared JSON parsing with HTTP 400 errors. |
src/nodenorm/handlers/__init__.py |
Fixes route patterns and adds the OpenAPI redirect. |
src/nodenorm/config/config.default.json |
Excludes the OpenAPI redirect from tracing. |
README.md |
Documents version 1.2.0 and OpenAPI maintenance checks. |
pyproject.toml |
Adds OpenAPI and JSON Schema test dependencies. |
docker/configuration/config.json |
Applies the tracing exclusion to Docker deployments. |
Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

NodeNorm ES still publishes a copied OpenAPI document with stale service metadata, irrelevant TRAPI schemas, and endpoint contracts that do not match the Elasticsearch implementation. This updates the document for SmartAPI registration, corrects the affected endpoint responses and input handling, and exposes the document at
/openapi.json.Continues #42 with Gaurav Vaidya’s original commits for compatibility with the current
mainbranches:X-Babel-Versionheaders, and/statuscontract.The OpenAPI sync tests now resolve shared parameter references, recognize Tornado query readers, and check each HTTP method and its helpers separately. Offline response-schema checks cover both Babel releases and the current status response.
Validation: 290 tests passed, plus 12 subtests. Four integration tests requiring the
su10Elasticsearch host were excluded. OpenAPI file/served-document validation and diff whitespace checks pass.Closes #35. Closes #36. Closes #47. Supports the SmartAPI registration work in #34; registration follows production deployment.