Commit Graph
6 Commits
Author SHA1 Message Date
Xavier Morel 083c70bbb6 [IMP] core: replace dedicated uid cache by ormcache
Before this, invalidations to the UID cache is not synchronised
between workers because it's an ad-hoc solution (so a user changing
their password or an admin disabling a user would only lock out an
attacker currently using the API of one of possibly several
workers). Shift the entire thing to ormcache which already has proper
support for synchronising cache invalidation between workers.

Also simplify the cache invalidation mess in Users.write because the
caches have been unified into a single registry-level LRU, so the
half-dozen cache clears on specific ormcached methods & models is
pretty much the same as repeatedly calling clear_caches on the current
model.

**However** registry.cache is trivially accessible from server actions
and safe_eval as long as they provide access to a model (through
`model.pool.cache`). Which is common, and an issue given we're very
much putting sensible data in there.

Fix this by renaming `Registry.cache` to `Registry.__cache`, this
requires few editions and mangled names are not accessible from
safe_eval contexts.

The alternative would have been to add more bespoke handling of the
uid cache to hook it into the cache invalidation propagation
machinery.

After discussion with (@)odony, fixing LRU access and using that seems
cleaner and less error-prone.

Note on lazy_property
=====================

Make Registry.cache / Registry.__cache into a regular attribute: the
overhead of the LRU is not that high (compared to that of the registry
itself), it's rare that we *don't* need it, and it's assumed to be a
persisted attribute (it's not just a cache) so making it a normal
attribute seems fine; and lazy_property doesn't work for mangled
names: the name of the property is mangled using the name of the
definition class, but the name of the symbol (fget) is not mangled so
lazy_property would set the __cache attribute but then Python would
lookup _Registry__cache, creating a new cache every access.

And we can't (always) mangle things correctly on `__get__(obj,
owner)`: `owner` is just `type(obj)`, meaning in the case of
inheritance the type we get is the type through which the property is
accessed rather than the one it's defined on. So it would work in the
cases where no inheritance is involved (such as Registry.__cache) but
not in general (lest we want to play around walking the MRO ourselves
to find the definition source, which doesn't seem worth it).

lazy_property *could* be made to work properly on Python 3.6+: the
descriptor protocol gains `__set_name__(name, owner)`, which is called
with the properly mangled name — and with the definition class to boot
(though there might still be issues when overriding lazy properties as
the override will be mangled & named differently... or maybe that's a
feature?). However we're still supporting 3.5 at this point, AFAIK, so
that's not an option. Plus it feels unnecessary / not very useful.

However add an assertion to `lazy_property` so it signals when we try
to use it on a mangled method (as otherwise it kinda sorta work in the
sense that the property / object is accessible but is in effect a
slower way to write a regular property).
2020-08-14 23:03:27 +00:00
Xavier Morel 1ecb0641ef [FIX] core: calling read_group / name_search over xmlrpc
Also non-browser jsonrpc (as it goes through a similar process): for
internal performance reasons, name_search and read_group have been
converted to a *lazy* name_get, so the "display name" is not
unnecessarily computed.

However this is an issue for the RPC endpoints (/xmlrpc and /jsonrpc)
as they have no support for `lazy` and thus tend to blow up and / or
do the wrong thing when trying to output a lazy:

* xmlrpc has no way to handle lazy at all and straight blows up
* jsonrpc falls back to `json_default` so they try to stringify the
  lazy, which might have worked except

*Problematically* both endpoints delegate the actual work to
`dispatch_rpc` which handles dispatching between various services and
ultimately creates a *new* cursor before calling model
methods (`object` service and `execute`/`execute_kw`).

This means by the time the result is serialized to be output, the
lazy's cursor has long been closed, and thus any access to an
unevaluated `lazy` errors out when trying to fetch the underlying
item.

This also means we can't just add a hook to serialize the lazy
in the xmlrpc marshaller, though we do have to do that. We *also* (for
both xmlrpc and jsonrpc) have to force evluation of lazy values before
our cursor is closed, meaning it has to be done right after the method
is invoked, iterating the entire response.

Related to task 2170343

closes odoo/odoo#49286

X-original-commit: e2b5a359c1d5eccbe725c1c3169b4130d7bca49b
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
2020-04-09 09:06:34 +00:00
Raphael Collet 2a2ff6eb5c [FIX] tools: serialize lazy to json 2018-08-16 16:39:20 +02:00
Raphael Collet 0ab6845ed8 [FIX] models: make read_group() call name_get() on demand only
This improves the performance of code using read_group() without actually using
the display names of the result.  It also optimizes name_search().
2018-07-24 17:02:07 +02:00
xmo-odoo fffaf735f5 [FIX] P3: list -> iterable builtins (#16811)
In Python 3:

* various builtins and dict methods were changed to return
  view/iterable objects rather than lists
* and the separate Python 2 view/iterable builtins and methods were
  removed altogether

This is problematic when using these items as list (which the happens
repeatedly in Odoo), but more viciously when iterating *multiple times*
over them (which also happens, which I've messed up multiple times while
writing this, and which is a pain to debug even when you've just created
the issue).

Convert all code using these to semantics-matching cross-version
helper functions to get the LCD behaviour between P2 and P3, and
forbid the builtins via lint.

issue #8530
2017-05-10 09:39:55 +02:00
Raphael Collet 9e64f9f951 [REF] openerp: move openerp to odoo 2016-09-02 17:28:12 +02:00