From 23fa3141f2d5251c9a07200d1463f2a904d12e81 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 22 Oct 2026 15:53:42 +0000 Subject: [PATCH 01/34] FFL-3459: Support flagSelection on Flags precompute requests Add FlagsConfiguration.Builder.flagSelection(tags, keyPatterns). The selection is canonicalized and validated like Compute and ffe-service, sent as flag_selection on the precompute POST, and stored with the persisted state so a cache fetched with another selection is ignored. Report DD-Flag-Selection-Ignored, and treat a valid empty configuration as loaded so a later failed fetch reports Stale. Ref: FFL-3459 --- detekt_custom_safe_calls_third_party.yml | 2 + features/dd-sdk-android-flags/api/apiSurface | 1 + .../api/dd-sdk-android-flags.api | 1 + .../com/datadog/android/flags/FlagsClient.kt | 1 + .../android/flags/FlagsConfiguration.kt | 35 +++- .../android/flags/internal/FlagsFeature.kt | 15 +- .../internal/evaluation/EvaluationsManager.kt | 9 +- .../flags/internal/model/FlagsStateEntry.kt | 4 +- .../model/PrecomputedFlagConstants.kt | 3 + .../net/PrecomputedAssignmentsDownloader.kt | 20 +- .../net/PrecomputedAssignmentsReader.kt | 14 +- .../PrecomputedAssignmentsRequestFactory.kt | 12 +- .../persistence/FlagsPersistenceManager.kt | 5 +- .../persistence/FlagsStateDeserializer.kt | 16 +- .../persistence/FlagsStateSerializer.kt | 10 + .../repository/DefaultFlagsRepository.kt | 15 +- .../internal/repository/FlagsRepository.kt | 4 + .../flags/internal/selection/FlagSelection.kt | 121 +++++++++++ .../selection/FlagSelectionLogging.kt | 46 ++++ .../android/flags/FlagsConfigurationTest.kt | 48 +++++ .../flags/internal/FlagsFeatureTest.kt | 46 ++++ .../evaluation/EvaluationsManagerTest.kt | 128 ++++++++++-- .../PrecomputedAssignmentsDownloaderTest.kt | 28 ++- ...recomputedAssignmentsRequestFactoryTest.kt | 78 ++++++- .../persistence/FlagsStateDeserializerTest.kt | 28 +++ .../persistence/FlagsStateSerializerTest.kt | 49 ++++- .../repository/DefaultFlagsRepositoryTest.kt | 133 +++++++++++- .../repository/FirstFlagsInstallationTest.kt | 6 +- .../FlagSelectionCanonicalizerTest.kt | 197 ++++++++++++++++++ 29 files changed, 1013 insertions(+), 62 deletions(-) create mode 100644 features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt create mode 100644 features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt create mode 100644 features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt diff --git a/detekt_custom_safe_calls_third_party.yml b/detekt_custom_safe_calls_third_party.yml index 7fc6a3c29d..c1e68bdce8 100644 --- a/detekt_custom_safe_calls_third_party.yml +++ b/detekt_custom_safe_calls_third_party.yml @@ -865,6 +865,8 @@ datadog: - "kotlin.ByteArray.constructor(kotlin.Int)" - "kotlin.ByteArray.copyTo(kotlin.Int, kotlin.ByteArray, kotlin.Int, kotlin.Int, com.datadog.android.api.InternalLogger)" - "kotlin.Char.digitToIntOrNull()" + - "kotlin.Char.isHighSurrogate()" + - "kotlin.Char.isLowSurrogate()" - "kotlin.Char.isLowerCase()" - "kotlin.Char.titlecase(java.util.Locale)" - "kotlin.CharArray.constructor(kotlin.Int, kotlin.Function1)" diff --git a/features/dd-sdk-android-flags/api/apiSurface b/features/dd-sdk-android-flags/api/apiSurface index 8b95feb419..d2aeef4c48 100644 --- a/features/dd-sdk-android-flags/api/apiSurface +++ b/features/dd-sdk-android-flags/api/apiSurface @@ -33,6 +33,7 @@ data class com.datadog.android.flags.FlagsConfiguration fun evaluationFlushInterval(Long): Builder fun useCustomFlagEndpoint(String): Builder fun initializationTimeout(Long): Builder + fun flagSelection(List, List): Builder fun rumIntegrationEnabled(Boolean): Builder fun gracefulModeEnabled(Boolean): Builder fun build(): FlagsConfiguration diff --git a/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api b/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api index 0c62e195f7..ea6752b8f9 100644 --- a/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api +++ b/features/dd-sdk-android-flags/api/dd-sdk-android-flags.api @@ -66,6 +66,7 @@ public final class com/datadog/android/flags/FlagsConfiguration$Builder { public fun ()V public final fun build ()Lcom/datadog/android/flags/FlagsConfiguration; public final fun evaluationFlushInterval (J)Lcom/datadog/android/flags/FlagsConfiguration$Builder; + public final fun flagSelection (Ljava/util/List;Ljava/util/List;)Lcom/datadog/android/flags/FlagsConfiguration$Builder; public final fun gracefulModeEnabled (Z)Lcom/datadog/android/flags/FlagsConfiguration$Builder; public final fun initializationTimeout (J)Lcom/datadog/android/flags/FlagsConfiguration$Builder; public final fun rumIntegrationEnabled (Z)Lcom/datadog/android/flags/FlagsConfiguration$Builder; diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt index 76ab698407..da0f70997d 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt @@ -442,6 +442,7 @@ interface FlagsClient { featureSdkCore = featureSdkCore, dataStore = datastore, instanceName = name, + flagSelection = flagsFeature.flagSelection, deliverFirstFlags = { delivery -> val worker = featureSdkCore.createSingleThreadExecutorService(FLAGS_FIRST_FLAGS_EXECUTOR_NAME) try { diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index 88d2b2405a..f8bf636bae 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -6,6 +6,8 @@ package com.datadog.android.flags +import com.datadog.android.flags.internal.selection.FlagSelection + private const val DEFAULT_INITIALIZATION_TIMEOUT_MS = 5_000L /** @@ -21,10 +23,11 @@ data class FlagsConfiguration internal constructor( internal val evaluationFlushIntervalMs: Long, internal val rumIntegrationEnabled: Boolean, internal val gracefulModeEnabled: Boolean, - internal val initializationTimeoutMs: Long? + internal val initializationTimeoutMs: Long?, + internal val flagSelection: FlagSelection? ) { /** - * Copies this configuration and preserves the initialization timeout. + * Copies this configuration and preserves the initialization timeout and the flag selection. * * This overload keeps the public JVM signature that existed before the initialization timeout was added. */ @@ -46,7 +49,8 @@ data class FlagsConfiguration internal constructor( evaluationFlushIntervalMs = evaluationFlushIntervalMs, rumIntegrationEnabled = rumIntegrationEnabled, gracefulModeEnabled = gracefulModeEnabled, - initializationTimeoutMs = initializationTimeoutMs + initializationTimeoutMs = initializationTimeoutMs, + flagSelection = flagSelection ) /** @@ -62,6 +66,7 @@ data class FlagsConfiguration internal constructor( private var rumIntegrationEnabled: Boolean = true private var gracefulModeEnabled: Boolean = true private var initializationTimeoutMs: Long? = DEFAULT_INITIALIZATION_TIMEOUT_MS + private var flagSelection: FlagSelection? = null /** * Sets whether exposures should be logged to the dedicated exposures intake endpoint. @@ -170,6 +175,27 @@ data class FlagsConfiguration internal constructor( initializationTimeoutMs = timeoutMs.takeIf { it > 0 } } + /** + * Loads only the flags that match the given tags or key patterns. + * + * Tags match exactly and are case-sensitive; a flag matches if it has any of the tags. Key patterns are + * whole-key globs where only `*` is special. Leading and trailing Unicode whitespace is trimmed from every + * value, empty values are dropped, and duplicates are removed. Each list accepts at most 31 values, and each + * tag and key pattern is at most 200 UTF-8 bytes. A selection over a limit is not sent, and the full flag + * configuration is loaded. Flags outside the selection are not loaded, so evaluating them returns the + * default value. + * + * The selection is fixed when the Flags feature is enabled. Calling this method again replaces the + * previous selection. + * + * @param tags The flag tags to load. + * @param keyPatterns The flag key patterns to load. + * @return this [Builder] instance for method chaining. + */ + fun flagSelection(tags: List, keyPatterns: List): Builder = apply { + flagSelection = FlagSelection(tags.toList(), keyPatterns.toList()) + } + /** * Sets whether RUM evaluation logging is enabled. * This adds the result of evaluating a feature flag to the view. @@ -215,7 +241,8 @@ data class FlagsConfiguration internal constructor( evaluationFlushIntervalMs = evaluationFlushIntervalMs, rumIntegrationEnabled = rumIntegrationEnabled, gracefulModeEnabled = gracefulModeEnabled, - initializationTimeoutMs = initializationTimeoutMs + initializationTimeoutMs = initializationTimeoutMs, + flagSelection = flagSelection ) internal companion object { diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagsFeature.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagsFeature.kt index 94ec13af65..a1637449ba 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagsFeature.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagsFeature.kt @@ -20,6 +20,10 @@ import com.datadog.android.flags.FlagsConfiguration import com.datadog.android.flags.internal.evaluation.InitializationTimeoutScheduler import com.datadog.android.flags.internal.net.ExposuresRequestFactory import com.datadog.android.flags.internal.net.PrecomputedAssignmentsRequestFactory +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection +import com.datadog.android.flags.internal.selection.FlagSelectionCanonicalizer +import com.datadog.android.flags.internal.selection.FlagSelectionResult +import com.datadog.android.flags.internal.selection.logDroppedFlagSelection import com.datadog.android.flags.internal.storage.ExposureEventRecordWriter import com.datadog.android.flags.internal.storage.NoOpRecordWriter import com.datadog.android.flags.internal.storage.RecordWriter @@ -75,10 +79,16 @@ internal class FlagsFeature( // region Domain Objects + private val flagSelectionResult = FlagSelectionCanonicalizer.canonicalize(flagsConfiguration.flagSelection) + + internal val flagSelection: CanonicalFlagSelection? = + (flagSelectionResult as? FlagSelectionResult.Selected)?.selection + internal val precomputedRequestFactory = PrecomputedAssignmentsRequestFactory( internalLogger = sdkCore.internalLogger, - customFlagEndpoint = flagsConfiguration.customFlagEndpoint + customFlagEndpoint = flagsConfiguration.customFlagEndpoint, + flagSelection = flagSelection ) internal val initializationTimeoutScheduler = InitializationTimeoutScheduler { timeoutMs, action -> @@ -122,6 +132,9 @@ internal class FlagsFeature( } isDebugBuild = (appContext.applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE) != 0 isInitialized = true + (flagSelectionResult as? FlagSelectionResult.Dropped)?.let { + sdkCore.internalLogger.logDroppedFlagSelection(it.error) + } dataWriter = createDataWriter() processor = ExposureEventsProcessor( writer = dataWriter, diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index 05ff5c0b20..5378742a27 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -17,8 +17,10 @@ import com.datadog.android.flags.internal.FlagsStateManager import com.datadog.android.flags.internal.model.PrecomputedFlag import com.datadog.android.flags.internal.net.NetworkRequestFailedException import com.datadog.android.flags.internal.net.PrecomputedAssignmentsReader +import com.datadog.android.flags.internal.net.PrecomputedAssignmentsResponse import com.datadog.android.flags.internal.repository.FlagsRepository import com.datadog.android.flags.internal.repository.net.PrecomputeMapper +import com.datadog.android.flags.internal.selection.logIgnoredFlagSelection import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.FlagsClientState import java.util.concurrent.Executor @@ -146,7 +148,7 @@ internal class EvaluationsManager( hadFlags && flagsRepository.getEvaluationContext() == context ) val response = datadogContext?.let { assignmentsReader.readPrecomputedFlags(context, it) } - val flagsMap = response?.let { precomputeMapper.map(it) } + val flagsMap = response?.let { mapResponse(it) } if (flagsMap != null) { installFlags(context, flagsMap, initializationCompletion, callback) } else { @@ -198,6 +200,11 @@ internal class EvaluationsManager( } } + private fun mapResponse(response: PrecomputedAssignmentsResponse): Map? { + response.flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } + return precomputeMapper.map(response.body) + } + private fun installFlags( context: EvaluationContext, flagsMap: Map, diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/FlagsStateEntry.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/FlagsStateEntry.kt index d61d3f383f..0a40bfaed6 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/FlagsStateEntry.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/FlagsStateEntry.kt @@ -6,10 +6,12 @@ package com.datadog.android.flags.internal.model +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext internal data class FlagsStateEntry( val evaluationContext: EvaluationContext, val flags: Map, - val lastUpdateTimestamp: Long + val lastUpdateTimestamp: Long, + val flagSelection: CanonicalFlagSelection? ) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/PrecomputedFlagConstants.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/PrecomputedFlagConstants.kt index 4a013b0da1..68e6503861 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/PrecomputedFlagConstants.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/model/PrecomputedFlagConstants.kt @@ -15,6 +15,9 @@ internal enum class JsonKeys(val value: String) { // FlagsStateEntry keys FLAGS("flags"), LAST_UPDATE_TIMESTAMP("lastUpdateTimestamp"), + FLAG_SELECTION("flagSelection"), + TAGS("tags"), + KEY_PATTERNS("keyPatterns"), // PrecomputedFlag keys VARIATION_TYPE("variationType"), diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index 65311982c4..e68f0ef4f5 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -28,14 +28,17 @@ internal class PrecomputedAssignmentsDownloader( ) : PrecomputedAssignmentsReader { @WorkerThread - override fun readPrecomputedFlags(context: EvaluationContext, datadogContext: DatadogContext): String? { + override fun readPrecomputedFlags( + context: EvaluationContext, + datadogContext: DatadogContext + ): PrecomputedAssignmentsResponse? { val request = requestFactory.create(context, datadogContext) ?: return null return executeDownloadRequest(request) } @Suppress("TooGenericExceptionCaught") - private fun executeDownloadRequest(request: Request): String? = try { + private fun executeDownloadRequest(request: Request): PrecomputedAssignmentsResponse? = try { val response = callFactory.newCall(request).execute() handleResponse(response) } catch (e: Throwable) { @@ -48,9 +51,14 @@ internal class PrecomputedAssignmentsDownloader( null } - private fun handleResponse(response: Response): String? = if (response.isSuccessful) { + private fun handleResponse(response: Response): PrecomputedAssignmentsResponse? = if (response.isSuccessful) { @Suppress("UnsafeThirdPartyFunctionCall") // Safe: wrapped in outer try-catch - response.body?.use { it.string() } + response.body?.use { it.string() }?.let { body -> + PrecomputedAssignmentsResponse( + body = body, + flagSelectionIgnoredReason = response.header(HEADER_FLAG_SELECTION_IGNORED) + ) + } } else { internalLogger.log( InternalLogger.Level.ERROR, @@ -70,4 +78,8 @@ internal class PrecomputedAssignmentsDownloader( null } + + private companion object { + const val HEADER_FLAG_SELECTION_IGNORED = "DD-Flag-Selection-Ignored" + } } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt index e254a9c08e..a17478a480 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt @@ -14,5 +14,17 @@ import com.datadog.tools.annotation.NoOpImplementation @NoOpImplementation internal interface PrecomputedAssignmentsReader { @WorkerThread - fun readPrecomputedFlags(context: EvaluationContext, datadogContext: DatadogContext): String? + fun readPrecomputedFlags( + context: EvaluationContext, + datadogContext: DatadogContext + ): PrecomputedAssignmentsResponse? } + +/** + * @param body the raw response body + * @param flagSelectionIgnoredReason the `DD-Flag-Selection-Ignored` header value, or null when absent + */ +internal data class PrecomputedAssignmentsResponse( + val body: String, + val flagSelectionIgnoredReason: String? +) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt index e7414d824c..1347263ba7 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt @@ -10,11 +10,13 @@ import com.datadog.android.api.InternalLogger import com.datadog.android.api.context.DatadogContext import com.datadog.android.api.feature.Feature import com.datadog.android.flags.internal.getFlagsEndpoint +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import okhttp3.Headers import okhttp3.Request import okhttp3.RequestBody import okhttp3.RequestBody.Companion.toRequestBody +import org.json.JSONArray import org.json.JSONException import org.json.JSONObject @@ -23,7 +25,8 @@ import org.json.JSONObject */ internal class PrecomputedAssignmentsRequestFactory( private val internalLogger: InternalLogger, - private val customFlagEndpoint: String? + private val customFlagEndpoint: String?, + private val flagSelection: CanonicalFlagSelection? ) { /** @@ -94,6 +97,7 @@ internal class PrecomputedAssignmentsRequestFactory( .put("env", env) .put("source", source) .put("subject", subject) + flagSelection?.let { attributes.put("flag_selection", buildFlagSelectionPayload(it)) } val data = JSONObject() .put("type", "precompute-assignments-request") .put("attributes", attributes) @@ -134,6 +138,12 @@ internal class PrecomputedAssignmentsRequestFactory( .put("sdk_name", SDK_NAME) .put("sdk_version", datadogContext.sdkVersion) + @Suppress("UnsafeThirdPartyFunctionCall") // call wrapped in try/catch + private fun buildFlagSelectionPayload(selection: CanonicalFlagSelection): JSONObject = + JSONObject() + .put("tags", JSONArray(selection.tags)) + .put("key_patterns", JSONArray(selection.keyPatterns)) + private val DatadogContext.rumApplicationId: String? get() = featuresContext.get(Feature.RUM_FEATURE_NAME) ?.get("application_id") as? String diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt index 9563a6eb8d..c26c6f5310 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt @@ -13,6 +13,7 @@ import com.datadog.android.api.storage.datastore.DataStoreWriteCallback import com.datadog.android.core.persistence.datastore.DataStoreContent import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext internal class FlagsPersistenceManager( @@ -32,13 +33,15 @@ internal class FlagsPersistenceManager( internal fun saveFlagsState( context: EvaluationContext, flags: Map, + flagSelection: CanonicalFlagSelection?, currentTimestamp: Long, callback: DataStoreWriteCallback? = null ) { val entry = FlagsStateEntry( evaluationContext = context, flags = flags, - lastUpdateTimestamp = currentTimestamp + lastUpdateTimestamp = currentTimestamp, + flagSelection = flagSelection ) dataStore.setValue( diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializer.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializer.kt index c0216edeec..1803d3a326 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializer.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializer.kt @@ -11,7 +11,9 @@ import com.datadog.android.core.internal.persistence.Deserializer import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.JsonKeys import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext +import org.json.JSONArray import org.json.JSONException import org.json.JSONObject @@ -32,6 +34,8 @@ internal class FlagsStateDeserializer(private val internalLogger: InternalLogger @Suppress("UnsafeThirdPartyFunctionCall") // JSONObject operations wrapped in try-catch val timestamp = json.getLong(JsonKeys.LAST_UPDATE_TIMESTAMP.value) + val flagSelection = json.optJSONObject(JsonKeys.FLAG_SELECTION.value)?.let { deserializeFlagSelection(it) } + // A nonempty cache with no readable flags is not a valid empty configuration. if (flags.isEmpty() && flagsJson.length() > 0) { null @@ -39,7 +43,8 @@ internal class FlagsStateDeserializer(private val internalLogger: InternalLogger FlagsStateEntry( evaluationContext = evaluationContext, flags = flags, - lastUpdateTimestamp = timestamp + lastUpdateTimestamp = timestamp, + flagSelection = flagSelection ) } } catch (e: JSONException) { @@ -74,6 +79,15 @@ internal class FlagsStateDeserializer(private val internalLogger: InternalLogger return EvaluationContext(targetingKey, attributes) } + @Suppress("UnsafeThirdPartyFunctionCall") // JSONObject operations wrapped in try-catch + private fun deserializeFlagSelection(selectionJson: JSONObject): CanonicalFlagSelection = CanonicalFlagSelection( + tags = selectionJson.getJSONArray(JsonKeys.TAGS.value).toStringList(), + keyPatterns = selectionJson.getJSONArray(JsonKeys.KEY_PATTERNS.value).toStringList() + ) + + @Suppress("UnsafeThirdPartyFunctionCall") // JSONArray operations wrapped in try-catch + private fun JSONArray.toStringList(): List = List(length()) { getString(it) } + @Suppress("UnsafeThirdPartyFunctionCall") // JSONObject operations wrapped in try-catch private fun deserializeAttributes(attributesJson: JSONObject): Map { val attributes = mutableMapOf() diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializer.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializer.kt index e950b6bdda..7872830827 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializer.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializer.kt @@ -11,6 +11,8 @@ import com.datadog.android.core.persistence.Serializer import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.JsonKeys import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection +import org.json.JSONArray import org.json.JSONException import org.json.JSONObject @@ -37,6 +39,8 @@ internal class FlagsStateSerializer(private val internalLogger: InternalLogger) json.put(JsonKeys.LAST_UPDATE_TIMESTAMP.value, model.lastUpdateTimestamp) + model.flagSelection?.let { json.put(JsonKeys.FLAG_SELECTION.value, serializeFlagSelection(it)) } + json.toString() } catch (e: JSONException) { internalLogger.log( @@ -57,6 +61,12 @@ internal class FlagsStateSerializer(private val internalLogger: InternalLogger) return attributesJson } + @Suppress("UnsafeThirdPartyFunctionCall") // JSONObject operations wrapped in try-catch + private fun serializeFlagSelection(selection: CanonicalFlagSelection): JSONObject = JSONObject().apply { + put(JsonKeys.TAGS.value, JSONArray(selection.tags)) + put(JsonKeys.KEY_PATTERNS.value, JSONArray(selection.keyPatterns)) + } + @Suppress("UnsafeThirdPartyFunctionCall") // JSONObject operations wrapped in try-catch private fun serializePrecomputedFlag(flag: PrecomputedFlag): JSONObject = JSONObject().apply { put(JsonKeys.VARIATION_TYPE.value, flag.variationType) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt index 5687a6f274..6fb6f1d176 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt @@ -12,6 +12,7 @@ import com.datadog.android.api.storage.datastore.DataStoreHandler import com.datadog.android.api.storage.datastore.DataStoreWriteCallback import com.datadog.android.flags.internal.model.PrecomputedFlag import com.datadog.android.flags.internal.persistence.FlagsPersistenceManager +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.ResolutionReason import java.util.concurrent.CountDownLatch @@ -22,6 +23,7 @@ internal class DefaultFlagsRepository( private val featureSdkCore: FeatureSdkCore, private val instanceName: String, private val dataStore: DataStoreHandler, + private val flagSelection: CanonicalFlagSelection?, private val internalLogger: InternalLogger = featureSdkCore.internalLogger, private val persistenceLoadTimeoutMs: Long = PERSISTENCE_LOAD_TIMEOUT_MS, private val deliverFirstFlags: (Runnable) -> Unit = Runnable::run @@ -37,7 +39,9 @@ internal class DefaultFlagsRepository( dataStore = dataStore, instanceName = instanceName, internalLogger = internalLogger - ) { persistedState -> + ) { loadedState -> + // State fetched with a different selection holds a different set of flags. + val persistedState = loadedState?.takeIf { it.flagSelection == flagSelection } val installed = try { persistedState != null && atomicState.compareAndSet( null, @@ -68,6 +72,7 @@ internal class DefaultFlagsRepository( persistenceManager.saveFlagsState( context = context, flags = flags, + flagSelection = flagSelection, currentTimestamp = featureSdkCore.timeProvider.getDeviceTimestampMillis(), object : DataStoreWriteCallback { override fun onSuccess() { @@ -139,13 +144,11 @@ internal class DefaultFlagsRepository( override fun hasFlags(): Boolean { waitForPersistenceLoad() - return atomicState.get()?.flags?.isNotEmpty() ?: false + return atomicState.get() != null } - override fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean { - val state = atomicState.get() - return state?.context == context && state.flags.isNotEmpty() - } + override fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean = + atomicState.get()?.context == context override fun getPrecomputedFlagWithContext(key: String): Pair? { val state = atomicState.get() ?: return null diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt index 1bcab8e477..a5f2aee06f 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt @@ -29,7 +29,11 @@ internal interface FlagsRepository { // Nonblocking snapshot. Missing flags retain their context from the same atomic state read. fun getPrecomputedFlagWithContext(key: String): Pair? val firstFlags: FirstFlagsLatch + + /** True once a configuration is installed, including a valid configuration with zero flags. */ fun hasFlags(): Boolean + + /** True when the installed configuration is for [context], including a valid configuration with zero flags. */ fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean fun getFlagsSnapshot(): Map } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt new file mode 100644 index 0000000000..23fa30df49 --- /dev/null +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -0,0 +1,121 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.flags.internal.selection + +/** + * The flag selection as configured by the application, before canonicalization. + * Elements are nullable because Java callers can pass lists that hold nulls. + */ +internal data class FlagSelection( + val tags: List, + val keyPatterns: List +) + +/** + * A flag selection that was trimmed, deduplicated, sorted in UTF-8 byte order and validated. + */ +internal data class CanonicalFlagSelection( + val tags: List, + val keyPatterns: List +) + +/** + * Reasons a flag selection is dropped. Values match the reasons used by Compute and ffe-service. + */ +internal enum class FlagSelectionError(val reason: String, val limit: String) { + MALFORMED_SELECTION( + "malformed_selection", + "tags and key patterns must be non-null strings without unpaired UTF-16 surrogates" + ), + TOO_MANY_VALUES( + "too_many_values", + "each list accepts at most ${FlagSelectionCanonicalizer.MAX_VALUES} values" + ), + INVALID_KEY_PATTERN( + "invalid_key_pattern", + "each key pattern is at most ${FlagSelectionCanonicalizer.MAX_KEY_PATTERN_BYTES} UTF-8 bytes" + ), + INVALID_TAG( + "invalid_tag", + "each tag is at most ${FlagSelectionCanonicalizer.MAX_TAG_BYTES} UTF-8 bytes" + ) +} + +internal sealed interface FlagSelectionResult { + object None : FlagSelectionResult + data class Selected(val selection: CanonicalFlagSelection) : FlagSelectionResult + data class Dropped(val error: FlagSelectionError) : FlagSelectionResult +} + +/** + * Canonicalizes and validates a flag selection the same way Compute and ffe-service do. + */ +internal object FlagSelectionCanonicalizer { + + internal const val MAX_VALUES = 31 + internal const val MAX_TAG_BYTES = 200 + internal const val MAX_KEY_PATTERN_BYTES = 200 + + // The Unicode White_Space property, which Compute trims with Rust's str::trim. + // Kotlin's trim() differs: it also trims U+001C..U+001F and does not trim U+0085. + private const val WHITE_SPACE = "\u0009\u000A\u000B\u000C\u000D\u0020\u0085\u00A0\u1680" + + "\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A" + + "\u2028\u2029\u202F\u205F\u3000" + + @Suppress("ReturnCount") + fun canonicalize(selection: FlagSelection?): FlagSelectionResult { + if (selection == null) return FlagSelectionResult.None + val rawTags = selection.tags.wellFormedOrNull() + val rawKeyPatterns = selection.keyPatterns.wellFormedOrNull() + if (rawTags == null || rawKeyPatterns == null) { + return FlagSelectionResult.Dropped(FlagSelectionError.MALFORMED_SELECTION) + } + + val tags = canonicalizeList(rawTags) + val keyPatterns = canonicalizeList(rawKeyPatterns) + + val error = when { + tags.size > MAX_VALUES || keyPatterns.size > MAX_VALUES -> FlagSelectionError.TOO_MANY_VALUES + keyPatterns.any { it.utf8Size() > MAX_KEY_PATTERN_BYTES } -> FlagSelectionError.INVALID_KEY_PATTERN + tags.any { it.utf8Size() > MAX_TAG_BYTES } -> FlagSelectionError.INVALID_TAG + else -> null + } + return when { + error != null -> FlagSelectionResult.Dropped(error) + tags.isEmpty() && keyPatterns.isEmpty() -> FlagSelectionResult.None + else -> FlagSelectionResult.Selected(CanonicalFlagSelection(tags, keyPatterns)) + } + } + + private fun canonicalizeList(values: List): List = values + .asSequence() + .map { value -> value.trim { it in WHITE_SPACE } } + .filter { it.isNotEmpty() } + .sortedBy { it.utf8SortKey() } + .toList() + .distinct() + + // Latin-1 maps each byte to one char, so comparing these strings compares unsigned UTF-8 bytes. + private fun String.utf8SortKey(): String = String(toByteArray(Charsets.UTF_8), Charsets.ISO_8859_1) + + private fun String.utf8Size(): Int = toByteArray(Charsets.UTF_8).size + + private fun List.wellFormedOrNull(): List? = + if (all { it != null && !it.hasUnpairedSurrogate() }) filterIsInstance() else null + + private fun String.hasUnpairedSurrogate(): Boolean { + var index = 0 + var unpaired = false + while (index < length && !unpaired) { + val char = this[index] + val isPair = char.isHighSurrogate() && index + 1 < length && this[index + 1].isLowSurrogate() + unpaired = !isPair && (char.isHighSurrogate() || char.isLowSurrogate()) + index += if (isPair) 2 else 1 + } + return unpaired + } +} diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt new file mode 100644 index 0000000000..b0300b4f9d --- /dev/null +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt @@ -0,0 +1,46 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.flags.internal.selection + +import com.datadog.android.api.InternalLogger + +// Telemetry carries only the reason, never selection values. +private const val TELEMETRY_REASON_KEY = "reason" + +internal const val FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE = "Flag selection dropped" +internal const val FLAG_SELECTION_IGNORED_TELEMETRY_MESSAGE = "Flag selection ignored" + +internal fun InternalLogger.logDroppedFlagSelection(error: FlagSelectionError) { + log( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + { + "flagSelection was not sent (${error.reason}: ${error.limit}). " + + "Loading the full flag configuration." + } + ) + log( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + { FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE }, + additionalProperties = mapOf(TELEMETRY_REASON_KEY to error.reason) + ) +} + +internal fun InternalLogger.logIgnoredFlagSelection(reason: String) { + log( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + { "flagSelection was ignored by the server ($reason). Loaded the full flag configuration." } + ) + log( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + { FLAG_SELECTION_IGNORED_TELEMETRY_MESSAGE }, + additionalProperties = mapOf(TELEMETRY_REASON_KEY to reason) + ) +} diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt index f0dc4b7136..a8c4e8f2b9 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt @@ -6,6 +6,7 @@ package com.datadog.android.flags +import com.datadog.android.flags.internal.selection.FlagSelection import fr.xgouchet.elmyr.annotation.BoolForgery import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeExtension @@ -30,6 +31,7 @@ internal class FlagsConfigurationTest { assertThat(configuration.customFlagEndpoint).isNull() assertThat(configuration.gracefulModeEnabled).isTrue() assertThat(configuration.initializationTimeoutMs).isEqualTo(5_000L) + assertThat(configuration.flagSelection).isNull() } @Test @@ -181,5 +183,51 @@ internal class FlagsConfigurationTest { assertThat(copiedConfiguration.initializationTimeoutMs).isEqualTo(2_500L) } + @Test + fun `M set the flag selection W flagSelection() { called twice }`() { + // Given + val builder = FlagsConfiguration.Builder() + .flagSelection(tags = listOf("web"), keyPatterns = listOf("web-*")) + + // When + val configuration = builder + .flagSelection(tags = listOf("mobile"), keyPatterns = emptyList()) + .build() + + // Then + assertThat(configuration.flagSelection) + .isEqualTo(FlagSelection(tags = listOf("mobile"), keyPatterns = emptyList())) + } + + @Test + fun `M copy the lists W flagSelection() { caller mutates the lists after the call }`() { + // Given + val tags = mutableListOf("mobile") + val keyPatterns = mutableListOf("checkout-*") + val builder = FlagsConfiguration.Builder().flagSelection(tags, keyPatterns) + + // When + tags += "web" + keyPatterns.clear() + + // Then + assertThat(builder.build().flagSelection) + .isEqualTo(FlagSelection(tags = listOf("mobile"), keyPatterns = listOf("checkout-*"))) + } + + @Test + fun `M preserve the flag selection W copy() { legacy parameters }`() { + // Given + val testedConfiguration = FlagsConfiguration.Builder() + .flagSelection(tags = listOf("mobile"), keyPatterns = listOf("checkout-*")) + .build() + + // When + val copiedConfiguration = testedConfiguration.copy(trackExposures = false) + + // Then + assertThat(copiedConfiguration.flagSelection).isEqualTo(testedConfiguration.flagSelection) + } + // endregion } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt index 0c83863fac..f88f5f6db5 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt @@ -12,8 +12,10 @@ import com.datadog.android.api.InternalLogger import com.datadog.android.api.feature.Feature.Companion.FLAGS_FEATURE_NAME import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.flags.FlagsConfiguration +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.internal.storage.ExposureEventRecordWriter import com.datadog.android.flags.internal.storage.NoOpRecordWriter +import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat @@ -104,6 +106,50 @@ internal class FlagsFeatureTest { ).isNotNull() } + @Test + fun `M expose the canonical flag selection W constructor { valid selection }`() { + // When + val feature = FlagsFeature( + sdkCore = mockSdkCore, + flagsConfiguration = FlagsConfiguration.Builder() + .flagSelection(tags = listOf(" web", "mobile", "web"), keyPatterns = listOf("checkout-*")) + .build() + ) + + // Then + assertThat(feature.flagSelection) + .isEqualTo(CanonicalFlagSelection(tags = listOf("mobile", "web"), keyPatterns = listOf("checkout-*"))) + } + + @Test + fun `M log the dropped flag selection W onInitialize() { selection over a limit }`() { + // Given + val feature = FlagsFeature( + sdkCore = mockSdkCore, + flagsConfiguration = FlagsConfiguration.Builder() + .flagSelection(tags = List(32) { "tag-$it" }, keyPatterns = emptyList()) + .build() + ) + + // When + feature.onInitialize(mockContext) + + // Then + assertThat(feature.flagSelection).isNull() + mockInternalLogger.verifyLog( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + "flagSelection was not sent (too_many_values: each list accepts at most 31 values). " + + "Loading the full flag configuration." + ) + mockInternalLogger.verifyLog( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + "Flag selection dropped", + additionalProperties = mapOf("reason" to "too_many_values") + ) + } + @Test fun `M reset dataWriter to NoOp W onStop`() { // Given diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index bbadb39bad..9309951484 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -21,6 +21,7 @@ import com.datadog.android.flags.internal.FlagsStateManager import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedFlag import com.datadog.android.flags.internal.net.PrecomputedAssignmentsReader +import com.datadog.android.flags.internal.net.PrecomputedAssignmentsResponse import com.datadog.android.flags.internal.repository.DefaultFlagsRepository import com.datadog.android.flags.internal.repository.FlagsRepository import com.datadog.android.flags.internal.repository.NoOpFlagsRepository @@ -29,6 +30,7 @@ import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.FlagsClientState import com.datadog.android.flags.utils.forge.ForgeConfigurator import com.datadog.android.internal.utils.DDCoreStateHolder +import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.annotation.Forgery import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration @@ -196,7 +198,8 @@ internal class EvaluationsManagerTest { initializationTimeoutMs = null, initializationTimeoutScheduler = { _, _ -> {} } ) - whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)).thenReturn("response") + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse("response", null)) whenever(mockPrecomputeMapper.map("response")).thenReturn(emptyMap()) // When @@ -249,7 +252,7 @@ internal class EvaluationsManagerTest { ) whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(mockResponse) + .thenReturn(PrecomputedAssignmentsResponse(mockResponse, null)) whenever(mockPrecomputeMapper.map(mockResponse)).thenReturn(expectedFlags) // When @@ -312,7 +315,7 @@ internal class EvaluationsManagerTest { context, fakeDatadogContext ) - ).thenReturn(invalidResponse) + ).thenReturn(PrecomputedAssignmentsResponse(invalidResponse, null)) whenever(mockPrecomputeMapper.map(invalidResponse)).thenReturn(null) // When @@ -362,7 +365,7 @@ internal class EvaluationsManagerTest { val expectedFlags = mapOf() whenever(mockAssignmentsDownloader.readPrecomputedFlags(publicContext, fakeDatadogContext)) - .thenReturn(mockResponse) + .thenReturn(PrecomputedAssignmentsResponse(mockResponse, null)) whenever(mockPrecomputeMapper.map(mockResponse)).thenReturn(expectedFlags) // When @@ -444,7 +447,7 @@ internal class EvaluationsManagerTest { val flagsMap = emptyMap() whenever(mockAssignmentsDownloader.readPrecomputedFlags(publicContext, fakeDatadogContext)) - .thenReturn(jsonResponse) + .thenReturn(PrecomputedAssignmentsResponse(jsonResponse, null)) whenever(mockPrecomputeMapper.map(jsonResponse)).thenReturn(flagsMap) // When @@ -505,7 +508,7 @@ internal class EvaluationsManagerTest { val flagsMap = emptyMap() whenever(mockAssignmentsDownloader.readPrecomputedFlags(publicContext, fakeDatadogContext)) - .thenReturn(jsonResponse) + .thenReturn(PrecomputedAssignmentsResponse(jsonResponse, null)) whenever(mockPrecomputeMapper.map(jsonResponse)).thenReturn(flagsMap) // When/Then - should not throw @@ -525,7 +528,7 @@ internal class EvaluationsManagerTest { null } whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -659,7 +662,7 @@ internal class EvaluationsManagerTest { null } whenever(mockAssignmentsDownloader.readPrecomputedFlags(any(), eq(fakeDatadogContext))) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val stateManager = FlagsStateManager( DDCoreStateHolder.create( @@ -808,7 +811,7 @@ internal class EvaluationsManagerTest { null }.whenever(mockFlagsStateManager).updateState(any()) whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -851,7 +854,7 @@ internal class EvaluationsManagerTest { var timeoutAction: (() -> Unit)? = null var cancellationCount = 0 whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -878,7 +881,7 @@ internal class EvaluationsManagerTest { val mockCallback = mock() var timeoutAction: (() -> Unit)? = null whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenAnswer { checkNotNull(timeoutAction).invoke() emptyMap() @@ -907,7 +910,7 @@ internal class EvaluationsManagerTest { val mockCallback = mock() var scheduleCount = 0 whenever(mockAssignmentsDownloader.readPrecomputedFlags(any(), eq(fakeDatadogContext))) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -932,7 +935,7 @@ internal class EvaluationsManagerTest { val mockCallback = mock() var scheduleCount = 0 whenever(mockAssignmentsDownloader.readPrecomputedFlags(any(), eq(fakeDatadogContext))) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val testedManager = createManager( initializationTimeoutMs = null, @@ -987,7 +990,7 @@ internal class EvaluationsManagerTest { val mockCallback = mock() var timeoutAction: (() -> Unit)? = null whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) - .thenReturn(EMPTY_FLAGS_RESPONSE_JSON) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) val stateManager = FlagsStateManager( DDCoreStateHolder.create( @@ -1111,7 +1114,7 @@ internal class EvaluationsManagerTest { } whenever(mockAssignmentsDownloader.readPrecomputedFlags(publicContext, fakeDatadogContext)) - .thenReturn(jsonResponse) + .thenReturn(PrecomputedAssignmentsResponse(jsonResponse, null)) whenever(mockPrecomputeMapper.map(jsonResponse)).thenReturn(flagsMap) // When @@ -1126,7 +1129,7 @@ internal class EvaluationsManagerTest { // Given whenever(mockSdkCore.internalLogger).thenReturn(mockInternalLogger) whenever(mockSdkCore.timeProvider).thenReturn(mock()) - val repository = DefaultFlagsRepository(mockSdkCore, "late-success", mock(), persistenceLoadTimeoutMs = 1) + val repository = DefaultFlagsRepository(mockSdkCore, "late-success", mock(), null, persistenceLoadTimeoutMs = 1) val state = FlagsStateManager( DDCoreStateHolder.create(FlagsClientState.NotReady, FlagsStateListener::onStateChanged) ) @@ -1148,7 +1151,8 @@ internal class EvaluationsManagerTest { val callback = mock() val firstKeys = mutableListOf>() repository.firstFlags.whenComplete { firstKeys.add(it) } - whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)).thenReturn("response") + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse("response", null)) whenever(mockPrecomputeMapper.map("response")).thenReturn(mapOf("enabled" to mock())) // When @@ -1180,7 +1184,7 @@ internal class EvaluationsManagerTest { whenever(mockSdkCore.timeProvider).thenReturn(mock()) doThrow(storageError).whenever(store) .setValue(any(), any(), any(), anyOrNull(), any()) - val repository = DefaultFlagsRepository(mockSdkCore, "throwing", store, persistenceLoadTimeoutMs = 1) + val repository = DefaultFlagsRepository(mockSdkCore, "throwing", store, null, persistenceLoadTimeoutMs = 1) val state = FlagsStateManager( DDCoreStateHolder.create(FlagsClientState.NotReady, FlagsStateListener::onStateChanged) ) @@ -1206,7 +1210,8 @@ internal class EvaluationsManagerTest { firstFlagsCount++ manager.updateEvaluationsForContext(EvaluationContext("newer")) } - whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)).thenReturn("response") + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse("response", null)) whenever(mockPrecomputeMapper.map("response")).thenReturn(emptyMap()) // When @@ -1241,7 +1246,8 @@ internal class EvaluationsManagerTest { val persistedEntry = FlagsStateEntry( evaluationContext = context, flags = mapOf("cached-flag" to flag), - lastUpdateTimestamp = 0L + lastUpdateTimestamp = 0L, + flagSelection = null ) // Configure DataStore to fire callback synchronously during DefaultFlagsRepository @@ -1264,6 +1270,7 @@ internal class EvaluationsManagerTest { val realRepository = DefaultFlagsRepository( featureSdkCore = mockSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "integration-test" ) @@ -1295,6 +1302,87 @@ internal class EvaluationsManagerTest { // endregion + @Test + fun `M log and install the configuration W updateEvaluationsForContext() { flag selection ignored }`( + @StringForgery(regex = "[a-z_]+") fakeIgnoredReason: String + ) { + // Given + val context = EvaluationContext(fakeTargetingKey, emptyMap()) + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, fakeIgnoredReason)) + whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) + + // When + evaluationsManager.updateEvaluationsForContext(context) + + // Then + mockInternalLogger.verifyLog( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + "flagSelection was ignored by the server ($fakeIgnoredReason). Loaded the full flag configuration." + ) + mockInternalLogger.verifyLog( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + "Flag selection ignored", + additionalProperties = mapOf("reason" to fakeIgnoredReason) + ) + verify(mockFlagsRepository).setFlagsAndContext(eq(context), eq(emptyMap()), any()) + verify(mockFlagsStateManager).updateState(FlagsClientState.Ready) + } + + @Test + fun `M notify STALE W updateEvaluationsForContext() { failed fetch after an empty configuration }`() { + // Given + val context = EvaluationContext(fakeTargetingKey, emptyMap()) + val mockDataStore = mock() + whenever( + mockDataStore.value( + key = any(), + version = anyOrNull(), + callback = any(), + deserializer = any() + ) + ).doAnswer { + it.getArgument>(2).onFailure() + null + } + whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger + whenever(mockSdkCore.timeProvider) doReturn mock() + val realRepository = DefaultFlagsRepository( + featureSdkCore = mockSdkCore, + dataStore = mockDataStore, + flagSelection = null, + instanceName = "empty-configuration-test" + ) + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null), null) + whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(emptyMap()) + val testedManager = EvaluationsManager( + sdkCore = mockSdkCore, + executorService = mockExecutorService, + internalLogger = mockInternalLogger, + flagsRepository = realRepository, + assignmentsReader = mockAssignmentsDownloader, + precomputeMapper = mockPrecomputeMapper, + flagStateManager = mockFlagsStateManager, + initializationTimeoutMs = null, + initializationTimeoutScheduler = { _, _ -> {} } + ) + + // When + testedManager.updateEvaluationsForContext(context) + testedManager.updateEvaluationsForContext(context) + + // Then + inOrder(mockFlagsStateManager) { + verify(mockFlagsStateManager).updateState(FlagsClientState.Reconciling) + verify(mockFlagsStateManager).updateState(FlagsClientState.Ready) + verify(mockFlagsStateManager).updateState(FlagsClientState.Reconciling) + verify(mockFlagsStateManager).updateState(FlagsClientState.Stale) + } + } + private fun createManager( flagStateManager: FlagsStateManager = mockFlagsStateManager, initializationTimeoutMs: Long? = null, diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index 31a0c48a4a..53d932a889 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -115,10 +115,36 @@ internal class PrecomputedAssignmentsDownloaderTest { val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) // Then - assertThat(result).isEqualTo(fakeResponseBody) + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, null)) verify(mockRequestFactory).create(fakeEvaluationContext, fakeDatadogContext) } + @Test + fun `M return flag selection ignored reason W readPrecomputedFlags() { DD-Flag-Selection-Ignored header }`( + @StringForgery fakeResponseBody: String, + @StringForgery(regex = "[a-z_]+") fakeIgnoredReason: String, + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + val fakeRequest = Request.Builder() + .url(fakeUrl) + .build() + val fakeResponse = createSuccessfulResponse(fakeResponseBody, fakeUrl) + .newBuilder() + .header("DD-Flag-Selection-Ignored", fakeIgnoredReason) + .build() + whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext)) + .doReturn(fakeRequest) + whenever(mockCallFactory.newCall(fakeRequest)).doReturn(mockCall) + whenever(mockCall.execute()).doReturn(fakeResponse) + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) + + // Then + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) + } + @Test fun `M call factory with correct parameters W readPrecomputedFlags()`( @StringForgery fakeResponseBody: String, diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt index 3a427c85f7..69a573a9bc 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt @@ -10,6 +10,7 @@ import com.datadog.android.DatadogSite import com.datadog.android.api.InternalLogger import com.datadog.android.api.context.DatadogContext import com.datadog.android.api.feature.Feature +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.utils.forge.ForgeConfigurator import fr.xgouchet.elmyr.Forge @@ -26,6 +27,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.EnumSource +import org.junit.jupiter.params.provider.ValueSource import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings @@ -59,7 +61,7 @@ internal class PrecomputedAssignmentsRequestFactoryTest { mapOf(Feature.RUM_FEATURE_NAME to mapOf("application_id" to fakeRumApplicationId.toString())) ) - testedFactory = PrecomputedAssignmentsRequestFactory(mockInternalLogger, null) + testedFactory = PrecomputedAssignmentsRequestFactory(mockInternalLogger, null, null) } // region create() - Success cases @@ -120,7 +122,7 @@ internal class PrecomputedAssignmentsRequestFactoryTest { ) { // Given val fakeCustomEndpoint = "https://custom-proxy.example.com/flags/assignments" - testedFactory = PrecomputedAssignmentsRequestFactory(mockInternalLogger, fakeCustomEndpoint) + testedFactory = PrecomputedAssignmentsRequestFactory(mockInternalLogger, fakeCustomEndpoint, null) val context = EvaluationContext( targetingKey = fakeTargetingKey, attributes = emptyMap() @@ -253,6 +255,78 @@ internal class PrecomputedAssignmentsRequestFactoryTest { assertThat(targetingAttributes.getString("attr4")).isEqualTo("value4") } + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `M add flag_selection to attributes W create() { flag selection configured }`( + useCustomEndpoint: Boolean, + @StringForgery fakeTargetingKey: String + ) { + // Given + val fakeCustomEndpoint = "https://custom-proxy.example.com/flags/assignments".takeIf { useCustomEndpoint } + testedFactory = PrecomputedAssignmentsRequestFactory( + mockInternalLogger, + fakeCustomEndpoint, + CanonicalFlagSelection(tags = listOf("mobile", "web"), keyPatterns = listOf("checkout-*")) + ) + val context = EvaluationContext(targetingKey = fakeTargetingKey, attributes = emptyMap()) + + // When + val request = testedFactory.create(context, fakeDatadogContext) + + // Then + checkNotNull(request) + val flagSelection = JSONObject(extractRequestBodyAsString(request)) + .getJSONObject("data") + .getJSONObject("attributes") + .getJSONObject("flag_selection") + assertThat(flagSelection.keys().asSequence().toList()).containsExactlyInAnyOrder("tags", "key_patterns") + assertThat(flagSelection.getJSONArray("tags").toString()).isEqualTo("""["mobile","web"]""") + assertThat(flagSelection.getJSONArray("key_patterns").toString()).isEqualTo("""["checkout-*"]""") + } + + @Test + fun `M send both lists W create() { flag selection with key patterns only }`( + @StringForgery fakeTargetingKey: String + ) { + // Given + testedFactory = PrecomputedAssignmentsRequestFactory( + mockInternalLogger, + null, + CanonicalFlagSelection(tags = emptyList(), keyPatterns = listOf("checkout-*")) + ) + val context = EvaluationContext(targetingKey = fakeTargetingKey, attributes = emptyMap()) + + // When + val request = testedFactory.create(context, fakeDatadogContext) + + // Then + checkNotNull(request) + val flagSelection = JSONObject(extractRequestBodyAsString(request)) + .getJSONObject("data") + .getJSONObject("attributes") + .getJSONObject("flag_selection") + assertThat(flagSelection.getJSONArray("tags").length()).isZero() + assertThat(flagSelection.getJSONArray("key_patterns").toString()).isEqualTo("""["checkout-*"]""") + } + + @Test + fun `M not add flag_selection W create() { no flag selection }`( + @StringForgery fakeTargetingKey: String + ) { + // Given + val context = EvaluationContext(targetingKey = fakeTargetingKey, attributes = emptyMap()) + + // When + val request = testedFactory.create(context, fakeDatadogContext) + + // Then + checkNotNull(request) + val attributes = JSONObject(extractRequestBodyAsString(request)) + .getJSONObject("data") + .getJSONObject("attributes") + assertThat(attributes.keys().asSequence().toList()).containsExactlyInAnyOrder("env", "source", "subject") + } + // endregion // region create() - Error cases diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializerTest.kt index f31f70f6f1..69a3ecf7f3 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateDeserializerTest.kt @@ -10,6 +10,7 @@ import com.datadog.android.api.InternalLogger import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat +import org.json.JSONArray import org.json.JSONException import org.json.JSONObject import org.junit.jupiter.api.BeforeEach @@ -319,6 +320,33 @@ internal class FlagsStateDeserializerTest { assertThat(result.evaluationContext.attributes["valid_boolean"]).isEqualTo(validBoolean) } + @Test + fun `M return no flag selection W deserialize() { state persisted without a flag selection }`(forge: Forge) { + // Given + val json = buildStateJson(forge.anAlphabeticalString(), JSONObject().put("flag1", buildFlagJson())) + + // When + val result = testedDeserializer.deserialize(json) + + // Then + checkNotNull(result) + assertThat(result.flagSelection).isNull() + } + + @Test + fun `M return null W deserialize() { malformed flag selection }`(forge: Forge) { + // Given + val json = JSONObject(buildStateJson(forge.anAlphabeticalString(), JSONObject())) + .put("flagSelection", JSONObject().put("tags", JSONArray())) + .toString() + + // When + val result = testedDeserializer.deserialize(json) + + // Then + assertThat(result).isNull() + } + private fun buildFlagJson(): JSONObject = JSONObject().apply { put("variationType", "boolean") put("variationValue", "true") diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt index 0173a21741..9935ce977c 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt @@ -9,6 +9,7 @@ package com.datadog.android.flags.internal.persistence import com.datadog.android.api.InternalLogger import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.annotation.LongForgery @@ -69,7 +70,7 @@ internal class FlagsStateSerializerTest { ) ) - val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp) + val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp, null) // When val serialized = testedSerializer.serialize(flagsState) @@ -110,7 +111,7 @@ internal class FlagsStateSerializerTest { serialId = null ) ) - val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp) + val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp, null) // When val serialized = testedSerializer.serialize(flagsState) @@ -126,7 +127,7 @@ internal class FlagsStateSerializerTest { // Given val targetingKey = forge.anAlphabeticalString() val evaluationContext = EvaluationContext(targetingKey, emptyMap()) - val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp) + val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp, null) // When val serialized = testedSerializer.serialize(flagsState) @@ -143,7 +144,7 @@ internal class FlagsStateSerializerTest { // Given // Create a state that could potentially cause serialization issues val evaluationContext = EvaluationContext("key", emptyMap()) - val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp) + val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp, null) // When val serialized = testedSerializer.serialize(flagsState) @@ -154,4 +155,44 @@ internal class FlagsStateSerializerTest { SerializedFlagsStateAssert.assertThatSerializedFlagsState(serialized) .hasEmptyFlags() } + + @Test + fun `M round trip the flag selection W serialize() + deserialize()`(forge: Forge) { + // Given + val flagSelection = CanonicalFlagSelection( + tags = listOf("mobile", "\u00E9t\u00E9"), + keyPatterns = listOf("checkout-*") + ) + val flagsState = FlagsStateEntry( + EvaluationContext(forge.anAlphabeticalString(), emptyMap()), + emptyMap(), + fakeTimestamp, + flagSelection + ) + + // When + val serialized = testedSerializer.serialize(flagsState) + val deserialized = FlagsStateDeserializer(mockInternalLogger).deserialize(serialized) + + // Then + checkNotNull(deserialized) + assertThat(deserialized.flagSelection).isEqualTo(flagSelection) + } + + @Test + fun `M omit the flag selection W serialize() { no flag selection }`(forge: Forge) { + // Given + val flagsState = FlagsStateEntry( + EvaluationContext(forge.anAlphabeticalString(), emptyMap()), + emptyMap(), + fakeTimestamp, + null + ) + + // When + val serialized = testedSerializer.serialize(flagsState) + + // Then + assertThat(JSONObject(serialized).has("flagSelection")).isFalse() + } } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 6ccdfbe060..21731fe4b7 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -13,6 +13,7 @@ import com.datadog.android.api.storage.datastore.DataStoreReadCallback import com.datadog.android.core.persistence.datastore.DataStoreContent import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.ResolutionReason import com.datadog.android.flags.utils.forge.ForgeConfigurator @@ -88,6 +89,7 @@ internal class DefaultFlagsRepositoryTest { testedRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "default" ) @@ -140,6 +142,7 @@ internal class DefaultFlagsRepositoryTest { val testedRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = forge.anAlphabeticalString(), persistenceLoadTimeoutMs = TimeUnit.SECONDS.toMillis(10) ) @@ -201,6 +204,7 @@ internal class DefaultFlagsRepositoryTest { val testedRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "loading", persistenceLoadTimeoutMs = TimeUnit.SECONDS.toMillis(10) ) @@ -241,6 +245,7 @@ internal class DefaultFlagsRepositoryTest { val testedRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "async" ) val asyncThread = Thread { @@ -273,6 +278,7 @@ internal class DefaultFlagsRepositoryTest { val testedRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "timeout", persistenceLoadTimeoutMs = 1L ) @@ -323,7 +329,7 @@ internal class DefaultFlagsRepositoryTest { }.toMap() val callback = preparePersistenceLoad(forge) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) val snapshot = testedRepository.getFlagsSnapshot() flags.forEach { (key, originalFlag) -> @@ -350,7 +356,7 @@ internal class DefaultFlagsRepositoryTest { anAlphabeticalString() to getForgery() }.toMap() val callback = preparePersistenceLoad(forge) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) val cachedSnapshot = testedRepository.getFlagsSnapshot() testedRepository.setFlagsAndContext(context, flags) @@ -379,7 +385,7 @@ internal class DefaultFlagsRepositoryTest { val networkFlags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) testedRepository.setFlagsAndContext(networkContext, networkFlags) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) assertThat(testedRepository.getFlagsSnapshot()).isSameAs(networkFlags) assertThat(testedRepository.getEvaluationContext()).isEqualTo(networkContext) @@ -392,12 +398,12 @@ internal class DefaultFlagsRepositoryTest { val callback = preparePersistenceLoad(forge) // When - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, emptyMap(), forge.aLong()))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, emptyMap(), forge.aLong(), null))) // Then assertThat(testedRepository.getEvaluationContext()).isEqualTo(context) assertThat(testedRepository.getFlagsSnapshot()).isEmpty() - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasFlags()).isTrue() verify(mockDataStore, never()).setValue( key = any(), data = any(), @@ -415,7 +421,7 @@ internal class DefaultFlagsRepositoryTest { val networkContext = diskContext.copy(targetingKey = diskContext.targetingKey + "-network") val diskFlags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) val callback = preparePersistenceLoad(forge) - val diskContent = DataStoreContent(forge.anInt(), FlagsStateEntry(diskContext, diskFlags, forge.aLong())) + val diskContent = DataStoreContent(forge.anInt(), FlagsStateEntry(diskContext, diskFlags, forge.aLong(), null)) // When if (diskFirst) { @@ -442,13 +448,114 @@ internal class DefaultFlagsRepositoryTest { assertThat(entry.firstValue.flags).isEmpty() } - private fun preparePersistenceLoad(forge: Forge): DataStoreReadCallback { + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `M ignore persisted state W initial disk read completes { different flag selection }`( + configureSelection: Boolean, + forge: Forge + ) { + // Given + val configuredSelection = CanonicalFlagSelection(listOf("mobile"), emptyList()).takeIf { configureSelection } + val persistedSelection = CanonicalFlagSelection(listOf("web"), emptyList()) + val flags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) + val callback = preparePersistenceLoad(forge, configuredSelection) + + // When + callback.onSuccess( + DataStoreContent( + forge.anInt(), + FlagsStateEntry(forge.getForgery(), flags, forge.aLong(), persistedSelection) + ) + ) + + // Then + assertThat(testedRepository.getEvaluationContext()).isNull() + assertThat(testedRepository.hasFlags()).isFalse() + } + + @Test + fun `M install persisted state W initial disk read completes { same flag selection }`(forge: Forge) { + // Given + val context = forge.getForgery() + val flagKey = forge.anAlphabeticalString() + val callback = preparePersistenceLoad(forge, CanonicalFlagSelection(listOf("mobile"), listOf("checkout-*"))) + + // When + callback.onSuccess( + DataStoreContent( + forge.anInt(), + FlagsStateEntry( + context, + mapOf(flagKey to forge.getForgery()), + forge.aLong(), + CanonicalFlagSelection(listOf("mobile"), listOf("checkout-*")) + ) + ) + ) + + // Then + assertThat(testedRepository.getEvaluationContext()).isEqualTo(context) + assertThat(testedRepository.getPrecomputedFlag(flagKey)).isNotNull() + } + + @Test + fun `M persist the configured flag selection W setFlagsAndContext()`(forge: Forge) { + // Given + val flagSelection = CanonicalFlagSelection(listOf("mobile"), listOf("checkout-*")) + preparePersistenceLoad(forge, flagSelection) + + // When + testedRepository.setFlagsAndContext(forge.getForgery(), emptyMap()) + + // Then + val entry = argumentCaptor() + verify(mockDataStore).setValue( + key = any(), + data = entry.capture(), + version = any(), + callback = anyOrNull(), + serializer = any() + ) + assertThat(entry.firstValue.flagSelection).isEqualTo(flagSelection) + } + + @Test + fun `M return true W hasLoadedFlagsForContext() { empty flags map for the same context }`(forge: Forge) { + // Given + val context = forge.getForgery() + testedRepository.setFlagsAndContext(context, emptyMap()) + + // When / Then + assertThat(testedRepository.hasLoadedFlagsForContext(context)).isTrue() + } + + @Test + fun `M return false W hasLoadedFlagsForContext() { different context }`(forge: Forge) { + // Given + val context = forge.getForgery() + testedRepository.setFlagsAndContext(context, singleFlagMap) + + // When / Then + assertThat( + testedRepository.hasLoadedFlagsForContext(context.copy(targetingKey = context.targetingKey + "-other")) + ).isFalse() + } + + private fun preparePersistenceLoad( + forge: Forge, + flagSelection: CanonicalFlagSelection? = null + ): DataStoreReadCallback { lateinit var callback: DataStoreReadCallback doAnswer { callback = it.getArgument(2) null }.whenever(mockDataStore).value(any(), anyOrNull(), any(), any()) - testedRepository = DefaultFlagsRepository(mockFeatureSdkCore, forge.anAlphabeticalString(), mockDataStore) + testedRepository = DefaultFlagsRepository( + mockFeatureSdkCore, + forge.anAlphabeticalString(), + mockDataStore, + flagSelection + ) return callback } @@ -461,7 +568,7 @@ internal class DefaultFlagsRepositoryTest { } @Test - fun `M return false W hasFlags() { empty flags map }`(forge: Forge) { + fun `M return true W hasFlags() { empty flags map }`(forge: Forge) { // Given testedRepository.setFlagsAndContext( EvaluationContext(forge.anAlphabeticalString(), emptyMap()), @@ -469,7 +576,7 @@ internal class DefaultFlagsRepositoryTest { ) // When + Then - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasFlags()).isTrue() } @Test @@ -496,7 +603,8 @@ internal class DefaultFlagsRepositoryTest { val persistedEntry = FlagsStateEntry( flags = singleFlagMap, evaluationContext = testContext, - lastUpdateTimestamp = 0L + lastUpdateTimestamp = 0L, + flagSelection = null ) doAnswer { it.getArgument>(2) @@ -511,6 +619,7 @@ internal class DefaultFlagsRepositoryTest { val repository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "with-flags" ) @@ -534,6 +643,7 @@ internal class DefaultFlagsRepositoryTest { val repository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "no-data" ) @@ -556,6 +666,7 @@ internal class DefaultFlagsRepositoryTest { val timeoutRepository = DefaultFlagsRepository( featureSdkCore = mockFeatureSdkCore, dataStore = mockDataStore, + flagSelection = null, instanceName = "timeout", persistenceLoadTimeoutMs = 1L ) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt index 4ccb2e44eb..68728edf74 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt @@ -252,7 +252,7 @@ internal class FirstFlagsInstallationTest { try { // When val loading = executor.submit { - disk.onSuccess(DataStoreContent(1, FlagsStateEntry(context, flags, 0))) + disk.onSuccess(DataStoreContent(1, FlagsStateEntry(context, flags, 0, null))) } // Then @@ -365,13 +365,13 @@ internal class FirstFlagsInstallationTest { disk = it.getArgument(2) null }.whenever(store).value(any(), anyOrNull(), any(), any()) - return DefaultFlagsRepository(core, "first", store, persistenceLoadTimeoutMs = timeoutMs).also { + return DefaultFlagsRepository(core, "first", store, null, persistenceLoadTimeoutMs = timeoutMs).also { it.firstFlags.whenComplete { keys -> events.add(keys) } } } private fun restored(keys: List): DataStoreContent = DataStoreContent( 1, - FlagsStateEntry(context, keys.associateWith { mock() }, 0) + FlagsStateEntry(context, keys.associateWith { mock() }, 0, null) ) } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt new file mode 100644 index 0000000000..6dac8ad484 --- /dev/null +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -0,0 +1,197 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.flags.internal.selection + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource + +internal class FlagSelectionCanonicalizerTest { + + @Test + fun `M return None W canonicalize() { no selection }`() { + // When / Then + assertThat(FlagSelectionCanonicalizer.canonicalize(null)).isEqualTo(FlagSelectionResult.None) + } + + @Test + fun `M return None W canonicalize() { lists empty after canonicalization }`() { + // Given + val selection = FlagSelection(tags = listOf("", " \t"), keyPatterns = listOf("\u3000")) + + // When / Then + assertThat(FlagSelectionCanonicalizer.canonicalize(selection)).isEqualTo(FlagSelectionResult.None) + } + + @Test + fun `M trim the Unicode White_Space set W canonicalize()`() { + // Given + val whiteSpace = "\t\n\u000B\u000C\r \u0085\u00A0\u1680\u2000\u200A\u2028\u2029\u202F\u205F\u3000" + val selection = FlagSelection( + tags = listOf("${whiteSpace}mobile$whiteSpace"), + keyPatterns = listOf("\uFEFFcheckout\uFEFF", "\u001Cpayment\u001C") + ) + + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).isEqualTo( + FlagSelectionResult.Selected( + CanonicalFlagSelection( + tags = listOf("mobile"), + keyPatterns = listOf("\u001Cpayment\u001C", "\uFEFFcheckout\uFEFF") + ) + ) + ) + } + + @Test + fun `M drop empty values and dedupe after trimming W canonicalize()`() { + // Given + val selection = FlagSelection( + tags = listOf("web", " mobile", "", "mobile ", "web"), + keyPatterns = emptyList() + ) + + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).isEqualTo( + FlagSelectionResult.Selected( + CanonicalFlagSelection(tags = listOf("mobile", "web"), keyPatterns = emptyList()) + ) + ) + } + + @Test + fun `M sort in UTF-8 byte order W canonicalize() { UTF-16 order differs }`() { + // Given + val fullwidthTilde = "\uFF5E" + val grinningFace = "\uD83D\uDE00" + val selection = FlagSelection( + tags = listOf(grinningFace, fullwidthTilde, "a"), + keyPatterns = listOf("b*", "B*", "\u00E9*") + ) + + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).isEqualTo( + FlagSelectionResult.Selected( + CanonicalFlagSelection( + tags = listOf("a", fullwidthTilde, grinningFace), + keyPatterns = listOf("B*", "b*", "\u00E9*") + ) + ) + ) + } + + @ParameterizedTest(name = "{0}") + @MethodSource("acceptedSelections") + fun `M keep the selection W canonicalize() { at a limit }`( + description: String, + selection: FlagSelection + ) { + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).describedAs(description).isInstanceOf(FlagSelectionResult.Selected::class.java) + } + + @ParameterizedTest(name = "{0}") + @MethodSource("droppedSelections") + fun `M drop the selection W canonicalize() { invalid selection }`( + description: String, + selection: FlagSelection, + expectedError: FlagSelectionError + ) { + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).describedAs(description).isEqualTo(FlagSelectionResult.Dropped(expectedError)) + } + + companion object { + private const val GRINNING_FACE = "\uD83D\uDE00" + + private fun values(count: Int) = List(count) { "value-$it" } + + private fun tags(vararg tags: String?) = FlagSelection(tags = tags.toList(), keyPatterns = emptyList()) + + private fun keyPatterns(vararg keyPatterns: String?) = + FlagSelection(tags = emptyList(), keyPatterns = keyPatterns.toList()) + + @JvmStatic + fun acceptedSelections() = listOf( + Arguments.of("31 tags", FlagSelection(values(31), emptyList())), + Arguments.of("31 key patterns", FlagSelection(emptyList(), values(31))), + Arguments.of("32 tags with one duplicate", FlagSelection(values(31) + "value-0", emptyList())), + Arguments.of("200 one-byte characters", tags("a".repeat(200))), + Arguments.of("100 two-byte characters", tags("\u00E9".repeat(100))), + Arguments.of("66 three-byte characters and 2 bytes", tags("\u20AC".repeat(66) + "ab")), + Arguments.of("50 four-byte characters", keyPatterns(GRINNING_FACE.repeat(50))), + Arguments.of("200 bytes after trimming", keyPatterns(" ${"a".repeat(200)}\u3000")) + ) + + @JvmStatic + fun droppedSelections() = listOf( + Arguments.of("32 tags", FlagSelection(values(32), emptyList()), FlagSelectionError.TOO_MANY_VALUES), + Arguments.of( + "32 key patterns", + FlagSelection(emptyList(), values(32)), + FlagSelectionError.TOO_MANY_VALUES + ), + Arguments.of("201-byte tag", tags("a".repeat(201)), FlagSelectionError.INVALID_TAG), + Arguments.of( + "201-byte tag with two-byte characters", + tags("\u00E9".repeat(100) + "a"), + FlagSelectionError.INVALID_TAG + ), + Arguments.of( + "201-byte tag with three-byte characters", + tags("\u20AC".repeat(67)), + FlagSelectionError.INVALID_TAG + ), + Arguments.of( + "201-byte key pattern with four-byte characters", + keyPatterns(GRINNING_FACE.repeat(50) + "*"), + FlagSelectionError.INVALID_KEY_PATTERN + ), + Arguments.of("lone high surrogate", tags("a\uD83D"), FlagSelectionError.MALFORMED_SELECTION), + Arguments.of("lone low surrogate", keyPatterns("\uDE00a"), FlagSelectionError.MALFORMED_SELECTION), + Arguments.of( + "reversed surrogate pair", + tags("\uDE00\uD83D"), + FlagSelectionError.MALFORMED_SELECTION + ), + Arguments.of("null tag", tags("mobile", null), FlagSelectionError.MALFORMED_SELECTION), + Arguments.of("null key pattern", keyPatterns(null), FlagSelectionError.MALFORMED_SELECTION), + Arguments.of( + "malformed before too many values", + FlagSelection(values(32) + null, emptyList()), + FlagSelectionError.MALFORMED_SELECTION + ), + Arguments.of( + "too many values before invalid key pattern", + FlagSelection(values(32), listOf("a".repeat(201))), + FlagSelectionError.TOO_MANY_VALUES + ), + Arguments.of( + "invalid key pattern before invalid tag", + FlagSelection(listOf("a".repeat(201)), listOf("a".repeat(201))), + FlagSelectionError.INVALID_KEY_PATTERN + ) + ) + } +} From 845e923bd01431506fb04a1055361fb917b08190 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 22 Oct 2026 15:58:26 +0000 Subject: [PATCH 02/34] FFL-3459: Tighten flag selection internals Ref: FFL-3459 --- .../flags/internal/net/PrecomputedAssignmentsReader.kt | 3 +-- .../android/flags/internal/selection/FlagSelectionLogging.kt | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt index a17478a480..f9d9f0c2b6 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt @@ -21,8 +21,7 @@ internal interface PrecomputedAssignmentsReader { } /** - * @param body the raw response body - * @param flagSelectionIgnoredReason the `DD-Flag-Selection-Ignored` header value, or null when absent + * [flagSelectionIgnoredReason] is the `DD-Flag-Selection-Ignored` header value, or null when absent. */ internal data class PrecomputedAssignmentsResponse( val body: String, diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt index b0300b4f9d..9c84618cb5 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt @@ -11,8 +11,8 @@ import com.datadog.android.api.InternalLogger // Telemetry carries only the reason, never selection values. private const val TELEMETRY_REASON_KEY = "reason" -internal const val FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE = "Flag selection dropped" -internal const val FLAG_SELECTION_IGNORED_TELEMETRY_MESSAGE = "Flag selection ignored" +private const val FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE = "Flag selection dropped" +private const val FLAG_SELECTION_IGNORED_TELEMETRY_MESSAGE = "Flag selection ignored" internal fun InternalLogger.logDroppedFlagSelection(error: FlagSelectionError) { log( From d61bd6ec1a3f5b3e0578f27178a4b4dc82937cbd Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Mon, 26 Oct 2026 17:33:00 +0000 Subject: [PATCH 03/34] FFL-3459: Read the ignored header only when a selection was sent The downloader reports DD-Flag-Selection-Ignored only for requests that carried flag_selection, and passes the value through verbatim. Ref: FFL-3459 --- .../net/PrecomputedAssignmentsDownloader.kt | 6 ++- .../PrecomputedAssignmentsRequestFactory.kt | 2 + .../evaluation/EvaluationsManagerTest.kt | 33 +++++++++++-- .../PrecomputedAssignmentsDownloaderTest.kt | 49 ++++++++++++++----- ...recomputedAssignmentsRequestFactoryTest.kt | 2 + 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index e68f0ef4f5..d613954828 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -56,7 +56,11 @@ internal class PrecomputedAssignmentsDownloader( response.body?.use { it.string() }?.let { body -> PrecomputedAssignmentsResponse( body = body, - flagSelectionIgnoredReason = response.header(HEADER_FLAG_SELECTION_IGNORED) + flagSelectionIgnoredReason = if (requestFactory.sendsFlagSelection) { + response.header(HEADER_FLAG_SELECTION_IGNORED) + } else { + null + } ) } } else { diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt index dd45945bdf..52dd72792e 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt @@ -31,6 +31,8 @@ internal class PrecomputedAssignmentsRequestFactory( private val flagSelection: CanonicalFlagSelection? ) { + val sendsFlagSelection: Boolean = flagSelection != null + /** * Creates an OkHttp Request for fetching precomputed flag assignments. * diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index 44e4aa437e..79168f2da0 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -52,6 +52,7 @@ import org.mockito.junit.jupiter.MockitoSettings import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.atLeast import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn import org.mockito.kotlin.doThrow @@ -1397,9 +1398,10 @@ internal class EvaluationsManagerTest { // endregion - @Test - fun `M log and install the configuration W updateEvaluationsForContext() { flag selection ignored }`( - @StringForgery(regex = "[a-z_]+") fakeIgnoredReason: String + @ParameterizedTest + @ValueSource(strings = ["too_many_values", "tag_load_failed", "Some Future-Reason"]) + fun `M log the reason verbatim and install W updateEvaluationsForContext() { flag selection ignored }`( + fakeIgnoredReason: String ) { // Given val context = EvaluationContext(fakeTargetingKey, emptyMap()) @@ -1426,6 +1428,31 @@ internal class EvaluationsManagerTest { verify(mockFlagsStateManager).updateState(FlagsClientState.Ready) } + @Test + fun `M not log a flag selection message W updateEvaluationsForContext() { no ignored reason }`() { + // Given + val context = EvaluationContext(fakeTargetingKey, emptyMap()) + whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext)) + .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) + + // When + evaluationsManager.updateEvaluationsForContext(context) + + // Then + val messages = argumentCaptor<() -> String>() + verify(mockInternalLogger, atLeast(0)).log( + any(), + any(), + messages.capture(), + anyOrNull(), + any(), + anyOrNull() + ) + assertThat(messages.allValues.map { it() }).noneMatch { "flagSelection" in it || "Flag selection" in it } + verify(mockFlagsRepository).setFlagsAndContext(eq(context), eq(emptyMap()), anyOrNull(), any()) + } + @Test fun `M notify STALE W updateEvaluationsForContext() { failed fetch after an empty configuration }`() { // Given diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index 4f10ed8f72..691d7ce0ea 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -28,6 +28,8 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings @@ -110,30 +112,40 @@ internal class PrecomputedAssignmentsDownloaderTest { verify(mockRequestFactory).create(fakeEvaluationContext, fakeDatadogContext) } + @ParameterizedTest + @ValueSource(strings = ["too_many_values", "tag_load_failed", "Some Future-Reason"]) + fun `M return the header value verbatim W readPrecomputedFlags() { selection sent, ignored header }`( + fakeIgnoredReason: String, + forge: Forge + ) { + // Given + val fakeResponseBody = forge.anAlphabeticalString() + val fakeUrl = "https://${forge.anAlphabeticalString()}.com/precompute" + stubSuccessfulCall(fakeResponseBody, fakeUrl, ignoredReason = fakeIgnoredReason) + whenever(mockRequestFactory.sendsFlagSelection) doReturn true + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) + + // Then + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) + } + @Test - fun `M return flag selection ignored reason W readPrecomputedFlags() { DD-Flag-Selection-Ignored header }`( + fun `M return no ignored reason W readPrecomputedFlags() { no selection sent, ignored header }`( @StringForgery fakeResponseBody: String, @StringForgery(regex = "[a-z_]+") fakeIgnoredReason: String, @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String ) { // Given - val fakeRequest = Request.Builder() - .url(fakeUrl) - .build() - val fakeResponse = createSuccessfulResponse(fakeResponseBody, fakeUrl) - .newBuilder() - .header("DD-Flag-Selection-Ignored", fakeIgnoredReason) - .build() - whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext)) - .doReturn(fakeRequest) - whenever(mockCallFactory.newCall(fakeRequest)).doReturn(mockCall) - whenever(mockCall.execute()).doReturn(fakeResponse) + stubSuccessfulCall(fakeResponseBody, fakeUrl, ignoredReason = fakeIgnoredReason) + whenever(mockRequestFactory.sendsFlagSelection) doReturn false // When val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) // Then - assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, null)) } @Test @@ -419,6 +431,17 @@ internal class PrecomputedAssignmentsDownloaderTest { // region Helper methods + private fun stubSuccessfulCall(body: String, url: String, ignoredReason: String) { + val fakeRequest = Request.Builder().url(url).build() + val fakeResponse = createSuccessfulResponse(body, url) + .newBuilder() + .header("DD-Flag-Selection-Ignored", ignoredReason) + .build() + whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext)) doReturn fakeRequest + whenever(mockCallFactory.newCall(fakeRequest)) doReturn mockCall + whenever(mockCall.execute()) doReturn fakeResponse + } + private fun createSuccessfulResponse(body: String, url: String): Response = Response.Builder() .request(Request.Builder().url(url).build()) .protocol(Protocol.HTTP_1_1) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt index 02f154ca3f..74ca4a69c3 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt @@ -321,6 +321,7 @@ internal class PrecomputedAssignmentsRequestFactoryTest { assertThat(flagSelection.keys().asSequence().toList()).containsExactlyInAnyOrder("tags", "key_patterns") assertThat(flagSelection.getJSONArray("tags").toString()).isEqualTo("""["mobile","web"]""") assertThat(flagSelection.getJSONArray("key_patterns").toString()).isEqualTo("""["checkout-*"]""") + assertThat(testedFactory.sendsFlagSelection).isTrue() } @Test @@ -364,6 +365,7 @@ internal class PrecomputedAssignmentsRequestFactoryTest { .getJSONObject("data") .getJSONObject("attributes") assertThat(attributes.keys().asSequence().toList()).containsExactlyInAnyOrder("env", "source", "subject") + assertThat(testedFactory.sendsFlagSelection).isFalse() } // endregion From 9e783442f4bf1fb68817c60c72d123eaeb44dd91 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Mon, 26 Oct 2026 17:36:28 +0000 Subject: [PATCH 04/34] FFL-3459: Accept null lists in flagSelection Builder.flagSelection takes List? for tags and key patterns. A null list is treated as absent, matching the browser SDK; null elements inside a list remain malformed_selection. Ref: FFL-3459 --- features/dd-sdk-android-flags/api/apiSurface | 2 +- .../android/flags/FlagsConfiguration.kt | 4 +-- .../flags/internal/selection/FlagSelection.kt | 10 +++---- .../android/flags/FlagsConfigurationTest.kt | 12 ++++++++ .../FlagSelectionCanonicalizerTest.kt | 28 +++++++++++++++++++ 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/features/dd-sdk-android-flags/api/apiSurface b/features/dd-sdk-android-flags/api/apiSurface index 382cb309dc..45b9e1779c 100644 --- a/features/dd-sdk-android-flags/api/apiSurface +++ b/features/dd-sdk-android-flags/api/apiSurface @@ -33,7 +33,7 @@ data class com.datadog.android.flags.FlagsConfiguration fun evaluationFlushInterval(Long): Builder fun useCustomFlagEndpoint(String): Builder fun initializationTimeout(Long): Builder - fun flagSelection(List, List): Builder + fun flagSelection(List?, List?): Builder fun rumIntegrationEnabled(Boolean): Builder fun gracefulModeEnabled(Boolean): Builder fun build(): FlagsConfiguration diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index f8bf636bae..1f4c06962d 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -192,8 +192,8 @@ data class FlagsConfiguration internal constructor( * @param keyPatterns The flag key patterns to load. * @return this [Builder] instance for method chaining. */ - fun flagSelection(tags: List, keyPatterns: List): Builder = apply { - flagSelection = FlagSelection(tags.toList(), keyPatterns.toList()) + fun flagSelection(tags: List?, keyPatterns: List?): Builder = apply { + flagSelection = FlagSelection(tags?.toList(), keyPatterns?.toList()) } /** diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index 23fa30df49..e81868c8ac 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -8,11 +8,11 @@ package com.datadog.android.flags.internal.selection /** * The flag selection as configured by the application, before canonicalization. - * Elements are nullable because Java callers can pass lists that hold nulls. + * Lists and elements are nullable because Java callers can pass nulls. A null list means the list is absent. */ internal data class FlagSelection( - val tags: List, - val keyPatterns: List + val tags: List?, + val keyPatterns: List? ) /** @@ -69,8 +69,8 @@ internal object FlagSelectionCanonicalizer { @Suppress("ReturnCount") fun canonicalize(selection: FlagSelection?): FlagSelectionResult { if (selection == null) return FlagSelectionResult.None - val rawTags = selection.tags.wellFormedOrNull() - val rawKeyPatterns = selection.keyPatterns.wellFormedOrNull() + val rawTags = selection.tags.orEmpty().wellFormedOrNull() + val rawKeyPatterns = selection.keyPatterns.orEmpty().wellFormedOrNull() if (rawTags == null || rawKeyPatterns == null) { return FlagSelectionResult.Dropped(FlagSelectionError.MALFORMED_SELECTION) } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt index a8c4e8f2b9..b85c6338bb 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt @@ -215,6 +215,18 @@ internal class FlagsConfigurationTest { .isEqualTo(FlagSelection(tags = listOf("mobile"), keyPatterns = listOf("checkout-*"))) } + @Test + fun `M keep null lists as absent W flagSelection() { null lists }`() { + // When + val configuration = FlagsConfiguration.Builder() + .flagSelection(tags = null, keyPatterns = listOf("checkout-*")) + .build() + + // Then + assertThat(configuration.flagSelection) + .isEqualTo(FlagSelection(tags = null, keyPatterns = listOf("checkout-*"))) + } + @Test fun `M preserve the flag selection W copy() { legacy parameters }`() { // Given diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index 6dac8ad484..e64d8005b4 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -29,6 +29,34 @@ internal class FlagSelectionCanonicalizerTest { assertThat(FlagSelectionCanonicalizer.canonicalize(selection)).isEqualTo(FlagSelectionResult.None) } + @Test + fun `M return None W canonicalize() { both lists null }`() { + // Given + val selection = FlagSelection(tags = null, keyPatterns = null) + + // When / Then + assertThat(FlagSelectionCanonicalizer.canonicalize(selection)).isEqualTo(FlagSelectionResult.None) + } + + @Test + fun `M treat a null list as empty W canonicalize() { one list null }`() { + // Given + val tagsOnly = FlagSelection(tags = listOf("mobile"), keyPatterns = null) + val keyPatternsOnly = FlagSelection(tags = null, keyPatterns = listOf("checkout-*")) + + // When + val tagsOnlyResult = FlagSelectionCanonicalizer.canonicalize(tagsOnly) + val keyPatternsOnlyResult = FlagSelectionCanonicalizer.canonicalize(keyPatternsOnly) + + // Then + assertThat(tagsOnlyResult).isEqualTo( + FlagSelectionResult.Selected(CanonicalFlagSelection(tags = listOf("mobile"), keyPatterns = emptyList())) + ) + assertThat(keyPatternsOnlyResult).isEqualTo( + FlagSelectionResult.Selected(CanonicalFlagSelection(tags = emptyList(), keyPatterns = listOf("checkout-*"))) + ) + } + @Test fun `M trim the Unicode White_Space set W canonicalize()`() { // Given From e5562f4ed1aaf9804c1a8eae96a1a3ee545d13d0 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Mon, 26 Oct 2026 17:37:05 +0000 Subject: [PATCH 05/34] FFL-3459: Document flagSelection limits and server behavior State that limits count values after dedupe, that flags outside the selection return the default with FLAG_NOT_FOUND, and when the server returns the full configuration instead. Ref: FFL-3459 --- .../android/flags/FlagsConfiguration.kt | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index 1f4c06962d..7904ffa36c 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -7,6 +7,7 @@ package com.datadog.android.flags import com.datadog.android.flags.internal.selection.FlagSelection +import com.datadog.android.flags.model.ErrorCode private const val DEFAULT_INITIALIZATION_TIMEOUT_MS = 5_000L @@ -180,16 +181,22 @@ data class FlagsConfiguration internal constructor( * * Tags match exactly and are case-sensitive; a flag matches if it has any of the tags. Key patterns are * whole-key globs where only `*` is special. Leading and trailing Unicode whitespace is trimmed from every - * value, empty values are dropped, and duplicates are removed. Each list accepts at most 31 values, and each - * tag and key pattern is at most 200 UTF-8 bytes. A selection over a limit is not sent, and the full flag - * configuration is loaded. Flags outside the selection are not loaded, so evaluating them returns the - * default value. + * value, empty values are dropped, and duplicates are removed. A null list is treated as absent. + * + * Each list accepts at most 31 values after duplicates are removed, and each tag and key pattern is at most + * 200 UTF-8 bytes. A selection over a limit, or one that contains a null value, is not sent, and the full + * flag configuration is loaded. The server may also ignore a very large selection (`too_many_values`), in + * which case the full flag configuration is loaded. Servers that do not support flag selection, or have it + * disabled, return the full flag configuration without a warning. + * + * Flags outside the selection are not loaded, so evaluating them returns the default value with + * [ErrorCode.FLAG_NOT_FOUND]. * * The selection is fixed when the Flags feature is enabled. Calling this method again replaces the * previous selection. * - * @param tags The flag tags to load. - * @param keyPatterns The flag key patterns to load. + * @param tags The flag tags to load, or null for none. + * @param keyPatterns The flag key patterns to load, or null for none. * @return this [Builder] instance for method chaining. */ fun flagSelection(tags: List?, keyPatterns: List?): Builder = apply { From 6766ce916a04593a659ff7db635dfe8a8cfa9a4f Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Mon, 26 Oct 2026 17:38:16 +0000 Subject: [PATCH 06/34] FFL-3459: Cover the shared flag selection vectors in unit tests Add hand-written canonicalizer cases for the selection vectors with sdk expectations that the existing tests did not cover. Ref: FFL-3459 --- .../FlagSelectionCanonicalizerTest.kt | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index e64d8005b4..03bd713f74 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource +import java.util.Locale internal class FlagSelectionCanonicalizerTest { @@ -150,9 +151,28 @@ internal class FlagSelectionCanonicalizerTest { assertThat(result).describedAs(description).isEqualTo(FlagSelectionResult.Dropped(expectedError)) } + @ParameterizedTest(name = "{0}") + @MethodSource("sharedVectorSelections") + fun `M match the shared selection vector W canonicalize()`( + vectorName: String, + selection: FlagSelection, + expectedResult: FlagSelectionResult + ) { + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result).describedAs(vectorName).isEqualTo(expectedResult) + } + companion object { private const val GRINNING_FACE = "\uD83D\uDE00" + private fun numbered(prefix: String, count: Int) = List(count) { "$prefix%02d".format(Locale.US, it + 1) } + + private fun selected(tags: List, keyPatterns: List) = + FlagSelectionResult.Selected(CanonicalFlagSelection(tags, keyPatterns)) + private fun values(count: Int) = List(count) { "value-$it" } private fun tags(vararg tags: String?) = FlagSelection(tags = tags.toList(), keyPatterns = emptyList()) @@ -172,6 +192,97 @@ internal class FlagSelectionCanonicalizerTest { Arguments.of("200 bytes after trimming", keyPatterns(" ${"a".repeat(200)}\u3000")) ) + // Cases with `sdk` expectations in DataDog/ffe-system-test-data@e8777633 + // flag-selection/selection-vectors.json that the tests above do not already cover. + @JvmStatic + fun sharedVectorSelections() = listOf( + Arguments.of( + "leading star", + keyPatterns("*-v2"), + selected(emptyList(), listOf("*-v2")) + ), + Arguments.of( + "star matches an empty run and multiple stars are allowed", + keyPatterns("*search*auto*complete*"), + selected(emptyList(), listOf("*search*auto*complete*")) + ), + Arguments.of( + "lone star matches every key", + keyPatterns("*"), + selected(emptyList(), listOf("*")) + ), + Arguments.of( + "question mark and brackets are literal", + keyPatterns("search.ranking-v?", "[a]*", "{a,b}"), + selected(emptyList(), listOf("[a]*", "search.ranking-v?", "{a,b}")) + ), + Arguments.of( + "braces are literal even when an alternative names a flag", + keyPatterns("{mobile.dark-mode,missing}"), + selected(emptyList(), listOf("{mobile.dark-mode,missing}")) + ), + Arguments.of( + "tags are never globs", + tags("team:*"), + selected(listOf("team:*"), emptyList()) + ), + Arguments.of( + "tags match case-sensitively", + tags("Team:Payments", "MOBILE"), + selected(listOf("MOBILE", "Team:Payments"), emptyList()) + ), + Arguments.of( + "characters outside Unicode White_Space are not trimmed", + tags("\uFEFFmobile", "mobile\u200B"), + selected(listOf("mobile\u200B", "\uFEFFmobile"), emptyList()) + ), + Arguments.of( + "values sort by UTF-8 bytes", + tags(GRINNING_FACE, "\uFF5E", "a", "Z"), + selected(listOf("Z", "a", "\uFF5E", GRINNING_FACE), emptyList()) + ), + Arguments.of( + "31 values in each list are allowed", + FlagSelection(numbered("t", 31), numbered("p", 31)), + selected(numbered("t", 31), numbered("p", 31)) + ), + Arguments.of( + "duplicates do not count toward the limit", + FlagSelection(numbered("t", 31) + "t01" + " t02 ", emptyList()), + selected(numbered("t", 31), emptyList()) + ), + Arguments.of( + "key pattern length counts UTF-8 bytes: 100 two-byte characters are allowed", + keyPatterns("\u00E9".repeat(100)), + selected(emptyList(), listOf("\u00E9".repeat(100))) + ), + Arguments.of( + "tag length is measured after trimming", + tags(" ${"a".repeat(200)} "), + selected(listOf("a".repeat(200)), emptyList()) + ), + Arguments.of( + "32 values in one list are rejected even when the other is within the limit", + FlagSelection(numbered("t", 31), numbered("p", 32)), + FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ), + Arguments.of( + "key pattern length counts UTF-8 bytes: 201 bytes are invalid", + keyPatterns("x" + "\u00E9".repeat(100)), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_KEY_PATTERN) + ), + Arguments.of( + "a long tag invalidates the selection even when other tags match", + tags("mobile", "a".repeat(201)), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_TAG) + ), + Arguments.of( + "too_many_values takes precedence over invalid_tag", + FlagSelection(numbered("t", 31) + "a".repeat(201), emptyList()), + FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ) + ) + @JvmStatic fun droppedSelections() = listOf( Arguments.of("32 tags", FlagSelection(values(32), emptyList()), FlagSelectionError.TOO_MANY_VALUES), From 61f6476eeaa59fc7699dddc54ad03ea673f304b3 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Tue, 27 Oct 2026 19:14:05 +0000 Subject: [PATCH 07/34] FFL-3459: Ignore an empty flag selection ignored header Treat an empty DD-Flag-Selection-Ignored value as absent, as the browser SDK does, so it no longer logs a server ignore with an empty reason. Clarify in the flagSelection KDoc that the byte limit applies after trimming, that unpaired UTF-16 surrogates are malformed, and that FLAG_NOT_FOUND applies when the server applies the selection. Ref: FFL-3459 --- .../datadog/android/flags/FlagsConfiguration.kt | 12 ++++++------ .../net/PrecomputedAssignmentsDownloader.kt | 2 +- .../net/PrecomputedAssignmentsDownloaderTest.kt | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index 7904ffa36c..5c526f0c7b 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -184,13 +184,13 @@ data class FlagsConfiguration internal constructor( * value, empty values are dropped, and duplicates are removed. A null list is treated as absent. * * Each list accepts at most 31 values after duplicates are removed, and each tag and key pattern is at most - * 200 UTF-8 bytes. A selection over a limit, or one that contains a null value, is not sent, and the full - * flag configuration is loaded. The server may also ignore a very large selection (`too_many_values`), in - * which case the full flag configuration is loaded. Servers that do not support flag selection, or have it - * disabled, return the full flag configuration without a warning. + * 200 UTF-8 bytes after trimming. A selection over a limit, or one that contains a null value or an unpaired + * UTF-16 surrogate, is not sent, and the full flag configuration is loaded. The server may also ignore a very + * large selection (`too_many_values`), in which case the full flag configuration is loaded. Servers that do + * not support flag selection, or have it disabled, return the full flag configuration without a warning. * - * Flags outside the selection are not loaded, so evaluating them returns the default value with - * [ErrorCode.FLAG_NOT_FOUND]. + * When the server applies the selection, flags outside it are not loaded, so evaluating them returns the + * default value with [ErrorCode.FLAG_NOT_FOUND]. * * The selection is fixed when the Flags feature is enabled. Calling this method again replaces the * previous selection. diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index d613954828..3e1ee7b9e4 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -57,7 +57,7 @@ internal class PrecomputedAssignmentsDownloader( PrecomputedAssignmentsResponse( body = body, flagSelectionIgnoredReason = if (requestFactory.sendsFlagSelection) { - response.header(HEADER_FLAG_SELECTION_IGNORED) + response.header(HEADER_FLAG_SELECTION_IGNORED)?.takeIf { it.isNotEmpty() } } else { null } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index 691d7ce0ea..92276f60e1 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -131,6 +131,22 @@ internal class PrecomputedAssignmentsDownloaderTest { assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) } + @Test + fun `M return no ignored reason W readPrecomputedFlags() { selection sent, empty ignored header }`( + @StringForgery fakeResponseBody: String, + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + stubSuccessfulCall(fakeResponseBody, fakeUrl, ignoredReason = "") + whenever(mockRequestFactory.sendsFlagSelection) doReturn true + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext) + + // Then + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, null)) + } + @Test fun `M return no ignored reason W readPrecomputedFlags() { no selection sent, ignored header }`( @StringForgery fakeResponseBody: String, From e9a63066920a1c82b64718e1e608f7b008a6e39d Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Tue, 27 Oct 2026 19:14:06 +0000 Subject: [PATCH 08/34] FFL-3459: Cover selection wiring, legacy caches and the trim set Verify that FlagsClient passes the configured selection to the repository, so a persisted state saved under that selection is served. Verify that a persisted state saved without a selection is discarded once a selection is configured. Extend the trim test to every Unicode White_Space character. Ref: FFL-3459 --- .../datadog/android/flags/FlagsClientTest.kt | 77 +++++++++++++++++++ .../repository/DefaultFlagsRepositoryTest.kt | 18 +++++ .../FlagSelectionCanonicalizerTest.kt | 3 +- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt index d43dd3df32..77593ab50a 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt @@ -11,13 +11,22 @@ import com.datadog.android.api.feature.Feature import com.datadog.android.api.feature.Feature.Companion.FLAGS_FEATURE_NAME import com.datadog.android.api.feature.FeatureScope import com.datadog.android.api.feature.FeatureSdkCore +import com.datadog.android.api.storage.datastore.DataStoreHandler +import com.datadog.android.api.storage.datastore.DataStoreReadCallback +import com.datadog.android.core.persistence.datastore.DataStoreContent +import com.datadog.android.flags.internal.DatadogFlagsClient import com.datadog.android.flags.internal.FlagsFeature import com.datadog.android.flags.internal.NoOpFlagsClient +import com.datadog.android.flags.internal.model.FlagsStateEntry +import com.datadog.android.flags.internal.model.PrecomputedFlag +import com.datadog.android.flags.internal.selection.CanonicalFlagSelection import com.datadog.android.flags.model.EvaluationContext import fr.xgouchet.elmyr.annotation.BoolForgery import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeExtension +import okhttp3.Call import org.assertj.core.api.Assertions.assertThat +import org.json.JSONObject import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -26,12 +35,16 @@ import org.junit.jupiter.api.extension.ExtendWith import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.mockito.quality.Strictness +import java.util.concurrent.ExecutorService @ExtendWith(MockitoExtension::class, ForgeExtension::class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -118,6 +131,70 @@ internal class FlagsClientTest { flagsFeature?.clearClients() } + // region Flag Selection Wiring + + @Test + fun `M key persisted flags by the configured selection W createInternal() { flag selection }`( + @StringForgery fakeFlagKey: String, + @StringForgery fakeClientName: String + ) { + // Given + val mockCore = mock() + val mockDataStore = mock() + val mockFeatureScope = mock() + whenever(mockCore.internalLogger).thenReturn(mockInternalLogger) + whenever(mockCore.createSingleThreadExecutorService(any())).thenReturn(mock()) + whenever(mockCore.createOkHttpCallFactory(any())).thenReturn(mock()) + whenever(mockFeatureScope.dataStore).thenReturn(mockDataStore) + whenever(mockCore.getFeature(FLAGS_FEATURE_NAME)).thenReturn(mockFeatureScope) + lateinit var readCallback: DataStoreReadCallback + doAnswer { + readCallback = it.getArgument(2) + null + }.whenever(mockDataStore).value(any(), anyOrNull(), any(), any()) + val configuration = FlagsConfiguration.Builder() + .trackEvaluations(false) + .flagSelection(tags = listOf("mobile"), keyPatterns = null) + .build() + val client = FlagsClient.createInternal( + configuration = configuration, + featureSdkCore = mockCore, + flagsFeature = FlagsFeature(mockCore, configuration), + evaluationsFeature = null, + name = fakeClientName, + evaluationTelemetryOptOutParameterEnabled = false + ) as DatadogFlagsClient + + // When + readCallback.onSuccess( + DataStoreContent( + 1, + FlagsStateEntry( + evaluationContext = EvaluationContext("user"), + flags = mapOf( + fakeFlagKey to + PrecomputedFlag( + "boolean", + "true", + false, + "allocation", + "on", + JSONObject(), + "TARGETING_MATCH" + ) + ), + lastUpdateTimestamp = 0L, + flagSelection = CanonicalFlagSelection(tags = listOf("mobile"), keyPatterns = emptyList()) + ) + ) + ) + + // Then + assertThat(client.getFlagAssignmentsSnapshot()).containsOnlyKeys(fakeFlagKey) + } + + // endregion + // region Static Flag Resolution Methods @Test diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 9867b34ed0..afc7542667 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -486,6 +486,24 @@ internal class DefaultFlagsRepositoryTest { assertThat(testedRepository.hasFlags()).isFalse() } + @Test + fun `M ignore persisted state W initial disk read completes { state saved without a flag selection }`( + forge: Forge + ) { + // Given + val flags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) + val callback = preparePersistenceLoad(forge, CanonicalFlagSelection(listOf("mobile"), emptyList())) + + // When + callback.onSuccess( + DataStoreContent(forge.anInt(), FlagsStateEntry(forge.getForgery(), flags, forge.aLong())) + ) + + // Then + assertThat(testedRepository.getEvaluationContext()).isNull() + assertThat(testedRepository.hasFlags()).isFalse() + } + @Test fun `M install persisted state W initial disk read completes { same flag selection }`(forge: Forge) { // Given diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index 03bd713f74..84185cc71d 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -61,7 +61,8 @@ internal class FlagSelectionCanonicalizerTest { @Test fun `M trim the Unicode White_Space set W canonicalize()`() { // Given - val whiteSpace = "\t\n\u000B\u000C\r \u0085\u00A0\u1680\u2000\u200A\u2028\u2029\u202F\u205F\u3000" + val whiteSpace = "\t\n\u000B\u000C\r \u0085\u00A0\u1680" + + "\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u2028\u2029\u202F\u205F\u3000" val selection = FlagSelection( tags = listOf("${whiteSpace}mobile$whiteSpace"), keyPatterns = listOf("\uFEFFcheckout\uFEFF", "\u001Cpayment\u001C") From 2d0533426910e690871870a2b11f7110e1f6cd6d Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Tue, 27 Oct 2026 19:14:07 +0000 Subject: [PATCH 09/34] FFL-3459: Simplify flag selection internals and tests Dedupe selection values before sorting, compute each UTF-8 sort key once, and skip sorting a list that is already over the value limit. Pass the response mapper by reference. Drop positional null arguments in tests that only restated the default obfuscation value. Ref: FFL-3459 --- .../internal/evaluation/EvaluationsManager.kt | 2 +- .../flags/internal/selection/FlagSelection.kt | 21 ++++++++++++------- .../persistence/FlagsStateSerializerTest.kt | 8 +++---- .../repository/DefaultFlagsRepositoryTest.kt | 10 ++++----- .../repository/FirstFlagsInstallationTest.kt | 4 ++-- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index 75874efdcd..4b28d13725 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -150,7 +150,7 @@ internal class EvaluationsManager( hadFlags && flagsRepository.getEvaluationContext() == context ) val response = datadogContext?.let { assignmentsReader.readPrecomputedFlags(context, it) } - val assignments = response?.let { mapResponse(it) } + val assignments = response?.let(::mapResponse) ?.takeIf { it.obfuscation == null || supportsObfuscation } if (assignments != null) { installFlags(context, assignments, initializationCompletion, callback) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index e81868c8ac..988aec5e7e 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -91,13 +91,20 @@ internal object FlagSelectionCanonicalizer { } } - private fun canonicalizeList(values: List): List = values - .asSequence() - .map { value -> value.trim { it in WHITE_SPACE } } - .filter { it.isNotEmpty() } - .sortedBy { it.utf8SortKey() } - .toList() - .distinct() + private fun canonicalizeList(values: List): List { + val distinct = values + .asSequence() + .map { value -> value.trim { it in WHITE_SPACE } } + .filter { it.isNotEmpty() } + .toList() + .distinct() + // An over-limit list is dropped as too_many_values, so its order does not matter. + if (distinct.size > MAX_VALUES) return distinct + return distinct + .map { it to it.utf8SortKey() } + .sortedBy { it.second } + .map { it.first } + } // Latin-1 maps each byte to one char, so comparing these strings compares unsigned UTF-8 bytes. private fun String.utf8SortKey(): String = String(toByteArray(Charsets.UTF_8), Charsets.ISO_8859_1) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt index 4ae942584e..106ebfae07 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/persistence/FlagsStateSerializerTest.kt @@ -104,7 +104,7 @@ internal class FlagsStateSerializerTest { ) ) - val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp, null) + val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp) // When val serialized = testedSerializer.serialize(flagsState) @@ -145,7 +145,7 @@ internal class FlagsStateSerializerTest { serialId = null ) ) - val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp, null) + val flagsState = FlagsStateEntry(evaluationContext, flags, fakeTimestamp) // When val serialized = testedSerializer.serialize(flagsState) @@ -161,7 +161,7 @@ internal class FlagsStateSerializerTest { // Given val targetingKey = forge.anAlphabeticalString() val evaluationContext = EvaluationContext(targetingKey, emptyMap()) - val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp, null) + val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp) // When val serialized = testedSerializer.serialize(flagsState) @@ -178,7 +178,7 @@ internal class FlagsStateSerializerTest { // Given // Create a state that could potentially cause serialization issues val evaluationContext = EvaluationContext("key", emptyMap()) - val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp, null) + val flagsState = FlagsStateEntry(evaluationContext, emptyMap(), fakeTimestamp) // When val serialized = testedSerializer.serialize(flagsState) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index afc7542667..7b624d4c6d 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -342,7 +342,7 @@ internal class DefaultFlagsRepositoryTest { }.toMap() val callback = preparePersistenceLoad(forge) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) val snapshot = testedRepository.getFlagsSnapshot() flags.forEach { (key, originalFlag) -> @@ -369,7 +369,7 @@ internal class DefaultFlagsRepositoryTest { anAlphabeticalString() to getForgery() }.toMap() val callback = preparePersistenceLoad(forge) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) val cachedSnapshot = testedRepository.getFlagsSnapshot() testedRepository.setFlagsAndContext(context, flags) @@ -398,7 +398,7 @@ internal class DefaultFlagsRepositoryTest { val networkFlags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) testedRepository.setFlagsAndContext(networkContext, networkFlags) - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong(), null))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, flags, forge.aLong()))) assertThat(testedRepository.getFlagsSnapshot()).isSameAs(networkFlags) assertThat(testedRepository.getEvaluationContext()).isEqualTo(networkContext) @@ -411,7 +411,7 @@ internal class DefaultFlagsRepositoryTest { val callback = preparePersistenceLoad(forge) // When - callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, emptyMap(), forge.aLong(), null))) + callback.onSuccess(DataStoreContent(forge.anInt(), FlagsStateEntry(context, emptyMap(), forge.aLong()))) // Then assertThat(testedRepository.getEvaluationContext()).isEqualTo(context) @@ -434,7 +434,7 @@ internal class DefaultFlagsRepositoryTest { val networkContext = diskContext.copy(targetingKey = diskContext.targetingKey + "-network") val diskFlags = mapOf(forge.anAlphabeticalString() to forge.getForgery()) val callback = preparePersistenceLoad(forge) - val diskContent = DataStoreContent(forge.anInt(), FlagsStateEntry(diskContext, diskFlags, forge.aLong(), null)) + val diskContent = DataStoreContent(forge.anInt(), FlagsStateEntry(diskContext, diskFlags, forge.aLong())) // When if (diskFirst) { diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt index 6c3ab53690..b9f4ef0839 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt @@ -252,7 +252,7 @@ internal class FirstFlagsInstallationTest { try { // When val loading = executor.submit { - disk.onSuccess(DataStoreContent(1, FlagsStateEntry(context, flags, 0, null))) + disk.onSuccess(DataStoreContent(1, FlagsStateEntry(context, flags, 0))) } // Then @@ -372,6 +372,6 @@ internal class FirstFlagsInstallationTest { private fun restored(keys: List): DataStoreContent = DataStoreContent( 1, - FlagsStateEntry(context, keys.associateWith { mock() }, 0, null) + FlagsStateEntry(context, keys.associateWith { mock() }, 0) ) } From ec0039c6552ac32ae3666edd5914dadd1d0ec978 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:46:09 -0400 Subject: [PATCH 10/34] FFL-3459: Test the ignored reason with an unparseable body The reason is reported before the body is parsed, so it is logged even when the configuration is not installed. --- .../evaluation/EvaluationsManagerTest.kt | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index b1b0c5afaf..a63c5a8eb0 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -1679,6 +1679,40 @@ internal class EvaluationsManagerTest { verify(mockFlagsStateManager).updateState(FlagsClientState.Ready) } + @Test + fun `M log the ignored reason and not install W updateEvaluationsForContext() { ignored reason, invalid body }`( + @StringForgery fakeInvalidBody: String + ) { + // Given + val context = EvaluationContext(fakeTargetingKey, emptyMap()) + whenever( + mockAssignmentsDownloader.readPrecomputedFlags( + eq(context), + eq(fakeDatadogContext), + isA() + ) + ).thenReturn(PrecomputedAssignmentsResponse(fakeInvalidBody, "too_many_values")) + whenever(mockPrecomputeMapper.map(fakeInvalidBody)).thenReturn(null) + + // When + evaluationsManager.updateEvaluationsForContext(context) + + // Then + mockInternalLogger.verifyLog( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + "flagSelection was ignored by the server (too_many_values). Loaded the full flag configuration." + ) + mockInternalLogger.verifyLog( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + "Flag selection ignored", + additionalProperties = mapOf("reason" to "too_many_values") + ) + verify(mockFlagsRepository, never()).setFlagsAndContext(any(), any(), anyOrNull(), any()) + verify(mockFlagsStateManager).updateState(isA()) + } + @Test fun `M not log a flag selection message W updateEvaluationsForContext() { no ignored reason }`() { // Given From a8cfcab920c5a50d888b498b12cbe51027e65af3 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:46:10 -0400 Subject: [PATCH 11/34] FFL-3459: Assert the selection reaches the request factory --- .../com/datadog/android/flags/internal/FlagsFeatureTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt index 8709c2bf02..c7c11f2e99 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt @@ -115,6 +115,7 @@ internal class FlagsFeatureTest { // Then assertThat(feature.flagSelection) .isEqualTo(CanonicalFlagSelection(tags = listOf("mobile", "web"), keyPatterns = listOf("checkout-*"))) + assertThat(feature.precomputedRequestFactory.sendsFlagSelection).isTrue() } @Test @@ -132,6 +133,7 @@ internal class FlagsFeatureTest { // Then assertThat(feature.flagSelection).isNull() + assertThat(feature.precomputedRequestFactory.sendsFlagSelection).isFalse() mockInternalLogger.verifyLog( InternalLogger.Level.ERROR, InternalLogger.Target.USER, From fce86258710029e5d01034df5467a0e078138588 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:46:11 -0400 Subject: [PATCH 12/34] FFL-3459: Describe flag selection rules without backend names --- .../android/flags/internal/selection/FlagSelection.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index 988aec5e7e..998378766b 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -24,7 +24,7 @@ internal data class CanonicalFlagSelection( ) /** - * Reasons a flag selection is dropped. Values match the reasons used by Compute and ffe-service. + * Reasons a flag selection is dropped. The values are the reason codes that the server and the other Datadog SDKs use. */ internal enum class FlagSelectionError(val reason: String, val limit: String) { MALFORMED_SELECTION( @@ -52,7 +52,7 @@ internal sealed interface FlagSelectionResult { } /** - * Canonicalizes and validates a flag selection the same way Compute and ffe-service do. + * Canonicalizes and validates a flag selection with the same rules as the server. */ internal object FlagSelectionCanonicalizer { @@ -60,7 +60,7 @@ internal object FlagSelectionCanonicalizer { internal const val MAX_TAG_BYTES = 200 internal const val MAX_KEY_PATTERN_BYTES = 200 - // The Unicode White_Space property, which Compute trims with Rust's str::trim. + // The Unicode White_Space property, which the server trims. // Kotlin's trim() differs: it also trims U+001C..U+001F and does not trim U+0085. private const val WHITE_SPACE = "\u0009\u000A\u000B\u000C\u000D\u0020\u0085\u00A0\u1680" + "\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A" + From 1cc677143792acd7959c1757c520d91a06ac336c Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:48:13 -0400 Subject: [PATCH 13/34] FFL-3459: Document flagSelection in the module README --- features/dd-sdk-android-flags/README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index 47e6a7fca5..d2b279ea7a 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -93,6 +93,25 @@ val flagsConfig = FlagsConfiguration.Builder() .build() ``` +#### Load only selected flags + +To load only the flags that your app uses, set a flag selection. The SDK sends it with the flag request, and the server returns only the flags that have any of the tags or whose key matches any of the key patterns: + +```kotlin +val flagsConfig = FlagsConfiguration.Builder() + .flagSelection( + tags = listOf("mobile"), + keyPatterns = listOf("checkout-*") + ) + .build() +``` + +- Tags match exactly and are case-sensitive. A key pattern matches the whole flag key, and only `*` is a wildcard. +- Each list accepts at most 31 values, and each value is at most 200 UTF-8 bytes. The SDK does not send a selection that is over a limit. It logs an error and loads the full flag configuration. +- When the server applies the selection, evaluating a flag outside it returns the default value with `ErrorCode.FLAG_NOT_FOUND`. +- The selection is fixed when you call `Flags.enable`. +- With a custom flag endpoint, the proxy must forward the `DD-Flag-Selection-Ignored` response header. Otherwise, the SDK cannot report a selection that the server ignored. + ## Use the Feature Flags SDK ### Create a Flags client From d365cbb9a29ab7ce23f9569c5d5afbf4258099c9 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:53:31 -0400 Subject: [PATCH 14/34] FFL-3459: Run every shared selection vector case by name List all 38 cases that have an sdk expectation in the shared vectors, in file order, so each one maps to one test row. --- .../FlagSelectionCanonicalizerTest.kt | 152 +++++++++++++++--- 1 file changed, 129 insertions(+), 23 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index 84185cc71d..3c8617f9b8 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -193,94 +193,200 @@ internal class FlagSelectionCanonicalizerTest { Arguments.of("200 bytes after trimming", keyPatterns(" ${"a".repeat(200)}\u3000")) ) - // Cases with `sdk` expectations in DataDog/ffe-system-test-data@e8777633 - // flag-selection/selection-vectors.json that the tests above do not already cover. + // Every case with an `sdk` expectation in DataDog/ffe-system-test-data@e8777633 + // flag-selection/selection-vectors.json, in file order and under the same name. An empty + // expected selection is no selection on Android. @JvmStatic fun sharedVectorSelections() = listOf( Arguments.of( - "leading star", - keyPatterns("*-v2"), - selected(emptyList(), listOf("*-v2")) + "no parameters selects every flag", + FlagSelection(emptyList(), emptyList()), + FlagSelectionResult.None + ), + Arguments.of( + "tag selects flags having any of the tags", + FlagSelection(listOf("team:search", "mobile"), emptyList()), + selected(listOf("mobile", "team:search"), emptyList()) + ), + Arguments.of( + "tags and key patterns are combined with OR", + FlagSelection(listOf("mobile"), listOf("checkout.*")), + selected(listOf("mobile"), listOf("checkout.*")) + ), + Arguments.of( + "key pattern must match the whole key", + FlagSelection(emptyList(), listOf("checkout")), + selected(emptyList(), listOf("checkout")) + ), + Arguments.of( + "key pattern without a star selects exactly that key", + FlagSelection(emptyList(), listOf("checkout.new-flow")), + selected(emptyList(), listOf("checkout.new-flow")) ), Arguments.of( "star matches an empty run and multiple stars are allowed", - keyPatterns("*search*auto*complete*"), + FlagSelection(emptyList(), listOf("*search*auto*complete*")), selected(emptyList(), listOf("*search*auto*complete*")) ), + Arguments.of( + "leading star", + FlagSelection(emptyList(), listOf("*-v2")), + selected(emptyList(), listOf("*-v2")) + ), Arguments.of( "lone star matches every key", - keyPatterns("*"), + FlagSelection(emptyList(), listOf("*")), selected(emptyList(), listOf("*")) ), Arguments.of( "question mark and brackets are literal", - keyPatterns("search.ranking-v?", "[a]*", "{a,b}"), + FlagSelection(emptyList(), listOf("search.ranking-v?", "[a]*", "{a,b}")), selected(emptyList(), listOf("[a]*", "search.ranking-v?", "{a,b}")) ), Arguments.of( "braces are literal even when an alternative names a flag", - keyPatterns("{mobile.dark-mode,missing}"), + FlagSelection(emptyList(), listOf("{mobile.dark-mode,missing}")), selected(emptyList(), listOf("{mobile.dark-mode,missing}")) ), Arguments.of( "tags are never globs", - tags("team:*"), + FlagSelection(listOf("team:*"), emptyList()), selected(listOf("team:*"), emptyList()) ), Arguments.of( "tags match case-sensitively", - tags("Team:Payments", "MOBILE"), + FlagSelection(listOf("Team:Payments", "MOBILE"), emptyList()), selected(listOf("MOBILE", "Team:Payments"), emptyList()) ), + Arguments.of( + "key patterns match case-sensitively", + FlagSelection(emptyList(), listOf("CHECKOUT.*", "Mobile.Dark-Mode")), + selected(emptyList(), listOf("CHECKOUT.*", "Mobile.Dark-Mode")) + ), + Arguments.of( + "values are trimmed, empties dropped and duplicates removed", + FlagSelection(listOf(" mobile ", "mobile", "", " "), emptyList()), + selected(listOf("mobile"), emptyList()) + ), + Arguments.of( + "lists that are empty after normalization select every flag", + FlagSelection(listOf("", " "), listOf(" ")), + FlagSelectionResult.None + ), + Arguments.of( + "values are trimmed of Unicode white space", + FlagSelection(listOf("\u00A0mobile\u3000", "\u0085ui\u2028", "\u0009 team:search \u000A"), emptyList()), + selected(listOf("mobile", "team:search", "ui"), emptyList()) + ), Arguments.of( "characters outside Unicode White_Space are not trimmed", - tags("\uFEFFmobile", "mobile\u200B"), + FlagSelection(listOf("\uFEFFmobile", "mobile\u200B"), emptyList()), selected(listOf("mobile\u200B", "\uFEFFmobile"), emptyList()) ), Arguments.of( "values sort by UTF-8 bytes", - tags(GRINNING_FACE, "\uFF5E", "a", "Z"), + FlagSelection(listOf(GRINNING_FACE, "\uFF5E", "a", "Z"), emptyList()), selected(listOf("Z", "a", "\uFF5E", GRINNING_FACE), emptyList()) ), + Arguments.of( + "partial match serves only the matching flags", + FlagSelection(listOf("mobile", "missing"), listOf("nothing.*")), + selected(listOf("missing", "mobile"), listOf("nothing.*")) + ), + Arguments.of( + "31 values in one list are allowed", + FlagSelection(numbered("t", 31), emptyList()), + selected(numbered("t", 31), emptyList()) + ), + Arguments.of( + "32 values in one list are rejected", + FlagSelection(emptyList(), numbered("k", 32)), + FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ), Arguments.of( "31 values in each list are allowed", FlagSelection(numbered("t", 31), numbered("p", 31)), selected(numbered("t", 31), numbered("p", 31)) ), + Arguments.of( + "32 values in one list are rejected even when the other is within the limit", + FlagSelection(numbered("t", 31), numbered("p", 32)), + FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ), Arguments.of( "duplicates do not count toward the limit", - FlagSelection(numbered("t", 31) + "t01" + " t02 ", emptyList()), + FlagSelection(numbered("t", 31) + listOf("t01", " t02 "), emptyList()), selected(numbered("t", 31), emptyList()) ), + Arguments.of( + "key pattern of 200 bytes is allowed", + FlagSelection(emptyList(), listOf("*" + "x".repeat(199))), + selected(emptyList(), listOf("*" + "x".repeat(199))) + ), + Arguments.of( + "key pattern of 201 bytes is invalid", + FlagSelection(emptyList(), listOf("*" + "x".repeat(200))), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_KEY_PATTERN) + ), + Arguments.of( + "key pattern length is measured after trimming", + FlagSelection(emptyList(), listOf(" *" + "x".repeat(199) + " ")), + selected(emptyList(), listOf("*" + "x".repeat(199))) + ), Arguments.of( "key pattern length counts UTF-8 bytes: 100 two-byte characters are allowed", - keyPatterns("\u00E9".repeat(100)), + FlagSelection(emptyList(), listOf("\u00E9".repeat(100))), selected(emptyList(), listOf("\u00E9".repeat(100))) ), + Arguments.of( + "key pattern length counts UTF-8 bytes: 201 bytes are invalid", + FlagSelection(emptyList(), listOf("x" + "\u00E9".repeat(100))), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_KEY_PATTERN) + ), + Arguments.of( + "tag of 200 bytes is allowed", + FlagSelection(listOf("a".repeat(200)), emptyList()), + selected(listOf("a".repeat(200)), emptyList()) + ), + Arguments.of( + "tag of 201 bytes is invalid", + FlagSelection(listOf("a".repeat(201)), emptyList()), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_TAG) + ), Arguments.of( "tag length is measured after trimming", - tags(" ${"a".repeat(200)} "), + FlagSelection(listOf(" " + "a".repeat(200) + " "), emptyList()), selected(listOf("a".repeat(200)), emptyList()) ), Arguments.of( - "32 values in one list are rejected even when the other is within the limit", - FlagSelection(numbered("t", 31), numbered("p", 32)), - FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + "tag length counts UTF-8 bytes: 100 two-byte characters are allowed", + FlagSelection(listOf("\u00E9".repeat(100)), emptyList()), + selected(listOf("\u00E9".repeat(100)), emptyList()) ), Arguments.of( - "key pattern length counts UTF-8 bytes: 201 bytes are invalid", - keyPatterns("x" + "\u00E9".repeat(100)), - FlagSelectionResult.Dropped(FlagSelectionError.INVALID_KEY_PATTERN) + "tag length counts UTF-8 bytes: 201 bytes are invalid", + FlagSelection(listOf("x" + "\u00E9".repeat(100)), emptyList()), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_TAG) ), Arguments.of( "a long tag invalidates the selection even when other tags match", - tags("mobile", "a".repeat(201)), + FlagSelection(listOf("mobile", "a".repeat(201)), emptyList()), FlagSelectionResult.Dropped(FlagSelectionError.INVALID_TAG) ), + Arguments.of( + "too_many_values takes precedence over invalid_key_pattern", + FlagSelection(numbered("t", 32), listOf("*" + "x".repeat(200))), + FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ), Arguments.of( "too_many_values takes precedence over invalid_tag", FlagSelection(numbered("t", 31) + "a".repeat(201), emptyList()), FlagSelectionResult.Dropped(FlagSelectionError.TOO_MANY_VALUES) + ), + Arguments.of( + "invalid_key_pattern takes precedence over invalid_tag", + FlagSelection(listOf("a".repeat(201)), listOf("a".repeat(201))), + FlagSelectionResult.Dropped(FlagSelectionError.INVALID_KEY_PATTERN) ) ) From f80f2c3f747e6bf52b448e591aa8db78d446c9e4 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:53:33 -0400 Subject: [PATCH 15/34] FFL-3459: Test the ignored header name without case sensitivity --- .../PrecomputedAssignmentsDownloaderTest.kt | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index dd453c36a5..f6caeb29b7 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -135,6 +135,27 @@ internal class PrecomputedAssignmentsDownloaderTest { assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) } + @Test + fun `M return the ignored reason W readPrecomputedFlags() { selection sent, lowercase header name }`( + @StringForgery fakeResponseBody: String, + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + stubSuccessfulCall( + fakeResponseBody, + fakeUrl, + ignoredReason = "too_many_values", + headerName = "dd-flag-selection-ignored" + ) + whenever(mockRequestFactory.sendsFlagSelection) doReturn true + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext, fakeDiagnostics) + + // Then + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, "too_many_values")) + } + @Test fun `M return no ignored reason W readPrecomputedFlags() { selection sent, empty ignored header }`( @StringForgery fakeResponseBody: String, @@ -452,11 +473,16 @@ internal class PrecomputedAssignmentsDownloaderTest { // region Helper methods - private fun stubSuccessfulCall(body: String, url: String, ignoredReason: String) { + private fun stubSuccessfulCall( + body: String, + url: String, + ignoredReason: String, + headerName: String = "DD-Flag-Selection-Ignored" + ) { val fakeRequest = Request.Builder().url(url).build() val fakeResponse = createSuccessfulResponse(body, url) .newBuilder() - .header("DD-Flag-Selection-Ignored", ignoredReason) + .header(headerName, ignoredReason) .build() whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext, fakeDiagnostics)) .doReturn(fakeRequest) From f79b4d776942a788e4d22f58fd6c9b5e1ceaad57 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:53:33 -0400 Subject: [PATCH 16/34] FFL-3459: Test equivalent selections and empty configurations in the repository --- .../repository/DefaultFlagsRepositoryTest.kt | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 7b624d4c6d..412ba8773c 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -15,6 +15,9 @@ import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedAssignments import com.datadog.android.flags.internal.model.PrecomputedFlag import com.datadog.android.flags.internal.selection.CanonicalFlagSelection +import com.datadog.android.flags.internal.selection.FlagSelection +import com.datadog.android.flags.internal.selection.FlagSelectionCanonicalizer +import com.datadog.android.flags.internal.selection.FlagSelectionResult import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.ResolutionReason import com.datadog.android.flags.utils.forge.ForgeConfigurator @@ -529,6 +532,33 @@ internal class DefaultFlagsRepositoryTest { assertThat(testedRepository.getPrecomputedFlag(flagKey)).isNotNull() } + @Test + fun `M install persisted state W initial disk read completes { equivalent configured selection }`(forge: Forge) { + // Given + val context = forge.getForgery() + val flagKey = forge.anAlphabeticalString() + val persistedSelection = canonical(FlagSelection(tags = listOf("checkout"), keyPatterns = null)) + val configuredSelection = canonical(FlagSelection(tags = listOf(" checkout", "checkout"), keyPatterns = null)) + val callback = preparePersistenceLoad(forge, configuredSelection) + + // When + callback.onSuccess( + DataStoreContent( + forge.anInt(), + FlagsStateEntry( + context, + mapOf(flagKey to forge.getForgery()), + forge.aLong(), + flagSelection = persistedSelection + ) + ) + ) + + // Then + assertThat(testedRepository.getEvaluationContext()).isEqualTo(context) + assertThat(testedRepository.getPrecomputedFlag(flagKey)).isNotNull() + } + @Test fun `M persist the configured flag selection W setFlagsAndContext()`(forge: Forge) { // Given @@ -550,6 +580,19 @@ internal class DefaultFlagsRepositoryTest { assertThat(entry.firstValue.flagSelection).isEqualTo(flagSelection) } + @Test + fun `M return no flag with the context W getPrecomputedFlagWithContext() { empty flags map }`(forge: Forge) { + // Given + val context = forge.getForgery() + testedRepository.setFlagsAndContext(context, emptyMap()) + + // When + val result = testedRepository.getPrecomputedFlagWithContext(forge.anAlphabeticalString()) + + // Then + assertThat(result).isEqualTo(null to context) + } + @Test fun `M return true W hasLoadedFlagsForContext() { empty flags map for the same context }`(forge: Forge) { // Given @@ -572,6 +615,9 @@ internal class DefaultFlagsRepositoryTest { ).isFalse() } + private fun canonical(selection: FlagSelection): CanonicalFlagSelection = + checkNotNull(FlagSelectionCanonicalizer.canonicalize(selection) as? FlagSelectionResult.Selected).selection + private fun preparePersistenceLoad( forge: Forge, flagSelection: CanonicalFlagSelection? = null From ca5b0801a31fa9392eba5096dcf1f4be788be1fb Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:53:34 -0400 Subject: [PATCH 17/34] FFL-3459: Test selections with no values after canonicalization --- .../flags/internal/FlagsFeatureTest.kt | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt index c7c11f2e99..24f4e258e2 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt @@ -23,12 +23,17 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource import org.mockito.Mock import org.mockito.Mockito.mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.atLeast import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq import org.mockito.kotlin.verify @@ -118,6 +123,39 @@ internal class FlagsFeatureTest { assertThat(feature.precomputedRequestFactory.sendsFlagSelection).isTrue() } + @ParameterizedTest(name = "{0}") + @MethodSource("selectionsWithoutValues") + fun `M send no selection and log nothing W onInitialize() { no values after canonicalization }`( + description: String, + tags: List?, + keyPatterns: List? + ) { + // Given + val feature = FlagsFeature( + sdkCore = mockSdkCore, + flagsConfiguration = FlagsConfiguration.Builder() + .flagSelection(tags = tags, keyPatterns = keyPatterns) + .build() + ) + + // When + feature.onInitialize(mockContext) + + // Then + assertThat(feature.flagSelection).describedAs(description).isNull() + assertThat(feature.precomputedRequestFactory.sendsFlagSelection).describedAs(description).isFalse() + val messages = argumentCaptor<() -> String>() + verify(mockInternalLogger, atLeast(0)).log( + any(), + any(), + messages.capture(), + anyOrNull(), + any(), + anyOrNull() + ) + assertThat(messages.allValues.map { it() }).noneMatch { "flagSelection" in it || "Flag selection" in it } + } + @Test fun `M log the dropped flag selection W onInitialize() { selection over a limit }`() { // Given @@ -440,4 +478,13 @@ internal class FlagsFeatureTest { } // endregion + + companion object { + @JvmStatic + fun selectionsWithoutValues() = listOf( + Arguments.of("empty lists", emptyList(), emptyList()), + Arguments.of("whitespace-only values", listOf(" ", "\u3000"), listOf("")), + Arguments.of("null lists", null, null) + ) + } } From db50cef33c1250fea890be327dfa2725cbd51d9c Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 10:55:30 -0400 Subject: [PATCH 18/34] FFL-3459: Drop a selection with non-string elements instead of throwing A Java caller can pass a raw list, so the canonicalizer checks each element's type. A non-string element is malformed_selection, as in the browser SDK, instead of a ClassCastException from Flags.enable. --- .../flags/internal/selection/FlagSelection.kt | 5 +++-- .../selection/FlagSelectionCanonicalizerTest.kt | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index 998378766b..06f657a384 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -111,8 +111,9 @@ internal object FlagSelectionCanonicalizer { private fun String.utf8Size(): Int = toByteArray(Charsets.UTF_8).size - private fun List.wellFormedOrNull(): List? = - if (all { it != null && !it.hasUnpairedSurrogate() }) filterIsInstance() else null + // A Java caller can pass a raw list, so check each element's type instead of trusting the generic type. + private fun List<*>.wellFormedOrNull(): List? = + if (all { it is String && !it.hasUnpairedSurrogate() }) filterIsInstance() else null private fun String.hasUnpairedSurrogate(): Boolean { var index = 0 diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index 3c8617f9b8..e2f4109c4b 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -176,6 +176,10 @@ internal class FlagSelectionCanonicalizerTest { private fun values(count: Int) = List(count) { "value-$it" } + // Java callers can pass a raw list, so the elements are not checked to be strings. + @Suppress("UNCHECKED_CAST") + private fun uncheckedStrings(vararg values: Any?) = values.toList() as List + private fun tags(vararg tags: String?) = FlagSelection(tags = tags.toList(), keyPatterns = emptyList()) private fun keyPatterns(vararg keyPatterns: String?) = @@ -423,6 +427,16 @@ internal class FlagSelectionCanonicalizerTest { ), Arguments.of("null tag", tags("mobile", null), FlagSelectionError.MALFORMED_SELECTION), Arguments.of("null key pattern", keyPatterns(null), FlagSelectionError.MALFORMED_SELECTION), + Arguments.of( + "non-string tag from an unchecked cast", + FlagSelection(tags = uncheckedStrings("mobile", 1), keyPatterns = emptyList()), + FlagSelectionError.MALFORMED_SELECTION + ), + Arguments.of( + "non-string key pattern from an unchecked cast", + FlagSelection(tags = emptyList(), keyPatterns = uncheckedStrings(true)), + FlagSelectionError.MALFORMED_SELECTION + ), Arguments.of( "malformed before too many values", FlagSelection(values(32) + null, emptyList()), From 765454c72d8d4245d007f2b3d1c4d25dfd3f4785 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 11:12:18 -0400 Subject: [PATCH 19/34] FFL-3459: Remove the region markers from the flag selection client test --- .../test/kotlin/com/datadog/android/flags/FlagsClientTest.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt index f476f086d6..6328f1efeb 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsClientTest.kt @@ -131,8 +131,6 @@ internal class FlagsClientTest { flagsFeature?.clearClients() } - // region Flag Selection Wiring - @Test fun `M key persisted flags by the configured selection W createInternal() { flag selection }`( @StringForgery fakeFlagKey: String, @@ -194,8 +192,6 @@ internal class FlagsClientTest { assertThat(client.getFlagAssignmentsSnapshot()).containsOnlyKeys(fakeFlagKey) } - // endregion - // region Static Flag Resolution Methods @Test From ff7c44462bc05eea64b02e9ec2782593b199f640 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 11:12:19 -0400 Subject: [PATCH 20/34] FFL-3459: Verify the absent selection logs with verifyLog --- .../flags/internal/FlagsFeatureTest.kt | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt index 24f4e258e2..6f62a3f056 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt @@ -13,6 +13,7 @@ import com.datadog.android.api.feature.Feature.Companion.FLAGS_FEATURE_NAME import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.flags.FlagsConfiguration import com.datadog.android.flags.internal.selection.CanonicalFlagSelection +import com.datadog.android.flags.internal.selection.FlagSelectionError import com.datadog.android.flags.internal.storage.ExposureEventRecordWriter import com.datadog.android.flags.internal.storage.NoOpRecordWriter import com.datadog.android.utils.verifyLog @@ -31,11 +32,10 @@ import org.mockito.Mockito.mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor -import org.mockito.kotlin.atLeast import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever @@ -144,16 +144,21 @@ internal class FlagsFeatureTest { // Then assertThat(feature.flagSelection).describedAs(description).isNull() assertThat(feature.precomputedRequestFactory.sendsFlagSelection).describedAs(description).isFalse() - val messages = argumentCaptor<() -> String>() - verify(mockInternalLogger, atLeast(0)).log( - any(), - any(), - messages.capture(), - anyOrNull(), - any(), - anyOrNull() + mockInternalLogger.verifyLog( + InternalLogger.Level.ERROR, + InternalLogger.Target.USER, + { "flagSelection" in it }, + mode = never() ) - assertThat(messages.allValues.map { it() }).noneMatch { "flagSelection" in it || "Flag selection" in it } + FlagSelectionError.entries.forEach { + mockInternalLogger.verifyLog( + InternalLogger.Level.DEBUG, + InternalLogger.Target.TELEMETRY, + "Flag selection dropped", + mode = never(), + additionalProperties = mapOf("reason" to it.reason) + ) + } } @Test From d29e8e3d1ea70c088ff40c15f7195d81dab7917f Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 11:13:39 -0400 Subject: [PATCH 21/34] FFL-3459: Cover every pair of the selection error order --- .../selection/FlagSelectionCanonicalizerTest.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index e2f4109c4b..5c9f895ef8 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -442,6 +442,16 @@ internal class FlagSelectionCanonicalizerTest { FlagSelection(values(32) + null, emptyList()), FlagSelectionError.MALFORMED_SELECTION ), + Arguments.of( + "malformed before invalid key pattern", + FlagSelection(listOf("a\uD83D"), listOf("a".repeat(201))), + FlagSelectionError.MALFORMED_SELECTION + ), + Arguments.of( + "malformed before invalid tag", + FlagSelection(listOf("a".repeat(201)), listOf(null)), + FlagSelectionError.MALFORMED_SELECTION + ), Arguments.of( "too many values before invalid key pattern", FlagSelection(values(32), listOf("a".repeat(201))), From f68612f3e41cfc959bc2ee8bdaed1fe8ee449a91 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 13:00:57 -0400 Subject: [PATCH 22/34] FFL-3459: Keep selected flag state out of the key older SDKs read SDK versions without flag selection read flags-state- with any version and ignore the flagSelection field, so a rollback served a selected subset as the full configuration. Selected state now uses flags-selection-state-, and creating an instance removes the other key so one state remains per instance. Unselected state stays where older SDKs read it. --- .../persistence/FlagsPersistenceManager.kt | 12 ++- .../repository/DefaultFlagsRepository.kt | 2 +- .../repository/DefaultFlagsRepositoryTest.kt | 95 +++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt index 12c750724d..bdf5b81bff 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt @@ -20,21 +20,28 @@ import com.datadog.android.flags.model.EvaluationContext internal class FlagsPersistenceManager( private val dataStore: DataStoreHandler, instanceName: String, + private val flagSelection: CanonicalFlagSelection?, private val internalLogger: InternalLogger, onStateLoaded: (FlagsStateEntry?) -> Unit ) { private val serializer = FlagsStateSerializer(internalLogger) private val deserializer = FlagsStateDeserializer(internalLogger) - private val flagsStateKey: String = "$FLAGS_STATE_KEY_PREFIX-$instanceName" + + // SDK versions without flag selection read only the unselected key and ignore the flagSelection field, + // so selected state uses its own key and is never read by them as a full configuration. + private val unselectedStateKey = "$FLAGS_STATE_KEY_PREFIX-$instanceName" + private val selectedStateKey = "$FLAGS_SELECTION_STATE_KEY_PREFIX-$instanceName" + private val flagsStateKey: String = if (flagSelection == null) unselectedStateKey else selectedStateKey init { + // One persisted state per instance: changing to or from a selection discards the other key's state. + dataStore.removeValue(if (flagSelection == null) selectedStateKey else unselectedStateKey) loadFlagsState(onStateLoaded) } internal fun saveFlagsState( context: EvaluationContext, flags: Map, - flagSelection: CanonicalFlagSelection?, currentTimestamp: Long, callback: DataStoreWriteCallback? = null, obfuscation: FlagKeyObfuscation? = null @@ -79,5 +86,6 @@ internal class FlagsPersistenceManager( companion object { private const val FLAGS_STATE_KEY_PREFIX = "flags-state" + private const val FLAGS_SELECTION_STATE_KEY_PREFIX = "flags-selection-state" } } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt index a0a92a6329..0f3d4515af 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt @@ -52,6 +52,7 @@ internal class DefaultFlagsRepository( private val persistenceManager = FlagsPersistenceManager( dataStore = dataStore, instanceName = instanceName, + flagSelection = flagSelection, internalLogger = internalLogger ) { loadedState -> // State fetched with a different selection holds a different set of flags. @@ -89,7 +90,6 @@ internal class DefaultFlagsRepository( persistenceManager.saveFlagsState( context = context, flags = flags, - flagSelection = flagSelection, currentTimestamp = featureSdkCore.timeProvider.getDeviceTimestampMillis(), callback = object : DataStoreWriteCallback { override fun onSuccess() { diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 412ba8773c..543e2fb8ac 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -22,6 +22,7 @@ import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.ResolutionReason import com.datadog.android.flags.utils.forge.ForgeConfigurator import fr.xgouchet.elmyr.Forge +import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat @@ -40,6 +41,7 @@ import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -559,6 +561,99 @@ internal class DefaultFlagsRepositoryTest { assertThat(testedRepository.getPrecomputedFlag(flagKey)).isNotNull() } + @Test + fun `M persist the state under the selection key W setFlagsAndContext() { flag selection }`( + @StringForgery(regex = "fake-[a-z]{4,8}") fakeInstanceName: String, + forge: Forge + ) { + // Given + testedRepository = DefaultFlagsRepository( + mockFeatureSdkCore, + fakeInstanceName, + mockDataStore, + CanonicalFlagSelection(listOf("mobile"), emptyList()) + ) + + // When + testedRepository.setFlagsAndContext(forge.getForgery(), emptyMap()) + + // Then + verify(mockDataStore).setValue( + key = eq("flags-selection-state-$fakeInstanceName"), + data = any(), + version = any(), + callback = anyOrNull(), + serializer = any() + ) + verify(mockDataStore, never()).setValue( + key = eq("flags-state-$fakeInstanceName"), + data = any(), + version = any(), + callback = anyOrNull(), + serializer = any() + ) + } + + @Test + fun `M persist the state under the unselected key W setFlagsAndContext() { no flag selection }`( + @StringForgery(regex = "fake-[a-z]{4,8}") fakeInstanceName: String, + forge: Forge + ) { + // Given + testedRepository = DefaultFlagsRepository(mockFeatureSdkCore, fakeInstanceName, mockDataStore, null) + + // When + testedRepository.setFlagsAndContext(forge.getForgery(), emptyMap()) + + // Then + verify(mockDataStore).setValue( + key = eq("flags-state-$fakeInstanceName"), + data = any(), + version = any(), + callback = anyOrNull(), + serializer = any() + ) + } + + @Test + fun `M read the selection key and remove the unselected state W init { flag selection }`( + @StringForgery(regex = "fake-[a-z]{4,8}") fakeInstanceName: String + ) { + // When + testedRepository = DefaultFlagsRepository( + mockFeatureSdkCore, + fakeInstanceName, + mockDataStore, + CanonicalFlagSelection(listOf("mobile"), emptyList()) + ) + + // Then + verify(mockDataStore).value( + key = eq("flags-selection-state-$fakeInstanceName"), + version = anyOrNull(), + callback = any(), + deserializer = any() + ) + verify(mockDataStore).removeValue(eq("flags-state-$fakeInstanceName"), anyOrNull()) + } + + @Test + fun `M read the unselected key and remove the selected state W init { no flag selection }`( + @StringForgery(regex = "fake-[a-z]{4,8}") fakeInstanceName: String + ) { + // When + testedRepository = DefaultFlagsRepository(mockFeatureSdkCore, fakeInstanceName, mockDataStore, null) + + // Then + verify(mockDataStore).value( + key = eq("flags-state-$fakeInstanceName"), + version = anyOrNull(), + callback = any(), + deserializer = any() + ) + verify(mockDataStore).removeValue(eq("flags-selection-state-$fakeInstanceName"), anyOrNull()) + } + @Test fun `M persist the configured flag selection W setFlagsAndContext()`(forge: Forge) { // Given From f10b90f815b646ba1780dafd55569642686148a4 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 13:28:47 -0400 Subject: [PATCH 23/34] FFL-3459: Report only known DD-Flag-Selection-Ignored reasons The header value reaches USER logs and telemetry, so keep the five reasons the server sends and report any other non-empty value as unknown. The selection is still treated as ignored and the full configuration is still installed. --- .../net/PrecomputedAssignmentsDownloader.kt | 5 +- .../net/PrecomputedAssignmentsReader.kt | 2 +- .../flags/internal/selection/FlagSelection.kt | 22 ++++++++ .../evaluation/EvaluationsManagerTest.kt | 4 +- .../PrecomputedAssignmentsDownloaderTest.kt | 33 +++++++++++- .../FlagSelectionIgnoredReasonTest.kt | 52 +++++++++++++++++++ 6 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index edd9b3b80c..476a753b4e 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -10,6 +10,7 @@ import androidx.annotation.WorkerThread import com.datadog.android.api.InternalLogger import com.datadog.android.api.context.DatadogContext import com.datadog.android.flags.internal.model.PrecomputedAssignmentsRequest.Diagnostics +import com.datadog.android.flags.internal.selection.FlagSelectionIgnoredReason import com.datadog.android.flags.model.EvaluationContext import okhttp3.Call import okhttp3.Request @@ -58,7 +59,9 @@ internal class PrecomputedAssignmentsDownloader( PrecomputedAssignmentsResponse( body = body, flagSelectionIgnoredReason = if (requestFactory.sendsFlagSelection) { - response.header(HEADER_FLAG_SELECTION_IGNORED)?.takeIf { it.isNotEmpty() } + response.header(HEADER_FLAG_SELECTION_IGNORED) + ?.takeIf { it.isNotEmpty() } + ?.let(FlagSelectionIgnoredReason::sanitize) } else { null } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt index 42666a17e2..0f5c7c7615 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsReader.kt @@ -23,7 +23,7 @@ internal interface PrecomputedAssignmentsReader { } /** - * [flagSelectionIgnoredReason] is the `DD-Flag-Selection-Ignored` header value, or null when absent. + * [flagSelectionIgnoredReason] is the sanitized `DD-Flag-Selection-Ignored` reason, or null when the header is absent. */ internal data class PrecomputedAssignmentsResponse( val body: String, diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index 06f657a384..eac8cfbc10 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -45,6 +45,28 @@ internal enum class FlagSelectionError(val reason: String, val limit: String) { ) } +/** + * Sanitizes the `DD-Flag-Selection-Ignored` response header, so a raw header value never reaches logs or telemetry. + * A known reason is kept, and any other non-empty value becomes [UNKNOWN]. + */ +internal object FlagSelectionIgnoredReason { + + internal const val UNKNOWN = "unknown" + + // The longest known reason has 19 characters. The limit bounds the work on an oversized header. + private const val MAX_LENGTH = 32 + + private val KNOWN_REASONS = setOf( + FlagSelectionError.MALFORMED_SELECTION.reason, + FlagSelectionError.TOO_MANY_VALUES.reason, + FlagSelectionError.INVALID_KEY_PATTERN.reason, + FlagSelectionError.INVALID_TAG.reason, + "tag_load_failed" + ) + + fun sanitize(value: String): String = if (value.length <= MAX_LENGTH && value in KNOWN_REASONS) value else UNKNOWN +} + internal sealed interface FlagSelectionResult { object None : FlagSelectionResult data class Selected(val selection: CanonicalFlagSelection) : FlagSelectionResult diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index a63c5a8eb0..64037a78e5 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -1644,8 +1644,8 @@ internal class EvaluationsManagerTest { // endregion @ParameterizedTest - @ValueSource(strings = ["too_many_values", "tag_load_failed", "Some Future-Reason"]) - fun `M log the reason verbatim and install W updateEvaluationsForContext() { flag selection ignored }`( + @ValueSource(strings = ["too_many_values", "tag_load_failed", "unknown"]) + fun `M log the reason and install W updateEvaluationsForContext() { flag selection ignored }`( fakeIgnoredReason: String ) { // Given diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt index f6caeb29b7..1348a73465 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt @@ -18,6 +18,7 @@ import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import okhttp3.Call +import okhttp3.Headers import okhttp3.MediaType.Companion.toMediaType import okhttp3.Protocol import okhttp3.Request @@ -117,8 +118,10 @@ internal class PrecomputedAssignmentsDownloaderTest { } @ParameterizedTest - @ValueSource(strings = ["too_many_values", "tag_load_failed", "Some Future-Reason"]) - fun `M return the header value verbatim W readPrecomputedFlags() { selection sent, ignored header }`( + @ValueSource( + strings = ["malformed_selection", "too_many_values", "invalid_key_pattern", "invalid_tag", "tag_load_failed"] + ) + fun `M return the known reason W readPrecomputedFlags() { selection sent, ignored header }`( fakeIgnoredReason: String, forge: Forge ) { @@ -135,6 +138,32 @@ internal class PrecomputedAssignmentsDownloaderTest { assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, fakeIgnoredReason)) } + @ParameterizedTest + @ValueSource(strings = ["Some Future-Reason", "Too_Many_Values", "too_many_values\u0001", "too_many_values\u00E9"]) + fun `M return the fallback reason W readPrecomputedFlags() { selection sent, unknown ignored header }`( + fakeIgnoredReason: String, + @StringForgery fakeResponseBody: String, + @StringForgery(regex = "https://[a-z]+\\.(com|net)/[a-z]+") fakeUrl: String + ) { + // Given + val fakeRequest = Request.Builder().url(fakeUrl).build() + val fakeResponse = createSuccessfulResponse(fakeResponseBody, fakeUrl) + .newBuilder() + .headers(Headers.Builder().addUnsafeNonAscii("DD-Flag-Selection-Ignored", fakeIgnoredReason).build()) + .build() + whenever(mockRequestFactory.create(fakeEvaluationContext, fakeDatadogContext, fakeDiagnostics)) + .doReturn(fakeRequest) + whenever(mockCallFactory.newCall(fakeRequest)).doReturn(mockCall) + whenever(mockCall.execute()).doReturn(fakeResponse) + whenever(mockRequestFactory.sendsFlagSelection) doReturn true + + // When + val result = testedDownloader.readPrecomputedFlags(fakeEvaluationContext, fakeDatadogContext, fakeDiagnostics) + + // Then + assertThat(result).isEqualTo(PrecomputedAssignmentsResponse(fakeResponseBody, "unknown")) + } + @Test fun `M return the ignored reason W readPrecomputedFlags() { selection sent, lowercase header name }`( @StringForgery fakeResponseBody: String, diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt new file mode 100644 index 0000000000..1baefa9c97 --- /dev/null +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt @@ -0,0 +1,52 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.flags.internal.selection + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +internal class FlagSelectionIgnoredReasonTest { + + @ParameterizedTest + @ValueSource( + strings = ["malformed_selection", "too_many_values", "invalid_key_pattern", "invalid_tag", "tag_load_failed"] + ) + fun `M keep the reason W sanitize() { known reason }`(reason: String) { + // When / Then + assertThat(FlagSelectionIgnoredReason.sanitize(reason)).isEqualTo(reason) + } + + @ParameterizedTest + @ValueSource( + strings = [ + "Some Future-Reason", + "unknown_reason", + "Too_Many_Values", + "TAG_LOAD_FAILED", + " too_many_values", + "too_many_values\u0001", + "invalid_tag\n", + "too_many_values\u00E9", + "\uD83D\uDE00" + ] + ) + fun `M return the fallback W sanitize() { unknown reason }`(reason: String) { + // When / Then + assertThat(FlagSelectionIgnoredReason.sanitize(reason)).isEqualTo("unknown") + } + + @ParameterizedTest + @ValueSource(ints = [33, 200, 10_000]) + fun `M return the fallback W sanitize() { value longer than 32 characters }`(length: Int) { + // Given + val reason = "too_many_values".padEnd(length, '_') + + // When / Then + assertThat(FlagSelectionIgnoredReason.sanitize(reason)).isEqualTo("unknown") + } +} From 344f0f8d4a3cc22f68df704a4da941aa3d3ccede Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Wed, 28 Oct 2026 13:28:49 -0400 Subject: [PATCH 24/34] FFL-3459: Document unknown ignored reasons and the proxy header --- features/dd-sdk-android-flags/README.md | 1 + .../main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index d2b279ea7a..e5b93c77f4 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -110,6 +110,7 @@ val flagsConfig = FlagsConfiguration.Builder() - Each list accepts at most 31 values, and each value is at most 200 UTF-8 bytes. The SDK does not send a selection that is over a limit. It logs an error and loads the full flag configuration. - When the server applies the selection, evaluating a flag outside it returns the default value with `ErrorCode.FLAG_NOT_FOUND`. - The selection is fixed when you call `Flags.enable`. +- If the server ignores a selection, the SDK loads the full flag configuration and logs an error with the reason. A reason that the SDK does not know is reported as `unknown`. - With a custom flag endpoint, the proxy must forward the `DD-Flag-Selection-Ignored` response header. Otherwise, the SDK cannot report a selection that the server ignored. ## Use the Feature Flags SDK diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index 5c526f0c7b..49d5c21284 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -188,6 +188,8 @@ data class FlagsConfiguration internal constructor( * UTF-16 surrogate, is not sent, and the full flag configuration is loaded. The server may also ignore a very * large selection (`too_many_values`), in which case the full flag configuration is loaded. Servers that do * not support flag selection, or have it disabled, return the full flag configuration without a warning. + * A server reason that the SDK does not know is reported as `unknown`. With [useCustomFlagEndpoint], the proxy + * must forward the `DD-Flag-Selection-Ignored` response header for an ignored selection to be reported. * * When the server applies the selection, flags outside it are not loaded, so evaluating them returns the * default value with [ErrorCode.FLAG_NOT_FOUND]. From 99e58b4d2e74287b34e9fdd6e65b1c15287fb646 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 29 Oct 2026 16:53:23 -0400 Subject: [PATCH 25/34] FFL-3459: Rename flag presence checks to configuration checks hasFlags() and hasLoadedFlagsForContext() report whether a configuration is installed, including a valid one with zero flags. Name them hasConfiguration() and hasConfigurationForContext() so call sites read as intended. --- .../internal/evaluation/EvaluationsManager.kt | 6 +-- .../repository/DefaultFlagsRepository.kt | 4 +- .../internal/repository/FlagsRepository.kt | 4 +- .../repository/NoOpFlagsRepository.kt | 4 +- .../evaluation/EvaluationsManagerTest.kt | 24 ++++----- .../DefaultFlagsRepositoryObfuscationTest.kt | 4 +- .../repository/DefaultFlagsRepositoryTest.kt | 50 +++++++++---------- .../repository/FirstFlagsInstallationTest.kt | 2 +- 8 files changed, 49 insertions(+), 49 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index 82f16e413f..4a65ab04a0 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -166,9 +166,9 @@ internal class EvaluationsManager( { "Processing evaluation context: ${context.targetingKey}" } ) - val hadFlags = flagsRepository.hasFlags() + val hadConfiguration = flagsRepository.hasConfiguration() matchingCachedAssignments.set( - hadFlags && flagsRepository.getEvaluationContext() == context + hadConfiguration && flagsRepository.getEvaluationContext() == context ) val response = datadogContext?.let { assignmentsReader.readPrecomputedFlags(context, it, diagnostics) } val assignments = response?.let(::mapResponse) @@ -264,7 +264,7 @@ internal class EvaluationsManager( val currentState = flagStateManager.getCurrentState() if (currentState != FlagsClientState.Ready && currentState != FlagsClientState.Stale) { val hasMatchingCachedAssignments = matchingCachedAssignments.get() || - flagsRepository.hasLoadedFlagsForContext(context) + flagsRepository.hasConfigurationForContext(context) val timeoutState = if (hasMatchingCachedAssignments) { FlagsClientState.Stale } else { diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt index 0f3d4515af..aae178e5b3 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt @@ -161,12 +161,12 @@ internal class DefaultFlagsRepository( return readableState?.context } - override fun hasFlags(): Boolean { + override fun hasConfiguration(): Boolean { waitForPersistenceLoad() return readableState != null } - override fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean = + override fun hasConfigurationForContext(context: EvaluationContext): Boolean = readableState?.context == context override fun getPrecomputedFlagWithContext(key: String): Pair? { diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt index 8fbcb4d160..f5eda7aafb 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt @@ -33,9 +33,9 @@ internal interface FlagsRepository { val firstFlags: FirstFlagsLatch /** True once a configuration is installed, including a valid configuration with zero flags. */ - fun hasFlags(): Boolean + fun hasConfiguration(): Boolean /** True when the installed configuration is for [context], including a valid configuration with zero flags. */ - fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean + fun hasConfigurationForContext(context: EvaluationContext): Boolean fun getFlagsSnapshot(): Map } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/NoOpFlagsRepository.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/NoOpFlagsRepository.kt index c367dd145e..933e2b61db 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/NoOpFlagsRepository.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/NoOpFlagsRepository.kt @@ -31,9 +31,9 @@ internal class NoOpFlagsRepository : FlagsRepository { override val firstFlags = FirstFlagsLatch() - override fun hasFlags(): Boolean = false + override fun hasConfiguration(): Boolean = false - override fun hasLoadedFlagsForContext(context: EvaluationContext): Boolean = false + override fun hasConfigurationForContext(context: EvaluationContext): Boolean = false override fun getFlagsSnapshot(): Map = emptyMap() } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index 64037a78e5..d74be78c6e 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -276,7 +276,7 @@ internal class EvaluationsManagerTest { // Then inOrder(mockFlagsRepository, mockAssignmentsDownloader) { - verify(mockFlagsRepository).hasFlags() + verify(mockFlagsRepository).hasConfiguration() verify(mockAssignmentsDownloader).readPrecomputedFlags( eq(EvaluationContext(fakeTargetingKey, emptyMap())), eq(fakeDatadogContext), @@ -518,7 +518,7 @@ internal class EvaluationsManagerTest { // Given val publicContext = EvaluationContext(fakeTargetingKey, emptyMap()) - whenever(mockFlagsRepository.hasFlags()).thenReturn(false) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(false) // When evaluationsManager.updateEvaluationsForContext(publicContext) @@ -535,7 +535,7 @@ internal class EvaluationsManagerTest { // Given val publicContext = EvaluationContext(fakeTargetingKey, emptyMap()) - whenever(mockFlagsRepository.hasFlags()).thenReturn(true) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(true) whenever(mockFlagsRepository.getEvaluationContext()).thenReturn(publicContext) // When @@ -554,7 +554,7 @@ internal class EvaluationsManagerTest { val requestedContext = EvaluationContext(fakeTargetingKey, emptyMap()) val cachedContext = EvaluationContext("different-user", emptyMap()) - whenever(mockFlagsRepository.hasFlags()).thenReturn(true) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(true) whenever(mockFlagsRepository.getEvaluationContext()).thenReturn(cachedContext) whenever( mockAssignmentsDownloader.readPrecomputedFlags( @@ -609,7 +609,7 @@ internal class EvaluationsManagerTest { val publicContext = EvaluationContext(fakeTargetingKey, emptyMap()) val mockCallback = mock() - whenever(mockFlagsRepository.hasFlags()).thenReturn(false) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(false) // When evaluationsManager.updateEvaluationsForContext(publicContext, callback = mockCallback) @@ -627,7 +627,7 @@ internal class EvaluationsManagerTest { val publicContext = EvaluationContext(fakeTargetingKey, emptyMap()) val mockCallback = mock() - whenever(mockFlagsRepository.hasFlags()).thenReturn(true) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(true) // When evaluationsManager.updateEvaluationsForContext(publicContext, callback = mockCallback) @@ -736,7 +736,7 @@ internal class EvaluationsManagerTest { callbackError = error } } - whenever(mockFlagsRepository.hasFlags()).thenReturn(true) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(true) whenever(mockFlagsRepository.getEvaluationContext()).thenReturn(context) whenever( mockAssignmentsDownloader.readPrecomputedFlags( @@ -777,8 +777,8 @@ internal class EvaluationsManagerTest { onStateChanged = FlagsStateListener::onStateChanged ) ) - whenever(mockFlagsRepository.hasFlags()).thenReturn(false) - whenever(mockFlagsRepository.hasLoadedFlagsForContext(context)).thenReturn(true) + whenever(mockFlagsRepository.hasConfiguration()).thenReturn(false) + whenever(mockFlagsRepository.hasConfigurationForContext(context)).thenReturn(true) whenever( mockAssignmentsDownloader.readPrecomputedFlags( eq(context), @@ -882,7 +882,7 @@ internal class EvaluationsManagerTest { val callbackCompleted = CountDownLatch(1) var timeoutAction: (() -> Unit)? = null whenever(mockExecutorService.execute(any())).thenAnswer { null } - whenever(mockFlagsRepository.hasFlags()).thenAnswer { + whenever(mockFlagsRepository.hasConfiguration()).thenAnswer { cacheReadStarted.countDown() check(releaseCacheRead.await(1, TimeUnit.SECONDS)) false @@ -1567,7 +1567,7 @@ internal class EvaluationsManagerTest { val failure = argumentCaptor() verify(mockCallback).onFailure(failure.capture()) assertThat(failure.firstValue.message).isEqualTo("Unable to read the feature flags response.") - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasConfiguration()).isFalse() assertThat(testedRepository.getFlagsSnapshot()).isEmpty() } @@ -1592,7 +1592,7 @@ internal class EvaluationsManagerTest { ) // Configure DataStore to fire callback synchronously during DefaultFlagsRepository - // construction, so the persistence latch is counted down before hasFlags() is called. + // construction, so the persistence latch is counted down before hasConfiguration() is called. val mockDataStore = mock() whenever( mockDataStore.value( diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryObfuscationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryObfuscationTest.kt index 42b9cc67f6..1fedd7eaa7 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryObfuscationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryObfuscationTest.kt @@ -135,8 +135,8 @@ internal class DefaultFlagsRepositoryObfuscationTest { // Then val readable = !encoded || source == "android" assertThat(result?.variationValue).isEqualTo(if (readable) fakeFlag.variationValue else null) - assertThat(testedRepository.hasFlags()).isEqualTo(readable) - assertThat(testedRepository.hasLoadedFlagsForContext(fakeContext)).isEqualTo(readable) + assertThat(testedRepository.hasConfiguration()).isEqualTo(readable) + assertThat(testedRepository.hasConfigurationForContext(fakeContext)).isEqualTo(readable) assertThat(testedRepository.getEvaluationContext()).isEqualTo(if (readable) fakeContext else null) assertThat(testedRepository.getPrecomputedFlagWithContext(fakeFlagKey) != null).isEqualTo(readable) assertThat(testedRepository.getFlagsSnapshot().isEmpty()).isEqualTo(encoded) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 543e2fb8ac..3fa24ec99e 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -99,7 +99,7 @@ internal class DefaultFlagsRepositoryTest { instanceName = "default" ) - // Setup test fixtures for hasFlags tests + // Setup test fixtures for hasConfiguration tests testContext = EvaluationContext(forge.anAlphabeticalString(), emptyMap()) singleFlagMap = mapOf( @@ -211,7 +211,7 @@ internal class DefaultFlagsRepositoryTest { } @Test - fun `M not wait for persistence W hasLoadedFlagsForContext() { persistence is loading }`(forge: Forge) { + fun `M not wait for persistence W hasConfigurationForContext() { persistence is loading }`(forge: Forge) { // Given doAnswer { null }.whenever(mockDataStore).value( key = any(), @@ -230,7 +230,7 @@ internal class DefaultFlagsRepositoryTest { val completed = CountDownLatch(1) val context = EvaluationContext(forge.anAlphabeticalString(), emptyMap()) val readThread = Thread { - result.set(testedRepository.hasLoadedFlagsForContext(context)) + result.set(testedRepository.hasConfigurationForContext(context)) completed.countDown() }.apply { isDaemon = true } @@ -421,7 +421,7 @@ internal class DefaultFlagsRepositoryTest { // Then assertThat(testedRepository.getEvaluationContext()).isEqualTo(context) assertThat(testedRepository.getFlagsSnapshot()).isEmpty() - assertThat(testedRepository.hasFlags()).isTrue() + assertThat(testedRepository.hasConfiguration()).isTrue() verify(mockDataStore, never()).setValue( key = any(), data = any(), @@ -488,7 +488,7 @@ internal class DefaultFlagsRepositoryTest { // Then assertThat(testedRepository.getEvaluationContext()).isNull() - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasConfiguration()).isFalse() } @Test @@ -506,7 +506,7 @@ internal class DefaultFlagsRepositoryTest { // Then assertThat(testedRepository.getEvaluationContext()).isNull() - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasConfiguration()).isFalse() } @Test @@ -689,24 +689,24 @@ internal class DefaultFlagsRepositoryTest { } @Test - fun `M return true W hasLoadedFlagsForContext() { empty flags map for the same context }`(forge: Forge) { + fun `M return true W hasConfigurationForContext() { empty flags map for the same context }`(forge: Forge) { // Given val context = forge.getForgery() testedRepository.setFlagsAndContext(context, emptyMap()) // When / Then - assertThat(testedRepository.hasLoadedFlagsForContext(context)).isTrue() + assertThat(testedRepository.hasConfigurationForContext(context)).isTrue() } @Test - fun `M return false W hasLoadedFlagsForContext() { different context }`(forge: Forge) { + fun `M return false W hasConfigurationForContext() { different context }`(forge: Forge) { // Given val context = forge.getForgery() testedRepository.setFlagsAndContext(context, singleFlagMap) // When / Then assertThat( - testedRepository.hasLoadedFlagsForContext(context.copy(targetingKey = context.targetingKey + "-other")) + testedRepository.hasConfigurationForContext(context.copy(targetingKey = context.targetingKey + "-other")) ).isFalse() } @@ -731,16 +731,16 @@ internal class DefaultFlagsRepositoryTest { return callback } - // region hasFlags + // region hasConfiguration @Test - fun `M return false W hasFlags() { no state set }`() { + fun `M return false W hasConfiguration() { no state set }`() { // When + Then - assertThat(testedRepository.hasFlags()).isFalse() + assertThat(testedRepository.hasConfiguration()).isFalse() } @Test - fun `M return true W hasFlags() { empty flags map }`(forge: Forge) { + fun `M return true W hasConfiguration() { empty flags map }`(forge: Forge) { // Given testedRepository.setFlagsAndContext( EvaluationContext(forge.anAlphabeticalString(), emptyMap()), @@ -748,29 +748,29 @@ internal class DefaultFlagsRepositoryTest { ) // When + Then - assertThat(testedRepository.hasFlags()).isTrue() + assertThat(testedRepository.hasConfiguration()).isTrue() } @Test - fun `M return true W hasFlags() { single flag }`() { + fun `M return true W hasConfiguration() { single flag }`() { // Given testedRepository.setFlagsAndContext(testContext, singleFlagMap) // When + Then - assertThat(testedRepository.hasFlags()).isTrue() + assertThat(testedRepository.hasConfiguration()).isTrue() } @Test - fun `M return true W hasFlags() { multiple flags }`() { + fun `M return true W hasConfiguration() { multiple flags }`() { // Given testedRepository.setFlagsAndContext(testContext, multipleFlagsMap) // When + Then - assertThat(testedRepository.hasFlags()).isTrue() + assertThat(testedRepository.hasConfiguration()).isTrue() } @Test - fun `M return true W hasFlags() { persistence loads with non-empty flags }`() { + fun `M return true W hasConfiguration() { persistence loads with non-empty flags }`() { // Given val persistedEntry = FlagsStateEntry( flags = singleFlagMap, @@ -796,11 +796,11 @@ internal class DefaultFlagsRepositoryTest { ) // When + Then - assertThat(repository.hasFlags()).isTrue() + assertThat(repository.hasConfiguration()).isTrue() } @Test - fun `M return false W hasFlags() { persistence loads with no data }`() { + fun `M return false W hasConfiguration() { persistence loads with no data }`() { // Given doAnswer { it.getArgument>(2) @@ -820,11 +820,11 @@ internal class DefaultFlagsRepositoryTest { ) // When + Then - assertThat(repository.hasFlags()).isFalse() + assertThat(repository.hasConfiguration()).isFalse() } @Test - fun `M return false W hasFlags() { persistence callback never fires within timeout }`() { + fun `M return false W hasConfiguration() { persistence callback never fires within timeout }`() { // Given doAnswer { // Never call the callback @@ -844,7 +844,7 @@ internal class DefaultFlagsRepositoryTest { ) // When - val result = timeoutRepository.hasFlags() + val result = timeoutRepository.hasConfiguration() // Then assertThat(result).isFalse() diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt index b9f4ef0839..858d90ae91 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt @@ -208,7 +208,7 @@ internal class FirstFlagsInstallationTest { assertThat(tested.getPrecomputedFlag("unknown")).isNull() assertThat(tested.getEvaluationContext()).isNull() assertThat(tested.getFlagsSnapshot()).isEmpty() - assertThat(tested.hasFlags()).isFalse() + assertThat(tested.hasConfiguration()).isFalse() assertThat(tested.getPrecomputedFlagWithContext("unknown")).isNull() } if (missing) disk.onSuccess(null) else disk.onFailure() From e47cbe7ae0271b4946901ef28843b97569819c3a Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 29 Oct 2026 16:53:58 -0400 Subject: [PATCH 26/34] FFL-3459: Share the empty flags response in EvaluationsManagerTest --- .../evaluation/EvaluationsManagerTest.kt | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index d74be78c6e..4a2663fa37 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -679,7 +679,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -831,7 +831,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val stateManager = FlagsStateManager( DDCoreStateHolder.create( @@ -998,7 +998,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -1047,7 +1047,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -1080,7 +1080,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenAnswer { checkNotNull(timeoutAction).invoke() PrecomputedAssignments(emptyMap()) @@ -1115,7 +1115,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = createManager( initializationTimeoutMs = 2_500L, @@ -1146,7 +1146,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = createManager( initializationTimeoutMs = null, @@ -1207,7 +1207,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val stateManager = FlagsStateManager( DDCoreStateHolder.create( @@ -1724,7 +1724,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null)) + .thenReturn(EMPTY_FLAGS_RESPONSE) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) // When @@ -1775,7 +1775,7 @@ internal class EvaluationsManagerTest { isA() ) ) - .thenReturn(PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null), null) + .thenReturn(EMPTY_FLAGS_RESPONSE, null) whenever(mockPrecomputeMapper.map(EMPTY_FLAGS_RESPONSE_JSON)).thenReturn(PrecomputedAssignments(emptyMap())) val testedManager = EvaluationsManager( sdkCore = mockSdkCore, @@ -1853,5 +1853,6 @@ internal class EvaluationsManagerTest { }}} """.trimIndent() private const val EMPTY_FLAGS_RESPONSE_JSON = "{\"data\": {\"attributes\": {\"flags\": {}}}}" + private val EMPTY_FLAGS_RESPONSE = PrecomputedAssignmentsResponse(EMPTY_FLAGS_RESPONSE_JSON, null) } } From 9c970b18dbb241fbabc6e816700a797cefbe3faa Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 29 Oct 2026 17:03:52 -0400 Subject: [PATCH 27/34] FFL-3459: Report an ignored selection only for an accepted response Log the DD-Flag-Selection-Ignored reason and send its telemetry only after the response maps to a configuration and passes the obfuscation filter, just before it is installed. A body that fails to parse, or an encoded configuration this source cannot use, no longer reports a reason for a configuration that was never loaded. --- .../internal/evaluation/EvaluationsManager.kt | 9 +-- .../evaluation/EvaluationsManagerTest.kt | 62 ++++++++++++------- 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index 4a65ab04a0..845d8ac85d 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -22,7 +22,6 @@ import com.datadog.android.flags.internal.model.PrecomputedAssignmentsRequest.Sd import com.datadog.android.flags.internal.model.PrecomputedAssignmentsRequest.Trigger import com.datadog.android.flags.internal.net.NetworkRequestFailedException import com.datadog.android.flags.internal.net.PrecomputedAssignmentsReader -import com.datadog.android.flags.internal.net.PrecomputedAssignmentsResponse import com.datadog.android.flags.internal.repository.FlagsRepository import com.datadog.android.flags.internal.repository.net.PrecomputeMapper import com.datadog.android.flags.internal.selection.logIgnoredFlagSelection @@ -171,9 +170,10 @@ internal class EvaluationsManager( hadConfiguration && flagsRepository.getEvaluationContext() == context ) val response = datadogContext?.let { assignmentsReader.readPrecomputedFlags(context, it, diagnostics) } - val assignments = response?.let(::mapResponse) + val assignments = response?.let { precomputeMapper.map(it.body) } ?.takeIf { it.obfuscation == null || supportsObfuscation } if (assignments != null) { + response.flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } installFlags(context, assignments, initializationCompletion, callback) } else { val message = if (response == null) NETWORK_REQUEST_FAILED_MESSAGE else INVALID_RESPONSE_MESSAGE @@ -218,11 +218,6 @@ internal class EvaluationsManager( } } - private fun mapResponse(response: PrecomputedAssignmentsResponse): PrecomputedAssignments? { - response.flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } - return precomputeMapper.map(response.body) - } - private fun installFlags( context: EvaluationContext, assignments: PrecomputedAssignments, diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index 4a2663fa37..e2a04a2821 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -18,6 +18,7 @@ import com.datadog.android.flags.EvaluationContextCallback import com.datadog.android.flags.FlagsInitializationTimeoutException import com.datadog.android.flags.FlagsStateListener import com.datadog.android.flags.internal.FlagsStateManager +import com.datadog.android.flags.internal.model.FlagKeyObfuscation import com.datadog.android.flags.internal.model.FlagsStateEntry import com.datadog.android.flags.internal.model.PrecomputedAssignments import com.datadog.android.flags.internal.model.PrecomputedAssignmentsRequest.Diagnostics @@ -1680,7 +1681,7 @@ internal class EvaluationsManagerTest { } @Test - fun `M log the ignored reason and not install W updateEvaluationsForContext() { ignored reason, invalid body }`( + fun `M not log the ignored reason W updateEvaluationsForContext() { ignored reason, invalid body }`( @StringForgery fakeInvalidBody: String ) { // Given @@ -1698,17 +1699,32 @@ internal class EvaluationsManagerTest { evaluationsManager.updateEvaluationsForContext(context) // Then - mockInternalLogger.verifyLog( - InternalLogger.Level.ERROR, - InternalLogger.Target.USER, - "flagSelection was ignored by the server (too_many_values). Loaded the full flag configuration." - ) - mockInternalLogger.verifyLog( - InternalLogger.Level.DEBUG, - InternalLogger.Target.TELEMETRY, - "Flag selection ignored", - additionalProperties = mapOf("reason" to "too_many_values") + assertThat(capturedLogMessages()).noneMatch { "flagSelection" in it || "Flag selection" in it } + verify(mockFlagsRepository, never()).setFlagsAndContext(any(), any(), anyOrNull(), any()) + verify(mockFlagsStateManager).updateState(isA()) + } + + @Test + fun `M not log the ignored reason W updateEvaluationsForContext() { ignored reason, unsupported encoding }`() { + // Given + fakeDatadogContext = fakeDatadogContext.copy(source = "react-native") + val context = EvaluationContext(fakeTargetingKey, emptyMap()) + whenever( + mockAssignmentsDownloader.readPrecomputedFlags( + eq(context), + eq(fakeDatadogContext), + isA() + ) + ).thenReturn(PrecomputedAssignmentsResponse(ENCODED_RESPONSE_JSON, "too_many_values")) + whenever(mockPrecomputeMapper.map(ENCODED_RESPONSE_JSON)).thenReturn( + PrecomputedAssignments(emptyMap(), FlagKeyObfuscation.fromSalt("000102030405060708090a0b0c0d0e0f")) ) + + // When + evaluationsManager.updateEvaluationsForContext(context) + + // Then + assertThat(capturedLogMessages()).noneMatch { "flagSelection" in it || "Flag selection" in it } verify(mockFlagsRepository, never()).setFlagsAndContext(any(), any(), anyOrNull(), any()) verify(mockFlagsStateManager).updateState(isA()) } @@ -1731,16 +1747,7 @@ internal class EvaluationsManagerTest { evaluationsManager.updateEvaluationsForContext(context) // Then - val messages = argumentCaptor<() -> String>() - verify(mockInternalLogger, atLeast(0)).log( - any(), - any(), - messages.capture(), - anyOrNull(), - any(), - anyOrNull() - ) - assertThat(messages.allValues.map { it() }).noneMatch { "flagSelection" in it || "Flag selection" in it } + assertThat(capturedLogMessages()).noneMatch { "flagSelection" in it || "Flag selection" in it } verify(mockFlagsRepository).setFlagsAndContext(eq(context), eq(emptyMap()), anyOrNull(), any()) } @@ -1840,6 +1847,19 @@ internal class EvaluationsManagerTest { initializationTimeoutScheduler = scheduler ) + private fun capturedLogMessages(): List { + val messages = argumentCaptor<() -> String>() + verify(mockInternalLogger, atLeast(0)).log( + any(), + any(), + messages.capture(), + anyOrNull(), + any(), + anyOrNull() + ) + return messages.allValues.map { it() } + } + companion object { private val ENCODED_RESPONSE_JSON = """ {"data":{"attributes":{ From 75d6d26adfd3f52298ec218b10dea3a113dff1aa Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 29 Oct 2026 17:03:55 -0400 Subject: [PATCH 28/34] FFL-3459: Test that interior Unicode whitespace is kept --- .../FlagSelectionCanonicalizerTest.kt | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt index 5c9f895ef8..5026efe905 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionCanonicalizerTest.kt @@ -61,10 +61,8 @@ internal class FlagSelectionCanonicalizerTest { @Test fun `M trim the Unicode White_Space set W canonicalize()`() { // Given - val whiteSpace = "\t\n\u000B\u000C\r \u0085\u00A0\u1680" + - "\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u2028\u2029\u202F\u205F\u3000" val selection = FlagSelection( - tags = listOf("${whiteSpace}mobile$whiteSpace"), + tags = listOf("${UNICODE_WHITE_SPACE}mobile$UNICODE_WHITE_SPACE"), keyPatterns = listOf("\uFEFFcheckout\uFEFF", "\u001Cpayment\u001C") ) @@ -82,6 +80,23 @@ internal class FlagSelectionCanonicalizerTest { ) } + @Test + fun `M keep interior Unicode White_Space W canonicalize()`() { + assertThat(UNICODE_WHITE_SPACE).hasSize(25) + UNICODE_WHITE_SPACE.forEach { whiteSpace -> + // Given + val selection = tags("${whiteSpace}a${whiteSpace}b$whiteSpace") + + // When + val result = FlagSelectionCanonicalizer.canonicalize(selection) + + // Then + assertThat(result) + .describedAs("U+%04X", whiteSpace.code) + .isEqualTo(selected(listOf("a${whiteSpace}b"), emptyList())) + } + } + @Test fun `M drop empty values and dedupe after trimming W canonicalize()`() { // Given @@ -169,6 +184,10 @@ internal class FlagSelectionCanonicalizerTest { companion object { private const val GRINNING_FACE = "\uD83D\uDE00" + // https://www.unicode.org/Public/UCD/latest/ucd/PropList.txt lists 25 White_Space code points. + private const val UNICODE_WHITE_SPACE = "\t\n\u000B\u000C\r \u0085\u00A0\u1680" + + "\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u2028\u2029\u202F\u205F\u3000" + private fun numbered(prefix: String, count: Int) = List(count) { "$prefix%02d".format(Locale.US, it + 1) } private fun selected(tags: List, keyPatterns: List) = From 28e82cdfd612300dcf6a7e2d5d3310f161b1a0e8 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Thu, 29 Oct 2026 17:03:55 -0400 Subject: [PATCH 29/34] FFL-3459: Document that the selection applies to every client and cache --- features/dd-sdk-android-flags/README.md | 2 +- .../kotlin/com/datadog/android/flags/FlagsConfiguration.kt | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index e5b93c77f4..d563539193 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -109,7 +109,7 @@ val flagsConfig = FlagsConfiguration.Builder() - Tags match exactly and are case-sensitive. A key pattern matches the whole flag key, and only `*` is a wildcard. - Each list accepts at most 31 values, and each value is at most 200 UTF-8 bytes. The SDK does not send a selection that is over a limit. It logs an error and loads the full flag configuration. - When the server applies the selection, evaluating a flag outside it returns the default value with `ErrorCode.FLAG_NOT_FOUND`. -- The selection is fixed when you call `Flags.enable`. +- The selection is fixed when the feature is enabled and applies to every `FlagsClient`. Flags cached under a different selection are not loaded at startup; until the next successful fetch, a client starts without cached flags. - If the server ignores a selection, the SDK loads the full flag configuration and logs an error with the reason. A reason that the SDK does not know is reported as `unknown`. - With a custom flag endpoint, the proxy must forward the `DD-Flag-Selection-Ignored` response header. Otherwise, the SDK cannot report a selection that the server ignored. diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt index 49d5c21284..3ec7b60909 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt @@ -194,8 +194,9 @@ data class FlagsConfiguration internal constructor( * When the server applies the selection, flags outside it are not loaded, so evaluating them returns the * default value with [ErrorCode.FLAG_NOT_FOUND]. * - * The selection is fixed when the Flags feature is enabled. Calling this method again replaces the - * previous selection. + * The selection is fixed when the feature is enabled and applies to every [FlagsClient]. Flags cached under a + * different selection are not loaded at startup; until the next successful fetch, a client starts without + * cached flags. Calling this method again replaces the previous selection. * * @param tags The flag tags to load, or null for none. * @param keyPatterns The flag key patterns to load, or null for none. From 00787acfd7d3fbce6b2acd80d3836d9c5d72f5c9 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Fri, 30 Oct 2026 13:34:04 -0400 Subject: [PATCH 30/34] FFL-3459: Send flag selection telemetry once per process The dropped and ignored selection telemetry events now use onlyOnce, so frequent fetches cannot flood telemetry. The USER error log is unchanged and still reports every ignored response. --- .../flags/internal/selection/FlagSelectionLogging.kt | 8 +++++--- .../datadog/android/flags/internal/FlagsFeatureTest.kt | 1 + .../flags/internal/evaluation/EvaluationsManagerTest.kt | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt index 9c84618cb5..a8b9b4efb2 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionLogging.kt @@ -8,7 +8,7 @@ package com.datadog.android.flags.internal.selection import com.datadog.android.api.InternalLogger -// Telemetry carries only the reason, never selection values. +// Telemetry carries only the reason, never selection values, and is sent once per process. private const val TELEMETRY_REASON_KEY = "reason" private const val FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE = "Flag selection dropped" @@ -27,7 +27,8 @@ internal fun InternalLogger.logDroppedFlagSelection(error: FlagSelectionError) { InternalLogger.Level.DEBUG, InternalLogger.Target.TELEMETRY, { FLAG_SELECTION_DROPPED_TELEMETRY_MESSAGE }, - additionalProperties = mapOf(TELEMETRY_REASON_KEY to error.reason) + additionalProperties = mapOf(TELEMETRY_REASON_KEY to error.reason), + onlyOnce = true ) } @@ -41,6 +42,7 @@ internal fun InternalLogger.logIgnoredFlagSelection(reason: String) { InternalLogger.Level.DEBUG, InternalLogger.Target.TELEMETRY, { FLAG_SELECTION_IGNORED_TELEMETRY_MESSAGE }, - additionalProperties = mapOf(TELEMETRY_REASON_KEY to reason) + additionalProperties = mapOf(TELEMETRY_REASON_KEY to reason), + onlyOnce = true ) } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt index 07193a0660..6f16c2a9ce 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt @@ -261,6 +261,7 @@ internal class FlagsFeatureTest { InternalLogger.Level.DEBUG, InternalLogger.Target.TELEMETRY, "Flag selection dropped", + onlyOnce = true, additionalProperties = mapOf("reason" to "too_many_values") ) } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index 05a1b7dbff..f5a37d43ff 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -1954,6 +1954,7 @@ internal class EvaluationsManagerTest { InternalLogger.Level.DEBUG, InternalLogger.Target.TELEMETRY, "Flag selection ignored", + onlyOnce = true, additionalProperties = mapOf("reason" to fakeIgnoredReason) ) verify(mockFlagsRepository).installFlagsAndContext(eq(context), eq(emptyMap()), anyOrNull()) From 161f308daaefb187090c9b4758aeaa64ad38a596 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Fri, 30 Oct 2026 14:02:28 -0400 Subject: [PATCH 31/34] FFL-3459: Report an ignored selection only when its flags are installed With the experimentation opt-out, a newer context can supersede an accepted response, which is then not installed. Log the ignored reason after installFlags confirms the install, so the message never claims a configuration that was not loaded. --- .../internal/evaluation/EvaluationsManager.kt | 17 ++++++++++--- .../evaluation/EvaluationsManagerTest.kt | 25 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index be4fb1aeb6..92de400034 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -209,8 +209,13 @@ internal class EvaluationsManager( } ?.takeIf { it.obfuscation == null || supportsObfuscation } if (assignments != null) { - response.flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } - installFlags(request, assignments, initializationCompletion, callback) + installFlags( + request, + assignments, + response.flagSelectionIgnoredReason, + initializationCompletion, + callback + ) } else { handleRequestFailure( request, @@ -268,11 +273,12 @@ internal class EvaluationsManager( private fun installFlags( request: ExperimentationOptOut.Request, assignments: PrecomputedAssignments, + flagSelectionIgnoredReason: String?, initializationCompletion: InitializationCompletion?, callback: EvaluationContextCallback? ) { val context = request.context - synchronized(initializationTerminalLock) { + val installed = synchronized(initializationTerminalLock) { val installed = experimentationOptOut.ifCurrent(request) { val installation = flagsRepository.installFlagsAndContext( context, @@ -294,8 +300,13 @@ internal class EvaluationsManager( FlagsContextSupersededException(SUPERSEDED_MESSAGE) ) } + installed } effects.drain() + // A superseded response is not installed, so its ignored reason would describe flags that never loaded. + if (installed) { + flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } + } internalLogger.log( InternalLogger.Level.DEBUG, InternalLogger.Target.MAINTAINER, diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index f5a37d43ff..762d2a38ca 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -2010,6 +2010,31 @@ internal class EvaluationsManagerTest { verify(mockFlagsStateManager).updateState(isA()) } + @ParameterizedTest + @EnumSource(ExperimentationOptOutMode::class, names = ["IMMEDIATE", "FUTURE_EXPERIMENTS_ONLY"]) + fun `M not log the ignored reason W updateEvaluationsForContext() { ignored reason, superseded request }`( + mode: ExperimentationOptOutMode, + forge: Forge + ) { + // Given + val tasks = mutableListOf() + val testedManager = queuedManager(mode, tasks) + val first = EvaluationContext(forge.anAlphabeticalString()) + val firstCallback = mock() + whenever(mockAssignmentsDownloader.readPrecomputedFlags(eq(first), any(), any())) doReturn + PrecomputedAssignmentsResponse("response", "too_many_values") + whenever(mockPrecomputeMapper.map(any(), anyOrNull())) doReturn PrecomputedAssignments(emptyMap(), null) + testedManager.updateEvaluationsForContext(first, firstCallback) + testedManager.updateEvaluationsForContext(EvaluationContext(forge.anAlphabeticalString())) + + // When + tasks[0].run() + + // Then + verify(firstCallback).onFailure(isA()) + assertThat(capturedLogMessages()).noneMatch { "flagSelection" in it || "Flag selection" in it } + } + @Test fun `M not log a flag selection message W updateEvaluationsForContext() { no ignored reason }`() { // Given From 8db220a97b39d25f797fe840014c6ef6f04fd515 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Fri, 30 Oct 2026 14:02:30 -0400 Subject: [PATCH 32/34] FFL-3459: Remove an inline comment from the opt-out key test --- .../flags/internal/repository/DefaultFlagsRepositoryTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt index 1f6f6dae27..d6bee059e3 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt @@ -703,7 +703,6 @@ internal class DefaultFlagsRepositoryTest { callback = anyOrNull(), serializer = any() ) - // Builds without flag selection read only the unselected key of their mode. verify(mockDataStore, never()).setValue( key = eq(siblingKey), data = any(), From 9ce29b14f4c1b0c9672810441440ebceb7d2bd8e Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Mon, 2 Nov 2026 10:23:29 -0500 Subject: [PATCH 33/34] FFL-3459: Clarify the flag selection scope in the README Say that the flag selection is what every FlagsClient shares, and use a function reference for the empty ignored-reason header check. --- features/dd-sdk-android-flags/README.md | 2 +- .../flags/internal/net/PrecomputedAssignmentsDownloader.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index cfa42466ef..8cebf37890 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -109,7 +109,7 @@ val flagsConfig = FlagsConfiguration.Builder() - Tags match exactly and are case-sensitive. A key pattern matches the whole flag key, and only `*` is a wildcard. - Each list accepts at most 31 values, and each value is at most 200 UTF-8 bytes. The SDK does not send a selection that is over a limit. It logs an error and loads the full flag configuration. - When the server applies the selection, evaluating a flag outside it returns the default value with `ErrorCode.FLAG_NOT_FOUND`. -- The selection is fixed when the feature is enabled and applies to every `FlagsClient`. Flags cached under a different selection are not loaded at startup; until the next successful fetch, a client starts without cached flags. +- The flag selection is fixed when the Flags feature is enabled and applies to every `FlagsClient`. Flags cached under a different selection are not loaded at startup; until the next successful fetch, a client starts without cached flags. - If the server ignores a selection, the SDK loads the full flag configuration and logs an error with the reason. A reason that the SDK does not know is reported as `unknown`. - With a custom flag endpoint, the proxy must forward the `DD-Flag-Selection-Ignored` response header. Otherwise, the SDK cannot report a selection that the server ignored. diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt index 476a753b4e..578222adc1 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloader.kt @@ -60,7 +60,7 @@ internal class PrecomputedAssignmentsDownloader( body = body, flagSelectionIgnoredReason = if (requestFactory.sendsFlagSelection) { response.header(HEADER_FLAG_SELECTION_IGNORED) - ?.takeIf { it.isNotEmpty() } + ?.takeIf(String::isNotEmpty) ?.let(FlagSelectionIgnoredReason::sanitize) } else { null From 198c6848c9c81c1a6b816e43af6aef4d3ea9e294 Mon Sep 17 00:00:00 2001 From: Mark Cai Date: Tue, 3 Nov 2026 13:25:41 -0500 Subject: [PATCH 34/34] FFL-3459: Clarify install flow logging and model server ignored reasons --- .../internal/evaluation/EvaluationsManager.kt | 51 +++++++++++-------- .../persistence/FlagsPersistenceManager.kt | 3 +- .../flags/internal/selection/FlagSelection.kt | 28 ++++++---- .../evaluation/EvaluationsManagerTest.kt | 5 +- .../FlagSelectionIgnoredReasonTest.kt | 8 +++ 5 files changed, 63 insertions(+), 32 deletions(-) diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt index 92de400034..3b9f3d22b9 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt @@ -96,6 +96,7 @@ private class InitializationCompletion(private var callback: EvaluationContextCa * @param experimentationOptOut validates contexts and prevents superseded installations * @param callbackExecutor delivers customer callbacks outside state delivery and network operations */ +@Suppress("TooManyFunctions") // Each step of the fetch, install and completion lifecycle is a separate function. internal class EvaluationsManager( private val sdkCore: FeatureSdkCore, private val executorService: ExecutorService, @@ -279,39 +280,49 @@ internal class EvaluationsManager( ) { val context = request.context val installed = synchronized(initializationTerminalLock) { - val installed = experimentationOptOut.ifCurrent(request) { - val installation = flagsRepository.installFlagsAndContext( - context, - assignments.flags, - assignments.obfuscation - ) - val result = takeCompletion(initializationCompletion, callback) - currentState = FlagsClientState.Ready - effects.enqueue { - installation.complete { - effects.notifySafely { flagStateManager.updateState(FlagsClientState.Ready) } - complete(result) - } - } + val isCurrent = experimentationOptOut.ifCurrent(request) { + installCurrentFlags(context, assignments, initializationCompletion, callback) } - if (!installed) { + if (!isCurrent) { enqueueCompletion( takeCompletion(initializationCompletion, callback), FlagsContextSupersededException(SUPERSEDED_MESSAGE) ) } - installed + isCurrent } effects.drain() // A superseded response is not installed, so its ignored reason would describe flags that never loaded. if (installed) { flagSelectionIgnoredReason?.let { internalLogger.logIgnoredFlagSelection(it) } + internalLogger.log( + InternalLogger.Level.DEBUG, + InternalLogger.Target.MAINTAINER, + { "Successfully processed context ${context.targetingKey} with ${assignments.flags.size} flags" } + ) } - internalLogger.log( - InternalLogger.Level.DEBUG, - InternalLogger.Target.MAINTAINER, - { "Successfully processed context ${context.targetingKey} with ${assignments.flags.size} flags" } + } + + // Callers hold initializationTerminalLock; the Ready notification and callback run later from effects.drain(). + private fun installCurrentFlags( + context: EvaluationContext, + assignments: PrecomputedAssignments, + initializationCompletion: InitializationCompletion?, + callback: EvaluationContextCallback? + ) { + val installation = flagsRepository.installFlagsAndContext( + context, + assignments.flags, + assignments.obfuscation ) + val result = takeCompletion(initializationCompletion, callback) + currentState = FlagsClientState.Ready + effects.enqueue { + installation.complete { + effects.notifySafely { flagStateManager.updateState(FlagsClientState.Ready) } + complete(result) + } + } } private fun startInitializationTimeout( diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt index 39fcbe265b..766b6913a3 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/persistence/FlagsPersistenceManager.kt @@ -34,10 +34,11 @@ internal class FlagsPersistenceManager( private val unselectedStateKey = stateKey(instanceName, experimentationOptOutMode, selected = false) private val selectedStateKey = stateKey(instanceName, experimentationOptOutMode, selected = true) private val flagsStateKey: String = if (flagSelection == null) unselectedStateKey else selectedStateKey + private val staleStateKey: String = if (flagSelection == null) selectedStateKey else unselectedStateKey init { // One persisted state per instance: changing to or from a selection discards the other key's state. - dataStore.removeValue(if (flagSelection == null) selectedStateKey else unselectedStateKey) + dataStore.removeValue(staleStateKey) loadFlagsState(onStateLoaded) } diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt index eac8cfbc10..aaa9dddd59 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/selection/FlagSelection.kt @@ -45,9 +45,22 @@ internal enum class FlagSelectionError(val reason: String, val limit: String) { ) } +/** + * Reasons the server reports in the `DD-Flag-Selection-Ignored` response header. + * The server validates with the same rules as the SDK, so validation failures reuse the [FlagSelectionError] codes. + * [TAG_LOAD_FAILED] is reported only by the server, so it is not a [FlagSelectionError]. + */ +internal enum class ServerIgnoredReason(val reason: String) { + MALFORMED_SELECTION(FlagSelectionError.MALFORMED_SELECTION.reason), + TOO_MANY_VALUES(FlagSelectionError.TOO_MANY_VALUES.reason), + INVALID_KEY_PATTERN(FlagSelectionError.INVALID_KEY_PATTERN.reason), + INVALID_TAG(FlagSelectionError.INVALID_TAG.reason), + TAG_LOAD_FAILED("tag_load_failed") +} + /** * Sanitizes the `DD-Flag-Selection-Ignored` response header, so a raw header value never reaches logs or telemetry. - * A known reason is kept, and any other non-empty value becomes [UNKNOWN]. + * A [ServerIgnoredReason] is kept, and any other non-empty value becomes [UNKNOWN]. */ internal object FlagSelectionIgnoredReason { @@ -56,15 +69,10 @@ internal object FlagSelectionIgnoredReason { // The longest known reason has 19 characters. The limit bounds the work on an oversized header. private const val MAX_LENGTH = 32 - private val KNOWN_REASONS = setOf( - FlagSelectionError.MALFORMED_SELECTION.reason, - FlagSelectionError.TOO_MANY_VALUES.reason, - FlagSelectionError.INVALID_KEY_PATTERN.reason, - FlagSelectionError.INVALID_TAG.reason, - "tag_load_failed" - ) - - fun sanitize(value: String): String = if (value.length <= MAX_LENGTH && value in KNOWN_REASONS) value else UNKNOWN + fun sanitize(value: String): String { + if (value.length > MAX_LENGTH) return UNKNOWN + return ServerIgnoredReason.entries.firstOrNull { it.reason == value }?.reason ?: UNKNOWN + } } internal sealed interface FlagSelectionResult { diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt index 762d2a38ca..d9a7e3f10d 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManagerTest.kt @@ -1957,6 +1957,7 @@ internal class EvaluationsManagerTest { onlyOnce = true, additionalProperties = mapOf("reason" to fakeIgnoredReason) ) + assertThat(capturedLogMessages()).contains("Successfully processed context $fakeTargetingKey with 0 flags") verify(mockFlagsRepository).installFlagsAndContext(eq(context), eq(emptyMap()), anyOrNull()) verify(mockFlagsStateManager).updateState(FlagsClientState.Ready) } @@ -2032,7 +2033,9 @@ internal class EvaluationsManagerTest { // Then verify(firstCallback).onFailure(isA()) - assertThat(capturedLogMessages()).noneMatch { "flagSelection" in it || "Flag selection" in it } + assertThat(capturedLogMessages()).noneMatch { + "flagSelection" in it || "Flag selection" in it || "Successfully processed" in it + } } @Test diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt index 1baefa9c97..4a51f18fbf 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/selection/FlagSelectionIgnoredReasonTest.kt @@ -8,6 +8,7 @@ package com.datadog.android.flags.internal.selection import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.EnumSource import org.junit.jupiter.params.provider.ValueSource internal class FlagSelectionIgnoredReasonTest { @@ -21,6 +22,13 @@ internal class FlagSelectionIgnoredReasonTest { assertThat(FlagSelectionIgnoredReason.sanitize(reason)).isEqualTo(reason) } + @ParameterizedTest + @EnumSource(ServerIgnoredReason::class) + fun `M keep the reason W sanitize() { any server ignored reason }`(reason: ServerIgnoredReason) { + // When / Then + assertThat(FlagSelectionIgnoredReason.sanitize(reason.reason)).isEqualTo(reason.reason) + } + @ParameterizedTest @ValueSource( strings = [