Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source documentation #4

Open
cinemast opened this issue May 13, 2019 · 8 comments
Open

Source documentation #4

cinemast opened this issue May 13, 2019 · 8 comments

Comments

@cinemast
Copy link
Contributor

No description provided.

@jimlloyd
Copy link

Yes, that would be nice. :)

Seriously, I'm finding it hard to figure out how to use the NamedParamMappingargument of JsonRpcServer::Add(). I have the sense that once I have the right insight that it will make perfect sense, but with no doc or comments in the source that insight is eluding me.

@jimlloyd
Copy link

I did get the right insight. The following javascript code illustrates one RPC:

function createGame({players}) {
    const method = 'createGame';
    const id = 1;
    const game = {players};
    const params = {game};
    return rpc({method, id, params});
}

The required name mapping here is {"game"}. If the params object for the method has multiple required keys, I expect all of them would have to be listed.

@cinemast
Copy link
Contributor Author

Here is a little bit of sample code at least: https://github.com/jsonrpcx/json-rpc-cxx/blob/master/examples/warehouse/main.cpp

@cinemast
Copy link
Contributor Author

I am not sure what you mean with the JavaScript example.

@jimlloyd
Copy link

My apologies -- I didn't see your response, and since I had figured out how it worked, I had no need to come back to check for an answer.

The JavaScript shows how my client side rpc api works. Your name mapping is simply the keys that must appear in the params object for the given method.

@bjorn-jaes
Copy link

Hi there, just stated using the library a few days ago. Took me a bit of reading the source code to get it woking.
So i decided to write down the issues that i encountered.

  1. GetHandle does not support const methods. IE. int foo(int arg) const;
  2. "id" field is used to distinguish between Notifications and Methods.
  3. "params" value cant be an empty object params: { }, but can be params: [ ] or params: null

So i thought i would make an example that covers most of the use case that I have. Though in my case I am only using the server part not the client code.

#include <iostream>
#include <vector>

#include "jsonrpccxx/server.hpp"
#include "nlohmann/json.hpp"

struct Point
{
  int x{};
  int y{}
};

NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Point, x, y);

class Foo
{
public:

  void Notification1() /*const not supported*/
  {
    std::cout << num_ << std::endl;
  }

  void Notification2(int num)
  {
    num_ = num;
    std::cout << num_ << std::endl;
  }

  int Method1()
  {
    return points_.size()
  }

  Point Method2(int index)
  {
    if(index > 0 && index < points_.size())
    {
      return points_[index];
    }
    return {};
 }

  std::vector<Point> Method3(Point point)
  {
    points_.push_back(p);
    return points_;
 }

private:
  int num_{0}
  std::vector<Point> points_{{1,2},{5,10},{6,8}};

};

int main()
{
  jsonrpccxx::JsonRpc2Server rpc_server{};
  Foo foo{};

  bool added = rpc_server.Add("Notification1", jsonrpccxx::GetHandle(&Foo::Notification1, foo));
  assert(added);
  added = rpc_server.Add("Notification2", jsonrpccxx::GetHandle(&Foo::Notification2, foo),  {"num"});
  assert(added);
  added = rpc_server.Add("Method1", jsonrpccxx::GetHandle(&Foo::Method1, foo));
  assert(added);
  added = rpc_server.Add("Method2", jsonrpccxx::GetHandle(&Foo::Method2,    foo),  {"index"});
  assert(added);
  added = rpc_server.Add("Method3", jsonrpccxx::GetHandle(&Foo::Method3,    foo),  {"point"});
  assert(added);

  //Empty params cant be "params": {}
  //No id field on notifications

  std::string req1 = "{"jsonrpc": "2.0", "method": "Notification1", "params":[]}";
  std::string req2 = "{"jsonrpc": "2.0", "method": "Notification2", "params":{"num": 10}}";
  std::string req3 = "{"jsonrpc": "2.0", "method": "Method1", "params": null, "id": 0}";
  std::string req4 = "{"jsonrpc": "2.0", "method": "Method2", "params":{"index": 0}, "id": 1}";
  std::string req5 = "{"jsonrpc": "2.0", "method": "Method3", "params":{"point": {"x": 56, "y": 48}}, "id":2}";

  std::cout << rpc_server.HandleRequest(req1) << std::endl;
  std::cout << rpc_server.HandleRequest(req2) << std::endl;
  std::cout << rpc_server.HandleRequest(req3) << std::endl;
  std::cout << rpc_server.HandleRequest(req4) << std::endl;
  std::cout << rpc_server.HandleRequest(req5) << std::endl;
}

Thought something like this might be useful to have somewhere as a smaller example. As the warehouse example is a bit more complex.

@bjorn-jaes
Copy link

One other thing that I wanted to mention is in regards to the following code in server.hpp [98 - 110].
I was considering creating an issue for this, but given that notifications are not supposed to have a return nothing is really wrong with the code.

if (!has_key(request, "id")) {
  try {
    dispatcher.InvokeNotification(request["method"], request["params"]);
    return json();
  } catch (std::exception &) {
    return json();
  }
} else {
  return {{"jsonrpc", "2.0"}, {"id", request["id"]}, {"result", dispatcher.InvokeMethod(request["method"], request["params"])}};
}

So I had problems when I provided the wrong parameters to a notification. As the above code would catch and return nothing. This meant the message would be sent and no errors happened, but not code was executed. Which was very confusing. So my first instinct was to look at my own code rather then checking the json file. I then read through the implementation to find this. And then implemented Notification as a method instead, and was the able to figure out the error. Not sure it would be possible to add a return json or not, but documentation would definitely help.

@cinemast
Copy link
Contributor Author

Thank you for documenting the usage issues that you found.

Regarding the notifications, they should not return anything by definition of the JSON-RPC 2.0 specification. Also in case of errors.

... The Server MUST NOT reply to a Notification, including those that are within a batch request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants