From f1f573862da19c6c7ef7f3991d1a861a821bbf46 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Wed, 12 Aug 2026 16:49:58 -0400 Subject: [PATCH] geenforce: order backup revisions in Python, not in MySQL The reports table 500'd on every load: the backup lookup ordered with ORDER BY lastseenat DESC NULLS LAST, which SQLite accepts and MySQL rejects outright. Every test passed and the real database refused the query - the tests run on SQLite, so the dialect difference was invisible. Sorting in Python removes the dependency for nothing: the rows are one per host per kind. The regression test pins which revision wins, including that one never confirmed does not, and says why the sort lives here so it does not get helpfully moved back into SQL. --- plugins/geenforce/api/routes.py | 15 ++++++- .../test_geenforce_publish_gate.py | 42 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 0473502..57be038 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -11,6 +11,7 @@ Two audiences: import ipaddress import os import time +from datetime import datetime from functools import wraps from flask import Blueprint, request, Response, send_file, current_app, g @@ -24,6 +25,9 @@ from shopdb.api import ( SHAREROOT_SETTING = 'geenforce_share_root' +# Sort floor for revisions with no lastseenat (they sort last regardless). +_EPOCH = datetime.min + from ..models import ( ManifestScope, ManifestEntry, ManifestPublishedVersion, ManifestEnforcementReport, ManifestEnforcementResult, ManifestPayload, @@ -1037,12 +1041,19 @@ def _attach_backup_state(facts, hostnames): return rows = (BackupRevision.query .filter(BackupRevision.sourcehostname.in_(hostnames)) - .order_by(BackupRevision.lastseenat.desc().nullslast()) .all()) + # Newest confirmed first, ordered HERE rather than in SQL: putting nulls + # last needs ORDER BY ... NULLS LAST, which SQLite accepts and MySQL + # rejects outright - so the query passed every test and 500'd on the real + # database. Row counts here are one per host per kind, so sorting in Python + # costs nothing. + rows.sort(key=lambda revision: (revision.lastseenat is not None, + revision.lastseenat or _EPOCH), + reverse=True) for revision in rows: key = (revision.sourcehostname or '').lower() entry = facts.get(key) - # First row per host wins: ordered newest-confirmed first. + # First row per host wins. if entry and entry['backuplastseen'] is None: entry['backupkind'] = revision.backupkind entry['backuplastseen'] = ( diff --git a/tests/test_plugins/test_geenforce_publish_gate.py b/tests/test_plugins/test_geenforce_publish_gate.py index a1b77e5..354f0ce 100644 --- a/tests/test_plugins/test_geenforce_publish_gate.py +++ b/tests/test_plugins/test_geenforce_publish_gate.py @@ -151,3 +151,45 @@ def test_hosts_below_libversion_reports_the_floor(db): 'gea-shopfloor-cmm', 'runtime', '2.6') assert hosts == ['A', 'B'] assert floor == '2.3' + + +# -- backup state on the reports table --------------------------------------- + +def test_backup_state_prefers_the_newest_confirmed_revision(db): + """A host with several revisions shows the most recently CONFIRMED one, and + a revision that has never been confirmed (lastseenat NULL) never wins. + + Ordering happens in Python on purpose. Doing it in SQL needs + ORDER BY ... NULLS LAST, which SQLite accepts and MySQL rejects outright - + so it passed every test here and returned a 500 against the real database. + """ + from datetime import timedelta + + from plugins.backups.models import BackupRevision + from plugins.geenforce.api.routes import _attach_backup_state + from shopdb.api import Asset, AssetType + + assettype = AssetType.query.first() + if not assettype: + assettype = AssetType(assettype='computer') + db.session.add(assettype) + db.session.flush() + asset = Asset(assetnumber='PC-1', name='PC-1', + assettypeid=assettype.assettypeid) + db.session.add(asset) + db.session.flush() + + now = datetime.now(timezone.utc).replace(tzinfo=None) + for kind, lastseen in (('ntlars', now - timedelta(days=9)), + ('udc', now), + ('files', None)): + db.session.add(BackupRevision( + assetid=asset.assetid, backupkind=kind, contenthash=kind * 8, + sourcehostname='WJPC01', lastseenat=lastseen, createdat=now)) + db.session.flush() + + facts = {'wjpc01': {'backupkind': None, 'backuplastseen': None}} + _attach_backup_state(facts, {'WJPC01'}) + + assert facts['wjpc01']['backupkind'] == 'udc' + assert facts['wjpc01']['backuplastseen'] is not None