Apply code-review fixes: identity/pagination, load races, TLS pin, leaks
Addresses a multi-agent code review (26 confirmed findings + a URLSession leak) across correctness, concurrency, security, and cleanup: - MongoDocument.id: deterministic fallback (was UUID() on every access) to stop SwiftUI List/ForEach identity churn; getAll pagination drives its cursor off the real _id via documentId, so a missing _id stops paging instead of looping on a fabricated cursor. - View models: per-load generation guard (+ captured-tab guard on ONU detail) so a cancelled or overlapping load can't commit stale results or land data on the wrong tab. - JSONValue.intValue: guard Int(d) against NaN/inf/out-of-range (crash). - ServerTrustEvaluator: hash the real SubjectPublicKeyInfo (prepend the algorithm-specific ASN.1 SPKI header) so a standard openssl-captured public-key pin actually matches; unknown key types fail closed. - AppEnvironment.configure: clear stale cookies + Keychain creds on host change. - FirmwareFile.isCompatible: empty metadata no longer passes as compatible-for-all. - APIClient: invalidate URLSession on deinit (per-configure/probe leak). - Dashboard best-effort sections keep last-good on transient failure; unified optical thresholds; LoginView prefill-once; populatedBuckets unique ForEach id; Format rounding rollover; dead-code/docstring/dedup cleanups. Verification pending on the Mac (no Swift toolchain on the Linux dev box). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8de962eeb8
commit
203465913c
21 changed files with 303 additions and 103 deletions
|
|
@ -80,6 +80,12 @@ enum AlarmSeverity: String, Decodable, CaseIterable, Comparable {
|
|||
}
|
||||
|
||||
static func < (lhs: AlarmSeverity, rhs: AlarmSeverity) -> Bool { lhs.rank < rhs.rank }
|
||||
|
||||
/// Read a severity from a Mongo doc that may key it as `Severity` or `severity`.
|
||||
static func from(_ raw: JSONValue) -> AlarmSeverity {
|
||||
guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue else { return .unknown }
|
||||
return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Flexible documents
|
||||
|
|
@ -96,15 +102,41 @@ protocol MongoDocument: Decodable, Identifiable {
|
|||
}
|
||||
|
||||
extension MongoDocument {
|
||||
/// The document's real Mongo `_id`, or nil when absent. Use THIS for the
|
||||
/// pagination cursor (`getAll`) — never the `id` fallback below.
|
||||
var documentId: String? { raw["_id"]?.stringValue }
|
||||
}
|
||||
|
||||
// NOTE on `id`: each document's `id` falls back to `raw.compactDescription` when
|
||||
// `_id` is missing — a deterministic value derived from the content, NOT a fresh
|
||||
// `UUID()`. `id` is a *computed* property, so a random UUID would change on every
|
||||
// access and hand SwiftUI a new identity each render, tearing down and rebuilding
|
||||
// List/ForEach rows (lost scroll position, broken animations). The content-derived
|
||||
// key is stable for the same instance, so identity holds across re-renders.
|
||||
|
||||
/// nil when the string is nil or empty, else the string. Shared by the document
|
||||
/// accessors that should treat "" as "absent".
|
||||
private func nonEmpty(_ s: String?) -> String? { (s?.isEmpty == false) ? s : nil }
|
||||
|
||||
/// Single source of truth for ONU optical thresholds (dBm, user guide §7). Shared
|
||||
/// by the dashboard abnormal-Rx filter and the detail-view tinting so the two
|
||||
/// screens can't give the operator contradicting verdicts for the same reading.
|
||||
enum OpticalThreshold {
|
||||
static let rxGreenFloor = -28.0 // at/above (and ≤ rxHigh) is in-spec / green
|
||||
static let rxRedFloor = -30.0 // below this is critical / red
|
||||
static let rxHigh = -10.0 // above this is abnormal (too hot)
|
||||
static let txGreenFloor = 3.0
|
||||
|
||||
/// Whether an RX reading (dBm) is abnormal: too weak or too hot.
|
||||
static func rxAbnormal(_ dBm: Double) -> Bool { dBm < rxGreenFloor || dBm > rxHigh }
|
||||
}
|
||||
|
||||
/// ONU state document (`ONU-STATE`).
|
||||
struct ONUStateDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
|
||||
var serialNumber: String? { raw["_id"]?.stringValue }
|
||||
/// Parent OLT MAC, from `OLT.MAC Address` (dev guide Procedure 2 §2).
|
||||
|
|
@ -143,7 +175,7 @@ struct ONUStateDoc: MongoDocument {
|
|||
struct ONUConfigDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
/// Operator-set label (`ONU.Name`, §5.1).
|
||||
var name: String? { raw["ONU"]?["Name"]?.stringValue }
|
||||
var netconfName: String? { raw["Netconf"]?["Name"]?.stringValue }
|
||||
|
|
@ -153,7 +185,7 @@ struct ONUConfigDoc: MongoDocument {
|
|||
struct OLTConfigDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // MAC address
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // MAC address
|
||||
/// Operator label from `OLT.Name` (confirmed against a live OLT-CFG, e.g.
|
||||
/// "OLT69-RIN"). Note `NETCONF.Name` defaults to the MAC, so it's *not* the
|
||||
/// friendly name. Returns nil when unset.
|
||||
|
|
@ -168,11 +200,12 @@ struct OLTConfigDoc: MongoDocument {
|
|||
struct OLTStateDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // OLT MAC address
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // OLT MAC address
|
||||
var lastUpdate: String? { raw["Time"]?.stringValue }
|
||||
|
||||
/// Operator name from OLT-CFG (`Netconf.Name`); resolved + stamped by the
|
||||
/// list VM via `OLTRepository.nameMap()`. `nil` until resolved / if unset.
|
||||
/// Operator name from OLT-CFG (`OLT.Name` — NOT `Netconf.Name`, which defaults
|
||||
/// to the MAC); resolved + stamped by the list VM via `OLTRepository.nameMap()`.
|
||||
/// `nil` until resolved / if unset.
|
||||
var resolvedName: String?
|
||||
/// Best label to show: the operator name if set, else the MAC id.
|
||||
var displayName: String {
|
||||
|
|
@ -194,14 +227,18 @@ struct OLTStateDoc: MongoDocument {
|
|||
}
|
||||
}
|
||||
|
||||
/// Non-empty registration buckets, ordered by lifecycle severity.
|
||||
var populatedBuckets: [(state: ONULifecycleState, ids: [String])] {
|
||||
/// Non-empty registration buckets, ordered by lifecycle severity. Carries the
|
||||
/// raw bucket `name` so callers key `ForEach` on it — keying on `state` would
|
||||
/// collide when two unrecognized bucket names both map to `.unknown`.
|
||||
var populatedBuckets: [(name: String, state: ONULifecycleState, ids: [String])] {
|
||||
onuStatesByBucket
|
||||
.filter { !$0.value.isEmpty }
|
||||
.map { (state: ONULifecycleState(rawValue: $0.key) ?? .unknown, ids: $0.value.sorted()) }
|
||||
.map { (name: $0.key, state: ONULifecycleState(rawValue: $0.key) ?? .unknown, ids: $0.value.sorted()) }
|
||||
.sorted {
|
||||
let order = ONULifecycleState.allCases
|
||||
return (order.firstIndex(of: $0.state) ?? .max) < (order.firstIndex(of: $1.state) ?? .max)
|
||||
let a = order.firstIndex(of: $0.state) ?? .max
|
||||
let b = order.firstIndex(of: $1.state) ?? .max
|
||||
return (a, $0.name) < (b, $1.name) // name tiebreak → stable order for unknowns
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -210,7 +247,7 @@ struct OLTStateDoc: MongoDocument {
|
|||
struct ControllerConfigDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString } // MAC address
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription } // MAC address
|
||||
var version: String? { raw["CNTL"]?["Version"]?.stringValue }
|
||||
}
|
||||
|
||||
|
|
@ -218,14 +255,10 @@ struct ControllerConfigDoc: MongoDocument {
|
|||
struct AlarmHistoryDoc: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
|
||||
// TODO: confirm key paths.
|
||||
var severity: AlarmSeverity {
|
||||
guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue
|
||||
else { return .unknown }
|
||||
return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown
|
||||
}
|
||||
var severity: AlarmSeverity { AlarmSeverity.from(raw) }
|
||||
var message: String? { raw["Message"]?.stringValue ?? raw["message"]?.stringValue }
|
||||
var raisedAt: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue }
|
||||
}
|
||||
|
|
@ -234,7 +267,7 @@ struct AlarmHistoryDoc: MongoDocument {
|
|||
struct StatSample: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
var timestamp: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue }
|
||||
|
||||
/// Pull a numeric counter by key for charting.
|
||||
|
|
@ -251,12 +284,8 @@ struct StatSample: MongoDocument {
|
|||
struct LogEntry: MongoDocument {
|
||||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var severity: AlarmSeverity {
|
||||
guard let s = raw["Severity"]?.stringValue ?? raw["severity"]?.stringValue
|
||||
else { return .unknown }
|
||||
return AlarmSeverity(rawValue: s.lowercased()) ?? .unknown
|
||||
}
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
var severity: AlarmSeverity { AlarmSeverity.from(raw) }
|
||||
var message: String? { raw["Message"]?.stringValue ?? raw["message"]?.stringValue }
|
||||
var time: String? { raw["Time"]?.stringValue ?? raw["timestamp"]?.stringValue }
|
||||
}
|
||||
|
|
@ -266,7 +295,7 @@ struct CPEState: MongoDocument {
|
|||
let raw: JSONValue
|
||||
init(from decoder: Decoder) throws { raw = try JSONValue(from: decoder) }
|
||||
init(raw: JSONValue) { self.raw = raw }
|
||||
var id: String { raw["_id"]?.stringValue ?? UUID().uuidString }
|
||||
var id: String { raw["_id"]?.stringValue ?? raw.compactDescription }
|
||||
var mac: String? { raw["_id"]?.stringValue }
|
||||
|
||||
// IPv4 (DHCP)
|
||||
|
|
@ -290,8 +319,6 @@ struct CPEState: MongoDocument {
|
|||
}
|
||||
|
||||
var hasLease: Bool { ipv4 != nil || ipv6 != nil }
|
||||
|
||||
private func nonEmpty(_ value: String?) -> String? { (value?.isEmpty == false) ? value : nil }
|
||||
}
|
||||
|
||||
// MARK: - ONU-STATE detail accessors (status / firmware / UNI / traffic)
|
||||
|
|
@ -391,8 +418,6 @@ extension OLTStateDoc {
|
|||
var switchName: String? { nonEmpty(raw["Switch"]?["System Name"]?.stringValue) }
|
||||
var switchPort: String? { nonEmpty(raw["Switch"]?["Port ID"]?.stringValue) }
|
||||
var switchAddress: String? { nonEmpty(raw["Switch"]?["IPv4 Address"]?.stringValue) }
|
||||
|
||||
private func nonEmpty(_ s: String?) -> String? { (s?.isEmpty == false) ? s : nil }
|
||||
}
|
||||
|
||||
// MARK: - OLT action payloads
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue