Merge pull request #203 from TelemetryDeck/feature/fix-reserved-keys

Avoid checking for reserved keys in internal signals being sent
TelemetryDeck · Nov 8, 2024 · 8c557f2 · 8c557f2
2 parents 06bd6c3 + 57a4ec0
commit 8c557f2
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 51 deletions.
diff --git a/Sources/TelemetryDeck/Presets/TelemetryDeck+Errors.swift b/Sources/TelemetryDeck/Presets/TelemetryDeck+Errors.swift
@@ -28,7 +28,7 @@ public extension TelemetryDeck {
             errorParameters["TelemetryDeck.Error.message"] = message
         }
 
-        self.signal(
+        self.internalSignal(
             "TelemetryDeck.Error.occurred",
             parameters: errorParameters.merging(parameters) { $1 },
             floatValue: floatValue,

diff --git a/Sources/TelemetryDeck/Presets/TelemetryDeck+Navigation.swift b/Sources/TelemetryDeck/Presets/TelemetryDeck+Navigation.swift
@@ -23,7 +23,7 @@ extension TelemetryDeck {
     public static func navigationPathChanged(from source: String, to destination: String, customUserID: String? = nil) {
         NavigationStatus.shared.previousNavigationPath = destination
 
-        self.signal(
+        self.internalSignal(
             "TelemetryDeck.Navigation.pathChanged",
             parameters: [
                 "TelemetryDeck.Navigation.schemaVersion": "1",

diff --git a/Sources/TelemetryDeck/Presets/TelemetryDeck+Purchases.swift b/Sources/TelemetryDeck/Presets/TelemetryDeck+Purchases.swift
@@ -73,7 +73,7 @@ extension TelemetryDeck {
             }
         }
 
-        self.signal(
+        self.internalSignal(
             "TelemetryDeck.Purchase.completed",
             parameters: purchaseParameters.merging(parameters) { $1 },
             floatValue: priceValueInUSD,

diff --git a/Sources/TelemetryDeck/Signals/SignalManager.swift b/Sources/TelemetryDeck/Signals/SignalManager.swift
@@ -18,22 +18,6 @@ protocol SignalManageable {
 final class SignalManager: SignalManageable, @unchecked Sendable {
     static let minimumSecondsToPassBetweenRequests: Double = 10
 
-    static let reservedKeysLowercased: Set<String> = Set(
-        [
-            "type", "clientUser", "appID", "sessionID", "floatValue",
-            "newSessionBegan", "platform", "systemVersion", "majorSystemVersion", "majorMinorSystemVersion", "appVersion", "buildNumber",
-            "isSimulator", "isDebug", "isTestFlight", "isAppStore", "modelName", "architecture", "operatingSystem", "targetEnvironment",
-            "locale", "region", "appLanguage", "preferredLanguage", "telemetryClientVersion",
-        ].map { $0.lowercased() }
-    )
-
-    static let internalSignalNames: Set<String> = [
-        "TelemetryDeck.Error.occurred",
-        "TelemetryDeck.Navigation.pathChanged",
-        "TelemetryDeck.Purchase.completed",
-        "TelemetryDeck.Session.started",
-    ]
-
     private var signalCache: SignalCache<SignalPostBody>
     let configuration: TelemetryManagerConfiguration
 
@@ -91,33 +75,6 @@ final class SignalManager: SignalManageable, @unchecked Sendable {
         customUserID: String?,
         configuration: TelemetryManagerConfiguration
     ) {
-        // warn users about reserved keys to avoid unexpected behavior
-        if signalName.lowercased().hasPrefix("telemetrydeck."), !Self.internalSignalNames.contains(signalName) {
-            configuration.logHandler?.log(
-                .error,
-                message: "Sending signal with reserved prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
-            )
-        } else if Self.reservedKeysLowercased.contains(signalName.lowercased()) {
-            configuration.logHandler?.log(
-                .error,
-                message: "Sending signal with reserved name '\(signalName)' will cause unexpected behavior. Please use another name instead."
-            )
-        }
-
-        for parameterKey in parameters.keys {
-            if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
-                configuration.logHandler?.log(
-                    .error,
-                    message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
-                )
-            } else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
-                configuration.logHandler?.log(
-                    .error,
-                    message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
-                )
-            }
-        }
-
         // enqueue signal to sending cache
         DispatchQueue.main.async {
             let defaultUserIdentifier = self.defaultUserIdentifier

diff --git a/Sources/TelemetryDeck/TelemetryClient.swift b/Sources/TelemetryDeck/TelemetryClient.swift
@@ -70,7 +70,7 @@ public struct TelemetryManagerConfiguration: Sendable {
     public var sessionID = UUID() {
         didSet {
             if sendNewSessionBeganSignal {
-                TelemetryDeck.signal("TelemetryDeck.Session.started")
+                TelemetryDeck.internalSignal("TelemetryDeck.Session.started")
             }
         }
     }

diff --git a/Sources/TelemetryDeck/TelemetryDeck.swift b/Sources/TelemetryDeck/TelemetryDeck.swift
@@ -5,6 +5,15 @@ public enum TelemetryDeck {
     /// This alias makes it easier to migrate the configuration type into the TelemetryDeck namespace in future versions when deprecated code is fully removed.
     public typealias Config = TelemetryManagerConfiguration
 
+    static let reservedKeysLowercased: Set<String> = Set(
+        [
+            "type", "clientUser", "appID", "sessionID", "floatValue",
+            "newSessionBegan", "platform", "systemVersion", "majorSystemVersion", "majorMinorSystemVersion", "appVersion", "buildNumber",
+            "isSimulator", "isDebug", "isTestFlight", "isAppStore", "modelName", "architecture", "operatingSystem", "targetEnvironment",
+            "locale", "region", "appLanguage", "preferredLanguage", "telemetryClientVersion",
+        ].map { $0.lowercased() }
+    )
+
     /// Initializes TelemetryDeck with a customizable configuration.
     ///
     /// - Parameter configuration: An instance of `Configuration` which includes all the settings required to configure TelemetryDeck.
@@ -37,12 +46,69 @@ public enum TelemetryDeck {
         guard !configuration.swiftUIPreviewMode, !configuration.analyticsDisabled else { return }
 
         let combinedSignalName = (configuration.defaultSignalPrefix ?? "") + signalName
-        let combinedParameters = configuration.defaultParameters()
-            .merging(parameters) { $1 }
-            .mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }
+        let prefixedParameters = parameters.mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }
+
+        // warn users about reserved keys to avoid unexpected behavior
+        if combinedSignalName.lowercased().hasPrefix("telemetrydeck.") {
+            configuration.logHandler?.log(
+                .error,
+                message: "Sending signal with reserved prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
+            )
+        } else if Self.reservedKeysLowercased.contains(combinedSignalName.lowercased()) {
+            configuration.logHandler?.log(
+                .error,
+                message: "Sending signal with reserved name '\(combinedSignalName)' will cause unexpected behavior. Please use another name instead."
+            )
+        }
+
+        // only check parameters (not default ones)
+        for parameterKey in prefixedParameters.keys {
+            if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
+                configuration.logHandler?.log(
+                    .error,
+                    message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
+                )
+            } else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
+                configuration.logHandler?.log(
+                    .error,
+                    message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
+                )
+            }
+        }
+
+        self.internalSignal(combinedSignalName, parameters: prefixedParameters, floatValue: floatValue, customUserID: customUserID)
+    }
+
+    /// A signal being sent without enriching the signal name with  a prefix. Also, any reserved signal name check are skipped. Only for internal use.
+    static func internalSignal(
+        _ signalName: String,
+        parameters: [String: String] = [:],
+        floatValue: Double? = nil,
+        customUserID: String? = nil
+    ) {
+        let manager = TelemetryManager.shared
+        let configuration = manager.configuration
+
+        let prefixedDefaultParameters = configuration.defaultParameters().mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }
+        let combinedParameters = prefixedDefaultParameters.merging(parameters) { $1 }
+
+        // check only default parameters
+        for parameterKey in prefixedDefaultParameters.keys {
+            if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
+                configuration.logHandler?.log(
+                    .error,
+                    message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
+                )
+            } else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
+                configuration.logHandler?.log(
+                    .error,
+                    message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
+                )
+            }
+        }
 
         manager.signalManager.processSignal(
-            combinedSignalName,
+            signalName,
             parameters: combinedParameters,
             floatValue: floatValue,
             customUserID: customUserID,