[dev] RPC and REST

[Jan Schneider]

Zitat von Ralf Lang <lang at b1-systems.de>:

> Am 17.10.2017 um 22:07 schrieb Ralf Lang:
>> Hallo,
>>
>> lately I have been examining the REST and RPC topics discussed in January.
>>
>> Bad news is it doesn't quite fit and I am not sure if it's a good  
>> idea to cram it into the RPC framework. It looks more and more like  
>> Rest should sit on top of the inter-app API, separate from RPC.
>>
>>
>> For example, let's have
>>
>> PUT /rpc/rest/admin/user/henry
>>
>> to create a new user "henry" (identity details and auth credentials  
>> random or undefined)

This is not good (or correct) API design. The following is broken:
- PUT instead of POST which is more correct for creating new items and  
doesn't require the method to be idempotent
- singular instead of plural for resource names (see below)
- Not using a standardized payload.

Instead, this should be a

POST /rpc/rest/admin/users

request with a JSON body:

{"user":"henry","name":"Henry Hillabilly","email":"henry at example.com"}

or, even better if we want to separate REST and RPC anyway:

POST /rest/v1/users

>> /rpc/ (or /rpc.php) is our universal endpoint for all rpc-ish stuff.
>> /rpc/rest/ to discern rest from other users of the rpc endpoint  
>> (dav, phpgw, json-rpc, whatever)
>>
>> The existing RPC backends all have a clear and easy separation of  
>> api, command/method and parameters which can be mapped to the  
>> internal inter-app api and are happy to just return any scalar or  
>> array structure as the single result.
>>
>>
> Maybe it's more straight forward to do one thing at a time - provide  
> a minimal rest API now, assuming data is always scalars and arrays  
> (which it currently almost always is) and split/wrap inter-app from  
> external apis in a separate move. It's just getting too much for a  
> single change.

This might make sense, besides using JSON instead of arrays. Those  
would need a serialization anyway.

>> To make REST automatically fit into the existing RPC framework, it  
>> would require to limit it to a fixed /api/resource/$param/$param..  
>> format with resource and method defining the horde api method - and  
>> then we would need to create method names like admin.putUser("henry")
> No, these are already there - at least turba and kronolith have the  
> sync/DAV-oriented browse()/put()/delete() and friends. A minimum  
> rest api could translate this to get/put/post on the app's primary  
> collection (calendar, addressbook) and item event, contact) types.

I still think we need a generic, simple API that's also being used  
inter-app and may event pass Horde objects around.
Then we need wrappers for each RPC type (REST too), and serializers  
for those type too, if necessary.

But this still could be done step by step.

> For the proposed admin/ api, collections would be user(s), group(s),  
> permission(s)
>
> Is there any english language pluralization on any horde groupware  
> items that does not pluralize by adding "s"?

It's probably not a good idea to use rules for those, since they are  
going to break. Instead, always use plural. "GET /rest/users/henry" is  
just as fine.

That being said, we have a class for pluralizing, see  
Horde_Support_Inflector ;-)

>> We can make this more straight forward if every resource is its own api
>>
>> user.put("henry") -> PUT /rpc/rest/user/henry
>>
>> Of course, allowing for named parameters would make this more  
>> versatile but registry->call does not have named parameters.

Yes, we absolutely need named parameters, though this covered when  
using JSON objects in the REST API.

>> Still, methods like put and get instead of create/list don't blend  
>> well with the rest of rpc methods.
>>
>> Bottom line is:
>>
>> It might make sense to have rpc.php as the common endpoint for  
>> everything (dav, json-rpc, rest).
>>
>> It might also make sense to have both RPC and REST wrap the bare  
>> inter-app API into something providing more context on allowed,  
>> required and optional parameters (and formats, auth / no auth,  
>> limiting...) but I do not see any way to make REST feel right as  
>> yet another Rpc driver interfacing the same set of remote execution  
>> methods. At least not without yet another wrapping layer to be  
>> coded for each exposed resource.

Agreed.

>> Apart from that, Horde_Rpc has some limitations I looked into:
>>
>> It's implicitly assuming to have a horde registry global variable  
>> around to provide Api Methods (see Horde_Rpc_Xmlrpc) line 34

Known issue, it never has been refactored for Horde 4.

>> It's not allowing any parameter to override this
>>
>> The specific class factory is part of the base class
>>
>> No way to ensure methods for Horde inter-app communication and  
>> external Apis can be separated

Absolutely, see above.

>> Authentication and Authorization is not really separated from the modules

Known issue, high prio.

>> For example, some reading calls may make sense to expose to the  
>> unauthenticated public, others should only be available to  
>> authenticated users, to admins or to users with a specific horde  
>> permission.
>>
>> Inter-App calls should be able to yield PHP Objects, but the Rpc  
>> modules are currently not fit to auto-convert them into arrays and  
>> scalars for serialization to external interfaces

Agreed, see above.

>> If an app has two apis, currently all methods show up for both  
>> apis. Separate per api classes instead of one monolithic Api.php  
>> per app may help.

Indeed, not good.

>> I'd like to go for separate per action classes but this creates  
>> problems with implementing listMethods, unless I explicitly declare  
>> them in registry.php or in said per api class.

I think this is overshooting. It should be per-resource classes with  
the individual actions, later mapped to REST calls. Maybe via  
annotations. Or naming conventions (interfaces).

>> Separate the api provider (list of valid methods) from the Rpc  
>> backends (ship a dummy provider with Horde_Rpc, ship the  
>> horde-specific provider with Horde_Core)
>>
>> Separate the authentication provider from the Rpc backends (for  
>> now, AuthHorde, AuthSeparate and Null should be sufficient)



-- 
Jan Schneider
The Horde Project
https://www.horde.org/