[RFC][POC] new XMLRPC endpoint

[xmo-odoo]

The current XMLRPC endpoints (/xmlrpc/ and /xmlrpc/2/) use the old calling conventions (CC). They can't be converted to the new CC as there is not enough information at their level to do so (knowledge of whether/where ids and contexts are). This means new-CC methods must currently be decorated with @model or @multi to be exposed over RPC, or calling them raises a TypeError (number of parameters does not match). The old CC can't be deprecated/removed until an RPC endpoint using the new CC is available.

At the same time, while working on the webservice doc I found out Odoo's xmlrpc layer is pretty idiosyncratic and doesn't really take advantage of xmlrpc and the facilities provided by xmlrpc libraries. The endpoint switch caused by the new CC is the occasion to explore/improve that part as well.

Problems of the existing endpoint / system

1. does RPC over RPC (implements its own Odoo RPC over XML-RPC) so can't take advantage of library conveniences when they exist
2. only works with dicts and lists, not recordset or other types of mappings and sequences
3. doesn't allow None/nil/null, will break clients if support is enabled, this is an issue because json-rpc serialises None by default so devs lose the habit of returning a dummy placeholder as it works fine from the web client
4. non-standard authentication

Calling Conventions (improves on 1 and 2)

a_call(subject[, args:list][, kwargs:dict])

Splits the ids (if any) and context (if any) from positional and keyword arguments in order to better match the "new API" calling conventions (where these informations are respectively part of the recordset and environment not the method call). In the RPC layer, these two items are part of the subject positional argument which can be:

• falsy, in which case the method is called on an empty recordset with a context-less environment
• a list, which is assumed to be a list of record ids used for the subject recordset
• a mapping which may provide the keys records and context. The records key is browsed (or ignored if missing/empty) and the context key is used to seed the Environment (or ignored if missing)

This scheme seems to provide the best flexibility and terseness, as well as a measure of extensibility by allowing / using more keys from the subject mapping in the future.

Like execute_kw, calls then take a args list and a kwargs dict, both are optional rather than just the kwargs: a method can be called with only args, only kwargs, both or neither.

Custom XMLRPC serializer (improves on 2)

New CC methods can return recordsets, which the new RPC endpoint has to automatically convert somehow, the easiest was to serialise recordsets as lists of ids (that's coherent with the old/new API decorators, and means e.g. search returns the same thing).

Customizing xmlrpclib.Marshaller didn't work because it dispatches by strict type equality, so it wasn't possible to hook into it to serialise arbitrary recordsets (they get types created on-the-fly). A new marshaller was reimplemented on top of lxml instead. It provides the following features not provided by xmlrpclib.Marshaller:

• serialises recordsets to lists of ids, uses BaseModel.ids (ignores NewId)
• converts all mapping keys to strings (same as json.dumps) instead of raising an error (see XmlRcp error: dictionary key must be string #12667 and probably others I didn't find)
• can serialise arbitrary mappings (e.g. Counter, defaultdict) not just dict
• can serialise arbitrary iterables, not just list and tuple

Allow None/nil/null (fixes 3)

Of the 4 libraries we have examples for, 3 either enable the feature by default or via a simple flag. The last one (Apache XML-RPC) can be coerced into supporting it using ~15 lines of code which is downright terse as far as the combination of Java and dynamically typed RPC goes.

This allows more natural returns from methods (aka don't return anything instead of the artificial unconditional return True and return False peppered through the codebase) and is in line with JSON-RPC.

Endpoint (avoids conflict with existing)

Uses /RPC2:

• does not conflict with existing endpoints
• used for the examples in the XML-RPC specification
• a number of documents call it the conventional XML-RPC path
• at least one stdlib defaults to /RPC2 when no path is provided

Authentication (fixes 4, also improves on 1)

All xmlrpc libraries surveyed provide built-in support for Basic Authentication. The new endpoint thus replaces the custom authentication by BA

• no less (or more) secure than the existing scheme: relies on underlying transport being HTTPS for security
• avoids an extraneous auth step to resolve a uid (although that means if the "current user"'s uid is needed it has to be resolved separately)
• no need to keep uid/password around, they're embedded in the xmlrpc proxy object
• easy to integrate (Basic sends "cleartext" username and password, they're just base64-encoded)

Leverage XML-RPC conventions (improves on 1)

XML-RPC methodName is conventionally a dotted path (e.g. system.listMethods), which some libraries (ripcord, xmlrpclib) can map onto the language's own dereferencing/attribute access. This maps nicely to Odoo's models: the last segment of the methodName is the method itself, the rest is the model name. This means read for account.account can be called as:

a_db.account.account.read(…)

Ruby's stdlib XMLRPC::Client has no separation between dereferencing and method call, but it's still possible to proxy to a model then call a number of methods on it e.g.

partner = a_db.proxy('res.partner')
partner.read(...)
partner.write(...)

Because authentication depends on the db (or lack thereof), the database name is part of the endpoint URL (query parameter)

• Global methods (e.g. create/drop database) are done on a DB-less endpoint
• DB-specific methods (none currently but maybe printing reports? Sending workflow signals?) are function calls on DB'd endpoint (straight methodName without dots)
• method calls on models are calls on DB'd endpoint with at least one dot in methodName

Code comparison

setup

common = xmlrpclib.ServerProxy('https://{}/xmlrpc/2/common'.format(domain))
uid = common.authenticate(db, username, password, {})
models = xmlrpclib.ServerProxy('https://{}/xmlrpc/2/object'.format(domain))
partners = lambda *args: models.execute_kw(db, uid, password, 'res.partner', *args)
partners('read', …)

becomes

db = xmlrpclib.ServerProxy('https://{}:{}@{}/RPC2/{}'.format(username, password, domain, db))
partners = db.res.partner
partners.read(…)

or in Ruby

common = XMLRPC::Client.new(host, "/xmlrpc/2/common", 8069)
uid = common.authenticate(db, username, password, {})
models = XMLRPC::Client.new(host, "/xmlrpc/2/object", 8069)
partners = lambda {|*args| models.execute_kw(db, uid, password, 'res.partner', *args) }
partners.call('read', …)

becomes

db = XMLRPC::Client.new(host, '/RPC2/' + db, 8069, nil, nil, username, password)
partners = db.proxy('res.partner')
partners.read(…)

simple method call

models.execute_kw(db, uid, password, 'res.partner', 'check_access_rights', ['read'], {'raise_exception': False})

becomes

partners.check_access_rights((), ['read'], {'raise_exception': False})

search

models.execute_kw(db, uid, password, 'res.partner', 'search', [[['is_company', '=', True], ['customer', '=', True]]])

becomes

partners.search((), [[['is_company', '=', True], ['customer', '=', True]]])

read

models.execute_kw(db, uid, password, 'res.partner', 'read', [ids])

becomes

partners.read(ids)

JSON-RPC ISO

The endpoint also supports JSON-RPC2 with the same signature, using basic auth. The dispatching is performed based on the request's mime type.

Nota: maybe the XML-RPC serialiser should reject datetimes and binaries? These are not types JSON understands...

Possible additions/reflexions

• standard extension system.multicall, allows performing multiple independent calls over a single RPC requests, has somewhat built-in support on all reviewed RPC libraries (Python's stdlib, Ruby's stdlib, PHP's Ripcord and Java's Apache XML-RPC)

• similar but non-standard extensions system.cascade and system.sequence, the first would allow a sequence of ~independent calls to the same subject, the latter would instead be a call chain, where call 2 would be performed on the result of call 1 e.g.

  cascade(subject, action_confirm, read)
  

  would call "action_confirm()" then "read()" on the subject but

  sequence(subject, mapped('partner_ids'), read)
  

  would get the subject's associated partners then read those.

/cc @odony @rco-odoo

[KangOl]

Why written as a module? This can be done in the core.

[xmo-odoo]

Because it's a proof of concept, that way I don't have to deal with conflicts if it sits here for 3 years.

[antonylesuisse]

@xmo-odoo we nned to discuss this with @odony when you come back. I think it should be address in a much more simple and backward compatible way.

[xmo-odoo]

Should be with(closing(registry.cursor())) for identical behaviour, otherwise the transaction will be committed. Shouldn't have any impact though, possibly slightly less traffic (cursors are rollbacked when closed, so a regular with cursor will commit then rollback I think)

[tde-banana-odoo]

@xmo-odoo Any news on this ? I heard it should be compatible with Python 1.3.

cc @Julien00859 who is that kind of guy who still works with python 1.

[tde-banana-odoo]

@xmo-odoo we nned to discuss this with @odony when you come back. I think it should be address in a much more simple and backward compatible way.

Did you discuss ?

[tivisse]

Should be merged in 17.0 once the small conflict with http.py is resolved.

#neverstopeveloving

[tivisse]

Because it's a proof of concept, that way I don't have to deal with conflicts if it sits here for 3 years.

Well, it's been 9 years and you do have conflicts ¯_(ツ)_/¯

[tde-banana-odoo]

It seems your last rebase did not go well, could you do it again ? There are still some conflicts.

[Yenthe666]

By now this pull request deserves some special medal or award I believe 😄 Waiting for the 10-year party in december!

[tde-banana-odoo]

v18 is near ! Is it merged ?

[william-andre]

Only 7 files conflicting, mostly doc.
Should be fixed in a few seconds

[tde-banana-odoo]

@Julien00859 Could you rebase and r+ ?