diff --git a/content/developer/howtos/rdtraining/06_firstui.rst b/content/developer/howtos/rdtraining/06_firstui.rst index cb37e41ec..15ebbed0c 100644 --- a/content/developer/howtos/rdtraining/06_firstui.rst +++ b/content/developer/howtos/rdtraining/06_firstui.rst @@ -252,7 +252,7 @@ Reserved Fields --------------- **Reference**: the documentation related to this topic can be found in -:ref:`reference/orm/fields/reserved`. +:ref:`reference/fields/reserved`. A few field names are reserved for pre-defined behaviors. They should be defined on a model when the related behavior is desired. diff --git a/content/developer/reference/guidelines.rst b/content/developer/reference/guidelines.rst index 042eb5388..d8eb6e20b 100644 --- a/content/developer/reference/guidelines.rst +++ b/content/developer/reference/guidelines.rst @@ -1,4 +1,3 @@ - .. highlight:: python .. _reference/guidelines: @@ -48,7 +47,6 @@ Other optional directories compose the module. - *report/* : contains the printable reports and models based on SQL views. Python objects and XML views are included in this directory - *tests/* : contains the Python tests - File naming ----------- @@ -87,8 +85,8 @@ defined in ``_security.xml``. Concerning *views*, backend views should be split like models and suffixed by ``_views.xml``. Backend views are list, form, kanban, activity, graph, pivot, .. -views. To ease split by model in views main menus not linked to specific actions -may be extracted into an optional ``_menus.xml`` file. Templates (QWeb +views. In case of large modules specifying a lot of menus, the menus should +be extracted into a separate ``_menus.xml`` file. Templates (QWeb pages used notably for portal / website display) and bundles (import of JS and CSS assets) are put in separate files. Those are respectively ``_templates.xml`` and ``assets.xml`` files. @@ -211,15 +209,25 @@ The complete tree of our Odoo module therefore looks like | |-- lib/ | | |-- external_lib/ | |-- src/ + | | |-- css/ | | |-- js/ | | | |-- widget_a.js | | | |-- widget_b.js + | | | |-- nursery_tour.js (onboarding tour) + | | |-- less/ | | |-- scss/ | | | |-- widget_a.scss | | | |-- widget_b.scss | | |-- xml/ | | | |-- widget_a.xml - | | | |-- widget_a.xml + | | | |-- widget_b.xml + | |-- tests/ + | | |-- nursery_tests.js + | | |-- tours/ + | | | |-- plant_nursery.js (test tour) + |-- tests/ + | |-- __init__.py + | |-- test_nursery.py |-- views/ | |-- assets.xml | |-- plant_nursery_menus.xml @@ -233,7 +241,7 @@ The complete tree of our Odoo module therefore looks like | |--make_plant_order_views.xml .. note:: File names should only contain ``[a-z0-9_]`` (lowercase - alphanumerics and ``_``) + alphanumerics and ``_``) .. warning:: Use correct file permissions : folder 755 and file 644. @@ -242,22 +250,32 @@ The complete tree of our Odoo module therefore looks like XML files ========= +Modules (/Applications) data can be imported in Odoo through XML or CSV :ref:`files `. + +.. note:: + + Data import is faster through CSV files (because records are created in batch). + + If you're creating a lot of records of a given model and the XML features aren't needed, + CSV import should be priveleged. + Format ------ + To declare a record in XML, the **record** notation (using **) is recommended: - Place ``id`` attribute before ``model`` -- For field declaration, ``name`` attribute is first. Then place the - *value* either in the ``field`` tag, either in the ``eval`` - attribute, and finally other attributes (widget, options, ...) - ordered by importance. - -- Try to group the record by model. In case of dependencies between - action/menu/views, this convention may not be applicable. -- Use naming convention defined at the next point +- For field declaration, ``name`` attribute is first. + Then place the *value* either in the ``field`` tag, + either in the ``ref`` attr for :class:`~odoo.fields.Many2one` fields, + either in the ``eval`` attribute, and finally place the other attributes + (widget, options, ...) ordered by importance. +- Try to group the ``record`` declarations by model. In case of dependencies between + action/menu/views, this convention may not be applicable. +- Use naming conventions defined at the next point. - The tag ** is only used to set not-updatable data with ``noupdate=1``. - If there is only not-updatable data in the file, the ``noupdate=1`` can be - set on the ```` tag and do not set a ```` tag. + If there is only not-updatable data in the file, the ``noupdate=1`` can be + set on the ```` tag, the ```` tag is unnecessary. .. code-block:: xml @@ -268,97 +286,132 @@ To declare a record in XML, the **record** notation (using **) is recomm - + -Odoo supports custom tags acting as syntactic sugar: +Odoo supports :ref:`custom tags ` acting as syntactic sugar: -- menuitem: use it as a shortcut to declare a ``ir.ui.menu`` -- template: use it to declare a QWeb View requiring only the ``arch`` section of the view. -- report: use to declare a :ref:`report action ` -- act_window: use it if the record notation can't do what you want +- ``menuitem``: use it as a shortcut to declare a ``ir.ui.menu`` +- ``template``: use it to declare a QWeb View requiring only the ``arch`` section of the view. +- ``report``: use to declare a :ref:`report action ` +- ``act_window``: use it if the ``record`` notation can't do what you want (:ref:`window action `) -The 4 first tags are preferred over the *record* notation. +The 2 first tags are preferred over the *record* notation. +IDs and naming +-------------- -XML IDs and naming ------------------- +.. todo:: actions report naming guidelines ? -Security, View and Action -~~~~~~~~~~~~~~~~~~~~~~~~~ +Use the following patterns: -Use the following pattern : +* Menus ``|ir.ui.menu``: -* For a menu: :samp:`{}_menu`, or :samp:`{}_menu_{do_stuff}` for submenus. -* For a view: :samp:`{}_view_{}`, where *view_type* is - ``kanban``, ``form``, ``tree``, ``search``, ... -* For an action: the main action respects :samp:`{}_action`. - Others are suffixed with :samp:`_{}`, where *detail* is a - lowercase string briefly explaining the action. This is used only if - multiple actions are declared for the model. -* For window actions: suffix the action name by the specific view information - like :samp:`{}_action_view_{}`. -* For a group: :samp:`{}_group_{}` where *group_name* - is the name of the group, generally 'user', 'manager', ... -* For a rule: :samp:`{}_rule_{}` where - *concerned_group* is the short name of the concerned group ('user' - for the 'model_name_group_user', 'public' for public user, 'company' - for multi-company rules, ...). + * :samp:`{}_menu_root` for main application menu (in case of application modules). + * :samp:`{}_menu`, or :samp:`{}_menu_{do_stuff}` for submenus with/without actions. +* Views ``ir.ui.view``: -Name should be identical to xml id with dots replacing underscores. Actions -should have a real naming as it is used as display name. + :samp:`{}_view_{}`, where *view_type* is + ``kanban``, ``form``, ``tree``, ``search``, ... +* Actions ``ir.actions.*``: + + * the main action respects :samp:`{}_action`. + * Secondary actions are suffixed with :samp:`_{}`, where *detail* is a + lowercase string briefly explaining the action. This is used only if + multiple actions are declared for the model. +* Window Actions ``|ir.actions.act_window``: + + Suffix the action name by the specific view information + like :samp:`{}_action_view_{}`. +* Groups ``res.groups``: + + :samp:`{}_group_{}` where *group_name* + is the name of the group, generally 'user', 'manager', ... +* Rules ``ir.rule``: + + :samp:`{}_rule_{}` where + *concerned_group* is the short name of the concerned group ('user' + for the 'model_name_group_user', 'public' for public user, 'company' + for multi-company rules, ...). + +For views, the `name` should be identical to xml id with dots replacing underscores. +For the other technical models, the name of the record should be detailed +to explain their role/use/target ... + +.. note:: + + If an action or a view is targeting specific user groups (e.g. showing some feature only to managers), + an additionnal suffix can be useful to clearly highlight this information. + + E.g. ``product_template_view_form_sale_manager`` .. code-block:: xml - - - model.name.view.form - ... - + + + + model.name.view.form + ... + - - model.name.view.kanban - ... - + + model.name.view.kanban + ... + - - - Model Main Action - ... - + + + Model Main Action + ... + - - Model Access Children - + + Model Access Children + + - - - +.. code-block:: XML - - - ... - + + + - - ... - + - - ... - + + + +.. code-block:: XML + + + + + ... + + + + + + ... + + + + ... + + + Inheriting XML ~~~~~~~~~~~~~~ @@ -390,267 +443,195 @@ based upon the first one. ... +File ordering +------------- + +.. todo:: in manifest: data (security, other data, assets, views, menus.xml) + +.. todo:: guidelines for ir_model_access.csv ? security.xml? assets.xml ? + +Views +~~~~~ + +Ideally, one views file by model: project_views.xml, task_views.xml, product_views, ... + +#. Views + + #. search + #. form + #. kanban + #. tree + #. pivot + #. graph + #. gantt + #. cohort + #. activity + #. map + #. QWeb + +#. Actions +#. Link actions (``ir.actions.act_url``) +#. Report actions (``ir.actions.report``) + +Menus +~~~~~ + +.. todo:: in master update menuitems example to show recursive menuitems. + +For application modules, menuitems should be defined in a separate file (and ordered hierarchically) to have a clear app organisation. +In this case, the ``_menus.xml`` file should be the last specified in the ``__manifest__.py`` ``data`` imports +For generic modules (e.g. bridges), menuitems, if not many in number, can be defined after their respective actions/views. + .. _reference/guidelines/python: Python ====== +Linting +------- + +Using a linter (e.g. Flake8) can help show syntax and semantic warnings or errors. Odoo +source code tries to respect Python PEP8 standard, but some of them can be ignored. + +- E501: line too long + +.. note:: A good max line length standard would be **99** characters for us, + but not mandatory. + +- E301: expected 1 blank line, found 0 +- E302: expected 2 blank lines, found 1 + +Idiomatics of Programming (Python) +---------------------------------- + +- Always favor *readability* over *conciseness* or using the language features or idioms. +- Use meaningful variable/class/method names +- Think about :ref:`*performance* ` + and :ref:`*security* ` all along the development process. +- As a good developer, document your code (docstring on key methods, simple + comments for tricky part of code) +- Know your builtins : You should at least have a basic understanding of + the `Python builtins `_. +- Learn list/dict comprehensions : Use list comprehension, dict comprehension, and + basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code + easier to read. +- Don't hesitate to refresh your `knowledge `_ or + to get more familiar with `Python `_ + +Programming in Odoo +------------------- + +- Avoid to create generators and decorators: only use the ones provided by + the Odoo API. +- As in python, use ``filtered``, ``mapped``, ``sorted``, ... :ref:`ORM ` methods to + ease code reading and performance. +- Don't reinvent the wheel: use or extend existing functionalities when you need them. + Use :ref:`Odoo Mixins ` to integrate interesting functionalities easily. +- Note that empty recordset are falsy + +.. code-block:: python + + def do_something(self): + if not self: + return + ... + .. warning:: Do not forget to read the :ref:`Security Pitfalls ` section as well to write secure code. -PEP8 options ------------- - -Using a linter can help show syntax and semantic warnings or errors. Odoo -source code tries to respect Python standard, but some of them can be ignored. - -- E501: line too long -- E301: expected 1 blank line, found 0 -- E302: expected 2 blank lines, found 1 - -Imports -------- -The imports are ordered as - -#. External libraries (one per line sorted and split in python stdlib) -#. Imports of ``odoo`` -#. Imports from Odoo modules (rarely, and only if necessary) - -Inside these 3 groups, the imported lines are alphabetically sorted. - -.. code-block:: python - - # 1 : imports of python lib - import base64 - import re - import time - from datetime import datetime - # 2 : imports of odoo - import odoo - from odoo import api, fields, models, _ # alphabetically ordered - from odoo.tools.safe_eval import safe_eval as eval - # 3 : imports from odoo addons - from odoo.addons.website.models.website import slug - from odoo.addons.web.controllers.main import login_redirect - -Idiomatics of Programming (Python) ----------------------------------- - -- Always favor *readability* over *conciseness* or using the language features or idioms. -- Don't use ``.clone()`` - -.. code-block:: python - - # bad - new_dict = my_dict.clone() - new_list = old_list.clone() - # good - new_dict = dict(my_dict) - new_list = list(old_list) - -- Python dictionary : creation and update - -.. code-block:: python - - # -- creation empty dict - my_dict = {} - my_dict2 = dict() - - # -- creation with values - # bad - my_dict = {} - my_dict['foo'] = 3 - my_dict['bar'] = 4 - # good - my_dict = {'foo': 3, 'bar': 4} - - # -- update dict - # bad - my_dict['foo'] = 3 - my_dict['bar'] = 4 - my_dict['baz'] = 5 - # good - my_dict.update(foo=3, bar=4, baz=5) - my_dict = dict(my_dict, **my_dict2) - -- Use meaningful variable/class/method names -- Useless variable : Temporary variables can make the code clearer by giving - names to objects, but that doesn't mean you should create temporary variables - all the time: - -.. code-block:: python - - # pointless - schema = kw['schema'] - params = {'schema': schema} - # simpler - params = {'schema': kw['schema']} - -- Multiple return points are OK, when they're simpler - -.. code-block:: python - - # a bit complex and with a redundant temp variable - def axes(self, axis): - axes = [] - if type(axis) == type([]): - axes.extend(axis) - else: - axes.append(axis) - return axes - - # clearer - def axes(self, axis): - if type(axis) == type([]): - return list(axis) # clone the axis - else: - return [axis] # single-element list - -- Know your builtins : You should at least have a basic understanding of all - the Python builtins (http://docs.python.org/library/functions.html) - -.. code-block:: python - - value = my_dict.get('key', None) # very very redundant - value = my_dict.get('key') # good - -Also, ``if 'key' in my_dict`` and ``if my_dict.get('key')`` have very different -meaning, be sure that you're using the right one. - -- Learn list comprehensions : Use list comprehension, dict comprehension, and - basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code - easier to read. - -.. code-block:: python - - # not very good - cube = [] - for i in res: - cube.append((i['id'],i['name'])) - # better - cube = [(i['id'], i['name']) for i in res] - -- Collections are booleans too : In python, many objects have "boolean-ish" value - when evaluated in a boolean context (such as an if). Among these are collections - (lists, dicts, sets, ...) which are "falsy" when empty and "truthy" when containing - items: - -.. code-block:: python - - bool([]) is False - bool([1]) is True - bool([False]) is True - -So, you can write ``if some_collection:`` instead of ``if len(some_collection):``. - - -- Iterate on iterables - -.. code-block:: python - - # creates a temporary list and looks bar - for key in my_dict.keys(): - "do something..." - # better - for key in my_dict: - "do something..." - # accessing the key,value pair - for key, value in my_dict.items(): - "do something..." - -- Use dict.setdefault - -.. code-block:: python - - # longer.. harder to read - values = {} - for element in iterable: - if element not in values: - values[element] = [] - values[element].append(other_value) - - # better.. use dict.setdefault method - values = {} - for element in iterable: - values.setdefault(element, []).append(other_value) - -- As a good developer, document your code (docstring on methods, simple - comments for tricky part of code) -- In additions to these guidelines, you may also find the following link - interesting: http://python.net/~goodger/projects/pycon/2007/idiomatic/handout.html - (a little bit outdated, but quite relevant) - -Programming in Odoo -------------------- - -- Avoid to create generators and decorators: only use the ones provided by - the Odoo API. -- As in python, use ``filtered``, ``mapped``, ``sorted``, ... methods to - ease code reading and performance. - - Make your method work in batch ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -When adding a function, make sure it can process multiple records by iterating -on self to treat each record. + +When adding a function, make sure it can process multiple records: + +* by iterating on self to treat each record. +* by using adapted methods to treat all records together. + +This is the basis for a lot of performance improvements, as it allows batching low level operations +of the ORM, speeding up values/records processing (e.g. CRUD calls triggering database queries). .. code-block:: python - def my_method(self) + def my_method(self): for record in self: record.do_cool_stuff() -For performance issue, when developing a 'stat button' (for instance), do not -perform a ``search`` or a ``search_count`` in a loop. It -is recommended to use ``read_group`` method, to compute all value in only one request. + def _get_total(self): + return sum(self.mapped('total')) -.. code-block:: python + def _confirm(self): + return self.write({'state': 'confirmed'}) - def _compute_equipment_count(self): - """ Count the number of equipment per category """ - equipment_data = self.env['hr.equipment'].read_group([('category_id', 'in', self.ids)], ['category_id'], ['category_id']) - mapped_data = dict([(m['category_id'][0], m['category_id_count']) for m in equipment_data]) - for category in self: - category.equipment_count = mapped_data.get(category.id, 0) +.. note:: + The previous examples also work fine when self is an empty recordset. + Depending on the objective of a method, it's always better to consider self as a potentially empty record, + to ensure the stability of your code. + + The majority of the ORM methods works with 0, 1 or more records:: + + # won't crash even if filtered returns an empty recordset + self.filtered('wrong_state').unlink() + +.. note:: + + If a method can only be called with a unique record, it can be easily enforced + with :meth:`~odoo.models.Model.ensure_one`. + + .. code-block:: python + + def action_open_form(self): + self.ensure_one() # === assert(len(self) == 1) + ... Propagate the context ~~~~~~~~~~~~~~~~~~~~~ + +Odoo operations are done in a given environment, holding the database cursor, the user id +and the context. The context may contain "global" variables, such as the language, the timezone, +the company(ies) in which the user is logged, and any other information specified. + The context is a ``frozendict`` that cannot be modified. To call a method with -a different context, the ``with_context`` method should be used : +a different context, the :meth:`~odoo.models.Model.with_context` method should be used: .. code-block:: python + # Replace the current context --> Potential loss of information + # Do not use unless that's really what you want ! records.with_context(new_context).do_stuff() # all the context is replaced - records.with_context(**additionnal_context).do_other_stuff() # additionnal_context values override native context ones + + # Update the context content --> Safe + # additionnal_context values override native context ones + records.with_context(**additionnal_context).do_other_stuff() .. warning:: - Passing parameter in context can have dangerous side-effects. - Since the values are propagated automatically, some unexpected behavior may appear. - Calling ``create()`` method of a model with *default_my_field* key in context - will set the default value of *my_field* for the concerned model. - But if during this creation, other objects (such as sale.order.line, on sale.order creation) - having a field name *my_field* are created, their default value will be set too. + Passing parameters in context can have dangerous side-effects. + + Since the values are propagated automatically, some unexpected behavior may appear. + Calling :meth:`~odoo.models.Model.create` method of a model with *default_my_field* key in context + will set the default value of *my_field* for the concerned model if not already specified. + But if during this creation, other objects (such as sale.order.line, on sale.order creation) + having a field name *my_field* are created, their default value will be set too. If you need to create a key context influencing the behavior of some object, -choice a good name, and eventually prefix it by the name of the module to +choose a good name, and eventually prefix it by the name of the module to isolate its impact. A good example are the keys of ``mail`` module : *mail_create_nosubscribe*, *mail_notrack*, *mail_notify_user_signature*, ... - Think extendable ~~~~~~~~~~~~~~~~ Functions and methods should not contain too much logic: having a lot of small and simple methods is more advisable than having few large and complex methods. A good rule of thumb is to split a method as soon as it has more than one -responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle). +responsibility. -Hardcoding a business logic in a method should be avoided as it prevents to be -easily extended by a submodule. +.. seealso:: https://en.wikipedia.org/wiki/Single_responsibility_principle + +Hardcoding a business logic in a method should be avoided as it prevents an easy extension by a submodule. .. code-block:: python @@ -682,11 +663,12 @@ Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation. This recommendation is also relevant for classes, files, modules and packages. -(See also http://en.wikipedia.org/wiki/Cyclomatic_complexity) +.. seealso:: https://en.wikipedia.org/wiki/Cyclomatic_complexity Never commit the transaction ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + The Odoo framework is in charge of providing the transactional context for all RPC calls. The principle is that a new database cursor is opened at the beginning of each RPC call, and committed when the call has returned, just @@ -723,8 +705,8 @@ among others: #. inconsistent business data, usually data loss #. workflow desynchronization, documents stuck permanently #. tests that can't be rolled back cleanly, and will start polluting the - database, and triggering error (this is true even if no error occurs - during the transaction) + database, and triggering errors (this is true even if no error occurs + during the transaction) Here is the very simple rule: You should **NEVER** call ``cr.commit()`` yourself, **UNLESS** you have @@ -737,24 +719,331 @@ Here is the very simple rule: And contrary to popular belief, you do not even need to call ``cr.commit()`` in the following situations: -- in the ``_auto_init()`` method of an *models.Model* object: this is taken -care of by the addons initialization method, or by the ORM transaction when -creating custom models + +- in the :meth:`~odoo.models.model._auto_init` method of an *models.Model* object: + this is taken care of by the addons initialization method, + or by the ORM transaction when creating custom models - in reports: the ``commit()`` is handled by the framework too, so you can -update the database even from within a report + update the database even from within a report. - within *models.Transient* methods: these methods are called exactly like -regular *models.Model* ones, within a transaction and with the corresponding -``cr.commit()/rollback()`` at the end + regular *models.Model* ones, within a transaction and with the corresponding + ``cr.commit()/rollback()`` at the end. - etc. (see general rule above if you have in doubt!) -All ``cr.commit()`` calls outside of the server framework from now on must -have an **explicit comment** explaining why they are absolutely necessary, why -they are indeed correct, and why they do not break the transactions. Otherwise -they can and will be removed ! +.. warning:: + All ``cr.commit()`` calls outside of the server framework from now on must + have an **explicit comment** explaining why they are absolutely necessary, why + they are indeed correct, and why they do not break the transactions. Otherwise + they can and will be removed ! -Use translation method correctly -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +.. _reference/guidelines/perf: + +Performance +----------- + +#. Avoid unnecessary operations +#. Batch & factorize operations +#. Use the right tool for the right operation. + Know Python & the ORM behavior/abilities + +Avoid unnecessary operations +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Globally, even outside Odoo, the easiest performance improvement is to avoid operations when possible. +We should always consider duplicated checks, operations in loop that can be moved outside, ... + +For Odoo code, the best way to avoid useless operations is to know the behavior of the main ORM methods ! + +* :meth:`~odoo.models.Model.filtered` returns a new recordset, subset or equal to the current one. + +Note that it goes through the recordset to return a new filtered recordset. +Unless you will do batch operations on the new recordset, in which case the filtered may be useful, +it is unnecessary to use :meth:`~odoo.models.Model.filtered` (and browse the recordset twice). + +.. code-block:: python3 + + # ~2 walkthrough of self + for rec in self.filtered(lambda r: r.state == 'open'): + rec.do_something() + + # one walkthrough: + for rec in self: + if rec.state == 'open': + rec.do_something() + + # If do_something is optimized for batch recordset operations, + # Then the filtered can be useful. + self.filtered(lambda r: r.state == 'open').do_something() + +* :meth:`~odoo.api.Environment.ref()` returns the reference record after verifying it still exists! + +For that verification, ref always calls :meth:`~odoo.models.Model.exists()` which makes one SQL query +(``SELECT id FROM 'model' WHERE id IN ('id')``). + +.. code-block:: python3 + + # len(self) ref queries + for rec in self: + if rec.uom_id == rec.env.ref('uom.uom_day'): + rec.in_day_uom = True + + # one ref query + day_uom = self.env.ref('uom.uom_day') + for rec in self: + if rec.uom_id == day_uom: + rec.in_day_uom = True + +* :meth:`~odoo.models.Model.create()` supports batch records creation. + +Depending on the model specification, creating records in batch can be up to 10 times faster ! +When you need to create multiple records of the same model, please call :meth:`~odoo.models.Model.create()` +with the list of record values (dictionaries). + +... + +.. warning:: When you override existing ORM methods, know and follow their original API. + + * :meth:`~odoo.models.Model.create()` is by default implemented to support batch creation of records. + If your overriden method doesn't support batch records creation (:meth:`~odoo.api.model_create_multi()`), + your model creation may be consequently slower when creating multiple records together. + + Furthermore, note that the values may be empty and the method shouldn't crash (same for :meth:`~odoo.models.Model.write()`)! + + * :meth:`~odoo.models.Model.default_get()` receives a list of requested fields. + There is no need to specify any value if not requested, operations can sometimes be avoided. + + ... + +Database interactions +~~~~~~~~~~~~~~~~~~~~~ + +One of the main performance bottlenecks lies in the database interactions. Fetching +data in database can be quite slow, depending on the queries complexity. + +Database query counts are not strictly a good performance indicator, as using more queries +can be sometimes more efficient, but it's still a good starting point to investigate performance issues. + +SQL analysis can highlight: + +* overly wide search domains (fetching too much data) +* duplicated queries (same searches, :meth:`~odoo.api.Environment.ref()` calls, ...) +* unexpected operations (while investigating the source of unexpected queries) + +You will probably discover a lot of ORM specificities and improve your knowledge of Odoo. + +.. note:: + + The :option:`--log-sql ` option can be used to display the SQL queries + executed. + +Furthermore, the SQL level has the advantage of being more consistent. +Time based tests have proven multiple times to be quite indeterministic, depending on configuration, +other operations, ... +Query count based tests are way more deterministic, and therefore a good testing point to reduce +and/or catch performance reductions due to new SQL queries, highlighting low level changes sometimes +forgotten during the development process. + +.. note:: + + The :meth:`~odoo.tests.common.BaseCase.assertQueryCount()` method can be used to enforce + a maximum number of SQL queries done in a given context:: + + with self.assertQueryCount(4): + self.env[model].do_something() + +Reduce/Simplify queries as much as possible: + +* Batch database requests (:meth:`~odoo.models.Model.search`, ...): + + * Use the right domain + * Use the right ORM methods (:meth:`~odoo.models.Model.search_count`, :meth:`~odoo.models.Model.read_group`, ...) + +.. code-block:: python3 + + # very bad: up to len(self) queries + for record in self: + if record.env['model'].search([('id', '=', record.id)]): + return True + return False + + # better: one query + if self.env['model'].search([('id', 'in', self.ids)]): + return True + return False + + # even better: read and browse only one id + # NOTE: returning first id found is faster than using a search_count + if self.env['model'].search([('id', 'in', self.ids)], limit=1): + return True + return False + +* Only load what you really need: + + * a `limit` can be given for :meth:`~odoo.models.Model.search` calls + * Read specific fields when you don't need all fields values (:meth:`~odoo.models.Model.read`, :meth:`~odoo.models.Model.read_group`) + * Disable fields prefetching for targeted operations :ref:`reference/guidelines/perf/prefetch`. + +* Use ORM-cached methods to avoid "useless" SQL queries. + + .. code-block:: python3 + + # One query by ref call ! + # Do not use, unless you want to manage + # the case where the record doesn't exist anymore. + record = self.env.ref(xml_id) + record_id = record.id + + # Better, use the cache + model, id = self.env['ir.model.data'].xmlid_to_res_model_res_id(xml_id) + record = self.env[model].browse(id) + record_id = record.id + + # if you only need the id: + record_id = self.env['ir.model.data'].xmlid_to_res_id(xml_id) + + .. code-block:: python3 + + model = self.env['ir.model'].search([('name', '=', name)], limit=1) + model_id = model.id + + # better + model = self.env['ir.model']._get(name) + + # better (id) + model_id = self.env['ir.model']._get_id(name) + +SQL processing +'''''''''''''' + +The ORM provides multiple tools to batch/execute operations at a lower level. +Use those tools to delegate processing to the SQL level as much as possible. + +.. code-block:: python3 + + # bad: loading & browsing all found ids for nothing + for record in self: + record.model_count = len(self.env['model'].search([('rec_id', '=', record.id)])) + + # better, but still wrong : len(self) queries + for record in self: + record.model_count = self.env['model'].search_count([('rec_id', '=', record.id)]) + + # best + data = self.env['model'].read_group([('rec_id', 'in', self.ids)], ['rec_id'], ['rec_id']) + mapped_data = dict([(d['rec_id'][0], d['rec_id_count']) for d in data]) + for record in self: + record.model_count = mapped_data.get(record.id, 0) + +Prefer SQL constraints to Python constraints: + +#. SQL constraints are strictly enforced, whereas python constraints can be bypassed + by SQL queries and old module data (in case of updates) could be wrong. +#. When creating records, SQL constraints are evaluated earlier, at INSERT, whereas + the verification of Python constraints is done later, after the SQL INSERT's. + +SQL constraints are more efficient, and will raise & rollback earlier in the creation process. +Python constraints should be used for more detailed/targeted explanation, and/or when the +constraint cannot be applied at the SQL level. + +.. code-block:: python3 + + # less efficient + @api.constrains('begin_date', 'end_date') + def _check_period(self): + for record in self: + if record.begin_date > end_date: + raise UserError(_("Beginning date must be earlier than ending date.")) + + # good: earlier and stricter check: + _sql_constraints = [ + ('valid_period', + "CHECK(begin_date < end_date)", + "Beginning date must be earlier than ending date"), + ] + +Use the content in cache +'''''''''''''''''''''''' + +When the information is already available, do not request it again from database + +.. code-block:: python3 + + # probably bad, you have the records already in cache, no need to go through the database + def action_validate(self): + self.env.search([('validated', '=', False), ('id', 'in', self.ids)]).validated = True + + # probably better: use the cached values to filter the current recordset + def action_validate(self): + self.filtered_domain([('validated', '=', False)]).validated = True + +.. _reference/guidelines/perf/prefetch: + +Prefetch +~~~~~~~~ + +By default, the ORM prefetches all the records fields when reading any field on a recordset. +It considers that when we're working on a record, the value of multiple fields will probably be needed. + +If you're developing a costly operation of some sort (e.g. working on big recordsets), or even targeted operations +(for which you do not need the values of all the fields, especially on big models), you may want to disable/avoid the prefetch. + +Let's consider a basic case where you only need one or more fields on a recordset. + +.. code-block:: python3 + + for record in self: + if record.fieldA: + record.do_something() + else: + record.do_something_else() + +In the previous code, when accessing ``fieldA`` on the first operation, the ORM will prefetch all stored model fields +for the records in self. If ``self`` is huge and/or the model has a lot of fields, the database query can be quite slow. +There are 2 main ways to avoid prefetching all the fields in this case: + +* Disabling the prefetch on the recordset + + The context key `prefetch_fields`, if set to ``False``, can disable the *fields* prefetch on the ORM level. + +.. code-block:: python3 + + self = self.with_context(prefetch_fields=False) + for record in self: + if record.fieldA: + record.do_something() + else: + record.do_something_else() + +Only ``fieldA`` will be prefetched on ``self``. + +.. warning:: The context is propagated to subsequent calls ! + + The execution context in *do_something*/*do_something_else* will also have the disabled prefetch. + Do not use the ``prefetch_fields`` context key without clearly knowing the subsequent scopes. + Disabling the prefetch can greatly hinder performance if not done wisely... + + If possible, prefer the use of :meth:`~odoo.models.Model.read()` to "disable" prefetch cleanly. + +* Manually prefetching the needed field(s) + + To manually prefetch some field(s), the :meth:`~odoo.models.Model.read()` method has an useful side-effect. + It returns the requested data, but it also fills the cache with the requested values. + If the data for the requested field is already in cache, the ORM won't prefetch the remaining fields. + +.. code-block:: python3 + + self.read(['fieldA']) + for record in self: + if record.fieldA: + record.do_something() + else: + record.do_something_else() + +As ``fieldA`` is already in cache, the ORM won't prefetch the remaining fields. + +Translations +------------ Odoo uses a GetText-like method named "underscore" ``_( )`` to indicate that a static string used in the code needs to be translated at runtime using the @@ -773,7 +1062,6 @@ in the code, it will not work to translate field values, such as Product names, etc. This must be done instead using the translate flag on the corresponding field. -The method accepts optional positional or named parameter The rule is very simple: calls to the underscore method should always be in the form ``_('literal string')`` and nothing else: @@ -783,23 +1071,19 @@ the form ``_('literal string')`` and nothing else: error = _('This record is locked!') # good: strings with formatting patterns included - error = _('Record %s cannot be modified!', record) + error = _('Record %s cannot be modified!') % record # ok too: multi-line literal strings error = _("""This is a bad multiline example - about record %s!""", record) + about record %s!""") % record error = _('Record %s cannot be modified' \ - 'after being validated!', record) + 'after being validated!') % record # bad: tries to translate after string formatting # (pay attention to brackets!) # This does NOT work and messes up the translations! error = _('Record %s cannot be modified!' % record) - # bad: formatting outside of translation - # This won't benefit from fallback mechanism in case of bad translation - error = _('Record %s cannot be modified!') % record - # bad: dynamic string, string concatenation, etc are forbidden! # This does NOT work and messes up the translations! error = _("'" + que_rec['question'] + "' \n") @@ -810,31 +1094,32 @@ the form ``_('literal string')`` and nothing else: # and the following will of course not work as already explained: error = _("Product %s is out of stock!" % product.name) + # bad: field values are automatically translated by the framework + # This is useless and will not work the way you think: + error = _("Product %s is not available!") % _(product.name) + # and the following will of course not work as already explained: + error = _("Product %s is not available!" % product.name) + # Instead you can do the following and everything will be translated, # including the product name if its field definition has the # translate flag properly set: - error = _("Product %s is not available!", product.name) + error = _("Product %s is not available!") % product.name Also, keep in mind that translators will have to work with the literal values that are passed to the underscore function, so please try to make them easy to understand and keep spurious characters and formatting to a minimum. Translators -must be aware that formatting patterns such as ``%s`` or ``%d``, newlines, etc. -need to be preserved, but it's important to use these in a sensible and obvious -manner: +must be aware that formatting patterns such as %s or %d, newlines, etc. need +to be preserved, but it's important to use these in a sensible and obvious manner: .. code-block:: python # Bad: makes the translations hard to work with error = "'" + question + _("' \nPlease enter an integer value ") - # Ok (pay attention to position of the brackets too!) + # Better (pay attention to position of the brackets too!) error = _("Answer to question %s is not valid.\n" \ - "Please enter an integer value.", question) - - # Better - error = _("Answer to question %(title)s is not valid.\n" \ - "Please enter an integer value.", title=question) + "Please enter an integer value.") % question In general in Odoo, when manipulating strings, prefer ``%`` over ``.format()`` (when only one variable to replace in a string), and prefer ``%(varname)`` instead @@ -842,61 +1127,135 @@ of position (when multiple variables have to be replaced). This makes the translation easier for the community translators. -Symbols and Conventions +Conventions and Symbols ----------------------- -- Model name (using the dot notation, prefix by the module name) : - - When defining an Odoo Model : use singular form of the name (*res.partner* - and *sale.order* instead of *res.partnerS* and *saleS.orderS*) - - When defining an Odoo Transient (wizard) : use ``.`` - where *related_base_model* is the base model (defined in *models/*) related - to the transient, and *action* is the short name of what the transient do. Avoid the *wizard* word. - For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ... - - When defining *report* model (SQL views e.i.) : use - ``.report.``, based on the Transient convention. +Naming +~~~~~~ + +Model & Class +''''''''''''' + +- Model name (using the dot notation, prefixed by the module name): + + - When defining an Odoo :class:`~odoo.models.Model`: use singular form of the name (*res.partner* + and *sale.order* instead of *res.partnerS* and *saleS.orderS*) + - When defining an Odoo :class:`~odoo.models.TransientModel` (wizard): use ``.`` + where *related_base_model* is the base model (defined in *models/*) related + to the transient, and *action* is the short name of what the transient do. Avoid the *wizard* word. + For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ... + - When defining *report* model (SQL views e.i.): use + ``.report.``, based on the Transient convention. - Odoo Python Class : use camelcase (Object-oriented style). - .. code-block:: python class AccountInvoice(models.Model): + _name = "account.invoice" + ... +Fields +'''''' + +- :class:`~odoo.fields.One2many` and :class:`~odoo.fields.Many2many` fields should always have *_ids* as suffix (example: sale_order_line_ids) +- :class:`~odoo.fields.Many2one` fields should have *_id* as suffix (example: partner_id, user_id, ...) + +.. note:: + + Some field names have specific meaning in Odoo, know their meaning/use before using/overriding those. + + * The :ref:`automatic fields (id, create_date, write_date, ...)` are automatically created + on a given model unless it is specified as `_auto = False`. + * Some :ref:`reserved field names (state, parent_id, ...)` provides specific abilities/behavior. + +Methods +''''''' + +- Method conventions & patterns: + + - Compute method: ``_compute_`` + - Search method: ``_search_`` + - Default method: ``_default_`` + - Selection method: ``_selection_`` + - Onchange method: ``_onchange_`` + - Constraint method: ``_check_`` + - Action method: an object action method is prefixed with ``action_``. + If it can only be called on one record, add ``self.ensure_one()`` + at the beginning of the method. + +Variables +''''''''' + - Variable name : - - use camelcase for model variable - - use underscore lowercase notation for common variable. - - suffix your variable name with *_id* or *_ids* if it contains a record id or list of id. Don't use ``partner_id`` to contain a record of res.partner + + - Use camelcase for model variable (empty model recordsets). + - Use underscore lowercase notation for common variable. + - Suffix your variable name with *_id* (*_ids*) if it contains a record id (list of ids). + Don't use ``partner_id``(``partner_ids``) for a res.partner recordset. .. code-block:: python Partner = self.env['res.partner'] + partner = partner.browse(id) partners = Partner.browse(ids) partner_id = partners[0].id + partner_ids = partners.ids -- ``One2Many`` and ``Many2Many`` fields should always have *_ids* as suffix (example: sale_order_line_ids) -- ``Many2One`` fields should have *_id* as suffix (example : partner_id, user_id, ...) -- Method conventions - - Compute Field : the compute method pattern is *_compute_* - - Search method : the search method pattern is *_search_* - - Default method : the default method pattern is *_default_* - - Selection method: the selection method pattern is *_selection_* - - Onchange method : the onchange method pattern is *_onchange_* - - Constraint method : the constraint method pattern is *_check_* - - Action method : an object action method is prefix with *action_*. - Since it uses only one record, add ``self.ensure_one()`` - at the beginning of the method. +File organization +~~~~~~~~~~~~~~~~~ -- In a Model attribute order should be - #. Private attributes (``_name``, ``_description``, ``_inherit``, ...) - #. Default method and ``_default_get`` - #. Field declarations - #. Compute, inverse and search methods in the same order as field declaration - #. Selection method (methods used to return computed values for selection fields) - #. Constrains methods (``@api.constrains``) and onchange methods (``@api.onchange``) - #. CRUD methods (ORM overrides) - #. Action methods - #. And finally, other business methods. +Imports +''''''' + +The imports are ordered as: + +#. External libraries (one per line sorted and split in python stdlib) +#. Imports of ``odoo`` +#. Imports from Odoo modules (rarely, and only if necessary) + +Inside these 3 groups, the imported lines are alphabetically sorted. + +.. code-block:: python + + # 1: imports of python lib + import base64 + import re + import time + from datetime import datetime + + # 2: imports of odoo + from odoo import api, fields, models, _, _lt # alphabetically ordered + from odoo.tools.safe_eval import safe_eval as eval + + # 3: imports from odoo addons + from odoo.addons.website.models.website import slug + from odoo.addons.web.controllers.main import login_redirect + +Model attributes +'''''''''''''''' + +In a :class:`~odoo.models.Model`, the attribute order should be: + +#. Private attributes (``_name``, ``_description``, ``_inherit``, ...) +#. Default methods and :meth:`~odoo.models.Model.default_get` +#. :class:`Fields <~odoo.fields.Field>` declarations: + + * Main fields first (e.g. required fields) + * Computed/Related fields should be defined after their dependencies. +#. SQL constraints, defined through the ``_sql_constraints`` attribute. +#. :ref:`Compute`, inverse and search methods in the same order as field declaration +#. Selection method (methods used to return computed values for selection fields) +#. Constrains methods (:meth:`@api.constrains`) and onchange methods (:meth:`@api.onchange`) +#. CRUD methods (ORM overrides: :meth:`~odoo.models.Model.create`, :meth:`~odoo.models.Model.unlink`, :meth:`~odoo.models.Model.write`, ...) +#. Action methods +#. And finally, other business methods. + +.. todo:: field attributes order ??? + +Generic structure +~~~~~~~~~~~~~~~~~ .. code-block:: python @@ -909,20 +1268,40 @@ Symbols and Conventions def _default_name(self): ... + @api.model + def default_get(self, fields_list): + ... + # Fields declaration name = fields.Char(string='Name', default=_default_name) - seats_reserved = fields.Integer(oldname='register_current', string='Reserved Seats', - store=True, readonly=True, compute='_compute_seats') - seats_available = fields.Integer(oldname='register_avail', string='Available Seats', - store=True, readonly=True, compute='_compute_seats') price = fields.Integer(string='Price') event_type = fields.Selection(string="Type", selection='_selection_type') + seats_max = fields.Integer(string='Maximum Attendees Number') + registration_ids = fields.One2many( + 'event.registration', 'event_id', string='Attendees') + date_begin = fields.Datetime(required=True) + date_end = fields.Datetime() + + seats_reserved = fields.Integer( + string='Reserved Seats', + store=True, compute='_compute_seats') + seats_available = fields.Integer( + string='Available Seats', + store=True, compute='_compute_seats') + + # SQL constraints + _sql_constraints = [ + ('valid_period', + "CHECK(date_begin IS NULL OR date_end IS NULL OR date_begin < date_end)", + "Beginning date must be before ending date") + ] # compute and search fields, in the same order of fields declaration @api.depends('seats_max', 'registration_ids.state', 'registration_ids.nb_register') def _compute_seats(self): ... + # Selection methods @api.model def _selection_type(self): return [] @@ -937,7 +1316,8 @@ Symbols and Conventions ... # CRUD methods (and name_get, name_search, ...) overrides - def create(self, values): + @api.model_create_multi + def create(self, vals_list): ... # Action methods @@ -969,21 +1349,21 @@ The convention is to organize the code according to the following structure: - *static*: all static files in general - - *static/lib*: this is the place where js libs should be located, in a sub folder. + - *static/lib*: this is the place where js libs should be located, in a sub folder. So, for example, all files from the *jquery* library are in *addons/web/static/lib/jquery* - - *static/src*: the generic static source code folder + - *static/src*: the generic static source code folder - *static/src/css*: all css files - *static/src/fonts* - *static/src/img* - *static/src/js* - - *static/src/js/tours*: end user tour files (tutorials, not tests) - + - *static/src/js/tours*: end user tour files (tutorials, not tests) + - *static/src/scss*: scss files - *static/src/xml*: all qweb templates that will be rendered in JS - - *static/tests*: this is where we put all test related files. + - *static/tests*: this is where we put all test related files. - *static/tests/tours*: this is where we put all tour test files (not tutorials). @@ -1003,10 +1383,10 @@ CSS coding guidelines --------------------- - Prefix all your classes with *o_* where *module_name* is the - technical name of the module ('sale', 'im_chat', ...) or the main route - reserved by the module (for website module mainly, i.e. : 'o_forum' for - *website_forum* module). The only exception for this rule is the - webclient: it simply uses *o_* prefix. + technical name of the module ('sale', 'im_chat', ...) or the main route + reserved by the module (for website module mainly, i.e. : 'o_forum' for + *website_forum* module). The only exception for this rule is the + webclient: it simply uses *o_* prefix. - Avoid using *id* tag - Use Bootstrap native classes - Use underscore lowercase notation to name class @@ -1024,12 +1404,12 @@ way towards making your commits more helpful: - Be sure to define both the user.email and user.name in your local git config - .. code-block:: text + .. code-block:: text - git config --global + git config --global - Be sure to add your full name to your Github profile here. Please feel fancy - and add your team, avatar, your favorite quote, and whatnot ;-) + and add your team, avatar, your favorite quote, and whatnot ;-) Commit message structure ------------------------ @@ -1039,22 +1419,22 @@ description. Try to follow the preferred structure for your commit messages .. code-block:: text - [TAG] module: describe your change in a short sentence (ideally < 50 chars) + [TAG] module: describe your change in a short sentence (ideally < 50 chars) - Long version of the change description, including the rationale for the change, - or a summary of the feature being introduced. + Long version of the change description, including the rationale for the change, + or a summary of the feature being introduced. - Please spend a lot more time describing WHY the change is being done rather - than WHAT is being changed. This is usually easy to grasp by actually reading - the diff. WHAT should be explained only if there are technical choices - or decision involved. In that case explain WHY this decision was taken. + Please spend a lot more time describing WHY the change is being done rather + than WHAT is being changed. This is usually easy to grasp by actually reading + the diff. WHAT should be explained only if there are technical choices + or decision involved. In that case explain WHY this decision was taken. - End the message with references, such as task or bug numbers, PR numbers, and - OPW tickets, following the suggested format: - task-123 (related to task) - Fixes #123 (close related issue on Github) - Closes #123 (close related PR on Github) - opw-123 (related to ticket) + End the message with references, such as task or bug numbers, PR numbers, and + OPW tickets, following the suggested format: + task-123 (related to task) + Fixes #123 (close related issue on Github) + Closes #123 (close related PR on Github) + opw-123 (related to ticket) Tag and module name ------------------- @@ -1062,21 +1442,21 @@ Tag and module name Tags are used to prefix your commit. They should be one of the following - **[FIX]** for bug fixes: mostly used in stable version but also valid if you - are fixing a recent bug in development version; + are fixing a recent bug in development version; - **[REF]** for refactoring: when a feature is heavily rewritten; - **[ADD]** for adding new modules; - **[REM]** for removing resources: removing dead code, removing views, - removing modules, ...; + removing modules, ...; - **[REV]** for reverting commits: if a commit causes issues or is not wanted - reverting it is done using this tag; + reverting it is done using this tag; - **[MOV]** for moving files: use git move and do not change content of moved file - otherwise Git may loose track and history of the file; also used when moving - code from one file to another; + otherwise Git may loose track and history of the file; also used when moving + code from one file to another; - **[REL]** for release commits: new major or minor stable versions; - **[IMP]** for improvements: most of the changes done in development version - are incremental improvements not related to another tag; + are incremental improvements not related to another tag; - **[MERGE]** for merge commits: used in forward port of bug fixes but also as - main commit for feature involving several separated commits; + main commit for feature involving several separated commits; - **[CLA]** for signing the Odoo Individual Contributor License; - **[I18N]** for changes in translation files; @@ -1133,24 +1513,24 @@ Finally here are some examples of correct commit messages : .. code-block:: text - [REF] models: use `parent_path` to implement parent_store + [REF] models: use `parent_path` to implement parent_store - This replaces the former modified preorder tree traversal (MPTT) with the - fields `parent_left`/`parent_right`[...] + This replaces the former modified preorder tree traversal (MPTT) with the + fields `parent_left`/`parent_right`[...] - [FIX] account: remove frenglish + [FIX] account: remove frenglish - [...] + [...] - Closes #22793 - Fixes #22769 + Closes #22793 + Fixes #22769 - [FIX] website: remove unused alert div, fixes look of input-group-btn + [FIX] website: remove unused alert div, fixes look of input-group-btn - Bootstrap's CSS depends on the input-group-btn - element being the first/last child of its parent. - This was not the case because of the invisible - and useless alert. + Bootstrap's CSS depends on the input-group-btn + element being the first/last child of its parent. + This was not the case because of the invisible + and useless alert. .. note:: Use the long description to explain the *why* not the - *what*, the *what* can be seen in the diff + *what*, the *what* can be seen in the diff diff --git a/content/developer/reference/orm.rst b/content/developer/reference/orm.rst index 9d9061f98..27a005790 100644 --- a/content/developer/reference/orm.rst +++ b/content/developer/reference/orm.rst @@ -431,7 +431,7 @@ as :attr:`~odoo.models.BaseModel._auto` .. warning:: :attr:`~odoo.models.BaseModel._log_access` *must* be enabled on :class:`~odoo.models.TransientModel`. -.. _reference/orm/fields/reserved: +.. _reference/fields/reserved: Reserved Field names --------------------