diff --git a/OneSignalSDK/build.gradle b/OneSignalSDK/build.gradle index c70a36b00..c5a8437f5 100644 --- a/OneSignalSDK/build.gradle +++ b/OneSignalSDK/build.gradle @@ -17,6 +17,7 @@ buildscript { kotlinVersion = '1.9.25' dokkaVersion = '1.9.10' coroutinesVersion = '1.7.3' + kotlinxSerializationJsonVersion = '1.6.3' kotestVersion = '5.8.0' ioMockVersion = '1.13.2' // AndroidX Lifecycle and Activity versions @@ -38,14 +39,15 @@ buildscript { maven { url 'https://developer.huawei.com/repo/' } } sharedDeps = [ - "com.android.tools.build:gradle:$androidGradlePluginVersion", - "com.google.gms:google-services:$googleServicesGradlePluginVersion", - "com.huawei.agconnect:agcp:$huaweiAgconnectVersion", - "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion", - "org.jetbrains.dokka:dokka-gradle-plugin:$dokkaVersion", - "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:$detektVersion", - "com.diffplug.spotless:spotless-plugin-gradle:$spotlessVersion", - "com.vanniktech.maven.publish:com.vanniktech.maven.publish.gradle.plugin:0.32.0" + "com.android.tools.build:gradle:$androidGradlePluginVersion", + "com.google.gms:google-services:$googleServicesGradlePluginVersion", + "com.huawei.agconnect:agcp:$huaweiAgconnectVersion", + "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion", + "org.jetbrains.kotlin:kotlin-serialization:$kotlinVersion", + "org.jetbrains.dokka:dokka-gradle-plugin:$dokkaVersion", + "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:$detektVersion", + "com.diffplug.spotless:spotless-plugin-gradle:$spotlessVersion", + "com.vanniktech.maven.publish:com.vanniktech.maven.publish.gradle.plugin:0.32.0" ] } diff --git a/OneSignalSDK/onesignal/core/build.gradle b/OneSignalSDK/onesignal/core/build.gradle index bf1142512..41d78a3a1 100644 --- a/OneSignalSDK/onesignal/core/build.gradle +++ b/OneSignalSDK/onesignal/core/build.gradle @@ -1,6 +1,7 @@ plugins { id 'com.android.library' id 'kotlin-android' + id 'org.jetbrains.kotlin.plugin.serialization' id 'com.diffplug.spotless' id 'com.vanniktech.maven.publish' id 'io.gitlab.arturbosch.detekt' @@ -73,6 +74,7 @@ dependencies { implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlinVersion" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutinesVersion" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:$coroutinesVersion" + implementation "org.jetbrains.kotlinx:kotlinx-serialization-json:$kotlinxSerializationJsonVersion" // AndroidX Lifecycle and Activity implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:$lifecycleVersion" 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 9d34231d6..9998fd213 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 @@ -4,12 +4,15 @@ import com.onesignal.common.modules.IModule import com.onesignal.common.services.ServiceBuilder import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.application.impl.ApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService import com.onesignal.core.internal.backend.IParamsBackendService +import com.onesignal.core.internal.backend.impl.FeatureFlagsBackendService import com.onesignal.core.internal.backend.impl.ParamsBackendService import com.onesignal.core.internal.background.IBackgroundManager import com.onesignal.core.internal.background.impl.BackgroundManager import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.config.impl.ConfigModelStoreListener +import com.onesignal.core.internal.config.impl.FeatureFlagsRefreshService import com.onesignal.core.internal.database.IDatabaseProvider import com.onesignal.core.internal.database.impl.DatabaseProvider import com.onesignal.core.internal.device.IDeviceService @@ -61,7 +64,9 @@ internal class CoreModule : IModule { builder.register().provides() builder.register().provides() builder.register().provides() + builder.register().provides() builder.register().provides() + builder.register().provides() // Operations builder.register().provides() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt new file mode 100644 index 000000000..cc71fae45 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt @@ -0,0 +1,40 @@ +package com.onesignal.core.internal.backend + +import kotlinx.serialization.json.JsonObject + +/** + * Result of the dedicated SDK feature-flags endpoint (separate from [IParamsBackendService]). + * + * @param enabledKeys Feature keys that should be treated as enabled for this device/SDK. + * @param metadata Optional per-flag payload (e.g. weights), keyed by flag id. Parsed from sibling + * keys in the response JSON (see [com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser]). + */ +internal data class RemoteFeatureFlagsResult( + val enabledKeys: List, + val metadata: JsonObject?, +) { + companion object { + val EMPTY = RemoteFeatureFlagsResult(emptyList(), null) + } +} + +/** + * Outcome of [IFeatureFlagsBackendService.fetchRemoteFeatureFlags]. + * [Unavailable] means the client did not get a trustworthy response (HTTP error, invalid body, etc.); + * callers should keep previously cached flags. [Success] includes a valid HTTP parse, including an + * empty `features` array from the server. + */ +internal sealed class RemoteFeatureFlagsFetchOutcome { + data class Success(val result: RemoteFeatureFlagsResult) : RemoteFeatureFlagsFetchOutcome() + + data object Unavailable : RemoteFeatureFlagsFetchOutcome() +} + +/** + * Fetches remote feature flags for the current app via **GET** + * `apps/{app_id}/sdk/features/{platform}/{sdk_version}` (Turbine; see + * [com.onesignal.core.internal.backend.impl.FeatureFlagsBackendService]). + */ +internal interface IFeatureFlagsBackendService { + suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt index 54f3cd85e..8773a23af 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt @@ -35,7 +35,6 @@ internal class ParamsObject( var locationShared: Boolean? = null, var requiresUserPrivacyConsent: Boolean? = null, var opRepoExecutionInterval: Long? = null, - val features: List = emptyList(), var influenceParams: InfluenceParamsObject, var fcmParams: FCMParamsObject, val remoteLoggingParams: RemoteLoggingParamsObject, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt new file mode 100644 index 000000000..0fbd26d5f --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -0,0 +1,120 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.common.OneSignalUtils +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.http.IHttpClient +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging + +/** + * Turbine SDK feature flags endpoint ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). + * + * HTTP, logging, and [OneSignalUtils] are platform-specific; path shape and validation live in + * [TurbineSdkFeatureFlagsPath], and JSON parsing in [FeatureFlagsJsonParser] (both KMP-friendly). + * + * **GET** `apps/{app_id}/sdk/features/{platform}/{sdk_version}` relative to + * [com.onesignal.core.internal.config.ConfigModel.apiUrl] (app-provided base URL). + * + * - **platform** is always **`android`** for this SDK client. + * - **sdk_version** is [OneSignalUtils.sdkVersion] (same label as the `SDK-Version` header segment), e.g. + * `050801` or `050801-beta`; see [isValidFeaturesSdkVersionLabel]. + * + * Response: `{ "features": [ "flag_key", ... ] }`. + */ +internal class FeatureFlagsBackendService( + private val http: IHttpClient, +) : IFeatureFlagsBackendService { + override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome { + Logging.log(LogLevel.DEBUG, "FeatureFlagsBackendService.fetchRemoteFeatureFlags(appId=$appId)") + + val sdkVersion = OneSignalUtils.sdkVersion + if (!isValidFeaturesSdkVersionLabel(sdkVersion)) { + Logging.warn( + "FeatureFlagsBackendService: sdk version not usable for Turbine path (expected " + + "6-digit label optional -suffix, e.g. 050801 or 050801-beta): '$sdkVersion'", + ) + return RemoteFeatureFlagsFetchOutcome.Unavailable + } + + val path = + TurbineSdkFeatureFlagsPath.buildGetPath( + appId = appId, + platform = TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = sdkVersion, + ) + + val response = http.get(path, null) + val body = response.payload + if (!response.isSuccess) { + val msg = + "FeatureFlagsBackendService: non-success status=${response.statusCode} body=${bodySnippet(body)}" + // 4xx is likely a permanent misconfiguration (e.g. 403 Forbidden when the app is not + // enrolled for Turbine feature flags) and worth surfacing at WARN; other failures are + // typically transient (network blip, 5xx) and stay at DEBUG to avoid log spam. + if (response.isClientError) Logging.warn(msg) else Logging.debug(msg) + return RemoteFeatureFlagsFetchOutcome.Unavailable + } + if (body.isNullOrBlank()) { + Logging.warn( + "FeatureFlagsBackendService: empty body for success status=${response.statusCode}", + ) + return RemoteFeatureFlagsFetchOutcome.Unavailable + } + + val parsed = FeatureFlagsJsonParser.parseSuccessful(body) + return if (parsed != null) { + RemoteFeatureFlagsFetchOutcome.Success(parsed) + } else { + Logging.warn( + "FeatureFlagsBackendService: response body is not valid Turbine feature-flags JSON: " + + bodySnippet(body), + ) + RemoteFeatureFlagsFetchOutcome.Unavailable + } + } + + /** + * Trim [body] to a short, single-line snippet safe for logcat. Caps at + * [LOG_BODY_SNIPPET_MAX_CHARS] so we never dump large payloads into logs. + */ + private fun bodySnippet(body: String?): String { + if (body.isNullOrEmpty()) return "" + val flattened = body.replace('\n', ' ').replace('\r', ' ') + return if (flattened.length <= LOG_BODY_SNIPPET_MAX_CHARS) { + flattened + } else { + flattened.take(LOG_BODY_SNIPPET_MAX_CHARS) + "…" + } + } + + companion object { + /** + * Turbine `:platform` segment for the OneSignal Android SDK (this client). + */ + const val TURBINE_FEATURES_PLATFORM_ANDROID = "android" + + /** + * Max chars of an HTTP response body included in diagnostic logs. Turbine error bodies + * (e.g. `{"errors":["Forbidden"]}`) are tiny, so 200 chars is plenty and bounds worst-case + * log size if an unexpected payload is returned. + */ + private const val LOG_BODY_SNIPPET_MAX_CHARS = 200 + + /** + * Returns true when [label] is safe to send as the Turbine `:sdk_version` path segment. + * @see TurbineSdkFeatureFlagsPath.isValidFeaturesSdkVersionLabel + */ + fun isValidFeaturesSdkVersionLabel(label: String): Boolean = TurbineSdkFeatureFlagsPath.isValidFeaturesSdkVersionLabel(label) + + /** + * Path only (relative to API base), matching `/apps/:app_id/sdk/features/:platform/:sdk_version`. + * @see TurbineSdkFeatureFlagsPath.buildGetPath + */ + internal fun buildFeatureFlagsGetPath( + appId: String, + platform: String, + sdkVersion: String, + ): String = TurbineSdkFeatureFlagsPath.buildGetPath(appId, platform, sdkVersion) + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt new file mode 100644 index 000000000..9a6a85231 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -0,0 +1,149 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject + +/** + * **What this is:** strict JSON parsing for the Turbine SDK feature-flags response + * ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). + * + * **Wire shape:** root object with a `features` array of **string** flag ids. Optional per-flag JSON + * objects may appear as **sibling root properties** (same name as the id); those are merged into + * [RemoteFeatureFlagsResult.metadata]. + * + * **Optional metadata:** for a string entry `"foo"`, if the root also has a property `"foo"` (or + * case-insensitive match) whose value is a JSON object, that object is stored per-flag in + * [RemoteFeatureFlagsResult.metadata]. That lets a future API add per-flag config without a new + * top-level field; the SDK persists it in [com.onesignal.core.internal.config.ConfigModel.sdkRemoteFeatureFlagMetadata] + * so a later process or SDK version can read it without re-fetching. + * + * **API surface:** [parseSuccessful] for HTTP 200 bodies; [parse] for lenient “best effort”; + * [encodeMetadata] / [parseStoredMetadataMap] for the persisted metadata column. + * + * Uses only Kotlin stdlib + kotlinx.serialization (Kotlin Multiplatform-friendly). + */ +internal object FeatureFlagsJsonParser { + /** + * RFC 8259–style JSON only (no lenient tokens like unquoted keys, `NaN`, trailing commas). + * That keeps [encodeMetadata] strings portable: the same text can be parsed later by + * `org.json.JSONObject`, Gson, or kotlinx without relying on kotlinx-only quirks. + */ + val format = + Json { + ignoreUnknownKeys = true + isLenient = false + allowSpecialFloatingPointValues = false + prettyPrint = false + } + + private const val FEATURES_PROPERTY = "features" + + fun parse(payload: String): RemoteFeatureFlagsResult = parseSuccessful(payload) ?: RemoteFeatureFlagsResult.EMPTY + + /** + * Parses a 200 response body. Returns `null` if the text is not JSON, not an object, or does not + * contain a `features` **array** of the expected element types. Returns an empty result for + * `{"features":[]}`. + */ + fun parseSuccessful(payload: String): RemoteFeatureFlagsResult? { + return try { + val root = format.parseToJsonElement(payload) as? JsonObject ?: return null + parseRootStrict(root) + } catch (_: Throwable) { + null + } + } + + @Suppress("ReturnCount") + private fun parseRootStrict(root: JsonObject): RemoteFeatureFlagsResult? { + val featuresEl = root[FEATURES_PROPERTY] ?: return null + val featuresArray = featuresEl as? JsonArray ?: return null + val flagEntries = + featuresArray.mapNotNull { el -> + (el as? JsonPrimitive) + ?.takeIf { it.isString } + ?.content + ?.trim() + ?.takeIf { it.isNotEmpty() } + ?.let { raw -> raw to canonicalFeatureFlagId(raw) } + }.distinctBy { it.second } + + if (flagEntries.isEmpty()) { + // `[]` is an authoritative empty config; a non-empty array that filtered down + // to empty is a contract violation. Null surfaces as Unavailable upstream so + // callers preserve the cached list instead of overwriting it with []. + return if (featuresArray.isEmpty()) RemoteFeatureFlagsResult(emptyList(), null) else null + } + + val keys = flagEntries.map { it.second } + + val metadata = + buildJsonObject { + for ((rawKey, canonicalKey) in flagEntries) { + findSiblingJsonObject(root, rawKey, canonicalKey)?.let { put(canonicalKey, it) } + } + } + val metaOut = if (metadata.isEmpty()) null else metadata + return RemoteFeatureFlagsResult(keys, metaOut) + } + + @Suppress("ReturnCount") + private fun findSiblingJsonObject( + root: JsonObject, + rawKeyFromFeaturesArray: String, + canonicalKey: String, + ): JsonObject? { + for (candidate in listOf(rawKeyFromFeaturesArray, canonicalKey)) { + if (candidate == FEATURES_PROPERTY) { + continue + } + when (val v = root[candidate]) { + is JsonObject -> return v + else -> Unit + } + } + for ((k, v) in root) { + if (k == FEATURES_PROPERTY) { + continue + } + if (k.equals(rawKeyFromFeaturesArray, ignoreCase = true) && v is JsonObject) { + return v + } + } + return null + } + + private fun canonicalFeatureFlagId(raw: String): String = + buildString(raw.length) { + for (c in raw) { + append(c.lowercaseChar()) + } + } + + fun encodeMetadata(metadata: JsonObject?): String? = + metadata?.let { format.encodeToString(JsonElement.serializer(), it) } + + /** + * Decodes [ConfigModel.sdkRemoteFeatureFlagMetadata] (a JSON object of flag id → object) into a map. + * Non-object values are skipped so each entry stays a [JsonObject] for nested decoding (e.g. with `Json.decodeFromJsonElement`). + */ + @Suppress("ReturnCount") + fun parseStoredMetadataMap(raw: String?): Map { + if (raw.isNullOrBlank()) { + return emptyMap() + } + return try { + val root = format.parseToJsonElement(raw) as? JsonObject ?: return emptyMap() + root.entries.mapNotNull { (key, value) -> + (value as? JsonObject)?.let { key to it } + }.toMap() + } catch (_: Throwable) { + emptyMap() + } + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt index ec0af8605..dfaaa027d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt @@ -68,19 +68,6 @@ internal class ParamsBackendService( ) } - val features = - responseJson.optJSONArray("features") - ?.let { featuresJson -> - buildList { - for (i in 0 until featuresJson.length()) { - val featureName = featuresJson.optString(i, "") - if (featureName.isNotBlank()) { - add(featureName) - } - } - } - } ?: emptyList() - return ParamsObject( googleProjectNumber = responseJson.safeString("android_sender_id"), enterprise = responseJson.safeBool("enterp"), @@ -97,7 +84,6 @@ internal class ParamsBackendService( requiresUserPrivacyConsent = responseJson.safeBool("requires_user_privacy_consent"), // TODO: New opRepoExecutionInterval = responseJson.safeLong("oprepo_execution_interval"), - features = features, influenceParams = influenceParams ?: InfluenceParamsObject(), fcmParams = fcmParams ?: FCMParamsObject(), remoteLoggingParams = remoteLoggingParams ?: RemoteLoggingParamsObject(), diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt new file mode 100644 index 000000000..c533f8e50 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt @@ -0,0 +1,67 @@ +package com.onesignal.core.internal.backend.impl + +/** + * Builds the Turbine-relative path for SDK feature flags and validates the SDK version label. + * + * Pure Kotlin only (no `java.*` / Android APIs) so this file can be reused from Kotlin Multiplatform `commonMain`. + */ +internal object TurbineSdkFeatureFlagsPath { + private val FEATURES_SDK_VERSION_LABEL_REGEX = Regex("""^\d{6}(-[^/\s]+)?$""") + + /** + * Labels expected in the `:sdk_version` path segment: six digits, optionally `-` and a non-empty suffix + * (no slashes or whitespace). + */ + fun isValidFeaturesSdkVersionLabel(label: String): Boolean = FEATURES_SDK_VERSION_LABEL_REGEX.matches(label) + + /** + * Path only (relative to API base), matching `apps/{app_id}/sdk/features/{platform}/{sdk_version}`. + * [platform] and [sdkVersion] are UTF-8 percent-encoded per RFC 3986 (unreserved bytes left as-is). + */ + fun buildGetPath( + appId: String, + platform: String, + sdkVersion: String, + ): String { + val p = percentEncodePathSegmentUtf8(platform) + val v = percentEncodePathSegmentUtf8(sdkVersion) + return "apps/$appId/sdk/features/$p/$v" + } + + /** + * Percent-encodes a UTF-8 string as a single path segment: unreserved bytes stay literal; everything else + * becomes `%HH` (uppercase hex). Space becomes `%20` (not `+`), matching typical REST path encoding. + */ + internal fun percentEncodePathSegmentUtf8(segment: String): String { + val bytes = segment.encodeToByteArray() + return buildString(bytes.size * PCT_ENCODED_MAX_OUTPUT_CHARS_PER_INPUT_BYTE) { + for (b in bytes) { + val u = b.toInt() and BYTE_MASK + if (isUnreservedByte(u)) { + append(u.toChar()) + } else { + append('%') + append(HEX_DIGITS[u shr HEX_NYBBLE_SHIFT]) + append(HEX_DIGITS[u and HEX_NYBBLE_MASK]) + } + } + } + } + + /** Per RFC 3986 section 2.3 unreserved (ALPHA / DIGIT / "-" / "." / "_" / "~"). */ + private fun isUnreservedByte(u: Int): Boolean = + u in 'A'.code..'Z'.code || + u in 'a'.code..'z'.code || + u in '0'.code..'9'.code || + u == '-'.code || + u == '.'.code || + u == '_'.code || + u == '~'.code + + private const val BYTE_MASK: Int = 0xFF + private const val PCT_ENCODED_MAX_OUTPUT_CHARS_PER_INPUT_BYTE: Int = 3 + private const val HEX_NYBBLE_MASK: Int = 0b1111 + private const val HEX_NYBBLE_SHIFT: Int = 4 + + private const val HEX_DIGITS = "0123456789ABCDEF" +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt index a88739e05..616fa1916 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt @@ -291,13 +291,27 @@ class ConfigModel : Model() { } /** - * Remote feature switches controlled by backend. - * Presence of a feature name indicates enabled. + * Feature ids from the Turbine SDK feature-flags HTTP endpoint + * ([com.onesignal.core.internal.backend.IFeatureFlagsBackendService]), updated while the app is + * foreground. [com.onesignal.core.internal.features.FeatureManager] unions these with any local + * test overrides when evaluating [com.onesignal.core.internal.features.FeatureFlag] entries. */ - var features: List - get() = getListProperty(::features.name) { emptyList() } + var sdkRemoteFeatureFlags: List + get() = getListProperty(::sdkRemoteFeatureFlags.name) { emptyList() } set(value) { - setListProperty(::features.name, value) + setListProperty(::sdkRemoteFeatureFlags.name, value) + } + + /** + * Compacted JSON object: canonical flag id → metadata object, from the same Turbine response + * as [sdkRemoteFeatureFlags]. Persisted so metadata survives process death and is available + * to [com.onesignal.core.internal.features.IFeatureManager.remoteFeatureFlagMetadata] without + * a new network fetch on the next launch. + */ + var sdkRemoteFeatureFlagMetadata: String? + get() = getOptStringProperty(::sdkRemoteFeatureFlagMetadata.name) + set(value) { + setOptStringProperty(::sdkRemoteFeatureFlagMetadata.name, value) } /** @@ -344,7 +358,7 @@ class ConfigModel : Model() { property: String, jsonArray: JSONArray, ): List<*>? { - if (property == ::features.name) { + if (property == ::sdkRemoteFeatureFlags.name) { return buildList { for (i in 0 until jsonArray.length()) { val featureName = jsonArray.optString(i, "") diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt new file mode 100644 index 000000000..dbe741753 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt @@ -0,0 +1,14 @@ +package com.onesignal.core.internal.config + +/** + * [ModelChangeTags][com.onesignal.common.modeling.ModelChangeTags]-style values used only for + * [ConfigModel] / [ConfigModelStore] replace notifications. + */ +internal object ConfigModelChangeTags { + /** + * A partial update from the Turbine feature-flags refresh: only + * [ConfigModel.sdkRemoteFeatureFlags] and [ConfigModel.sdkRemoteFeatureFlagMetadata] changed + * (in-place on the live model, not a full [ConfigModel] replace). + */ + const val REMOTE_FEATURE_FLAGS = "REMOTE_FEATURE_FLAGS" +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt index 9e5c4fc32..e15b6faa0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt @@ -95,7 +95,6 @@ internal class ConfigModelStoreListener( params.locationShared?.let { config.locationShared = it } params.requiresUserPrivacyConsent?.let { config.consentRequired = it } params.opRepoExecutionInterval?.let { config.opRepoExecutionInterval = it } - config.features = params.features params.influenceParams.notificationLimit?.let { config.influenceParams.notificationLimit = it } params.influenceParams.indirectNotificationAttributionWindow?.let { config.influenceParams.indirectNotificationAttributionWindow = it } params.influenceParams.iamLimit?.let { config.influenceParams.iamLimit = it } @@ -107,6 +106,11 @@ internal class ConfigModelStoreListener( params.remoteLoggingParams.logLevel?.let { config.remoteLoggingParams.logLevel = it } config.remoteLoggingParams.isEnabled = params.remoteLoggingParams.isEnabled + // Re-source from live model: replace() is a full data.clear()+putAll(), + // and these fields are written in-place by FeatureFlagsRefreshService. + config.sdkRemoteFeatureFlags = _configModelStore.model.sdkRemoteFeatureFlags + config.sdkRemoteFeatureFlagMetadata = _configModelStore.model.sdkRemoteFeatureFlagMetadata + _configModelStore.replace(config, ModelChangeTags.HYDRATE) success = true } catch (ex: BackendException) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt new file mode 100644 index 000000000..40407f821 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -0,0 +1,168 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.common.threading.OneSignalDispatchers +import com.onesignal.core.internal.application.IApplicationLifecycleHandler +import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelChangeTags +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.startup.IStartableService +import com.onesignal.debug.internal.logging.Logging +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlin.coroutines.coroutineContext + +/** + * Fetches remote SDK feature flags when the app is in the foreground, immediately on focus and then + * every [refreshIntervalMs] while the session stays in the foreground. Updates + * [ConfigModel.sdkRemoteFeatureFlags] / [ConfigModel.sdkRemoteFeatureFlagMetadata] so + * [com.onesignal.core.internal.features.FeatureManager] stays in sync. + * + * Remote fields are updated in place on the live [ConfigModel] (with [ConfigModelChangeTags.REMOTE_FEATURE_FLAGS]) + * so concurrent hydration cannot be overwritten by a stale full-model snapshot. + * + * Polling is keyed on the active [ConfigModel.appId]: once a poll loop is running for a given + * appId, redundant triggers (e.g. [com.onesignal.common.modeling.ModelChangeTags.HYDRATE] replaces + * from [ConfigModelStoreListener.fetchParams] that write the same appId back) are a no-op so we + * don't double-fire the Turbine GET at startup. Genuine appId changes still cancel and restart. + * + * IMPORTANT: Constructor parameters must remain limited to types the IoC reflection can resolve + * via [com.onesignal.common.services.IServiceProvider] (registered services or `List`). + * Configuration knobs like [refreshIntervalMs] live as `internal var` instead so reflection in + * [com.onesignal.common.services.ServiceRegistrationReflection.resolve] still picks the only + * constructor and tests can still override the value before [start]. + */ +internal class FeatureFlagsRefreshService( + private val applicationService: IApplicationService, + private val configModelStore: ConfigModelStore, + private val featureFlagsBackend: IFeatureFlagsBackendService, +) : IStartableService, + IApplicationLifecycleHandler, + ISingletonModelStoreChangeHandler { + /** + * Foreground polling cadence; defaults to [DEFAULT_REFRESH_INTERVAL_MS] (8 minutes). Test-only + * override (must be set before [start] / focus) – keep out of the constructor so the IoC's + * reflection-based resolver doesn't trip on a non-service `Long` parameter (see class KDoc). + */ + internal var refreshIntervalMs: Long = DEFAULT_REFRESH_INTERVAL_MS + + private var pollJob: Job? = null + + /** + * AppId currently being polled. Used to dedup redundant `restartForegroundPolling` triggers + * (e.g. HYDRATE replace from [ConfigModelStoreListener.fetchParams] that doesn't change the + * appId). Cleared in [onUnfocused] so the next focus event always re-arms polling. + */ + private var pollingAppId: String? = null + + override fun start() { + applicationService.addApplicationLifecycleHandler(this) + configModelStore.subscribe(this) + // Foreground-at-subscribe is handled by [IApplicationService.addApplicationLifecycleHandler] (fires onFocus). + } + + override fun onFocus(firedOnSubscribe: Boolean) { + restartForegroundPolling() + } + + override fun onUnfocused() { + synchronized(this) { + pollJob?.cancel() + pollJob = null + pollingAppId = null + } + } + + override fun onModelUpdated( + args: ModelChangedArgs, + tag: String, + ) { + if (args.property != ConfigModel::appId.name) { + return + } + if (applicationService.isInForeground) { + restartForegroundPolling() + } + } + + override fun onModelReplaced( + model: ConfigModel, + tag: String, + ) { + if (tag != ModelChangeTags.HYDRATE && tag != ModelChangeTags.NORMAL) { + return + } + if (model.appId.isNotEmpty() && applicationService.isInForeground) { + restartForegroundPolling() + } + } + + private fun restartForegroundPolling() { + synchronized(this) { + val appId = configModelStore.model.appId + if (appId.isEmpty()) { + pollJob?.cancel() + pollJob = null + pollingAppId = null + return + } + // Dedup: an active loop for the same appId is already covering us. + if (pollingAppId == appId) { + return + } + pollJob?.cancel() + pollingAppId = appId + pollJob = + OneSignalDispatchers.launchOnIO { + while (coroutineContext.isActive) { + if (!applicationService.isInForeground) { + break + } + val current = configModelStore.model.appId + if (current.isNotEmpty()) { + try { + fetchAndApply(current) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Logging.warn("FeatureFlagsRefreshService: fetch failed", e) + } + } + delay(refreshIntervalMs) + } + } + } + } + + private suspend fun fetchAndApply(appId: String) { + val result = + when (val outcome = featureFlagsBackend.fetchRemoteFeatureFlags(appId)) { + RemoteFeatureFlagsFetchOutcome.Unavailable -> return + is RemoteFeatureFlagsFetchOutcome.Success -> outcome.result + } + val current = configModelStore.model + val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) + val beforeKeys = current.sdkRemoteFeatureFlags.toSet() + val afterKeys = result.enabledKeys.toSet() + if (afterKeys == beforeKeys && newMetaString == current.sdkRemoteFeatureFlagMetadata) { + return + } + + val tag = ConfigModelChangeTags.REMOTE_FEATURE_FLAGS + current.setListProperty(ConfigModel::sdkRemoteFeatureFlags.name, result.enabledKeys, tag) + current.setOptStringProperty(ConfigModel::sdkRemoteFeatureFlagMetadata.name, newMetaString, tag) + } + + companion object { + // Foreground polling cadence for remote feature flags (8 minutes). + private const val DEFAULT_REFRESH_INTERVAL_MS = 480_000L + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt index f9bad1176..324421a71 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt @@ -17,6 +17,8 @@ internal enum class FeatureActivationMode { /** * Backend-driven feature switches used by the SDK. + * + * [key] values are **lowercase** strings as returned from remote config / Turbine `features` arrays. */ internal enum class FeatureFlag( val key: String, @@ -24,14 +26,10 @@ internal enum class FeatureFlag( ) { // Threading mode is selected once per app startup to avoid mixed-mode behavior mid-session. // - // Naming convention: - // SDK__ + // Remote key (lowercase) must match backend / Turbine flag id. // - // Example: - // SDK_050800_BACKGROUND_THREADING - // - SDK_050800_BACKGROUND_THREADING( - "SDK_050800_BACKGROUND_THREADING", + SDK_BACKGROUND_THREADING( + "sdk_background_threading", FeatureActivationMode.APP_STARTUP ), ; 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 0a05fb5ae..738186db2 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 @@ -4,12 +4,22 @@ import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.modeling.ModelChangedArgs import com.onesignal.common.threading.ThreadingMode +import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.debug.internal.logging.Logging +import kotlinx.serialization.json.JsonObject internal interface IFeatureManager { fun isEnabled(feature: FeatureFlag): Boolean + + /** + * Per-flag payloads from [com.onesignal.core.internal.backend.IFeatureFlagsBackendService]. + * Each value is a [JsonObject] so callers can decode nested fields or map to `@Serializable` types. + * + * `null` when no metadata has been stored yet ([ConfigModel.sdkRemoteFeatureFlagMetadata] null/blank). + */ + fun remoteFeatureFlagMetadata(): Map? } @Suppress("TooGenericExceptionCaught") @@ -31,6 +41,14 @@ internal class FeatureManager( override fun isEnabled(feature: FeatureFlag): Boolean = featureStates[feature] ?: false + override fun remoteFeatureFlagMetadata(): Map? { + val raw = configModelStore.model.sdkRemoteFeatureFlagMetadata + if (raw.isNullOrBlank()) { + return null + } + return FeatureFlagsJsonParser.parseStoredMetadataMap(raw) + } + @Suppress("TooGenericExceptionCaught") override fun onModelReplaced( model: ConfigModel, @@ -51,7 +69,9 @@ internal class FeatureManager( args: ModelChangedArgs, tag: String, ) { - if (args.property == ConfigModel::features.name) { + if (args.property == ConfigModel::sdkRemoteFeatureFlags.name || + args.property == ConfigModel::sdkRemoteFeatureFlagMetadata.name + ) { Logging.debug("OneSignal: FeatureManager.onModelUpdated(property=${args.property}, tag=$tag)") try { refreshEnabledFeatures(configModelStore.model, applyNextRunOnlyFeatures = false) @@ -66,7 +86,11 @@ internal class FeatureManager( model: ConfigModel, applyNextRunOnlyFeatures: Boolean, ) { - val enabledFeatureKeys = (model.features + localFeatureOverrides).toSet() + val enabledFeatureKeys = + ( + model.sdkRemoteFeatureFlags.map { canonicalizeFeatureKey(it) } + + localFeatureOverrides.map { canonicalizeFeatureKey(it) } + ).toSet() if (localFeatureOverrides.isNotEmpty()) { Logging.warn( "OneSignal: Local feature override enabled for testing only: $localFeatureOverrides", @@ -103,12 +127,19 @@ internal class FeatureManager( featureStates = nextStates } + private fun canonicalizeFeatureKey(key: String): String = + buildString(key.length) { + for (c in key) { + append(c.lowercaseChar()) + } + } + private fun applySideEffects( feature: FeatureFlag, enabled: Boolean, ) { when (feature) { - FeatureFlag.SDK_050800_BACKGROUND_THREADING -> + FeatureFlag.SDK_BACKGROUND_THREADING -> ThreadingMode.updateUseBackgroundThreading( enabled = enabled, source = "FeatureManager:${feature.activationMode}" @@ -120,10 +151,10 @@ internal class FeatureManager( /** * Local-only test hook for forcing features ON without backend config. * Add feature keys here while testing locally, e.g.: - * setOf(FeatureFlag.BACKGROUND_THREADING.key) + * setOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) */ private val localFeatureOverrides: Set = emptySet() // private val localFeatureOverrides: Set = -// setOf(FeatureFlag.BACKGROUND_THREADING.key) +// setOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt index fb62ccc24..64902b337 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt @@ -34,4 +34,11 @@ class HttpResponse( */ val isSuccess: Boolean get() = statusCode == HttpURLConnection.HTTP_OK || statusCode == HttpURLConnection.HTTP_ACCEPTED || statusCode == HttpURLConnection.HTTP_NOT_MODIFIED || statusCode == HttpURLConnection.HTTP_CREATED + + /** + * Whether the response is a client error (HTTP 4xx). Useful for distinguishing "probably + * permanent misconfiguration" from transient failures (5xx, network errors with statusCode=0). + */ + val isClientError: Boolean + get() = statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt index b195716cc..7caa7602d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt @@ -19,7 +19,7 @@ internal class StartupService( val useBackgroundThreading = try { val featureManager = services.getService() - featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) + featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } catch (t: Throwable) { Logging.warn("OneSignal: Failed to resolve BACKGROUND_THREADING in StartupService. Falling back to legacy thread.", t) false diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt index 8fc34c5d9..5b8fe7497 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt @@ -50,7 +50,7 @@ internal class OneSignalCrashUploaderWrapper( @Suppress("TooGenericExceptionCaught") override fun start() { if (!OtelSdkSupport.isSupported) return - if (featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING)) { + if (featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING)) { OneSignalDispatchers.launchOnIO { try { uploader.start() 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 5cd9cb917..b58d28aba 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 @@ -180,7 +180,7 @@ internal class OneSignalImp( } return try { - featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) + featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } catch (t: Throwable) { Logging.warn("OneSignal: Failed to resolve BACKGROUND_THREADING feature, defaulting to legacy mode.", t) false diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt new file mode 100644 index 000000000..c20b933b0 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt @@ -0,0 +1,199 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.common.OneSignalUtils +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.http.HttpResponse +import com.onesignal.core.internal.http.IHttpClient +import com.onesignal.debug.ILogListener +import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.shouldBe +import io.mockk.coEvery +import io.mockk.mockk + +/** + * Verifies that [FeatureFlagsBackendService] surfaces enough diagnostic information for a + * developer to understand why a remote feature-flags fetch returned + * [RemoteFeatureFlagsFetchOutcome.Unavailable]. In particular: + * - 4xx failures (e.g. 403 Forbidden for apps not enrolled in Turbine) should log at [LogLevel.WARN] + * and include a body snippet so the Turbine error payload shows up in logcat by default. + * - 5xx / transient failures should stay at [LogLevel.DEBUG] to avoid noisy logs for flaky networks. + * - A 200 with an unexpected body shape should log at [LogLevel.WARN] with a body snippet since it + * indicates a real contract/config issue that would otherwise be silent. + */ +class FeatureFlagsBackendServiceTests : FunSpec({ + lateinit var capturedLogs: MutableList + lateinit var listener: ILogListener + var originalLogLevel: LogLevel = LogLevel.WARN + + beforeEach { + originalLogLevel = Logging.logLevel + Logging.logLevel = LogLevel.NONE + capturedLogs = mutableListOf() + listener = ILogListener { event -> capturedLogs.add(event) } + Logging.addListener(listener) + } + + afterEach { + Logging.removeListener(listener) + Logging.logLevel = originalLogLevel + } + + fun logsForService(): List = + capturedLogs.filter { it.entry.contains("FeatureFlagsBackendService:") } + + test("403 Forbidden logs at WARN with body snippet and returns Unavailable") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, """{"errors":["Forbidden"]}""") + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + + outcome shouldBe RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns shouldHaveSize 1 + warns[0].entry.contains("status=403") shouldBe true + warns[0].entry.contains("""{"errors":["Forbidden"]}""") shouldBe true + } + + test("404 Not Found also logs at WARN with body snippet") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(404, """{"errors":["Not Found"]}""") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns shouldHaveSize 1 + warns[0].entry.contains("status=404") shouldBe true + } + + test("500 server error stays at DEBUG to avoid noisy logs for transient failures") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(500, "boom") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + logsForService().any { it.level == LogLevel.WARN } shouldBe false + val debugs = logsForService().filter { it.level == LogLevel.DEBUG } + debugs.any { it.entry.contains("status=500") && it.entry.contains("body=boom") } shouldBe true + } + + test("network error (statusCode=0) stays at DEBUG") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(0, null) + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + logsForService().any { it.level == LogLevel.WARN } shouldBe false + val debugs = logsForService().filter { it.level == LogLevel.DEBUG } + debugs.any { it.entry.contains("status=0") && it.entry.contains("body=") } shouldBe true + } + + test("200 with non-contract JSON body logs at WARN with body snippet") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, """{"errors":["Forbidden"]}""") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = + logsForService().filter { + it.level == LogLevel.WARN && it.entry.contains("not valid Turbine feature-flags JSON") + } + warns shouldHaveSize 1 + warns[0].entry.contains("""{"errors":["Forbidden"]}""") shouldBe true + } + + test("body snippet caps long payloads at 200 chars") { + val longBody = "x".repeat(1_000) + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, longBody) + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + val warn = logsForService().first { it.level == LogLevel.WARN } + // Snippet is truncated to 200 chars plus an ellipsis marker. + warn.entry.contains("x".repeat(200) + "…") shouldBe true + warn.entry.contains("x".repeat(201)) shouldBe false + } + + test("newlines in body are flattened so log entry stays single-line") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, "line1\nline2\rline3") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + val warn = logsForService().first { it.level == LogLevel.WARN } + warn.entry.contains("body=line1 line2 line3") shouldBe true + warn.entry.contains("\n") shouldBe false + warn.entry.contains("\r") shouldBe false + } + + test("200 with valid empty features array is Success and does not log at WARN") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, """{"features":[]}""") + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + outcome.shouldBeSuccessWithEmptyKeys() + logsForService().any { it.level == LogLevel.WARN } shouldBe false + } + + test("200 with empty body logs at WARN for contract anomaly and returns Unavailable") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, null) + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + outcome shouldBe RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns.any { it.entry.contains("empty body for success status=200") } shouldBe true + } + + test("buildFeatureFlagsGetPath matches Turbine /apps/:app_id/sdk/features/:platform/:sdk_version") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801" + } + + test("buildFeatureFlagsGetPath encodes prerelease sdk version label") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801-beta", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" + } + + test("isValidFeaturesSdkVersionLabel accepts only 6-digit labels with optional -suffix") { + val valid = + listOf( + "050801", + "050801-beta", + "050801-beta1", + "010203-rc.2", + "000000", + ) + val invalid = + listOf( + "5.8.1", + "05080", + "0508010", + "", + "050801-", + "050801/", + "v050801", + ) + valid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe true } + invalid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe false } + } + + test("OneSignalUtils.sdkVersion from BuildConfig matches Turbine label rules") { + FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(OneSignalUtils.sdkVersion) shouldBe true + } +}) + +private fun RemoteFeatureFlagsFetchOutcome.shouldBeSuccessWithEmptyKeys() { + check(this is RemoteFeatureFlagsFetchOutcome.Success) { + "expected Success, got $this" + } + this.result.enabledKeys shouldBe emptyList() +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt new file mode 100644 index 000000000..43983fce8 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -0,0 +1,143 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive + +class FeatureFlagsJsonParserTests : FunSpec({ + test("parses features array with sibling flag objects (sample contract)") { + val payload = + """ + { + "features": ["feature_a", "feature_b"], + "feature_a": { "weight": 0.1 }, + "feature_b": { "enabled": true } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("feature_a", "feature_b") + val meta = requireNotNull(r.metadata) + meta.getValue("feature_a").toString().contains("weight") shouldBe true + meta.getValue("feature_a").toString().contains("0.1") shouldBe true + meta.getValue("feature_b").toString().contains("enabled") shouldBe true + } + + test("omits metadata entry when flag has no sibling object") { + val payload = """{"features":["only_key"]}""" + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("only_key") + r.metadata shouldBe null + } + + test("features list plus sibling object for only some ids (valid JSON with commas)") { + val payload = + """ + { + "features": ["feature_a", "feature_b"], + "feature_a": { "weight": 0.1 } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("feature_a", "feature_b") + val meta = requireNotNull(r.metadata) + meta.containsKey("feature_a") shouldBe true + meta.containsKey("feature_b") shouldBe false + val weight = requireNotNull(meta.getValue("feature_a").jsonObject["weight"]).jsonPrimitive.content + weight shouldBe "0.1" + } + + test("normalizes feature ids to lowercase and resolves sibling with mismatched casing") { + val payload = + """ + { + "features": ["SDK_Background_Threading"], + "sdk_background_threading": { "weight": 0.5 } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("sdk_background_threading") + val meta = requireNotNull(r.metadata) + meta.getValue("sdk_background_threading").jsonObject.getValue("weight").jsonPrimitive.content shouldBe "0.5" + } + + test("invalid json returns empty") { + FeatureFlagsJsonParser.parse("{") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("parseSuccessful returns empty enabled keys for valid empty features array") { + val r = requireNotNull(FeatureFlagsJsonParser.parseSuccessful("""{"features":[]}""")) + r.enabledKeys shouldBe emptyList() + r.metadata shouldBe null + } + + test("parseSuccessful returns null for invalid json or contract") { + FeatureFlagsJsonParser.parseSuccessful("{") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"feature_a":{}}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":"bad"}""") shouldBe null + } + + test("parseSuccessful returns null when features array is non-empty but every element is contract-violating") { + // Element-type violations on a non-empty array must be Unavailable, not Success(empty); + // otherwise callers would overwrite the cached list with []. + FeatureFlagsJsonParser.parseSuccessful("""{"features":[1,2,3]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[null]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[{}]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[[]]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[true,false]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[""]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[" "]}""") shouldBe null + } + + test("parseSuccessful keeps mixed arrays (drops invalid elements, keeps valid ones)") { + // Tolerance contract: any valid string id keeps the result; invalid siblings are dropped. + val r = requireNotNull(FeatureFlagsJsonParser.parseSuccessful("""{"features":["good", 1, null, {}, ""]}""")) + r.enabledKeys shouldBe listOf("good") + r.metadata shouldBe null + } + + test("parseSuccessful returns null for Turbine error body (e.g. 403 Forbidden)") { + FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Forbidden"]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Not Found"]}""") shouldBe null + } + + test("encodeMetadata round-trips for storage string") { + val payload = + """ + {"features":["x"],"x":{"weight":2.5}} + """.trimIndent() + val r = FeatureFlagsJsonParser.parse(payload) + val encoded = requireNotNull(FeatureFlagsJsonParser.encodeMetadata(r.metadata)) + encoded.contains("weight") shouldBe true + encoded.contains("2.5") shouldBe true + } + + test("missing features key returns empty") { + FeatureFlagsJsonParser.parse("""{"feature_a":{}}""") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("features not an array returns empty") { + FeatureFlagsJsonParser.parse("""{"features":"bad"}""") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("parseStoredMetadataMap splits root object into flag id to JsonObject") { + val map = FeatureFlagsJsonParser.parseStoredMetadataMap("""{"a":{"weight":1},"b":2}""") + map.keys shouldBe setOf("a") + requireNotNull(map.getValue("a")["weight"]).toString() shouldBe "1" + } + + test("parseStoredMetadataMap blank or invalid returns empty") { + FeatureFlagsJsonParser.parseStoredMetadataMap(null) shouldBe emptyMap() + FeatureFlagsJsonParser.parseStoredMetadataMap("") shouldBe emptyMap() + FeatureFlagsJsonParser.parseStoredMetadataMap("{") shouldBe emptyMap() + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt new file mode 100644 index 000000000..13f16cb34 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt @@ -0,0 +1,26 @@ +package com.onesignal.core.internal.backend.impl + +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class TurbineSdkFeatureFlagsPathTest : FunSpec({ + test("percentEncodePathSegmentUtf8 leaves unreserved characters unchanged") { + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("android") shouldBe "android" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("050801-beta") shouldBe "050801-beta" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("aZ09-._~") shouldBe "aZ09-._~" + } + + test("percentEncodePathSegmentUtf8 encodes reserved and space as percent-hex") { + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("a/b") shouldBe "a%2Fb" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("a b") shouldBe "a%20b" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("café") shouldBe "caf%C3%A9" + } + + test("buildGetPath matches expected Turbine relative path") { + TurbineSdkFeatureFlagsPath.buildGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = "android", + sdkVersion = "050801-beta", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt new file mode 100644 index 000000000..c1ac8b6e6 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt @@ -0,0 +1,81 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.core.internal.backend.IParamsBackendService +import com.onesignal.core.internal.backend.ParamsObject +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.mocks.IOMockHelper +import com.onesignal.mocks.IOMockHelper.awaitIO +import com.onesignal.mocks.MockPreferencesService +import com.onesignal.user.internal.subscriptions.ISubscriptionManager +import com.onesignal.user.internal.subscriptions.SubscriptionList +import com.onesignal.user.subscriptions.IPushSubscription +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk + +/** + * Regression coverage for the snapshot/replace race in [ConfigModelStoreListener.fetchParams]: + * concurrent in-place writes to sdkRemoteFeatureFlags{,Metadata} from + * [FeatureFlagsRefreshService] must survive replace(). + */ +class ConfigModelStoreListenerTests : FunSpec({ + listener(IOMockHelper) + + test("fetchParams does not clobber sdkRemoteFeatureFlags written between snapshot and replace") { + // Real store so replace() goes through Model.initializeFromModel (data.clear()+putAll()). + val store = ConfigModelStore(MockPreferencesService()) + store.model.appId = "test-app-id" + store.model.sdkRemoteFeatureFlags = emptyList() + store.model.sdkRemoteFeatureFlagMetadata = null + + // Race trigger: locationShared is read between snapshot and replace, so its getter + // side-effect lands inside the window. Returning null short-circuits the let { } + // body; only the side-effect matters. + val params = mockk(relaxed = true) + every { params.locationShared } answers { + store.model.sdkRemoteFeatureFlags = listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata = """{"sdk_background_threading":{"x":1}}""" + null + } + + val paramsBackend = mockk() + coEvery { paramsBackend.fetchParams(any(), any()) } returns params + + val pushSub = mockk(relaxed = true) + val subscriptionManager = mockk() + every { subscriptionManager.subscriptions } returns SubscriptionList(emptyList(), pushSub) + + val listener = ConfigModelStoreListener(store, paramsBackend, subscriptionManager) + listener.start() + awaitIO() + + // Without the fix, the stale snapshot wins and these are [] / null. + store.model.sdkRemoteFeatureFlags shouldBe listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata shouldBe """{"sdk_background_threading":{"x":1}}""" + } + + test("fetchParams preserves sdkRemoteFeatureFlags already on the live model when no in-flight write occurs") { + // Smoke: no race -> existing values round-trip through snapshot + replace. + val store = ConfigModelStore(MockPreferencesService()) + store.model.appId = "test-app-id" + store.model.sdkRemoteFeatureFlags = listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata = """{"sdk_background_threading":{"x":1}}""" + + val params = mockk(relaxed = true) + val paramsBackend = mockk() + coEvery { paramsBackend.fetchParams(any(), any()) } returns params + + val pushSub = mockk(relaxed = true) + val subscriptionManager = mockk() + every { subscriptionManager.subscriptions } returns SubscriptionList(emptyList(), pushSub) + + val listener = ConfigModelStoreListener(store, paramsBackend, subscriptionManager) + listener.start() + awaitIO() + + store.model.sdkRemoteFeatureFlags shouldBe listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata shouldBe """{"sdk_background_threading":{"x":1}}""" + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt new file mode 100644 index 000000000..5ceb40e77 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt @@ -0,0 +1,190 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.application.IApplicationLifecycleHandler +import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.mocks.IOMockHelper +import com.onesignal.mocks.IOMockHelper.awaitIO +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.slot + +/** + * Regression coverage for the duplicate Turbine feature-flags fetch at SDK startup. + * + * Two startables fire at init: + * 1. [FeatureFlagsRefreshService.start] -> [IApplicationService.addApplicationLifecycleHandler] + * synchronously delivers `onFocus(firedOnSubscribe = true)` while the app is foregrounded, + * which kicks off polling and issues fetch #1. + * 2. [ConfigModelStoreListener.fetchParams] (Android params) finishes and calls + * `_configModelStore.replace(config, ModelChangeTags.HYDRATE)`, which fires `onModelReplaced` + * on every subscriber. The replaced model carries the **same** `appId`, but pre-fix the + * listener still triggered a second poll, producing fetch #2 a few hundred ms after #1. + * + * The fix tracks the appId currently being polled and treats a re-trigger for the same appId as + * a no-op. Tests below assert both directions: same-appId hydrates are deduped, but a genuine + * appId change still cancels and re-fetches. + */ +class FeatureFlagsRefreshServiceTests : FunSpec({ + listener(IOMockHelper) + + fun mockBackend(): Pair Int> { + val backend = mockk() + var count = 0 + coEvery { backend.fetchRemoteFeatureFlags(any()) } answers { + count++ + RemoteFeatureFlagsFetchOutcome.Unavailable + } + return backend to { count } + } + + /** + * Builds an [IApplicationService] mock that mirrors production behavior at init: + * - `addApplicationLifecycleHandler` synchronously fires `onFocus(true)` (matches + * [com.onesignal.core.internal.application.impl.ApplicationService.addApplicationLifecycleHandler] + * when `current != null`). + * - `isInForeground` returns the supplied [foregroundSequence] in order; tests pick exactly + * the right pattern of `true`/`false` so that each polling loop runs one fetch then exits + * on the next iteration. Coupled with `refreshIntervalMs = 0L` this keeps tests + * deterministic without needing a virtual time scheduler. + * + * Per-trigger budget for [foregroundSequence]: + * - Initial `start()` -> `onFocus(true)` -> polling loop: **2** values (`true`, `false`). + * - `onModelReplaced` / `onModelUpdated` event handler condition: **1** value. + * - Each polling loop kicked off by an event handler when not deduped: **2** values. + */ + fun foregroundedAppService(vararg foregroundSequence: Boolean): IApplicationService { + val app = mockk() + val handlerSlot = slot() + every { app.addApplicationLifecycleHandler(capture(handlerSlot)) } answers { + handlerSlot.captured.onFocus(true) + } + every { app.removeApplicationLifecycleHandler(any()) } just Runs + every { app.isInForeground } returnsMany foregroundSequence.toList() + return app + } + + fun mockConfigStore(model: ConfigModel): ConfigModelStore { + val store = mockk() + every { store.model } returns model + every { store.subscribe(any()) } just Runs + every { store.unsubscribe(any()) } just Runs + return store + } + + test("HYDRATE replace with the same appId does not trigger a duplicate fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] (loop iter1=true, iter2=false break) + onModelReplaced check: [true] + val app = foregroundedAppService(true, false, true) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + + // Mirrors `OneSignalImp.internalInit` -> `startupService.scheduleStart()`: synchronously + // attaches the lifecycle handler, which fires `onFocus(true)` -> initial fetch. + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // Mirrors `ConfigModelStoreListener.fetchParams` -> `replace(config, HYDRATE)` with the + // same appId already on the live model. Pre-fix this fired a second Turbine GET. + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + awaitIO() + fetchCount() shouldBe 1 + } + + test("NORMAL replace with the same appId does not trigger a duplicate fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + val app = foregroundedAppService(true, false, true) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + service.onModelReplaced(model, ModelChangeTags.NORMAL) + awaitIO() + fetchCount() shouldBe 1 + } + + test("HYDRATE replace with a different appId cancels and re-fetches") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onModelReplaced check: [true] + new poll loop: [true, false]. + val app = foregroundedAppService(true, false, true, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // Genuine appId change (e.g. re-init with a different App ID). + model.appId = "appId-2" + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + awaitIO() + fetchCount() shouldBe 2 + + coVerify(exactly = 1) { backend.fetchRemoteFeatureFlags("appId-1") } + coVerify(exactly = 1) { backend.fetchRemoteFeatureFlags("appId-2") } + } + + test("appId property update via onModelUpdated triggers a single re-fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onModelUpdated check: [true] + new poll loop: [true, false]. + val app = foregroundedAppService(true, false, true, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + model.appId = "appId-2" + service.onModelUpdated( + com.onesignal.common.modeling.ModelChangedArgs( + model = model, + path = ConfigModel::appId.name, + property = ConfigModel::appId.name, + oldValue = "appId-1", + newValue = "appId-2", + ), + ModelChangeTags.NORMAL, + ) + awaitIO() + fetchCount() shouldBe 2 + } + + test("unfocus then refocus restarts polling for the same appId") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onFocus restart loop: [true, false]. + val app = foregroundedAppService(true, false, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // onUnfocused must clear the cached pollingAppId so a subsequent refocus is not deduped. + service.onUnfocused() + service.onFocus(firedOnSubscribe = false) + awaitIO() + fetchCount() shouldBe 2 + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt index 2a7c77cfe..54cf41b3a 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt @@ -4,15 +4,15 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe class FeatureFlagTests : FunSpec({ - test("feature flags use SDK_050800 style key naming") { - val keyPattern = Regex("^SDK_[0-9]{6}_[A-Z0-9_]+$") + test("feature flag remote keys are lowercase with underscores") { + val keyPattern = Regex("^[a-z0-9_]+$") FeatureFlag.entries.forEach { feature -> keyPattern.matches(feature.key) shouldBe true } } - test("BACKGROUND_THREADING uses the expected canonical key") { - FeatureFlag.SDK_050800_BACKGROUND_THREADING.key shouldBe "SDK_050800_BACKGROUND_THREADING" + test("SDK_BACKGROUND_THREADING uses the expected remote key") { + FeatureFlag.SDK_BACKGROUND_THREADING.key shouldBe "sdk_background_threading" } }) 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 e7edae371..128a50b63 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 @@ -10,93 +10,138 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs +import kotlinx.serialization.json.jsonPrimitive class FeatureManagerTests : FunSpec({ beforeEach { ThreadingMode.useBackgroundThreading = false } - test("initial state should enable BACKGROUND_THREADING when feature is present") { - // Given + fun stubConfigModel(model: ConfigModel) { + every { model.sdkRemoteFeatureFlags } returns emptyList() + every { model.sdkRemoteFeatureFlagMetadata } returns null + } + + test("initial state enables BACKGROUND_THREADING when key is present in sdk remote flags") { val initialModel = mockk() - every { initialModel.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(initialModel) + every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - // When val manager = FeatureManager(configModelStore) - // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe true + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true + ThreadingMode.useBackgroundThreading shouldBe true + } + + test("initial state enables BACKGROUND_THREADING when remote key differs only by letter case") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.sdkRemoteFeatureFlags } returns listOf("SDK_Background_Threading") + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore) + + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true ThreadingMode.useBackgroundThreading shouldBe true } test("onModelReplaced should not switch threading mode after startup") { // Given val initialModel = mockk() - every { initialModel.features } returns emptyList() + stubConfigModel(initialModel) val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) val updatedModel = mockk() - every { updatedModel.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(updatedModel) + every { updatedModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelReplaced(updatedModel, ModelChangeTags.HYDRATE) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe false + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe false ThreadingMode.useBackgroundThreading shouldBe false } test("onModelUpdated should not switch threading mode after startup") { // Given val model = mockk() - every { model.features } returns emptyList() + stubConfigModel(model) val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) - every { model.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + every { model.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelUpdated( args = mockk { - every { property } returns ConfigModel::features.name + every { property } returns ConfigModel::sdkRemoteFeatureFlags.name }, tag = ModelChangeTags.NORMAL ) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe false + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe false ThreadingMode.useBackgroundThreading shouldBe false } test("onModelUpdated should keep startup mode when initial mode is enabled") { // Given val model = mockk() - every { model.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(model) + every { model.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) - every { model.features } returns emptyList() + every { model.sdkRemoteFeatureFlags } returns emptyList() // When manager.onModelUpdated( args = mockk { - every { property } returns ConfigModel::features.name + every { property } returns ConfigModel::sdkRemoteFeatureFlags.name }, tag = ModelChangeTags.NORMAL ) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe true + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true ThreadingMode.useBackgroundThreading shouldBe true } + + test("remoteFeatureFlagMetadata returns parsed JSON from config") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.sdkRemoteFeatureFlagMetadata } returns """{"X":{"note":"y"}}""" + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore) + + val meta = requireNotNull(manager.remoteFeatureFlagMetadata()) + requireNotNull(meta.getValue("X")["note"]).jsonPrimitive.content shouldBe "y" + } + + test("remoteFeatureFlagMetadata is null when config has no stored metadata") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.sdkRemoteFeatureFlagMetadata } returns null + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + FeatureManager(configModelStore).remoteFeatureFlagMetadata() shouldBe null + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index dc7fa72bb..d8333ac8e 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -26,7 +26,8 @@ class StartupServiceTests : FunSpec({ backgroundThreadingEnabled: Boolean = true, ): ServiceProvider { val featureManager = mockk() - every { featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.remoteFeatureFlagMetadata() } returns null val serviceBuilder = ServiceBuilder() serviceBuilder.register(featureManager).provides() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt index 63ff63b74..1f30da699 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt @@ -41,7 +41,8 @@ class OneSignalCrashUploaderWrapperTest : FunSpec({ fun mockFeatureManager(backgroundThreadingEnabled: Boolean = true): IFeatureManager { val featureManager = mockk() - every { featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.remoteFeatureFlagMetadata() } returns null return featureManager }