feat(lifecycle): wire emitter into AnalyticsClient + openURL API [3/4]#44
Conversation
e072bd0 to
aaac4fc
Compare
f7d52fe to
eca2233
Compare
| await self.dispatcher.stopFlushLoop() | ||
| await self.dispatcher.cancelScheduledRetry() | ||
| } | ||
| onForeground: { [weak self] in self?.handleForeground() }, |
There was a problem hiding this comment.
just cleaning up these closures as they were getting quite long
|
|
||
| public func openURL(_ url: URL, sourceApplication: String?) { | ||
| guard let coordinator = lifecycleCoordinator else { | ||
| Logger.warn("openURL called but trackLifecycleEvents is disabled — ignoring") |
There was a problem hiding this comment.
for the time being this method is a noop if there is no lifecycle coordinator and trackLifecycleEvents is false.
| case .setAdvertisingId(let advertisingId): r.setAdvertisingId(advertisingId) | ||
| case .clearAdvertisingId: r.clearAdvertisingId() | ||
| case .setTracing(let enabled): r.setTracing(enabled) | ||
| case .openURL(let url, let source): r.openURL(url, sourceApplication: source) |
There was a problem hiding this comment.
passing through proxy
| libraryName: String = "metarouter-ios-sdk", | ||
| libraryVersion: String = MetaRouterSDK.version | ||
| libraryVersion: String = MetaRouterSDK.version, | ||
| appContext: AppContext = .fromBundle() |
There was a problem hiding this comment.
since. we are using the AppContext in event context and in the LifeCycleCoordinator wanted to expose here and read from both places.
| @@ -0,0 +1,67 @@ | |||
| import Foundation | |||
|
|
|||
| /// Bridges `AnalyticsClient`'s init / foreground / background / deep-link callbacks | |||
There was a problem hiding this comment.
wraps calls to the Emitter in previous PR
aaac4fc to
7052d4a
Compare
Slice 3 of 4 of the iOS lifecycle events feature (parent: sc-36764). Connects the emitter (slice 2) to AnalyticsClient via a new LifecycleCoordinator and ships the public openURL deep-link API. After this PR, the feature works end-to-end behind trackLifecycleEvents=true. Default stays false so no existing customer is impacted on upgrade. LifecycleCoordinator: - Wraps the emitter, owns the cold-launch UIKit probe (single #if canImport(UIKit) block), exposes onForeground / onBackground / onReady / openURL. - Single seam between the platform-notification observer and the emitter; UIApplication access stays out of AnalyticsClient. AnalyticsClient integration: - Snapshots AppContext.fromBundle() once at init; injects into both DeviceContextProvider (replacing per-event Bundle reads) and LifecycleEventEmitter. - Conditionally constructs LifecycleCoordinator when trackLifecycleEvents == true. - Named handleForeground() / handleBackground() async methods replace inline closures so the load-bearing emit-before-flush ordering rule lives next to the code it describes. - coordinator.onReady() fires inside initTask after .ready, before drainDiskStoreToNetwork, so cold-launch events ship in the same drain. Public API: - openURL(_ url: URL, sourceApplication: String?) on AnalyticsInterface, AnalyticsClient, and AnalyticsProxy. First param positional — mirrors UIApplication.application(_:open:) and Segment's API. - Logs Logger.warn when called while feature disabled (silent no-op was bad DX). - AnalyticsProxy buffers openURL identical to other proxied methods. Opt-in default: - InitOptions.trackLifecycleEvents: Bool = false. - InitOptionsTests cover default + explicit override. Test seams: - AnalyticsDependencies gains appContext, lifecycleStorage, identityStorage, initialAppState for integration test injection. - AppLifecycleEventIntegrationTests drives a real AnalyticsClient via DI, posts UIApplication/NSApplication notifications through NotificationCenter, asserts events flow through queue + network. Covers: cold launch end-to-end, flag-disabled path emits zero events, reset() preserves lifecycle storage, re-init same version emits only Opened, background notification triggers Application Backgrounded, foreground after background emits Opened with from_background:true. Ticket: sc-38230 Parent: sc-36764 Stack: sc-38228 -> sc-38229 -> this PR -> sc-38231
Code review follow-ups (sc-38230).
- Add openURL round-trip test through AnalyticsProxy: forward-when-bound
and queued-before-bind-replayed-in-order paths. Closes the test gap
the reviewer flagged where the new openURL Call enum case had no
dedicated coverage even though the dispatch is mechanical.
- Add testOpenURLWithFeatureDisabledLogsWarning in
AppLifecycleEventIntegrationTests. Verifies the Logger.warn line
fires when openURL is called while trackLifecycleEvents=false, so a
silent-no-op misconfiguration is diagnosable from logs.
- Hoist captureStderrAndStdout from InitOptionsTests (private) to
TestHelpers (file-level). Add an async variant with a `settle`
parameter for tests where the block under test fires fire-and-forget
Tasks (the openURL flow does this — Task { ... } around the actor
call).
- Track tempDir on Setup and remove it in deinit so per-test integration
tmp dirs don't accumulate in /var/folders/.../T.
Sleep migration in integration tests deferred — would require adding
a non-draining count API to PersistentEventQueue, which is a production
surface change that the reviewer flagged as low priority. Tests pass
reliably as-is.
Test count goes 446 -> 454 (+5 emitter tests from slice 2's amend,
+2 proxy tests, +1 warn-capture test).
eca2233 to
eac8bb2
Compare
brandon-metarouter
left a comment
There was a problem hiding this comment.
Few minor comments, nothing blocking.
| guard let self else { return } | ||
| await self.dispatcher.startFlushLoop(intervalSeconds: self.options.flushIntervalSeconds) | ||
| await self.dispatcher.flush() | ||
| await self.lifecycleCoordinator?.onForeground() |
There was a problem hiding this comment.
Feel free to disregard, just looking at from a consistency POV. Any value to to having self.lifecycleCoordinator?.onForeground() before dispatcher flushes? If not, maybe have a comment similar to what you have for handleBackground to give context on flush then onForeground .
(EX: If there are a lot of events pilled up, wasn't sure if delaying onForeground would cause any issues)
There was a problem hiding this comment.
Yes I added a comment here clarifying that we are draining before Application Opened so that we start with a clean state / queue.
| func openURL(_ url: URL, sourceApplication: String?) async { | ||
| await emitter.openURL( | ||
| url: url.absoluteString, | ||
| sourceApplication: sourceApplication | ||
| ) | ||
| } |
There was a problem hiding this comment.
okay, I now I see how openURL is wired through! You can disregard that comment in my previous PR review 😅
| XCTAssertEqual(bundle.lifecycleStorage.getVersion(), "1.5.0") | ||
|
|
||
| bundle.client.reset() | ||
| try? await Task.sleep(nanoseconds: 300_000_000) |
There was a problem hiding this comment.
Claude is calling these out: "should these (longer) sleeps use await TestUtilities.waitFor(?
| object: nil | ||
| ) | ||
| } | ||
| try? await Task.sleep(nanoseconds: 500_000_000) |
There was a problem hiding this comment.
Same here, "should these (longer) sleeps use await TestUtilities.waitFor(?
* feat: add MetaRouter.Analytics.shared, deprecate client() Closes sc-38240. Add a property-style accessor `MetaRouter.Analytics.shared` that returns the same buffered proxy as `client()`. Mark `client()` as deprecated with `@available(*, deprecated, renamed: "shared")` so existing callers get a one-line fix-it warning. Why: - Idiomatic Swift convention. Apple's framework singletons are all property accessors: URLSession.shared, UserDefaults.standard, FileManager.default, NotificationCenter.default. iOS engineers reach for `.shared` first by muscle memory. - Surfaced during the lifecycle MR (sc-36764) review when README snippets defaulted to `MetaRouter.Analytics.shared.openURL(...)`, which didn't compile because `shared` didn't exist. The fix landed as `client()` for consistency, but `shared` is the right end state. - Reduces friction for Segment iOS migrators (Segment uses Analytics.shared at the call-site level). Migration: - Soft deprecation in this minor release. `client()` keeps working but emits a yellow warning at every call site, with auto-fix-it pointing at `shared`. - v2.0 will remove `client()` entirely (separate ticket when major version bump lands). Internal call sites migrated in this PR: - Tests/MetaRouterTests/MetaRouterTests.swift (2 sites) - Tests/MetaRouterTests/MetaRouterIntegrationTests.swift (6 sites) No README changes — the lifecycle MR (sc-36764) currently uses `client()` consistently in its in-flight slice 4 README; that PR will update to `.shared` after this lands as a follow-up. Behavior change: none. Both accessors return the same proxy instance. 418 tests pass; no new deprecation warnings (all internal callers migrated). * docs: add MetaRouter.Analytics.shared to API Reference Surface the new property-style accessor in the README's API Reference section so it's discoverable. Adds a short subsection between initialize(with:) and Analytics Interface that: - Explains the convention (matches URLSession.shared / UserDefaults .standard / FileManager.default) - Shows the typical usage pattern (initialize once, call .shared anywhere) - Notes that calls before initialize are buffered identically to the proxy returned from initialize(with:) - Calls out that client() is now deprecated and points users at .shared No new TOC entry — matches the existing convention of listing only top-level sections in the TOC, not sub-sections.
* feat(lifecycle): storage + bundle metadata foundation Slice 1 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the storage layer and shared types that the upcoming emitter / wiring slices depend on. Pure additions — no behavior change to AnalyticsClient, no events emitted yet. - LifecycleStorage: UserDefaults wrapper for (version, build) under the metarouter:lifecycle:* namespace, separate from identity keys so reset() cannot wipe install/update state. - IdentityStorage.hasAnyValue(): helper for the emitter (slice 2) to snapshot identity presence at construction time, before IdentityManager.initialize() auto-creates an anonymousId. - AppContext: gains Equatable + fromBundle(_:) — single source of truth for app metadata, replacing the per-event Bundle.main.infoDictionary reads that DeviceContextProvider does today (consumed in slice 3). - AppForegroundState enum: platform-neutral active/inactive/background trichotomy used by the emitter and coordinator. Ticket: sc-38228 Parent: sc-36764 Stack: this PR -> sc-38229 -> sc-38230 -> sc-38231 * refactor(lifecycle): mark LifecycleStorage.clear() as internal Code review follow-up (sc-38228 M1). The `clear()` method is a test-only seam, not part of the SDK's public contract. Marking it `public` would expose a "wipe install/update state" affordance to consumers that contradicts the entire rationale for the `metarouter:lifecycle:*` namespace separation: nothing — not even `reset()` — should be able to wipe this state. `@testable import MetaRouter` already gives test code access to `internal` symbols, so this is purely a tightening of the public surface with no functional change. * fix: fixing review comments
* feat(lifecycle): storage + bundle metadata foundation
Slice 1 of 4 of the iOS lifecycle events feature (parent: sc-36764).
Adds the storage layer and shared types that the upcoming emitter / wiring
slices depend on. Pure additions — no behavior change to AnalyticsClient,
no events emitted yet.
- LifecycleStorage: UserDefaults wrapper for (version, build) under the
metarouter:lifecycle:* namespace, separate from identity keys so reset()
cannot wipe install/update state.
- IdentityStorage.hasAnyValue(): helper for the emitter (slice 2) to
snapshot identity presence at construction time, before
IdentityManager.initialize() auto-creates an anonymousId.
- AppContext: gains Equatable + fromBundle(_:) — single source of truth
for app metadata, replacing the per-event Bundle.main.infoDictionary
reads that DeviceContextProvider does today (consumed in slice 3).
- AppForegroundState enum: platform-neutral active/inactive/background
trichotomy used by the emitter and coordinator.
Ticket: sc-38228
Parent: sc-36764
Stack: this PR -> sc-38229 -> sc-38230 -> sc-38231
* refactor(lifecycle): mark LifecycleStorage.clear() as internal
Code review follow-up (sc-38228 M1).
The `clear()` method is a test-only seam, not part of the SDK's public
contract. Marking it `public` would expose a "wipe install/update state"
affordance to consumers that contradicts the entire rationale for the
`metarouter:lifecycle:*` namespace separation: nothing — not even
`reset()` — should be able to wipe this state.
`@testable import MetaRouter` already gives test code access to
`internal` symbols, so this is purely a tightening of the public surface
with no functional change.
* feat(lifecycle): LifecycleEventEmitter actor + unit tests
Slice 2 of 4 of the iOS lifecycle events feature (parent: sc-36764).
Adds the lifecycle event emitter — the actor that owns install/update
detection, the cold-launch state machine, foreground/background
transitions, and the one-shot deep-link buffer. Standalone in this
slice; not yet wired into AnalyticsClient.
The emitter exposes four entrypoints:
- emitColdLaunchSequence(initialAppState:): decides Installed vs Updated
vs no-op based on persisted (version, build) and identity-existed-
before-init snapshot, then conditionally emits Application Opened
with from_background:false (suppressed for background-launched
processes; the next true foreground entry emits the deferred Opened
via the cold-launch bridge).
- emitForegroundFromBackground(): handles bridge case, suppresses the
first didBecomeActive during init, filters inactive→active, and emits
Application Opened with from_background:true on real
background→active transitions.
- emitBackgrounded(): updates lastTrackedAppState and emits Application
Backgrounded.
- openURL(url:sourceApplication:): one-shot buffer (last-write-wins),
consumed by the next emitOpened.
State machine flags (coldLaunchEmitted, coldLaunchSuppressed,
lastTrackedAppState, pendingDeepLink) are serialised through actor
isolation.
22 unit tests cover the install/update decision tree, cold-launch
sequencing, resume scenarios, deep-link buffer semantics, and double-
emit suppression.
Ticket: sc-38229
Parent: sc-36764
Stack: sc-38228 -> this PR -> sc-38230 -> sc-38231
* fix(lifecycle): emitter race guard + buffer log lines + missing tests
Code review follow-ups (sc-38229).
- Guard emitBackgrounded against firing before the cold-launch sequence
runs. Race scenario: process woken in .background, observer already
registered, didEnterBackground arrives before the async
initTask → onReady chain. Without the guard, we'd emit Backgrounded
with no preceding Opened — a spec violation.
- Add log line on the cold-launch bridge emit (suppressed cold launch
→ first true foreground entry). This path is rare and notoriously
hard to diagnose in field reports; one log line earns its keep.
- Add log line when openURL overwrites an existing pending deep link
(last-write-wins), making field debugging of "why did Opened carry
a different URL than I expected" tractable.
- Drop the unnecessary lastTrackedAppState = .active write on the
pre-cold-launch suppressed branch. The cold-launch path is the
canonical source of truth for this flag (set in
emitColdLaunchSequence); writing it during the suppression window
is a no-op at best and hides intent at worst.
- Add 5 missing test branches:
- Cold launch with initialAppState == .inactive (distinct from
.background) suppresses + bridges
- Deep-link buffered before a suppressed cold launch survives the
suppression and attaches to the bridge Opened
- emitBackgrounded does NOT consume the deep-link buffer (one-shot
per Application Opened, not per any-emit)
- Same-bundle no-op cold launch still persists (version, build) to
storage (regression guard against accidental skip)
- Backgrounded before cold launch is suppressed (the new guard)
Test count goes from 438 -> 443. All passing.
* fix: review comments rename recordUrl
Ticket
sc-38230 — iOS Lifecycle PR 3: AnalyticsClient wiring + openURL public API
Parent: sc-36764
Base: #43 (slice 2) — review/merge that first
Summary
Slice 3 of 4 — the integration. Connects the emitter from #43 to
AnalyticsClientvia a newLifecycleCoordinator, ships the publicopenURLdeep-link API, and adds the opt-in flag. After this PR merges, the feature works end-to-end when enabled. Default staysfalseso existing customers aren't surprised.Depends on
What's in
Architecture
Sources/MetaRouter/lifecycle/LifecycleCoordinator.swift(new) — bridgesAnalyticsClient's init / foreground / background / deep-link callbacks to the emitter. Owns the cold-launchUIApplication.applicationStateprobe (single#if canImport(UIKit)block at the bottom of the file). Single seam between the platform-notification observer and the emitter; UIKit access stays out ofAnalyticsClient.AnalyticsClientwiringAppContext.fromBundle()exactly once at init; injects the same instance into bothDeviceContextProvider(replacing per-eventBundle.main.infoDictionaryreads) andLifecycleEventEmitter.LifecycleCoordinatorwhentrackLifecycleEvents == true. OtherwiselifecycleCoordinator == niland all coordinator calls are optional-chained no-ops.handleForeground()/handleBackground() asyncmethods so the load-bearing emit-before-flush ordering rule lives next to the code it describes (not buried in a closure body).coordinator.onReady()fires insideinitTaskafterlifecycleState = .ready, beforedrainDiskStoreToNetwork, so cold-launch events ship in the same drain as any persisted events from the previous session.Public API
AnalyticsInterface.openURL(_ url: URL, sourceApplication: String?)— first param positional, mirrorsUIApplication.application(_:open:)and Segment's API.AnalyticsProxy.openURL— pre-bind buffering identical to other proxied methods (capped queue, replay in order on bind).AnalyticsClient.openURL— forwards to coordinator. LogsLogger.warnif called whiletrackLifecycleEventsisfalse(silent no-op is bad DX).Opt-in flag
InitOptions.trackLifecycleEvents: Bool = falsein both URL and String initialisers.InitOptionsTestscovers default + explicit override.Test seams + integration coverage
AnalyticsDependenciesgainsappContext,lifecycleStorage,identityStorage,initialAppStatefor test injection (the integration test pre-seedsUserDefaultsand pipes into both lifecycle and identity storage so test and SDK see the same state).Tests/MetaRouterTests/AppLifecycleEventIntegrationTests.swift(new) — drives a realAnalyticsClientvia DI:Application InstalledthenApplication Openedvia the standard track pathtrack()still worksreset()preserves lifecycle storage; re-init with same(version, build)emits onlyApplication OpenedUIApplication.didEnterBackgroundNotificationenqueuesApplication BackgroundedApplication Opened {from_background: true}Tests/MetaRouterTests/Helpers/TestHelpers.swift— addsopenURLtoMockAnalyticsInterface+AnalyticsCallenum.DeviceContextProviderAppContextvia init param.collectAppContext()returns the cached snapshot directly — no per-event bundle reads.What's not in
Test plan
swift buildclean.swift test— 446 tests pass, 0 failures (full feature now exercised end-to-end).swift test --filter Lifecycle— 28 lifecycle-specific tests pass.swift test --filter InitOptions— opt-in flag covered.import UIKitinAnalyticsClient.swift— UIKit access localised toLifecycleCoordinatorandAppLifecycleObserver.Stack
LifecycleEventEmitteractor + unit testsAnalyticsClientwiring +openURLpublic API + integration tests