From e02b12e8e2e5464545a0a9a1fd79e38633c85ff3 Mon Sep 17 00:00:00 2001 From: Laurent Date: Fri, 27 Sep 2024 15:33:37 +0200 Subject: [PATCH] Fix nasty bug executing api calls forever --- LeStorage/ApiCallCollection.swift | 151 +++++++++++++++--------------- LeStorage/StoreCenter.swift | 16 ++-- 2 files changed, 87 insertions(+), 80 deletions(-) diff --git a/LeStorage/ApiCallCollection.swift b/LeStorage/ApiCallCollection.swift index 0c4d979..82b203e 100644 --- a/LeStorage/ApiCallCollection.swift +++ b/LeStorage/ApiCallCollection.swift @@ -7,15 +7,14 @@ import Foundation - protocol SomeCallCollection { - + func findCallById(_ id: String) async -> (any SomeCall)? func deleteById(_ id: String) async - + func hasPendingCalls() async -> Bool func contentOfFile() async -> String? - + func reset() async } @@ -24,19 +23,19 @@ protocol SomeCallCollection { /// The Api calls are serialized and stored in a JSON file /// Failing Api calls are stored forever and will be executed again later actor ApiCallCollection: SomeCallCollection { - + /// The list of api calls fileprivate(set) var items: [ApiCall] = [] - + /// The number of time an execution loop has been called fileprivate var _attemptLoops: Int = 0 - + /// Indicates if the collection is currently retrying ApiCalls - fileprivate var _isRetryingCalls: Bool = false + fileprivate var _isRescheduling: Bool = false /// The task of waiting and executing ApiCalls fileprivate var _reschedulingTask: Task? = nil - + /// Indicates whether the collection content has changed /// Initiates a write when true fileprivate var _hasChanged: Bool = false { @@ -47,7 +46,7 @@ actor ApiCallCollection: SomeCallCollection { } } } - + /// Starts the JSON file decoding synchronously or asynchronously /// Reschedule Api calls if not empty func loadFromFile() throws { @@ -59,15 +58,15 @@ actor ApiCallCollection: SomeCallCollection { fileprivate func _urlForJSONFile() throws -> URL { return try ApiCall.urlForJSONFile() } - + /// Decodes the json file into the items array fileprivate func _decodeJSONFile() throws { let fileURL = try self._urlForJSONFile() - + if FileManager.default.fileExists(atPath: fileURL.path()) { let jsonString: String = try FileUtils.readFile(fileURL: fileURL) let decoded: [ApiCall] = try jsonString.decodeArray() ?? [] -// Logger.log("loaded \(fileURL.lastPathComponent) with \(decoded.count) items") + // Logger.log("loaded \(fileURL.lastPathComponent) with \(decoded.count) items") self.items = decoded } } @@ -76,17 +75,17 @@ actor ApiCallCollection: SomeCallCollection { fileprivate func _write() { let fileName = ApiCall.fileName() DispatchQueue(label: "lestorage.queue.write", qos: .utility).asyncAndWait { -// Logger.log("Start write to \(fileName)...") + // Logger.log("Start write to \(fileName)...") do { let jsonString: String = try self.items.jsonString() try T.writeToStorageDirectory(content: jsonString, fileName: fileName) } catch { Logger.error(error) } -// Logger.log("End write") + // Logger.log("End write") } } - + /// Adds or update an API call instance func addOrUpdate(_ instance: ApiCall) { if let index = self.items.firstIndex(where: { $0.id == instance.id }) { @@ -96,13 +95,14 @@ actor ApiCallCollection: SomeCallCollection { } self._hasChanged = true } - + /// Deletes an API call by [id] func deleteById(_ id: String) { self.items.removeAll(where: { $0.id == id }) + Logger.log("\(T.resourceName()) > Delete by id, count after deletion = \(self.items.count)") self._hasChanged = true } - + /// Deletes a call by a data id func deleteByDataId(_ dataId: String) { if let apiCallIndex = self.items.firstIndex(where: { $0.dataId == dataId }) { @@ -110,12 +110,12 @@ actor ApiCallCollection: SomeCallCollection { self._hasChanged = true } } - + /// Returns the Api call associated with the provided id func findById(_ id: String) -> ApiCall? { return self.items.first(where: { $0.id == id }) } - + /// Returns the Api call associated with the provided id func findCallById(_ id: String) async -> (any SomeCall)? { return self.findById(id) @@ -125,7 +125,7 @@ actor ApiCallCollection: SomeCallCollection { func reset() { self._reschedulingTask?.cancel() self.items.removeAll() - + do { let url: URL = try self._urlForJSONFile() if FileManager.default.fileExists(atPath: url.path()) { @@ -136,48 +136,60 @@ actor ApiCallCollection: SomeCallCollection { } } - /// Reschedule the execution of API calls - fileprivate func _rescheduleApiCalls() { + fileprivate func _wait() async { + let delay = pow(2, self._attemptLoops) + let seconds = NSDecimalNumber(decimal: delay).intValue + Logger.log("\(T.resourceName()): wait for \(seconds) sec") + do { + try await Task.sleep(until: .now + .seconds(seconds)) + } catch { + Logger.error(error) + } + } + + /// Reschedule API calls if necessary + func rescheduleApiCallsIfNecessary() { + Task { + await self._rescheduleApiCalls() + } + } + + /// Reschedule the execution of API calls + fileprivate func _rescheduleApiCalls() async { + + guard !self._isRescheduling else { return } + self._isRescheduling = true + guard self.items.isNotEmpty else { return } - - self._isRetryingCalls = true + self._attemptLoops += 1 - - self._reschedulingTask = Task { - - let delay = pow(2, self._attemptLoops) - let seconds = NSDecimalNumber(decimal: delay).intValue - Logger.log("\(T.resourceName()): wait for \(seconds) sec") - try await Task.sleep(until: .now + .seconds(seconds)) - - let apiCallsCopy = self.items - for apiCall in apiCallsCopy { - apiCall.attemptsCount += 1 - apiCall.lastAttemptDate = Date() - - do { - let _ = try await self._executeApiCall(apiCall) - } catch { - Logger.error(error) - } - } - self._hasChanged = true + await self._wait() + + let apiCallsCopy = self.items + for (index, apiCall) in apiCallsCopy.enumerated() { + apiCall.attemptsCount += 1 + apiCall.lastAttemptDate = Date() - if self.items.isEmpty { - self._isRetryingCalls = false - } else { - self._rescheduleApiCalls() + do { + let _ = try await self._executeApiCall(apiCall) + } catch { + Logger.error(error) } - } + + self._isRescheduling = false + if self.items.isNotEmpty { + await self._rescheduleApiCalls() + } + } - + // MARK: - Synchronization - + /// Returns an APICall instance for the Storable [instance] and an HTTP [method] /// The method updates existing calls or creates a new one fileprivate func _callForInstance(_ instance: T, method: HTTPMethod) throws -> ApiCall? { @@ -185,13 +197,13 @@ actor ApiCallCollection: SomeCallCollection { if let existingCall = self.items.first(where: { $0.dataId == instance.stringId }) { switch method { case .delete: - self.deleteById(existingCall.id) // delete the existing call as we don't need it + self.deleteById(existingCall.id) // delete the existing call as we don't need it if existingCall.method == HTTPMethod.post { - return nil // if the post has not been done, we can just stop here + return nil // if the post has not been done, we can just stop here } else { - return try self._createCall(instance, method: method) // otherwise it's a put and we want to send the delete + return try self._createCall(instance, method: method) // otherwise it's a put and we want to send the delete } - default: // here we should only trying to PUT, so we update the existing POST/PUT with the instance new values + default: // here we should only trying to PUT, so we update the existing POST/PUT with the instance new values existingCall.body = try instance.jsonString() return existingCall } @@ -199,7 +211,7 @@ actor ApiCallCollection: SomeCallCollection { return try self._createCall(instance, method: method) } } - + /// Creates an API call for the Storable [instance] and an HTTP [method] fileprivate func _createCall(_ instance: T, method: HTTPMethod) throws -> ApiCall { let jsonString = try instance.jsonString() @@ -212,14 +224,7 @@ actor ApiCallCollection: SomeCallCollection { apiCall.attemptsCount += 1 self.addOrUpdate(apiCall) } - - /// Reschedule API calls if necessary - func rescheduleApiCallsIfNecessary() { - if !self._isRetryingCalls { - self._rescheduleApiCalls() - } - } - + /// Sends an insert api call for the provided [instance] func sendInsertion(_ instance: T) async throws -> T? { do { @@ -229,9 +234,9 @@ actor ApiCallCollection: SomeCallCollection { Logger.error(error) } return nil - + } - + /// Sends an update api call for the provided [instance] func sendUpdate(_ instance: T) async throws -> T? { do { @@ -242,7 +247,7 @@ actor ApiCallCollection: SomeCallCollection { } return nil } - + /// Sends an delete api call for the provided [instance] func sendDeletion(_ instance: T) async throws -> T? { do { @@ -253,7 +258,7 @@ actor ApiCallCollection: SomeCallCollection { } return nil } - + /// Initiates the process of sending the data with the server fileprivate func _synchronize(_ instance: T, method: HTTPMethod) async throws -> T? { if let apiCall = try self._callForInstance(instance, method: method) { @@ -263,13 +268,13 @@ actor ApiCallCollection: SomeCallCollection { return nil } } - + /// Executes an API call /// For POST requests, potentially copies additional data coming from the server during the insert fileprivate func _executeApiCall(_ apiCall: ApiCall) async throws -> T { return try await StoreCenter.main.execute(apiCall: apiCall) } - + /// Returns the content of the API call file as a String func contentOfFile() -> String? { guard let fileURL = try? self._urlForJSONFile() else { return nil } @@ -278,10 +283,10 @@ actor ApiCallCollection: SomeCallCollection { } return nil } - + /// Returns if the API call collection is not empty func hasPendingCalls() -> Bool { return self.items.isNotEmpty } - + } diff --git a/LeStorage/StoreCenter.swift b/LeStorage/StoreCenter.swift index a6210ee..aa91214 100644 --- a/LeStorage/StoreCenter.swift +++ b/LeStorage/StoreCenter.swift @@ -154,13 +154,15 @@ public class StoreCenter { /// Instantiates and loads an ApiCallCollection with the provided type public func loadApiCallCollection(type: T.Type) { - let apiCallCollection = ApiCallCollection() - self._apiCallCollections[T.resourceName()] = apiCallCollection - Task { - do { - try await apiCallCollection.loadFromFile() - } catch { - Logger.error(error) + if self._apiCallCollections[T.resourceName()] == nil { + let apiCallCollection = ApiCallCollection() + self._apiCallCollections[T.resourceName()] = apiCallCollection + Task { + do { + try await apiCallCollection.loadFromFile() + } catch { + Logger.error(error) + } } } }