[Sessions] Fix how Session ID is generated (#10430)

Author: samedson

FirebaseSessions/Sources/Identifiers.swift

@@ -17,9 +17,6 @@ import Foundation
 
 @_implementationOnly import FirebaseInstallations
 
-let sessionIDUserDefaultsKey = "com.firebase.sessions.sessionID"
-let lastSessionIDUserDefaultsKey = "com.firebase.sessions.lastSessionID"
-
 protocol IdentifierProvider {
   var installationID: String {
     get
@@ -29,7 +26,7 @@ protocol IdentifierProvider {
     get
   }
 
-  var previousSessionID: String {
+  var previousSessionID: String? {
     get
   }
 }
@@ -44,23 +41,18 @@ protocol IdentifierProvider {
 class Identifiers: IdentifierProvider {
   private let installations: InstallationsProtocol
 
-  private var _sessionID: UUID
+  private var _sessionID: String?
+  private var _previousSessionID: String?
 
   init(installations: InstallationsProtocol) {
     self.installations = installations
-    _sessionID = UUID()
   }
 
   // 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() {
-    _sessionID = UUID()
-
-    let lastStoredSessionID = UserDefaults.standard.string(forKey: sessionIDUserDefaultsKey) ?? ""
-    UserDefaults.standard.set(lastStoredSessionID, forKey: lastSessionIDUserDefaultsKey)
-
-    let newSessionID = _sessionID.uuidString.replacingOccurrences(of: "-", with: "").lowercased()
-    UserDefaults.standard.set(newSessionID, forKey: sessionIDUserDefaultsKey)
+    _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
@@ -103,10 +95,14 @@ class Identifiers: IdentifierProvider {
   }
 
   var sessionID: String {
-    return UserDefaults.standard.string(forKey: sessionIDUserDefaultsKey) ?? ""
+    guard let _sessionID = _sessionID else {
+      Logger.logError("Error: Sessions SDK did not generate a Session ID")
+      return ""
+    }
+    return _sessionID
   }
 
-  var previousSessionID: String {
-    return UserDefaults.standard.string(forKey: lastSessionIDUserDefaultsKey) ?? ""
+  var previousSessionID: String? {
+    return _previousSessionID
   }
 }

FirebaseSessions/Sources/SessionStartEvent.swift

@@ -33,7 +33,7 @@ class SessionStartEvent: NSObject, GDTCOREventDataObject {
 
     proto.event_type = firebase_appquality_sessions_EventType_SESSION_START
     proto.session_data.session_id = makeProtoString(identifiers.sessionID)
-    proto.session_data.previous_session_id = makeProtoString(identifiers.previousSessionID)
+    proto.session_data.previous_session_id = makeProtoStringOrNil(identifiers.previousSessionID)
     proto.session_data.event_timestamp_us = time.timestampUS
 
     proto.application_info.app_id = makeProtoString(appInfo.appID)
@@ -70,6 +70,13 @@ class SessionStartEvent: NSObject, GDTCOREventDataObject {
 
   // MARK: - Data Conversion
 
+  private func makeProtoStringOrNil(_ string: String?) -> UnsafeMutablePointer<pb_bytes_array_t>? {
+    guard let string = string else {
+      return nil
+    }
+    return FIRSESEncodeString(string)
+  }
+
   private func makeProtoString(_ string: String) -> UnsafeMutablePointer<pb_bytes_array_t>? {
     return FIRSESEncodeString(string)
   }

FirebaseSessions/Tests/Unit/IdentifiersTests.swift

@@ -48,10 +48,18 @@ class IdentifiersTests: XCTestCase {
     return true
   }
 
+  // This test case isn't important behavior. When Crash and Perf integrate
+  // with the Sessions SDK, we may want to move to a lazy solution where
+  // sessionID can never be empty
+  func test_sessionID_beforeGenerateReturnsNothing() throws {
+    XCTAssert(identifiers.sessionID.count == 0)
+    XCTAssertNil(identifiers.previousSessionID)
+  }
+
   func test_generateNewSessionID_generatesValidID() throws {
     identifiers.generateNewSessionID()
     XCTAssert(isValidSessionID(identifiers.sessionID))
-    XCTAssert(identifiers.previousSessionID.count == 0)
+    XCTAssertNil(identifiers.previousSessionID)
   }
 
   /// Ensures that generating a Session ID multiple times results in the last Session ID being set in the previousSessionID field
@@ -60,15 +68,15 @@ class IdentifiersTests: XCTestCase {
 
     let firstSessionID = identifiers.sessionID
     XCTAssert(isValidSessionID(identifiers.sessionID))
-    XCTAssert(identifiers.previousSessionID.count == 0)
+    XCTAssertNil(identifiers.previousSessionID)
 
     identifiers.generateNewSessionID()
 
     XCTAssert(isValidSessionID(identifiers.sessionID))
-    XCTAssert(isValidSessionID(identifiers.previousSessionID))
+    XCTAssert(isValidSessionID(identifiers.previousSessionID!))
 
     // Ensure the new lastSessionID is equal to the sessionID from earlier
-    XCTAssert(identifiers.previousSessionID.compare(firstSessionID) == ComparisonResult.orderedSame)
+    XCTAssertEqual(identifiers.previousSessionID, firstSessionID)
   }
 
   // Fetching FIIDs requires that we are on a background thread.

FirebaseSessions/Tests/Unit/Mocks/MockIdentifierProvider.swift

@@ -20,7 +20,7 @@ import Foundation
 class MockIdentifierProvider: IdentifierProvider {
   var sessionID: String = ""
 
-  var previousSessionID: String = ""
+  var previousSessionID: String?
 
   var installationID: String = ""