From 84201fff9034147c717720adf9c071626d86c604 Mon Sep 17 00:00:00 2001 From: Jin Xu Date: Tue, 17 Dec 2024 11:56:43 -0800 Subject: [PATCH] revision --- .../TestAmplitude.swift | 21 +++----- Sources/Amplitude/Types.swift | 49 ++++++++++++++++++- .../Amplitude/Utilities/EventPipeline.swift | 20 ++++---- .../PersistentStorageResponseHandler.swift | 30 ++++++++++++ .../Supports/TestUtilities.swift | 43 ++++++++-------- .../Utilities/EventPipelineTests.swift | 7 +-- ...ersistentStorageResponseHandlerTests.swift | 4 +- 7 files changed, 122 insertions(+), 52 deletions(-) diff --git a/Examples/AmplitudeObjCExample/AmplitudeObjCExampleTests/TestAmplitude.swift b/Examples/AmplitudeObjCExample/AmplitudeObjCExampleTests/TestAmplitude.swift index d80fe29..4d5bebe 100644 --- a/Examples/AmplitudeObjCExample/AmplitudeObjCExampleTests/TestAmplitude.swift +++ b/Examples/AmplitudeObjCExample/AmplitudeObjCExampleTests/TestAmplitude.swift @@ -102,39 +102,32 @@ class TestStorage: Storage { ) -> ResponseHandler { class TestResponseHandler: ResponseHandler { - func handle(result: Result) -> Bool { + func handle(result: Result) { // no-op - return false } - func handleSuccessResponse(code: Int) -> Bool { + func handleSuccessResponse(code: Int) { // no-op - return false } - func handleBadRequestResponse(data: [String: Any]) -> Bool { + func handleBadRequestResponse(data: [String: Any]) { // no-op - return false } - func handlePayloadTooLargeResponse(data: [String: Any]) -> Bool { + func handlePayloadTooLargeResponse(data: [String: Any]) { // no-op - return false } - func handleTooManyRequestsResponse(data: [String: Any]) -> Bool { + func handleTooManyRequestsResponse(data: [String: Any]) { // no-op - return false } - func handleTimeoutResponse(data: [String: Any]) -> Bool { + func handleTimeoutResponse(data: [String: Any]) { // no-op - return false } - func handleFailedResponse(data: [String: Any]) -> Bool { + func handleFailedResponse(data: [String: Any]) { // no-op - return false } } return TestResponseHandler() diff --git a/Sources/Amplitude/Types.swift b/Sources/Amplitude/Types.swift index e4ab18e..0c217fa 100644 --- a/Sources/Amplitude/Types.swift +++ b/Sources/Amplitude/Types.swift @@ -149,7 +149,17 @@ extension Plugin { func onOptOutChanged(_ optOut: Bool) {} } -public protocol ResponseHandler { +public protocol ResponseHandler: ResponseHandlerV2 { + func handle(result: Result) + func handleSuccessResponse(code: Int) + func handleBadRequestResponse(data: [String: Any]) + func handlePayloadTooLargeResponse(data: [String: Any]) + func handleTooManyRequestsResponse(data: [String: Any]) + func handleTimeoutResponse(data: [String: Any]) + func handleFailedResponse(data: [String: Any]) +} + +public protocol ResponseHandlerV2 { // return true if some attempts to recover are implemented func handle(result: Result) -> Bool func handleSuccessResponse(code: Int) -> Bool @@ -171,3 +181,40 @@ extension ResponseHandler { return indices } } + +extension ResponseHandler { + public func handle(result: Result) -> Bool { + handle(result: result) + return false + } + + public func handleSuccessResponse(code: Int) -> Bool { + handleSuccessResponse(code: code) + return false + } + + public func handleBadRequestResponse(data: [String: Any]) -> Bool { + handleBadRequestResponse(data: data) + return false + } + + public func handlePayloadTooLargeResponse(data: [String: Any]) -> Bool { + handlePayloadTooLargeResponse(data: data) + return false + } + + public func handleTooManyRequestsResponse(data: [String: Any]) -> Bool { + handleTooManyRequestsResponse(data: data) + return false + } + + public func handleTimeoutResponse(data: [String: Any]) -> Bool { + handleTimeoutResponse(data: data) + return false + } + + public func handleFailedResponse(data: [String: Any]) -> Bool { + handleFailedResponse(data: data) + return false + } +} diff --git a/Sources/Amplitude/Utilities/EventPipeline.swift b/Sources/Amplitude/Utilities/EventPipeline.swift index 0275219..ccd509e 100644 --- a/Sources/Amplitude/Utilities/EventPipeline.swift +++ b/Sources/Amplitude/Utilities/EventPipeline.swift @@ -21,7 +21,6 @@ public class EventPipeline { private var flushCompletions: [() -> Void] = [] private var currentUpload: URLSessionTask? - private(set) var continuousFailure: Int = 0 init(amplitude: Amplitude) { storage = amplitude.storage @@ -69,7 +68,7 @@ public class EventPipeline { } } - private func sendNextEventFile() { + private func sendNextEventFile(failures: Int = 0) { autoreleasepool { guard currentUpload == nil else { logger?.log(message: "Existing upload in progress, skipping...") @@ -103,18 +102,19 @@ public class EventPipeline { eventBlock: nextEventFile, eventsString: eventsString ) - let handled = responseHandler.handle(result: result) + let handled: Bool = responseHandler.handle(result: result) + var failures = failures switch result { case .success: - self.continuousFailure = 0 + failures = 0 case .failure: if !handled { - self.continuousFailure += 1 + failures += 1 } } - if self.continuousFailure > self.maxRetryCount { + if failures > self.maxRetryCount { self.uploadsQueue.async { self.currentUpload = nil } @@ -127,15 +127,15 @@ public class EventPipeline { return } self.currentUpload = nil - self.sendNextEventFile() + self.sendNextEventFile(failures: failures) } - if self.continuousFailure == 0 || handled { + if failures == 0 || handled { self.uploadsQueue.async(execute: nextFileBlock) } else { - let sendingInterval = min(self.maxRetryInterval, pow(2, Double(self.continuousFailure - 1))) + let sendingInterval = min(self.maxRetryInterval, pow(2, Double(failures - 1))) self.uploadsQueue.asyncAfter(deadline: .now() + sendingInterval, execute: nextFileBlock) - self.logger?.debug(message: "Request failed \(self.continuousFailure) times, send next event file in \(sendingInterval) seconds") + self.logger?.debug(message: "Request failed \(failures) times, send next event file in \(sendingInterval) seconds") } } } diff --git a/Sources/Amplitude/Utilities/PersistentStorageResponseHandler.swift b/Sources/Amplitude/Utilities/PersistentStorageResponseHandler.swift index 8310c9e..dcdcef3 100644 --- a/Sources/Amplitude/Utilities/PersistentStorageResponseHandler.swift +++ b/Sources/Amplitude/Utilities/PersistentStorageResponseHandler.swift @@ -190,3 +190,33 @@ extension PersistentStorageResponseHandler { } } } + +extension PersistentStorageResponseHandler { + func handle(result: Result) { + let _: Bool = handle(result: result) + } + + func handleSuccessResponse(code: Int) { + let _: Bool = handleSuccessResponse(code: code) + } + + func handleBadRequestResponse(data: [String: Any]) { + let _: Bool = handleBadRequestResponse(data: data) + } + + func handlePayloadTooLargeResponse(data: [String: Any]) { + let _: Bool = handlePayloadTooLargeResponse(data: data) + } + + func handleTooManyRequestsResponse(data: [String: Any]) { + let _: Bool = handleTooManyRequestsResponse(data: data) + } + + func handleTimeoutResponse(data: [String: Any]) { + let _: Bool = handleTimeoutResponse(data: data) + } + + func handleFailedResponse(data: [String: Any]) { + let _: Bool = handleFailedResponse(data: data) + } +} diff --git a/Tests/AmplitudeTests/Supports/TestUtilities.swift b/Tests/AmplitudeTests/Supports/TestUtilities.swift index 3499316..1b42a7c 100644 --- a/Tests/AmplitudeTests/Supports/TestUtilities.swift +++ b/Tests/AmplitudeTests/Supports/TestUtilities.swift @@ -186,38 +186,41 @@ class FakeResponseHandler: ResponseHandler { self.eventsString = eventsString } - func handle(result: Result) -> Bool { - switch result { - case .success(let code): - return handleSuccessResponse(code: code) - default: - return false - } + func handle(result: Result) { + let _: Bool = handle(result: result) } - func handleSuccessResponse(code: Int) -> Bool { - storage.remove(eventBlock: eventBlock) - return true + func handleSuccessResponse(code: Int) { + let _: Bool = handleSuccessResponse(code: code) } - func handleBadRequestResponse(data: [String: Any]) -> Bool { - return true + func handleBadRequestResponse(data: [String: Any]) { } - func handlePayloadTooLargeResponse(data: [String: Any]) -> Bool { - return true + func handlePayloadTooLargeResponse(data: [String: Any]) { + } + + func handleTooManyRequestsResponse(data: [String: Any]) { } - func handleTooManyRequestsResponse(data: [String: Any]) -> Bool { - return false + func handleTimeoutResponse(data: [String: Any]) { } - func handleTimeoutResponse(data: [String: Any]) -> Bool { - return false + func handleFailedResponse(data: [String: Any]) { } - func handleFailedResponse(data: [String: Any]) -> Bool { - return false + func handle(result: Result) -> Bool { + switch result { + case .success(let code): + return handleSuccessResponse(code: code) + default: + return false + } + } + + func handleSuccessResponse(code: Int) -> Bool { + storage.remove(eventBlock: eventBlock) + return true } } diff --git a/Tests/AmplitudeTests/Utilities/EventPipelineTests.swift b/Tests/AmplitudeTests/Utilities/EventPipelineTests.swift index dce8d10..99bd19c 100644 --- a/Tests/AmplitudeTests/Utilities/EventPipelineTests.swift +++ b/Tests/AmplitudeTests/Utilities/EventPipelineTests.swift @@ -206,13 +206,11 @@ final class EventPipelineTests: XCTestCase { wait(for: [uploadExpectations[0], uploadExpectations[1]], timeout: 2) XCTAssertEqual(httpClient.uploadCount, 2) - XCTAssertEqual(pipeline.continuousFailure, 2) XCTAssertEqual(pipeline.configuration.offline, false) wait(for: [uploadExpectations[2]], timeout: 3) XCTAssertEqual(httpClient.uploadCount, 3) - XCTAssertEqual(pipeline.continuousFailure, 3) XCTAssertEqual(pipeline.configuration.offline, true) pipeline.configuration.offline = false @@ -223,7 +221,6 @@ final class EventPipelineTests: XCTestCase { wait(for: [uploadExpectations[3], flushExpectation], timeout: 1) XCTAssertEqual(httpClient.uploadCount, 4) - XCTAssertEqual(pipeline.continuousFailure, 0) } func testContinuesHandledFailure() { @@ -257,10 +254,10 @@ final class EventPipelineTests: XCTestCase { } wait(for: [uploadExpectations[0], uploadExpectations[1]], timeout: 1) XCTAssertEqual(httpClient.uploadCount, 2) - XCTAssertEqual(pipeline.continuousFailure, 0) + XCTAssertEqual(pipeline.configuration.offline, false) wait(for: [uploadExpectations[2], flushExpectation], timeout: 1) XCTAssertEqual(httpClient.uploadCount, 3) - XCTAssertEqual(pipeline.continuousFailure, 0) + XCTAssertEqual(pipeline.configuration.offline, false) } } diff --git a/Tests/AmplitudeTests/Utilities/PersistentStorageResponseHandlerTests.swift b/Tests/AmplitudeTests/Utilities/PersistentStorageResponseHandlerTests.swift index fba3e74..c54e11f 100644 --- a/Tests/AmplitudeTests/Utilities/PersistentStorageResponseHandlerTests.swift +++ b/Tests/AmplitudeTests/Utilities/PersistentStorageResponseHandlerTests.swift @@ -119,7 +119,7 @@ final class PersistentStorageResponseHandlerTests: XCTestCase { eventsString: eventsString ) - _ = handler.handleSuccessResponse(code: 200) + let _: Bool = handler.handleSuccessResponse(code: 200) XCTAssertEqual( fakePersistentStorage.haveBeenCalledWith[0], "remove(eventBlock: \(eventBlock.absoluteURL))" @@ -150,7 +150,7 @@ final class PersistentStorageResponseHandlerTests: XCTestCase { eventsString: eventsString ) - _ = handler.handleBadRequestResponse(data: ["error": "Invalid API key: \(configuration.apiKey)"]) + let _: Bool = handler.handleBadRequestResponse(data: ["error": "Invalid API key: \(configuration.apiKey)"]) XCTAssertEqual( fakePersistentStorage.haveBeenCalledWith[0], "remove(eventBlock: \(eventBlock.absoluteURL))"