From 55f5d5e072ed23d5cb7c23fd2bb67ec727f9e20a Mon Sep 17 00:00:00 2001 From: Optic00 Date: Sat, 25 Jul 2026 17:18:04 +0200 Subject: [PATCH 1/2] fix(monitoring): harden ONT/CM/Cellular Disable/Enable toggle (review follow-ups) Pattern-wide robustness fixes from the adversarial review of the toggle, applied across all three device types. #1 Atomic anti-resurrection guard: UpdateXxxPollResultAsync now performs a single conditional ExecuteUpdate WHERE Id == id && Enabled instead of a read-then-SaveChanges, closing the TOCTOU where a Disable committing between the read and the write could still overwrite a paused row's frozen state. Repo tests moved from the EF InMemory provider to SQLite in-memory (InMemory cannot execute ExecuteUpdate) + a relational mid-poll-disable regression per type. #2 Disabled devices are no longer contacted or falsely reported on the service paths: PollCmAsync / PollModemAsync / the ONT manual poll short-circuit when the config is disabled (Cellular returns a clear 'disabled' message instead of a false 'polled successfully'), and the Cellular card skips auto-polling a disabled config on initial load and when paging between modems. The dashboard-card side of this (Refresh button state, server-side refresh guard, timer config reload) is already covered by 618c2e90 and is not duplicated here. The non-persisting Test/Probe buttons are unchanged. #3 Removed the redundant 'Enable polling' checkbox from the ONT/CM/Cellular edit forms (the row Disable/Enable toggle is the single control) and stopped SaveXxxConfigurationAsync from copying Enabled on update, so a stale open edit form can no longer resurrect a row the toggle just disabled. Starlink untouched. #4 A persistence exception is no longer swallowed into the same 'false' the guard uses for a disabled row: the poll success helpers rethrow, so a real DB write failure on an enabled device surfaces via the error path instead of silently dropping valid stats. Both call sites already run inside a catch that records the failure on the config, so the poll loops stay alive. --- .../Repositories/CmRepository.cs | 26 +++---- .../Repositories/ModemRepository.cs | 26 +++---- .../Repositories/OntRepository.cs | 26 +++---- .../Components/Pages/Settings.razor | 20 ------ .../Shared/CellularStatsPanel.razor | 4 +- .../Services/CableModemMonitorService.cs | 7 +- .../Services/CellularModemService.cs | 10 ++- .../Services/OntMonitorService.cs | 5 ++ .../CmRepositoryTests.cs | 68 ++++++++++++++++++- .../ModemRepositoryTests.cs | 63 ++++++++++++++++- .../OntRepositoryTests.cs | 68 ++++++++++++++++++- 11 files changed, 255 insertions(+), 68 deletions(-) diff --git a/src/NetworkOptimizer.Storage/Repositories/CmRepository.cs b/src/NetworkOptimizer.Storage/Repositories/CmRepository.cs index 2fb4100cd1..e3188d1d28 100644 --- a/src/NetworkOptimizer.Storage/Repositories/CmRepository.cs +++ b/src/NetworkOptimizer.Storage/Repositories/CmRepository.cs @@ -84,7 +84,6 @@ public async Task SaveCmConfigurationAsync(CmConfiguration config, CancellationT existing.Username = config.Username; existing.Password = config.Password; existing.StatusPagePath = config.StatusPagePath; - existing.Enabled = config.Enabled; existing.PollingIntervalSeconds = config.PollingIntervalSeconds; existing.LastPolled = config.LastPolled; existing.LastError = config.LastError; @@ -154,19 +153,20 @@ public async Task UpdateCmPollResultAsync(int id, DateTime? lastPolled, st { try { - var config = await _context.CmConfigurations.FirstOrDefaultAsync(c => c.Id == id, cancellationToken); - // Never resurrect or overwrite a config that was disabled while the poll was - // in flight - its frozen state (and cleared LastError) must stand. - if (config == null || !config.Enabled) - return false; - + var now = DateTime.UtcNow; + int rows; if (lastPolled.HasValue) - config.LastPolled = lastPolled.Value; - config.LastError = lastError; - config.UpdatedAt = DateTime.UtcNow; - - await _context.SaveChangesAsync(cancellationToken); - return true; + rows = await _context.CmConfigurations.Where(c => c.Id == id && c.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(c => c.LastPolled, lastPolled.Value) + .SetProperty(c => c.LastError, lastError) + .SetProperty(c => c.UpdatedAt, now), cancellationToken); + else + rows = await _context.CmConfigurations.Where(c => c.Id == id && c.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(c => c.LastError, lastError) + .SetProperty(c => c.UpdatedAt, now), cancellationToken); + return rows > 0; } catch (Exception ex) { diff --git a/src/NetworkOptimizer.Storage/Repositories/ModemRepository.cs b/src/NetworkOptimizer.Storage/Repositories/ModemRepository.cs index b5a60fd2bf..65379a8e4f 100644 --- a/src/NetworkOptimizer.Storage/Repositories/ModemRepository.cs +++ b/src/NetworkOptimizer.Storage/Repositories/ModemRepository.cs @@ -107,7 +107,6 @@ public async Task SaveModemConfigurationAsync(ModemConfiguration config, Cancell existing.ModemType = config.ModemType; existing.QmiDevice = config.QmiDevice; existing.Provider = config.Provider; - existing.Enabled = config.Enabled; existing.PollingIntervalSeconds = config.PollingIntervalSeconds; existing.LastPolled = config.LastPolled; existing.LastError = config.LastError; @@ -182,19 +181,20 @@ public async Task UpdateModemPollResultAsync(int id, DateTime? lastPolled, { try { - var config = await _context.ModemConfigurations.FirstOrDefaultAsync(m => m.Id == id, cancellationToken); - // Never resurrect or overwrite a config that was disabled while the poll was - // in flight - its frozen state (and cleared LastError) must stand. - if (config == null || !config.Enabled) - return false; - + var now = DateTime.UtcNow; + int rows; if (lastPolled.HasValue) - config.LastPolled = lastPolled.Value; - config.LastError = lastError; - config.UpdatedAt = DateTime.UtcNow; - - await _context.SaveChangesAsync(cancellationToken); - return true; + rows = await _context.ModemConfigurations.Where(m => m.Id == id && m.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(m => m.LastPolled, lastPolled.Value) + .SetProperty(m => m.LastError, lastError) + .SetProperty(m => m.UpdatedAt, now), cancellationToken); + else + rows = await _context.ModemConfigurations.Where(m => m.Id == id && m.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(m => m.LastError, lastError) + .SetProperty(m => m.UpdatedAt, now), cancellationToken); + return rows > 0; } catch (Exception ex) { diff --git a/src/NetworkOptimizer.Storage/Repositories/OntRepository.cs b/src/NetworkOptimizer.Storage/Repositories/OntRepository.cs index 3531f21453..24ccccd7b6 100644 --- a/src/NetworkOptimizer.Storage/Repositories/OntRepository.cs +++ b/src/NetworkOptimizer.Storage/Repositories/OntRepository.cs @@ -85,7 +85,6 @@ public async Task SaveOntConfigurationAsync(OntConfiguration config, Cancellatio existing.Password = config.Password; existing.PrivateKeyPath = config.PrivateKeyPath; existing.AttachedSfpId = config.AttachedSfpId; - existing.Enabled = config.Enabled; existing.PollingIntervalSeconds = config.PollingIntervalSeconds; existing.LastPolled = config.LastPolled; existing.LastError = config.LastError; @@ -155,19 +154,20 @@ public async Task UpdateOntPollResultAsync(int id, DateTime? lastPolled, s { try { - var config = await _context.OntConfigurations.FirstOrDefaultAsync(o => o.Id == id, cancellationToken); - // Never resurrect or overwrite a config that was disabled while the poll was - // in flight - its frozen state (and cleared LastError) must stand. - if (config == null || !config.Enabled) - return false; - + var now = DateTime.UtcNow; + int rows; if (lastPolled.HasValue) - config.LastPolled = lastPolled.Value; - config.LastError = lastError; - config.UpdatedAt = DateTime.UtcNow; - - await _context.SaveChangesAsync(cancellationToken); - return true; + rows = await _context.OntConfigurations.Where(o => o.Id == id && o.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(o => o.LastPolled, lastPolled.Value) + .SetProperty(o => o.LastError, lastError) + .SetProperty(o => o.UpdatedAt, now), cancellationToken); + else + rows = await _context.OntConfigurations.Where(o => o.Id == id && o.Enabled) + .ExecuteUpdateAsync(s => s + .SetProperty(o => o.LastError, lastError) + .SetProperty(o => o.UpdatedAt, now), cancellationToken); + return rows > 0; } catch (Exception ex) { diff --git a/src/NetworkOptimizer.Web/Components/Pages/Settings.razor b/src/NetworkOptimizer.Web/Components/Pages/Settings.razor index 19777ac6a5..baa77227e4 100644 --- a/src/NetworkOptimizer.Web/Components/Pages/Settings.razor +++ b/src/NetworkOptimizer.Web/Components/Pages/Settings.razor @@ -1236,12 +1236,6 @@ } } -
- -
}
@@ -1438,13 +1432,6 @@
-
- -
-
-
- -
-
-
- -
-