Skip to content

Commit 16b0c01

Browse files
committed
fix unsafe case, keep original default value and add tests
1 parent b0c4d34 commit 16b0c01

File tree

4 files changed

+80
-6
lines changed

4 files changed

+80
-6
lines changed

features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/DatadogFlagsClient.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,15 @@ internal class DatadogFlagsClient(
141141
* @return The map value of the flag, or the default value if unavailable.
142142
*/
143143
override fun resolveStructureValue(flagKey: String, defaultValue: Map<String, Any?>): Map<String, Any?> {
144-
// Convert Map to JSONObject for resolution, then convert back to Map
145144
val jsonDefault = defaultValue.toJSONObject()
146145
val jsonResult = resolveValue(flagKey, jsonDefault)
147-
return jsonResult.toMap()
146+
147+
// Preserve original default value on failure (avoids round-trip conversion)
148+
return if (jsonResult === jsonDefault) {
149+
defaultValue
150+
} else {
151+
jsonResult.toMap()
152+
}
148153
}
149154

150155
/**

features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/JsonExtensions.kt

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ internal fun List<Any?>.toJSONArray(): JSONArray {
119119
* Converts a Kotlin value to a JSON-compatible value.
120120
*
121121
* Recursively handles nested structures:
122-
* - Map<*, *> → JSONObject
122+
* - Map<*, *> → JSONObject (non-String keys converted via toString())
123123
* - List<*> → JSONArray
124124
* - null → JSONObject.NULL
125125
* - Primitives → unchanged
@@ -129,8 +129,16 @@ internal fun List<Any?>.toJSONArray(): JSONArray {
129129
*/
130130
private fun convertToJsonValue(value: Any?): Any = when (value) {
131131
null -> JSONObject.NULL
132-
is Map<*, *> -> (value as Map<String, Any?>).toJSONObject()
133-
is List<*> -> (value as List<Any?>).toJSONArray()
132+
is Map<*, *> -> {
133+
// Convert keys to String (supports non-String keys via toString())
134+
@Suppress("UNCHECKED_CAST")
135+
val stringMap = value.entries.associate { (k, v) -> k.toString() to v }
136+
stringMap.toJSONObject()
137+
}
138+
is List<*> -> {
139+
@Suppress("UNCHECKED_CAST")
140+
(value as List<Any?>).toJSONArray()
141+
}
134142
else -> value // Primitives: String, Int, Long, Double, Boolean
135143
}
136144

features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,8 @@ internal class DatadogFlagsClientTest {
646646
}
647647
val fakeFlag = forge.getForgery<PrecomputedFlag>().copy(
648648
variationType = VariationType.OBJECT.value,
649-
variationValue = fakeFlagValue.toString()
649+
variationValue = fakeFlagValue.toString(),
650+
doLog = true
650651
)
651652
val fakeContext = EvaluationContext(
652653
targetingKey = forge.anAlphabeticalString(),
@@ -664,6 +665,13 @@ internal class DatadogFlagsClientTest {
664665
assertThat(result["nested"]).isInstanceOf(Map::class.java)
665666
val nested = result["nested"] as Map<*, *>
666667
assertThat(nested["city"]).isEqualTo("NYC")
668+
669+
// Verify exposure tracked
670+
verify(mockProcessor).processEvent(
671+
flagName = eq(fakeFlagKey),
672+
context = eq(fakeContext),
673+
data = eq(fakeFlag)
674+
)
667675
}
668676

669677
@Test
@@ -681,6 +689,7 @@ internal class DatadogFlagsClientTest {
681689

682690
// Then
683691
assertThat(result).isEqualTo(fakeDefaultValue)
692+
verifyNoInteractions(mockProcessor)
684693
}
685694

686695
@Test
@@ -699,6 +708,45 @@ internal class DatadogFlagsClientTest {
699708
verifyNoInteractions(mockRumEvaluationLogger)
700709
}
701710

711+
@Test
712+
fun `M return empty map W resolveStructureValue() {empty map default, flag not found}`(forge: Forge) {
713+
// Given
714+
val fakeFlagKey = forge.anAlphabeticalString()
715+
val fakeDefaultValue = emptyMap<String, Any?>()
716+
whenever(mockFlagsRepository.getPrecomputedFlagWithContext(fakeFlagKey)) doReturn null
717+
718+
// When
719+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
720+
721+
// Then
722+
assertThat(result).isEmpty()
723+
assertThat(result).isSameAs(fakeDefaultValue) // Preserves original reference
724+
verifyNoInteractions(mockProcessor)
725+
}
726+
727+
@Test
728+
fun `M return default map W resolveStructureValue() {map default, type mismatch}`(forge: Forge) {
729+
// Given
730+
val fakeFlagKey = forge.anAlphabeticalString()
731+
val fakeDefaultValue = mapOf("key" to "value")
732+
val fakeFlag = forge.getForgery<PrecomputedFlag>().copy(
733+
variationType = VariationType.STRING.value,
734+
variationValue = "not-an-object"
735+
)
736+
val fakeContext = EvaluationContext(
737+
targetingKey = forge.anAlphabeticalString(),
738+
attributes = emptyMap()
739+
)
740+
whenever(mockFlagsRepository.getPrecomputedFlagWithContext(fakeFlagKey)) doReturn (fakeFlag to fakeContext)
741+
742+
// When
743+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
744+
745+
// Then
746+
assertThat(result).isEqualTo(fakeDefaultValue)
747+
verifyNoInteractions(mockProcessor) // No exposure tracked for type mismatch
748+
}
749+
702750
// endregion
703751

704752
// region Constructor Logic

features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/JsonExtensionsTest.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,19 @@ internal class JsonExtensionsTest {
367367
assertThat(obj.getInt("age")).isEqualTo(30)
368368
}
369369

370+
@Test
371+
fun `M preserve nulls W toJSONArray then toList() {null in list}`() {
372+
// Given
373+
val list = listOf("a", null, "b", null)
374+
375+
// When
376+
val jsonArray = list.toJSONArray()
377+
val result = jsonArray.toList()
378+
379+
// Then
380+
assertThat(result).containsExactly("a", null, "b", null)
381+
}
382+
370383
// endregion
371384

372385
// region Round-trip conversion

0 commit comments

Comments
 (0)