refactor(rules): centralize document commits

ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController+CustomRules.swift

@@ -1,38 +1,8 @@
 import Foundation
 
-// MARK: - 自定义规则更新结果
-
-/// The result of accepting and saving a rule document, independent of runtime.
-/// `busy`/`superseded` are transaction-gate rejections: no save was attempted.
-enum CustomRuleUpdateOutcome: Equatable, Sendable {
-  /// 规则已保存；运行规则模式时应用。
-  case saved
-  /// 校验拒绝：整批不落地，旧规则保持不变；附可解释原因。
-  case rejected([RejectedCustomRule])
-  /// 持久化失败：旧规则保持不变。
-  case persistenceFailed
-  case busy
-  /// 事务门拒绝：意图捕获后事实已前进（集合版本/完整性变化），本次更新未尝试保存。
-  case superseded
-  case invalidDocument(detail: String)
-
-  var isSuccess: Bool {
-    switch self {
-    case .saved: true
-    default: false
-    }
-  }
-}
-
-/// Final persisted facts, independent of whether runtime application succeeded.
-struct RuleDocumentCommit: Sendable {
-  let outcome: CustomRuleUpdateOutcome
-  let document: CustomRuleDocument?
-}
-
 // MARK: - 结果呈现文案
 
-// outcome→文案映射与 outcome 类型同住，供状态行与编辑器两个视图共享。
+// outcome→文案映射供状态行与编辑器两个视图共享。
 
 extension RulesCommitFeedback {
   var summary: String {
@@ -90,36 +60,9 @@ extension ProxyRuntimeController {
     -> RuleDocumentCommit
   {
     let preset = preset ?? settings.rulePreset
-    let session = documents(for: preset)
-    let previous = try? session.load()
-    let outcome = await updateRuleDocument(document, preset: preset)
-    return RuleDocumentCommit(outcome: outcome, document: session.current ?? previous)
-  }
-
-  func updateRuleDocument(_ document: CustomRuleDocument, preset: RulePreset? = nil) async
-    -> CustomRuleUpdateOutcome
-  {
-    let preset = preset ?? settings.rulePreset
-    let session = documents(for: preset)
-    guard !isUpdatingRules else { return .busy }
-    isUpdatingRules = true
-    defer { isUpdatingRules = false }
-    let validation = validateCustomRulesForPersistence(document.rules)
-    guard validation.rejected.isEmpty else { return .rejected(validation.rejected) }
-    guard
-      document.disabledIdentities.isDisjoint(
-        with:
-          Set(RuleCoverage.fixedLocalMatches.map { ProxyRule(action: .direct, match: $0) }))
-    else { return .invalidDocument(detail: "Fixed local policy cannot be disabled") }
-    do {
-      _ = try session.load()
-      try session.save(document)
-    } catch {
-      RuntimeLog.emit(.runtimePersistFailed(detail: String(describing: error)))
-      return .persistenceFailed
-    }
-    if preset == settings.rulePreset { scheduleRuleApplication() }
-    return .saved
+    let result = documents(for: preset).commit(document)
+    if result.outcome.isSuccess && preset == settings.rulePreset { scheduleRuleApplication() }
+    return result
   }
 
   /// One worker owns rule deployments. Saves never cancel an in-flight restart;
@@ -140,20 +83,6 @@ extension ProxyRuntimeController {
     }
   }
 
-  /// 保存前校验：固定本地冲突与重复整批拒绝并返回可解释原因。
-  /// 同行动覆盖和相反行动遮蔽不阻止保存；编译时按当前骨架解释完整集合。
-  func validateCustomRulesForPersistence(_ rules: [CustomRule]) -> (
-    acceptedRules: [CustomRule], rejected: [RejectedCustomRule]
-  ) {
-    let base = CustomRuleValidator.hardValidation(custom: rules)
-    let hardRejected = base.rejected.filter {
-      $0.reason == .conflictsWithFixedLocalScope || $0.reason == .duplicate
-    }
-    let hardRejectedTokens = Set(hardRejected.map(\.rule.contentToken))
-    let accepted = rules.filter { !hardRejectedTokens.contains($0.contentToken) }
-    return (accepted, hardRejected)
-  }
-
   /// 自定义规则安全摘要：数量 + 内容版本，不含原始域名。
   func readCustomRuleSummary() -> CustomRuleSummary? {
     try? CustomRuleSummary.summarizing(ruleDocuments.load())

ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController.swift

@@ -54,7 +54,6 @@ final class ProxyRuntimeController {
   let controlPreferences: ProxyControlPreferences
   let listeningStore: ProxyListeningStoring
   /// 自定义规则持久化：规则模式 ACL 合并的用户入口。
-  var isUpdatingRules = false
   var ruleApplicationTask: Task<Void, Never>?
   var ruleApplicationGeneration = 0
   var ruleApplicationFailure: RuntimeFailureFacts? { didSet { factChanges.markChanged() } }

ShadowsocksX-NG2/Domain/Rules/Custom/CustomRuleStore.swift

@@ -87,30 +87,6 @@ struct CustomRuleStore {
   }
 }
 
-/// One saved-document owner per app process. Only successful atomic saves update
-/// this fact; runtime recovery never restores an older rule document.
-@MainActor
-final class RuleDocumentSession {
-  private let store: CustomRuleStore
-  private(set) var current: CustomRuleDocument?
-  private(set) var revision = 0
-
-  init(store: CustomRuleStore) { self.store = store }
-
-  func load() throws -> CustomRuleDocument {
-    if let current { return current }
-    let loaded = try store.loadDocument()
-    current = loaded
-    return loaded
-  }
-
-  func save(_ document: CustomRuleDocument) throws {
-    try store.saveDocument(document)
-    if current != document { revision += 1 }
-    current = document
-  }
-}
-
 private struct StoredCustomRule: Codable {
   let id: UUID
   let rule: StoredProxyRule

ShadowsocksX-NG2/Domain/Rules/Custom/RuleDocumentSession.swift

@@ -0,0 +1,79 @@
+import Foundation
+
+// MARK: - 自定义规则更新结果
+
+/// The result of accepting and saving a rule document, independent of runtime.
+/// `busy`/`superseded` are transaction-gate rejections: no save was attempted.
+enum CustomRuleUpdateOutcome: Equatable, Sendable {
+  /// 规则已保存；运行规则模式时应用。
+  case saved
+  /// 校验拒绝：整批不落地，旧规则保持不变；附可解释原因。
+  case rejected([RejectedCustomRule])
+  /// 持久化失败：旧规则保持不变。
+  case persistenceFailed
+  case busy
+  /// 事务门拒绝：意图捕获后事实已前进（集合版本/完整性变化），本次更新未尝试保存。
+  case superseded
+  case invalidDocument(detail: String)
+
+  var isSuccess: Bool {
+    switch self {
+    case .saved: true
+    default: false
+    }
+  }
+}
+
+/// Final persisted facts, independent of whether runtime application succeeded.
+struct RuleDocumentCommit: Sendable {
+  let outcome: CustomRuleUpdateOutcome
+  let document: CustomRuleDocument?
+}
+
+/// One saved-document owner per preset. Only successful atomic saves replace
+/// loaded facts; runtime recovery never restores an older rule document.
+@MainActor
+final class RuleDocumentSession {
+  private let store: CustomRuleStore
+  private(set) var current: CustomRuleDocument?
+  private(set) var revision = 0
+
+  init(store: CustomRuleStore) { self.store = store }
+
+  func load() throws -> CustomRuleDocument {
+    if let current { return current }
+    let loaded = try store.loadDocument()
+    current = loaded
+    return loaded
+  }
+
+  func commit(_ document: CustomRuleDocument) -> RuleDocumentCommit {
+    _ = try? load()
+    let rejected = CustomRuleValidator.hardValidation(custom: document.rules).rejected
+    guard rejected.isEmpty else {
+      return RuleDocumentCommit(outcome: .rejected(rejected), document: current)
+    }
+    guard
+      document.disabledIdentities.isDisjoint(
+        with: Set(RuleCoverage.fixedLocalMatches.map { ProxyRule(action: .direct, match: $0) }))
+    else {
+      return RuleDocumentCommit(
+        outcome: .invalidDocument(detail: "Fixed local policy cannot be disabled"),
+        document: current)
+    }
+    do {
+      _ = try load()
+      try save(document)
+      return RuleDocumentCommit(outcome: .saved, document: current)
+    } catch {
+      RuntimeLog.emit(.runtimePersistFailed(detail: String(describing: error)))
+      return RuleDocumentCommit(outcome: .persistenceFailed, document: current)
+    }
+  }
+
+  private func save(_ document: CustomRuleDocument) throws {
+    try store.saveDocument(document)
+    if current != document { revision += 1 }
+    current = document
+  }
+}

ShadowsocksX-NG2/Tests/Rules/Custom/RuleDocumentSessionTests.swift

@@ -0,0 +1,147 @@
+import Foundation
+import Testing
+
+@testable import ShadowsocksX_NG2
+
+@MainActor
+struct RuleDocumentSessionTests {
+  @Test func successfulCommitPublishesSavedFactsAndOnlyChangedContentAdvancesRevision() throws {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    let store = CustomRuleStore(
+      preset: .chinaProxy, baseURL: directory.appendingPathComponent("rules.json"))
+    let session = RuleDocumentSession(store: store)
+    let document = CustomRuleDocument(rules: [
+      CustomRule(action: .proxy, match: try RuleMatch(domainExact: "saved.example"))
+    ])
+
+    let result = session.commit(document)
+
+    #expect(result.outcome == .saved)
+    #expect(result.document == document)
+    #expect(try session.load() == document)
+    #expect(session.revision == 1)
+    #expect(session.commit(document).outcome == .saved)
+    #expect(session.revision == 1)
+    #expect(try RuleDocumentSession(store: store).load() == document)
+  }
+}
+
+extension RuleDocumentSessionTests {
+  @Test(arguments: ["fixedConflict", "duplicate", "fixedDisabled"])
+  func invalidCommitRetainsSavedFactsAndRevision(reason: String) throws {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    let store = CustomRuleStore(fileURL: directory.appendingPathComponent("rules.json"))
+    let session = RuleDocumentSession(store: store)
+    let kept = CustomRule(action: .direct, match: try RuleMatch(domainExact: "kept.example"))
+    let original = CustomRuleDocument(rules: [kept])
+    #expect(session.commit(original).outcome == .saved)
+    let proposal: CustomRuleDocument
+    switch reason {
+    case "fixedConflict":
+      proposal = CustomRuleDocument(rules: [
+        kept,
+        CustomRule(action: .proxy, match: try RuleMatch(ipv4CIDR: "127.0.0.0/8")),
+      ])
+    case "duplicate":
+      proposal = CustomRuleDocument(rules: [
+        kept,
+        CustomRule(action: kept.action, match: kept.rule.match),
+      ])
+    default:
+      proposal = CustomRuleDocument(
+        rules: [kept],
+        disabledIdentities: [
+          ProxyRule(action: .direct, match: try RuleMatch(ipv4CIDR: "127.0.0.0/8"))
+        ])
+    }
+
+    let result = session.commit(proposal)
+
+    if reason == "fixedDisabled" {
+      #expect(result.outcome == .invalidDocument(detail: "Fixed local policy cannot be disabled"))
+    } else {
+      guard case .rejected(let rejected) = result.outcome else {
+        Issue.record("Expected whole-document rejection, got \(result.outcome)")
+        return
+      }
+      #expect(
+        rejected.map(\.reason).allSatisfy {
+          $0 == (reason == "duplicate" ? .duplicate : .conflictsWithFixedLocalScope)
+        })
+      #expect(!rejected.isEmpty)
+    }
+    #expect(result.document == original)
+    #expect(session.current == original)
+    #expect(session.revision == 1)
+    #expect(try RuleDocumentSession(store: store).load() == original)
+  }
+}
+
+extension RuleDocumentSessionTests {
+  @Test func corruptFirstLoadRejectsCommitWithoutOverwritingFile() throws {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
+    let store = CustomRuleStore(fileURL: directory.appendingPathComponent("rules.json"))
+    let corrupt = Data("broken".utf8)
+    try corrupt.write(to: store.fileURL)
+    let session = RuleDocumentSession(store: store)
+
+    let result = session.commit(CustomRuleDocument(rules: []))
+
+    #expect(result.outcome == .persistenceFailed)
+    #expect(result.document == nil)
+    #expect(session.current == nil)
+    #expect(session.revision == 0)
+    #expect(try Data(contentsOf: store.fileURL) == corrupt)
+  }
+
+  @Test func failedWriteRetainsOwnedFactsAndRevision() throws {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    let storage = directory.appendingPathComponent("storage")
+    let store = CustomRuleStore(fileURL: storage.appendingPathComponent("rules.json"))
+    let session = RuleDocumentSession(store: store)
+    let original = CustomRuleDocument(rules: [
+      CustomRule(action: .direct, match: try RuleMatch(domainExact: "kept.example"))
+    ])
+    #expect(session.commit(original).outcome == .saved)
+    let backup = directory.appendingPathComponent("backup")
+    try FileManager.default.moveItem(at: storage, to: backup)
+    try Data("blocked".utf8).write(to: storage)
+
+    let result = session.commit(CustomRuleDocument(rules: []))
+
+    #expect(result.outcome == .persistenceFailed)
+    #expect(result.document == original)
+    #expect(try session.load() == original)
+    #expect(session.revision == 1)
+    try FileManager.default.removeItem(at: storage)
+    try FileManager.default.moveItem(at: backup, to: storage)
+    #expect(try RuleDocumentSession(store: store).load() == original)
+  }
+
+  @Test func overlappingOppositeActionsAndOrphanedDisablementRemainSavedIntent() throws {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    let store = CustomRuleStore(fileURL: directory.appendingPathComponent("rules.json"))
+    let session = RuleDocumentSession(store: store)
+    let document = CustomRuleDocument(
+      rules: [
+        CustomRule(action: .direct, match: try RuleMatch(domainSuffix: "example.com")),
+        CustomRule(action: .proxy, match: try RuleMatch(domainExact: "a.example.com")),
+        CustomRule(action: .direct, match: try RuleMatch(domainExact: "a.example.com")),
+      ],
+      disabledIdentities: [
+        ProxyRule(action: .proxy, match: try RuleMatch(domainExact: "orphan.example"))
+      ])
+
+    let result = session.commit(document)
+
+    #expect(result.outcome == .saved)
+    #expect(result.document == document)
+    #expect(try RuleDocumentSession(store: store).load() == document)
+  }
+}

ShadowsocksX-NG2/Tests/Rules/RuleEnablementControllerTests.swift

@@ -300,7 +300,7 @@ extension RuleEnablementControllerTests {
       deploymentIsRunning = true
     }
     let disabledDocument = CustomRuleDocument(rules: [rule], disabledIdentities: [rule.identity])
-    let pending = Task { await controller.updateRuleDocument(disabledDocument) }
+    let pending = Task { await controller.commitRuleDocument(disabledDocument).outcome }
     try await withRuleApplicationTask(
       pending, controller: controller, release: deploymentGate.resume,
       body: {
@@ -372,8 +372,9 @@ extension RuleEnablementControllerTests {
       settings: ProxyRuntimeSettings(
         listen: ActivationFixture.listen, rulePreset: .chinaDirect, agentEnabled: false))
     let identity = ProxyRule(action: .proxy, match: try RuleMatch(domainExact: "saved.example"))
-    let outcome = await controller.updateRuleDocument(
-      CustomRuleDocument(rules: [], disabledIdentities: [identity]))
+    let outcome = await controller.commitRuleDocument(
+      CustomRuleDocument(rules: [], disabledIdentities: [identity])
+    ).outcome
     XCTAssertEqual(outcome, .saved)
     await controller.ruleApplicationTask?.value
     XCTAssertEqual(agent.registerCount, 0)
@@ -384,7 +385,7 @@ extension RuleEnablementControllerTests {
       store: store,
       settings: ProxyRuntimeSettings(
         listen: ActivationFixture.listen, rulePreset: .chinaDirect, agentEnabled: false))
-    let rejected = await newSession.updateRuleDocument(CustomRuleDocument(rules: []))
+    let rejected = await newSession.commitRuleDocument(CustomRuleDocument(rules: [])).outcome
     XCTAssertEqual(rejected, .persistenceFailed)
     XCTAssertEqual(try Data(contentsOf: store.fileURL), broken)
     XCTAssertEqual(agent.registerCount, 0)

ShadowsocksX-NG2/Tests/Rules/RuleEnablementWorkflowTests.swift

@@ -61,15 +61,7 @@ final class RuleEnablementWorkflowTests: XCTestCase {
     let workflow = RulesWorkflow(
       preset: .chinaDirect,
       loadDocument: { try session.load() },
-      commitDocument: { document in
-        do {
-          try session.save(document)
-          return RuleDocumentCommit(outcome: .saved, document: session.current)
-        } catch {
-          XCTFail("Fixture save failed: \(error)")
-          return RuleDocumentCommit(outcome: .persistenceFailed, document: session.current)
-        }
-      },
+      commitDocument: { session.commit($0) },
       analysisDelay: { await gate.wait() },
       builtinSnapshots: BuiltinRuleSnapshots(loader: { source in
         if source == .geolocationCN {

ShadowsocksX-NG2/Tests/Rules/RulePresetWorkflowTests.swift

@@ -150,14 +150,8 @@ extension RulePresetWorkflowTests {
       let session = RuleDocumentSession(store: CustomRuleStore(preset: preset, baseURL: baseURL))
       return RulesWorkflow(
         preset: preset, loadDocument: { try session.load() },
-        commitDocument: { document in
-          do {
-            try session.save(document)
-            return RuleDocumentCommit(outcome: .saved, document: document)
-          } catch {
-            return RuleDocumentCommit(outcome: .persistenceFailed, document: session.current)
-          }
-        }, builtinSnapshots: BuiltinRuleSnapshots(loader: { rulesFixture($0) }))
+        commitDocument: { session.commit($0) },
+        builtinSnapshots: BuiltinRuleSnapshots(loader: { rulesFixture($0) }))
     }
   }
 }