Skip to content

Commit c4750b4

Browse files
khvn26khvn26
committed
fix engine and evaluator tests
1 parent 7be7e3d commit c4750b4

File tree

11 files changed

+87
-132
lines changed

11 files changed

+87
-132
lines changed

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@ public static EvaluationResult getEvaluationResult(EvaluationContext context) {
2121

2222
for (SegmentContext segmentContext : context.getSegments().getAdditionalProperties().values()) {
2323
if (SegmentEvaluator.isContextInSegment(context, segmentContext)) {
24-
SegmentResult segmentResult = new SegmentResult().withKey(segmentContext.getKey())
25-
.withName(segmentContext.getName());
26-
segments.add(segmentResult);
24+
segments.add(new SegmentResult().withKey(segmentContext.getKey())
25+
.withName(segmentContext.getName()));
2726

28-
if (segmentContext.getOverrides() != null) {
29-
for (FeatureContext featureContext : segmentContext.getOverrides()) {
27+
List<FeatureContext> segmentOverrides = segmentContext.getOverrides();
28+
29+
if (segmentOverrides != null) {
30+
for (FeatureContext featureContext : segmentOverrides) {
3031
String featureKey = featureContext.getFeatureKey();
3132

3233
if (segmentFeatureContexts.containsKey(featureKey)) {
@@ -41,7 +42,7 @@ public static EvaluationResult getEvaluationResult(EvaluationContext context) {
4142
? Double.POSITIVE_INFINITY
4243
: featureContext.getPriority();
4344

44-
if (existingPriority > featurePriority) {
45+
if (existingPriority < featurePriority) {
4546
continue;
4647
}
4748
}
@@ -66,7 +67,7 @@ public static EvaluationResult getEvaluationResult(EvaluationContext context) {
6667
.withFeatureKey(featureContext.getFeatureKey())
6768
.withName(featureContext.getName())
6869
.withValue(featureContext.getValue())
69-
.withReason("TARGETING MATCH; segment=" + segmentNameWithFeatureContext.getLeft()));
70+
.withReason("TARGETING_MATCH; segment=" + segmentNameWithFeatureContext.getLeft()));
7071
} else {
7172
flags.add(getFlagResultFromFeatureContext(featureContext, identityKey));
7273
}

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ private static Boolean contextMatchesCondition(
9191
List<?> maybeConditionList = (List<?>) conditionValue;
9292
conditionList = maybeConditionList.stream()
9393
.filter(String.class::isInstance)
94-
.map(String.class::cast)
94+
.map(Object::toString)
9595
.collect(Collectors.toList());
9696
} else if (conditionValue instanceof String) {
9797
String stringConditionValue = (String) conditionValue;
@@ -106,7 +106,11 @@ private static Boolean contextMatchesCondition(
106106
}
107107
}
108108

109-
return conditionList.contains(contextValue.toString());
109+
if (!(contextValue instanceof Boolean)) {
110+
contextValue = contextValue.toString();
111+
}
112+
113+
return conditionList.contains(contextValue);
110114

111115
case PERCENTAGE_SPLIT:
112116
String key = (contextValue != null) ? contextValue.toString()
@@ -175,7 +179,13 @@ private static Boolean contextMatchesCondition(
175179
if (contextValue == null) {
176180
return false;
177181
}
178-
return TypeCasting.compare(operator, contextValue, conditionValue);
182+
try {
183+
return TypeCasting.compare(operator, contextValue, conditionValue);
184+
} catch (Exception e) {
185+
throw new RuntimeException(
186+
"Error comparing values: "
187+
+ String.valueOf(contextValue) + " and " + String.valueOf(conditionValue));
188+
}
179189
}
180190
}
181191

‎src/main/java/com/flagsmith/flagengine/utils/types/TypeCasting.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ public class TypeCasting {
1111
* Compare the values value1 and value2 with the provided condition.
1212
*
1313
* @param condition SegmentCondition criteria to compare values against.
14-
* @param value1 Value to compare.
15-
* @param value2 Value to compare against.
14+
* @param value1 Value to compare.
15+
* @param value2 Value to compare against.
1616
*/
1717
public static Boolean compare(SegmentConditions condition, Object value1, Object value2) {
1818

@@ -39,8 +39,8 @@ public static Boolean compare(SegmentConditions condition, Object value1, Object
3939
* Run comparison with condition of primitive type.
4040
*
4141
* @param condition SegmentCondition criteria to compare values against.
42-
* @param value1 Value to compare.
43-
* @param value2 Value to compare against.
42+
* @param value1 Value to compare.
43+
* @param value2 Value to compare against.
4444
*/
4545
public static Boolean compare(SegmentConditions condition, Comparable value1, Comparable value2) {
4646
if (condition.equals(SegmentConditions.EQUAL)) {
@@ -163,7 +163,8 @@ public static Boolean isBoolean(Object str) {
163163
public static ComparableVersion toSemver(Object str) {
164164
try {
165165
String value = SemanticVersioning.isSemver((String) str)
166-
? SemanticVersioning.removeSemver((String) str) : ((String) str);
166+
? SemanticVersioning.removeSemver((String) str)
167+
: ((String) str);
167168
return new ComparableVersion(value);
168169
} catch (Exception nfe) {
169170
return null;
@@ -180,14 +181,17 @@ public static Boolean isSemver(Object str) {
180181
}
181182

182183
/**
183-
* Modulo is a special case as the condition value holds both the divisor and remainder.
184-
* This method compares the conditionValue and the traitValue by dividing the traitValue
184+
* Modulo is a special case as the condition value holds both the divisor and
185+
* remainder.
186+
* This method compares the conditionValue and the traitValue by dividing the
187+
* traitValue
185188
* by the divisor and verifying that it correctly equals the remainder.
186189
*
187190
* @param conditionValue conditionValue in the format 'divisor|remainder'
188-
* @param traitValue the value of the matched trait
189-
* @return true if expression evaluates to true, false if unable to evaluate expression or
190-
* it evaluates to false
191+
* @param traitValue the value of the matched trait
192+
* @return true if expression evaluates to true, false if unable to evaluate
193+
* expression or
194+
* it evaluates to false
191195
*/
192196
public static Boolean compareModulo(String conditionValue, Object traitValue) {
193197
try {

‎src/main/java/com/flagsmith/mappers/EngineMappers.java‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ public static EvaluationContext mapEnvironmentDocumentToContext(
105105
// Map identity overrides
106106
JsonNode identityOverrides = environmentDocument.get("identity_overrides");
107107
if (identityOverrides != null && identityOverrides.isArray()) {
108-
Map<String, SegmentContext> identityOverrideSegments =
109-
mapIdentityOverridesToSegments(identityOverrides);
108+
Map<String, SegmentContext> identityOverrideSegments = mapIdentityOverridesToSegments(
109+
identityOverrides);
110110
segments.putAll(identityOverrideSegments);
111111
}
112112

@@ -163,9 +163,7 @@ private static Map<String, SegmentContext> mapIdentityOverridesToSegments(
163163
.withFeatureKey(feature.get("id").asText())
164164
.withName(feature.get("name").asText())
165165
.withEnabled(featureState.get("enabled").asBoolean())
166-
.withValue(featureState.get("feature_state_value") != null
167-
? featureState.get("feature_state_value").asText()
168-
: null)
166+
.withValue(getFeatureStateValue(featureState, "feature_state_value"))
169167
.withPriority(Double.NEGATIVE_INFINITY); // Highest possible priority
170168
overridesKey.add(featureContext);
171169
}
@@ -234,7 +232,7 @@ private static List<SegmentRule> mapEnvironmentDocumentRulesToContextRules(
234232
SegmentCondition segmentCondition = new SegmentCondition()
235233
.withProperty(condition.get("property_").asText())
236234
.withOperator(SegmentConditions.valueOf(condition.get("operator").asText()))
237-
.withValue(condition.get("value"));
235+
.withValue(condition.get("value").asText());
238236
conditions.add(segmentCondition);
239237
}
240238
}
@@ -294,6 +292,24 @@ private static String getFeatureStateKey(JsonNode featureState) {
294292
return "";
295293
}
296294

295+
private static Object getFeatureStateValue(JsonNode featureState, String fieldName) {
296+
JsonNode valueNode = featureState.get(fieldName);
297+
if (valueNode.isTextual() || valueNode.isLong()) {
298+
return valueNode.asText();
299+
} else if (valueNode.isNumber()) {
300+
if (valueNode.isInt()) {
301+
return valueNode.asInt();
302+
} else {
303+
return valueNode.asDouble();
304+
}
305+
} else if (valueNode.isBoolean()) {
306+
return valueNode.asBoolean();
307+
} else if (valueNode.isArray() || valueNode.isObject()) {
308+
return valueNode;
309+
}
310+
return null;
311+
}
312+
297313
/**
298314
* Maps a single feature state to feature context.
299315
*
@@ -308,9 +324,7 @@ private static FeatureContext mapFeatureStateToFeatureContext(JsonNode featureSt
308324
.withFeatureKey(feature.get("id").asText())
309325
.withName(feature.get("name").asText())
310326
.withEnabled(featureState.get("enabled").asBoolean())
311-
.withValue(featureState.get("feature_state_value") != null
312-
? featureState.get("feature_state_value").asText()
313-
: null);
327+
.withValue(getFeatureStateValue(featureState, "feature_state_value"));
314328

315329
// Handle multivariate feature state values
316330
JsonNode multivariateValues = featureState.get("multivariate_feature_state_values");
@@ -321,7 +335,8 @@ private static FeatureContext mapFeatureStateToFeatureContext(JsonNode featureSt
321335
sortedMultivariate.sort((a, b) -> a.get("id").asText().compareTo(b.get("id").asText()));
322336
for (JsonNode multivariateValue : sortedMultivariate) {
323337
FeatureValue variant = new FeatureValue()
324-
.withValue(multivariateValue.get("multivariate_feature_option").get("value").asText())
338+
.withValue(getFeatureStateValue(
339+
multivariateValue.get("multivariate_feature_option"), "value"))
325340
.withWeight(multivariateValue.get("percentage_allocation").asDouble());
326341
variants.add(variant);
327342
}

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

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

33
import com.fasterxml.jackson.annotation.JsonProperty;
4-
import com.flagsmith.flagengine.utils.Hashing;
54
import com.flagsmith.utils.models.BaseModel;
6-
import java.util.Arrays;
75
import java.util.List;
86
import java.util.UUID;
9-
import java.util.stream.Collectors;
107
import lombok.Data;
118

129
@Data
@@ -63,69 +60,4 @@ public Float getSortValue() {
6360
private Object value;
6461
@JsonProperty("feature_segment")
6562
private FeatureSegmentModel featureSegment;
66-
67-
/**
68-
* Returns the value object.
69-
*
70-
* @param identityId Identity ID
71-
*/
72-
public Object getValue(Object identityId) {
73-
74-
if (identityId != null && multivariateFeatureStateValues != null
75-
&& multivariateFeatureStateValues.size() > 0) {
76-
return getMultiVariateValue(identityId);
77-
}
78-
79-
return value;
80-
}
81-
82-
/**
83-
* Determines the multi variate value.
84-
*
85-
* @param identityId Identity ID
86-
*/
87-
private Object getMultiVariateValue(Object identityId) {
88-
89-
List<String> objectIds = Arrays.asList(
90-
(djangoId != null && djangoId != 0 ? djangoId.toString() : featurestateUuid),
91-
identityId.toString());
92-
93-
Float percentageValue = Hashing.getInstance().getHashedPercentageForObjectIds(objectIds);
94-
Float startPercentage = 0f;
95-
96-
List<MultivariateFeatureStateValueModel> sortedMultiVariateFeatureStates =
97-
multivariateFeatureStateValues
98-
.stream()
99-
.sorted((smvfs1, smvfs2) -> smvfs1.getSortValue().compareTo(smvfs2.getSortValue()))
100-
.collect(Collectors.toList());
101-
102-
for (MultivariateFeatureStateValueModel multiVariate : sortedMultiVariateFeatureStates) {
103-
Float limit = multiVariate.getPercentageAllocation() + startPercentage;
104-
105-
if (startPercentage <= percentageValue && percentageValue < limit) {
106-
return multiVariate.getMultivariateFeatureOption().getValue();
107-
}
108-
109-
startPercentage = limit;
110-
}
111-
112-
return value;
113-
}
114-
115-
/**
116-
* Another FeatureStateModel is deemed to be higher priority if and only if
117-
* it has a FeatureSegment and either this.FeatureSegment is null or the
118-
* value of other.FeatureSegment.priority is lower than that of
119-
* this.FeatureSegment.priority.
120-
*
121-
* @param other the other FeatureStateModel to compare priority with
122-
* @return true if `this` is higher priority than `other`
123-
*/
124-
public boolean isHigherPriority(FeatureStateModel other) {
125-
if (this.featureSegment == null || other.featureSegment == null) {
126-
return this.featureSegment != null && other.featureSegment == null;
127-
}
128-
129-
return this.featureSegment.getPriority() < other.featureSegment.getPriority();
130-
}
13163
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@ public class Flag extends BaseFlag {
1212
* return flag from feature state model and identity id.
1313
*
1414
* @param featureState feature state model
15-
* @param identityId identity id
1615
*/
17-
public static Flag fromFeatureStateModel(FeatureStateModel featureState, Object identityId) {
16+
public static Flag fromFeatureStateModel(FeatureStateModel featureState) {
1817
Flag flag = new Flag();
1918

2019
flag.setFeatureId(featureState.getFeature().getId());
21-
flag.setValue(featureState.getValue(identityId));
20+
flag.setValue(featureState.getValue());
2221
flag.setFeatureName(featureState.getFeature().getName());
2322
flag.setEnabled(featureState.getEnabled());
2423

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

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,41 +28,26 @@ public class Flags {
2828
public static Flags fromFeatureStateModels(
2929
List<FeatureStateModel> featureStates,
3030
AnalyticsProcessor analyticsProcessor) {
31-
return fromFeatureStateModels(featureStates, analyticsProcessor, null, null);
31+
return fromFeatureStateModels(featureStates, analyticsProcessor, null);
3232
}
3333

3434
/**
3535
* Build flags object from list of feature states.
3636
*
3737
* @param featureStates list of feature states
3838
* @param analyticsProcessor instance of analytics processor
39-
* @param identityId identity ID (optional)
40-
*/
41-
public static Flags fromFeatureStateModels(
42-
List<FeatureStateModel> featureStates,
43-
AnalyticsProcessor analyticsProcessor,
44-
Object identityId) {
45-
return fromFeatureStateModels(featureStates, analyticsProcessor, identityId, null);
46-
}
47-
48-
/**
49-
* Build flags object from list of feature states.
50-
*
51-
* @param featureStates list of feature states
52-
* @param analyticsProcessor instance of analytics processor
53-
* @param identityId identity ID (optional)
5439
* @param defaultFlagHandler default flags (optional)
5540
*/
5641
public static Flags fromFeatureStateModels(
5742
List<FeatureStateModel> featureStates,
5843
AnalyticsProcessor analyticsProcessor,
59-
Object identityId, DefaultFlagHandler defaultFlagHandler) {
44+
DefaultFlagHandler defaultFlagHandler) {
6045

6146
Map<String, BaseFlag> flagMap = featureStates.stream()
6247
.collect(
6348
Collectors.toMap(
6449
(fs) -> fs.getFeature().getName(),
65-
(fs) -> Flag.fromFeatureStateModel(fs, identityId)));
50+
(fs) -> Flag.fromFeatureStateModel(fs)));
6651

6752
Flags flags = new Flags();
6853
flags.setFlags(flagMap);
@@ -117,7 +102,7 @@ public static Flags fromApiFlags(
117102
for (FeatureStateModel flag : apiFlags) {
118103
flagMap.put(
119104
flag.getFeature().getName(),
120-
Flag.fromFeatureStateModel(flag, null));
105+
Flag.fromFeatureStateModel(flag));
121106
}
122107

123108
Flags flags = new Flags();

‎src/test/java/com/flagsmith/FlagsmithTestHelper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public static BaseFlag flag(
239239
feature.setName(name);
240240
feature.setType(type);
241241

242-
return Flag.fromFeatureStateModel(result, null);
242+
return Flag.fromFeatureStateModel(result);
243243
}
244244

245245
public static BaseFlag flag(String name, String description, boolean enabled) {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ private static Stream<Arguments> engineTestData() {
5757
EvaluationContext evaluationContext = EngineMappers.mapContextAndIdentityDataToContext(
5858
baseEvaluationContext, identity.get("identifier").asText(), traits);
5959

60+
if (identity.hasNonNull("django_id")) {
61+
evaluationContext.getIdentity().setKey(identity.get("django_id").asText());
62+
}
63+
6064
JsonNode expectedResponse = identityAndResponse.get("response");
6165

6266
returnValues.add(Arguments.of(evaluationContext, expectedResponse));
@@ -67,7 +71,8 @@ private static Stream<Arguments> engineTestData() {
6771
return returnValues.stream();
6872

6973
} catch (Exception e) {
70-
System.out.println(e.getMessage());
74+
System.out.println("Exception in engineTestData: " + e.getMessage());
75+
e.printStackTrace();
7176
}
7277
return null;
7378
}

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

0 commit comments

Comments
 (0)