Accept POST as well as GET on the CATMAID pass-through

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
 # ---------------------------------------------------------------------------

src/vfbquery/ha_api.py

@@ -30,6 +30,10 @@
     GET /catmaid                                       # hosted CATMAID instances
     GET /catmaid/{instance}                            # metadata + commands
     GET /catmaid/{instance}/{command}?ids=<skids/VFB 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)