From 37c63af7e2e46bd34ee42d547160d6c9c5d526e6 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 24 Aug 2026 22:55:21 -0400 Subject: [PATCH] =?UTF-8?q?[ADD]=20migration:=20brancher=20les=20deux=20r?= =?UTF-8?q?=C3=A9parations=20qui=20ne=20tournaient=20jamais?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix_duplicate_index et restore_config_defaults étaient écrits, éprouvés, et absents du pilote. Chaque migration refabriquait donc ses index redondants et reperdait sa liste de prix. Mesuré sur DEUX chaînes 12 → 18 indépendantes, même base source : 68 relations orphelines, 9 langues au drapeau NULL, une liste de prix absente, 376 paires d'index en double — les mêmes nombres des deux côtés. Ce n'est pas un accident d'exécution, c'est le chemin lui-même. Les index à partir du palier 17 : avant, la convention n'a pas changé. Les réglages au DERNIER palier : l'outil charge le registre, et seul l'état final compte. Les deux en wait_at_error=False — avec --apply, le code 1 dit « il en reste », pas « je suis tombé », et cela n'arrête pas six paliers. --- EN --- fix_duplicate_index and restore_config_defaults were written, proven, and absent from the driver. Every migration therefore rebuilt its redundant indexes and lost its default pricelist again. Measured on TWO independent 12 → 18 chains from the same source: 68 orphan relations, 9 languages with a NULL flag, one missing pricelist, 376 duplicate index pairs — the same numbers on both. Not a fluke of one run: the path itself. Indexes from step 17 onward: before that the convention had not changed. Config defaults at the LAST step: the tool loads the registry, and only the final state matters. Both with wait_at_error=False — with --apply, exit 1 means "some remain", not "I crashed", and that must not halt six steps. Assisted-by: Claude Opus 5 --- script/analyse/check_migration_residue.py | 4 +- script/todo/todo_i18n.py | 6 +- script/todo/todo_upgrade.py | 68 +++++++ test/test_todo_upgrade_repairs.py | 205 ++++++++++++++++++++++ 4 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 test/test_todo_upgrade_repairs.py diff --git a/script/analyse/check_migration_residue.py b/script/analyse/check_migration_residue.py index d09e1b2..eaef2d6 100755 --- a/script/analyse/check_migration_residue.py +++ b/script/analyse/check_migration_residue.py @@ -115,7 +115,9 @@ CONTROLES = ( "key": "duplicate_index", "title": "Indexes duplicated by the Odoo 17 renaming", "why": "Odoo 17 changed the naming convention without dropping the" - " old index: both are maintained on every write.", + " old index: both are maintained on every write. This count is a" + " cheap signal — the repair tool compares columns and" + " uniqueness, and is the one to trust.", "sql": "SELECT count(*) FROM pg_indexes a WHERE a.schemaname='public'" " AND a.indexname ~ '__[a-z0-9_]+_index$'" " AND EXISTS (SELECT 1 FROM pg_indexes b WHERE b.schemaname='public'" diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 447bfd6..8543898 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -9544,9 +9544,9 @@ TRANSLATIONS = { "fr": "Index doublés par le renommage d'Odoo 17", "en": "Indexes duplicated by the Odoo 17 renaming", }, - "Odoo 17 changed the naming convention without dropping the old index: both are maintained on every write.": { - "fr": "Odoo 17 a changé la convention de nommage sans supprimer l'ancien index : les deux sont entretenus à chaque écriture.", - "en": "Odoo 17 changed the naming convention without dropping the old index: both are maintained on every write.", + "Odoo 17 changed the naming convention without dropping the old index: both are maintained on every write. This count is a cheap signal — the repair tool compares columns and uniqueness, and is the one to trust.": { + "fr": "Odoo 17 a changé la convention de nommage sans supprimer l'ancien index : les deux sont entretenus à chaque écriture. Ce compte est un indicateur bon marché — l'outil de réparation compare les colonnes et l'unicité, c'est lui qui fait foi.", + "en": "Odoo 17 changed the naming convention without dropping the old index: both are maintained on every write. This count is a cheap signal — the repair tool compares columns and uniqueness, and is the one to trust.", }, "Default pricelist missing while product is installed": { "fr": "Liste de prix par défaut absente alors que product est installé", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index e33b413..908099a 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2781,6 +2781,14 @@ class TodoUpgrade: # ici : /blog//post/ et /contactus, alors que # le journal de migration n'avait rien signalé. self.prompt_database_cleanup(database_name_upgrade) + # APRÈS le nettoyage : il supprime des colonnes et des + # tables, donc leurs index avec. Avant lui, on travaillerait + # sur des index qui vont disparaître. + self.drop_duplicate_index(database_name_upgrade, next_version) + self.restore_config_defaults( + database_name_upgrade, + index == len(lst_next_version) - 1, + ) self.prompt_smoke_public_url(database_name_upgrade) # Le trou que ni les comptages ni le test de fumée ne @@ -3684,6 +3692,66 @@ class TodoUpgrade: wait_at_error=False, ) + def drop_duplicate_index(self, database_name, next_version): + """Les index qu'Odoo 17 a rebaptisés sans supprimer les anciens. + + `make_index_name` a changé de convention en 17 — `table_col_index` + est devenu `table__col_index` — et rien ne retire le premier. Les + deux restent, et PostgreSQL les entretient TOUS LES DEUX à chaque + écriture. Mesuré sur deux chaînes 12 → 18 indépendantes : 414 dans + l'une et 414 dans l'autre, à l'index près. Ce n'est pas un accident + d'exécution, c'est le chemin lui-même. + + À partir de 17 seulement : avant, la convention n'a pas changé et + l'outil ne trouverait rien — le lancer six fois pour rien ferait + du bruit dans un journal qu'on lit déjà mal. + + `wait_at_error=False` : avec `--apply`, le code 1 signifie « il en + reste », pas « je suis tombé ». Un index redondant qui survit ne + justifie pas d'arrêter une migration de six paliers ; l'outil + l'écrit, et le journal le garde. + """ + if next_version < 17: + return + outil = os.path.join(PATH_MIGRATION_GLOBAL, "fix_duplicate_index.py") + if not os.path.isfile(outil): + return + self.todo_upgrade_execute( + f"{PYTHON_BIN} ./{outil} -d {database_name} --apply", + wait_at_error=False, + ) + + def restore_config_defaults(self, database_name, is_last_version): + """Les réglages par défaut qu'aucune migration ne recrée. + + `product.list0` n'a été déclaré que jusqu'à Odoo 16 et + `account.reconciliation_model_default_rule` seulement en 12 : ils + naissent aujourd'hui d'un événement qu'une migration ne déclenche + jamais. La base arrive donc en 18 sans liste de prix par défaut, et + cela ne se découvre qu'au premier devis. Mesuré sur deux chaînes + indépendantes : absent des deux. + + AU DERNIER PALIER seulement. L'outil charge le registre Odoo — une + quarantaine de secondes — et seul l'état final compte : recréer la + liste au palier 13 pour la voir disparaître au 16 ne servirait qu'à + allonger six fois la migration. + + `wait_at_error=False` : même raison que pour les index. Le code 1 + veut dire « il en manque encore après la réparation », ce que + l'outil écrit lui-même. + """ + if not is_last_version: + return + outil = os.path.join( + PATH_MIGRATION_GLOBAL, "restore_config_defaults.py" + ) + if not os.path.isfile(outil): + return + self.todo_upgrade_execute( + f"{PYTHON_BIN} ./{outil} -d {database_name} --apply", + wait_at_error=False, + ) + def diff_cow_views(self, database_name, label_before, label_after): """Print what the version bump did to the website COW views.""" directory = os.path.join( diff --git a/test/test_todo_upgrade_repairs.py b/test/test_todo_upgrade_repairs.py new file mode 100644 index 0000000..b7ce67f --- /dev/null +++ b/test/test_todo_upgrade_repairs.py @@ -0,0 +1,205 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Deux réparations existaient et ne tournaient jamais. + +`fix_duplicate_index.py` et `restore_config_defaults.py` étaient écrits, +éprouvés sur copie, et absents du pilote. Chaque migration refabriquait +donc ses index redondants et reperdait sa liste de prix par défaut — +mesuré à l'identique sur DEUX chaînes 12 → 18 indépendantes : 414 index +et une liste manquante dans l'une comme dans l'autre. + +Un outil qui existe sans être appelé est le défaut le plus discret de ce +dépôt : rien n'échoue, rien ne l'écrit, et l'on croit le problème réglé +parce qu'on se souvient d'avoir écrit le correctif. Ce fichier vérifie +donc les DEUX choses — que les méthodes se comportent bien, et qu'elles +sont réellement appelées depuis la boucle des paliers. + +Les docstrings de ces méthodes CITENT `wait_at_error=False` pour +l'expliquer. Un test qui lirait le texte du fichier le trouverait là et +passerait au vert sans que l'argument soit passé nulle part. On lit donc +l'arbre syntaxique, jamais le texte. +""" + +import ast +import unittest +from pathlib import Path + +from script.todo.todo_upgrade import TodoUpgrade + +PILOTE = ( + Path(__file__).resolve().parent.parent + / "script" + / "todo" + / "todo_upgrade.py" +) + + +def arbre_du_pilote(): + """L'AST du pilote. `read_text` referme le fichier.""" + return ast.parse(PILOTE.read_text(encoding="utf-8")) + + +class FauxPilote: + """Un pilote qui note ce qu'on lui demande d'exécuter.""" + + def __init__(self): + self.appels = [] + + def todo_upgrade_execute(self, cmd, wait_at_error=True, **kwargs): + self.appels.append((cmd, wait_at_error)) + return 0 + + +class TestWhenTheIndexRepairRuns(unittest.TestCase): + def _lancer(self, version): + faux = FauxPilote() + TodoUpgrade.drop_duplicate_index(faux, "ma_base", version) + return faux.appels + + def test_it_stays_quiet_before_odoo_17(self): + """La convention n'a pas changé avant : il ne trouverait rien.""" + for version in (13, 14, 15, 16): + self.assertEqual(self._lancer(version), [], version) + + def test_it_runs_from_17_onward(self): + for version in (17, 18): + self.assertEqual(len(self._lancer(version)), 1, version) + + def test_it_repairs_rather_than_reports(self): + cmd, _ = self._lancer(18)[0] + self.assertIn("--apply", cmd) + self.assertIn("fix_duplicate_index.py", cmd) + self.assertIn("-d ma_base", cmd) + + def test_a_leftover_index_does_not_halt_the_migration(self): + """Avec --apply, le code 1 veut dire « il en reste », pas « échec ».""" + _, wait_at_error = self._lancer(18)[0] + self.assertFalse(wait_at_error) + + +class TestWhenTheDefaultsRepairRuns(unittest.TestCase): + def _lancer(self, dernier): + faux = FauxPilote() + TodoUpgrade.restore_config_defaults(faux, "ma_base", dernier) + return faux.appels + + def test_it_waits_for_the_last_step(self): + """Il charge le registre Odoo : six fois pour rien coûterait cher.""" + self.assertEqual(self._lancer(False), []) + + def test_it_runs_on_the_last_step(self): + self.assertEqual(len(self._lancer(True)), 1) + + def test_it_repairs_rather_than_reports(self): + cmd, wait_at_error = self._lancer(True)[0] + self.assertIn("--apply", cmd) + self.assertIn("restore_config_defaults.py", cmd) + self.assertFalse(wait_at_error) + + +class TestTheyAreActuallyCalled(unittest.TestCase): + """Le défaut visé : une méthode écrite que personne n'appelle.""" + + @classmethod + def setUpClass(cls): + cls.arbre = arbre_du_pilote() + cls.appels = {} + for noeud in ast.walk(cls.arbre): + if isinstance(noeud, ast.Call) and isinstance( + noeud.func, ast.Attribute + ): + cls.appels.setdefault(noeud.func.attr, []).append(noeud) + + def _boucle_des_paliers(self): + for noeud in ast.walk(self.arbre): + if ( + isinstance(noeud, ast.For) + and isinstance(noeud.iter, ast.Call) + and getattr(noeud.iter.func, "id", "") == "enumerate" + and [ + t.id + for t in ast.walk(noeud.target) + if isinstance(t, ast.Name) + ] + == ["index", "next_version"] + ): + return noeud + raise AssertionError("boucle des paliers introuvable") + + def test_both_repairs_are_called_from_the_tier_loop(self): + boucle = self._boucle_des_paliers() + for nom in ("drop_duplicate_index", "restore_config_defaults"): + dedans = [ + n + for n in self.appels.get(nom, []) + if boucle.lineno < n.lineno < boucle.end_lineno + ] + self.assertEqual(len(dedans), 1, nom) + + def test_the_index_repair_comes_after_the_cleanup(self): + """Le nettoyage supprime des colonnes, donc leurs index avec.""" + nettoyage = self.appels["prompt_database_cleanup"][-1].lineno + index = self.appels["drop_duplicate_index"][0].lineno + fumee = self.appels["prompt_smoke_public_url"][-1].lineno + self.assertLess(nettoyage, index) + self.assertLess(index, fumee) + + def test_the_defaults_repair_runs_before_the_smoke_test(self): + """Le test de fumée doit voir une base cohérente.""" + defauts = self.appels["restore_config_defaults"][0].lineno + fumee = self.appels["prompt_smoke_public_url"][-1].lineno + self.assertLess(defauts, fumee) + + def test_the_last_step_is_computed_from_the_loop_itself(self): + """`is_last` doit venir de la liste, pas d'un numéro écrit en dur.""" + appel = self.appels["restore_config_defaults"][0] + source = ast.dump(appel) + self.assertIn("lst_next_version", source) + self.assertNotIn("Constant(value=18)", source) + + +class TestTheDocstringTrapIsNotWhatWeRead(unittest.TestCase): + """La preuve que ce fichier ne se paie pas de mots. + + `wait_at_error=False` apparaît dans les docstrings des deux méthodes. + Si l'argument disparaissait des APPELS, un test textuel resterait vert. + """ + + def test_the_docstrings_do_mention_it(self): + for nom in ("drop_duplicate_index", "restore_config_defaults"): + methode = getattr(TodoUpgrade, nom) + self.assertIn("wait_at_error=False", methode.__doc__ or "", nom) + + def test_and_yet_the_keyword_is_really_passed(self): + arbre = arbre_du_pilote() + for nom in ("drop_duplicate_index", "restore_config_defaults"): + corps = None + for noeud in ast.walk(arbre): + if isinstance(noeud, ast.FunctionDef) and noeud.name == nom: + corps = noeud + self.assertIsNotNone(corps, nom) + # Le corps SANS sa docstring : c'est là que doit vivre l'argument. + sans_texte = [ + n + for n in corps.body + if not ( + isinstance(n, ast.Expr) + and isinstance(n.value, ast.Constant) + and isinstance(n.value.value, str) + ) + ] + trouve = False + for noeud in sans_texte: + for interne in ast.walk(noeud): + if isinstance(interne, ast.keyword) and ( + interne.arg == "wait_at_error" + ): + self.assertIs(interne.value.value, False, nom) + trouve = True + self.assertTrue(trouve, nom) + + +if __name__ == "__main__": + unittest.main()