Re: RPC and REST

Zitat von Jan Schneider <[email protected]>:

> Zitat von Ralf Lang <[email protected]>:
>
>> 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":"[email protected]"}
>
> 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)

Finally, we should from the start go with OpenAPI.

And generally,  
https://docs.microsoft.com/azure/architecture/best-practices/api-design pretty  
well sums up the current state of API art.

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

-- 
dev mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: [email protected]