Refactored Duplicate Methods in Functions (#13771)

yakovmanshin · web-flow · commit 995c9339f9b3 · 2024-10-03T16:07:46.000-04:00
diff --git a/FirebaseFunctions/Sources/Functions.swift b/FirebaseFunctions/Sources/Functions.swift
@@ -135,7 +135,7 @@ enum FunctionsConstants {
   /// - Parameter name: The name of the Callable HTTPS trigger.
   /// - Returns: A reference to a Callable HTTPS trigger.
   @objc(HTTPSCallableWithName:) open func httpsCallable(_ name: String) -> HTTPSCallable {
-    return HTTPSCallable(functions: self, name: name)
+    HTTPSCallable(functions: self, url: functionURL(for: name)!)
   }
 
   /// Creates a reference to the Callable HTTPS trigger with the given name and configuration
@@ -147,7 +147,7 @@ enum FunctionsConstants {
   @objc(HTTPSCallableWithName:options:) public func httpsCallable(_ name: String,
                                                                   options: HTTPSCallableOptions)
     -> HTTPSCallable {
-    return HTTPSCallable(functions: self, name: name, options: options)
+    HTTPSCallable(functions: self, url: functionURL(for: name)!, options: options)
   }
 
   /// Creates a reference to the Callable HTTPS trigger with the given name.
@@ -369,46 +369,23 @@ enum FunctionsConstants {
               appCheck: appCheck)
   }
 
-  func urlWithName(_ name: String) -> String {
+  func functionURL(for name: String) -> URL? {
     assert(!name.isEmpty, "Name cannot be empty")
 
     // Check if we're using the emulator
     if let emulatorOrigin {
-      return "\(emulatorOrigin)/\(projectID)/\(region)/\(name)"
+      return URL(string: "\(emulatorOrigin)/\(projectID)/\(region)/\(name)")
     }
 
     // Check the custom domain.
     if let customDomain {
-      return "\(customDomain)/\(name)"
+      return URL(string: "\(customDomain)/\(name)")
     }
 
-    return "https://\(region)-\(projectID).cloudfunctions.net/\(name)"
+    return URL(string: "https://\(region)-\(projectID).cloudfunctions.net/\(name)")
   }
 
-  func callFunction(name: String,
-                    withObject data: Any?,
-                    options: HTTPSCallableOptions?,
-                    timeout: TimeInterval,
-                    completion: @escaping ((Result<HTTPSCallableResult, Error>) -> Void)) {
-    // Get context first.
-    contextProvider.getContext(options: options) { context, error in
-      // Note: context is always non-nil since some checks could succeed, we're only failing if
-      // there's an error.
-      if let error {
-        completion(.failure(error))
-      } else {
-        let url = self.urlWithName(name)
-        self.callFunction(url: URL(string: url)!,
-                          withObject: data,
-                          options: options,
-                          timeout: timeout,
-                          context: context,
-                          completion: completion)
-      }
-    }
-  }
-
-  func callFunction(url: URL,
+  func callFunction(at url: URL,
                     withObject data: Any?,
                     options: HTTPSCallableOptions?,
                     timeout: TimeInterval,
diff --git a/FirebaseFunctions/Sources/HTTPSCallable.swift b/FirebaseFunctions/Sources/HTTPSCallable.swift
@@ -39,12 +39,7 @@ open class HTTPSCallable: NSObject {
   // The functions client to use for making calls.
   private let functions: Functions
 
-  private enum EndpointType {
-    case name(String)
-    case url(URL)
-  }
-
-  private let endpoint: EndpointType
+  private let url: URL
 
   private let options: HTTPSCallableOptions?
 
@@ -53,16 +48,10 @@ open class HTTPSCallable: NSObject {
   /// The timeout to use when calling the function. Defaults to 70 seconds.
   @objc open var timeoutInterval: TimeInterval = 70
 
-  init(functions: Functions, name: String, options: HTTPSCallableOptions? = nil) {
-    self.functions = functions
-    self.options = options
-    endpoint = .name(name)
-  }
-
   init(functions: Functions, url: URL, options: HTTPSCallableOptions? = nil) {
     self.functions = functions
+    self.url = url
     self.options = options
-    endpoint = .url(url)
   }
 
   /// Executes this Callable HTTPS trigger asynchronously.
@@ -98,20 +87,13 @@ open class HTTPSCallable: NSObject {
       }
     }
 
-    switch endpoint {
-    case let .name(name):
-      functions.callFunction(name: name,
-                             withObject: data,
-                             options: options,
-                             timeout: timeoutInterval,
-                             completion: callback)
-    case let .url(url):
-      functions.callFunction(url: url,
-                             withObject: data,
-                             options: options,
-                             timeout: timeoutInterval,
-                             completion: callback)
-    }
+    functions.callFunction(
+      at: url,
+      withObject: data,
+      options: options,
+      timeout: timeoutInterval,
+      completion: callback
+    )
   }
 
   /// Executes this Callable HTTPS trigger asynchronously. This API should only be used from
diff --git a/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift b/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift
@@ -30,14 +30,16 @@ private let expectationTimeout: TimeInterval = 2
 
 class MockFunctions: Functions {
   let mockCallFunction: () throws -> HTTPSCallableResult
-  var verifyParameters: ((_ name: String, _ data: Any?, _ timeout: TimeInterval) throws -> Void)?
-  override func callFunction(name: String,
+  var verifyParameters: ((_ url: URL, _ data: Any?, _ timeout: TimeInterval) throws -> Void)?
+  override func callFunction(at url: URL,
                              withObject data: Any?,
                              options: HTTPSCallableOptions?,
                              timeout: TimeInterval,
-                             completion: @escaping ((Result<HTTPSCallableResult, Error>) -> Void)) {
+                             completion: @escaping (
+                               (Result<HTTPSCallableResult, any Error>) -> Void
+                             )) {
     do {
-      try verifyParameters?(name, data, timeout)
+      try verifyParameters?(url, data, timeout)
       let result = try mockCallFunction()
       completion(.success(result))
     } catch {
@@ -49,7 +51,7 @@ class MockFunctions: Functions {
     self.mockCallFunction = mockCallFunction
     super.init(
       projectID: "dummy-project",
-      region: "",
+      region: "test-region",
       customDomain: nil,
       auth: nil,
       messaging: nil,
@@ -122,8 +124,11 @@ class HTTPSCallableTests: XCTestCase {
       httpsFunctionWasCalledExpectation.fulfill()
       return HTTPSCallableResultFake(data: expectedResult)
     }
-    functions.verifyParameters = { name, data, timeout in
-      XCTAssertEqual(name as String, "dummyFunction")
+    functions.verifyParameters = { url, data, timeout in
+      XCTAssertEqual(
+        url.absoluteString,
+        "https://test-region-dummy-project.cloudfunctions.net/dummyFunction"
+      )
       XCTAssertEqual(data as? String, inputParameter)
       XCTAssertEqual(timeout as TimeInterval, timeoutInterval)
     }
@@ -169,8 +174,11 @@ class HTTPSCallableTests: XCTestCase {
                     code: FunctionsErrorCode.internal.rawValue,
                     userInfo: [NSLocalizedDescriptionKey: "Response is missing data field."])
     }
-    functions.verifyParameters = { name, data, timeout in
-      XCTAssertEqual(name as String, "dummyFunction")
+    functions.verifyParameters = { url, data, timeout in
+      XCTAssertEqual(
+        url.absoluteString,
+        "https://test-region-dummy-project.cloudfunctions.net/dummyFunction"
+      )
       XCTAssertEqual(data as? String, inputParameter)
       XCTAssertEqual(timeout as TimeInterval, timeoutInterval)
     }
diff --git a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift
@@ -79,26 +79,34 @@ class FunctionsTests: XCTestCase {
     XCTAssertEqual(functions1, functions2)
   }
 
-  func testURLWithName() throws {
-    let url = try XCTUnwrap(functions?.urlWithName("my-endpoint"))
-    XCTAssertEqual(url, "https://my-region-my-project.cloudfunctions.net/my-endpoint")
+  func testFunctionURLForName() throws {
+    XCTAssertEqual(
+      functions?.functionURL(for: "my-endpoint")?.absoluteString,
+      "https://my-region-my-project.cloudfunctions.net/my-endpoint"
+    )
   }
 
-  func testRegionWithEmulator() throws {
+  func testFunctionURLForNameEmulator() throws {
     functionsCustomDomain?.useEmulator(withHost: "localhost", port: 5005)
-    let url = try XCTUnwrap(functionsCustomDomain?.urlWithName("my-endpoint"))
-    XCTAssertEqual(url, "http://localhost:5005/my-project/my-region/my-endpoint")
+    XCTAssertEqual(
+      functionsCustomDomain?.functionURL(for: "my-endpoint")?.absoluteString,
+      "http://localhost:5005/my-project/my-region/my-endpoint"
+    )
   }
 
-  func testRegionWithEmulatorWithScheme() throws {
+  func testFunctionURLForNameRegionWithEmulatorWithScheme() throws {
     functionsCustomDomain?.useEmulator(withHost: "http://localhost", port: 5005)
-    let url = try XCTUnwrap(functionsCustomDomain?.urlWithName("my-endpoint"))
-    XCTAssertEqual(url, "http://localhost:5005/my-project/my-region/my-endpoint")
+    XCTAssertEqual(
+      functionsCustomDomain?.functionURL(for: "my-endpoint")?.absoluteString,
+      "http://localhost:5005/my-project/my-region/my-endpoint"
+    )
   }
 
-  func testCustomDomain() throws {
-    let url = try XCTUnwrap(functionsCustomDomain?.urlWithName("my-endpoint"))
-    XCTAssertEqual(url, "https://mydomain.com/my-endpoint")
+  func testFunctionURLForNameCustomDomain() throws {
+    XCTAssertEqual(
+      functionsCustomDomain?.functionURL(for: "my-endpoint")?.absoluteString,
+      "https://mydomain.com/my-endpoint"
+    )
   }
 
   func testSetEmulatorSettings() throws {
@@ -303,7 +311,7 @@ class FunctionsTests: XCTestCase {
 
     let completionExpectation = expectation(description: "completionExpectation")
     functionsCustomDomain?.callFunction(
-      name: "fake_func",
+      at: URL(string: "https://example.com/fake_func")!,
       withObject: nil,
       options: nil,
       timeout: 10
