From c05a8640898b9febf76dfc4f09a863756b82be22 Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Fri, 23 Oct 2026 14:59:13 -0400 Subject: [PATCH 1/2] feat: add an onFeatureFlags listener you can unsubscribe Registers a PostHogFeatureFlagsCallback that runs after every feature flags load, replays the current flags to late subscribers, and returns a PostHogFeatureFlagsSubscription, matching posthog-ios onFeatureFlags(_:). --- .changeset/android-v4-major.md | 1 + posthog/api/posthog.api | 16 +++- posthog/src/main/java/com/posthog/PostHog.kt | 96 ++++++++++++++++--- .../posthog/PostHogFeatureFlagsCallback.kt | 2 +- .../PostHogFeatureFlagsSubscription.kt | 30 ++++++ .../main/java/com/posthog/PostHogInterface.kt | 29 ++++++ .../posthog/internal/PostHogRemoteConfig.kt | 18 ++-- .../java/com/posthog/PostHogBootstrapTest.kt | 16 ++++ .../com/posthog/PostHogFeatureFlagsTest.kt | 79 +++++++++++++++ .../src/test/java/com/posthog/PostHogTest.kt | 4 +- .../internal/PostHogRemoteConfigTest.kt | 2 +- .../java/com/posthog/PostHogFake.kt | 2 + 12 files changed, 265 insertions(+), 30 deletions(-) create mode 100644 posthog/src/main/java/com/posthog/PostHogFeatureFlagsSubscription.kt diff --git a/.changeset/android-v4-major.md b/.changeset/android-v4-major.md index 4d232426d..bfd0a7e4f 100644 --- a/.changeset/android-v4-major.md +++ b/.changeset/android-v4-major.md @@ -9,3 +9,4 @@ - **Breaking:** remove the deprecated `getFeatureFlagPayload()`; use `getFeatureFlagResult(key)?.payload` instead - **Breaking:** remove the deprecated `PostHogSessionReplayConfig.debouncerDelayMs`; use `throttleDelayMs` instead - **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, returning a `PostHogFeatureFlagsSubscription` you can `unsubscribe()` diff --git a/posthog/api/posthog.api b/posthog/api/posthog.api index 7d086cb98..bf451168b 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 @@ -357,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 @@ -407,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 @@ -1069,10 +1077,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 867bdf7c2..fb94ab150 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,63 @@ 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 + + private class FeatureFlagsListener(val callback: PostHogFeatureFlagsCallback) { + @Volatile + var active = true + + // Only touched on the remote config thread, 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. + private val internalOnFeatureFlagsLoaded: (errorsLoading: Boolean) -> Unit = + { errorsLoading -> + startSessionReplayOnFeatureFlagsLoaded() + notifyFeatureFlagsListeners(errorsLoading) + } + // 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) { + val loaded = PostHogFeatureFlagsLoaded(remoteConfig?.getFeatureFlags() ?: emptyMap(), errorsLoading) + lastFeatureFlagsLoaded = loaded + featureFlagsListeners.forEach { deliverFeatureFlags(it, loaded) } + } + + 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. + if (!listener.active || lastFeatureFlagsLoaded !== 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) { @@ -398,7 +444,9 @@ public class PostHog private constructor( // Bootstrap already seeded the flags in RemoteConfig's init; fire the flags-loaded // callbacks now so listeners aren't blocked on the first network response. if (remoteConfig?.hasBootstrapFlags() == true) { - internalOnFeatureFlagsLoaded() + startSessionReplayOnFeatureFlagsLoaded() + // listeners run on the remote config thread, queued ahead of setup's own load + remoteConfigExecutor.executeSafely { notifyFeatureFlagsListeners(errorsLoading = false) } notifyFeatureFlagsCallback(config.onFeatureFlags, remoteConfig?.getFeatureFlags(), errorsLoading = false) } when { @@ -633,6 +681,7 @@ public class PostHog private constructor( pushSubscriptionManager = null featureFlagsCalled.clear() + lastFeatureFlagsLoaded = null lastScreenName = null synchronized(replayDebugPropertiesLock) { lastReplayDebugPropertiesAt = null @@ -1946,8 +1995,27 @@ public class PostHog private constructor( ) } + public override fun onFeatureFlags(callback: PostHogFeatureFlagsCallback): PostHogFeatureFlagsSubscription { + val listener = FeatureFlagsListener(callback) + featureFlagsListeners.add(listener) + + remoteConfigExecutor.executeSafely { + // Read at delivery time so a delivery already queued can't be followed by an older value, + // and skip it if that delivery already passed these values. + val current = lastFeatureFlagsLoaded + if (current != null && current !== listener.lastDelivered) { + deliverFeatureFlags(listener, current) + } + } + + return PostHogFeatureFlagsSubscription { + listener.active = false + featureFlagsListeners.remove(listener) + } + } + private fun loadFeatureFlagsRequest( - internalOnFeatureFlags: () -> Unit, + internalOnFeatureFlags: (errorsLoading: Boolean) -> Unit, onFeatureFlags: PostHogFeatureFlagsCallback? = null, ) { @Suppress("UNCHECKED_CAST") @@ -1977,7 +2045,7 @@ public class PostHog private constructor( } private fun loadRemoteConfigRequest( - internalOnFeatureFlags: (() -> Unit)?, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)?, onFeatureFlags: PostHogFeatureFlagsCallback?, ) { @Suppress("UNCHECKED_CAST") @@ -2234,6 +2302,7 @@ public class PostHog private constructor( getPreferences().clear(except = except.toList()) } remoteConfig?.clear() + lastFeatureFlagsLoaded = null featureFlagsCalled.clear() lastScreenName = null synchronized(cachedPersonPropertiesLock) { @@ -2720,6 +2789,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/PostHogFeatureFlagsCallback.kt b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt index 2252f392a..4851e61d9 100644 --- a/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt +++ b/posthog/src/main/java/com/posthog/PostHogFeatureFlagsCallback.kt @@ -3,7 +3,7 @@ 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 + * It's used by `PostHogConfig.onFeatureFlags`, `reloadFeatureFlags` and `onFeatureFlags`. 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. 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..cdf534705 100644 --- a/posthog/src/main/java/com/posthog/PostHogInterface.kt +++ b/posthog/src/main/java/com/posthog/PostHogInterface.kt @@ -141,6 +141,35 @@ public interface PostHogInterface : PostHogCoreInterface { */ public fun reloadFeatureFlags(onFeatureFlags: PostHogFeatureFlagsCallback? = null) + /** + * Registers a callback that's invoked whenever feature flags are loaded or updated. + * + * The callback runs on the SDK's background remote config thread, never the main 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 c1bb8ab0d..4ee34d587 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt @@ -60,7 +60,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, ) @@ -249,13 +249,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? = @@ -278,13 +276,13 @@ 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) } + internalOnFeatureFlags?.let { runSafely { it(errorsLoading) } } onFeatureFlags?.let { callback -> runSafely { callback.loaded(PostHogFeatureFlagsLoaded(getFeatureFlags() ?: emptyMap(), errorsLoading)) } } @@ -305,7 +303,7 @@ public class PostHogRemoteConfig( distinctId: String, anonymousId: String?, groups: Map?, - internalOnFeatureFlags: (() -> Unit)? = null, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)? = null, onFeatureFlags: PostHogFeatureFlagsCallback? = null, ) { executor.executeSafely { @@ -846,7 +844,7 @@ public class PostHogRemoteConfig( distinctId: String, anonymousId: String?, groups: Map?, - internalOnFeatureFlags: (() -> Unit)?, + internalOnFeatureFlags: ((errorsLoading: Boolean) -> Unit)?, onFeatureFlags: PostHogFeatureFlagsCallback?, notifyRemoteConfigLoaded: Boolean = true, ) { @@ -1109,7 +1107,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 6e8a4c9ce..a5d12347d 100644 --- a/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogBootstrapTest.kt @@ -12,6 +12,8 @@ import org.junit.Rule import org.junit.rules.TemporaryFolder import java.io.File 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 @@ -381,6 +383,20 @@ internal class PostHogBootstrapTest { sut.close() } + @Test + fun `onFeatureFlags replays bootstrapped flags to a listener registered after setup`() { + val sut = getSut(bootstrap = PostHogBootstrapConfig(featureFlags = mapOf("beta-ui" to true))) + + val updates = LinkedBlockingQueue() + sut.onFeatureFlags { updates.add(it) } + + val loaded = updates.poll(5, TimeUnit.SECONDS)!! + assertEquals(mapOf("beta-ui" to true), loaded.variants) + assertFalse(loaded.errorsLoading) + + sut.close() + } + @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 31603d9dc..41f9823fb 100644 --- a/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt @@ -9,11 +9,16 @@ 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.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 { @@ -402,4 +407,78 @@ 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 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 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()) + } } diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 21916d135..ae7bb6b52 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -1519,7 +1519,7 @@ internal class PostHogTest { val onFlagsLoaded = PostHog::class.java.getDeclaredField("internalOnFeatureFlagsLoaded") .apply { isAccessible = true } - .get(sut) as () -> Unit + .get(sut) as (Boolean) -> Unit // optOut runs while the flags-loaded callback is between reading the handler and starting it var optOut: Thread? = null @@ -1533,7 +1533,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 e5468c6c3..b041d52b6 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, From 46c6dd152e3fe4a3bbb4322986b0679e0313c678 Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Fri, 23 Oct 2026 18:25:31 -0400 Subject: [PATCH 2/2] fix: drop flag loads superseded by reset() or close() for listeners A load that started before reset() or close() could finish afterwards, reach listeners, and be replayed to late subscribers, including the previous user's flags after reset(). Each load now captures a generation that reset() and close() bump, and listener updates from an older generation are dropped. --- posthog/src/main/java/com/posthog/PostHog.kt | 42 ++++++++++++++----- .../com/posthog/PostHogFeatureFlagsTest.kt | 20 +++++++++ .../src/test/java/com/posthog/PostHogTest.kt | 4 +- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index fb94ab150..ca9eaab4a 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -198,6 +198,11 @@ public class PostHog private constructor( @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 @@ -207,11 +212,21 @@ public class PostHog private constructor( } // Runs on the remote config thread after every flags load, whether it succeeded or failed. - private val internalOnFeatureFlagsLoaded: (errorsLoading: Boolean) -> Unit = - { errorsLoading -> + // Create it when the load starts, so listeners skip a load that reset() or close() superseded. + private fun internalOnFeatureFlagsLoaded(): (errorsLoading: Boolean) -> Unit { + val generation = featureFlagsGeneration + return { errorsLoading -> startSessionReplayOnFeatureFlagsLoaded() - notifyFeatureFlagsListeners(errorsLoading) + 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 fun startSessionReplayOnFeatureFlagsLoaded() { @@ -226,9 +241,15 @@ public class PostHog private constructor( } } - private fun notifyFeatureFlagsListeners(errorsLoading: Boolean) { + private fun notifyFeatureFlagsListeners( + errorsLoading: Boolean, + generation: Int, + ) { val loaded = PostHogFeatureFlagsLoaded(remoteConfig?.getFeatureFlags() ?: emptyMap(), errorsLoading) - lastFeatureFlagsLoaded = loaded + synchronized(featureFlagsGenerationLock) { + if (featureFlagsGeneration != generation) return + lastFeatureFlagsLoaded = loaded + } featureFlagsListeners.forEach { deliverFeatureFlags(it, loaded) } } @@ -446,13 +467,14 @@ public class PostHog private constructor( if (remoteConfig?.hasBootstrapFlags() == true) { startSessionReplayOnFeatureFlagsLoaded() // listeners run on the remote config thread, queued ahead of setup's own load - remoteConfigExecutor.executeSafely { notifyFeatureFlagsListeners(errorsLoading = false) } + val generation = featureFlagsGeneration + remoteConfigExecutor.executeSafely { notifyFeatureFlagsListeners(errorsLoading = false, generation) } notifyFeatureFlagsCallback(config.onFeatureFlags, remoteConfig?.getFeatureFlags(), errorsLoading = false) } when { config.remoteConfig -> loadRemoteConfigRequest( - internalOnFeatureFlagsLoaded, + internalOnFeatureFlagsLoaded(), config.onFeatureFlags, ) @@ -681,7 +703,7 @@ public class PostHog private constructor( pushSubscriptionManager = null featureFlagsCalled.clear() - lastFeatureFlagsLoaded = null + clearFeatureFlagsLoaded() lastScreenName = null synchronized(replayDebugPropertiesLock) { lastReplayDebugPropertiesAt = null @@ -1990,7 +2012,7 @@ public class PostHog private constructor( return } loadFeatureFlagsRequest( - internalOnFeatureFlags = internalOnFeatureFlagsLoaded, + internalOnFeatureFlags = internalOnFeatureFlagsLoaded(), onFeatureFlags = onFeatureFlags, ) } @@ -2302,7 +2324,7 @@ public class PostHog private constructor( getPreferences().clear(except = except.toList()) } remoteConfig?.clear() - lastFeatureFlagsLoaded = null + clearFeatureFlagsLoaded() featureFlagsCalled.clear() lastScreenName = null synchronized(cachedPersonPropertiesLock) { diff --git a/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt b/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt index 41f9823fb..a8c58fe6a 100644 --- a/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogFeatureFlagsTest.kt @@ -467,6 +467,26 @@ internal class PostHogFeatureFlagsTest { 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 `onFeatureFlags listener is not invoked by a reload while disabled`() { val sut = getSut(mockHttp().url("/").toString(), preloadFeatureFlags = false) diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index ae7bb6b52..784621099 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -1517,9 +1517,9 @@ internal class PostHogTest { config.sessionReplay = true @Suppress("UNCHECKED_CAST") val onFlagsLoaded = - PostHog::class.java.getDeclaredField("internalOnFeatureFlagsLoaded") + PostHog::class.java.getDeclaredMethod("internalOnFeatureFlagsLoaded") .apply { isAccessible = true } - .get(sut) as (Boolean) -> Unit + .invoke(sut) as (Boolean) -> Unit // optOut runs while the flags-loaded callback is between reading the handler and starting it var optOut: Thread? = null