From 64a8671fea7b0169adf12c4bd19262fb5af3ea88 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 21 Oct 2026 11:54:39 -0500 Subject: [PATCH 1/3] feat: [SDK-5581] share startup flags latching across hosts - StartupFlagsLatch reads a host-provided IStartupFlagsStore once per process and mirrors the APP_STARTUP subset of remote flags back to it, writing only when it changed. - StartupFlags reports where the values came from (store, missing, unreadable, no store) and rejects non APP_STARTUP flags. - FeatureManager.refreshAtStartup latches APP_STARTUP flags from that snapshot, so isEnabled matches code that ran before init. Later refreshes never latch APP_STARTUP flags from remote keys. Co-authored-by: Cursor --- .../com/onesignal/features/FeatureManager.kt | 95 ++++++++----- .../com/onesignal/features/StartupFlags.kt | 81 +++++++++++ .../onesignal/features/StartupFlagsTest.kt | 134 ++++++++++++++++++ 3 files changed, 277 insertions(+), 33 deletions(-) create mode 100644 kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt create mode 100644 kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt index 243e9d0..519c89f 100644 --- a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt @@ -34,6 +34,9 @@ class FeatureManager { private var featureStates: Map = emptyMap() private var hasCompletedFirstRefresh = false + // Once set, APP_STARTUP flags latch from here and never from remote keys. + private var startupKeys: Set? = null + fun isEnabled(feature: FeatureFlag): Boolean = lock.withLock { featureStates[feature] ?: false @@ -69,45 +72,71 @@ class FeatureManager { remoteKeys: List, applyAppStartupFlags: Boolean, localOverrides: List, + ): List = lock.withLock { refreshLocked(remoteKeys, applyAppStartupFlags, localOverrides) } + + fun refreshAtStartup( + remoteKeys: List, + startup: StartupFlags, + ): List = refreshAtStartup(remoteKeys, startup, emptyList()) + + /** + * First refresh for a host that latches APP_STARTUP flags from [startup] (see [StartupFlagsLatch]). + * IMMEDIATE flags still follow [remoteKeys], and later [refresh] calls never latch APP_STARTUP + * flags from remote keys, so [isEnabled] matches what code running before init latched. + */ + fun refreshAtStartup( + remoteKeys: List, + startup: StartupFlags, + localOverrides: List, ): List = lock.withLock { - val enabledKeys = - (remoteKeys.asSequence() + localOverrides.asSequence()) - .map { canonicalizeFeatureFlagId(it) } - .toSet() + startupKeys = startup.keys + refreshLocked(remoteKeys, true, localOverrides) + } + + // Callers hold [lock]; PlatformLock is not reentrant on iOS. + private fun refreshLocked( + remoteKeys: List, + applyAppStartupFlags: Boolean, + localOverrides: List, + ): List { + val overrideKeys = localOverrides.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } + val enabledKeys = remoteKeys.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } + overrideKeys + val startup = startupKeys - val nextStates = featureStates.toMutableMap() - val deferred = mutableListOf() - for (feature in FeatureFlag.entries) { - val desired = feature.isEnabledIn(enabledKeys) - when (feature.activationMode) { - FeatureActivationMode.IMMEDIATE -> nextStates[feature] = desired - FeatureActivationMode.APP_STARTUP -> { - // After the first refresh every catalog flag is present in - // [featureStates], including those that resolved to false. - val alreadyLatched = nextStates.containsKey(feature) - val applyStartup = - !alreadyLatched || - (applyAppStartupFlags && !hasCompletedFirstRefresh) - if (applyStartup) { - nextStates[feature] = desired - } else { - val latched = nextStates[feature] ?: false - if (latched != desired) { - deferred.add( - DeferredFeatureActivation( - key = feature.key, - desiredEnabled = desired, - latchedEnabled = latched, - ), - ) - } + val nextStates = featureStates.toMutableMap() + val deferred = mutableListOf() + for (feature in FeatureFlag.entries) { + val desired = feature.isEnabledIn(enabledKeys) + when (feature.activationMode) { + FeatureActivationMode.IMMEDIATE -> nextStates[feature] = desired + FeatureActivationMode.APP_STARTUP -> { + // After the first refresh every catalog flag is present in + // [featureStates], including those that resolved to false. + val alreadyLatched = nextStates.containsKey(feature) + val applyStartup = + !alreadyLatched || + (applyAppStartupFlags && !hasCompletedFirstRefresh) + if (applyStartup) { + nextStates[feature] = + if (startup != null) feature.key in startup || feature.key in overrideKeys else desired + } else { + val latched = nextStates[feature] ?: false + if (latched != desired) { + deferred.add( + DeferredFeatureActivation( + key = feature.key, + desiredEnabled = desired, + latchedEnabled = latched, + ), + ) } } } } - featureStates = nextStates - hasCompletedFirstRefresh = true - deferred } + featureStates = nextStates + hasCompletedFirstRefresh = true + return deferred + } } diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt new file mode 100644 index 0000000..b72c32a --- /dev/null +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt @@ -0,0 +1,81 @@ +package com.onesignal.features + +/** Where a process's [StartupFlags] came from. Anything but [STORE] means every startup flag is off. */ +enum class StartupFlagsSource { + STORE, + MISSING, + UNREADABLE, + NO_STORE, +} + +/** The [FeatureActivationMode.APP_STARTUP] flags as one process latched them. */ +class StartupFlags( + val source: StartupFlagsSource, + val keys: Set, +) { + /** Only APP_STARTUP flags: IMMEDIATE flags change mid-run, so a startup copy would go stale. */ + @Throws(IllegalArgumentException::class) + fun isEnabled(flag: FeatureFlag): Boolean { + require(flag.activationMode == FeatureActivationMode.APP_STARTUP) { "${flag.key} is not APP_STARTUP" } + return flag.key in keys + } +} + +/** + * Platform storage for the startup flags: a small store the host can read before SDK init without + * loading its main persisted state. Android uses its own SharedPreferences file. + */ +interface IStartupFlagsStore { + /** Keys from the last [write], or null when nothing was ever written. May throw when unreadable. */ + fun read(): Set? + + /** Must be durable when it returns: the next process reads it at startup. */ + fun write(keys: Set) +} + +/** Canonical keys of the APP_STARTUP catalog flags present in [remoteKeys]. */ +fun appStartupKeysIn(remoteKeys: Collection): Set { + val canonical = remoteKeys.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } + return FeatureFlag.entries + .filter { it.activationMode == FeatureActivationMode.APP_STARTUP && it.key in canonical } + .mapTo(LinkedHashSet()) { it.key } +} + +/** + * Reads the startup flags once per process and keeps the store in step with the remote flags. + * Hosts keep one instance for the process. The first [snapshot] wins, so code that runs before + * init and [FeatureManager.refreshAtStartup] latch the same values. + */ +class StartupFlagsLatch { + private val lock = PlatformLock() + private var snapshot: StartupFlags? = null + + /** [store] is only called on the first read; null means the host has no store yet. */ + fun snapshot(store: () -> IStartupFlagsStore?): StartupFlags = + lock.withLock { + snapshot ?: read(store()).also { snapshot = it } + } + + /** Writes the APP_STARTUP subset of [remoteKeys] for the next process. Returns false when unchanged. */ + fun mirror( + store: IStartupFlagsStore, + remoteKeys: Collection, + ): Boolean { + val wanted = appStartupKeysIn(remoteKeys) + if (store.read() == wanted) return false + store.write(wanted) + return true + } + + @Suppress("TooGenericExceptionCaught") + private fun read(store: IStartupFlagsStore?): StartupFlags { + if (store == null) return StartupFlags(StartupFlagsSource.NO_STORE, emptySet()) + val stored = + try { + store.read() + } catch (t: Throwable) { + return StartupFlags(StartupFlagsSource.UNREADABLE, emptySet()) + } ?: return StartupFlags(StartupFlagsSource.MISSING, emptySet()) + return StartupFlags(StartupFlagsSource.STORE, appStartupKeysIn(stored)) + } +} diff --git a/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt new file mode 100644 index 0000000..e11ab91 --- /dev/null +++ b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt @@ -0,0 +1,134 @@ +package com.onesignal.features + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +private class FakeStartupFlagsStore( + var stored: Set? = null, + var failReads: Boolean = false, +) : IStartupFlagsStore { + var reads = 0 + var writes = 0 + + override fun read(): Set? { + reads++ + if (failReads) throw IllegalStateException("locked") + return stored + } + + override fun write(keys: Set) { + writes++ + stored = keys + } +} + +class StartupFlagsTest { + private val startup = FeatureFlag.SDK_CUSTOM_LOGGING + private val immediate = FeatureFlag.SDK_IDENTITY_VERIFICATION + + @Test + fun appStartupKeysKeepsOnlyStartupCatalogFlagsInCanonicalCase() { + val keys = appStartupKeysIn(listOf("SDK_Custom_Logging", immediate.key, "not_in_catalog")) + assertEquals(setOf(startup.key), keys) + } + + @Test + fun snapshotReadsTheStoreOnce() { + val store = FakeStartupFlagsStore(setOf(startup.key)) + val latch = StartupFlagsLatch() + + assertTrue(latch.snapshot { store }.isEnabled(startup)) + store.stored = emptySet() + + assertTrue(latch.snapshot { store }.isEnabled(startup)) + assertEquals(1, store.reads) + } + + @Test + fun snapshotSourcesAreAllOffExceptStore() { + assertEquals(StartupFlagsSource.MISSING, StartupFlagsLatch().snapshot { FakeStartupFlagsStore() }.source) + assertEquals(StartupFlagsSource.NO_STORE, StartupFlagsLatch().snapshot { null }.source) + val unreadable = StartupFlagsLatch().snapshot { FakeStartupFlagsStore(setOf(startup.key), failReads = true) } + assertEquals(StartupFlagsSource.UNREADABLE, unreadable.source) + assertFalse(unreadable.isEnabled(startup)) + } + + @Test + fun snapshotDropsKeysThatAreNoLongerStartupFlags() { + val flags = StartupFlagsLatch().snapshot { FakeStartupFlagsStore(setOf(startup.key, immediate.key, "retired")) } + assertEquals(setOf(startup.key), flags.keys) + } + + @Test + fun isEnabledRejectsImmediateFlags() { + val flags = StartupFlagsLatch().snapshot { FakeStartupFlagsStore(emptySet()) } + assertFailsWith { flags.isEnabled(immediate) } + } + + @Test + fun mirrorWritesOnlyWhenTheStartupSubsetChanges() { + val store = FakeStartupFlagsStore() + val latch = StartupFlagsLatch() + + assertTrue(latch.mirror(store, listOf(startup.key, immediate.key))) + assertFalse(latch.mirror(store, listOf("SDK_CUSTOM_LOGGING"))) + assertFalse(latch.mirror(store, listOf(startup.key, "not_in_catalog"))) + + assertEquals(setOf(startup.key), store.stored) + assertEquals(1, store.writes) + } + + @Test + fun turningAFlagOffReachesTheNextProcess() { + val store = FakeStartupFlagsStore() + val firstProcess = StartupFlagsLatch() + firstProcess.mirror(store, listOf(startup.key)) + assertTrue(StartupFlagsLatch().snapshot { store }.isEnabled(startup)) + + val running = StartupFlagsLatch() + assertTrue(running.snapshot { store }.isEnabled(startup)) + running.mirror(store, emptyList()) + assertTrue(running.snapshot { store }.isEnabled(startup)) + + val nextProcess = StartupFlagsLatch().snapshot { store } + assertEquals(StartupFlagsSource.STORE, nextProcess.source) + assertFalse(nextProcess.isEnabled(startup)) + } + + @Test + fun refreshAtStartupLatchesStartupFlagsFromTheSnapshotAndImmediateFromRemote() { + val manager = FeatureManager() + val snapshot = StartupFlags(StartupFlagsSource.STORE, emptySet()) + + manager.refreshAtStartup(listOf(startup.key, immediate.key), snapshot) + + assertFalse(manager.isEnabled(startup)) + assertTrue(manager.isEnabled(immediate)) + assertEquals(listOf(immediate.key), manager.enabledFeatureKeys()) + } + + @Test + fun laterRefreshesDeferStartupChangesAgainstTheSnapshot() { + val manager = FeatureManager() + manager.refreshAtStartup(emptyList(), StartupFlags(StartupFlagsSource.STORE, setOf(startup.key))) + assertTrue(manager.isEnabled(startup)) + + val deferred = manager.refresh(emptyList(), applyAppStartupFlags = false) + + assertTrue(manager.isEnabled(startup)) + assertEquals(listOf(DeferredFeatureActivation(startup.key, desiredEnabled = false, latchedEnabled = true)), deferred) + } + + @Test + fun aSecondStartupRefreshCannotLatchFromRemoteKeys() { + val manager = FeatureManager() + manager.refreshAtStartup(emptyList(), StartupFlags(StartupFlagsSource.MISSING, emptySet())) + + manager.refresh(listOf(startup.key), applyAppStartupFlags = true) + + assertFalse(manager.isEnabled(startup)) + } +} From 8f5e5d8fda6d9bb834300b49ae142335599eb5d5 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 21 Oct 2026 14:08:58 -0500 Subject: [PATCH 2/3] fix: [SDK-5581] make the startup flags store Swift-safe - Mark IStartupFlagsStore.read/write and StartupFlagsLatch.mirror @Throws, so a Swift store can report an unreadable store or failed write instead of crashing, and Swift callers can catch mirror failures. - StartupFlags keys are canonical and empty unless the source is STORE, so isEnabled and refreshAtStartup can never report a flag on from a missing or unreadable store. - Point the FeatureManager class doc at refreshAtStartup, and note that store implementations must not call back into the latch. Co-authored-by: Cursor --- .../com/onesignal/features/FeatureManager.kt | 21 ++++------------ .../com/onesignal/features/StartupFlags.kt | 24 ++++++++++++------- .../onesignal/features/StartupFlagsTest.kt | 24 +++++++++++++++++++ 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt index 519c89f..9dcdcc4 100644 --- a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt @@ -14,20 +14,8 @@ data class DeferredFeatureActivation( ) /** - * In-process latch for [FeatureFlag] state. - * - * Hosts pass the latest trusted remote keys (from cache at process start, then - * from [FeatureFlagsClient] after a successful fetch). Persistence, HTTP, and - * lifecycle stay on the platform. - * - * Call [refresh] with [applyAppStartupFlags] `true` once at process start, then - * `false` for later updates so [FeatureActivationMode.APP_STARTUP] flags do not - * flip mid-run. A second `true` is ignored for already-latched APP_STARTUP flags; - * [FeatureManager] tracks that initialization itself. [refresh] returns any - * APP_STARTUP changes that were held. - * - * State updates are guarded by an internal lock so [isEnabled] / [refresh] / - * [enabledFeatureKeys] are safe across host threads. + * Thread-safe per-process latch for [FeatureFlag] state. Call [refreshAtStartup] once with the [StartupFlagsLatch] snapshot, then [refresh] with `false` after fetches. + * `refresh(applyAppStartupFlags = true)` never reads the startup store, so APP_STARTUP flags would not match code that ran before init. */ class FeatureManager { private val lock = PlatformLock() @@ -80,9 +68,8 @@ class FeatureManager { ): List = refreshAtStartup(remoteKeys, startup, emptyList()) /** - * First refresh for a host that latches APP_STARTUP flags from [startup] (see [StartupFlagsLatch]). - * IMMEDIATE flags still follow [remoteKeys], and later [refresh] calls never latch APP_STARTUP - * flags from remote keys, so [isEnabled] matches what code running before init latched. + * First refresh: APP_STARTUP flags latch from [startup], IMMEDIATE flags from [remoteKeys]. + * Later [refresh] calls never latch APP_STARTUP flags from remote keys. */ fun refreshAtStartup( remoteKeys: List, diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt index b72c32a..cfe6260 100644 --- a/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt @@ -11,8 +11,12 @@ enum class StartupFlagsSource { /** The [FeatureActivationMode.APP_STARTUP] flags as one process latched them. */ class StartupFlags( val source: StartupFlagsSource, - val keys: Set, + keys: Set, ) { + /** Canonical keys, and always empty unless [source] is [StartupFlagsSource.STORE]. */ + val keys: Set = + if (source == StartupFlagsSource.STORE) keys.mapTo(LinkedHashSet()) { canonicalizeFeatureFlagId(it) } else emptySet() + /** Only APP_STARTUP flags: IMMEDIATE flags change mid-run, so a startup copy would go stale. */ @Throws(IllegalArgumentException::class) fun isEnabled(flag: FeatureFlag): Boolean { @@ -22,14 +26,16 @@ class StartupFlags( } /** - * Platform storage for the startup flags: a small store the host can read before SDK init without - * loading its main persisted state. Android uses its own SharedPreferences file. + * Platform storage for the startup flags, readable before SDK init without loading the host's main + * persisted state. Implementations must not call back into [StartupFlagsLatch]: its lock is not reentrant. */ interface IStartupFlagsStore { - /** Keys from the last [write], or null when nothing was ever written. May throw when unreadable. */ + /** Keys from the last [write], or null only when nothing was ever written. Throw when unreadable. */ + @Throws(Exception::class) fun read(): Set? - /** Must be durable when it returns: the next process reads it at startup. */ + /** Must be durable when it returns, since the next process reads it at startup. Throw on failure. */ + @Throws(Exception::class) fun write(keys: Set) } @@ -42,21 +48,21 @@ fun appStartupKeysIn(remoteKeys: Collection): Set { } /** - * Reads the startup flags once per process and keeps the store in step with the remote flags. - * Hosts keep one instance for the process. The first [snapshot] wins, so code that runs before - * init and [FeatureManager.refreshAtStartup] latch the same values. + * Reads the startup flags once per process and mirrors remote flags back for the next one. + * Keep one instance per process: the first [snapshot] wins, including an unreadable one. */ class StartupFlagsLatch { private val lock = PlatformLock() private var snapshot: StartupFlags? = null - /** [store] is only called on the first read; null means the host has no store yet. */ + /** [store] runs only until a snapshot is taken; null means the host has no store yet. */ fun snapshot(store: () -> IStartupFlagsStore?): StartupFlags = lock.withLock { snapshot ?: read(store()).also { snapshot = it } } /** Writes the APP_STARTUP subset of [remoteKeys] for the next process. Returns false when unchanged. */ + @Throws(Exception::class) fun mirror( store: IStartupFlagsStore, remoteKeys: Collection, diff --git a/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt index e11ab91..cc4c1a5 100644 --- a/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt +++ b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt @@ -68,6 +68,30 @@ class StartupFlagsTest { assertFailsWith { flags.isEnabled(immediate) } } + @Test + fun onlyAStoreSourceCanReportAFlagOn() { + for (source in StartupFlagsSource.entries - StartupFlagsSource.STORE) { + val flags = StartupFlags(source, setOf(startup.key)) + assertEquals(emptySet(), flags.keys, "$source") + assertFalse(flags.isEnabled(startup), "$source") + } + assertTrue(StartupFlags(StartupFlagsSource.STORE, setOf(startup.key)).isEnabled(startup)) + } + + @Test + fun storeKeysAreCanonical() { + val flags = StartupFlags(StartupFlagsSource.STORE, setOf("SDK_Custom_Logging")) + assertEquals(setOf(startup.key), flags.keys) + assertTrue(flags.isEnabled(startup)) + } + + @Test + fun refreshAtStartupIgnoresKeysFromANonStoreSource() { + val manager = FeatureManager() + manager.refreshAtStartup(listOf(startup.key), StartupFlags(StartupFlagsSource.UNREADABLE, setOf(startup.key))) + assertFalse(manager.isEnabled(startup)) + } + @Test fun mirrorWritesOnlyWhenTheStartupSubsetChanges() { val store = FakeStartupFlagsStore() From 05c808f891d59e2b3127609758b59ae01892388a Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 21 Oct 2026 15:27:53 -0500 Subject: [PATCH 3/3] fix: address startup flags review - Mirror writes when the stored file can't be read. - refreshAtStartup latches the snapshot even after an earlier refresh. - Mirror serializes under its own lock and takes a supplier for the latest keys. - Startup flags that disagree with remote come back as deferred activations. - Cancellation is rethrown instead of cached as UNREADABLE. - isEnabled returns false for IMMEDIATE flags instead of throwing. - Header check covers the startup flag types. Co-authored-by: Cursor --- kmp/build.gradle | 3 +- .../com/onesignal/features/FeatureManager.kt | 19 ++++--- .../com/onesignal/features/StartupFlags.kt | 44 +++++++++++----- .../onesignal/features/StartupFlagsTest.kt | 51 +++++++++++++++++-- 4 files changed, 93 insertions(+), 24 deletions(-) diff --git a/kmp/build.gradle b/kmp/build.gradle index 2c5202c..5d485d4 100644 --- a/kmp/build.gradle +++ b/kmp/build.gradle @@ -347,7 +347,8 @@ if (!publishSharedKmp) { 'FeatureFlagsClient', 'IFeatureFlagsHttp', 'FeatureManager', 'FeatureFlag', 'DeferredFeatureActivation', 'RemoteFeatureFlagsFetchOutcome', 'RemoteFeatureFlagsResult', 'RemoteFeatureFlagsUnavailableReason', - 'FeatureFlagMetadata'].each { publicType -> + 'FeatureFlagMetadata', 'StartupFlags', 'StartupFlagsSource', 'StartupFlagsLatch', + 'IStartupFlagsStore'].each { publicType -> if (!headerText.contains("swift_name(\"${publicType}\")")) { throw new GradleException("Generated Objective-C header does not expose ${publicType}") } diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt index 9dcdcc4..04bba56 100644 --- a/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/FeatureManager.kt @@ -68,8 +68,8 @@ class FeatureManager { ): List = refreshAtStartup(remoteKeys, startup, emptyList()) /** - * First refresh: APP_STARTUP flags latch from [startup], IMMEDIATE flags from [remoteKeys]. - * Later [refresh] calls never latch APP_STARTUP flags from remote keys. + * APP_STARTUP flags latch from [startup] (even after an earlier [refresh]), IMMEDIATE flags from + * [remoteKeys]. Startup flags whose remote value differs are returned as deferred. Only the first call latches. */ fun refreshAtStartup( remoteKeys: List, @@ -77,8 +77,9 @@ class FeatureManager { localOverrides: List, ): List = lock.withLock { - startupKeys = startup.keys - refreshLocked(remoteKeys, true, localOverrides) + val firstSnapshot = startupKeys == null + if (firstSnapshot) startupKeys = startup.keys + refreshLocked(remoteKeys, true, localOverrides, latchFromSnapshot = firstSnapshot) } // Callers hold [lock]; PlatformLock is not reentrant on iOS. @@ -86,6 +87,7 @@ class FeatureManager { remoteKeys: List, applyAppStartupFlags: Boolean, localOverrides: List, + latchFromSnapshot: Boolean = false, ): List { val overrideKeys = localOverrides.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } val enabledKeys = remoteKeys.mapTo(HashSet()) { canonicalizeFeatureFlagId(it) } + overrideKeys @@ -102,11 +104,14 @@ class FeatureManager { // [featureStates], including those that resolved to false. val alreadyLatched = nextStates.containsKey(feature) val applyStartup = - !alreadyLatched || + latchFromSnapshot || !alreadyLatched || (applyAppStartupFlags && !hasCompletedFirstRefresh) if (applyStartup) { - nextStates[feature] = - if (startup != null) feature.key in startup || feature.key in overrideKeys else desired + val latched = if (startup != null) feature.key in startup || feature.key in overrideKeys else desired + nextStates[feature] = latched + if (latched != desired) { + deferred.add(DeferredFeatureActivation(feature.key, desiredEnabled = desired, latchedEnabled = latched)) + } } else { val latched = nextStates[feature] ?: false if (latched != desired) { diff --git a/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt index cfe6260..20097d6 100644 --- a/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt +++ b/kmp/src/commonMain/kotlin/com/onesignal/features/StartupFlags.kt @@ -1,5 +1,7 @@ package com.onesignal.features +import kotlin.coroutines.cancellation.CancellationException + /** Where a process's [StartupFlags] came from. Anything but [STORE] means every startup flag is off. */ enum class StartupFlagsSource { STORE, @@ -17,12 +19,8 @@ class StartupFlags( val keys: Set = if (source == StartupFlagsSource.STORE) keys.mapTo(LinkedHashSet()) { canonicalizeFeatureFlagId(it) } else emptySet() - /** Only APP_STARTUP flags: IMMEDIATE flags change mid-run, so a startup copy would go stale. */ - @Throws(IllegalArgumentException::class) - fun isEnabled(flag: FeatureFlag): Boolean { - require(flag.activationMode == FeatureActivationMode.APP_STARTUP) { "${flag.key} is not APP_STARTUP" } - return flag.key in keys - } + /** False for IMMEDIATE flags: they change mid-run, so a startup copy would go stale. */ + fun isEnabled(flag: FeatureFlag): Boolean = flag.activationMode == FeatureActivationMode.APP_STARTUP && flag.key in keys } /** @@ -55,6 +53,9 @@ class StartupFlagsLatch { private val lock = PlatformLock() private var snapshot: StartupFlags? = null + // Separate from [lock]: a slow durable write must not block a snapshot read, often on main. + private val mirrorLock = PlatformLock() + /** [store] runs only until a snapshot is taken; null means the host has no store yet. */ fun snapshot(store: () -> IStartupFlagsStore?): StartupFlags = lock.withLock { @@ -66,13 +67,31 @@ class StartupFlagsLatch { fun mirror( store: IStartupFlagsStore, remoteKeys: Collection, - ): Boolean { - val wanted = appStartupKeysIn(remoteKeys) - if (store.read() == wanted) return false - store.write(wanted) - return true - } + ): Boolean = mirror(store) { remoteKeys } + + /** [currentKeys] runs under the mirror lock, so racing mirrors always leave the latest keys stored. */ + @Throws(Exception::class) + @Suppress("TooGenericExceptionCaught", "SwallowedException") + fun mirror( + store: IStartupFlagsStore, + currentKeys: () -> Collection, + ): Boolean = + mirrorLock.withLock { + val wanted = appStartupKeysIn(currentKeys()) + // An unreadable store counts as changed: skipping the write would leave it unreadable for good. + val stored = + try { + store.read() + } catch (t: Throwable) { + if (t is CancellationException) throw t + null + } + if (stored == wanted) return@withLock false + store.write(wanted) + true + } + // Errors other than cancellation are cached too: re-reading could give a later caller a different answer. @Suppress("TooGenericExceptionCaught") private fun read(store: IStartupFlagsStore?): StartupFlags { if (store == null) return StartupFlags(StartupFlagsSource.NO_STORE, emptySet()) @@ -80,6 +99,7 @@ class StartupFlagsLatch { try { store.read() } catch (t: Throwable) { + if (t is CancellationException) throw t return StartupFlags(StartupFlagsSource.UNREADABLE, emptySet()) } ?: return StartupFlags(StartupFlagsSource.MISSING, emptySet()) return StartupFlags(StartupFlagsSource.STORE, appStartupKeysIn(stored)) diff --git a/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt index cc4c1a5..35a4d83 100644 --- a/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt +++ b/kmp/src/commonTest/kotlin/com/onesignal/features/StartupFlagsTest.kt @@ -1,5 +1,6 @@ package com.onesignal.features +import kotlin.coroutines.cancellation.CancellationException import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -9,13 +10,14 @@ import kotlin.test.assertTrue private class FakeStartupFlagsStore( var stored: Set? = null, var failReads: Boolean = false, + var readFailure: () -> Throwable = { IllegalStateException("locked") }, ) : IStartupFlagsStore { var reads = 0 var writes = 0 override fun read(): Set? { reads++ - if (failReads) throw IllegalStateException("locked") + if (failReads) throw readFailure() return stored } @@ -63,9 +65,50 @@ class StartupFlagsTest { } @Test - fun isEnabledRejectsImmediateFlags() { - val flags = StartupFlagsLatch().snapshot { FakeStartupFlagsStore(emptySet()) } - assertFailsWith { flags.isEnabled(immediate) } + fun isEnabledIsFalseForImmediateFlags() { + val flags = StartupFlags(StartupFlagsSource.STORE, setOf(immediate.key)) + assertFalse(flags.isEnabled(immediate)) + } + + @Test + fun aCancelledReadIsNotCachedAsUnreadable() { + val store = FakeStartupFlagsStore(setOf(startup.key), failReads = true, readFailure = { CancellationException("cancelled") }) + val latch = StartupFlagsLatch() + assertFailsWith { latch.snapshot { store } } + + store.failReads = false + assertTrue(latch.snapshot { store }.isEnabled(startup)) + } + + @Test + fun mirrorWritesWhenTheStoreIsUnreadable() { + val store = FakeStartupFlagsStore(failReads = true) + + assertTrue(StartupFlagsLatch().mirror(store, listOf(startup.key))) + + assertEquals(setOf(startup.key), store.stored) + store.failReads = false + assertTrue(StartupFlagsLatch().snapshot { store }.isEnabled(startup)) + } + + @Test + fun refreshAtStartupLatchesTheSnapshotEvenAfterAnEarlierRefresh() { + val manager = FeatureManager() + manager.refresh(listOf(startup.key), applyAppStartupFlags = true) + assertTrue(manager.isEnabled(startup)) + + manager.refreshAtStartup(listOf(startup.key), StartupFlags(StartupFlagsSource.STORE, emptySet())) + + assertFalse(manager.isEnabled(startup)) + } + + @Test + fun refreshAtStartupReportsASnapshotThatDisagreesWithRemote() { + val manager = FeatureManager() + + val deferred = manager.refreshAtStartup(listOf(startup.key), StartupFlags(StartupFlagsSource.MISSING, emptySet())) + + assertEquals(listOf(DeferredFeatureActivation(startup.key, desiredEnabled = true, latchedEnabled = false)), deferred) } @Test