diff --git a/Braintree.xcodeproj/project.pbxproj b/Braintree.xcodeproj/project.pbxproj index da061a903b..b335d61bf7 100644 --- a/Braintree.xcodeproj/project.pbxproj +++ b/Braintree.xcodeproj/project.pbxproj @@ -117,7 +117,6 @@ 80581A8C25531D0A00006F53 /* BTConfiguration+ThreeDSecure_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80581A8B25531D0A00006F53 /* BTConfiguration+ThreeDSecure_Tests.swift */; }; 80581B1D2553319C00006F53 /* BTGraphQLHTTP_SSLPinning_IntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80581B1C2553319C00006F53 /* BTGraphQLHTTP_SSLPinning_IntegrationTests.swift */; }; 806C85632B90EBED00A2754C /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = 806C85622B90EBED00A2754C /* PrivacyInfo.xcprivacy */; }; - 807296042C41B63F0093F2AB /* ConfigurationCallbackStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 807296032C41B63F0093F2AB /* ConfigurationCallbackStorage.swift */; }; 8075CBEE2B1B735200CA6265 /* BTAPIRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8075CBED2B1B735200CA6265 /* BTAPIRequest.swift */; }; 80842DA72B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = 80842DA62B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy */; }; 8087C10F2BFBACCA0020FC2E /* TokenizationKey_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8087C10E2BFBACCA0020FC2E /* TokenizationKey_Tests.swift */; }; @@ -857,7 +856,6 @@ 8064F3942B1E4FEB0059C4CB /* BTShopperInsightsClient_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTShopperInsightsClient_Tests.swift; sourceTree = ""; }; 8064F3962B1E63800059C4CB /* BTShopperInsightsRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTShopperInsightsRequest.swift; sourceTree = ""; }; 806C85622B90EBED00A2754C /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = ""; }; - 807296032C41B63F0093F2AB /* ConfigurationCallbackStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationCallbackStorage.swift; sourceTree = ""; }; 8075CBED2B1B735200CA6265 /* BTAPIRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTAPIRequest.swift; sourceTree = ""; }; 80842DA62B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = ""; }; 8087C10E2BFBACCA0020FC2E /* TokenizationKey_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TokenizationKey_Tests.swift; sourceTree = ""; }; @@ -1509,7 +1507,6 @@ BE5C8C0228A2C183004F9130 /* BTConfiguration+Core.swift */, 804DC45C2B2D08FF00F17A15 /* BTConfigurationRequest.swift */, BE698EA128AA8EEA001D9B10 /* ConfigurationCache.swift */, - 807296032C41B63F0093F2AB /* ConfigurationCallbackStorage.swift */, 45EFC3962C2DBF32005E7F5B /* ConfigurationLoader.swift */, ); path = Configuration; @@ -3280,7 +3277,6 @@ 800E78C429E0DD5300D1B0FC /* FPTIBatchData.swift in Sources */, BED00CB028A579D700D74AEC /* BTClientToken.swift in Sources */, 5708E0A628809AD9007946B9 /* BTJSON.swift in Sources */, - 807296042C41B63F0093F2AB /* ConfigurationCallbackStorage.swift in Sources */, 574891ED286F7ECA0020DA36 /* BTClientMetadata.swift in Sources */, BE24C66E28E49A730067B11A /* BTAPIClientError.swift in Sources */, BC17F9BC28D24C9E004B18CC /* BTGraphQLErrorTree.swift in Sources */, diff --git a/CHANGELOG.md b/CHANGELOG.md index 55b8e30601..2d8fb02c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## unreleased * BraintreeCore * Fix bug where some analytics wouldn't send if `BTAPIClient` instantiated on button click + * Fix low-memory crash in ConfigurationCallbackStorage (fixes #1382) ## 6.23.2 (2024-07-30) * BraintreePayPal diff --git a/Sources/BraintreeCore/BTAPIClient.swift b/Sources/BraintreeCore/BTAPIClient.swift index 4479a94a05..dd31f020d6 100644 --- a/Sources/BraintreeCore/BTAPIClient.swift +++ b/Sources/BraintreeCore/BTAPIClient.swift @@ -93,23 +93,20 @@ import Foundation /// cached on subsequent calls for better performance. @_documentation(visibility: private) public func fetchOrReturnRemoteConfiguration(_ completion: @escaping (BTConfiguration?, Error?) -> Void) { - configurationLoader.getConfig { [weak self] configuration, error in - guard let self else { - completion(nil, BTAPIClientError.deallocated) - return - } - - if let error { + // TODO: - Consider updating all feature clients to use async version of this method? + + Task { @MainActor in + do { + let configuration = try await configurationLoader.getConfig() + setupHTTPCredentials(configuration) + completion(configuration, nil) + } catch { completion(nil, error) - return } - - setupHTTPCredentials(configuration) - completion(configuration, nil) } } - func fetchConfiguration() async throws -> BTConfiguration { + @MainActor func fetchConfiguration() async throws -> BTConfiguration { try await configurationLoader.getConfig() } diff --git a/Sources/BraintreeCore/BTHTTP.swift b/Sources/BraintreeCore/BTHTTP.swift index ef72f275e9..13074d106d 100644 --- a/Sources/BraintreeCore/BTHTTP.swift +++ b/Sources/BraintreeCore/BTHTTP.swift @@ -77,6 +77,22 @@ class BTHTTP: NSObject, URLSessionTaskDelegate { completion(nil, nil, error) } } + + func get( + _ path: String, + configuration: BTConfiguration? = nil, + parameters: Encodable? = nil + ) async throws -> (BTJSON?, HTTPURLResponse?) { + try await withCheckedThrowingContinuation { continuation in + get(path, configuration: configuration, parameters: parameters) { body, response, error in + if let error { + continuation.resume(throwing: error) + } else { + continuation.resume(returning: (body, response)) + } + } + } + } // TODO: - Remove when all POST bodies use Codable, instead of BTJSON/raw dictionaries func post( diff --git a/Sources/BraintreeCore/Configuration/ConfigurationCallbackStorage.swift b/Sources/BraintreeCore/Configuration/ConfigurationCallbackStorage.swift deleted file mode 100644 index 5f4d540d7c..0000000000 --- a/Sources/BraintreeCore/Configuration/ConfigurationCallbackStorage.swift +++ /dev/null @@ -1,26 +0,0 @@ -import Foundation - -/// Used to store, access, and manage an array of to-be-invoked `BTConfiguration` GET result callbacks in a thread-safe manner -class ConfigurationCallbackStorage { - - private let queue = DispatchQueue(label: "com.braintreepayments.ConfigurationCallbackStorage") - private var pendingCompletions: [(BTConfiguration?, Error?) -> Void] = [] - - /// The number of completions that are waiting to be invoked - var count: Int { - queue.sync { pendingCompletions.count } - } - - /// Adds a pending, yet to-be-invoked completion handler - func add(_ completion: @escaping (BTConfiguration?, Error?) -> Void) { - queue.sync { pendingCompletions.append(completion) } - } - - /// Executes and clears all pending completion handlers - func invoke(_ configuration: BTConfiguration?, _ error: Error?) { - queue.sync { - pendingCompletions.forEach { $0(configuration, error) } - pendingCompletions.removeAll() - } - } -} diff --git a/Sources/BraintreeCore/Configuration/ConfigurationLoader.swift b/Sources/BraintreeCore/Configuration/ConfigurationLoader.swift index ecc15220f6..bfa9212d8d 100644 --- a/Sources/BraintreeCore/Configuration/ConfigurationLoader.swift +++ b/Sources/BraintreeCore/Configuration/ConfigurationLoader.swift @@ -1,5 +1,9 @@ import Foundation +@globalActor actor ConfigurationActor { + static let shared = ConfigurationActor() +} + class ConfigurationLoader { // MARK: - Private Properties @@ -7,9 +11,10 @@ class ConfigurationLoader { private let configPath = "v1/configuration" private let configurationCache = ConfigurationCache.shared private let http: BTHTTP - private let pendingCompletions = ConfigurationCallbackStorage() - - // MARK: - Intitializer + + private var existingTask: Task? + + // MARK: - Initializer init(http: BTHTTP) { self.http = http @@ -35,56 +40,38 @@ class ConfigurationLoader { /// - `BTConfiguration?`: The configuration object if it is successfully fetched or retrieved from the cache. /// - `Error?`: An error object if fetching the configuration fails or if the instance is deallocated. @_documentation(visibility: private) - func getConfig(completion: @escaping (BTConfiguration?, Error?) -> Void) { + @ConfigurationActor + func getConfig() async throws -> BTConfiguration { + if let existingTask { + return try await existingTask.value + } + if let cachedConfig = try? configurationCache.getFromCache(authorization: http.authorization.bearer) { - completion(cachedConfig, nil) - return + return cachedConfig } - - pendingCompletions.add(completion) - - // If this is the 1st `v1/config` GET attempt, proceed with firing the network request. - // Otherwise, there is already a pending network request. - if pendingCompletions.count == 1 { - http.get(configPath, parameters: BTConfigurationRequest()) { [weak self] body, response, error in - guard let self else { - self?.notifyCompletions(nil, BTAPIClientError.deallocated) - return - } - - if let error { - notifyCompletions(nil, error) - return - } else if response?.statusCode != 200 || body == nil { - notifyCompletions(nil, BTAPIClientError.configurationUnavailable) - return + + let task = Task { [weak self] in + guard let self else { + throw BTAPIClientError.deallocated + } + + defer { self.existingTask = nil } + do { + let (body, response) = try await http.get(configPath, parameters: BTConfigurationRequest()) + + if response?.statusCode != 200 || body == nil { + throw BTAPIClientError.configurationUnavailable } else { let configuration = BTConfiguration(json: body) - try? configurationCache.putInCache(authorization: http.authorization.bearer, configuration: configuration) - - notifyCompletions(configuration, nil) - return - } - } - } - } - - func getConfig() async throws -> BTConfiguration { - try await withCheckedThrowingContinuation { continuation in - getConfig { configuration, error in - if let error { - continuation.resume(throwing: error) - } else if let configuration { - continuation.resume(returning: configuration) + return configuration } + } catch { + throw error } } - } - - // MARK: - Private Methods - - func notifyCompletions(_ configuration: BTConfiguration?, _ error: Error?) { - pendingCompletions.invoke(configuration, error) + + existingTask = task + return try await task.value } } diff --git a/UnitTests/BraintreeCoreTests/Configuration/ConfigurationLoader_Tests.swift b/UnitTests/BraintreeCoreTests/Configuration/ConfigurationLoader_Tests.swift index a0198fc5bf..9bf0238d64 100644 --- a/UnitTests/BraintreeCoreTests/Configuration/ConfigurationLoader_Tests.swift +++ b/UnitTests/BraintreeCoreTests/Configuration/ConfigurationLoader_Tests.swift @@ -20,89 +20,80 @@ class ConfigurationLoader_Tests: XCTestCase { super.tearDown() } - func testGetConfig_whenCached_returnsConfigFromCache() { + func testGetConfig_whenCached_returnsConfigFromCache() async { let sampleJSON = ["test": "value", "environment": "fake-env1"] try? ConfigurationCache.shared.putInCache(authorization: "development_tokenization_key", configuration: BTConfiguration(json: BTJSON(value: sampleJSON))) - let expectation = expectation(description: "Callback invoked") - sut.getConfig { configuration, error in - XCTAssertEqual(configuration?.environment, "fake-env1") - XCTAssertEqual(configuration?.json?["test"].asString(), "value") + do { + let configuration = try await sut.getConfig() + XCTAssertEqual(configuration.environment, "fake-env1") + XCTAssertEqual(configuration.json?["test"].asString(), "value") XCTAssertNil(self.mockHTTP.lastRequestEndpoint) - expectation.fulfill() + } catch { + XCTFail("Should not fail") } - waitForExpectations(timeout: 1) } - func testGetConfig_performsGETWithCorrectPayload() { + func testGetConfig_performsGETWithCorrectPayload() async { mockHTTP.stubRequest(withMethod: "GET", toEndpoint: "/v1/configuration", respondWith: [] as [Any?], statusCode: 200) - - let expectation = expectation(description: "Callback invoked") - sut.getConfig { _, _ in - XCTAssertEqual(self.mockHTTP.lastRequestEndpoint, "v1/configuration") - XCTAssertEqual(self.mockHTTP.lastRequestParameters?["configVersion"] as? String, "3") - expectation.fulfill() + + do { + let _ = try await sut.getConfig() + } catch { + // no-op } - waitForExpectations(timeout: 1) + XCTAssertEqual(self.mockHTTP.lastRequestEndpoint, "v1/configuration") + XCTAssertEqual(self.mockHTTP.lastRequestParameters?["configVersion"] as? String, "3") } - func testGetConfig_canGetRemoteConfiguration() { + func testGetConfig_canGetRemoteConfiguration() async { mockHTTP.cannedConfiguration = BTJSON(value: ["test": true]) mockHTTP.cannedStatusCode = 200 - - let expectation = expectation(description: "Fetch configuration") - sut.getConfig { configuration, error in + + do { + let configuration = try await sut.getConfig() XCTAssertNotNil(configuration) - XCTAssertNil(error) XCTAssertGreaterThanOrEqual(self.mockHTTP.GETRequestCount, 1) - - guard let json = configuration?.json else { return } + guard let json = configuration.json else { return } XCTAssertTrue(json["test"].isTrue) - expectation.fulfill() + } catch { + XCTFail("Should not fail") } - - waitForExpectations(timeout: 1) } - func testGetConfig_whenServerRespondsWithNon200StatusCode_returnsAPIClientError() { + func testGetConfig_whenServerRespondsWithNon200StatusCode_returnsAPIClientError() async { mockHTTP.stubRequest( withMethod: "GET", toEndpoint: "/client_api/v1/configuration", respondWith: ["error_message": "Something bad happened"], statusCode: 503 ) - - let expectation = expectation(description: "Callback invoked") - sut.getConfig { configuration, error in + + do { + let _ = try await sut.getConfig() + } catch { guard let error = error as NSError? else { return } - XCTAssertNil(configuration) XCTAssertEqual(error.domain, BTAPIClientError.errorDomain) XCTAssertEqual(error.code, BTAPIClientError.configurationUnavailable.rawValue) XCTAssertEqual(error.localizedDescription, "The operation couldn’t be completed. Unable to fetch remote configuration from Braintree API at this time.") - expectation.fulfill() } - - waitForExpectations(timeout: 1) } - func testGetConfig_whenNetworkHasError_returnsNetworkErrorInCallback() { + func testGetConfig_whenNetworkHasError_returnsNetworkErrorInCallback() async { ConfigurationCache.shared.cacheInstance.removeAllObjects() let mockError: NSError = NSError(domain: NSURLErrorDomain, code: NSURLErrorCannotConnectToHost) mockHTTP.stubRequest(withMethod: "GET", toEndpoint: "/client_api/v1/configuration", respondWithError: mockError) - let expectation = expectation(description: "Fetch configuration") - sut.getConfig { configuration, error in + do { + let _ = try await sut.getConfig() + } catch { // BTAPIClient fetches the config when initialized so there can potentially be 2 requests here XCTAssertLessThanOrEqual(self.mockHTTP.GETRequestCount, 2) - XCTAssertNil(configuration) XCTAssertEqual(error as NSError?, mockError) - expectation.fulfill() } - - waitForExpectations(timeout: 1) } - + func testGetConfig_returnsConfiguration() async throws { mockHTTP.cannedConfiguration = BTJSON(value: ["test": true]) mockHTTP.cannedStatusCode = 200 @@ -132,11 +123,16 @@ class ConfigurationLoader_Tests: XCTestCase { } } - func testGetConfig_whenCalledInQuickSequence_onlySendsOneNetworkRequest() { - sut.getConfig() { _, _ in } - sut.getConfig() { _, _ in } - sut.getConfig() { _, _ in } - sut.getConfig() { _, _ in } + func testGetConfig_whenCalledInQuickSequence_onlySendsOneNetworkRequest() async { + do { + async let functionOne = sut.getConfig() + async let two = sut.getConfig() + async let three = sut.getConfig() + async let four = sut.getConfig() + let _ = try await (functionOne, two, three, four) + } catch { + // no op + } XCTAssertEqual(mockHTTP.GETRequestCount, 1) } diff --git a/UnitTests/BraintreeCoreTests/Configuration/MockConfigurationLoader.swift b/UnitTests/BraintreeCoreTests/Configuration/MockConfigurationLoader.swift index 27a9687541..b3ebcbfe92 100644 --- a/UnitTests/BraintreeCoreTests/Configuration/MockConfigurationLoader.swift +++ b/UnitTests/BraintreeCoreTests/Configuration/MockConfigurationLoader.swift @@ -12,21 +12,11 @@ class MockConfigurationLoader: ConfigurationLoader { super.init(http: http) } - override func getConfig(completion: @escaping (BTConfiguration?, Error?) -> Void) { - if let error = mockError { - completion(nil, error) - } else { - completion(mockConfig, nil) - } - } - override func getConfig() async throws -> BTConfiguration { if let error = mockError { throw error - } else if let config = mockConfig { - return config } else { - throw BTAPIClientError.configurationUnavailable + return BTConfiguration(json: nil) } } }