Implement batch client

[francislavoie]

The client implementation here is pretty simplistic, it just generates the request JSON but doesn't really do anything with the response JSON.

Would be nice if it was enhanced to also handle batch responses and validate them etc.

I may write a PR later if I find that I actually need it.

[smortensen]

For general usefulness, it would be ideal to let the end-user handle the validation of their own data. But the client should be validating that the JSON-RPC response is a well-formed response object: http://www.jsonrpc.org/specification

Right now, some of the internal JSON-RPC format details are leaking outside of the client, since the end-user has to know about the structure of the response ({"jsonrpc": "2.0", "result": ..., "id": ...}). Maybe there is a way to keep those JSON-RPC details internal to the class?

[francislavoie]

One idea: if it's a valid JSON-RPC response (has jsonrpc, id, result/error), then we could turn those results into objects like this (kinda pseudocode):

return [
  "$id_1" => [
    'success' => isset($result),
    'data' => $result ?? $error,
  ],
  "$id_2" => [
    'success' => isset($result),
    'data' => $result ?? $error,
  ],
  ...
];

I.e. flatten the data to key -> value pairs where the key is the request ID. Maybe I'm crazy though :stuck_out_tongue_winking_eye:

Would have the advantage of quick array access by ID when dealing with mapping the results to their request ID

[francislavoie]

Another random idea: Could extend query() to take a closure as the 4th arg as a results callback? Would map the result directly to where it needs to go

[smortensen]

The id is a fickle beast. The specifications allow it to be a null, integer, float, or string value.

If we use this id as the key in a PHP array, then some of these id values will be coerced by PHP into a different key. For example, PHP will convert a null key to an empty string, a float key will be truncated down to an integer, and some string keys (e.g. "1") will be coerced into integers. As a result, some distinct id values will be converted into the same key--causing a data loss! Gosh! And PHP thought it was being helpful.

But, we would be safe if we were to present a zero-indexed list of response and error entities. We could turn the error and response entities into Error and Response objects instead of raw associative arrays. Each entity would have an id attached to it. What do you think? Would that work?

[francislavoie]

Dang. Good point re: IDs. I feel like in 99% of cases if you're acting as the client yourself, then you won't use a weird value as the ID, so it's probably a safe assumption to make for this lib. It could be optional opt-in behavior too.

I think Error/Response objects would be good. They'd need to share a base class and have an "hasError" and "hasResult" function to differentiate. Would allow for implementing a Promise-like syntax where you can provide a result and error callback from a higher-level API too, with class type hinting for those callbacks.

[smortensen]

So, I think the callback idea would belong to one of the higher classes, for example: https://github.com/datto/php-json-rpc-http

...because it's at a higher level than just the raw JSON-RPC syntax.

Also--and here's a crazy idea--what if that HTTP library were to do away with the ids altogether? Maybe the HTTP client could just have a function signature like this:

public function query($method, array $arguments = null, &$response = null) { ... }

And then the library could auto-generate an id for each request, and use that id to insert the correct response into the $response variable for the user.

That would almost be like room service. Would it make people too lazy?

[francislavoie]

Nah I think auto-generating IDs is definitely fair. I was looking at a Swift implementation last week, https://github.com/bricklife/JSONRPCKit and it implements a NumberIdGenerator() which yields $id++ every time it's invoked. We could do that with PHP generators with the yield keyword! Can make it an optional arg, i.e. default implementation as that kind of generator but allowing a custom implementation to be passed if they don't want numbers.

[francislavoie]

Actually an additional feature that might be nice is a reset() function on the RPC Client to clear the queries and any other state. Would be nice to allow reuse of a client class over and over. Not a huge deal, but it could be useful.

[francislavoie]

Another thing that I think wouldn't hurt to do is make query() return $this so it becomes easily chainable. No real downside to that I think.

brainstorming 😂

Edit: Just wanted to mention this anecdotally - this lib really inspired me to just totally drop REST altogether in most of my apps. It's just so much cleaner to have all the actual semantics of the API centralized into the request/response body rather than also having meaning from status codes, headers, paths, verbs... The mobile apps were doing crazy code gymnastics to deal with all the different possible status codes, verbs, paths, etc. that my code could throw at them. Now it's much cleaner. Auth stuff happens entirely within headers, execution happens entirely within the body. It's already saving us a ton of time in turnaround for implementing new features 😄

FWIW this is how my controllers look when acting like an RPC server, which I think is so clean!

use Datto\JsonRpc\Server as RpcServer;
use Datto\JsonRpc\Evaluator as RpcEvaluator;
use Datto\JsonRpc\Exception as RpcException;
use Illuminate\Http\Request;

class Controller extends BaseController implements RpcEvaluator
{
    // JSON-RPC entrypoint
    public function rpc(Request $request)
    {
        $server = new RpcServer($this);
        $reply = $server->reply($request->getContent());

        return response($reply)->header('Content-Type', 'application/json');
    }

    // JSON-RPC method handler 
    public function evaluate($method, $arguments)
    {
        switch($method) {
            case 'index': return $this->rpcIndex($arguments);
            case 'add': return $this->rpcAdd($arguments);
            case 'subtract': return $this->rpcSubtract($arguments);
            default: throw new RpcException\Method();
        }
    }

    [...]
}

[smortensen]

A new release (5.0.0) is out: https://github.com/datto/php-json-rpc/blob/master/CHANGELOG.md

• The JSON server response is now validated, and converted into an array of "Response" objects.
• The "query" and "notify" methods now return the object handle, so method calls can be chained.
• Batch response are supported. The client resets after each encoding.

I'm planning to use references in the "php-json-rpc-http" library to get rid of the request ids, while keeping full JSON-RPC 2.0 support in this low-level library.

A lot of the ideas in this release came from your messages, Francis: Would you willing to provide feedback on another project of mine as well? I value your feedback.

Here's my email address: http://lens.guide/contact/email.jpg

~Spencer