From c25b68f324f8d3604b33380504c3428585eb7efa Mon Sep 17 00:00:00 2001
From: Adrian Torres <adt@odoo.com>
Date: Fri, 1 Jun 2018 10:55:35 +0200
Subject: [PATCH] [FIX] orm: re-create constraints of extended fields

Before this commit:

* Module A defines a field X of model M
* Module B inherits from model M without touching field X
* Module C inherits from model M and extends field X by giving an INDEX
/ NOT NULL constraint.
* Module B and C depend from Module A, but not each other

If all three modules are installed and Module B is updated, the INDEX /
NOT NULL constraint could be dropped.

This happens because Module B can be loaded before Module C is loaded,
if that's the case, then after the upgrade of Module B, during the
schema checking, we verify that the field object we have and the field
on the DB are the same, since Module B doesn't introduce the index then
this check is false and we drop the index. When we get to loading Module
C, we do not do any schema checking because the module is not marked as
`to upgrade`, therefore the index is lost forever.

To solve this, we re-init the models that belong to the set of the intersection
between upgraded and modified models and loaded and modified models.

Fixes #24958
---
 odoo/modules/loading.py  | 53 ++++++++++++++++++++++++++++++++++------
 odoo/modules/registry.py |  2 ++
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/odoo/modules/loading.py b/odoo/modules/loading.py
index 950704e45e9e..750c33880642 100644
--- a/odoo/modules/loading.py
+++ b/odoo/modules/loading.py
@@ -26,7 +26,8 @@ _logger = logging.getLogger(__name__)
 _test_logger = logging.getLogger('odoo.tests')
 
 
-def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=None, report=None):
+def load_module_graph(cr, graph, status=None, perform_checks=True,
+                      skip_modules=None, report=None, models_to_check=None):
     """Migrates+Updates or Installs all module nodes from ``graph``
        :param graph: graph of module nodes to load
        :param status: deprecated parameter, unused, left to avoid changing signature in 8.0
@@ -97,6 +98,9 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
             if kind in ('demo', 'test'):
                 threading.currentThread().testing = False
 
+    if models_to_check is None:
+        models_to_check = set()
+
     processed_modules = []
     loaded_modules = []
     registry = odoo.registry(cr.dbname)
@@ -110,6 +114,8 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
     t0 = time.time()
     t0_sql = odoo.sql_db.sql_counter
 
+    models_updated = set()
+
     for index, package in enumerate(graph, 1):
         module_name = package.name
         module_id = package.id
@@ -131,9 +137,20 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
         model_names = registry.load(cr, package)
 
         loaded_modules.append(package.name)
-        if hasattr(package, 'init') or hasattr(package, 'update') or package.state in ('to install', 'to upgrade'):
+        if (hasattr(package, 'init') or hasattr(package, 'update')
+                or package.state in ('to install', 'to upgrade')):
+            models_updated |= set(model_names)
+            models_to_check -= set(model_names)
             registry.setup_models(cr, partial=True)
             registry.init_models(cr, model_names, {'module': package.name})
+            cr.commit()
+        elif package.state != 'to remove':
+            # The current module has simply been loaded. The models extended by this module
+            # and for which we updated the schema, must have their schema checked again.
+            # This is because the extension may have changed the model,
+            # e.g. adding required=True to an existing field, but the schema has not been
+            # updated by this module because it's not marked as 'to upgrade/to install'.
+            models_to_check |= set(model_names) & models_updated
 
         idref = {}
 
@@ -223,9 +240,14 @@ def _check_module_names(cr, module_names):
             incorrect_names = mod_names.difference([x['name'] for x in cr.dictfetchall()])
             _logger.warning('invalid module names, ignored: %s', ", ".join(incorrect_names))
 
-def load_marked_modules(cr, graph, states, force, progressdict, report, loaded_modules, perform_checks):
+def load_marked_modules(cr, graph, states, force, progressdict, report,
+                        loaded_modules, perform_checks, models_to_check=None):
     """Loads modules marked with ``states``, adding them to ``graph`` and
        ``loaded_modules`` and returns a list of installed/upgraded modules."""
+
+    if models_to_check is None:
+        models_to_check = set()
+
     processed_modules = []
     while True:
         cr.execute("SELECT name from ir_module_module WHERE state IN %s" ,(tuple(states),))
@@ -234,7 +256,10 @@ def load_marked_modules(cr, graph, states, force, progressdict, report, loaded_m
             break
         graph.add_modules(cr, module_list, force)
         _logger.debug('Updating graph with %d more modules', len(module_list))
-        loaded, processed = load_module_graph(cr, graph, progressdict, report=report, skip_modules=loaded_modules, perform_checks=perform_checks)
+        loaded, processed = load_module_graph(
+            cr, graph, progressdict, report=report, skip_modules=loaded_modules,
+            perform_checks=perform_checks, models_to_check=models_to_check
+        )
         processed_modules.extend(processed)
         loaded_modules.extend(loaded)
         if not processed:
@@ -248,6 +273,8 @@ def load_modules(db, force_demo=False, status=None, update_module=False):
     if force_demo:
         force.append('demo')
 
+    models_to_check = set()
+
     cr = db.cursor()
     try:
         if not odoo.modules.db.is_initialized(cr):
@@ -277,7 +304,9 @@ def load_modules(db, force_demo=False, status=None, update_module=False):
         # processed_modules: for cleanup step after install
         # loaded_modules: to avoid double loading
         report = registry._assertion_report
-        loaded_modules, processed_modules = load_module_graph(cr, graph, status, perform_checks=update_module, report=report)
+        loaded_modules, processed_modules = load_module_graph(
+            cr, graph, status, perform_checks=update_module,
+            report=report, models_to_check=models_to_check)
 
         load_lang = tools.config.pop('load_language')
         if load_lang or update_module:
@@ -332,11 +361,11 @@ def load_modules(db, force_demo=False, status=None, update_module=False):
             previously_processed = len(processed_modules)
             processed_modules += load_marked_modules(cr, graph,
                 ['installed', 'to upgrade', 'to remove'],
-                force, status, report, loaded_modules, update_module)
+                force, status, report, loaded_modules, update_module, models_to_check)
             if update_module:
                 processed_modules += load_marked_modules(cr, graph,
                     ['to install'], force, status, report,
-                    loaded_modules, update_module)
+                    loaded_modules, update_module, models_to_check)
 
         registry.setup_models(cr)
 
@@ -398,6 +427,16 @@ def load_modules(db, force_demo=False, status=None, update_module=False):
                 api.Environment.reset()
                 return odoo.modules.registry.Registry.new(cr.dbname, force_demo, status, update_module)
 
+        # STEP 5.5: Verify extended fields on every model
+        # This will fix the schema of all models in a situation such as:
+        #   - module A is loaded and defines model M;
+        #   - module B is installed/upgraded and extends model M;
+        #   - module C is loaded and extends model M;
+        #   - module B and C depend on A but not on each other;
+        # The changes introduced by module C are not taken into account by the upgrade of B.
+        if models_to_check:
+            registry.init_models(cr, list(models_to_check), {'models_to_check': True})
+
         # STEP 6: verify custom views on every model
         if update_module:
             View = env['ir.ui.view']
diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py
index 1d05ec466d59..9d06e34a49c8 100644
--- a/odoo/modules/registry.py
+++ b/odoo/modules/registry.py
@@ -308,6 +308,8 @@ class Registry(Mapping):
         """
         if 'module' in context:
             _logger.info('module %s: creating or updating database tables', context['module'])
+        elif context.get('models_to_check', False):
+            _logger.info("verifying fields for every extended model")
 
         context = dict(context, todo=[])
         env = odoo.api.Environment(cr, SUPERUSER_ID, context)
-- 
GitLab