Skip to content

Commit 890879a

Browse files
fabriziodemariafabriziodemaria
authored
perf: parse resolve responses without JSON round-trips (#239)
* perf: parse resolve responses without JSON round-trips `NetworkResolvedFlagSerializer`, `FlagsSerializer` and `convertToValue` were serializing each parsed `JsonElement` back to a string with `JsonElement.toString()` and then handing that string to `Json.decodeFromString(...)` (or to manual unquoting via `replace("\"", "")`). For every flag we therefore re-tokenized JSON we had already tokenized once. This change reads primitives directly via `.jsonPrimitive.content` (avoiding the `toString()` -> `replace("\"", "")` quote-stripping hack) and replaces every `Json.decodeFromString(<serializer>, element.toString())` call with `Json.decodeFromJsonElement(<serializer>, element)`, which is the supported zero-copy entry point for nested deserialization. Also pre-sizes the `ResolvedFlag` list to the parsed array size so we don't grow-and-copy while filling it. For ~950 flags this removes thousands of `JsonElement.toString()` allocations + retokenizations per resolve. Behaviour is preserved including `ResolveReason` enum decoding and existing `flagSchema` / `value` / `structSchema` paths. Made-with: Cursor * test: cover multi-flag, all reasons, and nested struct schema Add three deserialization tests to lock in equivalence on the resolve hot path touched by this PR: - testDeserializeMultipleFlagsInOneResponse: pins the FlagsSerializer loop with array.size > 1 and the ArrayList(array.size) preallocation. Existing payloads only had a single flag. - testDeserializeAllResolveReasons: round-trips every declared ResolveReason value through the new ResolveReason.valueOf path, guarding against future divergence (e.g. a stray @SerialName). - testDeserializeNestedStructSchema: pins the recursive convertToSchemaTypeValue / decodeFromJsonElement(SchemaTypeSerializer) path two levels deep with mixed leaf types. Existing tests only nest structSchema one level. Made-with: Cursor
1 parent 75d621c commit 890879a

2 files changed

Lines changed: 238 additions & 19 deletions

File tree

‎Confidence/src/main/java/com/spotify/confidence/serializers/Serializers.kt‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import kotlinx.serialization.json.JsonContentPolymorphicSerializer
2121
import kotlinx.serialization.json.JsonDecoder
2222
import kotlinx.serialization.json.JsonElement
2323
import kotlinx.serialization.json.JsonNull
24+
import kotlinx.serialization.json.boolean
2425
import kotlinx.serialization.json.jsonArray
2526
import kotlinx.serialization.json.jsonObject
2627
import kotlinx.serialization.json.jsonPrimitive
@@ -77,16 +78,12 @@ internal object NetworkResolvedFlagSerializer : KSerializer<ResolvedFlag> {
7778
override fun deserialize(decoder: Decoder): ResolvedFlag {
7879
val jsonDecoder = decoder as JsonDecoder
7980
val json = jsonDecoder.decodeJsonElement().jsonObject
80-
val flag = json["flag"].toString().split("/")
81-
.last()
82-
.replace("\"", "")
83-
84-
val variant = json["variant"]
85-
.toString()
86-
.replace("\"", "")
87-
88-
val resolvedReason = Json.decodeFromString<ResolveReason>(json["reason"].toString())
89-
val shouldApply = Json.decodeFromString<Boolean>(json["shouldApply"].toString())
81+
// Read primitives via .jsonPrimitive.content rather than JsonElement.toString()
82+
// to avoid serialize-then-reparse round trips for every flag.
83+
val flag = json["flag"]!!.jsonPrimitive.content.substringAfterLast('/')
84+
val variant = json["variant"]!!.jsonPrimitive.content
85+
val resolvedReason = ResolveReason.valueOf(json["reason"]!!.jsonPrimitive.content)
86+
val shouldApply = json["shouldApply"]!!.jsonPrimitive.boolean
9087
val flagSchemaJsonElement = json["flagSchema"]
9188

9289
val schemasJson = if (flagSchemaJsonElement != null && flagSchemaJsonElement != JsonNull) {
@@ -97,10 +94,9 @@ internal object NetworkResolvedFlagSerializer : KSerializer<ResolvedFlag> {
9794

9895
return if (schemasJson != null) {
9996
val flagSchema =
100-
Json.decodeFromString(SchemaTypeSerializer, schemasJson.toString())
101-
val valueJson = json["value"].toString()
97+
Json.decodeFromJsonElement(SchemaTypeSerializer, schemasJson)
10298
val values: ConfidenceValue.Struct =
103-
Json.decodeFromString(FlagValueSerializer(flagSchema), valueJson)
99+
Json.decodeFromJsonElement(FlagValueSerializer(flagSchema), json["value"]!!)
104100

105101
if (flagSchema.schema.size != values.map.size) {
106102
throw ParseError(
@@ -163,11 +159,11 @@ internal object FlagsSerializer : KSerializer<Flags> {
163159
get() = ListSerializer(String.serializer()).descriptor
164160

165161
override fun deserialize(decoder: Decoder): Flags {
166-
val list = mutableListOf<ResolvedFlag>()
167162
val jsonDecoder = decoder as JsonDecoder
168163
val array = jsonDecoder.decodeJsonElement().jsonArray
164+
val list = ArrayList<ResolvedFlag>(array.size)
169165
for (json in array) {
170-
list.add(Json.decodeFromString(NetworkResolvedFlagSerializer, json.toString()))
166+
list.add(Json.decodeFromJsonElement(NetworkResolvedFlagSerializer, json))
171167
}
172168
return Flags(list)
173169
}
@@ -206,9 +202,9 @@ private fun JsonElement.convertToValue(key: String, schemaType: SchemaType): Con
206202
if (jsonObject.isEmpty()) {
207203
ConfidenceValue.Struct(mapOf())
208204
} else {
209-
val serializedMap = Json.decodeFromString(
205+
val serializedMap = Json.decodeFromJsonElement(
210206
FlagValueSerializer(schemaType),
211-
jsonObject.toString()
207+
jsonObject
212208
).map
213209

214210
ConfidenceValue.Struct(serializedMap)
@@ -227,9 +223,9 @@ private fun JsonElement.convertToSchemaTypeValue(): SchemaType = when {
227223
jsonObject.keys.contains("intSchema") -> SchemaType.IntSchema
228224
jsonObject.keys.contains("boolSchema") -> SchemaType.BoolSchema
229225
jsonObject.keys.contains("structSchema") -> {
230-
val value = jsonObject["structSchema"]!!.jsonObject["schema"]
226+
val value = jsonObject["structSchema"]!!.jsonObject["schema"]!!
231227
SchemaType.SchemaStruct(
232-
Json.decodeFromString(SchemaTypeSerializer, value.toString()).schema
228+
Json.decodeFromJsonElement(SchemaTypeSerializer, value).schema
233229
)
234230
}
235231
else -> error("not a valid schema")

‎Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt‎

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,229 @@ internal class ConfidenceRemoteClientTests {
653653
assertEquals(ConfidenceValue.Integer(10), struct.map["nested_int"])
654654
}
655655

656+
@Test
657+
fun testDeserializeMultipleFlagsInOneResponse() = runTest {
658+
// Pins the FlagsSerializer.deserialize loop with array.size > 1 — every existing
659+
// payload in this file has exactly one flag, so the per-element decode and the
660+
// ArrayList(array.size) preallocation are not differentiated otherwise.
661+
val testDispatcher = UnconfinedTestDispatcher(testScheduler)
662+
val jsonPayload = """
663+
{
664+
"resolvedFlags": [
665+
{
666+
"flag": "flags/flag-a",
667+
"variant": "flags/flag-a/variants/v1",
668+
"value": { "n": 1 },
669+
"flagSchema": { "schema": { "n": { "intSchema": {} } } },
670+
"reason": "RESOLVE_REASON_MATCH",
671+
"shouldApply": true
672+
},
673+
{
674+
"flag": "flags/flag-b",
675+
"variant": "flags/flag-b/variants/v2",
676+
"value": { "s": "two" },
677+
"flagSchema": { "schema": { "s": { "stringSchema": {} } } },
678+
"reason": "RESOLVE_REASON_NO_SEGMENT_MATCH",
679+
"shouldApply": false
680+
},
681+
{
682+
"flag": "flags/flag-c",
683+
"variant": "",
684+
"value": null,
685+
"flagSchema": null,
686+
"reason": "RESOLVE_REASON_TARGETING_KEY_ERROR",
687+
"shouldApply": false
688+
}
689+
],
690+
"resolveToken": "token1"
691+
}
692+
""".trimIndent()
693+
694+
mockWebServer.enqueue(
695+
MockResponse()
696+
.setResponseCode(200)
697+
.setBody(jsonPayload)
698+
)
699+
val parsedResponse = RemoteFlagResolver(
700+
clientSecret = "",
701+
region = ConfidenceRegion.EUROPE,
702+
baseUrl = mockWebServer.url("/v1/flags:resolve"),
703+
dispatcher = testDispatcher,
704+
httpClient = OkHttpClient(),
705+
telemetry = Telemetry("", Telemetry.Library.CONFIDENCE, "")
706+
).resolve(listOf(), mapOf("targeting_key" to ConfidenceValue.String("user1")))
707+
708+
val expected = listOf(
709+
ResolvedFlag(
710+
flag = "flag-a",
711+
variant = "flags/flag-a/variants/v1",
712+
value = mutableMapOf("n" to ConfidenceValue.Integer(1)),
713+
reason = ResolveReason.RESOLVE_REASON_MATCH,
714+
shouldApply = true
715+
),
716+
ResolvedFlag(
717+
flag = "flag-b",
718+
variant = "flags/flag-b/variants/v2",
719+
value = mutableMapOf("s" to ConfidenceValue.String("two")),
720+
reason = ResolveReason.RESOLVE_REASON_NO_SEGMENT_MATCH,
721+
shouldApply = false
722+
),
723+
ResolvedFlag(
724+
flag = "flag-c",
725+
variant = "",
726+
reason = ResolveReason.RESOLVE_REASON_TARGETING_KEY_ERROR,
727+
shouldApply = false
728+
)
729+
)
730+
assertEquals(expected, (parsedResponse as Result.Success<FlagResolution>).data.flags)
731+
}
732+
733+
@Test
734+
fun testDeserializeAllResolveReasons() = runTest {
735+
// Both old (Json.decodeFromString<ResolveReason>) and new (ResolveReason.valueOf)
736+
// route through the enum entry name. Pin every declared value so any future divergence
737+
// — e.g. someone adding @SerialName to one entry — is caught.
738+
val testDispatcher = UnconfinedTestDispatcher(testScheduler)
739+
val reasons = ResolveReason.values()
740+
val jsonPayload = buildString {
741+
append("{ \"resolvedFlags\": [")
742+
reasons.forEachIndexed { i, r ->
743+
if (i > 0) append(",")
744+
append(
745+
"""
746+
{
747+
"flag": "flags/flag-$i",
748+
"variant": "",
749+
"value": null,
750+
"flagSchema": null,
751+
"reason": "${r.name}",
752+
"shouldApply": false
753+
}
754+
""".trimIndent()
755+
)
756+
}
757+
append("], \"resolveToken\": \"t\" }")
758+
}
759+
760+
mockWebServer.enqueue(
761+
MockResponse()
762+
.setResponseCode(200)
763+
.setBody(jsonPayload)
764+
)
765+
val parsedResponse = RemoteFlagResolver(
766+
clientSecret = "",
767+
region = ConfidenceRegion.EUROPE,
768+
baseUrl = mockWebServer.url("/v1/flags:resolve"),
769+
dispatcher = testDispatcher,
770+
httpClient = OkHttpClient(),
771+
telemetry = Telemetry("", Telemetry.Library.CONFIDENCE, "")
772+
).resolve(listOf(), mapOf("targeting_key" to ConfidenceValue.String("user1")))
773+
774+
val flags = (parsedResponse as Result.Success<FlagResolution>).data.flags
775+
assertEquals(reasons.size, flags.size)
776+
reasons.forEachIndexed { i, r ->
777+
assertEquals(r, flags[i].reason)
778+
}
779+
}
780+
781+
@Test
782+
fun testDeserializeNestedStructSchema() = runTest {
783+
// Pins the recursive convertToSchemaTypeValue / decodeFromJsonElement(SchemaTypeSerializer, ...)
784+
// path two levels deep. Existing tests only nest structSchema one level.
785+
val testDispatcher = UnconfinedTestDispatcher(testScheduler)
786+
val jsonPayload = """
787+
{
788+
"resolvedFlags": [
789+
{
790+
"flag": "flags/nested",
791+
"variant": "flags/nested/variants/v1",
792+
"value": {
793+
"outer": {
794+
"leaf_int": 1,
795+
"middle": {
796+
"leaf_str": "deep",
797+
"inner": {
798+
"leaf_bool": true,
799+
"leaf_double": 1.5
800+
}
801+
}
802+
}
803+
},
804+
"flagSchema": {
805+
"schema": {
806+
"outer": {
807+
"structSchema": {
808+
"schema": {
809+
"leaf_int": { "intSchema": {} },
810+
"middle": {
811+
"structSchema": {
812+
"schema": {
813+
"leaf_str": { "stringSchema": {} },
814+
"inner": {
815+
"structSchema": {
816+
"schema": {
817+
"leaf_bool": { "boolSchema": {} },
818+
"leaf_double": { "doubleSchema": {} }
819+
}
820+
}
821+
}
822+
}
823+
}
824+
}
825+
}
826+
}
827+
}
828+
}
829+
},
830+
"reason": "RESOLVE_REASON_MATCH",
831+
"shouldApply": true
832+
}
833+
],
834+
"resolveToken": "token1"
835+
}
836+
""".trimIndent()
837+
838+
mockWebServer.enqueue(
839+
MockResponse()
840+
.setResponseCode(200)
841+
.setBody(jsonPayload)
842+
)
843+
val parsedResponse = RemoteFlagResolver(
844+
clientSecret = "",
845+
region = ConfidenceRegion.EUROPE,
846+
baseUrl = mockWebServer.url("/v1/flags:resolve"),
847+
dispatcher = testDispatcher,
848+
httpClient = OkHttpClient(),
849+
telemetry = Telemetry("", Telemetry.Library.CONFIDENCE, "")
850+
).resolve(listOf(), mapOf("targeting_key" to ConfidenceValue.String("user1")))
851+
852+
val expected = ResolvedFlag(
853+
flag = "nested",
854+
variant = "flags/nested/variants/v1",
855+
value = mutableMapOf(
856+
"outer" to ConfidenceValue.Struct(
857+
mapOf(
858+
"leaf_int" to ConfidenceValue.Integer(1),
859+
"middle" to ConfidenceValue.Struct(
860+
mapOf(
861+
"leaf_str" to ConfidenceValue.String("deep"),
862+
"inner" to ConfidenceValue.Struct(
863+
mapOf(
864+
"leaf_bool" to ConfidenceValue.Boolean(true),
865+
"leaf_double" to ConfidenceValue.Double(1.5)
866+
)
867+
)
868+
)
869+
)
870+
)
871+
)
872+
),
873+
reason = ResolveReason.RESOLVE_REASON_MATCH,
874+
shouldApply = true
875+
)
876+
assertEquals(listOf(expected), (parsedResponse as Result.Success<FlagResolution>).data.flags)
877+
}
878+
656879
@Test
657880
fun testSerializeResolveRequest() = runTest {
658881
val testDispatcher = UnconfinedTestDispatcher(testScheduler)

0 commit comments

Comments
 (0)