jsonrpcx/json-rpc-cxx

1.0 Send / Receive functions interface suggestion to support more use cases

dtugend opened this issue · 4 comments

Thank you for your work.

Not sure if you plan to do a 1.0 one day, but in case you do I wanted to suggest to do a more general interface then.

Since your library heavily depends on nlohman/json, it might be a good idea to just have the interfaces pass nlohman json objects instead of strings and leave the serialization / deserialization of those to the caller / consumer.

The reason for that idea is that it would allow to use other features, e.g. such as reading from / writing to streams: https://github.com/nlohmann/json#tofrom-streams-eg-files-string-streams

Hm, I actually like the idea, I am just not sure if it is worth breaking the API. Do you have a concrete connector example where the usage of streams might be beneficial?

There's not yet, but I wanted to use json rpc to communicate between two processes over stdout / stdin and without streams support it's a bit tricky, since I would need to make an extra delimiter for the messages exchanged (or parse the json twice to split it myself somehow).

Maybe the API (btw it is not ABI stable) breakage can be avoided as follows:


Client end:

Introduce new interface:

    class IClientJsonConnector {
    public:
        virtual ~IClientJsonConnector() = default;
        virtual json Send(json &requestJson) = 0;
    };
class JsonRpcClient {
  public:
    JsonRpcClient(IClientConnector &connector, version v) : connector(new ClientJsonConnectorFromClientConnector(connector), v(v) {}
    JsonRpcClient(IClientJsonConnector &jsonConnector, version v) : connector(jsonConnector), v(v) {}

    // Convenience function for people using the new interface, since the parse error is outside now:
    static void ThrowParseErrorJsonRpcException(const json::parse_error &e) { /* [....] */ }
    // [....]    
};

The ClientJsonConnectorFromClientConnector is a helper class that wraps IClientConnector into IClientJsonConnector.


Server end:

For server.hpp there's no need to polute the "JsonRpcServer" I think, can just be done in the JsonRpc2Server :

class JsonRpc2Server {
  public:
   // [....]
   // new:
   json HandleRequestJson(const json &json)  { /* [....] */ }
   // Convenience function for people using the new interface, since the parse error is outside now:
   static json CreateParseErrorResponse(const json::parse_error &parseError) { /* [....] */ }
   // [....]
}

(On the server end we usually have access to the actual instance for JsonRPCServer.)


Not sure if you understand what I intend, but I can try to make a pull request and we can refactor it together until it meets your standards / requirements, if you want.

Additionally we could add helper classes / functions that help implementing for istream and ostream, but that might be beyond your scope (e.g. ClientJsonConnectorFromStreams class on the client end that takes an istream and ostream as argument)

sure, feel free to open a PR.

See #37