From f0cc4d123c101759e68fe2f2f733240bd773db3d Mon Sep 17 00:00:00 2001 From: "Jokob @NetAlertX" <96159884+jokob-sk@users.noreply.github.com> Date: Thu, 28 May 2026 05:31:09 +0000 Subject: [PATCH] Enhance carbon-copy behavior to prevent overwriting devPresentLastScan in sync operations #1651 --- front/plugins/sync/sync.py | 16 +++++++++++++--- test/db_test_helpers.py | 3 ++- test/plugins/test_sync_protocol.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/front/plugins/sync/sync.py b/front/plugins/sync/sync.py index dc464d52..e98de315 100755 --- a/front/plugins/sync/sync.py +++ b/front/plugins/sync/sync.py @@ -326,12 +326,22 @@ def main(): placeholders = ', '.join('?' for _ in insert_cols) if sync_behavior == 'carbon-copy': - # UPSERT: on MAC conflict update all columns except devMac. + # UPSERT: on MAC conflict update all columns except devMac and + # devPresentLastScan. # devMac is the PRIMARY KEY so it is excluded from the SET clause. - # NOTE: this raw SQL bypasses can_overwrite_field() — ALL fields + # devPresentLastScan is excluded to prevent a node's offline report + # from clobbering the hub's own scan result: if a device is online + # on the hub network but offline on a node, the raw UPSERT would + # flip devPresentLastScan = 0 every sync cycle, triggering + # Connected/Disconnected events on each scan and causing the device + # to be flagged as Flapping. Presence is owned by + # update_presence_from_CurrentScan(); the carbon-copy path respects + # that contract by leaving devPresentLastScan to the normal pipeline. + # NOTE: this raw SQL bypasses can_overwrite_field() — ALL other fields # including USER/LOCKED-sourced ones are overwritten. Node is fully # authoritative in this mode. - update_cols = [col for col in insert_cols if col != 'devMac'] + _CARBON_COPY_SKIP = {'devMac', 'devPresentLastScan'} + update_cols = [col for col in insert_cols if col not in _CARBON_COPY_SKIP] update_clause = ', '.join(f'{col}=excluded.{col}' for col in update_cols) sql = ( f'INSERT INTO Devices ({columns}) VALUES ({placeholders}) ' diff --git a/test/db_test_helpers.py b/test/db_test_helpers.py index 7af01c4a..c61d2ec9 100644 --- a/test/db_test_helpers.py +++ b/test/db_test_helpers.py @@ -351,7 +351,8 @@ def sync_insert_devices( placeholders = ", ".join("?" for _ in insert_cols) if behavior == "carbon-copy": - update_cols = [col for col in insert_cols if col != "devMac"] + _CARBON_COPY_SKIP = {"devMac", "devPresentLastScan"} + update_cols = [col for col in insert_cols if col not in _CARBON_COPY_SKIP] update_clause = ", ".join(f"{col}=excluded.{col}" for col in update_cols) sql = ( f"INSERT INTO Devices ({columns}) VALUES ({placeholders}) " diff --git a/test/plugins/test_sync_protocol.py b/test/plugins/test_sync_protocol.py index 4b5e443e..387cd211 100644 --- a/test/plugins/test_sync_protocol.py +++ b/test/plugins/test_sync_protocol.py @@ -668,6 +668,36 @@ class TestSyncBehavior: cur.execute("SELECT COUNT(*) AS cnt FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",)) assert cur.fetchone()["cnt"] == 1 + def test_carbon_copy_does_not_overwrite_devPresentLastScan(self, conn): + """Regression: carbon-copy must NOT clobber devPresentLastScan. + + Scenario: device is online on the hub (devPresentLastScan=1) but the + node reports it as offline (devPresentLastScan=0). Without the fix the + UPSERT would flip presence to 0, triggering a Device Down event on the + next scan cycle and a Connected event on the scan after that, causing + the device to accumulate enough churn events to be flagged as Flapping. + """ + cur = conn.cursor() + # Hub already knows this device and currently sees it as online. + cur.execute( + "INSERT INTO Devices (devMac, devName, devPresentLastScan) VALUES (?, ?, ?)", + ("aa:bb:cc:dd:ee:01", "HubDevice", 1), + ) + conn.commit() + + # Node reports same MAC as offline. + device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devPresentLastScan=0) + sync_insert_devices(conn, [device], behavior="carbon-copy") + + cur.execute( + "SELECT devPresentLastScan FROM Devices WHERE devMac = ?", + ("aa:bb:cc:dd:ee:01",), + ) + row = cur.fetchone() + assert row["devPresentLastScan"] == 1, ( + "carbon-copy must not overwrite devPresentLastScan with a node's offline value" + ) + # ------------------------------------------------------------------ # hub-defaults — no direct write, hub pipeline handles it # ------------------------------------------------------------------