Skip to content

Commit bebac37

Browse files
khvn26khvn26
committed
use latest schemas and engine-test-data
 - switch to latest engine-test-data (with the segment conditions null values fix) - switch to latest EvaluationResult schema - fix a case with null context value being coerced to string for the IN operator - fix weight display in SPLIT reasons
1 parent 4730f9a commit bebac37

File tree

7 files changed

+113
-115
lines changed

7 files changed

+113
-115
lines changed

‎.gitmodules‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
[submodule "src/test/java/com/flagsmith/flagengine/enginetestdata"]
22
path = src/test/java/com/flagsmith/flagengine/enginetestdata
33
url = [email protected]:Flagsmith/engine-test-data.git
4-
branch = feat/context-values
4+
branch = fix/schema-errors

‎src/main/java/com/flagsmith/flagengine/Engine.java‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class Engine {
1717
public static EvaluationResult getEvaluationResult(EvaluationContext context) {
1818
List<SegmentResult> segments = new ArrayList<>();
1919
HashMap<String, ImmutablePair<String, FeatureContext>> segmentFeatureContexts = new HashMap<>();
20-
List<FlagResult> flags = new ArrayList<>();
20+
Flags flags = new Flags();
2121

2222
for (SegmentContext segmentContext : context.getSegments().getAdditionalProperties().values()) {
2323
if (SegmentEvaluator.isContextInSegment(context, segmentContext)) {
@@ -58,22 +58,29 @@ public static EvaluationResult getEvaluationResult(EvaluationContext context) {
5858
? context.getIdentity().getKey()
5959
: null;
6060

61-
for (FeatureContext featureContext : context.getFeatures().getAdditionalProperties().values()) {
62-
if (segmentFeatureContexts.containsKey(featureContext.getFeatureKey())) {
63-
ImmutablePair<String, FeatureContext> segmentNameWithFeatureContext = segmentFeatureContexts
64-
.get(featureContext.getFeatureKey());
65-
featureContext = segmentNameWithFeatureContext.getRight();
66-
flags.add(new FlagResult().withEnabled(featureContext.getEnabled())
67-
.withFeatureKey(featureContext.getFeatureKey())
68-
.withName(featureContext.getName())
69-
.withValue(featureContext.getValue())
70-
.withReason("TARGETING_MATCH; segment=" + segmentNameWithFeatureContext.getLeft()));
71-
} else {
72-
flags.add(getFlagResultFromFeatureContext(featureContext, identityKey));
61+
Features contextFeatures = context.getFeatures();
62+
if (contextFeatures != null) {
63+
for (FeatureContext featureContext : contextFeatures.getAdditionalProperties().values()) {
64+
if (segmentFeatureContexts.containsKey(featureContext.getFeatureKey())) {
65+
ImmutablePair<String, FeatureContext> segmentNameFeaturePair = segmentFeatureContexts
66+
.get(featureContext.getFeatureKey());
67+
featureContext = segmentNameFeaturePair.getRight();
68+
flags.setAdditionalProperty(
69+
featureContext.getName(),
70+
new FlagResult().withEnabled(featureContext.getEnabled())
71+
.withFeatureKey(featureContext.getFeatureKey())
72+
.withName(featureContext.getName())
73+
.withValue(featureContext.getValue())
74+
.withReason(
75+
"TARGETING_MATCH; segment=" + segmentNameFeaturePair.getLeft()));
76+
} else {
77+
flags.setAdditionalProperty(featureContext.getName(),
78+
getFlagResultFromFeatureContext(featureContext, identityKey));
79+
}
7380
}
7481
}
7582

76-
return new EvaluationResult().withContext(context).withFlags(flags).withSegments(segments);
83+
return new EvaluationResult().withFlags(flags).withSegments(segments);
7784
}
7885

7986
private static FlagResult getFlagResultFromFeatureContext(
@@ -95,7 +102,7 @@ private static FlagResult getFlagResultFromFeatureContext(
95102
.withFeatureKey(featureContext.getFeatureKey())
96103
.withName(featureContext.getName())
97104
.withValue(variant.getValue())
98-
.withReason("SPLIT; weight=" + weight);
105+
.withReason("SPLIT; weight=" + weight.intValue());
99106
}
100107
startPercentage = limit;
101108
}

‎src/main/java/com/flagsmith/flagengine/SegmentCondition.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
package com.flagsmith.flagengine;
22

3+
import com.fasterxml.jackson.annotation.JsonSetter;
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.fasterxml.jackson.databind.node.ArrayNode;
36
import com.flagsmith.flagengine.segments.constants.SegmentConditions;
47
import java.util.List;
8+
import java.util.stream.Collectors;
9+
import java.util.stream.StreamSupport;
510
import lombok.Data;
611

712
@Data
@@ -41,6 +46,32 @@ public SegmentCondition(SegmentConditions operator, String property, Object valu
4146
this.value = value;
4247
}
4348

49+
/**
50+
* Set JsonNode value. Required for engine tests.
51+
*
52+
* @param node the JsonNode value.
53+
* @throws IllegalArgumentException if value is not a String or List of Strings
54+
*/
55+
@JsonSetter("value")
56+
public void setValue(JsonNode node) {
57+
if (node.isArray()) {
58+
ArrayNode arr = (ArrayNode) node;
59+
List<String> values = StreamSupport.stream(arr.spliterator(), false)
60+
.peek(el -> {
61+
if (!el.isTextual()) {
62+
throw new IllegalArgumentException("Array elements must be strings");
63+
}
64+
})
65+
.map(JsonNode::asText)
66+
.collect(Collectors.toList());
67+
this.setValue(values);
68+
} else if (node.isTextual()) {
69+
this.setValue(node.asText());
70+
} else {
71+
throw new IllegalArgumentException("Value must be a String or List of Strings");
72+
}
73+
}
74+
4475
/**
4576
* Set String value.
4677
*

‎src/main/java/com/flagsmith/flagengine/segments/SegmentEvaluator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ private static Boolean contextMatchesCondition(
102102
}
103103
}
104104

105-
if (!(contextValue instanceof Boolean)) {
105+
if (!(contextValue instanceof Boolean) && contextValue != null) {
106106
contextValue = String.valueOf(contextValue);
107107
}
108108

‎src/main/java/com/flagsmith/models/Flags.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
import com.flagsmith.exceptions.FeatureNotFoundError;
66
import com.flagsmith.exceptions.FlagsmithClientError;
77
import com.flagsmith.flagengine.EvaluationResult;
8+
import com.flagsmith.flagengine.FlagResult;
89
import com.flagsmith.interfaces.DefaultFlagHandler;
910
import com.flagsmith.threads.AnalyticsProcessor;
1011
import java.util.HashMap;
1112
import java.util.List;
1213
import java.util.Map;
14+
import java.util.Map.Entry;
1315
import java.util.stream.Collectors;
1416
import lombok.Data;
1517

@@ -22,7 +24,7 @@ public class Flags {
2224
/**
2325
* Build flags object from list of feature states.
2426
*
25-
* @param featureStates list of feature states
27+
* @param featureStates list of feature states
2628
* @param analyticsProcessor instance of analytics processor
2729
*/
2830
public static Flags fromFeatureStateModels(
@@ -34,7 +36,7 @@ public static Flags fromFeatureStateModels(
3436
/**
3537
* Build flags object from list of feature states.
3638
*
37-
* @param featureStates list of feature states
39+
* @param featureStates list of feature states
3840
* @param analyticsProcessor instance of analytics processor
3941
* @param defaultFlagHandler default flags (optional)
4042
*/
@@ -60,7 +62,7 @@ public static Flags fromFeatureStateModels(
6062
/**
6163
* Return the flags instance.
6264
*
63-
* @param apiFlags Dictionary with api flags
65+
* @param apiFlags Dictionary with api flags
6466
* @param analyticsProcessor instance of analytics processor
6567
* @param defaultFlagHandler handler for default flags if present
6668
*/
@@ -88,7 +90,7 @@ public static Flags fromApiFlags(
8890
/**
8991
* Return the flags instance.
9092
*
91-
* @param apiFlags Dictionary with api flags
93+
* @param apiFlags Dictionary with api flags
9294
* @param analyticsProcessor instance of analytics processor
9395
* @param defaultFlagHandler handler for default flags if present
9496
*/
@@ -116,23 +118,26 @@ public static Flags fromApiFlags(
116118
/**
117119
* Build flags object from evaluation result.
118120
*
119-
* @param evaluationResult evaluation result
121+
* @param evaluationResult evaluation result
120122
* @param analyticsProcessor instance of analytics processor
121123
* @param defaultFlagHandler handler for default flags if present
122124
*/
123125
public static Flags fromEvaluationResult(
124126
EvaluationResult evaluationResult,
125127
AnalyticsProcessor analyticsProcessor,
126128
DefaultFlagHandler defaultFlagHandler) {
127-
Map<String, BaseFlag> flagMap = evaluationResult.getFlags().stream()
129+
Map<String, BaseFlag> flagMap = evaluationResult.getFlags().getAdditionalProperties()
130+
.entrySet()
131+
.stream()
128132
.collect(
129133
Collectors.toMap(
130-
(fs) -> fs.getName(),
131-
(fs) -> {
134+
Entry::getKey,
135+
entry -> {
136+
FlagResult flagResult = entry.getValue();
132137
Flag flag = new Flag();
133-
flag.setFeatureName(fs.getName());
134-
flag.setValue(fs.getValue());
135-
flag.setEnabled(fs.getEnabled());
138+
flag.setFeatureName(flagResult.getName());
139+
flag.setValue(flagResult.getValue());
140+
flag.setEnabled(flagResult.getEnabled());
136141
return flag;
137142
}));
138143

‎src/test/java/com/flagsmith/flagengine/EngineTest.java‎

Lines changed: 41 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,103 +1,58 @@
11
package com.flagsmith.flagengine;
22

3-
import com.fasterxml.jackson.core.type.TypeReference;
3+
import com.fasterxml.jackson.core.json.JsonReadFeature;
4+
import com.fasterxml.jackson.databind.json.JsonMapper;
45
import com.fasterxml.jackson.databind.JsonNode;
5-
import com.flagsmith.MapperFactory;
6-
import com.flagsmith.mappers.EngineMappers;
7-
import com.flagsmith.models.FeatureStateModel;
8-
import com.flagsmith.models.Flags;
9-
10-
import groovy.util.Eval;
11-
import com.fasterxml.jackson.databind.ObjectMapper;
12-
import org.junit.BeforeClass;
6+
import java.io.BufferedReader;
7+
import java.io.IOException;
8+
import java.nio.file.Files;
9+
import java.nio.file.Path;
10+
import java.nio.file.Paths;
11+
import java.util.stream.Stream;
12+
import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
1313
import org.junit.jupiter.params.ParameterizedTest;
1414
import org.junit.jupiter.params.provider.Arguments;
1515
import org.junit.jupiter.params.provider.MethodSource;
16-
import java.io.File;
17-
import java.util.ArrayList;
18-
import java.util.HashMap;
19-
import java.util.List;
20-
import java.util.Map;
21-
import java.util.Optional;
22-
import java.util.stream.Collectors;
23-
import java.util.stream.Stream;
24-
import java.util.stream.StreamSupport;
25-
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.assertj.core.api.Assertions.assertThat;
2617

2718
public class EngineTest {
28-
private static final String ENVIRONMENT_JSON_FILE_LOCATION =
29-
"src/test/java/com/flagsmith/flagengine/enginetestdata/" +
30-
"data/environment_n9fbf9h3v4fFgH3U3ngWhb.json";
31-
private static ObjectMapper objectMapper = MapperFactory.getMapper();
32-
33-
private static Stream<Arguments> engineTestData() {
34-
try {
35-
JsonNode engineTestData = objectMapper.readTree(new File(ENVIRONMENT_JSON_FILE_LOCATION));
36-
37-
JsonNode environmentDocument = engineTestData.get("environment");
38-
JsonNode identitiesAndResponses = engineTestData.get("identities_and_responses");
39-
40-
EvaluationContext baseEvaluationContext = EngineMappers.mapEnvironmentDocumentToContext(environmentDocument);
41-
42-
List<Arguments> returnValues = new ArrayList<>();
43-
44-
if (identitiesAndResponses.isArray()) {
45-
for (JsonNode identityAndResponse : identitiesAndResponses) {
46-
JsonNode identity = identityAndResponse.get("identity");
47-
Map<String, Object> traits = Optional.ofNullable(identity.get("identity_traits"))
48-
.filter(JsonNode::isArray)
49-
.map(node -> StreamSupport.stream(node.spliterator(), false)
50-
.filter(trait -> trait.hasNonNull("trait_key"))
51-
.collect(Collectors.toMap(
52-
trait -> trait.get("trait_key").asText(),
53-
trait -> objectMapper.convertValue(trait.get("trait_value"), Object.class))))
54-
.orElseGet(HashMap::new);
55-
56-
EvaluationContext evaluationContext = EngineMappers.mapContextAndIdentityDataToContext(
57-
baseEvaluationContext, identity.get("identifier").asText(), traits);
58-
59-
if (identity.hasNonNull("django_id")) {
60-
evaluationContext.getIdentity().setKey(identity.get("django_id").asText());
61-
}
62-
63-
JsonNode expectedResponse = identityAndResponse.get("response");
64-
65-
returnValues.add(Arguments.of(evaluationContext, expectedResponse));
66-
67-
}
68-
}
69-
70-
return returnValues.stream();
71-
72-
} catch (Exception e) {
73-
System.out.println("Exception in engineTestData: " + e.getMessage());
74-
e.printStackTrace();
19+
private static final Path testCasesPath = Paths
20+
.get("src/test/java/com/flagsmith/flagengine/enginetestdata/test_cases");
21+
private static JsonMapper mapper = JsonMapper.builder()
22+
.enable(JsonReadFeature.ALLOW_JAVA_COMMENTS.mappedFeature())
23+
.build();
24+
RecursiveComparisonConfiguration recursiveComparisonConfig = RecursiveComparisonConfiguration.builder()
25+
.build();
26+
27+
private static Arguments engineTestDataFromFile(Path path) {
28+
try (BufferedReader reader = Files.newBufferedReader(path)) {
29+
JsonNode root = mapper.readTree(reader);
30+
return Arguments.of(
31+
mapper.treeToValue(root.get("context"), EvaluationContext.class),
32+
mapper.treeToValue(root.get("result"), EvaluationResult.class));
33+
} catch (IOException e) {
34+
throw new RuntimeException("Failed to read test data from file: " + path, e);
7535
}
76-
return null;
36+
}
37+
38+
private static Stream<Arguments> engineTestData() throws IOException {
39+
return Files.walk(testCasesPath)
40+
.filter(Files::isRegularFile)
41+
.filter(p -> {
42+
String n = p.getFileName().toString();
43+
return n.endsWith(".json") || n.endsWith(".jsonc");
44+
})
45+
.map(EngineTest::engineTestDataFromFile);
7746
}
7847

7948
@ParameterizedTest()
8049
@MethodSource("engineTestData")
81-
public void testEngine(EvaluationContext evaluationContext, JsonNode expectedResponse) {
50+
public void testEngine(EvaluationContext evaluationContext, EvaluationResult expectedResult) {
8251
EvaluationResult evaluationResult = Engine.getEvaluationResult(evaluationContext);
8352

84-
List<FeatureStateModel> flags = objectMapper.convertValue(
85-
expectedResponse.get("flags"),
86-
new TypeReference<List<FeatureStateModel>>() {});
87-
88-
flags.sort((fsm1, fsm2) -> fsm1.getFeature().getName().compareTo(fsm2.getFeature().getName()));
89-
List<FlagResult> sortedResults = evaluationResult.getFlags().stream()
90-
.sorted((fr1, fr2) -> fr1.getName().compareTo(fr2.getName()))
91-
.collect(Collectors.toList());
92-
93-
assertEquals(flags.size(), sortedResults.size());
94-
for (int i = 0; i < flags.size(); i++) {
95-
FeatureStateModel fsm = flags.get(i);
96-
FlagResult fr = sortedResults.get(i);
97-
98-
assertEquals(fsm.getFeature().getName(), fr.getName());
99-
assertEquals(fsm.getEnabled(), fr.getEnabled());
100-
assertEquals(fsm.getValue(), fr.getValue());
101-
}
53+
assertThat(evaluationResult)
54+
.usingRecursiveComparison(recursiveComparisonConfig)
55+
.ignoringAllOverriddenEquals()
56+
.isEqualTo(expectedResult);
10257
}
10358
}

‎src/test/java/com/flagsmith/flagengine/enginetestdata‎

Submodule enginetestdata updated 147 files

0 commit comments

Comments
 (0)