From 1519fcbd4d3493190c48f747e76fd3e9cf97b70f Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Mon, 28 Aug 2017 12:33:43 +0200 Subject: [PATCH 1/2] [FIX][module_auto_update] Record base addon checksum (#948) --- module_auto_update/models/module.py | 39 ++++++-------------- module_auto_update/tests/test_module.py | 17 ++++----- module_auto_update/wizards/module_upgrade.py | 14 ++++++- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/module_auto_update/models/module.py b/module_auto_update/models/module.py index 273d72588..398809e61 100644 --- a/module_auto_update/models/module.py +++ b/module_auto_update/models/module.py @@ -41,36 +41,21 @@ class Module(models.Model): "Cannot compute dir hash for %s, module not found", r.display_name) + @api.multi def _store_checksum_installed(self, vals): - if self.env.context.get('retain_checksum_installed'): - return + """Store the right installed checksum, if addon is installed.""" if 'checksum_installed' not in vals: - if vals.get('state') == 'installed': - for r in self: - r.checksum_installed = r.checksum_dir - elif vals.get('state') == 'uninstalled': + try: + version = vals["latest_version"] + except KeyError: + return # Not [un]installing/updating any addon + if version is False: + # Uninstalling self.write({'checksum_installed': False}) - - @api.multi - def button_uninstall(self): - return super( - Module, - self.with_context(module_uninstall=True), - ).button_uninstall() - - @api.multi - def button_uninstall_cancel(self): - return super( - Module, - self.with_context(retain_checksum_installed=True), - ).button_uninstall_cancel() - - @api.multi - def button_upgrade_cancel(self): - return super( - Module, - self.with_context(retain_checksum_installed=True), - ).button_upgrade_cancel() + else: + # Installing or updating + for one in self: + one.checksum_installed = one.checksum_dir @api.model def create(self, vals): diff --git a/module_auto_update/tests/test_module.py b/module_auto_update/tests/test_module.py index bd25ed07a..f8fcf7278 100644 --- a/module_auto_update/tests/test_module.py +++ b/module_auto_update/tests/test_module.py @@ -10,6 +10,7 @@ import mock from odoo.modules import get_module_path from odoo.tests.common import TransactionCase +from odoo.tools import mute_logger from .. import post_init_hook @@ -81,24 +82,19 @@ class TestModule(TransactionCase): def test_store_checksum_installed_state_installed(self): """It should set the module's checksum_installed equal to - checksum_dir when vals contain state 'installed'""" + checksum_dir when vals contain a ``latest_version`` str.""" self.own_module.checksum_installed = 'test' - self.own_module._store_checksum_installed({'state': 'installed'}) + self.own_module._store_checksum_installed({'latest_version': '1.0'}) self.assertEqual( self.own_module.checksum_installed, self.own_module.checksum_dir, - 'Setting state to installed does not store checksum_dir ' - 'as checksum_installed', ) def test_store_checksum_installed_state_uninstalled(self): """It should clear the module's checksum_installed when vals - contain state 'uninstalled'""" + contain ``"latest_version": False``""" self.own_module.checksum_installed = 'test' - self.own_module._store_checksum_installed({'state': 'uninstalled'}) - self.assertEqual( - self.own_module.checksum_installed, False, - 'Setting state to uninstalled does not clear checksum_installed', - ) + self.own_module._store_checksum_installed({'latest_version': False}) + self.assertIs(self.own_module.checksum_installed, False) def test_store_checksum_installed_vals_contain_checksum_installed(self): """It should not set checksum_installed to False or checksum_dir when @@ -198,6 +194,7 @@ class TestModule(TransactionCase): '_store_checksum_installed', ) + @mute_logger("openerp.modules.module") @mock.patch('%s.get_module_path' % model) def test_get_module_list(self, module_path_mock): """It should change the state of modules with different diff --git a/module_auto_update/wizards/module_upgrade.py b/module_auto_update/wizards/module_upgrade.py index 188030dd8..eaf278651 100644 --- a/module_auto_update/wizards/module_upgrade.py +++ b/module_auto_update/wizards/module_upgrade.py @@ -31,4 +31,16 @@ class ModuleUpgrade(models.TransientModel): # Compute updates by checksum when called in @api.model fashion if not self: self.get_module_list() - return super(ModuleUpgrade, self).upgrade_module() + # Get base adddon status before updating + base = self.env["ir.module.module"].search([("name", "=", "base")]) + pre_state = base.state + result = super(ModuleUpgrade, self).upgrade_module() + # Update base addon checksum if its state changed + base.invalidate_cache() + if base.state != pre_state: + # This triggers the write hook that should have been triggered + # when the module was [un]installed/updated in the base-only + # module graph inside above call to super(), and updates its + # dir checksum as needed + base.latest_version = base.latest_version + return result From e8bc937d7f0fa57b92afeb94c92eb8a56c424aec Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Tue, 29 Aug 2017 10:55:36 +0200 Subject: [PATCH 2/2] [FIX][module_auto_update] Always store changes in lower graphs The same problem that was fixed for the `base` addon in #948 happened with random addons that do not depend on `module_auto_update` (a.k.a. any addon) that Odoo decided to load before that one in the graph. Now we always check for all addons if their state has changed, and make sure to trigger the udpate mechanism that stores the right value in `installed_checksum_dir` field. If you installed and uninstalled the addon right away, you'd get a ProgrammingError saying that some columns exist no more. Checks are done now using `search_read`, which lets us limit the fields being fetched, and the environment is cleared to make sure nothing fails. Also we now guess if this own addon has been uninstalled and skip further logic if so, given it would hit broken triggers otherwise as it did before. --- module_auto_update/wizards/module_upgrade.py | 53 +++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/module_auto_update/wizards/module_upgrade.py b/module_auto_update/wizards/module_upgrade.py index eaf278651..3ab297d18 100644 --- a/module_auto_update/wizards/module_upgrade.py +++ b/module_auto_update/wizards/module_upgrade.py @@ -10,37 +10,40 @@ class ModuleUpgrade(models.TransientModel): @api.model def get_module_list(self): - if not self.env.context.get('module_uninstall'): - Module = self.env["ir.module.module"] - installed_modules = Module.search([('state', '=', 'installed')]) - upgradeable_modules = installed_modules.filtered( - lambda r: r.checksum_dir != r.checksum_installed, - ) - upgradeable_modules.button_upgrade() + """Set modules to upgrade searching by their dir checksum.""" + Module = self.env["ir.module.module"] + installed_modules = Module.search([('state', '=', 'installed')]) + upgradeable_modules = installed_modules.filtered( + lambda r: r.checksum_dir != r.checksum_installed, + ) + upgradeable_modules.button_upgrade() return super(ModuleUpgrade, self).get_module_list() - @api.multi - def upgrade_module_cancel(self): - return super( - ModuleUpgrade, - self.with_context(retain_checksum_installed=True), - ).upgrade_module_cancel() - @api.multi def upgrade_module(self): + """Make a fully automated addon upgrade.""" # Compute updates by checksum when called in @api.model fashion if not self: self.get_module_list() - # Get base adddon status before updating - base = self.env["ir.module.module"].search([("name", "=", "base")]) - pre_state = base.state + Module = self.env["ir.module.module"] + # Get every addon state before updating + pre_states = {addon["name"]: addon["state"] + for addon in Module.search_read([], ["name", "state"])} + # Perform upgrades, possibly in a limited graph that excludes me result = super(ModuleUpgrade, self).upgrade_module() - # Update base addon checksum if its state changed - base.invalidate_cache() - if base.state != pre_state: - # This triggers the write hook that should have been triggered - # when the module was [un]installed/updated in the base-only - # module graph inside above call to super(), and updates its - # dir checksum as needed - base.latest_version = base.latest_version + # Reload environments, anything may have changed + self.env.clear() + # Update addons checksum if state changed and I wasn't uninstalled + own = Module.search_read( + [("name", "=", "module_auto_update")], + ["state"], + limit=1) + if own and own[0]["state"] != "uninstalled": + for addon in Module.search([]): + if addon.state != pre_states.get(addon.name): + # Trigger the write hook that should have been + # triggered when the module was [un]installed/updated in + # the limited module graph inside above call to super(), + # and updates its dir checksum as needed + addon.latest_version = addon.latest_version return result