[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.
- 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

@@ -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,4 @@
 
 from . import models
 from . import wizards
-from .hooks import post_init_hook
+from .hooks import uninstall_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,7 @@
     'license': 'LGPL-3',
     'application': False,
     'installable': True,
-    'post_init_hook': 'post_init_hook',
+    'uninstall_hook': 'uninstall_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/hooks.py

@@ -4,7 +4,15 @@
 
 from openerp 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()

module_auto_update/migrations/9.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")

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,
@@ -28,14 +32,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"]

module_auto_update/tests/test_module_deprecated.py

@@ -3,7 +3,6 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
 import os
-import tempfile
 
 import mock
 
@@ -11,12 +10,12 @@ 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):
@@ -24,6 +23,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),
         ])
@@ -42,37 +42,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."""
@@ -201,16 +170,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',
-        )

module_auto_update/tests/test_module_upgrade_deprecated.py

@@ -8,12 +8,15 @@ from openerp.modules import get_module_path
 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

@@ -2,15 +2,40 @@
 # Copyright 2017 LasLabs Inc.
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
+import logging
+
 from openerp import api, models
 
+from ..models.module_deprecated import PARAM_DEPRECATED
+
+_logger = logging.getLogger(__name__)
+
 
 class ModuleUpgrade(models.TransientModel):
     _inherit = 'base.module.upgrade'
 
+    @api.model
+    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."""
+        if self._autoupdate_deprecated():
             Module = self.env["ir.module.module"]
             installed_modules = Module.search([('state', '=', 'installed')])
             upgradeable_modules = installed_modules.filtered(
@@ -22,16 +47,25 @@ class ModuleUpgrade(models.TransientModel):
     @api.multi
     def upgrade_module(self):
         """Make a fully automated addon upgrade."""
+        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"])}
+            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()
+        if self._autoupdate_deprecated():
             self.env.cr.autocommit(False)
             # Reload environments, anything may have changed
             self.env.clear()
@@ -44,8 +78,8 @@ class ModuleUpgrade(models.TransientModel):
                 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
+                        # 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