diff --git a/server_environment/models/server_env_mixin.py b/server_environment/models/server_env_mixin.py index e45052829..3c8c32ea9 100644 --- a/server_environment/models/server_env_mixin.py +++ b/server_environment/models/server_env_mixin.py @@ -439,6 +439,13 @@ def _preserve_not_env_managed_data(self, field_name_list): This method forces to 'persist' these values if they are not explicitly overridden by the current environment configuration. + + Note: if a field is already server-env managed (ie it already has a + value stored in its ``_env_default`` companion field, for + instance because the mixin was already applied to it by another + module before), its raw column may only contain stale data from + before it became a non-stored field. In that case, we must not + overwrite the existing (up to date) default value with it. """ self.env.cr.execute(f"SELECT * FROM {self._table}") for row in self.env.cr.dictfetchall(): @@ -450,6 +457,15 @@ def _preserve_not_env_managed_data(self, field_name_list): if record: record_values = {} for field_name in field_name_list: - if field_name in row: - record_values[field_name] = row[field_name] - record.update(record_values) + if field_name not in row: + continue + default_field = self._server_env_default_fieldname(field_name) + if default_field and record[default_field]: + # A value is already preserved for this field + # (eg. set by a previous server-env managed version + # of the field): keep it, do not clobber it with the + # (possibly stale) raw column value. + continue + record_values[field_name] = row[field_name] + if record_values: + record.update(record_values) diff --git a/server_environment/tests/__init__.py b/server_environment/tests/__init__.py index 73093474f..6c0446da9 100644 --- a/server_environment/tests/__init__.py +++ b/server_environment/tests/__init__.py @@ -1,2 +1,3 @@ from . import test_server_environment from . import test_environment_variable +from . import test_preserve_not_env_managed_data diff --git a/server_environment/tests/fake_models.py b/server_environment/tests/fake_models.py new file mode 100644 index 000000000..b2a76f2ba --- /dev/null +++ b/server_environment/tests/fake_models.py @@ -0,0 +1,22 @@ +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from odoo import api, models + +# pylint: disable=consider-merging-classes-inherited + + +class FakePartner(models.Model): + _name = "res.partner" + _inherit = ["res.partner", "server.env.mixin"] + + @property + def _server_env_fields(self): + base_fields = super()._server_env_fields + partner_fields = { + "city": {}, + } + partner_fields.update(base_fields) + return partner_fields + + @api.model + def _server_env_global_section_name(self): + return "partner" diff --git a/server_environment/tests/test_preserve_not_env_managed_data.py b/server_environment/tests/test_preserve_not_env_managed_data.py new file mode 100644 index 000000000..d7579f9b7 --- /dev/null +++ b/server_environment/tests/test_preserve_not_env_managed_data.py @@ -0,0 +1,102 @@ +# Copyright 2026 Camptocamp SA +# License LGPL-3.0 or later (https://www.gnu.org/licenses/lgpl.html) + +from odoo_test_helper import FakeModelLoader + +from . import common + + +class TestPreserveNotEnvManagedData(common.ServerEnvironmentCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.loader = FakeModelLoader(cls.env, cls.__module__) + cls.loader.backup_registry() + cls._origin_fields = {} + for model in ("res.partner", "res.users"): + cls._origin_fields[model] = set(dir(cls.env[model].__class__)) + + def remove_mixin_fields(self): + """Clean up what FakeModelLoader.restore_registry() leaves behind. + + It never touches non-field attributes (e.g. _inverse_server_env_city). + """ + for model in ("res.partner", "res.users"): + extra = set(self.env[model].__class__.__dict__) - self._origin_fields[model] + for attr in extra: + delattr(self.env[model].__class__, attr) + + def setUp(self): + super().setUp() + from .fake_models import FakePartner + + self.loader.update_registry((FakePartner,)) + self.addCleanup(self.loader.restore_registry) + self.addCleanup(self.remove_mixin_fields) + # Deliberately not setting "city" here: writing it would already + # populate city_env_default through the mixin's inverse method, + # which would defeat the "no default yet" scenario below. + self.partner = self.env["res.partner"].create({"name": "Test partner"}) + + def _set_raw_city_column(self, value): + """Bypass the ORM to simulate a stale/orphaned raw column value. + + Once a field is taken over by the mixin it becomes non-stored, so + the ORM never reads or writes its physical column again. The column + itself is never dropped though, so it can keep holding old data from + before the field became server-env managed. + """ + self.env.cr.execute( + "UPDATE res_partner SET city = %s WHERE id = %s", + (value, self.partner.id), + ) + self.partner.invalidate_recordset(["city"]) + + def test_preserve_rescues_value_when_no_default_yet(self): + """Rescue the raw column value when there is no default yet. + + First-time adoption: no default stored yet, so the raw column + value must be rescued into the new default field. + """ + self._set_raw_city_column("Legacy Raw City") + self.env["res.partner"]._preserve_not_env_managed_data(["city"]) + self.partner.invalidate_recordset() + self.assertEqual(self.partner.city_env_default, "Legacy Raw City") + self.assertEqual(self.partner.city, "Legacy Raw City") + + def test_preserve_does_not_overwrite_existing_default(self): + """Do not overwrite a default value that is already set. + + A default already set (e.g. because the field was already + server-env managed by another module before) must not be clobbered + by a stale raw column value. + """ + # Simulate the field having already been server-env managed: a + # legitimate, up to date default is already stored. + self.partner.write({"city": "Current Default City"}) + self.assertEqual(self.partner.city_env_default, "Current Default City") + # The underlying (now unused) raw column still holds ancient data + # from before the field became non-stored. + self._set_raw_city_column("Ancient Stale City") + self.env["res.partner"]._preserve_not_env_managed_data(["city"]) + self.partner.invalidate_recordset() + self.assertEqual(self.partner.city_env_default, "Current Default City") + self.assertEqual(self.partner.city, "Current Default City") + + def test_preserve_ignores_unknown_column(self): + """Fields without a matching raw column are silently skipped.""" + # Must not raise even though the field name doesn't exist as a + # column on the table. + self.env["res.partner"]._preserve_not_env_managed_data(["field_not_a_column"]) + + def test_preserve_does_not_leak_into_env_configured_field(self): + """Keep reading from the environment when a config key is defined. + + When a config key is defined, the field must keep reading from + the environment, regardless of any raw column value. + """ + self._set_raw_city_column("Legacy Raw City") + with self.load_config(public="[partner]\ncity = From Env\n"): + self.env["res.partner"]._preserve_not_env_managed_data(["city"]) + self.partner.invalidate_recordset() + self.assertEqual(self.partner.city, "From Env") diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 000000000..4ad8e0ece --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1 @@ +odoo-test-helper