[IMP] core: introduce api.ondelete decorator

With this commit, a new ORM api decorator is introduced:
`api.ondelete(*, at_uninstall)`.

This decorator is to be applied to Model methods that check for specific
business conditions when attempting to unlink a record via the
interface.

E.g. trying to unlink a validated journal entry

This decorator allows this logic to exist outside the `BaseModel.unlink`
method and is automatically bypassed when in uninstall mode, this means
that during an uninstall any and all data related to a module can and
will be removed easily and cleanly while still being able to apply
business logic to manual deletion of records.

This feature opens the gates to solving a very big problem with
uninstalls: records and tables that remain in a database despite the
relevant module being uninstalled, because if an override to unlink
raises an error, the data will never be deleted from the database.

Henceforth, overrides of unlink shall solely be used for data-cleaning
purposes, i.e. deletion or modification of data that is related to the
one currently being deleted but cannot be automatically deleted because
there are no proper SQL relations.

In certain very specific, low-level scenarios an unlink may be
overridden to raise an error, but this should only be done if you know
what the fuck you're doing, most of the time you'll want to resort to
`@api.ondelete`.

Note that this new decorator includes a keyword-only, required argument
called `at_uninstall`, in most business cases this argument shall be
False as this argument dictates whether or not this method should be
executed in the `unlink` call during uninstall. It should only be set to
True if you are certain of all the implications which most likely means
that the records of the model in which this ondelete function is defined
will NOT be removed during uninstall, they will forever linger in the DB
until manual intervention, this in turn can mean a wide range of
undefined problems due to crap left on the database.

Following commits will replace any `unlink` overrides that raise
business errors by methods decorated with `api.ondelete`, another commit
will introduce a pylint checker that will raise a warning any time that
an unlink override raises an error.

odoo/addons/test_new_api/models/test_new_api.py

@@ -1179,3 +1179,23 @@ class ComputeEditableLine(models.Model):
     def _compute_edit(self):
         for line in self:
             line.edit = line.value
+
+
+class ConstrainedUnlinks(models.Model):
+    _name = 'test_new_api.model_constrained_unlinks'
+    _description = 'Model with unlink override that is constrained'
+
+    foo = fields.Char()
+    bar = fields.Integer()
+
+    @api.ondelete(at_uninstall=False)
+    def _unlink_except_bar_gt_five(self):
+        for rec in self:
+            if rec.bar and rec.bar > 5:
+                raise ValueError("Nooooooooo bar can't be greater than five!!")
+
+    @api.ondelete(at_uninstall=True)
+    def _unlink_except_prosciutto(self):
+        for rec in self:
+            if rec.foo and rec.foo == 'prosciutto':
+                raise ValueError("You didn't say if you wanted it crudo or cotto...")

odoo/addons/test_new_api/security/ir.model.access.csv

@@ -68,3 +68,4 @@ access_test_new_api_compute_container,access_test_new_api_compute_container,mode
 access_test_new_api_compute_member,access_test_new_api_compute_member,model_test_new_api_compute_member,,1,1,1,1
 access_test_new_api_compute_editable,access_test_new_api_compute_editable,model_test_new_api_compute_editable,,1,1,1,1
 access_test_new_api_compute_editable_line,access_test_new_api_compute_editable_line,model_test_new_api_compute_editable_line,,1,1,1,1
+access_test_new_api_model_constrained_unlinks,access_test_new_api_model_constrained_unlinks,model_test_new_api_model_constrained_unlinks,,1,1,1,1

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -3109,3 +3109,42 @@ class test_shared_cache(TransactionCaseWithUserDemo):
                 line.amount = 2
             # The new value for total_amount, should be 3, not 2.
             self.assertEqual(task_form.total_amount, 2)
+
+
+@common.tagged('unlink_constraints')
+class TestUnlinkConstraints(common.TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        MODEL = cls.env['test_new_api.model_constrained_unlinks']
+
+        cls.deletable_bar = MODEL.create({'bar': 5})
+        cls.undeletable_bar = MODEL.create({'bar': 6})
+        cls.deletable_foo = MODEL.create({'foo': 'formaggio'})
+        cls.undeletable_foo = MODEL.create({'foo': 'prosciutto'})
+
+        from odoo.addons.base.models.ir_model import MODULE_UNINSTALL_FLAG
+        uninstall = {MODULE_UNINSTALL_FLAG: True}
+        cls.undeletable_bar_uninstall = cls.undeletable_bar.with_context(**uninstall)
+        cls.undeletable_foo_uninstall = cls.undeletable_foo.with_context(**uninstall)
+
+    def test_unlink_constraint_manual_bar(self):
+        self.assertTrue(self.deletable_bar.unlink())
+        with self.assertRaises(ValueError, msg="Nooooooooo bar can't be greater than five!!"):
+            self.undeletable_bar.unlink()
+
+    def test_unlink_constraint_uninstall_bar(self):
+        self.assertTrue(self.deletable_bar.unlink())
+        # should succeed since it's at_uninstall=False
+        self.assertTrue(self.undeletable_bar_uninstall.unlink())
+
+    def test_unlink_constraint_manual_foo(self):
+        self.assertTrue(self.deletable_foo.unlink())
+        with self.assertRaises(ValueError, msg="You didn't say if you wanted it crudo or cotto..."):
+            self.undeletable_foo.unlink()
+
+    def test_unlink_constraint_uninstall_foo(self):
+        self.assertTrue(self.deletable_foo)
+        # should fail since it's at_uninstall=True
+        with self.assertRaises(ValueError, msg="You didn't say if you wanted it crudo or cotto..."):
+            self.undeletable_foo_uninstall.unlink()

odoo/api.py

@@ -37,6 +37,7 @@ _logger = logging.getLogger(__name__)
 #  - method._returns: set by @returns, specifies return model
 #  - method._onchange: set by @onchange, specifies onchange fields
 #  - method.clear_cache: set by @ormcache, used to clear the cache
+#  - method._ondelete: set by @ondelete, used to raise errors for unlink operations
 #
 # On wrapping method only:
 #  - method._api: decorator function, used for re-applying decorator
@@ -130,6 +131,65 @@ def constrains(*args):
     return attrsetter('_constrains', args)
 
 
+def ondelete(*, at_uninstall):
+    """
+    Mark a method to be executed during :meth:`~odoo.models.BaseModel.unlink`.
+
+    The goal of this decorator is to allow client-side errors when unlinking
+    records if, from a business point of view, it does not make sense to delete
+    such records. For instance, a user should not be able to delete a validated
+    sales order.
+
+    While this could be implemented by simply overriding the method ``unlink``
+    on the model, it has the drawback of not being compatible with module
+    uninstallation. When uninstalling the module, the override could raise user
+    errors, but we shouldn't care because the module is being uninstalled, and
+    thus **all** records related to the module should be removed anyway.
+
+    This means that by overriding ``unlink``, there is a big chance that some
+    tables/records may remain as leftover data from the uninstalled module. This
+    leaves the database in an inconsistent state. Moreover, there is a risk of
+    conflicts if the module is ever reinstalled on that database.
+
+    Methods decorated with ``@ondelete`` should raise an error following some
+    conditions, and by convention, the method should be named either
+    ``_unlink_if_<condition>`` or ``_unlink_except_<not_condition>``.
+
+    .. code-block:: python
+
+        @api.ondelete(at_uninstall=False)
+        def _unlink_if_user_inactive(self):
+            if any(user.active for user in self):
+                raise UserError("Can't delete an active user!")
+
+        # same as above but with _unlink_except_* as method name
+        @api.ondelete(at_uninstall=False)
+        def _unlink_except_active_user(self):
+            if any(user.active for user in self):
+                raise UserError("Can't delete an active user!")
+
+    :param bool at_uninstall: Whether the decorated method should be called if
+        the module that implements said method is being uninstalled. Should
+        almost always be ``False``, so that module uninstallation does not
+        trigger those errors.
+
+    .. danger::
+        The parameter ``at_uninstall`` should only be set to ``True`` if the
+        check you are implementing also applies when uninstalling the module.
+
+        For instance, it doesn't matter if when uninstalling ``sale``, validated
+        sales orders are being deleted because all data pertaining to ``sale``
+        should be deleted anyway, in that case ``at_uninstall`` should be set to
+        ``False``.
+
+        However, it makes sense to prevent the removal of the default language
+        if no other languages are installed, since deleting the default language
+        will break a lot of basic behavior. In this case, ``at_uninstall``
+        should be set to ``True``.
+    """
+    return attrsetter('_ondelete', at_uninstall)
+
+
 def onchange(*args):
     """Return a decorator to decorate an onchange method for given fields.
 

odoo/models.py

@@ -672,6 +672,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
 
         # reset properties memoized on cls
         cls._constraint_methods = BaseModel._constraint_methods
+        cls._ondelete_methods = BaseModel._ondelete_methods
         cls._onchange_methods = BaseModel._onchange_methods
 
     @property
@@ -695,6 +696,18 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         cls._constraint_methods = methods
         return methods
 
+    @property
+    def _ondelete_methods(self):
+        """ Return a list of methods implementing checks before unlinking. """
+        def is_ondelete(func):
+            return callable(func) and hasattr(func, '_ondelete')
+
+        cls = type(self)
+        methods = [func for _, func in getmembers(cls, is_ondelete)]
+        # optimization: memoize results on cls, it will not be recomputed
+        cls._ondelete_methods = methods
+        return methods
+
     @property
     def _onchange_methods(self):
         """ Return a dictionary mapping field names to onchange methods. """
@@ -3413,6 +3426,12 @@ Fields:
         self.check_access_rights('unlink')
         self._check_concurrency()
 
+        from odoo.addons.base.models.ir_model import MODULE_UNINSTALL_FLAG
+        for func in self._ondelete_methods:
+            # func._ondelete is True if it should be called during uninstallation
+            if func._ondelete or not self._context.get(MODULE_UNINSTALL_FLAG):
+                func(self)
+
         # mark fields that depend on 'self' to recompute them after 'self' has
         # been deleted (like updating a sum of lines after deleting one line)
         self.flush()