Skip to content

Commit

Permalink
feat: add shouldApply to the apply call to depend on backend (#192)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nickybondarenko
nickybondarenko authored Feb 6, 2025
1 parent 37c243e commit 70598bd
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 21 deletions.
6 changes: 4 additions & 2 deletions Confidence/src/main/java/com/spotify/confidence/Confidence.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,12 @@ class Confidence internal constructor(
key,
default,
evaluationContext
) { flagName, resolveToken ->
) { flagName, resolveToken, shouldApply ->
// this lambda will be invoked inside the evaluation process
// and only if the resolve reason is not targeting key error.
apply(flagName, resolveToken)
if (shouldApply) {
apply(flagName, resolveToken)
}
}
// we are using a custom serializer so that the Json is serialized correctly in the logs
val newMap: Map<String, @Serializable(NetworkConfidenceValueSerializer::class) ConfidenceValue> =
Expand Down
4 changes: 2 additions & 2 deletions Confidence/src/main/java/com/spotify/confidence/ConfidenceFlagEvaluation.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fun <T> FlagResolution?.getEvaluation(
flag: String,
defaultValue: T,
context: Map<String, ConfidenceValue>,
applyFlag: (String, String) -> Unit = { _, _ -> }
applyFlag: (String, String, Boolean) -> Unit = { _, _, _ -> }
): Evaluation<T> {
val parsedKey = FlagKey(flag)
if (this == null) {
Expand All @@ -25,7 +25,7 @@ fun <T> FlagResolution?.getEvaluation(
)

if (resolvedFlag.reason != ResolveReason.RESOLVE_REASON_TARGETING_KEY_ERROR) {
applyFlag(parsedKey.flagName, resolveToken)
applyFlag(parsedKey.flagName, resolveToken, resolvedFlag.shouldApply)
} else {
return Evaluation(
value = defaultValue,
Expand Down
3 changes: 2 additions & 1 deletion Confidence/src/main/java/com/spotify/confidence/client/Types.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ data class ResolvedFlag(
val flag: String,
val variant: String,
val value: ConfidenceValueMap = mapOf(),
val reason: ResolveReason
val reason: ResolveReason,
val shouldApply: Boolean
)

sealed interface SchemaType {
Expand Down
7 changes: 5 additions & 2 deletions Confidence/src/main/java/com/spotify/confidence/serializers/Serializers.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ internal object NetworkResolvedFlagSerializer : KSerializer<ResolvedFlag> {
.replace("\"", "")

val resolvedReason = Json.decodeFromString<ResolveReason>(json["reason"].toString())
val shouldApply = Json.decodeFromString<Boolean>(json["shouldApply"].toString())
val flagSchemaJsonElement = json["flagSchema"]

val schemasJson = if (flagSchemaJsonElement != null && flagSchemaJsonElement != JsonNull) {
Expand All @@ -112,14 +113,16 @@ internal object NetworkResolvedFlagSerializer : KSerializer<ResolvedFlag> {
flag = flag,
variant = variant,
reason = resolvedReason,
value = values.map
value = values.map,
shouldApply = shouldApply
)
} else {
ResolvedFlag(
flag = flag,
variant = variant,
reason = resolvedReason,
value = mutableMapOf()
value = mutableMapOf(),
shouldApply = shouldApply
)
}
}
Expand Down
53 changes: 50 additions & 3 deletions Confidence/src/test/java/com/spotify/confidence/ConfidenceEvaluationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,19 @@ internal class ConfidenceEvaluationTest {
"test-kotlin-flag-1",
"flags/test-kotlin-flag-1/variants/variant-1",
resolvedValueAsMap,
ResolveReason.RESOLVE_REASON_MATCH
ResolveReason.RESOLVE_REASON_MATCH,
shouldApply = true
)
)
)
private val resolvedFlagsNoApply = Flags(
listOf(
ResolvedFlag(
"test-kotlin-flag-2",
"flags/test-kotlin-flag-2/variants/variant-1",
resolvedValueAsMap,
ResolveReason.RESOLVE_REASON_MATCH,
shouldApply = false
)
)
)
Expand Down Expand Up @@ -919,7 +931,8 @@ internal class ConfidenceEvaluationTest {
"test-kotlin-flag-1",
"",
mapOf(),
ResolveReason.RESOLVE_REASON_TARGETING_KEY_ERROR
ResolveReason.RESOLVE_REASON_TARGETING_KEY_ERROR,
shouldApply = true
)
)
)
Expand Down Expand Up @@ -965,7 +978,8 @@ internal class ConfidenceEvaluationTest {
flag = "test-kotlin-flag-1",
variant = "",
mapOf(),
reason
reason,
true
)
)
)
Expand Down Expand Up @@ -1097,6 +1111,39 @@ internal class ConfidenceEvaluationTest {
)
TestCase.assertEquals(ErrorCode.PARSE_ERROR, ex.errorCode)
}

@Test
fun testShouldApplyFalse() = runTest {
val testDispatcher = UnconfinedTestDispatcher(testScheduler)
val context = mapOf("targeting_key" to ConfidenceValue.String("foo"))
val flagResolver: FlagResolver = mock()
val mockConfidence = getConfidence(
testDispatcher,
initialContext = context,
flagResolver = flagResolver
)
whenever(
flagResolver.resolve(
any(),
eq(context)
)
).thenReturn(
Result.Success(
FlagResolution(
context,
resolvedFlagsNoApply.list,
"token1"
)
)
)
mockConfidence.fetchAndActivate()
advanceUntilIdle()
val evalString = mockConfidence.getFlag("test-kotlin-flag-2.mystring", "default")

// Resolve is correct, but no Apply sent
TestCase.assertEquals("red", evalString.value)
verify(flagApplierClient, times(0)).apply(any(), any())
}
}

private const val cacheFileData = "{\n" +
Expand Down
3 changes: 2 additions & 1 deletion Confidence/src/test/java/com/spotify/confidence/ConfidenceIntegrationTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ class ConfidenceIntegrationTests {
"my-integer" to ConfidenceValue.Integer(
storedValue
)
)
),
shouldApply = true
)
)

Expand Down
27 changes: 18 additions & 9 deletions Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ internal class ConfidenceRemoteClientTests {
" }\n" +
" }\n" +
" },\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\"\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\",\n" +
" \"shouldApply\": true\n" +
" }\n" +
" ],\n" +
" \"resolveToken\": \"token1\"\n" +
Expand Down Expand Up @@ -132,7 +133,8 @@ internal class ConfidenceRemoteClientTests {
)
)
),
ResolveReason.RESOLVE_REASON_MATCH
ResolveReason.RESOLVE_REASON_MATCH,
shouldApply = true
)
)
)
Expand All @@ -144,7 +146,7 @@ internal class ConfidenceRemoteClientTests {
}

@Test
fun testDeserializeResolveResponseNoMatch() = runTest {
fun testDeserializeResolveResponseNoApply() = runTest {
val testDispatcher = UnconfinedTestDispatcher(testScheduler)
val jsonPayload = "{\n" +
" \"resolvedFlags\": [\n" +
Expand All @@ -153,7 +155,8 @@ internal class ConfidenceRemoteClientTests {
" \"variant\": \"\",\n" +
" \"value\": null,\n" +
" \"flagSchema\": null,\n" +
" \"reason\": \"RESOLVE_REASON_NO_SEGMENT_MATCH\"\n" +
" \"reason\": \"RESOLVE_REASON_NO_SEGMENT_MATCH\",\n" +
" \"shouldApply\": false\n" +
" }\n" +
" ],\n" +
" \"resolveToken\": \"token1\"\n" +
Expand All @@ -179,7 +182,8 @@ internal class ConfidenceRemoteClientTests {
ResolvedFlag(
flag = "test-kotlin-flag-1",
variant = "",
reason = com.spotify.confidence.ResolveReason.RESOLVE_REASON_NO_SEGMENT_MATCH
reason = com.spotify.confidence.ResolveReason.RESOLVE_REASON_NO_SEGMENT_MATCH,
shouldApply = false
)
)
),
Expand Down Expand Up @@ -207,6 +211,7 @@ internal class ConfidenceRemoteClientTests {
" }\n" +
" }\n" +
" },\n" +
" \"shouldApply\": true,\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\"\n" +
" }\n" +
" ],\n" +
Expand Down Expand Up @@ -237,7 +242,8 @@ internal class ConfidenceRemoteClientTests {
mutableMapOf(
"mydouble" to ConfidenceValue.Double(3.0)
),
com.spotify.confidence.ResolveReason.RESOLVE_REASON_MATCH
com.spotify.confidence.ResolveReason.RESOLVE_REASON_MATCH,
shouldApply = true
)
)
),
Expand All @@ -263,7 +269,8 @@ internal class ConfidenceRemoteClientTests {
" }\n" +
" }\n" +
" },\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\"\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\",\n" +
" \"shouldApply\": true\n" +
" }\n" +
" ],\n" +
" \"resolveToken\": \"token1\"\n" +
Expand Down Expand Up @@ -308,7 +315,8 @@ internal class ConfidenceRemoteClientTests {
" }\n" +
" }\n" +
" },\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\"\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\",\n" +
" \"shouldApply\": true\n" +
" }\n" +
" ],\n" +
" \"resolveToken\": \"token1\"\n" +
Expand Down Expand Up @@ -393,7 +401,8 @@ internal class ConfidenceRemoteClientTests {
" }\n" +
" }\n" +
" },\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\"\n" +
" \"reason\": \"RESOLVE_REASON_MATCH\",\n" +
" \"shouldApply\": true\n" +
" }\n" +
" ],\n" +
" \"resolveToken\": \"token1\"\n" +
Expand Down
3 changes: 2 additions & 1 deletion Confidence/src/test/java/com/spotify/confidence/StorageFileCacheTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class StorageFileCacheTests {
),
"mynull" to ConfidenceValue.Null
),
ResolveReason.RESOLVE_REASON_MATCH
ResolveReason.RESOLVE_REASON_MATCH,
shouldApply = true
)
)
)
Expand Down

0 comments on commit 70598bd

Please sign in to comment.