diff --git a/Sources/PixelKit/DebugEvent.swift b/Sources/PixelKit/DebugEvent.swift new file mode 100644 index 000000000..5aff63d17 --- /dev/null +++ b/Sources/PixelKit/DebugEvent.swift @@ -0,0 +1,92 @@ +// +// DebugEvent.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 + +/// 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 + } +} diff --git a/Sources/PixelKit/NonStandardEvent.swift b/Sources/PixelKit/NonStandardEvent.swift new file mode 100644 index 000000000..274b68e64 --- /dev/null +++ b/Sources/PixelKit/NonStandardEvent.swift @@ -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 + } +} diff --git a/Sources/PixelKit/PixelKit.swift b/Sources/PixelKit/PixelKit.swift index d647b2883..6af7381c1 100644 --- a/Sources/PixelKit/PixelKit.swift +++ b/Sources/PixelKit/PixelKit.swift @@ -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") @@ -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) { @@ -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)") @@ -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)" } @@ -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 diff --git a/Sources/PixelKit/PixelKitEvent.swift b/Sources/PixelKit/PixelKitEvent.swift index ca352f334..04573f0e7 100644 --- a/Sources/PixelKit/PixelKitEvent.swift +++ b/Sources/PixelKit/PixelKitEvent.swift @@ -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 - } -} diff --git a/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift b/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift index 78766b559..c12e95213 100644 --- a/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift +++ b/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift @@ -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 @@ -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)) @@ -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 } diff --git a/Tests/PixelKitTests/PixelKitTests.swift b/Tests/PixelKitTests/PixelKitTests.swift index cf51b5031..51d1834b7 100644 --- a/Tests/PixelKitTests/PixelKitTests.swift +++ b/Tests/PixelKitTests/PixelKitTests.swift @@ -27,23 +27,38 @@ final class PixelKitTests: XCTestCase { } /// Test events for convenience - /// + private enum TestEvent: String, PixelKitEvent { + + case testEventPrefixed = "m_mac_testEventPrefixed" + case testEvent + + var name: String { + return rawValue + } + + var parameters: [String: String]? { + return nil + } + + var error: Error? { + return nil + } + } + + private enum TestEventV2: String, PixelKitEventV2 { + case testEvent case testEventWithoutParameters case dailyEvent case dailyEventWithoutParameters case dailyAndContinuousEvent case dailyAndContinuousEventWithoutParameters - case uniqueEvent + case uniqueEvent = "uniqueEvent_u" + case nameWithDot = "test.pixel.with.dot" var name: String { - switch self { - case .uniqueEvent: - return "\(rawValue)_u" - default: - return rawValue - } + return rawValue } var parameters: [String: String]? { @@ -53,14 +68,18 @@ final class PixelKitTests: XCTestCase { "eventParam1": "eventParamValue1", "eventParam2": "eventParamValue2" ] - case .testEventWithoutParameters, .dailyEventWithoutParameters, .dailyAndContinuousEventWithoutParameters: + default: return nil } } + var error: Error? { + return nil + } + var frequency: PixelKit.Frequency { switch self { - case .testEvent, .testEventWithoutParameters: + case .testEvent, .testEventWithoutParameters, .nameWithDot: return .standard case .uniqueEvent: return .unique @@ -83,7 +102,95 @@ final class PixelKitTests: XCTestCase { XCTFail("This callback should not be executed when doing a dry run") } - pixelKit.fire(TestEvent.testEvent) + pixelKit.fire(TestEventV2.testEvent) + } + + func testNonStandardEvent() { + func testReportBrokenSitePixel() { + fire(NonStandardEvent(TestEventV2.testEvent), + frequency: .standard, + and: .expect(pixelName: TestEventV2.testEvent.name), + file: #filePath, + line: #line) + } + } + + func testDebugEventPrefixed() { + let appVersion = "1.0.5" + let headers = ["a": "2", "b": "3", "c": "2000"] + let event = DebugEvent(TestEvent.testEventPrefixed) + let userDefaults = userDefaults() + + // Set expectations + let expectedPixelName = TestEvent.testEventPrefixed.name + let fireCallbackCalled = expectation(description: "Expect the pixel firing callback to be called") + + // Prepare mock to validate expectations + let pixelKit = PixelKit(dryRun: false, + appVersion: appVersion, + defaultHeaders: headers, + dailyPixelCalendar: nil, + defaults: userDefaults) { firedPixelName, firedHeaders, parameters, _, _, _ in + + fireCallbackCalled.fulfill() + XCTAssertEqual(expectedPixelName, firedPixelName) + } + // Run test + pixelKit.fire(event) + // Wait for expectations to be fulfilled + wait(for: [fireCallbackCalled], timeout: 0.5) + } + + func testDebugEventNotPrefixed() { + let appVersion = "1.0.5" + let headers = ["a": "2", "b": "3", "c": "2000"] + let event = DebugEvent(TestEvent.testEvent) + let userDefaults = userDefaults() + + // Set expectations + let expectedPixelName = "m_mac_debug_\(TestEvent.testEvent.name)" + let fireCallbackCalled = expectation(description: "Expect the pixel firing callback to be called") + + // Prepare mock to validate expectations + let pixelKit = PixelKit(dryRun: false, + appVersion: appVersion, + defaultHeaders: headers, + dailyPixelCalendar: nil, + defaults: userDefaults) { firedPixelName, firedHeaders, parameters, _, _, _ in + + fireCallbackCalled.fulfill() + XCTAssertEqual(expectedPixelName, firedPixelName) + } + // Run test + pixelKit.fire(event) + // Wait for expectations to be fulfilled + wait(for: [fireCallbackCalled], timeout: 0.5) + } + + func testDebugEventDaily() { + let appVersion = "1.0.5" + let headers = ["a": "2", "b": "3", "c": "2000"] + let event = DebugEvent(TestEvent.testEvent) + let userDefaults = userDefaults() + + // Set expectations + let expectedPixelName = "m_mac_debug_\(TestEvent.testEvent.name)_d" + let fireCallbackCalled = expectation(description: "Expect the pixel firing callback to be called") + + // Prepare mock to validate expectations + let pixelKit = PixelKit(dryRun: false, + appVersion: appVersion, + defaultHeaders: headers, + dailyPixelCalendar: nil, + defaults: userDefaults) { firedPixelName, firedHeaders, parameters, _, _, _ in + + fireCallbackCalled.fulfill() + XCTAssertEqual(expectedPixelName, firedPixelName) + } + // Run test + pixelKit.fire(event, frequency: .daily) + // Wait for expectations to be fulfilled + wait(for: [fireCallbackCalled], timeout: 0.5) } /// Tests firing a sample pixel and ensuring that all fields are properly set in the fire request callback. @@ -92,7 +199,7 @@ final class PixelKitTests: XCTestCase { // Prepare test parameters let appVersion = "1.0.5" let headers = ["a": "2", "b": "3", "c": "2000"] - let event = TestEvent.testEvent + let event = TestEventV2.testEvent let userDefaults = userDefaults() // Set expectations @@ -136,7 +243,7 @@ final class PixelKitTests: XCTestCase { // Prepare test parameters let appVersion = "1.0.5" let headers = ["a": "2", "b": "3", "c": "2000"] - let event = TestEvent.dailyEvent + let event = TestEventV2.dailyEvent let userDefaults = userDefaults() // Set expectations @@ -180,7 +287,7 @@ final class PixelKitTests: XCTestCase { // Prepare test parameters let appVersion = "1.0.5" let headers = ["a": "2", "b": "3", "c": "2000"] - let event = TestEvent.dailyEvent + let event = TestEventV2.dailyEvent let userDefaults = userDefaults() // Set expectations @@ -226,7 +333,7 @@ final class PixelKitTests: XCTestCase { // Prepare test parameters let appVersion = "1.0.5" let headers = ["a": "2", "b": "3", "c": "2000"] - let event = TestEvent.dailyEvent + let event = TestEventV2.dailyEvent let userDefaults = userDefaults() let timeMachine = TimeMachine() @@ -270,7 +377,7 @@ final class PixelKitTests: XCTestCase { // Prepare test parameters let appVersion = "1.0.5" let headers = ["a": "2", "b": "3", "c": "2000"] - let event = TestEvent.uniqueEvent + let event = TestEventV2.uniqueEvent let userDefaults = userDefaults() let timeMachine = TimeMachine()