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

@@ -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'] = (

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