diff --git a/plugins/warranty/api/routes.py b/plugins/warranty/api/routes.py index 8276cc66..2018a2e7 100644 --- a/plugins/warranty/api/routes.py +++ b/plugins/warranty/api/routes.py @@ -23,7 +23,10 @@ from shopdb.api import ( ) from ..models import Warranty, WarrantyAsset -from ..services import get_provider, ProviderNotConfigured, WarrantyLookupError +from ..services import ( + get_provider, ProviderNotConfigured, WarrantyLookupError, + run_dell_sync, ROUTE_SYNC_LIMIT, +) warranty_bp = Blueprint('warranty', __name__) @@ -320,118 +323,36 @@ def refresh_warranty(warrantyid): @warranty_bp.route('/sync/dell', methods=['POST']) @require_permission('warranty.edit') def sync_dell(): - """Look up every asset serial (= Dell service tag) that has no warranty yet + """Look up asset serials (= Dell service tags) that have no warranty yet and create warranties for the ones Dell recognizes. + Bounded to ROUTE_SYNC_LIMIT tags per call. Dell is queried 100 tags at a + time, so an unbounded fleet-sized sync is dozens of sequential HTTP calls + inside one request and a worker timeout or proxy kills it first. The + response carries `remaining` so the caller knows to run it again; nothing + drains it in the background. `flask warranty sync-dell` does the same work + from the command line for a large first-run backfill. + Only assets that lack a dated warranty are looked up, so re-running is cheap and non-Dell serials are filtered out by Dell (they return no coverage). """ - provider = get_provider('dell') # ?all=true re-checks assets that already have a dated warranty too. recheck_all = request.args.get('all', 'false').lower() == 'true' - # Assets already covered by a dated warranty - skipped unless recheck_all. - covered = set() - if not recheck_all: - for link in WarrantyAsset.query.all(): - w = db.session.get(Warranty, link.warrantyid) - if w and w.isactive and w.enddate: - covered.add(link.assetid) - - # Candidate assets: active, have a serial, not already covered. - candidates = (Asset.query - .filter(Asset.isactive.is_(True), - Asset.serialnumber.isnot(None), - Asset.serialnumber != '') - .all()) - by_tag = {} - for asset in candidates: - if asset.assetid in covered: - continue - by_tag.setdefault(asset.serialnumber.strip().upper(), []).append(asset.assetid) - - if not by_tag: - return success_response({ - '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())) + summary = run_dell_sync(limit=ROUTE_SYNC_LIMIT, recheck_all=recheck_all) 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) + if not summary['tags']: + return success_response(summary, message='No PCs need a warranty lookup.') - created = updated = matched = 0 - now = datetime.now(timezone.utc).replace(tzinfo=None) - for tag, assetids in by_tag.items(): - found = results.get(tag) - if not found: - continue - matched += 1 - for assetid in assetids: - # Reuse an existing Dell warranty for this asset if there is one. - existing = None - 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.provider == 'dell' - or (candidate.servicetag or '').strip().upper() == tag - or (candidate.vendor or '').strip().lower() == 'dell'): - existing = candidate - break - if existing: - # Canonicalize to Dell so later re-checks match by provider and - # never fall through to a duplicate. - existing.provider = 'dell' - existing.vendor = existing.vendor or 'Dell' - existing.servicelevel = found.get('servicelevel') - existing.startdate = _vendor_date(found.get('startdate')) - existing.enddate = _vendor_date(found.get('enddate')) - existing.servicetag = tag - existing.lastcheckeddate = now - updated += 1 - else: - warranty = Warranty( - vendor='Dell', provider='dell', servicetag=tag, - servicelevel=found.get('servicelevel'), - startdate=_vendor_date(found.get('startdate')), - enddate=_vendor_date(found.get('enddate')), - lastcheckeddate=now, - ) - warranty.links.append(WarrantyAsset(assetid=assetid)) - db.session.add(warranty) - created += 1 - - db.session.commit() - return success_response({ - 'candidates': sum(len(v) for v in by_tag.values()), - 'tags': len(by_tag), - 'matched': matched, - 'created': created, - 'updated': updated, - }, message=f'Dell sync: {created} added, {updated} updated, {matched} tags matched.') + message = (f"Dell sync: {summary['created']} added, {summary['updated']} updated, " + f"{summary['matched']} tags matched.") + if summary['remaining']: + message += (f" {summary['remaining']} still to check - run the sync again " + f"to continue.") + return success_response(summary, message=message) # ============================================================================= diff --git a/plugins/warranty/plugin.py b/plugins/warranty/plugin.py index bb2f9e4c..68c4e32b 100644 --- a/plugins/warranty/plugin.py +++ b/plugins/warranty/plugin.py @@ -10,12 +10,14 @@ import logging from pathlib import Path from typing import List, Dict, Optional, Type +import click from flask import Flask, Blueprint from shopdb.plugins.base import BasePlugin, PluginMeta from .api import warranty_bp from .models import Warranty, WarrantyAsset +from .services import CLI_SYNC_LIMIT logger = logging.getLogger(__name__) @@ -182,5 +184,44 @@ class WarrantyPlugin(BasePlugin): """ return ['warranty.view'] + def get_cli_commands(self) -> List: + """Return CLI commands for this plugin.""" + + @click.group('warranty') + def warrantycli(): + """Warranty plugin commands.""" + pass + + @warrantycli.command('sync-dell') + @click.option('--limit', default=None, type=int, + help=f'Tags to look up this run (default {CLI_SYNC_LIMIT}).') + @click.option('--all', 'recheck_all', is_flag=True, + help='Re-check assets that already have a dated warranty.') + def syncdell(limit, recheck_all): + """Look up Dell warranties for a bounded chunk of pending assets. + + Manual counterpart to the Dell sync button, for a large first-run + backfill where repeated clicking would be tedious. Nothing runs this + on a schedule. Each run takes one Dell call by default; assets that + already hold a dated warranty are skipped, so runs are cheap and + safely repeatable until `remaining` reaches zero. + """ + from flask import current_app + from .services import run_dell_sync, CLI_SYNC_LIMIT as default_limit + from .services import ProviderNotConfigured, WarrantyLookupError + + with current_app.app_context(): + try: + summary = run_dell_sync(limit=limit or default_limit, + recheck_all=recheck_all) + except (ProviderNotConfigured, WarrantyLookupError) as exc: + raise click.ClickException(str(exc)) + click.echo( + f"Dell sync: {summary['tags']} tag(s) checked, " + f"{summary['matched']} matched, {summary['created']} added, " + f"{summary['updated']} updated, {summary['remaining']} remaining.") + + return [warrantycli] + def init_app(self, app: Flask, db_instance) -> None: logger.info(f"Warranty plugin initialized (v{self.meta.version})") diff --git a/plugins/warranty/services/__init__.py b/plugins/warranty/services/__init__.py index efe3743f..19b797cf 100644 --- a/plugins/warranty/services/__init__.py +++ b/plugins/warranty/services/__init__.py @@ -6,5 +6,7 @@ from .providers import ( ProviderNotConfigured, WarrantyLookupError, ) +from .dellsync import run_dell_sync, ROUTE_SYNC_LIMIT, CLI_SYNC_LIMIT -__all__ = ['get_provider', 'provider_names', 'ProviderNotConfigured', 'WarrantyLookupError'] +__all__ = ['get_provider', 'provider_names', 'ProviderNotConfigured', 'WarrantyLookupError', + 'run_dell_sync', 'ROUTE_SYNC_LIMIT', 'CLI_SYNC_LIMIT'] diff --git a/plugins/warranty/services/dellsync.py b/plugins/warranty/services/dellsync.py new file mode 100644 index 00000000..3b3eff8d --- /dev/null +++ b/plugins/warranty/services/dellsync.py @@ -0,0 +1,169 @@ +"""Dell warranty sync, callable from a request or from the CLI. + +Extracted from the POST /sync/dell route so the same logic can run in bounded +chunks off the request path. `bulk_lookup` calls Dell 100 tags at a time at +HTTP_TIMEOUT seconds each, so an unbounded fleet-sized sync is dozens of +sequential HTTP calls inside one web request - long enough for a gunicorn +worker timeout or a proxy to kill it first, leaving the user with a failure and +no idea what landed. + +The sync is resumable by design: assets that already hold a dated warranty are +skipped, so a run that covers part of the fleet simply leaves the rest for the +next run. That is what makes chunking safe - no job queue, no progress table, +no state machine. Runs are operator-driven (the sync button, or +`flask warranty sync-dell` for a big first-run backfill); nothing drains the +remainder on a schedule, so callers report `remaining` and ask for another run. +""" + +import random +from datetime import datetime, timezone + +from shopdb.api import db, Asset, DateValueError, parse_date + +from ..models import Warranty, WarrantyAsset +from .providers import get_provider + +# One Dell call is 100 tags. The synchronous route stays a couple of calls deep +# so it cannot outlive a worker timeout; the CLI default is one call per run. +ROUTE_SYNC_LIMIT = 200 +CLI_SYNC_LIMIT = 100 + + +def _vendor_date(value): + """A date off a VENDOR response, which this code does not control. + + Kept lenient on purpose, unlike a date a person typed: a provider that + starts returning a shape we do not parse must not fail the refresh of an + otherwise good record. The field is simply left as it was. + """ + try: + return parse_date(value) + except DateValueError: + return None + + +def collect_pending_tags(recheck_all=False): + """Service tags still wanting a Dell lookup, mapped to their asset ids.""" + covered = set() + if not recheck_all: + for link in WarrantyAsset.query.all(): + warranty = db.session.get(Warranty, link.warrantyid) + if warranty and warranty.isactive and warranty.enddate: + covered.add(link.assetid) + + candidates = (Asset.query + .filter(Asset.isactive.is_(True), + Asset.serialnumber.isnot(None), + Asset.serialnumber != '') + .all()) + by_tag = {} + for asset in candidates: + if asset.assetid in covered: + continue + by_tag.setdefault(asset.serialnumber.strip().upper(), []).append(asset.assetid) + return by_tag + + +def run_dell_sync(limit=None, recheck_all=False): + """Look up service tags that have no warranty yet and record what Dell knows. + + `limit` caps how many TAGS are sent to Dell in this run. Returns the same + counters the route has always returned plus `remaining`, so a caller can + tell the user how much of the fleet is still pending. + + Raises ProviderNotConfigured / WarrantyLookupError from the provider; the + caller decides how to surface those. + """ + provider = get_provider('dell') + by_tag = collect_pending_tags(recheck_all=recheck_all) + pending = len(by_tag) + + if not by_tag: + return {'candidates': 0, 'tags': 0, 'matched': 0, 'created': 0, + 'updated': 0, 'remaining': 0} + + tags = list(by_tag.keys()) + if limit is not None and limit < len(tags): + # Shuffle before truncating. A serial Dell does not recognize never + # gains a warranty, so it stays pending forever - and under a stable + # order those same unrecognized tags would occupy every chunk and the + # sync would never reach the rest of the fleet. Random selection makes + # progress regardless of how many dead serials are in the set. + tags = random.sample(tags, limit) + by_tag = {tag: by_tag[tag] for tag in tags} + + # 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() + + results = provider.bulk_lookup(tags) + + # 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(): + found = results.get(tag) + if not found: + continue + matched += 1 + for assetid in assetids: + # Reuse an existing Dell warranty for this asset if there is one. + existing = None + 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.provider == 'dell' + or (candidate.servicetag or '').strip().upper() == tag + or (candidate.vendor or '').strip().lower() == 'dell'): + existing = candidate + break + if existing: + # Canonicalize to Dell so later re-checks match by provider and + # never fall through to a duplicate. + existing.provider = 'dell' + existing.vendor = existing.vendor or 'Dell' + existing.servicelevel = found.get('servicelevel') + existing.startdate = _vendor_date(found.get('startdate')) + existing.enddate = _vendor_date(found.get('enddate')) + existing.servicetag = tag + existing.lastcheckeddate = now + updated += 1 + else: + warranty = Warranty( + vendor='Dell', provider='dell', servicetag=tag, + servicelevel=found.get('servicelevel'), + startdate=_vendor_date(found.get('startdate')), + enddate=_vendor_date(found.get('enddate')), + lastcheckeddate=now, + ) + warranty.links.append(WarrantyAsset(assetid=assetid)) + db.session.add(warranty) + created += 1 + + db.session.commit() + return { + 'candidates': sum(len(v) for v in by_tag.values()), + 'tags': len(by_tag), + 'matched': matched, + 'created': created, + 'updated': updated, + 'remaining': max(pending - len(by_tag), 0), + } diff --git a/tests/test_plugins/test_warranty_connection_release.py b/tests/test_plugins/test_warranty_connection_release.py index 69239640..0bd25772 100644 --- a/tests/test_plugins/test_warranty_connection_release.py +++ b/tests/test_plugins/test_warranty_connection_release.py @@ -14,6 +14,7 @@ continues to pass. """ from plugins.warranty.api import routes as warranty_routes +from plugins.warranty.services import dellsync as warranty_dellsync from plugins.warranty.models import Warranty, WarrantyAsset @@ -104,7 +105,7 @@ def test_sync_dell_releases_the_connection_before_the_bulk_call( 'startdate': '2023-05-02', 'enddate': '2026-05-02', }) - monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe) + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: probe) response = client.post('/api/warranty/sync/dell', headers=auth_headers) @@ -147,7 +148,7 @@ def test_sync_dell_updates_instead_of_duplicating_on_a_recheck( 'startdate': '2024-02-02', 'enddate': '2028-02-02', }) - monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe) + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: probe) response = client.post('/api/warranty/sync/dell?all=true', headers=auth_headers) diff --git a/tests/test_plugins/test_warranty_dell_sync_chunking.py b/tests/test_plugins/test_warranty_dell_sync_chunking.py new file mode 100644 index 00000000..000b5be3 --- /dev/null +++ b/tests/test_plugins/test_warranty_dell_sync_chunking.py @@ -0,0 +1,116 @@ +"""The Dell sync must be bounded per run and must make progress across runs. + +`bulk_lookup` asks Dell about 100 tags per HTTP call at HTTP_TIMEOUT seconds +each, so an unbounded fleet-sized sync is dozens of sequential calls inside one +web request - long enough for a gunicorn worker timeout or a proxy to kill it +first. The fix is a per-run cap, with the operator re-running until nothing is +left pending. + +What would silently regress: the cap being dropped (the route goes back to +unbounded), or the chunk being taken off a stable ordering. The second is +subtle - a serial Dell does not recognize never gains a warranty, so it stays +pending forever, and under a stable order those dead tags would fill every +chunk and the sync would never reach the rest of the fleet. +""" + +from plugins.warranty.services import dellsync as warranty_dellsync +from plugins.warranty.services.dellsync import run_dell_sync +from plugins.warranty.models import Warranty, WarrantyAsset + + +def _assets(db, count, prefix='PC'): + 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() + + for index in range(count): + db.session.add(Asset( + assetnumber=f'{prefix}-{index:04d}', + name=f'Bench PC {index}', + assettypeid=assettype.assettypeid, + statusid=status.statusid, + serialnumber=f'TAG{index:04d}', + )) + db.session.commit() + + +class _CountingProvider: + """Records every tag it was asked about, and answers for none of them.""" + + def __init__(self, known=None): + self.seen_batches = [] + self._known = known or {} + + def bulk_lookup(self, servicetags): + self.seen_batches.append(list(servicetags)) + return {tag: self._known[tag] for tag in servicetags if tag in self._known} + + +def test_sync_is_capped_at_the_limit_and_reports_the_remainder(app, db, monkeypatch): + with app.app_context(): + _assets(db, 25) + provider = _CountingProvider() + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: provider) + + summary = run_dell_sync(limit=10) + + assert len(provider.seen_batches) == 1 + assert len(provider.seen_batches[0]) == 10, 'more tags went to Dell than the limit' + assert summary['tags'] == 10 + assert summary['remaining'] == 15 + + +def test_no_limit_checks_everything(app, db, monkeypatch): + with app.app_context(): + _assets(db, 12) + provider = _CountingProvider() + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: provider) + + summary = run_dell_sync(limit=None) + + assert summary['tags'] == 12 + assert summary['remaining'] == 0 + + +def test_repeated_runs_reach_tags_dell_does_not_recognize(app, db, monkeypatch): + """Unmatched tags stay pending, so chunks must not keep re-picking the same ones. + + Every asset here is unknown to Dell, which is the worst case: nothing ever + becomes covered, so the pending set never shrinks. A stable ordering would + send the same first chunk every run and the later tags would never be + looked up at all. + """ + with app.app_context(): + _assets(db, 30) + provider = _CountingProvider() + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: provider) + + for _ in range(12): + run_dell_sync(limit=5) + + reached = {tag for batch in provider.seen_batches for tag in batch} + assert len(reached) > 5, ( + 'repeated runs never got past the first chunk - the sync cannot drain' + ) + + +def test_covered_assets_are_skipped_so_runs_resume(app, db, monkeypatch): + with app.app_context(): + _assets(db, 5) + known = { + 'TAG0000': {'servicelevel': 'Basic Onsite', + 'startdate': '2023-05-02', 'enddate': '2026-05-02'}, + } + provider = _CountingProvider(known=known) + monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: provider) + + first = run_dell_sync(limit=None) + assert first['created'] == 1 + + second = run_dell_sync(limit=None) + assert second['tags'] == 4, 'a covered asset was looked up again' + assert 'TAG0000' not in provider.seen_batches[-1]