relationships: refuse links that cannot both be true, and report the ones already stored
Prod grew rows saying both "PC controls 2005" and "2005 controls PC", and a CMM PC showing "<- controls from CMM4" beside its own outgoing link. Only one direction can be true: a PC drives a machine, never the reverse. Nothing stopped it. The duplicate check was keyed on (source, target, type), so the inverse inserted cleanly, and the Add Relationship dialog offers an incoming direction that writes exactly that. The legacy import stores controls the wrong way round as well. Directional creates now refuse the reverse with a 409 naming the row that already holds it, and refuse self-links, which render as a duplicate on the asset's own page and mean nothing. Symmetric types are exempt: Dualpath stores both directions on purpose and the card collapses them. The propagation fan-out got the same guard so a rail meant to spread one direction across sibling bays cannot manufacture a pair. fix-controls-direction only matched source assettype 'machine', so every measuring_tool, printer and network_device row it was written to clean survived it - which is why running it would never have fixed the CMM. It now matches any non-computer controlled BY a computer. New `flask relationships audit` reports what is already stored: reciprocal pairs, self-links, and PCs controlling several assets of one type. Read-only, and it prints each row's label because that usually names the writer outright - collector:* means this code made it, anything else means a person or the import did. That distinction decides the fix for duplicate device assets, which is not in this commit: the collector keys idempotency on its own label, so a device somebody created by hand is invisible to it and it mints another, and the adoption rule needs the audit run against prod before it can be written. Two false positives were found writing it, against the dev database, and both would have made the report useless. A self-link is its own inverse, so it was counted as a reciprocal pair AND printed twice. And Dualpath siblings looked like duplicate devices - a dual-bay machine is one physical machine with one controller and controls is propagated to both bays deliberately. That was 30 of 32 findings, consecutive bay numbers pair by pair.
This commit is contained in:
100
tests/test_core/test_relationship_inverse_guard.py
Normal file
100
tests/test_core/test_relationship_inverse_guard.py
Normal file
@@ -0,0 +1,100 @@
|
||||
"""A directional relationship points ONE way.
|
||||
|
||||
Prod grew rows saying both `PC controls 2005` and `2005 controls PC`, and a CMM
|
||||
PC showing `<- controls from CMM4` beside its own outgoing link. Only one
|
||||
direction can be true - a PC drives a machine, never the reverse - but the
|
||||
duplicate check was keyed on (source, target, type), so the inverse inserted
|
||||
cleanly and both rows then rendered on both asset pages.
|
||||
|
||||
Symmetric types are exempt on purpose: they store both directions and the card
|
||||
collapses them to one entry per peer.
|
||||
"""
|
||||
|
||||
from shopdb.extensions import db
|
||||
from shopdb.core.models import Asset, AssetType
|
||||
from shopdb.core.models.relationship import RelationshipType
|
||||
|
||||
|
||||
def _asset(assetnumber):
|
||||
assettype = AssetType.query.filter_by(assettype='machine').first()
|
||||
if not assettype:
|
||||
assettype = AssetType(assettype='machine')
|
||||
db.session.add(assettype)
|
||||
db.session.flush()
|
||||
asset = Asset(assetnumber=assetnumber, assettypeid=assettype.assettypeid)
|
||||
db.session.add(asset)
|
||||
db.session.flush()
|
||||
return asset
|
||||
|
||||
|
||||
def _reltype(name, directional=True):
|
||||
existing = RelationshipType.query.filter_by(relationshiptype=name).first()
|
||||
if existing:
|
||||
existing.isdirectional = directional
|
||||
db.session.flush()
|
||||
return existing
|
||||
reltype = RelationshipType(relationshiptype=name, isdirectional=directional)
|
||||
db.session.add(reltype)
|
||||
db.session.flush()
|
||||
return reltype
|
||||
|
||||
|
||||
def _create(client, headers, source, target, typeid):
|
||||
return client.post('/api/assets/relationships',
|
||||
json={'sourceassetid': source, 'targetassetid': target,
|
||||
'relationshiptypeid': typeid},
|
||||
headers=headers)
|
||||
|
||||
|
||||
def test_inverse_of_a_directional_link_is_refused(client, db, auth_headers):
|
||||
pc, machine = _asset('PC-INV-1'), _asset('MACH-INV-1')
|
||||
controls = _reltype('controls')
|
||||
db.session.commit()
|
||||
|
||||
first = _create(client, auth_headers, pc.assetid, machine.assetid,
|
||||
controls.relationshiptypeid)
|
||||
assert first.status_code == 201, first.get_json()
|
||||
|
||||
inverse = _create(client, auth_headers, machine.assetid, pc.assetid,
|
||||
controls.relationshiptypeid)
|
||||
assert inverse.status_code == 409, inverse.get_json()
|
||||
message = inverse.get_json()['data']['error']['message'].lower()
|
||||
assert 'reverse' in message
|
||||
assert 'one way' in message
|
||||
|
||||
|
||||
def test_the_same_direction_is_still_refused_as_a_duplicate(client, db, auth_headers):
|
||||
pc, machine = _asset('PC-INV-2'), _asset('MACH-INV-2')
|
||||
controls = _reltype('controls')
|
||||
db.session.commit()
|
||||
|
||||
assert _create(client, auth_headers, pc.assetid, machine.assetid,
|
||||
controls.relationshiptypeid).status_code == 201
|
||||
again = _create(client, auth_headers, pc.assetid, machine.assetid,
|
||||
controls.relationshiptypeid)
|
||||
assert again.status_code == 409
|
||||
assert 'already exists' in again.get_json()['data']['error']['message'].lower()
|
||||
|
||||
|
||||
def test_symmetric_types_may_store_both_directions(client, db, auth_headers):
|
||||
"""Dualpath bays link both ways by design - the guard must not touch them."""
|
||||
baya, bayb = _asset('BAY-INV-A'), _asset('BAY-INV-B')
|
||||
dualpath = _reltype('Dualpath', directional=False)
|
||||
db.session.commit()
|
||||
|
||||
assert _create(client, auth_headers, baya.assetid, bayb.assetid,
|
||||
dualpath.relationshiptypeid).status_code == 201
|
||||
both = _create(client, auth_headers, bayb.assetid, baya.assetid,
|
||||
dualpath.relationshiptypeid)
|
||||
assert both.status_code == 201, both.get_json()
|
||||
|
||||
|
||||
def test_an_asset_cannot_relate_to_itself(client, db, auth_headers):
|
||||
lonely = _asset('SELF-INV-1')
|
||||
controls = _reltype('controls')
|
||||
db.session.commit()
|
||||
|
||||
resp = _create(client, auth_headers, lonely.assetid, lonely.assetid,
|
||||
controls.relationshiptypeid)
|
||||
assert resp.status_code == 400, resp.get_json()
|
||||
assert 'itself' in resp.get_json()['data']['error']['message'].lower()
|
||||
Reference in New Issue
Block a user