Experte-Tab: Port-Konflikt-Pruefung wie im Einrichten-Assistenten

Per explizitem Nutzerwunsch die "Port freimachen?"-Abfrage samt allen
Warnungen aus dem LAN-Schritt des Wizards auch auf den Experte-Tab
angewendet, ausgeloest durch echten Live-Fall (ether4 wurde manuell
ueber Expert als eigenes Netz angelegt, blieb dabei unbemerkt Bridge-
Mitglied - zwei DHCP-Server im selben Broadcast-Domain).

PortConflictWarningView aus LanStepView.swift in Features/Shared/
extrahiert (jetzt von Wizard UND Experte-Tab genutzt), neuer
immediateApply-Parameter fuer die kontextabhaengige Abschluss-Meldung
(Wizard: erst bei "Jetzt anwenden"; Experte: sofort bei "Anlegen"/
"Speichern"). Neue PortConflict.resolutionCommandsIncludingBridgeDetach()
- der Experte-Tab hat anders als der Wizard keinen separaten,
automatischen Bridge-Detach-Schritt, muss die Bridge-Entfernung also
selbst mit auflisten.

ExpertViewModel bekommt dieselbe Race-sichere Generation-Zaehler-Logik
wie SetupViewModel (bugs.md #1), angewendet auf das .interfacePick-Feld
des jeweils offenen Schemas. Speichern-Button gesperrt bis Konflikt
bestaetigt oder Port gewechselt.

6 neue Regressionstests. Build + alle 111 Unit-Tests gruen.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Kay
2026-09-17 22:21:36 +02:00
co-authored by Claude Sonnet 5
parent 57850cfe03
commit b503bac82c
7 changed files with 288 additions and 73 deletions
@@ -62,4 +62,22 @@ struct PortConflict: Equatable {
}
}
}
/// `resolutionCommands()` alone for the Setup-Wizard context, where `DhcpServerCommandBuilder`
/// already unconditionally detaches the port from any bridge as its own separate safety net
/// see that method's doc comment. Contexts without that separate detach (the Experte tab's
/// generic "Port freimachen?" flow, applied to whatever menu the user is actually editing, not
/// specifically the LAN/DHCP command set) need the bridge-membership removal included here
/// instead, or acknowledging a bridge-member conflict there would silently do nothing for
/// that specific reason.
func resolutionCommandsIncludingBridgeDetach() -> [RouterOSCommand] {
let bridgeDetach: [RouterOSCommand] = reasons.contains(where: { if case .bridgeMember = $0 { return true } else { return false } })
? [RouterOSCommand.remove(
menuPath: "/interface bridge port", restPath: "interface/bridge/port",
matchField: "interface", matchValue: interfaceName,
summary: "\(interfaceName) aus Bridge lösen"
)]
: []
return bridgeDetach + resolutionCommands()
}
}
@@ -154,6 +154,25 @@ struct ExpertItemEditView: View {
fieldEditor(for: field)
}
// Gleiche "Port-Konflikt-Prüfung" wie im Einrichten-Assistenten (LAN-Schritt,
// found.md #1), hier auf das `.interfacePick`-Feld dieses Schemas angewendet per
// explizitem Nutzerwunsch ("die Abfrage vom Einrichten-Assistenten auf Expert
// anwenden ... mit allen Warnungen").
if let conflict = viewModel.interfacePortConflict {
PortConflictWarningView(
conflict: conflict,
isAcknowledged: viewModel.acknowledgedInterfacePortConflict,
appLanguage: appLanguage,
immediateApply: true,
onConfirm: { viewModel.acknowledgeInterfacePortConflict() }
)
} else if viewModel.isCheckingInterfacePortConflict {
HStack {
ProgressView().controlSize(.small)
Text(L10n.t("Prüfe, ob der Port frei ist…", appLanguage)).appFont(.caption).foregroundStyle(.secondary)
}
}
Section(LocalizedStringKey(L10n.t("Weitere Parameter (frei)", appLanguage))) {
Text(L10n.t("Für alles, was oben nicht als eigenes Feld aufgeführt ist — RouterOS-Parametername genau wie in der Dokumentation.", appLanguage))
.appFont(.caption)
@@ -223,13 +242,19 @@ struct ExpertItemEditView: View {
Button(L10n.t(isNew ? "Anlegen" : "Speichern", appLanguage)) {
showApplyConfirmation = true
}
.disabled(viewModel.isApplying || viewModel.pendingCommand == nil)
.disabled(viewModel.isApplying || viewModel.pendingCommand == nil || viewModel.hasUnresolvedInterfacePortConflict)
}
}
}
.formStyle(.grouped)
}
.frame(minWidth: 900, idealWidth: 900, minHeight: 520, idealHeight: 660)
.onAppear {
viewModel.checkInterfacePortConflict()
}
.onChange(of: viewModel.formValues[interfaceFieldKey ?? "", default: ""]) { _, _ in
viewModel.checkInterfacePortConflict()
}
.confirmationDialog(
L10n.t("Jetzt am Router anwenden?", appLanguage),
isPresented: $showApplyConfirmation,
@@ -249,6 +274,13 @@ struct ExpertItemEditView: View {
}
}
/// The current schema's `.interfacePick` field key, if it has one mirrors
/// `ExpertViewModel.interfaceFieldKey`, kept here too since `.onChange` needs a concrete
/// key path to observe (a schema only ever has at most one such field).
private var interfaceFieldKey: String? {
schema.fields.first { if case .interfacePick = $0.kind { return true } else { return false } }?.key
}
private func extraColumnHeader(_ key: String) -> some View {
Text(L10n.t(key, appLanguage))
.appFont(.caption)
@@ -37,6 +37,19 @@ final class ExpertViewModel: ObservableObject {
@Published private(set) var applyError: String?
@Published var pendingRemoval: RouterOSMenuItem?
/// Same "Port-Konflikt-Prüfung" as the Setup Wizard's LAN step (found.md #1), applied here to
/// whichever `.interfacePick` field the current schema has per explicit request ("die
/// Abfrage vom Einrichten-Assistenten auf Expert anwenden ... mit allen Warnungen"). Unlike
/// the wizard (one conflict per `LanDhcpConfig.ID` in a list), the edit sheet only ever has
/// one interface field open at a time, so a single value (not a dictionary) is enough.
@Published private(set) var interfacePortConflict: PortConflict?
@Published private(set) var isCheckingInterfacePortConflict = false
@Published private(set) var acknowledgedInterfacePortConflict = false
/// Bumped on every `checkInterfacePortConflict()` call, same race-safety reasoning as
/// `SetupViewModel.portConflictRequestGeneration` (bugs.md #1) a slower, older in-flight
/// check discards its own result if a newer one has since started.
private var interfacePortConflictGeneration = 0
private let connectionService: ConnectionService
private let backupService: BackupService
@@ -125,6 +138,7 @@ final class ExpertViewModel: ObservableObject {
})
extraFields = []
applyError = nil
resetInterfacePortConflictState()
}
func startEditing(_ item: RouterOSMenuItem) {
@@ -137,6 +151,56 @@ final class ExpertViewModel: ObservableObject {
.sorted { $0.key < $1.key }
.map { ExtraField(key: $0.key, value: $0.value) }
applyError = nil
resetInterfacePortConflictState()
}
/// The key of the current schema's `.interfacePick` field, if it has one a schema only
/// ever has at most one (matches the one physical/logical port the whole menu item applies
/// to, e.g. `/ip address`'s "interface").
private var interfaceFieldKey: String? {
selectedSchema?.fields.first { if case .interfacePick = $0.kind { return true } else { return false } }?.key
}
private func resetInterfacePortConflictState() {
interfacePortConflict = nil
isCheckingInterfacePortConflict = false
acknowledgedInterfacePortConflict = false
interfacePortConflictGeneration += 1
}
/// Mirrors `SetupViewModel.checkPortConflict(for:)` called whenever the `.interfacePick`
/// field's value changes (`ExpertMenuDetailView`'s `.onChange`). Best-effort, same as the
/// wizard's version: a failed check just means no warning shown for that attempt, never
/// blocks the sheet outright.
func checkInterfacePortConflict() {
guard let key = interfaceFieldKey else { return }
let interfaceName = formValues[key] ?? ""
acknowledgedInterfacePortConflict = false
interfacePortConflict = nil
guard !interfaceName.isEmpty else { return }
isCheckingInterfacePortConflict = true
interfacePortConflictGeneration += 1
let generation = interfacePortConflictGeneration
Task {
let result = try? await connectionService.checkPortConflict(interfaceName: interfaceName)
guard generation == interfacePortConflictGeneration else { return }
interfacePortConflict = result
isCheckingInterfacePortConflict = false
}
}
/// The user has been shown what's on this port and, after two explicit confirmations
/// (`PortConflictWarningView`), chose to have the app clear it immediately as part of this
/// save unlike the wizard (deferred to "Jetzt anwenden"), the Experte tab has no separate
/// review step, so acknowledging here takes effect on the very next "Anlegen"/"Speichern".
func acknowledgeInterfacePortConflict() {
acknowledgedInterfacePortConflict = true
}
/// Blocks "Anlegen"/"Speichern" until a found conflict is either acknowledged or the
/// interface field is changed to something actually free.
var hasUnresolvedInterfacePortConflict: Bool {
interfacePortConflict != nil && !acknowledgedInterfacePortConflict
}
func cancelEditing() {
@@ -144,6 +208,7 @@ final class ExpertViewModel: ObservableObject {
formValues = [:]
extraFields = []
applyError = nil
resetInterfacePortConflictState()
}
/// The command that saving the current edit sheet would run shown to the user before it
@@ -221,6 +286,14 @@ final class ExpertViewModel: ObservableObject {
defer { connectionService.endWrite() }
do {
try await ensureSessionBackup()
// Port-Konflikt-Auflösung (found.md #1-style, siehe `PortConflictWarningView`) läuft
// hier sofort vor dem eigentlichen Befehl anders als im Wizard gibt es im Experte-
// Tab keinen separaten Review-Schritt, an dem das gebündelt würde.
if let conflict = interfacePortConflict, acknowledgedInterfacePortConflict {
for resolutionCommand in conflict.resolutionCommandsIncludingBridgeDetach() {
try await connectionService.apply(resolutionCommand)
}
}
if selectedSchema?.writesRequireSSH == true {
try await connectionService.applyViaSSH(command)
} else {
@@ -0,0 +1,85 @@
import SwiftUI
/// Shown inline wherever a user is about to configure a physical port that
/// `ConnectionService.checkPortConflict(interfaceName:)` found already carrying other
/// configuration originally the Setup Wizard's LAN step only (found.md #1's "Port-Konflikt-
/// Prüfung"), now also the Experte tab's generic `.interfacePick` fields (per explicit request:
/// "die Abfrage vom Einrichten-Assistenten auf Expert anwenden ... mit allen Warnungen").
/// Requires two separate confirmations before the app is allowed to clear it per explicit
/// request: this silently overriding a port's existing role (an active WAN dial-up, a bridge
/// membership, a manually-set address) previously wasn't visible to the user at all beyond
/// `DhcpServerCommandBuilder`'s own unconditional bridge detach; this makes the consequences
/// explicit and opt-in instead.
struct PortConflictWarningView: View {
let conflict: PortConflict
let isAcknowledged: Bool
let appLanguage: String
/// Wizard: resolution runs later, batched with everything else at "Jetzt anwenden" the
/// final confirmation dialog says so, and that a port switch above still undoes it. Experte
/// tab: there's no separate review/apply step, "Anlegen"/"Speichern" runs it immediately
/// a different final-confirmation message reflects that instead.
let immediateApply: Bool
let onConfirm: () -> Void
@State private var showConsequencesConfirmation = false
@State private var showFinalConfirmation = false
var body: some View {
if isAcknowledged {
Label("\(conflict.interfaceName) " + L10n.t("wird beim Anwenden freigemacht", appLanguage), systemImage: "checkmark.shield")
.appFont(.caption)
.foregroundStyle(.orange)
} else {
VStack(alignment: .leading, spacing: 6) {
Label(L10n.t("Port", appLanguage) + " \(conflict.interfaceName) " + L10n.t("ist nicht frei", appLanguage), systemImage: "exclamationmark.triangle.fill")
.appFont(.subheadline, bold: true)
.foregroundStyle(.red)
ForEach(Array(conflict.reasons.enumerated()), id: \.offset) { _, reason in
Text("\(reason.description)")
.appFont(.caption)
}
Text(L10n.t("Wähle oben einen anderen, freien Port — oder mache diesen jetzt frei. Die bestehende Konfiguration wird dabei entfernt.", appLanguage))
.appFont(.caption)
.foregroundStyle(.secondary)
Button(L10n.t("Port jetzt freimachen…", appLanguage), role: .destructive) {
showConsequencesConfirmation = true
}
}
.padding(10)
.background(RoundedRectangle(cornerRadius: 8).fill(Color.red.opacity(0.08)))
.confirmationDialog(
L10n.t("Port", appLanguage) + " \(conflict.interfaceName) " + L10n.t("freimachen?", appLanguage),
isPresented: $showConsequencesConfirmation,
titleVisibility: .visible
) {
Button(L10n.t("Fortfahren", appLanguage), role: .destructive) {
showFinalConfirmation = true
}
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {}
} message: {
Text(consequenceText)
}
.confirmationDialog(
L10n.t("Wirklich sicher?", appLanguage),
isPresented: $showFinalConfirmation,
titleVisibility: .visible
) {
Button(L10n.t("Ja, endgültig freimachen", appLanguage), role: .destructive) {
onConfirm()
}
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {}
} message: {
Text(immediateApply
? L10n.t("Wird sofort ausgeführt, sobald du unten auf \"Anlegen\"/\"Speichern\" klickst.", appLanguage)
: L10n.t("Tatsächlich ausgeführt wird das erst mit \"Jetzt anwenden\" am Ende des Assistenten — bis dahin kannst du das rückgängig machen, indem du hier oben einen anderen Port wählst.", appLanguage))
}
}
}
private var consequenceText: String {
([L10n.t("Folgendes wird entfernt, bevor", appLanguage) + " \(conflict.interfaceName) " + L10n.t("als neues Netzwerk eingerichtet wird:", appLanguage)]
+ conflict.reasons.map { "\($0.description)" }
+ [L10n.t("Bestehender Datenverkehr über diesen Port (z.B. eine laufende Internetverbindung oder Geräte im bisherigen Netz) wird dadurch unterbrochen.", appLanguage)])
.joined(separator: "\n")
}
}
@@ -32,6 +32,7 @@ struct LanStepView: View {
conflict: conflict,
isAcknowledged: viewModel.acknowledgedPortConflicts.contains(config.id),
appLanguage: appLanguage,
immediateApply: false,
onConfirm: { viewModel.acknowledgePortConflict(for: config.id) }
)
}
@@ -100,75 +101,5 @@ struct LanStepView: View {
}
}
/// Shown inline under a LAN config's port Picker once `SetupViewModel.checkPortConflict(for:)`
/// finds the chosen port already carries other configuration. Requires two separate confirmations
/// before the app is allowed to clear it per explicit request: this silently overriding a port's
/// existing role (an active WAN dial-up, a bridge membership, a manually-set address) previously
/// wasn't visible to the user at all beyond `DhcpServerCommandBuilder`'s own unconditional bridge
/// detach; this makes the consequences explicit and opt-in instead.
private struct PortConflictWarningView: View {
let conflict: PortConflict
let isAcknowledged: Bool
let appLanguage: String
let onConfirm: () -> Void
@State private var showConsequencesConfirmation = false
@State private var showFinalConfirmation = false
var body: some View {
if isAcknowledged {
Label("\(conflict.interfaceName) " + L10n.t("wird beim Anwenden freigemacht", appLanguage), systemImage: "checkmark.shield")
.appFont(.caption)
.foregroundStyle(.orange)
} else {
VStack(alignment: .leading, spacing: 6) {
Label(L10n.t("Port", appLanguage) + " \(conflict.interfaceName) " + L10n.t("ist nicht frei", appLanguage), systemImage: "exclamationmark.triangle.fill")
.appFont(.subheadline, bold: true)
.foregroundStyle(.red)
ForEach(Array(conflict.reasons.enumerated()), id: \.offset) { _, reason in
Text("\(reason.description)")
.appFont(.caption)
}
Text(L10n.t("Wähle oben einen anderen, freien Port — oder mache diesen jetzt frei. Die bestehende Konfiguration wird dabei entfernt.", appLanguage))
.appFont(.caption)
.foregroundStyle(.secondary)
Button(L10n.t("Port jetzt freimachen…", appLanguage), role: .destructive) {
showConsequencesConfirmation = true
}
}
.padding(10)
.background(RoundedRectangle(cornerRadius: 8).fill(Color.red.opacity(0.08)))
.confirmationDialog(
L10n.t("Port", appLanguage) + " \(conflict.interfaceName) " + L10n.t("freimachen?", appLanguage),
isPresented: $showConsequencesConfirmation,
titleVisibility: .visible
) {
Button(L10n.t("Fortfahren", appLanguage), role: .destructive) {
showFinalConfirmation = true
}
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {}
} message: {
Text(consequenceText)
}
.confirmationDialog(
L10n.t("Wirklich sicher?", appLanguage),
isPresented: $showFinalConfirmation,
titleVisibility: .visible
) {
Button(L10n.t("Ja, endgültig freimachen", appLanguage), role: .destructive) {
onConfirm()
}
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {}
} message: {
Text(L10n.t("Tatsächlich ausgeführt wird das erst mit \"Jetzt anwenden\" am Ende des Assistenten — bis dahin kannst du das rückgängig machen, indem du hier oben einen anderen Port wählst.", appLanguage))
}
}
}
private var consequenceText: String {
([L10n.t("Folgendes wird entfernt, bevor", appLanguage) + " \(conflict.interfaceName) " + L10n.t("als neues Netzwerk eingerichtet wird:", appLanguage)]
+ conflict.reasons.map { "\($0.description)" }
+ [L10n.t("Bestehender Datenverkehr über diesen Port (z.B. eine laufende Internetverbindung oder Geräte im bisherigen Netz) wird dadurch unterbrochen.", appLanguage)])
.joined(separator: "\n")
}
}
// `PortConflictWarningView` now lives in Features/Shared/ shared with the Experte tab's
// equivalent flow (see that file's doc comment for why).
@@ -77,4 +77,58 @@ final class ExpertViewModelTests: XCTestCase {
XCTAssertNil(viewModel.pendingCommand?.arguments["distance"])
XCTAssertEqual(viewModel.pendingCommand?.arguments["comment"], "bridge")
}
/// Regression tests for the Experte-tab port-conflict check (per explicit request: "die
/// Abfrage vom Einrichten-Assistenten auf Expert anwenden ... mit allen Warnungen"). No live
/// connection is set up here, so `checkInterfacePortConflict()`'s actual network call always
/// fails/is skipped these only cover the surrounding logic that doesn't need one: schemas
/// without an `.interfacePick` field are correctly ignored, and the unresolved-conflict gate
/// starts clear.
private func makeSchemaWithInterfaceField() -> RouterOSMenuSchema {
RouterOSMenuSchema(
menuPath: "/ip address", restPath: "ip/address", category: .ipAddressing,
displayName: "Test", summary: "", explanation: "",
fields: [
RouterOSFieldSchema(key: "address", label: "Adresse", kind: .text, help: ""),
RouterOSFieldSchema(key: "interface", label: "Interface", kind: .interfacePick, help: "")
]
)
}
func testCheckInterfacePortConflictNoOpsForSchemaWithoutInterfaceField() {
let viewModel = ExpertViewModel(connectionService: ConnectionService())
viewModel.selectedSchema = makeSchema() // no .interfacePick field
viewModel.startNewItem()
viewModel.checkInterfacePortConflict()
XCTAssertFalse(viewModel.isCheckingInterfacePortConflict)
XCTAssertNil(viewModel.interfacePortConflict)
XCTAssertFalse(viewModel.hasUnresolvedInterfacePortConflict)
}
func testCheckInterfacePortConflictNoOpsWhenInterfaceFieldIsEmpty() {
let viewModel = ExpertViewModel(connectionService: ConnectionService())
viewModel.selectedSchema = makeSchemaWithInterfaceField()
viewModel.startNewItem() // "interface" has no default, starts empty
viewModel.checkInterfacePortConflict()
XCTAssertFalse(viewModel.isCheckingInterfacePortConflict)
XCTAssertNil(viewModel.interfacePortConflict)
}
func testStartingOrCancelingEditResetsAcknowledgedConflictState() {
let viewModel = ExpertViewModel(connectionService: ConnectionService())
viewModel.selectedSchema = makeSchemaWithInterfaceField()
viewModel.startNewItem()
viewModel.acknowledgeInterfacePortConflict()
XCTAssertFalse(viewModel.hasUnresolvedInterfacePortConflict, "acknowledging clears the block even with no conflict object set")
viewModel.cancelEditing()
viewModel.startNewItem()
// A fresh edit must not inherit a stale acknowledgement from a previous one.
XCTAssertNil(viewModel.interfacePortConflict)
}
}
@@ -29,4 +29,26 @@ final class PortConflictTests: XCTestCase {
XCTAssertEqual(commands[1].menuPath, "/interface pppoe-client")
XCTAssertEqual(commands[1].operation, .remove(matchField: "interface", matchValue: "ether1"))
}
/// Regression test for the Experte-tab "Port freimachen?" flow (per explicit request): unlike
/// the Setup Wizard, the Experte tab has no separate step that unconditionally detaches a
/// bridge port on its own, so its resolution list must include that removal explicitly
/// `resolutionCommands()` alone (the wizard's version) deliberately leaves it out.
func testResolutionCommandsIncludingBridgeDetachAddsBridgePortRemoval() {
let conflict = PortConflict(interfaceName: "ether4", reasons: [.bridgeMember(bridgeName: "bridge")])
let commands = conflict.resolutionCommandsIncludingBridgeDetach()
XCTAssertEqual(commands.count, 1)
XCTAssertEqual(commands[0].menuPath, "/interface bridge port")
XCTAssertEqual(commands[0].operation, .remove(matchField: "interface", matchValue: "ether4"))
}
func testResolutionCommandsIncludingBridgeDetachKeepsOtherReasonsToo() {
let conflict = PortConflict(interfaceName: "ether4", reasons: [.bridgeMember(bridgeName: "bridge"), .dhcpClient])
let commands = conflict.resolutionCommandsIncludingBridgeDetach()
XCTAssertEqual(commands.count, 2)
XCTAssertEqual(commands[0].menuPath, "/interface bridge port")
XCTAssertEqual(commands[1].menuPath, "/ip dhcp-client")
}
}