Add Tests Against Sessions SDK and Internal API, Organize Errors (#10669)

Author: samedson

FirebaseSessions/Sources/FirebaseSessions.swift

@@ -32,11 +32,11 @@ private enum GoogleDataTransportConfig {
   private let appID: String
 
   /// Top-level Classes in the Sessions SDK
-  private let coordinator: SessionCoordinator
+  private let coordinator: SessionCoordinatorProtocol
   private let initiator: SessionInitiator
   private let sessionGenerator: SessionGenerator
-  private let appInfo: ApplicationInfo
-  private let settings: SessionsSettings
+  private let appInfo: ApplicationInfoProtocol
+  private let settings: SettingsProtocol
 
   /// Subscribers
   /// `subscribers` are used to determine the Data Collection state of the Sessions SDK.
@@ -82,12 +82,50 @@ private enum GoogleDataTransportConfig {
               coordinator: coordinator,
               initiator: initiator,
               appInfo: appInfo,
-              settings: settings)
+              settings: settings) { result in
+      switch result {
+      case .success(()):
+        Logger.logInfo("Successfully logged Session Start event")
+      case let .failure(sessionsError):
+        switch sessionsError {
+        case let .SessionInstallationsError(error):
+          Logger.logError(
+            "Error getting Firebase Installation ID: \(error). Skipping this Session Event"
+          )
+        case let .DataTransportError(error):
+          Logger
+            .logError(
+              "Error logging Session Start event to GoogleDataTransport: \(error)."
+            )
+        case .NoDependenciesError:
+          Logger
+            .logError(
+              "Sessions SDK did not have any dependent SDKs register as dependencies. Events will not be sent."
+            )
+        case .SessionSamplingError:
+          Logger
+            .logDebug(
+              "Sessions SDK has sampled this session"
+            )
+        case .DisabledViaSettingsError:
+          Logger
+            .logDebug(
+              "Sessions SDK is disabled via Settings"
+            )
+        case .DataCollectionError:
+          Logger
+            .logDebug(
+              "Data Collection is disabled for all subscribers. Skipping this Session Event"
+            )
+        }
+      }
+    }
   }
 
   // Initializes the SDK and begines the process of listening for lifecycle events and logging events
-  init(appID: String, sessionGenerator: SessionGenerator, coordinator: SessionCoordinator,
-       initiator: SessionInitiator, appInfo: ApplicationInfo, settings: SessionsSettings) {
+  init(appID: String, sessionGenerator: SessionGenerator, coordinator: SessionCoordinatorProtocol,
+       initiator: SessionInitiator, appInfo: ApplicationInfoProtocol, settings: SettingsProtocol,
+       loggedEventCallback: @escaping (Result<Void, FirebaseSessionsError>) -> Void) {
     self.appID = appID
 
     self.sessionGenerator = sessionGenerator
@@ -115,14 +153,18 @@ private enum GoogleDataTransportConfig {
 
       let event = SessionStartEvent(sessionInfo: sessionInfo, appInfo: self.appInfo)
 
+      // If there are no Dependencies, then the Sessions SDK can't acknowledge
+      // any products data collection state, so the Sessions SDK won't send events.
+      guard !self.subscriberPromises.isEmpty else {
+        loggedEventCallback(.failure(.NoDependenciesError))
+        return
+      }
+
       // Wait until all subscriber promises have been fulfilled before
       // doing any data collection.
       all(self.subscriberPromises.values).then(on: .global(qos: .background)) { _ in
         guard self.isAnyDataCollectionEnabled else {
-          Logger
-            .logDebug(
-              "Data Collection is disabled for all subscribers. Skipping this Session Event"
-            )
+          loggedEventCallback(.failure(.DataCollectionError))
           return
         }
 
@@ -136,15 +178,18 @@ private enum GoogleDataTransportConfig {
 
         self.addEventDataCollectionState(event: event)
 
-        if !(self.settings.sessionsEnabled && sessionInfo.shouldDispatchEvents) {
-          Logger
-            .logDebug(
-              "Session Event logging is disabled sessionsEnabled: \(self.settings.sessionsEnabled), shouldDispatchEvents: \(sessionInfo.shouldDispatchEvents)"
-            )
+        guard sessionInfo.shouldDispatchEvents else {
+          loggedEventCallback(.failure(.SessionSamplingError))
+          return
+        }
+
+        guard self.settings.sessionsEnabled else {
+          loggedEventCallback(.failure(.DisabledViaSettingsError))
           return
         }
 
         self.coordinator.attemptLoggingSessionStart(event: event) { result in
+          loggedEventCallback(result)
         }
       }
     }

FirebaseSessions/Sources/FirebaseSessionsError.swift

@@ -19,7 +19,15 @@ enum FirebaseSessionsError: Error {
   /// Event sampling related error
   case SessionSamplingError
   /// Firebase Installation ID related error
-  case SessionInstallationsError
-  /// Settings related error
-  case SettingsError(String)
+  case SessionInstallationsError(Error)
+  /// Error from the GoogleDataTransport SDK
+  case DataTransportError(Error)
+  /// Sessions SDK is disabled via settings error
+  case DisabledViaSettingsError
+  /// Sessions SDK is disabled because all Subscribers have their
+  /// data collection disabled
+  case DataCollectionError
+  /// Sessions SDK didn't have any Subscribers depend
+  /// on it via addDependency in SessionDependencies
+  case NoDependenciesError
 }

FirebaseSessions/Sources/SessionCoordinator.swift

@@ -14,11 +14,16 @@
 
 import Foundation
 
+protocol SessionCoordinatorProtocol {
+  func attemptLoggingSessionStart(event: SessionStartEvent,
+                                  callback: @escaping (Result<Void, FirebaseSessionsError>) -> Void)
+}
+
 ///
 /// SessionCoordinator is responsible for coordinating the systems in this SDK
 /// involved with sending a Session Start event.
 ///
-class SessionCoordinator {
+class SessionCoordinator: SessionCoordinatorProtocol {
   let installations: InstallationsProtocol
   let fireLogger: EventGDTLoggerProtocol
 
@@ -30,7 +35,8 @@ class SessionCoordinator {
 
   // 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) {
+                                  callback: @escaping (Result<Void, FirebaseSessionsError>)
+                                    -> 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.
@@ -42,22 +48,13 @@ class SessionCoordinator {
         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))
+            callback(.failure(FirebaseSessionsError.DataTransportError(error)))
           }
         }
       case let .failure(error):
-        Logger
-          .logError(
-            "Error getting Firebase Installation ID: \(error). Skip sending event."
-          )
-        callback(.failure(FirebaseSessionsError.SessionInstallationsError))
+        callback(.failure(FirebaseSessionsError.SessionInstallationsError(error)))
       }
     }
   }

FirebaseSessions/Sources/SessionGenerator.swift

@@ -37,9 +37,9 @@ struct SessionInfo {
 ///
 class SessionGenerator {
   private var thisSession: SessionInfo?
-  private var settings: SessionsSettings
+  private var settings: SettingsProtocol
 
-  init(settings: SessionsSettings) {
+  init(settings: SettingsProtocol) {
     self.settings = settings
   }
 

FirebaseSessions/Sources/SessionInitiator.swift

@@ -31,11 +31,11 @@ import Foundation
 ///
 class SessionInitiator {
   let currentTime: () -> Date
-  var settings: SessionsSettings
+  var settings: SettingsProtocol
   var backgroundTime = Date.distantFuture
   var initiateSessionStart: () -> Void = {}
 
-  init(settings: SessionsSettings, currentTimeProvider: @escaping () -> Date = Date.init) {
+  init(settings: SettingsProtocol, currentTimeProvider: @escaping () -> Date = Date.init) {
     currentTime = currentTimeProvider
     self.settings = settings
   }

FirebaseSessions/Sources/Settings/LocalOverrideSettings.swift

@@ -16,7 +16,7 @@
 import Foundation
 
 /// Class that manages the local overrides configs related to the library.
-class LocalOverrideSettings: SettingsProvider, SettingsProtocol {
+class LocalOverrideSettings: SettingsProvider {
   static let PlistKey_sessions_enabled = "FirebaseSessionsEnabled"
   static let PlistKey_sessions_timeout = "FirebaseSessionsTimeout"
   static let PlistKey_sessions_samplingRate = "FirebaseSessionsSampingRate"

FirebaseSessions/Sources/Settings/RemoteSettings.swift

@@ -20,7 +20,7 @@ extension ApplicationInfoProtocol {
   var synthesizedVersion: String { return "\(appDisplayVersion) (\(appBuildVersion))" }
 }
 
-class RemoteSettings: SettingsProvider, SettingsProtocol {
+class RemoteSettings: SettingsProvider {
   private static let cacheDurationSecondsDefault: TimeInterval = 60 * 60
   private static let flagSessionsEnabled = "sessions_enabled"
   private static let flagSamplingRate = "sampling_rate"

FirebaseSessions/Sources/Settings/SDKDefaultSettings.swift

@@ -16,7 +16,7 @@
 import Foundation
 
 /// Class that manages the local overrides configs related to the library.
-class SDKDefaultSettings: SettingsProvider, SettingsProtocol {
+class SDKDefaultSettings: SettingsProvider {
   var sessionsEnabled: Bool? {
     // Default is sessions enabled
     return true

FirebaseSessions/Sources/Settings/SessionsSettings.swift

@@ -16,7 +16,7 @@
 import Foundation
 
 /// Class that manages the configs related to the settings library
-class SessionsSettings {
+class SessionsSettings: SettingsProtocol {
   private let appInfo: ApplicationInfoProtocol
   private let installations: InstallationsProtocol
   private let sdkDefaults: SDKDefaultSettings

FirebaseSessions/Sources/Settings/SettingsDownloadClient.swift

@@ -22,7 +22,18 @@ import Foundation
 #endif // SWIFT_PACKAGE
 
 protocol SettingsDownloadClient {
-  func fetch(completion: @escaping (Result<[String: Any], Error>) -> Void)
+  func fetch(completion: @escaping (Result<[String: Any], SettingsDownloaderError>) -> Void)
+}
+
+enum SettingsDownloaderError: Error {
+  /// Error contructing the URL
+  case URLError(String)
+  /// Error from the URLSession task
+  case URLSessionError(String)
+  /// Error parsing the JSON response from Settings
+  case JSONParseError(String)
+  /// Error getting the Installation ID
+  case InstallationIDError(String)
 }
 
 class SettingsDownloader: SettingsDownloadClient {
@@ -34,9 +45,9 @@ class SettingsDownloader: SettingsDownloadClient {
     self.installations = installations
   }
 
-  func fetch(completion: @escaping (Result<[String: Any], Error>) -> Void) {
+  func fetch(completion: @escaping (Result<[String: Any], SettingsDownloaderError>) -> Void) {
     guard let validURL = url else {
-      completion(.failure(FirebaseSessionsError.SettingsError("Invalid URL")))
+      completion(.failure(.URLError("Invalid URL")))
       return
     }
 
@@ -49,17 +60,18 @@ class SettingsDownloader: SettingsDownloadClient {
             if let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
               completion(.success(dict))
             } else {
-              completion(.failure(FirebaseSessionsError
-                  .SettingsError("Failed to parse JSON to dictionary")))
+              completion(.failure(
+                .JSONParseError("Failed to parse JSON to dictionary")
+              ))
             }
           } else if let error = error {
-            completion(.failure(FirebaseSessionsError.SettingsError(error.localizedDescription)))
+            completion(.failure(.URLSessionError(error.localizedDescription)))
           }
         }
         // Start the task that sends the network request
         task.resume()
       case let .failure(error):
-        completion(.failure(FirebaseSessionsError.SettingsError(error.localizedDescription)))
+        completion(.failure(.InstallationIDError(error.localizedDescription)))
       }
     }
   }