From 222aa70c3fc67d8ab9fb5bc8ba299d9ecd86a643 Mon Sep 17 00:00:00 2001 From: Qiu Yuzhou Date: Sun, 25 Oct 2026 12:59:22 +0800 Subject: [PATCH] refactor(rules): centralize document commits --- .../ProxyRuntimeController+CustomRules.swift | 79 +--------- .../ProxyRuntime/ProxyRuntimeController.swift | 1 - .../Domain/Rules/Custom/CustomRuleStore.swift | 24 --- .../Rules/Custom/RuleDocumentSession.swift | 79 ++++++++++ .../Custom/RuleDocumentSessionTests.swift | 147 ++++++++++++++++++ .../Rules/RuleEnablementControllerTests.swift | 9 +- .../Rules/RuleEnablementWorkflowTests.swift | 10 +- .../Tests/Rules/RulePresetWorkflowTests.swift | 10 +- 8 files changed, 238 insertions(+), 121 deletions(-) create mode 100644 ShadowsocksX-NG2/Domain/Rules/Custom/RuleDocumentSession.swift create mode 100644 ShadowsocksX-NG2/Tests/Rules/Custom/RuleDocumentSessionTests.swift diff --git a/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController+CustomRules.swift b/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController+CustomRules.swift index 8e29e388..bed9e00b 100644 --- a/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController+CustomRules.swift +++ b/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()) diff --git a/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController.swift b/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController.swift index 0403b5cf..dcd587e7 100644 --- a/ShadowsocksX-NG2/App/Application/ProxyRuntime/ProxyRuntimeController.swift +++ b/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? var ruleApplicationGeneration = 0 var ruleApplicationFailure: RuntimeFailureFacts? { didSet { factChanges.markChanged() } } diff --git a/ShadowsocksX-NG2/Domain/Rules/Custom/CustomRuleStore.swift b/ShadowsocksX-NG2/Domain/Rules/Custom/CustomRuleStore.swift index 03777ee8..4c86fd76 100644 --- a/ShadowsocksX-NG2/Domain/Rules/Custom/CustomRuleStore.swift +++ b/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 diff --git a/ShadowsocksX-NG2/Domain/Rules/Custom/RuleDocumentSession.swift b/ShadowsocksX-NG2/Domain/Rules/Custom/RuleDocumentSession.swift new file mode 100644 index 00000000..7d29bd07 --- /dev/null +++ b/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 + } +} diff --git a/ShadowsocksX-NG2/Tests/Rules/Custom/RuleDocumentSessionTests.swift b/ShadowsocksX-NG2/Tests/Rules/Custom/RuleDocumentSessionTests.swift new file mode 100644 index 00000000..b30fe920 --- /dev/null +++ b/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) + } +} diff --git a/ShadowsocksX-NG2/Tests/Rules/RuleEnablementControllerTests.swift b/ShadowsocksX-NG2/Tests/Rules/RuleEnablementControllerTests.swift index 20eacda9..ac5ebb75 100644 --- a/ShadowsocksX-NG2/Tests/Rules/RuleEnablementControllerTests.swift +++ b/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) diff --git a/ShadowsocksX-NG2/Tests/Rules/RuleEnablementWorkflowTests.swift b/ShadowsocksX-NG2/Tests/Rules/RuleEnablementWorkflowTests.swift index 709ef800..9029093c 100644 --- a/ShadowsocksX-NG2/Tests/Rules/RuleEnablementWorkflowTests.swift +++ b/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 { diff --git a/ShadowsocksX-NG2/Tests/Rules/RulePresetWorkflowTests.swift b/ShadowsocksX-NG2/Tests/Rules/RulePresetWorkflowTests.swift index 0d4a27b4..135c41ac 100644 --- a/ShadowsocksX-NG2/Tests/Rules/RulePresetWorkflowTests.swift +++ b/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) })) } } }