[cache] Allow to store JsonNumber in Record (#6139)

* Add tests for JsonNumber

* Allow JsonNumber in RecordWeighter

libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo/api/json/JsonReaders.kt

@@ -15,14 +15,16 @@ fun Map<String, Any?>.jsonReader(): JsonReader {
 }
 
 /**
- * Reads the reader and maps numbers to the closest representation possible in that order:
+ * Returns the Kotlin representation of the given [JsonReader]
+ *
+ * JSON numbers are mapped to built-in types when possible in that order:
  * - Int
  * - Long
  * - Double
  * - JsonNumber
  */
 @ApolloInternal
-fun JsonReader.readAny(): Any? {
+fun JsonReader.readAny(): ApolloJsonElement {
   return when (val token = peek()) {
     JsonReader.Token.NULL -> nextNull()
     JsonReader.Token.BOOLEAN -> nextBoolean()
@@ -60,6 +62,13 @@ private fun JsonReader.guessNumber(): Any {
   } catch (_: Exception) {
   }
   try {
+    /**
+     * XXX: this can lose precision on large numbers (String.toDouble() may approximate)
+     * This hasn't been an issue so far, and it's used quite extensively, so I'm keeping it
+     * as is for the time being.
+     * If you're reading this, it probably means it became an issue. In that case, nextDouble()
+     * here should be skipped and the calling code be adapted.
+     */
     return nextDouble()
   } catch (_: Exception) {
   }

libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo/api/json/MapJsonReader.kt

@@ -415,12 +415,13 @@ constructor(
  * generally be avoided in application code.
  *
  * [ApolloJsonElement] can be any of:
+ * - null
  * - String
+ * - Boolean
  * - Int
- * - Double
  * - Long
+ * - Double
  * - JsonNumber
- * - null
  * - Map<String, ApolloJsonElement> where values are any of these values recursively
  * - List<ApolloJsonElement> where values are any of these values recursively
  * - dynamic for advanced use cases using @JsExport

libraries/apollo-normalized-cache-api/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/api/Record.kt

@@ -23,7 +23,7 @@ class Record(
      * - Map (for custom scalars)
      * - null
      */
-    val fields: Map<String, Any?>,
+    val fields: Map<String, RecordValue>,
     val mutationId: Uuid? = null,
 ) : Map<String, Any?> by fields {
 
@@ -124,3 +124,14 @@ class Record(
     }
   }
 }
+
+/**
+ * A typealias for a type-unsafe Kotlin representation of a Record value. This typealias is
+ * mainly for internal documentation purposes and low-level manipulations and should
+ * generally be avoided in application code.
+ *
+ * [RecordValue] can be any of:
+ * - [com.apollographql.apollo.api.json.ApolloJsonElement]
+ * - [CacheKey]
+ */
+typealias RecordValue = Any?

libraries/apollo-normalized-cache-api/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/api/internal/JsonRecordSerializer.kt

@@ -3,10 +3,12 @@ package com.apollographql.apollo.cache.normalized.api.internal
 import com.apollographql.apollo.annotations.ApolloInternal
 import com.apollographql.apollo.api.json.BufferedSinkJsonWriter
 import com.apollographql.apollo.api.json.BufferedSourceJsonReader
+import com.apollographql.apollo.api.json.JsonNumber
 import com.apollographql.apollo.api.json.JsonWriter
 import com.apollographql.apollo.api.json.readAny
 import com.apollographql.apollo.cache.normalized.api.CacheKey
 import com.apollographql.apollo.cache.normalized.api.Record
+import com.apollographql.apollo.cache.normalized.api.RecordValue
 import okio.Buffer
 import okio.ByteString.Companion.encodeUtf8
 import okio.use
@@ -26,7 +28,7 @@ object JsonRecordSerializer {
     BufferedSinkJsonWriter(buffer).use { jsonWriter ->
       jsonWriter.beginObject()
       for ((key, value) in fields) {
-        jsonWriter.name(key).writeJsonValue(value)
+        jsonWriter.name(key).writeRecordValue(value)
       }
       jsonWriter.endObject()
     }
@@ -69,24 +71,25 @@ object JsonRecordSerializer {
   }
 
   @Suppress("UNCHECKED_CAST")
-  private fun JsonWriter.writeJsonValue(value: Any?) {
+  private fun JsonWriter.writeRecordValue(value: RecordValue) {
     when (value) {
       null -> this.nullValue()
       is String -> this.value(value)
       is Boolean -> this.value(value)
       is Int -> this.value(value)
       is Long -> this.value(value)
       is Double -> this.value(value)
+      is JsonNumber -> this.value(value)
       is CacheKey -> this.value(value.serialize())
       is List<*> -> {
         this.beginArray()
-        value.forEach { writeJsonValue(it) }
+        value.forEach { writeRecordValue(it) }
         this.endArray()
       }
       is Map<*, *> -> {
         this.beginObject()
         for (entry in value as Map<String, Any?>) {
-          this.name(entry.key).writeJsonValue(entry.value)
+          this.name(entry.key).writeRecordValue(entry.value)
         }
         this.endObject()
       }

libraries/apollo-normalized-cache-api/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/api/internal/Normalizer.kt

@@ -9,6 +9,7 @@ import com.apollographql.apollo.api.CompiledSelection
 import com.apollographql.apollo.api.CompiledType
 import com.apollographql.apollo.api.Executable
 import com.apollographql.apollo.api.isComposite
+import com.apollographql.apollo.api.json.ApolloJsonElement
 import com.apollographql.apollo.cache.normalized.api.CacheKey
 import com.apollographql.apollo.cache.normalized.api.CacheKeyGenerator
 import com.apollographql.apollo.cache.normalized.api.CacheKeyGeneratorContext
@@ -25,7 +26,7 @@ internal class Normalizer(
 ) {
   private val records = mutableMapOf<String, Record>()
 
-  fun normalize(map: Map<String, Any?>, selections: List<CompiledSelection>, parentType: String): Map<String, Record> {
+  fun normalize(map: Map<String, ApolloJsonElement>, selections: List<CompiledSelection>, parentType: String): Map<String, Record> {
     buildRecord(map, rootKey, selections, parentType)
 
     return records
@@ -38,7 +39,7 @@ internal class Normalizer(
    * @return the CacheKey
    */
   private fun buildRecord(
-      obj: Map<String, Any?>,
+      obj: Map<String, ApolloJsonElement>,
       key: String,
       selections: List<CompiledSelection>,
       parentType: String,

libraries/apollo-normalized-cache-api/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/api/internal/RecordWeigher.kt

@@ -1,7 +1,9 @@
 package com.apollographql.apollo.cache.normalized.api.internal
 
+import com.apollographql.apollo.api.json.JsonNumber
 import com.apollographql.apollo.cache.normalized.api.CacheKey
 import com.apollographql.apollo.cache.normalized.api.Record
+import com.apollographql.apollo.cache.normalized.api.RecordValue
 import okio.internal.commonAsUtf8ToByteArray
 import kotlin.jvm.JvmStatic
 
@@ -30,27 +32,30 @@ internal object RecordWeigher {
     return size
   }
 
-  private fun weighField(field: Any?): Int {
+  private fun weighField(field: RecordValue): Int {
     return when (field) {
       null -> SIZE_OF_NULL
       is String -> field.commonAsUtf8ToByteArray().size
       is Boolean -> SIZE_OF_BOOLEAN
       is Int -> SIZE_OF_INT
       is Long -> SIZE_OF_LONG // Might happen with LongDataAdapter
       is Double -> SIZE_OF_DOUBLE
+      is JsonNumber -> field.value.commonAsUtf8ToByteArray().size + SIZE_OF_LONG
+      /**
+       * Custom scalars with a json object representation are stored directly in the record
+       */
+      is Map<*, *> -> {
+        SIZE_OF_MAP_OVERHEAD + field.keys.sumOf { weighField(it) } + field.values.sumOf { weighField(it) }
+      }
+
       is List<*> -> {
         SIZE_OF_ARRAY_OVERHEAD + field.sumOf { weighField(it) }
       }
 
       is CacheKey -> {
         SIZE_OF_CACHE_KEY_OVERHEAD + field.key.commonAsUtf8ToByteArray().size
       }
-      /**
-       * Custom scalars with a json object representation are stored directly in the record
-       */
-      is Map<*, *> -> {
-        SIZE_OF_MAP_OVERHEAD + field.keys.sumOf { weighField(it) } + field.values.sumOf { weighField(it) }
-      }
+
       else -> error("Unknown field type in Record: '$field'")
     }
   }

libraries/apollo-normalized-cache-api/src/commonTest/kotlin/com/apollographql/apollo/cache/normalized/RecordWeigherTest.kt

@@ -1,5 +1,6 @@
 package com.apollographql.apollo.cache.normalized
 
+import com.apollographql.apollo.api.json.JsonNumber
 import com.apollographql.apollo.cache.normalized.api.CacheKey
 import com.apollographql.apollo.cache.normalized.api.Record
 import kotlin.test.Test
@@ -13,6 +14,7 @@ class RecordWeigherTest {
     val expectedLongValue = Long.MAX_VALUE
     val expectedStringValue = "StringValue"
     val expectedBooleanValue = true
+    val expectedNumberValue = JsonNumber("10")
     val expectedCacheKey = CacheKey("foo")
     val expectedCacheKeyList = listOf(CacheKey("bar"), CacheKey("baz"))
     val expectedScalarList = listOf("scalarOne", "scalarTwo")
@@ -23,13 +25,14 @@ class RecordWeigherTest {
             "string" to expectedStringValue,
             "boolean" to expectedBooleanValue,
             "long" to expectedLongValue,
+            "number" to expectedNumberValue,
             "cacheReference" to expectedCacheKey,
             "scalarList" to expectedScalarList,
             "referenceList" to expectedCacheKeyList,
         )
     )
 
-    assertTrue(record.sizeInBytes <= 230)
-    assertTrue(record.sizeInBytes >= 226) // JS takes less space, maybe for strings?
+    assertTrue(record.sizeInBytes <= 246)
+    assertTrue(record.sizeInBytes >= 242) // JS takes less space, maybe for strings?
   }
 }

tests/number_scalar/build.gradle.kts

@@ -0,0 +1,21 @@
+plugins {
+  id("org.jetbrains.kotlin.jvm")
+  id("com.apollographql.apollo")
+}
+
+apolloTest()
+
+dependencies {
+  implementation(libs.apollo.api)
+  implementation(libs.apollo.testingsupport)
+  testImplementation(libs.kotlin.test)
+  testImplementation(libs.apollo.testingsupport)
+  testImplementation(libs.apollo.normalizedcache)
+}
+
+apollo {
+  service("service") {
+    packageName.set("com.example")
+    mapScalar("Number", "kotlin.String")
+  }
+}

tests/number_scalar/src/main/graphql/operation.graphql

@@ -0,0 +1,3 @@
+query GetNumber {
+  number
+}

tests/number_scalar/src/main/graphql/schema.graphqls

@@ -0,0 +1,5 @@
+scalar Number
+
+type Query {
+  number: Number
+}

tests/number_scalar/src/test/kotlin/test/NumberTest.kt

@@ -0,0 +1,49 @@
+package test
+
+
+import com.apollographql.apollo.ApolloClient
+import com.apollographql.apollo.api.Adapter
+import com.apollographql.apollo.api.CustomScalarAdapters
+import com.apollographql.apollo.api.json.JsonNumber
+import com.apollographql.apollo.api.json.JsonReader
+import com.apollographql.apollo.api.json.JsonWriter
+import com.apollographql.apollo.cache.normalized.FetchPolicy
+import com.apollographql.apollo.cache.normalized.api.MemoryCacheFactory
+import com.apollographql.apollo.cache.normalized.fetchPolicy
+import com.apollographql.apollo.cache.normalized.normalizedCache
+import com.apollographql.apollo.testing.QueueTestNetworkTransport
+import com.apollographql.apollo.testing.enqueueTestResponse
+import com.example.GetNumberQuery
+import kotlinx.coroutines.runBlocking
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class NumberTest {
+  @Test
+  fun test(): Unit = runBlocking {
+    ApolloClient.Builder()
+        .networkTransport(QueueTestNetworkTransport())
+        .normalizedCache(MemoryCacheFactory())
+        .addCustomScalarAdapter(com.example.type.Number.type, NumberAdapter())
+        .build()
+        .use { apolloClient ->
+          apolloClient.enqueueTestResponse(GetNumberQuery(), GetNumberQuery.Data("12345"))
+          apolloClient.query(GetNumberQuery()).fetchPolicy(FetchPolicy.NetworkOnly).execute().apply {
+            assertEquals("12345", data?.number)
+          }
+          apolloClient.query(GetNumberQuery()).fetchPolicy(FetchPolicy.CacheOnly).execute().apply {
+            assertEquals("12345", data?.number)
+          }
+        }
+  }
+}
+
+class NumberAdapter: Adapter<String> {
+  override fun fromJson(reader: JsonReader, customScalarAdapters: CustomScalarAdapters): String {
+    return reader.nextNumber().value
+  }
+
+  override fun toJson(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, value: String) {
+    writer.value(JsonNumber(value))
+  }
+}