From 006b822ce3347e85809a85ecb5dc90a7bd15bf7f Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Fri, 16 Mar 2018 12:28:20 +0000 Subject: [PATCH] [REF] module_auto_update: Step 3, backwards compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation of this addon proved being extremely buggy: - It supplied out of the box a enabled cron to update Odoo that didn't restart the server, which possibly meant that upgrades broke things. - It overloaded standard Odoo upgrade methods that made i.e. installing an addon sometimes forced to upgrade all other addons in the database. - The checksum system wasn't smart enough, and some files that didn't need a module upgrade triggered the upgrade. - It was based on a dirhash library that was untested. - Some updates were not detected properly. - Storing a column into `ir.module.module` sometimes forbids uninstalling the addon. Thanks to Stéphane Bidoul (ACSONE), now we have new methods to perform the same work in a safer and more stable way. All I'm doing here is: - Cron is disabled by default. - Installed checksums are no longer saved at first install. - Old installations should keep most functionality intact thanks to the migration script. - Drop some duplicated tests. - Allow module uninstallation by pre-removing the fields from ir.mode.model. - When uninstalling the addon, the deprecated features will get removed for next installs always. Besides that, fixes for the new implementation too: - When uninstalling the addon, we remove the stored checksum data, so further installations work as if the addon was installed from scratch. --- module_auto_update/README.rst | 21 +++++ module_auto_update/__init__.py | 2 +- module_auto_update/__manifest__.py | 4 +- .../data/cron_data_deprecated.xml | 2 +- module_auto_update/hooks.py | 12 ++- .../migrations/10.0.2.0.0/pre-migrate.py | 23 +++++ .../models/module_deprecated.py | 13 ++- .../tests/test_module_deprecated.py | 52 +---------- .../tests/test_module_upgrade_deprecated.py | 3 + .../wizards/module_upgrade_deprecated.py | 93 +++++++++++++------ 10 files changed, 139 insertions(+), 86 deletions(-) create mode 100644 module_auto_update/migrations/10.0.2.0.0/pre-migrate.py diff --git a/module_auto_update/README.rst b/module_auto_update/README.rst index 8989df79a..1dd66fbbd 100644 --- a/module_auto_update/README.rst +++ b/module_auto_update/README.rst @@ -48,6 +48,27 @@ in an Odoo shell session:: :alt: Try me on Runbot :target: https://runbot.odoo-community.org/runbot/149/11.0 +Known issues / Roadmap +====================== + +* Since version ``2.0.0``, some features have been deprecated. + When you upgrade from previous versions, these features will be kept for + backwards compatibility, but beware! They are buggy! + + If you install this addon from scratch, these features are disabled by + default. + + To force enabling or disabling the deprecated features, set a configuration + parameter called ``module_auto_update.enable_deprecated`` to either ``1`` + or ``0``. It is recommended that you disable them. + + Keep in mind that from this version, all upgrades are assumed to run in a + separate odoo instance, dedicated exclusively to upgrade Odoo. + +* When migrating the addon to new versions, the deprecated features should be + removed. To make it simple all deprecated features are found in files + suffixed with ``_deprecated``. + Bug Tracker =========== diff --git a/module_auto_update/__init__.py b/module_auto_update/__init__.py index c80c51237..a0c82fd06 100644 --- a/module_auto_update/__init__.py +++ b/module_auto_update/__init__.py @@ -2,4 +2,4 @@ from . import models from . import wizards -from .hooks import post_init_hook +from .hooks import uninstall_hook diff --git a/module_auto_update/__manifest__.py b/module_auto_update/__manifest__.py index 4f4375fc7..d39962c1c 100644 --- a/module_auto_update/__manifest__.py +++ b/module_auto_update/__manifest__.py @@ -6,7 +6,7 @@ 'summary': 'Automatically update Odoo modules', 'version': '11.0.1.0.0', 'category': 'Extra Tools', - 'website': 'https://odoo-community.org/', + 'website': 'https://github.com/OCA/server-tools', 'author': 'LasLabs, ' 'Juan José Scarafía, ' 'Tecnativa, ' @@ -15,7 +15,7 @@ 'license': 'LGPL-3', 'application': False, 'installable': True, - 'post_init_hook': 'post_init_hook', + 'uninstall_hook': 'uninstall_hook', 'depends': [ 'base', ], diff --git a/module_auto_update/data/cron_data_deprecated.xml b/module_auto_update/data/cron_data_deprecated.xml index 9b5b1cc6b..d903dbda6 100644 --- a/module_auto_update/data/cron_data_deprecated.xml +++ b/module_auto_update/data/cron_data_deprecated.xml @@ -6,7 +6,7 @@ Perform Module Upgrades - + 1 days diff --git a/module_auto_update/hooks.py b/module_auto_update/hooks.py index 396f78707..cd161a246 100644 --- a/module_auto_update/hooks.py +++ b/module_auto_update/hooks.py @@ -3,7 +3,15 @@ from odoo import SUPERUSER_ID, api +from .models.module import PARAM_INSTALLED_CHECKSUMS +from .models.module_deprecated import PARAM_DEPRECATED -def post_init_hook(cr, registry): + +def uninstall_hook(cr, registry): env = api.Environment(cr, SUPERUSER_ID, {}) - env['ir.module.module']._save_installed_checksums() + env["ir.config_parameter"].set_param(PARAM_INSTALLED_CHECKSUMS, False) + # TODO Remove from here when removing deprecated features + env["ir.config_parameter"].set_param(PARAM_DEPRECATED, False) + prefix = "module_auto_update.field_ir_module_module_checksum_%s" + fields = env.ref(prefix % "dir") | env.ref(prefix % "installed") + fields.with_context(_force_unlink=True).unlink() diff --git a/module_auto_update/migrations/10.0.2.0.0/pre-migrate.py b/module_auto_update/migrations/10.0.2.0.0/pre-migrate.py new file mode 100644 index 000000000..4fe36ede7 --- /dev/null +++ b/module_auto_update/migrations/10.0.2.0.0/pre-migrate.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 Tecnativa - Jairo Llopis +# License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl). +import logging +from psycopg2 import IntegrityError +from openerp.addons.module_auto_update.models.module_deprecated import \ + PARAM_DEPRECATED + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + """Autoenable deprecated behavior.""" + try: + cr.execute( + "INSERT INTO ir_config_parameter (key, value) VALUES (%s, '1')", + (PARAM_DEPRECATED,) + ) + _logger.warn("Deprecated features have been autoenabled, see " + "addon's README to know how to upgrade to the new " + "supported autoupdate mechanism.") + except IntegrityError: + _logger.info("Deprecated features setting exists, not autoenabling") diff --git a/module_auto_update/models/module_deprecated.py b/module_auto_update/models/module_deprecated.py index 5f1f5b8e2..0cb9defaa 100644 --- a/module_auto_update/models/module_deprecated.py +++ b/module_auto_update/models/module_deprecated.py @@ -3,14 +3,18 @@ from odoo import api, fields, models +PARAM_DEPRECATED = "module_auto_update.enable_deprecated" + class Module(models.Model): _inherit = 'ir.module.module' checksum_dir = fields.Char( + deprecated=True, compute='_compute_checksum_dir', ) checksum_installed = fields.Char( + deprecated=True, compute='_compute_checksum_installed', inverse='_inverse_checksum_installed', store=False, @@ -27,14 +31,17 @@ class Module(models.Model): rec.checksum_installed = saved_checksums.get(rec.name, False) def _inverse_checksum_installed(self): - saved_checksums = self._get_saved_checksums() + checksums = self._get_saved_checksums() for rec in self: - saved_checksums[rec.name] = rec.checksum_installed - self._save_installed_checksums() + checksums[rec.name] = rec.checksum_installed + self._save_checksums(checksums) @api.multi def _store_checksum_installed(self, vals): """Store the right installed checksum, if addon is installed.""" + if not self.env["base.module.upgrade"]._autoupdate_deprecated(): + # Skip if deprecated features are disabled + return if 'checksum_installed' not in vals: try: version = vals["latest_version"] diff --git a/module_auto_update/tests/test_module_deprecated.py b/module_auto_update/tests/test_module_deprecated.py index b82fd6748..99d0ee4ec 100644 --- a/module_auto_update/tests/test_module_deprecated.py +++ b/module_auto_update/tests/test_module_deprecated.py @@ -2,7 +2,6 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). import os -import tempfile import mock @@ -10,12 +9,12 @@ from odoo.modules import get_module_path from odoo.tests.common import TransactionCase from odoo.tools import mute_logger -from ..addon_hash import addon_hash +from openerp.addons.module_auto_update.addon_hash import addon_hash -from .. import post_init_hook +from ..models.module_deprecated import PARAM_DEPRECATED -model = 'odoo.addons.module_auto_update.models.module_deprecated' +model = 'odoo.addons.module_auto_update.models.module' class EndTestException(Exception): @@ -27,6 +26,7 @@ class TestModule(TransactionCase): def setUp(self): super(TestModule, self).setUp() module_name = 'module_auto_update' + self.env["ir.config_parameter"].set_param(PARAM_DEPRECATED, "1") self.own_module = self.env['ir.module.module'].search([ ('name', '=', module_name), ]) @@ -45,37 +45,6 @@ class TestModule(TransactionCase): test_module = self.env['ir.module.module'].create(vals) return test_module - def test_compute_checksum_dir(self): - """It should compute the directory's SHA-1 hash""" - self.assertEqual( - self.own_module.checksum_dir, self.own_checksum, - 'Module directory checksum not computed properly', - ) - - def test_compute_checksum_dir_ignore_excluded(self): - """It should exclude .pyc/.pyo extensions from checksum - calculations""" - if not self.own_writeable: - self.skipTest("Own directory not writeable") - with tempfile.NamedTemporaryFile( - suffix='.pyc', dir=self.own_dir_path): - self.assertEqual( - self.own_module.checksum_dir, self.own_checksum, - 'SHA1 checksum does not ignore excluded extensions', - ) - - def test_compute_checksum_dir_recomputes_when_file_added(self): - """It should return a different value when a non-.pyc/.pyo file is - added to the module directory""" - if not self.own_writeable: - self.skipTest("Own directory not writeable") - with tempfile.NamedTemporaryFile( - suffix='.py', dir=self.own_dir_path): - self.assertNotEqual( - self.own_module.checksum_dir, self.own_checksum, - 'SHA1 checksum not recomputed', - ) - def test_store_checksum_installed_state_installed(self): """It should set the module's checksum_installed equal to checksum_dir when vals contain a ``latest_version`` str.""" @@ -239,16 +208,3 @@ class TestModule(TransactionCase): self.env['ir.module.module']._revert_method( '_store_checksum_installed', ) - - def test_post_init_hook(self): - """It should set checksum_installed equal to checksum_dir for all - installed modules""" - installed_modules = self.env['ir.module.module'].search([ - ('state', '=', 'installed'), - ]) - post_init_hook(self.env.cr, None) - self.assertListEqual( - installed_modules.mapped('checksum_dir'), - installed_modules.mapped('checksum_installed'), - 'Installed modules did not have checksum_installed stored', - ) diff --git a/module_auto_update/tests/test_module_upgrade_deprecated.py b/module_auto_update/tests/test_module_upgrade_deprecated.py index 880c80d00..13e729594 100644 --- a/module_auto_update/tests/test_module_upgrade_deprecated.py +++ b/module_auto_update/tests/test_module_upgrade_deprecated.py @@ -7,12 +7,15 @@ from odoo.modules import get_module_path from odoo.modules.registry import Registry from odoo.tests.common import TransactionCase +from ..models.module_deprecated import PARAM_DEPRECATED + class TestModuleUpgrade(TransactionCase): def setUp(self): super(TestModuleUpgrade, self).setUp() module_name = 'module_auto_update' + self.env["ir.config_parameter"].set_param(PARAM_DEPRECATED, "1") self.own_module = self.env['ir.module.module'].search([ ('name', '=', module_name), ]) diff --git a/module_auto_update/wizards/module_upgrade_deprecated.py b/module_auto_update/wizards/module_upgrade_deprecated.py index 8634c38e5..b44ec684d 100644 --- a/module_auto_update/wizards/module_upgrade_deprecated.py +++ b/module_auto_update/wizards/module_upgrade_deprecated.py @@ -1,49 +1,84 @@ # Copyright 2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). +import logging + from odoo import api, models +from ..models.module_deprecated import PARAM_DEPRECATED + +_logger = logging.getLogger(__name__) + class ModuleUpgrade(models.TransientModel): _inherit = 'base.module.upgrade' @api.model - @api.returns('ir.module.module') + def _autoupdate_deprecated(self): + """Know if we should enable deprecated features.""" + deprecated = ( + self.env["ir.config_parameter"].get_param(PARAM_DEPRECATED)) + if deprecated is False: + # Enable deprecated features if this is the 1st automated update + # after the version that deprecated them (X.Y.2.0.0) + own_module = self.env["ir.module.module"].search([ + ("name", "=", "module_auto_update"), + ]) + try: + if own_module.latest_version.split(".")[2] == "1": + deprecated = "1" + except AttributeError: + pass # 1st install, there's no latest_version + return deprecated == "1" + + @api.model def get_module_list(self): """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() + if self._autoupdate_deprecated(): + 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(self): """Make a fully automated addon upgrade.""" - # Compute updates by checksum when called in @api.model fashion - if not self: - self.get_module_list() - 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"])} + if self._autoupdate_deprecated(): + _logger.warning( + "You are possibly using an unsupported upgrade system; " + "set '%s' system parameter to '0' and start calling " + "`env['ir.module.module'].upgrade_changed_checksum()` from " + "now on to get rid of this message. See module's README's " + "Known Issues section for further information on the matter." + ) + # Compute updates by checksum when called in @api.model fashion + self.env.cr.autocommit(True) # Avoid transaction lock + if not self: + self.get_module_list() + 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() - # 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 + if self._autoupdate_deprecated(): + self.env.cr.autocommit(False) + # 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