Strongly type settings for the network extension - #806
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces dictionary-based network-extension settings with a new NSSecureCoding class Changes
Sequence Diagram(s)sequenceDiagram
participant NetExtClient as NetExt Client
participant XPC as XPC Endpoint
participant Daemon as santad Daemon
participant NetExtQueue as NetworkExtensionQueue
NetExtClient->>XPC: registerNetworkExtensionWithProtocolVersion(version)
XPC->>Daemon: forward registerNetworkExtensionWithProtocolVersion(version)
Daemon->>NetExtQueue: handleRegistrationWithProtocolVersion(version)
NetExtQueue-->>Daemon: SNTNetworkExtensionSettings(enable: Bool)
Daemon-->>XPC: reply(SNTNetworkExtensionSettings, santaProtocolVersion, error)
XPC->>NetExtClient: deliver reply(SNTNetworkExtensionSettings, santaProtocolVersion, error)
Note over NetExtQueue, XPC: On settings changes:
NetExtQueue->>XPC: updateNetworkExtensionSettings(SNTNetworkExtensionSettings)
XPC->>NetExtClient: deliver update(SNTNetworkExtensionSettings)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Source/common/ne/SNTNetworkExtensionSettingsTest.mm`:
- Around line 76-105: The test method testForwardCompatibility has a
clang-format styling violation around the NSKeyedUnarchiver initialization and
subsequent calls; reformat the block (or run clang-format on the file) so the
NSKeyedUnarchiver initialization line (initForReadingFromData:error:), the
requiresSecureCoding assignment, the setClass:forClassName: call, and the
decodeObjectOfClass:forKey: call conform to the project's clang-format style;
ensure indentation, line breaks, and spacing in the testForwardCompatibility
method match surrounding code.
- Around line 107-122: The test uses a freshly encoded
SNTNetworkExtensionSettings so the 'enable' key is present; to simulate a legacy
archive that omits that key, add a small helper legacy class (e.g.
LegacyNetworkExtensionSettings) whose -encodeWithCoder: intentionally does not
encode the @"enable" key, archive an instance of that legacy class, then when
unarchiving remap the legacy class name to SNTNetworkExtensionSettings using
NSKeyedUnarchiver's class-for-name mapping (e.g. +[NSKeyedUnarchiver
setClass:forClassName:]) before calling unarchivedObjectOfClass:fromData:error:
and assert deserialized.enable is NO; modify testBackwardCompatibility to
perform these steps and remove the current direct-archiving of
SNTNetworkExtensionSettings.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Source/common/ne/SNTNetworkExtensionSettingsTest.mm
Remove reliance on a dictionary and move to using real types.
Part of SNT-261