diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index a8d1105..8ca4c61 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -106,6 +106,13 @@ class ComputersPlugin(BasePlugin): 'fields': { 'hostname': {'type': 'string', 'required': True}, 'machinenumber': {'type': 'string'}, + # The instrument this PC drives, named by the enrollment file + # measuringtool-id.txt. SEPARATE from machinenumber on purpose: + # "which bay is this" and "which instrument is this" are + # different facts, and machinenumber is what GE-Enforce + # TargetMachineNumbers gates on - repointing it at a tool would + # silently stop every bay-gated manifest entry matching. + 'measuringtoolid': {'type': 'string'}, 'pctype': {'type': 'string'}, 'pcsubtype': {'type': 'string'}, 'serialnumber': {'type': 'string'}, @@ -187,7 +194,7 @@ class ComputersPlugin(BasePlugin): """Whether the PC-to-machine alerts may be sent. Off by default, deliberately. A site may legitimately run several PCs - on one machine number - part markers do at West Jefferson - and there + on one machine number - part markers do at the reference site - and there both the handover and the contested case fire on normal, correct data, which is noise rather than news. Turn it on at a site where a machine number means exactly one PC. @@ -370,7 +377,9 @@ class ComputersPlugin(BasePlugin): # Measuring-tool sync: metrology PCs (CMM/Keyence/Genspect/wax-trace) # get an attached MeasuringTool asset auto-created and linked. measuringtoollinks = self._sync_measuringtool_link( - comp.asset, pctype, hostname, warnings) + comp.asset, pctype, hostname, warnings, + measuringtoolid=payload.get('measuringtoolid'), + machinenumber=machinenumber) db.session.commit() return { @@ -619,9 +628,7 @@ class ComputersPlugin(BasePlugin): 'run flask seed reference-data') return [] - machine = Asset.query.filter( - Asset.assetnumber.ilike(machinenumber), - Asset.isactive.is_(True)).first() + machine = self._asset_by_number(machinenumber) if not machine: # Reported a machine ShopDB does not know. Warn rather than invent # an asset: a mistyped number would create a machine nobody can @@ -791,22 +798,48 @@ class ComputersPlugin(BasePlugin): db.session.flush() hostname = comp.hostname - markerasset = Asset( - assetnumber='{}-{}'.format( - pcasset.assetnumber or hostname, spec['suffix']), - name='{} ({})'.format(spec['typename'], hostname), - assettypeid=coretype.assettypeid, - statusid=1) - db.session.add(markerasset) - db.session.flush() - db.session.add(Machine(assetid=markerasset.assetid, - machinetypeid=devicetype.machinetypeid)) - db.session.add(AssetRelationship( + assetnumber = '{}-{}'.format( + pcasset.assetnumber or hostname, spec['suffix']) + + # ADOPT AN EXISTING ASSET OF THAT NUMBER before minting. The number + # is derived from the PC, so it is entirely predictable and may + # already exist - left by an earlier run whose link was archived, or + # created by a person. assetnumber is unique, so inserting a second + # one raised and the whole report 500'd, on every cycle, forever. + # + # Deliberately NOT filtered on isactive: an inactive asset still + # holds the number, and it is the constraint that decides this. + markerasset = Asset.query.filter( + db.func.lower(Asset.assetnumber) == assetnumber.lower()).first() + if markerasset is None: + markerasset = Asset( + assetnumber=assetnumber, + name='{} ({})'.format(spec['typename'], hostname), + assettypeid=coretype.assettypeid, + statusid=1) + db.session.add(markerasset) + db.session.flush() + + # The extension row may be missing on an adopted asset, and the link + # may already exist under another label - both get-or-create. + if not Machine.query.filter_by(assetid=markerasset.assetid).first(): + db.session.add(Machine(assetid=markerasset.assetid, + machinetypeid=devicetype.machinetypeid)) + controlrow = AssetRelationship.query.filter_by( sourceassetid=pcasset.assetid, targetassetid=markerasset.assetid, - relationshiptypeid=controls.relationshiptypeid, - label=label, - isactive=True)) + relationshiptypeid=controls.relationshiptypeid).first() + if controlrow is not None: + controlrow.isactive = True + if not controlrow.label: + controlrow.label = label + else: + db.session.add(AssetRelationship( + sourceassetid=pcasset.assetid, + targetassetid=markerasset.assetid, + relationshiptypeid=controls.relationshiptypeid, + label=label, + isactive=True)) # One marker per PC: archive any other collector marker link. for rel in existing: @@ -847,9 +880,7 @@ class ComputersPlugin(BasePlugin): 'run flask seed reference-data') return None - operation = Asset.query.filter( - Asset.assetnumber.ilike(machinenumber), - Asset.isactive.is_(True)).first() + operation = self._asset_by_number(machinenumber) if not operation: warnings.append( 'no asset for machine number {!r}; marker not filed under an ' @@ -865,19 +896,34 @@ class ComputersPlugin(BasePlugin): machinenumber)) return None - links = AssetRelationship.query.filter( - AssetRelationship.sourceassetid == markerasset.assetid, - AssetRelationship.relationshiptypeid == partof.relationshiptypeid, - AssetRelationship.label == label, - ).all() - found = None - for rel in links: - if rel.targetassetid == operation.assetid: - rel.isactive = True - found = rel - else: + # Our own rows first: a marker that moved to another operation has its + # old membership archived, never deleted, so where it used to live stays + # answerable. + for rel in AssetRelationship.query.filter( + AssetRelationship.sourceassetid == markerasset.assetid, + AssetRelationship.relationshiptypeid == partof.relationshiptypeid, + AssetRelationship.label == label).all(): + if rel.targetassetid != operation.assetid and rel.isactive: rel.isactive = False - if found is None: + + # Then get-or-create KEYED ON THE UNIQUE TRIPLE, not on the label. + # assetrelationships is unique on (source, target, type), so a row made + # by hand or by the legacy import - carrying a different label, or none + # at all - was invisible to the label-filtered lookup this used to do, + # and the insert then violated that constraint. The PC reported 200 on + # the cycle that minted the marker and 500 on every cycle after. + found = AssetRelationship.query.filter_by( + sourceassetid=markerasset.assetid, + targetassetid=operation.assetid, + relationshiptypeid=partof.relationshiptypeid).first() + if found is not None: + found.isactive = True + if not found.label: + # Unlabelled means the import or a person made it. Stamping ours + # is what makes the next cycle reuse it. A row carrying somebody + # else's label is left as theirs. + found.label = label + else: db.session.add(AssetRelationship( sourceassetid=markerasset.assetid, targetassetid=operation.assetid, @@ -1022,18 +1068,39 @@ class ComputersPlugin(BasePlugin): 'PC-superseded alert failed for machine %s', getattr(machine, 'assetnumber', '?')) - def _sync_measuringtool_link(self, pcasset, pctype, hostname, warnings): - """Auto-create + link the MeasuringTool a metrology PC drives. + def _sync_measuringtool_link(self, pcasset, pctype, hostname, warnings, + measuringtoolid=None, machinenumber=None): + """Link the MeasuringTool a metrology PC drives. ADOPT before minting. - A CMM / Keyence / Genspect / wax-and-trace imaging pc-type means the - shopfloor PC controls an attached measuring instrument. This creates - that instrument once as a MeasuringTool asset and a directional - PC->tool 'controls' relationship, tagged MEASURINGTOOL_LINK_ORIGIN so - it is idempotent and self-archiving. The PC's own ComputerType is left - alone (it stays a shopfloor PC). A non-metrology pc-type archives any - collector-created tool link (e.g. a PC re-imaged to another type) but - never deletes the tool asset, which may carry calibration history. - Returns the desired-link list. + A CMM / Keyence / Genspect / wax-and-trace pc-type means the PC controls + an attached instrument. This links it with a directional PC->tool + 'controls' relationship tagged MEASURINGTOOL_LINK_ORIGIN. + + RESOLUTION ORDER, most stable identity first: + + 1. measuringtoolid - the instrument named by the enrollment file + measuringtool-id.txt. Explicit, survives a PC swap, and it is what + a Keyence or Genspect bay has instead of a bay id. + 2. a prior collector link on this PC (reactivate + retype). + 3. an EXISTING measuring tool this PC already controls that the + collector did not create - a legacy MT-#### row, or one somebody + made by hand. Adopting stamps the label so it is ours from then on. + 4. the reported machine number, when it resolves to a measuring tool. + This is the CMM case: cmmid.txt already reports CMM4. + 5. mint, only when none of the above found anything. + + WHY THIS ORDER. Minting derives an asset number from the HOSTNAME, so a + permanent instrument inherited the identity of whichever PC drove it that + week - replace the PC and either the number lies or a second tool appears + for the same physical unit. It also could not see a tool it had not + created itself, so on prod 43 legacy MT-#### tools were shadowed by + minted -CMM twins, three records deep in places. Minting is now the + last resort, and what it produces should be treated as a placeholder + until a real identifier is recorded. + + A non-metrology pc-type archives any collector-created tool link (a PC + re-imaged to another type) but never deletes the tool asset, which may + carry calibration history. Returns the desired-link list. """ from shopdb.api import AssetRelationship, RelationshipType, Asset from .pctypemap import metrology_tool_for @@ -1073,11 +1140,75 @@ class ComputersPlugin(BasePlugin): typename, typedescription = tool_spec tooltype = self._ensure_measuringtool_type(typename, typedescription) - # Reuse any prior collector link (reactivate + retype) before creating, - # so a re-metrology PC never duplicates the tool asset. + # --- 1. an explicitly named instrument wins over everything --------- + adopted = None + named = (measuringtoolid or '').strip() + if named: + candidate = self._asset_by_number(named) + if candidate is None: + # Warn rather than invent: a typo in the file must not mint a + # phantom instrument that nobody can account for. + warnings.append( + 'no asset for measuring tool {!r}; not linked'.format(named)) + elif candidate.assetid not in self._measuringtool_assetids(): + # The name resolved, but not to an instrument. measuringtool-id + # .txt holding a machine number would otherwise link the PC to + # that MACHINE under a measuring-tool label - a link that reads + # as an instrument everywhere downstream, on a row the machine + # sync also owns. Refuse and say which asset it hit. + warnings.append( + 'asset {!r} is not a measuring tool; not linked'.format( + candidate.assetnumber)) + else: + adopted = candidate + + # --- 2. a prior collector link (reactivate + retype) ----------------- reuse = next((rel for rel in existing if rel.isactive), None) \ or (existing[0] if existing else None) - if reuse: + + # --- 3/4. adopt a tool this PC already controls that we did not make -- + # Ordered after the collector's own link so a settled PC keeps what it + # has, and before minting so an existing instrument is never twinned. + if adopted is None and reuse is None: + adopted = self._adoptable_measuringtool( + pcasset, controls, machinenumber) + + if adopted is not None: + # CLAIM the existing link rather than adding a second one. A row for + # (pc, tool, controls) usually already exists - that is how the tool + # was found - and assetrelationships is unique on exactly that + # triple, so inserting would raise. + claimed = AssetRelationship.query.filter_by( + sourceassetid=pcasset.assetid, + targetassetid=adopted.assetid, + relationshiptypeid=controls.relationshiptypeid).first() + if claimed is not None and claimed.label == MACHINE_LINK_ORIGIN: + # LEAVE IT ALONE. On a CMM the instrument IS the reported bay, + # so the machine sync has already made the row for this exact + # triple - and it finds its own rows BY that label. Relabelling + # hid the link from it, so the next cycle built a second row for + # the same triple and MySQL rejected it: the PC reported 200, + # then 500 forever after. The row's lifecycle (including staying + # dormant while a bay is contested) belongs to the machine sync; + # adoption only needs the identity, which it has. + pass + elif claimed is not None: + # Relabelling is what adoption MEANS for any other row: the link + # is ours from here, so a later cycle reuses it instead of + # minting. + claimed.label = MEASURINGTOOL_LINK_ORIGIN + claimed.isactive = True + else: + db.session.add(AssetRelationship( + sourceassetid=pcasset.assetid, + targetassetid=adopted.assetid, + relationshiptypeid=controls.relationshiptypeid, + label=MEASURINGTOOL_LINK_ORIGIN)) + if adopted.measuringtool and tooltype: + adopted.measuringtool.measuringtooltypeid = \ + tooltype.measuringtooltypeid + targetid = adopted.assetid + elif reuse: reuse.isactive = True toolasset = db.session.get(Asset, reuse.targetassetid) if toolasset and toolasset.measuringtool and tooltype: @@ -1111,14 +1242,96 @@ class ComputersPlugin(BasePlugin): label=MEASURINGTOOL_LINK_ORIGIN)) targetid = toolasset.assetid - # Only one tool link is desired; archive any other collector rows. + # Only one tool link is desired; archive any other collector row. + # + # Keyed on the TARGET, not on object identity. `rel is not reuse` spared + # the row reuse pointed at even when adoption had just resolved a + # different instrument - so a bay whose measuringtool-id.txt named the + # real tool kept its minted twin link alive alongside the adopted one, + # and the PC read as controlling two instruments. That is the exact + # duplicate this whole adoption path exists to remove. for rel in existing: - if rel is not reuse and rel.isactive: + if rel.targetassetid != targetid and rel.isactive: rel.isactive = False return [{'assetid': targetid, 'relationshiptype': 'controls', 'measuringtooltype': typename}] + @staticmethod + def _asset_by_number(assetnumber): + """The active asset with exactly this asset number, case-insensitively. + + NOT `ilike`. Every identifier resolved here arrives from a file on a + shopfloor PC - cmmid.txt, measuringtool-id.txt, the reported machine + number - and ilike reads `_` and `%` in that value as WILDCARDS. A + reported '%' matched whatever active asset happened to come first, and + 'MT-600_' matched MT-6001: an identifier silently resolving to an asset + it does not name, then linked to as though it did. + """ + from shopdb.api import Asset + number = (assetnumber or '').strip() + if not number: + return None + return Asset.query.filter( + db.func.lower(Asset.assetnumber) == number.lower(), + Asset.isactive.is_(True)).first() + + def _measuringtool_assetids(self): + """Asset ids that really are measuring tools. + + Empty when the measuringtools plugin is not installed, which makes both + callers fall through rather than guess: an asset number alone does not + say what kind of thing it names. + """ + try: + from plugins.measuringtools.models import MeasuringTool + except ImportError: + return set() + return {row.assetid for row in + MeasuringTool.query.with_entities(MeasuringTool.assetid)} + + def _adoptable_measuringtool(self, pcasset, controls, machinenumber): + """A measuring tool this PC already controls that we did not create. + + Two sources, in order: + + a. any ACTIVE controls link from this PC to a measuring_tool asset + that carries a different label - the legacy import's MT-#### rows, + or one somebody added by hand. These were invisible to the + collector, which keyed idempotency on its OWN label and so minted a + twin for the same physical instrument on every fresh PC. + b. the reported machine number, when it names a measuring tool. That + is the CMM case: cmmid.txt reports CMM4, which already resolves to + a real asset through the machine-link path. + + Returns the Asset to adopt, or None. Never creates anything. + """ + from shopdb.api import AssetRelationship, Asset + + toolassetids = self._measuringtool_assetids() + if not toolassetids: + return None + + # a. already linked, by someone else + for rel in (AssetRelationship.query + .filter(AssetRelationship.sourceassetid == pcasset.assetid, + AssetRelationship.relationshiptypeid + == controls.relationshiptypeid, + AssetRelationship.isactive.is_(True)) + .order_by(AssetRelationship.relationshipid.asc()).all()): + if rel.label == MEASURINGTOOL_LINK_ORIGIN: + continue + if rel.targetassetid in toolassetids: + return db.session.get(Asset, rel.targetassetid) + + # b. the reported machine number naming a tool + number = (machinenumber or '').strip() + if number: + candidate = self._asset_by_number(number) + if candidate is not None and candidate.assetid in toolassetids: + return candidate + return None + def _ensure_measuringtool_type(self, name, description): """Find or create a MeasuringToolType (metrology types are not in the measuringtools starter seed).""" diff --git a/tests/test_plugins/test_collector_measuringtool_adoption.py b/tests/test_plugins/test_collector_measuringtool_adoption.py new file mode 100644 index 0000000..dc2a4da --- /dev/null +++ b/tests/test_plugins/test_collector_measuringtool_adoption.py @@ -0,0 +1,232 @@ +"""The collector ADOPTS an existing instrument instead of minting a twin. + +Minting derived the asset number from the HOSTNAME, so a permanent instrument +inherited the identity of whichever PC drove it that week. And because the +collector keyed idempotency on its OWN label, it could not see a tool it had not +created - on prod that left 43 legacy MT-#### tools shadowed by minted +-CMM twins, three records deep for some CMMs. +""" + +import pytest + +from shopdb.core.models import Asset, AssetType +from shopdb.core.models.relationship import RelationshipType, AssetRelationship +from plugins.measuringtools.models import MeasuringTool + +KEY = 'collector-adoption-key' + + +@pytest.fixture +def collector_key(app): + old = app.config.get('COLLECTOR_API_KEY') + app.config['COLLECTOR_API_KEY'] = KEY + yield KEY + app.config['COLLECTOR_API_KEY'] = old + + +@pytest.fixture +def rig(db): + """Asset types, a controls relationship type, and nothing else.""" + for name in ('computer', 'measuring_tool'): + if not AssetType.query.filter_by(assettype=name).first(): + db.session.add(AssetType(assettype=name)) + if not RelationshipType.query.filter_by(relationshiptype='controls').first(): + db.session.add(RelationshipType(relationshiptype='controls')) + db.session.commit() + + +def _tool(db, assetnumber): + """A measuring tool asset the collector did NOT create.""" + assettype = AssetType.query.filter_by(assettype='measuring_tool').first() + asset = Asset(assetnumber=assetnumber, assettypeid=assettype.assettypeid) + db.session.add(asset) + db.session.flush() + db.session.add(MeasuringTool(assetid=asset.assetid)) + db.session.commit() + return asset + + +def _report(client, key, hostname, **extra): + payload = {'hostname': hostname, 'pctype': 'gea-shopfloor-cmm'} + payload.update(extra) + return client.post('/api/collector/computers', json=payload, + headers={'X-API-Key': key}) + + +def _tools_controlled(hostname): + pc = Asset.query.filter_by(assetnumber=hostname).first() + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + toolids = {r.assetid for r in MeasuringTool.query.with_entities(MeasuringTool.assetid)} + rows = AssetRelationship.query.filter( + AssetRelationship.sourceassetid == pc.assetid, + AssetRelationship.relationshiptypeid == controls.relationshiptypeid, + AssetRelationship.isactive.is_(True)).all() + return [Asset.query.get(r.targetassetid).assetnumber + for r in rows if r.targetassetid in toolids] + + +def test_a_named_instrument_is_adopted_not_minted(client, db, rig, collector_key): + """measuringtool-id.txt names the instrument; nothing new is created.""" + _tool(db, 'MT-6227') + assert _report(client, collector_key, 'FB7FB1V3', + measuringtoolid='MT-6227').status_code in (200, 201) + + assert _tools_controlled('FB7FB1V3') == ['MT-6227'] + assert Asset.query.filter_by(assetnumber='FB7FB1V3-CMM').first() is None + + +def test_a_tool_already_controlled_is_adopted(client, db, rig, collector_key): + """The legacy MT-#### case: linked by the import, invisible to the collector. + + The link must pre-date the first report - that is the real sequence, and it + is the whole point. Reporting first would mint the twin before adoption ever + had a chance, which is exactly the bug. + """ + from plugins.computers.models import Computer + tool = _tool(db, 'MT-6224') + computertype = AssetType.query.filter_by(assettype='computer').first() + pc = Asset(assetnumber='FJPX1GT3', assettypeid=computertype.assettypeid) + db.session.add(pc) + db.session.flush() + db.session.add(Computer(assetid=pc.assetid, hostname='FJPX1GT3')) + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + db.session.add(AssetRelationship( + sourceassetid=pc.assetid, targetassetid=tool.assetid, + relationshiptypeid=controls.relationshiptypeid, label=None)) + db.session.commit() + + assert _report(client, collector_key, 'FJPX1GT3').status_code in (200, 201) + assert _tools_controlled('FJPX1GT3') == ['MT-6224'] + assert Asset.query.filter_by(assetnumber='FJPX1GT3-CMM').first() is None + + +def test_the_reported_bay_id_is_adopted(client, db, rig, collector_key): + """The CMM case: cmmid.txt already reports CMM4, which is a real asset.""" + _tool(db, 'CMM4') + assert _report(client, collector_key, 'FB7FB1V3', + machinenumber='CMM4').status_code in (200, 201) + + assert 'CMM4' in _tools_controlled('FB7FB1V3') + assert Asset.query.filter_by(assetnumber='FB7FB1V3-CMM').first() is None + + +def test_minting_still_happens_when_nothing_exists(client, db, rig, collector_key): + """A genuinely new instrument has no identity to adopt - last resort.""" + assert _report(client, collector_key, 'FNEWPC01').status_code in (200, 201) + assert _tools_controlled('FNEWPC01') == ['FNEWPC01-CMM'] + + +def test_a_named_instrument_that_does_not_exist_warns_and_mints_nothing_wrong( + client, db, rig, collector_key): + """A typo must not mint a phantom instrument nobody can account for.""" + resp = _report(client, collector_key, 'FTYPO001', measuringtoolid='MT-NOPE') + assert resp.status_code in (200, 201) + body = resp.get_json()['data'] + assert any('MT-NOPE' in w for w in body.get('warnings', [])) + + +def test_adoption_is_idempotent(client, db, rig, collector_key): + _tool(db, 'MT-6331') + for _ in range(3): + _report(client, collector_key, 'F9LQSDB4', measuringtoolid='MT-6331') + assert _tools_controlled('F9LQSDB4') == ['MT-6331'] + + +def test_a_named_instrument_supersedes_a_minted_twin( + client, db, rig, collector_key): + """The migration case: the twin already exists when the file names the real + one. + + This is the state prod is actually in - 43 minted twins already linked - so + adoption that only works on a clean PC fixes nothing. The twin's link has to + be archived, or the PC reads as controlling two instruments. + """ + _tool(db, 'MT-6227') + _report(client, collector_key, 'FB7FB1V3') + assert _tools_controlled('FB7FB1V3') == ['FB7FB1V3-CMM'] + + _report(client, collector_key, 'FB7FB1V3', measuringtoolid='MT-6227') + assert _tools_controlled('FB7FB1V3') == ['MT-6227'] + + +def test_a_named_id_that_is_not_a_measuring_tool_is_refused( + client, db, rig, collector_key): + """A machine number in measuringtool-id.txt must not link the machine. + + The name resolving is not enough: linked under the measuring-tool label it + would read as an instrument everywhere downstream, on a row the machine sync + also owns. + """ + machinetype = AssetType.query.filter_by(assettype='machine').first() + if machinetype is None: + machinetype = AssetType(assettype='machine') + db.session.add(machinetype) + db.session.flush() + db.session.add(Asset(assetnumber='2335', assettypeid=machinetype.assettypeid)) + db.session.commit() + + resp = _report(client, collector_key, 'FTYPO777', measuringtoolid='2335') + assert resp.status_code in (200, 201) + body = resp.get_json()['data'] + assert any('2335' in w for w in body.get('warnings', [])) + assert '2335' not in _all_controlled('FTYPO777') + + +def test_a_cmm_reporting_its_bay_survives_a_second_cycle( + client, db, rig, collector_key): + """The instrument IS the reported bay, so both syncs want the same triple. + + Adoption used to relabel the machine sync's row. The machine sync finds its + rows BY that label, so cycle 2 built a second row for the same + (pc, tool, controls) triple and MySQL rejected it on the unique constraint: + 200, then 500 on every report after. The machine link must survive as a + machine link. + """ + _tool(db, 'CMM4') + for _ in range(3): + resp = _report(client, collector_key, 'FB7FB1V3', machinenumber='CMM4') + assert resp.status_code in (200, 201) + + assert _all_controlled('FB7FB1V3') == ['CMM4'] + assert Asset.query.filter_by(assetnumber='FB7FB1V3-CMM').first() is None + + pc = Asset.query.filter_by(assetnumber='FB7FB1V3').first() + tool = Asset.query.filter_by(assetnumber='CMM4').first() + rows = AssetRelationship.query.filter_by( + sourceassetid=pc.assetid, targetassetid=tool.assetid).all() + assert len(rows) == 1 + assert rows[0].label == 'collector:machine' + + +def test_a_reported_identifier_is_not_a_wildcard(client, db, rig, collector_key): + """`_` and `%` in a reported identifier must match themselves, nothing else. + + These values come from a text file on a shopfloor PC. Resolved with ilike, + 'MT-600_' matched MT-6001 and a bare '%' matched whatever active asset came + first - an identifier resolving to an asset it does not name, then linked to + as though it did. + """ + _tool(db, 'MT-6001') + resp = _report(client, collector_key, 'FWILD001', measuringtoolid='MT-600_') + assert resp.status_code in (200, 201) + assert 'MT-6001' not in _tools_controlled('FWILD001') + + resp = _report(client, collector_key, 'FWILD002', measuringtoolid='%') + assert resp.status_code in (200, 201) + assert 'MT-6001' not in _tools_controlled('FWILD002') + + +def _all_controlled(hostname): + """Every active controls target, tool or not - the refusal cases need to see + a link the tool-only helper would filter out.""" + pc = Asset.query.filter_by(assetnumber=hostname).first() + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + rows = AssetRelationship.query.filter( + AssetRelationship.sourceassetid == pc.assetid, + AssetRelationship.relationshiptypeid == controls.relationshiptypeid, + AssetRelationship.isactive.is_(True)).all() + targets = [r.targetassetid for r in rows] + if not targets: + return [] + return sorted(a.assetnumber for a in + Asset.query.filter(Asset.assetid.in_(targets)).all()) diff --git a/tests/test_plugins/test_collector_partmarker_links.py b/tests/test_plugins/test_collector_partmarker_links.py new file mode 100644 index 0000000..151acc2 --- /dev/null +++ b/tests/test_plugins/test_collector_partmarker_links.py @@ -0,0 +1,97 @@ +"""A part-marker PC keeps reporting after the first cycle. + +Both links a part-marker PC produces are rows in assetrelationships, which is +unique on (source, target, type). Both were looked up by LABEL, so a row made by +hand or by the legacy import was invisible and the insert violated the +constraint; and the minted marker's asset number is derived from the PC, so it +could already be taken. Either way the PC reported 200 on the cycle that minted +the marker and 500 on every cycle after - the same shape as the CMM adoption bug +in test_collector_measuringtool_adoption.py. +""" + +import pytest + +from shopdb.core.models import Asset, AssetType +from shopdb.core.models.relationship import RelationshipType, AssetRelationship + +KEY = 'partmarker-links-key' + + +@pytest.fixture +def collector_key(app): + old = app.config.get('COLLECTOR_API_KEY') + app.config['COLLECTOR_API_KEY'] = KEY + yield KEY + app.config['COLLECTOR_API_KEY'] = old + + +@pytest.fixture +def rig(db): + for name in ('computer', 'machine'): + if not AssetType.query.filter_by(assettype=name).first(): + db.session.add(AssetType(assettype=name)) + for name in ('controls', 'partof'): + if not RelationshipType.query.filter_by(relationshiptype=name).first(): + db.session.add(RelationshipType(relationshiptype=name)) + db.session.commit() + + +def _report(client, key, hostname, **extra): + payload = {'hostname': hostname, 'pctype': 'gea-shopfloor-partmarker'} + payload.update(extra) + return client.post('/api/collector/computers', json=payload, + headers={'X-API-Key': key}) + + +def _operation(db, assetnumber): + machinetype = AssetType.query.filter_by(assettype='machine').first() + asset = Asset(assetnumber=assetnumber, assettypeid=machinetype.assettypeid) + db.session.add(asset) + db.session.commit() + return asset + + +def test_repeat_cycles_do_not_500(client, db, rig, collector_key): + _operation(db, '0613') + codes = [_report(client, collector_key, 'FMARK001', machinenumber='0613').status_code + for _ in range(3)] + assert codes == [codes[0]] * 3, codes + assert codes[0] in (200, 201), codes + + +def test_a_partof_row_under_another_label_is_not_duplicated( + client, db, rig, collector_key): + """A partof row made by hand or by the legacy import carries no label.""" + operation = _operation(db, '0615') + # First cycle mints the marker and links it. + assert _report(client, collector_key, 'FMARK002', + machinenumber='0615').status_code in (200, 201) + marker = Asset.query.filter_by(assetnumber='FMARK002-PARTMARKER').first() + assert marker is not None + partof = RelationshipType.query.filter_by(relationshiptype='partof').first() + row = AssetRelationship.query.filter_by( + sourceassetid=marker.assetid, targetassetid=operation.assetid, + relationshiptypeid=partof.relationshiptypeid).first() + assert row is not None + # Somebody relabels it (or it came from the import unlabelled). + row.label = None + db.session.commit() + + resp = _report(client, collector_key, 'FMARK002', machinenumber='0615') + assert resp.status_code in (200, 201), resp.status_code + rows = AssetRelationship.query.filter_by( + sourceassetid=marker.assetid, targetassetid=operation.assetid, + relationshiptypeid=partof.relationshiptypeid).all() + assert len(rows) == 1 + + +def test_a_taken_marker_asset_number_does_not_500(client, db, rig, collector_key): + """The minted number is derived from the PC, so it can already exist.""" + machinetype = AssetType.query.filter_by(assettype='machine').first() + db.session.add(Asset(assetnumber='FMARK003-PARTMARKER', + assettypeid=machinetype.assettypeid)) + db.session.commit() + _operation(db, '0617') + + resp = _report(client, collector_key, 'FMARK003', machinenumber='0617') + assert resp.status_code in (200, 201), resp.status_code