Remove installations ID from Identifiers and move it to SessionCoordinator (#10545)

Author: visumickey

FirebaseSessions/Sources/FirebaseSessions.swift

@@ -54,9 +54,10 @@ protocol SessionsProvider {
 
     let fireLogger = EventGDTLogger(googleDataTransport: googleDataTransport!)
 
-    let identifiers = Identifiers(installations: installations)
+    let identifiers = Identifiers()
     let coordinator = SessionCoordinator(
       identifiers: identifiers,
+      installations: installations,
       fireLogger: fireLogger,
       sampler: SessionSampler()
     )

FirebaseSessions/Sources/FirebaseSessionsError.swift

@@ -18,4 +18,6 @@ import Foundation
 enum FirebaseSessionsError: Error {
   /// Event sampling related error
   case SessionSamplingError
+  /// Firebase Installation ID related error
+  case SessionInstallationsError
 }

FirebaseSessions/Sources/Identifiers.swift

@@ -18,10 +18,6 @@ import Foundation
 @_implementationOnly import FirebaseInstallations
 
 protocol IdentifierProvider {
-  var installationID: String {
-    get
-  }
-
   var sessionID: String {
     get
   }
@@ -39,61 +35,16 @@ protocol IdentifierProvider {
 ///   (Maybe) 4) Persisting, reading, and incrementing an increasing index
 ///
 class Identifiers: IdentifierProvider {
-  private let installations: InstallationsProtocol
-
   private var _sessionID: String?
   private var _previousSessionID: String?
 
-  init(installations: InstallationsProtocol) {
-    self.installations = installations
-  }
-
   // Generates a new Session ID. If there was already a generated Session ID
   // from the last session during the app's lifecycle, it will also set the last Session ID
   func generateNewSessionID() {
     _previousSessionID = _sessionID
     _sessionID = UUID().uuidString.replacingOccurrences(of: "-", with: "").lowercased()
   }
 
-  // Fetches the Installation ID from Firebase Installation Service. This method must be run on a background thread due to how Firebase Installations
-  // handles threading.
-  var installationID: String {
-    if Thread.isMainThread {
-      Logger
-        .logError(
-          "Error: Identifiers.installationID getter must be called on a background thread. Using an empty ID"
-        )
-      return ""
-    }
-
-    var localInstallationID = ""
-
-    let semaphore = DispatchSemaphore(value: 0)
-
-    installations.installationID { result in
-      switch result {
-      case let .success(fiid):
-        localInstallationID = fiid
-      case let .failure(error):
-        Logger
-          .logError(
-            "Error getting Firebase Installation ID: \(error). Using an empty ID"
-          )
-      }
-
-      semaphore.signal()
-    }
-
-    switch semaphore.wait(timeout: DispatchTime.now() + 1.0) {
-    case .success:
-      break
-    case .timedOut:
-      Logger.logError("Error: took too long to get the Firebase Installation ID. Using an empty ID")
-    }
-
-    return localInstallationID
-  }
-
   var sessionID: String {
     guard let _sessionID = _sessionID else {
       Logger.logError("Error: Sessions SDK did not generate a Session ID")

FirebaseSessions/Sources/SessionCoordinator.swift

@@ -20,33 +20,51 @@ import Foundation
 ///
 class SessionCoordinator {
   let identifiers: IdentifierProvider
+  let installations: InstallationsProtocol
   let fireLogger: EventGDTLoggerProtocol
   let sampler: SessionSamplerProtocol
 
-  init(identifiers: IdentifierProvider, fireLogger: EventGDTLoggerProtocol,
+  init(identifiers: IdentifierProvider,
+       installations: InstallationsProtocol,
+       fireLogger: EventGDTLoggerProtocol,
        sampler: SessionSamplerProtocol) {
     self.identifiers = identifiers
+    self.installations = installations
     self.fireLogger = fireLogger
     self.sampler = sampler
   }
 
   // Begins the process of logging a SessionStartEvent to FireLog, while taking into account Data Collection, Sampling, and fetching Settings
   func attemptLoggingSessionStart(event: SessionStartEvent,
                                   callback: @escaping (Result<Void, Error>) -> Void) {
+    /// Order of execution
+    /// 1. Check if the session can be sent. If yes, move to 2. Else, drop the event.
+    /// 2. Fetch the installations Id. If successful, move to 3. Else, drop sending the event.
+    /// 3. Log the event. If successful, all is good. Else, log the message with error.
     if sampler.shouldSendEventForSession(sessionId: identifiers.sessionID) {
-      event.setInstallationID(identifiers: identifiers)
-
-      fireLogger.logEvent(event: event) { result in
+      installations.installationID { result in
         switch result {
-        case .success():
-          Logger.logInfo("Successfully logged Session Start event to GoogleDataTransport")
-          callback(.success(()))
+        case let .success(fiid):
+          event.setInstallationID(installationId: fiid)
+          self.fireLogger.logEvent(event: event) { logResult in
+            switch logResult {
+            case .success():
+              Logger.logInfo("Successfully logged Session Start event to GoogleDataTransport")
+              callback(.success(()))
+            case let .failure(error):
+              Logger
+                .logError(
+                  "Error logging Session Start event to GoogleDataTransport: \(error)."
+                )
+              callback(.failure(error))
+            }
+          }
         case let .failure(error):
           Logger
             .logError(
-              "Error logging Session Start event to GoogleDataTransport: \(error)."
+              "Error getting Firebase Installation ID: \(error). Skip sending event."
             )
-          callback(.failure(error))
+          callback(.failure(FirebaseSessionsError.SessionInstallationsError))
         }
       }
     } else {

FirebaseSessions/Sources/SessionStartEvent.swift

@@ -60,8 +60,8 @@ class SessionStartEvent: NSObject, GDTCOREventDataObject {
       .performance = firebase_appquality_sessions_DataCollectionState_COLLECTION_UNKNOWN
   }
 
-  func setInstallationID(identifiers: IdentifierProvider) {
-    proto.session_data.firebase_installation_id = makeProtoString(identifiers.installationID)
+  func setInstallationID(installationId: String) {
+    proto.session_data.firebase_installation_id = makeProtoString(installationId)
   }
 
   func setSamplingRate(samplingRate: Double) {

FirebaseSessions/Tests/Unit/IdentifiersTests.swift

@@ -16,10 +16,8 @@
 import XCTest
 
 @testable import FirebaseSessions
-@testable import FirebaseInstallations
 
 class IdentifiersTests: XCTestCase {
-  var installations: MockInstallationsProtocol!
   var identifiers: Identifiers!
 
   override func setUp() {
@@ -28,8 +26,7 @@ class IdentifiersTests: XCTestCase {
       UserDefaults.standard.removePersistentDomain(forName: appDomain)
     }
 
-    installations = MockInstallationsProtocol()
-    identifiers = Identifiers(installations: installations)
+    identifiers = Identifiers()
   }
 
   func isValidSessionID(_ sessionID: String) -> Bool {
@@ -78,34 +75,4 @@ class IdentifiersTests: XCTestCase {
     // Ensure the new lastSessionID is equal to the sessionID from earlier
     XCTAssertEqual(identifiers.previousSessionID, firstSessionID)
   }
-
-  // Fetching FIIDs requires that we are on a background thread.
-  func test_installationID_getsValidID() throws {
-    // Make our mock return an ID
-    let testID = "testID"
-    installations.result = .success(testID)
-
-    let expectation = XCTestExpectation(description: "Get the Installation ID Asynchronously")
-
-    DispatchQueue.global().async { [self] in
-      XCTAssertEqual(self.identifiers.installationID, testID)
-      expectation.fulfill()
-    }
-
-    wait(for: [expectation], timeout: 1.0)
-  }
-
-  func test_installationID_handlesFailedFetch() throws {
-    // Make our mock return an error
-    installations.result = .failure(NSError(domain: "FestFailedFIIDErrorDomain", code: 0))
-
-    let expectation = XCTestExpectation(description: "Get the Installation ID Asynchronously")
-
-    DispatchQueue.global().async { [self] in
-      XCTAssertEqual(self.identifiers.installationID, "")
-      expectation.fulfill()
-    }
-
-    wait(for: [expectation], timeout: 1.0)
-  }
 }

FirebaseSessions/Tests/Unit/Mocks/MockInstallationsProtocol.swift

@@ -18,7 +18,8 @@ import FirebaseInstallations
 @testable import FirebaseSessions
 
 class MockInstallationsProtocol: InstallationsProtocol {
-  var result: Result<String, Error> = .success("adsgsadg")
+  static let testInstallationId = "testInstallationId"
+  var result: Result<String, Error> = .success(testInstallationId)
 
   func installationID(completion: @escaping (Result<String, Error>) -> Void) {
     completion(result)

FirebaseSessions/Tests/Unit/SessionCoordinatorTests.swift

@@ -19,6 +19,7 @@ import XCTest
 
 class SessionCoordinatorTests: XCTestCase {
   var identifiers = MockIdentifierProvider()
+  var installations = MockInstallationsProtocol()
   var time = MockTimeProvider()
   var fireLogger = MockGDTLogger()
   var appInfo = MockApplicationInfo()
@@ -31,6 +32,7 @@ class SessionCoordinatorTests: XCTestCase {
 
     coordinator = SessionCoordinator(
       identifiers: identifiers,
+      installations: installations,
       fireLogger: fireLogger,
       sampler: sampler
     )
@@ -53,7 +55,7 @@ class SessionCoordinatorTests: XCTestCase {
     // Make sure we've set the Installation ID
     assertEqualProtoString(
       event.proto.session_data.firebase_installation_id,
-      expected: MockIdentifierProvider.testInstallationID,
+      expected: MockInstallationsProtocol.testInstallationId,
       fieldName: "installation_id"
     )
 
@@ -82,7 +84,7 @@ class SessionCoordinatorTests: XCTestCase {
     // Make sure we've set the Installation ID
     assertEqualProtoString(
       event.proto.session_data.firebase_installation_id,
-      expected: MockIdentifierProvider.testInstallationID,
+      expected: MockInstallationsProtocol.testInstallationId,
       fieldName: "installation_id"
     )
 

FirebaseSessions/Tests/Unit/SessionStartEventTests.swift

@@ -111,7 +111,7 @@ class SessionStartEventTests: XCTestCase {
     identifiers.mockAllValidIDs()
 
     let event = SessionStartEvent(identifiers: identifiers, appInfo: appInfo, time: time)
-    event.setInstallationID(identifiers: identifiers)
+    event.setInstallationID(installationId: "testInstallationID")
 
     testProtoAndDecodedProto(sessionEvent: event) { proto in
       assertEqualProtoString(