refactor(cordova/apple): move network settings logic to Swift #1658

fortuna · 2023-07-05T21:42:31Z

The goal is to eventually get rid of all Objective-C code.
This PR illustrates how we can incrementally move code.

With the move, I'm also able to add unit tests.

fortuna · 2023-07-05T21:43:20Z

For review, I recommend hiding the whitespaces. I was fighting with the editor, so I just had to reformat the entire file.

codecov · 2023-07-05T21:46:08Z

Codecov Report

Patch coverage: 66% and project coverage change: -1 ⚠️

Comparison is base (97dae88) 35% compared to head (37f3d17) 35%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #1658    +/-   ##
=======================================
- Coverage      35%     35%    -1%     
=======================================
  Files          45      47     +2     
  Lines        2532    2852   +320     
  Branches      314     314            
=======================================
+ Hits          904    1012   +108     
- Misses       1628    1840   +212

Flag	Coverage Δ
apple	`13% <66%> (+11%)`	⬆️
ios	`?`
macos	`13% <66%> (+11%)`	⬆️
unittests	`35% <66%> (-1%)`	⬇️
www	`45% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ces/OutlineTunnelSources/OutlineConnectivity.swift	`0% <ø> (ø)`
...eLib/Sources/OutlineTunnelSources/OutlineVpn.swift	`0% <0%> (ø)`
...AppleLib/Sources/OutlineTunnelSources/Subnet.swift	`84% <ø> (+84%)`	⬆️
...PacketTunnelProviderSources/PacketTunnelProvider.m	`0% <0%> (ø)`
...b/Sources/OutlineTunnelSources/OutlineTunnel.swift	`67% <66%> (+67%)`	⬆️
...ib/Tests/OutlineTunnelTest/OutlineTunnelTest.swift	`100% <100%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

jyyi1

Thanks, one more step closer to a modern app now.

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineTunnel.swift

…/OutlineTunnel.swift Co-authored-by: J. Yi <[email protected]>

fortuna

Thanks for the review and the Swift tips. I don't really know Swift! 😅
Please take another look.
While waiting for the review I moved some extra files from the plugin to the lib. This makes allows us to develop in place (no rsync!), add tests and will make it easier to refactor and cleanup.

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineTunnel.swift

jyyi1

LGTM, just with some questions to confirm.

src/cordova/setup.action.mjs

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineTunnel.swift

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

fortuna

I need to fix an issue with macOS. The launcher has a dependency on OutlineVpn that I need to fix.

src/cordova/setup.action.mjs

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineTunnel.swift

fortuna · 2023-07-07T13:19:34Z

This is ready for re-review.

@daniellacosse can you make the coverage test not fail?

fortuna · 2023-07-07T19:38:31Z

src/cordova/apple/OutlineAppleLib/Package.swift

@@ -8,10 +8,13 @@ let package = Package(
    products: [
        .library(
            name: "OutlineAppleLib",
-            targets: ["Tun2socks", "OutlineTunnel", "PacketTunnelProvider"]),
+            targets: ["Tun2socks", "OutlineTunnel"]),
+        .library(


The VpnExtension can now depend on the PacketTunnelProvider only.

fortuna · 2023-07-07T19:38:59Z

src/cordova/apple/OutlineAppleLib/Package.swift

    ],
    dependencies: [
-        .package(url: "https://github.com/CocoaLumberjack/CocoaLumberjack.git", exact: "3.7.4"),
+        .package(url: "https://github.com/CocoaLumberjack/CocoaLumberjack.git", from: "3.7.4"),


The exact directive prevented the XCode project from upgrading the dependencies.

fortuna · 2023-07-07T19:39:59Z

src/cordova/apple/xcode/ios/Outline.xcodeproj/project.pbxproj

@@ -17,17 +17,13 @@
 		302D95F114D2391D003F00A1 /* MainViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 302D95EF14D2391D003F00A1 /* MainViewController.m */; };
 		302D95F214D2391D003F00A1 /* MainViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 302D95F014D2391D003F00A1 /* MainViewController.xib */; };
 		3B0347531F212F0200C8EF1F /* VpnExtension.appex in Embed App Extensions */ = {isa = PBXBuildFile; fileRef = 3B0347481F212F0100C8EF1F /* VpnExtension.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
-		3B0347611F212F6900C8EF1F /* libz.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = 941052A220F54953928FF2E2 /* libz.tbd */; };


These are VpnExtension dependencies that were not needed because they are not used or listed in the Swift package.

fortuna · 2023-07-07T19:40:55Z

src/cordova/apple/xcode/ios/Outline.xcodeproj/project.pbxproj

 		52CE53E7295B6A310064D03D /* Sentry in Frameworks */ = {isa = PBXBuildFile; productRef = 52CE53E6295B6A310064D03D /* Sentry */; };
+		52E783062A5880CF00355E64 /* PacketTunnelProvider in Frameworks */ = {isa = PBXBuildFile; productRef = 52E783052A5880CF00355E64 /* PacketTunnelProvider */; };


The new dependency on the PacketTunnelProvider, replacing all the other ones.

fortuna · 2023-07-07T19:41:37Z

src/cordova/apple/xcode/ios/Outline.xcodeproj/project.pbxproj

@@ -99,7 +88,6 @@
 		0168F53D3AFF46F5B346C874 /* CDVStatusBar.h */ = {isa = PBXFileReference; explicitFileType = undefined; fileEncoding = 4; includeInIndex = 0; lastKnownFileType = sourcecode.c.h; name = CDVStatusBar.h; path = "cordova-plugin-statusbar/CDVStatusBar.h"; sourceTree = "<group>"; };
 		0207DA571B56EA530066E2B4 /* Images.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; name = Images.xcassets; path = Outline/Images.xcassets; sourceTree = SOURCE_ROOT; };
 		0394302BA6114B2AB648D4FF /* CDVStatusBar.m */ = {isa = PBXFileReference; explicitFileType = undefined; fileEncoding = 4; includeInIndex = 0; lastKnownFileType = sourcecode.c.objc; name = CDVStatusBar.m; path = "cordova-plugin-statusbar/CDVStatusBar.m"; sourceTree = "<group>"; };
-		119806AF98394D7D8749BB30 /* OutlineConnectivity.swift */ = {isa = PBXFileReference; explicitFileType = undefined; fileEncoding = 4; includeInIndex = 0; lastKnownFileType = sourcecode.swift; name = OutlineConnectivity.swift; path = "cordova-plugin-outline/OutlineConnectivity.swift"; sourceTree = "<group>"; };


This and others are now in the Swift package, not in the XCode project.

fortuna · 2023-07-07T19:42:08Z

src/cordova/apple/xcode/ios/Outline.xcodeproj/project.pbxproj

@@ -835,16 +809,16 @@
 			isa = XCRemoteSwiftPackageReference;
 			repositoryURL = "https://github.com/CocoaLumberjack/CocoaLumberjack";
 			requirement = {
-				kind = exactVersion;
-				version = 3.7.4;
+				kind = upToNextMajorVersion;


To allow for upgrades. The Package.resolved file pins the exact version.

fortuna · 2023-07-07T19:43:11Z

src/cordova/apple/xcode/ios/Outline.xcodeproj/xcshareddata/xcschemes/VpnExtension.xcscheme

@@ -21,30 +21,13 @@
               ReferencedContainer = "container:Outline.xcodeproj">
            </BuildableReference>
         </BuildActionEntry>
-         <BuildActionEntry


Bogus dependency on the Outline target. I think this schema was originally created by copying the one for the app.

fortuna · 2023-07-07T19:44:00Z

src/cordova/apple/xcode/macos/Outline.xcodeproj/project.pbxproj

 				FC5FF9471F3E1E8B0032A745 /* NetworkExtension.framework in Frameworks */,
-				52CBB84D295BC7F700D0073F /* CocoaLumberjack in Frameworks */,
-				52CBB84F295BC7F700D0073F /* CocoaLumberjackSwift in Frameworks */,
+				52C198972A587525006E0ACE /* PacketTunnelProvider in Frameworks */,


Cleaned up dependencies for the VpnExtension.

fortuna · 2023-07-07T19:45:40Z

src/cordova/test.action.mjs

@@ -51,7 +51,7 @@ export async function main(...parameters) {
    'clean',
    'test',
    '-scheme',
-    APPLE_LIBRARY_NAME,
+    `${APPLE_LIBRARY_NAME}-Package`,


This had to change because now the package has two products, one with the same name as the package, so the auto-generated scheme uses -Package for the package scheme. The library doesn't have the tests.

Move address selection logic to Swift

3df28c6

fortuna requested a review from jyyi1 July 5, 2023 21:42

fortuna requested a review from a team as a code owner July 5, 2023 21:42

github-actions bot added the size/M label Jul 5, 2023

fortuna changed the title ~~Move address selection logic to Swift~~ refactor(cordova/apple): Move address selection logic to Swift Jul 5, 2023

fortuna changed the title ~~refactor(cordova/apple): Move address selection logic to Swift~~ refactor(cordova/apple): move address selection logic to Swift Jul 5, 2023

fortuna added 2 commits July 5, 2023 20:33

Fix plists

fa8af37

getTunnelNetworkSettings in Swift

14ce1ff

github-actions bot added size/L and removed size/M labels Jul 6, 2023

fortuna changed the title ~~refactor(cordova/apple): move address selection logic to Swift~~ refactor(cordova/apple): move network settings logic to Swift Jul 6, 2023

jyyi1 approved these changes Jul 6, 2023

View reviewed changes

Move more code to Swift package

1ac4eee

fortuna requested a review from a team as a code owner July 6, 2023 18:08

fortuna and others added 2 commits July 6, 2023 14:18

Update src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources…

d0a5e95

…/OutlineTunnel.swift Co-authored-by: J. Yi <[email protected]>

Update src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources…

f6728ac

…/OutlineTunnel.swift Co-authored-by: J. Yi <[email protected]>

fortuna commented Jul 6, 2023

View reviewed changes

fortuna requested a review from jyyi1 July 6, 2023 18:32

fortuna added 4 commits July 6, 2023 14:34

Address review comments

79a4ea5

Clean up log imports

95d9cdf

Get build info

fbe28e4

Fix Apple projects

fe1994e

jyyi1 approved these changes Jul 6, 2023

View reviewed changes

fortuna added 2 commits July 6, 2023 17:48

Update dependencies

b46650b

Review

80287e4

fortuna commented Jul 6, 2023

View reviewed changes

fortuna added 2 commits July 6, 2023 23:41

Fix dependencies

1b71213

Fix schema

a9951bf

github-actions bot added size/XL and removed size/L labels Jul 7, 2023

Fix schema

1e71e03

fortuna requested a review from jyyi1 July 7, 2023 13:18

fortuna added 3 commits July 7, 2023 13:25

Extract PacketTunnelProvider

94bddfb

Fix test

25b5bde

Remove deps

37f3d17

fortuna commented Jul 7, 2023

View reviewed changes

jyyi1 approved these changes Jul 7, 2023

View reviewed changes

fortuna merged commit bccebce into master Jul 7, 2023
12 of 13 checks passed

fortuna deleted the fortuna-swift branch July 7, 2023 21:30

fortuna added the cleanup label Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cordova/apple): move network settings logic to Swift #1658

refactor(cordova/apple): move network settings logic to Swift #1658

fortuna commented Jul 5, 2023

fortuna commented Jul 5, 2023

codecov bot commented Jul 5, 2023 •

edited

Loading

jyyi1 left a comment

fortuna left a comment

jyyi1 left a comment

fortuna left a comment

fortuna commented Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

fortuna Jul 7, 2023

		52CE53E7295B6A310064D03D /* Sentry in Frameworks / = {isa = PBXBuildFile; productRef = 52CE53E6295B6A310064D03D / Sentry */; };
		52E783062A5880CF00355E64 /* PacketTunnelProvider in Frameworks / = {isa = PBXBuildFile; productRef = 52E783052A5880CF00355E64 / PacketTunnelProvider */; };

refactor(cordova/apple): move network settings logic to Swift #1658

refactor(cordova/apple): move network settings logic to Swift #1658

Conversation

fortuna commented Jul 5, 2023

fortuna commented Jul 5, 2023

codecov bot commented Jul 5, 2023 • edited Loading

Codecov Report

jyyi1 left a comment

Choose a reason for hiding this comment

fortuna left a comment

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

fortuna left a comment

Choose a reason for hiding this comment

fortuna commented Jul 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 5, 2023 •

edited

Loading