From 08e96c8d0882db7d8d631db924910061e0e425cb Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 15:02:49 -0300 Subject: [PATCH 01/16] Fix wait logic in PushAdminTests.setUp The current logic assumes that the deviceRegistrations.save calls will complete in the order that they are started (and hence, that when the final device in the list has been registered, all of the devices have been registered). This is not correct, and means that sometimes we end up trying to create a channel subscription for a device that has not been registered yet, causing the channelSubscriptions.save call to fail. --- Test/Tests/PushAdminTests.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index 95ad83ea5..33260824e 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -111,10 +111,12 @@ class PushAdminTests: XCTestCase { let group = DispatchGroup() group.enter() + var numberOfRemainingDevicesToRegister = allDeviceDetails.count for device in allDeviceDetails { rest.push.admin.deviceRegistrations.save(device) { error in assert(error == nil, error?.message ?? "no message") - if allDeviceDetails.last == device { + numberOfRemainingDevicesToRegister -= 1 + if numberOfRemainingDevicesToRegister == 0 { group.leave() } } @@ -122,10 +124,12 @@ class PushAdminTests: XCTestCase { group.wait() group.enter() + var numberOfRemainingSubscriptionsToSave = allSubscriptions.count for subscription in allSubscriptions { rest.push.admin.channelSubscriptions.save(subscription) { error in assert(error == nil, error?.message ?? "no message") - if allSubscriptions.last == subscription { + numberOfRemainingSubscriptionsToSave -= 1 + if numberOfRemainingSubscriptionsToSave == 0 { group.leave() } } From f176792a4695e7a9c26935b094280daed87fd345 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:33:17 -0300 Subject: [PATCH 02/16] Remove AblyTests.jsonRestOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It doesn’t provide any functionality. --- Test/Test Utilities/TestUtilities.swift | 9 +-------- Test/Tests/AuthTests.swift | 20 ++++++++++---------- Test/Tests/RestClientChannelTests.swift | 2 +- Test/Tests/RestClientStatsTests.swift | 2 +- Test/Tests/RestClientTests.swift | 4 ++-- 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index fb6dae3d1..1976aac3e 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -92,13 +92,6 @@ class AblyTests { checkError(errorInfo, withAlternative: "") } - class var jsonRestOptions: ARTClientOptions { - get { - let options = AblyTests.clientOptions() - return options - } - } - static var testApplication: JSON? struct QueueIdentity { @@ -166,7 +159,7 @@ class AblyTests { } class func commonAppSetup(_ debug: Bool = false) -> ARTClientOptions { - return AblyTests.setupOptions(AblyTests.jsonRestOptions, debug: debug) + return AblyTests.setupOptions(AblyTests.clientOptions(), debug: debug) } class func clientOptions(_ debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { diff --git a/Test/Tests/AuthTests.swift b/Test/Tests/AuthTests.swift index 7080f705c..ed7a45566 100644 --- a/Test/Tests/AuthTests.swift +++ b/Test/Tests/AuthTests.swift @@ -75,7 +75,7 @@ class AuthTests: XCTestCase { // RSA1 func test__003__Basic__should_work_over_HTTPS_only() { - let clientOptions = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) clientOptions.tls = false expect { ARTRest(options: clientOptions) }.to(raiseException()) @@ -83,7 +83,7 @@ class AuthTests: XCTestCase { // RSA11 func test__004__Basic__should_send_the_API_key_in_the_Authorization_header() throws { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) let client = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) client.internal.httpExecutor = testHTTPExecutor @@ -882,7 +882,7 @@ class AuthTests: XCTestCase { func test__042__Token__token_auth_and_clientId__should_check_clientId_consistency__on_realtime() throws { let expectedClientId = "client_string" - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) options.clientId = expectedClientId options.autoConnect = false options.testOptions.transportFactory = TestProxyTransportFactory() @@ -913,7 +913,7 @@ class AuthTests: XCTestCase { } func test__043__Token__token_auth_and_clientId__should_check_clientId_consistency__with_wildcard() { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) options.clientId = "*" expect { ARTRest(options: options) }.to(raiseException()) expect { ARTRealtime(options: options) }.to(raiseException()) @@ -1021,7 +1021,7 @@ class AuthTests: XCTestCase { let tokenParams = ARTTokenParams() XCTAssertNil(tokenParams.capability) - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) let rest = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) rest.internal.httpExecutor = testHTTPExecutor @@ -1053,7 +1053,7 @@ class AuthTests: XCTestCase { let tokenParams = ARTTokenParams() tokenParams.capability = "{\"*\":[\"*\"]}" - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) let rest = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) rest.internal.httpExecutor = testHTTPExecutor @@ -1107,7 +1107,7 @@ class AuthTests: XCTestCase { // RSA7a2 func test__047__Token__clientId_and_authenticated_clients__should_obtain_a_token_if_clientId_is_assigned() { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) options.clientId = "client_string" let client = ARTRest(options: options) @@ -1130,7 +1130,7 @@ class AuthTests: XCTestCase { // RSA7a3 func test__048__Token__clientId_and_authenticated_clients__should_convenience_clientId_return_a_string() { - let clientOptions = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) clientOptions.clientId = "String" XCTAssertEqual(ARTRest(options: clientOptions).internal.options.clientId, "String") @@ -1225,7 +1225,7 @@ class AuthTests: XCTestCase { // RSA7b1 func test__053__Token__clientId_and_authenticated_clients__auth_clientId_not_null__when_clientId_attribute_is_assigned_on_client_options() { - let clientOptions = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) clientOptions.clientId = "Exist" XCTAssertEqual(ARTRest(options: clientOptions).auth.clientId, "Exist") @@ -1301,7 +1301,7 @@ class AuthTests: XCTestCase { // RSA7c func test__050__Token__clientId_and_authenticated_clients__should_clientId_be_null_or_string() { - let clientOptions = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) clientOptions.clientId = "*" expect { ARTRest(options: clientOptions) }.to(raiseException()) diff --git a/Test/Tests/RestClientChannelTests.swift b/Test/Tests/RestClientChannelTests.swift index f706ee6b8..afd62acb2 100644 --- a/Test/Tests/RestClientChannelTests.swift +++ b/Test/Tests/RestClientChannelTests.swift @@ -119,7 +119,7 @@ class RestClientChannelTests: XCTestCase { override func setUp() { super.setUp() - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) client = ARTRest(options: options) testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) } diff --git a/Test/Tests/RestClientStatsTests.swift b/Test/Tests/RestClientStatsTests.swift index 3f58637f1..3b6ac410e 100644 --- a/Test/Tests/RestClientStatsTests.swift +++ b/Test/Tests/RestClientStatsTests.swift @@ -5,7 +5,7 @@ import XCTest import SwiftyJSON private func postTestStats(_ stats: JSON) -> ARTClientOptions { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions, forceNewApp: true) + let options = AblyTests.setupOptions(AblyTests.clientOptions(), forceNewApp: true) let keyBase64 = encodeBase64(options.key ?? "") diff --git a/Test/Tests/RestClientTests.swift b/Test/Tests/RestClientTests.swift index 75d30a532..cb6e57702 100644 --- a/Test/Tests/RestClientTests.swift +++ b/Test/Tests/RestClientTests.swift @@ -437,7 +437,7 @@ class RestClientTests: XCTestCase { // RSC5 func test__002__RestClient__should_provide_access_to_the_AuthOptions_object_passed_in_ClientOptions() { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) let client = ARTRest(options: options) let authOptions = client.auth.internal.options @@ -469,7 +469,7 @@ class RestClientTests: XCTestCase { // RSC16 func test__034__RestClient__time__should_return_server_time() { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.setupOptions(AblyTests.clientOptions()) let client = ARTRest(options: options) var time: NSDate? From 46ce7f97bb3550d10990ba0d6340ca973d0b5415 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:46:17 -0300 Subject: [PATCH 03/16] Replace public use of setupOptions with commonAppSetup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There aren’t any places where we use setupOptions in a way whose behaviour differs from that of commonAppSetup. --- Test/Test Utilities/TestUtilities.swift | 4 ++-- Test/Tests/AuthTests.swift | 20 ++++++++++---------- Test/Tests/RestClientChannelTests.swift | 2 +- Test/Tests/RestClientStatsTests.swift | 2 +- Test/Tests/RestClientTests.swift | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 1976aac3e..2679df97b 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -158,8 +158,8 @@ class AblyTests { return options } - class func commonAppSetup(_ debug: Bool = false) -> ARTClientOptions { - return AblyTests.setupOptions(AblyTests.clientOptions(), debug: debug) + class func commonAppSetup(_ debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { + return AblyTests.setupOptions(AblyTests.clientOptions(), forceNewApp: forceNewApp, debug: debug) } class func clientOptions(_ debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { diff --git a/Test/Tests/AuthTests.swift b/Test/Tests/AuthTests.swift index ed7a45566..283b4a4a7 100644 --- a/Test/Tests/AuthTests.swift +++ b/Test/Tests/AuthTests.swift @@ -75,7 +75,7 @@ class AuthTests: XCTestCase { // RSA1 func test__003__Basic__should_work_over_HTTPS_only() { - let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) + let clientOptions = AblyTests.commonAppSetup() clientOptions.tls = false expect { ARTRest(options: clientOptions) }.to(raiseException()) @@ -83,7 +83,7 @@ class AuthTests: XCTestCase { // RSA11 func test__004__Basic__should_send_the_API_key_in_the_Authorization_header() throws { - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() let client = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) client.internal.httpExecutor = testHTTPExecutor @@ -882,7 +882,7 @@ class AuthTests: XCTestCase { func test__042__Token__token_auth_and_clientId__should_check_clientId_consistency__on_realtime() throws { let expectedClientId = "client_string" - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() options.clientId = expectedClientId options.autoConnect = false options.testOptions.transportFactory = TestProxyTransportFactory() @@ -913,7 +913,7 @@ class AuthTests: XCTestCase { } func test__043__Token__token_auth_and_clientId__should_check_clientId_consistency__with_wildcard() { - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() options.clientId = "*" expect { ARTRest(options: options) }.to(raiseException()) expect { ARTRealtime(options: options) }.to(raiseException()) @@ -1021,7 +1021,7 @@ class AuthTests: XCTestCase { let tokenParams = ARTTokenParams() XCTAssertNil(tokenParams.capability) - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() let rest = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) rest.internal.httpExecutor = testHTTPExecutor @@ -1053,7 +1053,7 @@ class AuthTests: XCTestCase { let tokenParams = ARTTokenParams() tokenParams.capability = "{\"*\":[\"*\"]}" - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() let rest = ARTRest(options: options) let testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) rest.internal.httpExecutor = testHTTPExecutor @@ -1107,7 +1107,7 @@ class AuthTests: XCTestCase { // RSA7a2 func test__047__Token__clientId_and_authenticated_clients__should_obtain_a_token_if_clientId_is_assigned() { - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() options.clientId = "client_string" let client = ARTRest(options: options) @@ -1130,7 +1130,7 @@ class AuthTests: XCTestCase { // RSA7a3 func test__048__Token__clientId_and_authenticated_clients__should_convenience_clientId_return_a_string() { - let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) + let clientOptions = AblyTests.commonAppSetup() clientOptions.clientId = "String" XCTAssertEqual(ARTRest(options: clientOptions).internal.options.clientId, "String") @@ -1225,7 +1225,7 @@ class AuthTests: XCTestCase { // RSA7b1 func test__053__Token__clientId_and_authenticated_clients__auth_clientId_not_null__when_clientId_attribute_is_assigned_on_client_options() { - let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) + let clientOptions = AblyTests.commonAppSetup() clientOptions.clientId = "Exist" XCTAssertEqual(ARTRest(options: clientOptions).auth.clientId, "Exist") @@ -1301,7 +1301,7 @@ class AuthTests: XCTestCase { // RSA7c func test__050__Token__clientId_and_authenticated_clients__should_clientId_be_null_or_string() { - let clientOptions = AblyTests.setupOptions(AblyTests.clientOptions()) + let clientOptions = AblyTests.commonAppSetup() clientOptions.clientId = "*" expect { ARTRest(options: clientOptions) }.to(raiseException()) diff --git a/Test/Tests/RestClientChannelTests.swift b/Test/Tests/RestClientChannelTests.swift index afd62acb2..f3bb4db9a 100644 --- a/Test/Tests/RestClientChannelTests.swift +++ b/Test/Tests/RestClientChannelTests.swift @@ -119,7 +119,7 @@ class RestClientChannelTests: XCTestCase { override func setUp() { super.setUp() - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() client = ARTRest(options: options) testHTTPExecutor = TestProxyHTTPExecutor(logger: .init(clientOptions: options)) } diff --git a/Test/Tests/RestClientStatsTests.swift b/Test/Tests/RestClientStatsTests.swift index 3b6ac410e..a3e638751 100644 --- a/Test/Tests/RestClientStatsTests.swift +++ b/Test/Tests/RestClientStatsTests.swift @@ -5,7 +5,7 @@ import XCTest import SwiftyJSON private func postTestStats(_ stats: JSON) -> ARTClientOptions { - let options = AblyTests.setupOptions(AblyTests.clientOptions(), forceNewApp: true) + let options = AblyTests.commonAppSetup(forceNewApp: true) let keyBase64 = encodeBase64(options.key ?? "") diff --git a/Test/Tests/RestClientTests.swift b/Test/Tests/RestClientTests.swift index cb6e57702..11e6d81c9 100644 --- a/Test/Tests/RestClientTests.swift +++ b/Test/Tests/RestClientTests.swift @@ -437,7 +437,7 @@ class RestClientTests: XCTestCase { // RSC5 func test__002__RestClient__should_provide_access_to_the_AuthOptions_object_passed_in_ClientOptions() { - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() let client = ARTRest(options: options) let authOptions = client.auth.internal.options @@ -469,7 +469,7 @@ class RestClientTests: XCTestCase { // RSC16 func test__034__RestClient__time__should_return_server_time() { - let options = AblyTests.setupOptions(AblyTests.clientOptions()) + let options = AblyTests.commonAppSetup() let client = ARTRest(options: options) var time: NSDate? From 03a97b91fbca777086546a736b2287b6e9b757bc Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:55:02 -0300 Subject: [PATCH 04/16] Remove AblyTests.setupOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wasn’t adding anything useful. --- Test/Test Utilities/TestUtilities.swift | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 2679df97b..51788caeb 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -116,14 +116,18 @@ class AblyTests { return DispatchQueue.getSpecific(key: queueIdentityKey)?.label } - class func setupOptions(_ options: ARTClientOptions, forceNewApp: Bool = false, debug: Bool = false) -> ARTClientOptions { + class func commonAppSetup(_ debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { + let options = AblyTests.clientOptions() options.testOptions.channelNamePrefix = "test-\(UUID().uuidString)" if forceNewApp { testApplication = nil } - guard let app = testApplication else { + let app: JSON + if let testApplication { + app = testApplication + } else { let request = NSMutableURLRequest(url: URL(string: "https://\(options.restHost):\(options.tlsPort)/apps")!) request.httpMethod = "POST" request.httpBody = try? appSetupJson["post_apps"].rawData() @@ -139,13 +143,12 @@ class AblyTests { fatalError(error.localizedDescription) } - testApplication = try! JSON(data: responseData!) + app = try! JSON(data: responseData!) + testApplication = app if debug { - print(testApplication!) + print(app) } - - return setupOptions(options, debug: debug) } let key = app["keys"][0] @@ -157,10 +160,6 @@ class AblyTests { } return options } - - class func commonAppSetup(_ debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { - return AblyTests.setupOptions(AblyTests.clientOptions(), forceNewApp: forceNewApp, debug: debug) - } class func clientOptions(_ debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { let options = ARTClientOptions() From 26213caa6bf0d60d0a440d0724eb0b0a445777db Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:57:14 -0300 Subject: [PATCH 05/16] Add missing parameter label to commonSetup / clientOptions methods For call site readability. --- Test/Test Utilities/TestUtilities.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 51788caeb..d513b503d 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -116,7 +116,7 @@ class AblyTests { return DispatchQueue.getSpecific(key: queueIdentityKey)?.label } - class func commonAppSetup(_ debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { + class func commonAppSetup(debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { let options = AblyTests.clientOptions() options.testOptions.channelNamePrefix = "test-\(UUID().uuidString)" @@ -161,7 +161,7 @@ class AblyTests { return options } - class func clientOptions(_ debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { + class func clientOptions(debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { let options = ARTClientOptions() options.environment = getEnvironment() options.logExceptionReportingUrl = nil From eae398ed50f7f258c430e4b07ea4dc0b0b5bfa91 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:59:18 -0300 Subject: [PATCH 06/16] =?UTF-8?q?Remove=20redundant=20setting=20of=20prope?= =?UTF-8?q?rties=20in=20commonAppSetup(=E2=80=A6)=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s already covered in the clientOptions(…) method. --- Test/Test Utilities/TestUtilities.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index d513b503d..3c3226397 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -153,8 +153,6 @@ class AblyTests { let key = app["keys"][0] options.key = key["keyStr"].stringValue - options.dispatchQueue = DispatchQueue.main - options.internalDispatchQueue = queue if debug { options.logLevel = .verbose } From 05f4430df73b30efb86b843d814a80ce538e4ff3 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 11:59:18 -0300 Subject: [PATCH 07/16] =?UTF-8?q?Remove=20duplicate=20setting=20of=20logLe?= =?UTF-8?q?vel=20in=20commonAppSetup(=E2=80=A6)=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s already covered in the clientOptions(…) method. I’ve chosen to go with .verbose as this feels more useful for someone wanting to debug a misbehaving test. --- Test/Test Utilities/TestUtilities.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 3c3226397..d32de94dd 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -117,7 +117,7 @@ class AblyTests { } class func commonAppSetup(debug: Bool = false, forceNewApp: Bool = false) -> ARTClientOptions { - let options = AblyTests.clientOptions() + let options = AblyTests.clientOptions(debug: debug) options.testOptions.channelNamePrefix = "test-\(UUID().uuidString)" if forceNewApp { @@ -153,9 +153,7 @@ class AblyTests { let key = app["keys"][0] options.key = key["keyStr"].stringValue - if debug { - options.logLevel = .verbose - } + return options } @@ -164,7 +162,7 @@ class AblyTests { options.environment = getEnvironment() options.logExceptionReportingUrl = nil if debug { - options.logLevel = .debug + options.logLevel = .verbose } if let key = key { options.key = key From c0cbd35c7aaf22cd7ed79cafd09851f6862b57c5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 12:05:05 -0300 Subject: [PATCH 08/16] Remove setting of logExceptionReportingUrl in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK doesn’t use this property any more. --- Test/Test Utilities/TestUtilities.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index d32de94dd..7eb8e48bf 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -160,7 +160,6 @@ class AblyTests { class func clientOptions(debug: Bool = false, key: String? = nil, requestToken: Bool = false) -> ARTClientOptions { let options = ARTClientOptions() options.environment = getEnvironment() - options.logExceptionReportingUrl = nil if debug { options.logLevel = .verbose } From bd9a48d7aeeb92f64feff4f16907336d9c6157e7 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 14 Apr 2023 05:10:09 -0300 Subject: [PATCH 09/16] Allow injecting an object for fetching ARTLocalDevice instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is just a refactor, but the intention is that it form part of #1601 (avoiding using global SDK state for controlling test behaviour) — we’ll now be able to inject a mock device fetcher instead of having to use the shared one. --- Ably.xcodeproj/project.pbxproj | 28 +++++++++ Source/ARTLocalDeviceFetcher.m | 57 +++++++++++++++++++ Source/ARTRest.m | 48 ++-------------- Source/ARTTestClientOptions.m | 3 + Source/Ably.modulemap | 2 + .../Ably/ARTLocalDeviceFetcher+Testing.h | 16 ++++++ .../Ably/ARTLocalDeviceFetcher.h | 41 +++++++++++++ Source/PrivateHeaders/Ably/ARTRest+Private.h | 5 -- .../Ably/ARTTestClientOptions.h | 6 ++ Source/include/Ably.modulemap | 2 + .../DefaultLocalDeviceFetcherTests.swift | 46 +++++++++++++++ .../PushActivationStateMachineTests.swift | 8 +-- Test/Tests/PushChannelTests.swift | 2 +- Test/Tests/PushTests.swift | 4 +- 14 files changed, 214 insertions(+), 54 deletions(-) create mode 100644 Source/ARTLocalDeviceFetcher.m create mode 100644 Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h create mode 100644 Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h create mode 100644 Test/Tests/DefaultLocalDeviceFetcherTests.swift diff --git a/Ably.xcodeproj/project.pbxproj b/Ably.xcodeproj/project.pbxproj index 034c8b5e4..0aa530d13 100644 --- a/Ably.xcodeproj/project.pbxproj +++ b/Ably.xcodeproj/project.pbxproj @@ -165,6 +165,10 @@ 2147F03529E5872C0071CB94 /* MockInternalLogCore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */; }; 2147F03629E5872C0071CB94 /* MockInternalLogCore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */; }; 2147F03729E5872C0071CB94 /* MockInternalLogCore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */; }; + 214D3F3129EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */ = {isa = PBXBuildFile; fileRef = 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 214D3F3229EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */ = {isa = PBXBuildFile; fileRef = 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 214D3F3329EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */ = {isa = PBXBuildFile; fileRef = 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 214D3F3529EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 214D3F3429EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift */; }; 215F75F82922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; 215F75F92922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; 215F75FA2922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -177,6 +181,12 @@ 215F76032922C76C009E0E76 /* ARTClientInformation+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F76022922C76C009E0E76 /* ARTClientInformation+Private.h */; settings = {ATTRIBUTES = (Private, ); }; }; 215F76042922C76C009E0E76 /* ARTClientInformation+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F76022922C76C009E0E76 /* ARTClientInformation+Private.h */; settings = {ATTRIBUTES = (Private, ); }; }; 215F76052922C76C009E0E76 /* ARTClientInformation+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F76022922C76C009E0E76 /* ARTClientInformation+Private.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 2169CBAF29E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 2169CBAE29E93D95003FE670 /* ARTLocalDeviceFetcher.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 2169CBB029E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 2169CBAE29E93D95003FE670 /* ARTLocalDeviceFetcher.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 2169CBB129E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 2169CBAE29E93D95003FE670 /* ARTLocalDeviceFetcher.h */; settings = {ATTRIBUTES = (Private, ); }; }; + 2169CBB329E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 2169CBB229E93E8D003FE670 /* ARTLocalDeviceFetcher.m */; }; + 2169CBB429E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 2169CBB229E93E8D003FE670 /* ARTLocalDeviceFetcher.m */; }; + 2169CBB529E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 2169CBB229E93E8D003FE670 /* ARTLocalDeviceFetcher.m */; }; 217D1829254222F500DFF07E /* ARTSRHash.m in Sources */ = {isa = PBXBuildFile; fileRef = 217D180425421FED00DFF07E /* ARTSRHash.m */; }; 217D182A254222F500DFF07E /* ARTSRRandom.m in Sources */ = {isa = PBXBuildFile; fileRef = 217D180025421FED00DFF07E /* ARTSRRandom.m */; }; 217D182B254222F500DFF07E /* ARTSRWebSocket.m in Sources */ = {isa = PBXBuildFile; fileRef = 217D181F25421FED00DFF07E /* ARTSRWebSocket.m */; }; @@ -1153,10 +1163,14 @@ 2147F02C29E583AD0071CB94 /* ARTInternalLogCore.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARTInternalLogCore.h; path = PrivateHeaders/Ably/ARTInternalLogCore.h; sourceTree = ""; }; 2147F03029E583CE0071CB94 /* ARTInternalLogCore.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTInternalLogCore.m; sourceTree = ""; }; 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockInternalLogCore.swift; sourceTree = ""; }; + 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "ARTLocalDeviceFetcher+Testing.h"; path = "PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h"; sourceTree = ""; }; + 214D3F3429EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultLocalDeviceFetcherTests.swift; sourceTree = ""; }; 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARTClientInformation.h; path = include/Ably/ARTClientInformation.h; sourceTree = ""; }; 215F75F72922B1DB009E0E76 /* ARTClientInformation.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTClientInformation.m; sourceTree = ""; }; 215F75FE2922B30F009E0E76 /* ClientInformationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClientInformationTests.swift; sourceTree = ""; }; 215F76022922C76C009E0E76 /* ARTClientInformation+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "ARTClientInformation+Private.h"; path = "PrivateHeaders/Ably/ARTClientInformation+Private.h"; sourceTree = ""; }; + 2169CBAE29E93D95003FE670 /* ARTLocalDeviceFetcher.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARTLocalDeviceFetcher.h; path = PrivateHeaders/Ably/ARTLocalDeviceFetcher.h; sourceTree = ""; }; + 2169CBB229E93E8D003FE670 /* ARTLocalDeviceFetcher.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTLocalDeviceFetcher.m; sourceTree = ""; }; 217D17F125421FED00DFF07E /* ARTSRProxyConnect.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ARTSRProxyConnect.h; sourceTree = ""; }; 217D17F225421FED00DFF07E /* ARTSRProxyConnect.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTSRProxyConnect.m; sourceTree = ""; }; 217D17F425421FED00DFF07E /* ARTSRRunLoopThread.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTSRRunLoopThread.m; sourceTree = ""; }; @@ -1672,6 +1686,7 @@ 56190953238C3D3200A862A6 /* CryptoTest.m */, 2132C22129D233EB000C4355 /* DefaultErrorCheckerTests.swift */, 217FCF4529D626F6006E5F2D /* DefaultJitterCoefficientGeneratorTests.swift */, + 214D3F3429EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift */, D798554723EB96C000946BE2 /* DeltaCodecTests.swift */, D5A22171266F526600C87C42 /* GCDTests.swift */, 2124B79629DB144600AD8361 /* DefaultInternalLogCoreTests.swift */, @@ -2106,6 +2121,9 @@ D7DEAFCF1E65926D00D23F24 /* ARTLocalDevice.h */, EBFFAC181E97919C003E7326 /* ARTLocalDevice+Private.h */, D7DEAFD01E65926D00D23F24 /* ARTLocalDevice.m */, + 2169CBAE29E93D95003FE670 /* ARTLocalDeviceFetcher.h */, + 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */, + 2169CBB229E93E8D003FE670 /* ARTLocalDeviceFetcher.m */, D75F49C2205ACFEC003DE04F /* ARTDeviceStorage.h */, D7DF73881EA645300013CD36 /* ARTLocalDeviceStorage.h */, D7DF73891EA645300013CD36 /* ARTLocalDeviceStorage.m */, @@ -2203,6 +2221,7 @@ D71966EA1E5E006E000974DD /* ARTPushActivationState.h in Headers */, EB89D40F1C62303E007FA5B7 /* ARTConnection+Private.h in Headers */, D746AE281BBB61C9003ECEF8 /* ARTPresence.h in Headers */, + 2169CBAF29E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */, EB89D4091C61C5ED007FA5B7 /* ARTRealtimeChannels.h in Headers */, 96E408431A38939E00087F77 /* ARTProtocolMessage.h in Headers */, D78D780921271FB10016808B /* ARTHTTPPaginatedResponse+Private.h in Headers */, @@ -2314,6 +2333,7 @@ EB1AE0CC1C5C1EB200D62250 /* ARTEventEmitter+Private.h in Headers */, 215F75F82922B1DB009E0E76 /* ARTClientInformation.h in Headers */, D7DF738A1EA645300013CD36 /* ARTLocalDeviceStorage.h in Headers */, + 214D3F3129EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */, 960D07931A45F1D800ED8C8C /* ARTCrypto.h in Headers */, 2132C21A29D230CE000C4355 /* ARTErrorChecker.h in Headers */, D5BB20FC26A7F3FF00AA5F3E /* NSURLRequest+ARTSRWebSocket.h in Headers */, @@ -2370,6 +2390,7 @@ D710D55721949C8C008F54AD /* ARTPushActivationEvent.h in Headers */, 21447D40254A2ECE00B3905A /* ARTSRWebSocket.h in Headers */, EBB721CC2376B454001C3550 /* ARTURLSession.h in Headers */, + 214D3F3229EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */, D710D4CE21949BB2008F54AD /* ARTWebSocketTransport+Private.h in Headers */, D710D56C21949CB9008F54AD /* ARTPushChannelSubscriptions.h in Headers */, D710D56721949CA1008F54AD /* ARTPushActivationStateMachine+Private.h in Headers */, @@ -2396,6 +2417,7 @@ 2132C2BE29D482E8000C4355 /* ARTClientOptions+TestConfiguration.h in Headers */, D710D58F21949D29008F54AD /* ARTStats.h in Headers */, D710D4B521949B47008F54AD /* ARTRestPresence+Private.h in Headers */, + 2169CBB029E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */, D710D4A921949ADF008F54AD /* ARTRestChannels.h in Headers */, D710D4D821949BF9008F54AD /* ARTRealtimePresence.h in Headers */, D710D48F21949AAE008F54AD /* ARTRest.h in Headers */, @@ -2537,6 +2559,7 @@ D710D5AF21949D2A008F54AD /* ARTBaseMessage.h in Headers */, D710D55D21949C8D008F54AD /* ARTPushActivationEvent.h in Headers */, D520C4E62680A882000012B2 /* ARTStringifiable+Private.h in Headers */, + 214D3F3329EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */, 21447D45254A2ED100B3905A /* ARTSRWebSocket.h in Headers */, EBB721CD2376B454001C3550 /* ARTURLSession.h in Headers */, D710D4D021949BB3008F54AD /* ARTWebSocketTransport+Private.h in Headers */, @@ -2563,6 +2586,7 @@ 2132C2BF29D482E8000C4355 /* ARTClientOptions+TestConfiguration.h in Headers */, D710D5B521949D2A008F54AD /* ARTStats.h in Headers */, D710D4BB21949B48008F54AD /* ARTRestPresence+Private.h in Headers */, + 2169CBB129E93D95003FE670 /* ARTLocalDeviceFetcher.h in Headers */, D710D4AF21949AE0008F54AD /* ARTRestChannels.h in Headers */, D710D4E821949BFB008F54AD /* ARTRealtimePresence.h in Headers */, D710D49121949AAF008F54AD /* ARTRest.h in Headers */, @@ -2978,6 +3002,7 @@ 217FCF4629D626F6006E5F2D /* DefaultJitterCoefficientGeneratorTests.swift in Sources */, 217FCF2E29D623DD006E5F2D /* ConstantRetryDelayCalculatorTests.swift in Sources */, D74EFAEB1C4D09B500CFF98E /* RealtimeClientChannelTests.swift in Sources */, + 214D3F3529EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift in Sources */, 2147F03529E5872C0071CB94 /* MockInternalLogCore.swift in Sources */, 851674EF1B7BA5CD00D35169 /* StatsTests.swift in Sources */, EB1AE0CE1C5C3A4900D62250 /* UtilitiesTests.swift in Sources */, @@ -3103,6 +3128,7 @@ D7588AF41BFF91B800BB8279 /* ARTURLSessionServerTrust.m in Sources */, D74CBC04212EB58700D090E4 /* ARTNSHTTPURLResponse+ARTPaginated.m in Sources */, 2124B78F29DB13BD00AD8361 /* ARTInternalLog.m in Sources */, + 2169CBB329E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */, D5BB213626AAA60500AA5F3E /* ARTNSError+ARTUtils.m in Sources */, 217D1839254222F600DFF07E /* ARTSRHTTPConnectMessage.m in Sources */, 2132C2F929D5BE3D000C4355 /* ARTRetrySequence.m in Sources */, @@ -3330,6 +3356,7 @@ D710D5DA21949D78008F54AD /* ARTBaseMessage.m in Sources */, D710D63421949E03008F54AD /* ARTNSMutableURLRequest+ARTPaginated.m in Sources */, 2124B79029DB13BD00AD8361 /* ARTInternalLog.m in Sources */, + 2169CBB429E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */, 217D1850254222F700DFF07E /* ARTSRHTTPConnectMessage.m in Sources */, D5BB213726AAA60500AA5F3E /* ARTNSError+ARTUtils.m in Sources */, 2132C2FA29D5BE3D000C4355 /* ARTRetrySequence.m in Sources */, @@ -3455,6 +3482,7 @@ D520C4DB26809BB5000012B2 /* ARTStringifiable.m in Sources */, D710D64421949E04008F54AD /* ARTNSMutableURLRequest+ARTPaginated.m in Sources */, 2124B79129DB13BD00AD8361 /* ARTInternalLog.m in Sources */, + 2169CBB529E93E8D003FE670 /* ARTLocalDeviceFetcher.m in Sources */, 217D1867254222FA00DFF07E /* ARTSRHTTPConnectMessage.m in Sources */, D710D57A21949CC5008F54AD /* ARTPushDeviceRegistrations.m in Sources */, 2132C2FB29D5BE3D000C4355 /* ARTRetrySequence.m in Sources */, diff --git a/Source/ARTLocalDeviceFetcher.m b/Source/ARTLocalDeviceFetcher.m new file mode 100644 index 000000000..5024111b1 --- /dev/null +++ b/Source/ARTLocalDeviceFetcher.m @@ -0,0 +1,57 @@ +#import "ARTLocalDeviceFetcher.h" +#import "ARTLocalDeviceFetcher+Testing.h" +#import "ARTLocalDevice+Private.h" + +@interface ARTDefaultLocalDeviceFetcher () + +@property (nonatomic, nullable) ARTLocalDevice *device; +@property (nonatomic, readonly) dispatch_semaphore_t semaphore; + +@end + +@implementation ARTDefaultLocalDeviceFetcher + +- (instancetype)init { + if (self = [super init]) { + _semaphore = dispatch_semaphore_create(1); + } + + return self; +} + +// The device is shared in a static variable because it's a reflection +// of what's persisted. Having a device instance per ARTRest instance +// could leave some instances in a stale state, if, through another +// instance, the persisted state is changed. +// +// As a side effect, the first ARTRest instance "wins" at setting the device's +// client ID. ++ (ARTDefaultLocalDeviceFetcher *)sharedInstance { + static ARTDefaultLocalDeviceFetcher *sharedInstance; + static dispatch_once_t onceToken; + + dispatch_once(&onceToken, ^{ + sharedInstance = [[ARTDefaultLocalDeviceFetcher alloc] init]; + }); + + return sharedInstance; +} + +- (ARTLocalDevice *)fetchLocalDeviceWithClientID:(NSString *)clientID storage:(id)storage logger:(ARTInternalLog *)logger { + dispatch_semaphore_wait(self.semaphore, DISPATCH_TIME_FOREVER); + if (!self.device) { + self.device = [ARTLocalDevice load:clientID storage:storage logger:logger]; + } + ARTLocalDevice *const device = self.device; + dispatch_semaphore_signal(self.semaphore); + + return device; +} + +- (void)resetDevice { + dispatch_semaphore_wait(self.semaphore, DISPATCH_TIME_FOREVER); + self.device = nil; + dispatch_semaphore_signal(self.semaphore); +} + +@end diff --git a/Source/ARTRest.m b/Source/ARTRest.m index adef3e600..9e9819bfa 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -43,6 +43,7 @@ #import "ARTLogAdapter.h" #import "ARTClientOptions+TestConfiguration.h" #import "ARTTestClientOptions.h" +#import "ARTLocalDeviceFetcher.h" @implementation ARTRest { ARTQueuedDealloc *_dealloc; @@ -150,6 +151,7 @@ - (ARTLocalDevice *)device_nosync { @interface ARTRestInternal () @property (nonatomic, readonly) ARTInternalLog *logger; +@property (nonatomic, readonly) id localDeviceFetcher; @end @@ -175,6 +177,7 @@ - (instancetype)initWithOptions:(ARTClientOptions *)options realtime:(ARTRealtim _realtime = realtime; _options = [options copy]; _logger = logger; + _localDeviceFetcher = options.testOptions.localDeviceFetcher; _queue = options.internalDispatchQueue; _userQueue = options.dispatchQueue; #if TARGET_OS_IOS @@ -735,48 +738,9 @@ - (ARTLocalDevice *)device { } - (ARTLocalDevice *)device_nosync { - NSString *clientId = self.auth.clientId_nosync; - __block ARTLocalDevice *ret; - dispatch_sync(ARTRestInternal.deviceAccessQueue, ^{ - ret = [self deviceWithClientId_onlyCallOnDeviceAccessQueue:clientId]; - }); - return ret; -} - -+ (dispatch_queue_t)deviceAccessQueue { - static dispatch_queue_t queue; - static dispatch_once_t onceToken; - - dispatch_once(&onceToken, ^{ - queue = dispatch_queue_create("io.ably.deviceAccess", DISPATCH_QUEUE_SERIAL); - }); - - return queue; -} - -static BOOL sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES; - -- (ARTLocalDevice *)deviceWithClientId_onlyCallOnDeviceAccessQueue:(NSString *)clientId { - // The device is shared in a static variable because it's a reflection - // of what's persisted. Having a device instance per ARTRest instance - // could leave some instances in a stale state, if, through another - // instance, the persisted state is changed. - // - // As a side effect, the first instance "wins" at setting the device's - // client ID. - - static id device; - if (sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue) { - device = [ARTLocalDevice load:clientId storage:self.storage logger:self.logger]; - sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = NO; - } - return device; -} - -- (void)resetDeviceSingleton { - dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ - sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES; - }); + return [self.localDeviceFetcher fetchLocalDeviceWithClientID:self.auth.clientId_nosync + storage:self.storage + logger:self.logger]; } #endif diff --git a/Source/ARTTestClientOptions.m b/Source/ARTTestClientOptions.m index b866a1e24..eea664b0e 100644 --- a/Source/ARTTestClientOptions.m +++ b/Source/ARTTestClientOptions.m @@ -2,6 +2,7 @@ #import "ARTDefault.h" #import "ARTFallback+Private.h" #import "ARTRealtimeTransportFactory.h" +#import "ARTLocalDeviceFetcher.h" @implementation ARTTestClientOptions @@ -10,6 +11,7 @@ - (instancetype)init { _realtimeRequestTimeout = [ARTDefault realtimeRequestTimeout]; _shuffleArray = ARTFallback_shuffleArray; _transportFactory = [[ARTDefaultRealtimeTransportFactory alloc] init]; + _localDeviceFetcher = ARTDefaultLocalDeviceFetcher.sharedInstance; } return self; @@ -21,6 +23,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone { copied.realtimeRequestTimeout = self.realtimeRequestTimeout; copied.shuffleArray = self.shuffleArray; copied.transportFactory = self.transportFactory; + copied.localDeviceFetcher = self.localDeviceFetcher; return copied; } diff --git a/Source/Ably.modulemap b/Source/Ably.modulemap index fa0202676..743822fb1 100644 --- a/Source/Ably.modulemap +++ b/Source/Ably.modulemap @@ -107,5 +107,7 @@ framework module Ably { header "ARTInternalLogCore+Testing.h" header "ARTDataEncoder.h" header "ARTRealtimeTransportFactory.h" + header "ARTLocalDeviceFetcher.h" + header "ARTLocalDeviceFetcher+Testing.h" } } diff --git a/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h new file mode 100644 index 000000000..b39403ab2 --- /dev/null +++ b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h @@ -0,0 +1,16 @@ +@import Foundation; + +NS_ASSUME_NONNULL_BEGIN + +@interface ARTDefaultLocalDeviceFetcher () + +/** + Clears the fetcher’s internal reference to the device object, such that the next time it receives a `-fetchLocalDeviceWithClientID:storage:logger:` message, it initializes a new `ARTLocalDevice` instance. + + This method should only be used in test code. + */ +- (void)resetDevice; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h new file mode 100644 index 000000000..eba164fc8 --- /dev/null +++ b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h @@ -0,0 +1,41 @@ +@import Foundation; + +@protocol ARTDeviceStorage; +@class ARTLocalDevice; +@class ARTInternalLog; + +NS_ASSUME_NONNULL_BEGIN + +/** +`ARTLocalDeviceFetcher` is responsible for fetching an instance of `ARTLocalDevice`. + */ +NS_SWIFT_NAME(LocalDeviceFetcher) +@protocol ARTLocalDeviceFetcher + +/** + Fetches an `ARTLocalDevice` instance. The receiver may ignore the arguments (e.g. if it has already instantiated an `ARTLocalDevice` instance, it may instead return that instance). + + This method can safely be called from any thread. + */ +- (ARTLocalDevice *)fetchLocalDeviceWithClientID:(NSString *)clientID + storage:(id)storage + logger:(nullable ARTInternalLog *)logger; + +@end + +/** + The implementation of `ARTLocalDeviceFetcher` that should be used in non-test code. Its `sharedInstance` class property manages the `ARTLocalDevice` instance shared by all `ARTRest` instances. + */ +NS_SWIFT_NAME(DefaultLocalDeviceFetcher) +@interface ARTDefaultLocalDeviceFetcher: NSObject + +/** + Use `ARTDefaultLocalDeviceFetcher.sharedInstance` instead of `init`. + */ +- (instancetype)init NS_UNAVAILABLE; + +@property (nonatomic, class, readonly) ARTDefaultLocalDeviceFetcher *sharedInstance; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Source/PrivateHeaders/Ably/ARTRest+Private.h b/Source/PrivateHeaders/Ably/ARTRest+Private.h index dfcb74305..ba7b26a6d 100644 --- a/Source/PrivateHeaders/Ably/ARTRest+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRest+Private.h @@ -74,11 +74,6 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSObject *)internetIsUp:(void (^)(BOOL isUp))cb; -#if TARGET_OS_IOS -// This is only intended to be called from test code. -- (void)resetDeviceSingleton; -#endif - @end @interface ARTRest () diff --git a/Source/PrivateHeaders/Ably/ARTTestClientOptions.h b/Source/PrivateHeaders/Ably/ARTTestClientOptions.h index f75bbda51..e11b071c3 100644 --- a/Source/PrivateHeaders/Ably/ARTTestClientOptions.h +++ b/Source/PrivateHeaders/Ably/ARTTestClientOptions.h @@ -1,6 +1,7 @@ @import Foundation; @protocol ARTRealtimeTransportFactory; +@protocol ARTLocalDeviceFetcher; NS_ASSUME_NONNULL_BEGIN @@ -31,6 +32,11 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic) id transportFactory; +/** + Initial value is `ARTDefaultLocalDeviceFetcher.sharedInstance`. + */ +@property (nonatomic) id localDeviceFetcher; + @end NS_ASSUME_NONNULL_END diff --git a/Source/include/Ably.modulemap b/Source/include/Ably.modulemap index 9e3b028bc..f6728c37a 100644 --- a/Source/include/Ably.modulemap +++ b/Source/include/Ably.modulemap @@ -107,5 +107,7 @@ framework module Ably { header "Ably/ARTInternalLogCore+Testing.h" header "Ably/ARTDataEncoder.h" header "Ably/ARTRealtimeTransportFactory.h" + header "Ably/ARTLocalDeviceFetcher.h" + header "Ably/ARTLocalDeviceFetcher+Testing.h" } } diff --git a/Test/Tests/DefaultLocalDeviceFetcherTests.swift b/Test/Tests/DefaultLocalDeviceFetcherTests.swift new file mode 100644 index 000000000..6a88598a6 --- /dev/null +++ b/Test/Tests/DefaultLocalDeviceFetcherTests.swift @@ -0,0 +1,46 @@ +import Ably.Private +import XCTest + +class DefaultLocalDeviceFetcherTests: XCTestCase { + func test_fetchLocalDevice_returnsSameDeviceInstance() { + let fetcher = DefaultLocalDeviceFetcher.sharedInstance + defer { fetcher.resetDevice() } + + let device1 = fetcher.fetchLocalDevice( + withClientID: "clientID-1", + storage: MockDeviceStorage(), + logger: nil + ) + + let device2 = fetcher.fetchLocalDevice( + withClientID: "clientID-2", + storage: MockDeviceStorage(), + logger: nil + ) + + XCTAssertTrue(device1 === device2) + XCTAssertEqual(device1.clientId, "clientID-1") + } + + func test_resetDevice() { + let fetcher = DefaultLocalDeviceFetcher.sharedInstance + defer { fetcher.resetDevice() } + + let device1 = fetcher.fetchLocalDevice( + withClientID: "clientID-1", + storage: MockDeviceStorage(), + logger: nil + ) + + fetcher.resetDevice() + + let device2 = fetcher.fetchLocalDevice( + withClientID: "clientID-2", + storage: MockDeviceStorage(), + logger: nil + ) + + XCTAssertTrue(device1 !== device2) + XCTAssertEqual(device2.clientId, "clientID-2") + } +} diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 72d1bb368..1bcac9470 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -40,7 +40,7 @@ class PushActivationStateMachineTests: XCTestCase { } override func tearDown() { - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() super.tearDown() } @@ -119,7 +119,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__014__Activation_state_machine__State_NotActivated__on_Event_CalledActivate__local_device__should_have_a_generated_id() { beforeEach__Activation_state_machine__State_NotActivated() - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() XCTAssertEqual(rest.device.id.count, 36) } @@ -473,7 +473,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__028__Activation_state_machine__State_WaitingForDeviceRegistration__on_Event_GotDeviceRegistration() { beforeEach__Activation_state_machine__State_WaitingForDeviceRegistration() - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() var activatedCallbackCalled = false let hook = stateMachine.testSuite_injectIntoMethod(after: NSSelectorFromString("callActivatedCallback:")) { @@ -911,7 +911,7 @@ class PushActivationStateMachineTests: XCTestCase { let storage = MockDeviceStorage() rest.internal.storage = storage rest.device.setAndPersistIdentityTokenDetails(nil) - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() XCTAssertNil(rest.device.identityTokenDetails) XCTAssertEqual(rest.device.isRegistered(), false) XCTAssertNil(storage.object(forKey: ARTDeviceIdentityTokenKey)) diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index 055792eee..6107f7420 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -27,7 +27,7 @@ class PushChannelTests: XCTestCase { rest = ARTRest(options: options) rest.internal.options.clientId = "tester" rest.internal.httpExecutor = mockHttpExecutor - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() } // RSH7 diff --git a/Test/Tests/PushTests.swift b/Test/Tests/PushTests.swift index 10bec61ac..cbb429c31 100644 --- a/Test/Tests/PushTests.swift +++ b/Test/Tests/PushTests.swift @@ -28,7 +28,7 @@ class PushTests: XCTestCase { super.setUp() rest = ARTRest(key: "xxxx:xxxx") - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() mockHttpExecutor = MockHTTPExecutor() rest.internal.httpExecutor = mockHttpExecutor storage = MockDeviceStorage() @@ -126,7 +126,7 @@ class PushTests: XCTestCase { let storage = MockDeviceStorage() rest.internal.storage = storage - rest.internal.resetDeviceSingleton() + DefaultLocalDeviceFetcher.sharedInstance.resetDevice() var stateMachine: ARTPushActivationStateMachine! waitUntil(timeout: testTimeout) { done in From c3decc996f9146ef69cc8a9a4fc7a77899f3bedb Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 12:12:23 -0300 Subject: [PATCH 10/16] Add a mock instance of ARTLocalDeviceFetcher --- Ably.xcodeproj/project.pbxproj | 8 ++++++++ .../MockLocalDeviceFetcher.swift | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 Test/Test Utilities/MockLocalDeviceFetcher.swift diff --git a/Ably.xcodeproj/project.pbxproj b/Ably.xcodeproj/project.pbxproj index 0aa530d13..8cd0687e0 100644 --- a/Ably.xcodeproj/project.pbxproj +++ b/Ably.xcodeproj/project.pbxproj @@ -169,6 +169,9 @@ 214D3F3229EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */ = {isa = PBXBuildFile; fileRef = 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */; settings = {ATTRIBUTES = (Private, ); }; }; 214D3F3329EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h in Headers */ = {isa = PBXBuildFile; fileRef = 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */; settings = {ATTRIBUTES = (Private, ); }; }; 214D3F3529EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 214D3F3429EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift */; }; + 214D3F3929EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 214D3F3829EEDA420080D080 /* MockLocalDeviceFetcher.swift */; }; + 214D3F3A29EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 214D3F3829EEDA420080D080 /* MockLocalDeviceFetcher.swift */; }; + 214D3F3B29EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 214D3F3829EEDA420080D080 /* MockLocalDeviceFetcher.swift */; }; 215F75F82922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; 215F75F92922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; 215F75FA2922B1DB009E0E76 /* ARTClientInformation.h in Headers */ = {isa = PBXBuildFile; fileRef = 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -1165,6 +1168,7 @@ 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockInternalLogCore.swift; sourceTree = ""; }; 214D3F3029EECE6A0080D080 /* ARTLocalDeviceFetcher+Testing.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "ARTLocalDeviceFetcher+Testing.h"; path = "PrivateHeaders/Ably/ARTLocalDeviceFetcher+Testing.h"; sourceTree = ""; }; 214D3F3429EED43C0080D080 /* DefaultLocalDeviceFetcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultLocalDeviceFetcherTests.swift; sourceTree = ""; }; + 214D3F3829EEDA420080D080 /* MockLocalDeviceFetcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockLocalDeviceFetcher.swift; sourceTree = ""; }; 215F75F62922B1DB009E0E76 /* ARTClientInformation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARTClientInformation.h; path = include/Ably/ARTClientInformation.h; sourceTree = ""; }; 215F75F72922B1DB009E0E76 /* ARTClientInformation.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ARTClientInformation.m; sourceTree = ""; }; 215F75FE2922B30F009E0E76 /* ClientInformationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClientInformationTests.swift; sourceTree = ""; }; @@ -1736,6 +1740,7 @@ 2124B78629DB127900AD8361 /* MockVersion2Log.swift */, 2147F03429E5872C0071CB94 /* MockInternalLogCore.swift */, 210F67B029E9DB62007B9345 /* TestProxyTransportFactory.swift */, + 214D3F3829EEDA420080D080 /* MockLocalDeviceFetcher.swift */, ); path = "Test Utilities"; sourceTree = ""; @@ -2985,6 +2990,7 @@ EB1B53F922F85CE4006A59AC /* ObjectLifetimesTests.swift in Sources */, 210F67B129E9DB62007B9345 /* TestProxyTransportFactory.swift in Sources */, D7DF73851EA600240013CD36 /* PushActivationStateMachineTests.swift in Sources */, + 214D3F3929EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */, 211A60D729D6D2C300D169C5 /* BackoffRetryDelayCalculatorTests.swift in Sources */, 217FCF3229D62460006E5F2D /* RetrySequenceTests.swift in Sources */, D7FC1ECB209CEA2E001E4153 /* PushTests.swift in Sources */, @@ -3192,6 +3198,7 @@ 217FCF3329D62460006E5F2D /* RetrySequenceTests.swift in Sources */, 2147F03629E5872C0071CB94 /* MockInternalLogCore.swift in Sources */, 21113B6429DDF7ED00652C86 /* ARTInternalLogTests.m in Sources */, + 214D3F3A29EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */, D7093C2A219E466E00723F17 /* UtilitiesTests.swift in Sources */, D798554923EB96C000946BE2 /* DeltaCodecTests.swift in Sources */, 2124B78829DB127900AD8361 /* MockVersion2Log.swift in Sources */, @@ -3243,6 +3250,7 @@ 217FCF3429D62460006E5F2D /* RetrySequenceTests.swift in Sources */, 2147F03729E5872C0071CB94 /* MockInternalLogCore.swift in Sources */, 21113B6529DDF7EF00652C86 /* ARTInternalLogTests.m in Sources */, + 214D3F3B29EEDA420080D080 /* MockLocalDeviceFetcher.swift in Sources */, D7093C7C219EE26400723F17 /* RealtimeClientConnectionTests.swift in Sources */, D798554A23EB96C000946BE2 /* DeltaCodecTests.swift in Sources */, 2124B78929DB127900AD8361 /* MockVersion2Log.swift in Sources */, diff --git a/Test/Test Utilities/MockLocalDeviceFetcher.swift b/Test/Test Utilities/MockLocalDeviceFetcher.swift new file mode 100644 index 000000000..93ab188fc --- /dev/null +++ b/Test/Test Utilities/MockLocalDeviceFetcher.swift @@ -0,0 +1,20 @@ +import Ably.Private + +@objc(ARTMockLocalDeviceFetcher) +class MockLocalDeviceFetcher: NSObject, LocalDeviceFetcher { + private let semaphore = DispatchSemaphore(value: 1) + private var device: ARTLocalDevice? + + func fetchLocalDevice(withClientID clientID: String, storage: ARTDeviceStorage, logger: InternalLog?) -> ARTLocalDevice { + semaphore.wait() + let device: ARTLocalDevice + if let existingDevice = self.device { + device = existingDevice + } else { + device = ARTLocalDevice.load(clientID, storage: storage, logger: logger) + self.device = device + } + semaphore.signal() + return device + } +} From f9a112537e313e4a2c5ebb554370c090ecf6185d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 13:35:55 -0300 Subject: [PATCH 11/16] Change ARTLocalDevice initialization clientID to nullable nil is being passed in the tests, and then the Swift mock is seeing it as an empty string --- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h | 2 +- Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h | 2 +- Test/Test Utilities/MockLocalDeviceFetcher.swift | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index 89a67ee81..1586ffb68 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -14,7 +14,7 @@ extern NSString *const ARTAPNSDeviceTokenKey; @property (nonatomic) id storage; -+ (ARTLocalDevice *)load:(NSString *)clientId storage:(id)storage logger:(nullable ARTInternalLog *)logger; ++ (ARTLocalDevice *)load:(nullable NSString *)clientId storage:(id)storage logger:(nullable ARTInternalLog *)logger; - (nullable NSString *)apnsDeviceToken; - (void)setAndPersistAPNSDeviceToken:(nullable NSString *)deviceToken; - (void)setAndPersistIdentityTokenDetails:(nullable ARTDeviceIdentityTokenDetails *)tokenDetails; diff --git a/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h index eba164fc8..69a4ad69e 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDeviceFetcher.h @@ -17,7 +17,7 @@ NS_SWIFT_NAME(LocalDeviceFetcher) This method can safely be called from any thread. */ -- (ARTLocalDevice *)fetchLocalDeviceWithClientID:(NSString *)clientID +- (ARTLocalDevice *)fetchLocalDeviceWithClientID:(nullable NSString *)clientID storage:(id)storage logger:(nullable ARTInternalLog *)logger; diff --git a/Test/Test Utilities/MockLocalDeviceFetcher.swift b/Test/Test Utilities/MockLocalDeviceFetcher.swift index 93ab188fc..42b89c30d 100644 --- a/Test/Test Utilities/MockLocalDeviceFetcher.swift +++ b/Test/Test Utilities/MockLocalDeviceFetcher.swift @@ -5,7 +5,7 @@ class MockLocalDeviceFetcher: NSObject, LocalDeviceFetcher { private let semaphore = DispatchSemaphore(value: 1) private var device: ARTLocalDevice? - func fetchLocalDevice(withClientID clientID: String, storage: ARTDeviceStorage, logger: InternalLog?) -> ARTLocalDevice { + func fetchLocalDevice(withClientID clientID: String?, storage: ARTDeviceStorage, logger: InternalLog?) -> ARTLocalDevice { semaphore.wait() let device: ARTLocalDevice if let existingDevice = self.device { From 4cf201478555b08a5b65427ccc5d9a267b89ffd5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 14:03:50 -0300 Subject: [PATCH 12/16] Stop using localDevice variable that will have nothing to do with the Realtime TODO describe better i.e. just use the bits of TestEnvironment that are actually relevant to the test --- Test/Tests/PushAdminTests.swift | 107 +++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 28 deletions(-) diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index 33260824e..27500832b 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -2,11 +2,6 @@ import Ably import Nimble import XCTest -private var rest: ARTRest! -private var mockHttpExecutor: MockHTTPExecutor! -private var storage: MockDeviceStorage! -private var localDevice: ARTLocalDevice! - private let recipient = [ "clientId": "bob", ] @@ -164,10 +159,6 @@ class PushAdminTests: XCTestCase { // XCTest invokes this method before executing the first test in the test suite. We use it to ensure that the global variables are initialized at the same moment, and in the same order, as they would have been when we used the Quick testing framework. override class var defaultTestSuite: XCTestSuite { - _ = rest - _ = mockHttpExecutor - _ = storage - _ = localDevice _ = recipient _ = payload _ = quxChannelName @@ -176,28 +167,35 @@ class PushAdminTests: XCTestCase { return super.defaultTestSuite } - override func setUp() { - super.setUp() - - rest = ARTRest(key: "xxxx:xxxx") - mockHttpExecutor = MockHTTPExecutor() - rest.internal.httpExecutor = mockHttpExecutor - storage = MockDeviceStorage() - rest.internal.storage = storage - localDevice = rest.device + private struct TestEnvironment { + var rest: ARTRest + var mockHttpExecutor: MockHTTPExecutor + var storage: MockDeviceStorage + var localDevice: ARTLocalDevice + + init() { + rest = ARTRest(key: "xxxx:xxxx") + mockHttpExecutor = MockHTTPExecutor() + rest.internal.httpExecutor = mockHttpExecutor + storage = MockDeviceStorage() + rest.internal.storage = storage + localDevice = rest.device + } } // RSH1a func test__001__publish__should_perform_an_HTTP_request_to__push_publish() throws { + let testEnvironment = TestEnvironment() + waitUntil(timeout: testTimeout) { done in - rest.push.admin.publish(recipient, data: payload) { error in + testEnvironment.rest.push.admin.publish(recipient, data: payload) { error in XCTAssertNil(error) done() } } - let request = try XCTUnwrap(mockHttpExecutor.requests.first, "No request found") + let request = try XCTUnwrap(testEnvironment.mockHttpExecutor.requests.first, "No request found") let url = try XCTUnwrap(request.url, "No request url found") expect(url.absoluteString).to(contain("/push/publish")) @@ -366,6 +364,8 @@ class PushAdminTests: XCTestCase { func test__008__Device_Registrations__get__push_device_authentication__should_include_DeviceIdentityToken_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails( @@ -376,9 +376,12 @@ class PushAdminTests: XCTestCase { clientId: "" ) + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) - realtime.internal.rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { realtime.internal.rest.device.setAndPersistIdentityTokenDetails(nil) } + localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) + defer { localDevice.setAndPersistIdentityTokenDetails(nil) } waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.get(localDevice.id) { _, _ in @@ -395,8 +398,14 @@ class PushAdminTests: XCTestCase { func test__009__Device_Registrations__get__push_device_authentication__should_include_DeviceSecret_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device + waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.get(localDevice.id) { _, _ in done() @@ -478,7 +487,10 @@ class PushAdminTests: XCTestCase { options.pushFullWait = true let realtime = ARTRealtime(options: options) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.remove(Self.deviceDetails.id) { error in XCTAssertNil(error) @@ -500,7 +512,10 @@ class PushAdminTests: XCTestCase { options.pushFullWait = true let realtime = ARTRealtime(options: options) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.save(Self.deviceDetails) { error in XCTAssertNil(error) @@ -520,6 +535,8 @@ class PushAdminTests: XCTestCase { options.pushFullWait = true let realtime = ARTRealtime(options: options) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails( @@ -530,9 +547,12 @@ class PushAdminTests: XCTestCase { clientId: "" ) + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) - realtime.internal.rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { realtime.internal.rest.device.setAndPersistIdentityTokenDetails(nil) } + localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) + defer { localDevice.setAndPersistIdentityTokenDetails(nil) } waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.save(localDevice) { error in @@ -553,8 +573,14 @@ class PushAdminTests: XCTestCase { options.pushFullWait = true let realtime = ARTRealtime(options: options) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device + waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.save(localDevice) { error in XCTAssertNil(error) @@ -594,6 +620,7 @@ class PushAdminTests: XCTestCase { } } + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor waitUntil(timeout: testTimeout) { done in @@ -699,6 +726,8 @@ class PushAdminTests: XCTestCase { func test__022__Channel_Subscriptions__save__push_device_authentication__should_include_DeviceIdentityToken_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails( @@ -709,9 +738,12 @@ class PushAdminTests: XCTestCase { clientId: "" ) + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) - realtime.internal.rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { realtime.internal.rest.device.setAndPersistIdentityTokenDetails(nil) } + localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) + defer { localDevice.setAndPersistIdentityTokenDetails(nil) } let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) @@ -731,8 +763,14 @@ class PushAdminTests: XCTestCase { func test__023__Channel_Subscriptions__save__push_device_authentication__should_include_DeviceSecret_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device + let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) waitUntil(timeout: testTimeout) { done in @@ -822,6 +860,8 @@ class PushAdminTests: XCTestCase { func test__027__Channel_Subscriptions__remove__push_device_authentication__should_include_DeviceIdentityToken_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails( @@ -832,9 +872,12 @@ class PushAdminTests: XCTestCase { clientId: "" ) + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) - realtime.internal.rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { realtime.internal.rest.device.setAndPersistIdentityTokenDetails(nil) } + localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) + defer { localDevice.setAndPersistIdentityTokenDetails(nil) } let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) @@ -854,8 +897,14 @@ class PushAdminTests: XCTestCase { func test__028__Channel_Subscriptions__remove__push_device_authentication__should_include_DeviceSecret_HTTP_header() throws { let realtime = ARTRealtime(options: AblyTests.commonAppSetup()) defer { realtime.dispose(); realtime.close() } + + let mockHttpExecutor = MockHTTPExecutor() realtime.internal.rest.httpExecutor = mockHttpExecutor + realtime.internal.rest.storage = MockDeviceStorage() + + let localDevice = realtime.internal.rest.device + let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) waitUntil(timeout: testTimeout) { done in @@ -1046,6 +1095,8 @@ class PushAdminTests: XCTestCase { } func test__033__local_device__should_include_an_id_and_a_secret() { + let testEnvironment = TestEnvironment() + let localDevice = testEnvironment.localDevice XCTAssertNotNil(localDevice.id) XCTAssertNotNil(localDevice.secret) XCTAssertNil(localDevice.identityTokenDetails) From f240b671a61fe6a12274113bf5a95de47a9ee380 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 12:19:59 -0300 Subject: [PATCH 13/16] WIP work on not calling default local device fetcher in tests TODO are there tests that rely on using the same fetcher in both cases? Confirmed (by adding logging) that DefaultLocalDeviceFetcher is not being used in the test suite now, except for in its own tests. --- Test/Test Utilities/TestUtilities.swift | 1 + .../PushActivationStateMachineTests.swift | 24 +++++++++++-------- Test/Tests/PushAdminTests.swift | 5 +++- Test/Tests/PushChannelTests.swift | 2 +- Test/Tests/PushTests.swift | 23 ++++++++++++------ 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 7eb8e48bf..d20e53b27 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -171,6 +171,7 @@ class AblyTests { } options.dispatchQueue = DispatchQueue.main options.internalDispatchQueue = queue + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() return options } diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 1bcac9470..6eafc493d 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -31,7 +31,10 @@ class PushActivationStateMachineTests: XCTestCase { override func setUp() { super.setUp() - rest = ARTRest(key: "xxxx:xxxx") + let options = ARTClientOptions(key: "xxxx:xxxx") + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() + + rest = ARTRest(options: options) httpExecutor = MockHTTPExecutor() rest.internal.httpExecutor = httpExecutor storage = MockDeviceStorage() @@ -39,12 +42,6 @@ class PushActivationStateMachineTests: XCTestCase { initialStateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) } - override func tearDown() { - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() - - super.tearDown() - } - func test__002__Activation_state_machine__should_set_NotActivated_state_as_current_state_when_disk_is_empty() { expect(initialStateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) } @@ -119,7 +116,8 @@ class PushActivationStateMachineTests: XCTestCase { func test__014__Activation_state_machine__State_NotActivated__on_Event_CalledActivate__local_device__should_have_a_generated_id() { beforeEach__Activation_state_machine__State_NotActivated() - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() + // TODO was the reset here in a special place? + XCTAssertEqual(rest.device.id.count, 36) } @@ -138,6 +136,7 @@ class PushActivationStateMachineTests: XCTestCase { let options = ARTClientOptions(key: "xxxx:xxxx") options.clientId = "deviceClient" + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() let rest = ARTRest(options: options) rest.internal.storage = storage XCTAssertEqual(rest.device.clientId, "deviceClient") @@ -473,7 +472,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__028__Activation_state_machine__State_WaitingForDeviceRegistration__on_Event_GotDeviceRegistration() { beforeEach__Activation_state_machine__State_WaitingForDeviceRegistration() - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() + // TODO was the reset here in a special place? var activatedCallbackCalled = false let hook = stateMachine.testSuite_injectIntoMethod(after: NSSelectorFromString("callActivatedCallback:")) { @@ -907,11 +906,12 @@ class PushActivationStateMachineTests: XCTestCase { expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateWaitingForPushDeviceDetails.self)) } + // TODO what is this a test of? func test__001__should_remove_identityTokenDetails_from_cache_and_storage() { let storage = MockDeviceStorage() rest.internal.storage = storage rest.device.setAndPersistIdentityTokenDetails(nil) - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() +// DefaultLocalDeviceFetcher.sharedInstance.resetDevice() XCTAssertNil(rest.device.identityTokenDetails) XCTAssertEqual(rest.device.isRegistered(), false) XCTAssertNil(storage.object(forKey: ARTDeviceIdentityTokenKey)) @@ -930,14 +930,18 @@ class PushActivationStateMachineTests: XCTestCase { func test__the_local_device_has_id_and_deviceIdentityToken__emits_a_SyncRegistrationFailed_event_with_code_61002_if_client_IDs_don_t_match() { contextBeforeEach?() + let localDeviceFetcher = MockLocalDeviceFetcher() + let options = ARTClientOptions(key: "xxxx:xxxx") options.clientId = "deviceClient" + options.testOptions.localDeviceFetcher = localDeviceFetcher let rest = ARTRest(options: options) rest.internal.storage = storage XCTAssertEqual(rest.device.clientId, "deviceClient") let newOptions = ARTClientOptions(key: "xxxx:xxxx") newOptions.clientId = "instanceClient" + newOptions.testOptions.localDeviceFetcher = localDeviceFetcher let newRest = ARTRest(options: newOptions) newRest.internal.storage = storage let stateMachine = ARTPushActivationStateMachine(rest: newRest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index 27500832b..dfc14d4c9 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -174,7 +174,10 @@ class PushAdminTests: XCTestCase { var localDevice: ARTLocalDevice init() { - rest = ARTRest(key: "xxxx:xxxx") + let options = ARTClientOptions(key: "xxxx:xxxx") + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() + + rest = ARTRest(options: options) mockHttpExecutor = MockHTTPExecutor() rest.internal.httpExecutor = mockHttpExecutor storage = MockDeviceStorage() diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index 6107f7420..a357c5cbc 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -24,10 +24,10 @@ class PushChannelTests: XCTestCase { userQueue = AblyTests.createUserQueue() options.dispatchQueue = userQueue options.internalDispatchQueue = AblyTests.queue + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() rest = ARTRest(options: options) rest.internal.options.clientId = "tester" rest.internal.httpExecutor = mockHttpExecutor - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() } // RSH7 diff --git a/Test/Tests/PushTests.swift b/Test/Tests/PushTests.swift index cbb429c31..3ff691a79 100644 --- a/Test/Tests/PushTests.swift +++ b/Test/Tests/PushTests.swift @@ -27,8 +27,10 @@ class PushTests: XCTestCase { override func setUp() { super.setUp() - rest = ARTRest(key: "xxxx:xxxx") - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() + let options = ARTClientOptions(key: "xxxx:xxxx") + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() + + rest = ARTRest(options: options) mockHttpExecutor = MockHTTPExecutor() rest.internal.httpExecutor = mockHttpExecutor storage = MockDeviceStorage() @@ -126,8 +128,6 @@ class PushTests: XCTestCase { let storage = MockDeviceStorage() rest.internal.storage = storage - DefaultLocalDeviceFetcher.sharedInstance.resetDevice() - var stateMachine: ARTPushActivationStateMachine! waitUntil(timeout: testTimeout) { done in rest.push.internal.getActivationMachine { machine in @@ -203,8 +203,11 @@ class PushTests: XCTestCase { // RSH8 func test__008__LocalDevice__has_a_device_method_that_returns_a_LocalDevice() { - let _: ARTLocalDevice = ARTRest(key: "fake:key").device - let _: ARTLocalDevice = ARTRealtime(key: "fake:key").device + let options = ARTClientOptions(key: "fake:key") + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() + + let _: ARTLocalDevice = ARTRest(options: options).device + let _: ARTLocalDevice = ARTRealtime(options: options).device } // RSH8a @@ -218,7 +221,10 @@ class PushTests: XCTestCase { clientId: "" ) - let rest = ARTRest(key: "fake:key") + let options = ARTClientOptions(key: "fake:key") + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() + + let rest = ARTRest(options: options) rest.internal.storage = storage storage.simulateOnNextRead(string: testToken, for: ARTAPNSDeviceTokenKey) storage.simulateOnNextRead(data: testIdentity.archive(withLogger: nil), for: ARTDeviceIdentityTokenKey) @@ -239,6 +245,7 @@ class PushTests: XCTestCase { callback(ARTTokenDetails(token: "fake:token", expires: nil, issued: nil, capability: nil, clientId: "testClient"), nil) } } + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() let realtime = ARTRealtime(options: options) XCTAssertNil(realtime.device.clientId) @@ -258,6 +265,7 @@ class PushTests: XCTestCase { let options = ARTClientOptions(key: "fake:key") options.autoConnect = false options.testOptions.transportFactory = TestProxyTransportFactory() + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() let realtime = ARTRealtime(options: options) XCTAssertNil(realtime.device.clientId) @@ -295,6 +303,7 @@ class PushTests: XCTestCase { callback(ARTTokenDetails(token: "fake:token", expires: nil, issued: nil, capability: nil, clientId: expectedClient), nil) } } + options.testOptions.localDeviceFetcher = MockLocalDeviceFetcher() let realtime = ARTRealtime(options: options) let mockHttpExecutor = MockHTTPExecutor() From 4471b5b33fd3bd7145050e7fa422291f703b863c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 15:32:56 -0300 Subject: [PATCH 14/16] Use MockDeviceStorage in PushChannelTests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not quite sure why we’ve started needing this now --- Test/Tests/PushChannelTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index a357c5cbc..f36736e41 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -28,6 +28,7 @@ class PushChannelTests: XCTestCase { rest = ARTRest(options: options) rest.internal.options.clientId = "tester" rest.internal.httpExecutor = mockHttpExecutor + rest.internal.storage = MockDeviceStorage() } // RSH7 From 2bb9beb96838dfaedf8bde1c4c97177572dd2006 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 14:10:28 -0300 Subject: [PATCH 15/16] remove now-unnecessary defer --- .../PushActivationStateMachineTests.swift | 19 +++++-------------- Test/Tests/PushAdminTests.swift | 4 ---- Test/Tests/PushChannelTests.swift | 11 ----------- 3 files changed, 5 insertions(+), 29 deletions(-) diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 6eafc493d..3a6e6a02d 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -151,7 +151,6 @@ class PushActivationStateMachineTests: XCTestCase { let testDeviceToken = "xxxx-xxxx-xxxx-xxxx-xxxx" stateMachine.rest.device.setAndPersistAPNSDeviceToken(testDeviceToken) - defer { stateMachine.rest.device.setAndPersistAPNSDeviceToken(nil) } waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) @@ -295,10 +294,9 @@ class PushActivationStateMachineTests: XCTestCase { stateMachine.delegate = delegate var setAndPersistIdentityTokenDetailsCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { + stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { setAndPersistIdentityTokenDetailsCalled = true } - defer { hookDevice.remove() } waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) @@ -389,10 +387,9 @@ class PushActivationStateMachineTests: XCTestCase { stateMachine.delegate = delegate var setAndPersistIdentityTokenDetailsCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { + stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { setAndPersistIdentityTokenDetailsCalled = true } - defer { hookDevice.remove() } waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) @@ -438,7 +435,6 @@ class PushActivationStateMachineTests: XCTestCase { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForPushDeviceDetails(machine: initialStateMachine, logger: .init(core: MockInternalLogCore()))) rest.internal.storage = storage rest.device.setAndPersistAPNSDeviceToken("foo") - defer { rest.device.setAndPersistAPNSDeviceToken(nil) } var registered = false @@ -481,10 +477,9 @@ class PushActivationStateMachineTests: XCTestCase { defer { hook.remove() } var setAndPersistIdentityTokenDetailsCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { + stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { setAndPersistIdentityTokenDetailsCalled = true } - defer { hookDevice.remove() } let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails( token: "123456", @@ -615,10 +610,9 @@ class PushActivationStateMachineTests: XCTestCase { beforeEach() var setAndPersistIdentityTokenDetailsCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { + stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { setAndPersistIdentityTokenDetailsCalled = true } - defer { hookDevice.remove() } let delegate = StateMachineDelegate() stateMachine.delegate = delegate @@ -822,10 +816,9 @@ class PushActivationStateMachineTests: XCTestCase { defer { hook.remove() } var setAndPersistIdentityTokenDetailsCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { + stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("setAndPersistIdentityTokenDetails:")) { setAndPersistIdentityTokenDetailsCalled = true } - defer { hookDevice.remove() } stateMachine.send(ARTPushActivationEventDeregistered()) expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) @@ -950,7 +943,6 @@ class PushActivationStateMachineTests: XCTestCase { let testDeviceIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "deviceClient") stateMachine.rest.device.setAndPersistIdentityTokenDetails(testDeviceIdentityTokenDetails) - defer { stateMachine.rest.device.setAndPersistIdentityTokenDetails(nil) } waitUntil(timeout: testTimeout) { done in stateMachine.transitions = { event, _, _ in @@ -1196,7 +1188,6 @@ class PushActivationStateMachineTests: XCTestCase { XCTAssertNil(rest.device.identityTokenDetails) rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } XCTAssertNotNil(rest.device.identityTokenDetails) waitUntil(timeout: testTimeout) { done in diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index dfc14d4c9..1fa73e2a2 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -384,7 +384,6 @@ class PushAdminTests: XCTestCase { let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { localDevice.setAndPersistIdentityTokenDetails(nil) } waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.get(localDevice.id) { _, _ in @@ -555,7 +554,6 @@ class PushAdminTests: XCTestCase { let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { localDevice.setAndPersistIdentityTokenDetails(nil) } waitUntil(timeout: testTimeout) { done in realtime.push.admin.deviceRegistrations.save(localDevice) { error in @@ -746,7 +744,6 @@ class PushAdminTests: XCTestCase { let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { localDevice.setAndPersistIdentityTokenDetails(nil) } let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) @@ -880,7 +877,6 @@ class PushAdminTests: XCTestCase { let localDevice = realtime.internal.rest.device XCTAssertNil(localDevice.identityTokenDetails) localDevice.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { localDevice.setAndPersistIdentityTokenDetails(nil) } let subscription = ARTPushChannelSubscription(deviceId: localDevice.id, channel: quxChannelName) diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index f36736e41..842fe0eb1 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -53,7 +53,6 @@ class PushChannelTests: XCTestCase { func test__002__Push_Channel__subscribeDevice__should_do_a_POST_request_to__push_channelSubscriptions_and_include_device_authentication() throws { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } let channel = rest.channels.get(uniqueChannelName()) waitUntil(timeout: testTimeout) { done in @@ -85,11 +84,8 @@ class PushChannelTests: XCTestCase { func test__003__Push_Channel__subscribeClient__should_fail_if_the_LocalDevice_doesn_t_have_a_clientId() { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } - let originalClientId = rest.device.clientId rest.device.clientId = nil - defer { rest.device.clientId = originalClientId } waitUntil(timeout: testTimeout) { [userQueue] done in rest.channels.get(uniqueChannelName()).push.subscribeClient { error in @@ -107,7 +103,6 @@ class PushChannelTests: XCTestCase { func test__004__Push_Channel__subscribeClient__should_do_a_POST_request_to__push_channelSubscriptions() throws { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } let channel = rest.channels.get(uniqueChannelName()) waitUntil(timeout: testTimeout) { done in @@ -152,7 +147,6 @@ class PushChannelTests: XCTestCase { func test__006__Push_Channel__unsubscribeDevice__should_do_a_DELETE_request_to__push_channelSubscriptions_and_include_device_authentication() throws { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } let channel = rest.channels.get(uniqueChannelName()) waitUntil(timeout: testTimeout) { done in @@ -182,11 +176,8 @@ class PushChannelTests: XCTestCase { func test__007__Push_Channel__unsubscribeClient__should_fail_if_the_LocalDevice_doesn_t_have_a_clientId() { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } - let originalClientId = rest.device.clientId rest.device.clientId = nil - defer { rest.device.clientId = originalClientId } waitUntil(timeout: testTimeout) { [userQueue] done in rest.channels.get(uniqueChannelName()).push.unsubscribeClient { error in @@ -204,7 +195,6 @@ class PushChannelTests: XCTestCase { func test__008__Push_Channel__unsubscribeClient__should_do_a_DELETE_request_to__push_channelSubscriptions() throws { let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } let channel = rest.channels.get(uniqueChannelName()) waitUntil(timeout: testTimeout) { done in @@ -308,7 +298,6 @@ class PushChannelTests: XCTestCase { // Activate device let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) - defer { rest.device.setAndPersistIdentityTokenDetails(nil) } let channel = rest.channels.get("pushenabled:\(uniqueChannelName())") waitUntil(timeout: testTimeout) { done in From 9ae828fec60d6e49ed186124e54d25aaf327c887 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 18 Apr 2023 14:19:28 -0300 Subject: [PATCH 16/16] add some logging --- Source/ARTLocalDeviceFetcher.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/ARTLocalDeviceFetcher.m b/Source/ARTLocalDeviceFetcher.m index 5024111b1..0da1c862b 100644 --- a/Source/ARTLocalDeviceFetcher.m +++ b/Source/ARTLocalDeviceFetcher.m @@ -38,6 +38,7 @@ + (ARTDefaultLocalDeviceFetcher *)sharedInstance { } - (ARTLocalDevice *)fetchLocalDeviceWithClientID:(NSString *)clientID storage:(id)storage logger:(ARTInternalLog *)logger { + NSLog(@"fetchLocalDeviceWithClientID on default called"); dispatch_semaphore_wait(self.semaphore, DISPATCH_TIME_FOREVER); if (!self.device) { self.device = [ARTLocalDevice load:clientID storage:storage logger:logger];