From 44c52a3cf4968c8f3daf656723578909f9b5f65e Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Thu, 10 Sep 2026 14:27:44 +1000 Subject: [PATCH] BE: conditional device import support, notification supression support during scan #1721 --- .claude/skills/scan-pipeline/SKILL.md | 4 +- .gemini/skills/scan-pipeline/SKILL.md | 4 +- .github/skills/scan-pipeline/SKILL.md | 4 +- docs/NOTIFICATIONS.md | 2 +- docs/PLUGINS_DEV_DATA_CONTRACT.md | 14 ++--- server/scan/device_handling.py | 8 ++- server/scan/session_events.py | 30 +++++++--- test/scan/test_scan_notification_mode.py | 71 ++++++++++++++++++++++++ test/scan/test_scan_presence.py | 45 +++++++++++++++ 9 files changed, 162 insertions(+), 20 deletions(-) diff --git a/.claude/skills/scan-pipeline/SKILL.md b/.claude/skills/scan-pipeline/SKILL.md index f271a53ae..ab0eff0e1 100644 --- a/.claude/skills/scan-pipeline/SKILL.md +++ b/.claude/skills/scan-pipeline/SKILL.md @@ -51,7 +51,9 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. 2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. -3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. +3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. + +**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. ## When to read this vs. other docs/skills diff --git a/.gemini/skills/scan-pipeline/SKILL.md b/.gemini/skills/scan-pipeline/SKILL.md index 848e156c1..65352942d 100644 --- a/.gemini/skills/scan-pipeline/SKILL.md +++ b/.gemini/skills/scan-pipeline/SKILL.md @@ -51,7 +51,9 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. 2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. -3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. +3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. + +**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. ## When to read this vs. other docs/skills diff --git a/.github/skills/scan-pipeline/SKILL.md b/.github/skills/scan-pipeline/SKILL.md index 1c84d3e2f..6e455886c 100644 --- a/.github/skills/scan-pipeline/SKILL.md +++ b/.github/skills/scan-pipeline/SKILL.md @@ -51,7 +51,9 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. 2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. -3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. +3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle. + +**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale. ## When to read this vs. other docs/skills diff --git a/docs/NOTIFICATIONS.md b/docs/NOTIFICATIONS.md index 574ebb7a1..7c992e83d 100755 --- a/docs/NOTIFICATIONS.md +++ b/docs/NOTIFICATIONS.md @@ -20,7 +20,7 @@ There are 4 ways how to influence notifications: The following device properties influence notifications. You can: -1. **Alert Events** - Enables alerts of connections, disconnections, IP changes (down and down reconnected notifications are still sent even if this is disabled). +1. **Alert Events** - Enables alerts of disconnections and IP changes (down and down reconnected notifications are still sent even if this is disabled). **Note:** a device *reconnecting* (coming back online without having triggered a Down alert first) currently always generates a notification regardless of this setting — Alert Events does not gate that case today, only disconnections and IP changes. 2. **Alert Down** - Alerts when a device goes down. This setting overrides a disabled **Alert Events** setting, so you will get a notification of a device going down even if you don't have **Alert Events** ticked. Disabling this will disable down and down reconnected notifications on the device. 3. **Can Sleep** - Marks the device as sleep-capable (e.g. a battery-powered sensor that deep-sleeps between readings). When enabled, offline periods within the **Alert down after (sleep)** (`NTFPRCS_sleep_time`) global window are shown as **Sleeping** (aqua badge 🌙) instead of **Down**, and no down alert is fired during that window. Once the window expires the device falls back to normal down-alert logic. ⚠ Requires **Alert Down** to be enabled — sleeping suppresses the alert during the window only. 4. **Skip repeated notifications**, if for example you know there is a temporary issue and want to pause the same notification for this device for a given time. diff --git a/docs/PLUGINS_DEV_DATA_CONTRACT.md b/docs/PLUGINS_DEV_DATA_CONTRACT.md index e278d1c6c..7493e1b8c 100644 --- a/docs/PLUGINS_DEV_DATA_CONTRACT.md +++ b/docs/PLUGINS_DEV_DATA_CONTRACT.md @@ -155,7 +155,7 @@ CREATE TABLE CurrentScan ( ) ``` -As the documentation might become outdated, it's good practice to check the latest definition of the `CurrentScan` table in `server/db/db_upgrade.py`'s `ensure_CurrentScan()` (the version that actually runs) in the code base — not `app.sql`, which is a reference-only copy the running application never loads. +As the documentation might become outdated, it's good practice to check the latest definition of the `CurrentScan` table in `server/db/db_upgrade.py`'s `ensure_CurrentScan()` (the version that actually runs) in the code base. `app.sql` bootstraps the schema for every fresh install, but `CurrentScan` is one of the few tables `ensure_CurrentScan()` unconditionally drops and recreates on every startup, so `app.sql`'s copy of it never actually persists. ### Import Behavior Columns @@ -167,13 +167,13 @@ Three optional `CurrentScan` columns, all independent of each other, control wha | `scanNotificationMode` | text (`normal` \| `quiet`) | `normal` | Whether creating/reconnecting this device should dispatch a notification. `quiet` still writes the `Events` row (audit trail intact) but suppresses the outbound email/push. Creation-time-only: it seeds `devAlertDown`/`devAlertEvents` to `0` on the device when it's first created, rather than being an ongoing, per-cycle re-evaluated policy — reclassifying a plugin's row later does not retroactively change an already-created device's alert settings. | | `scanPresence` | boolean | `1` | Whether this row asserts the device is *currently online*. `0` means "identity/inventory data, no presence claim" — not "offline". A reservation, a lease record, or a static IPAM entry are typical `0` cases. | -**Fallback for missing/invalid values** +**Missing vs. invalid values — these behave differently, not interchangeably:** -| Column | Missing/invalid value → | -|---|---| -| `scanCreatesDevice` | `1` (create) | -| `scanNotificationMode` | `normal` | -| `scanPresence` | `1` (asserts presence) | +| Column | Column never mapped (missing) | Mapped but sent an unexpected value (invalid) | +|---|---|---| +| `scanCreatesDevice` | `1` (schema `DEFAULT`) | `CHECK (scanCreatesDevice IN (0, 1))` — anything else fails the `INSERT` outright, it does not silently fall back to `1` | +| `scanNotificationMode` | `normal` (schema `DEFAULT`) | No `CHECK` constraint — any string other than the literal `'quiet'` is treated as `normal`, since the SQL only special-cases that exact value | +| `scanPresence` | `1` (schema `DEFAULT`) | `CHECK (scanPresence IN (0, 1))` — same as `scanCreatesDevice`, invalid values fail the `INSERT`, they don't default | **Multiple plugins reporting the same MAC in the same scan cycle** (the normal case, not an edge case — see the `scan-pipeline` skill) resolve per column, not uniformly: `scanCreatesDevice` and `scanPresence` are most-permissive-wins (any row saying `1` wins), while `scanNotificationMode` is most-*restrictive*-wins (any row saying `quiet` suppresses the notification, even if a sibling row says `normal`) — erring toward under-notifying rather than spamming. diff --git a/server/scan/device_handling.py b/server/scan/device_handling.py index 2d7efe0d8..14949a6a7 100755 --- a/server/scan/device_handling.py +++ b/server/scan/device_handling.py @@ -231,7 +231,12 @@ def update_presence_from_CurrentScan(db): def update_devLastConnection_from_CurrentScan(db): """ - Update devLastConnection to current time for all devices seen in CurrentScan. + Update devLastConnection to current time for devices with a presence- + asserting CurrentScan row (scanPresence = 1) - a row that only carries + identity/inventory data (scanPresence = 0) must not make an offline + device look recently connected. Same predicate as + update_presence_from_CurrentScan(); found missing this check during + review of a shipped commit - see scan-pipeline-hardening.md. """ sql = db.sql startTime = timeNowUTC() @@ -243,6 +248,7 @@ def update_devLastConnection_from_CurrentScan(db): WHERE EXISTS ( SELECT 1 FROM CurrentScan WHERE devMac = scanMac + AND scanPresence = 1 ) """) diff --git a/server/scan/session_events.py b/server/scan/session_events.py index bda637d83..bf8fc95d1 100755 --- a/server/scan/session_events.py +++ b/server/scan/session_events.py @@ -265,18 +265,32 @@ def insert_events(db): # Check IP Changed mylog("debug", "[Events] - 4 - IP Changes") + # Unlike Device Down/Disconnected (which fire on row *absence* and have no + # live scanNotificationMode to read), IP Changed fires from a present row - + # a live value is available, so quiet is consulted here too, additively: + # suppress if EITHER the live aggregate says quiet OR the device's own + # devAlertEvents is off. Doesn't replace the user's toggle, only adds a + # plugin-level reason to suppress on top of it - most-restrictive-wins, + # same as the New Connections query's aggregate. sql.execute(f"""INSERT OR IGNORE INTO Events (eveMac, eveIp, eveDateTime, eveEventType, eveAdditionalInfo, evePendingAlertEmail) - SELECT scanMac, scanLastIP, '{startTime}', 'IP Changed', - 'Previous IP: '|| devLastIP, devAlertEvents + SELECT CurrentScan.scanMac, CurrentScan.scanLastIP, '{startTime}', 'IP Changed', + 'Previous IP: '|| devLastIP, + CASE WHEN agg.scanQuiet = 1 THEN 0 ELSE devAlertEvents END FROM Devices, CurrentScan - WHERE devMac = scanMac - AND scanLastIP IS NOT NULL - AND scanLastIP NOT IN ({NULL_EQUIVALENTS_SQL}) - AND scanLastIP <> COALESCE(devPrimaryIPv4, '') - AND scanLastIP <> COALESCE(devPrimaryIPv6, '') - AND scanLastIP <> COALESCE(devLastIP, '') """) + JOIN ( + SELECT scanMac, + MAX(CASE WHEN scanNotificationMode = 'quiet' THEN 1 ELSE 0 END) AS scanQuiet + FROM CurrentScan + GROUP BY scanMac + ) agg ON agg.scanMac = CurrentScan.scanMac + WHERE devMac = CurrentScan.scanMac + AND CurrentScan.scanLastIP IS NOT NULL + AND CurrentScan.scanLastIP NOT IN ({NULL_EQUIVALENTS_SQL}) + AND CurrentScan.scanLastIP <> COALESCE(devPrimaryIPv4, '') + AND CurrentScan.scanLastIP <> COALESCE(devPrimaryIPv6, '') + AND CurrentScan.scanLastIP <> COALESCE(devLastIP, '') """) mylog("debug", "[Events] - Events end") diff --git a/test/scan/test_scan_notification_mode.py b/test/scan/test_scan_notification_mode.py index d9d6585f7..a04a3841a 100644 --- a/test/scan/test_scan_notification_mode.py +++ b/test/scan/test_scan_notification_mode.py @@ -199,3 +199,74 @@ class TestQuietReconnection: ).fetchall() assert len(rows) == 1 assert rows[0]["evePendingAlertEmail"] == 0 + + +class TestQuietSuppressesIpChangedToo: + """IP Changed fires from a present CurrentScan row (unlike Device Down/ + Disconnected, which fire on absence and have no live value to read) - a + live scanNotificationMode check is possible here and, per the decision + recorded in the PRD's open issue, should apply additively on top of the + device's own devAlertEvents toggle.""" + + def test_quiet_row_suppresses_ip_changed(self): + conn = make_db() + insert_device(conn, MAC, alert_down=1, last_ip="192.168.1.10") + conn.execute("UPDATE Devices SET devAlertEvents = 1 WHERE devMac = ?", (MAC,)) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict( + MAC, scanLastIP="192.168.1.99", scanNotificationMode="quiet" + ) + ) + db = DummyDB(conn) + + insert_events(db) + + row = conn.execute( + "SELECT evePendingAlertEmail FROM Events WHERE eveMac = ? AND eveEventType = 'IP Changed'", + (MAC,), + ).fetchone() + assert row is not None, "IP Changed event should still be logged (audit trail intact)" + assert row["evePendingAlertEmail"] == 0 + + def test_normal_row_with_alert_events_on_does_not_suppress(self): + """Regression guard: existing devAlertEvents-driven behavior unchanged.""" + conn = make_db() + insert_device(conn, MAC, alert_down=1, last_ip="192.168.1.10") + conn.execute("UPDATE Devices SET devAlertEvents = 1 WHERE devMac = ?", (MAC,)) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict( + MAC, scanLastIP="192.168.1.99", scanNotificationMode="normal" + ) + ) + db = DummyDB(conn) + + insert_events(db) + + row = conn.execute( + "SELECT evePendingAlertEmail FROM Events WHERE eveMac = ? AND eveEventType = 'IP Changed'", + (MAC,), + ).fetchone() + assert row is not None + assert row["evePendingAlertEmail"] == 1 + + def test_devalertevents_off_still_suppresses_regardless_of_quiet(self): + """The plugin-level quiet check is additive, not a replacement for + the user's own toggle.""" + conn = make_db() + insert_device(conn, MAC, alert_down=1, last_ip="192.168.1.10") + conn.execute("UPDATE Devices SET devAlertEvents = 0 WHERE devMac = ?", (MAC,)) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict( + MAC, scanLastIP="192.168.1.99", scanNotificationMode="normal" + ) + ) + db = DummyDB(conn) + + insert_events(db) + + row = conn.execute( + "SELECT evePendingAlertEmail FROM Events WHERE eveMac = ? AND eveEventType = 'IP Changed'", + (MAC,), + ).fetchone() + assert row is not None + assert row["evePendingAlertEmail"] == 0 diff --git a/test/scan/test_scan_presence.py b/test/scan/test_scan_presence.py index 25e5c5f22..1d9dd14ea 100644 --- a/test/scan/test_scan_presence.py +++ b/test/scan/test_scan_presence.py @@ -83,6 +83,51 @@ class TestPresenceGateOnExistingDevice: assert _present(db, MAC) == 1, "a contradicting presence=1 row must still win" +class TestDevLastConnectionRespectsPresence: + """update_devLastConnection_from_CurrentScan() - found missing this check + during review of a shipped commit: an inventory/reservation row was + making offline devices look recently connected.""" + + def test_presence_zero_does_not_bump_last_connection(self): + conn = make_db() + insert_device( + conn, MAC, alert_down=1, present_last_scan=0, + last_connection="2020-01-01 00:00:00", + ) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=0) + ) + db = DummyDB(conn) + + device_handling.update_devLastConnection_from_CurrentScan(db) + + row = conn.execute( + "SELECT devLastConnection FROM Devices WHERE devMac = ?", (MAC,) + ).fetchone() + assert row["devLastConnection"] == "2020-01-01 00:00:00", ( + "a scanPresence=0 row must not make an offline device look recently connected" + ) + + def test_presence_one_still_bumps_last_connection(self): + """Regression guard: default (1) preserves today's behavior.""" + conn = make_db() + insert_device( + conn, MAC, alert_down=1, present_last_scan=0, + last_connection="2020-01-01 00:00:00", + ) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=1) + ) + db = DummyDB(conn) + + device_handling.update_devLastConnection_from_CurrentScan(db) + + row = conn.execute( + "SELECT devLastConnection FROM Devices WHERE devMac = ?", (MAC,) + ).fetchone() + assert row["devLastConnection"] != "2020-01-01 00:00:00" + + class TestNewConnectionsRespectsPresence: """insert_events()'s New Connections query must not fire Connected for a scanPresence=0-only row - this is what would otherwise leave a session