review

Author: ethanndickson

Coder Desktop/Coder Desktop.xcodeproj/project.pbxproj

@@ -984,7 +984,7 @@
 				MARKETING_VERSION = 1.0;
 				MODULE_VERIFIER_SUPPORTED_LANGUAGES = "objective-c objective-c++";
 				MODULE_VERIFIER_SUPPORTED_LANGUAGE_STANDARDS = "gnu17 gnu++20";
-				PRODUCT_BUNDLE_IDENTIFIER = coder.VPNLib;
+				PRODUCT_BUNDLE_IDENTIFIER = "$(APPLICATION_IDENTIFIER).VPNLib";
 				PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)";
 				SKIP_INSTALL = YES;
 				SWIFT_EMIT_LOC_STRINGS = YES;
@@ -1020,7 +1020,7 @@
 				MARKETING_VERSION = 1.0;
 				MODULE_VERIFIER_SUPPORTED_LANGUAGES = "objective-c objective-c++";
 				MODULE_VERIFIER_SUPPORTED_LANGUAGE_STANDARDS = "gnu17 gnu++20";
-				PRODUCT_BUNDLE_IDENTIFIER = coder.VPNLib;
+				PRODUCT_BUNDLE_IDENTIFIER = "$(APPLICATION_IDENTIFIER).VPNLib";
 				PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)";
 				SKIP_INSTALL = YES;
 				SWIFT_EMIT_LOC_STRINGS = YES;
@@ -1040,7 +1040,7 @@
 				GENERATE_INFOPLIST_FILE = YES;
 				MACOSX_DEPLOYMENT_TARGET = 15.0;
 				MARKETING_VERSION = 1.0;
-				PRODUCT_BUNDLE_IDENTIFIER = coder.VPNLibTests;
+				PRODUCT_BUNDLE_IDENTIFIER = "com.coder.Coder-Desktop.VPNLibTests";
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				SWIFT_EMIT_LOC_STRINGS = NO;
 				SWIFT_VERSION = 6.0;
@@ -1057,7 +1057,7 @@
 				GENERATE_INFOPLIST_FILE = YES;
 				MACOSX_DEPLOYMENT_TARGET = 15.0;
 				MARKETING_VERSION = 1.0;
-				PRODUCT_BUNDLE_IDENTIFIER = coder.VPNLibTests;
+				PRODUCT_BUNDLE_IDENTIFIER = "com.coder.Coder-Desktop.VPNLibTests";
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				SWIFT_EMIT_LOC_STRINGS = NO;
 				SWIFT_VERSION = 6.0;

Coder Desktop/VPNLib/Downloader.swift

@@ -107,7 +107,7 @@ public struct SignatureValidator: Validator {
     }
 }
 
-public actor Downloader {
+public struct Downloader {
     let validator: Validator
     public init(validator: Validator = SignatureValidator()) {
         self.validator = validator
@@ -116,14 +116,14 @@ public actor Downloader {
     public func download(src: URL, dest: URL) async throws {
         var req = URLRequest(url: src)
         if FileManager.default.fileExists(atPath: dest.path) {
-            if let existingFileData = try? Data(contentsOf: dest) {
+            if let existingFileData = try? Data(contentsOf: dest, options: .mappedIfSafe) {
                 req.setValue(etag(data: existingFileData), forHTTPHeaderField: "If-None-Match")
             }
         }
         // TODO: Add Content-Length headers to coderd, add download progress delegate
         let (tempURL, response) = try await URLSession.shared.download(for: req)
         defer {
-            if FileManager.default.fileExists(atPath: dest.path) {
+            if FileManager.default.fileExists(atPath: tempURL.path) {
                 do { try FileManager.default.removeItem(at: tempURL) } catch {}
             }
         }
@@ -132,6 +132,7 @@ public actor Downloader {
             throw DownloadError.invalidResponse
         }
         guard httpResponse.statusCode != 304 else {
+            // We already have the latest dylib downloaded on disk
             return
         }
         guard (200 ..< 300).contains(httpResponse.statusCode) else {

Coder Desktop/VPNLibTests/DownloaderTests.swift

@@ -43,11 +43,17 @@ struct DownloaderTests {
         let downloadedData = try Data(contentsOf: destinationURL)
         #expect(downloadedData == testData)
 
-        Mock(url: fileURL, contentType: .html, statusCode: 304, data: [.get: Data()]).register()
+        var mock = Mock(url: fileURL, contentType: .html, statusCode: 304, data: [.get: Data()])
+        var etagIncluded = false
+        mock.onRequestHandler = OnRequestHandler { request in
+            etagIncluded = request.value(forHTTPHeaderField: "If-None-Match") == etag(data: testData)
+        }
+        mock.register()
 
         try await downloader.download(src: fileURL, dest: destinationURL)
         let unchangedData = try Data(contentsOf: destinationURL)
         #expect(unchangedData == testData)
+        #expect(etagIncluded)
     }
 
     @Test
@@ -66,10 +72,16 @@ struct DownloaderTests {
         var downloadedData = try Data(contentsOf: destinationURL)
         #expect(downloadedData == ogData)
 
-        Mock(url: fileURL, contentType: .html, statusCode: 200, data: [.get: newData]).register()
+        var mock = Mock(url: fileURL, contentType: .html, statusCode: 200, data: [.get: newData])
+        var etagIncluded = false
+        mock.onRequestHandler = OnRequestHandler { request in
+            etagIncluded = request.value(forHTTPHeaderField: "If-None-Match") == etag(data: ogData)
+        }
+        mock.register()
 
         try await downloader.download(src: fileURL, dest: destinationURL)
         downloadedData = try Data(contentsOf: destinationURL)
         #expect(downloadedData == newData)
+        #expect(etagIncluded)
     }
 }