From adc7ef43f97c1bd51693b394167f11e158ed209c Mon Sep 17 00:00:00 2001 From: cproudlock Date: Mon, 24 Aug 2026 11:25:02 -0400 Subject: [PATCH] Stop the collector being able to set a printer assignment The printer feature rests on one rule: a collector report is OBSERVED state and never ASSIGNED state. If a drifted bay's own state can become what it is told to install, enforcement stops meaning anything, because a bay that installed the wrong printer makes itself right simply by reporting it. _sync_printer_links broke that rule. It ran on every collector PC push and, when the payload carried defaultprinter or printers, wrote defaultprinter and connectedto rows from the report - defaultprinter being one of the two types _reconcile_edges is meant to own alone, from authenticated routes a human calls. It was recorded as dormant. It was not. The collector schema ADVERTISED both keys and the example client in COLLECTOR-INTEGRATION.md set $payload['defaultprinter'], so the documentation was recruiting callers for it - while the same document, under "Observed is not assigned", promised that nothing on this path writes a usesprinter or defaultprinter row. The document contradicted itself and the code followed the wrong half. Nothing in the fleet armed it, because the first-party reporter never sent those fields, but any third-party client written from the docs would have. Retired the function, its _sync_one helper and the now-unused PRINTER_LINK_ORIGIN label, and removed both keys from the schema. IGNORED, NOT REJECTED. Unknown payload keys are not validated, so a client written against the old docs keeps pushing successfully and only its printer keys stop doing anything. The response warns, naming the keys and pointing at POST /api/collector/printers for reporting queues and seed-from-observed for adopting them. Dropping them in silence would leave an integrator believing assignments were still being written, which is a worse failure than the one being fixed. The docs were the larger half of this. The schema table rows are gone, the "PC -> printer relationship sync" section is replaced by a note saying what happens now to anyone still sending the keys, and the example client no longer enumerates Win32_Printer at all - it points at Report-PrintersToShopDB.ps1, which posts to the observed endpoint and posts NOTHING on a failed enumeration, since an empty list would erase the host's rows. test_collector_printers.py pinned the old behaviour, so it now pins the opposite: the keys create no relationships, the push still succeeds and says why, a payload without them stays quiet, a human-made default survives a report naming a different printer, and the schema no longer offers the keys. --- CHANGELOG.md | 14 ++ docs/COLLECTOR-INTEGRATION.md | 66 ++----- docs/PROJECT-MAP.md | 7 +- plugins/computers/plugin.py | 199 +++++---------------- tests/test_core/test_collector_printers.py | 174 +++++++----------- 5 files changed, 148 insertions(+), 312 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1cd9113..4e0cc259 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,20 @@ ADR-007 and ADR-002. ### Changed +- **The computers collector no longer accepts `defaultprinter` or `printers`, + and can no longer write a printer assignment.** Those keys used to become + `defaultprinter`/`connectedto` relationship rows straight from a report, which + contradicts the rule the whole printer feature rests on: a collector report is + OBSERVED state and never ASSIGNED state, or a bay that installed the wrong + printer makes itself right simply by reporting it. It was not a dormant code + path - the collector schema advertised both keys and the example client in + `docs/COLLECTOR-INTEGRATION.md` set `defaultprinter`, while another page of + the same document promised nothing on that path writes an assignment row. + **A client still sending them keeps working**: the keys are ignored, not + rejected, and the response carries a warning naming them and pointing at + `POST /api/collector/printers` for reporting queues and + `seed-from-observed` for adopting them. Silently dropping them would have left + an integrator believing assignments were still being written. - **Tech Tools no longer asks people to sign in for tools that need no account.** `/tools`, `/tools/codes` and `/tools/secure-send` were all behind `requiresAuth`. The label generator runs entirely in the browser - no API, no diff --git a/docs/COLLECTOR-INTEGRATION.md b/docs/COLLECTOR-INTEGRATION.md index 7f343c85..7362aa58 100644 --- a/docs/COLLECTOR-INTEGRATION.md +++ b/docs/COLLECTOR-INTEGRATION.md @@ -214,8 +214,6 @@ a column. | `modelnumber` | string | `Computer.modelnumberid`, scoped to the vendor when known. Model row auto-created if missing. | | `osname` | string | `Computer.osid`. Controlled vocab: looked up in `operatingsystems`, NOT auto-created. Unknown value -> warning (row still written, `osid` left unset). | | `installedsoftware` | array of `{name, version}` | `ComputerInstalledApp` rows for applications shopdb already tracks. Unknown app name -> warning, skipped. | -| `defaultprinter` | string | The default printer's identifier (windows name / share / hostname / port IP). Resolved to a printer asset and linked PC -> printer as a `defaultprinter` relationship. Unresolved -> warning. | -| `printers` | array of strings | All installed network printer identifiers. Each resolves to a printer asset and is linked PC -> printer as a `connectedto` relationship (the default is skipped here since it already links as `defaultprinter`). Unresolved entries -> warning. | Schema source of truth: `get_collector_schema` in `plugins/computers/plugin.py`. If you change the payload, change it there and re-check this table. @@ -313,31 +311,20 @@ because several markers serve one operation. Naming the marker in `asset-id.txt` changes which marker asset the PC controls; it does not change which operation that marker files under. -### PC -> printer relationship sync +### PC -> printer relationship sync (REMOVED) -When a payload carries `defaultprinter` and/or `printers`, the collector syncs -`AssetRelationship` rows so a PC page shows its printers and a printer page shows -the PCs that use it (both render in the shared Relationships card). +**A collector report has never been allowed to set a printer assignment, and as +of 2026-08-24 it no longer can.** `defaultprinter` and `printers` used to be +accepted here and written as `defaultprinter`/`connectedto` relationship rows. +That contradicted the rule stated under "Observed is not assigned" below, and +this document carried both claims at once while the code followed the wrong one. -- Resolution: each identifier is matched, first hit wins, against the printer's - `windowsname`, `hostname`, `sharename`, its asset number/name, then any active - printer communications IP. Case-insensitive except the IP (exact). An - identifier that resolves to nothing adds a warning and is skipped; it never - fails the whole push. -- Link types: the default printer links with `defaultprinter` (directional, PC - is the source); every other reported printer links with `connectedto` - (symmetric). A printer that is both default and in `printers` links only as - the default. -- Idempotent: re-reporting the same set creates no duplicate rows (an existing - matching row is reactivated if it was archived, otherwise left as is). -- Stale-link archive: on every push, collector-created links to printers no - longer reported are set inactive. Collector-created rows are tagged in - `assetrelationships.label = 'collector:printers'`; only tagged rows are ever - archived, so links you create by hand in the UI are never touched. A payload - that omits BOTH printer keys leaves all existing printer links untouched - (report an empty `printers: []` to clear the auto links instead). -- Response: the collector response carries `printerlinkcount` and a - `printerlinks` list of `{assetid, relationshiptype}` for the links kept. +If you are still sending either key: **your push still succeeds** - they are +ignored, and the response carries a warning naming them. Nothing else about your +payload changed. To report what a PC actually has, use +`POST /api/collector/printers`; to turn that into an assignment, a person calls +`POST /api/printers/assignments/seed-from-observed/` after reviewing the +comparison. ### pc-type mapping (configurable per site) @@ -766,32 +753,15 @@ function Send-ShopdbCollectorReport { try { $pcSubType = (Get-Content -LiteralPath 'C:\Enrollment\pc-subtype.txt' -First 1 -ErrorAction Stop).Trim() } catch {} } - # --- Installed printers (Win32_Printer). The Default flag marks the one - # default printer. We report each printer's port name (an IP or a queue - # host for network printers) and fall back to the share/printer name, which - # the collector resolves flexibly against printer windowsname/hostname/IP. --- - $defaultPrinter = '' - $printerIds = @() - try { - $printers = Get-CimInstance -ClassName Win32_Printer -ErrorAction Stop - foreach ($p in $printers) { - if ($p.Local) { continue } # skip local-only (XPS/PDF/OneNote) - # Prefer the port name (IP or queue host); fall back to ShareName, - # then the printer Name. - $identity = $p.PortName - if (-not $identity) { $identity = $p.ShareName } - if (-not $identity) { $identity = $p.Name } - if (-not $identity) { continue } - $printerIds += $identity - if ($p.Default) { $defaultPrinter = $identity } - } - $printerIds = @($printerIds | Select-Object -Unique) - } catch { Write-CollectorLog "WARN printer read failed: $($_.Exception.Message)" } + # --- No printer enumeration here, on purpose. This example used to collect + # Win32_Printer and send defaultprinter/printers, which wrote ASSIGNMENT + # rows from a report - the one thing this path must never do. Report what a + # PC has to POST /api/collector/printers instead (Report-PrintersToShopDB.ps1 + # in the printers plugin does exactly that, and posts NOTHING when the + # enumeration fails, because an empty list would erase the host's rows). --- # --- Build payload. Field names MUST match get_collector_schema exactly. --- $payload = @{ hostname = $hostname } - if ($defaultPrinter) { $payload['defaultprinter'] = $defaultPrinter } - if ($printerIds.Count) { $payload['printers'] = $printerIds } if ($machineNumber) { $payload['machinenumber'] = $machineNumber } if ($deviceId) { $payload['deviceid'] = $deviceId } if ($pcType) { $payload['pctype'] = $pcType } diff --git a/docs/PROJECT-MAP.md b/docs/PROJECT-MAP.md index 461da4bb..5e6852b9 100644 --- a/docs/PROJECT-MAP.md +++ b/docs/PROJECT-MAP.md @@ -36,6 +36,7 @@ with `flask plugin upgrade-all`. Both are needed on a deploy. | printedparts | `printedparts0004txnrev` | | printers | `printers0005observedqueues` | | slides | `slides0001anchor` | +| tools | `tools0001securesend` | | usb | `usb0002dropmachineid` | | warranty | `warranty0002proof` | @@ -55,7 +56,7 @@ with `flask plugin upgrade-all`. Both are needed on a deploy. | printedparts | 0.1.0 | >=0.16.0,<1.0.0 | yes | | printers | 1.0.0 | >=0.20.0,<1.0.0 | yes | | slides | 2.0.0 | >=0.2.0,<1.0.0 | yes | -| tools | 1.0.0 | >=0.16.0,<1.0.0 | no | +| tools | 1.0.0 | >=0.16.0,<1.0.0 | yes | | usb | 1.0.0 | >=0.1.0,<1.0.0 | yes | | warranty | 1.0.0 | >=0.2.0,<1.0.0 | yes | @@ -85,6 +86,6 @@ Manifest-less directories under `plugins/` are core frontend surface and always ## Size -- test functions defined: **1140** (parametrised cases collect higher) -- documented API paths: **282** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`) +- test functions defined: **1233** (parametrised cases collect higher) +- documented API paths: **285** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`) diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index 96f1e6b6..01bea5cd 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -16,16 +16,14 @@ from .api import computers_bp logger = logging.getLogger(__name__) -# Marker stamped on PC->printer links this collector creates. The stale-link -# archive only touches rows carrying this label, so manually-created printer -# relationships are never removed by a collector push. Stored in the -# assetrelationships.label column (no origin column exists; see BUILD notes). -PRINTER_LINK_ORIGIN = 'collector:printers' - # Marker stamped on PC->measuringtool "controls" links this collector creates -# for metrology PCs (CMM, Keyence, Genspect, wax-and-trace). Same stale-link -# discipline as PRINTER_LINK_ORIGIN: only rows carrying this label are archived -# by a collector push, so hand-made tool links survive. +# for metrology PCs (CMM, Keyence, Genspect, wax-and-trace). Only rows carrying +# this label are archived by a collector push, so hand-made tool links survive. +# Stored in assetrelationships.label (no origin column exists; see BUILD notes). +# +# There is deliberately no printer equivalent any more. PRINTER_LINK_ORIGIN went +# with _sync_printer_links: printer ASSIGNMENT is never collected, so there are +# no collector-owned printer rows left to label or archive. MEASURINGTOOL_LINK_ORIGIN = 'collector:measuringtool' # Marker stamped on the PC->machine "controls" link built from the reported @@ -139,26 +137,10 @@ class ComputersPlugin(BasePlugin): 'type': 'array', 'items': {'name': 'string', 'version': 'string'}, }, - # Printer identifiers reported by the GE-Enforce side, which - # runs Get-CimInstance Win32_Printer and marks the default with - # the Default flag. An identifier is the printer's windows name, - # share name, hostname, or port/IP; the collector resolves it - # flexibly to a printer asset. Both optional. Presence of either - # key drives the PC->printer relationship sync (and stale-link - # archive); absence leaves existing printer links untouched. - 'defaultprinter': { - 'type': 'string', - 'description': ('Default printer identifier (Win32_Printer ' - 'with Default=true): windows name, share ' - 'name, hostname, or port IP.'), - }, - 'printers': { - 'type': 'array', - 'items': {'type': 'string'}, - 'description': ('All installed network printer identifiers ' - '(Win32_Printer): windows name / share / ' - 'hostname / IP. Unresolved -> warning.'), - }, + # RETIRED: 'defaultprinter' and 'printers' used to live here and + # wrote PC->printer assignment rows from a report. That is the + # one thing this path must never do - see _RETIRED_PRINTER_KEYS. + # A client still sending them is warned, not rejected. 'accessprotocols': { 'type': 'array', 'items': {'type': 'string'}, @@ -423,8 +405,9 @@ class ComputersPlugin(BasePlugin): else: machinelinks = [] - # Printer relationship sync (only when the payload carried printer data). - printerlinks = self._sync_printer_links(comp.asset, payload, warnings) + # Printer assignment is NOT collected. See _warn_retired_printer_keys. + self._warn_retired_printer_keys(payload, warnings) + printerlinks = [] # Measuring-tool sync: metrology PCs (CMM/Keyence/Genspect/wax-trace) # get an attached MeasuringTool asset auto-created and linked. @@ -518,134 +501,38 @@ class ComputersPlugin(BasePlugin): AccessProtocol.protocolid.in_(wanted)).all()] if wanted else [] return sorted(names) - def _sync_printer_links(self, pcasset, payload, warnings): - """Idempotently sync PC->printer relationships from collector printer data. + def _warn_retired_printer_keys(self, payload, warnings): + """Refuse to turn a report into an assignment, out loud. - Default printer -> 'defaultprinter' (directional). Other reported - printers -> 'connectedto' (symmetric). Resolves each identifier to a - printer asset by windows name / share / hostname / asset number / name - or a communications IP. Unresolved identifiers add a warning and never - fail the push. + RETIRED 2026-08-24. This used to be _sync_printer_links, which wrote + 'defaultprinter' and 'connectedto' rows straight from a collector + payload. That contradicted the rule the whole printer feature rests on: + ASSIGNED and OBSERVED never mix. A bay that installed the wrong printer + would have made itself right simply by reporting it, and enforcement + would stop meaning anything. - Stale-link archive: on each push, collector-tagged links (label == - PRINTER_LINK_ORIGIN) whose (target, type) pair is not in the reported - desired set are set inactive. Only tagged rows are touched, so manual - links survive. Runs only when the payload carried a printer key - ('defaultprinter' or 'printers'); a PC that reports without printer data - keeps its existing links. Returns the desired-link list (created + kept). + Worse than dormant, it was ADVERTISED: the collector schema listed both + keys and COLLECTOR-INTEGRATION.md's example client set defaultprinter, + while the same document promised on another page that nothing on this + path writes an assignment row. The document contradicted itself and the + code sided with the wrong half. + + Observed queues have their own path (POST /api/collector/printers) and + become assignments only when a person asks, through + seed-from-observed after reviewing the comparison. + + WARNS RATHER THAN REJECTS. Unknown keys are not validated here, so a + client written against the old docs keeps pushing successfully; only its + printer keys stop doing anything. Silently ignoring them would leave an + integrator believing assignments were being written. """ - from shopdb.api import ( - AssetRelationship, RelationshipType, Asset, Communication) - - has_default = 'defaultprinter' in payload - has_list = 'printers' in payload - if not has_default and not has_list: - return [] - - try: - from plugins.printers.models import Printer - except ImportError: - warnings.append('printers plugin unavailable; printer links skipped') - return [] - - dp_type = RelationshipType.query.filter_by( - relationshiptype='defaultprinter').first() - ct_type = RelationshipType.query.filter_by( - relationshiptype='connectedto').first() - if not dp_type or not ct_type: - warnings.append('printer relationship types missing; ' - 'run flask seed reference-data') - return [] - - def resolve(identifier): - # first match wins: printer text identity, then a printer IP. - ident = (identifier or '').strip() - if not ident: - return None - printer = ( - Printer.query.join(Asset, Asset.assetid == Printer.assetid) - .filter(Asset.isactive == True) - .filter(db.or_( - Printer.windowsname.ilike(ident), - Printer.hostname.ilike(ident), - Printer.sharename.ilike(ident), - Asset.assetnumber.ilike(ident), - Asset.name.ilike(ident), - )).first()) - if printer: - return printer.asset - comm = ( - db.session.query(Communication) - .join(Printer, Printer.assetid == Communication.assetid) - .filter(Communication.ipaddress == ident) - .first()) - if comm: - return db.session.get(Asset, comm.assetid) - return None - - pcid = pcasset.assetid - desired = set() # (targetassetid, relationshiptypeid) to keep - printerlinks = [] - - default_id = None - default_ident = (payload.get('defaultprinter') or '').strip() - if default_ident: - target = resolve(default_ident) - if target: - default_id = target.assetid - desired.add((default_id, dp_type.relationshiptypeid)) - self._sync_one(pcid, default_id, dp_type) - printerlinks.append({'assetid': default_id, - 'relationshiptype': 'defaultprinter'}) - else: - warnings.append(f'unresolved default printer: {default_ident}') - - for ident in payload.get('printers') or []: - target = resolve(ident) - if not target: - warnings.append(f'unresolved printer: {ident}') - continue - if target.assetid == default_id: - continue # already the default link - desired.add((target.assetid, ct_type.relationshiptypeid)) - self._sync_one(pcid, target.assetid, ct_type) - printerlinks.append({'assetid': target.assetid, - 'relationshiptype': 'connectedto'}) - - # Archive collector-tagged links no longer reported (manual links, with - # a NULL/other label, are never matched here). - collector_rows = AssetRelationship.query.filter( - AssetRelationship.sourceassetid == pcid, - AssetRelationship.relationshiptypeid.in_( - [dp_type.relationshiptypeid, ct_type.relationshiptypeid]), - AssetRelationship.isactive == True, - AssetRelationship.label == PRINTER_LINK_ORIGIN, - ).all() - for rel in collector_rows: - if (rel.targetassetid, rel.relationshiptypeid) not in desired: - rel.isactive = False - - return printerlinks - - def _sync_one(self, pcid, printerid, reltype): - """Reactivate or create one collector PC->printer link (idempotent).""" - from shopdb.api import AssetRelationship - existing = AssetRelationship.query.filter_by( - sourceassetid=pcid, targetassetid=printerid, - relationshiptypeid=reltype.relationshiptypeid).first() - if existing: - # do not re-stamp label: a pre-existing manual row stays manual. - if not existing.isactive: - existing.isactive = True - return existing - rel = AssetRelationship( - sourceassetid=pcid, targetassetid=printerid, - relationshiptypeid=reltype.relationshiptypeid, - label=PRINTER_LINK_ORIGIN) - db.session.add(rel) - return rel - - # -- measuring-tool sync ----------------------------------------------- + sent = [key for key in ('defaultprinter', 'printers') if key in payload] + if sent: + warnings.append( + '%s ignored: a collector report never sets printer assignment. ' + 'Report queues to POST /api/collector/printers, then adopt them ' + 'with POST /api/printers/assignments/seed-from-observed.' + % ' and '.join(sent)) def _sync_machine_link(self, comp, machinenumber, warnings): """Link a PC to the machine it drives, from the reported machine number. diff --git a/tests/test_core/test_collector_printers.py b/tests/test_core/test_collector_printers.py index ac48bd93..576b039a 100644 --- a/tests/test_core/test_collector_printers.py +++ b/tests/test_core/test_collector_printers.py @@ -1,11 +1,19 @@ -"""Tests for the computers collector PC->printer relationship sync. +"""The computers collector must NEVER write a printer assignment. -A collector payload carrying defaultprinter/printers creates AssetRelationship -rows linking the PC asset to the resolved printer assets ('defaultprinter' for -the default, 'connectedto' for the rest). Stale collector links archive on -re-report; manual links survive; unresolved identifiers warn without failing. +This file used to pin the opposite: `_sync_printer_links` turned a payload's +`defaultprinter`/`printers` into AssetRelationship rows. That contradicted the +rule the printer feature rests on - ASSIGNED and OBSERVED never mix - because a +bay that installed the wrong printer would have made itself right just by +reporting it. + +It was not dormant. The collector schema advertised both keys and +COLLECTOR-INTEGRATION.md's example client set `defaultprinter`, while another +page of the same document promised nothing on this path writes an assignment +row. Retired 2026-08-24; these tests are the guard that it stays retired. + +Observed queues have their own path (POST /api/collector/printers) and become +assignments only when a person asks, via seed-from-observed. """ - import pytest from shopdb.core.models import ( @@ -81,119 +89,75 @@ def _active_links(db, pcid): sourceassetid=pcid, isactive=True).all() -def test_creates_default_and_connected_links(client, db, collector_key, scene): - """Default -> defaultprinter, others -> connectedto, resolved by three id forms.""" - resp = _post(client, { - 'hostname': 'WJPC-P1', - 'defaultprinter': 'HP-Materials', # windowsname - 'printers': ['prn-front.wjs.local', '10.1.2.3'], # hostname, IP - }) - assert resp.status_code == 200, resp.get_json() - data = resp.get_json()['data'] - assert data['printerlinkcount'] == 3 - assert not data['warnings'] - - pc = _pc_asset(db) - links = {(r.targetassetid, r.relationshiptype.relationshiptype) - for r in _active_links(db, pc.assetid)} - assert (scene['a'].assetid, 'defaultprinter') in links - assert (scene['b'].assetid, 'connectedto') in links - assert (scene['c'].assetid, 'connectedto') in links - - -def test_unresolved_identifier_warns_not_fails(client, db, collector_key, scene): - """An unknown printer identifier is a warning, the rest still link.""" - resp = _post(client, { - 'hostname': 'WJPC-P2', +def test_printer_keys_create_no_relationships(client, db, collector_key, scene): + """The reported case, inverted: sending them must change nothing.""" + response = _post(client, { + 'hostname': 'BAY-3105-PC', 'defaultprinter': 'HP-Materials', - 'printers': ['does-not-exist'], + 'printers': ['HP-Materials', 'prn-front.wjs.local', '10.1.2.3'], }) - assert resp.status_code == 200 - data = resp.get_json()['data'] - assert any('unresolved printer: does-not-exist' in w - for w in data['warnings']) - pc = _pc_asset(db) - assert data['printerlinkcount'] == 1 - assert len(_active_links(db, pc.assetid)) == 1 + assert response.status_code in (200, 201), response.get_json() + + pcid = _pc_asset(db).assetid + assert _active_links(db, pcid) == [], ( + 'a collector report wrote printer relationships') -def test_idempotent_no_duplicates(client, db, collector_key, scene): - """Re-report the same set: no new rows, still one link per printer.""" - payload = {'hostname': 'WJPC-P3', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']} - assert _post(client, payload).status_code == 200 - assert _post(client, payload).status_code == 200 +def test_the_push_still_succeeds_and_says_why(client, db, collector_key, scene): + """Warn, do not reject. - pc = _pc_asset(db) - all_rows = AssetRelationship.query.filter_by(sourceassetid=pc.assetid).all() - assert len(all_rows) == 2 # one default + one connected, no dupes - assert all(r.isactive for r in all_rows) + Unknown keys are not validated, so a client written against the old docs + keeps pushing successfully - only its printer keys stop doing anything. + Silently dropping them would leave an integrator believing assignments were + still being written, which is worse than the original bug. + """ + response = _post(client, { + 'hostname': 'BAY-3105-PC', + 'defaultprinter': 'HP-Materials', + }) + assert response.status_code in (200, 201) + warnings = ' '.join(response.get_json()['data']['warnings']).lower() + assert 'defaultprinter' in warnings + assert 'seed-from-observed' in warnings, ( + 'the warning should say where assignment actually comes from') -def test_dropped_printer_is_archived(client, db, collector_key, scene): - """A printer that stops being reported loses its collector link.""" - _post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']}) - pc = _pc_asset(db) - assert len(_active_links(db, pc.assetid)) == 2 - - # Re-report without Zebra-Ship. - _post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials', - 'printers': []}) - active = _active_links(db, pc.assetid) - assert len(active) == 1 - assert active[0].targetassetid == scene['a'].assetid - # the dropped row still exists but is inactive - zebra = AssetRelationship.query.filter_by( - sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first() - assert zebra is not None and zebra.isactive is False +def test_a_payload_without_printer_keys_warns_about_nothing( + client, db, collector_key, scene): + """The normal case must stay quiet - this is every push in the fleet.""" + response = _post(client, {'hostname': 'BAY-3105-PC'}) + assert response.status_code in (200, 201) + warnings = ' '.join(response.get_json()['data']['warnings']).lower() + assert 'printer' not in warnings -def test_default_change_archives_old_default(client, db, collector_key, scene): - """Changing the default printer archives the previous defaultprinter link.""" - _post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'HP-Materials'}) - pc = _pc_asset(db) - _post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'prn-front.wjs.local'}) - - active = _active_links(db, pc.assetid) - assert len(active) == 1 - assert active[0].targetassetid == scene['b'].assetid - assert active[0].relationshiptype.relationshiptype == 'defaultprinter' - - -def test_manual_link_survives_collector_sync(client, db, collector_key, scene): - """A manually-created link (no collector label) is never archived.""" - # PC created first via a bare report. - _post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']}) - pc = _pc_asset(db) - - # Manual connectedto link PC -> printer B (label stays NULL). +def test_a_manual_assignment_survives_a_report_that_names_printers( + client, db, collector_key, scene): + """The row a person made is the one that matters, and it stays untouched.""" + _post(client, {'hostname': 'BAY-3105-PC'}) + pcasset = _pc_asset(db) manual = AssetRelationship( - sourceassetid=pc.assetid, targetassetid=scene['b'].assetid, - relationshiptypeid=scene['ct_type'].relationshiptypeid, notes='by hand') + sourceassetid=pcasset.assetid, targetassetid=scene['a'].assetid, + relationshiptypeid=scene['dp_type'].relationshiptypeid) db.session.add(manual) db.session.commit() - manual_id = manual.relationshipid - # Re-report dropping Zebra-Ship and never mentioning printer B. - _post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials', - 'printers': []}) + _post(client, { + 'hostname': 'BAY-3105-PC', + 'defaultprinter': 'prn-front.wjs.local', # a DIFFERENT printer + 'printers': ['prn-front.wjs.local'], + }) - surviving = db.session.get(AssetRelationship, manual_id) - assert surviving.isactive is True # manual link untouched - # Zebra (collector) archived. - zebra = AssetRelationship.query.filter_by( - sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first() - assert zebra.isactive is False + links = _active_links(db, pcasset.assetid) + assert len(links) == 1 + assert links[0].targetassetid == scene['a'].assetid, ( + 'the report retargeted a human-made default printer') -def test_no_printer_keys_leaves_links_untouched(client, db, collector_key, scene): - """A report with no printer keys does not archive existing links.""" - _post(client, {'hostname': 'WJPC-P7', 'defaultprinter': 'HP-Materials'}) - pc = _pc_asset(db) - assert len(_active_links(db, pc.assetid)) == 1 +def test_the_retired_keys_are_gone_from_the_collector_schema(app): + """The schema advertised them, which is how integrators found them.""" + from plugins.computers.plugin import ComputersPlugin - # A plain update (no defaultprinter, no printers key) must not archive. - _post(client, {'hostname': 'WJPC-P7', 'currentuser': 'alice'}) - assert len(_active_links(db, pc.assetid)) == 1 + fields = ComputersPlugin().get_collector_schema()['fields'] + assert 'defaultprinter' not in fields + assert 'printers' not in fields