From 1ed6661132ef6f3ac4002cf8923457e4e29c6b5b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 16:32:14 +0000 Subject: [PATCH] Accept POST as well as GET on the CATMAID pass-through Bulk-id commands (compact_detail, annotations_for_skeletons, connectivity, ...) already send their ids to CATMAID as a POST body internally, but the client-facing /catmaid/{instance}/{command} route only accepted GET, so callers had to put the same ids in the query string. Past ~1000 ids that trips nginx's default header-buffer limit with a 414, forcing bulk callers to chunk into several requests, each carrying the same failure mode. Add a POST route for the same path, reading parameters from a JSON body or an application/x-www-form-urlencoded/multipart form instead of the query string. `project` and `raw` stay on the query string for both verbs, since they're view flags rather than command payload. The cache key doesn't include the HTTP method, so a GET and the equivalent POST share one cache entry rather than warming it twice. GET is unchanged and remains fully supported. --- src/test/test_catmaid_passthrough.py | 179 +++++++++++++++++++++++++++ src/vfbquery/ha_api.py | 96 +++++++++++++- 2 files changed, 271 insertions(+), 4 deletions(-) diff --git a/src/test/test_catmaid_passthrough.py b/src/test/test_catmaid_passthrough.py index 15e8c0f..f379450 100644 --- a/src/test/test_catmaid_passthrough.py +++ b/src/test/test_catmaid_passthrough.py @@ -6,12 +6,64 @@ with ``-m 'not integration'``. """ +import asyncio +import functools +import json + import pytest +from aiohttp.base_protocol import BaseProtocol +from aiohttp.streams import StreamReader +from aiohttp.test_utils import make_mocked_request import vfbquery.catmaid_client as cm import vfbquery.ha_api as ha_api +def sync(fn): + """Run an async test body on a fresh loop. + + Matches test_ha_api_dispatch.py's helper: deliberately not + pytest-asyncio, to avoid a test-only dependency for the handful of + coroutine tests in the suite. + """ + @functools.wraps(fn) + def wrapper(*args, **kwargs): + return asyncio.run(fn(*args, **kwargs)) + return wrapper + + +def _post_request(path, body=b"", content_type=None, headers=None): + """A mocked POST request carrying *body* as an unread payload. + + ``make_mocked_request`` wants a real (if empty) StreamReader for the + body, and ``_catmaid_body_params`` decides how to read it from + ``Content-Type`` — both are wired up here so callers only give the + bytes and the type. + """ + hdrs = dict(headers or {}) + if content_type is not None: + hdrs["Content-Type"] = content_type + loop = asyncio.get_event_loop() + stream = StreamReader(BaseProtocol(loop=loop), limit=2 ** 20, loop=loop) + if body: + stream.feed_data(body) + stream.feed_eof() + match_info = {} + parts = path.split("?", 1)[0].strip("/").split("/") + if len(parts) >= 3 and parts[0] == "catmaid": + match_info = {"instance": parts[1], "command": parts[2]} + return make_mocked_request("POST", path, headers=hdrs, payload=stream, + match_info=match_info) + + +def _get_request(path): + parts = path.split("?", 1)[0].strip("/").split("/") + match_info = {} + if len(parts) >= 3 and parts[0] == "catmaid": + match_info = {"instance": parts[1], "command": parts[2]} + return make_mocked_request("GET", path, match_info=match_info) + + # --------------------------------------------------------------------------- # Registry integrity # --------------------------------------------------------------------------- @@ -253,6 +305,133 @@ def test_catmaid_raw_view_unwraps_envelope_only(): assert ha_api._catmaid_raw_view("bare") == "bare" +# --------------------------------------------------------------------------- +# POST support: parameters travel in the body instead of the query string +# --------------------------------------------------------------------------- + +@sync +async def test_catmaid_body_params_reads_json_object(): + request = _post_request( + "/catmaid/fafb/compact_detail", + body=json.dumps({"ids": [1, 2, 3], "with_connectors": False}).encode(), + content_type="application/json") + params = await ha_api._catmaid_body_params(request) + assert params == {"ids": [1, 2, 3], "with_connectors": False} + + +@sync +async def test_catmaid_body_params_reads_form_encoded(): + request = _post_request( + "/catmaid/fafb/annotations_for_skeletons", + body=b"ids=1&ids=2&ids=3&with_connectors=false", + content_type="application/x-www-form-urlencoded") + params = await ha_api._catmaid_body_params(request) + assert params == {"ids": ["1", "2", "3"], "with_connectors": "false"} + + +@sync +async def test_catmaid_body_params_single_form_value_is_scalar(): + request = _post_request( + "/catmaid/fafb/skeleton_root", body=b"id=16", + content_type="application/x-www-form-urlencoded") + assert await ha_api._catmaid_body_params(request) == {"id": "16"} + + +@sync +async def test_catmaid_body_params_empty_body_is_empty_dict(): + request = _post_request("/catmaid/fafb/projects") + assert await ha_api._catmaid_body_params(request) == {} + + +@sync +async def test_catmaid_body_params_rejects_non_object_json(): + request = _post_request( + "/catmaid/fafb/compact_detail", body=b"[1, 2, 3]", + content_type="application/json") + with pytest.raises(ValueError, match="must be an object"): + await ha_api._catmaid_body_params(request) + + +@sync +async def test_catmaid_body_params_rejects_malformed_json(): + request = _post_request( + "/catmaid/fafb/compact_detail", body=b"{not json", + content_type="application/json") + with pytest.raises(ValueError, match="Malformed JSON body"): + await ha_api._catmaid_body_params(request) + + +@sync +async def test_handle_catmaid_command_post_matches_get_cache_key(monkeypatch): + """A GET and the equivalent POST must build the same params and cache + key — that's what lets a caller move from one to the other (e.g. once + an id list outgrows the query string) without cache-missing every + query it already warmed.""" + captured = [] + + async def fake_dispatch(request, cache_key, worker_fn, *args, **kwargs): + captured.append((cache_key, args)) + return {"ok": True} + + monkeypatch.setattr(ha_api, "_dispatch_to_pool", fake_dispatch) + + get_req = _get_request( + "/catmaid/fafb/annotations_for_skeletons?ids=1,2,3&raw=true") + await ha_api.handle_catmaid_command(get_req) + + post_req = _post_request( + "/catmaid/fafb/annotations_for_skeletons?raw=true", + body=json.dumps({"ids": "1,2,3"}).encode(), + content_type="application/json") + await ha_api.handle_catmaid_command(post_req) + + (get_key, get_args), (post_key, post_args) = captured + assert get_key == post_key + assert get_args == post_args == ("fafb", "annotations_for_skeletons", + None, {"ids": "1,2,3"}) + + +@sync +async def test_handle_catmaid_command_post_reads_project_and_raw_from_query( + monkeypatch): + """`project`/`raw` are view flags, not payload — they stay on the query + string for POST too, and are stripped out of the params dict.""" + captured = [] + + async def fake_dispatch(request, cache_key, worker_fn, *args, post_fn=None, + **kwargs): + captured.append((args, post_fn)) + return {"ok": True} + + monkeypatch.setattr(ha_api, "_dispatch_to_pool", fake_dispatch) + + request = _post_request( + "/catmaid/fafb/compact_detail?project=2&raw=true", + body=json.dumps({"id": "16", "project": "999", "raw": "false"}).encode(), + content_type="application/json") + await ha_api.handle_catmaid_command(request) + + (instance, command, project, params), post_fn = captured[0] + assert (instance, command, project) == ("fafb", "compact_detail", "2") + assert params == {"id": "16"} # body's project/raw dropped + assert post_fn is ha_api._catmaid_raw_view # raw=true from the query + + +@sync +async def test_handle_catmaid_command_post_rejects_malformed_body(monkeypatch): + async def fake_dispatch(*args, **kwargs): + raise AssertionError("should not reach dispatch on a bad body") + + monkeypatch.setattr(ha_api, "_dispatch_to_pool", fake_dispatch) + + request = _post_request( + "/catmaid/fafb/compact_detail", body=b"{not json", + content_type="application/json") + response = await ha_api.handle_catmaid_command(request) + assert response.status == 400 + assert b"Malformed JSON body" in response.body + + # --------------------------------------------------------------------------- # Live integration — hosted CATMAID + KB, like the rest of the suite # --------------------------------------------------------------------------- diff --git a/src/vfbquery/ha_api.py b/src/vfbquery/ha_api.py index ca6a43a..fafda49 100644 --- a/src/vfbquery/ha_api.py +++ b/src/vfbquery/ha_api.py @@ -30,6 +30,10 @@ GET /catmaid # hosted CATMAID instances GET /catmaid/{instance} # metadata + commands GET /catmaid/{instance}/{command}?ids=[&project=][&raw=true] + POST /catmaid/{instance}/{command} # same command, params in + # the body (JSON or form) — + # for id lists too long for + # a query string GET /health GET /status — queue depth, cache stats & worker utilisation @@ -3783,12 +3787,35 @@ async def handle_docs_json(request): # the untouched CATMAID response, and any # other parameter is forwarded to CATMAID # verbatim. +# POST /catmaid/{instance}/{command} identical to the GET, but parameters — +# `ids` in particular — travel in the +# request body instead of the query +# string: a JSON object, or an +# application/x-www-form-urlencoded / +# multipart form. `project` and `raw` +# are still read from the query string +# either way, since they are view flags +# rather than command payload. This is +# the only way to run a bulk-id command +# (`compact_detail`, +# `annotations_for_skeletons`, ...) over +# more ids than fit under a proxy's +# header/URL length limit — CATMAID +# itself already receives these as a +# POST body (see id_params in +# catmaid_client.CATMAID_COMMANDS); only +# the client-facing transport was +# GET-only. A GET and an equivalent POST +# for the same instance/command/ids +# share one cache entry (method is not +# part of the cache key). # # The heavy lifting — instance registry, id conversion through the KB xrefs, # neuron-id bridging and the response envelope — lives in catmaid_client; -# these handlers only parse the URL and ride the shared dispatch machinery -# (cache, coalescing, queue, compute budget). The envelope is what gets -# cached; `raw` is applied by post_fn so both views share one cache entry. +# these handlers only parse the request and ride the shared dispatch +# machinery (cache, coalescing, queue, compute budget). The envelope is what +# gets cached; `raw` is applied by post_fn so both views share one cache +# entry. # --------------------------------------------------------------------------- #: Query-string keys the /catmaid/{instance}/{command} handler itself reads. @@ -3851,8 +3878,54 @@ async def handle_catmaid_instance(request): instance, known_params=frozenset(), client_error_types=(ValueError,)) +async def _catmaid_body_params(request): + """Parse a POST body into a flat {name: value} dict of forward params. + + Accepts a JSON object — the natural choice for a bulk ``ids`` list, + which travels as a real JSON array rather than a delimited string — or + an ``application/x-www-form-urlencoded`` / ``multipart/form-data`` body + (what a plain ``curl -d`` or an HTML form sends), matching whichever a + caller finds convenient. A body-less POST (``compact_detail`` etc. take + all their input via ``id=``, which can still be short enough for the + query string) is fine and yields ``{}``. + + Raises :class:`ValueError` — turned into a 400 by the caller — on a + JSON body that fails to parse or is not an object; there is nothing + sensible to fall back to at that point. + """ + if not request.can_read_body: + return {} + content_type = (request.content_type or "").split(";", 1)[0].strip().lower() + if content_type == "application/json": + try: + body = await request.json() + except ValueError as exc: + raise ValueError("Malformed JSON body: %s" % exc) + if not isinstance(body, dict): + raise ValueError( + "JSON body must be an object of {parameter: value}, " + "e.g. {\"ids\": [1, 2, 3]}") + return body + # Form-encoded — aiohttp reads x-www-form-urlencoded and multipart + # here; any other content type (or none) comes back empty, same as an + # unparsed GET query would. + form = await request.post() + out = {} + for key in form.keys(): + values = form.getall(key) + out[key] = values[0] if len(values) == 1 else values + return out + + async def handle_catmaid_command(request): - """GET /catmaid/{instance}/{command} — run one pass-through command.""" + """GET or POST /catmaid/{instance}/{command} — run one pass-through + command. GET takes parameters from the query string; POST additionally + accepts them in the request body (see :func:`_catmaid_body_params`), + which is what a bulk id list needs once it is too long for a URL. + `project` and `raw` are always read from the query string — they are + view flags, not command payload, so there is no reason to make POST + callers put them in the body too. + """ instance = request.match_info["instance"].lower() command = request.match_info["command"].lower() if not _CATMAID_INSTANCE_RE.match(instance): @@ -3874,8 +3947,21 @@ async def handle_catmaid_command(request): values = request.query.getall(key) params[key] = values[0] if len(values) == 1 else values + if request.method == "POST": + try: + body_params = await _catmaid_body_params(request) + except ValueError as exc: + return web.json_response({"error": str(exc)}, status=400) + for key in _CATMAID_CONTROL_PARAMS: + body_params.pop(key, None) + params.update(body_params) + # `raw` is deliberately NOT part of the key: both views come from the # one cached envelope, unwrapped by post_fn after cache retrieval. + # The HTTP method is deliberately NOT part of the key either: GET and + # POST are two transports for the same request, and a bulk id list + # sent as a POST body should hit the same cache entry a short-enough + # GET with the same ids would have. cache_key = "catmaid|%s|%s|%s|%s" % ( instance, command, project or "", json.dumps(sorted(params.items()), separators=(",", ":"))) @@ -3962,6 +4048,8 @@ def create_app(max_workers=None, max_concurrent=None, max_queue_depth=None, app.router.add_get("/catmaid/{instance}/", handle_catmaid_instance) app.router.add_get("/catmaid/{instance}/{command}", handle_catmaid_command) app.router.add_get("/catmaid/{instance}/{command}/", handle_catmaid_command) + app.router.add_post("/catmaid/{instance}/{command}", handle_catmaid_command) + app.router.add_post("/catmaid/{instance}/{command}/", handle_catmaid_command) _warn_unreachable_routes(app)