diff --git a/script/analyse/check_migration_quality.py b/script/analyse/check_migration_quality.py index ff08063..bfdb363 100755 --- a/script/analyse/check_migration_quality.py +++ b/script/analyse/check_migration_quality.py @@ -226,9 +226,113 @@ def inspect(database): ) absents = missing_files(database, [ligne[0] for ligne in stockees or []]) etat["attachment_missing"] = None if absents is None else len(absents) + # Les NOMS, pas seulement le compte : sans eux, « 254 fichiers + # absents » ne dit pas lesquels, et l'on ne peut ni juger de la + # gravité ni retrouver ce qui a disparu. Bornés, car une base peut en + # aligner des dizaines de milliers et l'écran n'en montrera jamais tant. + etat["attachment_missing_list"] = (absents or [])[:MAX_MISSING] return etat +MAX_MISSING = 5000 + + +def missing_detail(database, lst_store_fname, limit=400): + """Les métadonnées des pièces jointes dont le fichier a disparu. + + À la DEMANDE, jamais pendant l'inspection : une requête de plus par + base allongerait un parcours qui tient aujourd'hui en quatre secondes, + pour une information qu'on ne regarde qu'en la demandant. + + `res_field` est le renseignement le plus utile du lot : il dit QUEL + champ a perdu son image — l'`image_1920` d'un pays n'a pas le même + poids qu'une pièce jointe de facture. + """ + if not lst_store_fname: + return [] + lst = list(lst_store_fname)[:limit] + valeurs = ", ".join("'" + nom.replace("'", "''") + "'" for nom in lst) + lignes = run_psql( + database, + "SELECT store_fname, coalesce(res_model, '-')," + " coalesce(res_field, '-'), coalesce(res_id::text, '-')," + " coalesce(mimetype, '-'), coalesce(file_size::text, '0')," + " coalesce(name, '-')" + f" FROM ir_attachment WHERE store_fname IN ({valeurs})" + " ORDER BY res_model, res_field, name", + ) + return [ + { + "store_fname": ligne[0], + "model": ligne[1], + "field": ligne[2], + "res_id": ligne[3], + "mimetype": ligne[4], + "size": int(ligne[5]) if ligne[5].isdigit() else 0, + "name": ligne[6], + } + for ligne in lignes or [] + if len(ligne) >= 7 + ] + + +def render_missing(etat, colour=False, limit=60): + """Ce qui manque, groupé d'abord, détaillé ensuite. + + Le groupement d'abord parce qu'il tranche : deux cent trente-trois + drapeaux de pays et treize logos de fournisseurs de paiement sont des + images livrées par les modules, qu'une mise à jour restaure. Huit + pièces jointes éparses, non. La liste brute mettait les deux sur le + même plan. + """ + from script.todo.migration_status import paint + + absents = etat.get("attachment_missing") or 0 + if not absents: + return f"✅ {t('every attachment file is present')}" + lignes = [ + paint( + f"❌ {absents}" + f" {t('attachment files missing from the filestore')}", + "fail", + colour, + ), + f" {etat['database']}", + "", + ] + detail = missing_detail( + etat["database"], etat.get("attachment_missing_list") or [] + ) + if not detail: + lignes.append(t("Could not read their metadata.")) + return "\n".join(lignes) + + groupe = {} + for item in detail: + cle = (item["model"], item["field"], item["mimetype"]) + groupe[cle] = groupe.get(cle, 0) + 1 + lignes.append(f"── {t('by model and field')} ──") + for (modele, champ, mime), nombre in sorted( + groupe.items(), key=lambda x: -x[1] + ): + lignes.append( + f" {nombre:>5} {paint(modele, 'step', colour):<34}" + f" {champ:<22} {mime}" + ) + lignes.append("") + lignes.append(f"── {t('one by one')} ──") + for item in detail[:limit]: + lignes.append( + f" {item['model']}#{item['res_id']}" + f" {paint(item['field'], 'dim', colour)}" + f" {item['size']:>8} o {item['name'][:44]}" + ) + lignes.append(f" {paint(item['store_fname'], 'cmd', colour)}") + if len(detail) > limit: + lignes.append(f" … {len(detail) - limit} {t('more')}") + return "\n".join(lignes) + + def compare(avant, apres): """Ce qui a été gagné et ce qui a été perdu entre deux paliers.""" if not avant.get("exists") or not apres.get("exists"): diff --git a/script/analyse/check_migration_quality_tui.py b/script/analyse/check_migration_quality_tui.py index dd8d5a9..fee2360 100644 --- a/script/analyse/check_migration_quality_tui.py +++ b/script/analyse/check_migration_quality_tui.py @@ -64,6 +64,9 @@ def rows(lst_snapshot): if rang: precedent = presents[rang - 1] diff = quality.compare(precedent, etat) if precedent else None + # Le précédent voyage avec la ligne : le panneau en a besoin pour + # écrire « 171 (−43) », et le recalculer là-bas ferait deux + # sources pour la même comparaison. perdu = ( 0 if diff is None or diff.get("unavailable") @@ -76,6 +79,7 @@ def rows(lst_snapshot): "detail": str(perdu) if perdu else "", "data": etat, "diff": diff, + "previous": precedent, } ) if len(presents) >= 2: @@ -107,34 +111,87 @@ def head_text(lst_snapshot): ) -def pane_text(lst_snapshot, row, colour=False): +def statistics(etat, precedent): + """[(libellé, valeur, écart)] pour un palier. L'écart est None au départ. + + Un chiffre seul ne dit rien : 2283 vues est un nombre, « +61 » est une + information. Le premier palier n'a pas de précédent, et inventer un + écart de zéro y laisserait croire à une comparaison qui n'existe pas. + """ + lst = [ + ( + t("modules"), + len(etat["installed"]), + None if not precedent else len(precedent["installed"]), + ), + ( + t("models"), + len(etat["model"]), + None if not precedent else len(precedent["model"]), + ), + ( + t("views"), + etat["view"], + None if not precedent else precedent["view"], + ), + ( + " · COW", + etat["view_cow"], + None if not precedent else precedent["view_cow"], + ), + ( + t("menus"), + etat["menu"], + None if not precedent else precedent["menu"], + ), + ( + t("attachments"), + etat["attachment"], + None if not precedent else precedent["attachment"], + ), + ] + return [ + (libelle, valeur, None if avant is None else valeur - avant) + for libelle, valeur, avant in lst + ] + + +def pane_text(lst_snapshot, row, colour=False, show_missing=False): """Le détail du palier choisi, ou le bilan d'ensemble.""" if row is None: return t("Nothing to show yet.") if row["kind"] == "missing": return f"⚠️ {row['data']['database']} : {t('database not found')}" - lignes = [] etat = row["data"] + if show_missing: + if not etat: + return t("Pick a step to see its missing files.") + return quality.render_missing(etat, colour) + lignes = [] if etat: lignes.append( status.paint(f"{etat['odoo']} {etat['database']}", "step", colour) ) lignes.append("") - for libelle, valeur in ( - (t("modules"), len(etat["installed"])), - (t("models"), len(etat["model"])), - (t("views"), etat["view"]), - (" · COW", etat["view_cow"]), - (t("menus"), etat["menu"]), - (t("attachments"), etat["attachment"]), - ): - lignes.append(f" {libelle:<28} {valeur:>7}") + for libelle, valeur, ecart in statistics(etat, row.get("previous")): + if ecart is None: + marque = "" + else: + marque = status.paint( + f"{ecart:+d}", "ok" if ecart >= 0 else "warn", colour + ) + lignes.append(f" {libelle:<28} {valeur:>7} {marque}") if etat.get("attachment_missing"): lignes.append( - f" {status.paint('❌ ' + t('attachment files missing from' - ' the filestore'), 'fail', colour)}" - f" {etat['attachment_missing']}" + " " + + status.paint( + f"❌ {etat['attachment_missing']}" + f" {t('attachment files missing from the filestore')}", + "fail", + colour, + ) ) + lignes.append(f" {t('press m to list them')}") lignes.append("") diff = row.get("diff") if diff: @@ -151,13 +208,17 @@ def build_app(lst_snapshot): class QualityApp(App): CSS = globals()["CSS"] - BINDINGS = [("q,escape", "quit", t("Quit"))] + BINDINGS = [ + ("q,escape", "quit", t("Quit")), + ("m", "toggle_missing", t("Missing files")), + ] def __init__(self, lst_snapshot): super().__init__() self.lst_snapshot = lst_snapshot self.lst_row = rows(lst_snapshot) self.index = 0 + self.show_missing = False def compose(self) -> ComposeResult: yield Header() @@ -186,9 +247,26 @@ def build_app(lst_snapshot): else None ) self.query_one("#content", Static).update( - Text.from_ansi(pane_text(self.lst_snapshot, row, colour=True)) + Text.from_ansi( + pane_text( + self.lst_snapshot, + row, + colour=True, + show_missing=self.show_missing, + ) + ) ) + def action_toggle_missing(self): + """Basculer entre les chiffres du palier et ses fichiers absents. + + Les métadonnées ne sont lues qu'ICI : une requête de plus par + base allongerait un parcours qui tient en quatre secondes, pour + une information qu'on ne regarde qu'en la demandant. + """ + self.show_missing = not self.show_missing + self._show() + def on_data_table_row_highlighted(self, event): if event.data_table.id == "left" and self.lst_row: self.index = event.cursor_row diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 03f72e4..58198f3 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5755,6 +5755,34 @@ TRANSLATIONS = { "fr": "processus.", "en": "process instead.", }, + "Missing files": { + "fr": "Fichiers absents", + "en": "Missing files", + }, + "every attachment file is present": { + "fr": "tous les fichiers de pièces jointes sont présents", + "en": "every attachment file is present", + }, + "Could not read their metadata.": { + "fr": "Impossible de lire leurs métadonnées.", + "en": "Could not read their metadata.", + }, + "by model and field": { + "fr": "par modèle et par champ", + "en": "by model and field", + }, + "one by one": { + "fr": "un par un", + "en": "one by one", + }, + "press m to list them": { + "fr": "m pour en voir la liste", + "en": "press m to list them", + }, + "Pick a step to see its missing files.": { + "fr": "Choisissez un palier pour voir ses fichiers absents.", + "en": "Pick a step to see its missing files.", + }, "Clean the database before testing the pages?": { "fr": "Nettoyer la base avant de tester les pages ?", "en": "Clean the database before testing the pages?", diff --git a/test/test_check_migration_quality.py b/test/test_check_migration_quality.py index eb264e6..21a8c49 100644 --- a/test/test_check_migration_quality.py +++ b/test/test_check_migration_quality.py @@ -284,6 +284,234 @@ class TestTheReportItself(Base): self.assertLess(texte.index("12.0"), texte.index("From start")) +class TestTheStatisticsCarryTheirDelta(Base): + """Un chiffre seul ne dit rien. + + « 2283 vues » est un nombre ; « +61 » est une information. C'est + l'écart qu'on lit, pas la valeur. + """ + + def test_each_figure_gets_its_change(self): + lignes = qtui.statistics( + snapshot(view=2283, installed=["a"]), + snapshot(view=2222, installed=["a", "b"]), + ) + par_nom = {libelle: ecart for libelle, _v, ecart in lignes} + self.assertEqual(par_nom["views"], 61) + self.assertEqual(par_nom["modules"], -1) + + def test_the_first_step_has_NO_delta(self): + # Inventer un écart de zéro laisserait croire à une comparaison + # qui n'existe pas : il n'y a rien avant le premier palier. + lignes = qtui.statistics(snapshot(), None) + self.assertTrue(all(ecart is None for _l, _v, ecart in lignes)) + + def test_an_unchanged_figure_shows_zero_not_nothing(self): + # Zéro est une réponse : « rien n'a bougé » se distingue de « on + # ne sait pas ». + lignes = qtui.statistics(snapshot(view=100), snapshot(view=100)) + par_nom = {libelle: ecart for libelle, _v, ecart in lignes} + self.assertEqual(par_nom["views"], 0) + + def test_every_figure_of_the_pane_is_covered(self): + libelles = [ + libelle + for libelle, _v, _e in qtui.statistics(snapshot(), snapshot()) + ] + for attendu in ("modules", "models", "views", "menus", "attachments"): + self.assertIn(attendu, libelles) + + def test_the_pane_prints_the_sign(self): + row = { + "kind": "step", + "data": snapshot(view=2283), + "previous": snapshot(view=2222), + "diff": None, + } + texte = qtui.pane_text([], row) + self.assertIn("+61", texte) + + +class TestListingTheMissingFiles(Base): + """« 254 fichiers absents » ne dit pas lesquels. + + Le groupement tranche : deux cent trente-quatre drapeaux de pays sont + des images livrées par un module, qu'une mise à jour restaure. Une + pièce jointe d'événement de 255 ko, non. La liste brute mettait les + deux sur le même plan. + """ + + DETAIL = [ + { + "store_fname": "aa/1", + "model": "res.country", + "field": "image", + "res_id": "1", + "mimetype": "image/png", + "size": 100, + "name": "fr", + }, + { + "store_fname": "aa/2", + "model": "res.country", + "field": "image", + "res_id": "2", + "mimetype": "image/png", + "size": 100, + "name": "ca", + }, + { + "store_fname": "aa/4", + "model": "res.country", + "field": "image_128", + "res_id": "1", + "mimetype": "image/png", + "size": 50, + "name": "fr128", + }, + { + "store_fname": "bb/3", + "model": "calendar.event", + "field": "-", + "res_id": "1", + "mimetype": "image/png", + "size": 255603, + "name": "photo.png", + }, + ] + + def render(self, etat=None, **kw): + original = quality.missing_detail + quality.missing_detail = lambda db, lst, limit=400: self.DETAIL + self.addCleanup(setattr, quality, "missing_detail", original) + return quality.render_missing( + etat + or snapshot( + attachment_missing=3, + attachment_missing_list=["aa/1", "aa/2", "bb/3"], + ), + **kw, + ) + + def test_nothing_missing_says_so_plainly(self): + texte = quality.render_missing(snapshot(attachment_missing=0)) + self.assertIn("every attachment file is present", texte) + + def test_the_grouping_comes_first(self): + texte = self.render() + self.assertLess( + texte.index("by model and field"), texte.index("one by one") + ) + + def test_the_grouping_counts_by_model_AND_field(self): + """Le champ est le renseignement le plus utile du lot. + + Il dit QUEL champ a perdu son image : l'`image_1920` d'un pays n'a + pas le même poids qu'une pièce jointe de facture. Grouper sur le + seul modèle fondrait `image` et `image_128` en une ligne, et l'on + perdrait exactement ce qu'on venait chercher. + """ + entete = self.render().split("one by one")[0] + lignes = [ + ligne for ligne in entete.splitlines() if "res.country" in ligne + ] + self.assertEqual(len(lignes), 2, entete) + self.assertTrue(any("image_128" in ligne for ligne in lignes)) + + def test_the_loudest_group_comes_first(self): + texte = self.render().split("one by one")[0] + self.assertLess(texte.index("res.country"), texte.index("calendar")) + + def test_each_file_names_its_record_and_its_path(self): + texte = self.render() + self.assertIn("calendar.event#1", texte) + self.assertIn("bb/3", texte) + self.assertIn("photo.png", texte) + + def test_a_long_list_is_cut_and_SAYS_so(self): + texte = self.render(limit=1) + self.assertIn("more", texte) + + def test_unreadable_metadata_is_admitted(self): + original = quality.missing_detail + quality.missing_detail = lambda db, lst, limit=400: [] + self.addCleanup(setattr, quality, "missing_detail", original) + texte = quality.render_missing( + snapshot(attachment_missing=3, attachment_missing_list=["a"]) + ) + self.assertIn("Could not read", texte) + + def test_the_metadata_is_read_ONLY_on_demand(self): + # Une requête de plus par base allongerait un parcours qui tient + # en quatre secondes, pour ce qu'on ne regarde qu'en le demandant. + import inspect + + self.assertNotIn("missing_detail", inspect.getsource(quality.inspect)) + self.assertIn( + "missing_detail", inspect.getsource(quality.render_missing) + ) + + def test_the_names_are_kept_but_bounded(self): + # Une base peut en aligner des dizaines de milliers ; l'écran n'en + # montrera jamais tant, et les garder toutes coûterait pour rien. + import inspect + + source = inspect.getsource(quality.inspect) + self.assertIn("attachment_missing_list", source) + self.assertIn("MAX_MISSING", source) + + def test_a_quote_in_a_filename_cannot_break_the_query(self): + vu = {} + original = quality.run_psql + quality.run_psql = lambda db, sql: vu.setdefault("sql", sql) and [] + self.addCleanup(setattr, quality, "run_psql", original) + quality.missing_detail("db", ["aa/o'brien"]) + self.assertIn("o''brien", vu["sql"]) + + +class TestTheMissingFilesButton(Base): + def test_m_is_bound(self): + app = qtui.build_app([snapshot()]) + touches = { + touche + for entree in app.BINDINGS + for touche in entree[0].split(",") + } + self.assertIn("m", touches) + + def test_the_action_exists(self): + app = qtui.build_app([snapshot()]) + self.assertTrue(hasattr(app, "action_toggle_missing")) + + def test_the_pane_switches_to_the_list(self): + original = quality.missing_detail + quality.missing_detail = lambda db, lst, limit=400: [] + self.addCleanup(setattr, quality, "missing_detail", original) + row = { + "kind": "step", + "data": snapshot( + attachment_missing=3, attachment_missing_list=["a"] + ), + "previous": None, + "diff": None, + } + chiffres = qtui.pane_text([], row, show_missing=False) + liste = qtui.pane_text([], row, show_missing=True) + self.assertIn("modules", chiffres) + self.assertNotIn("modules", liste) + + def test_the_pane_tells_you_the_key_exists(self): + # Une touche que rien n'annonce est une touche que personne ne + # presse. + row = { + "kind": "step", + "data": snapshot(attachment_missing=254), + "previous": None, + "diff": None, + } + self.assertIn("press m", qtui.pane_text([], row)) + + class TestItNeverWrites(Base): """Les bases de palier sont parfois la seule copie d'un état."""