Skip to content

Commit 4ba03b8

Browse files
fabriziodemariafabriziodemaria
authored
fix: Immutability of context (#151)
Signed-off-by: Fabrizio Demaria <[email protected]>
1 parent d8b58b4 commit 4ba03b8

File tree

10 files changed

+355
-12
lines changed

10 files changed

+355
-12
lines changed

‎android/api/android.api‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,8 @@ public final class dev/openfeature/sdk/Value$Integer : dev/openfeature/sdk/Value
557557
}
558558

559559
public final class dev/openfeature/sdk/Value$List : dev/openfeature/sdk/Value {
560-
public fun <init> (Ljava/util/List;)V
560+
public static final field Companion Ldev/openfeature/sdk/Value$List$Companion;
561+
public synthetic fun <init> (Ljava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
561562
public fun asBoolean ()Ljava/lang/Boolean;
562563
public fun asDate ()Ljava/util/Date;
563564
public fun asDouble ()Ljava/lang/Double;
@@ -575,6 +576,10 @@ public final class dev/openfeature/sdk/Value$List : dev/openfeature/sdk/Value {
575576
public fun toString ()Ljava/lang/String;
576577
}
577578

579+
public final class dev/openfeature/sdk/Value$List$Companion {
580+
public final fun invoke (Ljava/util/List;)Ldev/openfeature/sdk/Value$List;
581+
}
582+
578583
public final class dev/openfeature/sdk/Value$Null : dev/openfeature/sdk/Value {
579584
public static final field INSTANCE Ldev/openfeature/sdk/Value$Null;
580585
public fun asBoolean ()Ljava/lang/Boolean;
@@ -609,7 +614,8 @@ public final class dev/openfeature/sdk/Value$String : dev/openfeature/sdk/Value
609614
}
610615

611616
public final class dev/openfeature/sdk/Value$Structure : dev/openfeature/sdk/Value {
612-
public fun <init> (Ljava/util/Map;)V
617+
public static final field Companion Ldev/openfeature/sdk/Value$Structure$Companion;
618+
public synthetic fun <init> (Ljava/util/Map;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
613619
public fun asBoolean ()Ljava/lang/Boolean;
614620
public fun asDate ()Ljava/util/Date;
615621
public fun asDouble ()Ljava/lang/Double;
@@ -627,6 +633,10 @@ public final class dev/openfeature/sdk/Value$Structure : dev/openfeature/sdk/Val
627633
public fun toString ()Ljava/lang/String;
628634
}
629635

636+
public final class dev/openfeature/sdk/Value$Structure$Companion {
637+
public final fun invoke (Ljava/util/Map;)Ldev/openfeature/sdk/Value$Structure;
638+
}
639+
630640
public abstract interface class dev/openfeature/sdk/events/OpenFeatureProviderEvents {
631641
}
632642

‎android/src/main/java/dev/openfeature/sdk/ImmutableContext.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ class ImmutableContext(
44
private val targetingKey: String = "",
55
attributes: Map<String, Value> = mapOf()
66
) : EvaluationContext {
7-
private val structure: ImmutableStructure = ImmutableStructure(attributes)
7+
private val structure: ImmutableStructure = ImmutableStructure(attributes.toMap())
88
override fun getTargetingKey(): String {
99
return targetingKey
1010
}

‎android/src/main/java/dev/openfeature/sdk/ImmutableStructure.kt‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package dev.openfeature.sdk
22

3-
class ImmutableStructure(private val attributes: Map<String, Value> = mapOf()) : Structure {
3+
class ImmutableStructure(attributes: Map<String, Value> = mapOf()) : Structure {
4+
private val attributes: Map<String, Value> = attributes.toMap()
5+
46
constructor(vararg pairs: Pair<String, Value>) : this(pairs.toMap())
57

68
override fun keySet(): Set<String> {
@@ -12,22 +14,22 @@ class ImmutableStructure(private val attributes: Map<String, Value> = mapOf()) :
1214
}
1315

1416
override fun asMap(): Map<String, Value> {
15-
return attributes
17+
return attributes.toMap()
1618
}
1719

1820
override fun asObjectMap(): Map<String, Any?> {
19-
return attributes.mapValues { convertValue(it.value) }
21+
return attributes.mapValues { convertValue(it.value) }.toMap()
2022
}
2123

2224
private fun convertValue(value: Value): Any? {
2325
return when (value) {
24-
is Value.List -> value.list.map { t -> convertValue(t) }
25-
is Value.Structure -> value.structure.mapValues { t -> convertValue(t.value) }
26+
is Value.List -> value.list.map { t -> convertValue(t) }.toList()
27+
is Value.Structure -> value.structure.mapValues { t -> convertValue(t.value) }.toMap()
2628
is Value.Null -> return null
2729
is Value.String -> value.asString()
2830
is Value.Boolean -> value.asBoolean()
2931
is Value.Integer -> value.asInteger()
30-
is Value.Date -> value.asDate()
32+
is Value.Date -> value.asDate()?.clone()
3133
is Value.Double -> value.asDouble()
3234
}
3335
}

‎android/src/main/java/dev/openfeature/sdk/Value.kt‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,41 @@ sealed interface Value {
2121

2222
data class Date(val date: java.util.Date) : Value
2323

24-
data class Structure(val structure: Map<kotlin.String, Value>) : Value
24+
data class Structure private constructor(val structure: Map<kotlin.String, Value>) : Value {
25+
companion object {
26+
private fun deepCopyValue(value: Value): Value {
27+
return when (value) {
28+
is Structure -> Structure(value.structure.mapValues { (_, v) -> deepCopyValue(v) })
29+
is List -> List(value.list.map { deepCopyValue(it) })
30+
is Date -> Date(value.date.clone() as java.util.Date)
31+
else -> value
32+
}
33+
}
2534

26-
data class List(val list: kotlin.collections.List<Value>) : Value
35+
operator fun invoke(structure: Map<kotlin.String, Value>): Structure {
36+
val deepCopiedStructure = structure.mapValues { (_, value) -> deepCopyValue(value) }
37+
return Structure(deepCopiedStructure)
38+
}
39+
}
40+
}
41+
42+
data class List private constructor(val list: kotlin.collections.List<Value>) : Value {
43+
companion object {
44+
private fun deepCopyValue(value: Value): Value {
45+
return when (value) {
46+
is Structure -> Structure(value.structure.mapValues { (_, v) -> deepCopyValue(v) })
47+
is List -> List(value.list.map { deepCopyValue(it) })
48+
is Date -> Date(value.date.clone() as java.util.Date)
49+
else -> value
50+
}
51+
}
52+
53+
operator fun invoke(list: kotlin.collections.List<Value>): List {
54+
val deepCopiedList = list.map { value -> deepCopyValue(value) }
55+
return List(deepCopiedList)
56+
}
57+
}
58+
}
2759

2860
object Null : Value {
2961
override fun equals(other: Any?): kotlin.Boolean {

‎android/src/test/java/dev/openfeature/sdk/DeepCopyTest.kt‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package dev.openfeature.sdk
2+
3+
import org.junit.Assert.assertEquals
4+
import org.junit.Assert.assertNull
5+
import org.junit.Test
6+
7+
class DeepCopyTest {
8+
9+
@Test
10+
fun testDeepCopyOfStructure() {
11+
val nestedMap = mutableMapOf<String, Value>()
12+
nestedMap["key1"] = Value.String("value1")
13+
14+
val structure = Value.Structure(nestedMap)
15+
16+
nestedMap["key1"] = Value.String("modified")
17+
nestedMap["key2"] = Value.Integer(42)
18+
19+
assertEquals("value1", structure.structure["key1"]?.asString())
20+
assertNull(structure.structure["key2"])
21+
}
22+
23+
@Test
24+
fun testDeepCopyOfList() {
25+
val nestedList = mutableListOf<Value>()
26+
nestedList.add(Value.String("item1"))
27+
28+
val list = Value.List(nestedList)
29+
30+
nestedList[0] = Value.String("modified")
31+
nestedList.add(Value.Integer(42))
32+
33+
assertEquals("item1", list.list[0]?.asString())
34+
assertEquals(1, list.list.size)
35+
}
36+
37+
@Test
38+
fun testDeepCopyOfNestedStructures() {
39+
val innerMap = mutableMapOf<String, Value>()
40+
innerMap["innerKey"] = Value.String("innerValue")
41+
42+
val outerMap = mutableMapOf<String, Value>()
43+
outerMap["outerKey"] = Value.Structure(innerMap)
44+
45+
val structure = Value.Structure(outerMap)
46+
47+
innerMap["innerKey"] = Value.String("modified")
48+
innerMap["newKey"] = Value.Integer(123)
49+
50+
val outerStructure = structure.structure["outerKey"]?.asStructure()
51+
assertEquals("innerValue", outerStructure?.get("innerKey")?.asString())
52+
assertNull(outerStructure?.get("newKey"))
53+
}
54+
}

‎android/src/test/java/dev/openfeature/sdk/DeveloperExperienceTests.kt‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import dev.openfeature.sdk.helpers.DoSomethingProvider
99
import dev.openfeature.sdk.helpers.GenericSpyHookMock
1010
import dev.openfeature.sdk.helpers.OverlyEmittingProvider
1111
import dev.openfeature.sdk.helpers.SlowProvider
12+
import dev.openfeature.sdk.helpers.SpyProvider
1213
import kotlinx.coroutines.CoroutineScope
1314
import kotlinx.coroutines.ExperimentalCoroutinesApi
1415
import kotlinx.coroutines.async
@@ -367,4 +368,66 @@ class DeveloperExperienceTests {
367368
staleEvents
368369
)
369370
}
371+
372+
@Test
373+
fun setEvaluationContextDoesDeepEqualsOnAttributes() = runTest {
374+
val map = mutableMapOf<String, Value>()
375+
val provider = SpyProvider()
376+
OpenFeatureAPI.setProviderAndWait(provider)
377+
assertEquals(1, provider.initializeCalls.size)
378+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
379+
assertEquals(1, provider.onContextSetCalls.size)
380+
map["myKey"] = Value.String("myValue")
381+
println(OpenFeatureAPI.getEvaluationContext()?.asMap())
382+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
383+
assertEquals(2, provider.onContextSetCalls.size)
384+
}
385+
386+
@Test
387+
fun setEvaluationContextDoesNotDetectDeepChangesInNestedStructures() = runTest {
388+
val nestedMap = mutableMapOf<String, Value>()
389+
val map = mutableMapOf<String, Value>()
390+
map["nested"] = Value.Structure(nestedMap)
391+
392+
val provider = SpyProvider()
393+
OpenFeatureAPI.setProviderAndWait(provider)
394+
assertEquals(1, provider.initializeCalls.size)
395+
396+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
397+
assertEquals(1, provider.onContextSetCalls.size)
398+
399+
// This won't update the `nested` list in `map`, since Value.Structure returns an immutable copy
400+
nestedMap["deepKey"] = Value.String("deepValue")
401+
402+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
403+
assertEquals(1, provider.onContextSetCalls.size)
404+
405+
val currentContext = OpenFeatureAPI.getEvaluationContext()
406+
val nestedStructure = currentContext?.getValue("nested")?.asStructure()
407+
Assert.assertNull(nestedStructure?.get("deepKey")?.asString())
408+
}
409+
410+
@Test
411+
fun setEvaluationContextDoesNotDetectDeepChangesInNestedLists() = runTest {
412+
val nestedList = mutableListOf<Value>()
413+
val map = mutableMapOf<String, Value>()
414+
map["nested"] = Value.List(nestedList)
415+
416+
val provider = SpyProvider()
417+
OpenFeatureAPI.setProviderAndWait(provider)
418+
assertEquals(1, provider.initializeCalls.size)
419+
420+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
421+
assertEquals(1, provider.onContextSetCalls.size)
422+
423+
// This won't update the `nested` list in `map`, since Value.List returns an immutable copy
424+
nestedList.add(Value.String("deepValue"))
425+
426+
OpenFeatureAPI.setEvaluationContextAndWait(ImmutableContext(attributes = map))
427+
assertEquals(1, provider.onContextSetCalls.size)
428+
429+
val currentContext = OpenFeatureAPI.getEvaluationContext()
430+
val nestedListFromContext = currentContext?.getValue("nested")?.asList()
431+
assertEquals(0, nestedListFromContext?.size)
432+
}
370433
}

‎android/src/test/java/dev/openfeature/sdk/EvalContextTests.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,23 @@ class EvalContextTests {
167167

168168
Assert.assertEquals(ctx1, ctx2)
169169
}
170+
171+
@Test
172+
fun testContextIsTrulyImmutable() {
173+
val mutableAttributes = mutableMapOf("key1" to Value.String("value1"), "key2" to Value.Integer(42))
174+
val context = ImmutableContext("targetingKey", mutableAttributes)
175+
176+
// Verify initial state
177+
Assert.assertEquals("value1", context.getValue("key1")?.asString())
178+
Assert.assertEquals(42, context.getValue("key2")?.asInteger())
179+
180+
// Modify the original mutable map
181+
mutableAttributes["key1"] = Value.String("modified")
182+
mutableAttributes["key3"] = Value.Boolean(true)
183+
184+
// Verify that the context is not affected by the modifications
185+
Assert.assertEquals("value1", context.getValue("key1")?.asString())
186+
Assert.assertEquals(42, context.getValue("key2")?.asInteger())
187+
Assert.assertNull(context.getValue("key3"))
188+
}
170189
}

‎android/src/test/java/dev/openfeature/sdk/ImmutableContextTest.kt‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package dev.openfeature.sdk
2+
3+
import org.junit.Assert.assertEquals
4+
import org.junit.Assert.assertNull
5+
import org.junit.Assert.fail
6+
import org.junit.Test
7+
8+
class ImmutableContextTest {
9+
10+
@Test
11+
fun `should be immutable - modifications to input map should not affect context`() {
12+
val mutableAttributes = mutableMapOf("key1" to Value.String("value1"), "key2" to Value.Integer(42))
13+
14+
val context = ImmutableContext("targetingKey", mutableAttributes)
15+
16+
assertEquals("value1", context.getValue("key1")?.asString())
17+
assertEquals(42, context.getValue("key2")?.asInteger())
18+
19+
// Modify the original mutable map
20+
mutableAttributes["key1"] = Value.String("modified")
21+
mutableAttributes["key3"] = Value.Boolean(true)
22+
23+
// Verify that the context is not affected by the modifications
24+
assertEquals("value1", context.getValue("key1")?.asString())
25+
assertEquals(42, context.getValue("key2")?.asInteger())
26+
assertNull(context.getValue("key3"))
27+
}
28+
29+
@Test
30+
fun `should be immutable - modifications to returned map should not affect context`() {
31+
val attributes = mapOf("key1" to Value.String("value1"))
32+
val context = ImmutableContext("targetingKey", attributes)
33+
34+
val returnedMap = context.asMap()
35+
try {
36+
if (returnedMap is MutableMap) {
37+
returnedMap["key2"] = Value.String("newValue")
38+
fail("Returned map should be immutable")
39+
}
40+
} catch (_: UnsupportedOperationException) {
41+
}
42+
43+
assertEquals("value1", context.getValue("key1")?.asString())
44+
assertNull(context.getValue("key2"))
45+
}
46+
47+
@Test
48+
fun `should be immutable - nested structures should be immutable`() {
49+
val nestedMap = mutableMapOf("nestedKey" to Value.String("nestedValue"))
50+
val attributes = mapOf("key1" to Value.Structure(nestedMap))
51+
val context = ImmutableContext("targetingKey", attributes)
52+
53+
nestedMap["nestedKey"] = Value.String("modified")
54+
nestedMap["newNestedKey"] = Value.String("newValue")
55+
56+
val contextNestedStructure = context.getValue("key1")?.asStructure()
57+
58+
assertEquals("nestedValue", contextNestedStructure?.get("nestedKey")?.asString())
59+
assertNull(contextNestedStructure?.get("newNestedKey"))
60+
}
61+
62+
@Test
63+
fun `should be immutable - nested lists should be immutable`() {
64+
val nestedList = mutableListOf(Value.String("item1"), Value.Integer(42))
65+
val attributes = mapOf("key1" to Value.List(nestedList))
66+
val context = ImmutableContext("targetingKey", attributes)
67+
68+
nestedList[0] = Value.String("modified")
69+
nestedList.add(Value.Boolean(true))
70+
71+
val contextNestedList = context.getValue("key1")?.asList()
72+
73+
assertEquals("item1", contextNestedList?.get(0)?.asString())
74+
assertEquals(42, contextNestedList?.get(1)?.asInteger())
75+
assertEquals(2, contextNestedList?.size)
76+
}
77+
}

‎android/src/test/java/dev/openfeature/sdk/StructureTests.kt‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ class StructureTests {
1818
val structure = ImmutableStructure(map)
1919

2020
Assert.assertEquals("test", structure.getValue("key")?.asString())
21-
Assert.assertEquals(map, structure.asMap())
21+
// The structure should contain the same content as the input map, but not necessarily the same map reference
22+
Assert.assertEquals(map.toMap(), structure.asMap())
23+
24+
// Verify that modifying the original map doesn't affect the structure
25+
map["key"] = Value.String("modified")
26+
Assert.assertEquals("test", structure.getValue("key")?.asString())
2227
}
2328

2429
@Test
@@ -55,5 +60,13 @@ class StructureTests {
5560
val structure2 = ImmutableStructure(map2)
5661

5762
Assert.assertEquals(structure1, structure2)
63+
64+
// Verify that modifying the original maps doesn't affect the structures
65+
map["key"] = Value.String("modified1")
66+
map2["key"] = Value.String("modified2")
67+
68+
Assert.assertEquals(structure1, structure2)
69+
Assert.assertEquals("test", structure1.getValue("key")?.asString())
70+
Assert.assertEquals("test", structure2.getValue("key")?.asString())
5871
}
5972
}

0 commit comments

Comments
 (0)