From 57a4ec02e2fbe1af5ac2a58c06de895d9cd2fcd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cihat=20Gu=CC=88ndu=CC=88z?= Date: Fri, 8 Nov 2024 19:26:38 +0100 Subject: [PATCH] Avoid checking for reserved keys in internal signals being sent --- .../Presets/TelemetryDeck+Errors.swift | 2 +- .../Presets/TelemetryDeck+Navigation.swift | 2 +- .../Presets/TelemetryDeck+Purchases.swift | 2 +- .../TelemetryDeck/Signals/SignalManager.swift | 43 ----------- Sources/TelemetryDeck/TelemetryClient.swift | 2 +- Sources/TelemetryDeck/TelemetryDeck.swift | 74 ++++++++++++++++++- 6 files changed, 74 insertions(+), 51 deletions(-) diff --git a/Sources/TelemetryDeck/Presets/TelemetryDeck+Errors.swift b/Sources/TelemetryDeck/Presets/TelemetryDeck+Errors.swift index 5029436..6e985f5 100644 --- 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 index ca97ae1..b28cb8f 100644 --- 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 index 3717265..5fb4bb7 100644 --- 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 index e559acd..2dad01f 100644 --- 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 = 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 = [ - "TelemetryDeck.Error.occurred", - "TelemetryDeck.Navigation.pathChanged", - "TelemetryDeck.Purchase.completed", - "TelemetryDeck.Session.started", - ] - private var signalCache: SignalCache 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 index 70591e1..18479a3 100644 --- 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 index 75a1506..ab91cf7 100644 --- 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 = 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,