Merge pull request #14 from cursorinsight/denes/fixes

Implement minor fixes based on manual tests
cursorinsight · Jan 15, 2024 · afb9f9e · afb9f9e
2 parents 5fde1b2 + 89d883a
commit afb9f9e
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 36 deletions.
diff --git a/Sources/Trap/Manager/Manager.swift b/Sources/Trap/Manager/Manager.swift
@@ -40,12 +40,12 @@ public class TrapManager {
     private var hasLowBattery: Bool = false
 
     private var isRunning: Bool = false
-    
+
     // Currently used data collection config
     private var currentDataCollectionConfig : TrapConfig.DataCollection
 
     private var collectorChangeSemaphore : DispatchSemaphore  = DispatchSemaphore(value: 1)
-    
+
     /// Create an instance of the integration module,
     /// optionally with your configuration
     public init(
@@ -108,7 +108,12 @@ public class TrapManager {
         collectorChangeSemaphore.wait()
         self.hasLowBattery = getHasLowBattery()
         currentDataCollectionConfig = getDataCollectionConfig()
+        try startReportersAndCollectors()
+        collectorChangeSemaphore.signal()
         subscribeOnNotifications()
+    }
+
+    private func startReportersAndCollectors() throws {
         if !isRunning {
             if let actualDataMode = inLowDataMode {
                 isRunning = true
@@ -126,7 +131,6 @@ public class TrapManager {
                 }
             }
         }
-        collectorChangeSemaphore.signal()
     }
 
     /// Creates a collector
@@ -141,15 +145,19 @@ public class TrapManager {
     /// Turn off all collectors.
     public func haltAll() {
         collectorChangeSemaphore.wait()
+        stopReporterAndCollectors()
+        collectorChangeSemaphore.signal()
+        unsubscribeFromNotifications()
+    }
+
+    private func stopReporterAndCollectors() {
         if (isRunning) {
            isRunning = false
             collectors.values.forEach {
                 halt(collector: $0)
             }
             addStopMessage()
         }
-        unsubscribeFromNotifications()
-        collectorChangeSemaphore.signal()
     }
 
     private func unsubscribeFromNotifications() {
@@ -214,7 +222,7 @@ public class TrapManager {
     public func addCustomMetadata(key: String, value: String) {
         addCustomMetadata(key: key, value: DataType.string(value))
     }
-    
+
     public func removeCustomMetadata(key: String) {
         let metaCollectorKey = String(reflecting: TrapMetadataCollector.self)
         guard let metadataCollector = (collectors[metaCollectorKey] as? TrapMetadataCollector) else { return }
@@ -251,17 +259,24 @@ public class TrapManager {
     }
 
     private func networkChanged(path: NWPath){
-        let newValue = path.usesInterfaceType(.cellular) || path.isExpensive
-        if newValue != self.inLowDataMode {
-            self.inLowDataMode = newValue
-            maybeModifyConfigAndRestartCollection()
+        if let monitor = networkMonitor {
+            let newValue =
+              monitor.currentPath.status != .satisfied ||
+              monitor.currentPath.isExpensive ||
+              monitor.currentPath.isConstrained
+
+            if newValue != self.inLowDataMode {
+                self.inLowDataMode = newValue
+                maybeModifyConfigAndRestartCollection()
+            }
         }
     }
 
     @objc func batteryDidChange(_ notification: Notification) {
         let newValue = getHasLowBattery()
-        
+
         if newValue != self.hasLowBattery {
+            self.hasLowBattery = newValue
             maybeModifyConfigAndRestartCollection()
         }
         let batteryCollectorKey = String(reflecting: TrapBatteryCollector.self)
@@ -272,18 +287,19 @@ public class TrapManager {
     private func getHasLowBattery() -> Bool {
         return (UIDevice.current.batteryState == .unplugged || UIDevice.current.batteryState == .unknown) &&
             UIDevice.current.batteryLevel < config.lowBatteryThreshold &&
-            UIDevice.current.batteryLevel >= 0   
+            UIDevice.current.batteryLevel >= 0
     }
-    
+
     private func maybeModifyConfigAndRestartCollection() {
-        if (!isRunning || !(getDataCollectionConfig() != currentDataCollectionConfig)) {
-            haltAll()
-            do {
-                try runAll()
-            } catch {
-                print("Could not restart collectors")
-            }
+        collectorChangeSemaphore.wait()
+        stopReporterAndCollectors()
+        currentDataCollectionConfig = getDataCollectionConfig()
+        do {
+            try startReportersAndCollectors()
+        } catch {
+            print("Could not restart collectors")
         }
+        collectorChangeSemaphore.signal()
     }
 
     private func getDataCollectionConfig() -> TrapConfig.DataCollection {

diff --git a/Sources/Trap/Utils/Time.swift b/Sources/Trap/Utils/Time.swift
@@ -5,16 +5,16 @@ class TrapTime {
 
     public static func normalizeTime(_ time: TimeInterval) -> Int64 {
         if bootTime == nil {
-            // Query system boot time
-            var tv = timeval()
-            var tvSize = MemoryLayout<timeval>.size
-            let err = sysctlbyname("kern.boottime", &tv, &tvSize, nil, 0)
-            if err == 0, tvSize == MemoryLayout<timeval>.size {
-                bootTime = Date(timeIntervalSince1970: Double(tv.tv_sec) + Double(tv.tv_usec) / 1_000_000.0)
-            } else {
-                debugPrint("Cannot load the system boot time for touch precision timestamp")
+            // // Query system boot time
+            // var tv = timeval()
+            // var tvSize = MemoryLayout<timeval>.size
+            // let err = sysctlbyname("kern.boottime", &tv, &tvSize, nil, 0)
+            // if err == 0, tvSize == MemoryLayout<timeval>.size {
+            //     bootTime = Date(timeIntervalSince1970: Double(tv.tv_sec) + Double(tv.tv_usec) / 1_000_000.0)
+            // } else {
+            //     debugPrint("Cannot load the system boot time for touch precision timestamp")
                 bootTime = Date() - ProcessInfo().systemUptime
-            }
+            // }
         }
 
         return Int64((bootTime!.timeIntervalSince1970 + time) * 1000)

diff --git a/Tests/TrapTests/Manager/ManagerTests.swift b/Tests/TrapTests/Manager/ManagerTests.swift
@@ -9,10 +9,10 @@ class ManagerTests: XCTestCase {
         MockCollector.stopCalled = { stopCalled.fulfill() }
         var config = TrapConfig()
         config.defaultDataCollection.collectors = []
-        var manager: TrapManager? = try TrapManager(withConfig: config, withReporterQueue: nil, withCollectorQueue: nil)
+        let manager: TrapManager = TrapManager(withConfig: config, withReporterQueue: nil, withCollectorQueue: nil)
         let mockCollector = MockCollector()
-        try manager!.run(collector: mockCollector)
-        manager?.halt(collector: mockCollector)
+        try manager.run(collector: mockCollector)
+        manager.halt(collector: mockCollector)
 
         wait(for: [startCalled, stopCalled], timeout: 1)
     }
@@ -24,10 +24,10 @@ class ManagerTests: XCTestCase {
         MockCollector.stopCalled = { stopCalled.fulfill() }
         var config = TrapConfig()
         config.defaultDataCollection.collectors = [String(reflecting: MockCollector.self)]
-        var manager: TrapManager? = try TrapManager(withConfig: config, withReporterQueue: nil, withCollectorQueue: nil)
-        try manager?.runAll()
-        wait(for: [startCalled], timeout: 5)
-        manager?.haltAll()
+        let manager: TrapManager = TrapManager(withConfig: config, withReporterQueue: nil, withCollectorQueue: nil)
+        try manager.runAll()
+        wait(for: [startCalled], timeout: 10)
+        manager.haltAll()
         wait(for: [stopCalled], timeout: 1)
     }
 }