Skip to content

Commit 682d337

Browse files
authored
fix: multiple custom strategies calculation (#85)
1 parent c6c3529 commit 682d337

File tree

6 files changed

+80
-7
lines changed

6 files changed

+80
-7
lines changed

java-engine/src/test/java/io/getunleash/engine/CustomStrategiesEvaluatorTest.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static io.getunleash.engine.TestStrategies.alwaysFails;
44
import static io.getunleash.engine.TestStrategies.alwaysTrue;
5+
import static io.getunleash.engine.TestStrategies.onlyTrueIfParameterValueMatchesContext;
56
import static org.assertj.core.api.Assertions.assertThat;
67
import static org.junit.jupiter.api.Assertions.*;
78
import static org.junit.jupiter.params.provider.Arguments.of;
@@ -43,25 +44,25 @@ private static Stream<Arguments> twoStrategies() {
4344
of(
4445
alwaysTrue("custom"),
4546
alwaysFails("cus-tom"),
46-
Map.of("customStrategy1", false, "customStrategy2", true)),
47+
Map.of("customStrategy1", true, "customStrategy2", false)),
4748
of(
4849
alwaysFails("custom"),
4950
alwaysTrue("cus-tom"),
50-
Map.of("customStrategy1", true, "customStrategy2", false)),
51+
Map.of("customStrategy1", false, "customStrategy2", true)),
5152
of(
5253
alwaysTrue("wrongName"),
5354
alwaysTrue("wrongName"),
5455
Map.of("customStrategy1", false, "customStrategy2", false)),
5556
of(
5657
alwaysTrue("custom"),
5758
alwaysTrue("custom"),
58-
Map.of("customStrategy1", false, "customStrategy2", true)));
59+
Map.of("customStrategy1", true, "customStrategy2", false)));
5960
}
6061

6162
private static Stream<Arguments> singleStrategy() {
6263
return Stream.of(
63-
of("custom", Map.of("customStrategy1", false, "customStrategy2", true)),
64-
of("cus-tom", Map.of("customStrategy1", true, "customStrategy2", false)),
64+
of("custom", Map.of("customStrategy1", true, "customStrategy2", false)),
65+
of("cus-tom", Map.of("customStrategy1", false, "customStrategy2", true)),
6566
of("unknown", Map.of("customStrategy1", false, "customStrategy2", false)));
6667
}
6768

@@ -105,4 +106,16 @@ void faultyStrategy_shouldEvalToFalse() throws IOException, YggdrasilInvalidInpu
105106
Map.of("customStrategy1", false, "customStrategy2", false),
106107
unleashEngine.customStrategiesEvaluatorEval("Feature.Custom.Strategies", new Context()));
107108
}
109+
110+
@Test
111+
void repeated_strategy_with_different_parameters_should_evaluate_separately()
112+
throws IOException, YggdrasilInvalidInputException {
113+
UnleashEngine unleashEngine =
114+
new UnleashEngine(
115+
List.of(onlyTrueIfParameterValueMatchesContext("myFancyStrategy", "myFancy")));
116+
unleashEngine.takeState(ResourceReader.readResourceAsString("repeated_custom_strategy.json"));
117+
var context = new Context();
118+
context.setProperties(Map.of("myFancy", "one"));
119+
assertThat(unleashEngine.customStrategiesEvaluatorEval("repeated.custom", context)).hasSize(2);
120+
}
108121
}

java-engine/src/test/java/io/getunleash/engine/TestStrategies.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,19 @@ public boolean isEnabled(Map<String, String> parameters, Context context) {
5353
}
5454
};
5555
}
56+
57+
static IStrategy onlyTrueIfParameterValueMatchesContext(
58+
String strategyName, String parameterName) {
59+
return new IStrategy() {
60+
@Override
61+
public String getName() {
62+
return strategyName;
63+
}
64+
65+
@Override
66+
public boolean isEnabled(Map<String, String> parameters, Context context) {
67+
return context.getProperties().get(parameterName).equals(parameters.get(parameterName));
68+
}
69+
};
70+
}
5671
}

java-engine/src/test/resources/custom-strategy-tests.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,25 @@
8282
"variants": [],
8383
"description": "",
8484
"impressionData": false
85+
},
86+
{
87+
"name": "repeated.custom",
88+
"enabled": true,
89+
"strategies": [
90+
{
91+
"name": "myFancyStrategy",
92+
"parameters": {
93+
"myFancy": "one"
94+
}
95+
96+
},
97+
{
98+
"name": "myFancyStrategy",
99+
"parameters": {
100+
"myFancy": "two"
101+
}
102+
}
103+
]
85104
}
86105
]
87106
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"version": 2,
3+
"features": [
4+
{
5+
"name": "repeated.custom",
6+
"enabled": true,
7+
"strategies": [
8+
{
9+
"name": "myFancyStrategy",
10+
"parameters": {
11+
"myFancy": "one"
12+
}
13+
14+
},
15+
{
16+
"name": "myFancyStrategy",
17+
"parameters": {
18+
"myFancy": "two"
19+
}
20+
}
21+
]
22+
}
23+
]
24+
}

yggdrasilffi/src/flat/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pub unsafe fn flat_take_state(engine_pointer: *mut c_void, toggles_pointer: *con
110110
.collect();
111111
(strategy.name.clone(), params)
112112
})
113-
.collect::<BTreeMap<_, _>>();
113+
.collect::<Vec<_>>();
114114
(name, strategies)
115115
})
116116
.collect::<BTreeMap<_, _>>();

yggdrasilffi/src/flat/serialisation.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,12 @@ pub struct ResponseMessage<T> {
4040
pub impression_data: bool,
4141
}
4242

43+
pub type ParsedStrategies = BTreeMap<String, Vec<(String, BTreeMap<String, String>)>>;
44+
4345
pub struct TakeStateResult {
4446
pub warnings: Vec<EvalWarning>,
4547
pub error: Option<String>,
46-
pub feature_strategies_map: BTreeMap<String, BTreeMap<String, BTreeMap<String, String>>>,
48+
pub feature_strategies_map: ParsedStrategies,
4749
}
4850

4951
#[derive(Debug, Clone, Deserialize, Serialize)]

0 commit comments

Comments
 (0)