diff --git a/.changeset/android-v4-major.md b/.changeset/android-v4-major.md index fc2077550..b24de8f7e 100644 --- a/.changeset/android-v4-major.md +++ b/.changeset/android-v4-major.md @@ -12,3 +12,4 @@ - **Breaking:** make SDK-internal helpers in the `com.posthog.internal` and `com.posthog.android.replay.internal` packages non-public - **Breaking:** remove the deprecated `remoteConfig` config option; the SDK now always loads remote config at setup - **Breaking:** replace `PostHogOnFeatureFlags` with `PostHogFeatureFlagsCallback` in `PostHogConfig.onFeatureFlags` and `reloadFeatureFlags()`; it receives a `PostHogFeatureFlagsLoaded` with the enabled flags, their values, and `errorsLoading` +- Add `onFeatureFlags()` to get notified after every feature flags load (on the main thread on Android), returning a `PostHogFeatureFlagsSubscription` you can `unsubscribe()` diff --git a/posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt b/posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt index c6e845b6e..18c407822 100644 --- a/posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt +++ b/posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt @@ -35,6 +35,7 @@ import com.posthog.internal.PostHogSessionManager import com.posthog.internal.logIntegrationFailure import com.posthog.vendor.uuid.TimeBasedEpochGenerator import java.io.File +import java.util.concurrent.Executor /** * Main entry point for the Android SDK. @@ -236,6 +237,8 @@ public class PostHogAndroid private constructor() { config.sampleRateProvider = { config.sessionReplayConfig.sampleRate } val mainHandler = MainHandler() + // onFeatureFlags listeners usually update UI, so deliver them on the main thread + config.featureFlagsListenerExecutor = Executor { mainHandler.handler.post(it) } config.addIntegrationSafely("PostHogReplayIntegration") { PostHogReplayIntegration(context, config, mainHandler) } diff --git a/posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt b/posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt index 1f2e2de10..d92eae404 100644 --- a/posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt @@ -2,6 +2,7 @@ package com.posthog.android import android.content.Context import android.content.Intent +import android.os.Looper import androidx.test.ext.junit.runners.AndroidJUnit4 import com.posthog.PostHog import com.posthog.android.errortracking.PostHogNativeCrashIntegration @@ -22,6 +23,7 @@ import org.junit.Rule import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.robolectric.Shadows.shadowOf import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -145,6 +147,23 @@ internal class PostHogAndroidTest { assertTrue(config.context is PostHogAndroidContext) } + @Test + fun `runs feature flags listeners on the main thread`() { + val config = PostHogAndroidConfig(PROJECT_TOKEN) + + mockContextAppStart(context, tmpDir) + + PostHogAndroid.setup(context, config) + + var ranOn: Thread? = null + val worker = Thread { config.featureFlagsListenerExecutor!!.execute { ranOn = Thread.currentThread() } } + worker.start() + worker.join() + shadowOf(Looper.getMainLooper()).idle() + + assertSame(Looper.getMainLooper().thread, ranOn) + } + @Test fun `sets legacy storage path`() { val config = PostHogAndroidConfig(PROJECT_TOKEN) diff --git a/posthog/api/posthog.api b/posthog/api/posthog.api index c949a9dbd..fdf89f65f 100644 --- a/posthog/api/posthog.api +++ b/posthog/api/posthog.api @@ -48,6 +48,7 @@ public final class com/posthog/PostHog : com/posthog/PostHogStateless, com/posth public fun isOptOut ()Z public fun isSessionActive ()Z public fun isSessionReplayActive ()Z + public fun onFeatureFlags (Lcom/posthog/PostHogFeatureFlagsCallback;)Lcom/posthog/PostHogFeatureFlagsSubscription; public fun optIn ()V public fun optOut ()V public fun register (Ljava/lang/String;Ljava/lang/Object;)V @@ -96,6 +97,7 @@ public final class com/posthog/PostHog$Companion : com/posthog/PostHogInterface public fun isOptOut ()Z public fun isSessionActive ()Z public fun isSessionReplayActive ()Z + public fun onFeatureFlags (Lcom/posthog/PostHogFeatureFlagsCallback;)Lcom/posthog/PostHogFeatureFlagsSubscription; public fun optIn ()V public fun optOut ()V public final fun overrideSharedInstance (Lcom/posthog/PostHogInterface;)V @@ -172,6 +174,7 @@ public class com/posthog/PostHogConfig { public final fun getEvaluationContexts ()Ljava/util/List; public final fun getFeatureFlagCalledCacheSize ()I public final fun getFeatureFlagRequestMaxRetries ()I + public final fun getFeatureFlagsListenerExecutor ()Ljava/util/concurrent/Executor; public final fun getFlushAt ()I public final fun getFlushIntervalSeconds ()I public final fun getGetAnonymousId ()Lkotlin/jvm/functions/Function1; @@ -225,6 +228,7 @@ public class com/posthog/PostHogConfig { public final fun setEvaluationContexts (Ljava/util/List;)V public final fun setFeatureFlagCalledCacheSize (I)V public final fun setFeatureFlagRequestMaxRetries (I)V + public final fun setFeatureFlagsListenerExecutor (Ljava/util/concurrent/Executor;)V public final fun setFlushAt (I)V public final fun setFlushIntervalSeconds (I)V public final fun setGetAnonymousId (Lkotlin/jvm/functions/Function1;)V @@ -355,6 +359,11 @@ public final class com/posthog/PostHogFeatureFlagsLoaded { public final fun getVariants ()Ljava/util/Map; } +public final class com/posthog/PostHogFeatureFlagsSubscription { + public fun (Lkotlin/jvm/functions/Function0;)V + public final fun unsubscribe ()V +} + public abstract interface class com/posthog/PostHogIntegration { public fun install (Lcom/posthog/PostHogInterface;)V public fun onChange ()V @@ -405,6 +414,7 @@ public abstract interface class com/posthog/PostHogInterface : com/posthog/PostH public static synthetic fun isFeatureEnabled$default (Lcom/posthog/PostHogInterface;Ljava/lang/String;ZLjava/lang/Boolean;ILjava/lang/Object;)Z public abstract fun isSessionActive ()Z public abstract fun isSessionReplayActive ()Z + public abstract fun onFeatureFlags (Lcom/posthog/PostHogFeatureFlagsCallback;)Lcom/posthog/PostHogFeatureFlagsSubscription; public abstract fun register (Ljava/lang/String;Ljava/lang/Object;)V public abstract fun registerPushNotificationToken (Ljava/lang/String;Ljava/lang/String;)V public abstract fun reloadFeatureFlags (Lcom/posthog/PostHogFeatureFlagsCallback;)V @@ -1049,10 +1059,10 @@ public final class com/posthog/internal/PostHogRemoteConfig : com/posthog/intern public fun isMinimalFlagCalledEventsEnabled ()Z public final fun isNativeCrashCaptureEnabled ()Z public final fun isSessionReplayFlagActive ()Z - public final fun loadFeatureFlags (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function0;Lcom/posthog/PostHogFeatureFlagsCallback;)V - public static synthetic fun loadFeatureFlags$default (Lcom/posthog/internal/PostHogRemoteConfig;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function0;Lcom/posthog/PostHogFeatureFlagsCallback;ILjava/lang/Object;)V - public final fun loadRemoteConfig (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function0;Lcom/posthog/PostHogFeatureFlagsCallback;)V - public static synthetic fun loadRemoteConfig$default (Lcom/posthog/internal/PostHogRemoteConfig;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function0;Lcom/posthog/PostHogFeatureFlagsCallback;ILjava/lang/Object;)V + public final fun loadFeatureFlags (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function1;Lcom/posthog/PostHogFeatureFlagsCallback;)V + public static synthetic fun loadFeatureFlags$default (Lcom/posthog/internal/PostHogRemoteConfig;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function1;Lcom/posthog/PostHogFeatureFlagsCallback;ILjava/lang/Object;)V + public final fun loadRemoteConfig (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function1;Lcom/posthog/PostHogFeatureFlagsCallback;)V + public static synthetic fun loadRemoteConfig$default (Lcom/posthog/internal/PostHogRemoteConfig;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lkotlin/jvm/functions/Function1;Lcom/posthog/PostHogFeatureFlagsCallback;ILjava/lang/Object;)V public final fun makeSamplingDecision (Ljava/lang/String;)Z public final fun persistPushConfigCache ()V public final fun resetGroupPropertiesForFlags (Ljava/lang/String;)V diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index 63a8c0f65..6134a2108 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -34,6 +34,7 @@ import com.posthog.internal.PostHogSerializer import com.posthog.internal.PostHogSessionManager import com.posthog.internal.PostHogThreadFactory import com.posthog.internal.errortracking.PostHogExceptionStepsBuffer +import com.posthog.internal.executeSafely import com.posthog.internal.logIntegrationFailure import com.posthog.internal.personPropertiesContext import com.posthog.internal.replay.PostHogSessionReplayHandler @@ -46,6 +47,7 @@ import com.posthog.surveys.Survey import com.posthog.vendor.uuid.TimeBasedEpochGenerator import java.util.Date import java.util.UUID +import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.ExecutorService import java.util.concurrent.Executors @@ -187,19 +189,90 @@ public class PostHog private constructor( private var isIdentifiedLoaded: Boolean = false private var isPersonProcessingLoaded: Boolean = false + // Public onFeatureFlags listeners. Owned by this instance rather than the remote config, so + // listeners registered before setup() (or across close()/setup()) keep receiving updates. + private val featureFlagsListeners = CopyOnWriteArrayList() + + // The last update delivered to listeners, replayed to late subscribers. Null until flags load, + // and cleared by reset() and close() so an update they superseded is dropped. + @Volatile + private var lastFeatureFlagsLoaded: PostHogFeatureFlagsLoaded? = null + + // Bumped by reset() and close(), so a flags load that started before them can't reach listeners. + @Volatile + private var featureFlagsGeneration = 0 + private val featureFlagsGenerationLock = Any() + + private class FeatureFlagsListener(val callback: PostHogFeatureFlagsCallback) { + @Volatile + var active = true + + // Only touched on the listener thread (see runFeatureFlagsListener), where every delivery and replay runs. + var lastDelivered: PostHogFeatureFlagsLoaded? = null + } + + // Runs on the remote config thread after every flags load, whether it succeeded or failed. + // Create it when the load starts, so listeners skip a load that reset() or close() superseded. + private fun internalOnFeatureFlagsLoaded(generation: Int = featureFlagsGeneration): (errorsLoading: Boolean) -> Unit { + return { errorsLoading -> + startSessionReplayOnFeatureFlagsLoaded() + notifyFeatureFlagsListeners(errorsLoading, generation) + } + } + + private fun clearFeatureFlagsLoaded() { + synchronized(featureFlagsGenerationLock) { + featureFlagsGeneration++ + lastFeatureFlagsLoaded = null + } + } + // this is called if the feature flags are loaded for the first time and recording isn't started yet - private val internalOnFeatureFlagsLoaded: () -> Unit = - { - // under setupLock so a concurrent optOut can't uninstall replay between the check and the start - synchronized(setupLock) { - sessionReplayHandler?.let { - if (isSessionReplayConfigEnabled()) { - // start will bail if session replay is already active anyway - startSessionReplayInternal(resumeCurrent = true) - } + private fun startSessionReplayOnFeatureFlagsLoaded() { + // under setupLock so a concurrent optOut can't uninstall replay between the check and the start + synchronized(setupLock) { + sessionReplayHandler?.let { + if (isSessionReplayConfigEnabled()) { + // start will bail if session replay is already active anyway + startSessionReplayInternal(resumeCurrent = true) } } } + } + + private fun notifyFeatureFlagsListeners( + errorsLoading: Boolean, + generation: Int, + ) { + val loaded = PostHogFeatureFlagsLoaded(remoteConfig?.getFeatureFlags() ?: emptyMap(), errorsLoading) + synchronized(featureFlagsGenerationLock) { + if (featureFlagsGeneration != generation) return + lastFeatureFlagsLoaded = loaded + } + featureFlagsListeners.forEach { listener -> runFeatureFlagsListener { deliverFeatureFlags(listener, loaded) } } + } + + // Called on the remote config thread. Hands the block to the platform's listener executor (the main + // thread on Android), or runs it right away when there's none. + private fun runFeatureFlagsListener(block: () -> Unit) { + val executor = config?.featureFlagsListenerExecutor ?: return block() + executor.executeSafely(block) + } + + private fun deliverFeatureFlags( + listener: FeatureFlagsListener, + loaded: PostHogFeatureFlagsLoaded, + ) { + // An earlier callback may have unsubscribed this one, or reset()/close() or a newer load + // may have superseded this update. A replay queued next to the same update is skipped too. + if (!listener.active || lastFeatureFlagsLoaded !== loaded || listener.lastDelivered === loaded) return + listener.lastDelivered = loaded + try { + listener.callback.loaded(loaded) + } catch (e: Throwable) { + config?.logger?.log("Executing the feature flags listener failed: $e") + } + } public override fun setup(config: T) { synchronized(setupLock) { @@ -357,6 +430,16 @@ public class PostHog private constructor( super.enabled = true + // Captured before any user callout below (integrations, session hooks, the bootstrap + // config callback), any of which may close() and so supersede setup's flags load. + val generation = featureFlagsGeneration + + // Queued before integrations install, so a reload one starts there can't hold the + // bootstrapped flags behind its network response. Listeners run on the remote config thread. + if (reloadFeatureFlags && remoteConfig?.hasBootstrapFlags() == true) { + remoteConfigExecutor.executeSafely { notifyFeatureFlagsListeners(errorsLoading = false, generation) } + } + // Initialize device_id if not already set. getDeviceId() handles lazy init // by seeding from the anonymous ID, providing a stable identifier for // device-level feature flag bucketing that survives identify() and reset(). @@ -398,14 +481,14 @@ public class PostHog private constructor( // callbacks now so listeners aren't blocked on the first network response. if (remoteConfig?.hasBootstrapFlags() == true) { try { - internalOnFeatureFlagsLoaded() + startSessionReplayOnFeatureFlagsLoaded() } catch (e: Throwable) { config.logger.log("Executing the internal feature flags callback failed: $e") } notifyFeatureFlagsCallback(config.onFeatureFlags, remoteConfig?.getFeatureFlags(), errorsLoading = false) } loadRemoteConfigRequest( - internalOnFeatureFlagsLoaded, + internalOnFeatureFlagsLoaded(generation), config.onFeatureFlags, ) } @@ -631,6 +714,7 @@ public class PostHog private constructor( pushSubscriptionManager = null featureFlagsCalled.clear() + clearFeatureFlagsLoaded() lastScreenName = null synchronized(replayDebugPropertiesLock) { lastReplayDebugPropertiesAt = null @@ -1938,13 +2022,30 @@ public class PostHog private constructor( return } loadFeatureFlagsRequest( - internalOnFeatureFlags = internalOnFeatureFlagsLoaded, + internalOnFeatureFlags = internalOnFeatureFlagsLoaded(), onFeatureFlags = onFeatureFlags, ) } + public override fun onFeatureFlags(callback: PostHogFeatureFlagsCallback): PostHogFeatureFlagsSubscription { + val listener = FeatureFlagsListener(callback) + featureFlagsListeners.add(listener) + + remoteConfigExecutor.executeSafely { + runFeatureFlagsListener { + // Read at delivery time so a delivery already queued can't be followed by an older value. + lastFeatureFlagsLoaded?.let { deliverFeatureFlags(listener, it) } + } + } + + return PostHogFeatureFlagsSubscription { + listener.active = false + featureFlagsListeners.remove(listener) + } + } + private fun loadFeatureFlagsRequest( - internalOnFeatureFlags: () -> Unit, + internalOnFeatureFlags: (errorsLoading: Boolean) -> Unit, onFeatureFlags: PostHogFeatureFlagsCallback? = null, ) { @Suppress("UNCHECKED_CAST") @@ -1974,7 +2075,7 @@ public class PostHog private constructor( } private fun loadRemoteConfigRequest( - internalOnFeatureFlags: (() -> Unit)?, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)?, onFeatureFlags: PostHogFeatureFlagsCallback?, ) { @Suppress("UNCHECKED_CAST") @@ -2231,6 +2332,7 @@ public class PostHog private constructor( getPreferences().clear(except = except.toList()) } remoteConfig?.clear() + clearFeatureFlagsLoaded() featureFlagsCalled.clear() lastScreenName = null synchronized(cachedPersonPropertiesLock) { @@ -2717,6 +2819,9 @@ public class PostHog private constructor( shared.reloadFeatureFlags(onFeatureFlags) } + public override fun onFeatureFlags(callback: PostHogFeatureFlagsCallback): PostHogFeatureFlagsSubscription = + shared.onFeatureFlags(callback) + public override fun isFeatureEnabled( key: String, defaultValue: Boolean, diff --git a/posthog/src/main/java/com/posthog/PostHogConfig.kt b/posthog/src/main/java/com/posthog/PostHogConfig.kt index ee45ad3c4..f36d38c38 100644 --- a/posthog/src/main/java/com/posthog/PostHogConfig.kt +++ b/posthog/src/main/java/com/posthog/PostHogConfig.kt @@ -24,6 +24,7 @@ import com.posthog.surveys.PostHogSurveysConfig import okhttp3.OkHttpClient import java.net.Proxy import java.util.UUID +import java.util.concurrent.Executor import java.util.concurrent.ExecutorService /** @@ -490,6 +491,13 @@ public open class PostHogConfig( @PostHogInternal public var sampleRateProvider: (() -> Double?)? = null + /** + * Runs the [PostHog.onFeatureFlags] listeners. Set by platform-specific modules (e.g., Android) + * to deliver them on the main thread. When null, they run on the SDK's remote config thread. + */ + @PostHogInternal + public var featureFlagsListenerExecutor: Executor? = null + /** * Reference to the PostHogRemoteConfig instance, set during setup. * Used by integrations to check remote config values. diff --git a/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt index 2252f392a..355b9714f 100644 --- a/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt +++ b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt @@ -3,10 +3,15 @@ package com.posthog /** * Callback invoked when a feature flags load finishes, whether it succeeded or failed. * - * It's used by `PostHogConfig.onFeatureFlags` and `reloadFeatureFlags`. It runs on a background - * thread, or right away on the calling thread when no load is attempted (for example, the SDK is - * disabled). With bootstrapped flags, `PostHogConfig.onFeatureFlags` also runs once on the thread - * calling `setup()`, before the first load finishes. Hop to the main thread before touching UI. + * It's used by `PostHogConfig.onFeatureFlags`, `reloadFeatureFlags` and `onFeatureFlags`. + * + * For `PostHogConfig.onFeatureFlags` and `reloadFeatureFlags`, it runs on a background thread, or right + * away on the calling thread when no load is attempted (for example, the SDK is disabled). With + * bootstrapped flags, `PostHogConfig.onFeatureFlags` also runs once on the thread calling `setup()`, + * before the first load finishes. Hop to the main thread before touching UI. + * + * For `onFeatureFlags`, it runs on the main thread on Android, and on the SDK's background remote + * config thread on the JVM. * * Example: * ```kotlin diff --git a/posthog/src/main/java/com/posthog/PostHogFeatureFlagsSubscription.kt b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsSubscription.kt new file mode 100644 index 000000000..32aa85ec9 --- /dev/null +++ b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsSubscription.kt @@ -0,0 +1,30 @@ +package com.posthog + +/** + * A registration returned by `onFeatureFlags`. + * + * The callback stays registered until you call [unsubscribe]. You don't need to keep a reference + * to this object unless you want to unsubscribe later. + * + * Example: + * ```kotlin + * val subscription = PostHog.onFeatureFlags { loaded -> + * // React to the new flags + * } + * + * // Later, stop listening + * subscription.unsubscribe() + * ``` + */ +public class PostHogFeatureFlagsSubscription + @PostHogInternal + public constructor( + private val onUnsubscribe: () -> Unit, + ) { + /** + * Stops the callback from being invoked. Calling it more than once has no effect. + */ + public fun unsubscribe() { + onUnsubscribe() + } + } diff --git a/posthog/src/main/java/com/posthog/PostHogInterface.kt b/posthog/src/main/java/com/posthog/PostHogInterface.kt index e56fbf98b..cf0dd630f 100644 --- a/posthog/src/main/java/com/posthog/PostHogInterface.kt +++ b/posthog/src/main/java/com/posthog/PostHogInterface.kt @@ -141,6 +141,36 @@ public interface PostHogInterface : PostHogCoreInterface { */ public fun reloadFeatureFlags(onFeatureFlags: PostHogFeatureFlagsCallback? = null) + /** + * Registers a callback that's invoked whenever feature flags are loaded or updated. + * + * On Android, the callback runs on the main thread, after the getters already return the new + * values. On the JVM, it runs on the SDK's background remote config thread. It's + * invoked after the SDK loads flags at startup (including from bootstrap values), after + * [reloadFeatureFlags], and after calls that reload flags such as [identify], [group] or [reset]. + * If a load fails, [PostHogFeatureFlagsLoaded.errorsLoading] is `true` and the values are the last + * known flags. If flags have already loaded when you register, it's also invoked with the current + * values shortly afterwards. + * + * You can register before `setup()`, and the callback stays registered across `close()` and + * `setup()`. Treat each invocation as "flags may have changed", not as an exactly-once event. + * + * ```kotlin + * val subscription = PostHog.onFeatureFlags { loaded -> + * if (loaded.variants["new-checkout"] == "test") { + * // Show the test variant + * } + * } + * + * // Later, stop listening + * subscription.unsubscribe() + * ``` + * + * @param callback receives the enabled flags, their values, and whether loading failed + * @return a subscription; call [PostHogFeatureFlagsSubscription.unsubscribe] to stop receiving updates + */ + public fun onFeatureFlags(callback: PostHogFeatureFlagsCallback): PostHogFeatureFlagsSubscription + /** * Returns if a feature flag is enabled, the feature flag must be a Boolean * Docs https://posthog.com/docs/feature-flags and https://posthog.com/docs/experiments diff --git a/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt b/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt index c1091864a..f9bec33c9 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt @@ -64,7 +64,7 @@ public class PostHogRemoteConfig( val distinctId: String, val anonymousId: String?, val groups: Map?, - val internalOnFeatureFlags: List<() -> Unit>, + val internalOnFeatureFlags: List<(errorsLoading: Boolean) -> Unit>, val onFeatureFlags: List, ) @@ -257,13 +257,11 @@ public class PostHogRemoteConfig( } @JvmName("asSingleInternalCallback") - private fun List<() -> Unit>.asSingleCallback(): (() -> Unit)? = + private fun List<(errorsLoading: Boolean) -> Unit>.asSingleCallback(): ((errorsLoading: Boolean) -> Unit)? = when (size) { 0 -> null 1 -> first() - else -> { - { forEach { runSafely(it) } } - } + else -> { errorsLoading -> forEach { runSafely { it(errorsLoading) } } } } private fun List.asSingleCallback(): PostHogFeatureFlagsCallback? = @@ -286,17 +284,18 @@ public class PostHogRemoteConfig( } private fun runOnFeatureFlagsCallbacks( - internalOnFeatureFlags: (() -> Unit)?, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)?, onFeatureFlags: PostHogFeatureFlagsCallback?, errorsLoading: Boolean, ) { // if we don't load the feature flags (because there are none), we need to call the callback // because the app might be waiting for it. - internalOnFeatureFlags?.let { runSafely(it) } + if (internalOnFeatureFlags == null && onFeatureFlags == null) return + // a failed first load still reports the persisted flags, so hydrate them before either callback + runSafely { loadFeatureFlagsFromCacheIfNeeded() } + internalOnFeatureFlags?.let { runSafely { it(errorsLoading) } } onFeatureFlags?.let { callback -> runSafely { - // a failed first load still reports the persisted flags, so hydrate them first - loadFeatureFlagsFromCacheIfNeeded() callback.loaded(PostHogFeatureFlagsLoaded(getFeatureFlags() ?: emptyMap(), errorsLoading)) } } @@ -317,7 +316,7 @@ public class PostHogRemoteConfig( distinctId: String, anonymousId: String?, groups: Map?, - internalOnFeatureFlags: (() -> Unit)? = null, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)? = null, onFeatureFlags: PostHogFeatureFlagsCallback? = null, ) { executor.executeSafely { @@ -858,7 +857,7 @@ public class PostHogRemoteConfig( distinctId: String, anonymousId: String?, groups: Map?, - internalOnFeatureFlags: (() -> Unit)?, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)?, onFeatureFlags: PostHogFeatureFlagsCallback?, notifyRemoteConfigLoaded: Boolean = true, ) { @@ -1121,7 +1120,7 @@ public class PostHogRemoteConfig( distinctId: String, anonymousId: String?, groups: Map?, - internalOnFeatureFlags: (() -> Unit)? = null, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)? = null, onFeatureFlags: PostHogFeatureFlagsCallback? = null, ) { executor.executeSafely { diff --git a/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt b/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt index 19903d96a..8fb56afdd 100644 --- a/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt @@ -7,16 +7,25 @@ import com.posthog.internal.PostHogPreferences.Companion.DISTINCT_ID import com.posthog.internal.PostHogPreferences.Companion.IS_IDENTIFIED import com.posthog.internal.PostHogSerializer import com.posthog.internal.PostHogThreadFactory +import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest import org.junit.Rule import org.junit.rules.TemporaryFolder import java.io.File +import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors +import java.util.concurrent.LinkedBlockingQueue +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue internal class PostHogBootstrapTest { @@ -382,6 +391,105 @@ internal class PostHogBootstrapTest { sut.close() } + @Test + fun `onFeatureFlags replays bootstrapped flags to a listener registered after setup`() { + val sut = + getSut( + host = "http://127.0.0.1:1", + // the replay comes from setup's flag loading, before /config answers + reloadFeatureFlags = true, + bootstrap = PostHogBootstrapConfig(featureFlags = mapOf("beta-ui" to true)), + ) + + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + + // setup's load to the unreachable host may already have failed, which keeps the bootstrapped flags + val loaded = updates.poll(5, TimeUnit.SECONDS)!! + assertEquals(mapOf("beta-ui" to true), loaded.variants) + + sut.close() + } + + @Test + fun `onFeatureFlags delivers bootstrapped flags before a reload an integration starts`() { + val release = CountDownLatch(1) + val http = MockWebServer() + http.dispatcher = + object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + if (request.path!!.contains("/flags")) { + release.await(10, TimeUnit.SECONDS) + return MockResponse().setBody("""{"featureFlags":{"beta-ui":false}}""") + } + return MockResponse().setBody("""{"hasFeatureFlags":true}""") + } + } + http.start() + val config = + PostHogConfig(PROJECT_TOKEN, http.url("/").toString()).apply { + storagePrefix = tmpDir.newFolder().absolutePath + cachePreferences = PostHogMemoryPreferences() + bootstrap = PostHogBootstrapConfig(featureFlags = mapOf("beta-ui" to true)) + addIntegration( + object : PostHogIntegration { + override fun install(postHog: PostHogInterface) { + postHog.reloadFeatureFlags() + } + }, + ) + } + val updates = LinkedBlockingQueue() + val subscription = PostHog.onFeatureFlags { updates.add(it) } + + try { + PostHog.setup(config) + + // the integration's /flags request is held, so this can only be the bootstrap delivery + val loaded = updates.poll(5, TimeUnit.SECONDS) + assertEquals(mapOf("beta-ui" to true), loaded?.variants) + } finally { + subscription.unsubscribe() + release.countDown() + PostHog.close() + http.shutdown() + } + } + + @Test + fun `closing from the bootstrap flags callback drops setup's load for listeners`() { + val http = mockHttp(total = 5, response = MockResponse().setBody("""{"hasFeatureFlags":false}""")) + val completed = LinkedBlockingQueue() + val first = AtomicBoolean(true) + val config = + PostHogConfig(PROJECT_TOKEN, http.url("/").toString()).apply { + storagePrefix = tmpDir.newFolder().absolutePath + cachePreferences = PostHogMemoryPreferences() + bootstrap = PostHogBootstrapConfig(featureFlags = mapOf("beta-ui" to true)) + onFeatureFlags = + PostHogFeatureFlagsCallback { + // the bootstrap delivery closes the SDK; setup's own load completes later + if (first.compareAndSet(true, false)) PostHog.close() else completed.add(it) + } + } + + try { + PostHog.setup(config) + + // setup's load notifies listeners just before this completion + assertNotNull(completed.poll(5, TimeUnit.SECONDS)) + + // close() cleared the snapshot, so only a load it superseded could refill it + val updates = LinkedBlockingQueue() + val subscription = PostHog.onFeatureFlags { updates.add(it) } + assertNull(updates.poll(500, TimeUnit.MILLISECONDS)) + subscription.unsubscribe() + } finally { + PostHog.close() + http.shutdown() + } + } + @Test fun `minimal flag called event strips bootstrap fields even though this SDK sets them`() { // fixture returns minimalFlagCalledEvents: true and IAmInactive with has_experiment: false; diff --git a/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt b/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt index 399c7ee79..8c02709ce 100644 --- a/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt @@ -9,11 +9,17 @@ import okhttp3.mockwebserver.MockResponse import org.junit.Rule import org.junit.rules.TemporaryFolder import java.io.File +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executor import java.util.concurrent.Executors +import java.util.concurrent.LinkedBlockingQueue +import java.util.concurrent.TimeUnit import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotEquals +import kotlin.test.assertNull import kotlin.test.assertTrue internal class PostHogFeatureFlagsTest { @@ -57,6 +63,7 @@ internal class PostHogFeatureFlagsTest { beforeSend: PostHogBeforeSend? = null, evaluationContexts: List? = null, context: PostHogContext? = null, + featureFlagsListenerExecutor: Executor? = null, ): PostHogInterface { config = PostHogConfig(PROJECT_TOKEN, host).apply { @@ -78,6 +85,7 @@ internal class PostHogFeatureFlagsTest { } this.errorTrackingConfig.inAppIncludes.add("com.posthog") this.context = context + this.featureFlagsListenerExecutor = featureFlagsListenerExecutor } return PostHog.withInternal( config, @@ -401,4 +409,181 @@ internal class PostHogFeatureFlagsTest { assertTrue(result!!.flags.isEmpty()) assertTrue(result!!.variants.isEmpty()) } + + @Test + fun `onFeatureFlags listener receives each load until unsubscribed`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags)) + http.enqueue(MockResponse().setResponseCode(500)) + http.enqueue(MockResponse().setBody(flags)) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false) + val enabled = mapOf("4535-funnel-bar-viz" to true, "splashScreenName" to "SplashV2") + + val updates = LinkedBlockingQueue() + val subscription = sut.onFeatureFlags { updates.add(it) } + + sut.reloadFeatureFlags() + val loaded = updates.poll(5, TimeUnit.SECONDS)!! + assertEquals(enabled, loaded.variants) + assertFalse(loaded.errorsLoading) + + sut.reloadFeatureFlags() + val failed = updates.poll(5, TimeUnit.SECONDS)!! + assertEquals(enabled, failed.variants) + assertTrue(failed.errorsLoading) + + subscription.unsubscribe() + val reloaded = CountDownLatch(1) + sut.reloadFeatureFlags { reloaded.countDown() } + assertTrue(reloaded.await(5, TimeUnit.SECONDS)) + assertNull(updates.poll()) + } + + @Test + fun `onFeatureFlags reports and replays the persisted flags when the first load fails`() { + val persisted = mapOf("beta-ui" to true) + val cache = PostHogMemoryPreferences().apply { setValue("featureFlags", persisted) } + val http = mockHttp(response = MockResponse().setResponseCode(500)) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false, cachePreferences = cache) + + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + sut.reloadFeatureFlags() + + val failed = updates.poll(5, TimeUnit.SECONDS)!! + assertTrue(failed.errorsLoading) + assertEquals(persisted, failed.variants) + + val lateUpdates = LinkedBlockingQueue() + sut.onFeatureFlags { lateUpdates.add(it) } + assertEquals(persisted, lateUpdates.poll(5, TimeUnit.SECONDS)!!.variants) + } + + @Test + fun `onFeatureFlags replays loaded flags to a late listener`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags)) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false) + val reloaded = CountDownLatch(1) + sut.reloadFeatureFlags { reloaded.countDown() } + assertTrue(reloaded.await(5, TimeUnit.SECONDS)) + + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + + assertEquals(setOf("4535-funnel-bar-viz", "splashScreenName"), updates.poll(5, TimeUnit.SECONDS)!!.flags.toSet()) + } + + @Test + fun `onFeatureFlags listener survives close and setup`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags)) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false) + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + + sut.close() + sut.setup(config) + sut.reloadFeatureFlags() + + assertFalse(updates.poll(5, TimeUnit.SECONDS)!!.errorsLoading) + } + + @Test + fun `onFeatureFlags drops a load that finishes after close`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags).setHeadersDelay(500, TimeUnit.MILLISECONDS)) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false) + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + val reloaded = CountDownLatch(1) + sut.reloadFeatureFlags { reloaded.countDown() } + http.takeRequest(5, TimeUnit.SECONDS) + + sut.close() + assertTrue(reloaded.await(5, TimeUnit.SECONDS)) + sut.setup(config) + sut.onFeatureFlags { updates.add(it) } + + remoteConfigExecutor.submit {}.get(5, TimeUnit.SECONDS) + assertNull(updates.poll()) + } + + @Test + fun `closing from an integration's install drops setup's load for listeners`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(total = 2, response = MockResponse().setBody(flags)) + val updates = LinkedBlockingQueue() + val integration = + object : PostHogIntegration { + override fun install(postHog: PostHogInterface) { + postHog.onFeatureFlags { updates.add(it) } + postHog.close() + } + } + val sut = getSut(http.url("/").toString(), integration = integration) + + // setup's load runs on the remote config thread; close() cleared the snapshot it would refill + remoteConfigExecutor.submit {}.get(5, TimeUnit.SECONDS) + sut.onFeatureFlags { updates.add(it) } + remoteConfigExecutor.submit {}.get(5, TimeUnit.SECONDS) + assertNull(updates.poll()) + } + + @Test + fun `onFeatureFlags listener is not invoked by a reload while disabled`() { + val sut = getSut(mockHttp().url("/").toString(), preloadFeatureFlags = false) + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + sut.close() + + var result: PostHogFeatureFlagsLoaded? = null + sut.reloadFeatureFlags { result = it } + + remoteConfigExecutor.submit {}.get(5, TimeUnit.SECONDS) + assertTrue(result!!.errorsLoading) + assertNull(updates.poll()) + } + + @Test + fun `onFeatureFlags listeners run on the listener executor`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags)) + val listenerExecutor = Executors.newSingleThreadExecutor { Thread(it, "listener-thread") } + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false, featureFlagsListenerExecutor = listenerExecutor) + + val threads = LinkedBlockingQueue() + sut.onFeatureFlags { threads.add(Thread.currentThread().name) } + sut.reloadFeatureFlags() + assertEquals("listener-thread", threads.poll(5, TimeUnit.SECONDS)) + + sut.onFeatureFlags { threads.add(Thread.currentThread().name) } + assertEquals("listener-thread", threads.poll(5, TimeUnit.SECONDS)) + listenerExecutor.shutdownAndAwaitTermination() + } + + @Test + fun `onFeatureFlags delivers a queued update once and drops it after unsubscribe`() { + val flags = File("src/test/resources/json/basic-flags-with-non-active-flags.json").readText() + val http = mockHttp(response = MockResponse().setBody(flags)) + val queued = LinkedBlockingQueue() + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false, featureFlagsListenerExecutor = Executor { queued.add(it) }) + + val kept = LinkedBlockingQueue() + val dropped = LinkedBlockingQueue() + sut.onFeatureFlags { kept.add(it) } + val subscription = sut.onFeatureFlags { dropped.add(it) } + val reloaded = CountDownLatch(1) + sut.reloadFeatureFlags { reloaded.countDown() } + assertTrue(reloaded.await(5, TimeUnit.SECONDS)) + remoteConfigExecutor.submit {}.get(5, TimeUnit.SECONDS) + + // Each listener has its replay and the load's delivery queued, both carrying the loaded flags. + subscription.unsubscribe() + generateSequence { queued.poll() }.forEach { it.run() } + + assertEquals(1, kept.size) + assertFalse(kept.single().errorsLoading) + assertTrue(dropped.isEmpty()) + } } diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 0288881f1..ddb08ed2d 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -1500,9 +1500,9 @@ internal class PostHogTest { config.sessionReplay = true @Suppress("UNCHECKED_CAST") val onFlagsLoaded = - PostHog::class.java.getDeclaredField("internalOnFeatureFlagsLoaded") + PostHog::class.java.getDeclaredMethod("internalOnFeatureFlagsLoaded", Int::class.javaPrimitiveType) .apply { isAccessible = true } - .get(sut) as () -> Unit + .invoke(sut, 0) as (Boolean) -> Unit // optOut runs while the flags-loaded callback is between reading the handler and starting it var optOut: Thread? = null @@ -1516,7 +1516,7 @@ internal class PostHogTest { } } } - onFlagsLoaded() + onFlagsLoaded(false) optOut!!.join() assertTrue(sut.isOptOut()) diff --git a/posthog/src/test/java/com/posthog/internal/PostHogRemoteConfigTest.kt b/posthog/src/test/java/com/posthog/internal/PostHogRemoteConfigTest.kt index c242f2ecb..34fcdcd62 100644 --- a/posthog/src/test/java/com/posthog/internal/PostHogRemoteConfigTest.kt +++ b/posthog/src/test/java/com/posthog/internal/PostHogRemoteConfigTest.kt @@ -924,7 +924,7 @@ internal class PostHogRemoteConfigTest { called = true } - val internalOnFeatureFlags = { + val internalOnFeatureFlags = { _: Boolean -> calledInternal = true } diff --git a/posthog/src/testFixtures/java/com/posthog/PostHogFake.kt b/posthog/src/testFixtures/java/com/posthog/PostHogFake.kt index ca61394fe..dd5f42273 100644 --- a/posthog/src/testFixtures/java/com/posthog/PostHogFake.kt +++ b/posthog/src/testFixtures/java/com/posthog/PostHogFake.kt @@ -82,6 +82,8 @@ public class PostHogFake : PostHogInterface { override fun reloadFeatureFlags(onFeatureFlags: PostHogFeatureFlagsCallback?) { } + override fun onFeatureFlags(callback: PostHogFeatureFlagsCallback): PostHogFeatureFlagsSubscription = PostHogFeatureFlagsSubscription {} + override fun isFeatureEnabled( key: String, defaultValue: Boolean,