[REF] module_auto_update: Step 3, backwards compatibility

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.

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/9.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
 ===========
 

module_auto_update/__init__.py

@@ -4,4 +4,3 @@
 
 from . import models
 from . import wizards
-from .hooks import post_init_hook

module_auto_update/__openerp__.py

@@ -7,7 +7,7 @@
     'summary': 'Automatically update Odoo modules',
     'version': '9.0.2.0.0',
     'category': 'Extra Tools',
-    'website': 'https://odoo-community.org/',
+    'website': 'https://github.com/OCA/server-tools',
     'author': 'LasLabs, '
               'Juan José Scarafía, '
               'Tecnativa, '
@@ -16,7 +16,6 @@
     'license': 'LGPL-3',
     'application': False,
     'installable': True,
-    'post_init_hook': 'post_init_hook',
     'depends': [
         'base',
     ],

module_auto_update/data/cron_data_deprecated.xml

@@ -2,7 +2,7 @@
 <odoo noupdate="1">
     <record model="ir.cron" id="module_check_upgrades_cron">
         <field name="name">Perform Module Upgrades</field>
-        <field name="active" eval="True"/>
+        <field name="active" eval="False"/>
         <field name="user_id" ref="base.user_root"/>
         <field name="interval_number">1</field>
         <field name="interval_type">days</field>

module_auto_update/migrations/9.0.2.0.0/pre-migrate.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 Tecnativa - Jairo Llopis
+# License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl).
+import openupgradelib
+
+from openerp.addons.module_auto_update.models.module_deprecated import \
+    PARAM_DEPRECATED
+
+
+# TODO Remove when deprecated features are removed; see README's known issues
+@openupgradelib.migrate(no_version=True, use_env=True)
+def migrate(env):
+    """Move stored checksums to new JSON format."""
+    # Store module checksums in the new JSON format
+    env.cr.execute("""SELECT name, checksum_installed
+                      FROM ir_module_module
+                      WHERE checksum_installed""")
+    checksums = dict(env.cr.fetchall())
+    env["ir.module.module"]._save_checksums(checksums)
+    # Check if deprecation parameter existed
+    deprecated = env["ir.config_parameter"].get_param(PARAM_DEPRECATED)
+    if deprecated is False:
+        # If not, enable deprecated features now, to avoid breaking behavior
+        env["ir.config_parameter"].set_param(PARAM_DEPRECATED, "1")

module_auto_update/models/module_deprecated.py

@@ -4,14 +4,18 @@
 
 from openerp 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,
@@ -36,6 +40,9 @@ def _inverse_checksum_installed(self):
     @api.multi
     def _store_checksum_installed(self, vals):
         """Store the right installed checksum, if addon is installed."""
+        if self.env["ir.config_parameter"].get_param(PARAM_DEPRECATED) != "1":
+            # Skip if deprecated features are disabled
+            return
         if 'checksum_installed' not in vals:
             try:
                 version = vals["latest_version"]

module_auto_update/tests/test_module_deprecated.py

@@ -3,27 +3,27 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
 import os
-import tempfile
 
 import mock
 
 from openerp.modules import get_module_path
 from openerp.tests.common import TransactionCase
 from openerp.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 = 'openerp.addons.module_auto_update.models.module_deprecated'
+model = 'openerp.addons.module_auto_update.models.module'
 
 
 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),
         ])
@@ -42,37 +42,6 @@ def create_test_module(self, vals, get_module_path_mock):
         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."""
@@ -201,16 +170,3 @@ def test_write(self):
         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',
-        )

module_auto_update/tests/test_module_upgrade_deprecated.py

@@ -8,12 +8,15 @@
 from openerp.modules.registry import RegistryManager
 from openerp.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),
         ])

module_auto_update/wizards/module_upgrade_deprecated.py

@@ -4,48 +4,53 @@
 
 from openerp import api, models
 
+from ..models.module_deprecated import PARAM_DEPRECATED
+
 
 class ModuleUpgrade(models.TransientModel):
     _inherit = 'base.module.upgrade'
 
     @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.env["ir.config_parameter"].get_param(PARAM_DEPRECATED) == "1":
+            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"])}
-        # Perform upgrades, possibly in a limited graph that excludes me
-        self.env.cr.autocommit(True)  # Avoid transaction lock
+        if self.env["ir.config_parameter"].get_param(PARAM_DEPRECATED) == "1":
+            # 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"])}
+            # Perform upgrades, possibly in a limited graph that excludes me
+            self.env.cr.autocommit(True)  # Avoid transaction lock
         result = super(ModuleUpgrade, self).upgrade_module()
-        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
+        if self.env["ir.config_parameter"].get_param(PARAM_DEPRECATED) == "1":
+            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