Simplify unlock logic, don't spam challenges

Sesame-Watch Watch App/ContentView.swift

@@ -24,6 +24,13 @@ struct ContentView: View {
         .white
     }
     
+    private var stateText: String {
+        if coordinator.state == .notChecked {
+            return "Unlock"
+        }
+        return coordinator.state.description
+    }
+    
     var body: some View {
         HStack {
             Spacer()
@@ -39,7 +46,7 @@ struct ContentView: View {
                         .progressViewStyle(CircularProgressViewStyle())
                         .frame(width: 20, height: 20)
                 } else {
-                    Text("Unlock")
+                    Text(stateText)
                         .font(.subheadline)
                 }
             }

Sesame/Common/RequestCoordinator.swift

@@ -4,15 +4,9 @@ import SwiftData
 
 final class RequestCoordinator: ObservableObject {
     
-    @Published
-    var serverChallenge: ServerChallenge? = nil
-    
     @Published
     var state: MessageResult = .noKeyAvailable
     
-    @Published
-    private var timer: Timer?
-    
     @Published
     var pendingRequests: [PendingOperation] = []
     
@@ -22,6 +16,9 @@ final class RequestCoordinator: ObservableObject {
     @Published
     var keyManager = KeyManagement()
     
+    @Published
+    var isPerformingRequest: Bool = false
+    
     @AppStorage("server")
     var serverPath: String = "https://christophhagen.de/sesame/"
     
@@ -31,7 +28,11 @@ final class RequestCoordinator: ObservableObject {
     @AppStorage("connectionType")
     var connectionType: ConnectionStrategy = .remoteFirst
     
-    let modelContext: ModelContext
+    private let modelContext: ModelContext
+    
+    private let client = Client()
+    
+    private var timer: Timer?
     
     init(modelContext: ModelContext) {
         self.modelContext = modelContext
@@ -40,151 +41,97 @@ final class RequestCoordinator: ObservableObject {
         }
     }
     
-    let client = Client()
-    
-    var needsNewServerChallenge: Bool {
-        serverChallenge?.isExpired ?? true
-    }
-    
-    @Published
-    var isPerformingRequest: Bool = false
-    
     func startUnlock() {
-        addOperations(.challenge, .unlock)
+        guard !isPerformingRequest else {
-    }
-    
-    func startChallenge() {
-        addOperations(.challenge)
-    }
-    
-    private func addOperations(_ operations: RequestType...) {
-        #warning("Only perform challenge when doing unlock? Remove code complexity")
-        // Just add all operations for an unlock
-        // For every completed operation, the unnecessary ones will be removed without executing them
-        let operations = connectionType.transmissionTypes.map { route in
-            operations.map { PendingOperation(route: route, operation: $0) }
-        }.joined()
-        
-        pendingRequests.append(contentsOf: operations)
-        continueRequests()
-    }
-    
-    private func continueRequests() {
-        guard activeRequest == nil else {
             return
         }
-        guard !pendingRequests.isEmpty else {
+        isPerformingRequest = true
-            self.isPerformingRequest = false
-            return
-        }
-        let activeRequest = pendingRequests.removeFirst()
-        self.activeRequest = activeRequest
-        self.isPerformingRequest = true
         Task {
-            await process(request: activeRequest)
+            let finalResult = await performFullChallengeResponse()
+            DispatchQueue.main.async {
+                self.state = finalResult
+                self.isPerformingRequest = false
+            }
+            scheduleReturnToReadyState()
         }
     }
     
-    private func process(request: PendingOperation) async {
+    private func performFullChallengeResponse() async -> MessageResult {
+        let transmissionTypes = connectionType.transmissionTypes
+        for route in transmissionTypes.dropLast() {
+            if await performUnlockAndSaveItem(route: route) == .unlocked {
+                return .unlocked
+            }
+        }
+        guard let route = transmissionTypes.last else {
+            // No transmission types at all
+            return keyManager.hasAllKeys ? .notChecked : .noKeyAvailable
+        }
+        return await performUnlockAndSaveItem(route: route)
+    }
+    
+    private func performUnlockAndSaveItem(route: TransmissionType) async -> MessageResult {
         let startTime = Date.now
-        let (success, response, challenge) = await self.start(request)
+        let result = await performFullChallengeResponse(route: route)
         let endTime = Date.now
         let roundTripTime = endTime.timeIntervalSince(startTime)
-        
+        print("Unlock took \(Int(roundTripTime * 1000)) ms (\(result.result))")
-        if let s = challenge?.message {
+        let item = HistoryItem(message: result, startDate: startTime, route: route, finishDate: endTime)
-            print("\(s) took \(Int(roundTripTime * 1000)) ms")
+        modelContext.insert(item)
-        } else {
+        return result.result
-            print("\(request.operation.description) took \(Int(roundTripTime * 1000)) ms")
-        }
-        
-        if request.operation == .unlock, let response {
-            print("Saving history item")
-            let item = HistoryItem(message: response, startDate: startTime, route: request.route, finishDate: endTime)
-            modelContext.insert(item)
-        }
-
-        DispatchQueue.main.async {
-            self.filterPendingRequests(after: request, success: success, hasChallenge: challenge != nil)
-            if let response {
-                self.state = response.result
-            }
-            if let challenge {
-                self.serverChallenge = challenge
-            }
-            self.activeRequest = nil
-            self.continueRequests()
-        }
     }
     
-    private func filterPendingRequests(after operation: PendingOperation, success: Bool, hasChallenge: Bool) {
+    private func performFullChallengeResponse(route: TransmissionType) async -> Message {
-        if success {
+        let (challengeResponse, challenge) = await performChallenge(route: route)
-            // Filter all unlocks
+        guard let challenge else {
-            if operation.operation == .unlock {
+            return challengeResponse
-                // Successful unlock means no need for next challenge or unlocks, so remove all
-                self.pendingRequests = []
-            } else {
-                // Successful challenge means no need for additional challenges, but keep unlocks
-                self.pendingRequests = pendingRequests.filter { $0.operation != .challenge }
-            }
-        } else {
-            // Filter all operations with the same route for connection errors
-            // And with type, depending on error?
         }
+        
+        let (unlockResponse, secondaryChallenge) = await performUnlock(with: challenge.message, route: route)
+        guard let secondaryChallenge else {
+            return unlockResponse
+        }
+        let (secondUnlockResponse, _) = await performUnlock(with: secondaryChallenge.message, route: route)
+        return secondUnlockResponse
     }
     
-    private func start(_ operation: PendingOperation) async -> OptionalServerResponse {
+    private func performChallenge(route: TransmissionType) async -> ServerResponse {
-        switch operation.operation {
-        case .challenge:
-            if let serverChallenge, !serverChallenge.isExpired {
-                return (true, serverChallenge.message, serverChallenge)
-            }
-            return await performChallenge(route: operation.route)
-            
-        case .unlock:
-            guard let serverChallenge, !serverChallenge.isExpired else {
-                return (false, nil, nil)
-            }
-            return await performUnlock(with: serverChallenge.message, route: operation.route)
-        }
-    }
-    
-    private func performChallenge(route: TransmissionType) async -> OptionalServerResponse {
         let initialMessage = Message.initial()
         let (result, challenge) = await send(initialMessage, route: route)
         guard let message = challenge?.message else {
-            return (false, result, nil)
+            return (result, nil)
         }
         // Can't get here without the message being accepted
         guard message.messageType == .challenge else {
             print("Invalid message type for challenge: \(message)")
-            return (false, result.with(result: .invalidMessageTypeFromDevice), nil)
+            return (result.with(result: .invalidMessageTypeFromDevice), nil)
         }
-        return (true, result.with(result: .deviceAvailable), challenge)
+        return (result.with(result: .deviceAvailable), challenge)
     }
     
-    private func performUnlock(with challenge: Message, route: TransmissionType) async -> OptionalServerResponse {
+    private func performUnlock(with challenge: Message, route: TransmissionType) async -> ServerResponse {
         let request = challenge.requestMessage()
         let (unlockState, responseData) = await send(request, route: route)
         
         guard let response = responseData?.message else {
-            return (false, unlockState, nil)
+            return (unlockState, nil)
         }
         switch response.messageType {
         case .initial, .request:
             print("Invalid message type for response: \(response)")
-            return (false, response.with(result: .invalidMessageTypeFromDevice), nil)
+            return (response.with(result: .invalidMessageTypeFromDevice), nil)
         case .challenge:
             // New challenge received, challenge was expired
-            return (true, unlockState, responseData)
+            return (unlockState, responseData)
         case .response:
             break
         }
         
         guard response.serverChallenge == request.serverChallenge else {
             print("Invalid server challenge for unlock: \(response)")
-            return (false, response.with(result: .invalidServerChallengeFromDevice), nil)
+            return (response.with(result: .invalidServerChallengeFromDevice), nil)
         }
-        return (true, response.with(result: .unlocked), nil)
+        return (response.with(result: .unlocked), nil)
     }
     
     private func url(for route: TransmissionType) -> String {
@@ -204,26 +151,25 @@ final class RequestCoordinator: ObservableObject {
         return await client.send(message, to: url, through: route, using: keys)
     }
     
-    func startUpdatingServerChallenge() {
+    func resetState() {
-        guard timer == nil else {
+        let hasKeys = keyManager.hasAllKeys
-            return
-        }
         DispatchQueue.main.async {
-            self.timer = Timer.scheduledTimer(withTimeInterval: 3, repeats: true) { [weak self] timer in
+            self.state = hasKeys ? .notChecked : .noKeyAvailable
-                guard let self else {
-                    timer.invalidate()
-                    return
-                }
-                DispatchQueue.main.async {
-                    self.startChallenge()
-                }
-            }
-            self.timer!.fire()
         }
     }
 
-    func endUpdatingServerChallenge() {
+    func scheduleReturnToReadyState() {
         timer?.invalidate()
-        timer = nil
+        
+        DispatchQueue.main.async {
+            self.timer = Timer.scheduledTimer(withTimeInterval: 3, repeats: false) { [weak self] timer in
+                defer { timer.invalidate() }
+                guard let self else {
+                    return
+                }
+                self.resetState()
+                self.timer = nil
+            }
+        }
     }
 }

Sesame/ContentView.swift

@@ -65,8 +65,6 @@ struct ContentView: View {
             .padding(.horizontal, 30)
         }
         .background(coordinator.state.color)
-        .onAppear(perform: coordinator.startUpdatingServerChallenge)
-        .onDisappear(perform: coordinator.endUpdatingServerChallenge)
         .animation(.easeInOut, value: coordinator.state.color)
         .sheet(isPresented: $showSettingsSheet) {
             SettingsView(