collector: adopt an instrument before minting one, and stop the 500 loops
Minting derived a measuring tool's 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. And because idempotency was keyed on the collector's own label, it could not see a tool it had not created - on prod that left 43 legacy MT-#### tools shadowed by minted <HOST>-CMM twins, three records deep in places. Resolution order is now most-stable-identity-first: the instrument named by measuringtool-id.txt, then a prior collector link, then a tool this PC already controls that somebody else created, then the reported machine number, and only then mint. What minting produces should be read as a placeholder until a real identifier is recorded. Three separate 500 loops came out of the same mistake, looking a relationship up by LABEL when assetrelationships is unique on (source, target, type): - On a CMM the instrument IS the reported bay, so the machine sync has already made a row for that exact triple - and it finds its own rows by that label. Relabelling hid the link, so the next cycle built a second row for the same triple and MySQL rejected it: 200 once, then 500 forever. The machine link is now recognised and left alone; adoption only needs the identity. - A part-marker PC hit it twice over, once on its partof row and once because the marker's asset number is derived from the PC and could already be taken. Both are get-or-create on the triple now, and an existing asset of that number is adopted rather than duplicated. A named instrument also supersedes a minted twin properly: the stale link is archived by TARGET, not by object identity, which is what left a PC reading as the controller of two instruments. Reported identifiers are matched exactly rather than with ilike. They arrive from a text file on a shopfloor PC, and ilike reads _ and % as wildcards, so MT-600_ adopted MT-6001 and a bare % adopted whatever active asset came first. A named id that is not a measuring tool is refused with a warning rather than linked as one.
This commit is contained in:
cproudlock
232
tests/test_plugins/test_collector_measuringtool_adoption.py
Normal file
232
tests/test_plugins/test_collector_measuringtool_adoption.py
Normal file
@@ -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
|
||||
<HOST>-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())
|
||||
97
tests/test_plugins/test_collector_partmarker_links.py
Normal file
97
tests/test_plugins/test_collector_partmarker_links.py
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user