Fix numeric natural keys resolving to the wrong asset on import

shopdb/core/api/settings.py

@@ -334,23 +334,32 @@ def download_import_template(tablename):
 
 
 def _existing_hostname_map():
-    """Case-insensitive hostname -> assetnumber map for existing computers.
+    """Case-insensitive hostname -> assetid map for existing computers.
 
-    Azure device-group exports align on hostname (their displayName), while the
-    import engine keys assets on assetnumber. This lets the Azure adapter route
-    a matched device to its real asset so the import updates in place.
+    Azure and Thor exports align on hostname (their displayName), while the
+    import engine keys assets on assetnumber. This lets those adapters route a
+    matched device to its real asset so the import updates in place.
+
+    THE ID, NOT THE ASSETNUMBER, and that is not a style choice. The resolver
+    takes any all-digit value at face value as a literal id rather than looking
+    it up, so an assetnumber like `0600` resolved to asset 600 - a different
+    asset entirely, or none. This site numbers operations 0NNN, so those
+    assetnumbers are real and common: one of them pointed a PC at a printer's
+    record, and another at an id that does not exist, which is the foreign-key
+    error an Azure import died on. Handing over the id the resolver is going to
+    treat it as makes the passthrough correct by construction.
     """
     assets = db.metadata.tables['assets']
     computers = db.metadata.tables['computers']
     query = sa_select(
-        computers.c.hostname, assets.c.assetnumber
+        computers.c.hostname, assets.c.assetid
     ).select_from(
         computers.join(assets, computers.c.assetid == assets.c.assetid)
     ).where(computers.c.hostname.isnot(None))
     mapping = {}
-    for hostname, assetnumber in db.session.execute(query):
+    for hostname, assetid in db.session.execute(query):
         if hostname:
-            mapping[hostname.strip().lower()] = assetnumber
+            mapping[hostname.strip().lower()] = assetid
     return mapping
 
 

shopdb/core/services/csvimport.py

@@ -427,12 +427,41 @@ class Resolver:
     def resolve(self, tablename, idcolumn, value):
         if value is None:
             return None
-        # A number is taken at face value: exports round-trip, and some sites do
-        # know their ids.
-        if isinstance(value, int) or (isinstance(value, str) and value.strip().isdigit()):
+        # An int is an id. It cannot be anything else.
+        if isinstance(value, int):
             return int(value)
 
         label = _label_column(tablename)
+
+        # An ALL-DIGIT STRING IS AMBIGUOUS, and used to be taken at face value
+        # as an id. That is wrong wherever a site's natural keys are numeric,
+        # and this one numbers operations 0NNN: assetnumber `0600` was read as
+        # id 600, which is a different asset - a printer, in the case that
+        # found this - or none at all, which surfaced as a foreign-key error
+        # partway through an Azure device import.
+        #
+        # The label wins when one matches. Somebody writing `0600` in a column
+        # keyed on assetnumber means the asset called 0600, and silently
+        # binding a row to an unrelated record is far worse than the error.
+        # Fall through to the id reading only when no label matches, which
+        # keeps round-tripped exports and sites that do know their ids working.
+        if isinstance(value, str) and value.strip().isdigit():
+            if label:
+                key = (tablename, value.strip().lower())
+                if key in self._cache:
+                    return self._cache[key]
+                table = db.metadata.tables[tablename]
+                row = db.session.execute(
+                    table.select().where(
+                        db.func.lower(db.func.trim(table.c[label]))
+                        == value.strip().lower())
+                ).first()
+                if row is not None:
+                    found = getattr(row, idcolumn)
+                    self._cache[key] = found
+                    return found
+            return int(value)
+
         if not label:
             raise ValueError(
                 "cannot look up %s by name - give the numeric %s instead" % (tablename, idcolumn))

tests/test_csv_import.py

@@ -335,3 +335,29 @@ def test_reconcile_still_binds_normalized_exact(db):
     _db.session.rollback()
 
 
+
+
+def test_a_numeric_assetnumber_is_read_as_a_name_not_an_id(db):
+    """This site numbers operations 0NNN, so numeric natural keys are real.
+
+    An all-digit string used to be taken at face value as an id. `0600` became
+    id 600 - a different asset, or none - which is how an Azure device import
+    died on a foreign key partway through, and how a PC could instead have been
+    bound silently to a printer's record. The label wins when one matches.
+    """
+    resolver = Resolver()
+    seed_reference(resolver)
+    import_csv('assets', 'assetnumber,assettypeid\n0600,Machine\n',
+               resolver=resolver, commit=True)
+    realid = _db.session.execute(_db.text(
+        "select assetid from assets where assetnumber = '0600'")).scalar()
+
+    # Force the ambiguity: an id that is NOT the one named 0600.
+    assert resolver.resolve('assets', 'assetid', '0600') == realid
+
+
+def test_a_number_naming_nothing_is_still_read_as_an_id(db):
+    """Round-tripped exports and sites that know their ids keep working."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    assert resolver.resolve('assets', 'assetid', '999999') == 999999