[dev] RPC and REST

Fri Feb 16 16:24:13 UTC 2018

Am 16.02.2018 um 16:27 schrieb 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
> 

I totally agree. I think there was another answer on this initial draft
where you already pointed out the examples weren't really RESTful.

I personally also love the json body version. Purists will probably say
it's not HATEOS and JSON isn't really self-describing, but we can come
up with a complicated xml version later, if anybody wants to pay for
that ;-)

>>> /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.

Yes.

> 
>>> 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.

I agree. That's what the registry API provides (and in some cases, it
already passes Objects).

> 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.

I'd like to build the new infrastructure and test-drive it with REST
before moving the RPC subsystems over to it. Breaks less stuff.

>> 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 ;-)

;-)

Yeah, that would be the better approach IF we use inflection.

>>> 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).

I like that.

> 
>>> 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.

That's some weekend read :)

Nice weekend

Ralf

-- 
Ralf Lang
Linux Consultant / Developer
Tel.: +49-170-6381563
Mail: lang at b1-systems.de
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537