From d6a088e92699f7fb9838ff55136a6c1606c05b67 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Fri, 21 Aug 2026 22:14:48 -0400 Subject: [PATCH] [ADD] db_restore: check the filestore landed, and open the tool from the menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Odoo's shutil.move renames when the destination is absent and NESTS when it exists, so a leftover filestore// sends a whole backup into filestore//filestore/, where Odoo never looks. That happened once here and the clone copied it into all seven databases of the chain -- 1168 files, 133 MB each, and nothing said a word. The check runs after a real restore only. A clone copies its source as it stands, faults included: checking the mirror would say the same thing twice, and in the wrong place. It warns and names the fix rather than aborting -- the database is restored and usable, it is the layout that is wrong. --- FR --- Le shutil.move d'Odoo renomme quand la destination est absente et IMBRIQUE quand elle existe : un filestore// resté là envoie toute une sauvegarde dans filestore//filestore/, où Odoo ne regarde jamais. C'est arrivé une fois ici et le clone l'a recopié dans les sept bases de la chaîne -- 1168 fichiers, 133 Mo chacune, sans un mot. Le contrôle ne suit qu'une vraie restauration. Un clone recopie sa source telle quelle, défauts compris : contrôler le miroir dirait deux fois la même chose, au mauvais endroit. Il avertit et nomme la correction plutôt que d'interrompre -- la base est restaurée et utilisable, c'est la disposition qui cloche. Assisted-by: Claude Opus 5 --- script/analyse/check_filestore.py | 96 ++++++++++++++++++++++ script/database/db_restore.py | 107 ++++++++++++++++-------- script/todo/todo.py | 47 +++++++++++ test/test_check_filestore.py | 132 ++++++++++++++++++++++++++++++ 4 files changed, 348 insertions(+), 34 deletions(-) diff --git a/script/analyse/check_filestore.py b/script/analyse/check_filestore.py index 903127d..dc01984 100755 --- a/script/analyse/check_filestore.py +++ b/script/analyse/check_filestore.py @@ -221,6 +221,102 @@ def scan_backups(dossier): return trouves +def files_on_disk(racine, database): + """(au bon niveau, nichés) pour une base. Deux ensembles de noms.""" + base = os.path.join(racine, database) + bons, niches = set(), set() + if not os.path.isdir(base): + return bons, niches + for prefixe in os.listdir(base): + sous = os.path.join(base, prefixe) + if not os.path.isdir(sous): + continue + if prefixe == "filestore": + for deux in os.listdir(sous): + profond = os.path.join(sous, deux) + if not os.path.isdir(profond): + continue + for nom in os.listdir(profond): + if os.path.isfile(os.path.join(profond, nom)): + niches.add(f"{deux}/{nom}") + continue + for nom in os.listdir(sous): + if os.path.isfile(os.path.join(sous, nom)): + bons.add(f"{prefixe}/{nom}") + return bons, niches + + +def verify_restore(database, zip_path, config_path=None): + """La sauvegarde a-t-elle bien atterri ? À vérifier UNE fois. + + Après un clone, il n'y a rien à contrôler : `copytree` recopie la + source telle quelle, défauts compris — le contrôle appartient à la + restauration d'origine, pas au miroir. + + Ce qu'on cherche est précis. `shutil.move` d'Odoo renomme quand la + destination n'existe pas et IMBRIQUE quand elle existe. Un dossier + `filestore//` laissé par une restauration précédente suffit + donc à envoyer toute la sauvegarde dans + `filestore//filestore/`, où Odoo ne regardera jamais. Mesuré : + 1168 fichiers, 133 Mo, recopiés ensuite dans les six bases de la + chaîne par le clone, sans que rien ne le signale. + """ + attendus = set(scan_zip(zip_path)) + racine = filestore_root(config_path) + bons, niches = files_on_disk(racine, database) + return { + "database": database, + "zip": os.path.basename(zip_path), + "expected": len(attendus), + "placed": len(attendus & bons), + "nested": len(attendus & niches), + "missing": len(attendus - bons - niches), + "root": os.path.join(racine, database), + } + + +def scan_zip(chemin): + """Les noms de fichiers du `filestore/` d'une sauvegarde.""" + try: + with zipfile.ZipFile(chemin) as archive: + return [ + membre[len("filestore/") :] + for membre in archive.namelist() + if membre.startswith("filestore/") and not membre.endswith("/") + ] + except (OSError, zipfile.BadZipFile): + return [] + + +def render_verify(rapport): + """Se taire quand tout va bien : un contrôle bavard finit ignoré.""" + if not rapport["expected"]: + return [] + if not rapport["nested"] and not rapport["missing"]: + return [ + f"✅ {t('Filestore restored:')} {rapport['placed']}" + f"/{rapport['expected']} {t('file(s) in place')}" + ] + lignes = [ + f"⚠ {t('Filestore restore looks wrong for')} {rapport['database']}" + f" ({rapport['zip']}) :", + f" {rapport['placed']}/{rapport['expected']}" + f" {t('file(s) in place')}", + ] + if rapport["nested"]: + lignes.append( + f" {rapport['nested']}" + f" {t('landed in a nested filestore Odoo never reads')}" + ) + lignes.append( + f" {t('To fix:')} rsync -a --remove-source-files" + f" {rapport['root']}/filestore/ {rapport['root']}/" + ) + if rapport["missing"]: + lignes.append(f" {rapport['missing']} {t('never landed at all')}") + return lignes + + def classify(piece, present, ailleurs, niches, sauvegardes, champs_vivants): """Le verdict d'une pièce jointe. None si son fichier est là. diff --git a/script/database/db_restore.py b/script/database/db_restore.py index e377394..4442d63 100755 --- a/script/database/db_restore.py +++ b/script/database/db_restore.py @@ -10,6 +10,10 @@ import os import sys from subprocess import check_output +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..")) +) + logging.basicConfig(level=os.environ.get("LOGLEVEL", "INFO")) _logger = logging.getLogger(__name__) @@ -86,6 +90,74 @@ def get_list_db_cache(arg_base): return lst_db, lst_db_cache +def verify_filestore(database, image): + """Contrôler qu'une restauration a bien posé ses fichiers. + + Une seule fois, à la restauration d'origine. Après un clone il n'y a + rien à vérifier : `copytree` recopie la source telle quelle, défauts + compris — le contrôle appartient à ce qui a créé le défaut, pas à ce + qui l'a dupliqué. + + Le contrôle n'interrompt pas : la base est restaurée et utilisable, + c'est la DISPOSITION des fichiers qui est suspecte. Refuser ici + casserait des chaînes qui marchent, pour un défaut qui se répare + d'une commande. + """ + chemin = os.path.join("image_db", f"{image}.zip") + if not os.path.isfile(chemin): + return + try: + from script.analyse import check_filestore + except Exception: # pragma: no cover - l'outil d'analyse est optionnel + return + rapport = check_filestore.verify_restore(database, chemin) + for ligne in check_filestore.render_verify(rapport): + print(ligne) + + +def restore_or_clone(config, arg_base, cache_database, lst_db_cache): + """Restaurer depuis l'image, ou cloner le cache déjà restauré. + + Le contrôle du filestore ne suit QUE les vraies restaurations. Le + clone recopie sa source telle quelle : contrôler le miroir dirait + deux fois la même chose, et la seconde au mauvais endroit. + """ + if cache_database not in lst_db_cache and not config.ignore_cache: + _logger.info( + f"## Create cache {cache_database} from image {config.image} ##" + ) + arg = ( + f"{arg_base} --restore" + f" --restore_image {config.image} --database {cache_database}" + ) + print(check_output(arg.split(" ")).decode()) + verify_filestore(cache_database, config.image) + + if config.ignore_cache: + _logger.info( + f"## Restoring {config.image} to database {config.database} ##" + ) + arg = ( + f"{arg_base} --restore --restore_image" + f" {config.image} --database {config.database}" + ) + else: + _logger.info( + f"## Clone cache {cache_database} to database" + f" {config.database} ##" + ) + arg = ( + f"{arg_base} --clone --from_database" + f" {cache_database} --database {config.database}" + ) + if config.neutralize: + arg += " --neutralize" + print(arg) + print(check_output(arg.split(" ")).decode()) + if config.ignore_cache: + verify_filestore(config.database, config.image) + + def main(): config = get_config() @@ -139,40 +211,7 @@ def main(): print(out) if config.only_drop: return - # Check cache exist - if cache_database not in lst_db_cache and not config.ignore_cache: - _logger.info( - f"## Create cache {cache_database} from image" - f" {config.image} ##" - ) - arg = ( - f"{arg_base} --restore" - f" --restore_image {config.image} --database {cache_database}" - ) - out = check_output(arg.split(" ")).decode() - print(out) - # Clone database - if config.ignore_cache: - _logger.info( - f"## Restoring {config.image} to database {config.database} ##" - ) - arg = ( - f"{arg_base} --restore --restore_image" - f" {config.image} --database {config.database}" - ) - else: - _logger.info( - f"## Clone cache {cache_database} to database {config.database} ##" - ) - arg = ( - f"{arg_base} --clone --from_database" - f" {cache_database} --database {config.database}" - ) - if config.neutralize: - arg += " --neutralize" - print(arg) - out = check_output(arg.split(" ")).decode() - print(out) + restore_or_clone(config, arg_base, cache_database, lst_db_cache) if not config.clean_cache and not config.database: print("Nothing to do.") diff --git a/script/todo/todo.py b/script/todo/todo.py index 18c636c..d8549b8 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -10196,6 +10196,12 @@ class TODO: "Modules missing from the default package" ) }, + {"section": t("Files")}, + { + "prompt_description": t( + "Attachment files missing from the filestore" + ) + }, ] help_info = self.fill_help_info(choices) @@ -10214,6 +10220,8 @@ class TODO: self.execute_analyse_migration_quality() elif status == "5": self.execute_analyse_module_package() + elif status == "6": + self.execute_analyse_filestore() else: print(t("Command not found !")) @@ -10261,6 +10269,45 @@ class TODO: handler, ) + def execute_analyse_filestore(self): + """Ce qui manque au filestore, et ce qu'on peut encore récupérer. + + Pas d'option « sauvegarde .zip » : l'outil compare une BASE à son + filestore, et un zip porte les deux ensemble par construction — + il n'y a rien à y trouver. + """ + from script.analyse import check_filestore as filestore + + database = self._analyse_select_database() + if not database: + return + print(f"⧖ {t('Scanning filestores and backups…')}") + try: + rapport = filestore.audit(database) + except Exception as exc: + print(f"❌ {t('Analysis failed: ')}{exc}") + return + if rapport.get("unavailable"): + print(f"❌ {t('Cannot read the database: ')}{database}") + return + print("\n".join(filestore.render(rapport, limit=20))) + + def handler(rank): + if rank == 1: + print("\n".join(filestore.render(rapport, limit=0))) + else: + self._analyse_export_json( + rapport, os.path.basename(database), "filestore" + ) + + self._analyse_follow_up( + [ + {"prompt_description": t("Show every entry")}, + {"prompt_description": t("Export as JSON")}, + ], + handler, + ) + def _analyse_offer_install(self, database, rapport): """Proposer d'installer ce qui manque, quand c'est installable. diff --git a/test/test_check_filestore.py b/test/test_check_filestore.py index d00fdbe..b486f38 100644 --- a/test/test_check_filestore.py +++ b/test/test_check_filestore.py @@ -289,6 +289,138 @@ class TestTheReport(unittest.TestCase): self.assertIn("res.country / image × 2", resume) +class TestVerifyingARestore(unittest.TestCase): + """Le contrôle d'après-restauration, celui qui aurait vu le nichage.""" + + def setUp(self): + self.racine = tempfile.mkdtemp() + self.zip = os.path.join(self.racine, "sauv.zip") + with zipfile.ZipFile(self.zip, "w") as archive: + for nom in ("aa/un", "bb/deux", "cc/trois"): + archive.writestr(f"filestore/{nom}", "x") + archive.writestr("dump.sql", "x") + self.fs = os.path.join(self.racine, "fs") + self.vrai = fs.filestore_root + fs.filestore_root = lambda config=None: self.fs + + def tearDown(self): + fs.filestore_root = self.vrai + shutil.rmtree(self.racine) + + def pose(self, chemins): + for chemin in chemins: + complet = os.path.join(self.fs, "ma_base", 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_clean_restore_reports_everything_in_place(self): + self.pose(["aa/un", "bb/deux", "cc/trois"]) + r = fs.verify_restore("ma_base", self.zip) + self.assertEqual((r["expected"], r["placed"]), (3, 3)) + self.assertEqual((r["nested"], r["missing"]), (0, 0)) + + def test_a_nested_restore_is_caught(self): + # Le défaut exact qui a coûté 133 Mo par base, sept fois. + self.pose( + ["filestore/aa/un", "filestore/bb/deux", "filestore/cc/trois"] + ) + r = fs.verify_restore("ma_base", self.zip) + self.assertEqual(r["nested"], 3) + self.assertEqual(r["placed"], 0) + + def test_a_half_nested_restore_counts_both_sides(self): + self.pose(["aa/un", "filestore/bb/deux"]) + r = fs.verify_restore("ma_base", self.zip) + self.assertEqual((r["placed"], r["nested"], r["missing"]), (1, 1, 1)) + + def test_files_that_never_landed_are_counted(self): + self.pose(["aa/un"]) + self.assertEqual(fs.verify_restore("ma_base", self.zip)["missing"], 2) + + def test_a_zip_without_a_filestore_says_nothing(self): + # Se taire quand il n'y a rien à contrôler : un contrôle bavard + # à chaque restauration finit par ne plus être lu. + vide = os.path.join(self.racine, "vide.zip") + with zipfile.ZipFile(vide, "w") as archive: + archive.writestr("dump.sql", "x") + r = fs.verify_restore("ma_base", vide) + self.assertEqual(r["expected"], 0) + self.assertEqual(fs.render_verify(r), []) + + def test_the_fix_command_names_the_real_directory(self): + self.pose(["filestore/aa/un"]) + texte = "\n".join( + fs.render_verify(fs.verify_restore("ma_base", self.zip)) + ) + self.assertIn(os.path.join(self.fs, "ma_base"), texte) + self.assertIn(todo_i18n.t("To fix:"), texte) + + def test_a_clean_restore_still_says_so_briefly(self): + self.pose(["aa/un", "bb/deux", "cc/trois"]) + texte = "\n".join( + fs.render_verify(fs.verify_restore("ma_base", self.zip)) + ) + self.assertIn(todo_i18n.t("Filestore restored:"), texte) + self.assertNotIn(todo_i18n.t("To fix:"), texte) + + +class TestTheRestoreWiring(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_db_restore_checks_after_a_real_restore(self): + src = self.source("script/database/db_restore.py") + self.assertEqual(src.count("verify_filestore("), 3) + self.assertIn( + "if config.ignore_cache:\n verify_filestore(" + "config.database", + src, + "le contrôle d'après-restauration directe n'est plus gardé" + " par ignore_cache", + ) + self.assertIn( + "not config.ignore_cache:", + src, + "la création du cache n'est plus conditionnée", + ) + + def test_the_clone_path_is_NOT_checked(self): + # Le miroir recopie sa source, défauts compris : contrôler là + # dirait deux fois la même chose, et au mauvais endroit. + src = self.source("script/database/db_restore.py") + self.assertIn("--clone --from_database", src) + debut = src.index("--clone --from_database") + marque = "verify_filestore(config.database" + self.assertIn( + marque, src, "le contrôle d'après-restauration a disparu" + ) + fin = src.index(marque, debut) + self.assertNotIn("verify_filestore", src[debut:fin]) + + def test_the_analyse_menu_offers_the_tool(self): + src = self.source("script/todo/todo.py") + self.assertIn("Attachment files missing from the filestore", src) + self.assertIn("self.execute_analyse_filestore()", src) + self.assertIn("def execute_analyse_filestore", src) + + def test_the_menu_has_as_many_entries_as_branches(self): + src = self.source("script/todo/todo.py") + debut = src.index("def prompt_execute_analyse") + fin = src.index('print(t("Command not found !"))', debut) + bloc = src[debut:fin] + entrees = bloc.count('"prompt_description"') + branches = sum( + f'status == "{n}"' in bloc for n in range(1, entrees + 2) + ) + self.assertEqual(entrees, branches) + + class TestTheExitCodes(unittest.TestCase): def setUp(self): self.vrai = fs.audit