[ADD] filestore: purge once at the end, tidy at the restore, see the 30 MB

Not between bumps, and the measurement says why: two to eleven fields
vanish at one step and COME BACK at the next -- hr.employee.phone,
account.move.statement_id. "The field is gone" is a transient state
while a migration runs. And there would be nothing to gain: 1881 dead
rows appear at the 13 bump and the count never moves again, so one
final pass takes them all.

The nesting is born once, at the restore, and the clone copies it
identically into every step -- the six databases carried the same 1168
files. It is offered where it is born, never on a closed stdin.

Widened too: the tool was named after missing files and so looked only
at those. 1860 rows in 18 hold a live file for a field that is gone --
31 MB of res.partner.image and thumbnails from before Odoo 13 computed
them. Odoo's collector will never touch them while the row exists.

--- FR ---

Pas entre les paliers, et la mesure dit pourquoi : deux à onze champs
disparaissent à une étape et REVIENNENT à la suivante --
hr.employee.phone, account.move.statement_id. « Le champ n'existe plus »
est transitoire tant que la migration court. Et il n'y aurait rien à y
gagner : 1881 lignes mortes naissent au palier 13 et le compte ne bouge
plus, donc une passe finale les prend toutes.

Le nichage naît une fois, à la restauration, et le clone le recopie
partout — les six bases portaient les mêmes 1168 fichiers. Il se répare
là où il naît, jamais sur un stdin fermé.

Élargi aussi : l'outil portait le nom des fichiers absents et ne
regardait donc qu'eux. 1860 lignes en 18 retiennent un fichier bien
présent pour un champ disparu — 31 Mo d'images res.partner et de
vignettes d'avant qu'Odoo 13 ne les calcule.

Assisted-by: Claude Opus 5

script/analyse/check_filestore.py

@@ -175,9 +175,9 @@ def scan_filestores(racine, sauf=None):
     qu'on cherchera — mais notés à part : les remettre en place est un
     déplacement, pas une copie depuis ailleurs.
     """
-    ailleurs, niches = {}, {}
+    ailleurs, niches, par_base = {}, {}, {}
     if not os.path.isdir(racine):
-        return ailleurs, niches
+        return ailleurs, niches, par_base
     for base in sorted(os.listdir(racine)):
         chemin = os.path.join(racine, base)
         if not os.path.isdir(chemin):
@@ -187,19 +187,25 @@ def scan_filestores(racine, sauf=None):
             if not os.path.isdir(sous):
                 continue
             if prefixe == "filestore":
-                cible, marque = niches, base
+                # Un même fichier peut être niché dans PLUSIEURS bases —
+                # le clone les recopie toutes. L'index n'en retient qu'une
+                # (le premier `setdefault` gagne) : il sert à retrouver un
+                # fichier, pas à compter. D'où le décompte par base, sans
+                # lequel une base nichée s'entendait dire que le problème
+                # était chez les voisines.
                 for deux in sorted(os.listdir(sous)):
                     profond = os.path.join(sous, deux)
                     if not os.path.isdir(profond):
                         continue
                     for nom in os.listdir(profond):
-                        cible.setdefault(f"{deux}/{nom}", marque)
+                        niches.setdefault(f"{deux}/{nom}", base)
+                        par_base[base] = par_base.get(base, 0) + 1
                 continue
             if base == sauf:
                 continue
             for nom in os.listdir(sous):
                 ailleurs.setdefault(f"{prefixe}/{nom}", base)
-    return ailleurs, niches
+    return ailleurs, niches, par_base
 
 
 def scan_backups(dossier):
@@ -358,6 +364,18 @@ def render_verify(rapport):
     return lignes
 
 
+def is_dead_field(piece, champs_vivants):
+    """La pièce jointe porte-t-elle un champ qui n'existe plus ?
+
+    Sans `res_field` la question ne se pose pas : c'est un document
+    téléversé, pas la valeur d'un champ. Le juger sur un champ absent le
+    ferait disparaître du rapport.
+    """
+    if not piece.get("field"):
+        return False
+    return f"{piece['model']}.{piece['field']}" not in champs_vivants
+
+
 def classify(piece, present, ailleurs, niches, sauvegardes, champs_vivants):
     """Le verdict d'une pièce jointe. None si son fichier est là.
 
@@ -368,10 +386,8 @@ def classify(piece, present, ailleurs, niches, sauvegardes, champs_vivants):
         return None
     # Un champ disparu n'a rien à récupérer : la ligne est une scorie.
     # Le tester EN PREMIER évite de proposer une remise en place inutile.
-    if piece["field"]:
+    if is_dead_field(piece, champs_vivants):
-        cle = f"{piece['model']}.{piece['field']}"
+        return ("dead_field", f"{piece['model']}.{piece['field']}")
-        if cle not in champs_vivants:
-            return ("dead_field", cle)
     if piece["store_fname"] in niches:
         return ("nested", niches[piece["store_fname"]])
     if piece["store_fname"] in ailleurs:
@@ -401,18 +417,26 @@ def audit(database, config_path=None, backups=None):
             for nom in os.listdir(sous):
                 if os.path.isfile(os.path.join(sous, nom)):
                     present.add(f"{prefixe}/{nom}")
-    ailleurs, niches = scan_filestores(racine, sauf=database)
+    ailleurs, niches, par_base = scan_filestores(racine, sauf=database)
     sauvegardes = scan_backups(backups or os.path.join(REPO_ROOT, "image_db"))
     champs_vivants = live_fields(database)
 
     groupes = {verdict: [] for verdict in VERDICTS}
     vus = set()
     morts = []
+    gardees = []
     for piece in pieces:
         verdict = classify(
             piece, present, ailleurs, niches, sauvegardes, champs_vivants
         )
         if not verdict:
+            # Fichier présent — mais son champ vit-il encore ? Un outil
+            # nommé « fichiers absents » ne regardait pas là, et laissait
+            # dormir 1860 lignes et 30 Mo que plus rien ne lit.
+            if is_dead_field(piece, champs_vivants):
+                gardees.append(piece)
+                if str(piece.get("id", "")).isdigit():
+                    morts.append(int(piece["id"]))
             continue
         # La déduplication qui suit sert à compter des FICHIERS. Pour
         # effacer des LIGNES il les faut toutes : vingt-deux lignes
@@ -436,9 +460,14 @@ def audit(database, config_path=None, backups=None):
         "files_present": len(present),
         "missing": len(vus),
         "groups": groupes,
-        "nested_total": len(niches),
+        "nested_total": par_base.get(database, 0),
+        "nested_elsewhere": sum(
+            combien for base, combien in par_base.items() if base != database
+        ),
         "root": mien,
         "dead_ids": morts,
+        "dead_kept": gardees,
+        "dead_kept_size": sum(piece["size"] for piece in gardees),
     }
 
 
@@ -452,7 +481,7 @@ def render(rapport, limit=20):
     ]
     if not rapport["missing"]:
         lignes.append(f"   ✅ {t('every attachment file is present')}")
-        return lignes + render_nested(rapport)
+        return lignes + render_dead_kept(rapport) + render_nested(rapport)
     lignes.append(f"   {rapport['missing']} {t('file(s) missing')} :")
     for verdict in VERDICTS:
         groupe = rapport["groups"][verdict]
@@ -487,17 +516,53 @@ def render(rapport, limit=20):
             )
         if limit and len(groupe) > limit:
             lignes.append(f"         … {len(groupe) - limit} {t('more')}")
-    return lignes + render_nested(rapport)
+    return lignes + render_dead_kept(rapport) + render_nested(rapport)
+
+
+def render_dead_kept(rapport, limit=4):
+    """Les lignes mortes dont le FICHIER est toujours là.
+
+    Elles ne manquent à personne — c'est justement le problème : rien ne
+    les lit, et leur fichier occupe le disque tant que la ligne existe,
+    puisque le ramasse-miettes d'Odoo ne retire que ce qui n'est plus
+    référencé. Un outil nommé « fichiers absents » ne regardait pas là,
+    et laissait dormir 1860 lignes et 30 Mo.
+    """
+    gardees = rapport.get("dead_kept") or []
+    if not gardees:
+        return []
+    lignes = [
+        "",
+        f"   🕳 {len(gardees)}"
+        f" {t('row(s) whose field is gone still hold their file')}"
+        f"  ({rapport.get('dead_kept_size', 0) // 1024} ko)",
+    ]
+    apercu = summarise(gardees)
+    for texte in apercu[: limit or None]:
+        lignes.append(f"       {texte}")
+    if limit and len(apercu) > limit:
+        lignes.append(f"       … {len(apercu) - limit} {t('more')}")
+    return lignes
 
 
 def render_nested(rapport):
+    ailleurs = rapport.get("nested_elsewhere") or 0
     if not rapport.get("nested_total"):
+        # Rien ici, mais peut-être chez les voisines : le dire sans
+        # laisser croire que CETTE base est concernée.
+        if ailleurs:
+            return [
+                "",
+                f"   ↳ {ailleurs}"
+                f" {t('such file(s) sit in OTHER databases filestores.')}",
+            ]
         return []
-    return [
+    lignes = [
         "",
         f"   ↳ {rapport['nested_total']}"
-        f" {t('file(s) sit in nested filestores Odoo never reads.')}",
+        f" {t('file(s) sit in a nested filestore Odoo never reads.')}",
     ]
+    return lignes
 
 
 def alive_mark(piece):

script/database/db_restore.py

@@ -7,6 +7,7 @@ import configparser
 import getpass
 import logging
 import os
+import shutil
 import sys
 from subprocess import check_output
 
@@ -113,6 +114,41 @@ def verify_filestore(database, image):
     rapport = check_filestore.verify_restore(database, chemin)
     for ligne in check_filestore.render_verify(rapport):
         print(ligne)
+    if rapport.get("nested"):
+        offer_tidy(check_filestore, rapport)
+
+
+def offer_tidy(check_filestore, rapport):
+    """Proposer de ranger TOUT DE SUITE, là où le défaut naît.
+
+    C'est le seul endroit qui vaille. Le nichage se produit une fois, à
+    la restauration, puis le clone le recopie tel quel : mesuré, les six
+    bases de la chaîne portaient les mêmes 1168 fichiers. Ranger ici,
+    c'est ranger une fois ; ranger plus tard, c'est six fois.
+
+    Rien ne se fait sans réponse humaine, et rien du tout hors d'un
+    terminal : ce script tourne aussi sans personne devant, et une
+    question posée à un `stdin` fermé arrêterait la migration.
+    """
+    if not sys.stdin.isatty():
+        return
+    remonter, doublons = check_filestore.tidy_nested_plan(rapport)
+    if not remonter and not doublons:
+        return
+    print(f"   {len(remonter)} à remonter, {len(doublons)} doublons purs")
+    try:
+        reponse = input("💬 Ranger maintenant ? (y/N) : ").strip().lower()
+    except EOFError:
+        return
+    if reponse not in ("y", "yes", "o"):
+        return
+    for source, cible in remonter:
+        os.makedirs(os.path.dirname(cible), exist_ok=True)
+        shutil.move(source, cible)
+    for source, _cible in doublons:
+        os.remove(source)
+    shutil.rmtree(check_filestore.nested_dir(rapport), ignore_errors=True)
+    print(f"✅ {len(remonter)} remontés, {len(doublons)} doublons supprimés.")
 
 
 def restore_or_clone(config, arg_base, cache_database, lst_db_cache):

script/todo/todo_upgrade.py

@@ -2825,6 +2825,7 @@ class TodoUpgrade:
             f"✨ {t('Re-update i18n, purge the data and the tables')}"
             f" ({t('except mail_test and mail_test_full')})"
         )
+        self.prompt_purge_dead_attachments(database_name_upgrade)
         # waiting_input = self.ask("💬print Press any keyboard key to continue...")
         msg = "6 - Migration finished"
         self.print_step(msg)

test/test_check_filestore.py

@@ -149,18 +149,32 @@ class TestScanning(unittest.TestCase):
         shutil.rmtree(self.racine)
 
     def test_other_filestores_are_indexed_and_mine_is_skipped(self):
-        ailleurs, _n = fs.scan_filestores(self.racine, sauf="ma_base")
+        ailleurs, _n, _p = fs.scan_filestores(self.racine, sauf="ma_base")
         self.assertEqual(ailleurs.get("bb/ailleurs"), "autre")
         self.assertNotIn("aa/present", ailleurs)
 
     def test_nested_files_are_indexed_under_their_logical_name(self):
         # C'est sous « cc/niche » qu'on les cherchera, pas sous
         # « filestore/cc/niche ».
-        _a, niches = fs.scan_filestores(self.racine, sauf=None)
+        _a, niches, _p = fs.scan_filestores(self.racine, sauf=None)
         self.assertEqual(niches.get("cc/niche"), "ma_base")
 
+    def test_the_same_nested_file_in_two_databases_counts_in_BOTH(self):
+        # Le clone recopie le nichage : un même fichier dort dans
+        # plusieurs bases. L'index n'en retient qu'une — le premier
+        # `setdefault` gagne, et l'ordre est alphabétique — donc la 17
+        # s'entendait dire que le problème était chez les voisines.
+        for base in ("aaa_base", "zzz_base"):
+            complet = os.path.join(self.racine, base, "filestore", "ee", "x")
+            os.makedirs(os.path.dirname(complet), exist_ok=True)
+            with open(complet, "w", encoding="utf-8") as handle:
+                handle.write("x")
+        _a, _n, par_base = fs.scan_filestores(self.racine, sauf=None)
+        self.assertEqual(par_base.get("aaa_base"), 1)
+        self.assertEqual(par_base.get("zzz_base"), 1)
+
     def test_a_missing_root_is_empty_not_a_crash(self):
-        self.assertEqual(fs.scan_filestores("/nulle/part"), ({}, {}))
+        self.assertEqual(fs.scan_filestores("/nulle/part"), ({}, {}, {}))
 
     def test_backups_are_read_from_the_central_directory(self):
         dossier = tempfile.mkdtemp()
@@ -197,7 +211,7 @@ class TestTheAudit(unittest.TestCase):
             fs.filestore_root,
         )
         fs.live_fields = lambda base: set()
-        fs.scan_filestores = lambda racine, sauf=None: ({}, {})
+        fs.scan_filestores = lambda racine, sauf=None: ({}, {}, {})
         fs.scan_backups = lambda dossier: {}
         fs.filestore_root = lambda config=None: "/nulle/part"
 
@@ -751,6 +765,220 @@ class TestTidyingForReal(unittest.TestCase):
             self.assertEqual(handle.read(), "bon")
 
 
+class TestTheDeadRowsThatKeptTheirFile(unittest.TestCase):
+    """1860 lignes, 30 Mo, que l'outil ne voyait pas.
+
+    Il s'appelle « fichiers absents » et ne regardait donc que les
+    fichiers absents. Or une ligne dont le champ a disparu retient son
+    fichier tant qu'elle existe : le ramasse-miettes d'Odoo ne retire
+    que ce qui n'est plus référencé.
+    """
+
+    def setUp(self):
+        self.vrais = (
+            fs.attachments,
+            fs.live_fields,
+            fs.scan_filestores,
+            fs.scan_backups,
+            fs.filestore_root,
+        )
+        fs.live_fields = lambda base: {"res.partner.image_1920"}
+        fs.scan_filestores = lambda racine, sauf=None: ({}, {}, {})
+        fs.scan_backups = lambda dossier: {}
+        fs.filestore_root = lambda config=None: "/nulle/part"
+
+    def tearDown(self):
+        (
+            fs.attachments,
+            fs.live_fields,
+            fs.scan_filestores,
+            fs.scan_backups,
+            fs.filestore_root,
+        ) = self.vrais
+
+    def pose_fichier(self, chemin):
+        """Un VRAI fichier : c'est la présence qui distingue les deux cas."""
+        complet = os.path.join(self.racine, "db", chemin)
+        os.makedirs(os.path.dirname(complet), exist_ok=True)
+        with open(complet, "w", encoding="utf-8") as handle:
+            handle.write("x")
+
+    def test_a_dead_row_WITH_its_file_lands_in_dead_kept(self):
+        self.racine = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.racine, True)
+        fs.filestore_root = lambda config=None: self.racine
+        self.pose_fichier("aa/bb")
+        fs.attachments = lambda base: [
+            piece("aa/bb", "res.partner", "image", size=4096, pid="5")
+        ]
+        rapport = fs.audit("db")
+        # Fichier présent : ce n'est PAS un fichier manquant…
+        self.assertEqual(rapport["missing"], 0)
+        # …mais la ligne est morte, et son fichier occupe le disque.
+        self.assertEqual(len(rapport["dead_kept"]), 1)
+        self.assertEqual(rapport["dead_kept_size"], 4096)
+        self.assertEqual(rapport["dead_ids"], [5])
+
+    def test_a_living_row_with_its_file_is_left_alone(self):
+        self.racine = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.racine, True)
+        fs.filestore_root = lambda config=None: self.racine
+        self.pose_fichier("aa/bb")
+        fs.attachments = lambda base: [
+            piece("aa/bb", "res.partner", "image_1920", size=4096, pid="5")
+        ]
+        rapport = fs.audit("db")
+        self.assertEqual(rapport["dead_kept"], [])
+        self.assertEqual(rapport["dead_ids"], [])
+
+    def test_the_size_is_summed_by_the_audit_itself(self):
+        self.racine = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.racine, True)
+        fs.filestore_root = lambda config=None: self.racine
+        for chemin in ("aa/bb", "cc/dd"):
+            self.pose_fichier(chemin)
+        fs.attachments = lambda base: [
+            piece("aa/bb", "res.partner", "image", size=1024, pid="5"),
+            piece("cc/dd", "res.partner", "image", size=1024, pid="6"),
+        ]
+        self.assertEqual(fs.audit("db")["dead_kept_size"], 2048)
+
+    def test_the_report_shows_the_weight(self):
+        gardees = [piece("a/1", "res.partner", "image", size=2048)]
+        texte = "\n".join(
+            fs.render_dead_kept({"dead_kept": gardees, "dead_kept_size": 2048})
+        )
+        self.assertIn("2 ko", texte)
+        self.assertIn("res.partner / image", texte)
+
+    def test_nothing_kept_says_nothing(self):
+        self.assertEqual(fs.render_dead_kept({"dead_kept": []}), [])
+
+    def test_a_living_field_is_never_counted(self):
+        self.assertFalse(
+            fs.is_dead_field(
+                piece("a/1", "res.partner", "image_1920"),
+                {"res.partner.image_1920"},
+            )
+        )
+
+    def test_an_uploaded_document_has_no_field_so_is_never_dead(self):
+        self.assertFalse(fs.is_dead_field(piece("a/1", "project.task"), set()))
+
+
+class TestTheNestedCountIsPerDatabase(unittest.TestCase):
+    """« 1168 fichiers échoués » devant une base qu'on vient de ranger.
+
+    Le compte agrégeait tous les filestores de la machine : on rangeait,
+    le rapport affichait le même chiffre, et l'on rangeait à nouveau.
+    """
+
+    def setUp(self):
+        self.vrais = (
+            fs.attachments,
+            fs.live_fields,
+            fs.scan_filestores,
+            fs.scan_backups,
+            fs.filestore_root,
+        )
+        fs.attachments = lambda base: []
+        fs.live_fields = lambda base: set()
+        fs.scan_backups = lambda dossier: {}
+        fs.filestore_root = lambda config=None: "/nulle/part"
+        fs.scan_filestores = lambda racine, sauf=None: (
+            {},
+            {"a/1": "ma_base", "b/2": "voisine", "c/3": "voisine"},
+            {"ma_base": 1, "voisine": 2},
+        )
+
+    def tearDown(self):
+        (
+            fs.attachments,
+            fs.live_fields,
+            fs.scan_filestores,
+            fs.scan_backups,
+            fs.filestore_root,
+        ) = self.vrais
+
+    def test_only_this_database_counts_as_nested(self):
+        rapport = fs.audit("ma_base")
+        self.assertEqual(rapport["nested_total"], 1)
+        self.assertEqual(rapport["nested_elsewhere"], 2)
+
+    def test_a_tidy_database_is_not_told_it_has_work(self):
+        fs.scan_filestores = lambda racine, sauf=None: (
+            {},
+            {"b/2": "voisine"},
+            {"voisine": 1},
+        )
+        rapport = fs.audit("ma_base")
+        texte = "\n".join(fs.render_nested(rapport))
+        self.assertNotIn(
+            todo_i18n.t("file(s) sit in a nested filestore Odoo never reads."),
+            texte,
+        )
+        self.assertIn(
+            todo_i18n.t("such file(s) sit in OTHER databases filestores."),
+            texte,
+        )
+
+
+class TestTheMigrationWiring(unittest.TestCase):
+    RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), ".."))
+
+    def source(self, chemin):
+        with io.open(
+            os.path.join(self.RACINE, chemin), encoding="utf-8"
+        ) as handle:
+            return handle.read()
+
+    def test_the_purge_runs_ONCE_at_the_end_not_between_bumps(self):
+        # Entre deux paliers, deux à onze champs disparaissent puis
+        # REVIENNENT : purger là trancherait sur du transitoire.
+        src = self.source("script/todo/todo_upgrade.py")
+        self.assertIn("prompt_purge_dead_attachments", src)
+        appel = "self.prompt_purge_dead_attachments(database_name_upgrade)"
+        self.assertEqual(src.count(appel), 1)
+        boucle = src.index("for index, next_version in enumerate(")
+        self.assertGreater(
+            src.index(appel), boucle, "l'appel doit suivre la boucle"
+        )
+        etape = src.index('"5 - Cleaning up database after upgrade"')
+        self.assertGreater(src.index(appel), etape)
+
+    def test_the_purge_runs_BEFORE_the_final_backup(self):
+        # Qui veut garder l'état d'avant refuse la purge ; la sauvegarde
+        # qui suit doit capturer l'état nettoyé.
+        src = self.source("script/todo/todo_upgrade.py")
+        appel = "self.prompt_purge_dead_attachments(database_name_upgrade)"
+        self.assertLess(src.index(appel), src.index("cmd_backup_template"))
+
+    def test_the_restore_offers_to_tidy_where_the_fault_is_born(self):
+        # L'APPEL, pas le nom : `pass` à sa place laisse la fonction
+        # définie et le test passerait sur du code mort.
+        src = self.source("script/database/db_restore.py")
+        self.assertIn("tidy_nested_plan", src)
+        self.assertIn(
+            'if rapport.get("nested"):\n        offer_tidy(',
+            src,
+            "le rangement n'est plus proposé après la vérification",
+        )
+
+    def test_the_restore_never_asks_without_a_terminal(self):
+        # Ce script tourne aussi sans personne devant : une question
+        # posée à un stdin fermé arrêterait la migration.
+        src = self.source("script/database/db_restore.py")
+        debut = src.index("def offer_tidy")
+        fin = src.index("input(", debut)
+        self.assertIn("sys.stdin.isatty()", src[debut:fin])
+
+    def test_the_clone_path_still_offers_nothing(self):
+        src = self.source("script/database/db_restore.py")
+        debut = src.index("--clone --from_database")
+        fin = src.index("verify_filestore(config.database", debut)
+        self.assertNotIn("offer_tidy", src[debut:fin])
+
+
 class TestVerifyingARestore(unittest.TestCase):
     """Le contrôle d'après-restauration, celui qui aurait vu le nichage."""