FFL-3565: Accept every numeric type in Int and Double reads

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()"

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"

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

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.

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
+    }
 }