# HG changeset patch
# User Chris Cannam <c.cannam@qmul.ac.uk>
# Date 1476108227 -3600
# Node ID 7bfc07576830aa05f75376cb26e2c79bd93fe7da
# Parent  16acd7d24b1ad625c0c4520cb20e3760f4082628
Ensure id is passed through properly in convert and in server

diff -r 16acd7d24b1a -r 7bfc07576830 bits/RequestOrResponse.h
--- a/bits/RequestOrResponse.h	Fri Oct 07 16:43:18 2016 +0100
+++ b/bits/RequestOrResponse.h	Mon Oct 10 15:03:47 2016 +0100
@@ -51,16 +51,25 @@
     enum Direction {
 	Request, Response
     };
+
+    struct RpcId {
+        enum { Absent, Number, Tag } type;
+        int number;
+        std::string tag;
+    };
     
     RequestOrResponse() : // nothing by default
 	direction(Request),
 	type(RRType::NotValid),
-	success(false) { }
+	success(false),
+        id({ RpcId::Absent, 0, "" })
+    { }
 
     Direction direction;
     RRType type;
     bool success;
     std::string errorText;
+    RpcId id;
 
     Vamp::HostExt::ListResponse listResponse;
     Vamp::HostExt::LoadRequest loadRequest;
diff -r 16acd7d24b1a -r 7bfc07576830 json/VampJson.h
--- a/json/VampJson.h	Fri Oct 07 16:43:18 2016 +0100
+++ b/json/VampJson.h	Mon Oct 10 15:03:47 2016 +0100
@@ -72,8 +72,6 @@
  * success) before using the returned value.
  */
 
-//!!! todo: convert pmapper to err style
-
 class VampJson
 {
 public:
@@ -1048,12 +1046,13 @@
 public:
 
     static json11::Json
-    fromRpcRequest_List() {
+    fromRpcRequest_List(const json11::Json &id) {
 
         json11::Json::object jo;
         markRPC(jo);
 
         jo["method"] = "list";
+        addId(jo, id);
         return json11::Json(jo);
     }
 
@@ -1078,13 +1077,15 @@
     }
     
     static json11::Json
-    fromRpcRequest_Load(const Vamp::HostExt::LoadRequest &req) {
+    fromRpcRequest_Load(const Vamp::HostExt::LoadRequest &req,
+                        const json11::Json &id) {
 
         json11::Json::object jo;
         markRPC(jo);
 
         jo["method"] = "load";
         jo["params"] = fromLoadRequest(req);
+        addId(jo, id);
         return json11::Json(jo);
     }    
 
@@ -1110,13 +1111,15 @@
 
     static json11::Json
     fromRpcRequest_Configure(const Vamp::HostExt::ConfigurationRequest &req,
-                              const PluginHandleMapper &pmapper) {
+                             const PluginHandleMapper &pmapper,
+                             const json11::Json &id) {
 
         json11::Json::object jo;
         markRPC(jo);
 
         jo["method"] = "configure";
         jo["params"] = fromConfigurationRequest(req, pmapper);
+        addId(jo, id);
         return json11::Json(jo);
     }    
 
@@ -1143,13 +1146,15 @@
     static json11::Json
     fromRpcRequest_Process(const Vamp::HostExt::ProcessRequest &req,
                            const PluginHandleMapper &pmapper,
-                           BufferSerialisation serialisation) {
+                           BufferSerialisation serialisation,
+                           const json11::Json &id) {
 
         json11::Json::object jo;
         markRPC(jo);
 
         jo["method"] = "process";
         jo["params"] = fromProcessRequest(req, pmapper, serialisation);
+        addId(jo, id);
         return json11::Json(jo);
     }    
 
@@ -1175,7 +1180,8 @@
     
     static json11::Json
     fromRpcRequest_Finish(const Vamp::HostExt::FinishRequest &req,
-                           const PluginHandleMapper &pmapper) {
+                          const PluginHandleMapper &pmapper,
+                          const json11::Json &id) {
 
         json11::Json::object jo;
         markRPC(jo);
@@ -1185,6 +1191,7 @@
 
         jo["method"] = "finish";
         jo["params"] = fo;
+        addId(jo, id);
         return json11::Json(jo);
     }    
     
diff -r 16acd7d24b1a -r 7bfc07576830 test/test-vampipe-server.sh
--- a/test/test-vampipe-server.sh	Fri Oct 07 16:43:18 2016 +0100
+++ b/test/test-vampipe-server.sh	Mon Oct 10 15:03:47 2016 +0100
@@ -4,7 +4,10 @@
 
 reqfile="/tmp/$$.req.json"
 respfile="/tmp/$$.resp.json"
-trap "rm -f $reqfile $respfile" 0
+allrespfile="/tmp/$$.resp.all"
+expected="/tmp/$$.expected"
+obtained="/tmp/$$.obtained"
+trap "rm -f $reqfile $respfile $allrespfile $obtained $expected" 0
 
 schema=vamp-json-schema/schema
 
@@ -36,12 +39,33 @@
       VAMP_PATH=./vamp-plugin-sdk/examples bin/vampipe-server |
       bin/vampipe-convert response -i capnp -o json |
       while read response ; do
+          echo "$response" >> "$allrespfile"
           validate_response "$response"
       done
 ) <<EOF
 {"method":"list"}
-{"method":"load","params": {"key":"vamp-example-plugins:percussiononsets","inputSampleRate":44100,"adapterFlags":["AdaptInputDomain","AdaptBufferSize"]}}
-{"method":"configure","params":{"handle":1,"configuration":{"blockSize": 8, "channelCount": 1, "parameterValues": {"sensitivity": 40, "threshold": 3}, "stepSize": 8}}}
+{"method":"load","id":6,"params": {"key":"vamp-example-plugins:percussiononsets","inputSampleRate":44100,"adapterFlags":["AdaptInputDomain","AdaptBufferSize"]}}
+{"method":"configure","id":"weevil","params":{"handle":1,"configuration":{"blockSize": 8, "channelCount": 1, "parameterValues": {"sensitivity": 40, "threshold": 3}, "stepSize": 8}}}
 {"method":"process","params": {"handle": 1, "processInput": { "timestamp": {"s": 0, "n": 0}, "inputBuffers": [ [1,2,3,4,5,6,7,8] ]}}}
 {"method":"finish","params": {"handle": 1}}
 EOF
+
+# Expected output, apart from the plugin list which seems a bit
+# fragile to check here
+cat > "$expected" <<EOF
+{"id": 6, "jsonrpc": "2.0", "method": "load", "result": {"defaultConfiguration": {"blockSize": 1024, "channelCount": 1, "parameterValues": {"sensitivity": 40, "threshold": 3}, "stepSize": 1024}, "handle": 1, "staticData": {"basic": {"description": "Detect percussive note onsets by identifying broadband energy rises", "identifier": "percussiononsets", "name": "Simple Percussion Onset Detector"}, "basicOutputInfo": [{"description": "Percussive note onset locations", "identifier": "onsets", "name": "Onsets"}, {"description": "Broadband energy rise detection function", "identifier": "detectionfunction", "name": "Detection Function"}], "category": ["Time", "Onsets"], "copyright": "Code copyright 2006 Queen Mary, University of London, after Dan Barry et al 2005.  Freely redistributable (BSD license)", "inputDomain": "TimeDomain", "key": "vamp-example-plugins:percussiononsets", "maker": "Vamp SDK Example Plugins", "maxChannelCount": 1, "minChannelCount": 1, "parameters": [{"basic": {"description": "Energy rise within a frequency bin necessary to count toward broadband total", "identifier": "threshold", "name": "Energy rise threshold"}, "defaultValue": 3, "extents": {"max": 20, "min": 0}, "unit": "dB", "valueNames": []}, {"basic": {"description": "Sensitivity of peak detector applied to broadband detection function", "identifier": "sensitivity", "name": "Sensitivity"}, "defaultValue": 40, "extents": {"max": 100, "min": 0}, "unit": "%", "valueNames": []}], "programs": [], "version": 2}}}
+{"id": "weevil", "jsonrpc": "2.0", "method": "configure", "result": {"handle": 1, "outputList": [{"basic": {"description": "Percussive note onset locations", "identifier": "onsets", "name": "Onsets"}, "configured": {"binCount": 0, "binNames": [], "hasDuration": false, "sampleRate": 44100, "sampleType": "VariableSampleRate", "unit": ""}}, {"basic": {"description": "Broadband energy rise detection function", "identifier": "detectionfunction", "name": "Detection Function"}, "configured": {"binCount": 1, "binNames": [""], "hasDuration": false, "quantizeStep": 1, "sampleRate": 86.1328125, "sampleType": "FixedSampleRate", "unit": ""}}]}}
+{"jsonrpc": "2.0", "method": "process", "result": {"features": {}, "handle": 1}}
+{"jsonrpc": "2.0", "method": "finish", "result": {"features": {"detectionfunction": [{"featureValues": [0], "timestamp": {"n": 11609977, "s": 0}}]}, "handle": 1}}
+EOF
+
+# Skip plugin list
+tail -n +2 "$allrespfile" > "$obtained"
+
+echo "Checking response contents against expected contents..."
+if ! cmp "$obtained" "$expected"; then
+    diff -u1 "$obtained" "$expected"
+else
+    echo "OK"
+fi
+
diff -r 16acd7d24b1a -r 7bfc07576830 utilities/vampipe-convert.cpp
--- a/utilities/vampipe-convert.cpp	Fri Oct 07 16:43:18 2016 +0100
+++ b/utilities/vampipe-convert.cpp	Mon Oct 10 15:03:47 2016 +0100
@@ -79,6 +79,72 @@
 
 PreservingPluginHandleMapper mapper;
 
+static RequestOrResponse::RpcId
+readJsonId(const Json &j)
+{
+    RequestOrResponse::RpcId id;
+
+    if (j["id"].is_number()) {
+        id.type = RequestOrResponse::RpcId::Number;
+        id.number = j["id"].number_value();
+    } else if (j["id"].is_string()) {
+        id.type = RequestOrResponse::RpcId::Tag;
+        id.tag = j["id"].string_value();
+    } else {
+        id.type = RequestOrResponse::RpcId::Absent;
+    }
+
+    return id;
+}
+
+static Json
+writeJsonId(const RequestOrResponse::RpcId &id)
+{
+    if (id.type == RequestOrResponse::RpcId::Number) {
+        return id.number;
+    } else if (id.type == RequestOrResponse::RpcId::Tag) {
+        return id.tag;
+    } else {
+        return Json();
+    }
+}
+
+template <typename Reader>
+static RequestOrResponse::RpcId
+readCapnpId(const Reader &r)
+{
+    int number;
+    string tag;
+    switch (r.getId().which()) {
+    case RpcRequest::Id::Which::NUMBER:
+        number = r.getId().getNumber();
+        return { RequestOrResponse::RpcId::Number, number, "" };
+    case RpcRequest::Id::Which::TAG:
+        tag = r.getId().getTag();
+        return { RequestOrResponse::RpcId::Tag, 0, tag };
+    case RpcRequest::Id::Which::NONE:
+        return { RequestOrResponse::RpcId::Absent, 0, "" };
+    }
+    return {};
+}
+
+template <typename Builder>
+static void
+buildCapnpId(Builder &b, const RequestOrResponse::RpcId &id)
+{
+    switch (id.type) {
+    case RequestOrResponse::RpcId::Number:
+        b.getId().setNumber(id.number);
+        break;
+    case RequestOrResponse::RpcId::Tag:
+        b.getId().setTag(id.tag);
+        break;
+    case RequestOrResponse::RpcId::Absent:
+        b.getId().setNone();
+        break;
+    }
+}
+
 RequestOrResponse
 readRequestJson(string &err)
 {
@@ -97,7 +163,9 @@
 
     rr.type = VampJson::getRequestResponseType(j, err);
     if (err != "") return {};
-    
+
+    rr.id = readJsonId(j);
+
     VampJson::BufferSerialisation serialisation =
         VampJson::BufferSerialisation::Array;
 
@@ -135,23 +203,25 @@
          VampJson::BufferSerialisation::Base64 :
          VampJson::BufferSerialisation::Array);
 
+    Json id = writeJsonId(rr.id);
+    
     switch (rr.type) {
 
     case RRType::List:
-	j = VampJson::fromRpcRequest_List();
+	j = VampJson::fromRpcRequest_List(id);
 	break;
     case RRType::Load:
-	j = VampJson::fromRpcRequest_Load(rr.loadRequest);
+	j = VampJson::fromRpcRequest_Load(rr.loadRequest, id);
 	break;
     case RRType::Configure:
-	j = VampJson::fromRpcRequest_Configure(rr.configurationRequest, mapper);
+	j = VampJson::fromRpcRequest_Configure(rr.configurationRequest, mapper, id);
 	break;
     case RRType::Process:
 	j = VampJson::fromRpcRequest_Process
-	    (rr.processRequest, mapper, serialisation);
+	    (rr.processRequest, mapper, serialisation, id);
 	break;
     case RRType::Finish:
-	j = VampJson::fromRpcRequest_Finish(rr.finishRequest, mapper);
+	j = VampJson::fromRpcRequest_Finish(rr.finishRequest, mapper, id);
 	break;
     case RRType::NotValid:
 	break;
@@ -178,6 +248,8 @@
 
     rr.type = VampJson::getRequestResponseType(j, err);
     if (err != "") return {};
+
+    rr.id = readJsonId(j);
     
     VampJson::BufferSerialisation serialisation =
         VampJson::BufferSerialisation::Array;
@@ -219,31 +291,33 @@
          VampJson::BufferSerialisation::Base64 :
          VampJson::BufferSerialisation::Array);
 
+    Json id = writeJsonId(rr.id);
+
     if (!rr.success) {
 
-	j = VampJson::fromError(rr.errorText, rr.type);
+	j = VampJson::fromError(rr.errorText, rr.type, id);
 
     } else {
     
 	switch (rr.type) {
 
 	case RRType::List:
-	    j = VampJson::fromRpcResponse_List(rr.listResponse);
+	    j = VampJson::fromRpcResponse_List(rr.listResponse, id);
 	    break;
 	case RRType::Load:
-	    j = VampJson::fromRpcResponse_Load(rr.loadResponse, mapper);
+	    j = VampJson::fromRpcResponse_Load(rr.loadResponse, mapper, id);
 	    break;
 	case RRType::Configure:
 	    j = VampJson::fromRpcResponse_Configure(rr.configurationResponse,
-                                                     mapper);
+                                                    mapper, id);
 	    break;
 	case RRType::Process:
 	    j = VampJson::fromRpcResponse_Process
-		(rr.processResponse, mapper, serialisation);
+		(rr.processResponse, mapper, serialisation, id);
 	    break;
 	case RRType::Finish:
 	    j = VampJson::fromRpcResponse_Finish
-		(rr.finishResponse, mapper, serialisation);
+		(rr.finishResponse, mapper, serialisation, id);
 	    break;
 	case RRType::NotValid:
 	    break;
@@ -263,6 +337,7 @@
     RpcRequest::Reader reader = message.getRoot<RpcRequest>();
     
     rr.type = VampnProto::getRequestResponseType(reader);
+    rr.id = readCapnpId(reader);
 
     switch (rr.type) {
 
@@ -295,6 +370,8 @@
     ::capnp::MallocMessageBuilder message;
     RpcRequest::Builder builder = message.initRoot<RpcRequest>();
 
+    buildCapnpId(builder, rr.id);
+    
     switch (rr.type) {
 
     case RRType::List:
@@ -332,6 +409,7 @@
     rr.type = VampnProto::getRequestResponseType(reader);
     rr.success = true;
     rr.errorText = "";
+    rr.id = readCapnpId(reader);
     int errorCode = 0;
 
     switch (rr.type) {
@@ -368,6 +446,8 @@
     ::capnp::MallocMessageBuilder message;
     RpcResponse::Builder builder = message.initRoot<RpcResponse>();
 
+    buildCapnpId(builder, rr.id);
+
     if (!rr.success) {
 
 	VampnProto::buildRpcResponse_Error(builder, rr.errorText, rr.type);
diff -r 16acd7d24b1a -r 7bfc07576830 utilities/vampipe-server.cpp
--- a/utilities/vampipe-server.cpp	Fri Oct 07 16:43:18 2016 +0100
+++ b/utilities/vampipe-server.cpp	Mon Oct 10 15:03:47 2016 +0100
@@ -30,6 +30,38 @@
 
 static CountingPluginHandleMapper mapper;
 
+static RequestOrResponse::RpcId readId(const RpcRequest::Reader &r)
+{
+    int number;
+    string tag;
+    switch (r.getId().which()) {
+    case RpcRequest::Id::Which::NUMBER:
+        number = r.getId().getNumber();
+        return { RequestOrResponse::RpcId::Number, number, "" };
+    case RpcRequest::Id::Which::TAG:
+        tag = r.getId().getTag();
+        return { RequestOrResponse::RpcId::Tag, 0, tag };
+    case RpcRequest::Id::Which::NONE:
+        return { RequestOrResponse::RpcId::Absent, 0, "" };
+    }
+    return {};
+}
+
+static void buildId(RpcResponse::Builder &b, const RequestOrResponse::RpcId &id)
+{
+    switch (id.type) {
+    case RequestOrResponse::RpcId::Number:
+        b.getId().setNumber(id.number);
+        break;
+    case RequestOrResponse::RpcId::Tag:
+        b.getId().setTag(id.tag);
+        break;
+    case RequestOrResponse::RpcId::Absent:
+        b.getId().setNone();
+        break;
+    }
+}
+
 RequestOrResponse
 readRequestCapnp()
 {
@@ -48,6 +80,7 @@
     RpcRequest::Reader reader = message.getRoot<RpcRequest>();
     
     rr.type = VampnProto::getRequestResponseType(reader);
+    rr.id = readId(reader);
 
     switch (rr.type) {
 
@@ -80,6 +113,8 @@
     ::capnp::MallocMessageBuilder message;
     RpcResponse::Builder builder = message.initRoot<RpcResponse>();
 
+    buildId(builder, rr.id);
+    
     if (!rr.success) {
 
 	VampnProto::buildRpcResponse_Error(builder, rr.errorText, rr.type);
@@ -240,6 +275,7 @@
 	    }
 
 	    RequestOrResponse response = handleRequest(request);
+            response.id = request.id;
 
 	    cerr << "vampipe-server: request handled, writing response"
 		 << endl;