diff --git a/detekt_custom_safe_calls_third_party.yml b/detekt_custom_safe_calls_third_party.yml index 31b95eb122..ba94b46b9a 100644 --- a/detekt_custom_safe_calls_third_party.yml +++ b/detekt_custom_safe_calls_third_party.yml @@ -367,6 +367,10 @@ datadog: - "java.lang.StringBuilder.append(kotlin.String?)" - "java.lang.StringBuilder.clear()" - "java.lang.StringBuilder.constructor()" + - "java.math.BigDecimal.constructor(kotlin.Int)" + - "java.math.BigDecimal.scale()" + - "java.math.BigDecimal.signum()" + - "java.math.BigDecimal.toInt()" - "java.math.BigInteger.toHexString()" - "java.math.BigInteger.toLong()" - "java.nio.ByteBuffer.duplicate()" @@ -952,6 +956,7 @@ datadog: - "kotlin.String.removePrefix(kotlin.CharSequence)" - "kotlin.String.removeSuffix(kotlin.CharSequence)" - "kotlin.String.takeIf(kotlin.Function1)" + - "kotlin.String.toBigDecimalOrNull()" - "kotlin.String.toBigIntegerOrNull()" - "kotlin.String.toBoolean()" - "kotlin.String.toBooleanStrictOrNull()" diff --git a/detekt_custom_unsafe_calls.yml b/detekt_custom_unsafe_calls.yml index 8b96a87b70..db5212d32d 100644 --- a/detekt_custom_unsafe_calls.yml +++ b/detekt_custom_unsafe_calls.yml @@ -167,6 +167,7 @@ datadog: - "java.lang.System.loadLibrary(kotlin.String?):java.lang.SecurityException,java.lang.UnsatisfiedLinkError,java.lang.NullPointerException" - "java.lang.reflect.Field.get(kotlin.Any):java.lang.IllegalAccessException,java.lang.IllegalArgumentException,java.lang.NullPointerException,java.lang.ExceptionInInitializerError" - "java.lang.reflect.Field.get(kotlin.Any?):java.lang.IllegalAccessException,java.lang.IllegalArgumentException,java.lang.NullPointerException,java.lang.ExceptionInInitializerError" + - "java.math.BigDecimal.stripTrailingZeros():java.lang.ArithmeticException" - "java.math.BigInteger.and(java.math.BigInteger?):java.lang.NumberFormatException,java.lang.ArithmeticException" - "java.math.BigInteger.constructor(kotlin.String?, kotlin.Int):java.lang.NumberFormatException,java.lang.ArithmeticException" - "java.math.BigInteger.shiftRight(kotlin.Int):java.lang.NumberFormatException,java.lang.ArithmeticException" diff --git a/features/dd-sdk-android-flags/README.md b/features/dd-sdk-android-flags/README.md index acf64792de..71b9c28dc9 100644 --- a/features/dd-sdk-android-flags/README.md +++ b/features/dd-sdk-android-flags/README.md @@ -290,6 +290,8 @@ val maxRetries = client.resolveIntValue("max-retry-count", 3) val discountPercentage = client.resolveDoubleValue("discount-rate", 0.0) ``` +Any numeric flag can be read with either method. `resolveIntValue` returns the value only when it is a whole number within `Int` range, such as `5` or `5.0`. Otherwise, it returns the default value: `resolve()` reports `TYPE_MISMATCH` when the value is fractional, such as `1.2`, or outside `Int` range, such as `2147483648`, and `PARSE_ERROR` when the value can't be parsed. To bound parsing work, a value longer than 64 characters is not parsed unless it is a plain integer within `Int` range, so it reports `PARSE_ERROR`. `resolveDoubleValue` returns a fractional value as the nearest `Double`, such as `1.2`. It returns a whole value only when the `Double` has exactly that value, or prints as that value, such as `1e300`. For other whole values, such as `9007199254740993`, `resolve()` reports `TYPE_MISMATCH`. `NaN` and `Infinity` report `PARSE_ERROR`. The same applies to a value longer than 64 characters whose `Double` is 2^53 or more in magnitude. + **Structured flags** ```kotlin val defaultConfig = JSONObject("""{"timeout": 30, "retries": 3}""") @@ -347,7 +349,7 @@ if (result.errorCode != null) { ##### Error codes - `FLAG_NOT_FOUND` - The flag could not be found - `PARSE_ERROR` - Error parsing the flag value -- `TYPE_MISMATCH` - The flag type doesn't match the expected type +- `TYPE_MISMATCH` - The flag value's type doesn't match the expected type, such as a fractional value read with `resolveIntValue` - `TARGETING_KEY_MISSING` - No targeting key was provided - `INVALID_CONTEXT` - The evaluation context is invalid - `PROVIDER_NOT_READY` - The provider is not yet ready 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 754f9c004f..33c3ddf776 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 @@ -130,6 +130,11 @@ interface FlagsClient { /** * Resolves a numeric flag value. * + * Any numeric flag type is accepted. A fractional value, such as `1.2`, resolves to the nearest [Double]. A + * whole value resolves only when the [Double] has exactly that value, or prints as that value, such as `1e300`. + * A value such as `9007199254740993` returns the default value. So does a value that can't be parsed, such as + * `NaN`, and a value longer than 64 characters whose [Double] is 2^53 or more in magnitude. + * * @param flagKey The unique identifier of the flag to resolve. * @param defaultValue The value to return if the flag cannot be retrieved or parsed. * @return The numeric value of the flag as a double, or the default value if unavailable. @@ -139,6 +144,10 @@ interface FlagsClient { /** * Resolves an integer flag value. * + * Any numeric flag resolves when its value is a whole number within [Int] range, such as `5` or `5.0`. A fractional + * value, such as `1.2`, or a value outside [Int] range returns the default value. So does a value longer than + * 64 characters, unless it is a plain integer within [Int] range. + * * @param flagKey The unique identifier of the flag to resolve. * @param defaultValue The value to return if the flag cannot be retrieved or parsed. * @return The integer value of the flag, or the default value if unavailable. diff --git a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagValueConverter.kt b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagValueConverter.kt index a3c7e5b22a..b69bb48e13 100644 --- a/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagValueConverter.kt +++ b/features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagValueConverter.kt @@ -8,6 +8,7 @@ package com.datadog.android.flags.internal import com.datadog.android.flags.internal.model.VariationType import org.json.JSONObject +import java.math.BigDecimal import java.util.Locale import kotlin.reflect.KClass @@ -28,6 +29,16 @@ internal class TypeMismatchException(message: String) : Exception(message) */ internal object FlagValueConverter { + private val INT_MIN = BigDecimal(Int.MIN_VALUE) + private val INT_MAX = BigDecimal(Int.MAX_VALUE) + + // The longest decimal value that Int reads, and Double reads at or above 2^53, parse with BigDecimal. + // This bounds the BigDecimal work on each read. + private const val MAX_DECIMAL_LENGTH = 64 + + // 2^53. Below this magnitude, Double holds every whole value exactly. + private const val EXACT_DOUBLE_LIMIT = 9_007_199_254_740_992.0 + /** * Converts a flag's string value to the expected type. * @@ -36,7 +47,9 @@ internal object FlagValueConverter { * @param variationType The type specified in the flag metadata * @param targetType The target type class to convert to * @return Result containing the converted value on success, - * or a Failure with TypeMismatchException if types are incompatible, + * or a Failure with TypeMismatchException if types are incompatible + * or a numeric value can't be read as Int without loss, + * or a whole value matches neither the Double nor its printed form, * or a Failure with the parse exception if conversion failed */ @Suppress("UNCHECKED_CAST") @@ -53,8 +66,8 @@ internal object FlagValueConverter { val result: T? = when (targetType) { Boolean::class -> variationValue.lowercase(Locale.US).toBooleanStrictOrNull() as? T String::class -> variationValue as T - Int::class -> variationValue.toIntOrNull() as? T - Double::class -> variationValue.toDoubleOrNull() as? T + Int::class -> parseLosslessInt(variationValue, variationType) as? T + Double::class -> parseLosslessDouble(variationValue, variationType) as? T Map::class -> variationValue.toMap() as? T JSONObject::class -> { @Suppress("UnsafeThirdPartyFunctionCall") // Safe: wrapped in runCatching @@ -88,10 +101,7 @@ internal object FlagValueConverter { fun isTypeCompatible(variationType: String, targetType: KClass<*>): Boolean = when (targetType) { Boolean::class -> variationType == VariationType.BOOLEAN.value String::class -> variationType == VariationType.STRING.value - Int::class -> variationType == VariationType.INTEGER.value || variationType == VariationType.NUMBER.value - Double::class -> - variationType == VariationType.NUMBER.value || variationType == VariationType.FLOAT.value || - variationType == VariationType.INTEGER.value + Int::class, Double::class -> isNumeric(variationType) JSONObject::class -> variationType == VariationType.OBJECT.value Map::class -> variationType == VariationType.OBJECT.value else -> { @@ -115,4 +125,90 @@ internal object FlagValueConverter { Map::class -> "Map" else -> targetType.simpleName ?: "Unknown" } + + private fun isNumeric(variationType: String): Boolean = variationType == VariationType.INTEGER.value || + variationType == VariationType.NUMBER.value || + variationType == VariationType.FLOAT.value + + /** + * Int reads are lossless: a whole value within Int range, such as `5` or `5.0`, reads as Int whatever the + * numeric type. A fractional or out-of-range value is a type mismatch. A value that can't be parsed returns null, + * which [convert] reports as a parse error. So does a value that is not a plain Int and is longer than + * [MAX_DECIMAL_LENGTH] characters. + */ + @Throws(TypeMismatchException::class) + private fun parseLosslessInt(variationValue: String, variationType: String): Int? = variationValue.toIntOrNull() + ?: variationValue.takeIf { it.length <= MAX_DECIMAL_LENGTH } + ?.toBigDecimalOrNull() + ?.let { requireLosslessInt(it, variationValue, variationType) } + + // Compare with the Int bounds instead of calling BigDecimal.intValueExact(): on Android 9, intValueExact() + // expands the power of ten, so a short value such as `1e10000000` takes seconds. + @Throws(TypeMismatchException::class) + private fun requireLosslessInt(decimal: BigDecimal, variationValue: String, variationType: String): Int { + val problem = when { + isFractional(decimal) -> "fractional value '$variationValue'" + decimal < INT_MIN || decimal > INT_MAX -> "value '$variationValue' outside Int range" + else -> null + } + if (problem != null) { + @Suppress("ThrowingInternalException") // Caught by runCatching in convert() + throw TypeMismatchException("Flag has type '$variationType' with $problem but Int was requested") + } + return decimal.toInt() + } + + /** + * A whole value reads as Double only when the Double equals it, or when the Double prints as the same value, + * such as `1e300`. So `9007199254740993` is a type mismatch. A fractional value reads as the nearest Double, + * such as `1.2`. A value that can't be parsed returns null, which [convert] reports as a parse error. So does + * a value whose parsed Double is 2^53 or more in magnitude, when the value is longer than [MAX_DECIMAL_LENGTH] + * characters or [BigDecimal] can't parse it. + */ + @Throws(TypeMismatchException::class) + private fun parseLosslessDouble(variationValue: String, variationType: String): Double? = + variationValue.toDoubleOrNull()?.let { double -> + if (double > -EXACT_DOUBLE_LIMIT && double < EXACT_DOUBLE_LIMIT) { + double + } else { + variationValue.takeIf { it.length <= MAX_DECIMAL_LENGTH } + ?.toBigDecimalOrNull() + ?.let { requireLosslessDouble(it, double, variationValue, variationType) } + } + } + + @Throws(TypeMismatchException::class) + private fun requireLosslessDouble( + decimal: BigDecimal, + double: Double, + variationValue: String, + variationType: String + ): Double { + if (!isFractional(decimal) && !isExactDouble(decimal, double)) { + @Suppress("ThrowingInternalException") // Caught by runCatching in convert() + throw TypeMismatchException( + "Flag has type '$variationType' with whole value '$variationValue' that Double can't hold exactly " + + "but Double was requested" + ) + } + return double + } + + @Suppress("UnsafeThirdPartyFunctionCall") // Safe: BigDecimal(Double) and valueOf() only run for a finite Double + private fun isExactDouble(decimal: BigDecimal, double: Double): Boolean = + double != Double.POSITIVE_INFINITY && double != Double.NEGATIVE_INFINITY && + (decimal.compareTo(BigDecimal(double)) == 0 || decimal.compareTo(BigDecimal.valueOf(double)) == 0) + + /** + * A value with a scale of 0 or less is whole. Check the scale first: it also prevents + * [BigDecimal.stripTrailingZeros] from overflowing the scale for a value such as `100e2147483647`. + * Zero is whole at any scale: some older Android versions return a zero such as `0.0` unchanged from + * [BigDecimal.stripTrailingZeros]. + */ + private fun isFractional(decimal: BigDecimal): Boolean { + if (decimal.scale() <= 0 || decimal.signum() == 0) return false + @Suppress("UnsafeThirdPartyFunctionCall") // Safe: the scale is positive, so it can't overflow + val stripped = decimal.stripTrailingZeros() + return stripped.scale() > 0 + } } diff --git a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt index 0c5733fa1f..bfff47d084 100644 --- a/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt +++ b/features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt @@ -29,6 +29,7 @@ import com.datadog.android.flags.model.EvaluationContext import com.datadog.android.flags.model.FlagsClientEvent import com.datadog.android.flags.model.ResolutionReason import com.datadog.android.flags.utils.forge.ForgeConfigurator +import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration @@ -983,6 +984,204 @@ internal class DatadogFlagsClientTest { verifyNoInteractions(mockRumEvaluationLogger) } + @ParameterizedTest + @CsvSource( + "number, 5, 5", + "number, 5.0, 5", + "number, 1e3, 1000", + "number, -7.00, -7", + "number, 0.0, 0", + "number, -0.0, 0", + "number, 2147483647.0, 2147483647", + "number, -2147483648, -2147483648", + "number, -2147483648.0, -2147483648", + "integer, 7, 7", + "integer, 5.0, 5", + "float, 5, 5", + "float, 1e3, 1000", + "number, 0e5, 0", + "number, 10e-1, 1", + "number, 1.5e1, 15", + "number, 5.00000000000000000000000000000000000000000000000000000000000000, 5", + "number, 00000000000000000000000000000000000000000000000000000000000000005, 5" + ) + fun `M return value W resolve() { numeric flag, whole value, Int default }`( + fakeVariationType: String, + fakeVariationValue: String, + expectedValue: Int, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.anInt(min = 2000) + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(expectedValue) + assertThat(result.errorCode).isNull() + } + + @ParameterizedTest + @CsvSource( + "number, 1.2", + "number, -0.5", + "number, 5.000001", + "number, 999999999999999.99", + "number, 1e-400", + "float, 1.2", + "integer, 1.2" + ) + fun `M return default with TYPE_MISMATCH W resolve() { numeric flag, fractional value, Int default }`( + fakeVariationType: String, + fakeVariationValue: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.anInt() + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue, + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.TYPE_MISMATCH) + assertThat(result.errorMessage).isEqualTo( + "Flag '$fakeFlagKey': Flag has type '$fakeVariationType' with fractional value " + + "'$fakeVariationValue' but Int was requested" + ) + verifyNoInteractions(mockProcessor) + } + + @ParameterizedTest + @CsvSource( + "number, 2147483648", + "number, 3000000000", + "number, 1e20", + "number, -2147483649", + "number, -1e20", + "number, 100e2147483647", + "number, 1e2147483648", + "integer, 2147483648", + "float, 2147483648.0" + ) + fun `M return default with TYPE_MISMATCH W resolve() { numeric flag, value outside Int range, Int default }`( + fakeVariationType: String, + fakeVariationValue: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.anInt() + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue, + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.TYPE_MISMATCH) + assertThat(result.errorMessage).isEqualTo( + "Flag '$fakeFlagKey': Flag has type '$fakeVariationType' with value " + + "'$fakeVariationValue' outside Int range but Int was requested" + ) + verifyNoInteractions(mockProcessor) + } + + @ParameterizedTest + @CsvSource( + "number, not-a-number", + "integer, not-a-number", + "float, not-a-number", + "number, ''", + "number, ' 5'", + "number, NaN", + "number, Infinity", + "number, 5f", + "number, 0x10", + "number, 1e99999999999", + "number, 1e-2147483648", + "number, 1e2147483649", + "number, 10000000000000000000000000000000000000000000000000000000000000000.0", + "number, 5.000000000000000000000000000000000000000000000000000000000000000", + "float, NaN" + ) + fun `M return default with PARSE_ERROR W resolve() { malformed numeric value, Int default }`( + fakeVariationType: String, + fakeVariationValue: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.anInt() + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue, + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.PARSE_ERROR) + assertThat(result.errorMessage) + .isEqualTo("Flag '$fakeFlagKey': Failed to parse value '$fakeVariationValue' as Int") + verifyNoInteractions(mockProcessor) + } + + @ParameterizedTest + @ValueSource(strings = ["boolean", "string", "object"]) + fun `M return default with TYPE_MISMATCH W resolve() { non-numeric flag, Int default }`( + fakeVariationType: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.anInt(min = 6) + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = "5", + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.TYPE_MISMATCH) + assertThat(result.errorMessage) + .isEqualTo("Flag '$fakeFlagKey': Flag has type '$fakeVariationType' but Int was requested") + verifyNoInteractions(mockProcessor) + } + // endregion // region resolveDoubleValue() @@ -1018,6 +1217,151 @@ internal class DatadogFlagsClientTest { verifyNoInteractions(mockRumEvaluationLogger) } + @ParameterizedTest + @CsvSource( + "number, 5, 5.0", + "number, 1.5, 1.5", + "integer, 7, 7.0", + "integer, 5.0, 5.0", + "float, 5, 5.0", + "number, 9007199254740992, 9007199254740992.0", + "number, -9223372036854775808, -9223372036854775808.0", + "float, 18446744073709551616, 18446744073709551616.0", + "number, 1e300, 1e300", + "number, 1.20000000000000001, 1.2", + "float, 9007199254740993.5, 9007199254740994.0", + "number, 9007199254740992.00000000000000000000000000000000000000000000000, 9007199254740992.0", + "number, 1.000000000000000000000000000000000000000000000000000000000000000, 1.0" + ) + fun `M return value W resolve() { numeric flag, Double default }`( + fakeVariationType: String, + fakeVariationValue: String, + expectedValue: Double, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.aDouble(min = 100.0) + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(expectedValue) + assertThat(result.errorCode).isNull() + } + + @ParameterizedTest + @CsvSource( + "number, 9007199254740993", + "number, -9007199254740993", + "number, 9223372036854775807", + "number, 9007199254740993.0", + "number, 1e400", + "number, 1e2147483648", + "integer, 9007199254740993", + "float, 9007199254740993" + ) + fun `M return default with TYPE_MISMATCH W resolve() { whole value Double can't hold, Double default }`( + fakeVariationType: String, + fakeVariationValue: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.aDouble() + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue, + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.TYPE_MISMATCH) + assertThat(result.errorMessage).isEqualTo( + "Flag '$fakeFlagKey': Flag has type '$fakeVariationType' with whole value " + + "'$fakeVariationValue' that Double can't hold exactly but Double was requested" + ) + verifyNoInteractions(mockProcessor) + } + + @ParameterizedTest + @CsvSource( + "number, not-a-number", + "number, NaN", + "number, Infinity", + "float, -Infinity", + "number, 10000000000000000000000000000000000000000000000000000000000000000.0", + "number, 9007199254740992.0000000000000000000000000000000000000000000000001", + "number, 1e2147483649" + ) + fun `M return default with PARSE_ERROR W resolve() { malformed numeric value, Double default }`( + fakeVariationType: String, + fakeVariationValue: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.aDouble() + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = fakeVariationValue, + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.PARSE_ERROR) + assertThat(result.errorMessage) + .isEqualTo("Flag '$fakeFlagKey': Failed to parse value '$fakeVariationValue' as Double") + verifyNoInteractions(mockProcessor) + } + + @ParameterizedTest + @ValueSource(strings = ["boolean", "string", "object"]) + fun `M return default with TYPE_MISMATCH W resolve() { non-numeric flag, Double default }`( + fakeVariationType: String, + forge: Forge + ) { + // Given + val fakeFlagKey = forge.anAlphabeticalString() + val fakeDefaultValue = forge.aDouble(min = 6.0) + val fakeFlag = forge.getForgery().copy( + variationType = fakeVariationType, + variationValue = "5", + doLog = true + ) + whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) + + // When + val result = testedClient.resolve(fakeFlagKey, fakeDefaultValue) + + // Then + assertThat(result.value).isEqualTo(fakeDefaultValue) + assertThat(result.errorCode).isEqualTo(ErrorCode.TYPE_MISMATCH) + assertThat(result.errorMessage) + .isEqualTo("Flag '$fakeFlagKey': Flag has type '$fakeVariationType' but Double was requested") + verifyNoInteractions(mockProcessor) + } + // endregion // region resolveStructureValue() @@ -1858,29 +2202,29 @@ internal class DatadogFlagsClientTest { } @Test - fun `M return default value W resolveIntValue() { value exceeds Int MAX_VALUE }`(forge: Forge) { + fun `M log warning and return default W resolveIntValue() { number flag, fractional value }`(forge: Forge) { // Given val fakeFlagKey = forge.anAlphabeticalString() val fakeDefaultValue = forge.anInt() - // Value that's too large for Int - val oversizedValue = Int.MAX_VALUE.toLong() + 1L val fakeFlag = forge.getForgery().copy( - variationType = VariationType.INTEGER.value, - variationValue = oversizedValue.toString() - ) - val fakeContext = EvaluationContext( - targetingKey = forge.anAlphabeticalString(), - attributes = emptyMap() + variationType = VariationType.NUMBER.value, + variationValue = "1.2", + doLog = true ) whenever(mockFlagsRepository.getEvaluationSnapshot(fakeFlagKey)) doReturn - FlagEvaluationSnapshot(fakeFlag, fakeContext, false) + FlagEvaluationSnapshot(fakeFlag, fakeDefaultContext, false) // When val result = testedClient.resolveIntValue(fakeFlagKey, fakeDefaultValue) // Then - // toIntOrNull() returns null for values outside Int range, so default is returned assertThat(result).isEqualTo(fakeDefaultValue) + verifyNoInteractions(mockProcessor) + mockInternalLogger.verifyLog( + InternalLogger.Level.WARN, + InternalLogger.Target.USER, + "Flag '$fakeFlagKey': Flag has type 'number' with fractional value '1.2' but Int was requested" + ) } // endregion diff --git a/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/EvaluationTelemetryTest.kt b/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/EvaluationTelemetryTest.kt index e83c455fc4..136ae100c9 100644 --- a/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/EvaluationTelemetryTest.kt +++ b/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/EvaluationTelemetryTest.kt @@ -694,7 +694,8 @@ internal class EvaluationTelemetryTest { listOf( Arguments.of("missing", false, ErrorCode.FLAG_NOT_FOUND, optedOut), Arguments.of("enabled", "fallback", ErrorCode.TYPE_MISMATCH, optedOut), - Arguments.of("malformed", false, ErrorCode.PARSE_ERROR, optedOut) + Arguments.of("malformed", false, ErrorCode.PARSE_ERROR, optedOut), + Arguments.of("number", 0, ErrorCode.TYPE_MISMATCH, optedOut) ) } diff --git a/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/FlagsEvaluationTest.kt b/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/FlagsEvaluationTest.kt index d9180345d9..dd5e337090 100644 --- a/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/FlagsEvaluationTest.kt +++ b/reliability/single-fit/flags/src/test/kotlin/com/datadog/android/flags/integration/FlagsEvaluationTest.kt @@ -127,6 +127,7 @@ class FlagsEvaluationTest { "string" to assignment("string", fakeString), "int" to assignment("integer", fakeInt), "double" to assignment("number", fakeDouble), + "float" to assignment("float", fakeInt), "object" to assignment("object", JSONObject().put(fakeStructureKey, fakeStructureValue)) ) ) @@ -141,6 +142,8 @@ class FlagsEvaluationTest { assertThat(client.resolveStringValue("string", "")).isEqualTo(fakeString) assertThat(client.resolveIntValue("int", fakeInt.inv())).isEqualTo(fakeInt) assertThat(client.resolveDoubleValue("double", fakeDouble + 1)).isEqualTo(fakeDouble) + assertThat(client.resolveIntValue("float", fakeInt.inv())).isEqualTo(fakeInt) + assertThat(client.resolveDoubleValue("int", fakeInt.inv().toDouble())).isEqualTo(fakeInt.toDouble()) assertThat(client.resolveStructureValue("object", JSONObject()).getString(fakeStructureKey)) .isEqualTo(fakeStructureValue) assertThat(client.resolveStructureValue("object", emptyMap()))