Skip to content

fix: MutableContext and ImmutableContext merge are made recursive #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 8, 2023
7 changes: 4 additions & 3 deletions src/main/java/dev/openfeature/sdk/ImmutableContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ public EvaluationContext merge(EvaluationContext overridingContext) {
if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
newTargetingKey = overridingContext.getTargetingKey();
}
Map<String, Value> merged = new HashMap<>();

merged.putAll(this.asMap());
merged.putAll(overridingContext.asMap());
Map<String, Value> merged = this.merge(m -> new ImmutableStructure(m),
this.asMap(),
overridingContext.asMap());

return new ImmutableContext(newTargetingKey, merged);
}
}
20 changes: 13 additions & 7 deletions src/main/java/dev/openfeature/sdk/MutableContext.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package dev.openfeature.sdk;

import java.time.Instant;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -92,18 +91,25 @@ public EvaluationContext merge(EvaluationContext overridingContext) {
return new MutableContext(this.asMap());
}

Map<String, Value> merged = new HashMap<String, Value>();
Map<String, Value> merged = this.merge(map -> new MutableStructure(map),
this.asMap(),
overridingContext.asMap());

merged.putAll(this.asMap());
merged.putAll(overridingContext.asMap());
EvaluationContext ec = new MutableContext(merged);
String newTargetingKey = "";

if (this.getTargetingKey() != null && !this.getTargetingKey().trim().equals("")) {
ec.setTargetingKey(this.getTargetingKey());
newTargetingKey = this.getTargetingKey();
}

if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
ec.setTargetingKey(overridingContext.getTargetingKey());
newTargetingKey = overridingContext.getTargetingKey();
}

EvaluationContext ec = null;
if (newTargetingKey != null && !newTargetingKey.trim().equals("")) {
ec = new MutableContext(newTargetingKey, merged);
} else {
ec = new MutableContext(merged);
}

return ec;
Expand Down
32 changes: 32 additions & 0 deletions src/main/java/dev/openfeature/sdk/Structure.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

import dev.openfeature.sdk.exceptions.ValueNotConvertableError;

import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.Map.Entry;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -90,4 +93,33 @@ default Object convertValue(Value value) {
}
throw new ValueNotConvertableError();
}

/**
* Recursively merges the base map with the overriding map.
*
* @param <T> Structure type
* @param newStructure function to create the right structure
* @param base base map to merge
* @param overriding overriding map to merge
* @return resulting merged map
*/
default <T extends Structure> Map<String, Value> merge(Function<Map<String, Value>, Structure> newStructure,
Map<String, Value> base,
Map<String, Value> overriding) {
Map<String, Value> merged = new HashMap<>();

merged.putAll(base);
for (Entry<String, Value> overridingEntry : overriding.entrySet()) {
String key = overridingEntry.getKey();
if (overridingEntry.getValue().isStructure() && merged.containsKey(key) && merged.get(key).isStructure()) {
Structure mergedValue = merged.get(key).asStructure();
Structure overridingValue = overridingEntry.getValue().asStructure();
Map<String, Value> newMap = this.merge(newStructure, mergedValue.asMap(), overridingValue.asMap());
merged.put(key, new Value(newStructure.apply(newMap)));
} else {
merged.put(key, overridingEntry.getValue());
}
}
return merged;
}
}
50 changes: 50 additions & 0 deletions src/test/java/dev/openfeature/sdk/ImmutableContextTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

class ImmutableContextTest {

Expand Down Expand Up @@ -64,4 +65,53 @@ void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() {
assertEquals("targeting_key", merge.getTargetingKey());
assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());
}

@DisplayName("Merge should retain subkeys from the existing context when the overriding context has the same targeting key")
@Test
void mergeShouldRetainItsSubkeysWhenOverridingContextHasTheSameKey() {
HashMap<String, Value> attributes = new HashMap<>();
HashMap<String, Value> overridingAttributes = new HashMap<>();
HashMap<String, Value> key1Attributes = new HashMap<>();
HashMap<String, Value> ovKey1Attributes = new HashMap<>();

key1Attributes.put("key1_1", new Value("val1_1"));
attributes.put("key1", new Value(new ImmutableStructure(key1Attributes)));
attributes.put("key2", new Value("val2"));
ovKey1Attributes.put("overriding_key1_1", new Value("overriding_val_1_1"));
overridingAttributes.put("key1", new Value(new ImmutableStructure(ovKey1Attributes)));

EvaluationContext ctx = new ImmutableContext("targeting_key", attributes);
EvaluationContext overriding = new ImmutableContext("targeting_key", overridingAttributes);
EvaluationContext merge = ctx.merge(overriding);
assertEquals("targeting_key", merge.getTargetingKey());
assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());

Value key1 = merge.getValue("key1");
assertTrue(key1.isStructure());

Structure value = key1.asStructure();
assertArrayEquals(new Object[]{"key1_1","overriding_key1_1"}, value.keySet().toArray());
}

@DisplayName("Merge should retain subkeys from the existing context when the overriding context doesn't have targeting key")
@Test
void mergeShouldRetainItsSubkeysWhenOverridingContextHasNoTargetingKey() {
HashMap<String, Value> attributes = new HashMap<>();
HashMap<String, Value> key1Attributes = new HashMap<>();

key1Attributes.put("key1_1", new Value("val1_1"));
attributes.put("key1", new Value(new ImmutableStructure(key1Attributes)));
attributes.put("key2", new Value("val2"));

EvaluationContext ctx = new ImmutableContext(attributes);
EvaluationContext overriding = new ImmutableContext();
EvaluationContext merge = ctx.merge(overriding);
assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());

Value key1 = merge.getValue("key1");
assertTrue(key1.isStructure());

Structure value = key1.asStructure();
assertArrayEquals(new Object[]{"key1_1"}, value.keySet().toArray());
}
}