Reorder params by name for correct processing

[zabous]

Reorder params by name - according to JSON RPC spec - these two requests should have the same response: {"id":"0","jsonrpc":"2.0","method":"sluzba.metoda","params":{"param1":"hodnota1","param2":"hodnota2"}}

{"id":"0","jsonrpc":"2.0","method":"sluzba.metoda","params":{"param2":"hodnota2","param1":"hodnota1"}}

[foglcz]

Could you please provide some description, ie. what is it doing and why? :-)

[zabous]

Reorder params by name - according to JSON RPC spec - these two requests should have the same response: {"id":"0","jsonrpc":"2.0","method":"sluzba.metoda","params":{"param1":"hodnota1","param2":"hodnota2"}}

{"id":"0","jsonrpc":"2.0","method":"sluzba.metoda","params":{"param2":"hodnota2","param1":"hodnota1"}}

[scottchiefbaker]

Can you point out in the JSON spec the part you're referring to? In JSON hashes are ordered according to FIFO. I've never seen a JSON-RPC request reorder hash keys before.

[foglcz]

Ah, I see. Originally the server class used magic __call method, which took care of it via _getReflection. Using call_user_func breaks this, I didn't notice, sorry :(

I don't like this solution, since there is a perfectly valid _getReflection method for this too.

Specification is here (point 4.2): http://www.jsonrpc.org/specification#notification Also see third example: http://www.jsonrpc.org/specification#examples

[scottchiefbaker]

by-name: params MUST be an Object, with member names that match the Server expected parameter names. The absence of expected names MAY result in an error being generated. The names MUST match exactly, including case, to the method's expected parameters.

The names we return are exactly the same, so I don't think we're violating the spec.

[foglcz]

But I think following would break, wouldn't it?:

--> {"jsonrpc": "2.0", "method": "subtract", "params": [42, 23], "id": 1}
<-- {"jsonrpc": "2.0", "result": 19, "id": 1}

--> {"jsonrpc": "2.0", "method": "subtract", "params": [23, 42], "id": 2}
<-- {"jsonrpc": "2.0", "result": -19, "id": 2}

[scottchiefbaker]

Wouldn't that be:

by-position: params MUST be an Array, containing the values in the Server expected order.

The difference here is array vs hash right? Try these examples:

curl -d '{"params":["42","23"],"version":1.1,"method":"echo_data","id":1}' -o - http://www.perturb.org/api/json-rpc/
curl -d '{"params":{"param2":"hodnota1","param1":"hodnota2"},"version":1.1,"method":"echo_data","id":1}' -o - http://www.perturb.org/api/json-rpc/

[foglcz]

Oh shit, that was wrong example... The point is to have two named parameters but have them mapped to named parameters of the functions

--> {"jsonrpc": "2.0", "method": "subtract", "params": {"subtrahend": 23, "minuend": 42}, "id": 3}
<-- {"jsonrpc": "2.0", "result": 19, "id": 3}

--> {"jsonrpc": "2.0", "method": "subtract", "params": {"minuend": 42, "subtrahend": 23}, "id": 4}
<-- {"jsonrpc": "2.0", "result": 19, "id": 4}

Of course this means BC break when calling any method, since for now the lib works on positional parameters only regardless of the name and/or way how you called the lib

[scottchiefbaker]

I think the existing library will work fine with named parameters won't it? In the above example you're sending one parameter (it's a hash with multiple elements) in to the function "subtract". Inside of subtract it would look like:

function substract($hash) {
  $sub = $hash['subtrahend'];
  $min = $hash['minued'];

  return $sub - $min;
}

Right? Or am I missing something fundamental here?

[foglcz]

AH! I see the full picture now!

So, the substract function should look like this:

function substract($subtrahend, $minuend) {
    return $subtrahend - $min;
}

And, you should be able to call it like this:

--> {"jsonrpc": "2.0", "method": "subtract", "params": {"subtrahend": 23, "minuend": 42}, "id": 3}
--> {"jsonrpc": "2.0", "method": "subtract", "params": {"minuend": 42, "subtrahend": 23}, "id": 4}

The thing is, we're using json_decode without second parameter. Therefore, the "params" get translated into stdClass and that gets handled internally within the callbacks. Which was not the intended functionality, but it is how it works now.

Closing this PR though, as it's fixing (unintentionally) two different things.

[foglcz]

What I want to say is, that since the "params" don't convert into an array (but an std class), they don't get exploded into function params in call_user_func_array (because an stdClass is passed instead.)

If they would, the problem would be that if we'd mix the parameters around, the parameters passed into the function would mix around too, since they are not name-bound but position-bound.

[scottchiefbaker]

To be be able to call the function in the method you specify PHP itself would have to support named parameters which it currently does not. I know they were talking about it for 7.1, but for now if you want to do named parameters you have to break it out like I specified above (using a single hash).