From 5949e2f9d44a7b5cad7182737dfbd5805f8737c3 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 17 Jul 2026 19:32:46 -0400 Subject: [PATCH 1/4] Add configurable minimum query length for lookups Single-character lookups make Solr work hard while never returning a sensible result. Reject queries shorter than a configurable minimum (NAMERES_MINIMUM_QUERY_LENGTH, default 2) before they reach Solr. - Consolidate env-var config into a central `Config` dataclass; expose a public subset (currently just `minimum_query_length`) via `/status`. - `/lookup` returns HTTP 422 for too-short queries (documented in the OpenAPI schema); `/bulk-lookup` maps each too-short string to `[]` so one short string does not fail the whole batch. Note: empty-string `/lookup` now returns 422 instead of an empty list. Co-Authored-By: Claude Opus 4.8 --- api/server.py | 70 +++++++++++++++++++++++++++++++++++-------- tests/test_service.py | 34 +++++++++++++++++++-- 2 files changed, 89 insertions(+), 15 deletions(-) diff --git a/api/server.py b/api/server.py index 67296bfe..25111e35 100755 --- a/api/server.py +++ b/api/server.py @@ -9,10 +9,11 @@ import time import os import re +from dataclasses import dataclass from enum import Enum from typing import Dict, List, Union, Annotated, Optional -from fastapi import Body, FastAPI, Query +from fastapi import Body, FastAPI, Query, HTTPException from fastapi.responses import RedirectResponse import httpx from pydantic import BaseModel, Field @@ -20,8 +21,22 @@ from api.apidocs import get_app_info, construct_open_api_schema -SOLR_HOST = os.getenv("SOLR_HOST", "localhost") -SOLR_PORT = os.getenv("SOLR_PORT", "8983") +@dataclass(frozen=True) +class Config: + """Runtime configuration, populated from environment variables at import.""" + # Solr connection (private — not exposed via /status). + solr_host: str = os.getenv("SOLR_HOST", "localhost") + solr_port: str = os.getenv("SOLR_PORT", "8983") + # Queries shorter than this (after strip) are rejected: single-char queries + # are slow in Solr and never useful. Translator expects results at length 2. + minimum_query_length: int = int(os.getenv("NAMERES_MINIMUM_QUERY_LENGTH", "2")) + + def public(self) -> dict: + """Config values safe to surface via /status. Infra config stays private.""" + return {"minimum_query_length": self.minimum_query_length} + + +config = Config() app = FastAPI(**get_app_info()) logger = logging.getLogger(__name__) @@ -57,7 +72,7 @@ async def status_get() -> Dict: async def status() -> Dict: """ Return a dictionary containing status and count information for the underlying Solr instance. """ - query_url = f"http://{SOLR_HOST}:{SOLR_PORT}/solr/admin/cores" + query_url = f"http://{config.solr_host}:{config.solr_port}/solr/admin/cores" async with httpx.AsyncClient(timeout=None) as client: response = await client.get(query_url, params={ 'action': 'STATUS' @@ -102,6 +117,7 @@ async def status() -> Dict: 'download_url': biolink_model_download_url, }, 'nameres_version': nameres_version, + 'config': config.public(), 'startTime': core['startTime'], 'numDocs': index.get('numDocs', ''), 'maxDoc': index.get('maxDoc', ''), @@ -123,6 +139,7 @@ async def status() -> Dict: 'download_url': biolink_model_download_url, }, 'nameres_version': nameres_version, + 'config': config.public(), } @@ -219,7 +236,7 @@ async def synonyms_post( async def name_lookup(curies) -> Dict[str, Dict]: """Returns a list of synonyms for a particular CURIE.""" time_start = time.time_ns() - query = f"http://{SOLR_HOST}:{SOLR_PORT}/solr/name_lookup/select" + query = f"http://{config.solr_host}:{config.solr_port}/solr/name_lookup/select" curie_filter = " OR ".join( f"curie:\"{curie}\"" for curie in curies @@ -263,11 +280,22 @@ class LookupResult(BaseModel): "

You can find out more about this endpoint in the API documentation.

" "

Note that CURIEs are conflated with both GeneProtein and DrugChemical conflation, so that e.g. when searching for a protein, the identifier of the gene that encodes the protein will be returned itself. See Conflation documentation for more information.

", response_model=List[LookupResult], + responses={ + 422: { + "description": "The request was rejected without being searched. This includes queries shorter " + "than the configured minimum length (see `minimum_query_length` in `/status`, " + "default 2 characters after leading/trailing whitespace is stripped), since " + "single-character queries are slow and never return useful results, as well as any " + "other request parameter that failed validation." + } + }, tags=["lookup"] ) async def lookup_curies_get( string: Annotated[str, Query( - description="The string to search for." + description="The string to search for. Must be at least the configured minimum length " + "(see `minimum_query_length` in `/status`, default 2) after leading/trailing " + "whitespace is stripped; shorter queries are rejected with HTTP 422." )], autocomplete: Annotated[bool, Query( description="Is the input string incomplete (autocomplete=true) or a complete phrase (autocomplete=false)?" @@ -318,7 +346,7 @@ async def lookup_curies_get( """ Returns cliques with a name or synonym that contains a specified string. """ - return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug) + return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug, raise_on_too_short=True) @app.post("/lookup", @@ -327,11 +355,22 @@ async def lookup_curies_get( "

You can find out more about this endpoint in the API documentation.

" "

Note that CURIEs are conflated with both GeneProtein and DrugChemical conflation, so that e.g. when searching for a protein, the identifier of the gene that encodes the protein will be returned itself. See Conflation documentation for more information.

", response_model=List[LookupResult], + responses={ + 422: { + "description": "The request was rejected without being searched. This includes queries shorter " + "than the configured minimum length (see `minimum_query_length` in `/status`, " + "default 2 characters after leading/trailing whitespace is stripped), since " + "single-character queries are slow and never return useful results, as well as any " + "other request parameter that failed validation." + } + }, tags=["lookup"] ) async def lookup_curies_post( string: Annotated[str, Query( - description="The string to search for." + description="The string to search for. Must be at least the configured minimum length " + "(see `minimum_query_length` in `/status`, default 2) after leading/trailing " + "whitespace is stripped; shorter queries are rejected with HTTP 422." )], autocomplete: Annotated[bool, Query( description="Is the input string incomplete (autocomplete=true) or a complete phrase (autocomplete=false)?" @@ -382,7 +421,7 @@ async def lookup_curies_post( """ Returns cliques with a name or synonym that contains a specified string. """ - return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug) + return await lookup(string, autocomplete, highlighting, offset, limit, biolink_type, only_prefixes, exclude_prefixes, only_taxa, debug, raise_on_too_short=True) async def lookup(string: str, @@ -395,6 +434,7 @@ async def lookup(string: str, exclude_prefixes: str = "", only_taxa: str = "", debug: DebugOptions = 'none', + raise_on_too_short: bool = False, ) -> List[LookupResult]: """ Returns cliques with a name or synonym that contains a specified string. @@ -421,8 +461,14 @@ async def lookup(string: str, # let's detect and replace just those characters. string_lc = re.sub(r"[“”]", '"', re.sub(r"[‘’]", "'", string_lc)) - # Do we have a search string at all? - if string_lc == "": + # Is the query long enough to be worth searching? Single-character queries are + # slow in Solr and never return useful results (see config.minimum_query_length). + if len(string_lc) < config.minimum_query_length: + if raise_on_too_short: + raise HTTPException( + status_code=422, + detail=f"Query string must be at least {config.minimum_query_length} character(s).", + ) return [] # For reasons I don't understand, we need to use backslash to escape characters (e.g. "\(") to remove the special @@ -534,7 +580,7 @@ async def lookup(string: str, logger.debug(f"Query: {json.dumps(params, indent=2)}") time_solr_start = time.time_ns() - query_url = f"http://{SOLR_HOST}:{SOLR_PORT}/solr/name_lookup/select" + query_url = f"http://{config.solr_host}:{config.solr_port}/solr/name_lookup/select" async with httpx.AsyncClient(timeout=None) as client: response = await client.post(query_url, json=params) if response.status_code >= 300: diff --git a/tests/test_service.py b/tests/test_service.py index 2fa9a242..e2ad5c0c 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -16,11 +16,39 @@ def test_simple_check(): def test_empty(): - """ Checks that calling NameRes without an input string return an empty list. """ + """ An empty (too-short) query on /lookup is rejected with HTTP 422. """ client = TestClient(app) response = client.get("/lookup", params={'string':''}) - syns = response.json() - assert len(syns) == 0 + assert response.status_code == 422 + + +def test_minimum_query_length(): + """ + Queries shorter than config.minimum_query_length (default 2) are rejected on + /lookup with HTTP 422, but degrade to an empty list per-key on /bulk-lookup so + one short string doesn't fail the whole batch. The minimum is reported by /status. + """ + client = TestClient(app) + + # A single character on /lookup is too short -> 422. + response = client.get("/lookup", params={'string': 'a'}) + assert response.status_code == 422 + + # A valid two-character query is not rejected (goes to Solr, returns a 200 list). + response = client.get("/lookup", params={'string': 'ab'}) + assert response.status_code == 200 + assert isinstance(response.json(), list) + + # On /bulk-lookup, a too-short string yields [] for its key; others resolve; request is 200. + response = client.post("/bulk-lookup", json={'strings': ['a', 'Parkinson'], 'limit': 100}) + assert response.status_code == 200 + results = response.json() + assert results['a'] == [] + assert len(results['Parkinson']) == 34 + + # /status advertises the configured minimum under a nested config block. + response = client.get("/status") + assert response.json()['config'] == {'minimum_query_length': 2} def test_limit(): From dd1daa6598fba4a290e47fea643e32fe2293e722 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 17 Jul 2026 19:32:55 -0400 Subject: [PATCH 2/4] Document minimum query length in API.md Note the minimum-length requirement and 422 on /lookup, the graceful []-per-key behavior on /bulk-lookup, and the new config block in /status. Co-Authored-By: Claude Opus 4.8 --- documentation/API.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/documentation/API.md b/documentation/API.md index 5311a2d5..f6400e7d 100644 --- a/documentation/API.md +++ b/documentation/API.md @@ -118,7 +118,7 @@ Search for cliques by a fragment of a name or synonym. **Parameters:** -- `string` (required, string): The string to search for. +- `string` (required, string): The string to search for. Must be at least the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) after leading/trailing whitespace is stripped; shorter queries are rejected with HTTP 422. - `autocomplete` (optional, boolean, default: false): If `true`, treats the input string as incomplete and looks for terms that start with the final word. If `false`, treats the entire phrase as complete (entity linker mode). - `highlighting` (optional, boolean, default: false): If `true`, returns information on which labels and synonyms matched the search query. - `offset` (optional, integer, default: 0, minimum: 0): The number of results to skip. Used for pagination. @@ -191,7 +191,7 @@ Search for cliques for multiple strings in a single request. - `strings` (required, list of strings): A list of strings to search for. The returned results will be in a dictionary with these values as keys. - All other parameters are optional and apply to all searches. -**Returns:** A dictionary where each key is a string from the input `strings` array, and each value is a list of `LookupResult` objects (same structure as `/lookup` results). +**Returns:** A dictionary where each key is a string from the input `strings` array, and each value is a list of `LookupResult` objects (same structure as `/lookup` results). Unlike `/lookup`, a string shorter than the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) is not rejected: it simply maps to an empty list, so one too-short string does not fail the whole batch. **Example request:** @@ -312,7 +312,10 @@ Returns the status of the service. Most importantly, this returns the [Babel](ht version and changelog URL, which can be used to determine which version of Babel is currently loaded in this service. It also includes the NameRes version (also visible in the OpenAPI documentation) and the Biolink Model version used to build the Solr database, as well as bunch of information from the underlying -Solr database. +Solr database. The `config` object reports publicly-relevant configuration for this instance; currently just +`minimum_query_length`, the shortest query (in characters, after whitespace is stripped) that `/lookup` and +`/bulk-lookup` will search for. It defaults to 2 and can be set per deployment with the `NAMERES_MINIMUM_QUERY_LENGTH` +environment variable. ```json { @@ -326,6 +329,9 @@ Solr database. "download_url": "https://raw.githubusercontent.com/biolink/biolink-model/v4.2.6-rc5/biolink-model.yaml" }, "nameres_version": "v1.5.1", + "config": { + "minimum_query_length": 2 + }, "startTime": "2025-12-19T11:53:09.638Z", "numDocs": 425583391, "maxDoc": 425586610, From 421f8f04b02d626b497778be495b3d23a30644e5 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Mon, 31 Aug 2026 16:04:47 -0400 Subject: [PATCH 3/4] Document NAMERES_MINIMUM_QUERY_LENGTH in the env var lists The setting was described in API.md's /status section but missing from the two places that enumerate every environment variable, so a deployer reading the configuration guide had no way to discover it. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 1 + documentation/Deployment.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 45657a39..9e727f1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,6 +70,7 @@ pip install -r requirements.txt ### Environment Variables - `SOLR_HOST` / `SOLR_PORT` - Solr connection (default: `localhost:8983`) - `SOLR_MAX_CONCURRENT_LOOKUPS` / `SOLR_TIMEOUT_SECONDS` - Bulk-lookup fan-out bound and Solr query timeout (see `documentation/Deployment.md`) +- `NAMERES_MINIMUM_QUERY_LENGTH` - Shortest query `/lookup` and `/bulk-lookup` will search for (default: 2) - `LOGLEVEL` - Logging level - `SERVER_ROOT` - API root path prefix - `MATURITY_VALUE` / `LOCATION_VALUE` - TRAPI metadata fields diff --git a/documentation/Deployment.md b/documentation/Deployment.md index 7734e778..416755eb 100644 --- a/documentation/Deployment.md +++ b/documentation/Deployment.md @@ -107,6 +107,7 @@ NameRes can be configured by setting environmental variables: * `SOLR_CORE`: The Solr core to query (defaults to `name_lookup`). * `SOLR_MAX_CONCURRENT_LOOKUPS`: The greatest number of Solr queries a single `/bulk-lookup` request may have in flight at once (defaults to `100`, and is clamped to at least `1`). The bound is per request, so the queries this service has in flight against Solr is this multiplied by the number of bulk lookups being served at once. * `SOLR_TIMEOUT_SECONDS`: How long to wait for any one Solr query before giving up (defaults to `60`). Set it to `0` to wait indefinitely. +* `NAMERES_MINIMUM_QUERY_LENGTH`: The shortest query, in characters after leading/trailing whitespace is stripped, that `/lookup` and `/bulk-lookup` will search for (defaults to `2`). Shorter queries are rejected with an HTTP 422 from `/lookup` and return no results from `/bulk-lookup`, since they are slow in Solr and never useful. The value in force is reported as `minimum_query_length` by `/status`. * `SERVER_NAME`: The name of this server (defaults to `infores:sri-name-resolver`) * `SERVER_ROOT`: The server root (defaults to `/`) * `MATURITY_VALUE`: How mature is this NameRes (defaults to `maturity`, e.g. `development`) From b8b53493348e84b44355ff361e6bc23be62cdf86 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Mon, 31 Aug 2026 16:19:16 -0400 Subject: [PATCH 4/4] Narrow the query-length minimum to the tokenized search it was written for Code review on #279 found three problems with the minimum-length check and one with the test that covers it. The empty-string rejection it replaced was unconditional; the length check is not, so NAMERES_MINIMUM_QUERY_LENGTH=0 -- the obvious way to turn the minimum off, and what SOLR_TIMEOUT_SECONDS=0 means one setting above it -- let an empty query through to Solr as `"" OR ()`. That is a parse error, so an empty search box came back as an HTTP 500. The check now floors at 1 independently of the setting. The minimum is justified by the cost of a short query in the default tokenized search, where a one-character string matches a prefix of half the index. Exact mode has no such cost -- it is a single filter query against an untokenized field -- and single-character labels are real (the gene T, the element symbols), so it is now held to nothing but non-emptiness. A too-short query was reported with HTTPException(422), whose body is {"detail": ""} where FastAPI's own validation returns {"detail": [...]}, so a client iterating `detail` as a list of errors broke on this one rejection. It now raises RequestValidationError instead. The hand-written `responses={422: ...}` on both /lookup operations went with it: FastAPI only generates the HTTPValidationError body for a 422 the operation has not declared itself, so declaring one cost the endpoint its schema and left client generators with an untyped error. Finally, the /status assertion pinned both the default and the exact key set of the config block, so it failed for anyone setting the very override this PR adds. It now compares against the running config. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 + api/server.py | 60 ++++++++++++++++------------- documentation/API.md | 9 +++-- documentation/Deployment.md | 2 +- tests/test_exact_mode.py | 34 ++++++++++++++++ tests/test_service.py | 77 ++++++++++++++++++++++++++++++++++++- 6 files changed, 150 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9e727f1a..3487a982 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -96,6 +96,8 @@ Solr documents contain: `curie`, `preferred_name`, `names` (synonym list), and b - **The default search is *tokenized*, not *fuzzy*.** It matches the query's tokens in any order (order and adjacency are rewarded by the phrase-field boost, not required), and `autocomplete=true` makes the final token a prefix. There is no edit-distance matching, and `lookup()` escapes Solr's `~` out of the query so callers cannot request it. Do not describe it as "fuzzy" in docs or parameter descriptions -- that promises typo tolerance the service has never had. `tests/test_service.py` pins both halves of this. - **Solr's `filterCache` is bounded by entry count (512), not by memory.** It earns its keep on shared, reusable filters (`types:`, `taxa:`, `curie:`). A filter whose value varies per query -- as exact mode's does -- must be marked `{!cache=false}`, or one bulk request evicts the whole cache and slows the ordinary search path down as collateral damage. See the comment in `lookup()`. +- **The empty-query rejection is not the same rule as `minimum_query_length`.** An empty string reaches Solr as `"" OR ()`, which is a parse error and therefore an HTTP 500 for what is really an empty search box, so `lookup()` floors its length check at 1 (`max(1, config.minimum_query_length)`) rather than deriving it from the setting alone. Folding the two together breaks the moment someone sets `NAMERES_MINIMUM_QUERY_LENGTH=0` to turn the minimum off. Exact mode is exempt from the setting but not from the floor. +- **Do not declare a custom `responses={422: ...}` on an endpoint.** FastAPI adds the `HTTPValidationError` body only when the operation has not already declared a 422 of its own, so a hand-written one silently strips the schema and leaves client generators with an untyped error. For the same reason, `lookup()` reports a too-short query with `RequestValidationError`, not `HTTPException(422)`: the latter returns `{"detail": ""}` where FastAPI's own validation returns `{"detail": [...]}`. `tests/test_service.py` pins both. - **Query-side string normalization must not be applied to exact matching.** The `*_exactish` fields are a KeywordTokenizer plus a LowerCaseFilter and fold nothing else, so the smart-quote rewrite (and anything like it) would search for a string the caller never typed. The default path is unaffected because StandardTokenizer discards the punctuation anyway. ## Documentation diff --git a/api/server.py b/api/server.py index 7d021cb1..5c4c4258 100755 --- a/api/server.py +++ b/api/server.py @@ -16,6 +16,7 @@ from typing import Dict, List, Union, Annotated, Optional from fastapi import Body, FastAPI, HTTPException, Query +from fastapi.exceptions import RequestValidationError from fastapi.responses import RedirectResponse import httpx from pydantic import BaseModel, Field @@ -332,22 +333,15 @@ class LookupResult(BaseModel): "

You can find out more about this endpoint in the API documentation.

" "

Note that CURIEs are conflated with both GeneProtein and DrugChemical conflation, so that e.g. when searching for a protein, the identifier of the gene that encodes the protein will be returned itself. See Conflation documentation for more information.

", response_model=List[LookupResult], - responses={ - 422: { - "description": "The request was rejected without being searched. This includes queries shorter " - "than the configured minimum length (see `minimum_query_length` in `/status`, " - "default 2 characters after leading/trailing whitespace is stripped), since " - "single-character queries are slow and never return useful results, as well as any " - "other request parameter that failed validation." - } - }, tags=["lookup"] ) async def lookup_curies_get( string: Annotated[str, Query( description="The string to search for. Must be at least the configured minimum length " "(see `minimum_query_length` in `/status`, default 2) after leading/trailing " - "whitespace is stripped; shorter queries are rejected with HTTP 422." + "whitespace is stripped; shorter queries are rejected with HTTP 422. The " + "minimum does not apply when `exact` is set, where any non-empty string is " + "searched for." )], autocomplete: Annotated[bool, Query( description="Is the input string incomplete (autocomplete=true) or a complete phrase (autocomplete=false)?" @@ -413,22 +407,15 @@ async def lookup_curies_get( "

You can find out more about this endpoint in the API documentation.

" "

Note that CURIEs are conflated with both GeneProtein and DrugChemical conflation, so that e.g. when searching for a protein, the identifier of the gene that encodes the protein will be returned itself. See Conflation documentation for more information.

", response_model=List[LookupResult], - responses={ - 422: { - "description": "The request was rejected without being searched. This includes queries shorter " - "than the configured minimum length (see `minimum_query_length` in `/status`, " - "default 2 characters after leading/trailing whitespace is stripped), since " - "single-character queries are slow and never return useful results, as well as any " - "other request parameter that failed validation." - } - }, tags=["lookup"] ) async def lookup_curies_post( string: Annotated[str, Query( description="The string to search for. Must be at least the configured minimum length " "(see `minimum_query_length` in `/status`, default 2) after leading/trailing " - "whitespace is stripped; shorter queries are rejected with HTTP 422." + "whitespace is stripped; shorter queries are rejected with HTTP 422. The " + "minimum does not apply when `exact` is set, where any non-empty string is " + "searched for." )], autocomplete: Annotated[bool, Query( description="Is the input string incomplete (autocomplete=true) or a complete phrase (autocomplete=false)?" @@ -545,14 +532,33 @@ async def lookup(string: str, if not exact: string_lc = re.sub(r"[“”]", '"', re.sub(r"[‘’]", "'", string_lc)) - # Is the query long enough to be worth searching? Single-character queries are - # slow in Solr and never return useful results (see config.minimum_query_length). - if len(string_lc) < config.minimum_query_length: + # Is the query long enough to be worth searching? + # + # config.minimum_query_length exists because short queries are slow in the default tokenized + # search -- a one-character query matches a prefix of half the index -- and never return + # anything useful. Exact mode has neither problem: it is a single filter query against an + # untokenized field, and single-character labels are real (the gene T, the element symbols), + # so the minimum would put them permanently out of reach for no gain. It is therefore held to + # nothing but non-emptiness. + # + # The floor of 1 is not redundant with the setting. An empty query is rejected whatever + # minimum_query_length is set to, because it would otherwise reach Solr as `"" OR ()` and come + # back as a parse error -- an HTTP 500 for what is really an empty search box. + minimum_length = 1 if exact else max(1, config.minimum_query_length) + if len(string_lc) < minimum_length: if raise_on_too_short: - raise HTTPException( - status_code=422, - detail=f"Query string must be at least {config.minimum_query_length} character(s).", - ) + # A RequestValidationError rather than an HTTPException, so that a query rejected for + # its length is reported in the same shape -- and documented by the same schema -- as + # one rejected by FastAPI's own parameter validation. A caller iterating `detail` as a + # list of errors should not have to special-case this one rejection. + raise RequestValidationError([{ + "type": "string_too_short", + "loc": ("query", "string"), + "msg": f"String should have at least {minimum_length} character(s) after " + f"leading and trailing whitespace is stripped", + "input": string, + "ctx": {"min_length": minimum_length}, + }]) return [] # For reasons I don't understand, we need to use backslash to escape characters (e.g. "\(") to remove the special diff --git a/documentation/API.md b/documentation/API.md index 3843cb9f..9de44d9a 100644 --- a/documentation/API.md +++ b/documentation/API.md @@ -120,7 +120,7 @@ Search for cliques by a fragment of a name or synonym. **Parameters:** -- `string` (required, string): The string to search for. Must be at least the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) after leading/trailing whitespace is stripped; shorter queries are rejected with HTTP 422. +- `string` (required, string): The string to search for. Must be at least the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) after leading/trailing whitespace is stripped; shorter queries are rejected with HTTP 422, in the same body shape as any other parameter validation failure. The minimum does not apply when [`exact`](#exact-matching) is set — exact matching is a single filter query rather than a tokenized search, and short labels such as the gene `T` are legitimate targets for it — but an empty string is rejected in every mode. - `autocomplete` (optional, boolean, default: false): If `true`, treats the input string as incomplete and looks for terms that start with the final word. If `false`, treats the entire phrase as complete (entity linker mode). - `highlighting` (optional, boolean, default: false): If `true`, returns information on which labels and synonyms matched the search query. - `offset` (optional, integer, default: 0, minimum: 0): The number of results to skip. Used for pagination. @@ -195,7 +195,7 @@ Search for cliques for multiple strings in a single request. - `strings` (required, list of strings): A list of strings to search for. The returned results will be in a dictionary with these values as keys. - All other parameters are optional and apply to all searches. -**Returns:** A dictionary where each key is a string from the input `strings` array, and each value is a list of `LookupResult` objects (same structure as `/lookup` results). Unlike `/lookup`, a string shorter than the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) is not rejected: it simply maps to an empty list, so one too-short string does not fail the whole batch. +**Returns:** A dictionary where each key is a string from the input `strings` array, and each value is a list of `LookupResult` objects (same structure as `/lookup` results). Unlike `/lookup`, a string shorter than the configured minimum length (see `minimum_query_length` in [`/status`](#status), default 2) is not rejected: it simply maps to an empty list, so one too-short string does not fail the whole batch. An empty string, and a string shorter than the minimum in `exact` mode, are treated the same way. **Example request:** @@ -350,8 +350,9 @@ It also includes the NameRes version (also visible in the OpenAPI documentation) and the Biolink Model version used to build the Solr database, as well as bunch of information from the underlying Solr database. The `config` object reports publicly-relevant configuration for this instance; currently just `minimum_query_length`, the shortest query (in characters, after whitespace is stripped) that `/lookup` and -`/bulk-lookup` will search for. It defaults to 2 and can be set per deployment with the `NAMERES_MINIMUM_QUERY_LENGTH` -environment variable. +`/bulk-lookup` will search for in the default tokenized search. It defaults to 2 and can be set per deployment with the +`NAMERES_MINIMUM_QUERY_LENGTH` environment variable. It does not constrain [exact matching](#exact-matching), which +accepts any non-empty string. ```json { diff --git a/documentation/Deployment.md b/documentation/Deployment.md index 416755eb..b0b728d2 100644 --- a/documentation/Deployment.md +++ b/documentation/Deployment.md @@ -107,7 +107,7 @@ NameRes can be configured by setting environmental variables: * `SOLR_CORE`: The Solr core to query (defaults to `name_lookup`). * `SOLR_MAX_CONCURRENT_LOOKUPS`: The greatest number of Solr queries a single `/bulk-lookup` request may have in flight at once (defaults to `100`, and is clamped to at least `1`). The bound is per request, so the queries this service has in flight against Solr is this multiplied by the number of bulk lookups being served at once. * `SOLR_TIMEOUT_SECONDS`: How long to wait for any one Solr query before giving up (defaults to `60`). Set it to `0` to wait indefinitely. -* `NAMERES_MINIMUM_QUERY_LENGTH`: The shortest query, in characters after leading/trailing whitespace is stripped, that `/lookup` and `/bulk-lookup` will search for (defaults to `2`). Shorter queries are rejected with an HTTP 422 from `/lookup` and return no results from `/bulk-lookup`, since they are slow in Solr and never useful. The value in force is reported as `minimum_query_length` by `/status`. +* `NAMERES_MINIMUM_QUERY_LENGTH`: The shortest query, in characters after leading/trailing whitespace is stripped, that `/lookup` and `/bulk-lookup` will search for (defaults to `2`). Shorter queries are rejected with an HTTP 422 from `/lookup` and return no results from `/bulk-lookup`, since they are slow in Solr and never useful. This applies only to the default tokenized search: exact matching accepts any non-empty string, and an empty string is rejected whatever this is set to. The value in force is reported as `minimum_query_length` by `/status`. * `SERVER_NAME`: The name of this server (defaults to `infores:sri-name-resolver`) * `SERVER_ROOT`: The server root (defaults to `/`) * `MATURITY_VALUE`: How mature is this NameRes (defaults to `maturity`, e.g. `development`) diff --git a/tests/test_exact_mode.py b/tests/test_exact_mode.py index a6a9e7db..59086f14 100644 --- a/tests/test_exact_mode.py +++ b/tests/test_exact_mode.py @@ -1,8 +1,10 @@ # This file tests the exact-match mode (the `exact` parameter) on the lookup and bulk_lookup # endpoints. Tests for the default, tokenized search live in test_service.py. +import dataclasses import json import logging +import api.server from api.server import app from fastapi.testclient import TestClient @@ -221,3 +223,35 @@ def test_exact_rejects_autocomplete_in_bulk_lookup(): }) assert response.status_code == 400 assert 'autocomplete' in response.json()['detail'] + + +def test_exact_is_not_subject_to_the_minimum_query_length(monkeypatch): + """ + config.minimum_query_length guards the default tokenized search, where a very short query is + slow and useless. Exact mode is a single filter query against an untokenized field, and short + labels are real -- the gene T, the element symbols, and "AD" for Alzheimer disease here -- so + the minimum must not apply to it. + + The minimum is raised well above the query length rather than relying on the default of 2, + so that the test says something whatever the deployment default becomes. + """ + client = TestClient(app) + monkeypatch.setattr(api.server, "config", + dataclasses.replace(api.server.config, minimum_query_length=5)) + + # "AD" is a synonym of MONDO:0004975 (Alzheimer disease), and shorter than the minimum. + response = client.get("/lookup", params={'string': 'AD', 'exact': 'synonyms', 'limit': 100}) + assert response.status_code == 200 + assert 'MONDO:0004975' in [r['curie'] for r in response.json()] + + # The same query without exact mode is still rejected, which is what makes the case above a + # property of exact mode rather than of the minimum being unset. + response = client.get("/lookup", params={'string': 'AD', 'limit': 100}) + assert response.status_code == 422 + + # Exemption from the minimum is not exemption from the empty check: an empty exact query + # would still reach Solr as an unparseable query and come back as a 500. + for string in ('', ' '): + response = client.get("/lookup", params={'string': string, 'exact': 'any'}) + assert response.status_code == 422, \ + f"Expected exact lookup of {string!r} to be rejected, got HTTP {response.status_code}" diff --git a/tests/test_service.py b/tests/test_service.py index 33db37cb..4e5e41a3 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -47,9 +47,82 @@ def test_minimum_query_length(): assert results['a'] == [] assert len(results['Parkinson']) == 34 - # /status advertises the configured minimum under a nested config block. + # /status advertises the configured minimum under a nested config block. Compared against + # the running config rather than a literal 2, so that this still tests the wiring in a + # deployment that sets NAMERES_MINIMUM_QUERY_LENGTH, and survives another key being added. response = client.get("/status") - assert response.json()['config'] == {'minimum_query_length': 2} + assert response.json()['config']['minimum_query_length'] == api.server.config.minimum_query_length + + +def test_too_short_is_reported_as_a_validation_error(): + """ + A query rejected for its length must come back in the same shape as one rejected by FastAPI's + own parameter validation -- `detail` a list of error objects, not a bare string -- because a + client iterating `detail` should not have to special-case this one rejection. That is also + what the endpoint's documented 422 schema (HTTPValidationError) promises; see + test_lookup_documents_the_validation_error_schema. + """ + client = TestClient(app) + response = client.get("/lookup", params={'string': 'a'}) + assert response.status_code == 422 + + detail = response.json()['detail'] + assert isinstance(detail, list), f"Expected a list of errors, got {detail!r}" + assert detail[0]['loc'] == ['query', 'string'] + assert detail[0]['type'] == 'string_too_short' + assert 'msg' in detail[0] + + # An ordinary validation failure on the same endpoint (limit above its maximum) is shaped the + # same way, which is the point of the comparison. + response = client.get("/lookup", params={'string': 'Parkinson', 'limit': 100000}) + assert response.status_code == 422 + assert isinstance(response.json()['detail'], list) + + +def test_lookup_documents_the_validation_error_schema(): + """ + FastAPI only generates the HTTPValidationError body for a 422 that the operation has not + already declared itself, so a hand-written `responses={422: ...}` on these endpoints would + silently strip the schema from the OpenAPI document and leave client generators with an + untyped error. Pin that the schema is there for both /lookup operations. + """ + client = TestClient(app) + schema = client.get("/openapi.json").json() + + for method in ('get', 'post'): + response_422 = schema['paths']['/lookup'][method]['responses']['422'] + ref = response_422['content']['application/json']['schema']['$ref'] + assert ref.endswith('/HTTPValidationError'), \ + f"/lookup {method.upper()} documents 422 as {ref}, not HTTPValidationError" + + +def test_empty_query_is_rejected_however_low_the_minimum_is(monkeypatch): + """ + An empty query must never reach Solr: it builds the query `"" OR ()`, which Solr rejects as a + parse error, turning an empty search box into an HTTP 500. The length check is what stops it, + so it has to hold even when minimum_query_length is set to 0 -- the obvious way to turn the + minimum off, and the same thing SOLR_TIMEOUT_SECONDS=0 means one setting above it. + """ + client = TestClient(app) + monkeypatch.setattr(api.server, "config", + dataclasses.replace(api.server.config, minimum_query_length=0)) + + # A single character is now allowed through to Solr... + response = client.get("/lookup", params={'string': 'a'}) + assert response.status_code == 200 + + # ...but an empty string, and one that is empty once stripped, are still rejected. + for string in ('', ' '): + response = client.get("/lookup", params={'string': string}) + assert response.status_code == 422, \ + f"Expected {string!r} to be rejected, got HTTP {response.status_code}" + + # And on /bulk-lookup it degrades to [] for that key rather than failing the whole batch. + response = client.post("/bulk-lookup", json={'strings': ['', 'Parkinson'], 'limit': 100}) + assert response.status_code == 200 + results = response.json() + assert results[''] == [] + assert len(results['Parkinson']) > 0 def test_limit(): client = TestClient(app)