Add sample handler and docs for non-JSON body

sphinx/source/developers/logging_cpp.rst

@@ -57,17 +57,29 @@ The logging app defines :cpp:class:`ccfapp::LoggerHandlers`, which creates and i
     :end-before: SNIPPET_END: get
     :dedent: 6
 
-Each function is installed as the handler for a specific RPC ``method``, optionally with schema included:
+This handler uses the simple signatures provided by the ``json_adapter`` wrapper function, which handles parsing of a JSON params object from the HTTP request body.
+
+Each function is installed as the handler for a specific RPC ``method``, the name of the HTTP resource at which your handler will be invoked:
 
 .. literalinclude:: ../../../src/apps/logging/logging.cpp
     :language: cpp
     :start-after: SNIPPET_START: install_get
     :end-before: SNIPPET_END: install_get
     :dedent: 6
 
-These handlers use the simple signature provided by the ``handler_adapter`` wrapper function, which pre-parses a JSON params object from the HTTP request body.
+The return value from ``install`` is a ``Handler&`` object which can be used to alter how the handler is executed. For example, the handler for ``LOG_get`` shown above sets a `schema` for the handler, which will be used in calls to the ``/getSchema`` endpoint. It also marks the handler as `GET-only`, so the framework will return a ``405 Method Not Allowed`` for any requests which are not HTTP ``GET``.
+
+To process the raw body directly, a handler should use the general lambda signature which takes a single ``RequestArgs&`` parameter. Examples of this are also included in the logging sample app. For instance the ``log_record_text`` handler takes a raw string as the request body:
+
+.. literalinclude:: ../../../src/apps/logging/logging.cpp
+    :language: cpp
+    :start-after: SNIPPET_START: log_record_text
+    :end-before: SNIPPET_END: log_record_text
+    :dedent: 6
+
+Rather than parsing the request body as JSON and extracting the message from it, in this case `the entire body` is the message to be logged, and the ID to associate it with is passed as a request header.
 
-For direct access to the request and response objects, the handler signature should take a single ``RequestArgs&`` argument. An example of this is included in the logging app:
+This general form of handler (taking a single ``RequestArgs&`` parameter) also allows a handler to see additional caller context. An example of this is the ``log_record_prefix_cert`` handler:
 
 .. literalinclude:: ../../../src/apps/logging/logging.cpp
     :language: cpp

src/apps/logging/logging.cpp

@@ -28,6 +28,7 @@ namespace ccfapp
 
     static constexpr auto LOG_RECORD_PREFIX_CERT = "LOG_record_prefix_cert";
     static constexpr auto LOG_RECORD_ANONYMOUS_CALLER = "LOG_record_anonymous";
+    static constexpr auto LOG_RECORD_RAW_TEXT = "LOG_record_raw_text";
   };
 
   // SNIPPET: table_definition
@@ -214,6 +215,46 @@ namespace ccfapp
           return make_success(true);
         };
 
+      // SNIPPET_START: log_record_text
+      auto log_record_text = [this](RequestArgs& args) {
+        const auto expected = http::headervalues::contenttype::TEXT;
+        const auto actual =
+          args.rpc_ctx->get_request_header(http::headers::CONTENT_TYPE)
+            .value_or("");
+        if (expected != actual)
+        {
+          args.rpc_ctx->set_response_status(HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE);
+          args.rpc_ctx->set_response_header(
+            http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
+          args.rpc_ctx->set_response_body(fmt::format(
+            "Expected content-type '{}'. Got '{}'.", expected, actual));
+          return;
+        }
+
+        constexpr auto log_id_header = "x-log-id";
+        const auto id_it = args.rpc_ctx->get_request_header(log_id_header);
+        if (!id_it.has_value())
+        {
+          args.rpc_ctx->set_response_status(HTTP_STATUS_BAD_REQUEST);
+          args.rpc_ctx->set_response_header(
+            http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
+          args.rpc_ctx->set_response_body(
+            fmt::format("Missing ID header '{}'", log_id_header));
+          return;
+        }
+
+        const auto id = strtoul(id_it.value().c_str(), nullptr, 10);
+
+        const std::vector<uint8_t>& content = args.rpc_ctx->get_request_body();
+        const std::string log_line(content.begin(), content.end());
+
+        auto view = args.tx.get_view(records);
+        view->put(id, log_line);
+
+        args.rpc_ctx->set_response_status(HTTP_STATUS_OK);
+      };
+      // SNIPPET_END: log_record_text
+
       install(Procs::LOG_RECORD, json_adapter(record), Write)
         .set_auto_schema<LoggingRecord::In, bool>();
       // SNIPPET_START: install_get
@@ -236,6 +277,7 @@ namespace ccfapp
         Write)
         .set_auto_schema<LoggingRecord::In, bool>()
         .set_require_client_identity(false);
+      install(Procs::LOG_RECORD_RAW_TEXT, log_record_text, Write);
 
       nwt.signatures.set_global_hook([this, &notifier](
                                        kv::Version version,

tests/e2e_logging.py

@@ -73,7 +73,7 @@ def test_cert_prefix(network, args):
 
 
 @reqs.description("Write as anonymous caller")
-@reqs.supports_methods("LOG_record_prefix_cert", "LOG_get")
+@reqs.supports_methods("LOG_record_anonymous", "LOG_get")
 def test_anonymous_caller(network, args):
     if args.package == "liblogging":
         primary, _ = network.find_primary()
@@ -101,6 +101,31 @@ def test_anonymous_caller(network, args):
     return network
 
 
+@reqs.description("Write non-JSON body")
+@reqs.supports_methods("LOG_record_raw_text", "LOG_get")
+def test_raw_text(network, args):
+    if args.package == "liblogging":
+        primary, _ = network.find_primary()
+
+        log_id = 101
+        msg = "This message is not in JSON"
+        with primary.user_client() as c:
+            r = c.rpc(
+                "LOG_record_raw_text",
+                msg,
+                headers={"content-type": "text/plain", "x-log-id": str(log_id)},
+            )
+            assert r.status == http.HTTPStatus.OK.value
+            r = c.rpc("LOG_get", {"id": log_id})
+            assert r.result is not None
+            assert msg in r.result["msg"]
+
+    else:
+        LOG.warning("Skipping test_cert_prefix as application is not C++")
+
+    return network
+
+
 @reqs.description("Testing forwarding on member and node frontends")
 @reqs.supports_methods("mkSign")
 @reqs.at_least_n_nodes(2)
@@ -189,6 +214,7 @@ def run(args):
             network = test_update_lua(network, args)
             network = test_cert_prefix(network, args)
             network = test_anonymous_caller(network, args)
+            network = test_raw_text(network, args)
 
 
 if __name__ == "__main__":

tests/infra/clients.py

@@ -37,11 +37,14 @@ def truncate(string, max_len=256):
 
 
 class Request:
-    def __init__(self, method, params=None, readonly_hint=None, http_verb="POST"):
+    def __init__(
+        self, method, params=None, readonly_hint=None, http_verb="POST", headers={}
+    ):
         self.method = method
         self.params = params
         self.readonly_hint = readonly_hint
         self.http_verb = http_verb
+        self.headers = headers
 
 
 def int_or_none(v):
@@ -78,10 +81,20 @@ def to_dict(self):
         return d
 
     def from_requests_response(rr):
+        content_type = rr.headers.get("content-type")
+        if content_type == "application/json":
+            parsed_body = rr.json()
+        elif content_type == "text/plain":
+            parsed_body = rr.text
+        elif content_type is None:
+            parsed_body = None
+        else:
+            raise ValueError(f"Unhandled content type: {content_type}")
+
         return Response(
             status=rr.status_code,
-            result=rr.json() if rr.ok else None,
-            error=None if rr.ok else rr.text,
+            result=parsed_body if rr.ok else None,
+            error=None if rr.ok else parsed_body,
             commit=int_or_none(rr.headers.get(CCF_COMMIT_HEADER)),
             term=int_or_none(rr.headers.get(CCF_TERM_HEADER)),
             global_commit=int_or_none(rr.headers.get(CCF_GLOBAL_COMMIT_HEADER)),
@@ -93,10 +106,21 @@ def from_raw(raw):
         response.begin()
         raw_body = response.read(raw)
         ok = response.status == 200
+
+        content_type = response.headers.get("content-type")
+        if content_type == "application/json":
+            parsed_body = json.loads(raw_body)
+        elif content_type == "text/plain":
+            parsed_body = raw_body.decode()
+        elif content_type is None:
+            parsed_body = None
+        else:
+            raise ValueError(f"Unhandled content type: {content_type}")
+
         return Response(
             status=response.status,
-            result=json.loads(raw_body) if ok else None,
-            error=None if ok else raw_body.decode(),
+            result=parsed_body if ok else None,
+            error=None if ok else parsed_body,
             commit=int_or_none(response.getheader(CCF_COMMIT_HEADER)),
             term=int_or_none(response.getheader(CCF_TERM_HEADER)),
             global_commit=int_or_none(response.getheader(CCF_GLOBAL_COMMIT_HEADER)),
@@ -215,23 +239,28 @@ def _just_request(self, request, is_signed=False):
                 url,
                 "-X",
                 request.http_verb,
-                "-H",
-                "Content-Type: application/json",
                 "-i",
                 f"-m {self.request_timeout}",
             ]
 
             if not is_get:
-                msg = (
-                    json.dumps(request.params).encode()
-                    if request.params is not None
-                    else bytes()
-                )
-                LOG.debug(f"Writing request body: {msg}")
-                nf.write(msg)
+                if request.params is None:
+                    msg_bytes = bytes()
+                elif isinstance(request.params, bytes):
+                    msg_bytes = request.params
+                else:
+                    msg_bytes = json.dumps(request.params).encode()
+                LOG.debug(f"Writing request body: {msg_bytes}")
+                nf.write(msg_bytes)
                 nf.flush()
                 cmd.extend(["--data-binary", f"@{nf.name}"])
 
+            # Set requested headers first - so they take precedence over defaults
+            for k, v in request.headers.items():
+                cmd.extend(["-H", f"{k}: {v}"])
+
+            cmd.extend(["-H", "Content-Type: application/json"])
+
             if request.readonly_hint:
                 cmd.extend(["-H", f"{CCF_READ_ONLY_HEADER}: true"])
 
@@ -241,6 +270,7 @@ def _just_request(self, request, is_signed=False):
                 cmd.extend(["--key", self.key])
             if self.cert:
                 cmd.extend(["--cert", self.cert])
+
             LOG.debug(f"Running: {' '.join(cmd)}")
             rc = subprocess.run(cmd, capture_output=True)
 
@@ -319,6 +349,8 @@ def _just_request(self, request, is_signed=False):
         if request.readonly_hint:
             extra_headers[CCF_READ_ONLY_HEADER] = "true"
 
+        extra_headers.update(request.headers)
+
         request_args = {
             "method": request.http_verb,
             "url": f"https://{self.host}:{self.port}/{request.method}",