diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index ee61d46732..35d538aace 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit ee61d4673265876e5cab6328b7a0745f79b7a5aa +Subproject commit 35d538aacecc28b493bb53b83c2b2e31085a46a2 diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/AppContext.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/AppContext.kt new file mode 100644 index 0000000000..9f0615170b --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/AppContext.kt @@ -0,0 +1,30 @@ +package com.onesignal.common + +import android.content.Context + +/** + * The process's application Context, noted by SDK entry points, so code that runs before init + * (or before ApplicationService starts) can read app storage without a Context of its own. + */ +internal object AppContext { + @Volatile + private var context: Context? = null + + fun get(): Context? = context + + /** First caller wins. Only the application Context is kept, so an Activity never leaks here. */ + @Suppress("TooGenericExceptionCaught", "SwallowedException") + fun note(context: Context) { + if (this.context != null) return + this.context = + try { + context.applicationContext + } catch (_: Throwable) { + null + } + } + + internal fun resetForTest() { + context = null + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt index 614903b8f2..e20d9b44ee 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt @@ -79,7 +79,9 @@ internal class CoreModule : IModule { // Params (Config) builder.register().provides() - builder.register().provides() + builder.register() + .provides() + .provides() builder.register().provides() builder.register().provides() builder.register().provides() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializer.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializer.kt index 9fde58fc5b..e910e4fa67 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializer.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializer.kt @@ -3,6 +3,7 @@ package com.onesignal.core.internal.application.impl import android.app.Application import android.content.Context import androidx.startup.Initializer +import com.onesignal.common.AppContext import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.core.internal.application.IApplicationService @@ -21,6 +22,7 @@ import com.onesignal.core.internal.application.IApplicationService */ class ActivityLifecycleInitializer : Initializer { override fun create(context: Context): IApplicationService { + AppContext.note(context) OneSignalDispatchers.prewarm() val applicationService = ApplicationService.getInstance() (context.applicationContext as? Application)?.let { application -> diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 2a261acf88..7cf2f79d57 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -1,13 +1,18 @@ package com.onesignal.core.internal.features +import android.content.Context import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.internal.logging.Logging +import com.onesignal.features.DeferredFeatureActivation import com.onesignal.features.FeatureFlag import com.onesignal.features.FeatureFlagMetadata +import com.onesignal.features.StartupFlags import com.onesignal.features.FeatureManager as SharedFeatureManager /** @@ -38,31 +43,44 @@ interface IFeatureManager { } /** - * Android host for the shared [SharedFeatureManager] latch. - * - * Persistence ([ConfigModel]) and store subscriptions stay here. Catalog, activation - * modes, and isEnabled latching live in KMP so iOS can call the same - * [SharedFeatureManager.refresh] with cached keys at process start - * (`applyAppStartupFlags = true`) and again after later fetches (`false`). - * - * Shared [SharedFeatureManager] is thread-safe; this host does not add an extra lock. + * Android host for the shared [SharedFeatureManager]: owns [ConfigModel] persistence, the startup + * flags file, and store subscriptions. Latching lives in KMP. See README.md in this package. */ @Suppress("TooGenericExceptionCaught") internal class FeatureManager( private val configModelStore: ConfigModelStore, -) : IFeatureManager, ISingletonModelStoreChangeHandler { + private val applicationService: IApplicationService, +) : IFeatureManager, IStartableService, ISingletonModelStoreChangeHandler { private val latch = SharedFeatureManager() + // Writes wait for start() and run on the caller's thread, so staying off main is caller-dependent: + // StartupService runs start() on the default lane, and the flag fetches update ConfigModel on IO. + @Volatile + private var started = false + init { Logging.debug("OneSignal: FeatureManager initializing from cached config features") + if (localFeatureOverrides.isNotEmpty()) { + Logging.warn("OneSignal: Local feature override enabled for testing only: $localFeatureOverrides") + } + // APP_STARTUP flags come from the snapshot pre-init code also latched from, so telemetry + // never reports a startup flag the running process is not using. + val startup = StartupFlagsCache.snapshot { appContextOrNull() } + Logging.debug("OneSignal: startup flags source=${startup.source} enabled=${startup.keys}") try { - refreshFrom(configModelStore.model, applyAppStartupFlags = true) + logDeferred(latch.refreshAtStartup(configModelStore.model.sdkRemoteFeatureFlags, startup, localFeatureOverrides)) } catch (t: Throwable) { Logging.error("OneSignal: Failed to initialize feature states from cached config", t) + latch.refreshAtStartup(emptyList(), startup, localFeatureOverrides) } configModelStore.subscribe(this) } + override fun start() { + started = true + mirrorStartupFlags() + } + override fun isEnabled(feature: FeatureFlag): Boolean = latch.isEnabled(feature) override fun enabledFeatureKeys(): List = latch.enabledFeatureKeys() @@ -81,9 +99,10 @@ internal class FeatureManager( tag: String, ) { Logging.debug("OneSignal: FeatureManager.onModelReplaced(tag=$tag)") + if (started) mirrorStartupFlags() if (tag == ModelChangeTags.HYDRATE || tag == ModelChangeTags.NORMAL) { try { - refreshFrom(model, applyAppStartupFlags = false) + refreshFrom(model) } catch (t: Throwable) { Logging.error("OneSignal: Failed to refresh features on model replace", t) } @@ -99,25 +118,22 @@ internal class FeatureManager( args.property == ConfigModel::sdkRemoteFeatureFlagMetadata.name ) { Logging.debug("OneSignal: FeatureManager.onModelUpdated(property=${args.property}, tag=$tag)") + if (started && args.property == ConfigModel::sdkRemoteFeatureFlags.name) { + mirrorStartupFlags() + } try { - refreshFrom(configModelStore.model, applyAppStartupFlags = false) + refreshFrom(configModelStore.model) } catch (t: Throwable) { Logging.error("OneSignal: Failed to refresh features on model update", t) } } } - private fun refreshFrom( - model: ConfigModel, - applyAppStartupFlags: Boolean, - ) { - if (localFeatureOverrides.isNotEmpty()) { - Logging.warn( - "OneSignal: Local feature override enabled for testing only: $localFeatureOverrides", - ) - } - val deferred = - latch.refresh(model.sdkRemoteFeatureFlags, applyAppStartupFlags, localFeatureOverrides) + private fun refreshFrom(model: ConfigModel) { + logDeferred(latch.refresh(model.sdkRemoteFeatureFlags, false, localFeatureOverrides)) + } + + private fun logDeferred(deferred: List) { for (change in deferred) { Logging.info( "OneSignal: Feature ${change.key} changed remotely to ${change.desiredEnabled} " + @@ -126,7 +142,30 @@ internal class FeatureManager( } } + // Reads the store's model inside the write lock: a list captured by the caller could be stale. + private fun mirrorStartupFlags() { + try { + StartupFlagsCache.write(applicationService.appContext) { configModelStore.model.sdkRemoteFeatureFlags } + } catch (t: Throwable) { + Logging.error("OneSignal: failed to mirror startup flags: ${t.message}", t) + } + } + + @Suppress("SwallowedException") + private fun appContextOrNull(): Context? = + try { + applicationService.appContext + } catch (t: Throwable) { + null + } + companion object { + /** APP_STARTUP flags for code that runs before SDK init. Same values [isEnabled] reports. */ + fun startupFlags(context: () -> Context?): StartupFlags = StartupFlagsCache.snapshot(context) + + /** Forgets this process's startup flags, as a new process would. */ + internal fun resetStartupFlagsForTest() = StartupFlagsCache.setSnapshotForTest(null) + /** * Local-only test hook for forcing features ON without backend config. * Add feature keys here while testing locally, e.g. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/README.md b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/README.md new file mode 100644 index 0000000000..43e4c331a0 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/README.md @@ -0,0 +1,76 @@ +# Startup flags + +`FeatureManager` owns every feature flag. For `APP_STARTUP` flags it also keeps a tiny prefs file, +`OneSignalStartupFlags`, so code that runs before SDK init can read them without loading the whole +OneSignal prefs file (operations queue included) on the main thread. Typical pre-init readers make a +choice that lives for the whole process, such as which thread pools or logging pipeline to build. + +## Adding a startup flag + +1. Add it to `FeatureFlag` in the KMP catalog with `FeatureActivationMode.APP_STARTUP`. +2. Read it: + - After init (anything with an injected `IFeatureManager`): `featureManager.isEnabled(flag)`. + - Before init (no service container yet): `FeatureManager.startupFlags { context }.isEnabled(flag)`. + +Nothing else. Both calls return the same value for the life of the process, unless +`FeatureManager.localFeatureOverrides` is set: local overrides only reach `isEnabled` (see Tests). + +## Where the logic lives + +The snapshot, key filtering, write skipping, and latching are in KMP (`StartupFlagsLatch`, +`StartupFlags`, `FeatureManager.refreshAtStartup`), so iOS gets the same behavior by implementing +`IStartupFlagsStore`. Android only provides that store (`PrefsStartupFlagsStore`) and the +process-wide holder (`StartupFlagsCache`). + +## How it works + +1. Process starts. The first reader, before or after init, triggers a single read of the file. That + snapshot is the process value for every APP_STARTUP flag. +2. `FeatureManager` is created and latches APP_STARTUP flags from the snapshot, IMMEDIATE flags from + `ConfigModel`. +3. `FeatureManager.start()` (an `IStartableService`, so off main) copies the APP_STARTUP subset of + `ConfigModel.sdkRemoteFeatureFlags` into the file. +4. A flags fetch updates `ConfigModel`. IMMEDIATE flags apply now. APP_STARTUP changes, on or off, + are logged as deferred, and the file is rewritten for the next process. The write uses `commit()`, + so it is on disk before the process can die. + +## Rules + +- **Only APP_STARTUP flags.** IMMEDIATE flags change mid-run and the file lags the store, so they are + never written and `StartupFlags.isEnabled` returns false for them. +- **The snapshot is the process value.** Telemetry tags and `isEnabled` must match what early code + latched, so APP_STARTUP flags never come from the store at runtime. +- **Missing or unreadable means off.** A missing file, a read that throws, or no application context + all give an empty snapshot. The source (`STORE`, `MISSING`, `UNREADABLE`, `NO_STORE`) is logged at + debug when `FeatureManager` is built. +- **SDK entry points note the context.** When the caller has no context yet (for example, init's + first log builds `FeatureManager` before `ApplicationService` starts), the snapshot uses the one in + `AppContext`, noted by the startup initializer and at the start of init. No reflection: if nothing + noted one, the snapshot is `NO_STORE` and logs a warning. +- **One value per process, even when unreadable.** The first read wins, including an `UNREADABLE` + one. Later readers in the same process (sync jobs, FCM receivers, prewarm) never re-read and get a + different answer, so code that already latched "off" never disagrees with code that runs later. +- **Device-protected storage.** On API 24+ the file lives in device-protected storage + (`createDeviceProtectedStorageContext()`), so receivers and jobs that start before first unlock + read real values instead of `UNREADABLE`. Below 24 there is no direct boot, so the normal context + is used. Never put anything secret in this file: it is readable while the device is locked. +- **One launch behind after an upgrade.** The file does not exist on the first launch of the version + that introduces it, so APP_STARTUP flags are off for that launch even if the store already has them. +- **Writes are serialized and read the latest flags.** `start()` and a flags fetch can mirror at the + same time. Each write reads `ConfigModel` inside the latch's mirror lock, so the last write to finish + always holds the newest flags, never a list captured before another write landed. A file that can't + be read is rewritten rather than skipped, and so is one whose last `commit()` failed, since + `SharedPreferences` keeps the uncommitted value in memory. +- **No writes before `start()`.** `FeatureManager` can be constructed on main; the file is only written + after start. +- **Main thread cost.** The first reader opens one small XML file. Cheap, but StrictMode can still report + it as a disk read when that reader is on main. + +## Tests + +- `FeatureManager.resetStartupFlagsForTest()` forgets the snapshot, as a new process would. + `OneSignalTestProjectConfig` calls it around every spec. +- `StartupFlagsCache.setSnapshotForTest(...)` installs a fixed snapshot; `readFile` reads the file + without touching the snapshot. +- `FeatureManager.localFeatureOverrides` does not reach the snapshot. Pre-init readers need their own + test hooks. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/StartupFlagsCache.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/StartupFlagsCache.kt new file mode 100644 index 0000000000..4ecdc02770 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/StartupFlagsCache.kt @@ -0,0 +1,101 @@ +package com.onesignal.core.internal.features + +import android.content.Context +import android.content.SharedPreferences +import android.os.Build +import android.util.Log +import com.onesignal.common.AppContext +import com.onesignal.features.IStartupFlagsStore +import com.onesignal.features.StartupFlags +import com.onesignal.features.StartupFlagsLatch + +/** + * Process-wide holder for the shared [StartupFlagsLatch], backed by a small prefs file so pre-init + * code (often on main) never loads the OneSignal store. See README.md in this package. + */ +internal object StartupFlagsCache { + @Volatile + private var latch = StartupFlagsLatch() + + /** The first caller in the process reads the file; everyone after gets the same answer. */ + fun snapshot(context: () -> Context?): StartupFlags = latch.snapshot { storeFor(context() ?: AppContext.get()) } + + // The answer is latched for the whole process, so a missing context must be visible in logcat. + private fun storeFor(context: Context?): IStartupFlagsStore? { + if (context == null) Log.e(PrefsStartupFlagsStore.TAG, "startup flags: no application context, all off for this process") + return context?.let(::PrefsStartupFlagsStore) + } + + /** + * Mirrors the APP_STARTUP subset of [currentFlags] for the next process. Call off the main thread. + * [currentFlags] runs under the latch's mirror lock, so racing writers always leave the latest flags on disk. + */ + fun write( + context: Context, + currentFlags: () -> List, + ) { + latch.mirror(PrefsStartupFlagsStore(context), currentFlags) + } + + /** Reads the file now, without touching this process's snapshot. */ + internal fun readFile(context: Context?): StartupFlags = StartupFlagsLatch().snapshot { context?.let(::PrefsStartupFlagsStore) } + + /** Null forgets the snapshot, as a new process would. */ + internal fun setSnapshotForTest(flags: StartupFlags?) { + PrefsStartupFlagsStore.commitFailed = false + latch = + StartupFlagsLatch().also { fresh -> + if (flags != null) fresh.snapshot { FixedStore(flags.keys) } + } + } + + private class FixedStore( + private val keys: Set, + ) : IStartupFlagsStore { + override fun read(): Set = keys + + override fun write(keys: Set) = Unit + } +} + +internal class PrefsStartupFlagsStore( + private val context: Context, +) : IStartupFlagsStore { + @Suppress("TooGenericExceptionCaught") + override fun read(): Set? = + try { + check(!commitFailed) { "last startup flags commit failed" } + prefs().getStringSet(KEY_ENABLED, null)?.toSet() + } catch (t: Throwable) { + // android.util.Log, not Logging: Logging can dispatch, and this can run before SDK init. + Log.w(TAG, "startup flags unreadable", t) + throw t + } + + // commit, not apply: the next process reads this at startup, so it must be on disk when we return. + override fun write(keys: Set) { + val committed = prefs().edit().putStringSet(KEY_ENABLED, keys).commit() + commitFailed = !committed + check(committed) { "startup flags commit failed" } + } + + private fun prefs(): SharedPreferences = storageContext(context).getSharedPreferences(STORE, Context.MODE_PRIVATE) + + internal companion object { + const val STORE = "OneSignalStartupFlags" + private const val KEY_ENABLED = "enabled" + const val TAG = "OneSignal" + + // A failed commit still updates the in-memory prefs, so reads can't be trusted until a commit lands. + @Volatile + var commitFailed = false + + // Device-protected so receivers and jobs that start before first unlock read the real values. + // Must hold nothing secret: this storage is readable while the device is still locked. + fun storageContext(context: Context): Context { + // Null during attachBaseContext and some direct-boot paths. + val app: Context = context.applicationContext ?: context + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) app.createDeviceProtectedStorageContext() else app + } + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 7325c27971..56f11bd106 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -10,6 +10,7 @@ import com.onesignal.OneSignalError import com.onesignal.OneSignalResult import com.onesignal.OneSignalUserProfile import com.onesignal.common.AndroidUtils +import com.onesignal.common.AppContext import com.onesignal.common.DeviceUtils import com.onesignal.common.OneSignalUtils import com.onesignal.common.isMissing @@ -247,6 +248,8 @@ internal class OneSignalImp : IOneSignal, } private fun initEssentials(context: Context) { + // Before the logger: its first log can build FeatureManager, whose startup flags need a context. + AppContext.note(context) // LoggerLifecycleManager comes up early so crash handling and remote logging can capture // anything that happens during the rest of init. FeatureManager is wired in via a // lazy supplier — `enabledFeatureFlags` is read per-event, so resolving the manager diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalTestProjectConfig.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalTestProjectConfig.kt index 4406110166..b4bc385edd 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalTestProjectConfig.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalTestProjectConfig.kt @@ -1,6 +1,7 @@ package com.onesignal import com.onesignal.common.threading.OneSignalDispatchers +import com.onesignal.core.internal.features.FeatureManager import io.kotest.core.config.AbstractProjectConfig import io.kotest.core.extensions.Extension import io.kotest.core.listeners.AfterSpecListener @@ -31,9 +32,11 @@ class OneSignalTestProjectConfig : AbstractProjectConfig() { private object OneSignalDispatchersResetListener : BeforeSpecListener, AfterSpecListener { override suspend fun beforeSpec(spec: Spec) { OneSignalDispatchers.resetForTest() + FeatureManager.resetStartupFlagsForTest() } override suspend fun afterSpec(spec: Spec) { OneSignalDispatchers.resetForTest() + FeatureManager.resetStartupFlagsForTest() } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializerTests.kt index d6774628c6..d23c11fd49 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/impl/ActivityLifecycleInitializerTests.kt @@ -2,8 +2,10 @@ package com.onesignal.core.internal.application.impl import android.app.Application import android.content.Context +import com.onesignal.common.AppContext import com.onesignal.common.threading.OneSignalDispatchers import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe import io.mockk.every import io.mockk.mockk import io.mockk.mockkObject @@ -27,11 +29,13 @@ class ActivityLifecycleInitializerTests : FunSpec({ mockkObject(OneSignalDispatchers) every { OneSignalDispatchers.prewarm() } returns Unit resetSharedApplicationService() + AppContext.resetForTest() } afterTest { unmockkObject(OneSignalDispatchers) resetSharedApplicationService() + AppContext.resetForTest() } test("create prewarms before registering activity lifecycle callbacks") { @@ -50,6 +54,15 @@ class ActivityLifecycleInitializerTests : FunSpec({ } } + test("create notes the application context for code that runs before init") { + val application = mockk(relaxed = true) + every { application.applicationContext } returns application + + ActivityLifecycleInitializer().create(application) + + AppContext.get() shouldBe application + } + test("create prewarms even when application context is unavailable") { val context = mockk(relaxed = true) every { context.applicationContext } returns context diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index 0ea10f8758..792bc781d0 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -1,6 +1,7 @@ package com.onesignal.core.internal.features import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.debug.ILogListener @@ -8,15 +9,27 @@ import com.onesignal.debug.LogLevel import com.onesignal.debug.OneSignalLogEvent import com.onesignal.debug.internal.logging.Logging import com.onesignal.features.FeatureFlag +import com.onesignal.features.StartupFlags +import com.onesignal.features.StartupFlagsSource import com.onesignal.user.internal.jwt.JwtRequirement import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkStatic import io.mockk.runs +import io.mockk.unmockkStatic +import io.mockk.verify class FeatureManagerTests : FunSpec({ + fun startupSnapshot(vararg flags: FeatureFlag) { + StartupFlagsCache.setSnapshotForTest(StartupFlags(StartupFlagsSource.STORE, flags.map { it.key }.toSet())) + } + + beforeTest { startupSnapshot() } + afterTest { StartupFlagsCache.setSnapshotForTest(null) } + fun stubConfigModel(model: ConfigModel) { every { model.sdkRemoteFeatureFlags } returns emptyList() every { model.sdkRemoteFeatureFlagMetadata } returns null @@ -30,7 +43,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe false } @@ -43,7 +56,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true } @@ -56,7 +69,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true } @@ -69,7 +82,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) val meta = requireNotNull(manager.remoteFeatureFlagMetadata()) meta.getValue("X").contains("\"note\"") shouldBe true @@ -84,7 +97,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - FeatureManager(configModelStore).remoteFeatureFlagMetadata() shouldBe null + FeatureManager(configModelStore, mockk(relaxed = true)).remoteFeatureFlagMetadata() shouldBe null } test("enabledFeatureKeys is empty when no flags are enabled") { @@ -94,7 +107,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.enabledFeatureKeys() shouldBe emptyList() } @@ -107,7 +120,7 @@ class FeatureManagerTests : FunSpec({ every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.enabledFeatureKeys() shouldBe listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) } @@ -118,7 +131,7 @@ class FeatureManagerTests : FunSpec({ val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe false @@ -138,6 +151,7 @@ class FeatureManagerTests : FunSpec({ val initialModel = mockk() stubConfigModel(initialModel) every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_CUSTOM_LOGGING.key) + startupSnapshot(FeatureFlag.SDK_CUSTOM_LOGGING) val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs @@ -148,7 +162,7 @@ class FeatureManagerTests : FunSpec({ val listener = ILogListener { event -> capturedLogs.add(event) } Logging.addListener(listener) try { - val manager = FeatureManager(configModelStore) + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) manager.isEnabled(FeatureFlag.SDK_CUSTOM_LOGGING) shouldBe true val updatedModel = mockk() @@ -170,4 +184,53 @@ class FeatureManagerTests : FunSpec({ Logging.logLevel = originalLogLevel } } + + test("APP_STARTUP flags latch from the startup snapshot, not the store") { + val model = mockk() + stubConfigModel(model) + every { model.sdkRemoteFeatureFlags } returns + listOf(FeatureFlag.SDK_CUSTOM_LOGGING.key, FeatureFlag.SDK_IDENTITY_VERIFICATION.key) + val configModelStore = mockk() + every { configModelStore.model } returns model + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) + + manager.isEnabled(FeatureFlag.SDK_CUSTOM_LOGGING) shouldBe false + manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true + manager.enabledFeatureKeys() shouldBe listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) + } + + test("an APP_STARTUP flag in the snapshot is on even before the store has it") { + startupSnapshot(FeatureFlag.SDK_CUSTOM_LOGGING) + val model = mockk() + stubConfigModel(model) + val configModelStore = mockk() + every { configModelStore.model } returns model + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore, mockk(relaxed = true)) + + manager.isEnabled(FeatureFlag.SDK_CUSTOM_LOGGING) shouldBe true + } + + test("a failed startup flag mirror logs at ERROR") { + val model = mockk() + stubConfigModel(model) + val configModelStore = mockk() + every { configModelStore.model } returns model + every { configModelStore.subscribe(any()) } just runs + val applicationService = mockk() + val failure = IllegalStateException("no context") + every { applicationService.appContext } throws failure + mockkStatic(Logging::class) + every { Logging.error(any(), any()) } just runs + try { + FeatureManager(configModelStore, applicationService).start() + + verify(exactly = 1) { Logging.error("OneSignal: failed to mirror startup flags: no context", failure) } + } finally { + unmockkStatic(Logging::class) + } + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/StartupFlagsCacheTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/StartupFlagsCacheTests.kt new file mode 100644 index 0000000000..56b2d9943e --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/StartupFlagsCacheTests.kt @@ -0,0 +1,334 @@ +package com.onesignal.core.internal.features + +import android.content.Context +import android.content.SharedPreferences +import android.os.Build +import android.util.Log +import androidx.test.core.app.ApplicationProvider +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.common.AppContext +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.core.internal.config.CONFIG_NAME_SPACE +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys +import com.onesignal.core.internal.preferences.PreferenceStores +import com.onesignal.features.FeatureFlag +import com.onesignal.features.StartupFlagsSource +import com.onesignal.mocks.MockHelper +import com.onesignal.mocks.MockPreferencesService +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.json.JSONArray +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLog +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicReference +import kotlin.concurrent.thread + +@RobolectricTest +@Config(sdk = [Build.VERSION_CODES.O]) +class StartupFlagsCacheTests : FunSpec({ + val logging = FeatureFlag.SDK_CUSTOM_LOGGING + val key = logging.key + + afterTest { + clearStores() + FeatureManager.resetStartupFlagsForTest() + AppContext.resetForTest() + } + + test("a missing file is off") { + startupPrefs().edit().clear().commit() + val flags = StartupFlagsCache.readFile(appContext()) + + flags.source shouldBe StartupFlagsSource.MISSING + flags.isEnabled(logging) shouldBe false + } + + test("a failed commit is reported, not silently dropped") { + val context = mockk() + val prefs = mockk() + every { context.applicationContext } returns context + every { context.createDeviceProtectedStorageContext() } returns context + every { context.getSharedPreferences(any(), any()) } returns prefs + every { prefs.getStringSet(any(), any()) } returns null + every { prefs.edit().putStringSet(any(), any()).commit() } returns false + + shouldThrow { StartupFlagsCache.write(context) { listOf(key) } } + } + + test("turning a flag off remotely reaches the next process") { + val context = appContext() + val store = ConfigModelStore(MockPreferencesService()) + store.model.sdkRemoteFeatureFlags = listOf(key) + val applicationService = MockHelper.applicationService() + every { applicationService.appContext } returns context + + val first = FeatureManager(store, applicationService) + first.start() + FeatureManager.resetStartupFlagsForTest() + val second = FeatureManager(store, applicationService) + second.start() + second.isEnabled(logging) shouldBe true + + store.model.setListProperty(ConfigModel::sdkRemoteFeatureFlags.name, emptyList(), ModelChangeTags.NORMAL) + second.isEnabled(logging) shouldBe true + + FeatureManager.resetStartupFlagsForTest() + FeatureManager.startupFlags { context }.isEnabled(logging) shouldBe false + FeatureManager(store, applicationService).isEnabled(logging) shouldBe false + } + + test("the file follows the remote flags, regardless of case") { + val context = appContext() + StartupFlagsCache.write(context) { listOf("SDK_Custom_Logging") } + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe true + StartupFlagsCache.write(context) { listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) } + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe false + } + + test("racing writes leave the newest flags on disk") { + val context = appContext() + val current = AtomicReference(listOf(key)) + val slowHasRead = CountDownLatch(1) + val releaseSlow = CountDownLatch(1) + + val slow = + thread { + StartupFlagsCache.write(context) { + val read = current.get() + slowHasRead.countDown() + releaseSlow.await() + read + } + } + slowHasRead.await() + current.set(emptyList()) + val fast = thread { StartupFlagsCache.write(context) { current.get() } } + fast.join(200) + releaseSlow.countDown() + slow.join() + fast.join() + + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe false + } + + test("only APP_STARTUP catalog keys reach the file") { + val context = appContext() + StartupFlagsCache.write(context) { listOf(key, FeatureFlag.SDK_IDENTITY_VERIFICATION.key, "not_in_catalog") } + startupPrefs().getStringSet("enabled", null) shouldBe setOf(key) + } + + test("the file lives in device-protected storage, so it is readable before first unlock") { + val context = appContext() + StartupFlagsCache.write(context) { listOf(key) } + + startupPrefs().getStringSet("enabled", null) shouldBe setOf(key) + context.getSharedPreferences(PrefsStartupFlagsStore.STORE, Context.MODE_PRIVATE).getStringSet("enabled", null) shouldBe null + } + + test("a locked boot still reads the flags") { + val unlockedOnly = mockk() + every { unlockedOnly.getStringSet(any(), any()) } throws IllegalStateException("credential storage locked") + val deviceProtected = mockk() + every { deviceProtected.getStringSet(any(), any()) } returns setOf(key) + val dpContext = mockk() + every { dpContext.getSharedPreferences(any(), any()) } returns deviceProtected + val locked = mockk() + every { locked.applicationContext } returns locked + every { locked.getSharedPreferences(any(), any()) } returns unlockedOnly + every { locked.createDeviceProtectedStorageContext() } returns dpContext + + val flags = StartupFlagsCache.readFile(locked) + + flags.source shouldBe StartupFlagsSource.STORE + flags.isEnabled(logging) shouldBe true + } + + test("a context with no application context yet still reads device-protected flags") { + val deviceProtected = mockk() + every { deviceProtected.getStringSet(any(), any()) } returns setOf(key) + val dpContext = mockk() + every { dpContext.getSharedPreferences(any(), any()) } returns deviceProtected + val attaching = mockk() + every { attaching.applicationContext } returns null + every { attaching.createDeviceProtectedStorageContext() } returns dpContext + + val flags = StartupFlagsCache.readFile(attaching) + + flags.source shouldBe StartupFlagsSource.STORE + flags.isEnabled(logging) shouldBe true + } + + test("an IMMEDIATE flag is off even if its key is in the file") { + val immediate = FeatureFlag.SDK_IDENTITY_VERIFICATION + startupPrefs().edit().putStringSet("enabled", setOf(immediate.key)).commit() + + StartupFlagsCache.readFile(appContext()).isEnabled(immediate) shouldBe false + } + + test("read ignores the OneSignal config store") { + val context = appContext() + writeConfigStore(context, listOf(key)) + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe false + } + + test("an unreadable file is off") { + val broken = mockk() + every { broken.applicationContext } returns broken + every { broken.createDeviceProtectedStorageContext() } returns broken + every { broken.getSharedPreferences(any(), any()) } throws SecurityException("prefs unavailable") + + val flags = StartupFlagsCache.readFile(broken) + + flags.source shouldBe StartupFlagsSource.UNREADABLE + flags.isEnabled(logging) shouldBe false + } + + test("an unreadable file is rewritten") { + val context = mockk() + val prefs = mockk() + val editor = mockk() + every { context.applicationContext } returns context + every { context.createDeviceProtectedStorageContext() } returns context + every { context.getSharedPreferences(any(), any()) } returns prefs + every { prefs.getStringSet(any(), any()) } throws ClassCastException("not a string set") + every { prefs.edit() } returns editor + every { editor.putStringSet(any(), any()) } returns editor + every { editor.commit() } returns true + + StartupFlagsCache.write(context) { listOf(key) } + + verify { editor.putStringSet("enabled", setOf(key)) } + } + + test("no application context is off") { + StartupFlagsCache.readFile(null).source shouldBe StartupFlagsSource.NO_STORE + } + + test("the snapshot is read once per process") { + val context = appContext() + StartupFlagsCache.write(context) { listOf(key) } + FeatureManager.resetStartupFlagsForTest() + FeatureManager.startupFlags { context }.isEnabled(logging) shouldBe true + + StartupFlagsCache.write(context) { emptyList() } + FeatureManager.startupFlags { context }.isEnabled(logging) shouldBe true + + FeatureManager.resetStartupFlagsForTest() + FeatureManager.startupFlags { context }.isEnabled(logging) shouldBe false + } + + test("FeatureManager reports the snapshot, not the store, when they disagree") { + val context = appContext() + StartupFlagsCache.write(context) { emptyList() } + FeatureManager.startupFlags { context }.isEnabled(logging) shouldBe false + + val store = MockHelper.configModelStore { it.sdkRemoteFeatureFlags = listOf(key) } + val applicationService = MockHelper.applicationService() + every { applicationService.appContext } returns context + val manager = FeatureManager(store, applicationService) + + manager.isEnabled(logging) shouldBe false + manager.enabledFeatureKeys() shouldBe emptyList() + } + + test("FeatureManager built before ApplicationService starts reads the file through the noted context") { + StartupFlagsCache.write(appContext()) { listOf(key) } + FeatureManager.resetStartupFlagsForTest() + AppContext.note(appContext()) + val applicationService = MockHelper.applicationService() + every { applicationService.appContext } throws NullPointerException("not started") + + val manager = FeatureManager(MockHelper.configModelStore { it.sdkRemoteFeatureFlags = listOf(key) }, applicationService) + + manager.isEnabled(logging) shouldBe true + FeatureManager.startupFlags { null }.source shouldBe StartupFlagsSource.STORE + } + + test("with no context anywhere the snapshot is off rather than guessed") { + StartupFlagsCache.write(appContext()) { listOf(key) } + FeatureManager.resetStartupFlagsForTest() + ShadowLog.clear() + + FeatureManager.startupFlags { null }.source shouldBe StartupFlagsSource.NO_STORE + ShadowLog.getLogsForTag(PrefsStartupFlagsStore.TAG).any { + it.type == Log.ERROR && "no application context" in it.msg + } shouldBe true + } + + test("after a failed commit the next write retries, even with the same flags") { + val context = mockk() + val prefs = mockk() + val editor = mockk() + var inMemory: Set? = null + var pending: Set? = null + val commits = ArrayDeque(listOf(false, true)) + every { context.applicationContext } returns context + every { context.createDeviceProtectedStorageContext() } returns context + every { context.getSharedPreferences(any(), any()) } returns prefs + every { prefs.getStringSet(any(), any()) } answers { inMemory } + every { prefs.edit() } returns editor + every { editor.putStringSet(any(), any()) } answers { pending = secondArg(); editor } + // Like SharedPreferences, a failed commit still updates the in-memory copy. + every { editor.commit() } answers { inMemory = pending; commits.removeFirst() } + + shouldThrow { StartupFlagsCache.write(context) { listOf(key) } } + StartupFlagsCache.write(context) { listOf(key) } + + verify(exactly = 2) { editor.commit() } + } + + test("FeatureManager mirrors the cached flags on start and on later updates, never before start") { + val context = appContext() + val store = MockHelper.configModelStore { it.sdkRemoteFeatureFlags = listOf(key) } + val applicationService = MockHelper.applicationService() + every { applicationService.appContext } returns context + val manager = FeatureManager(store, applicationService) + val update = + ModelChangedArgs(store.model, ConfigModel::sdkRemoteFeatureFlags.name, ConfigModel::sdkRemoteFeatureFlags.name, null, null) + + manager.onModelUpdated(update, ModelChangeTags.NORMAL) + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe false + + manager.start() + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe true + + store.model.sdkRemoteFeatureFlags = emptyList() + manager.onModelUpdated(update, ModelChangeTags.NORMAL) + StartupFlagsCache.readFile(context).isEnabled(logging) shouldBe false + } +}) + +private fun appContext(): Context = ApplicationProvider.getApplicationContext() + +private fun startupPrefs(): SharedPreferences = + appContext().createDeviceProtectedStorageContext().getSharedPreferences(PrefsStartupFlagsStore.STORE, Context.MODE_PRIVATE) + +private fun clearStores() { + val context = appContext() + StartupFlagsCache.write(context) { emptyList() } + context + .getSharedPreferences(PreferenceStores.ONESIGNAL, Context.MODE_PRIVATE) + .edit() + .remove(PreferenceOneSignalKeys.MODEL_STORE_PREFIX + CONFIG_NAME_SPACE) + .commit() +} + +private fun writeConfigStore( + context: Context, + flags: List, +) { + val model = ConfigModel().apply { sdkRemoteFeatureFlags = flags } + context + .getSharedPreferences(PreferenceStores.ONESIGNAL, Context.MODE_PRIVATE) + .edit() + .putString(PreferenceOneSignalKeys.MODEL_STORE_PREFIX + CONFIG_NAME_SPACE, JSONArray().put(model.toJSON()).toString()) + .commit() +}