Release the database connection across Dell warranty lookups

Both Dell routes held a pooled connection open for the duration of the
vendor HTTP call. sync_dell was the severe case: bulk_lookup asks Dell
about 100 service tags per request and calls them sequentially, so a
fleet-sized sync pinned one connection for minutes. On gevent workers
that exhausts the pool and fails every request in the application, not
just warranty.

Copy the values the lookup needs into locals, commit to return the
connection, then reacquire for the writes. The locals are load-bearing:
a commit expires loaded attributes, so reading them afterwards would
quietly check a connection back out and restore the defect.

Preload the existing warranty links in one query. The write loop ran a
query per asset plus one per link, which would have traded the long
network hold for a long write transaction.

Transaction boundaries do not show up in behavioural tests, so the new
tests assert session state at the moment the provider is called. Both
were confirmed to fail against the unfixed code, including the
post-commit attribute-access variant that leaves all other tests green.

Single commit and the existing error handling are unchanged, so the
sync stays all-or-nothing.

plugins/warranty/api/routes.py

@@ -281,14 +281,27 @@ def refresh_warranty(warrantyid):
     warranty = db.session.get(Warranty, warrantyid)
     if not warranty:
         return error_response(ErrorCodes.NOT_FOUND, 'Warranty not found', http_code=404)
-    provider = get_provider(warranty.provider)
+    # Copy to locals and end the transaction before the vendor call: the lookup
+    # can block for the full HTTP timeout, and holding a pooled connection
+    # across it starves the pool under load. Reading ORM attributes after the
+    # commit would check a connection straight back out.
+    providername = warranty.provider
+    servicetag = warranty.servicetag
+    vendor = warranty.vendor
+    db.session.commit()
+
+    provider = get_provider(providername)
     try:
-        result = provider.lookup(warranty.servicetag, warranty.vendor)
+        result = provider.lookup(servicetag, vendor)
     except (ProviderNotConfigured, WarrantyLookupError) as exc:
         return error_response(ErrorCodes.VALIDATION_ERROR, str(exc), http_code=400)
     if not result:
         return error_response(ErrorCodes.VALIDATION_ERROR,
                               'This warranty is manual - nothing to refresh.', http_code=400)
+
+    warranty = db.session.get(Warranty, warrantyid)
+    if not warranty:
+        return error_response(ErrorCodes.NOT_FOUND, 'Warranty not found', http_code=404)
     if result.get('servicelevel'):
         warranty.servicelevel = result['servicelevel']
     if result.get('startdate'):
@@ -342,11 +355,31 @@ def sync_dell():
             'candidates': 0, 'matched': 0, 'created': 0, 'updated': 0,
         }, message='No PCs need a warranty lookup.')
 
+    # by_tag is plain strings and ints, so it survives the commit. Release the
+    # connection before bulk_lookup: Dell is called 100 tags at a time and a
+    # fleet-sized sync would otherwise pin a pooled connection for minutes.
+    db.session.commit()
+
     try:
         results = provider.bulk_lookup(list(by_tag.keys()))
     except (ProviderNotConfigured, WarrantyLookupError) as exc:
         return error_response(ErrorCodes.VALIDATION_ERROR, str(exc), http_code=400)
 
+    # Preload every warranty already linked to a candidate asset in one query.
+    # Done per asset this was a query per asset plus one per link, which would
+    # just trade the long network hold for a long write transaction.
+    matched_assetids = [assetid
+                        for tag, assetids in by_tag.items() if results.get(tag)
+                        for assetid in assetids]
+    existing_by_assetid = {}
+    if matched_assetids:
+        rows = (db.session.query(WarrantyAsset.assetid, Warranty)
+                .join(Warranty, Warranty.warrantyid == WarrantyAsset.warrantyid)
+                .filter(WarrantyAsset.assetid.in_(matched_assetids))
+                .all())
+        for assetid, warranty in rows:
+            existing_by_assetid.setdefault(assetid, []).append(warranty)
+
     created = updated = matched = 0
     now = datetime.now(timezone.utc).replace(tzinfo=None)
     for tag, assetids in by_tag.items():
@@ -357,15 +390,13 @@ def sync_dell():
         for assetid in assetids:
             # Reuse an existing Dell warranty for this asset if there is one.
             existing = None
-            for link in WarrantyAsset.query.filter_by(assetid=assetid).all():
-                candidate = db.session.get(Warranty, link.warrantyid)
+            for candidate in existing_by_assetid.get(assetid, []):
                 # Reuse a warranty that is Dell by ANY signal, not just
                 # provider: a manually-added or imported Dell warranty carries
                 # provider 'manual' (create default) but the same service tag or
                 # a "Dell" vendor. Matching only provider=='dell' made re-check
                 # duplicate every one of those instead of updating it.
-                if candidate and (
-                        candidate.provider == 'dell'
+                if (candidate.provider == 'dell'
                         or (candidate.servicetag or '').strip().upper() == tag
                         or (candidate.vendor or '').strip().lower() == 'dell'):
                     existing = candidate

tests/test_plugins/test_warranty_connection_release.py

@@ -0,0 +1,163 @@
+"""The Dell routes must not hold a database connection across the vendor call.
+
+A bulk sync asks Dell about 100 service tags per request and a fleet-sized run
+is dozens of those, so a route that keeps its transaction open for the duration
+pins a pooled connection for minutes. Under gevent workers that exhausts the
+pool and takes down every other request, not just warranty.
+
+These pin the boundary itself rather than the result: a fake provider inspects
+the session at the moment it is called and records whether a transaction was
+still open. Behaviour is covered elsewhere - what would silently regress here is
+someone reading an ORM attribute after the commit, which re-checks a connection
+straight back out and restores the original defect while every other test
+continues to pass.
+"""
+
+from plugins.warranty.api import routes as warranty_routes
+from plugins.warranty.models import Warranty, WarrantyAsset
+
+
+def _asset(db, assetnumber='PC-0001', serialnumber='ABC1234'):
+    from shopdb.core.models import AssetType, AssetStatus
+    from shopdb.core.models.asset import Asset
+
+    assettype = AssetType(assettype='computer')
+    status = AssetStatus(status='In Use')
+    db.session.add_all([assettype, status])
+    db.session.flush()
+
+    asset = Asset(
+        assetnumber=assetnumber,
+        name='Bench PC',
+        assettypeid=assettype.assettypeid,
+        statusid=status.statusid,
+        serialnumber=serialnumber,
+    )
+    db.session.add(asset)
+    db.session.commit()
+    return asset
+
+
+class _ProbeProvider:
+    """Stands in for DellProvider and records the session state when called.
+
+    The real provider blocks on HTTP here; what matters is only whether the
+    caller was still inside a transaction at that instant.
+    """
+
+    def __init__(self, db, result):
+        self._db = db
+        self._result = result
+        self.in_transaction_at_call = None
+        self.called = False
+
+    def _observe(self):
+        self.called = True
+        # db.session is a scoped_session proxy and does not forward
+        # in_transaction; calling it returns the real Session for this scope.
+        self.in_transaction_at_call = self._db.session().in_transaction()
+
+    def lookup(self, servicetag, vendor=None):
+        self._observe()
+        return self._result
+
+    def bulk_lookup(self, servicetags):
+        self._observe()
+        return {tag: self._result for tag in servicetags}
+
+
+def test_refresh_releases_the_connection_before_the_vendor_call(
+        client, db, auth_headers, monkeypatch):
+    warranty = Warranty(vendor='Dell', servicetag='ABC1234', provider='dell')
+    db.session.add(warranty)
+    db.session.commit()
+    warrantyid = warranty.warrantyid
+
+    probe = _ProbeProvider(db, {
+        'servicelevel': 'ProSupport',
+        'startdate': '2024-01-01',
+        'enddate': '2027-01-01',
+    })
+    monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe)
+
+    response = client.post(f'/api/warranty/{warrantyid}/refresh', headers=auth_headers)
+
+    assert response.status_code == 200, response.get_json()
+    assert probe.called
+    assert probe.in_transaction_at_call is False, (
+        'refresh_warranty held a database connection across the vendor lookup'
+    )
+    # The write half still has to land, or releasing early would be a
+    # regression dressed up as a fix.
+    refreshed = db.session.get(Warranty, warrantyid)
+    assert refreshed.servicelevel == 'ProSupport'
+    assert refreshed.lastcheckeddate is not None
+
+
+def test_sync_dell_releases_the_connection_before_the_bulk_call(
+        client, db, auth_headers, monkeypatch):
+    asset = _asset(db)
+    assetid = asset.assetid
+
+    probe = _ProbeProvider(db, {
+        'servicelevel': 'Basic Onsite',
+        'startdate': '2023-05-02',
+        'enddate': '2026-05-02',
+    })
+    monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe)
+
+    response = client.post('/api/warranty/sync/dell', headers=auth_headers)
+
+    assert response.status_code == 200, response.get_json()
+    assert probe.called
+    assert probe.in_transaction_at_call is False, (
+        'sync_dell held a database connection across the Dell bulk lookup'
+    )
+    body = response.get_json()['data']
+    assert body['created'] == 1
+    assert body['matched'] == 1
+
+    links = WarrantyAsset.query.filter_by(assetid=assetid).all()
+    assert len(links) == 1
+    created = db.session.get(Warranty, links[0].warrantyid)
+    assert created.provider == 'dell'
+    assert created.servicetag == 'ABC1234'
+
+
+def test_sync_dell_updates_instead_of_duplicating_on_a_recheck(
+        client, db, auth_headers, monkeypatch):
+    """Preloading the existing links must not lose the Dell-by-any-signal match.
+
+    The write loop now reads from one preloaded query instead of a query per
+    asset. A manually-added Dell warranty carries provider 'manual', so if the
+    preload missed it a re-check would create a second warranty for the asset
+    rather than updating the one already there.
+    """
+    asset = _asset(db)
+    assetid = asset.assetid
+
+    existing = Warranty(vendor='Dell', servicetag='ABC1234', provider='manual')
+    existing.links.append(WarrantyAsset(assetid=assetid))
+    db.session.add(existing)
+    db.session.commit()
+    existingid = existing.warrantyid
+
+    probe = _ProbeProvider(db, {
+        'servicelevel': 'ProSupport Plus',
+        'startdate': '2024-02-02',
+        'enddate': '2028-02-02',
+    })
+    monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe)
+
+    response = client.post('/api/warranty/sync/dell?all=true', headers=auth_headers)
+
+    assert response.status_code == 200, response.get_json()
+    body = response.get_json()['data']
+    assert body['created'] == 0
+    assert body['updated'] == 1
+
+    links = WarrantyAsset.query.filter_by(assetid=assetid).all()
+    assert len(links) == 1, 'a re-check duplicated the warranty instead of updating it'
+    updated = db.session.get(Warranty, existingid)
+    assert updated.provider == 'dell'
+    assert updated.servicelevel == 'ProSupport Plus'