Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pixels automatic naming prefixing fixed #810

Merged
merged 6 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions Sources/PixelKit/DebugEvent.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//
// DebugEvent.swift
Copy link
Member Author

@federicocappelli federicocappelli May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved in its own file

//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

/// Implementation of ``PixelKitEvent`` with specific logic for debug events.
public final class DebugEvent: PixelKitEvent {
public enum EventType {
case assertionFailure(message: String, file: StaticString, line: UInt)
case custom(_ event: PixelKitEvent)
}

public let eventType: EventType
public let error: Error?

public init(eventType: EventType, error: Error? = nil) {
self.eventType = eventType
self.error = error
}

public init(_ event: PixelKitEvent, error: Error? = nil) {
self.eventType = .custom(event)
self.error = error
}

public var name: String {
switch eventType {
case .assertionFailure:
return "assertion_failure"
case .custom(let event):
return event.name
}
}

public var parameters: [String: String]? {
var params: [String: String]

if case let .custom(event) = eventType,
let eventParams = event.parameters {
params = eventParams
} else {
params = [String: String]()
}

if let errorWithUserInfo = error as? ErrorWithPixelParameters {
params = errorWithUserInfo.errorParameters
}

if case let .assertionFailure(message, file, line) = eventType {
params[PixelKit.Parameters.assertionMessage] = message
params[PixelKit.Parameters.assertionFile] = String(file)
params[PixelKit.Parameters.assertionLine] = String(line)
}

if let error = error {
let nsError = error as NSError

params[PixelKit.Parameters.errorCode] = "\(nsError.code)"
params[PixelKit.Parameters.errorDomain] = nsError.domain

if let underlyingError = nsError.userInfo["NSUnderlyingError"] as? NSError {
params[PixelKit.Parameters.underlyingErrorCode] = "\(underlyingError.code)"
params[PixelKit.Parameters.underlyingErrorDomain] = underlyingError.domain
}

if let sqlErrorCode = nsError.userInfo["SQLiteResultCode"] as? NSNumber {
params[PixelKit.Parameters.underlyingErrorSQLiteCode] = "\(sqlErrorCode.intValue)"
}

if let sqlExtendedErrorCode = nsError.userInfo["SQLiteExtendedResultCode"] as? NSNumber {
params[PixelKit.Parameters.underlyingErrorSQLiteExtendedCode] = "\(sqlExtendedErrorCode.intValue)"
}
}

return params
}
}
41 changes: 41 additions & 0 deletions Sources/PixelKit/NonStandardEvent.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//
// NonStandardEvent.swift
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

/// This custom event is used for special cases, like pixels with non-standard names and uses, these pixels are sent as is and the names remain unchanged
public final class NonStandardEvent: PixelKitEventV2 {

let event: PixelKitEventV2

public init(_ event: PixelKitEventV2) {
self.event = event
}

public var name: String {
event.name
}

public var parameters: [String: String]? {
event.parameters
}

public var error: Error? {
event.error
}
}
23 changes: 19 additions & 4 deletions Sources/PixelKit/PixelKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ public final class PixelKit {
headers[Header.moreInfo] = "See " + Self.duckDuckGoMorePrivacyInfo.absoluteString
headers[Header.client] = "macOS"

// The event name can't contain `.`
reportErrorIf(pixel: pixelName, contains: ".")

switch frequency {
case .standard:
reportErrorIf(pixel: pixelName, endsWith: "_u")
Expand All @@ -204,6 +207,7 @@ public final class PixelKit {
reportErrorIf(pixel: pixelName, endsWith: "_d")
guard pixelName.hasSuffix("_u") else {
assertionFailure("Unique pixel: must end with _u")
onComplete(false, nil)
return
}
if !pixelHasBeenFiredEver(pixelName) {
Expand Down Expand Up @@ -253,6 +257,14 @@ public final class PixelKit {
}
}

/// If the pixel name contains the forbiddenString then an error is logged or an assertion failure is fired in debug
func reportErrorIf(pixel: String, contains forbiddenString: String) {
if pixel.contains(forbiddenString) {
logger.error("Pixel \(pixel, privacy: .public) must not contain \(forbiddenString, privacy: .public)")
assertionFailure("Pixel \(pixel) must not contain \(forbiddenString)")
}
}

private func printDebugInfo(pixelName: String, frequency: Frequency, parameters: [String: String], skipped: Bool = false) {
let params = parameters.filter { key, _ in !["test"].contains(key) }
logger.debug("👾[\(frequency.description, privacy: .public)-\(skipped ? "Skipped" : "Fired", privacy: .public)] \(pixelName, privacy: .public) \(params, privacy: .public)")
Expand All @@ -279,11 +291,14 @@ public final class PixelKit {

private func prefixedName(for event: Event) -> String {
if event.name.hasPrefix("m_mac_") {
// Can be a debug event or not, if already prefixed the name remains unchanged
return event.name
}

if let debugEvent = event as? DebugEvent {
} else if let debugEvent = event as? DebugEvent {
// Is a Debug event not already prefixed
return "m_mac_debug_\(debugEvent.name)"
} else if let nonStandardEvent = event as? NonStandardEvent {
// Special kind of pixel event that don't follow the standard naming conventions
return nonStandardEvent.name
} else {
return "m_mac_\(event.name)"
}
Expand Down Expand Up @@ -328,7 +343,7 @@ public final class PixelKit {
let error = event.error {

// For v2 events we only consider the error specified in the event
// and purposedly ignore the parameter in this call.
// and purposely ignore the parameter in this call.
// This is to encourage moving the error over to the protocol error
// instead of still relying on the parameter of this call.
newError = error
Expand Down
74 changes: 0 additions & 74 deletions Sources/PixelKit/PixelKitEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,77 +24,3 @@ public protocol PixelKitEvent {
var name: String { get }
var parameters: [String: String]? { get }
}

/// Implementation of ``PixelKitEvent`` with specific logic for debug events.
///
public final class DebugEvent: PixelKitEvent {
public enum EventType {
case assertionFailure(message: String, file: StaticString, line: UInt)
case custom(_ event: PixelKitEvent)
}

public let eventType: EventType
public let error: Error?

public init(eventType: EventType, error: Error? = nil) {
self.eventType = eventType
self.error = error
}

public init(_ event: PixelKitEvent, error: Error? = nil) {
self.eventType = .custom(event)
self.error = error
}

public var name: String {
switch eventType {
case .assertionFailure:
return "assertion_failure"
case .custom(let event):
return event.name
}
}

public var parameters: [String: String]? {
var params: [String: String]

if case let .custom(event) = eventType,
let eventParams = event.parameters {
params = eventParams
} else {
params = [String: String]()
}

if let errorWithUserInfo = error as? ErrorWithPixelParameters {
params = errorWithUserInfo.errorParameters
}

if case let .assertionFailure(message, file, line) = eventType {
params[PixelKit.Parameters.assertionMessage] = message
params[PixelKit.Parameters.assertionFile] = String(file)
params[PixelKit.Parameters.assertionLine] = String(line)
}

if let error = error {
let nsError = error as NSError

params[PixelKit.Parameters.errorCode] = "\(nsError.code)"
params[PixelKit.Parameters.errorDomain] = nsError.domain

if let underlyingError = nsError.userInfo["NSUnderlyingError"] as? NSError {
params[PixelKit.Parameters.underlyingErrorCode] = "\(underlyingError.code)"
params[PixelKit.Parameters.underlyingErrorDomain] = underlyingError.domain
}

if let sqlErrorCode = nsError.userInfo["SQLiteResultCode"] as? NSNumber {
params[PixelKit.Parameters.underlyingErrorSQLiteCode] = "\(sqlErrorCode.intValue)"
}

if let sqlExtendedErrorCode = nsError.userInfo["SQLiteExtendedResultCode"] as? NSNumber {
params[PixelKit.Parameters.underlyingErrorSQLiteExtendedCode] = "\(sqlExtendedErrorCode.intValue)"
}
}

return params
}
}
31 changes: 13 additions & 18 deletions Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,6 @@ public extension XCTestCase {
}
}

static var pixelPlatformPrefix: String {
#if os(macOS)
return "m_mac_"
#elseif os(iOS)
return "m_"
#endif
}

/// These parameters are known to be expected just based on the event definition.
///
/// They're not a complete list of parameters for the event, as the fire call may contain extra information
Expand Down Expand Up @@ -122,10 +114,14 @@ public extension XCTestCase {
let firedParameters = Self.filterStandardPixelParameters(from: firedParameters)

// Internal validations
XCTAssertTrue(expectedPixelNames.contains(firedPixelName), file: file, line: line)
var found = false
for expectedNameSuffix in expectedPixelNames where firedPixelName.hasSuffix(expectedNameSuffix) {
found = true
}
XCTAssertTrue(found, file: file, line: line)
XCTAssertTrue(knownExpectedParameters.allSatisfy { (key, value) in
firedParameters[key] == value
})
}, file: file, line: line)

if frequency == .dailyAndCount {
XCTAssertTrue(firedPixelName.hasPrefix(expectations.pixelName))
Expand All @@ -146,23 +142,22 @@ public extension XCTestCase {
}

func expectedPixelNames(originalName: String, frequency: PixelKit.Frequency) -> [String] {
let expectedPixelNameWithoutSuffix = originalName.hasPrefix(Self.pixelPlatformPrefix) ? originalName : Self.pixelPlatformPrefix + originalName
var expectedPixelNames: [String] = []

switch frequency {
case .standard:
expectedPixelNames.append(expectedPixelNameWithoutSuffix)
expectedPixelNames.append(originalName)
case .legacyInitial:
expectedPixelNames.append(expectedPixelNameWithoutSuffix)
expectedPixelNames.append(originalName)
case .unique:
expectedPixelNames.append(expectedPixelNameWithoutSuffix)
expectedPixelNames.append(originalName)
case .legacyDaily:
expectedPixelNames.append(expectedPixelNameWithoutSuffix)
expectedPixelNames.append(originalName)
case .daily:
expectedPixelNames.append(expectedPixelNameWithoutSuffix.appending("_d"))
expectedPixelNames.append(originalName.appending("_d"))
case .dailyAndCount:
expectedPixelNames.append(expectedPixelNameWithoutSuffix.appending("_d"))
expectedPixelNames.append(expectedPixelNameWithoutSuffix.appending("_c"))
expectedPixelNames.append(originalName.appending("_d"))
expectedPixelNames.append(originalName.appending("_c"))
}
return expectedPixelNames
}
Expand Down
Loading
Loading