Auditlog: Patching Causes Failures in Other Unit Tests

[AungKoKoLin1997]

Describe the Bug

When auditlog creates a rule during tests (e.g., for res.partner), it monkey-patches methods like res.partner.write and keeps a snapshot of the then-current implementation (usually the base method). If another addon (e.g., partner_contact_address_default) overrides write(), that override is skipped in tests and the call goes to the older base implementation. This causes other addons’ test cases to fail. In normal UI/server runs, everything works as expected; the problem appears only in test runs.

Impacted Versions

• 16.0

Haven't checked in other versions

To Reproduce

Run tests together with auditlog and other addons (for example, partner_contact_address_default).

A similar issue is also occurring with the base_user_role module, as described in OCA/server-backend#321 (comment).

@qrtl QT5614

[AungKoKoLin1997]

@amh-mw You might be interested in this issue.

[amh-mw]

This is why monkey patching has such a bad reputation.

[StefanRijnhart]

My suspicion is that we should not be patching the model class because we'd be patching some MetaModel's magic method. Instead, we might want to try and investigate the __mro__ to see if we can safely patch (and unpatch) the first 'regular' model class.

[StefanRijnhart]

Found a place where Odoo still applies monkeypatches to ORM models itself. When patching models, it still applies the same mechanism as auditlog does (showing that auditlog was a discarded Odoo module a long time ago): https://github.com/odoo/odoo/blob/5915842/addons/base_automation/models/base_automation.py#L929-L935 (nb. it uses self.env.registry[model] which seems to yield the same result as type(self.env[model]) in auditlog).

Interestingly, when unpatching models, it just strips off the method attributes from the object, not bothering with putting back the original method: https://github.com/odoo/odoo/blob/5915842/addons/base_automation/models/base_automation.py#L970C1-L978C25.

This is in line with my suspicion that what we are patching is the MetaModel and we should adjust our patching strategy to that fact, but its approach looks rather more straightforward. I suspect that if we refactor auditlog to follow the same approach when tearing down auditlog patches (e.g. in tests) it will solve the problem of loading additional write/create/unlink overrides afterwards.