fix(API): Reachability resolve to GraphQL API (#1167)

* fix(API): Reachability resolve to GraphQL API

* fix(API): fix unit tests

* fix(API): reachabilityMapLock to allow concurrent operations

* fix(API): PR comments move NSLock() into init()

Author: lawmicha

AmplifyPlugins/API/APICategoryPlugin.xcodeproj/project.pbxproj

@@ -113,6 +113,7 @@
 		21D7A118237B54D90057D00D /* APIKeyURLRequestInterceptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21D7A0D5237B54D90057D00D /* APIKeyURLRequestInterceptor.swift */; };
 		21D7A119237B54D90057D00D /* IAMURLRequestInterceptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21D7A0D6237B54D90057D00D /* IAMURLRequestInterceptor.swift */; };
 		21D7A11A237B54D90057D00D /* AWSAPICategoryPluginError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21D7A0D7237B54D90057D00D /* AWSAPICategoryPluginError.swift */; };
+		21DFD3EB2627514C0078833B /* AWSAPICategoryPlugin+ReachabilityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21DFD3EA2627514C0078833B /* AWSAPICategoryPlugin+ReachabilityTests.swift */; };
 		21F40A2B23A0423C0074678E /* GraphQLSyncBasedTests-amplifyconfiguration.json in Resources */ = {isa = PBXBuildFile; fileRef = 21F40A2923A0423C0074678E /* GraphQLSyncBasedTests-amplifyconfiguration.json */; };
 		21F40A2E23A0707E0074678E /* GraphQLModelBasedTests-amplifyconfiguration.json in Resources */ = {isa = PBXBuildFile; fileRef = 21F40A2D23A0707E0074678E /* GraphQLModelBasedTests-amplifyconfiguration.json */; };
 		21FDBB762587D9E40086FCDC /* GraphQLConnectionScenario6Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21FDBB752587D9E40086FCDC /* GraphQLConnectionScenario6Tests.swift */; };
@@ -427,6 +428,7 @@
 		21D7A0D6237B54D90057D00D /* IAMURLRequestInterceptor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IAMURLRequestInterceptor.swift; sourceTree = "<group>"; };
 		21D7A0D7237B54D90057D00D /* AWSAPICategoryPluginError.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AWSAPICategoryPluginError.swift; sourceTree = "<group>"; };
 		21D7A0DE237B54D90057D00D /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
+		21DFD3EA2627514C0078833B /* AWSAPICategoryPlugin+ReachabilityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AWSAPICategoryPlugin+ReachabilityTests.swift"; sourceTree = "<group>"; };
 		21F40A2923A0423C0074678E /* GraphQLSyncBasedTests-amplifyconfiguration.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "GraphQLSyncBasedTests-amplifyconfiguration.json"; sourceTree = "<group>"; };
 		21F40A2D23A0707E0074678E /* GraphQLModelBasedTests-amplifyconfiguration.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "GraphQLModelBasedTests-amplifyconfiguration.json"; sourceTree = "<group>"; };
 		21FDBB752587D9E40086FCDC /* GraphQLConnectionScenario6Tests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GraphQLConnectionScenario6Tests.swift; sourceTree = "<group>"; };
@@ -1114,6 +1116,7 @@
 				B4DFA5DE237A611D0013E17B /* AWSAPICategoryPlugin+ConfigureTests.swift */,
 				B4DFA5CC237A611D0013E17B /* AWSAPICategoryPlugin+GraphQLBehaviorTests.swift */,
 				B4DFA5C9237A611D0013E17B /* AWSAPICategoryPlugin+InterceptorBehaviorTests.swift */,
+				21DFD3EA2627514C0078833B /* AWSAPICategoryPlugin+ReachabilityTests.swift */,
 				B4DFA5CD237A611D0013E17B /* AWSAPICategoryPlugin+ResetTests.swift */,
 				B4DFA5CA237A611D0013E17B /* AWSAPICategoryPlugin+RESTClientBehaviorTests.swift */,
 				B4DFA5D5237A611D0013E17B /* AWSAPICategoryPlugin+URLSessionBehaviorDelegateTests.swift */,
@@ -2448,6 +2451,7 @@
 				FAF2199A24C0F25E00171A3D /* OperationTestBase.swift in Sources */,
 				B4DFA5F3237A611D0013E17B /* GraphQLOperationRequestValidateTests.swift in Sources */,
 				216449982587F9BC00C548A5 /* GraphQLResponseDecoder+DecodeDataTests.swift in Sources */,
+				21DFD3EB2627514C0078833B /* AWSAPICategoryPlugin+ReachabilityTests.swift in Sources */,
 				FA45D78D24C1032E006CBEE9 /* RESTCombineTests.swift in Sources */,
 				21A4F42425A62E3600E1047D /* AppSyncListPayloadTests.swift in Sources */,
 			);

AmplifyPlugins/API/AWSAPICategoryPlugin/AWSAPIPlugin+Reachability.swift

@@ -17,7 +17,20 @@ extension AWSAPIPlugin {
 
     @available(iOS 13.0, *)
     public func reachabilityPublisher(for apiName: String?) throws -> AnyPublisher<ReachabilityUpdate, Never>? {
-        let hostName = try determineHostName(apiName: apiName)
+        let endpoint = try pluginConfig.endpoints.getConfig(for: apiName)
+        guard let hostName = endpoint.baseURL.host else {
+            let error = APIError.invalidConfiguration("Invalid endpoint configuration",
+                """
+                baseURL does not contain a valid hostname
+                """
+            )
+            throw error
+        }
+
+        reachabilityMapLock.lock()
+        defer {
+            reachabilityMapLock.unlock()
+        }
         if let networkReachability = reachabilityMap[hostName] {
             return networkReachability.publisher
         }
@@ -32,51 +45,4 @@ extension AWSAPIPlugin {
             return nil
         }
     }
-
-    private func determineHostName(apiName: String?) throws -> String {
-        if let apiName = apiName {
-            guard let baseUrl = pluginConfig.endpoints[apiName]?.baseURL,
-                let host = baseUrl.host else {
-                    let error = APIError.invalidConfiguration("Invalid endpoint configuration for \(apiName)",
-                                                              """
-                                                              baseURL does not contain a valid hostname
-                                                              for apiName: \(apiName)
-                                                              """
-                    )
-                    throw error
-            }
-            return host
-        }
-
-        if pluginConfig.endpoints.count > 1 {
-            let error = APIError.invalidConfiguration("Unable to determine which endpoint configuration",
-                                                      """
-                                                      Pass in the apiName to disambiguate between which endpoint
-                                                      you are requesting reachability for
-                                                      """
-            )
-            throw error
-        }
-
-        guard let configEntry = pluginConfig.endpoints.first else {
-            let error = APIError.invalidConfiguration("No API configurations found",
-                                                      """
-                                                      Review how the API category is being instantiated and
-                                                      configured.
-                                                      """
-            )
-            throw error
-        }
-
-        guard let host = configEntry.value.baseURL.host else {
-            let error = APIError.invalidConfiguration("Invalid endpoint configuration",
-                """
-                baseURL does not contain a valid hostname
-                """
-            )
-            throw error
-        }
-        return host
-    }
-
 }

AmplifyPlugins/API/AWSAPICategoryPlugin/AWSAPIPlugin+Resettable.swift

@@ -26,7 +26,9 @@ extension AWSAPIPlugin: Resettable {
         authService = nil
 
         if #available(iOS 13.0, *) {
+            reachabilityMapLock.lock()
             reachabilityMap.removeAll()
+            reachabilityMapLock.unlock()
         }
 
         subscriptionConnectionFactory = nil

AmplifyPlugins/API/AWSAPICategoryPlugin/AWSAPIPlugin.swift

@@ -57,14 +57,18 @@ final public class AWSAPIPlugin: NSObject, APICategoryPlugin {
         }
     }
 
+    /// Lock used for performing operations atomically when getting and setting `reachabilityMap`.
+    let reachabilityMapLock: NSLock
+
     public init(
         modelRegistration: AmplifyModelRegistration? = nil,
         sessionFactory: URLSessionBehaviorFactory? = nil,
         apiAuthProviderFactory: APIAuthProviderFactory? = nil
     ) {
         self.mapper = OperationTaskMapper()
         self.queue = OperationQueue()
-        self.authProviderFactory =  apiAuthProviderFactory ?? APIAuthProviderFactory()
+        self.authProviderFactory = apiAuthProviderFactory ?? APIAuthProviderFactory()
+        self.reachabilityMapLock = NSLock()
         super.init()
 
         modelRegistration?.registerModels(registry: ModelRegistry.self)

AmplifyPlugins/API/AWSAPICategoryPlugin/Configuration/AWSAPICategoryPluginConfiguration+EndpointConfig.swift

@@ -281,29 +281,23 @@ public extension AWSAPICategoryPluginConfiguration {
 }
 
 extension Dictionary where Key == String, Value == AWSAPICategoryPluginConfiguration.EndpointConfig {
-    func getConfig(for apiName: String?,
-                   endpointType: AWSAPICategoryPluginEndpointType) throws ->
+
+    /// Getting the `EndpointConfig` resolves to the following rules:
+    /// 1. If `apiName` is specified, retrieve the endpoint configuration for this api
+    /// 2. If `apiName` is not specified, and `endpointType` is, retrieve the endpoint if there is only one.
+    /// 3. If nothing is specified, return the endpoint only if there is a single one, with GraphQL taking precedent
+    /// over REST.
+    func getConfig(for apiName: String? = nil,
+                   endpointType: AWSAPICategoryPluginEndpointType? = nil) throws ->
         AWSAPICategoryPluginConfiguration.EndpointConfig {
         if let apiName = apiName {
             return try getConfig(for: apiName)
         }
-
-        let apiForEndpointType = filter { (_, endpointConfig) -> Bool in
-            return endpointConfig.endpointType == endpointType
-        }
-
-        guard let endpointConfig = apiForEndpointType.first else {
-            throw APIError.invalidConfiguration("Missing API for \(endpointType) endpointType",
-                                                "Add the \(endpointType) API to configuration.")
-        }
-
-        if apiForEndpointType.count > 1 {
-            throw APIError.invalidConfiguration(
-                "More than one \(endpointType) API configured. Could not infer which API to call",
-                "Use the apiName to specify which API to call")
+        if let endpointType = endpointType {
+            return try getConfig(for: endpointType)
         }
 
-        return endpointConfig.value
+        return try getConfig()
     }
 
     private func getConfig(for apiName: String) throws -> AWSAPICategoryPluginConfiguration.EndpointConfig {
@@ -322,4 +316,49 @@ extension Dictionary where Key == String, Value == AWSAPICategoryPluginConfigura
 
         return endpointConfig
     }
+
+    /// Retrieve the endpoint configuration when there is only one endpoint of the specified `endpointType`
+    private func getConfig(for endpointType: AWSAPICategoryPluginEndpointType) throws ->
+        AWSAPICategoryPluginConfiguration.EndpointConfig {
+            let apiForEndpointType = filter { (_, endpointConfig) -> Bool in
+                return endpointConfig.endpointType == endpointType
+            }
+
+            guard let endpointConfig = apiForEndpointType.first else {
+                throw APIError.invalidConfiguration("Missing API for \(endpointType) endpointType",
+                                                    "Add the \(endpointType) API to configuration.")
+            }
+
+            if apiForEndpointType.count > 1 {
+                throw APIError.invalidConfiguration(
+                    "More than one \(endpointType) API configured. Could not infer which API to call",
+                    "Use the apiName to specify which API to call")
+            }
+            return endpointConfig.value
+    }
+
+    /// Retrieve the endpoint only if there is a single one, with GraphQL taking precedent over REST.
+    private func getConfig() throws -> AWSAPICategoryPluginConfiguration.EndpointConfig {
+        let graphQLEndpoints = filter { (_, endpointConfig) -> Bool in
+            return endpointConfig.endpointType == .graphQL
+        }
+
+        if graphQLEndpoints.count == 1, let endpoint = graphQLEndpoints.first {
+            return endpoint.value
+        }
+
+        let restEndpoints = filter { (_, endpointConfig) -> Bool in
+            return endpointConfig.endpointType == .rest
+        }
+
+        if restEndpoints.count == 1, let endpoint = restEndpoints.first {
+            return endpoint.value
+        }
+
+        throw APIError.invalidConfiguration("Unable to resolve endpoint configuration",
+                                            """
+                                            Pass in the apiName to specify the endpoint you are
+                                            retrieving the config for
+                                            """)
+    }
 }

AmplifyPlugins/API/AWSAPICategoryPluginTests/AWSAPICategoryPlugin+ReachabilityTests.swift

@@ -0,0 +1,132 @@
+//
+// Copyright Amazon.com Inc. or its affiliates.
+// All Rights Reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+import XCTest
+import Foundation
+import AWSPluginsCore
+
+@testable import Amplify
+@testable import AmplifyTestCommon
+@testable import AWSAPICategoryPlugin
+
+@available(iOS 13.0, *)
+class AWSAPICategoryPluginReachabilityTests: XCTestCase {
+
+    var apiPlugin: AWSAPIPlugin!
+
+    override func setUp() {
+        apiPlugin = AWSAPIPlugin()
+    }
+
+    override func tearDown() {
+        if let api = apiPlugin {
+            api.reset {
+            }
+        }
+    }
+
+    func testReachabilityReturnsGraphQLAPI() throws {
+        let graphQLAPI = "graphQLAPI"
+        do {
+            let endpointConfig = [graphQLAPI: try getEndpointConfig(apiName: graphQLAPI, endpointType: .graphQL)]
+            let pluginConfig = AWSAPICategoryPluginConfiguration(endpoints: endpointConfig)
+            let dependencies = AWSAPIPlugin.ConfigurationDependencies(
+                pluginConfig: pluginConfig,
+                authService: MockAWSAuthService(),
+                subscriptionConnectionFactory: AWSSubscriptionConnectionFactory()
+            )
+            apiPlugin.configure(using: dependencies)
+        } catch {
+            XCTFail("Failed to create endpoint config")
+        }
+
+        let publisher = try apiPlugin.reachabilityPublisher()
+        XCTAssertNotNil(publisher)
+        XCTAssertEqual(apiPlugin.reachabilityMap.count, 1)
+        guard let reachability = apiPlugin.reachabilityMap.first else {
+            XCTFail("Missing expeected `reachability`")
+            return
+        }
+        XCTAssertEqual(reachability.key, graphQLAPI)
+    }
+
+    func testReachabilityReturnsGraphQLAPIForMultipleEndpoints() throws {
+        let graphQLAPI = "graphQLAPI"
+        let restAPI = "restAPI"
+        do {
+            let endpointConfig = [graphQLAPI: try getEndpointConfig(apiName: graphQLAPI, endpointType: .graphQL),
+                                  restAPI: try getEndpointConfig(apiName: restAPI, endpointType: .rest)]
+            let pluginConfig = AWSAPICategoryPluginConfiguration(endpoints: endpointConfig)
+            let dependencies = AWSAPIPlugin.ConfigurationDependencies(
+                pluginConfig: pluginConfig,
+                authService: MockAWSAuthService(),
+                subscriptionConnectionFactory: AWSSubscriptionConnectionFactory()
+            )
+            apiPlugin.configure(using: dependencies)
+        } catch {
+            XCTFail("Failed to create endpoint config")
+        }
+
+        let publisher = try apiPlugin.reachabilityPublisher()
+        XCTAssertNotNil(publisher)
+        XCTAssertEqual(apiPlugin.reachabilityMap.count, 1)
+        guard let reachability = apiPlugin.reachabilityMap.first else {
+            XCTFail("Missing expeected `reachability`")
+            return
+        }
+        XCTAssertEqual(reachability.key, graphQLAPI)
+    }
+
+    func testReachabilityConcurrentPerform() throws {
+        let graphQLAPI = "graphQLAPI"
+        let restAPI = "restAPI"
+        do {
+            let endpointConfig = [graphQLAPI: try getEndpointConfig(apiName: graphQLAPI, endpointType: .graphQL),
+                                  restAPI: try getEndpointConfig(apiName: restAPI, endpointType: .rest)]
+            let pluginConfig = AWSAPICategoryPluginConfiguration(endpoints: endpointConfig)
+            let dependencies = AWSAPIPlugin.ConfigurationDependencies(
+                pluginConfig: pluginConfig,
+                authService: MockAWSAuthService(),
+                subscriptionConnectionFactory: AWSSubscriptionConnectionFactory()
+            )
+            apiPlugin.configure(using: dependencies)
+        } catch {
+            XCTFail("Failed to create endpoint config")
+        }
+
+        let concurrentPerformCompleted = expectation(description: "concurrent perform completed")
+        concurrentPerformCompleted.expectedFulfillmentCount = 1_000
+        DispatchQueue.concurrentPerform(iterations: 1_000) { _ in
+            do {
+                let graphQLAPIPublisher = try apiPlugin.reachabilityPublisher(for: graphQLAPI)
+                XCTAssertNotNil(graphQLAPIPublisher)
+                let restAPIPublisher = try apiPlugin.reachabilityPublisher(for: restAPI)
+                XCTAssertNotNil(restAPIPublisher)
+            } catch {
+                XCTFail("\(error)")
+            }
+            concurrentPerformCompleted.fulfill()
+
+        }
+        wait(for: [concurrentPerformCompleted], timeout: 1)
+        XCTAssertEqual(apiPlugin.reachabilityMap.count, 2)
+    }
+
+    // MARK: - Helpers
+
+    func getEndpointConfig(apiName: String, endpointType: AWSAPICategoryPluginEndpointType) throws ->
+    AWSAPICategoryPluginConfiguration.EndpointConfig {
+        try AWSAPICategoryPluginConfiguration.EndpointConfig(
+            name: apiName,
+            baseURL: URL(string: "http://\(apiName)")!,
+            region: nil,
+            authorizationType: AWSAuthorizationType.none,
+            authorizationConfiguration: AWSAuthorizationConfiguration.none,
+            endpointType: endpointType,
+            apiAuthProviderFactory: APIAuthProviderFactory())
+    }
+}