Skip to content

Adding deferred Automatic event initialization for SwiftUI issue #682

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.

Sign up for GitHub

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

Draft
santigracia wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Adding deferred Automatic event initialization for SwiftUI issue #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8af8ae
add deferred automatic event initialization for SwiftUI issue
santigracia Oct 23, 2025
22b699f
Make trackAutomaticEventsEnabled thread-safe
santigracia Nov 26, 2025
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
146 changes: 146 additions & 0 deletions MixpanelDemo/MixpanelDemoTests/MixpanelAutomaticEventsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,150 @@ class MixpanelAutomaticEventsTests: MixpanelBaseTests {
removeDBfile(mp.apiToken)
removeDBfile(mp2.apiToken)
}

// MARK: - Tests for Deferred Automatic Event Initialization (SwiftUI Early Init Fix)

func testEnableAutomaticEventsAfterInitializationWithFalse() {
let testToken = randomId()
// Initialize with trackAutomaticEvents: false (simulating SwiftUI early init scenario)
let testMixpanel = Mixpanel.initialize(
token: testToken, trackAutomaticEvents: false, flushInterval: 60)
testMixpanel.minimumSessionDuration = 0

// At this point, no automatic events should be tracked
waitForTrackingQueue(testMixpanel)
XCTAssertTrue(
eventQueue(token: testMixpanel.apiToken).count == 0,
"No automatic events should be tracked when initialized with trackAutomaticEvents: false")

// Now enable automatic events (simulating the app UI being ready)
testMixpanel.trackAutomaticEventsEnabled = true
waitForTrackingQueue(testMixpanel)

// After enabling, first open event should be tracked
XCTAssertTrue(
eventQueue(token: testMixpanel.apiToken).count >= 1,
"First open event should be tracked after enabling automatic events")
let firstOpenEvent = eventQueue(token: testMixpanel.apiToken).first
XCTAssertEqual(
firstOpenEvent?["event"] as? String, "$ae_first_open",
"Should have first open event after re-initialization")

removeDBfile(testMixpanel.apiToken)
}

func testEnableAutomaticEventsMethodAfterInitializationWithFalse() {
let testToken = randomId()
let testMixpanel = Mixpanel.initialize(
token: testToken, trackAutomaticEvents: false, flushInterval: 60)
testMixpanel.minimumSessionDuration = 0

waitForTrackingQueue(testMixpanel)
XCTAssertTrue(
eventQueue(token: testMixpanel.apiToken).count == 0,
"No automatic events should be tracked initially")

// Use the explicit enableAutomaticEvents() method
testMixpanel.enableAutomaticEvents()
waitForTrackingQueue(testMixpanel)

// Verify automatic events are now enabled
XCTAssertTrue(
testMixpanel.trackAutomaticEventsEnabled,
"trackAutomaticEventsEnabled should be true after calling enableAutomaticEvents()")
XCTAssertTrue(
eventQueue(token: testMixpanel.apiToken).count >= 1,
"First open event should be tracked after calling enableAutomaticEvents()")

removeDBfile(testMixpanel.apiToken)
}

func testSessionTrackingAfterDeferredAutomaticEventsEnable() {
let testToken = randomId()
let testMixpanel = Mixpanel.initialize(
token: testToken, trackAutomaticEvents: false, flushInterval: 60)
testMixpanel.minimumSessionDuration = 0

// Enable automatic events after initial setup
testMixpanel.enableAutomaticEvents()
waitForTrackingQueue(testMixpanel)

// Clear previous events (first open)
let countBefore = eventQueue(token: testMixpanel.apiToken).count

// Simulate app resignation
testMixpanel.automaticEvents.perform(
#selector(AutomaticEvents.appWillResignActive(_:)),
with: Notification(name: Notification.Name(rawValue: "test")))
waitForTrackingQueue(testMixpanel)

// Session event should be tracked
let countAfter = eventQueue(token: testMixpanel.apiToken).count
XCTAssertTrue(
countAfter > countBefore,
"Session event should be tracked after enabling deferred automatic events")

let sessionEvent = eventQueue(token: testMixpanel.apiToken).last
XCTAssertEqual(
sessionEvent?["event"] as? String, "$ae_session",
"Should track session event after deferred enable")

removeDBfile(testMixpanel.apiToken)
}

func testMultipleDeferredEnablesAreIdempotent() {
let testToken = randomId()
let testMixpanel = Mixpanel.initialize(
token: testToken, trackAutomaticEvents: false, flushInterval: 60)
testMixpanel.minimumSessionDuration = 0

// Enable multiple times
testMixpanel.trackAutomaticEventsEnabled = true
waitForTrackingQueue(testMixpanel)
let countAfterFirstEnable = eventQueue(token: testMixpanel.apiToken).count

testMixpanel.trackAutomaticEventsEnabled = true
waitForTrackingQueue(testMixpanel)
let countAfterSecondEnable = eventQueue(token: testMixpanel.apiToken).count

testMixpanel.enableAutomaticEvents()
waitForTrackingQueue(testMixpanel)
let countAfterThirdEnable = eventQueue(token: testMixpanel.apiToken).count

// The first event count should remain the same despite multiple enables
XCTAssertEqual(
countAfterFirstEnable, countAfterSecondEnable,
"Second enable should not duplicate first open event")
XCTAssertEqual(
countAfterSecondEnable, countAfterThirdEnable,
"Third enable should not duplicate first open event")

removeDBfile(testMixpanel.apiToken)
}
Comment on lines +252 to +280
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

This test claims to verify idempotent behavior when enabling automatic events multiple times, but it only checks that the event count stays the same. It doesn't verify that NotificationCenter observers or SKPaymentQueue observers aren't duplicated.

Consider adding assertions to verify that notification handlers don't fire multiple times after multiple enables. For example, trigger a notification and verify the session event is only tracked once, not multiple times.

Copilot uses AI. Check for mistakes.

func testDisablingAndReEnablingAutomaticEvents() {
let testToken = randomId()
let testMixpanel = Mixpanel.initialize(
token: testToken, trackAutomaticEvents: true, flushInterval: 60)
testMixpanel.minimumSessionDuration = 0

waitForTrackingQueue(testMixpanel)
let initialEventCount = eventQueue(token: testMixpanel.apiToken).count
XCTAssertTrue(
initialEventCount > 0, "Should have first open event initially")

// Disable automatic events
testMixpanel.trackAutomaticEventsEnabled = false
XCTAssertFalse(
testMixpanel.trackAutomaticEventsEnabled,
"Should be disabled after setting to false")

// Re-enable automatic events
testMixpanel.trackAutomaticEventsEnabled = true
XCTAssertTrue(
testMixpanel.trackAutomaticEventsEnabled,
"Should be re-enabled after setting to true")

removeDBfile(testMixpanel.apiToken)
}
Comment on lines +282 to +306
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

This test only verifies that the property value changes when disabling/re-enabling automatic events, but doesn't test that automatic events actually stop being tracked when disabled. The implementation doesn't remove notification observers when trackAutomaticEventsEnabled is set to false, so events would continue to be tracked.

Consider adding assertions to verify that after disabling, no session events are tracked when notifications fire. For example:

// After disabling
testMixpanel.automaticEvents.perform(
  #selector(AutomaticEvents.appWillResignActive(_:)),
  with: Notification(name: Notification.Name(rawValue: "test")))
waitForTrackingQueue(testMixpanel)
let eventCountAfterDisable = eventQueue(token: testMixpanel.apiToken).count
XCTAssertEqual(eventCountAfterDisable, initialEventCount, "No new events should be tracked when disabled")

Copilot uses AI. Check for mistakes.
}
33 changes: 30 additions & 3 deletions Sources/MixpanelInstance.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,36 @@ open class MixpanelInstance: CustomDebugStringConvertible, FlushDelegate, AEDele
/// Controls whether to show spinning network activity indicator when flushing
/// data to the Mixpanel servers. Defaults to true.
open var showNetworkActivityIndicator = true

private var _trackAutomaticEventsEnabled: Bool
/// This allows enabling or disabling collecting common mobile events,
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The comment on line 118 is incomplete. It should describe what the property does.

Consider completing the documentation:

/// This allows enabling or disabling collecting common mobile events.
/// When set to true, automatically tracks events like first open, app updated, and session events.
/// Setting this to true after initialization will initialize automatic event tracking.

Copilot uses AI. Check for mistakes.
open var trackAutomaticEventsEnabled: Bool
open var trackAutomaticEventsEnabled: Bool {
get {
var value = false
readWriteLock.read {
value = _trackAutomaticEventsEnabled
}
return value
}
set {
readWriteLock.write {
_trackAutomaticEventsEnabled = newValue
}
#if os(iOS) || os(tvOS) || os(visionOS)
if newValue && !MixpanelInstance.isiOSAppExtension() {
// Re-initialize automatic event tracking if being enabled
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The setter does not handle the case when trackAutomaticEventsEnabled is set to false. There is no logic to stop tracking automatic events, remove notification observers, or remove the SKPaymentQueue observer. This means once automatic events are enabled, they cannot be properly disabled.

Suggested change
automaticEvents.initializeEvents(instanceName: self.name)
automaticEvents.initializeEvents(instanceName: self.name)
} else if !newValue {
// Disable automatic event tracking and remove observers
automaticEvents.stopEvents()
automaticEvents.delegate = nil

Copilot uses AI. Check for mistakes.
}
Comment on lines +133 to +136
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Calling initializeEvents() multiple times will register duplicate NotificationCenter observers and add multiple SKPaymentQueue observers. The AutomaticEvents.initializeEvents() method (lines 84-96 in AutomaticEvents.swift) calls addObserver without first removing existing observers, which will cause notification handlers to fire multiple times.

Consider adding a guard to prevent re-initialization, or removing observers before re-adding them. For example:

if newValue && !MixpanelInstance.isiOSAppExtension() {
  if !automaticEvents.hasAddedObserver {
    automaticEvents.delegate = self
    automaticEvents.initializeEvents(instanceName: self.name)
  }
}
Suggested change
// Re-initialize automatic event tracking if being enabled
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}
if !automaticEvents.hasAddedObserver {
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}

Copilot uses AI. Check for mistakes.
#endif
Comment on lines +130 to +137
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The re-initialization logic in the setter has a race condition. The readWriteLock.write block only protects the assignment to _trackAutomaticEventsEnabled, but the subsequent calls to automaticEvents.delegate = self and automaticEvents.initializeEvents(instanceName: self.name) happen outside the lock. This means multiple threads could concurrently execute the re-initialization code if they both read the new value as true before the write lock is released.

Consider moving the re-initialization logic inside the write lock:

readWriteLock.write {
  _trackAutomaticEventsEnabled = newValue
  #if os(iOS) || os(tvOS) || os(visionOS)
    if newValue && !MixpanelInstance.isiOSAppExtension() {
      automaticEvents.delegate = self
      automaticEvents.initializeEvents(instanceName: self.name)
    }
  #endif
}
Suggested change
}
#if os(iOS) || os(tvOS) || os(visionOS)
if newValue && !MixpanelInstance.isiOSAppExtension() {
// Re-initialize automatic event tracking if being enabled
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}
#endif
#if os(iOS) || os(tvOS) || os(visionOS)
if newValue && !MixpanelInstance.isiOSAppExtension() {
// Re-initialize automatic event tracking if being enabled
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}
#endif
}

Copilot uses AI. Check for mistakes.
}
}

#if os(iOS) || os(tvOS) || os(visionOS)
/// Enables automatic event tracking for the Mixpanel instance.
public func enableAutomaticEvents() {
self.trackAutomaticEventsEnabled = true
}
#endif

/// Flush timer's interval.
/// Setting a flush interval of 0 will turn off the flush timer and you need to call the flush() API manually
Expand Down Expand Up @@ -366,7 +393,7 @@ open class MixpanelInstance: CustomDebugStringConvertible, FlushDelegate, AEDele
if let apiToken = apiToken, !apiToken.isEmpty {
self.apiToken = apiToken
}
trackAutomaticEventsEnabled = trackAutomaticEvents
_trackAutomaticEventsEnabled = trackAutomaticEvents
if let serverURL = serverURL {
self.serverURL = serverURL
}
Expand Down
Loading